fix(telemetry): add circuit breaker and shutdown draining to TelemetryService

[edelauna]

Related GitHub Issue

Refs: #830 (reliability/event-volume-protection half — split from #835; consent/privacy half shipped separately in #1069)

Description

TelemetryService had two reliability gaps:

1. No upper bound on retry-storm events. A CODE_INDEX_ERROR retry loop (e.g. a broken embedder config re-triggering on every file-system event) could flood the PostHog Product Analytics quota with no circuit breaker.
2. Unbounded, un-awaited shutdown. deactivate() called TelemetryService.instance.shutdown() without awaiting it, and shutdown() itself just fired client.shutdown() on each client without waiting for any in-flight capture()/captureException() calls to finish first — a capture that started just before shutdown could be silently dropped.

This PR adds:

• Circuit breaker for CODE_INDEX_ERROR, tracked independently of all other telemetry: a sliding 10-minute window, trips after 50 occurrences, then suppresses further captures of that event for a 10-minute cooldown before resetting. Unrelated events (TASK_CREATED, TOOL_USED, etc.) are never guarded and can't mask or reset the guarded count.
• In-flight call tracking: captureEvent/captureException now track the promise returned by each client's capture()/captureException() call. shutdown() drains these (looping, not a single snapshot, since draining one call's chain can enqueue more) before calling client.shutdown(), bounded to a 3-second timeout so a capture stuck on network I/O can't block extension deactivation indefinitely. A rejected capture is caught internally so it never surfaces as an unhandled rejection or blocks the drain.
• isShuttingDown gate so captures fired after shutdown() has started are dropped immediately instead of racing the drain loop.
• src/extension.ts: deactivate() now awaits TelemetryService.instance.shutdown(), wrapped in try/catch so a shutdown failure can't skip the remaining terminal cleanup (Terminal.setTerminalProfile, TerminalRegistry.cleanup).

Capture call sites (captureEvent/captureException) remain synchronous/non-blocking for callers — only shutdown() awaits the drain.

Out of scope

Kept narrow to service-level reliability, per the split from #835:

