diff --git a/harness/src/run-leaf.ts b/harness/src/run-leaf.ts index 701478e..d4ece49 100644 --- a/harness/src/run-leaf.ts +++ b/harness/src/run-leaf.ts @@ -16,6 +16,7 @@ import { BufferedRedisBackend } from "./buffered-redis-backend.js"; import { flushExtension } from "./flush-extension.js"; import { checkpointExtension } from "./checkpoint-extension.js"; import { submitVerdictExtension, VERDICT_ENTRY_TYPE, type VerdictCapture } from "./submit-verdict-tool.js"; +import { verdictTerminationExtension } from "./verdict-termination-extension.js"; import { validateVerdict, type Verdict } from "./verdict.js"; import type { GateCapture } from "./request-approval-tool.js"; import { computeGateState, decideSeed, validateDecision, type Decision, GATE_DECISION_ENTRY_TYPE } from "./gate.js"; @@ -446,6 +447,7 @@ export const realProduceVerdict: ProduceVerdict = async (item, env, config, capt k8sSandboxExtension({ config: selected?.config ?? null, transport: selected?.transport }), flushExtension(backend), checkpointExtension(store, sessionManager), + verdictTerminationExtension(capture, { maxTurns: env.maxTurns }), ], }); await resourceLoader.reload(); diff --git a/harness/src/submit-verdict-tool.ts b/harness/src/submit-verdict-tool.ts index cd0f8f5..5a80f89 100644 --- a/harness/src/submit-verdict-tool.ts +++ b/harness/src/submit-verdict-tool.ts @@ -50,7 +50,11 @@ export function submitVerdictExtension(capture: VerdictCapture, sink?: VerdictSi } capture.verdict = r.value; sink?.appendCustomEntry(VERDICT_ENTRY_TYPE, r.value); - return { content: [{ type: "text", text: "Verdict recorded." }] }; + // `terminate: true` tells agent-loop.ts to stop the turn loop after this tool call + // (AgentToolResult.terminate) — without it the model just keeps getting re-prompted + // and calls submit_verdict again, sometimes thousands of times, until something + // external kills the pod. + return { content: [{ type: "text", text: "Verdict recorded." }], terminate: true }; }, } as any); }; diff --git a/harness/src/verdict-termination-extension.ts b/harness/src/verdict-termination-extension.ts new file mode 100644 index 0000000..773eb97 --- /dev/null +++ b/harness/src/verdict-termination-extension.ts @@ -0,0 +1,58 @@ +import type { ExtensionAPI, ExtensionFactory } from "@earendil-works/pi-coding-agent"; +import type { VerdictCapture } from "./submit-verdict-tool.js"; + +export interface TurnCounter { + turns: number; +} + +// Envelopes don't always set `maxTurns` (it's optional on LeafEnvelope), but the runaway-loop +// bug this extension exists to prevent has no other guard — so fall back to a hard cap even when +// the caller omits one, rather than silently skipping turn-limiting for those leaves. +const DEFAULT_MAX_TURNS = 40; + +/** + * Backstop for the leaf agent loop: caps total turns even if a verdict is never submitted, and + * blocks any further tool calls once a verdict HAS been captured (in case the model tries to call + * more tools after submit_verdict, e.g. as part of a parallel batch that didn't all set + * `terminate`). The primary fix lives in submit-verdict-tool.ts, which now sets `terminate: true` + * on its own tool result — agent-loop.ts stops the turn loop as soon as every tool call in a batch + * sets that flag (AgentToolResult.terminate). This extension exists because that primary fix only + * covers the "model calls submit_verdict cleanly" path; without a cap, a model that never calls + * submit_verdict (or calls it alongside other tools that don't terminate) can still spin forever — + * one observed leaf called submit_verdict ~1,269 times before an external pod kill stopped it. + * + * `tool_call`'s `block` result is the only extension hook that can halt execution here — + * `tool_result`/`turn_end` results carry no loop-control field (AgentSession.afterToolCall drops + * any `terminate` an extension tries to set on `tool_result`). Blocking still costs one turn (the + * model sees a "blocked" tool error and gets re-prompted) rather than stopping instantly, so this + * is a safety net, not the fast path. `runLeaf` already treats a leaf with no captured verdict as + * `no_verdict` (a normal, already-handled failure outcome), so hitting the turn cap without a + * verdict fails cleanly instead of hanging the queue message forever. + */ +export function verdictTerminationExtension( + capture: VerdictCapture, + opts: { maxTurns?: number } = {}, +): ExtensionFactory { + const counter: TurnCounter = { turns: 0 }; + // Treat a missing OR non-positive maxTurns as "use the default" — a nullish-coalesce alone would + // let an explicit 0 (or negative) through, which then fails the `> 0` guard below and silently + // DISABLES the cap (unbounded), the opposite of what a caller passing 0 would expect. + const maxTurns = typeof opts.maxTurns === "number" && opts.maxTurns > 0 ? opts.maxTurns : DEFAULT_MAX_TURNS; + return (pi: ExtensionAPI) => { + pi.on("turn_start", () => { + counter.turns += 1; + }); + pi.on("tool_call", (event) => { + if (capture.verdict) { + return { block: true, reason: "Verdict already submitted for this item — task complete." }; + } + if (typeof maxTurns === "number" && maxTurns > 0 && counter.turns > maxTurns) { + return { + block: true, + reason: `Turn limit (${maxTurns}) reached without a submitted verdict — stopping.`, + }; + } + return {}; + }); + }; +} diff --git a/harness/test/submit-verdict-tool.test.ts b/harness/test/submit-verdict-tool.test.ts index 43a6e8a..3785d0c 100644 --- a/harness/test/submit-verdict-tool.test.ts +++ b/harness/test/submit-verdict-tool.test.ts @@ -25,6 +25,22 @@ describe("submitVerdictExtension", () => { expect(res.isError).toBeFalsy(); }); + it("sets terminate: true on a successful verdict, so agent-loop.ts stops the turn loop", async () => { + const capture: VerdictCapture = {}; + const { api, tools } = fakePi(); + submitVerdictExtension(capture)(api); + const res = await tools[0].execute("call-1", { item_id: "i1", verdict: "CLEAR", reason: "r" }, undefined, undefined, {} as any); + expect(res.terminate).toBe(true); + }); + + it("does not set terminate on an invalid verdict (agent should retry, not stop)", async () => { + const capture: VerdictCapture = {}; + const { api, tools } = fakePi(); + submitVerdictExtension(capture)(api); + const res = await tools[0].execute("call-1", { item_id: "i1", verdict: "MAYBE", reason: "r" }, undefined, undefined, {} as any); + expect(res.terminate).toBeFalsy(); + }); + it("rejects an invalid verdict and does not capture", async () => { const capture: VerdictCapture = {}; const { api, tools } = fakePi(); diff --git a/harness/test/verdict-termination-extension.test.ts b/harness/test/verdict-termination-extension.test.ts new file mode 100644 index 0000000..55073a4 --- /dev/null +++ b/harness/test/verdict-termination-extension.test.ts @@ -0,0 +1,72 @@ +import { describe, it, expect } from "vitest"; +import { verdictTerminationExtension } from "../src/verdict-termination-extension"; +import type { VerdictCapture } from "../src/submit-verdict-tool"; + +function fakePi() { + const handlers: Record = {}; + const pi = { on: (ev: string, h: Function) => { handlers[ev] = h; } }; + return { pi: pi as any, handlers }; +} + +describe("verdictTerminationExtension", () => { + it("does not block tool calls before a verdict is captured and under the turn cap", () => { + const capture: VerdictCapture = {}; + const { pi, handlers } = fakePi(); + verdictTerminationExtension(capture, { maxTurns: 5 })(pi); + const result = handlers["tool_call"]({}); + expect(result).toEqual({}); + }); + + it("blocks tool calls once a verdict has already been captured", () => { + const capture: VerdictCapture = { verdict: { item_id: "i1", verdict: "CLEAR", reason: "r" } }; + const { pi, handlers } = fakePi(); + verdictTerminationExtension(capture, { maxTurns: 5 })(pi); + const result = handlers["tool_call"]({}); + expect(result.block).toBe(true); + expect(result.reason).toMatch(/verdict already submitted/i); + }); + + it("blocks tool calls once maxTurns is exceeded without a verdict", () => { + const capture: VerdictCapture = {}; + const { pi, handlers } = fakePi(); + verdictTerminationExtension(capture, { maxTurns: 2 })(pi); + // turn_start fires once per turn; simulate 3 turns (exceeds cap of 2) + handlers["turn_start"](); + handlers["turn_start"](); + handlers["turn_start"](); + const result = handlers["tool_call"]({}); + expect(result.block).toBe(true); + expect(result.reason).toMatch(/turn limit/i); + }); + + it("does not block at exactly the turn cap (only after exceeding it)", () => { + const capture: VerdictCapture = {}; + const { pi, handlers } = fakePi(); + verdictTerminationExtension(capture, { maxTurns: 2 })(pi); + handlers["turn_start"](); + handlers["turn_start"](); + const result = handlers["tool_call"]({}); + expect(result).toEqual({}); + }); + + it("applies a default turn cap when maxTurns is omitted (never leaves the loop unbounded)", () => { + const capture: VerdictCapture = {}; + const { pi, handlers } = fakePi(); + verdictTerminationExtension(capture)(pi); + for (let i = 0; i < 41; i++) handlers["turn_start"](); + const result = handlers["tool_call"]({}); + expect(result.block).toBe(true); + expect(result.reason).toMatch(/turn limit/i); + }); + + it("treats maxTurns: 0 as 'use default' rather than disabling the cap", () => { + const capture: VerdictCapture = {}; + const { pi, handlers } = fakePi(); + verdictTerminationExtension(capture, { maxTurns: 0 })(pi); + // 0 must NOT mean unbounded — it should fall back to the default cap and still block. + for (let i = 0; i < 41; i++) handlers["turn_start"](); + const result = handlers["tool_call"]({}); + expect(result.block).toBe(true); + expect(result.reason).toMatch(/turn limit/i); + }); +});