Merge pull request #381 from code-yeongyu/fix/eval-timeout-state-aware

fix(codemode): tell the truth about eval kernel state on interrupt

Author: code-yeongyu

packages/senpi-codemode/scripts/qa-timeout-state.ts

@@ -0,0 +1,62 @@
+import { startBridgeServer } from "../src/bridge/http-server.ts";
+import { createInterpreterDetector } from "../src/interpreters/detect.ts";
+import { PythonKernel } from "../src/kernels/py/kernel.ts";
+import { createEvalTool } from "../src/tool/eval-tool.ts";
+import type { ExtensionContext } from "@code-yeongyu/senpi";
+
+class QaScenarioError extends Error {
+	readonly name = "QaScenarioError";
+}
+
+const detected = await createInterpreterDetector().detect("py");
+if (!detected.ok) throw new QaScenarioError("no python interpreter");
+const server = await startBridgeServer({
+	onCall: async () => {
+		throw new QaScenarioError("no bridge tools in qa");
+	},
+	onEmit: async () => {},
+	onCompletion: async () => {
+		throw new QaScenarioError("no completion in qa");
+	},
+});
+try {
+	const kernel = await PythonKernel.start({
+		interpreterPath: detected.path,
+		sessionId: "qa-timeout-state-" + crypto.randomUUID(),
+		cwd: process.cwd(),
+		connection: { port: server.port, token: server.token },
+	});
+	try {
+		const tool = createEvalTool({
+			enabledLanguages: { js: false, py: true, rb: false, jl: false },
+			kernelManager: { getKernel: async () => kernel },
+			cellTimeoutSeconds: 1,
+			executeTool: (async () => {
+				throw new QaScenarioError("no executeTool in qa");
+			}) as never,
+		});
+		const ctx = { mode: "print" } as unknown as ExtensionContext;
+
+		// Cooperative interrupt: the runner answers SIGINT, so state must survive and
+		// the timeout message must say the kernel remains running.
+		const cooperative = await tool
+			.execute("qa-coop", { language: "py", code: "x=42\nimport time\ntime.sleep(30)", on_timeout: "error", timeout: 1 }, undefined, undefined, ctx)
+			.then(() => "UNEXPECTED-SUCCESS", (error: Error) => `${error.name}: ${error.message}`);
+		console.log(`COOPERATIVE_TIMEOUT: ${cooperative}`);
+
+		// Assert on the kernel's own readback so output-capture formatting cannot
+		// mask whether the interrupted kernel actually kept its variables.
+		const readback = await kernel.run({ cellId: "qa-readback", code: "x", timeoutMs: 5_000 });
+		console.log(`STATE_READBACK: ${JSON.stringify(readback)}`);
+
+		if (!cooperative.includes("TimeoutError")) throw new QaScenarioError(`expected TimeoutError: ${cooperative}`);
+		if (!/remains running|preserved/i.test(cooperative))
+			throw new QaScenarioError(`timeout message did not name preserved state: ${cooperative}`);
+		if (!readback.ok || readback.valueRepr !== "42")
+			throw new QaScenarioError(`python state did not survive timeout: ${JSON.stringify(readback)}`);
+	} finally {
+		await kernel.close();
+	}
+} finally {
+	await server.close();
+}

packages/senpi-codemode/src/kernels/js/context-manager.ts

@@ -1,4 +1,5 @@
 import type { HostToKernelMessage, KernelToHostMessage } from "../../bridge/protocol.ts";
