fix(codex-supervisor): handle stdout/stderr stream errors in JSON-RPC client#102387
fix(codex-supervisor): handle stdout/stderr stream errors in JSON-RPC client#102387lsr911 wants to merge 1 commit into
Conversation
… client The stdio-proxy JSON-RPC connection registers data handlers on stdout and stderr streams, plus error handlers on stdin and the process itself, but does not register error handlers on stdout or stderr. If either stream errors (e.g. pipe broken), the error is unhandled and the connection may hang. Add error handlers on proc.stdout and proc.stderr that call this.fail(error), matching the existing stdin and process-level error handling. Co-Authored-By: Claude <[email protected]>
openclaw-barnacle
Bot
added
extensions: codex-supervisor
|
Thanks for the context here. I swept through the related work, and this is now duplicate or superseded. This PR should close because #101599 is the older, open, proof-positive canonical fix for the same Codex Supervisor stdio stream-error gap, and this branch also uses the unsafe stderr-as-fatal behavior that the canonical PR corrected. Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Canonical path: Close this duplicate branch and focus review on #101599, which preserves stdout as the fatal JSON-RPC channel and keeps stderr diagnostic-only with proof. So I’m closing this here and keeping the remaining discussion on #101599. Review detailsBest possible solution: Close this duplicate branch and focus review on #101599, which preserves stdout as the fatal JSON-RPC channel and keeps stderr diagnostic-only with proof. Do we have a high-confidence way to reproduce the issue? Yes. Current main has stdout/stderr data listeners in the Codex Supervisor stdio transport without readable stream error listeners, and the same path is exercised by the canonical replacement PR's real spawned Node stdio tests. Is this the best way to solve the issue? No. This PR is not the best fix because it makes stderr stream errors terminal; the better solution is the canonical replacement that matches the upstream Codex stdout JSON-RPC and stderr diagnostics contract. Security review: Security review cleared: No concrete security or supply-chain regression was found; the diff only changes child stdio stream error handling in an existing bundled plugin. AGENTS.md: found and applied where relevant. What I checked:
Likely related people:
Codex review notes: model internal, reasoning high; reviewed against e90bf3182927. |
clawsweeper
Bot
added
rating: 🧂 unranked krab
|
Superseded by #101599 which is the older, proof-positive canonical fix for the same Codex Supervisor stdio stream-error gap. |
1 participant
What Problem This Solves
The
StdioCodexJsonRpcConnectionregistersdatahandlers onproc.stdoutandproc.stderr, plus error handlers onproc.stdinand the process itself (proc.once("error")), but does not register error handlers on the stdout or stderr streams. If either stream errors (e.g. pipe broken, EPIPE), the error is unhandled and the JSON-RPC connection may hang silently.Why This Change Was Made
Adds
this.proc.stdout.on("error", ...)andthis.proc.stderr.on("error", ...)that callthis.fail(error), matching the existing stdin and process-level error handling.Same stream error handler pattern as #101505, #101597, #101401, #101088, #102385, and #102386.
Files Changed
extensions/codex-supervisor/src/json-rpc-client.ts🤖 Generated with Claude Code
Co-Authored-By: Claude [email protected]