fix(agent-core-v2): count validation-rejected tool calls toward the repeat breaker - #2317
Merged
Merged
Conversation
🦋 Changeset detectedLatest commit: 9d99e64 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related Issue
No linked issue — v2 port of #2313 (merged as de0ba9d), explained below.
Problem
Same gap as #2313, on the v2 engine: the repeat breaker (
toolDedupe) only counts tool calls that fireonBeforeExecuteTool. Calls rejected in preflight (missing/unavailable tool, guard denial, invalid args) never fire that veto event — and v2 additionally skipped theonDidExecuteToolhook for them, so the breaker never saw them at all. A model re-issuing the same invalid call got no reminders and no force-stop; the turn only ended at maxSteps, burning a provider request per step.Reproduction (
test/agent/toolDedupe/toolDedupe.test.ts): 12 identicalBashcalls missing the requiredcommand+ one trailing text response. Before: the turn consumes the 13th generation. After: it stops at exactly 12 with the fullnone → r1×2 → r2×3 → r3×4 → stopescalation chain.What changed
v2 port of #2313, adapted to the DI hook architecture — the v1 fix hooked
finalizeToolResultbecause every v1 call passes through it; in v2 the equivalent chokepoint (onDidExecuteTool) did not run for preflight-rejected calls, so the executor change comes first:toolExecutorService.ts: runonDidExecuteToolfor preflight-rejected calls too (toolstays unset for them — already optional in the hook contract). Existing hook consumers are unaffected: prompt delivery isdelivery-gated, goal-outcome matching requires a successfulUpdateGoal, and external hooks now firePostToolUseFailurefor rejected calls, matching v1 semantics. Rejected results also flow through the standard result normalization, so they now carry the same shape as runnable results (stopTurn: false).toolDedupeService.ts: register calls that bypassedonBeforeExecuteToollate, atonDidExecuteTooltime (registerSkipped, no-op when already registered). Args that failed JSON parsing normalize to{}, which would key every malformed-but-different attempt identically — those are keyed on the raw arguments text so only true re-issues count as repeats.parseToolCallArgumentsmoves fromtoolExecutorService.tsto the shared#/tool/tool-args-parsemodule so the dedupe domain reuses it instead of importing an impl file.Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.