• Consent defaults, banner/settings/webview gating, localization, PRIVACY.md (in fix(telemetry): default telemetry to opt-out with explicit consent UI #1069).
• Task Completed aggregation (toolsUsed/messageCount), AttemptCompletionTool changes.
• Tool-name validation or model-controlled-name bucketing.
• Model-cache throttling/deduplication (modelCache.ts).
• E2E test stabilization and unrelated extension cleanup.
• No changesets added.

Test Procedure

• pnpm --filter @roo-code/telemetry test: 42/42 passing, including two new files:
  • TelemetryService.circuit-breaker.test.ts — under-threshold pass-through, trip at 50 in-window, cooldown reset, unrelated events don't reset/count toward the guarded total, old occurrences expire out of the window, other event names are never guarded.
  • TelemetryService.shutdown.test.ts — shutdown waits for a pending capture that later resolves, drains a call still in flight across multiple loop passes, stops accepting new captures once shutdown has started, proceeds after the 3s timeout for a never-settling capture, a rejected capture doesn't prevent shutdown from completing.
• pnpm check-types: clean repo-wide.
• eslint on all 4 touched files: 0 errors; src/eslint-suppressions.json unchanged (no new suppressions).
• src/__tests__/extension.spec.ts: 4/4 passing, confirming the awaited-shutdown change doesn't break existing deactivation behavior.

Pre-Submission Checklist

• Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
• Scope: My changes are focused on the linked issue (one major feature/fix per PR).
• Self-Review: I have performed a thorough self-review of my code.
• Testing: New and/or updated tests have been added to cover my changes.
• Visual Snapshot (UI changes only): N/A — no UI changes.
• Documentation Impact: No documentation updates required (no user-facing behavior change; PRIVACY.md already covers the opt-out default via fix(telemetry): default telemetry to opt-out with explicit consent UI #1069).
• Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Documentation Updates

• No documentation updates are required.

Summary by CodeRabbit

• Bug Fixes
  • Improved telemetry reliability by limiting repeated event submissions.
  • Ensured telemetry captures complete gracefully during shutdown.
  • Prevented new telemetry captures from starting after shutdown begins.
  • Added timeout handling so shutdown cannot be blocked indefinitely.
  • Extension cleanup now continues even if telemetry shutdown encounters an error.

[coderabbitai]

📝 Walkthrough

Walkthrough

TelemetryService now rate-limits selected events, tracks asynchronous captures, and coordinates shutdown. Extension deactivation awaits telemetry shutdown, logs failures, and continues terminal cleanup.

Changes

Telemetry resilience

Layer / File(s)	Summary
Event circuit breaker and capture tracking packages/telemetry/src/TelemetryService.ts, packages/telemetry/src/__tests__/TelemetryService.circuit-breaker.test.ts	Selected events use sliding-window circuit breakers. Capture operations are tracked, and captures during shutdown are rejected. Tests cover thresholds, cooldowns, expiry, isolation, and unguarded events.
Coordinated telemetry shutdown packages/telemetry/src/TelemetryService.ts, packages/telemetry/src/__tests__/TelemetryService.shutdown.test.ts	Shutdown drains pending captures across multiple passes, uses a bounded timeout, handles rejected captures, and awaits client shutdown.
Extension deactivation integration src/extension.ts	Extension deactivation awaits telemetry shutdown, logs shutdown errors, and continues terminal cleanup.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Extension
  participant TelemetryService
  participant TelemetryClient
  participant TerminalCleanup
  Extension->>TelemetryService: await shutdown()
  TelemetryService->>TelemetryService: reject new captures
  TelemetryService->>TelemetryClient: drain pending captures
  TelemetryService->>TelemetryClient: await shutdown()
  TelemetryService-->>Extension: complete or report error
  Extension->>TerminalCleanup: continue cleanup

Loading

Suggested reviewers: hannesrudolph

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes the two primary changes: the telemetry circuit breaker and shutdown draining.
Description check	✅ Passed	The description includes the linked issue, implementation details, scope, test procedure, checklist, and documentation impact.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch issue/830-2

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

❌ Patch coverage is 93.02326% with 3 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
packages/telemetry/src/TelemetryService.ts	93.02%	3 Missing ⚠️

📢 Thoughts on this report? Let us know!

[coderabbitai]

🧹 Nitpick comments (1)

packages/telemetry/src/__tests__/TelemetryService.shutdown.test.ts (1)

120-144: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider adding a matching test for captureException shutdown gating.

captureException has the same isShuttingDown gate as captureEvent (TelemetryService.ts, lines 165-167), but no test here verifies that a captureException call is dropped after shutdown() has started. Add a test mirroring "stops accepting new captures once shutdown() has started" for captureException.

As per path instructions, "**/*.{test,spec}.{ts,tsx,js}: Use package-local unit tests for ... error handling."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/telemetry/src/__tests__/TelemetryService.shutdown.test.ts` around
lines 120 - 144, Add a package-local unit test alongside “stops accepting new
captures once shutdown() has started” that starts TelemetryService.shutdown(),
invokes captureException after shutdown begins, awaits shutdown completion, and
asserts the mock client’s captureException was not called. Mirror the existing
captureEvent gating test while targeting TelemetryService.captureException.

Source: Path instructions

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@packages/telemetry/src/__tests__/TelemetryService.shutdown.test.ts`:
- Around line 120-144: Add a package-local unit test alongside “stops accepting
new captures once shutdown() has started” that starts
TelemetryService.shutdown(), invokes captureException after shutdown begins,
awaits shutdown completion, and asserts the mock client’s captureException was
not called. Mirror the existing captureEvent gating test while targeting
TelemetryService.captureException.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: da2b6bef-3327-452d-b2d0-3addae8e222f

📥 Commits

Reviewing files that changed from the base of the PR and between cd29243 and ea78a27.

📒 Files selected for processing (4)

• packages/telemetry/src/TelemetryService.ts
• packages/telemetry/src/__tests__/TelemetryService.circuit-breaker.test.ts
• packages/telemetry/src/__tests__/TelemetryService.shutdown.test.ts
• src/extension.ts