fix(server): frame Codex app-server JSONL on LF only#2162
Conversation
node:readline treats literal U+2028/U+2029 as line boundaries on Node 24 / Electron, which splits valid Codex JSON-RPC records and times out session resume. Frame stdout on LF only, preserve those characters as JSON string data, keep CRLF compatibility, and add regression coverage.
|
| Filename | Overview |
|---|---|
| packages/server/src/server/agent/providers/codex/app-server-transport.ts | Replaces readline with a manual LF-only buffer; correctly fixes the U+2028/U+2029 framing bug, but chunk.toString() does not use StringDecoder, so multi-byte characters split across OS pipe reads can still be corrupted. |
| packages/server/src/server/agent/providers/codex/app-server-transport.test.ts | Adds three deterministic regression tests for U+2028/U+2029 passthrough, CRLF termination, and chunked reassembly; splits are at character boundaries so byte-boundary corruption is not exercised. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant CP as Codex Child Process
participant ST as child.stdout (stream)
participant BUF as stdoutBuffer
participant HL as handleLine()
participant NH as notificationHandler / pending
CP->>ST: write(chunk with U+2028/U+2029)
ST-->>BUF: data event → handleStdoutChunk(chunk.toString())
BUF->>BUF: append chunk, scan for LF
BUF->>BUF: slice line, strip trailing CR
BUF->>HL: handleLine(line)
HL->>HL: JSON.parse(line)
alt notification
HL->>NH: notificationHandler(method, params)
else response
HL->>NH: pending.resolve(result)
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant CP as Codex Child Process
participant ST as child.stdout (stream)
participant BUF as stdoutBuffer
participant HL as handleLine()
participant NH as notificationHandler / pending
CP->>ST: write(chunk with U+2028/U+2029)
ST-->>BUF: data event → handleStdoutChunk(chunk.toString())
BUF->>BUF: append chunk, scan for LF
BUF->>BUF: slice line, strip trailing CR
BUF->>HL: handleLine(line)
HL->>HL: JSON.parse(line)
alt notification
HL->>NH: notificationHandler(method, params)
else response
HL->>NH: pending.resolve(result)
end
Reviews (6): Last reviewed commit: "Merge branch 'main' into fix/2021-codex-..." | Re-trigger Greptile
![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: 09990a8283
ℹ️ 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".
| // JSONL framing must use LF only. node:readline also treats U+2028/U+2029 as | ||
| // line boundaries (Node 24+ / Electron), which splits valid JSON strings. | ||
| child.stdout.on("data", (chunk) => { | ||
| this.handleStdoutChunk(chunk.toString()); |
There was a problem hiding this comment.
When a child-process pipe splits a multibyte UTF-8 character between data events, converting each Buffer independently with chunk.toString() replaces the partial bytes with U+FFFD before handleStdoutChunk can reassemble the JSONL record. This silently corrupts Unicode in Codex responses and notifications (especially large records); the added chunk test does not cover this because it splits a JavaScript string rather than encoded bytes. Use a stateful UTF-8 decoder, such as StringDecoder or stream setEncoding("utf8"), before LF framing.
Useful? React with 👍 / 👎.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| ); | ||
|
|
||
| // Stream consumers process the write after the current turn. | ||
| await new Promise<void>((resolve) => setImmediate(resolve)); |
There was a problem hiding this comment.
The three new tests each place await new Promise<void>((resolve) => setImmediate(resolve)) directly in the test body — the same pattern the team's test-shape rules flag as "no inline promise/event waiting; mechanics live in helpers." The setImmediate yield is also unnecessary for the notification path because handleLine reaches notificationHandler?.(...) before its first await, so it runs synchronously inside the "data" event handler. Consider extracting a small helper such as waitForNotification(client) that returns the next notification and encapsulates any needed yield, the same way existing tests expose codex.nextResponse(). The pattern appears in all three new tests.
Rule Used: # Code Review Pattern Reference: Slop, Tests, Feat... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
Problem
Codex app-server JSONL was framed with
node:readline, which treats literalU+2028/U+2029 characters inside JSON strings as line boundaries. This split
valid JSON-RPC messages and caused session resume requests to time out.
Solution
node:readlineframing with an LF-only stdout bufferTesting
npx vitest run packages/server/src/server/agent/providers/codex/app-server-transport.test.ts --bail=1npm run lint -- <changed files>npm run typecheck --workspace=@getpaseo/serverCloses #2021