fix: address PR review feedback for configurable error & repetition limit

- Simplified conversion logic by removing Infinity conversion
- Added validation for negative values in UI component
- Removed unnecessary console logging in migration function
- Added JSDoc comments for consecutiveMistakeLimit parameter
- Improved test coverage for ToolRepetitionDetector behavior
- Fixed UI issue where 0 was showing as 3 in ApiOptions

Author: daniel-lxs

src/core/config/ProviderSettingsManager.ts

@@ -243,10 +243,6 @@ export class ProviderSettingsManager {
 			for (const [name, apiConfig] of Object.entries(providerProfiles.apiConfigs)) {
 				if (apiConfig.consecutiveMistakeLimit == null) {
 					apiConfig.consecutiveMistakeLimit = DEFAULT_CONSECUTIVE_MISTAKE_LIMIT
-				} else {
-					console.log(
-						`[ProviderSettingsManager] Profile ${name} already has consecutiveMistakeLimit: ${apiConfig.consecutiveMistakeLimit}`,
-					)
 				}
 			}
 		} catch (error) {

src/core/task/Task.ts

@@ -256,7 +256,7 @@ export class Task extends EventEmitter<ClineEvents> {
 		this.browserSession = new BrowserSession(provider.context)
 		this.diffEnabled = enableDiff
 		this.fuzzyMatchThreshold = fuzzyMatchThreshold
-		this.consecutiveMistakeLimit = consecutiveMistakeLimit === 0 ? Infinity : (consecutiveMistakeLimit ?? 3)
+		this.consecutiveMistakeLimit = consecutiveMistakeLimit ?? DEFAULT_CONSECUTIVE_MISTAKE_LIMIT
 		this.providerRef = new WeakRef(provider)
 		this.globalStoragePath = provider.context.globalStorageUri.fsPath
 		this.diffViewProvider = new DiffViewProvider(this.cwd)
@@ -290,9 +290,7 @@ export class Task extends EventEmitter<ClineEvents> {
 			})
 		}
 
-		this.toolRepetitionDetector = new ToolRepetitionDetector(
-			this.consecutiveMistakeLimit === Infinity ? 0 : this.consecutiveMistakeLimit,
-		)
+		this.toolRepetitionDetector = new ToolRepetitionDetector(this.consecutiveMistakeLimit)
 
 		onCreated?.(this)
 
@@ -1162,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

@@ -343,7 +343,7 @@ describe("Cline", () => {
 			expect(cline.consecutiveMistakeLimit).toBe(5)
 		})
 
-		it("should convert consecutiveMistakeLimit of 0 to Infinity", () => {
+		it("should keep consecutiveMistakeLimit of 0 as 0 for unlimited", () => {
 			const cline = new Task({
 				provider: mockProvider,
 				apiConfiguration: mockApiConfig,
@@ -352,10 +352,10 @@ describe("Cline", () => {
 				startTask: false,
 			})
 
-			expect(cline.consecutiveMistakeLimit).toBe(Infinity)
+			expect(cline.consecutiveMistakeLimit).toBe(0)
 		})
 
-		it("should pass correct value to ToolRepetitionDetector when limit is Infinity", () => {
+		it("should pass 0 to ToolRepetitionDetector for unlimited mode", () => {
 			const cline = new Task({
 				provider: mockProvider,
 				apiConfiguration: mockApiConfig,
@@ -364,10 +364,10 @@ describe("Cline", () => {
 				startTask: false,
 			})
 
-			// The toolRepetitionDetector should be initialized with 0 when limit is Infinity (unlimited)
+			// The toolRepetitionDetector should be initialized with 0 for unlimited mode
 			expect(cline.toolRepetitionDetector).toBeDefined()
-			// We can't directly check the internal state, but we can verify the limit was converted
-			expect(cline.consecutiveMistakeLimit).toBe(Infinity)
+			// Verify the limit remains as 0
+			expect(cline.consecutiveMistakeLimit).toBe(0)
 		})
 
 		it("should pass consecutiveMistakeLimit to ToolRepetitionDetector", () => {

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

@@ -312,5 +312,50 @@ describe("ToolRepetitionDetector", () => {
 				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()
+			}
+		})
 	})
 })

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,
@@ -549,7 +550,11 @@ const ApiOptions = ({
 						onChange={(value) => setApiConfigurationField("rateLimitSeconds", value)}
 					/>
 					<ConsecutiveMistakeLimitControl
-						value={apiConfiguration.consecutiveMistakeLimit ?? 3}
+						value={
+							apiConfiguration.consecutiveMistakeLimit !== undefined
+								? apiConfiguration.consecutiveMistakeLimit
+								: DEFAULT_CONSECUTIVE_MISTAKE_LIMIT
+						}
 						onChange={(value) => setApiConfigurationField("consecutiveMistakeLimit", value)}
 					/>
 				</>

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

@@ -13,7 +13,9 @@ export const ConsecutiveMistakeLimitControl: React.FC<ConsecutiveMistakeLimitCon
 
 	const handleValueChange = useCallback(
 		(newValue: number) => {
-			onChange(newValue)
+			// Ensure value is not negative
+			const validValue = Math.max(0, newValue)
+			onChange(validValue)
 		},
 		[onChange],
 	)
@@ -29,7 +31,7 @@ export const ConsecutiveMistakeLimitControl: React.FC<ConsecutiveMistakeLimitCon
 					step={1}
 					onValueChange={(newValue) => handleValueChange(newValue[0])}
 				/>
-				<span className="w-10">{value ?? DEFAULT_CONSECUTIVE_MISTAKE_LIMIT}</span>
+				<span className="w-10">{Math.max(0, value ?? DEFAULT_CONSECUTIVE_MISTAKE_LIMIT)}</span>
 			</div>
 			<div className="text-sm text-vscode-descriptionForeground">
 				{value === 0