fix(ssh-tunnel): handle spawn error to prevent unhandled rejection crash by cxbAsDev · Pull Request #99800 · openclaw/openclaw

cxbAsDev · 2026-07-04T04:00:45Z

What Problem This Solves

Fixes an issue where failing to spawn the SSH executable could escape as an unhandled child-process error and delay gateway status by another 1.5 seconds during cleanup.

Why This Change Was Made

The tunnel startup race now listens for the child error event. Cleanup returns immediately when SIGTERM cannot be delivered, matching Node's failed-spawn lifecycle where no PID exists, kill() returns false, and only error then close are emitted.

User Impact

Gateway status reports missing or unspawnable SSH failures promptly instead of crashing or waiting for an exit event that will never arrive. Live SSH children retain the existing graceful-then-forced cleanup behavior.

Evidence

Focused regression: node scripts/run-vitest.mjs src/infra/ssh-tunnel.test.ts (7 passed).
Real Node missing-executable probe:

{"killed":false,"exitCode":-2,"killResult":false,"events":[{"event":"error","code":"ENOENT"},{"event":"close","code":-2,"signal":null}]}

The regression models no PID, kill() === false, error followed by close, no exit, and zero pending cleanup timers.
oxfmt and git diff --check passed.
Fresh structured autoreview reported no accepted or actionable findings.
Exact prepared head: 1f8149cf92daa1d91a94a504a7acfcfef0302e4f.

When /usr/bin/ssh is missing (ENOENT), spawn() emits an async 'error' event. Without a handler, the unhandled error crashes the process and the Promise.race hangs until timeout. Forward the spawn error into the rejection path so tunnel setup fails gracefully. Co-Authored-By: Claude <noreply@anthropic.com>

clawsweeper · 2026-07-04T04:04:27Z

Codex review: needs maintainer review before merge. Reviewed July 4, 2026, 1:44 AM ET / 05:44 UTC.

Summary
The PR adds SSH child-process error handling, avoids waiting for cleanup when SIGTERM cannot be delivered after a failed spawn, and adds a regression test for missing-ssh ENOENT behavior.

PR surface: Source 0, Tests +42. Total +42 across 2 files.

Reproducibility: yes. Source inspection shows current main lacks a child error listener in the tunnel startup race, and a live Node missing-executable probe confirms the no-PID, error-then-close, no-exit lifecycle the patch covers.

Review metrics: none identified.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
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.

Rank-up moves:

none.

Next step before merge

No automated repair is needed; the remaining action is ordinary maintainer review, required-check gating, and merge judgment for the open PR.

Security
Cleared: The diff handles errors around an existing SSH spawn path and adds mocked regression coverage; it adds no dependency, workflow, secret, permission, or new command-execution surface.

Review details

Best possible solution:

Land this narrow failed-spawn mitigation after ordinary maintainer review and required checks, while keeping the broader trusted SSH executable-resolution work in #83289 and #93030 separate.

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

Yes. Source inspection shows current main lacks a child error listener in the tunnel startup race, and a live Node missing-executable probe confirms the no-PID, error-then-close, no-exit lifecycle the patch covers.

Is this the best way to solve the issue?

Yes for the narrow crash and cleanup-delay mitigation. Adding the error listener to the existing startup race and respecting kill() returning false is the smallest maintainable fix; the separate SSH executable-resolution issue remains out of scope.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

add proof: sufficient: Contributor real behavior proof is sufficient. The PR body and maintainer comment provide copied live Node missing-executable output plus exact-head Testbox validation, which is enough real behavior proof for this narrow non-visual child-process lifecycle fix.
add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body and maintainer comment provide copied live Node missing-executable output plus exact-head Testbox validation, which is enough real behavior proof for this narrow non-visual child-process lifecycle fix.
remove rating: 🧂 unranked krab: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

