diff --git a/packages/coding-agent/src/eval/__tests__/bridge-timeout.test.ts b/packages/coding-agent/src/eval/__tests__/bridge-timeout.test.ts index 6589cbfd6..aa6fb9be0 100644 --- a/packages/coding-agent/src/eval/__tests__/bridge-timeout.test.ts +++ b/packages/coding-agent/src/eval/__tests__/bridge-timeout.test.ts @@ -19,10 +19,12 @@ describe("withBridgeTimeoutPause", () => { await Bun.sleep(80); return "done"; }, + { deferExternalAbort: true }, ); expect(value).toBe("done"); expect(events.map(event => event.op)).toEqual([EVAL_TIMEOUT_PAUSE_OP, EVAL_TIMEOUT_RESUME_OP]); + expect(events.every(event => event.deferExternalAbort === true)).toBe(true); const settledCount = events.length; await Bun.sleep(40); @@ -75,27 +77,26 @@ class TestCancelledError extends Error { } } -it("defers external aborts until an in-flight bridge call resumes", async () => { +it("defers external aborts until an in-flight agent bridge call resumes", async () => { const abortController = new AbortController(); + const entered = Promise.withResolvers(); + const triggerAbort = Promise.withResolvers(); + const observed = Promise.withResolvers(); const release = Promise.withResolvers(); - let completed = false; - let kernelSignal: AbortSignal | undefined; const kernel: GenericKernel> = { async execute(_code, options) { - kernelSignal = options.signal; + entered.resolve(); + await triggerAbort.promise; options.onDisplay({ type: "status", - event: { op: EVAL_TIMEOUT_PAUSE_OP }, + event: { op: EVAL_TIMEOUT_PAUSE_OP, deferExternalAbort: true }, } satisfies KernelDisplayOutput); - abortController.abort(new Error("external interrupt")); - expect(kernelSignal?.aborted).toBe(false); - + observed.resolve(options.signal?.aborted ?? false); await release.promise; - completed = true; options.onDisplay({ type: "status", - event: { op: EVAL_TIMEOUT_RESUME_OP }, + event: { op: EVAL_TIMEOUT_RESUME_OP, deferExternalAbort: true }, } satisfies KernelDisplayOutput); return { status: "ok", cancelled: false, timedOut: false }; }, @@ -113,12 +114,57 @@ it("defers external aborts until an in-flight bridge call resumes", async () => formatTimeoutAnnotation: () => "timed out", }); - await Promise.resolve(); - expect(completed).toBe(false); - + await entered.promise; + triggerAbort.resolve(); + expect(await observed.promise).toBe(false); + release.resolve(); + const result = await resultPromise; + expect(result.cancelled).toBe(true); + expect(result.exitCode).toBeUndefined(); +}); + +it("does not defer external aborts for a completion bridge call", async () => { + const abortController = new AbortController(); + const entered = Promise.withResolvers(); + const triggerAbort = Promise.withResolvers(); + const observed = Promise.withResolvers(); + const release = Promise.withResolvers(); + const kernel: GenericKernel> = { + async execute(_code, options) { + entered.resolve(); + await triggerAbort.promise; + options.onDisplay({ + type: "status", + event: { op: EVAL_TIMEOUT_PAUSE_OP }, + } satisfies KernelDisplayOutput); + abortController.abort(new Error("external interrupt")); + observed.resolve(options.signal?.aborted ?? false); + await release.promise; + options.onDisplay({ + type: "status", + event: { op: EVAL_TIMEOUT_RESUME_OP }, + } satisfies KernelDisplayOutput); + return { status: "ok", cancelled: false, timedOut: false }; + }, + }; + + const resultPromise = executeWithKernelBase({ + kernel, + code: "completion('slow')", + options: { signal: abortController.signal }, + runIdPrefix: "test", + errorLogLabel: "test", + cancelledErrorClass: TestCancelledError, + buildKernelEnvPatch: () => ({}), + formatKernelTimeoutAnnotation: () => "kernel timed out", + formatTimeoutAnnotation: () => "timed out", + }); + + await entered.promise; + triggerAbort.resolve(); + expect(await observed.promise).toBe(true); release.resolve(); const result = await resultPromise; - expect(completed).toBe(true); expect(result.cancelled).toBe(true); expect(result.exitCode).toBeUndefined(); }); diff --git a/packages/coding-agent/src/eval/agent-bridge.ts b/packages/coding-agent/src/eval/agent-bridge.ts index 4d98fd898..43ab2d3d4 100644 --- a/packages/coding-agent/src/eval/agent-bridge.ts +++ b/packages/coding-agent/src/eval/agent-bridge.ts @@ -572,7 +572,7 @@ export async function runEvalAgent(args: unknown, options: EvalAgentBridgeOption options.session.recordEvalSubagentUsage?.(result.usage?.output ?? 0); return { result, mergeSummary, changesApplied }; - }); + }, { deferExternalAbort: true }); return { text: structured ? result.output : result.output + mergeSummary, diff --git a/packages/coding-agent/src/eval/bridge-timeout.ts b/packages/coding-agent/src/eval/bridge-timeout.ts index 90907b0e1..cdb714eb6 100644 --- a/packages/coding-agent/src/eval/bridge-timeout.ts +++ b/packages/coding-agent/src/eval/bridge-timeout.ts @@ -26,6 +26,15 @@ export function isEvalTimeoutControlEvent(event: JsStatusEvent): boolean { return event.op === EVAL_TIMEOUT_PAUSE_OP || event.op === EVAL_TIMEOUT_RESUME_OP; } +/** Optional behavior for a timeout pause around a host bridge call. */ +export interface BridgeTimeoutPauseOptions { + /** + * Marks the pause as an `agent()` call whose already-started work must finish + * before an external eval abort reaches the kernel. + */ + deferExternalAbort?: boolean; +} + /** * Run {@link operation} while suspending the eval watchdog through * {@link emitStatus}. A no-op wrapper when no status sink is wired. @@ -33,12 +42,21 @@ export function isEvalTimeoutControlEvent(event: JsStatusEvent): boolean { export async function withBridgeTimeoutPause( emitStatus: ((event: JsStatusEvent) => void) | undefined, operation: () => Promise, + options?: BridgeTimeoutPauseOptions, ): Promise { if (!emitStatus) return operation(); - emitStatus({ op: EVAL_TIMEOUT_PAUSE_OP }); + emitStatus( + options?.deferExternalAbort + ? { op: EVAL_TIMEOUT_PAUSE_OP, deferExternalAbort: true } + : { op: EVAL_TIMEOUT_PAUSE_OP }, + ); try { return await operation(); } finally { - emitStatus({ op: EVAL_TIMEOUT_RESUME_OP }); + emitStatus( + options?.deferExternalAbort + ? { op: EVAL_TIMEOUT_RESUME_OP, deferExternalAbort: true } + : { op: EVAL_TIMEOUT_RESUME_OP }, + ); } } diff --git a/packages/coding-agent/src/eval/executor-base.ts b/packages/coding-agent/src/eval/executor-base.ts index 82848eb67..edccaec61 100644 --- a/packages/coding-agent/src/eval/executor-base.ts +++ b/packages/coding-agent/src/eval/executor-base.ts @@ -198,6 +198,7 @@ function createBridgeAbortShield(source: AbortSignal | undefined): BridgeAbortSh shield.signal = controller.signal; shield.handleStatus = (event: JsStatusEvent): void => { + if (event.deferExternalAbort !== true) return; if (event.op === EVAL_TIMEOUT_PAUSE_OP) { pauseDepth++; return;