fix(agents): handle stdout/stderr stream errors in ssh sandbox commands by cxbAsDev · Pull Request #101031 · openclaw/openclaw

cxbAsDev · 2026-07-06T17:02:25Z

What Problem This Solves

runSshSandboxCommand and uploadDirectoryToSshTarget spawn ssh/tar children and read stdout/stderr. If any of those streams emits an error event, it is not handled and can become an unhandled exception. In uploadDirectoryToSshTarget the fail helper was also referenced before its declaration, causing a temporal dead zone error when registering the stream error listeners.

Why This Change Was Made

Add explicit stream error listeners to stdout/stderr/stdin on the ssh command child, killing the child before rejecting.
Move the fail helper in uploadDirectoryToSshTarget before its first use so the tar-over-ssh pipeline can register its stream error listeners correctly.

User Impact

SSH sandbox command and upload operations are more resilient to transient stream failures and no longer fail with a TDZ error on upload.

Evidence

Updated regression tests in src/agents/sandbox/ssh.stream-errors.test.ts to assert that the ssh child is killed on stdout/stderr/stdin stream errors.
Added real behavior proof at scripts/proof/ssh-sandbox-stream-errors.mts covering all three stream errors.
Local run:
- node scripts/run-vitest.mjs src/agents/sandbox/ssh.test.ts src/agents/sandbox/ssh.stream-errors.test.ts passed
- OPENCLAW_OXLINT_SKIP_PREPARE=1 node scripts/run-oxlint.mjs src/agents/sandbox/ssh.ts src/agents/sandbox/ssh.stream-errors.test.ts scripts/proof/ssh-sandbox-stream-errors.mts passed
- node --import tsx scripts/proof/ssh-sandbox-stream-errors.mts:

=== Proof: ssh sandbox stream error catch ===

PASS [stdout]: rejected with "stdout read failed" and killed the child.
PASS [stderr]: rejected with "stderr read failed" and killed the child.
PASS [stdin]: rejected with "stdin write failed" and killed the child.

cxbAsDev · 2026-07-06T17:11:28Z

@clawsweeper re-review

clawsweeper · 2026-07-06T17:11:31Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

clawsweeper · 2026-07-06T17:50:46Z

Codex review: needs maintainer review before merge. Reviewed July 6, 2026, 10:13 PM ET / 02:13 UTC.

Summary
The branch adds stream-error handlers to SSH sandbox command/upload child streams and adds focused regression tests plus a terminal proof script.

PR surface: Source +17, Tests +137, Other +108. Total +262 across 3 files.

Reproducibility: yes. Current main lacks stream error listeners on the SSH command/upload streams, and a direct Node EventEmitter probe confirms an emitted error without a listener throws.

Review metrics: none identified.

Root-cause cluster
Relationship: canonical
Canonical: #101031
Summary: This PR is the canonical open fix path for the SSH sandbox stdio stream-error issue; the older overlapping PR was closed as superseded by maintainer comment.

Members:

superseded: fix(agents): suppress unhandled stdout/stderr stream errors in sandbox SSH commands #100746 - A maintainer closed the older PR as superseded because this PR preserves the stream failure signal and covers the same command/upload surfaces.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

Update the branch or let the merge queue verify the current-base merge result because live GitHub reports BEHIND.

Risk before merge

[P1] Live GitHub reports the PR is mergeable but behind base, so final landing should update or verify the merge result against current main.

Maintainer options:

Decide the mitigation before merge
Land the focused SSH helper fix once maintainer review and current-base merge verification are satisfied, keeping stream-error ownership in src/agents/sandbox/ssh.ts.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

No automated repair job is needed because no concrete patch defect was found; this remains normal maintainer review and merge gating.

Security
Cleared: The diff changes child stream error handling and adds test/proof files, with no dependency, workflow, permission, secret, package-resolution, or SSH command-construction change.

Review details

Best possible solution:

Land the focused SSH helper fix once maintainer review and current-base merge verification are satisfied, keeping stream-error ownership in src/agents/sandbox/ssh.ts.

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

Yes. Current main lacks stream error listeners on the SSH command/upload streams, and a direct Node EventEmitter probe confirms an emitted error without a listener throws.

Is this the best way to solve the issue?

Yes. Handling stream failures in src/agents/sandbox/ssh.ts is the narrow owner-boundary fix because that module owns the spawned SSH/tar streams and sibling process wrappers already treat stdio failures as command failures.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

P2: This is a bounded SSH sandbox reliability fix with limited blast radius and no blocking patch defect found.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes copied terminal output from the after-fix proof script showing stdout, stderr, and stdin stream-error cases reject cleanly and kill the child.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied terminal output from the after-fix proof script showing stdout, stderr, and stdin stream-error cases reject cleanly and kill the child.

Evidence reviewed

PR surface:

Source +17, Tests +137, Other +108. Total +262 across 3 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	21	4	+17
Tests	1	137	0	+137
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	1	108	0	+108
Total	3	266	4	+262

What I checked:

Repository policy applied: Read root AGENTS.md plus scoped src/agents/AGENTS.md and scripts/AGENTS.md; their whole-path review, agents-test, and scripts-wrapper guidance shaped the review. (AGENTS.md:1, b1da734a1e7d)
Current main stream-error gap: Current main attaches data listeners to SSH stdout/stderr and pipes tar stdout into SSH stdin, but has no stream error listeners on those streams. (src/agents/sandbox/ssh.ts:685, b1da734a1e7d)
Node error-event contract checked: A direct Node EventEmitter probe confirmed that emitting an error without a listener throws, matching the reported failure mode for unhandled child streams.
PR head handles SSH command streams: The PR kills the SSH child and rejects when runSshSandboxCommand stdout, stderr, or stdin emits an error. (src/agents/sandbox/ssh.ts:685, 1d5fe138c52f)
PR head handles upload streams and TDZ path: The fail helper is declared before listener registration, and tar.stderr, tar.stdout, ssh.stdout, ssh.stderr, and ssh.stdin stream errors route through it before piping. (src/agents/sandbox/ssh.ts:806, 1d5fe138c52f)
Regression and proof inspected: The added tests cover stdout, stderr, and stdin stream-error rejection plus child kill, and the proof script runs the production helper against a real child process with all three stream-error cases. (src/agents/sandbox/ssh.stream-errors.test.ts:69, 1d5fe138c52f)

Likely related people:

