Repository navigation
fix: report PROTOCOL_ERROR when an older host sends a response over the cap - #322
Conversation
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 031b2936fd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
ThomasK33
changed the title
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Why
The advisor on #320 asked for three last changes:
sendRpchas its own over-cap check, and only an older host (without fix: accept RPC responses up to 64 MiB and refuse a larger success with PROTOCOL_ERROR #320's server check) can reach it. That check still reportedRPC_ERROR"RPC response exceeds maximum buffer size." withretryable: true. A retry gets the same size, so it must not say retryable.MAX_RPC_RESPONSE_CHARScomment must state how much memory the CLI needs for a response near the cap. If the peak goes over about 1.5 GB, the cap must come down.docs/USAGE.mdmust say what a response over the cap gives.What changed
src/host/rpcClient.ts: the over-cap path reportsPROTOCOL_ERROR"RPC response is over the 67108864-character limit.", withmethod,socketPathandmaxResponseCharsindetails.makeCliErrormarksPROTOCOL_ERRORas not retryable. The exit code stays 9.src/protocol/messages.ts: theMAX_RPC_RESPONSE_CHARScomment gives the measured CLI memory. The cap stays at 67,108,864.docs/USAGE.md(Exit Codes) andsrc/host/README.mddescribe the failure.Memory measurement
I used live sessions on #320's head 17e4ffe and
process.resourceUsage().maxRSSin the CLI process, from a preload hook.--include-cells--include-cells(999,999 styled cells)So the CLI needs about 14 bytes per response character above its baseline. A response at the cap takes about 1.1 GiB, which is under 1.5 GB, so the cap stays. The CLI wrote 134 MB of JSON for the 1,000x1,000 snapshot in about 7 s.
The session host's peak RSS for the same snapshot went from 340 MiB to 1.75 GiB (
VmHWM). The cap does not change that number. The host builds and serializes the whole result before its size check, as it did before #320, when the CLI dropped the response.Behavior changes for CLI and JSON users
A CLI talking to a host from before #320 now reports a response over the cap as exit 9
PROTOCOL_ERRORwithretryable: falseanddetails.maxResponseChars. Before, it reported exit 9RPC_ERRORwithretryable: true.Verification
test/unit/host/rpcClient.test.ts, a raw socket server stands in for an older host and sends 65 MiB without a newline.sendRpcgaveRPC_ERRORwithretryable: true.vitest run test/unit: 1989 passed.tsc --noEmit,oxlint src test vitest.config.tsandoxfmt --checkpass.Refs #320
Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:xhigh