fix: bound the tool-use loop to prevent runaway provider/tool cycles

[stealthwhizz]

Problem

pi-agent-core's agent loop continues for as long as the assistant keeps emitting tool calls — there is no turn cap in the loop, and the maxTurns value gitagent sets on the Agent state is never consumed by the library.

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. This is the DoS-class behavior in #58.

Root cause

• runLoop in pi-agent-core loops on while (hasMoreToolCalls || pendingMessages.length > 0) with hasMoreToolCalls = !executedToolBatch.terminate. Nothing counts turns.
• grep maxTurns node_modules/@mariozechner/pi-agent-core → no matches. gitagent's modelOptions.maxTurns is spread into initialState but never read.

Fix

A loop guard (src/loop-guard.ts) wired through the afterToolCall hook, which is the library's supported termination path (terminate: true on every result in a batch stops the loop). It terminates the run when either bound is hit:

• maxTurns — total assistant turns that request tools. Precedence: explicit query() option → agent manifest runtime.max_turns → default 50.
• maxRepeats — 5 consecutive identical tool calls (same name + args), the signature of a stuck loop. Catches the reproducer well under the 100-request threshold.

The guard latches once tripped so a mid-batch trip still terminates the next batch, and it preserves the last tool's real output while appending the stop reason for observability.

Tests

test/loop-guard.test.ts — normal varied use (no trip), repeat detection, counter reset on arg change, maxTurns cap, per-batch turn counting (shared assistantMessage reference = one turn), and output preservation. Full suite: 33 passing.

Closes #58

[shreyas-lyzr]

Solid fix for the unbounded loop issue (#58). The implementation is clean and the test coverage is thorough.

One minor observation: maxRepeats is clamped to Math.max(2, ...), meaning a caller passing maxRepeats: 1 gets silently bumped to 2. This is a reasonable guard but worth a comment noting that values below 2 are unsupported — not a blocker.

Everything else checks out: the afterToolCall hook is the correct termination path, the latch-once behavior ensures a mid-batch trip propagates correctly, per-batch turn counting via reference equality is the right approach, and the precedence order for maxTurns (explicit option → manifest → default) is sensible. All six test cases cover the meaningful edge cases.

[Copilot]

Pull request overview

Adds a safety guard to prevent unbounded tool-use loops when a provider repeatedly re-requests tools (DoS-class behavior described in #58), by enforcing a turn cap and wiring termination through the supported afterToolCall hook.

Changes:

• Adds a loop-guard unit test suite covering repeat detection, maxTurns, batching semantics, and output preservation.
• Wires createLoopGuard({ maxTurns }) into query() via the Agent’s afterToolCall hook and resolves maxTurns from (option → manifest → default).

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.

File	Description
test/loop-guard.test.ts	New unit tests validating loop-guard termination behavior and output preservation.
src/sdk.ts	Resolves maxTurns and attaches the loop-guard afterToolCall hook to bound tool-use loops.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[shreyas-lyzr]

Solid, targeted fix for the unbounded tool-use loop (issue #58).

What the PR does: Introduces a LoopGuard wired through pi-agent-core's afterToolCall hook — the library's supported termination path. When either bound is hit, the guard returns terminate: true on every result in the batch, which stops the loop cleanly. Two independent bounds:

• maxTurns: total assistant turns that invoked tools. Defaults to 50, can be overridden by manifest.runtime.max_turns or an explicit query option.
• maxRepeats: 5 consecutive identical tool calls (same name + args), catching stuck loops well before any turn cap.

Implementation review:

loop-guard.ts: The latching pattern (trippedReason) is critical — without it, a mid-batch trip would only terminate some results in the batch, not all, so the loop wouldn't stop. The latch correctly propagates through the rest of that batch and all subsequent batches. Good.

Turn counting by assistantMessage object reference is the right mechanism here — all tool calls in one batch share the same reference, so only distinct batches increment the counter. The test that verifies this (shared assistantMessage reference = one turn) directly validates the behavior.

signatureOf() uses JSON.stringify with a fallback to String(args) — covers the undefined/non-serializable edge cases cleanly.

Output preservation: the guard prepends the real content and details from the tool result and appends only the stop-reason text. Callers that inspect tool results still get the real output.

sdk.ts: The maxTurns precedence chain (explicit option → manifest → 50) is correct and the value is reflected in modelOptions.maxTurns for telemetry/inspection. The guard itself does the actual enforcement.

Tests (loop-guard.test.ts): Covers all meaningful paths: normal use (no trip), repeat detection, counter reset on arg change, maxTurns cap, per-batch turn counting via shared reference, and output preservation on termination. Test coverage is thorough for the changed logic.