fix(infra): handle detached respawn child errors#101489
Conversation
|
Codex review: passed. Reviewed July 7, 2026, 6:28 AM ET / 10:28 UTC. Summary PR surface: Source +3, Tests +30. Total +33 across 2 files. Reproducibility: yes. source-level reproduction is high confidence: current main lacks the child error listener, and a direct Node v24.18.0 probe shows the missing-executable child error crashes without a listener and survives with one. I did not run a full OpenClaw package update respawn E2E. Review metrics: none identified. Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land this focused listener-before-unref fix as the canonical fix for #101458 after exact-head validation. Do we have a high-confidence way to reproduce the issue? Yes, source-level reproduction is high confidence: current main lacks the child error listener, and a direct Node v24.18.0 probe shows the missing-executable child error crashes without a listener and survives with one. I did not run a full OpenClaw package update respawn E2E. Is this the best way to solve the issue? Yes. Attaching a best-effort child error listener immediately after spawn and before unref is the narrowest maintainable fix, matches the sibling restart-helper pattern, and preserves the existing unhealthy-child fallback path. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against cba20f9aec7b. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +3, Tests +30. Total +33 across 2 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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
Review history (3 earlier review cycles) |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper automerge |
|
ClawSweeper 🐠 automerge status ClawSweeper took another look; no safe branch change was available on this pass. Executor outcome: no planned fix actions. Worker actions:
This pass stayed observational only. No branch push, replacement, merge, or re-review was started. fish notes: reasoning high; reviewed against 918c4da. Automerge progress:
|
69fc48f to
0d8c38e
Compare
0d8c38e to
918c4da
Compare
|
Land-ready maintainer verification:
Known gap: no destructive end-to-end self-update restart was run. The direct Node contract proof, focused lifecycle test, caller fallback review, remote changed gate, and exact-head CI cover this narrow failure mode. |
|
Merged via squash.
|
* fix(infra): handle detached respawn child errors * docs(infra): explain detached respawn error listener * fix(infra): handle detached respawn child errors * fix(infra): handle detached respawn child errors --------- Co-authored-by: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com>
* fix(infra): handle detached respawn child errors * docs(infra): explain detached respawn error listener * fix(infra): handle detached respawn child errors * fix(infra): handle detached respawn child errors --------- Co-authored-by: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com>
What Problem This Solves
Fixes #101458. Detached gateway update respawns can emit an asynchronous ChildProcess
errorafterspawn()returns; without a listener, Node treats that as unhandled and crashes the current process.Why This Change Was Made
The detached respawn path now attaches a best-effort
errorlistener beforeunref(), matching the existing restart-helper pattern. The update handoff still returns the child/pid when spawn succeeds, and synchronous spawn failures still flow through the existing failed result.User Impact
Users hitting a missing or invalid executable during detached update respawn no longer get an extra uncaught child-process error on the parent process.
Evidence
npx -y [email protected]real ChildProcess probe with a missing detached executable:node scripts/run-vitest.mjs src/infra/process-respawn.test.tsgit diff --checkOPENCLAW_CHECK_CHANGED_REMOTE_CHILD=1 OPENCLAW_CHANGED_LANES_RAW_SYNC=1 CI=1 PNPM_CONFIG_VERIFY_DEPS_BEFORE_RUN=false pnpm check:changed -- src/infra/process-respawn.ts src/infra/process-respawn.test.ts