fix(telemetry): prevent idle timer from breaking tests with fake timers

Author: edelauna

src/__tests__/history-resume-delegation.spec.ts

@@ -1255,6 +1255,7 @@ describe("History resume delegation - parent metadata transitions", () => {
 				userMessageContent: [],
 				consecutiveMistakeCount: 0,
 				emitFinalTokenUsageUpdate: vi.fn(),
+				flushTelemetryInstallment: vi.fn(),
 			} as unknown as import("../core/task/Task").Task
 
 			const block = {

src/__tests__/nested-delegation-resume.spec.ts

@@ -203,6 +203,7 @@ describe("Nested delegation resume (A → B → C)", () => {
 			userMessageContent: [],
 			consecutiveMistakeCount: 0,
 			emitFinalTokenUsageUpdate: vi.fn(),
+			flushTelemetryInstallment: vi.fn(),
 		} as unknown as Task
 
 		const blockC = {
@@ -250,6 +251,7 @@ describe("Nested delegation resume (A → B → C)", () => {
 			userMessageContent: [],
 			consecutiveMistakeCount: 0,
 			emitFinalTokenUsageUpdate: vi.fn(),
+			flushTelemetryInstallment: vi.fn(),
 		} as unknown as Task
 
 		const blockB = {

src/core/task/Task.ts

@@ -617,12 +617,11 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 			{ leading: true, trailing: true, maxWait: this.TOKEN_USAGE_EMIT_INTERVAL_MS },
 		)
 
-		this.startIdleTelemetryCheck()
-
 		onCreated?.(this)
 
 		if (startTask) {
 			this._started = true
+			this.startIdleTelemetryCheck()
 			if (task || images) {
 				void this.startTask(task, images).catch((error) => {
 					console.error("[Task#constructor] startTask failed:", error)
@@ -1900,6 +1899,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 			return
 		}
 		this._started = true
+		this.startIdleTelemetryCheck()
 
 		const { task, images } = this.metadata
 
@@ -3697,8 +3697,14 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 							// userMessageWasRemoved so the message (and its count) is restored
 							// exactly once when the retry succeeds, keeping the total symmetric
 							// regardless of how many empty-response cycles occur first.
+							// Guard: only reverse a count this iteration actually added. The
+							// popped message may predate this turn (resumed history, or a
+							// message appended by flushPendingToolResultsToHistory, neither
+							// of which incremented messageCounts.user).
 							this.apiConversationHistory.pop()
-							this.messageCounts.user--
+							if (shouldAddUserMessage) {
+								this.messageCounts.user--
+							}
 						}
 					}
 
@@ -4755,8 +4761,8 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 		}
 
 		const messageCountDelta = {
-			user: this.messageCounts.user - this.telemetryMessageCountsBaseline.user,
-			assistant: this.messageCounts.assistant - this.telemetryMessageCountsBaseline.assistant,
+			user: Math.max(0, this.messageCounts.user - this.telemetryMessageCountsBaseline.user),
+			assistant: Math.max(0, this.messageCounts.assistant - this.telemetryMessageCountsBaseline.assistant),
 		}
 
 		const hasToolUsageDelta = Object.keys(toolUsageDelta).length > 0
@@ -4774,7 +4780,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 		this.lastTelemetryFlushAt = Date.now()
 	}
 
-	private startIdleTelemetryCheck(): void {
+	startIdleTelemetryCheck(): void {
 		this.idleTelemetryCheckInterval = setInterval(() => {
 			// lastMessageTs only moves forward on activity, so comparing it against the
 			// last flush tells us whether anything happened since that flush -- if the

src/core/task/__tests__/Task.spec.ts

@@ -3124,12 +3124,14 @@ describe("Telemetry installments (idle/shutdown flush)", () => {
 	})
 
 	function createTask() {
-		return new Task({
+		const task = new Task({
 			provider: mockProvider,
 			apiConfiguration: mockApiConfig,
 			task: "test task",
 			startTask: false,
 		})
+		task.startIdleTelemetryCheck()
+		return task
 	}
 
 	describe("flushTelemetryInstallment", () => {

src/core/task/__tests__/messageCounting.retrySymmetry.spec.ts

@@ -25,10 +25,17 @@ describe("empty-assistant-response retry keeps messageCounts.user symmetric", ()
 		}
 	}
 
-	function simulatePopOnEmptyResponse(messageCounts: { user: number; assistant: number }) {
-		// Mirrors Task.ts: popping the just-added user message from apiConversationHistory
-		// is paired with decrementing messageCounts.user to match.
-		messageCounts.user--
+	function simulatePopOnEmptyResponse(
+		messageCounts: { user: number; assistant: number },
+		shouldAddUserMessage = true,
+	) {
+		// Mirrors Task.ts: pop the last user message from apiConversationHistory and
+		// decrement messageCounts.user only if this iteration actually incremented it.
+		// Messages added by flushPendingToolResultsToHistory or loaded from resumed
+		// history are not counted, so popping them must not decrement.
+		if (shouldAddUserMessage) {
+			messageCounts.user--
+		}
 	}
 
 	function simulateDeclineRetry(messageCounts: { user: number; assistant: number }) {
@@ -103,4 +110,22 @@ describe("empty-assistant-response retry keeps messageCounts.user symmetric", ()
 		simulateDeclineRetry(messageCounts)
 		expect(messageCounts).toEqual({ user: 1, assistant: 1 })
 	})
+
+	it("does not go negative when the popped message was not counted by this iteration (e.g. flushPendingToolResultsToHistory)", () => {
+		// Scenario: a user message was appended to apiConversationHistory by
+		// flushPendingToolResultsToHistory (or loaded from resumed history) without
+		// incrementing messageCounts.user. shouldAddUserMessage is false for this
+		// turn (empty content). The assistant returns nothing and the pop fires.
+		// The count must stay at 0, not go to -1.
+		const messageCounts = { user: 0, assistant: 0 }
+
+		// This turn: empty content, so shouldAddUserMessageToHistory returns false.
+		const shouldAddUserMessage = false
+		simulateAttempt(messageCounts, { retryAttempt: 0, isEmptyUserContent: true, userMessageWasRemoved: false })
+		expect(messageCounts.user).toBe(0)
+
+		// Assistant returns nothing -- pop fires but guard skips the decrement.
+		simulatePopOnEmptyResponse(messageCounts, shouldAddUserMessage)
+		expect(messageCounts.user).toBe(0)
+	})
 })