feat(core): stop a run that keeps repeating the same tool call

[wudilyy999]

Summary

Closes #487. A run could repeat the same tool call until the generation timeout or the token budget ended it. This adds a detector in core and wires it into the beforeToolCall option of the Agent that packages/core/src/agent.ts creates, the hook that already handles cancellation and the done repair limit. (The tool_call in AGENTS.md is the pi-coding-agent extension event; core uses pi-agent-core's Agent directly.)

Why this is design-specific (hard constraint 7). pi-agent-core and pi-coding-agent 0.72.1 provide beforeToolCall, a terminate flag on tool results, and shouldStopAfterTurn in the low-level loop config, which Agent does not expose. They have no turn cap and no repeated-call detection. Only the wiring goes through pi's hooks. The loop behavior below is read from agent-loop.ts at v0.72.1.

What counts as a repeat

• A call is its tool name plus the validated arguments (BeforeToolCallContext.args), compared as order-independent canonical JSON. Key order does not hide a repeat; array order and the tool name still matter.
• Two signals: the same call several times in a row, and a cycle of two or three calls repeating unchanged. Warn at 3 repeats, halt at 5, both named constants.
• A call with different arguments breaks the pattern. There is no total per tool, so a long design with many different edits does not trip it. This is not a cap on tool calls per run.
• Blocked attempts keep counting. Every call enters the history, including calls blocked after a warning, so a model that ignores the warning reaches the halt (the third and fourth identical call get the warning, the fifth halts).
• State is one guard per generation. It lives across the transport and reasoning retries of that generation (the transcript continues) and resets when a queued user message is delivered, the same point where the done repair progress resets. It is never shared across generations.
• done is not counted. It has its own cap, whose message lists the remaining verifier errors; counting it here would stop three identical done calls before that report.

warn

The call is blocked with { block: true, reason }. pi turns that into an error tool result whose text is the reason, so the tool does not run and no message is injected. The reason says the call was repeated and tells the model to change approach or stop and say what blocks it.

halt, end to end

1. Record. toolLoop.halt holds the repeating tool names and the count; it is logged at error level. The halting call is blocked with a reason that says to stop.
2. No later tool runs. Once the halt is recorded, every later beforeToolCall blocks, including later calls in the same response. pi prepares all calls of a parallel response before running any, and a call prepared before the halting one either breaks the pattern (so there is no halt) or belongs to it and already got its own warning.
3. End the pi loop. The turn_end listener throws GENERATION_INCOMPLETE, at the same place the done repair limit does it, because Agent does not expose shouldStopAfterTurn. pi catches the throw and ends the loop at that event, before it polls the steering and follow-up queues, so no further model request is made.
4. Outer boundary. runHalted() (the done repair limit or the loop halt) makes the recovery loop and the provider-error mapping skip the failure message pi recorded. onComplete still persists the transcript, then generateViaAgent rethrows GENERATION_INCOMPLETE with copy that says the model repeated a tool call. Workspace files are untouched.
5. Queued messages. They are neither drained nor dropped. The throw comes before pi polls the queues, and the finally in generateViaAgent closes ActiveRunMessages, which clears the queues and marks pending messages not-delivered so the user can recover them.

Shared with the done repair limit: the turn_end throw, the recovery and error-mapping skip (generalized from repairLimitReached() to runHalted()), and the GENERATION_INCOMPLETE code. New: the detector, its state, and the texts.

Two commits: the detector with its unit tests, then the wiring, integration tests and changeset.

Type of change

• Bug fix
• New feature
• Refactor (no behavior change)
• Documentation
• Build / CI / tooling
• Breaking change

Linked issue

Closes #487

Checklist

• I checked the linked issue / relevant context before starting
• pnpm lint && pnpm typecheck && pnpm test passes locally
• Added/updated tests for the change
• Added a changeset (pnpm changeset) if user-visible
• Updated docs if behavior changed

Principles

• Compatibility: runs whose calls differ never see the guard. A run that repeats the same call is now stopped at five repeats and reports GENERATION_INCOMPLETE.
• Upgradeability: the guard is a pure function; the wiring uses beforeToolCall and the existing turn_end stop. No pi internals.
• No bloat: no new dependencies; 180 lines outside tests and the changeset (109 for the detector, 71 in agent.ts).
• Elegance: one detector decides, one runHalted() predicate ends the run for both the done repair limit and the loop.

Checks

