Skip to content

Commit cfad7fc

Browse files
committed
fix(vc-agent): durable 启动失败 gate 覆盖 fork-error 与 pre-ready exit
复审(Codex)揪出的 blocker:#3 的 gate 只堵了带 dispatchAttempt 的 structured error;durable 投递被 dispatch(queued)进还没 ready 的 worker、worker 崩在 ready 前时, fork-level `error`(直接 sessionReply)和 abrupt pre-ready `exit`(notifyStartupFailure 无 attempt)两条 fallback 仍会 out-of-band 发 Lark,silent 投递尤其会被误发、且与 receipt/lease 重试双发,绕过副作用边界。 修法: - 把 initTurnId/initDispatchAttempt 冻进 WorkerStartupState(fork 时)。 - fork-level `error` 与 pre-ready `exit` 都走同一 durable gate:VC receiver + 有 dispatchAttempt → 交给 receipt/lease→ambiguous,不 out-of-band reply。 - worker.ts crash-loop relaunch 显式透传 msg.dispatchAttempt(不再退回 stale current)。 - 补测:durable pre-ready exit / durable fork-error 各 0 reply;VC receiver 的 IM turn (无 attempt)fork-error 仍通知一次(证明 gate 精确,不 blanket 抑制)。
1 parent 7630f3d commit cfad7fc

4 files changed

Lines changed: 88 additions & 4 deletions

File tree

src/core/worker-pool.ts

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,13 @@ type WindowsForkOptions = ForkOptions & { windowsHide?: boolean };
9494
type WorkerStartupState = {
9595
ready: boolean;
9696
failureNotified: boolean;
97+
/** Init turn attribution frozen at fork. A durable VC delivery is dispatched
98+
* (queued) into a not-yet-ready worker; if that worker dies before ready
99+
* (fork ENOENT, syntax/import crash, abrupt exit) the fork-level `error` and
100+
* pre-ready `exit` paths must route the failure through the same receipt/lease
101+
* gate as a structured error, not reply out-of-band. */
102+
initTurnId?: string;
103+
initDispatchAttempt?: number;
97104
};
98105

99106
const __filename = fileURLToPath(import.meta.url);
@@ -1859,7 +1866,9 @@ export function forkWorker(ds: DaemonSession, prompt: string, resumeOrTurnId: bo
18591866
LARK_APP_SECRET: botCfg.larkAppSecret,
18601867
},
18611868
} as WindowsForkOptions);
1862-
const startupState: WorkerStartupState = { ready: false, failureNotified: false };
1869+
const startupState: WorkerStartupState = {
1870+
ready: false, failureNotified: false, initTurnId, initDispatchAttempt,
1871+
};
18631872

