Skip to content
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions packages/types/src/provider-settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
26 changes: 26 additions & 0 deletions src/core/config/ProviderSettingsManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
type ProviderSettingsEntry,
providerSettingsSchema,
providerSettingsSchemaDiscriminated,
DEFAULT_CONSECUTIVE_MISTAKE_LIMIT,
} from "@roo-code/types"
import { TelemetryService } from "@roo-code/telemetry"

Expand All @@ -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(),
})
Expand All @@ -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
},
}

Expand Down Expand Up @@ -113,6 +116,7 @@ export class ProviderSettingsManager {
rateLimitSecondsMigrated: false,
diffSettingsMigrated: false,
openAiHeadersMigrated: false,
consecutiveMistakeLimitMigrated: false,
} // Initialize with default values
isDirty = true
}
Expand All @@ -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)
}
Expand Down Expand Up @@ -228,6 +238,22 @@ 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
} else {
console.log(
`[ProviderSettingsManager] Profile ${name} already has consecutiveMistakeLimit: ${apiConfig.consecutiveMistakeLimit}`,
)
}
}
} catch (error) {
console.error(`[MigrateConsecutiveMistakeLimit] Failed to migrate consecutive mistake limit:`, error)
}
}

/**
* List all available configs with metadata.
*/
Expand Down
42 changes: 42 additions & 0 deletions src/core/config/__tests__/ProviderSettingsManager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ describe("ProviderSettingsManager", () => {
rateLimitSecondsMigrated: true,
diffSettingsMigrated: true,
openAiHeadersMigrated: true,
consecutiveMistakeLimitMigrated: true,
},
}),
)
Expand Down Expand Up @@ -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"))

Expand Down
9 changes: 6 additions & 3 deletions src/core/task/Task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
type ClineMessage,
type ClineSay,
type ToolProgressStatus,
DEFAULT_CONSECUTIVE_MISTAKE_LIMIT,
type HistoryItem,
TelemetryEventName,
TodoItem,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 === 0 ? Infinity : (consecutiveMistakeLimit ?? 3)
this.providerRef = new WeakRef(provider)
this.globalStoragePath = provider.context.globalStorageUri.fsPath
this.diffViewProvider = new DiffViewProvider(this.cwd)
Expand Down Expand Up @@ -289,7 +290,9 @@ export class Task extends EventEmitter<ClineEvents> {
})
}

this.toolRepetitionDetector = new ToolRepetitionDetector(this.consecutiveMistakeLimit)
this.toolRepetitionDetector = new ToolRepetitionDetector(
this.consecutiveMistakeLimit === Infinity ? 0 : this.consecutiveMistakeLimit,
)

onCreated?.(this)

Expand Down
64 changes: 64 additions & 0 deletions src/core/task/__tests__/Task.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 convert consecutiveMistakeLimit of 0 to Infinity", () => {
const cline = new Task({
provider: mockProvider,
apiConfiguration: mockApiConfig,
consecutiveMistakeLimit: 0,
task: "test task",
startTask: false,
})

expect(cline.consecutiveMistakeLimit).toBe(Infinity)
})

it("should pass correct value to ToolRepetitionDetector when limit is Infinity", () => {
const cline = new Task({
provider: mockProvider,
apiConfiguration: mockApiConfig,
consecutiveMistakeLimit: 0,
task: "test task",
startTask: false,
})

// The toolRepetitionDetector should be initialized with 0 when limit is Infinity (unlimited)
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)
})

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 })
Expand Down
7 changes: 5 additions & 2 deletions src/core/tools/ToolRepetitionDetector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions src/core/tools/__tests__/ToolRepetitionDetector.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -301,5 +301,16 @@ 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()
}
})
})
})
2 changes: 2 additions & 0 deletions src/core/webview/ClineProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -553,6 +553,7 @@ export class ClineProvider
enableDiff,
enableCheckpoints,
fuzzyMatchThreshold,
consecutiveMistakeLimit: apiConfiguration.consecutiveMistakeLimit,
task,
images,
experiments,
Expand Down Expand Up @@ -589,6 +590,7 @@ export class ClineProvider
enableDiff,
enableCheckpoints,
fuzzyMatchThreshold,
consecutiveMistakeLimit: apiConfiguration.consecutiveMistakeLimit,
historyItem,
experiments,
rootTask: historyItem.rootTask,
Expand Down
5 changes: 5 additions & 0 deletions webview-ui/src/components/settings/ApiOptions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,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"

