From 15a40c4879ab27f18aad76ee222e0d4d3caf730f Mon Sep 17 00:00:00 2001 From: 0xMink Date: Tue, 10 Feb 2026 23:22:25 -0500 Subject: [PATCH 1/3] fix(reliability): bound empty assistant output retries Route repeated empty assistant outputs (no text, no tool calls) into existing mistake-limit handling. Previously the empty-response path incremented consecutiveNoAssistantMessagesCount but never consecutiveMistakeCount, allowing unbounded auto-retry under auto-approval. Extract onEmptyAssistantOutput() and onNoToolUse() helpers to keep retry accounting consistent and testable. --- src/core/task/Task.ts | 41 ++++--- .../task/__tests__/grace-retry-errors.spec.ts | 115 ++++++++++++------ 2 files changed, 102 insertions(+), 54 deletions(-) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 9d27c4b90b0..58097e76161 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -3691,15 +3691,7 @@ export class Task extends EventEmitter implements TaskLike { ) if (!didToolUse) { - // Increment consecutive no-tool-use counter - this.consecutiveNoToolUseCount++ - - // Only show error and count toward mistake limit after 2 consecutive failures - if (this.consecutiveNoToolUseCount >= 2) { - await this.say("error", "MODEL_NO_TOOLS_USED") - // Only count toward mistake limit after second consecutive failure - this.consecutiveMistakeCount++ - } + await this.onNoToolUse() // Use the task's locked protocol for consistent behavior this.userMessageContent.push({ @@ -3729,14 +3721,7 @@ export class Task extends EventEmitter implements TaskLike { // or tool_use content blocks from API which we should assume is // an error. - // Increment consecutive no-assistant-messages counter - this.consecutiveNoAssistantMessagesCount++ - - // Only show error and count toward mistake limit after 2 consecutive failures - // This provides a "grace retry" - first failure retries silently - if (this.consecutiveNoAssistantMessagesCount >= 2) { - await this.say("error", "MODEL_NO_ASSISTANT_MESSAGES") - } + await this.onEmptyAssistantOutput() // IMPORTANT: We already added the user message to // apiConversationHistory at line 1876. Since the assistant failed to respond, @@ -4488,6 +4473,28 @@ export class Task extends EventEmitter implements TaskLike { yield* iterator } + // Increment counters and show error when the model returns no text and no tool calls. + // The first empty response is a silent "grace retry"; from the second onward we show + // an error and feed into the mistake-limit machinery so auto-retry is bounded. + private async onEmptyAssistantOutput(): Promise { + this.consecutiveNoAssistantMessagesCount++ + if (this.consecutiveNoAssistantMessagesCount >= 2) { + await this.say("error", "MODEL_NO_ASSISTANT_MESSAGES") + this.consecutiveMistakeCount++ + } + } + + // Increment counters and show error when the model returns text but no tool calls. + // Same grace-retry pattern: first occurrence is silent, second onward triggers error + // and feeds into the mistake-limit machinery. + private async onNoToolUse(): Promise { + this.consecutiveNoToolUseCount++ + if (this.consecutiveNoToolUseCount >= 2) { + await this.say("error", "MODEL_NO_TOOLS_USED") + this.consecutiveMistakeCount++ + } + } + // Shared exponential backoff for retries (first-chunk and mid-stream) private async backoffAndAnnounce(retryAttempt: number, error: any): Promise { try { diff --git a/src/core/task/__tests__/grace-retry-errors.spec.ts b/src/core/task/__tests__/grace-retry-errors.spec.ts index 283b402f69b..0ad8be27c0f 100644 --- a/src/core/task/__tests__/grace-retry-errors.spec.ts +++ b/src/core/task/__tests__/grace-retry-errors.spec.ts @@ -277,7 +277,7 @@ describe("Grace Retry Error Handling", () => { }) }) - describe("Grace Retry Pattern", () => { + describe("Grace Retry Pattern (via production helper)", () => { it("should not show error on first failure (grace retry)", async () => { const task = new Task({ provider: mockProvider, @@ -288,17 +288,10 @@ describe("Grace Retry Error Handling", () => { const saySpy = vi.spyOn(task, "say").mockResolvedValue(undefined) - // Simulate first empty response - should NOT show error - task.consecutiveNoAssistantMessagesCount = 0 - task.consecutiveNoAssistantMessagesCount++ - expect(task.consecutiveNoAssistantMessagesCount).toBe(1) - - // First failure: grace retry (silent) - if (task.consecutiveNoAssistantMessagesCount >= 2) { - await task.say("error", "MODEL_NO_ASSISTANT_MESSAGES") - } + // First call to the production helper — grace retry (silent) + await (task as any).onEmptyAssistantOutput() - // Verify error was NOT called (grace retry on first failure) + expect(task.consecutiveNoAssistantMessagesCount).toBe(1) expect(saySpy).not.toHaveBeenCalledWith("error", "MODEL_NO_ASSISTANT_MESSAGES") }) @@ -312,17 +305,11 @@ describe("Grace Retry Error Handling", () => { const saySpy = vi.spyOn(task, "say").mockResolvedValue(undefined) - // Simulate second consecutive empty response - task.consecutiveNoAssistantMessagesCount = 1 - task.consecutiveNoAssistantMessagesCount++ - expect(task.consecutiveNoAssistantMessagesCount).toBe(2) - - // Second failure: should show error - if (task.consecutiveNoAssistantMessagesCount >= 2) { - await task.say("error", "MODEL_NO_ASSISTANT_MESSAGES") - } + // Two consecutive calls to the production helper + await (task as any).onEmptyAssistantOutput() + await (task as any).onEmptyAssistantOutput() - // Verify error was called (after 2 consecutive failures) + expect(task.consecutiveNoAssistantMessagesCount).toBe(2) expect(saySpy).toHaveBeenCalledWith("error", "MODEL_NO_ASSISTANT_MESSAGES") }) @@ -336,17 +323,14 @@ describe("Grace Retry Error Handling", () => { const saySpy = vi.spyOn(task, "say").mockResolvedValue(undefined) - // Simulate third consecutive empty response - task.consecutiveNoAssistantMessagesCount = 2 - task.consecutiveNoAssistantMessagesCount++ - expect(task.consecutiveNoAssistantMessagesCount).toBe(3) + // Three consecutive calls + await (task as any).onEmptyAssistantOutput() + await (task as any).onEmptyAssistantOutput() + await (task as any).onEmptyAssistantOutput() - // Third failure: should also show error - if (task.consecutiveNoAssistantMessagesCount >= 2) { - await task.say("error", "MODEL_NO_ASSISTANT_MESSAGES") - } - - // Verify error was called + expect(task.consecutiveNoAssistantMessagesCount).toBe(3) + // Error called on 2nd and 3rd invocations + expect(saySpy).toHaveBeenCalledTimes(2) expect(saySpy).toHaveBeenCalledWith("error", "MODEL_NO_ASSISTANT_MESSAGES") }) }) @@ -408,12 +392,9 @@ describe("Grace Retry Error Handling", () => { const saySpy = vi.spyOn(task, "say").mockResolvedValue(undefined) - // Simulate the error condition (2 consecutive failures) - task.consecutiveNoAssistantMessagesCount = 2 - - if (task.consecutiveNoAssistantMessagesCount >= 2) { - await task.say("error", "MODEL_NO_ASSISTANT_MESSAGES") - } + // Call production helper twice to trigger error path + await (task as any).onEmptyAssistantOutput() + await (task as any).onEmptyAssistantOutput() // Verify the exact marker is used expect(saySpy).toHaveBeenCalledWith("error", "MODEL_NO_ASSISTANT_MESSAGES") @@ -441,4 +422,64 @@ describe("Grace Retry Error Handling", () => { expect(task.consecutiveNoToolUseCount).toBe(3) }) }) + + describe("Empty-response mistake limit bounding", () => { + it("should increment consecutiveMistakeCount after 2 consecutive empty responses", async () => { + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + vi.spyOn(task, "say").mockResolvedValue(undefined) + expect(task.consecutiveMistakeCount).toBe(0) + + // Two calls to the production helper — second triggers mistake count + await (task as any).onEmptyAssistantOutput() + expect(task.consecutiveMistakeCount).toBe(0) // grace retry: no increment yet + + await (task as any).onEmptyAssistantOutput() + expect(task.consecutiveMistakeCount).toBe(1) + }) + + it("should NOT increment consecutiveMistakeCount on first empty response (grace retry)", async () => { + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + const saySpy = vi.spyOn(task, "say").mockResolvedValue(undefined) + + // Single call — grace retry, no error, no mistake count + await (task as any).onEmptyAssistantOutput() + + expect(task.consecutiveMistakeCount).toBe(0) + expect(saySpy).not.toHaveBeenCalled() + }) + + it("should confirm no-tool-use path increments consecutiveMistakeCount independently", async () => { + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + const saySpy = vi.spyOn(task, "say").mockResolvedValue(undefined) + expect(task.consecutiveMistakeCount).toBe(0) + + // Two calls to the no-tool-use production helper + await (task as any).onNoToolUse() + await (task as any).onNoToolUse() + + expect(saySpy).toHaveBeenCalledWith("error", "MODEL_NO_TOOLS_USED") + expect(task.consecutiveMistakeCount).toBe(1) + + // Empty-response counter is unaffected + expect(task.consecutiveNoAssistantMessagesCount).toBe(0) + }) + }) }) From 51e9866e9c6b202bb7885e09e6ba2e7193054b76 Mon Sep 17 00:00:00 2001 From: 0xMink Date: Tue, 10 Feb 2026 23:34:58 -0500 Subject: [PATCH 2/3] fix(telemetry): distinguish consecutive mistake reason Record which failure path triggered the mistake count increment (no_tools_used vs no_assistant_messages) instead of hardcoding "no_tools_used" in ConsecutiveMistakeError telemetry. --- src/core/task/Task.ts | 8 +++-- .../task/__tests__/grace-retry-errors.spec.ts | 36 +++++++++++++++++++ 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 58097e76161..cf216301715 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -325,6 +325,7 @@ export class Task extends EventEmitter implements TaskLike { // Tool Use consecutiveMistakeCount: number = 0 + consecutiveMistakeReason: "no_tools_used" | "no_assistant_messages" | "unknown" = "unknown" consecutiveMistakeLimit: number consecutiveMistakeCountForApplyDiff: Map = new Map() consecutiveMistakeCountForEditFile: Map = new Map() @@ -2720,8 +2721,6 @@ export class Task extends EventEmitter implements TaskLike { if (this.consecutiveMistakeLimit > 0 && this.consecutiveMistakeCount >= this.consecutiveMistakeLimit) { // Track consecutive mistake errors in telemetry via event and PostHog exception tracking. - // The reason is "no_tools_used" because this limit is reached via initiateTaskLoop - // which increments consecutiveMistakeCount when the model doesn't use any tools. TelemetryService.instance.captureConsecutiveMistakeError(this.taskId) TelemetryService.instance.captureException( new ConsecutiveMistakeError( @@ -2729,7 +2728,7 @@ export class Task extends EventEmitter implements TaskLike { this.taskId, this.consecutiveMistakeCount, this.consecutiveMistakeLimit, - "no_tools_used", + this.consecutiveMistakeReason, this.apiConfiguration.apiProvider, getModelId(this.apiConfiguration), ), @@ -2752,6 +2751,7 @@ export class Task extends EventEmitter implements TaskLike { } this.consecutiveMistakeCount = 0 + this.consecutiveMistakeReason = "unknown" } // Getting verbose details is an expensive operation, it uses ripgrep to @@ -4480,6 +4480,7 @@ export class Task extends EventEmitter implements TaskLike { this.consecutiveNoAssistantMessagesCount++ if (this.consecutiveNoAssistantMessagesCount >= 2) { await this.say("error", "MODEL_NO_ASSISTANT_MESSAGES") + this.consecutiveMistakeReason = "no_assistant_messages" this.consecutiveMistakeCount++ } } @@ -4491,6 +4492,7 @@ export class Task extends EventEmitter implements TaskLike { this.consecutiveNoToolUseCount++ if (this.consecutiveNoToolUseCount >= 2) { await this.say("error", "MODEL_NO_TOOLS_USED") + this.consecutiveMistakeReason = "no_tools_used" this.consecutiveMistakeCount++ } } diff --git a/src/core/task/__tests__/grace-retry-errors.spec.ts b/src/core/task/__tests__/grace-retry-errors.spec.ts index 0ad8be27c0f..d230f97ef94 100644 --- a/src/core/task/__tests__/grace-retry-errors.spec.ts +++ b/src/core/task/__tests__/grace-retry-errors.spec.ts @@ -481,5 +481,41 @@ describe("Grace Retry Error Handling", () => { // Empty-response counter is unaffected expect(task.consecutiveNoAssistantMessagesCount).toBe(0) }) + + it("should set consecutiveMistakeReason to no_assistant_messages after empty-output threshold", async () => { + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + vi.spyOn(task, "say").mockResolvedValue(undefined) + expect(task.consecutiveMistakeReason).toBe("unknown") + + // First call — grace retry, reason stays unknown + await (task as any).onEmptyAssistantOutput() + expect(task.consecutiveMistakeReason).toBe("unknown") + + // Second call — reason set + await (task as any).onEmptyAssistantOutput() + expect(task.consecutiveMistakeReason).toBe("no_assistant_messages") + }) + + it("should set consecutiveMistakeReason to no_tools_used after no-tool-use threshold", async () => { + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + vi.spyOn(task, "say").mockResolvedValue(undefined) + expect(task.consecutiveMistakeReason).toBe("unknown") + + await (task as any).onNoToolUse() + await (task as any).onNoToolUse() + expect(task.consecutiveMistakeReason).toBe("no_tools_used") + }) }) }) From 6d410772f71b546ccbb164d90c18ebbb04168047 Mon Sep 17 00:00:00 2001 From: 0xMink Date: Wed, 11 Feb 2026 01:34:45 -0500 Subject: [PATCH 3/3] fix(types): add no_assistant_messages to ConsecutiveMistakeReason union The inline union on Task.consecutiveMistakeReason included "no_assistant_messages" but the canonical ConsecutiveMistakeReason type in @roo-code/types did not, causing TS2345 compile failure. - Add "no_assistant_messages" to the shared type - Import ConsecutiveMistakeReason in Task.ts instead of inline union --- packages/types/src/telemetry.ts | 2 +- src/core/task/Task.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/types/src/telemetry.ts b/packages/types/src/telemetry.ts index 68ed38fe326..f7fa48ba57a 100644 --- a/packages/types/src/telemetry.ts +++ b/packages/types/src/telemetry.ts @@ -483,7 +483,7 @@ export function extractApiProviderErrorProperties(error: ApiProviderError): Reco /** * Reason why the consecutive mistake limit was reached. */ -export type ConsecutiveMistakeReason = "no_tools_used" | "tool_repetition" | "unknown" +export type ConsecutiveMistakeReason = "no_tools_used" | "tool_repetition" | "no_assistant_messages" | "unknown" /** * Error class for "Roo is having trouble" consecutive mistake scenarios. diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index cf216301715..0c69bf69a74 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -51,6 +51,7 @@ import { MAX_CHECKPOINT_TIMEOUT_SECONDS, MIN_CHECKPOINT_TIMEOUT_SECONDS, ConsecutiveMistakeError, + type ConsecutiveMistakeReason, MAX_MCP_TOOLS_THRESHOLD, countEnabledMcpTools, } from "@roo-code/types" @@ -325,7 +326,7 @@ export class Task extends EventEmitter implements TaskLike { // Tool Use consecutiveMistakeCount: number = 0 - consecutiveMistakeReason: "no_tools_used" | "no_assistant_messages" | "unknown" = "unknown" + consecutiveMistakeReason: ConsecutiveMistakeReason = "unknown" consecutiveMistakeLimit: number consecutiveMistakeCountForApplyDiff: Map = new Map() consecutiveMistakeCountForEditFile: Map = new Map()