From 3081299c5e1e86e2584f46e4c173ac1337a3aba8 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Fri, 8 May 2026 16:37:50 +0000 Subject: [PATCH] fix: reset grace-retry counters on mistake limit and fix manual retry userMessageWasRemoved - Add missing userMessageWasRemoved flag to manual retry path for empty API responses (same fix as PR #12291). Without this, the user message is not re-added to conversation history on retry, causing cascading empty responses. - Reset consecutiveNoToolUseCount and consecutiveNoAssistantMessagesCount when the mistake_limit_reached dialog is shown and consecutiveMistakeCount is reset. Without this, these counters remain elevated after the user provides guidance, causing the very next no-tool-use or empty response to immediately show an error and re-increment consecutiveMistakeCount, creating a frustrating loop. - Add tests for counter reset behavior on mistake limit. Closes #12284 --- src/core/task/Task.ts | 8 +++ .../task/__tests__/grace-retry-errors.spec.ts | 64 +++++++++++++++++++ 2 files changed, 72 insertions(+) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 005bb0f292b..912cb2fc295 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -2559,6 +2559,12 @@ export class Task extends EventEmitter implements TaskLike { } this.consecutiveMistakeCount = 0 + // Also reset grace-retry counters so the model gets a fresh start + // after user provides guidance. Without this, the counters remain + // elevated and the very next no-tool-use or empty response would + // immediately show an error and re-increment consecutiveMistakeCount. + this.consecutiveNoToolUseCount = 0 + this.consecutiveNoAssistantMessagesCount = 0 } // Getting verbose details is an expensive operation, it uses ripgrep to @@ -3695,10 +3701,12 @@ export class Task extends EventEmitter implements TaskLike { await this.say("api_req_retried") // Push the same content back to retry + // Mark that user message was removed so it gets re-added on retry stack.push({ userContent: currentUserContent, includeFileDetails: false, retryAttempt: (currentItem.retryAttempt ?? 0) + 1, + userMessageWasRemoved: true, }) // Continue to retry the request diff --git a/src/core/task/__tests__/grace-retry-errors.spec.ts b/src/core/task/__tests__/grace-retry-errors.spec.ts index 283b402f69b..3f9f7284294 100644 --- a/src/core/task/__tests__/grace-retry-errors.spec.ts +++ b/src/core/task/__tests__/grace-retry-errors.spec.ts @@ -420,6 +420,70 @@ describe("Grace Retry Error Handling", () => { }) }) + describe("Grace-retry counter reset on mistake limit", () => { + it("should reset consecutiveNoToolUseCount when consecutiveMistakeCount is reset", () => { + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + // Simulate the state right after the mistake limit dialog resets consecutiveMistakeCount + // In the actual code, all three counters are reset together + task.consecutiveMistakeCount = 5 + task.consecutiveNoToolUseCount = 4 + task.consecutiveNoAssistantMessagesCount = 3 + + // Simulate the reset that happens in the mistake_limit_reached handler + task.consecutiveMistakeCount = 0 + task.consecutiveNoToolUseCount = 0 + task.consecutiveNoAssistantMessagesCount = 0 + + expect(task.consecutiveMistakeCount).toBe(0) + expect(task.consecutiveNoToolUseCount).toBe(0) + expect(task.consecutiveNoAssistantMessagesCount).toBe(0) + }) + + it("should allow grace retry period after mistake limit reset", () => { + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + // After mistake limit reset, counters should be at 0 + // This means the model gets a fresh grace retry period + task.consecutiveNoToolUseCount = 0 + task.consecutiveNoAssistantMessagesCount = 0 + + // First failure after reset: should be grace retry (no error shown) + task.consecutiveNoToolUseCount++ + expect(task.consecutiveNoToolUseCount).toBe(1) + expect(task.consecutiveNoToolUseCount >= 2).toBe(false) + + task.consecutiveNoAssistantMessagesCount++ + expect(task.consecutiveNoAssistantMessagesCount).toBe(1) + expect(task.consecutiveNoAssistantMessagesCount >= 2).toBe(false) + }) + + it("should show error on second failure after mistake limit reset", () => { + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + // Simulate first + second failure after reset + task.consecutiveNoToolUseCount = 1 + task.consecutiveNoToolUseCount++ + expect(task.consecutiveNoToolUseCount).toBe(2) + expect(task.consecutiveNoToolUseCount >= 2).toBe(true) + }) + }) + describe("Parallel with noToolsUsed error handling", () => { it("should have separate counters for noToolsUsed and noAssistantMessages", () => { const task = new Task({