18641873
// A fork-level failure (spawn ENOENT, etc.) emits 'error'; without a handler
18651874
// the unhandled event crashes the daemon. It also happens before worker IPC
@@ -1869,6 +1878,16 @@ export function forkWorker(ds: DaemonSession, prompt: string, resumeOrTurnId: bo
18691878
logger.error(`[${t}] Worker fork error: ${reason}`);
18701879
if (startupState.failureNotified) return;
18711880
startupState.failureNotified = true;
1881+
// A durable VC meeting delivery fork failure is fenced to the receipt/lease
1882+
// chain (workerGeneration → ambiguous → retry); replying here would bypass
1883+
// that boundary and could post on a silent delivery.
1884+
if (ds.session.vcMeetingReceiver && initDispatchAttempt !== undefined) {
1885+
logger.info(
1886+
`[${t}] VC durable fork failure left to receipt/lease recovery `
1887+
+ `turn=${initTurnId?.slice(0, 12) ?? '-'} attempt=${initDispatchAttempt}: ${reason}`,
1888+
);
1889+
return;
1890+
}
18721891
const cliName = getCliDisplayName(agentCfg.cliId);
18731892
const message = tr('worker.start_failed', { cliName, reason }, botLocale(botCfg));
18741893
emitSessionLifecycleHook(ds, 'session.requires_attention', {
@@ -3001,7 +3020,10 @@ function setupWorkerHandlers(
30013020
// replacement kills are excluded to avoid noisy false alarms.
30023021
if (!startupState.ready && !startupState.failureNotified && !worker.killed && ds.session.status !== 'closed') {
30033022
const reason = tr('worker.start_exited_early', { code: code ?? 'null' }, loc);
3004-
void notifyStartupFailure(reason);
3023+
// Carry the frozen init attribution so an abrupt pre-ready exit of a
3024+
// durable VC delivery is fenced to the receipt/lease chain, not replied
3025+
// out-of-band (which could post on a silent delivery).
3026+
void notifyStartupFailure(reason, startupState.initTurnId, startupState.initDispatchAttempt);
30053027
}
30063028
// Clear the current child before notifying durable consumers. A callback
30073029
// may schedule a retry; it must not observe/send to this dead IPC channel.

src/worker.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7332,7 +7332,10 @@ process.on('message', async (raw: unknown) => {
73327332
try {
73337333
spawnCli({ ...lastInitConfig, resume: true, prompt: '' });
73347334
} catch (err) {
7335-
await sendFatalWorkerErrorAndExit(err, msg.turnId);
7335+
// Pass the message's own attempt (not the stale currentBotmux* from a
7336+
// prior IM turn) so a durable delivery relaunch failure carries the
7337+
// right attribution for the daemon's receipt/lease gate.
7338+
await sendFatalWorkerErrorAndExit(err, msg.turnId, msg.dispatchAttempt);
73367339
return;
73377340
}
73387341
}

test/session-lifecycle-start.test.ts

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,62 @@ describe('worker startup failure delivery', () => {
316316
expect(sessionReply).not.toHaveBeenCalled();
317317
});
318318

319+
it('leaves a durable VC delivery pre-ready worker exit to the receipt chain (no reply)', async () => {
320+
const sessionReply = vi.fn(async () => 'om_error_reply');
321+
initWorkerPool({ sessionReply, getSessionWorkingDir: () => '/repo', getActiveCount: () => 1, closeSession: vi.fn() });
322+
const ds = makeDs();
323+
(ds.session as unknown as { vcMeetingReceiver: unknown }).vcMeetingReceiver = {
324+
meetingId: 'm1', memberId: 'mem1', memberEpoch: 1,
325+
};
326+
// Dispatched (queued) into a worker that dies before ready — no structured
327+
// error precedes it, so the abrupt-exit guard must use the frozen init attempt.
328+
forkWorker(ds, 'deliver', { turnId: 'vc-delivery', dispatchAttempt: 3 });
329+
const worker = forkMock.mock.results.at(-1)!.value;
330+
331+
worker.emit('exit', 9);
332+
await Promise.resolve();
333+
await Promise.resolve();
334+
335+
expect(sessionReply).not.toHaveBeenCalled();
336+
});
337+
338+
it('leaves a durable VC delivery fork-level error to the receipt chain (no reply)', async () => {
339+
const sessionReply = vi.fn(async () => 'om_error_reply');
340+
initWorkerPool({ sessionReply, getSessionWorkingDir: () => '/repo', getActiveCount: () => 1, closeSession: vi.fn() });
341+
const ds = makeDs();
342+
(ds.session as unknown as { vcMeetingReceiver: unknown }).vcMeetingReceiver = {
343+
meetingId: 'm1', memberId: 'mem1', memberEpoch: 1,
344+
};
345+
forkWorker(ds, 'deliver', { turnId: 'vc-delivery', dispatchAttempt: 3 });
346+
const worker = forkMock.mock.results.at(-1)!.value;
347+
348+
// OS-level fork failure (e.g. spawn ENOENT) surfaces via the child 'error' event.
349+
worker.emit('error', new Error('spawn ENOENT'));
350+
await Promise.resolve();
351+
await Promise.resolve();
352+
353+
expect(sessionReply).not.toHaveBeenCalled();
354+
});
355+
356+
it('still surfaces a VC receiver IM-turn (no dispatchAttempt) fork error exactly once', async () => {
357+
const sessionReply = vi.fn(async () => 'om_error_reply');
358+
initWorkerPool({ sessionReply, getSessionWorkingDir: () => '/repo', getActiveCount: () => 1, closeSession: vi.fn() });
359+
const ds = makeDs();
360+
(ds.session as unknown as { vcMeetingReceiver: unknown }).vcMeetingReceiver = {
361+
meetingId: 'm1', memberId: 'mem1', memberEpoch: 1,
362+
};
363+
// A listener-group @agent IM turn has no durable dispatchAttempt; the gate is
364+
// precise, so its startup failure is still surfaced (not blanket-suppressed).
365+
forkWorker(ds, 'deliver', { turnId: 'im-turn' });
366+
const worker = forkMock.mock.results.at(-1)!.value;
367+
368+
worker.emit('error', new Error('spawn ENOENT'));
369+
await Promise.resolve();
370+
await Promise.resolve();
371+
372+
expect(sessionReply).toHaveBeenCalledTimes(1);
373+
});
374+
319375
it('posts a generic fallback when the worker exits before ready or structured error', async () => {
320376
const sessionReply = vi.fn(async () => 'om_error_reply');
321377
initWorkerPool({

test/worker-pipe-initial-screen-order.test.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,10 @@ describe('worker pipe initial screen ordering', () => {
186186
expect(guard).toContain('throw new Error(backendGateUserMessage(');
187187
expect(guard).not.toContain("effectiveBackend = 'pty'");
188188
expect(source).toContain('await sendAndFlush({');
189-
expect(source).toContain('await sendFatalWorkerErrorAndExit(err, msg.turnId)');
189+
// The crash-loop relaunch carries the message's own durable attempt so a
190+
// meeting delivery relaunch failure is attributed to the right receipt
191+
// (not the stale currentBotmux* from a prior IM turn).
192+
expect(source).toContain('await sendFatalWorkerErrorAndExit(err, msg.turnId, msg.dispatchAttempt)');
190193
expect(source).toContain('await sendFatalWorkerErrorAndExit(err);');
191194
});
192195

0 commit comments

Comments
 (0)