refactor(eval): changed timeout from inactivity to wall-clock budget

- Only bridge heartbeats (`agent()`/`llm()`) now re-arm the watchdog; compute, stdout, `log()`/`phase()`, and ordinary tool calls count against the budget.
- Emitted an immediate heartbeat at bridge call start to avoid early abort near budget edge.
- Removed `idle` flag and "of inactivity" suffix from timeout annotation strings.
- Updated docs, prompts, and comments to reflect the new wall-clock semantics.
This commit is contained in:
can1357
2026-06-01 17:16:42 +02:00
parent c7923e77eb
commit dbd9489010
11 changed files with 200 additions and 66 deletions
+3 -3
View File
@@ -166,9 +166,9 @@ Python prelude helpers include `agent(prompt, *, agent_type="task", model=None,
### Cell timeout
Each eval cell `timeout` is in seconds, defaults to 30, and is clamped to `1..600`. It is an **inactivity (idle) budget, not a hard wall-clock cap**: the watchdog (`IdleTimeout`, `src/eval/idle-timeout.ts`) only fires once the cell goes the full window with **no progress signal**. Every status event re-arms it — `agent()` progress snapshots, `log()`/`phase()`, and tool-bridge activity all count — so a long-running fanout that keeps reporting progress runs to completion instead of being killed mid-stream.
Each eval cell `timeout` is in seconds, defaults to 30, and is clamped to `1..600`. It is a **wall-clock budget on the cell's own work** that the watchdog (`IdleTimeout`, `src/eval/idle-timeout.ts`) enforces, **but it is paused while a host-side `agent()`/`parallel()`/`llm()` bridge call is in flight**: those calls pump a heartbeat (`withBridgeHeartbeat`, `src/eval/heartbeat.ts`) that re-arms the watchdog, so a long fanout or a slow completion runs to completion instead of being killed mid-stream.
Raw `stdout`/`stderr` does **not** re-arm the watchdog, so a pure-compute runaway loop with no progress reporting is still bounded by `timeout`. The tool combines the caller abort signal, the session abort signal, and the idle watchdog's signal with `AbortSignal.any(...)`; no wall-clock deadline is passed to the backend, so neither runtime arms a competing fixed timer.
The heartbeat is the **sole** signal that extends the budget. Everything else the cell does — compute, `stdout`/`stderr`, `log()`/`phase()`, and ordinary (non-agent) tool calls — counts against `timeout`, so a cell that is not delegating to an agent/llm is bounded by a plain wall-clock timeout. The tool combines the caller abort signal, the session abort signal, and the watchdog's signal with `AbortSignal.any(...)`; no wall-clock deadline is passed to the backend, so neither runtime arms a competing fixed timer.
### Kernel execution cancellation
@@ -176,7 +176,7 @@ On abort/timeout:
- The host sends `kill("SIGINT")` to the runner subprocess.
- The runner's exec-time signal handler raises `KeyboardInterrupt` inside the user code.
- Result includes `cancelled=true`; the timeout path annotates output as `Command timed out after <n> seconds of inactivity`.
- Result includes `cancelled=true`; the timeout path annotates output as `Command timed out after <n> seconds`.
- Between requests the runner installs `SIG_IGN` for SIGINT so a stray cancel does not tear down the kernel.
If a second cancel is required (runner stuck in C code), the host escalates to `SIGTERM` and the session restarts on the next call.
+1 -1
View File
@@ -8,7 +8,7 @@
### Fixed
- Fixed the `eval` tool aborting in-flight `agent()`/`parallel()` subagents and `llm()` requests by mistaking them for a stalled cell. The per-cell `timeout` is an *inactivity* budget that only re-arms on status events, but a host-side bridge call can legitimately run long stretches with no intermediate status (a subagent's time-to-first-token on a reasoning model, a long quiet nested tool, or an entire oneshot `llm()` request). Those calls now pump a lightweight heartbeat while they await, re-arming the idle watchdog through the existing status channel; the heartbeat is a pure keepalive and is never persisted or rendered, so a genuinely stalled cell is still interrupted once the call settles.
- Fixed the `eval` tool's per-cell `timeout` killing cells that were not stalled. The timeout is now a plain wall-clock budget on the cell's **own** work that is **paused only while a host-side `agent()`/`parallel()`/`llm()` bridge call is in flight** — those calls pump a heartbeat that re-arms the watchdog, so a long fanout or a slow (e.g. reasoning-tier) completion runs to completion instead of being aborted mid-flight (a subagent's time-to-first-token, a long quiet nested tool, or an entire oneshot `llm()` request no longer trip it). Nothing else re-arms the budget: ordinary compute, `print`/stdout, `log()`/`phase()`, and non-agent tool calls all count against it, so a cell that is not delegating to an agent/llm is bounded by the regular wall-clock timeout (and the timeout message no longer says "of inactivity"). The heartbeat is a pure keepalive — never persisted or rendered.
## [15.7.5] - 2026-06-01
### Fixed
@@ -10,7 +10,7 @@ import { AgentOutputManager } from "../../task/output-manager";
import type { AgentDefinition, AgentProgress, SingleResult } from "../../task/types";
import type { ToolSession } from "../../tools";
import { EVAL_AGENT_MAX_DEPTH, runEvalAgent } from "../agent-bridge";
import { setBridgeHeartbeatIntervalMs } from "../heartbeat";
import { EVAL_HEARTBEAT_OP, setBridgeHeartbeatIntervalMs } from "../heartbeat";
import { IdleTimeout } from "../idle-timeout";
import { disposeAllVmContexts } from "../js/context-manager";
import { executeJs } from "../js/executor";
@@ -451,14 +451,73 @@ describe("agent() through eval runtimes", () => {
});
// Mirror the eval tool's wiring: an IdleTimeout drives cancellation and
// every status event re-arms it.
// ONLY a bridge heartbeat re-arms it.
using idle = new IdleTimeout(60);
const result = await runEvalAgent(
{ prompt: "investigate" },
{ session, signal: idle.signal, emitStatus: () => idle.bump() },
{
session,
signal: idle.signal,
emitStatus: event => {
if (event.op === EVAL_HEARTBEAT_OP) idle.bump();
},
},
);
expect(idle.signal.aborted).toBe(false);
expect(result.text).toBe("done");
});
it("does not let agent() progress snapshots re-arm the watchdog without a heartbeat", async () => {
using tempDir = TempDir.createSync("@omp-eval-agent-progress-no-rearm-");
const { session } = makeEvalSession(tempDir, "js-agent-progress-no-rearm");
mockAgents();
// Heartbeat slower than the budget: only the immediate beat at call start
// fires, so after the budget elapses nothing re-arms the watchdog.
setBridgeHeartbeatIntervalMs(10_000);
// Stream frequent progress snapshots (op:"agent") for well past the budget.
// Progress is rendered but MUST NOT count as activity — only heartbeats do.
vi.spyOn(taskExecutor, "runSubprocess").mockImplementation(async options => {
for (let i = 0; i < 40; i++) {
options.onProgress?.({
index: options.index,
id: options.id,
agent: options.agent.name,
agentSource: options.agent.source,
status: "running",
task: options.task,
assignment: options.assignment,
description: options.description,
recentTools: [],
recentOutput: [],
toolCount: i,
tokens: 0,
cost: 0,
durationMs: i * 10,
});
await Bun.sleep(10);
}
return singleResult(options, { output: "done" });
});
const ops: string[] = [];
using idle = new IdleTimeout(80);
await runEvalAgent(
{ prompt: "investigate" },
{
session,
signal: idle.signal,
emitStatus: event => {
ops.push(event.op);
if (event.op === EVAL_HEARTBEAT_OP) idle.bump();
},
},
);
// Progress streamed, but the watchdog still fired: agent snapshots never
// re-armed it, and the lone start heartbeat lapsed before the call ended.
expect(ops).toContain("agent");
expect(idle.signal.aborted).toBe(true);
});
});
@@ -31,6 +31,24 @@ describe("withBridgeHeartbeat", () => {
expect(events.length).toBe(settledCount);
});
it("emits a heartbeat immediately so a bridge call extends the budget at once", async () => {
// Interval far longer than the operation: the only beat that can fire is
// the immediate one at call start. It must still reach the sink.
setBridgeHeartbeatIntervalMs(10_000);
const events: JsStatusEvent[] = [];
await withBridgeHeartbeat(
event => events.push(event),
async () => {
await Bun.sleep(30);
return "done";
},
);
expect(events.length).toBe(1);
expect(events[0]?.op).toBe(EVAL_HEARTBEAT_OP);
});
it("runs the operation without emitting when no status sink is wired", async () => {
setBridgeHeartbeatIntervalMs(5);
let ran = 0;
@@ -8,7 +8,7 @@ import type { ModelRegistry } from "../../config/model-registry";
import { Settings } from "../../config/settings";
import type { ToolSession } from "../../tools";
import { ToolError } from "../../tools/tool-errors";
import { setBridgeHeartbeatIntervalMs } from "../heartbeat";
import { EVAL_HEARTBEAT_OP, setBridgeHeartbeatIntervalMs } from "../heartbeat";
import { IdleTimeout } from "../idle-timeout";
import { disposeAllVmContexts } from "../js/context-manager";
import { executeJs } from "../js/executor";
@@ -230,7 +230,14 @@ describe("runEvalLlm", () => {
using idle = new IdleTimeout(60);
const result = await runEvalLlm(
{ prompt: "q", model: "smol" },
{ session: makeSession(), signal: idle.signal, emitStatus: () => idle.bump() },
{
session: makeSession(),
signal: idle.signal,
// Mirror the eval tool: only a bridge heartbeat re-arms the watchdog.
emitStatus: event => {
if (event.op === EVAL_HEARTBEAT_OP) idle.bump();
},
},
);
expect(idle.signal.aborted).toBe(false);
+24 -16
View File
@@ -1,22 +1,25 @@
/**
* Keepalive for in-flight host-side eval bridge calls.
*
* The eval idle watchdog ({@link ../tools/eval IdleTimeout}) treats a cell's
* `timeout` as an *inactivity* budget and only re-arms when a status event
* reaches it. Host-side bridge helpers — `agent()`/`parallel()` (via
* `runSubprocess`) and `llm()` (a single completion) — can legitimately run for
* long stretches with **no** intermediate status: a subagent's time-to-first
* token on a reasoning model, a long quiet nested tool, or the entire body of a
* oneshot `llm()` call. Without a keepalive the watchdog mistakes that work for
* a stall and aborts the cell mid-flight, killing the subagent.
* The eval watchdog ({@link ../tools/eval IdleTimeout}) caps a cell's `timeout`
* as a wall-clock budget on the cell's *own* work, but pauses that budget while
* a host-side `agent()`/`parallel()` (via `runSubprocess`) or `llm()` (a single
* completion) call is in flight. Those calls are the only thing that re-arms the
* watchdog — and they can run for long stretches with **no** status of their own
* (a subagent's time-to-first-token on a reasoning model, a long quiet nested
* tool, or the entire body of a oneshot `llm()` call). Without a keepalive the
* watchdog would mistake that delegated work for the cell stalling and abort it
* mid-flight, killing the subagent.
*
* {@link withBridgeHeartbeat} fixes that by pumping a synthetic
* {@link EVAL_HEARTBEAT_OP} status event on a fixed cadence while the wrapped
* operation is pending. The event rides the same `emitStatus → onStatus` channel
* both runtimes already forward, so it re-arms the watchdog without any new
* plumbing. Consumers MUST treat the heartbeat as a pure keepalive: bump the
* {@link withBridgeHeartbeat} bridges that gap by emitting a synthetic
* {@link EVAL_HEARTBEAT_OP} status event immediately when the call begins and
* then on a fixed cadence until it settles. The event rides the same
* `emitStatus → onStatus` channel both runtimes already forward, so it re-arms
* the watchdog without any new plumbing. The heartbeat is the *sole* signal that
* extends the budget: consumers MUST treat it as a pure keepalive — bump the
* watchdog and drop it (never persist or render it) — see the executor display
* sinks and the eval tool's `onStatus` handler.
* sinks and the eval tool's `onStatus` handler. Every other status event
* (compute helpers, `log()`/`phase()`, tool results) counts against the budget.
*/
import type { JsStatusEvent } from "./js/shared/types";
@@ -47,14 +50,19 @@ export function setBridgeHeartbeatIntervalMs(ms?: number): void {
/**
* Run {@link operation}, pumping {@link EVAL_HEARTBEAT_OP} status events through
* {@link emitStatus} on a fixed cadence until it settles. A no-op wrapper when
* no `emitStatus` sink is wired (the heartbeat would reach nobody).
* {@link emitStatus} — one immediately, then on a fixed cadence — until it
* settles. The immediate beat pauses the watchdog the instant the call begins,
* so a bridge call that starts close to the budget edge (after the cell already
* spent most of it computing) is not aborted before the first interval tick. A
* no-op wrapper when no `emitStatus` sink is wired (the heartbeat would reach
* nobody).
*/
export async function withBridgeHeartbeat<T>(
emitStatus: ((event: JsStatusEvent) => void) | undefined,
operation: () => Promise<T>,
): Promise<T> {
if (!emitStatus) return operation();
emitStatus({ op: EVAL_HEARTBEAT_OP });
const timer = setInterval(() => emitStatus({ op: EVAL_HEARTBEAT_OP }), heartbeatIntervalMs);
// Never keep the event loop alive for the heartbeat alone.
timer.unref?.();
@@ -60,11 +60,10 @@ function isTimeoutReason(reason: unknown): boolean {
);
}
function formatJsTimeoutAnnotation(timeoutMs: number | undefined, idle: boolean): string {
const suffix = idle ? " of inactivity" : "";
function formatJsTimeoutAnnotation(timeoutMs: number | undefined): string {
if (timeoutMs === undefined) return "Command timed out";
const secs = Math.max(1, Math.round(timeoutMs / 1000));
return `Command timed out after ${secs} seconds${suffix}`;
return `Command timed out after ${secs} seconds`;
}
export async function executeJs(code: string, options: JsExecutorOptions): Promise<JsResult> {
@@ -86,10 +85,9 @@ export async function executeJs(code: string, options: JsExecutorOptions): Promi
options.signal && timeoutSignal
? AbortSignal.any([options.signal, timeoutSignal])
: (options.signal ?? timeoutSignal);
// Idle mode: the eval tool drives cancellation via an idle-aware `signal` and
// passes only an inactivity budget. Use it for worker cold-start headroom and
// timeout-annotation text; never derive a competing fixed timer from it.
const idleMode = legacyTimeoutMs === undefined && options.idleTimeoutMs !== undefined;
// The eval tool drives cancellation via an idle-aware `signal` and passes only
// an inactivity budget; use it solely as worker cold-start headroom and never
// derive a competing fixed timer from it.
const acquireBudgetMs = legacyTimeoutMs ?? options.idleTimeoutMs;
try {
@@ -133,7 +131,7 @@ export async function executeJs(code: string, options: JsExecutorOptions): Promi
if (signal?.aborted || isAbortError(error)) {
const timedOut = Boolean(timeoutSignal?.aborted) || isTimeoutReason(options.signal?.reason);
if (timedOut) {
outputSink.push(formatJsTimeoutAnnotation(legacyTimeoutMs ?? options.idleTimeoutMs, idleMode));
outputSink.push(formatJsTimeoutAnnotation(legacyTimeoutMs ?? options.idleTimeoutMs));
}
const summary = await outputSink.dump();
return {
+7 -17
View File
@@ -232,20 +232,18 @@ async function waitForPromiseWithCancellation<T>(
// Result formatting
// ---------------------------------------------------------------------------
function formatTimeoutAnnotation(timeoutMs?: number, idle = false): string | undefined {
const suffix = idle ? " of inactivity" : "";
function formatTimeoutAnnotation(timeoutMs?: number): string | undefined {
if (timeoutMs === undefined) return "Command timed out";
const secs = Math.max(1, Math.round(timeoutMs / 1000));
return `Command timed out after ${secs} seconds${suffix}`;
return `Command timed out after ${secs} seconds`;
}
function formatKernelTimeoutAnnotation(timeoutMs: number | undefined, kernelKilled: boolean, idle = false): string {
function formatKernelTimeoutAnnotation(timeoutMs: number | undefined, kernelKilled: boolean): string {
const secs = timeoutMs === undefined ? undefined : Math.max(1, Math.round(timeoutMs / 1000));
const suffix = idle ? " of inactivity" : "";
if (kernelKilled) {
return `eval cell timed out${suffix} and the kernel was unresponsive to interrupt; the kernel has been killed and will be recreated on the next call.`;
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${suffix}`;
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.`;
}
@@ -489,10 +487,6 @@ async function executeWithKernel(
const displayOutputs: KernelDisplayOutput[] = [];
const deadlineMs = getExecutionDeadlineMs(options);
let executionTimeoutMs: number | undefined;
// Idle mode: the caller (eval tool) drives cancellation via an idle-aware
// signal and passes no wall-clock deadline, so annotate timeouts with the
// configured inactivity budget rather than a remaining-deadline figure.
const idleMode = deadlineMs === undefined && options?.idleTimeoutMs !== undefined;
// Collect every display output and, for status events, stream them live so
// long-running bridge helpers (e.g. `agent()`) surface progress mid-cell.
@@ -530,11 +524,7 @@ async function executeWithKernel(
if (result.cancelled) {
const annotation = result.timedOut
? formatKernelTimeoutAnnotation(
executionTimeoutMs ?? options?.idleTimeoutMs,
result.kernelKilled ?? false,
idleMode,
)
? formatKernelTimeoutAnnotation(executionTimeoutMs ?? options?.idleTimeoutMs, result.kernelKilled ?? false)
: undefined;
return {
exitCode: undefined,
@@ -572,7 +562,7 @@ async function executeWithKernel(
displayOutputs,
stdinRequested: false,
...(await sink.dump(
timedOut ? formatTimeoutAnnotation(executionTimeoutMs ?? options?.idleTimeoutMs, idleMode) : undefined,
timedOut ? formatTimeoutAnnotation(executionTimeoutMs ?? options?.idleTimeoutMs) : undefined,
)),
};
}
@@ -8,7 +8,7 @@ Cell fields:
- `language` — {{#if py}}`"py"` for the IPython kernel{{/if}}{{#ifAll py js}}, {{/ifAll}}{{#if js}}`"js"` for the persistent JavaScript VM{{/if}}.
- `code` — cell body, verbatim. Newlines, quotes, and indentation are JSON-encoded; no fences, no headers.
- `title` (optional) — short label shown in the transcript (e.g. `"imports"`, `"load config"`).
- `timeout` (optional) — per-cell **inactivity** budget in seconds (1-600). Default 30. The cell is interrupted only after this long with no progress, and every status event (`agent()` updates, `log()`/`phase()`, tool activity) resets the clock — so a long `agent()`/`parallel()` fanout that keeps reporting progress is not killed. Raw `print`/stdout does not reset it; raise `timeout` for a cell that runs long without emitting status.
- `timeout` (optional) — per-cell wall-clock budget in seconds (1-600). Default 30. It bounds the cell's **own** work, but is paused while an `agent()`/`parallel()`/`llm()` call is in flight — so a long fanout or a slow completion runs to completion, while the cell itself is still bounded. Compute, `print`/stdout, `log()`/`phase()`, and ordinary tool calls all count against the budget; raise `timeout` for a cell that does heavy local work or long non-agent tool calls.
- `reset` (optional) — wipe this cell's language kernel before running.{{#ifAll py js}} Reset is per-language: a `py` cell's reset does not touch the JavaScript VM and vice versa.{{/ifAll}}
**Work incrementally:**
+19 -14
View File
@@ -348,15 +348,16 @@ export class EvalTool implements AgentTool<typeof evalSchema> {
for (let i = 0; i < cells.length; i++) {
const cell = cells[i];
const backend = cell.resolved.backend;
// The per-cell `timeout` is an *inactivity* budget, not a hard
// wall-clock cap: it bounds the gap between progress signals
// (status events — agent() updates, log()/phase(), tool-bridge
// activity), so a long fanout that keeps reporting progress runs to
// completion while a genuinely stalled cell (no progress for the
// whole window) is still interrupted. Raw stdout deliberately does
// NOT re-arm it, so pure-compute runaway loops stay bounded. The
// watchdog drives `combinedSignal`; we pass no wall-clock deadline
// downstream so the backends never arm a competing fixed timer.
// The per-cell `timeout` is a wall-clock budget on the cell's *own*
// work, but it is paused while a host-side `agent()`/`llm()` bridge
// call is in flight: those calls pump a heartbeat (see
// `withBridgeHeartbeat`) that re-arms the watchdog, so a long fanout
// or a slow completion runs to completion. Nothing else re-arms it —
// compute, stdout, `log()`/`phase()`, and ordinary tool calls all
// count against the budget — so a cell that is not delegating to an
// agent/llm is bounded by a plain wall-clock timeout. The watchdog
// drives `combinedSignal`; we pass no wall-clock deadline downstream
// so the backends never arm a competing fixed timer.
const idleTimeoutMs = timeoutSecondsFromMs(cell.timeoutMs) * 1000;
const idle = new IdleTimeout(idleTimeoutMs);
const combinedSignal = signal
@@ -389,12 +390,16 @@ export class EvalTool implements AgentTool<typeof evalSchema> {
outputSink!.push(chunk);
},
onStatus: event => {
// Every status event re-arms the inactivity watchdog. A
// heartbeat is a pure keepalive emitted while a host-side
// bridge call (agent()/llm()) runs: it bumps the timer but
// carries no payload, so don't persist or render it.
// Only a bridge heartbeat re-arms the watchdog: it is the
// keepalive `agent()`/`llm()` pump while a host-side call is
// in flight, so those calls effectively pause the budget. It
// carries no payload — bump and drop it. Every other event
// (compute helpers, log()/phase(), tool results) renders but
// counts against the plain wall-clock budget.
if (event.op === EVAL_HEARTBEAT_OP) {
idle.bump();
if (event.op === EVAL_HEARTBEAT_OP) return;
return;
}
cellResult.statusEvents ??= [];
upsertStatusEvent(cellResult.statusEvents, event);
pushUpdate();
@@ -0,0 +1,49 @@
import { afterAll, describe, expect, it } from "bun:test";
import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
import { disposeAllVmContexts } from "@oh-my-pi/pi-coding-agent/eval/js/context-manager";
import type { ToolSession } from "@oh-my-pi/pi-coding-agent/tools";
import { EvalTool } from "@oh-my-pi/pi-coding-agent/tools/eval";
function makeSession(): ToolSession {
return {
cwd: process.cwd(),
hasUI: false,
getSessionFile: () => null,
getSessionSpawns: () => null,
settings: Settings.isolated(),
} as unknown as ToolSession;
}
/**
* Defends the contract that a cell which does not delegate to an `agent()`/
* `llm()` bridge call is bounded by a *plain wall-clock* timeout — not the
* activity watchdog, which now only extends the budget while a bridge call is in
* flight. Regression guard for the watchdog killing ordinary compute cells and
* surfacing a misleading "of inactivity" message.
*/
describe("EvalTool timeout semantics", () => {
afterAll(async () => {
await disposeAllVmContexts();
});
it("bounds a compute cell (no agent/llm) by a plain wall-clock timeout", async () => {
const tool = new EvalTool(makeSession());
// 1s budget; the cell idles for 5s and emits no status, so nothing extends
// the budget — it must be cut off at the wall-clock limit.
const result = await tool.execute("call-compute-timeout", {
cells: [{ language: "js", code: "await Bun.sleep(5000); return 'never';", timeout: 1 }],
});
const text = result.content
.filter((block): block is { type: "text"; text: string } => block.type === "text")
.map(block => block.text)
.join("\n");
expect(text).toContain("timed out after 1 seconds");
// The new wording is a plain wall-clock timeout, not an inactivity stall.
expect(text).not.toContain("inactivity");
expect(text).not.toContain("never");
const cell = result.details?.cells?.[0];
expect(cell?.exitCode).toBeUndefined();
});
});