feat(runtime): stop a turn on the clock as well as the step count

commands/agent.tsx

@@ -11,7 +11,7 @@ import { registerCommands } from "./slash";
 import { isCommandTool, summarizeToolOutput } from "../tui/src/tool-display";
 import { parseApprovalMode } from "../runtime/approval";
 import { createEventLog, now } from "../runtime/eventLog";
-import { IterationBudgetExhaustedError } from "../runtime/loop";
+import { BudgetExhaustedError } from "../runtime/loop";
 import { enableSandbox } from "../runtime/sandbox";
 import { VERSION } from "../config/version";
 import { PassThrough } from "stream";
@@ -297,7 +297,7 @@ async function runHeadless(
     },
     onError(error) {
       failed = true;
-      if (error instanceof IterationBudgetExhaustedError) budgetExhausted = true;
+      if (error instanceof BudgetExhaustedError) budgetExhausted = true;
       log.write({ type: "error", ts: now(), message: error.message });
       process.stderr.write(`✖ ${error.message}\n`);
     },
@@ -320,7 +320,7 @@ async function runHeadless(
     await controller.run(prompt);
   } catch (error) {
     failed = true;
-    if (error instanceof IterationBudgetExhaustedError) budgetExhausted = true;
+    if (error instanceof BudgetExhaustedError) budgetExhausted = true;
     const message = error instanceof Error ? error.message : String(error);
     log.write({ type: "error", ts: now(), message });
     process.stderr.write(`✖ ${message}\n`);
@@ -333,8 +333,11 @@ async function runHeadless(
   process.stdout.write("\n");
   // Exit codes are a contract with automated callers:
   //   0 - the turn completed
-  //   2 - the loop ran out of iterations; work may be partially done, and the
-  //       caller should judge the result rather than treat this as a crash
+  //   2 - the loop ran out of budget, of iterations or of wall-clock time; work
+  //       may be partially done, and the caller should judge the result rather
+  //       than treat this as a crash. One code for both: a distinct one for the
+  //       deadline would be booked as an exception by any harness not yet
+  //       updated to know it, dropping those trials from the mean.
   //   1 - anything else went wrong
   process.exit(failed ? (budgetExhausted ? EXIT_BUDGET_EXHAUSTED : 1) : 0);
 }

config/types.ts

@@ -291,6 +291,11 @@ export interface AgentCallbacks {
    * nobody is there to ask, so the loop raises `IterationBudgetExhaustedError`
    * as it always has. Headless runs deliberately do not implement it, which is
    * what keeps their exit-code contract.
+   *
+   * The step ceiling is the only budget that asks. A wall-clock deadline
+   * (`WOOPCODE_MAX_WALL_SEC`) never consults this and raises
+   * `WallBudgetExhaustedError` directly: iterations do not tick while a human
+   * thinks about the question, and a clock does.
    */
   onBudgetExhausted?(info: { steps: number }): Promise<BudgetDecision>;
   /** Reported once per completed iteration, before the next one starts. */

docs/adr/0001-wall-clock-budget-for-the-agent-loop.md

@@ -0,0 +1,108 @@
+---
+title: Wall-clock budget for the agent loop
+type: concept
+summary: Why the loop measures a second budget in seconds, what the reserve holds back, and which alternatives were rejected.
+prerequisites: []
+related:
+  - /docs/reference/configuration
+since: 1.1.0
+---
+
+# Wall-clock budget for the agent loop
+
+Status: accepted
+
+The loop has only ever measured its budget in iterations. Harbor enforces a
+wall clock. Running the checked-in five-task config with
+`WOOPCODE_MAX_ITERATIONS=200`, every trial finished having spent between 2% and
+23% of the time it was given, and `make-mips-interpreter` was killed by its own
+200th iteration at 406s of 1800 — mid-work, with `exception_info: null` proving
+Harbor's timeout never fired. So the loop gets a second budget,
+`WOOPCODE_MAX_WALL_SEC`, and stops on whichever of the two binds first.
+
+## The measurement
+
+Per-task agent timeouts come from each task package's `task.toml`
+(`~/.cache/harbor/tasks/packages/terminal-bench/<task>/<digest>/task.toml`).
+Against the `jobs/tb2-post-1.1` run:
+
+| task | timeout | wall used | unused | iterations | what stopped it |
+| --- | --- | --- | --- | --- | --- |
+| build-pov-ray | 12000s | 287s | 98% | 79 | model chose to |
+| circuit-fibsqrt | 3600s | 400s | 89% | 165 | model chose to |
+| make-mips-interpreter | 1800s | 406s | **77%** | **200** | **our ceiling** |
+| overfull-hbox | 750s | 192s | 74% | 59 | model chose to |
+| video-processing | 3600s | 261s | 93% | 83 | model chose to |
+
+CLAUDE.md's benchmarking section records the opposite rule — *"Wall clock is the
+binding budget, not iterations"* — and a future reader will find it and assume
+this change is a mistake. That claim was measured on `overfull-hbox`, which has
+the shortest timeout in the set by 2.4×. It does not generalise to the other
+four, and correcting it is part of this work.
+
+Iteration rate, measured: `make-mips-interpreter` averaged 2.03s of wall per
+iteration (1.77s of it provider time), so its 1800s had room for roughly 885.
+
+## What was decided, and the alternatives
+
+**Both budgets stand; neither replaces the other.** With the wall bounding
+spend, the iteration ceiling reverts to the role `loop.ts` already claims for it
+— a guard against a pathological loop with nobody watching — and `job.yaml`'s
+`max_iterations` goes 200 → 1000 so it stops binding first.
+
+Rejected: **raising `max_iterations` alone.** Zero code, immediately testable,
+but the loop still cannot see a clock, so on `overfull-hbox`'s 750s a slow run
+gets hard-killed by Harbor mid-work instead of winding down. Also rejected:
+**per-task iteration budgets** derived from each timeout ÷ measured rate — honest
+to the data, but it is hand-tuning benchmark config per task, which is fragile
+and overfits.
+
+**The operator passes the whole budget; the loop subtracts its own reserve.**
+`agent.py` forwards Harbor's `timeout_sec` verbatim, so a published number traces
+back to `task.toml` with no arithmetic in between, and the safety margin stays
+one constant in one repository. Rejected: having the caller send a pre-reduced
+figure (`timeout_sec * 0.9`), which splits the reserve across two repos and
+scales a fixed wind-down cost proportionally, giving a 750s task the same
+fraction as a 12000s one.
+
+**Tool timeouts are clamped to the remaining budget.** Without it the deadline
+is advisory: `run_terminal` defaults to 300s and the model may ask for more, so
+one command started just inside the budget outlives it by minutes — on
+`overfull-hbox`, a single default-timeout call is 40% of the entire budget.
+
+**The deadline lives in module state (`runtime/deadline.ts`), not on
+`Tool.execute`.** `runtime/sandbox/registry.ts` argues this case in its own
+docstring for the same shape: three tools need it, and threading it through
+would change the `Tool` interface in `config/types.ts` and every tool signature.
+
+**`onBudgetExhausted` is not consulted when the wall deadline binds.** The
+iteration ceiling can afford to ask because iterations do not tick while a human
+thinks. A clock does — and the only path with a handler is the interactive one,
+which will not have the variable set.
+
+**`WallBudgetExhaustedError` is a sibling of `IterationBudgetExhaustedError`
+under a shared `BudgetExhaustedError`, and both exit 2.** The exit contract in
+`commands/agent.tsx` already means "worked, did not finish, judge the result
+rather than treat this as a crash", which is exactly what a deadline produces,
+and `agent.py` already maps 2 to success. A distinct exit code 3 was rejected:
+until `agent.py` was updated to match, Harbor would book those trials as
+exceptions and drop them from the mean, overstating measured accuracy.
+
+**The wind-down converts time into steps rather than warning separately.**
+Remaining time ÷ the turn's own mean wall-per-iteration gives a step count, and
+the existing `REMAINING_ITERATIONS_WARNING = 5` then serves both budgets through
+one message and one flag. A constant expressed in seconds was rejected as the
+wrong shape across this task set — 120s is 16% of `overfull-hbox`'s budget and
+1% of `build-pov-ray`'s.
+
+## Consequences
+
+- `WOOPCODE_MAX_WALL_SEC` and the exit-code behaviour become a contract with
+  `harbor_woopcode/agent.py`. Changing either means changing both.
+- The wind-down flag replaces an equality test (`iterations === budget - 5`)
+  that silently never fired when the ceiling was below five.
+- This fixes a loop that ends early. It makes **no claim** about benchmark
+  reward: with five tasks at one trial each, and three failures with three
+  unrelated causes, there is no power to attribute a score change to it. The
+  claim to verify is narrower — that the loop no longer kills itself with
+  budget in hand.

docs/reference/configuration.md

@@ -142,6 +142,7 @@ the run.
 | --- | --- | --- |
 | `WOOPCODE_PROVIDER` | `google` | Pairs with `WOOPCODE_API_KEY` |
 | `WOOPCODE_MAX_ITERATIONS` | `40` | Steps a turn may take before it stops to ask whether to keep going. Interactively the ceiling is a checkpoint, so it is set to catch a stuck loop rather than to ration requests — the provider rations those itself, and answering the checkpoint grants another `40`. A headless run has nobody to ask, so this is the whole budget and exhausting it exits `2` |
+| `WOOPCODE_MAX_WALL_SEC` | unset (off) | Wall-clock seconds a turn may take, counted from process start. Pass the whole budget the harness enforces; a reserve is held back internally so the last step, the final answer and the session write still land. The loop stops on whichever budget binds first, and a spent clock exits `2` like a spent ceiling. Unset interactively, where a person decides when a turn has gone on too long |
 | `WOOPCODE_MAX_ATTEMPTS` | `3` | Tries per provider request before the error surfaces |
 | `WOOPCODE_TOOL_HISTORY_BUDGET` | unset (off) | Characters of tool history to keep before older results are compacted. Off by default — see the measurements in `runtime/compaction.ts` |
 | `WOOPCODE_THINKING_BUDGET` | `-1` | Reasoning depth; see below |

packages/tests/runtime/deadline.test.ts

@@ -0,0 +1,122 @@
+import { describe, test, expect, afterEach } from "bun:test";
+import {
+  WALL_RESERVE_SEC,
+  clampToBudget,
+  clearDeadline,
+  deadlineReached,
+  now,
+  remainingMs,
+  setDeadline,
+} from "../../../runtime/deadline";
+
+/**
+ * The deadline is module state, and module state outlives a file.
+ *
+ * A test that installed a fake clock and did not clear it would leave every
+ * later test in the run measuring elapsed time against a number that never
+ * moves — which is exactly the class of failure the injected clock exists to
+ * avoid in the first place.
+ */
+afterEach(() => {
+  clearDeadline();
+});
+
+/** A clock the test moves by hand. */
+function fakeClock(start = 0) {
+  let at = start;
+  return {
+    now: () => at,
+    advance(ms: number) {
+      at += ms;
+    },
+  };
+}
+
+describe("the wall-clock deadline", () => {
+  test("the reserve is subtracted from the budget the operator gave", () => {
+    const clock = fakeClock();
+    setDeadline(600, { now: clock.now, startedAt: 0 });
+
+    expect(remainingMs()).toBe((600 - WALL_RESERVE_SEC) * 1000);
+  });
+
+  test("it is counted from process start, so a second turn cannot restart it", () => {
+    const clock = fakeClock();
+    setDeadline(600, { now: clock.now, startedAt: 0 });
+
+    // A turn's worth of time passes, then the next turn sets the same budget
+    // again. The harness has been counting throughout, so the deadline must not
+    // move — a turn granted the whole budget afresh would overrun it.
+    clock.advance(100_000);
+    setDeadline(600, { now: clock.now, startedAt: 0 });
+
+    expect(remainingMs()).toBe((600 - WALL_RESERVE_SEC) * 1000 - 100_000);
+  });
+
+  test("the deadline is reached once the budget less the reserve is spent", () => {
+    const clock = fakeClock();
+    setDeadline(600, { now: clock.now, startedAt: 0 });
+
+    clock.advance((600 - WALL_RESERVE_SEC) * 1000 - 1);
+    expect(deadlineReached()).toBe(false);
+
+    clock.advance(1);
+    expect(deadlineReached()).toBe(true);
+  });
+
+  test("a budget no larger than the reserve is spent before it starts", () => {
+    const clock = fakeClock();
+    setDeadline(WALL_RESERVE_SEC, { now: clock.now, startedAt: 0 });
+
+    expect(remainingMs()).toBe(0);
+    expect(deadlineReached()).toBe(true);
+  });
+
+  // Every reader has to answer "keep going" for a session that never opted in,
+  // or turning the feature off would end turns rather than leave them alone.
+  test("an unbudgeted session is never out of time", () => {
+    expect(deadlineReached()).toBe(false);
+    expect(remainingMs()).toBeUndefined();
+  });
+
+  test("now() reads the injected clock, and clearDeadline puts the real one back", () => {
+    const clock = fakeClock(1_000);
+    setDeadline(600, { now: clock.now, startedAt: 0 });
+    expect(now()).toBe(1_000);
+
+    clearDeadline();
+    // The real clock, not the fake one frozen at 1,000.
+    expect(now()).toBeGreaterThan(1_600_000_000_000);
+  });
+});
+
+describe("clamping a tool timeout to the budget", () => {
+  test("an unbudgeted session gets the timeout it asked for", () => {
+    expect(clampToBudget(300)).toBe(300);
+  });
+
+  test("a timeout that fits is passed through", () => {
+    const clock = fakeClock();
+    setDeadline(600, { now: clock.now, startedAt: 0 });
+
+    expect(clampToBudget(30)).toBe(30);
+  });
+
+  test("a timeout that outlives the budget is cut to what is left", () => {
+    const clock = fakeClock();
+    setDeadline(160, { now: clock.now, startedAt: 0 });
+
+    // 160s less the reserve leaves 100s, so a default 300s command gets 100.
+    expect(clampToBudget(300)).toBe(100);
+  });
+
+  // Zero or a negative number is not a shorter timeout, it is a command killed
+  // before it starts — reported as a failure that says nothing about the clock.
+  test("the clamp never returns less than a second", () => {
+    const clock = fakeClock();
+    setDeadline(600, { now: clock.now, startedAt: 0 });
+    clock.advance((600 - WALL_RESERVE_SEC) * 1000);
+
+    expect(clampToBudget(300)).toBe(1);
+  });
+});

packages/tests/runtime/iterationBudget.test.ts

@@ -164,14 +164,17 @@ describe("running out of budget", () => {
     expect(budgetNotices(messages)[0]!.content).toContain("Only 5 more steps");
   });
 
-  test("a budget too small to warn in still runs and still ends", async () => {
-    // The threshold is five from the end, so a budget of two never reaches it.
-    // The turn must still exhaust cleanly rather than warn about a negative
-    // number of remaining steps.
+  test("a budget below the warning distance is warned about immediately", async () => {
+    // The notice used to be an equality against the iteration count, which
+    // silently never fired when the whole ceiling was below the five steps of
+    // warning — the turns with least room to spare were the ones told nothing.
+    // It is a threshold now, so a budget of two is announced at the first step.
     process.env.WOOPCODE_MAX_ITERATIONS = "2";
 
     const { messages } = await runKeepingMessages();
-    expect(budgetNotices(messages)).toHaveLength(0);
+
+    expect(budgetNotices(messages)).toHaveLength(1);
+    expect(budgetNotices(messages)[0]!.content).toContain("Only 2 more steps");
   });
 });