fix(infra): register no-op error handler on detached gateway spawn child#101491
fix(infra): register no-op error handler on detached gateway spawn child#101491chenxiaoyu209 wants to merge 1 commit into
Conversation
|
Thanks for the context here. I swept through the related work, and this is now duplicate or superseded. Close as a duplicate landing candidate: the same production fix is already carried by an open maintainer-labeled PR with slightly stronger regression coverage, and this branch does not add a meaningful unique runtime change. Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Canonical path: #101489 is open, mergeable, maintainer-labeled, and carries the same production line while using a shared mock child helper and asserting the listener is registered before unref. So I’m closing this here and keeping the remaining discussion on #101489 and #101458. Review detailsBest possible solution: Land one canonical maintainer-owned fix for the linked gateway respawn crash and close duplicate fix PRs that do not add unique runtime behavior. Do we have a high-confidence way to reproduce the issue? Yes. Current main lacks the child error listener in the detached gateway respawn path, and a direct Node v24.18.0 child_process probe shows the parent exits on an unhandled child error without that listener. Is this the best way to solve the issue? No, not as a separate landing branch. The code change is correct, but the best path is the open maintainer-labeled canonical PR that carries the same runtime fix with stronger ordering coverage. Security review: Security review cleared: The diff only adds a no-op ChildProcess error listener and unit-test updates; it does not broaden secrets, dependency, workflow, or package execution surfaces. AGENTS.md: found and applied where relevant. What I checked:
Likely related people:
Codex review notes: model internal, reasoning high; reviewed against 24330a82b44a. |
What Problem This Solves
Fixes #101458 — In
spawnDetachedGatewayProcess, thechild_process.spawn()call creates a detached child process and immediately callschild.unref()without registering an error listener. If spawn fails asynchronously (e.g., ENOMEM, missing executable), the ChildProcess emits an unhandlederrorevent that crashes the parent Node.js process.Why This Change Was Made
The detached spawn error listener was missing, creating a crash path on async spawn failures during gateway update restarts. The sibling
restart-helper.tsalready uses the same pattern:child.on("error", () => {})beforechild.unref(). This fix mirrors that established pattern.User Impact
Prevents parent process crash when the detached gateway spawn child encounters an async error (ENOMEM, missing binary, etc.). The error is silently swallowed since the child is detached and the parent cannot recover — matching the best-effort handoff semantics of the existing restart-helper.
Evidence
src/cli/update-cli/restart-helper.ts:417— uses exact samechild.on("error", () => {})+child.unref()patternclawsweeper:fix-shape-clear,clawsweeper:source-repro,clawsweeper:queueable-fix🤖 Generated with Claude Code