Skip to content

[Feature]: Stop a run that keeps repeating the same tool call #487

Closed

Description

@wudilyy999

Problem / pain point

  • [Bug]: Model reasoning in loop burning all tokens #251 reported a run that spent 11 minutes and about $2 on a simple blog post, going around a fix-and-verify loop. fix(core): cap done repair loop #292 fixed that case by capping the done() repair loop at three error rounds. No other tool call has a limit like that.
  • Today a run can only be cut short by the transport retry cap (2), the done repair cap (3), or the wall-clock generation timeout (up to 7,200 s in Settings → Advanced). [Feature]: Unlimited generation timeout #411 asks for even longer or no timeout, for overnight local runs, and the same thread has a user who does not want a run that looped to go on indefinitely. The editor stops a path after three failed exact edits, but only by returning a message: the run continues, and no other tool or kind of repeat has a limit. A model that repeats the same call, or alternates between a view and an edit that never succeeds, runs until the clock or the token budget ends it.
  • Apart from that per-path message, I did not find any detection of a repeated tool call in packages/core/src, and none in pi-agent-core or pi-coding-agent 0.72.1.

Proposed solution

A pure detector in packages/core, wired into the beforeToolCall hook that agent.ts already uses for cancellation and the done repair limit:

  1. Detector. createToolLoopGuard() sees each tool call (tool name plus its validated arguments) and answers ok, warn or halt. It looks for two signals:
    • the same tool with identical arguments several times in a row (warn at 3, halt at 5);
    • a short cycle of identical calls repeating, such as view → edit → view → edit with the same arguments each time (a cycle of up to 3 calls, warn at 3 repeats of the cycle, halt at 5).
      Any call with different arguments breaks the pattern, so edits to different ranges or with different content do not count. There is no cap on how many times a tool is called in total, because a large design legitimately makes dozens of edits.
  2. On warn, the call is blocked with a reason that tells the model it has repeated the call and should change approach or finish. The model reads it as the tool result, so no extra message is injected.
  3. On halt, the call is blocked and the run ends the way the done repair limit ends it: no further tools run, the workspace files are kept, and the user sees a GENERATION_INCOMPLETE message that says the model kept repeating a tool call.
  4. The decision is logged through the existing logger with the tool name and the repeat count (no console.*). Thresholds are named constants; no new setting.
  5. Tests: the detector on its own (a streak, a cycle, a reset when the arguments change, the thresholds), and an agent.test.ts run with a scripted model that repeats a call, which stops with the files kept.

I expect this to come out a little over 400 lines with the tests, mostly the detector and its tests. I would open one PR with the detector and the wiring as separate commits, and I am happy to split it if you prefer.

Alternatives considered

  • Rely on the generation timeout: it is wall-clock only, and the ask in [Feature]: Unlimited generation timeout #411 goes the other way.
  • Cap the number of turns or tool calls per run: blunt, and it stops legitimate long designs.
  • Count calls per tool, as some harnesses do: edit-heavy designs would trip it.
  • Prompt text such as "stop after N rounds": fix(core): cap done repair loop #292 found the prompts disagreeing with each other and the model ignoring them, which is why that fix is in code.

Scope

Generation capability

Hard-constraints check

  • I have read the hard constraints in CLAUDE.md and this proposal does not conflict with any of them. No new dependencies, no bundled runtime, no new storage, no model or network call: it is a pure function in core that reads the tool calls the run already makes.

