Repository navigation
fix: accept RPC responses up to 64 MiB and refuse a larger success with PROTOCOL_ERROR - #320
Conversation
…th PROTOCOL_ERROR
|
@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: f81d785ee1
ℹ️ 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".
# Conflicts: # src/host/README.md # src/host/rpcServer.ts # src/protocol/README.md # src/protocol/messages.ts
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, 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
sendRpcread at most 1,048,576 characters of a host response. A structured snapshot with cells takes about 56 characters per fully styled cell and about 14 per plain cell. Soagent-tty snapshot --include-cellsfailed on a live session at realistic sizes, with exit 9RPC_ERROR"RPC response exceeds maximum buffer size.". I measured one response line on live sessions:RPC_ERRORRPC_ERRORRPC_ERRORThe other results stay small: a text or structured snapshot without cells of 500x200 takes about 105,000 characters,
inspectabout 755 andscreenshotabout 600. The renderer keeps about 75,000 cells of scrollback, so--include-scrollbackstays near 95,000.What changed
src/protocol/messages.ts:MAX_RPC_RESPONSE_CHARS= 67,108,864 (64 MiB). That fits about 1.2 million fully styled cells (a 1,000x1,000 screen), 12 times the 500x200 case, and still bounds the memory of one response.src/host/rpcClient.ts:sendRpcreads a response of up toMAX_RPC_RESPONSE_CHARS. It searches only each new chunk for the newline, so reading a large response stays linear (Codex round 1).src/host/rpcServer.ts:MAX_RPC_REQUEST_CHARS).sendResponserefuses a success response aboveMAX_RPC_RESPONSE_CHARS. It sendsPROTOCOL_ERROR"RPC response takes N characters, over the 67108864-character limit." withdetails: { responseChars, maxResponseChars }.src/cli/commands/snapshot.ts: a live snapshot with cells waits 60,000 ms instead of the 5,000 ms default (Codex round 1). The host sends nothing until it has built every cell. A fully styled 500x200 snapshot took 1.5 to 1.8 s to its first byte at load average 100 (about 17 microseconds per cell), so a grid near the cap takes about 20 s.src/host/README.mdandsrc/protocol/README.md.I chose
PROTOCOL_ERRORbecause it keeps exit 9, the exit code of this failure today.makeCliErrormarks it not retryable, which is correct here: a retry gives the same size.RPC_ERRORwould be marked retryable.Behavior changes for CLI and JSON users
snapshot --include-cellsworks on a live session up to about 1.2 million styled cells. The envelope shape does not change.PROTOCOL_ERROR, a message that names the size and the limit, and both numbers indetails. Before, it exited 9 withRPC_ERROR"RPC response exceeds maximum buffer size." andretryable: true.Verification
test/e2e/large-snapshot.test.tswith the newstyled-fillfixture:snapshot --include-cells --jsonon a live 300x100 fully styled session returnsRPC_ERROR"RPC response exceeds maximum buffer size.".test/unit/host/rpcServer.test.ts, with a realRpcServerandsendRpc: a 2,000,000-character text snapshot and a 67,108,864-character one both fail with the same message.PROTOCOL_ERRORwithmaxResponseChars: 67108864.agent-tty snapshot --include-cells --jsonwithok: true. The response line takes 5,610,221 characters.test/unit/commands/snapshot.test.ts: the snapshot RPC with cells gets a 60,000 ms timeout. Before the fix, it was called without one.test/unit/host/rpcServer.test.tscounts the characters thatindexOf('\n')reads for a 4,000,000-character response. Before the fix: 149,214,484. After: under 8,000,000.vitest run test/unit: 1987 passed on 041476d. Before the Codex fixes: 1958.vitest run --maxWorkers=1 --retry=2onhost-renderer-rpcandscreenshot-out: 17 passed.tsc --noEmit,oxlint src test vitest.config.tsandoxfmt --checkpass.This PR was stacked on #319 (request id cap). #319 has landed, GitHub moved the base to
main, and merge commit 17e4ffe brings inmain. The diff againstmainholds only this PR's changes. Refs #315Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:xhigh