feat: Add configurable error & repetition limit with unified control (RooCodeInc#5654) (RooCodeInc#5752)

Co-authored-by: Daniel Riccio <[email protected]>

Author: MuriloFP

packages/types/src/provider-settings.ts

@@ -53,12 +53,18 @@ export type ProviderSettingsEntry = z.infer<typeof providerSettingsEntrySchema>
  * ProviderSettings
  */
 
+/**
+ * Default value for consecutive mistake limit
+ */
+export const DEFAULT_CONSECUTIVE_MISTAKE_LIMIT = 3
+
 const baseProviderSettingsSchema = z.object({
 	includeMaxTokens: z.boolean().optional(),
 	diffEnabled: z.boolean().optional(),
 	fuzzyMatchThreshold: z.number().optional(),
 	modelTemperature: z.number().nullish(),
 	rateLimitSeconds: z.number().optional(),
+	consecutiveMistakeLimit: z.number().min(0).optional(),
 
 	// Model reasoning.
 	enableReasoningEffort: z.boolean().optional(),

src/core/config/ProviderSettingsManager.ts

@@ -5,6 +5,7 @@ import {
 	type ProviderSettingsEntry,
 	providerSettingsSchema,
 	providerSettingsSchemaDiscriminated,
+	DEFAULT_CONSECUTIVE_MISTAKE_LIMIT,
 } from "@roo-code/types"
 import { TelemetryService } from "@roo-code/telemetry"
 
@@ -26,6 +27,7 @@ export const providerProfilesSchema = z.object({
 			rateLimitSecondsMigrated: z.boolean().optional(),
 			diffSettingsMigrated: z.boolean().optional(),
 			openAiHeadersMigrated: z.boolean().optional(),
+			consecutiveMistakeLimitMigrated: z.boolean().optional(),
 		})
 		.optional(),
 })
@@ -48,6 +50,7 @@ export class ProviderSettingsManager {
 			rateLimitSecondsMigrated: true, // Mark as migrated on fresh installs
 			diffSettingsMigrated: true, // Mark as migrated on fresh installs
 			openAiHeadersMigrated: true, // Mark as migrated on fresh installs
+			consecutiveMistakeLimitMigrated: true, // Mark as migrated on fresh installs
 		},
 	}
 
@@ -113,6 +116,7 @@ export class ProviderSettingsManager {
 						rateLimitSecondsMigrated: false,
 						diffSettingsMigrated: false,
 						openAiHeadersMigrated: false,
+						consecutiveMistakeLimitMigrated: false,
 					} // Initialize with default values
 					isDirty = true
 				}
@@ -135,6 +139,12 @@ export class ProviderSettingsManager {
 					isDirty = true
 				}
 
+				if (!providerProfiles.migrations.consecutiveMistakeLimitMigrated) {
+					await this.migrateConsecutiveMistakeLimit(providerProfiles)
+					providerProfiles.migrations.consecutiveMistakeLimitMigrated = true
+					isDirty = true
+				}
+
 				if (isDirty) {
 					await this.store(providerProfiles)
 				}
@@ -228,6 +238,18 @@ export class ProviderSettingsManager {
 		}
 	}
 
+	private async migrateConsecutiveMistakeLimit(providerProfiles: ProviderProfiles) {
+		try {
+			for (const [name, apiConfig] of Object.entries(providerProfiles.apiConfigs)) {
+				if (apiConfig.consecutiveMistakeLimit == null) {
+					apiConfig.consecutiveMistakeLimit = DEFAULT_CONSECUTIVE_MISTAKE_LIMIT
+				}
+			}
+		} catch (error) {
+			console.error(`[MigrateConsecutiveMistakeLimit] Failed to migrate consecutive mistake limit:`, error)
+		}
+	}
+
 	/**
 	 * List all available configs with metadata.
 	 */

src/core/config/__tests__/ProviderSettingsManager.spec.ts

@@ -66,6 +66,7 @@ describe("ProviderSettingsManager", () => {
 						rateLimitSecondsMigrated: true,
 						diffSettingsMigrated: true,
 						openAiHeadersMigrated: true,
+						consecutiveMistakeLimitMigrated: true,
 					},
 				}),
 			)
@@ -144,6 +145,47 @@ describe("ProviderSettingsManager", () => {
 			expect(storedConfig.apiConfigs.existing.rateLimitSeconds).toEqual(43)
 		})
 
+		it("should call migrateConsecutiveMistakeLimit if it has not done so already", async () => {
+			mockSecrets.get.mockResolvedValue(
+				JSON.stringify({
+					currentApiConfigName: "default",
+					apiConfigs: {
+						default: {
+							config: {},
+							id: "default",
+							consecutiveMistakeLimit: undefined,
+						},
+						test: {
+							apiProvider: "anthropic",
+							consecutiveMistakeLimit: undefined,
+						},
+						existing: {
+							apiProvider: "anthropic",
+							// this should not really be possible, unless someone has loaded a hand edited config,
+							// but we don't overwrite so we'll check that
+							consecutiveMistakeLimit: 5,
+						},
+					},
+					migrations: {
+						rateLimitSecondsMigrated: true,
+						diffSettingsMigrated: true,
+						openAiHeadersMigrated: true,
+						consecutiveMistakeLimitMigrated: false,
+					},
+				}),
+			)
+
+			await providerSettingsManager.initialize()
+
+			// Get the last call to store, which should contain the migrated config
+			const calls = mockSecrets.store.mock.calls
+			const storedConfig = JSON.parse(calls[calls.length - 1][1])
+			expect(storedConfig.apiConfigs.default.consecutiveMistakeLimit).toEqual(3)
+			expect(storedConfig.apiConfigs.test.consecutiveMistakeLimit).toEqual(3)
+			expect(storedConfig.apiConfigs.existing.consecutiveMistakeLimit).toEqual(5)
+			expect(storedConfig.migrations.consecutiveMistakeLimitMigrated).toEqual(true)
+		})
+
 		it("should throw error if secrets storage fails", async () => {
 			mockSecrets.get.mockRejectedValue(new Error("Storage failed"))
 

src/core/task/Task.ts

@@ -18,6 +18,7 @@ import {
 	type ClineMessage,
 	type ClineSay,
 	type ToolProgressStatus,
+	DEFAULT_CONSECUTIVE_MISTAKE_LIMIT,
 	type HistoryItem,
 	TelemetryEventName,
 	TodoItem,
@@ -216,7 +217,7 @@ export class Task extends EventEmitter<ClineEvents> {
 		enableDiff = false,
 		enableCheckpoints = true,
 		fuzzyMatchThreshold = 1.0,
-		consecutiveMistakeLimit = 3,
+		consecutiveMistakeLimit = DEFAULT_CONSECUTIVE_MISTAKE_LIMIT,
 		task,
 		images,
 		historyItem,
@@ -255,7 +256,7 @@ export class Task extends EventEmitter<ClineEvents> {
 		this.browserSession = new BrowserSession(provider.context)
 		this.diffEnabled = enableDiff
 		this.fuzzyMatchThreshold = fuzzyMatchThreshold
-		this.consecutiveMistakeLimit = consecutiveMistakeLimit
+		this.consecutiveMistakeLimit = consecutiveMistakeLimit ?? DEFAULT_CONSECUTIVE_MISTAKE_LIMIT
 		this.providerRef = new WeakRef(provider)
 		this.globalStoragePath = provider.context.globalStorageUri.fsPath
 		this.diffViewProvider = new DiffViewProvider(this.cwd)
@@ -1159,7 +1160,7 @@ export class Task extends EventEmitter<ClineEvents> {
 			throw new Error(`[RooCode#recursivelyMakeRooRequests] task ${this.taskId}.${this.instanceId} aborted`)
 		}
 
-		if (this.consecutiveMistakeCount >= this.consecutiveMistakeLimit) {
+		if (this.consecutiveMistakeLimit > 0 && this.consecutiveMistakeCount >= this.consecutiveMistakeLimit) {
 			const { response, text, images } = await this.ask(
 				"mistake_limit_reached",
 				t("common:errors.mistake_limit_guidance"),

src/core/task/__tests__/Task.spec.ts

@@ -320,6 +320,70 @@ describe("Cline", () => {
 			expect(cline.diffStrategy).toBeDefined()
 		})
 
+		it("should use default consecutiveMistakeLimit when not provided", () => {
+			const cline = new Task({
+				provider: mockProvider,
+				apiConfiguration: mockApiConfig,
+				task: "test task",
+				startTask: false,
+			})
+
+			expect(cline.consecutiveMistakeLimit).toBe(3)
+		})
+
+		it("should respect provided consecutiveMistakeLimit", () => {
+			const cline = new Task({
+				provider: mockProvider,
+				apiConfiguration: mockApiConfig,
+				consecutiveMistakeLimit: 5,
+				task: "test task",
+				startTask: false,
+			})
+
+			expect(cline.consecutiveMistakeLimit).toBe(5)
+		})
+
+		it("should keep consecutiveMistakeLimit of 0 as 0 for unlimited", () => {
+			const cline = new Task({
+				provider: mockProvider,
+				apiConfiguration: mockApiConfig,
+				consecutiveMistakeLimit: 0,
+				task: "test task",
+				startTask: false,
+			})
+
+			expect(cline.consecutiveMistakeLimit).toBe(0)
+		})
+
+		it("should pass 0 to ToolRepetitionDetector for unlimited mode", () => {
+			const cline = new Task({
+				provider: mockProvider,
+				apiConfiguration: mockApiConfig,
+				consecutiveMistakeLimit: 0,
+				task: "test task",
+				startTask: false,
+			})
+
+			// The toolRepetitionDetector should be initialized with 0 for unlimited mode
+			expect(cline.toolRepetitionDetector).toBeDefined()
+			// Verify the limit remains as 0
+			expect(cline.consecutiveMistakeLimit).toBe(0)
+		})
+
+		it("should pass consecutiveMistakeLimit to ToolRepetitionDetector", () => {
+			const cline = new Task({
+				provider: mockProvider,
+				apiConfiguration: mockApiConfig,
+				consecutiveMistakeLimit: 5,
+				task: "test task",
+				startTask: false,
+			})
+
+			// The toolRepetitionDetector should be initialized with the same limit
+			expect(cline.toolRepetitionDetector).toBeDefined()
+			expect(cline.consecutiveMistakeLimit).toBe(5)
+		})
+
 		it("should require either task or historyItem", () => {
 			expect(() => {
 				new Task({ provider: mockProvider, apiConfiguration: mockApiConfig })

src/core/tools/ToolRepetitionDetector.ts

@@ -43,8 +43,11 @@ export class ToolRepetitionDetector {
 			this.previousToolCallJson = currentToolCallJson
 		}
 
-		// Check if limit is reached
-		if (this.consecutiveIdenticalToolCallCount >= this.consecutiveIdenticalToolCallLimit) {
+		// Check if limit is reached (0 means unlimited)
+		if (
+			this.consecutiveIdenticalToolCallLimit > 0 &&
+			this.consecutiveIdenticalToolCallCount >= this.consecutiveIdenticalToolCallLimit
+		) {
 			// Reset counters to allow recovery if user guides the AI past this point
 			this.consecutiveIdenticalToolCallCount = 0
 			this.previousToolCallJson = null

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

@@ -301,5 +301,61 @@ describe("ToolRepetitionDetector", () => {
 			expect(result3.allowExecution).toBe(false)
 			expect(result3.askUser).toBeDefined()
 		})
+
+		it("should never block when limit is 0 (unlimited)", () => {
+			const detector = new ToolRepetitionDetector(0)
+
+			// Try many identical calls
+			for (let i = 0; i < 10; i++) {
+				const result = detector.check(createToolUse("tool", "tool-name"))
+				expect(result.allowExecution).toBe(true)
+				expect(result.askUser).toBeUndefined()
+			}
+		})
+
+		it("should handle different limits correctly", () => {
+			// Test with limit of 5
+			const detector5 = new ToolRepetitionDetector(5)
+			const tool = createToolUse("tool", "tool-name")
+
+			// First 4 calls should be allowed
+			for (let i = 0; i < 4; i++) {
+				const result = detector5.check(tool)
+				expect(result.allowExecution).toBe(true)
+				expect(result.askUser).toBeUndefined()
+			}
+
+			// 5th call should be blocked
+			const result5 = detector5.check(tool)
+			expect(result5.allowExecution).toBe(false)
+			expect(result5.askUser).toBeDefined()
+			expect(result5.askUser?.messageKey).toBe("mistake_limit_reached")
+		})
+
+		it("should reset counter after blocking and allow new attempts", () => {
+			const detector = new ToolRepetitionDetector(2)
+			const tool = createToolUse("tool", "tool-name")
+
+			// First call allowed
+			expect(detector.check(tool).allowExecution).toBe(true)
+
+			// Second call should block (limit is 2)
+			const blocked = detector.check(tool)
+			expect(blocked.allowExecution).toBe(false)
+
+			// After blocking, counter should reset and allow new attempts
+			expect(detector.check(tool).allowExecution).toBe(true)
+		})
+
+		it("should handle negative limits as 0 (unlimited)", () => {
+			const detector = new ToolRepetitionDetector(-1)
+
+			// Should behave like unlimited
+			for (let i = 0; i < 5; i++) {
+				const result = detector.check(createToolUse("tool", "tool-name"))
+				expect(result.allowExecution).toBe(true)
+				expect(result.askUser).toBeUndefined()
+			}
+		})
 	})
 })

src/core/webview/ClineProvider.ts

@@ -553,6 +553,7 @@ export class ClineProvider
 			enableDiff,
 			enableCheckpoints,
 			fuzzyMatchThreshold,
+			consecutiveMistakeLimit: apiConfiguration.consecutiveMistakeLimit,
 			task,
 			images,
 			experiments,
@@ -589,6 +590,7 @@ export class ClineProvider
 			enableDiff,
 			enableCheckpoints,
 			fuzzyMatchThreshold,
+			consecutiveMistakeLimit: apiConfiguration.consecutiveMistakeLimit,
 			historyItem,
 			experiments,
 			rootTask: historyItem.rootTask,

webview-ui/src/components/settings/ApiOptions.tsx

@@ -6,6 +6,7 @@ import { VSCodeLink } from "@vscode/webview-ui-toolkit/react"
 import {
 	type ProviderName,
 	type ProviderSettings,
+	DEFAULT_CONSECUTIVE_MISTAKE_LIMIT,
 	openRouterDefaultModelId,
 	requestyDefaultModelId,
 	glamaDefaultModelId,
@@ -64,6 +65,7 @@ import { ThinkingBudget } from "./ThinkingBudget"
 import { DiffSettingsControl } from "./DiffSettingsControl"
 import { TemperatureControl } from "./TemperatureControl"
 import { RateLimitSecondsControl } from "./RateLimitSecondsControl"
+import { ConsecutiveMistakeLimitControl } from "./ConsecutiveMistakeLimitControl"
 import { BedrockCustomArn } from "./providers/BedrockCustomArn"
 import { buildDocLink } from "@src/utils/docLinks"
 
@@ -547,6 +549,14 @@ const ApiOptions = ({
 						value={apiConfiguration.rateLimitSeconds || 0}
 						onChange={(value) => setApiConfigurationField("rateLimitSeconds", value)}
 					/>
+					<ConsecutiveMistakeLimitControl
+						value={
+							apiConfiguration.consecutiveMistakeLimit !== undefined
+								? apiConfiguration.consecutiveMistakeLimit
+								: DEFAULT_CONSECUTIVE_MISTAKE_LIMIT
+						}
+						onChange={(value) => setApiConfigurationField("consecutiveMistakeLimit", value)}
+					/>
 				</>
 			)}
 		</div>