fix(reliability): bound empty assistant output retries

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.

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,6 +326,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 
 	// Tool Use
 	consecutiveMistakeCount: number = 0
+	consecutiveMistakeReason: ConsecutiveMistakeReason = "unknown"
 	consecutiveMistakeLimit: number
 	consecutiveMistakeCountForApplyDiff: Map<string, number> = new Map()
 	consecutiveMistakeCountForEditFile: Map<string, number> = new Map()
@@ -2720,16 +2722,14 @@ export class Task extends EventEmitter<TaskEvents> 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(
 						`Task reached consecutive mistake limit (${this.consecutiveMistakeLimit})`,
 						this.taskId,
 						this.consecutiveMistakeCount,
 						this.consecutiveMistakeLimit,
-						"no_tools_used",
+						this.consecutiveMistakeReason,
 						this.apiConfiguration.apiProvider,
 						getModelId(this.apiConfiguration),
 					),
@@ -2752,6 +2752,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 				}
 
 				this.consecutiveMistakeCount = 0
+				this.consecutiveMistakeReason = "unknown"
 			}
 
 			// Getting verbose details is an expensive operation, it uses ripgrep to
@@ -3691,15 +3692,7 @@ export class Task extends EventEmitter<TaskEvents> 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 +3722,7 @@ export class Task extends EventEmitter<TaskEvents> 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 +4474,30 @@ export class Task extends EventEmitter<TaskEvents> 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<void> {
+		this.consecutiveNoAssistantMessagesCount++
+		if (this.consecutiveNoAssistantMessagesCount >= 2) {
+			await this.say("error", "MODEL_NO_ASSISTANT_MESSAGES")
+			this.consecutiveMistakeReason = "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<void> {
+		this.consecutiveNoToolUseCount++
+		if (this.consecutiveNoToolUseCount >= 2) {
+			await this.say("error", "MODEL_NO_TOOLS_USED")
+			this.consecutiveMistakeReason = "no_tools_used"
+			this.consecutiveMistakeCount++
+		}
+	}
+
 	// Shared exponential backoff for retries (first-chunk and mid-stream)
 	private async backoffAndAnnounce(retryAttempt: number, error: any): Promise<void> {
 		try {

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,100 @@ 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)
+		})
+
+		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")
+		})
+	})
 })