fix(codex): handle stdout/stderr stream errors in CLI session exec resume#102385
fix(codex): handle stdout/stderr stream errors in CLI session exec resume#102385lsr911 wants to merge 1 commit into
Conversation
…sume
The exec resume child process pipes stdout/stderr for data collection
but does not register error handlers on the stdio streams. If a
stream errors (e.g. pipe broken), the Promise never settles because
only process-level errors are caught.
Add child.stdout.on('error', reject) and child.stderr.on('error',
reject) inside the Promise constructor alongside the existing
child.on('error', reject) to catch stream-level errors.
This follows the same pattern as openclaw#101505 (mushuiyu886), openclaw#101597
(Alix-007), openclaw#101401 (cxbAsDev), and openclaw#101088 (masatohoshino).
Co-Authored-By: Claude <[email protected]>
openclaw-barnacle
Bot
added
extensions: codex
size: XS
triage: needs-pr-context
|
Thanks for the context here. I swept through the related work, and this is now duplicate or superseded. This PR should close because the same Codex CLI resume stream-error problem is already covered by the open, stronger canonical PR at #102127, which preserves child-process lifecycle and includes real-spawn regression proof while this branch only rejects immediately on stream error. 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 branch and focus review on #102127 as the canonical implementation for this Codex CLI resume stream-error fix. So I’m closing this here and keeping the remaining discussion on #102127. Review detailsBest possible solution: Close this branch and focus review on #102127 as the canonical implementation for this Codex CLI resume stream-error fix. Do we have a high-confidence way to reproduce the issue? Yes. Current main has piped stdout/stderr data listeners in Is this the best way to solve the issue? No. This branch is only a partial mitigation because it rejects immediately; the better fix is the canonical replacement that terminates the child, waits for exit, and proves the behavior with tests. Security review: Security review cleared: No dependency, workflow, secret, package-resolution, or command-source security regression was found in this small child-process error-handling diff. AGENTS.md: found and applied where relevant. What I checked:
Likely related people:
Codex review notes: model internal, reasoning high; reviewed against 1017ea5bc804. |
clawsweeper
Bot
added
rating: 🧂 unranked krab
|
Superseded by wangmiao0668000666's #102127 which provides a more complete fix with real-spawn regression proof and proper child-process lifecycle preservation. |
1 participant
What Problem This Solves
The Codex CLI session exec resume handler spawns a child process and pipes
stdout/stderrfor data collection, but does not register error handlers on the stdio streams. If a stream errors (e.g. pipe broken, EPIPE), the Promise never settles because only process-level errors (child.on("error")) are caught.Unhandled stream errors can cause uncaught exceptions or leave the Promise hanging indefinitely.
Why This Change Was Made
Adds
child.stdout.on("error", reject)andchild.stderr.on("error", reject)inside the Promise constructor alongside the existingchild.on("error", reject)to catch stream-level errors.This follows the same established pattern used by:
fix(codex): handle app-server stdio stream errorsfix(matrix): handle stdout/stderr stream errors in dependency commandsfix(imessage): handle CLI child stdout/stderr stream errorsfix(discord): handle ffmpeg stderr stream errors in voice playbackFiles Changed
extensions/codex/src/node-cli-sessions.tschild.stdout.on("error", reject)+child.stderr.on("error", reject)🤖 Generated with Claude Code
Co-Authored-By: Claude [email protected]