Expand Down Expand Up @@ -547,6 +548,10 @@ const ApiOptions = ({
value={apiConfiguration.rateLimitSeconds || 0}
onChange={(value) => setApiConfigurationField("rateLimitSeconds", value)}
/>
<ConsecutiveMistakeLimitControl
value={apiConfiguration.consecutiveMistakeLimit ?? 3}
onChange={(value) => setApiConfigurationField("consecutiveMistakeLimit", value)}
/>
</>
)}
</div>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import React, { useCallback } from "react"
import { Slider } from "@/components/ui"
import { useAppTranslation } from "@/i18n/TranslationContext"
import { DEFAULT_CONSECUTIVE_MISTAKE_LIMIT } from "@roo-code/types"

interface ConsecutiveMistakeLimitControlProps {
value: number
onChange: (value: number) => void
}

export const ConsecutiveMistakeLimitControl: React.FC<ConsecutiveMistakeLimitControlProps> = ({ value, onChange }) => {
const { t } = useAppTranslation()

const handleValueChange = useCallback(
(newValue: number) => {
onChange(newValue)
},
[onChange],
)

return (
<div className="flex flex-col gap-1">
<label className="block font-medium mb-1">{t("settings:providers.consecutiveMistakeLimit.label")}</label>
<div className="flex items-center gap-2">
<Slider
value={[value ?? DEFAULT_CONSECUTIVE_MISTAKE_LIMIT]}
min={0}
max={10}
step={1}
onValueChange={(newValue) => handleValueChange(newValue[0])}
/>
<span className="w-10">{value ?? DEFAULT_CONSECUTIVE_MISTAKE_LIMIT}</span>
</div>
<div className="text-sm text-vscode-descriptionForeground">
{value === 0
? t("settings:providers.consecutiveMistakeLimit.unlimitedDescription")
: t("settings:providers.consecutiveMistakeLimit.description", {
value: value ?? DEFAULT_CONSECUTIVE_MISTAKE_LIMIT,
})}
</div>
{value === 0 && (
<div className="text-sm text-vscode-errorForeground mt-1">
{t("settings:providers.consecutiveMistakeLimit.warning")}
</div>
)}
</div>
)
}
6 changes: 6 additions & 0 deletions webview-ui/src/i18n/locales/ca/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,12 @@
"label": "Límit de freqüència",
"description": "Temps mínim entre sol·licituds d'API."
},
"consecutiveMistakeLimit": {
"label": "Límit d'errors i repeticions",
"description": "Nombre d'errors consecutius o accions repetides abans de mostrar el diàleg 'En Roo està tenint problemes'",
"unlimitedDescription": "Reintents il·limitats habilitats (procediment automàtic). El diàleg no apareixerà mai.",
"warning": "⚠️ Establir a 0 permet reintents il·limitats que poden consumir un ús significatiu de l'API"
},
"reasoningEffort": {
"label": "Esforç de raonament del model",
"high": "Alt",
Expand Down
6 changes: 6 additions & 0 deletions webview-ui/src/i18n/locales/de/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,12 @@
"label": "Ratenbegrenzung",
"description": "Minimale Zeit zwischen API-Anfragen."
},
"consecutiveMistakeLimit": {
"label": "Fehler- & Wiederholungslimit",
"description": "Anzahl aufeinanderfolgender Fehler oder wiederholter Aktionen, bevor der Dialog 'Roo hat Probleme' angezeigt wird",
"unlimitedDescription": "Unbegrenzte Wiederholungen aktiviert (automatisches Fortfahren). Der Dialog wird niemals angezeigt.",
"warning": "⚠️ Das Setzen auf 0 erlaubt unbegrenzte Wiederholungen, was zu erheblichem API-Verbrauch führen kann"
},
"reasoningEffort": {
"label": "Modell-Denkaufwand",
"high": "Hoch",
Expand Down
Loading