fix(telemetry): advance baseline before emit to prevent double-reporting on throw

Author: edelauna

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

@@ -26,7 +26,7 @@ vi.mock("vscode", () => {
 	return { window, workspace, env, Uri, commands, ExtensionMode, version }
 })
 
-// Mock TelemetryService (needed by attemptCompletionTool's emitTaskCompleted)
+// Mock TelemetryService (needed by attemptCompletionTool's emitPublicTaskCompleted)
 vi.mock("@roo-code/telemetry", () => ({
 	TelemetryService: {
 		instance: {

src/core/task/Task.ts

@@ -4767,12 +4767,16 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 			return
 		}
 
-		this.emitFinalTokenUsageUpdate()
-		TelemetryService.instance.captureTaskCompleted(this.taskId, toolUsageDelta, messageCountDelta, reason)
-
+		// Advance the baseline before emitting so a synchronous throw from an
+		// EventEmitter listener or TelemetryService client cannot leave the baseline
+		// behind the running totals, which would cause the same delta to re-appear
+		// on the next flush. The delta values are already captured in locals above.
 		this.telemetryToolUsageBaseline = JSON.parse(JSON.stringify(this.toolUsage))
 		this.telemetryMessageCountsBaseline = { ...this.messageCounts }
 		this.lastTelemetryFlushAt = Date.now()
+
+		this.emitFinalTokenUsageUpdate()
+		TelemetryService.instance.captureTaskCompleted(this.taskId, toolUsageDelta, messageCountDelta, reason)
 	}
 
 	startIdleTelemetryCheck(): void {

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

@@ -3252,6 +3252,40 @@ describe("Telemetry installments (idle/shutdown flush)", () => {
 
 			expect(captureTaskCompletedSpy).not.toHaveBeenCalled()
 		})
+
+		it("does not re-flush when lastMessageTs is recent even after a prior flush", () => {
+			// Regression guard for the Math.max(lastMessageTs, lastTelemetryFlushAt) fix.
+			// Without it, once lastMessageTs is set it never advances after a flush, so
+			// the idle check would fire on every 5-minute tick for the rest of the task's
+			// life even with no new activity.
+			vi.useFakeTimers()
+			const task = createTask()
+			task.recordToolUsage("read_file")
+
+			// First idle flush fires after 31 min.
+			vi.advanceTimersByTime(31 * 60 * 1000)
+			expect(captureTaskCompletedSpy).toHaveBeenCalledTimes(1)
+			captureTaskCompletedSpy.mockClear()
+
+			// New activity arrives 1 min after the flush.
+			vi.advanceTimersByTime(1 * 60 * 1000)
+			task.lastMessageTs = Date.now()
+			task.recordToolUsage("write_to_file")
+
+			// Only 10 min since the new activity — should not flush again yet.
+			vi.advanceTimersByTime(10 * 60 * 1000)
+			expect(captureTaskCompletedSpy).not.toHaveBeenCalled()
+
+			// 35 min since the new activity (past the 30-min threshold and the next
+			// 5-min interval tick) — should flush the new delta now.
+			vi.advanceTimersByTime(25 * 60 * 1000)
+			expect(captureTaskCompletedSpy).toHaveBeenCalledWith(
+				task.taskId,
+				{ write_to_file: { attempts: 1, failures: 0 } },
+				{ user: 0, assistant: 0 },
+				"idle",
+			)
+		})
 	})
 
 	describe("dispose", () => {

src/core/tools/__tests__/attemptCompletionTool.spec.ts

@@ -591,7 +591,12 @@ describe("attemptCompletionTool", () => {
 				expect(mockPushToolResult).not.toHaveBeenCalledWith("")
 				// Emission now happens once per validated attempt_completion call, before
 				// delegation is attempted -- independent of whether delegation succeeds.
-				expect(mockCaptureTaskCompleted).toHaveBeenCalledTimes(1)
+				expect(mockCaptureTaskCompleted).toHaveBeenCalledWith(
+					mockTask.taskId,
+					mockTask.toolUsage,
+					mockTask.messageCounts,
+					"attempt_completion",
+				)
 			})
 
 			it("does not resume the parent when the parent is no longer awaiting this child", async () => {