fix(telemetry): default telemetry to opt-out with explicit consent UI - #1069
Open
edelauna wants to merge 4 commits into
Open
fix(telemetry): default telemetry to opt-out with explicit consent UI#1069edelauna wants to merge 4 commits into
edelauna wants to merge 4 commits into
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe PR centralizes telemetry opt-in logic, gates telemetry with VS Code’s global setting, serializes webview updates, adds explicit consent actions, propagates telemetry state, updates tests, and revises privacy disclosures and translations. ChangesTelemetry control flow
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/core/webview/__tests__/webviewMessageHandler.spec.ts (1)
1889-1900: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider replacing the fixed microtask-flush count with an explicit signal.
The loop flushes exactly ten microtask turns as a fallback for the fixed code path. This count is a timing heuristic. If
webviewDidLaunchlater gains anotherawaitbefore it chains ontotelemetrySettingQueue, the flush may finish too early and the test would pass or fail for the wrong reason.The rest of this test already uses deferred-promise handshakes. An explicit signal from the queued update, or awaiting an exported queue handle, would remove the magic number.
🤖 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 `@src/core/webview/__tests__/webviewMessageHandler.spec.ts` around lines 1889 - 1900, Replace the fixed ten-turn microtask flush in the webviewDidLaunch test with an explicit deferred-promise or queue-completion signal tied to the queued telemetry update. Await that signal alongside snapshotTakenPromise so the test synchronizes with the actual operation rather than relying on timing, preserving the existing pre-fix snapshot behavior.webview-ui/src/__tests__/TelemetryClient.spec.ts (1)
107-134: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsolidate
TelemetryClientspec coverage into one file.
webview-ui/src/__tests__/TelemetryClient.spec.tsandwebview-ui/src/utils/__tests__/TelemetryClient.spec.tsboth coverTelemetryClientandupdateTelemetryState. Move one set or merge the specs so this coverage does not drift between two files.🤖 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 `@webview-ui/src/__tests__/TelemetryClient.spec.ts` around lines 107 - 134, Consolidate the duplicate TelemetryClient and updateTelemetryState test coverage from webview-ui/src/__tests__/TelemetryClient.spec.ts and webview-ui/src/utils/__tests__/TelemetryClient.spec.ts into a single spec file. Move or merge the relevant cases, remove the redundant file or duplicated tests, and preserve coverage for both telemetry-disabled and opted-in initialization behavior.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.
Inline comments:
In `@PRIVACY.md`:
- Around line 35-42: Update the Telemetry (Usage Data) section in PRIVACY.md to
state the active PostHog event-retention period or link to the authoritative
retention policy, replacing the vague “only as long as needed” wording. Verify
the stated period against the current PostHog plan or configuration before
merging.
In `@src/core/webview/webviewMessageHandler.ts`:
- Around line 659-666: Guard the queued telemetry update in the webview launch
handler with TelemetryService.hasInstance() before accessing
TelemetryService.instance. Preserve the existing telemetry-setting calculation
and update behavior when an instance exists, while skipping the update when it
does not.
In
`@webview-ui/src/components/common/__tests__/TelemetryBanner.visual.fixture.tsx`:
- Around line 19-20: Update the visual fixture’s i18next initialization so its
init promise is returned and supplied to the test setup, then await that promise
before calling mount for TelemetryBannerFixture. Ensure initialization completes
before Trans renders, without changing the existing translation configuration.
---
Nitpick comments:
In `@src/core/webview/__tests__/webviewMessageHandler.spec.ts`:
- Around line 1889-1900: Replace the fixed ten-turn microtask flush in the
webviewDidLaunch test with an explicit deferred-promise or queue-completion
signal tied to the queued telemetry update. Await that signal alongside
snapshotTakenPromise so the test synchronizes with the actual operation rather
than relying on timing, preserving the existing pre-fix snapshot behavior.
In `@webview-ui/src/__tests__/TelemetryClient.spec.ts`:
- Around line 107-134: Consolidate the duplicate TelemetryClient and
updateTelemetryState test coverage from
webview-ui/src/__tests__/TelemetryClient.spec.ts and
webview-ui/src/utils/__tests__/TelemetryClient.spec.ts into a single spec file.
Move or merge the relevant cases, remove the redundant file or duplicated tests,
and preserve coverage for both telemetry-disabled and opted-in initialization
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d0452b8c-95db-46c1-a02a-2b4d497d0cca
⛔ Files ignored due to path filters (1)
webview-ui/src/components/common/__tests__/__screenshots__/telemetry-banner-dark.pngis excluded by!**/*.png
📒 Files selected for processing (57)
PRIVACY.mdpackages/types/src/__tests__/telemetry.isTelemetryOptedIn.test.tspackages/types/src/telemetry.tspackages/types/src/vscode-extension-host.tssrc/__tests__/extension.spec.tssrc/core/webview/ClineProvider.tssrc/core/webview/__tests__/telemetrySettingsTracking.spec.tssrc/core/webview/__tests__/webviewMessageHandler.spec.tssrc/core/webview/webviewMessageHandler.tssrc/eslint-suppressions.jsonsrc/extension.tswebview-ui/playwright/vscode-theme-dark.csswebview-ui/src/App.tsxwebview-ui/src/__tests__/TelemetryClient.spec.tswebview-ui/src/components/common/TelemetryBanner.tsxwebview-ui/src/components/common/__tests__/TelemetryBanner.spec.tsxwebview-ui/src/components/common/__tests__/TelemetryBanner.visual.fixture.tsxwebview-ui/src/components/common/__tests__/TelemetryBanner.visual.tsxwebview-ui/src/components/settings/About.tsxwebview-ui/src/components/settings/__tests__/About.spec.tsxwebview-ui/src/i18n/locales/ca/settings.jsonwebview-ui/src/i18n/locales/ca/welcome.jsonwebview-ui/src/i18n/locales/de/settings.jsonwebview-ui/src/i18n/locales/de/welcome.jsonwebview-ui/src/i18n/locales/en/settings.jsonwebview-ui/src/i18n/locales/en/welcome.jsonwebview-ui/src/i18n/locales/es/settings.jsonwebview-ui/src/i18n/locales/es/welcome.jsonwebview-ui/src/i18n/locales/fr/settings.jsonwebview-ui/src/i18n/locales/fr/welcome.jsonwebview-ui/src/i18n/locales/hi/settings.jsonwebview-ui/src/i18n/locales/hi/welcome.jsonwebview-ui/src/i18n/locales/id/settings.jsonwebview-ui/src/i18n/locales/id/welcome.jsonwebview-ui/src/i18n/locales/it/settings.jsonwebview-ui/src/i18n/locales/it/welcome.jsonwebview-ui/src/i18n/locales/ja/settings.jsonwebview-ui/src/i18n/locales/ja/welcome.jsonwebview-ui/src/i18n/locales/ko/settings.jsonwebview-ui/src/i18n/locales/ko/welcome.jsonwebview-ui/src/i18n/locales/nl/settings.jsonwebview-ui/src/i18n/locales/nl/welcome.jsonwebview-ui/src/i18n/locales/pl/settings.jsonwebview-ui/src/i18n/locales/pl/welcome.jsonwebview-ui/src/i18n/locales/pt-BR/settings.jsonwebview-ui/src/i18n/locales/pt-BR/welcome.jsonwebview-ui/src/i18n/locales/ru/settings.jsonwebview-ui/src/i18n/locales/ru/welcome.jsonwebview-ui/src/i18n/locales/tr/settings.jsonwebview-ui/src/i18n/locales/tr/welcome.jsonwebview-ui/src/i18n/locales/vi/settings.jsonwebview-ui/src/i18n/locales/vi/welcome.jsonwebview-ui/src/i18n/locales/zh-CN/settings.jsonwebview-ui/src/i18n/locales/zh-CN/welcome.jsonwebview-ui/src/i18n/locales/zh-TW/settings.jsonwebview-ui/src/i18n/locales/zh-TW/welcome.jsonwebview-ui/src/utils/TelemetryClient.ts
💤 Files with no reviewable changes (1)
- src/eslint-suppressions.json
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
8 tasks
edelauna
marked this pull request as ready for review
July 31, 2026 13:51
edelauna
requested review from
JamesRobert20,
hannesrudolph,
navedmerchant and
taltas
as code owners
July 31, 2026 13:51
github-actions
Bot
added
the
awaiting-review
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related GitHub Issue
Refs: #830 (consent/privacy half only — Part A, event volume, is separate work in #835)
Description
Splits the consent and privacy fixes out of #835 into their own PR, so they can ship independent of the telemetry event-volume work in that PR.
Consent default. The banner used to send
telemetrySetting: "enabled"when a user dismissed it with the close button, with no way to decline. This PR changes the semantics to a disclosed opt-out default:"unset"(no choice made) and"enabled"both keep telemetry on; only an explicit"disabled"turns it off. The banner's close button now sends no message at all, so dismissing it leaves the default in effect instead of recording an affirmative choice.isTelemetryOptedIn()inpackages/types/src/telemetry.tsis the single source of truth for this logic — used by the banner, theAboutsettings checkbox, and both the extension-host and webview PostHog clients.Explicit consent UI.
TelemetryBanner.tsxgets separate Accept and Decline actions instead of only a close button.Live VS Code telemetry gating.
extension.tsnow listens tovscode.env.onDidChangeTelemetryEnabledand re-evaluates telemetry state immediately when the user flips VS Code's global telemetry toggle, ANDing it with the storedtelemetrySetting.webviewMessageHandler.tsdoes the same for the webview's own PostHog client, and serializes concurrenttelemetrySettingmessages through a queue so a fast toggle can't have an earlier, slower write clobber a later one.Docs and strings.
PRIVACY.mdnow discloses the opt-out default and that the "does not collect code or prompts" claim applies to the PostHog path specifically. Banner and settings copy updated in all locales.Visual regression coverage. Added a Playwright CT snapshot for
TelemetryBanner(first visual test to exerciseVSCodeButton) and fixed the CT harness's button mock, which rendered as a bare unstyled<button>with no VS Code styling — screenshots would have shown that instead of the real look. Styledbutton[appearance]in the CT-only theme CSS using the existing--vscode-button-*tokens; this fixes appearance for any future visual test usingVSCodeButton, not just this one.Explicitly out of scope (left for #835 or a follow-up): the circuit breaker for
CODE_INDEX_ERRORretry storms,Task Completedaggregation oftoolsUsed/messageCount, telemetry shutdown draining, and themodelCache.tsempty-response throttle. None of those files are touched here.Test Procedure
pnpm check-typespasses repo-wide.@roo-code/types,src, andwebview-ui— the exact files touched (telemetry consent logic, banner, settings checkbox, extension activation/deactivation, webview message handling).src/eslint-suppressions.jsononly lost the now-stale__tests__/extension.spec.tsentry, no new suppressions added.pnpm test:visual:dockerpasses deterministically for all 3 visual tests (2 pre-existing + the newTelemetryBannerbaseline).Pre-Submission Checklist
*.visual.tsxsnapshot inwebview-ui/. Seewebview-ui/AGENTS.md→ "When a UI change needs a snapshot".Visual Snapshots
Added
webview-ui/src/components/common/__tests__/TelemetryBanner.visual.tsxwith baseline__screenshots__/telemetry-banner-dark.png, showing the banner with its explicit Accept (primary) and Decline (secondary) buttons.Documentation Updates
PRIVACY.mdupdated in this PR to disclose the opt-out default and clarify PostHog-specific scope.Additional Notes
Split out of #835 per review — that PR mixed event-volume reduction with consent/privacy fixes across ~65 files. This PR contains only the consent/privacy hunks, branched clean from
main.Summary by CodeRabbit
New Features
Documentation
Bug Fixes