From 027dbb49005ca9ed711a94a2ffb07e7a0d26caf4 Mon Sep 17 00:00:00 2001 From: can1357 Date: Thu, 21 May 2026 14:51:21 +0900 Subject: [PATCH] fix(coding-agent): interrupt cell on timeout instead of killing kernel A per-cell timeout used to kill the persistent Python kernel, losing all session state. The kernel.ts timeout path now sends SIGINT first (letting the cell raise KeyboardInterrupt) and only escalates to shutdown after a 5s grace window if the interrupt is ignored. KernelExecuteResult gains an optional kernelKilled flag (defaulting to false, propagated by the escalation timer and the unexpected-exit handler). executor.ts formats two distinct timeout annotations: one that says the kernel is still alive and reset:true would clear state, one that says the kernel was killed and will be recreated. --- packages/coding-agent/src/eval/py/executor.ts | 13 +++++++- packages/coding-agent/src/eval/py/kernel.ts | 32 ++++++++++++++----- .../test/core/python-executor-mapping.test.ts | 2 +- .../core/python-executor-per-call.test.ts | 2 +- .../core/python-executor-streaming.test.ts | 2 +- .../test/core/python-executor-timeout.test.ts | 2 +- .../test/core/python-executor.result.test.ts | 2 +- .../test/core/python-executor.test.ts | 2 +- 8 files changed, 42 insertions(+), 15 deletions(-) diff --git a/packages/coding-agent/src/eval/py/executor.ts b/packages/coding-agent/src/eval/py/executor.ts index a33cd5040..42ec13b7f 100644 --- a/packages/coding-agent/src/eval/py/executor.ts +++ b/packages/coding-agent/src/eval/py/executor.ts @@ -209,6 +209,15 @@ function formatTimeoutAnnotation(timeoutMs?: number): string | undefined { return `Command timed out after ${secs} seconds`; } +function formatKernelTimeoutAnnotation(timeoutMs: number | undefined, kernelKilled: boolean): string { + const secs = timeoutMs === undefined ? undefined : Math.max(1, Math.round(timeoutMs / 1000)); + if (kernelKilled) { + return "eval cell timed out and the kernel was unresponsive to interrupt; the kernel has been killed and will be recreated on the next call."; + } + const duration = secs === undefined ? "the configured timeout" : `${secs}s`; + return `eval cell timed out after ${duration}; kernel interrupted but remains running. Reset the kernel via { reset: true } if state appears corrupted.`; +} + function createCancelledPythonResult(timedOut: boolean, timeoutMs?: number): PythonResult { const output = timedOut ? (formatTimeoutAnnotation(timeoutMs) ?? "Command timed out") : ""; const outputBytes = Buffer.byteLength(output, "utf-8"); @@ -434,7 +443,9 @@ async function executeWithKernel( }); if (result.cancelled) { - const annotation = result.timedOut ? formatTimeoutAnnotation(executionTimeoutMs) : undefined; + const annotation = result.timedOut + ? formatKernelTimeoutAnnotation(executionTimeoutMs, result.kernelKilled ?? false) + : undefined; return { exitCode: undefined, cancelled: true, diff --git a/packages/coding-agent/src/eval/py/kernel.ts b/packages/coding-agent/src/eval/py/kernel.ts index cdeee7722..07d2eb223 100644 --- a/packages/coding-agent/src/eval/py/kernel.ts +++ b/packages/coding-agent/src/eval/py/kernel.ts @@ -46,8 +46,10 @@ const STARTUP_TIMEOUT_MS = 10_000; // How long to wait after SIGINT for the runner to emit `done`. If the cell is // stuck in code that ignores Python signals (e.g. a C extension holding the // GIL), we escalate to a full subprocess shutdown so the host queue unblocks -// instead of hanging the session forever. -const INTERRUPT_ESCALATION_MS = 2_000; +// instead of hanging the session forever. The grace window is intentionally +// generous: a clean interrupt is far preferable to losing the persistent +// kernel's state, so we only kill as a last-resort recovery path. +const INTERRUPT_ESCALATION_MS = 5_000; export interface KernelExecuteOptions { signal?: AbortSignal; @@ -66,6 +68,12 @@ export interface KernelExecuteResult { cancelled: boolean; timedOut: boolean; stdinRequested: boolean; + /** + * True when the kernel subprocess was killed as part of settling this + * execution (e.g. SIGINT was ignored and we escalated to shutdown, or the + * kernel died unexpectedly). When false, the kernel remains reusable. + */ + kernelKilled?: boolean; } export interface KernelShutdownResult { @@ -162,6 +170,7 @@ interface PendingExecution { cancelled: boolean; timedOut: boolean; stdinRequested: boolean; + kernelKilled: boolean; settled: boolean; escalationTimer?: NodeJS.Timeout; } @@ -222,7 +231,7 @@ export class PythonKernel { kernel.#exitedPromise = proc.exited; void kernel.#exitedPromise.then(code => { kernel.#alive = false; - kernel.#abortPendingExecutions(`Python kernel exited with code ${code}`); + kernel.#abortPendingExecutions(`Python kernel exited with code ${code}`, { kernelKilled: true }); }); kernel.#startReader(proc.stdout as ReadableStream); @@ -261,6 +270,7 @@ export class PythonKernel { timedOut: false, stdinRequested: false, settled: false, + kernelKilled: false, }; this.#pending.set(msgId, pending); @@ -276,6 +286,7 @@ export class PythonKernel { cancelled: pending.cancelled, timedOut: pending.timedOut, stdinRequested: pending.stdinRequested, + kernelKilled: pending.kernelKilled, }); }; @@ -287,9 +298,12 @@ export class PythonKernel { logger.warn("Python runner did not respond to SIGINT; terminating subprocess", { kernelId: this.id, }); - // `shutdown()` aborts pending executions immediately and escalates to - // SIGTERM/SIGKILL, so the host queue unblocks even if the runner is - // stuck in a non-interruptible state. + // SIGINT was ignored; mark the cell as kernel-killed so callers can + // surface the harsher recovery message. `shutdown()` aborts pending + // executions immediately and escalates to SIGTERM/SIGKILL, so the + // host queue unblocks even if the runner is stuck in a + // non-interruptible state. + pending.kernelKilled = true; void this.shutdown(); }, INTERRUPT_ESCALATION_MS); escalation.unref?.(); @@ -363,7 +377,7 @@ export class PythonKernel { if (this.#shutdownConfirmed) return { confirmed: true }; this.#alive = false; - this.#abortPendingExecutions("Python kernel shutdown"); + this.#abortPendingExecutions("Python kernel shutdown", { kernelKilled: true }); const timeoutMs = options?.timeoutMs ?? SHUTDOWN_GRACE_MS; const proc = this.#proc; @@ -410,10 +424,11 @@ export class PythonKernel { return { confirmed }; } - #abortPendingExecutions(reason: string): void { + #abortPendingExecutions(reason: string, options?: { kernelKilled?: boolean }): void { if (this.#pending.size === 0) return; const pending = Array.from(this.#pending.values()); this.#pending.clear(); + const kernelKilledDefault = options?.kernelKilled ?? false; for (const entry of pending) { if (entry.settled) continue; entry.settled = true; @@ -425,6 +440,7 @@ export class PythonKernel { stdinRequested: entry.stdinRequested, executionCount: entry.executionCount, error: entry.error, + kernelKilled: entry.kernelKilled || kernelKilledDefault, }); } } diff --git a/packages/coding-agent/test/core/python-executor-mapping.test.ts b/packages/coding-agent/test/core/python-executor-mapping.test.ts index cc04c4f26..941baab99 100644 --- a/packages/coding-agent/test/core/python-executor-mapping.test.ts +++ b/packages/coding-agent/test/core/python-executor-mapping.test.ts @@ -21,7 +21,7 @@ describe("executePythonWithKernel mapping", () => { expect(result.cancelled).toBe(true); expect(result.exitCode).toBeUndefined(); - expect(result.output).toContain("Command timed out after 5 seconds"); + expect(result.output).toContain("eval cell timed out after 5s"); }); it("maps error status to non-zero exit code", async () => { diff --git a/packages/coding-agent/test/core/python-executor-per-call.test.ts b/packages/coding-agent/test/core/python-executor-per-call.test.ts index 1ea3ee5cc..14fb234d9 100644 --- a/packages/coding-agent/test/core/python-executor-per-call.test.ts +++ b/packages/coding-agent/test/core/python-executor-per-call.test.ts @@ -144,7 +144,7 @@ describe("executePython (per-call)", () => { expect(result.cancelled).toBe(true); expect(result.exitCode).toBeUndefined(); - expect(result.output).toContain("Command timed out after 2 seconds"); + expect(result.output).toContain("eval cell timed out after 2s"); expect(shutdownCalls).toBe(1); }); }); diff --git a/packages/coding-agent/test/core/python-executor-streaming.test.ts b/packages/coding-agent/test/core/python-executor-streaming.test.ts index 2a64c3e19..c31695500 100644 --- a/packages/coding-agent/test/core/python-executor-streaming.test.ts +++ b/packages/coding-agent/test/core/python-executor-streaming.test.ts @@ -25,7 +25,7 @@ describe("executePythonWithKernel streaming", () => { expect(result.cancelled).toBe(true); expect(result.exitCode).toBeUndefined(); - expect(result.output).toContain("Command timed out after 2 seconds"); + expect(result.output).toContain("eval cell timed out after 2s"); }); it("sanitizes ANSI and carriage returns", async () => { diff --git a/packages/coding-agent/test/core/python-executor-timeout.test.ts b/packages/coding-agent/test/core/python-executor-timeout.test.ts index 358d2f8cc..de8603969 100644 --- a/packages/coding-agent/test/core/python-executor-timeout.test.ts +++ b/packages/coding-agent/test/core/python-executor-timeout.test.ts @@ -30,6 +30,6 @@ describe("executePythonWithKernel cancellation", () => { expect(result.cancelled).toBe(true); expect(result.exitCode).toBeUndefined(); - expect(result.output).toContain("Command timed out after 5 seconds"); + expect(result.output).toContain("eval cell timed out after 5s"); }); }); diff --git a/packages/coding-agent/test/core/python-executor.result.test.ts b/packages/coding-agent/test/core/python-executor.result.test.ts index c8d263315..fbaf54b32 100644 --- a/packages/coding-agent/test/core/python-executor.result.test.ts +++ b/packages/coding-agent/test/core/python-executor.result.test.ts @@ -30,7 +30,7 @@ describe("executePythonWithKernel result mapping", () => { expect(result.exitCode).toBeUndefined(); expect(result.cancelled).toBe(true); - expect(result.output).toContain("Command timed out after 5 seconds"); + expect(result.output).toContain("eval cell timed out after 5s"); }); it("maps kernel error status to exit code 1", async () => { diff --git a/packages/coding-agent/test/core/python-executor.test.ts b/packages/coding-agent/test/core/python-executor.test.ts index 76d4463a8..8f3a81c86 100644 --- a/packages/coding-agent/test/core/python-executor.test.ts +++ b/packages/coding-agent/test/core/python-executor.test.ts @@ -75,7 +75,7 @@ describe("executePythonWithKernel", () => { expect(result.exitCode).toBeUndefined(); expect(result.cancelled).toBe(true); - expect(result.output).toContain("Command timed out after 4 seconds"); + expect(result.output).toContain("eval cell timed out after 4s"); }); it("returns cancelled result without timeout annotation", async () => {