From e8c1c75a83e3de549b440352865f60f3d3df93a3 Mon Sep 17 00:00:00 2001 From: Felipe Date: Thu, 30 Apr 2026 20:47:08 -0300 Subject: [PATCH] fix(dispatch): runWorkDispatch must throw, not exit, to preserve sibling spawns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ROOT CAUSE of the long-running #1589/#1600 phantom dispatch saga, found empirically on .25 after PR #1601's audit observability shipped: `runWorkDispatch` called `process.exit(1)` on three error paths (wish not found, group not found, startGroup failed). When `autoOrchestrateCommand` runs `Promise.allSettled([runWorkDispatch × N])` for a parallel wave, ANY group's process.exit terminates the entire node process — killing every sibling spawn mid-flight before: - handleWorkerSpawn's worker.spawn audit event fires - launchTmuxSpawn reaches the new validateSpawnedPane check - any of the new worker.spawn.failed / worker.spawn.ok events fire This explains why .24 + .25 still phantom-dispatched despite the `wish.dispatch.work` event firing correctly: the dispatch event landed during the brief window between Group N's PG state mutation and Group N's spawn pipeline being killed by Group N+1's exit(). The fix: throw `new Error(...)` on each runWorkDispatch error path. Both callers handle the throw correctly: - `workDispatchCommand` (single-group CLI): the outer commander handler in registerDispatchCommands already wraps in try/catch + process.exit, so single-group semantics are preserved. - `autoOrchestrateCommand` (wave): Promise.allSettled collects rejections into the `failed` array, my Round 2 (#1601) wish.dispatch.failed event fires per group, and stderr summary prints with the [ErrorClass] prefix. Empirical confirmation: Before: `genie work tui-bottom-bar-opentui` exits silently after Group 5's dependency error kills the process. No worker.spawn / .ok / .failed events fire for in-flight Group 1. After (this fix): Group 5's startGroup throws; Promise.allSettled collects the rejection. Group 1's spawn pipeline runs to completion, emitting either worker.spawn.ok (success) or worker.spawn.failed (real spawn failure surfaced). Tests: - 1 new regression-guard in dispatch.test.ts (#1600 Group 4): asserts zero EXECUTABLE process.exit calls in runWorkDispatch (comment text mentioning the OLD behavior is excluded), and >=3 throw-new-Error statements covering wish-not-found, group-not-found, startGroup-failed. - Total: 86 pass / 0 fail in dispatch.test.ts. - bun run typecheck: clean. Followup to #1599 + #1601. Should close the root-cause iteration on the long-running #1589/#1600 phantom-dispatch saga. Co-Authored-By: Claude Opus 4.7 --- src/term-commands/dispatch.test.ts | 33 ++++++++++++++++++++++++++++++ src/term-commands/dispatch.ts | 32 +++++++++++++++++------------ 2 files changed, 52 insertions(+), 13 deletions(-) diff --git a/src/term-commands/dispatch.test.ts b/src/term-commands/dispatch.test.ts index b527bb666..ba618bb1c 100644 --- a/src/term-commands/dispatch.test.ts +++ b/src/term-commands/dispatch.test.ts @@ -1238,6 +1238,39 @@ describe('#1600 spawn-pipeline silent-fail regression guards', () => { }); }); + describe('Group 4 — runWorkDispatch must never process.exit (kill-siblings prevention)', () => { + it('runWorkDispatch contains zero EXECUTABLE process.exit calls — they kill sibling spawns in Promise.allSettled', () => { + // ROOT CAUSE of the long-running #1589/#1600 phantom dispatch: when + // `autoOrchestrateCommand` runs `Promise.allSettled([runWorkDispatch, …])` + // for a parallel wave, ANY group's process.exit(1) terminates the entire + // node process — killing every sibling spawn mid-flight before audit + // events can fire. The fix: throw instead of exit. The single-group CLI + // caller `workDispatchCommand` already re-throws and the outer CLI + // handler does process.exit, so single-group semantics are preserved. + const fnAnchor = dispatchSrc.indexOf('async function runWorkDispatch'); + expect(fnAnchor).toBeGreaterThan(-1); + // Body ends at the next top-level `export async function` or `async function`. + // Use a regex that anchors on lines starting with `}` followed by a blank line then a docstring. + const afterFn = dispatchSrc.slice(fnAnchor + 1); + const nextFnRel = afterFn.search(/\nasync function |\nexport async function |\nexport function |\nfunction /); + const fnEnd = nextFnRel !== -1 ? fnAnchor + 1 + nextFnRel : dispatchSrc.length; + const body = dispatchSrc.slice(fnAnchor, fnEnd); + // Strip comments before checking — `process.exit(1)` may legitimately + // appear in JSDoc / inline comments explaining the OLD behavior. + const stripped = body + .split('\n') + .filter((line) => !line.trim().startsWith('//') && !line.trim().startsWith('*')) + .join('\n'); + // Hard guarantee: zero EXECUTABLE process.exit calls in non-comment code. + expect(stripped).not.toContain('process.exit'); + // Soft guarantee: the function explicitly throws on each error path so + // Promise.allSettled in autoOrchestrateCommand collects rejections. + const throwCount = (stripped.match(/throw new Error\(/g) ?? []).length; + // At minimum: wish-not-found, group-not-found, startGroup-failed. + expect(throwCount).toBeGreaterThanOrEqual(3); + }); + }); + describe('Group 3 — wish.dispatch.failed surfacing', () => { it('autoOrchestrateCommand emits wish.dispatch.failed per failed group', () => { const fnAnchor = dispatchSrc.indexOf('async function autoOrchestrateCommand'); diff --git a/src/term-commands/dispatch.ts b/src/term-commands/dispatch.ts index f76ccc74d..e61e9bec0 100644 --- a/src/term-commands/dispatch.ts +++ b/src/term-commands/dispatch.ts @@ -656,10 +656,20 @@ async function runWorkDispatch( wishPath: string, ref: string, ): Promise { + // CRITICAL (#1600 Round 3): never call `process.exit(1)` from this function. + // `runWorkDispatch` is invoked from TWO callers: + // 1. `workDispatchCommand` (single-group CLI) — wraps in try/catch and the + // outer CLI handler calls process.exit(1) with the error message. + // 2. `autoOrchestrateCommand` (wave dispatch via Promise.allSettled) — + // collects rejections and reports per-group failures. + // If we exit() here from within a parallel wave, we kill the whole node + // process mid-dispatch — including SIBLING spawns that are mid-flight in + // handleWorkerSpawn. That's THE silent-fail surface from #1589/#1600: + // any group's startGroup failure (dependency not done, already in_progress) + // killed every other group's spawn before audit events could fire. Throw + // instead so the wave-dispatcher's allSettled boundary handles it. if (!existsSync(wishPath)) { - console.error(`❌ Wish not found: ${wishPath}`); - console.error(` Create it first: genie wish ${slug}`); - process.exit(1); + throw new Error(`Wish not found: ${wishPath}. Create it first: genie wish ${slug}`); } const content = await readFile(wishPath, 'utf-8'); @@ -667,27 +677,23 @@ async function runWorkDispatch( // Extract the specific group section const groupSection = extractGroup(content, group); if (!groupSection) { - console.error(`❌ Group "${group}" not found in ${wishPath}`); - console.error(' Available groups:'); - const groups = content.match(/^### Group [A-Za-z0-9]+:.*$/gm); - if (groups) { - for (const g of groups) console.error(` ${g}`); - } - process.exit(1); + const availableGroups = content.match(/^### Group [A-Za-z0-9]+:.*$/gm); + const availableList = availableGroups ? availableGroups.join(', ') : '(none)'; + throw new Error(`Group "${group}" not found in ${wishPath}. Available: ${availableList}`); } // Auto-initialize state if missing (prevents polling loop when no state exists) const groups = parseWishGroups(content); await wishState.getOrCreateState(slug, groups); - // Start group in state machine (enforces dependencies) + // Start group in state machine (enforces dependencies). Throw on failure + // (NOT exit) — see top-of-function comment. try { await wishState.startGroup(slug, group, agentName); console.log(`✅ Group "${group}" set to in_progress (assigned to ${agentName})`); } catch (error) { const message = error instanceof Error ? error.message : String(error); - console.error(`❌ ${message}`); - process.exit(1); + throw new Error(message); } // Build context with wish-level info + group section