Skip to content

Commit d4a16f4

Browse files
fix: exclude browser scroll actions from repetition detection (#7471)
- Modified ToolRepetitionDetector to skip repetition detection for browser_action scroll_down and scroll_up actions - Added isBrowserScrollAction() helper method to identify scroll actions - Added comprehensive tests for the new behavior - Fixes issue where multiple scroll actions were incorrectly flagged as being stuck in a loop Resolves: https://github.com/RooCodeInc/Roo-Code/discussions/7470 Co-authored-by: Roo Code <[email protected]>
1 parent 3cb489d commit d4a16f4

File tree

2 files changed

+182
-0
lines changed

2 files changed

+182
-0
lines changed

src/core/tools/ToolRepetitionDetector.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,13 @@ export class ToolRepetitionDetector {
3232
messageDetail: string
3333
}
3434
} {
35+
// Browser scroll actions should not be subject to repetition detection
36+
// as they are frequently needed for navigating through web pages
37+
if (this.isBrowserScrollAction(currentToolCallBlock)) {
38+
// Allow browser scroll actions without counting them as repetitions
39+
return { allowExecution: true }
40+
}
41+
3542
// Serialize the block to a canonical JSON string for comparison
3643
const currentToolCallJson = this.serializeToolUse(currentToolCallBlock)
3744

@@ -66,6 +73,21 @@ export class ToolRepetitionDetector {
6673
return { allowExecution: true }
6774
}
6875

76+
/**
77+
* Checks if a tool use is a browser scroll action
78+
*
79+
* @param toolUse The ToolUse object to check
80+
* @returns true if the tool is a browser_action with scroll_down or scroll_up action
81+
*/
82+
private isBrowserScrollAction(toolUse: ToolUse): boolean {
83+
if (toolUse.name !== "browser_action") {
84+
return false
85+
}
86+
87+
const action = toolUse.params.action as string
88+
return action === "scroll_down" || action === "scroll_up"
89+
}
90+
6991
/**
7092
* Serializes a ToolUse object into a canonical JSON string for comparison
7193
*

src/core/tools/__tests__/ToolRepetitionDetector.spec.ts

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -402,4 +402,164 @@ describe("ToolRepetitionDetector", () => {
402402
}
403403
})
404404
})
405+
406+
// ===== Browser Scroll Action Exclusion tests =====
407+
describe("browser scroll action exclusion", () => {
408+
it("should not count browser scroll_down actions as repetitions", () => {
409+
const detector = new ToolRepetitionDetector(2)
410+
411+
// Create browser_action tool use with scroll_down
412+
const scrollDownTool: ToolUse = {
413+
type: "tool_use",
414+
name: "browser_action" as ToolName,
415+
params: { action: "scroll_down" },
416+
partial: false,
417+
}
418+
419+
// Should allow unlimited scroll_down actions
420+
for (let i = 0; i < 10; i++) {
421+
const result = detector.check(scrollDownTool)
422+
expect(result.allowExecution).toBe(true)
423+
expect(result.askUser).toBeUndefined()
424+
}
425+
})
426+
427+
it("should not count browser scroll_up actions as repetitions", () => {
428+
const detector = new ToolRepetitionDetector(2)
429+
430+
// Create browser_action tool use with scroll_up
431+
const scrollUpTool: ToolUse = {
432+
type: "tool_use",
433+
name: "browser_action" as ToolName,
434+
params: { action: "scroll_up" },
435+
partial: false,
436+
}
437+
438+
// Should allow unlimited scroll_up actions
439+
for (let i = 0; i < 10; i++) {
440+
const result = detector.check(scrollUpTool)
441+
expect(result.allowExecution).toBe(true)
442+
expect(result.askUser).toBeUndefined()
443+
}
444+
})
445+
446+
it("should not count alternating scroll_down and scroll_up as repetitions", () => {
447+
const detector = new ToolRepetitionDetector(2)
448+
449+
const scrollDownTool: ToolUse = {
450+
type: "tool_use",
451+
name: "browser_action" as ToolName,
452+
params: { action: "scroll_down" },
453+
partial: false,
454+
}
455+
456+
const scrollUpTool: ToolUse = {
457+
type: "tool_use",
458+
name: "browser_action" as ToolName,
459+
params: { action: "scroll_up" },
460+
partial: false,
461+
}
462+
463+
// Alternate between scroll_down and scroll_up
464+
for (let i = 0; i < 5; i++) {
465+
let result = detector.check(scrollDownTool)
466+
expect(result.allowExecution).toBe(true)
467+
expect(result.askUser).toBeUndefined()
468+
469+
result = detector.check(scrollUpTool)
470+
expect(result.allowExecution).toBe(true)
471+
expect(result.askUser).toBeUndefined()
472+
}
473+
})
474+
475+
it("should still apply repetition detection to other browser_action types", () => {
476+
const detector = new ToolRepetitionDetector(2)
477+
478+
// Create browser_action tool use with click action
479+
const clickTool: ToolUse = {
480+
type: "tool_use",
481+
name: "browser_action" as ToolName,
482+
params: { action: "click", coordinate: "[100, 200]" },
483+
partial: false,
484+
}
485+
486+
// First call allowed
487+
expect(detector.check(clickTool).allowExecution).toBe(true)
488+
489+
// Second call allowed
490+
expect(detector.check(clickTool).allowExecution).toBe(true)
491+
492+
// Third identical call should be blocked (limit is 2)
493+
const result = detector.check(clickTool)
494+
expect(result.allowExecution).toBe(false)
495+
expect(result.askUser).toBeDefined()
496+
})
497+
498+
it("should still apply repetition detection to non-browser tools", () => {
499+
const detector = new ToolRepetitionDetector(2)
500+
501+
const readFileTool = createToolUse("read_file", "read_file", { path: "test.txt" })
502+
503+
// First call allowed
504+
expect(detector.check(readFileTool).allowExecution).toBe(true)
505+
506+
// Second call allowed
507+
expect(detector.check(readFileTool).allowExecution).toBe(true)
508+
509+
// Third identical call should be blocked (limit is 2)
510+
const result = detector.check(readFileTool)
511+
expect(result.allowExecution).toBe(false)
512+
expect(result.askUser).toBeDefined()
513+
})
514+
515+
it("should not interfere with repetition detection of other tools when scroll actions are interspersed", () => {
516+
const detector = new ToolRepetitionDetector(2)
517+
518+
const scrollTool: ToolUse = {
519+
type: "tool_use",
520+
name: "browser_action" as ToolName,
521+
params: { action: "scroll_down" },
522+
partial: false,
523+
}
524+
525+
const otherTool = createToolUse("execute_command", "execute_command", { command: "ls" })
526+
527+
// First execute_command
528+
expect(detector.check(otherTool).allowExecution).toBe(true)
529+
530+
// Scroll actions in between (should not affect counter)
531+
expect(detector.check(scrollTool).allowExecution).toBe(true)
532+
expect(detector.check(scrollTool).allowExecution).toBe(true)
533+
534+
// Second execute_command
535+
expect(detector.check(otherTool).allowExecution).toBe(true)
536+
537+
// More scroll actions
538+
expect(detector.check(scrollTool).allowExecution).toBe(true)
539+
540+
// Third execute_command should be blocked
541+
const result = detector.check(otherTool)
542+
expect(result.allowExecution).toBe(false)
543+
expect(result.askUser).toBeDefined()
544+
})
545+
546+
it("should handle browser_action with missing or invalid action parameter gracefully", () => {
547+
const detector = new ToolRepetitionDetector(2)
548+
549+
// Browser action without action parameter
550+
const noActionTool: ToolUse = {
551+
type: "tool_use",
552+
name: "browser_action" as ToolName,
553+
params: {},
554+
partial: false,
555+
}
556+
557+
// Should apply normal repetition detection
558+
expect(detector.check(noActionTool).allowExecution).toBe(true)
559+
expect(detector.check(noActionTool).allowExecution).toBe(true)
560+
const result = detector.check(noActionTool)
561+
expect(result.allowExecution).toBe(false)
562+
expect(result.askUser).toBeDefined()
563+
})
564+
})
405565
})

0 commit comments

Comments
 (0)