Activity

  1. github-actions commented on Oct 13, 2026

    @github-actions
    Contributor

    Thanks for the detailed write-up — a repeated-tool-call guard is a reasonable gap to close, and the shape you describe (a pure detector in packages/core, no new dependency, no extra model or network call) is consistent with the project's hard constraints.

    What I could verify from the public tree

    • Hard constraints: the proposal stays inside them — no bundled runtime, no new storage, no proxied or model call. Your point 4 also matches the repo rule that packages/core/** must not use console.* and should log through the injected CoreLogger, and the repo requires at least one Vitest test for a new feature; your test plan (detector unit tests plus an agent.test.ts scripted-model run) covers that.
    • Your "Alternatives considered" framing of the timeout matches .changeset/forward-generation-timeout-to-requests.md:7, which describes the Generation timeout as applying both to each model HTTP request and to the overall run — it is still a clock, not a loop signal.
    • The stop-on-halt behaviour you want (no further tools run, workspace files kept, GENERATION_INCOMPLETE surfaced) is the right instinct: reusing the existing done-repair-limit termination path keeps one error surface instead of adding a second failure mode.

    Relevant code:

    • CLAUDE.md — hard constraints (no bundled runtimes, BYOK/no added network calls, no new SQLite state, permissive shipped deps) and the console.* ban for packages/core/**; Vitest is the required test runner.
    • AGENTS.md — "Reuse pi primitives first": pi-coding-agent owns sessions, built-in tools, bash execution, event streaming, model registry, provider registration, and capability data "unless a design-specific need proves otherwise". Tool calls are documented as gated through the pi tool_call hook and the Open CoDesign permission UI.
    • .changeset/forward-generation-timeout-to-requests.md:7 — the Generation timeout is applied to each model HTTP request as well as the overall run.
    • package.json — root version 0.2.0.
    • pnpm-lock.yaml — @mariozechner/pi-agent-core, @mariozechner/pi-ai, and @mariozechner/pi-coding-agent at ^0.72.1 in both apps/desktop and packages/core.

    Open questions before review (things I could not confirm from the public snippets)

    1. Where the guard belongs. Hard constraint 7 asks whether a pi primitive already covers this before packages/core grows its own. Can you check pi-agent-core / pi-coding-agent (0.72.1 per the lockfile) for an existing hook or guard, and state in the PR why this is design-specific if nothing exists?
    2. Hook name. The public AGENTS.md text refers to the pi tool_call hook; you cite beforeToolCall as the hook agent.ts already uses for cancellation and the done repair limit. Please make the PR match the actual current symbol in packages/core/src/agent.ts and say which one it is, so reviewers know the exact wiring point.
    3. warn/blocked-call semantics. Confirm the hook can return a blocking result that the model reads as the tool result (rather than needing to throw), so the "no extra message is injected" part holds.
    4. Error-code reuse. If the current termination path uses a specific code from packages/shared, reuse it rather than introducing a parallel one, and keep the user-visible copy consistent with the existing incomplete-generation message.

    Design notes

    • Argument-based matching is the right call: because any differing argument breaks both the streak and the cycle, edit-heavy designs — and repeated view of one path between genuinely different edits — will not trip it. Worth stating that explicitly in the PR so it does not read as "cap tool calls per run".
    • Warn 3 / halt 5 gives two rounds of grace, in the same spirit as the done repair cap. Since a false halt loses a run, logging the tool name and repeat count at both levels (as you plan) matters — it lets thresholds be tuned from real logs rather than guessed.
    • One case worth an explicit test: the model re-reading a file with identical arguments after a truncated or failed tool result. Make sure the warn reason tells it to change approach, not merely to retry.
    • Keeping the detector as a pure function with named-constant thresholds and no new setting is the right scope; a setting here would be premature.

    The references to #251, #292, and #411 line up with the areas you describe. If you want to open the PR, splitting it into detector and wiring commits is fine and easier to review.


    Open-CoDesign Bot

  2. wudilyy999 commented on Oct 13, 2026

    @wudilyy999
    ContributorAuthor

    Thanks, this is useful. Answers to the four questions, and one correction to my description.

    Correction. The description says nothing limits a repeated str_replace_based_edit_tool call. That is not accurate. text-editor.ts stops a path after three failed exact edits (FAILED_EXACT_EDIT_LIMIT): the tool returns text telling the model to stop using str_replace and insert on that file for the rest of the run. That is a message for one path, not a stop. The run continues, and the model can make the same call again or call another tool. The guard here covers what that limit does not: identical calls that succeed or fail in other ways, in any tool, and cycles across tools. I have updated the description.

    1. Where the guard belongs. I checked pi-agent-core and pi-coding-agent 0.72.1 (the lockfile versions) for a loop or turn guard and found none: no maximum number of turns and no detection of a repeated tool call. pi does provide what the guard builds on: beforeToolCall, which can block a call, terminate on a tool result, and the loop's shouldStopAfterTurn, which Agent does not expose. So the detector is design-specific and only the wiring goes through pi's hooks. The PR will say so.
    2. Hook name. beforeToolCall in the new Agent({ ... }) options in packages/core/src/agent.ts, the same hook that 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. The PR will name the symbol.
    3. Blocked-call semantics. BeforeToolCallResult is { block?: boolean; reason?: string }, and pi documents that a blocked call produces an error tool result whose text is reason. No throw is needed and no extra message is injected.
    4. Error code. GENERATION_INCOMPLETE, as the done repair limit uses, with copy that says the model kept repeating a tool call.

    I will add the test you suggested: the model re-reading with identical arguments after a failed or truncated tool result, where the warning tells it to change approach and not just retry.

  3. github-actions commented on Oct 13, 2026

    @github-actions
    Contributor

    Thanks for the correction and for the detailed answers — both are useful.

    On the correction. Agreed, and thanks for fixing the description. text-editor.ts returning a "stop using str_replace/insert on this file" message after FAILED_EXACT_EDIT_LIMIT failed exact edits is a per-path advisory, not a stop: the run continues, the model can re-issue the same call or move to another tool, and nothing bounds identical calls or cross-tool cycles. That is exactly the gap your guard covers, and it is worth stating that distinction explicitly in the PR so it does not read as "the editor already handles this."

    On the four questions — all four are now answerable in the PR description:

    1. Design-specific vs. pi primitive. Your finding that 0.72.1 exposes beforeToolCall, a terminate flag on tool results, and a loop-level shouldStopAfterTurn that Agent does not surface — but no turn cap and no repeated-call detection — is the justification AGENTS.md asks for under Hard Constraints item 7 ("Reuse pi primitives first … unless a design-specific need proves otherwise"). Say that in the PR.
    2. Hook symbol. Naming beforeToolCall in the new Agent({ ... }) options in packages/core/src/agent.ts resolves the ambiguity. AGENTS.md currently says "Gate tools through the pi tool_call hook and the Open CoDesign permission UI" — that is the pi-coding-agent extension event, and it is why the earlier question came up. No need to touch AGENTS.md in this PR; a one-line clarification that core uses pi-agent-core's Agent directly would prevent the same question recurring, but whether that belongs here or in a docs follow-up is a maintainer call.
    3. Blocked-call semantics. I can't verify BeforeToolCallResult's shape or the "reason becomes the error tool-result text" behavior from the public checkout — @mariozechner/pi-* are dependencies, not vendored. Since you've read them, please state the pi version (0.72.1 per the lockfile) and, if convenient, link the pi source for beforeToolCall / BeforeToolCallResult so a reviewer can confirm block semantics without installing the packages.
    4. Error code. Reusing GENERATION_INCOMPLETE and the existing incomplete-generation copy keeps one failure surface instead of two, which matches what was asked.

    One thing still worth pinning down. A blocked call returns an error tool result and the loop continues, and you noted Agent does not expose the loop's shouldStopAfterTurn. So the mechanism that turns halt into an actual end-of-run — no further tools, workspace files kept, GENERATION_INCOMPLETE surfaced — is the part a reviewer will want named.

    Need more info:

    • On halt, does the guard reuse the same termination path in agent.ts as the done repair limit (and cancellation) — i.e. does it abort the pi loop? If instead the run only ends because the model stops calling after repeated blocks, that is a soft stop and should be described as such, since the model could still answer with text or call a different tool.

    Small notes:

    • Cover the warn path in the scripted-model test too, not just halt — assert the blocked call's error text is the reason the model sees, so the "no extra message is injected" claim is test-backed.
    • Keep the thresholds as named constants reachable from tests (rather than re-hardcoding 3/5 in assertions).
    • Log at both warn and halt through the injected CoreLogger — packages/core/** must not use console.*.
    • Since this adds a new user-visible termination surface, consider whether it needs a changeset; the repo versions via Changesets and does not hand-edit CHANGELOG.md.

    Relevant code:

    • AGENTS.md — Hard Constraints item 7 (reuse pi primitives first), and the "Gate tools through the pi tool_call hook" line, which is the pi-coding-agent event and the source of the naming ambiguity.
    • CLAUDE.md — console.* ban for packages/core/**, Vitest as the required test runner, Changesets for versioning.
    • package.json — root version 0.2.0, Node >=22.
    • pnpm-lock.yaml — packages/core and apps/desktop both declare @mariozechner/pi-agent-core, pi-ai, and pi-coding-agent at ^0.72.1.
    • .changeset/forward-generation-timeout-to-requests.md:7 — the Generation timeout is applied per model HTTP request and to the overall run; still a clock, not a loop signal.

    Open-CoDesign Bot

  4. Sun-sunshine06 commented on Oct 13, 2026

    @Sun-sunshine06
    Collaborator

    Thanks for the clarification, @wudilyy999. The distinction between the editor's per-path failed-edit limit and a run-level loop guard is clear, as are the hook name and the choice of GENERATION_INCOMPLETE.

    The main point to make concrete in the implementation PR is the complete halt path. Please describe how it records the stop condition, prevents subsequent tool execution, ends the pi loop, and preserves GENERATION_INCOMPLETE at the outer generation boundary while keeping workspace files.

    In pi 0.72.1, returning { block: true, reason } produces an error tool result, and throwing inside beforeToolCall is also caught and converted into a tool error. Neither alone establishes a hard stop. The batch-level terminate check also requires every finalized result to request termination. These details are visible in the upstream tool-loop implementation. Please cover multiple calls in one assistant response, including calls already prepared for parallel execution, and queued steering/follow-up messages. Naming exactly which parts of the existing done-repair termination path are reused would help reviewers.

    For validation, I suggest a real pi Agent with a scripted model stream and no network calls, alongside the detector unit tests. The existing agent.test.ts scripted-tool mock directly invokes tools, so it would need extending or supplementing to exercise the actual hook and loop behavior. The key assertions would be:

    • warn: the blocked tool does not execute; its reason appears in the next model request as the error tool result, without an extra injected message.
    • halt: no later tool executes and no further model request is made; the run reaches idle, workspace contents are retained, and the caller receives GENERATION_INCOMPLETE, rather than success, cancellation, or a provider error.
    • Detection: streaks and two-/three-call cycles respect the named thresholds; changed arguments break the corresponding pattern; normal edits with different arguments remain allowed. Please also include the failed/truncated-read case already discussed.

    Please define whether blocked attempts continue counting toward halt, how argument equality is determined, and when guard state resets across runs or retries. Logging both decisions through CoreLogger and adding a changeset would complete the user-visible change.

    If an implementation PR is already available, please link it here so these points can be reviewed against the code.

  5. github-actions commented on Oct 13, 2026

    @github-actions
    Contributor

    Thanks @Sun-sunshine06 — this is the level of detail the implementation PR needs. Here is what I can align on from the public tree, and where the answer has to come from the PR itself.

    The pi 0.72.1 loop behavior is the crux. Your reading matches the upstream agent-loop.ts at v0.72.1: a beforeToolCall result of { block: true, reason } is surfaced to the model as an error tool result, and a throw inside the hook is caught and converted the same way — neither, on its own, stops the loop. The per-batch terminate check only fires when every finalized tool result in the batch requests termination. So halt cannot be a block alone: it has to reuse the same abort mechanism the done repair limit already uses to end the run.

    The PR description should name that mechanism and the exact symbols in packages/core/src/agent.ts — which abort call is made, what state it sets, how the outer generation boundary observes that state, and how it maps to GENERATION_INCOMPLETE without deleting the workspace. The public snippets I have do not include those agent.ts internals, so I can't confirm the current termination path here; naming it in the PR is what resolves it for reviewers. Whatever the guard does with a queued steering/follow-up message must be stated too: a message queued before the halt should not restart the loop or be silently dropped.

    Multiple calls in one assistant response. beforeToolCall runs per call, and parallel-prepared calls can be finalized together. The guard should handle a batch where one call trips it while others are already prepared: the trip must prevent the remaining calls from executing, and halt must not produce another model request.

    The definitions you asked for — the PR should state each explicitly and cover it with a test:

    • Do blocked attempts count toward halt? Recommend yes: a blocked warn attempt should still count as repeat evidence, otherwise a model that repeats the same call gets unbounded free retries. State the rule and assert it.
    • Argument equality. Canonical, order-independent comparison of the post-validation argument object (stable serialization / deep-equal), not raw JSON string equality — so key ordering does not mask a genuine repeat, and different content or ranges remain different calls.
    • Guard state reset. State whether state is per-run and reset on a new generation or retry, and confirm it is not carried across runs.

    Tests. Agreed that the existing agent.test.ts scripted-tool mock invokes tools directly and will not exercise beforeToolCall, so warn and halt need a scripted model stream against a real Agent with no network calls. The three assertion groups you list are the ones to land:

    • warn: the blocked tool does not execute; its reason appears in the next model request as the error tool result, with no injected message.
    • halt: no later tool executes, no further model request is made, the run reaches idle, workspace contents are retained, and the caller receives GENERATION_INCOMPLETE — not success, cancellation, or a provider error.
    • Detection: streaks and two-/three-call cycles respect the named thresholds, a changed argument breaks the pattern, and normal edits with different arguments stay allowed — including the failed/truncated-read re-read case.

    Logging and changeset. Both warn and halt should log through the injected CoreLogger with the tool name and repeat count; packages/core/** must not use console.* (see CLAUDE.md). A changeset is appropriate here because this adds a user-visible termination surface, and the repo versions via Changesets rather than hand-editing CHANGELOG.md.

    PR link. No implementation PR is linked in this thread yet, so there is nothing to review against these points at the moment.


    Open-CoDesign Bot

  6. added a commit that references this issue on Oct 15, 2026
    466e64d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions