fix(imessage): handle CLI child stdout/stderr stream errors by cxbAsDev · Pull Request #101401 · openclaw/openclaw

cxbAsDev · 2026-07-07T06:04:06Z

What Problem This Solves

runIMessageCliJson in both actions.runtime.ts and send.ts spawned an imsg child process and listened for data, child error, and close events, but it did not attach error listeners to the piped stdout and stderr streams. If either stream emitted an error (for example, a broken pipe or EPIPE while the child was terminating), the error became an unhandled stream error and could crash the gateway worker.

Why This Change Was Made

Node Readable streams can emit error independently of the child process error event. For transport code that runs inside the gateway process, unhandled stream errors are a reliability risk. Adding explicit error handlers on both child.stdout and child.stderr lets the promise reject cleanly and gives operators a concrete error message instead of an unhandled exception.

User Impact

iMessage CLI sends and actions now fail gracefully when the child's stdout or stderr stream errors.
The gateway no longer risks an unhandled stream exception from imsg child I/O.
Existing success paths and timeout/kill escalation behavior are unchanged.

Evidence

Added error event handlers to child.stdout and child.stderr in both extensions/imessage/src/actions.runtime.ts and extensions/imessage/src/send.ts.
Added regression tests in extensions/imessage/src/actions.runtime.test.ts and extensions/imessage/src/send.test.ts covering stdout and stderr stream errors.

Local test runs:

node scripts/run-vitest.mjs extensions/imessage/src/actions.runtime.test.ts --run  # 21 tests passed
node scripts/run-vitest.mjs extensions/imessage/src/send.test.ts --run            # 47 tests passed

clawsweeper · 2026-07-07T06:17:35Z

Codex review: needs real behavior proof before merge. Reviewed July 7, 2026, 2:21 AM ET / 06:21 UTC.

Summary
The PR adds stdout/stderr stream error listeners to the two iMessage one-shot CLI JSON helpers and covers them with regression tests.

PR surface: Source +40, Tests +117. Total +157 across 4 files.

Reproducibility: no. live high-confidence reproduction was established in this review. The source-level reproduction path is clear: emit an error on the mocked child stdout or stderr stream in current main and the one-shot helper has no listener for it.

Review metrics: none identified.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

[P1] Add redacted real behavior proof for the stream-error path, such as terminal output or logs from a real child-process failure or live imsg-adjacent reproduction.

Proof guidance:

[P1] Needs real behavior proof before merge: The PR body reports mocked unit tests only; add redacted terminal/live output, logs, or a real child-process stream-error transcript, then update the PR body to trigger a fresh ClawSweeper review.

Risk before merge

[P1] External PR proof is mock-only; maintainers still need redacted real behavior proof such as live terminal output, logs, or a real child-process stream-failure transcript before merge.

Maintainer options:

Decide the mitigation before merge
Keep the narrow iMessage plugin fix, add redacted real-behavior proof for the stream-error path, and land after exact-head CI and review remain clean.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

[P1] No automation repair is appropriate because the code change has no blocking finding; the next action is contributor or maintainer real-behavior proof before merge.

Security
Cleared: No concrete security or supply-chain concern found; the diff only adds iMessage stream error handling and adjacent tests, with no dependency, workflow, secret, or permission changes.

Review details

Best possible solution:

Keep the narrow iMessage plugin fix, add redacted real-behavior proof for the stream-error path, and land after exact-head CI and review remain clean.

Do we have a high-confidence way to reproduce the issue?

No live high-confidence reproduction was established in this review. The source-level reproduction path is clear: emit an error on the mocked child stdout or stderr stream in current main and the one-shot helper has no listener for it.

Is this the best way to solve the issue?

Yes, this appears to be the narrow owner-boundary fix: handle stdout/stderr stream errors in both iMessage one-shot CLI helpers, mirroring the sibling RPC client. The safer alternative is not a new abstraction here; the main missing piece is real behavior proof.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 3175d8e7423d.

Label changes

Label changes:

add P2: This is a focused reliability fix for an iMessage child-process error path with limited blast radius and no evidence of an active widespread outage.
add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body reports mocked unit tests only; add redacted terminal/live output, logs, or a real child-process stream-error transcript, then update the PR body to trigger a fresh ClawSweeper review.

Label justifications:

P2: This is a focused reliability fix for an iMessage child-process error path with limited blast radius and no evidence of an active widespread outage.
rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body reports mocked unit tests only; add redacted terminal/live output, logs, or a real child-process stream-error transcript, then update the PR body to trigger a fresh ClawSweeper review.

Evidence reviewed

PR surface:

Source +40, Tests +117. Total +157 across 4 files.

View PR surface stats

Area	Files	Added	Net
Source	2	40	+40
Tests	2	117	+117
Docs	0	0	0
Config	0	0	0
Generated	0	0	0
Other	0	0	0
Total	4	157	+157

What I checked:

Current actions helper lacks stream error handlers: Current main sets encodings, listens for stdout/stderr data, and listens for child process error/close, but there is no stdout or stderr error listener in this one-shot helper. (extensions/imessage/src/actions.runtime.ts:228, 3175d8e7423d)
Current send helper lacks stream error handlers: The attachment/send one-shot CLI helper has the same current-main shape: stdout/stderr data listeners plus child process error/close, without stream error listeners. (extensions/imessage/src/send.ts:629, 3175d8e7423d)
Sibling RPC client already owns the same invariant: The long-lived iMessage RPC client treats child/process stdio errors as terminal and attaches stdout/stderr error handlers, supporting the PR's direction for the remaining one-shot helpers. (extensions/imessage/src/client.ts:121, 3175d8e7423d)
PR diff covers both one-shot helpers: The PR adds stdout and stderr stream error handlers in both actions.runtime.ts and send.ts, and adds mocked regression tests for stdout and stderr stream errors in both adjacent test files. (extensions/imessage/src/actions.runtime.ts:243, b7d7030b3019)
No exact duplicate found: Targeted GitHub searches for the one-shot runIMessageCliJson stdout/stderr stream-error problem did not return another open canonical PR or issue.
Real behavior proof is mock-only: The PR body lists local node scripts/run-vitest.mjs runs and mocked EventEmitter stream-error tests, but no live imsg run, real child-process transcript, terminal output, logs, or artifact proving the after-fix behavior outside mocks. (b7d7030b3019)

Likely related people:

omarshahine: Authored the private-API support that introduced the actions runtime path and carried major recent iMessage send-transport work touching send.ts. (role: feature owner and recent area contributor; confidence: high; commits: e259751ec9c9, bfccbc3fee9a, 91f5a65df9f8; files: extensions/imessage/src/actions.runtime.ts, extensions/imessage/src/send.ts)
masatohoshino: Authored the merged RPC-client stdout/stderr stream-error fix in the same iMessage child-process failure family. (role: recent sibling-fix contributor; confidence: high; commits: 6522dbf66b27; files: extensions/imessage/src/client.ts)
steipete: Committed follow-up termination and real-stream test work for the sibling RPC-client stream-error fix and appears in recent actions.runtime.ts history. (role: recent area contributor and reviewer; confidence: medium; commits: 606456394768, 4ffcce0f5eab, 62fa5692cb86; files: extensions/imessage/src/client.ts, extensions/imessage/src/actions.runtime.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

steipete · 2026-07-07T07:19:56Z

Merged via squash.

Prepared head SHA: 906f58c1c21256b507b4b21d5f8a59d2dc9d2b3e
Landed commit: 08abf83a665970cf6523118685b4c8683f226e98

…#101401) * fix(imessage): handle CLI child stdout/stderr stream errors * fix(imessage): stop CLI children after stream failures --------- Co-authored-by: Peter Steinberger <[email protected]>

bindSignalCliOutput only subscribed to the 'data' event on signal-cli daemon stdout/stderr pipes. Unhandled 'error' events on those pipes crashed the agent runtime when the underlying fd broke (kill -9 on the daemon, pipe rupture, etc.). Mirror the imessage listenForIMessageCliStreamErrors pattern (PR openclaw#101401), but route the error through the daemon's error() callback instead of rejecting the spawn promise — the daemon is long-lived, and stream errors do not necessarily mean the process is dead. The existing child.on('error') and child.once('exit') handlers own process termination; stream errors just need to reach the log sink. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

fix(imessage): handle CLI child stdout/stderr stream errors

b7d7030

openclaw-barnacle Bot added channel: imessage Channel integration: imessage size: S labels Jul 7, 2026

steipete self-assigned this Jul 7, 2026

fix(imessage): stop CLI children after stream failures

906f58c

steipete merged commit 08abf83 into openclaw:main Jul 7, 2026
101 checks passed

github-actions Bot mentioned this pull request Jul 7, 2026

📡 Upstream Digest — 2026-07-07 07:31 UTC curtismercier/openclaw-mods#1228

Open

This was referenced Jul 7, 2026

fix(imessage): handle one-shot cli stream errors #101545

Closed

fix(session-memory): use shortenHomePath for safe home-path display in logs #99114

Closed

This was referenced Jul 9, 2026

fix(codex): handle stdout/stderr stream errors in CLI session exec resume #102385

Closed

fix(codex-supervisor): handle stdout/stderr stream errors in JSON-RPC client #102387

Closed

jiashu6 mentioned this pull request Jul 9, 2026

fix(signal): log signal-cli daemon stdout/stderr stream errors #102812

Closed

cxbAsDev deleted the fix/imessage-cli-stream-errors branch July 15, 2026 03:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(imessage): handle CLI child stdout/stderr stream errors#101401

fix(imessage): handle CLI child stdout/stderr stream errors#101401
steipete merged 2 commits into
openclaw:mainfrom
cxbAsDev:fix/imessage-cli-stream-errors

cxbAsDev commented Jul 7, 2026

Uh oh!

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading

Uh oh!

Uh oh!

steipete commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

cxbAsDev commented Jul 7, 2026

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Uh oh!

clawsweeper Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

steipete commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading