fix(ssh-tunnel): ignore stderr stream errors during teardown by cxbAsDev · Pull Request #100855 · openclaw/openclaw

cxbAsDev · 2026-07-06T11:07:36Z

What Problem This Solves

startSshPortForward in src/infra/ssh-tunnel.ts reads ssh stderr for diagnostics but does not attach an "error" listener to the stderr stream. If stderr errors during ssh teardown (e.g. EPIPE after the child exits), the unhandled error can crash the gateway.

Why This Change Was Made

Added a no-op "error" listener on child.stderr. stderr is used only for diagnostic lines, so a stream error there is not actionable; swallowing it prevents an unhandled exception while the normal child error/exit paths still surface ssh-level failures.

User Impact

SSH tunnel teardown and gateway stability improve: stderr stream errors no longer propagate as unhandled exceptions.

Evidence

Regression test:

pnpm test src/infra/ssh-tunnel.test.ts

Test Files  1 passed (1)
     Tests  8 passed (8)

Real behavior proof:

node_modules/.bin/tsx scripts/proof/ssh-tunnel-stderr-stream-errors.mts

=== Proof: ssh-tunnel stderr stream error handling ===

Tunnel established on local port 43210.
Emitted stderr stream error.

PASS: stderr stream error was ignored and tunnel stopped cleanly.

clawsweeper · 2026-07-06T11:51:43Z

Codex review: needs maintainer review before merge. Reviewed July 6, 2026, 11:59 AM ET / 15:59 UTC.

Summary
The branch adds an error listener to the SSH tunnel child stderr stream and regression coverage for active and teardown stream-error cases.

PR surface: Source +4, Tests +25. Total +29 across 2 files.

Reproducibility: yes. from source inspection. Current main observes stderr data without an stderr error listener, and Node's EventEmitter throws unhandled 'error' events when no listener is present.

Review metrics: none identified.

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

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

Risk before merge

[P1] The latest maintainer proof comment names head 9a0cf5773566efe2a3c439d35e314aab7ee994d1, while the live head is cafce4e with expanded tests; merge should still wait for exact-head required checks such as QA Smoke CI to finish.

Maintainer options:

Decide the mitigation before merge
Merge the focused runtime and regression-test fix after the current head's required checks complete, preserving child error and exit as the authoritative SSH failure path.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

No ClawSweeper repair lane is needed; the remaining action is maintainer merge after exact-head gates finish.

Security
Cleared: The diff adds no dependency, workflow, permission, secret, or new command-execution surface; it only observes a diagnostic stderr stream while preserving existing SSH child process failure handling.

Review details

Best possible solution:

Merge the focused runtime and regression-test fix after the current head's required checks complete, preserving child error and exit as the authoritative SSH failure path.

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

Yes, from source inspection. Current main observes stderr data without an stderr error listener, and Node's EventEmitter throws unhandled 'error' events when no listener is present.

Is this the best way to solve the issue?

Yes. Best-fix verdict: best; observing only the diagnostic stderr stream is the narrowest maintainable fix because child error and exit events still report real SSH failures.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

P2: This is a focused gateway SSH tunnel stability bug fix with limited blast radius and no blocking review finding.
rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes terminal output from an after-fix SSH tunnel proof, the current head keeps focused regression coverage, and the Real behavior proof check passed for the live head.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes terminal output from an after-fix SSH tunnel proof, the current head keeps focused regression coverage, and the Real behavior proof check passed for the live head.

Evidence reviewed

PR surface:

Source +4, Tests +25. Total +29 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	6	2	+4
Tests	1	25	0	+25
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	31	2	+29

What I checked:

Repository policy read: Root AGENTS.md was read fully in two ranges and applied to require source, sibling, dependency-contract, and best-fix review before verdict. (AGENTS.md:1, 8547682dd6f3)
Current main behavior: Current main reads SSH stderr for diagnostics but has no stderr stream error listener before returning the tunnel handle, so a stream error can use Node's unhandled error path. (src/infra/ssh-tunnel.ts:168, 8547682dd6f3)
PR runtime fix: The PR keeps a local stderrStream reference, attaches a no-op error listener, and leaves child process error and exit handling in place as the authoritative SSH failure signal. (src/infra/ssh-tunnel.ts:168, cafce4e492f3)
Regression coverage: The PR test emits stderr stream errors while the tunnel is active and during teardown, and asserts the emit does not throw and stop resolves. (src/infra/ssh-tunnel.test.ts:209, cafce4e492f3)
Caller path: Gateway status starts the SSH port forward through startSshPortForward and always attempts tunnel.stop() in finally, so teardown-time stream errors affect a real gateway status path. (src/commands/gateway-status/probe-run.ts:70, 8547682dd6f3)
Sibling invariant: The shared process executor already treats stdout/stderr stream errors as non-authoritative while child exit/close remains authoritative, matching this fix shape. (src/process/exec.ts:615, 8547682dd6f3)

Likely related people:

cxbAsDev: Authored the merged SSH tunnel spawn-error fix in fix(ssh-tunnel): handle spawn error to prevent unhandled rejection crash #99800 and co-authored recent output-stream hardening work, so the same SSH tunnel failure surface is familiar history. (role: recent area contributor; confidence: high; commits: c6ed9d8d06a4, d1ec54cb60c3; files: src/infra/ssh-tunnel.ts, src/infra/ssh-tunnel.test.ts)
vincentkoc: Merged the prior SSH tunnel crash fix, is assigned on this PR, and added the intermediate regression-suite commit that removed the one-off proof script. (role: recent reviewer and merger; confidence: high; commits: c6ed9d8d06a4, 9a0cf5773566; files: src/infra/ssh-tunnel.ts, src/infra/ssh-tunnel.test.ts)
steipete: Authored and merged the recent subprocess output-stream hardening PR and added the latest test expansion on this PR branch. (role: adjacent owner; confidence: high; commits: a4b032e5d710, cafce4e492f3; files: src/process/exec.ts, src/agents/sessions/exec.ts, src/infra/ssh-tunnel.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-06T11:55:55.043Z sha a2f940da59fe8be325c8140fffad28ac71644414 :: needs real behavior proof before merge. :: none
reviewed 2026-07-06T14:21:15.714Z sha 1bd24c334ca02eb29382e74275ce50144aacf672 :: needs changes before merge. :: [P3] Stop returning from the Promise executor
reviewed 2026-07-06T15:10:31.444Z sha c5f455e95f3a00986fd061c17f992f159539bd9f :: needs maintainer review before merge. :: none
reviewed 2026-07-06T15:19:30.543Z sha c5f455e95f3a00986fd061c17f992f159539bd9f :: needs maintainer review before merge. :: none

…rors

vincentkoc · 2026-07-06T15:50:07Z

Maintainer review complete for exact head cafce4e492f396e2785668a8a6f5024297783f30.

I kept the runtime fix, removed the 76-line one-off proof script, and retained the author's improved table-driven regression coverage for both active-use and teardown timing. The diagnostic stderr pipe is observed without changing the authoritative child error/exit lifecycle.

Validation:

node scripts/run-vitest.mjs src/infra/ssh-tunnel.test.ts src/commands/gateway-status.test.ts — 39 tests passed
node_modules/.bin/oxfmt --check src/infra/ssh-tunnel.ts src/infra/ssh-tunnel.test.ts
git diff --check
CI run https://github.com/openclaw/openclaw/actions/runs/28804406855 — passed, including QA Smoke CI
CodeQL run https://github.com/openclaw/openclaw/actions/runs/28804406429 — passed
fresh gpt-5.5 high autoreview — clean, confidence 0.90
native scripts/pr prepare-run 100855 — exact-head gates passed

No remaining proof gaps. Ready to squash merge.

vincentkoc · 2026-07-06T16:03:35Z

Merged via squash.

Prepared head SHA: cafce4e492f396e2785668a8a6f5024297783f30
Landed commit: 32cc9b08e6b3d6d7f297bdf1008b6fffc4e00b2b

steipete · 2026-07-06T16:05:03Z

Maintainer repair is land-ready at cafce4e492f396e2785668a8a6f5024297783f30.

Before: an SSH diagnostic stderr stream error could be emitted without an error observer and terminate the process.
After: the diagnostic pipe is observed before encoding/data listeners while child events remain the tunnel authority.
Repair: deleted the proof-only script and made active-use versus teardown timing explicit in the regression.
Local lightweight checks: git diff --check; targeted formatting was verified with oxfmt.
Exact-head proof: full hosted CI is green: https://github.com/openclaw/openclaw/actions/runs/28804406855
Fresh autoreview: no accepted/actionable findings.

Known gap: no external SSH server was used; the regression directly exercises both lifecycle phases around the owned child stream.

steipete · 2026-07-06T16:06:00Z

Merged via squash.

Prepared head SHA: cafce4e492f396e2785668a8a6f5024297783f30
Landed commit: 32cc9b08e6b3d6d7f297bdf1008b6fffc4e00b2b

…w#100855) * fix(ssh-tunnel): ignore stderr stream errors during teardown * chore(proof): add real behavior proof for ssh-tunnel stderr stream errors * chore(proof): fix promise executor lint in ssh-tunnel proof * test(ssh-tunnel): keep stream error proof in regression suite * test(ssh): cover active stream errors --------- Co-authored-by: Vincent Koc <vincentkoc@ieee.org> Co-authored-by: Peter Steinberger <steipete@gmail.com>

openclaw-barnacle Bot added the size: XS label Jul 6, 2026

openclaw-barnacle Bot added scripts Repository scripts size: S and removed size: XS labels Jul 6, 2026

cxbAsDev force-pushed the fix/ssh-tunnel-stderr-stream-errors branch from c811108 to 1bd24c3 Compare July 6, 2026 14:05

cxbAsDev force-pushed the fix/ssh-tunnel-stderr-stream-errors branch from c5f455e to 4ea5ebb Compare July 6, 2026 15:29

vincentkoc self-assigned this Jul 6, 2026

openclaw-barnacle Bot added size: XS and removed scripts Repository scripts size: S labels Jul 6, 2026

cxbAsDev and others added 5 commits July 6, 2026 11:49

fix(ssh-tunnel): ignore stderr stream errors during teardown

471fa5f

chore(proof): add real behavior proof for ssh-tunnel stderr stream er…

3acbc2e

…rors

chore(proof): fix promise executor lint in ssh-tunnel proof

58f5ace

test(ssh-tunnel): keep stream error proof in regression suite

9547817

test(ssh): cover active stream errors

cafce4e

steipete force-pushed the fix/ssh-tunnel-stderr-stream-errors branch from 9a0cf57 to cafce4e Compare July 6, 2026 15:49

vincentkoc merged commit 32cc9b0 into openclaw:main Jul 6, 2026
96 checks passed

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

📡 Upstream Digest — 2026-07-06 16:47 UTC curtismercier/openclaw-mods#1223

Open

clawsweeper Bot mentioned this pull request Jul 8, 2026

fix(ssh-tunnel): log stderr stream errors instead of silently swallowing them #102215

Closed

cxbAsDev deleted the fix/ssh-tunnel-stderr-stream-errors branch July 15, 2026 03:33

Navigation Menu

Uh oh!

fix(ssh-tunnel): ignore stderr stream errors during teardown - #100855

fix(ssh-tunnel): ignore stderr stream errors during teardown#100855
vincentkoc merged 5 commits into
openclaw:mainfrom
cxbAsDev:fix/ssh-tunnel-stderr-stream-errors

cxbAsDev commented Jul 6, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jul 6, 2026 •

edited

Loading

Uh oh!

vincentkoc commented Jul 6, 2026 •

edited

Loading

Uh oh!

Uh oh!

vincentkoc commented Jul 6, 2026

Uh oh!

steipete commented Jul 6, 2026

Uh oh!

steipete commented Jul 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

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!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

Uh oh!

vincentkoc commented Jul 6, 2026

Uh oh!

steipete commented Jul 6, 2026

Uh oh!

steipete commented Jul 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cxbAsDev commented Jul 6, 2026 •

edited

Loading

clawsweeper Bot commented Jul 6, 2026 •

edited

Loading

vincentkoc commented Jul 6, 2026 •

edited

Loading