steipete: History shows Peter Steinberger introduced the core SSH sandbox module, current line-level blame points to a recent broad restore, and steipete is the live assignee who closed the older overlapping PR as superseded by this one. (role: introduced behavior and likely decision owner; confidence: high; commits: b8bb8510a2a3, b978dcc63bcf; files: src/agents/sandbox/ssh.ts, src/agents/sandbox/ssh.test.ts, src/agents/sandbox/ssh-backend.ts)
vincentkoc: Vincent Koc authored SSH upload symlink hardening in the same helper and adjacent tests, making this history relevant to upload-pipeline behavior. (role: recent SSH sandbox upload contributor; confidence: high; commits: 3d5af14984ac; files: src/agents/sandbox/ssh.ts, src/agents/sandbox/ssh.test.ts, src/agents/sandbox/ssh.spawn-env.test.ts)
jacobtomlinson: Jacob Tomlinson changed the same SSH helper area for subprocess environment sanitization and added adjacent spawn-env coverage. (role: recent SSH subprocess contributor; confidence: high; commits: cfe14459531e; files: src/agents/sandbox/ssh.ts, src/agents/sandbox/ssh.spawn-env.test.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.

Review history (4 earlier review cycles)

reviewed 2026-07-06T18:10:11.967Z sha 604978a :: needs real behavior proof before merge. :: [P1] Move the upload failure handler before using it
reviewed 2026-07-06T19:39:55.579Z sha 5ad0d2f :: needs maintainer review before merge. :: none
reviewed 2026-07-06T20:45:54.552Z sha 5ad0d2f :: needs maintainer review before merge. :: none
reviewed 2026-07-07T00:49:11.100Z sha 51b0d2f :: needs maintainer review before merge. :: none

cxbAsDev · 2026-07-06T18:26:12Z

@clawsweeper re-review

修复了 ssh sandbox 的两个问题：

runSshSandboxCommand 的 stdout/stderr/stdin stream error handler 现在会先 kill 子进程再 reject，测试已断言 kill 被调用。
uploadDirectoryToSshTarget 中的 fail helper 被提前到 listener 注册之前，修复了 temporal dead zone 导致的 ReferenceError（ssh.test.ts 中相关测试现已通过）。

真实 proof 脚本已加入并运行通过，输出在 PR body 中。本地验证：

node scripts/run-vitest.mjs src/agents/sandbox/ssh.test.ts src/agents/sandbox/ssh.stream-errors.test.ts passed
lint passed
node --import tsx scripts/proof/ssh-sandbox-stream-errors.mts → PASS

clawsweeper · 2026-07-06T18:26:15Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

cxbAsDev · 2026-07-06T19:02:15Z

@clawsweeper re-review\n\nBranch rebased onto latest upstream/main. Test and proof both pass locally; proof output is in the PR body.

cxbAsDev · 2026-07-06T19:30:34Z

Closing temporarily to make room under the 20-active-PR limit for higher-priority fixes. Will reopen once other PRs land or are closed.

cxbAsDev · 2026-07-06T19:33:25Z

Reopening; closing newer QA-lab PRs instead to respect the 20-active-PR limit.

…andbox

steipete · 2026-07-07T03:20:15Z

Land-ready maintainer fixup complete at fe7ca02918c34c265acc1a01b87915b1ffd41a4e.

Replaced repeated direct rejection with once-only settlement for the SSH command child and tar-to-SSH upload pipeline.
Broken upload streams terminate both peer processes; later close/error races remain harmless.
Regression covers command stdout/stderr/stdin and all five upload stream boundaries (8 cases total).
Removed the 108-line proof-only script; final PR contains only src/agents/sandbox/ssh.ts and its focused regression test.
Sanitized AWS Crabbox proof: provider aws, lease cbx_8e04b07c49a8, run run_11f7ea5f47e6, 3 focused files / 39 tests passed at pre-rebase b8a5b45b85e478e15f6d341a17dc8df960daa38f; git range-diff confirms the runtime/test patch is identical at the final head.
Exact-head hosted CI: run 28838387201, 71 relevant checks succeeded, none pending/failing.
Fresh final-head Codex autoreview: clean, no accepted/actionable findings.
oxfmt and git diff --check: clean.

Known proof gaps: none.

steipete · 2026-07-07T03:20:26Z

Merged via squash.

Prepared head SHA: fe7ca02918c34c265acc1a01b87915b1ffd41a4e
Landed commit: d90c9a1ede8e9b098da0fd6ef17fb3432e82bfdf

…ds (openclaw#101031) * fix(agents): handle stdout/stderr stream errors in ssh sandbox commands * fix(agents): also handle stdin stream errors in ssh sandbox command * fix(agents): handle tar/ssh pipeline stream errors in sandbox upload * fix(agents): kill ssh child on stdout/stderr/stdin stream errors in sandbox * fix(agents): move upload fail helper before use to avoid TDZ in ssh sandbox * docs(proof): note ssh-sandbox upload TDZ fix in proof header * fix(agents): harden SSH sandbox stream errors --------- Co-authored-by: Peter Steinberger <[email protected]>

cxbAsDev requested a review from a team as a code owner July 6, 2026 17:02

openclaw-barnacle Bot added scripts Repository scripts agents Agent runtime and tooling size: S labels Jul 6, 2026

cxbAsDev force-pushed the fix/ssh-sandbox-stream-errors branch from dbfc1f2 to dc54abc Compare July 6, 2026 17:38

cxbAsDev force-pushed the fix/ssh-sandbox-stream-errors branch from dc54abc to 604978a Compare July 6, 2026 17:59

steipete mentioned this pull request Jul 6, 2026

fix(agents): suppress unhandled stdout/stderr stream errors in sandbox SSH commands #100746

Closed

openclaw-barnacle Bot added size: M and removed size: S labels Jul 6, 2026

cxbAsDev force-pushed the fix/ssh-sandbox-stream-errors branch from da7ecfd to 5ad0d2f Compare July 6, 2026 19:02

cxbAsDev closed this Jul 6, 2026

cxbAsDev reopened this Jul 6, 2026

cxbAsDev force-pushed the fix/ssh-sandbox-stream-errors branch from 5ad0d2f to 51b0d2f Compare July 7, 2026 00:36

steipete self-assigned this Jul 7, 2026

cxbAsDev force-pushed the fix/ssh-sandbox-stream-errors branch from 51b0d2f to 1d5fe13 Compare July 7, 2026 01:52

openclaw-barnacle Bot removed the scripts Repository scripts label Jul 7, 2026

cxbAsDev and others added 7 commits July 6, 2026 23:01

fix(agents): handle stdout/stderr stream errors in ssh sandbox commands

5a7fcd3

fix(agents): also handle stdin stream errors in ssh sandbox command

278ad7e

fix(agents): handle tar/ssh pipeline stream errors in sandbox upload

02e25af

fix(agents): kill ssh child on stdout/stderr/stdin stream errors in s…

2b04644

…andbox

fix(agents): move upload fail helper before use to avoid TDZ in ssh s…

9938cf8

…andbox

docs(proof): note ssh-sandbox upload TDZ fix in proof header

ba5e0b0

fix(agents): harden SSH sandbox stream errors

fe7ca02

steipete force-pushed the fix/ssh-sandbox-stream-errors branch from b8a5b45 to fe7ca02 Compare July 7, 2026 03:01

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

This was referenced Jul 7, 2026

fix(agent-core): handle stdout/stderr stream errors in harness exec #101370

Merged

fix(memory-host-sdk): handle stdout/stderr stream errors in runCliCommand #101402

Merged

sunlit-deng mentioned this pull request Jul 8, 2026

fix(agents): handle stdout/stderr stream errors in bash tool #101983

Closed

cxbAsDev deleted the fix/ssh-sandbox-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(agents): handle stdout/stderr stream errors in ssh sandbox commands#101031

fix(agents): handle stdout/stderr stream errors in ssh sandbox commands#101031
steipete merged 7 commits into
openclaw:mainfrom
cxbAsDev:fix/ssh-sandbox-stream-errors

cxbAsDev commented Jul 6, 2026 •

edited

Loading

Uh oh!

cxbAsDev commented Jul 6, 2026

Uh oh!

clawsweeper Bot commented Jul 6, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jul 6, 2026 •

edited

Loading

Uh oh!

cxbAsDev commented Jul 6, 2026

Uh oh!

clawsweeper Bot commented Jul 6, 2026 •

edited

Loading

Uh oh!

cxbAsDev commented Jul 6, 2026

Uh oh!

cxbAsDev commented Jul 6, 2026

Uh oh!

cxbAsDev commented Jul 6, 2026

Uh oh!

steipete commented Jul 7, 2026

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 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Uh oh!

cxbAsDev commented Jul 6, 2026

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

cxbAsDev commented Jul 6, 2026

Uh oh!

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

Uh oh!

Uh oh!

cxbAsDev commented Jul 6, 2026

Uh oh!

cxbAsDev commented Jul 6, 2026

Uh oh!

cxbAsDev commented Jul 6, 2026

Uh oh!

steipete commented Jul 7, 2026

Uh oh!

Uh oh!

steipete commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cxbAsDev commented Jul 6, 2026 •

edited

Loading

clawsweeper Bot commented Jul 6, 2026 •

edited

Loading

clawsweeper Bot commented Jul 6, 2026 •

edited

Loading

clawsweeper Bot commented Jul 6, 2026 •

edited

Loading