• pnpm -r typecheck, pnpm lint, CI=1 pnpm test: all passed; core 588 tests, shared 327, desktop 181 files / 2,640 tests.
• tool-loop-guard.test.ts (13): thresholds, the streak, two- and three-call cycles, a longer cycle that is ignored, a changed argument breaking either pattern, key order, array order, tool name, 60 different edits around a repeated preview, and reset.
• agent.native-lifecycle.test.ts (+7) runs a real pi Agent with a scripted model stream and no network:
  • warn: the blocked tool does not run (counted by reads of a file only that call touches), the reason is the error tool result in the next request, and no user message is added;
  • halt: no later tool runs, the provider is called exactly as many times as scripted, the run reaches idle, GENERATION_INCOMPLETE is thrown (not success, cancellation or a provider error), the files written earlier are kept, and a steering and a follow-up message queued before the stop end as not-delivered;
  • a halt inside one response: the call after the halting one does not write;
  • a cycle across a view and a failing edit halts at the fifth repeat;
  • many different edits around the same read finish normally;
  • three identical done calls still end with the done repair limit and three verifier runs;
  • a delivered steering message resets the count.
• Each of the following fails a test when removed: the warn block, the block of later calls, the turn_end throw, the final rethrow, the provider-error skip, the reset on delivery, the done exemption; in the detector, the key sort, the tool name in the key, the >= at the halt threshold, and the history length.
• Not caught by any test: replacing runHalted() with repairLimitReached() in the recovery loop. The stop message is never classified as a transport error, so the loop would break anyway. I kept it so a halted run is never retried by construction.

[github-actions]

Findings

No Blocker, Major, or Minor findings.

The detector (packages/core/src/tool-loop-guard.ts) is a pure function with no new dependencies, and the wiring in packages/core/src/agent.ts:1355 uses the existing pi beforeToolCall hook plus the existing turn_end stop path instead of reaching into pi internals, which is consistent with hard constraint 7. The loop-guard behavior matches the PR description and is exercised by both unit tests (packages/core/src/tool-loop-guard.test.ts) and real-Agent integration tests (packages/core/src/agent.native-lifecycle.test.ts).

I traced the thresholds and trimming for off-by-ones: HISTORY_LIMIT = TOOL_LOOP_HALT_REPEATS * TOOL_LOOP_MAX_CYCLE_LENGTH = 15 is exactly enough to observe 5 repeats of a length-3 cycle, trailingRepeats never indexes out of range (the while condition keeps block - repeats * length >= 0), and the verdict sequence [ok, ok, warn, warn, halt] is consistent with >= WARN / >= HALT. I did not find a correctness defect in the detector or the halt path.

Questions

• I could not fetch issue #487 in this run, so I validated only against the PR body's stated behavior. Does #487 name any additional surface (e.g. a diagnostics/report path) that this PR is expected to cover beyond the core loop guard?

Summary

• Review mode: initial
• Scope: the diff matches the PR's stated scope (core detector + wiring + tests + core patch changeset). No new prod dependency, no license concern, no release/distribution path touched.
• The guard compares tool name plus order-independent args (canonicalJson), counts warn-blocked calls so a model that ignores the warning still reaches halt, exempts done so its own repair limit still reports verifier errors, and resets when a queued user message is delivered (agent.native-lifecycle.test.ts). Each of the interesting failure paths (warn does not run the tool, in-response calls after the halt are blocked, queued messages end not-delivered, files are kept, the run reaches idle) is asserted.
• No security, data-loss, or silent-fallback concerns: a halt throws GENERATION_INCOMPLETE (user-visible), onComplete still persists the transcript, workspace files are untouched, and blocked warn/halt reasons surface as error tool results for the model.
• Residual observations (not blocking, no fix required for merge):
  • A repeating cycle of 4+ distinct calls is intentionally not detected (TOOL_LOOP_MAX_CYCLE_LENGTH = 3), so such a loop would still run to the generation timeout / token budget. Fine to keep as a deliberate scope limit, but a reasonable follow-up issue if 4+ call loops are observed in practice.
  • Because done is exempt from the guard, identical non-done calls separated only by done are treated as a consecutive streak. This is usually desirable (identical view/preview results are redundant), so I would not change it; the "…in a row" phrasing can just read slightly loosely in that one case.
  • Each check serializes the full validated args (including any large file_text) into a history key; bounded to 15 entries, so only a minor transient allocation cost on large writes.

Testing

• Not run (automation). The PR adds 13 detector unit tests and 7 integration tests over a real pi Agent with a scripted stream. Coverage of warn/halt, in-response ordering after a halt, 2- and 3-call cycles, the done exemption, key-order/array-order significance, and reset-on-delivery is proportionate to the change.

Open-CoDesign Bot

[wudilyy999]

Thanks. #487 names no other surface. It asks for the detector, the beforeToolCall wiring, and an end of run through the existing GENERATION_INCOMPLETE path, with no new setting and no change to diagnostics. The maintainers' follow-up there asked for the halt path to be spelled out, for queued steering and follow-up messages to be covered, and for tests on a real Agent with a scripted stream; those are in the description and in agent.native-lifecycle.test.ts. Nothing else is expected from this PR.