+import type { KernelInterruptHandle } from "../../tool/types.ts";
 import { createInlineWorker, type WorkerLike } from "./inline-worker.ts";
 import {
 	assertJavaScriptKernelOpen,
@@ -48,14 +49,16 @@ export class JavaScriptKernel {
 		return await promise;
 	}
 
-	async interrupt(reason = "interrupted"): Promise<void> {
+	async interrupt(reason = "interrupted"): Promise<KernelInterruptHandle> {
 		assertJavaScriptKernelOpen(this.#lifecycle, "interrupt");
 		const active = this.#runs.active;
 		const target = this.#runs.takeInterruptTarget();
-		if (!target) return;
+		if (!target) return { stateRetained: Promise.resolve(true) };
 		if (target === active) this.#clearTimeout();
 		this.#runs.settle(target, stoppedResult(target.input.cellId, `JS cell interrupted: ${reason}`));
 		await this.#restartAfterStop();
+		// A restart always replaces the worker VM, so no user global survives.
+		return { stateRetained: Promise.resolve(false) };
 	}
 
 	async reset(): Promise<void> {

packages/senpi-codemode/src/kernels/py/kernel-contract.ts

@@ -29,4 +29,6 @@ export interface PendingRun {
 	timeoutTimer: NodeJS.Timeout | null;
 	escalationTimer?: NodeJS.Timeout;
 	interruptReason?: string;
+	/** Set while an interrupt outcome is pending; resolved once the kernel knows whether state survived. */
+	resolveStateRetained?: (retained: boolean) => void;
 }

packages/senpi-codemode/src/kernels/py/kernel.ts

@@ -1,3 +1,4 @@
+import type { KernelInterruptHandle } from "../../tool/types.ts";
 import type { PendingRun, PythonKernelRunOptions, PythonKernelStartOptions, ResultMessage } from "./kernel-contract.ts";
 import { failedPythonResult, PythonKernelTransport } from "./transport.ts";
 
@@ -41,23 +42,28 @@ export class PythonKernel {
 		});
 	}
 
-	async interrupt(reason = "interrupted"): Promise<void> {
+	async interrupt(reason = "interrupted"): Promise<KernelInterruptHandle> {
 		if (this.#failure) throw this.#failure;
 		for (const pending of [...this.#queue]) {
 			pending.interruptReason = reason;
 			this.#settleRun(pending, failedPythonResult(pending.input.cellId, "Eval interrupted"));
 		}
 		const active = this.#active;
 		const transport = this.#transport;
-		if (!active || !transport || active.interruptReason !== undefined) return;
+		if (!active || !transport || active.interruptReason !== undefined)
+			return { stateRetained: Promise.resolve(true) };
 		active.interruptReason = reason;
 		if (active.timeoutTimer) clearTimeout(active.timeoutTimer);
 		active.timeoutTimer = null;
 		active.escalationTimer = setTimeout(
 			() => void this.#escalateInterruptedRun(active).catch(() => undefined),
 			interruptEscalationMs,
 		);
+		const stateRetained = new Promise<boolean>((resolve) => {
+			active.resolveStateRetained = resolve;
+		});
 		transport.interrupt(reason);
+		return { stateRetained };
 	}
 
 	async reset(): Promise<void> {
@@ -187,13 +193,18 @@ export class PythonKernel {
 		if (this.#transport !== transport) return;
 		const pending = this.#pending.get(result.cellId);
 		if (pending) this.#settleRun(pending, result);
+		// A result frame from the live runner proves the process survived the interrupt.
+		if (pending?.resolveStateRetained) pending.resolveStateRetained(true);
 	}
 
 	#onExit(transport: PythonKernelTransport, error: Error): void {
 		if (this.#transport !== transport) return;
 		this.#transport = null;
 		const active = this.#active;
-		if (active) this.#settleRun(active, failedPythonResult(active.input.cellId, "Python kernel died", error.message));
+		if (active) {
+			if (active.resolveStateRetained) active.resolveStateRetained(false);
+			this.#settleRun(active, failedPythonResult(active.input.cellId, "Python kernel died", error.message));
+		}
 		this.#startNext();
 	}
 
@@ -209,7 +220,10 @@ export class PythonKernel {
 			},
 		);
 		const active = this.#active;
-		if (active) this.#settleRun(active, failedPythonResult(active.input.cellId, "Python kernel died", error.message));
+		if (active) {
+			if (active.resolveStateRetained) active.resolveStateRetained(false);
+			this.#settleRun(active, failedPythonResult(active.input.cellId, "Python kernel died", error.message));
+		}
 	}
 
 	#settleRun(pending: PendingRun, result: ResultMessage): void {
@@ -258,6 +272,7 @@ export class PythonKernel {
 	async #escalateInterruptedRun(pending: PendingRun): Promise<void> {
 		if (this.#active !== pending || pending.interruptReason === undefined) return;
 		const transport = this.#transport;
+		if (pending.resolveStateRetained) pending.resolveStateRetained(false);
 		if (transport) await this.#beginRetirement(transport);
 		if (this.#pending.has(pending.input.cellId))
 			this.#settleRun(pending, failedPythonResult(pending.input.cellId, "Eval interrupted"));

packages/senpi-codemode/src/kernels/py/prelude.py

@@ -12,6 +12,7 @@
 import locale
 import os
 import re
+import signal
 import subprocess
 import sys
 import time
@@ -886,6 +887,9 @@ def run_cell(cell_id: str, code: str) -> None:
     start = time.monotonic()
     stdout = io.StringIO()
     stderr = io.StringIO()
+    # SIGINT must interrupt user code here; the idle baseline (set between
+    # cells) ignores it so a late signal cannot kill the stdin-read loop.
+    signal.signal(signal.SIGINT, signal.default_int_handler)
     try:
         with contextlib.redirect_stdout(stdout), contextlib.redirect_stderr(stderr):
             body, expression = compile_cell(code)
@@ -914,6 +918,8 @@ def run_cell(cell_id: str, code: str) -> None:
                 "durationMs": elapsed(start),
             }
         )
+    finally:
+        signal.signal(signal.SIGINT, signal.SIG_IGN)
 
 
 def elapsed(start: float) -> int:
@@ -942,6 +948,7 @@ def handle(message: dict[str, Any]) -> bool:
 
 
 def main() -> None:
+    signal.signal(signal.SIGINT, signal.SIG_IGN)
     for raw in sys.stdin:
         try:
             if not handle(json.loads(raw)):

packages/senpi-codemode/src/kernels/shared/subprocess-kernel.ts

@@ -1,5 +1,6 @@
 import type { HostToKernelMessage, KernelToHostMessage } from "../../bridge/protocol.ts";
 import { decodeBridgeFrame, encodeBridgeFrame, isKernelToHostMessage } from "../../bridge/protocol.ts";
+import type { KernelInterruptHandle } from "../../tool/types.ts";
 import type { KernelResult, KernelRunInput, SubprocessKernelOptions, ToolCallMessage } from "./subprocess-contract.ts";
 import { type SubprocessLike, SubprocessProcess, type SubprocessSpawn, spawnSubprocess } from "./subprocess-process.ts";
 import { SubprocessRunQueue } from "./subprocess-queue.ts";
@@ -44,24 +45,26 @@ export class SubprocessKernel {
 		return run;
 	}
 
-	async interrupt(reason = "interrupted"): Promise<void> {
-		if (this.closed) return;
+	async interrupt(reason = "interrupted"): Promise<KernelInterruptHandle> {
+		if (this.closed) return { stateRetained: Promise.resolve(true) };
 		if (!this.runs.active) {
-			if (!this.retirementPromise) return;
+			if (!this.retirementPromise) return { stateRetained: Promise.resolve(true) };
 			const queued = this.runs.takeWaiting();
 			if (queued) this.runs.settle(queued, failureResult(queued, new CellInterruptedError(reason)));
-			return;
+			return { stateRetained: Promise.resolve(true) };
 		}
 		const process = this.process;
 		process?.retire();
 		this.runs.clearToolCalls();
 		const run = this.runs.active;
-		if (!run) return;
+		if (!run) return { stateRetained: Promise.resolve(true) };
 		this.runs.releaseActive(run);
 		this.runs.settle(run, failureResult(run, new CellInterruptedError(reason)));
 		const signal = globalThis.process.platform === "win32" ? "SIGTERM" : "SIGINT";
 		await this.restartProcess(process, signal, 5_000);
 		if (this.failure) throw this.failure;
+		// Restart always spawns a fresh interpreter, so no user global survives.
+		return { stateRetained: Promise.resolve(false) };
 	}
 
 	nextToolCall(): Promise<ToolCallMessage> {

packages/senpi-codemode/src/prompt/eval-prompt.ts

@@ -128,13 +128,13 @@ Fields:
 - \`reset\` (optional) — wipe this language's kernel first.{{#ifAll py js}} Per-language: a \`py\` reset never touches the JS VM.{{/ifAll}}
 - \`action\` (optional) — defaults to \`"run"\`. A detached cell returns its id: use \`eval({ action: "peek", cell_id })\` for buffered output/state or \`eval({ action: "stop", cell_id })\` to cancel it.
 
-A detached cell keeps its language kernel busy while it finishes. Do not re-run a detached cell: the same-language busy error names its cell id and output tail; another language can continue. Completion arrives as one notification with the final value/error and buffered output. Python stop interrupts while preserving kernel state; JavaScript stop restarts a fresh worker, so its VM state is lost.
+A detached cell keeps its language kernel busy while it finishes. Do not re-run a detached cell: the same-language busy error names its cell id and output tail; another language can continue. Completion arrives as one notification with the final value/error and buffered output. Stopping a cell interrupts its kernel; the stop result states whether kernel state survived or the kernel was restarted and its variables lost.
 
 {{#if py}}Live event loop: use top-level \`await\` directly; \`asyncio.run(…)\` raises "cannot be called from a running event loop".{{/if}}
 {{#if js}}JS runs under Node.js worker: top-level \`await\`/\`return\` work; \`fetch\`/\`Buffer\` available.{{/if}}
 {{#if rb}}Ruby: synchronous; helper options are keyword args{{#if spawns}} (e.g. \`output("id", limit: 2)\`){{/if}}; the last expression auto-displays unless it is \`nil\`, an assignment, or a definition (like IRB).{{/if}}
 {{#if jl}}Julia: synchronous; helper options are standard keyword args{{#if spawns}} (e.g. \`output("id", limit=2)\`){{/if}}; the last expression auto-displays unless it is an assignment or a definition (like the Julia REPL).{{/if}}
-On error, fix and re-run only the failing step — prior calls' state survives.
+On error, fix and re-run only the failing step. State usually survives a normal error, but a timeout or stop may have restarted the kernel — its message says which. Before rebuilding state, check a sentinel (a variable you defined earlier); only re-establish what is actually gone, since blind re-runs duplicate side effects.
 </instruction>
 
 <prelude>

packages/senpi-codemode/src/tool/detached-cell-manager.ts

@@ -15,6 +15,8 @@ type ManagedCell = {
 	canDetach: boolean;
 	wasDetached: boolean;
 	kernel: EvalKernel | undefined;
+	/** Set after an interrupt-driven stop; undefined until the kernel reports its fate. */
+	stateRetained: boolean | undefined;
 	outputTail: (() => string) | undefined;
 	result: AgentToolResult<EvalToolDetails> | undefined;
 	notificationQueued: boolean;
@@ -27,6 +29,7 @@ export interface EvalDetachedCellSnapshot {
 	readonly state: EvalDetachedCellState;
 	readonly outputTail: string;
 	readonly result: AgentToolResult<EvalToolDetails> | undefined;
+	readonly stateRetained: boolean | undefined;
 }
 
 export interface EvalDetachedCellNotification {
@@ -78,6 +81,7 @@ export class EvalDetachedCellManager {
 			canDetach: false,
 			wasDetached: false,
 			kernel: undefined,
+			stateRetained: undefined,
 			outputTail: undefined,
 			result: undefined,
 			notificationQueued: false,
@@ -116,7 +120,10 @@ export class EvalDetachedCellManager {
 		const cell = this.#get(cellId);
 		if (cell.state === "detached" && this.#transition(cell, "cancelled")) {
 			const kernel = cell.kernel;
-			if (kernel !== undefined) await kernel.interrupt(reason);
+			if (kernel !== undefined) {
+				const handle = await kernel.interrupt(reason);
+				cell.stateRetained = await handle.stateRetained;
+			}
 		}
 		return this.#snapshot(cell);
 	}
@@ -220,6 +227,7 @@ export class EvalDetachedCellManager {
 			state: cell.state,
 			outputTail: cell.outputTail?.() ?? "",
 			result: cell.result,
+			stateRetained: cell.stateRetained,
 		};
 	}
 }

packages/senpi-codemode/src/tool/eval-tool.ts

@@ -10,6 +10,7 @@ import { IdleTimeout, type IdleTimeoutOptions, type TimeoutPauseHandle } from ".
 import { CellHandler, type CellState } from "./cell-handler.ts";
 import { EvalDetachedCellManager, type EvalDetachedCellSnapshot } from "./detached-cell-manager.ts";
 import type { EvalImageResizer } from "./image.ts";
+import { describeTimeoutState, interruptionStateNote } from "./interrupt-note.ts";
 import {
 	createEvalInputSchema,
 	type EnabledEvalLanguages,
@@ -154,6 +155,9 @@ class CellExecution {
 		this.#abort(this.#callerSignal.reason);
 	};
 
+	/** Outcome of the most recent interrupt, when a kernel was interrupted. */
+	interruptStateRetained: Promise<boolean> | undefined;
+
 	#abort(reason: unknown): void {
 		if (!this.#active) return;
 		this.#active = false;
@@ -167,7 +171,12 @@ class CellExecution {
 		}
 		this.#interruptDeadline = setTimeout(() => this.#settleAbort(error), INTERRUPT_DELIVERY_GRACE_MS);
 		void Promise.resolve()
-			.then(async () => await kernel.interrupt(error.message))
+			.then(async () => {
+				const handle = await kernel.interrupt(error.message);
+				// Kernels predating the interrupt-outcome contract resolve void; leave
+				// the outcome undefined so callers report an honest unknown state.
+				this.interruptStateRetained = handle?.stateRetained;
+			})
 			.then(
 				() => this.#settleAbort(error),
 				(interruptError: unknown) => this.#settleAbort(interruptError),
@@ -361,6 +370,7 @@ async function executeCell(
 	} catch (error) {
 		if (handler && error instanceof Error && error.name === "CodemodeSessionDisposedError")
 			return await handler.finalizeCancellation(error);
+		if (error instanceof Error && error.name === "TimeoutError") throw await describeTimeoutState(error, execution);
 		throw error;
 	} finally {
 		state.active = false;
@@ -445,11 +455,7 @@ function detachedResult(snapshot: EvalDetachedCellSnapshot, input: EvalToolInput
 
 function snapshotResult(snapshot: EvalDetachedCellSnapshot): AgentToolResult<EvalToolDetails> {
 	const terminationNote =
-		snapshot.state === "cancelled" && snapshot.language === "js"
-			? "JavaScript worker was restarted; VM state was lost."
-			: snapshot.state === "cancelled" && snapshot.language === "py"
-				? "Python kernel was interrupted; its existing variables are preserved."
-				: undefined;
+		snapshot.state === "cancelled" ? interruptionStateNote(snapshot.language, snapshot.stateRetained) : undefined;
 	const text = [
 		`Eval cell ${snapshot.cellId} (${snapshot.language}) is ${snapshot.state}.`,
 		snapshot.outputTail.length === 0 ? "(no buffered output)" : snapshot.outputTail,