fix(server): a driven request body that was cut short says so - #781
Merged
divshekhar merged 3 commits intoSep 6, 2026
Merged
Conversation
Rule 12 says a transform that truncates data on a path an agent reads must
report that it did. `buildNetworkDetail` caps the request body it takes off
the network stack at 8,192 characters and reports nothing.
The in-page observer caps the same body and does report it: `projectBody`
returns `{ body, truncated }` and the NET_REQUEST carries
`requestBodyTruncated`. So the two capture routes disagreed about whether a
shortened body announces itself, and the driven route is the one whose body
OVERWRITES the other in the merge, deliberately, because when they disagree
the disagreement is the finding.
That makes it concrete. A request whose page body was complete, merged with a
wire body that was cut, produced an event that read as a whole body and was
not, and an agent asserting over that payload reads the missing tail as an
absence. The reverse leaves a stale `requestBodyTruncated: true` caveating a
body that is now whole.
The bound is right and is untouched. Only the silence:
- `projectWireBody` returns `{ body, truncated }`, the shape
`sanitizeWithReport` set as the reference.
- `buildNetworkDetail` emits `requestBodyTruncated: true`, the same field
name the in-page path already uses, omitted when the whole body fits so
the caveat means something when it appears.
- The merge carries the flag with the body that won, and deletes it when the
body that replaced a truncated one is whole.
`packages/server/src/input/network-detail.ts` is now registered in
`scripts/check-lossy-transforms.mjs` with every export classified, since the
guard catches an unclassified export in a registered module and this module
was never registered. `attachNetworkDetail` is classified `silent`: it drops a
response whose `headers()` rejects, which is the right call on the stdio path
where an unhandled rejection takes down the MCP server, but it is a real gap
by the registry's own definition and belongs written down rather than
unlisted.
Signed-off-by: Abhijeet Sharma <meetabhijeet05@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Coding rule 12: a transform that can drop or truncate data on a path an agent reads must report that it did, in a machine-readable way.
buildNetworkDetailcaps the request body it takes off the network stack at 8,192 characters and says nothing.The in-page observer caps the same body and does say so.
projectBodyreturns{ body, truncated }and the NET_REQUEST carriesrequestBodyTruncated: true. So the two capture routes disagreed about whether a shortened body announces itself, and the driven route is the one whose body overwrites the other inmergeNetworkDetail, by design, because when they disagree the disagreement is the finding.That makes the failure concrete rather than theoretical. A request whose in-page body was complete, merged with a wire body that was cut, produced an event that read as a whole body and was not. An agent asserting over that payload gets an absence of the thing that was cut off, which is the false-green shape this repo has already paid for three times. The reverse case leaves a stale
requestBodyTruncated: truecaveating a body that is now whole.The cap itself is right, and I have not touched it. Only the silence.
Why the guard did not catch it
scripts/check-lossy-transforms.mjsis honest about its own limit: it catches a new export in a registered module, not a new behaviour, andpackages/server/src/input/network-detail.tswas never registered. It is now, with every export classified, so the next person to add a transform here is made to answer the question.No issue filed; found while reading the driven capture path.
How it was verified
packages/server/src/input/network-detail.lossy-conformance.test.ts, written RED first, on a fixture guaranteed to exceed the bound:requestBodyTruncated: trueThree of the four fail on
main.network-detail.test.tspasses unchanged.Guard:
node scripts/check-lossy-transforms.mjsreports 40 exports across 7 read-path modules, and--self-teststill catches all 12 synthetic problems.One classification worth arguing with
I marked
attachNetworkDetailassilentrather than quietly leaving it out. It drops a response whoseheaders()rejects, which happens when the page or CDP session is closing, and the existing comment explains why: on the stdiostart()path an unhandled rejection takes down the MCP server. That reasoning is sound and I have not changed the behaviour, but by the registry's own definition it is a real gap, so it seemed better written down than unlisted. Happy to reclassify if you read it differently.Gates run
pnpm lint && pnpm typecheck && pnpm test:unitpnpm test:e2epnpm gate:installpnpm test:e2e:desktopThis adds a field to an observed event, so
test:e2eis the tier and I could not run it on this machine, so I have left the box unticked; CI rane2e,desktop-e2eandverifyon this branch and all passed.Two unit tests fail on my machine on a clean
mainas well: the@reticlehq/nextloader and the@reticlehq/vite-pluginCJS require probe, both 5s timeouts under parallel load on Windows, plusformatGeneratedSource's Prettier sync resolution.Checklist
any, no free strings, no non-null!console.logor internal tracking codesCHANGELOG.mdupdated under[Unreleased]Heads up on ordering: this touches
packages/server/src/input/network-detail.ts, the same file as the NET_DETAIL URL redaction PR I opened alongside it. They are independent changes to different functions and both apply cleanly tomainon their own, but whichever lands second will want a trivial rebase. Happy to do that, in either order, or to fold them into one PR if you would rather review them together.