From 968811ab534c03a77a35662250fc0b88f09b752a Mon Sep 17 00:00:00 2001 From: Amogh Sunil Date: Mon, 6 Jul 2026 18:05:37 +0530 Subject: [PATCH 1/2] fix: bound the tool-use loop to prevent runaway provider/tool cycles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pi-agent-core's agent loop runs for as long as the assistant keeps emitting tool calls — it has no turn cap, and the maxTurns value gitagent configured was never consumed. A provider that re-requests the same tool call after every tool result (a broken or malicious backend) drives an unbounded loop: 100+ provider requests, duplicated tool results, and eventual timeout. Add a loop guard wired through the afterToolCall hook that terminates the run when either bound is hit: - maxTurns: total assistant turns that request tools (explicit option > manifest runtime.max_turns > default of 50) - maxRepeats: 5 consecutive identical tool calls (same name + args), the signature of a stuck loop The guard preserves the last tool's real output and appends the reason so the caller can see why the run stopped. Covered by unit tests for the normal path, repeat detection, per-batch turn counting, and output preservation. Closes #58 --- src/loop-guard.ts | Bin 0 -> 3397 bytes src/sdk.ts | 10 +++++ test/loop-guard.test.ts | 83 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+) create mode 100644 src/loop-guard.ts create mode 100644 test/loop-guard.test.ts diff --git a/src/loop-guard.ts b/src/loop-guard.ts new file mode 100644 index 0000000000000000000000000000000000000000..72f04c41f11f558a1ec1be71d61d2be1ade8405e GIT binary patch literal 3397 zcmaJ^+iu%N5bdk<6%*G$-Lk9%YEcl(qJhTStml%WM7?bKV;tR2k?->&lZ}nSj&RY>iz| zhFm;-`s-gl?lc)srKNdoiZC%Hh1!)kQzdjxE92*Sscvzd;Ay1_lNnokiOktsv(OG! zR3N7(PXB?BLX5(zmEO4{#Xyq2)$Ts7Izf%rrYP8$uizQU@Ch1u%~APU)3?vROJTdx zD(J-yA>zoI2P`wxOY?9b$QgTbJT?n zq}l~Q8K5s%9w?g9FYhnjCeeh+??>{0aAXUTGF97S@5RY(8}&G$x4*t7uPX(_p?jGs zAHxrds2mB~MMycwabzN@jDMLe3geN>#H(BB;CZO2pbR2)q(J-7?<7LsZqa&cu-E0-9clX&B z@3seDzNF`DB7$EiAK2=p&btZ$`>w`t#Wi9WUO{97_MSFtd{uaZ&92zsM_CsIj`3G> z(ur#J@1sPb6VRQWbkiEmQ;N$%2t+;8%95|=o|_Zg$L7avj+=uj7gFvVg|ln!a-s|n zF!ab>(-7@fJLM1+BFn8~4W855*2Mx85nKE&<@E#k3iL3dXFbj34rL&nO*?!jqQMf% z52ArV0$$X-OUb7t`{zJ)WZN+P`;Rt-YTcI^jqsl3dGZM+c?5KUX#p{U-}}uB{hu+| zO$-2yTSXLlzf?{)cBJcYm+CCM(cWXMK?!n97y#f59ir6hI9$y6RRdyKfk~t}7(lBz zz~q6AN7E^a+Oh0N9`0b!TLM|WbQ9#kr(@pnu`EDwuo~$P4Z~egGUgz3hwX66Jntp( zeT!*szH*(zlxs~?9f8}pdR%Qik}g0JL0!V`?nLJ?&fYyNpY~#9myeph7dg=S;+_7z z7gN@P%Q$1ShoFhlOXF3V3MzxajH`BVxOtHA_8=C0jd_w^3tHykrsri#!K}bu(Lra| zG^69z<{INvZdk!LUMMCIf*t*6OdWYAm9R@gd z##S++!+$}x4pkk{x_Ih{v0xo~geCmWSP1Rg77^LOybJ&=r4J3HNs@4dqDUmoHu|i5 zHu8r`N(cOR$pMBXe5Ew|t^B~+Gm*LJce=zB(hWz86hQ1TVw=op+gSX)`R agent manifest runtime.max_turns > safe default. + const DEFAULT_MAX_TURNS = 50; + const maxTurns = + options.maxTurns ?? loaded.manifest.runtime?.max_turns ?? DEFAULT_MAX_TURNS; if (options.maxTurns !== undefined) { modelOptions.maxTurns = options.maxTurns; } // 8. Create Agent + const loopGuard = createLoopGuard({ maxTurns }); const agent = new Agent({ initialState: { systemPrompt, @@ -306,6 +315,7 @@ export function query(options: QueryOptions): Query { tools, ...modelOptions, }, + afterToolCall: loopGuard.afterToolCall, }); // 9. Subscribe to events and map to GCMessage diff --git a/test/loop-guard.test.ts b/test/loop-guard.test.ts new file mode 100644 index 0000000..df0c6f8 --- /dev/null +++ b/test/loop-guard.test.ts @@ -0,0 +1,83 @@ +import { describe, it, before } from "node:test"; +import assert from "node:assert/strict"; + +let createLoopGuard: typeof import("../dist/loop-guard.js").createLoopGuard; + +before(async () => { + ({ createLoopGuard } = await import("../dist/loop-guard.js")); +}); + +// Build a minimal AfterToolCallContext. The guard only reads assistantMessage +// (by reference), toolCall.name, args, and result.content. +function ctx(turn: object, name: string, args: unknown) { + return { + assistantMessage: turn as any, + toolCall: { type: "toolCall", id: "x", name, arguments: args as any }, + args, + result: { content: [{ type: "text", text: "ok" }], details: undefined }, + isError: false, + context: {} as any, + }; +} + +describe("createLoopGuard", () => { + it("does not terminate under normal, varied tool use", async () => { + const guard = createLoopGuard({ maxTurns: 50 }); + for (let i = 0; i < 10; i++) { + const r = await guard.afterToolCall(ctx({}, "read", { path: `f${i}` })); + assert.equal(r, undefined); + } + }); + + it("terminates when the same tool call repeats maxRepeats times", async () => { + const guard = createLoopGuard({ maxTurns: 1000, maxRepeats: 5 }); + let terminatedAt = -1; + for (let i = 0; i < 10; i++) { + // Distinct turn each time, but identical name+args → stuck loop + const r = await guard.afterToolCall(ctx({}, "write", { path: "a", content: "" })); + if (r?.terminate) { + terminatedAt = i; + break; + } + } + assert.equal(terminatedAt, 4, "should terminate on the 5th identical call"); + }); + + it("resets the repeat counter when arguments change", async () => { + const guard = createLoopGuard({ maxTurns: 1000, maxRepeats: 3 }); + // 2 identical, then a different one, then 2 identical again → never 3 in a row + assert.equal((await guard.afterToolCall(ctx({}, "write", { p: 1 })))?.terminate, undefined); + assert.equal((await guard.afterToolCall(ctx({}, "write", { p: 1 })))?.terminate, undefined); + assert.equal((await guard.afterToolCall(ctx({}, "write", { p: 2 })))?.terminate, undefined); + assert.equal((await guard.afterToolCall(ctx({}, "write", { p: 2 })))?.terminate, undefined); + }); + + it("terminates when maxTurns distinct turns is exceeded", async () => { + const guard = createLoopGuard({ maxTurns: 3, maxRepeats: 100 }); + // Each call is a new turn object with varied args (no repeat trip). + assert.equal((await guard.afterToolCall(ctx({}, "read", { p: 1 })))?.terminate, undefined); + assert.equal((await guard.afterToolCall(ctx({}, "read", { p: 2 })))?.terminate, undefined); + const third = await guard.afterToolCall(ctx({}, "read", { p: 3 })); + assert.equal(third?.terminate, true, "3rd distinct turn hits maxTurns=3"); + }); + + it("counts one turn per batch (shared assistantMessage reference)", async () => { + const guard = createLoopGuard({ maxTurns: 2, maxRepeats: 100 }); + const turnA = {}; + // Two calls in the SAME batch → one turn, must not trip maxTurns=2 yet. + assert.equal((await guard.afterToolCall(ctx(turnA, "read", { p: 1 })))?.terminate, undefined); + assert.equal((await guard.afterToolCall(ctx(turnA, "read", { p: 2 })))?.terminate, undefined); + // New batch → second turn → hits maxTurns=2. + const turnB = {}; + const r = await guard.afterToolCall(ctx(turnB, "read", { p: 3 })); + assert.equal(r?.terminate, true); + }); + + it("preserves original tool output when terminating", async () => { + const guard = createLoopGuard({ maxTurns: 1, maxRepeats: 100 }); + const r = await guard.afterToolCall(ctx({}, "read", { p: 1 })); + assert.equal(r?.terminate, true); + assert.equal(r?.content?.[0]?.text, "ok"); + assert.match(r?.content?.[1]?.text ?? "", /loop-guard/); + }); +}); From c65c6eb3a14a2dc02780ba665806ed39109e8626 Mon Sep 17 00:00:00 2001 From: Amogh Sunil Date: Mon, 6 Jul 2026 19:22:42 +0530 Subject: [PATCH 2/2] refactor(loop-guard): harden signature, preserve details, reflect maxTurns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - signatureOf: coalesce JSON.stringify(undefined) → "" so undefined args don't produce an undefined signature; fix the misleading NUL separator comment - terminating afterToolCall result now carries through result.details instead of dropping it - sdk: set modelOptions.maxTurns to the resolved cap (option → manifest → default) so agent state/telemetry reflects the limit actually enforced - test: assert details preservation on termination --- src/loop-guard.ts | Bin 3397 -> 3607 bytes src/sdk.ts | 6 +++--- test/loop-guard.test.ts | 9 +++++---- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/loop-guard.ts b/src/loop-guard.ts index 72f04c41f11f558a1ec1be71d61d2be1ade8405e..7f784f222cd1a211b1d467e453a145ee86c70c70 100644 GIT binary patch delta 334 zcmZ9HF-`+P3`H9fX(QkOK=GA^tcW%}5=un_5){x;nOSEuVluOgy-`we2U2he3M9^e z#4$Ju21OK<-{1c4`{Ur{=Z)7$fqMGwfRI{GQwc$b|k@BttFQz>t=#sPMj7q{c9? zJ@3D(KHY!Z{T*)*K}LVXm9QwtJ{5=-)n6p|D3)Jqf+3kp&bixe{R6cQCm z^7C^P@)C1Xbrg#86-qKPixq$Svp6$9Pq#QRZSw;*7B(Iou(rGupqkXl+`MZh I-{iFe0RLb%7 diff --git a/src/sdk.ts b/src/sdk.ts index 1e0438d..0ada81a 100644 --- a/src/sdk.ts +++ b/src/sdk.ts @@ -302,9 +302,9 @@ export function query(options: QueryOptions): Query { const DEFAULT_MAX_TURNS = 50; const maxTurns = options.maxTurns ?? loaded.manifest.runtime?.max_turns ?? DEFAULT_MAX_TURNS; - if (options.maxTurns !== undefined) { - modelOptions.maxTurns = options.maxTurns; - } + // Reflect the effective cap in agent state so telemetry/inspection shows + // the limit the loop guard actually enforces, not just when it was passed. + modelOptions.maxTurns = maxTurns; // 8. Create Agent const loopGuard = createLoopGuard({ maxTurns }); diff --git a/test/loop-guard.test.ts b/test/loop-guard.test.ts index df0c6f8..03beec2 100644 --- a/test/loop-guard.test.ts +++ b/test/loop-guard.test.ts @@ -9,12 +9,12 @@ before(async () => { // Build a minimal AfterToolCallContext. The guard only reads assistantMessage // (by reference), toolCall.name, args, and result.content. -function ctx(turn: object, name: string, args: unknown) { +function ctx(turn: object, name: string, args: unknown, details: unknown = undefined) { return { assistantMessage: turn as any, toolCall: { type: "toolCall", id: "x", name, arguments: args as any }, args, - result: { content: [{ type: "text", text: "ok" }], details: undefined }, + result: { content: [{ type: "text", text: "ok" }], details }, isError: false, context: {} as any, }; @@ -73,11 +73,12 @@ describe("createLoopGuard", () => { assert.equal(r?.terminate, true); }); - it("preserves original tool output when terminating", async () => { + it("preserves original tool output (content + details) when terminating", async () => { const guard = createLoopGuard({ maxTurns: 1, maxRepeats: 100 }); - const r = await guard.afterToolCall(ctx({}, "read", { p: 1 })); + const r = await guard.afterToolCall(ctx({}, "read", { p: 1 }, { rows: 3 })); assert.equal(r?.terminate, true); assert.equal(r?.content?.[0]?.text, "ok"); assert.match(r?.content?.[1]?.text ?? "", /loop-guard/); + assert.deepEqual(r?.details, { rows: 3 }); }); });