P2: This is a normal-priority availability fix for optional SSH gateway tunneling with a small, well-bounded runtime surface.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit 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 and maintainer comment provide copied live Node missing-executable output plus exact-head Testbox validation, which is enough real behavior proof for this narrow non-visual child-process lifecycle fix.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body and maintainer comment provide copied live Node missing-executable output plus exact-head Testbox validation, which is enough real behavior proof for this narrow non-visual child-process lifecycle fix.

Evidence reviewed

PR surface:

Source 0, Tests +42. Total +42 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	2	2	0
Tests	1	42	0	+42
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	44	2	+42

What I checked:

Repository policy read: Root AGENTS.md was read fully; no scoped AGENTS.md owns src/infra, so root review-depth, dependency, proof, and owner-boundary guidance applied. (AGENTS.md:1, acdaf8ae31e6)
Current-main failure shape: Current main races local-listener readiness only against child exit; it has no child error listener in startSshPortForward, so a failed spawn can escape the intended rejection path. (src/infra/ssh-tunnel.ts:198, acdaf8ae31e6)
PR implementation: The branch adds child.once("error", reject) to the existing startup race and returns immediately from cleanup when child.kill("SIGTERM") returns false. (src/infra/ssh-tunnel.ts:197, 1f8149cf92da)
Caller contract: Gateway status already catches startSshPortForward failures, stores sshTunnelError, and continues probing, so rejecting the spawn error fits the existing user-facing status flow. (src/commands/gateway-status/probe-run.ts:70, acdaf8ae31e6)
Sibling SSH helper invariant: resolveSshConfig already handles child-process error and resolves null, supporting the invariant that SSH helper spawn failures should be handled instead of escaping as unhandled errors. (src/infra/ssh-config.ts:117, acdaf8ae31e6)
Dependency behavior proof: A live Node missing-executable probe produced no PID, error with ENOENT, close with code -2, no exit, and kill("SIGTERM") === false, matching the lifecycle the PR handles.

Likely related people:

