fix(telemetry): default telemetry to opt-out with explicit consent UI - #1069
fix(telemetry): default telemetry to opt-out with explicit consent UI#1069edelauna wants to merge 4 commits into
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 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://cdn.github.com/in/347564?s=60&v=4){kind=small}
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
| telemetrySettingQueue = telemetrySettingQueue | ||
| .catch(() => undefined) | ||
| .then(async () => { | ||
| const telemetrySetting = getGlobalState("telemetrySetting") || "unset" | ||
| TelemetryService.instance.updateTelemetryState( | ||
| isTelemetryOptedIn(telemetrySetting) && vscode.env.isTelemetryEnabled, | ||
| ) | ||
| }) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Guard this queued telemetry update with TelemetryService.hasInstance().
The "telemetrySetting" case at Line 2603 checks TelemetryService.hasInstance() before it touches TelemetryService.instance. This call site does not. If no instance exists when webviewDidLaunch arrives, TelemetryService.instance throws inside the queued callback. The handler does not await this chain and attaches no terminal catch, so the throw becomes an unhandled promise rejection instead of a logged error.
A guard fixes both problems and makes the two call sites consistent.
🛡️ Proposed guard
telemetrySettingQueue = telemetrySettingQueue
.catch(() => undefined)
.then(async () => {
+ if (!TelemetryService.hasInstance()) {
+ return
+ }
+
const telemetrySetting = getGlobalState("telemetrySetting") || "unset"
TelemetryService.instance.updateTelemetryState(
isTelemetryOptedIn(telemetrySetting) && vscode.env.isTelemetryEnabled,
)
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| telemetrySettingQueue = telemetrySettingQueue | |
| .catch(() => undefined) | |
| .then(async () => { | |
| const telemetrySetting = getGlobalState("telemetrySetting") || "unset" | |
| TelemetryService.instance.updateTelemetryState( | |
| isTelemetryOptedIn(telemetrySetting) && vscode.env.isTelemetryEnabled, | |
| ) | |
| }) | |
| telemetrySettingQueue = telemetrySettingQueue | |
| .catch(() => undefined) | |
| .then(async () => { | |
| if (!TelemetryService.hasInstance()) { | |
| return | |
| } | |
| const telemetrySetting = getGlobalState("telemetrySetting") || "unset" | |
| TelemetryService.instance.updateTelemetryState( | |
| isTelemetryOptedIn(telemetrySetting) && vscode.env.isTelemetryEnabled, | |
| ) | |
| }) |
🤖 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/webviewMessageHandler.ts` around lines 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.
| const visualTestI18n = i18next.createInstance() | ||
| void visualTestI18n.use(initReactI18next).init({ |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/sh
set -eu
rg -n -C 5 'TelemetryBannerFixture|visualTestI18n|createInstance|\.init\(' \
webview-ui/src/components/common/__tests__Repository: Zoo-Code-Org/Zoo-Code
Length of output: 4189
🏁 Script executed:
#!/bin/sh
set -eu
printf '\n--- relevant test files ---\n'
sed -n '1,120p' webview-ui/src/components/common/__tests__/TelemetryBanner.visual.fixture.tsx
printf '\n--- TelemetryBanner source references ---\n'
rg -n -C 8 'helpImproveMessage|settingsLink|settingsUrl|Trans|useTranslation|TranslationContext' webview-ui/src/components/common -S
printf '\n--- package deps for i18next versions ---\n'
fd -a 'package.json|pnpm-lock.yaml|package-lock.json|yarn.lock' . | sed 's#^\./##' | while read -r f; do
if rg -q '"i18next"|"react-i18next"|"initReactI18next"' "$f" || rg -q 'i18next@|react-i18next@' "$f"; then
echo "### $f"
rg -n '"i18next"|"react-i18next"|i18next@|react-i18next@' "$f" | head -n 20
fi
done
printf '\n--- imports in fixture/test ---\n'
sed -n '1,180p' webview-ui/src/components/common/__tests__/TelemetryBanner.visual.tsxRepository: Zoo-Code-Org/Zoo-Code
Length of output: 38219
🌐 Web query:
i18next 25 init returns Promise synchronous resources documentation
💡 Result:
In i18next, the i18next.init method consistently returns a Promise, regardless of whether you are using synchronous or asynchronous resource loading [1][2]. When using a backend that supports synchronous (blocking) loading, such as i18next-fs-backend, you can configure initialization to load resources synchronously by setting the initAsync option to false (in older versions, this option was referred to as initImmediate) [3][4]. Key technical details regarding this behavior: 1. Consistency: The init method always returns a Promise [1][2]. Even when initAsync is set to false, the return type of the function remains a Promise [3][4]. 2. Synchronous Configuration: Setting initAsync: false is intended for environments where the backend performs blocking, synchronous I/O [3]. This ensures that translation resources are available immediately after the init call returns, allowing you to use i18next.t without waiting for the Promise to resolve or using the initialization callback [3][4]. 3. TypeScript and Linting: Because init always returns a Promise, using it in a synchronous context may trigger "floating promise" warnings in ESLint configurations (e.g., @typescript-eslint/no-floating-promises) [4]. To handle this, you can explicitly mark the promise as ignored (e.g., using the void operator) or ensure your code structure respects the asynchronous nature of the initialization process [4]. In summary, while you can force the resource loading process to be synchronous via backend configuration, the i18next.init API signature itself does not change to a non-promise-returning function [3][2].
Citations:
- 1: https://www.i18next.com/overview/api.md
- 2: https://www.i18next.com/overview/api
- 3: https://www.i18next.com/overview/configuration-options.md
- 4: i18n.init called in non-async method and with initImmediate set to false makes eslint throw error i18next/i18next#1875
🌐 Web query:
react-i18next 15 Trans I18nextProvider documentation initialization
💡 Result:
In react-i18next v15, initialization and usage patterns for I18nextProvider and the Trans component remain consistent with established practices, as the v15 release primarily focused on internal build optimizations, dependency updates, and modernization of syntax [1][2]. Initialization and Setup To initialize react-i18next, you must use the initReactI18next plugin to connect i18next with your React application [3][4]. import i18n from 'i18next'; import { initReactI18next } from 'react-i18next'; i18n.use(initReactI18next).init({ // configuration options }); I18nextProvider The I18nextProvider is a context provider used to pass an i18next instance down to the component tree [5]. It is generally only required if your application needs to support multiple i18next instances simultaneously (e.g., in a component library) or for specific server-side rendering (SSR) configurations [5]. import { I18nextProvider } from 'react-i18next'; Trans Component The Trans component is designed for translating JSX trees while preserving structure, such as components or formatting within a string [6][7]. It automatically retrieves the translation function from the context (provided by I18nextProvider or the global instance) [6][8]. Welcome to React If you are working with React 15 or older, you must set the defaultTransParent option globally or pass the parent prop to the Trans component to ensure correct rendering [6][8]. In modern React environments (v16+), this is handled automatically [6]. Trans also supports various props such as i18nKey, values, and components for advanced interpolation [6][8].
Citations:
- 1: https://raw.githubusercontent.com/i18next/react-i18next/master/CHANGELOG.md
- 2: i18next/react-i18next@v14.1.2...v15.0.0
- 3: https://react.i18next.com/guides/quick-start
- 4: https://react.i18next.com/getting-started
- 5: https://react.i18next.com/latest/i18nextprovider
- 6: https://react.i18next.com/latest/trans-component.md
- 7: https://react.i18next.com/latest/using-with-hooks.md
- 8: https://react.i18next.com/latest/trans-component
🏁 Script executed:
#!/bin/sh
set -eu
printf '\n--- inspect locked i18next init source snippets from pnpm store ---\n'
lockdir=$(mktemp -d)
cd "$lockdir"
printf '%s\n' 'i18next@25.2.1' 'react-i18next@15.5.1' > import-map.txt
cat import-map.txt
printf '%s\n' '--- pnpm-store location candidates ---'
home="${HOME:-/root}"
for d in "$home/.local/share/pnpm/store/v3" "$home/.pnpm-store/v3" "$PWD/pnpm-store"; do
if [ -d "$d" ]; then
echo "FOUND $d"
fi
done
printf '%s\n' '--- fetch package tarballs and inspect init/Trans source ---'
tmp=$(mktemp -d)
cd "$tmp"
for pkg in i18next/react-i18next; do
case $pkg in
i18next) ver=25.2.1 ;;
react-i18next) ver=15.5.1 ;;
esac
tarball="$(npm view "$pkg@$ver" dist.tarball 2>/dev/null)"
echo "### $pkg@$ver tarball: $tarball"
if [ -n "$tarball" ]; then
mkdir -p "pkg-$pkg"
curl -fsSL "$tarball" -o "pkg-$pkg.tgz"
tar -xzf "pkg-$pkg.tgz" -C "pkg-$pkg"
echo "--- $pkg package files ---"
find "pkg-$pkg" -maxdepth 3 -type f \( -name '*.js' -o -name '*.ts' -o -name '*.mjs' \) | sort | head -n 40
echo "--- symbols in package ---"
rg -n 'function init|init\\(|initAsync|initImmediate|react-i18next|I18nextContext|TransContext|i18n\\.use|i18n\\.setDefaults|i18n\\.loadNamespace|i18n\\.defaultNS' "pkg-$pkg" || true
else
echo "npm tarball unavailable"
fi
done
printf '%s\n' '--- inspect likely init implementations ---'
for f in $(find "pkg-i18next" "pkg-react-i18next" -type f \( -name '*.js' -o -name '*.ts' -o -name '*.mjs' \) 2>/dev/null | rg 'lib/index|src/index|cjs|esm|trans|react' | head -n 80); do
if rg -q 'init\\(|initAsync|initImmediate|use.*init' "$f"; then
echo "### $f"
rg -n -C 8 'function init|const init|\.init|initAsync|initImmediate|init\\(' "$f" | head -n 220
fi
doneRepository: Zoo-Code-Org/Zoo-Code
Length of output: 401
🏁 Script executed:
#!/bin/sh
set -eu
tmp=$(mktemp -d)
cd "$tmp"
packages="i18next@25.2.1 react-i18next@15.5.1"
for pkg_name in i18next react-i18next; do
case $pkg_name in
i18next) ver=25.2.1 ;;
react-i18next) ver=15.5.1 ;;
esac
tarball="$(npm view "$pkg_name@$ver" dist.tarball 2>/dev/null)"
echo "### $pkg_name@$ver"
echo "tarball: $tarball"
mkdir -p "pkg-$pkg_name"
curl -fsSL "$tarball" -o "pkg-$pkg_name.tgz"
tar -xzf "pkg-$pkg_name.tgz" -C "pkg-$pkg_name"
done
printf '\n--- init-related declarations in i18next ---\n'
rg -n -C 6 'function init|const init|init\\(|initAsync|initImmediate|return.*Promise|loadNamespaces|isInitialized' pkg-i18next package package.json dist || true
printf '\n--- Trans-related declarations in react-i18next ---\n'
rg -n -C 8 'function Trans|const Trans|TransContext|useTranslation|initReactI18next|useIsomorphicLayoutEffect|useEffect' pkg-react-i18next package package.json dist || trueRepository: Zoo-Code-Org/Zoo-Code
Length of output: 50378
🏁 Script executed:
#!/bin/sh
set -eu
tmp=$(mktemp -d)
cd "$tmp"
pkg="pkg-i18next"
tarball="$(npm view i18next@25.2.1 dist.tarball 2>/dev/null)"
mkdir -p "$pkg"
curl -fsSL "$tarball" -o "$pkg.tgz"
tar -xzf "$pkg.tgz" -C "$pkg"
printf '\n--- init implementation ---\n'
rg -n -C 12 'function init|const init|initAsync|initImmediate|isInitialized|isLanguageChangingTo|loadNamespaces|deferred|return' "$pkg/package" "$pkg/dist" "$pkg/src" 2>/dev/null | head -n 260
printf '\n--- relevant source files ---\n'
fd . "$pkg/package" "$pkg/dist" "$pkg/src" -t f | sed 's#^\./##' | head -n 120Repository: Zoo-Code-Org/Zoo-Code
Length of output: 18186
🏁 Script executed:
#!/bin/sh
set -eu
tmp=$(mktemp -d)
cd "$tmp"
mkdir -p i18next
npm view i18next@25.2.1 dist.tarball >/tmp/tarball.txt 2>/dev/null || true
tarball="$(sed -n '1p' /tmp/tarball.txt)"
curl -fsSL "$tarball" -o i18next.tgz
tar -xzf i18next.tgz -C i18next
files="$(find i18next/package -name '*.js' -path '*/package/*' -printf '%p\n' | sort)"
for f in $files; do
if rg -q 'initAsync|initImmediate|function init\\(|const init|loadNamespace\\(|loadNamespaces\\(' "$f"; then
echo "### $f"
rg -n -C 16 'function init\\(|const init|isInitialized|isInitializing|initAsync|initImmediate|loadNamespaces|return Promise|deferred' "$f" | head -n 260
fi
doneRepository: Zoo-Code-Org/Zoo-Code
Length of output: 1887
🏁 Script executed:
#!/bin/sh
set -eu
tmp=$(mktemp -d)
cd "$tmp"
mkdir -p i18next
npm view i18next@25.2.1 dist.tarball >/tmp/tarball.txt 2>/dev/null || true
tarball="$(sed -n '1p' /tmp/tarball.txt)"
curl -fsSL "$tarball" -o i18next.tgz
tar -xzf i18next.tgz -C i18next
for f in $(find i18next/package -name '*.js' | sort); do
if grep -E 'initAsync|initImmediate|init\(|isInitialized|isInitializing|loadNamespaces|loadLanguage|deferred|loadNamespace' "$f" >/dev/null; then
echo "### $f"
grep -E -n -C 16 'init\(|isInitialized|isInitializing|initAsync|initImmediate|loadNamespaces|loadLanguage|deferred|loadNamespace' "$f" | head -n 280
fi
doneRepository: Zoo-Code-Org/Zoo-Code
Length of output: 50378
Await i18next initialization before mount returns.
init(...) returns a promise, and mount(<TelemetryBannerFixture />) awaits the fixture component rather than initialization. Since this bundle does not set initAsync: false, Trans can read before the welcome namespace is loaded and render fallback/missing text. Return the init promise from the fixture, pass it to the test, and await it before mount.
🤖 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/components/common/__tests__/TelemetryBanner.visual.fixture.tsx`
around lines 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.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
1 participant
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