From 56ce6e13e6fc4d631703b98f87abf43036f28d06 Mon Sep 17 00:00:00 2001 From: Jeremy Cohn Date: Fri, 17 Jul 2026 15:35:10 -0700 Subject: [PATCH 1/2] fix: stop Act 2 runaway tool-call loop on submit_verdict MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit submit_verdict now sets AgentToolResult.terminate: true on a successful verdict, which agent-loop.ts (pi-fork) already respects as a signal to stop the turn loop immediately once every tool call in a batch sets it. Without this, a leaf that submitted a valid verdict kept getting re-prompted and would call submit_verdict again — one observed leaf did this ~1,269 times before an external pod kill stopped it. Add verdictTerminationExtension as a backstop: caps total turns (default 40, or LeafEnvelope.maxTurns when set) and blocks further tool calls once a verdict has already been captured, covering leaves that never call submit_verdict cleanly (e.g. a model that keeps calling other tools instead). Wired in unconditionally in run-leaf.ts's realProduceVerdict so the cap applies even during gate-approval-pending phases. Assisted-By: Claude (Anthropic AI) Signed-off-by: Jeremy Cohn --- harness/src/run-leaf.ts | 2 + harness/src/submit-verdict-tool.ts | 6 +- harness/src/verdict-termination-extension.ts | 55 +++++++++++++++++ harness/test/submit-verdict-tool.test.ts | 16 +++++ .../verdict-termination-extension.test.ts | 61 +++++++++++++++++++ 5 files changed, 139 insertions(+), 1 deletion(-) create mode 100644 harness/src/verdict-termination-extension.ts create mode 100644 harness/test/verdict-termination-extension.test.ts 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..aabe176 --- /dev/null +++ b/harness/src/verdict-termination-extension.ts @@ -0,0 +1,55 @@ +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 }; + const maxTurns = 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..a865d2c --- /dev/null +++ b/harness/test/verdict-termination-extension.test.ts @@ -0,0 +1,61 @@ +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); + }); +}); From 3062081c640ebb03454c29a1bf63277c6f238c13 Mon Sep 17 00:00:00 2001 From: Jeremy Cohn Date: Thu, 23 Jul 2026 10:29:27 -0700 Subject: [PATCH 2/2] fix: treat maxTurns<=0 as default cap, not unbounded MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review nit on #149: `opts.maxTurns ?? DEFAULT_MAX_TURNS` let an explicit 0 (or negative) through, which then failed the `> 0` guard and silently DISABLED the turn cap — the opposite of what passing 0 implies. Now any missing/non-positive maxTurns falls back to the default. Adds a test asserting maxTurns: 0 still blocks at the default cap. Assisted-By: Claude (Anthropic AI) Signed-off-by: Jeremy Cohn --- harness/src/verdict-termination-extension.ts | 5 ++++- harness/test/verdict-termination-extension.test.ts | 11 +++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/harness/src/verdict-termination-extension.ts b/harness/src/verdict-termination-extension.ts index aabe176..773eb97 100644 --- a/harness/src/verdict-termination-extension.ts +++ b/harness/src/verdict-termination-extension.ts @@ -34,7 +34,10 @@ export function verdictTerminationExtension( opts: { maxTurns?: number } = {}, ): ExtensionFactory { const counter: TurnCounter = { turns: 0 }; - const maxTurns = opts.maxTurns ?? DEFAULT_MAX_TURNS; + // 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; diff --git a/harness/test/verdict-termination-extension.test.ts b/harness/test/verdict-termination-extension.test.ts index a865d2c..55073a4 100644 --- a/harness/test/verdict-termination-extension.test.ts +++ b/harness/test/verdict-termination-extension.test.ts @@ -58,4 +58,15 @@ describe("verdictTerminationExtension", () => { 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); + }); });