steipete: Introduced the SSH tunnel/gateway status SSH path and later security hardening touched the same helper surface. (role: feature-history owner; confidence: high; commits: d258c68ca1ab, 06289b36da72; files: src/infra/ssh-tunnel.ts, src/infra/ssh-config.ts, src/commands/gateway-status.ts)
wangwllu: Authored the recently merged SSH tunnel loopback preflight hardening in the same helper and adjacent test file. (role: recent area contributor; confidence: medium; commits: b60ce6d11d8f; files: src/infra/ssh-tunnel.ts, src/infra/ssh-tunnel.test.ts)
vincentkoc: Added the latest exact-head cleanup refinement and supplied maintainer proof for this failed-spawn mitigation. (role: recent PR-branch repair contributor; confidence: medium; commits: 1f8149cf92da; files: src/infra/ssh-tunnel.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.

Co-Authored-By: Claude <noreply@anthropic.com>

vincentkoc · 2026-07-04T05:36:53Z

maintainer repair and exact-head proof are complete on 1f8149cf92daa1d91a94a504a7acfcfef0302e4f.

changes:

catch the SSH child error event during tunnel startup
return immediately when SIGTERM cannot be delivered after a failed spawn
model Node's real ENOENT lifecycle: no PID, error then close, no exit, kill() === false

validation:

node scripts/run-vitest.mjs src/infra/ssh-tunnel.test.ts (7 passed)
real Node missing-executable lifecycle probe
fresh autoreview: no accepted/actionable findings
OPENCLAW_TESTBOX=1 pnpm check:changed
Testbox tbx_01kwnsw7vr8bd2f20fxbaga4rq (exit 0, 2m56s command time)

the PR body now contains the durable behavior evidence.

@clawsweeper re-review

clawsweeper · 2026-07-04T05:36:55Z

🦞🧹
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.

vincentkoc · 2026-07-04T05:55:14Z

Merged via squash.

Prepared head SHA: 1f8149cf92daa1d91a94a504a7acfcfef0302e4f
Landed commit: c6ed9d8d06a438a18f8314af24c1a669ed47a6c9

…ash (openclaw#99800) * fix(ssh-tunnel): handle spawn error to prevent unhandled rejection crash When /usr/bin/ssh is missing (ENOENT), spawn() emits an async 'error' event. Without a handler, the unhandled error crashes the process and the Promise.race hangs until timeout. Forward the spawn error into the rejection path so tunnel setup fails gracefully. Co-Authored-By: Claude <noreply@anthropic.com> * fix(ssh-tunnel): add missing localPortPreferred in test call Co-Authored-By: Claude <noreply@anthropic.com> * fix(ssh-tunnel): avoid failed-spawn cleanup delay --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Vincent Koc <vincentkoc@ieee.org>

…ash (openclaw#99800) * fix(ssh-tunnel): handle spawn error to prevent unhandled rejection crash When /usr/bin/ssh is missing (ENOENT), spawn() emits an async 'error' event. Without a handler, the unhandled error crashes the process and the Promise.race hangs until timeout. Forward the spawn error into the rejection path so tunnel setup fails gracefully. Co-Authored-By: Claude <noreply@anthropic.com> * fix(ssh-tunnel): add missing localPortPreferred in test call Co-Authored-By: Claude <noreply@anthropic.com> * fix(ssh-tunnel): avoid failed-spawn cleanup delay --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Vincent Koc <vincentkoc@ieee.org> (cherry picked from commit c6ed9d8)

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

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

🦞 OpenClaw 生态日报 2026-07-04 litang9/big_model_radar#160

Open

fix(ssh-tunnel): add missing localPortPreferred in test call

40fcd42

Co-Authored-By: Claude <noreply@anthropic.com>

openclaw-barnacle Bot added triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. and removed triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jul 4, 2026

vincentkoc self-assigned this Jul 4, 2026

vincentkoc added 2 commits July 3, 2026 22:32

Merge branch 'main' into fix/ssh-tunnel-spawn-error-handler

bd4edf3

fix(ssh-tunnel): avoid failed-spawn cleanup delay

1f8149c

vincentkoc merged commit c6ed9d8 into openclaw:main Jul 4, 2026
180 of 188 checks passed

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

📡 Upstream Digest — 2026-07-04 06:40 UTC curtismercier/openclaw-mods#1202

Open

cxbAsDev mentioned this pull request Jul 4, 2026

fix(lsp): suppress unhandled spawn error on LSP child process #99922

Closed

clawsweeper Bot mentioned this pull request Jul 6, 2026

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

Merged

cxbAsDev deleted the fix/ssh-tunnel-spawn-error-handler branch July 15, 2026 03:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(ssh-tunnel): handle spawn error to prevent unhandled rejection crash#99800

fix(ssh-tunnel): handle spawn error to prevent unhandled rejection crash#99800
vincentkoc merged 4 commits into
openclaw:mainfrom
cxbAsDev:fix/ssh-tunnel-spawn-error-handler

cxbAsDev commented Jul 4, 2026 •

edited by vincentkoc

Loading

Uh oh!

clawsweeper Bot commented Jul 4, 2026 •

edited

Loading

Uh oh!

vincentkoc commented Jul 4, 2026

Uh oh!

clawsweeper Bot commented Jul 4, 2026 •

edited

Loading

Uh oh!

Uh oh!

vincentkoc commented Jul 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

cxbAsDev commented Jul 4, 2026 • edited by vincentkoc 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 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vincentkoc commented Jul 4, 2026

Uh oh!

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

Uh oh!

Uh oh!

Uh oh!

vincentkoc commented Jul 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cxbAsDev commented Jul 4, 2026 •

edited by vincentkoc

Loading

clawsweeper Bot commented Jul 4, 2026 •

edited

Loading

clawsweeper Bot commented Jul 4, 2026 •

edited

Loading