Skip to content

Commit 1320b50

Browse files
p12ticp12tic
committed
feat: add tiered tool-repetition detection (soft warning + hard stop)
Replace the single-threshold ToolRepetitionDetector with a two-tier system: - Soft block (default 2nd identical call): the tool is not executed and the model receives an error asking it to justify the repeat. The user is not involved and the counter keeps incrementing toward the hard limit. - Hard stop: execution stops and the user is asked for guidance (previous behavior).
1 parent 8bec7c1 commit 1320b50

59 files changed

Lines changed: 1167 additions & 399 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎packages/types/src/provider-settings.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,13 @@ import {
5454

5555
export const DEFAULT_CONSECUTIVE_MISTAKE_LIMIT = 3
5656

57+
/**
58+
* Number of identical consecutive tool calls allowed before the tool is
59+
* soft-blocked. When reached, the tool is not executed and the model is
60+
* asked to justify why it needs to repeat the call. Set to 0 to disable.
61+
*/
62+
export const DEFAULT_TOOL_REPETITION_SOFT_LIMIT = 2
63+
5764
/**
5865
* DynamicProvider
5966
*

‎packages/types/src/provider-settings/common.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ export const baseProviderSettingsShape = {
1313
modelTemperature: z.number().nullish(),
1414
rateLimitSeconds: z.number().optional(),
1515
consecutiveMistakeLimit: z.number().min(0).optional(),
16+
toolRepetitionSoftLimit: z.number().min(0).optional(),
1617
enableReasoningEffort: z.boolean().optional(),
1718
reasoningEffort: reasoningEffortSettingSchema.optional(),
1819
modelMaxTokens: z.number().optional(),

‎packages/types/src/task.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ export interface CreateTaskOptions {
8787
taskId?: string
8888
enableCheckpoints?: boolean
8989
consecutiveMistakeLimit?: number
90+
toolRepetitionSoftLimit?: number
9091
experiments?: Record<string, boolean>
9192
initialTodos?: TodoItem[]
9293
/** Initial status for the task's history item (e.g., "active" for child tasks) */

‎src/core/assistant-message/__tests__/presentAssistantMessage-custom-tool.spec.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ describe("presentAssistantMessage - Custom Tool Recording", () => {
7474
recordToolUsage: vi.fn(),
7575
recordToolError: vi.fn(),
7676
toolRepetitionDetector: {
77-
check: vi.fn().mockReturnValue({ allowExecution: true }),
77+
check: vi.fn().mockReturnValue({ action: "allow" }),
7878
},
7979
providerRef: {
8080
deref: () => ({

‎src/core/assistant-message/__tests__/presentAssistantMessage-images.spec.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ describe("presentAssistantMessage - Image Handling in Native Tool Calling", () =
4848
},
4949
recordToolUsage: vi.fn(),
5050
toolRepetitionDetector: {
51-
check: vi.fn().mockReturnValue({ allowExecution: true }),
51+
check: vi.fn().mockReturnValue({ action: "allow" }),
5252
},
5353
providerRef: {
5454
deref: () => ({

‎src/core/assistant-message/__tests__/presentAssistantMessage-tool-repetition.spec.ts‎

Lines changed: 297 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,297 @@
1+
// npx vitest src/core/assistant-message/__tests__/presentAssistantMessage-tool-repetition.spec.ts
2+
3+
import { providerIdentifiers } from "@roo-code/types"
4+
5+
import { presentAssistantMessage } from "../presentAssistantMessage"
6+
7+
type ToolBlock = {
8+
type: string
9+
id?: string
10+
name?: string
11+
params?: Record<string, string>
12+
nativeArgs?: Record<string, string>
13+
partial?: boolean
14+
}
15+
16+
type ToolResultBlock = {
17+
type: string
18+
tool_use_id?: string
19+
content?: string
20+
is_error?: boolean
21+
text?: string
22+
}
23+
24+
type MockTask = Record<string, unknown>
25+
26+
type MockRepetitionDetector = { check: ReturnType<typeof vi.fn> }
27+
28+
// Mock dependencies
29+
vi.mock("../../task/Task")
30+
vi.mock("../../tools/validateToolUse", () => ({
31+
validateToolUse: vi.fn(),
32+
isValidToolName: vi.fn(() => true),
33+
}))
34+
35+
// Mock the read_file tool so we can assert the normal execution path is taken
36+
// when the repetition detector allows a tool call. The handle spy simulates a
37+
// successful tool run by pushing a tool_result, mirroring the real tool.
38+
// `vi.hoisted` is required because `vi.mock` factories are hoisted above
39+
// top-level variable declarations.
40+
const { readFileHandle } = vi.hoisted(() => ({
41+
readFileHandle: vi.fn(
42+
async (
43+
_cline: unknown,
44+
block: ToolBlock,
45+
{ pushToolResult }: { pushToolResult: (result: string) => void },
46+
) => {
47+
pushToolResult(`[read_file for '${block?.params?.path ?? block?.nativeArgs?.path}'] Result`)
48+
},
49+
),
50+
}))
51+
52+
vi.mock("../../tools/ReadFileTool", () => ({
53+
readFileTool: {
54+
handle: readFileHandle,
55+
getReadFileToolDescription: vi.fn(() => "[read_file]"),
56+
},
57+
}))
58+
59+
const captureConsecutiveMistakeError = vi.fn()
60+
const captureException = vi.fn()
61+
const captureToolUsage = vi.fn()
62+
63+
vi.mock("@roo-code/telemetry", () => ({
64+
TelemetryService: {
65+
instance: {
66+
get captureToolUsage() {
67+
return captureToolUsage
68+
},
69+
get captureConsecutiveMistakeError() {
70+
return captureConsecutiveMistakeError
71+
},
72+
get captureException() {
73+
return captureException
74+
},
75+
captureEvent: vi.fn(),
76+
},
77+
},
78+
}))
79+
80+
describe("presentAssistantMessage - Tool Repetition Detection", () => {
81+
let mockTask: MockTask
82+
83+
beforeEach(() => {
84+
vi.clearAllMocks()
85+
86+
mockTask = {
87+
taskId: "test-task-id",
88+
instanceId: "test-instance",
89+
abort: false,
90+
presentAssistantMessageLocked: false,
91+
presentAssistantMessageHasPendingUpdates: false,
92+
currentStreamingContentIndex: 0,
93+
assistantMessageContent: [],
94+
userMessageContent: [],
95+
didCompleteReadingStream: false,
96+
didRejectTool: false,
97+
didAlreadyUseTool: false,
98+
consecutiveMistakeCount: 0,
99+
consecutiveMistakeLimit: 5,
100+
clineMessages: [],
101+
getTaskMode: vi.fn().mockResolvedValue("code"),
102+
apiConfiguration: { apiProvider: providerIdentifiers.anthropic },
103+
api: {
104+
getModel: () => ({ id: "test-model", info: {} }),
105+
},
106+
recordToolUsage: vi.fn(),
107+
recordToolError: vi.fn(),
108+
toolRepetitionDetector: {
109+
check: vi.fn().mockReturnValue({ action: "allow" }),
110+
},
111+
providerRef: {
112+
deref: () => ({
113+
getState: vi.fn().mockResolvedValue({
114+
mode: "code",
115+
customModes: [],
116+
}),
117+
}),
118+
},
119+
say: vi.fn().mockResolvedValue(undefined),
120+
ask: vi.fn().mockResolvedValue({ response: "yesButtonClicked" }),
121+
}
122+
123+
mockTask.pushToolResultToUserContent = vi.fn().mockImplementation((toolResult: ToolResultBlock) => {
124+
const userMessageContent = mockTask.userMessageContent as ToolResultBlock[]
125+
const existingResult = userMessageContent.find(
126+
(block) => block.type === "tool_result" && block.tool_use_id === toolResult.tool_use_id,
127+
)
128+
if (existingResult) {
129+
return false
130+
}
131+
;(mockTask.userMessageContent as ToolResultBlock[]).push(toolResult)
132+
return true
133+
})
134+
})
135+
136+
it("should soft block without involving the user and return an error to the model", async () => {
137+
const toolCallId = "tool_call_soft_block"
138+
mockTask.assistantMessageContent = [
139+
{
140+
type: "tool_use",
141+
id: toolCallId,
142+
name: "read_file",
143+
params: { path: "test.txt" },
144+
nativeArgs: { path: "test.txt" },
145+
partial: false,
146+
},
147+
]
148+
149+
;(mockTask.toolRepetitionDetector as MockRepetitionDetector).check = vi.fn().mockReturnValue({
150+
action: "soft_block",
151+
message: "The tool 'read_file' was blocked because it was just called with identical parameters.",
152+
})
153+
154+
await presentAssistantMessage(mockTask as unknown as Parameters<typeof presentAssistantMessage>[0])
155+
156+
// The user should NOT have been asked anything for a soft block.
157+
expect(mockTask.ask).not.toHaveBeenCalled()
158+
159+
// The tool must not be executed when it is soft blocked.
160+
expect(readFileHandle).not.toHaveBeenCalled()
161+
162+
// A tool_result with the soft block message should have been pushed.
163+
const toolResult = (mockTask.userMessageContent as ToolResultBlock[]).find(
164+
(item) => item.type === "tool_result" && item.tool_use_id === toolCallId,
165+
)
166+
expect(toolResult).toBeDefined()
167+
expect(toolResult?.content).toContain("read_file")
168+
expect(toolResult?.content).toContain("identical parameters")
169+
170+
// No telemetry escalation for a soft block.
171+
expect(captureConsecutiveMistakeError).not.toHaveBeenCalled()
172+
expect(captureException).not.toHaveBeenCalled()
173+
})
174+
175+
it("should hard block, ask the user for guidance, and record telemetry", async () => {
176+
const toolCallId = "tool_call_hard_block"
177+
mockTask.assistantMessageContent = [
178+
{
179+
type: "tool_use",
180+
id: toolCallId,
181+
name: "read_file",
182+
params: { path: "test.txt" },
183+
nativeArgs: { path: "test.txt" },
184+
partial: false,
185+
},
186+
]
187+
188+
;(mockTask.toolRepetitionDetector as MockRepetitionDetector).check = vi.fn().mockReturnValue({
189+
action: "hard_block",
190+
askUser: {
191+
messageKey: "mistake_limit_reached",
192+
messageDetail: "Roo appears to be stuck in a loop calling {toolName} repeatedly.",
193+
},
194+
})
195+
196+
mockTask.ask = vi.fn().mockResolvedValue({ response: "yesButtonClicked" })
197+
198+
await presentAssistantMessage(mockTask as unknown as Parameters<typeof presentAssistantMessage>[0])
199+
200+
// The user must be asked for guidance with the resolved message key.
201+
expect(mockTask.ask).toHaveBeenCalledWith(
202+
"mistake_limit_reached",
203+
expect.stringContaining("read_file"),
204+
)
205+
206+
// Telemetry escalation should have fired for a hard block.
207+
expect(captureConsecutiveMistakeError).toHaveBeenCalledWith("test-task-id")
208+
expect(captureException).toHaveBeenCalled()
209+
210+
// A tool_result describing the repetition limit should have been pushed.
211+
const toolResult = (mockTask.userMessageContent as ToolResultBlock[]).find(
212+
(item) => item.type === "tool_result" && item.tool_use_id === toolCallId,
213+
)
214+
expect(toolResult).toBeDefined()
215+
expect(toolResult?.content).toContain("read_file")
216+
})
217+
218+
it("should incorporate user feedback when the user responds to a hard block", async () => {
219+
const toolCallId = "tool_call_hard_block_feedback"
220+
mockTask.assistantMessageContent = [
221+
{
222+
type: "tool_use",
223+
id: toolCallId,
224+
name: "read_file",
225+
params: { path: "test.txt" },
226+
nativeArgs: { path: "test.txt" },
227+
partial: false,
228+
},
229+
]
230+
231+
;(mockTask.toolRepetitionDetector as MockRepetitionDetector).check = vi.fn().mockReturnValue({
232+
action: "hard_block",
233+
askUser: {
234+
messageKey: "mistake_limit_reached",
235+
messageDetail: "Stuck calling {toolName} repeatedly.",
236+
},
237+
})
238+
239+
mockTask.ask = vi.fn().mockResolvedValue({
240+
response: "messageResponse",
241+
text: "try a different file",
242+
images: [],
243+
})
244+
245+
await presentAssistantMessage(mockTask as unknown as Parameters<typeof presentAssistantMessage>[0])
246+
247+
// User feedback should have been surfaced to the chat.
248+
expect(mockTask.say).toHaveBeenCalledWith("user_feedback", "try a different file", [])
249+
250+
// And appended to the user message content.
251+
const feedbackBlock = (mockTask.userMessageContent as ToolResultBlock[]).find(
252+
(item) => item.type === "text" && String(item.text).includes("try a different file"),
253+
)
254+
expect(feedbackBlock).toBeDefined()
255+
expect(feedbackBlock?.text).toContain("Tool repetition limit reached")
256+
})
257+
258+
it("should execute the tool normally when the detector allows it", async () => {
259+
const toolCallId = "tool_call_allow"
260+
mockTask.assistantMessageContent = [
261+
{
262+
type: "tool_use",
263+
id: toolCallId,
264+
name: "read_file",
265+
params: { path: "test.txt" },
266+
nativeArgs: { path: "test.txt" },
267+
partial: false,
268+
},
269+
]
270+
271+
;(mockTask.toolRepetitionDetector as MockRepetitionDetector).check = vi
272+
.fn()
273+
.mockReturnValue({ action: "allow" })
274+
275+
await presentAssistantMessage(mockTask as unknown as Parameters<typeof presentAssistantMessage>[0])
276+
277+
// The detector should have been consulted.
278+
expect((mockTask.toolRepetitionDetector as { check: ReturnType<typeof vi.fn> }).check).toHaveBeenCalled()
279+
// No hard block ask should have occurred.
280+
expect(mockTask.ask).not.toHaveBeenCalledWith("mistake_limit_reached", expect.anything())
281+
282+
// The tool must have actually continued into normal execution: the
283+
// read_file tool runner should have been dispatched with this block.
284+
expect(readFileHandle).toHaveBeenCalledTimes(1)
285+
const [, dispatchedBlock] = readFileHandle.mock.calls[0]
286+
expect(dispatchedBlock).toMatchObject({ name: "read_file", id: toolCallId })
287+
288+
// And the normal execution path should have produced a tool_result
289+
// (no soft/hard block error message).
290+
const toolResult = (mockTask.userMessageContent as ToolResultBlock[]).find(
291+
(item) => item.type === "tool_result" && item.tool_use_id === toolCallId,
292+
)
293+
expect(toolResult).toBeDefined()
294+
expect(toolResult?.content).toContain("read_file")
295+
expect(toolResult?.is_error).toBeUndefined()
296+
})
297+
})

‎src/core/assistant-message/__tests__/presentAssistantMessage-unknown-tool.spec.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ describe("presentAssistantMessage - Unknown Tool Handling", () => {
5151
recordToolUsage: vi.fn(),
5252
recordToolError: vi.fn(),
5353
toolRepetitionDetector: {
54-
check: vi.fn().mockReturnValue({ allowExecution: true }),
54+
check: vi.fn().mockReturnValue({ action: "allow" }),
5555
},
5656
providerRef: {
5757
deref: () => ({

‎src/core/assistant-message/presentAssistantMessage.ts‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -671,8 +671,17 @@ export async function presentAssistantMessage(cline: Task) {
671671
// block directly.
672672
const repetitionCheck = cline.toolRepetitionDetector.check(block)
673673

674-
// If execution is not allowed, notify user and break.
675-
if (!repetitionCheck.allowExecution && repetitionCheck.askUser) {
674+
// Soft block: do NOT involve the user. Return an error to the
675+
// model asking it to justify repeating the call (or try a
676+
// different approach). The detector keeps counting so continued
677+
// repetition will eventually escalate to a hard block.
678+
if (repetitionCheck.action === "soft_block") {
679+
pushToolResult(formatResponse.toolError(repetitionCheck.message))
680+
break
681+
}
682+
683+
// Hard block: stop and ask the user for guidance.
684+
if (repetitionCheck.action === "hard_block") {
676685
// Handle repetition similar to mistake_limit_reached pattern.
677686
const { response, text, images } = await cline.ask(
678687
repetitionCheck.askUser.messageKey as ClineAsk,

0 commit comments

Comments
 (0)