Skip to content

test: opt the test suite out of telemetry - #1668

Open
ryandemelo wants to merge 1 commit into
Fission-AI:mainfrom
ryandemelo:test/isolate-telemetry-in-tests
Open

test: opt the test suite out of telemetry#1668
ryandemelo wants to merge 1 commit into
Fission-AI:mainfrom
ryandemelo:test/isolate-telemetry-in-tests

Conversation

@ryandemelo

@ryandemelo ryandemelo commented Aug 15, 2026

Copy link
Copy Markdown

Problem

34 test files spawn the real CLI across ~62 call sites. Each spawn runs the preAction hook exactly like a user invocation, so running pnpm test locally:

  1. writes to the developer's real global config~/.config/openspec/config.json gains a persisted anonymousId and noticeSeen
  2. sends a command_executed event per spawn to edge.openspec.dev

anonymousId is only ever written by getOrCreateAnonymousId(), which is called solely from trackCommand() after the isTelemetryEnabled() gate — so its presence is proof that events were sent, not just that a file was touched.

CI never sees this. isCiEnvironment() disables telemetry whenever CI is set, so this only happens on contributor machines. The side effects are that contributors emit analytics they didn't opt into by running the test suite, and the maintainers' usage data gets skewed by hundreds of test-generated events per run.

Reproduction

Against main (2826b88), with an isolated HOME so the write is visible and doesn't touch the real config:

$ env -u CI -u OPENSPEC_TELEMETRY -u DO_NOT_TRACK \
    HOME=/tmp/probe XDG_CONFIG_HOME=/tmp/probe/.config \
    npx vitest run test/commands/spec.test.ts

$ cat /tmp/probe/.config/openspec/config.json
{
  "telemetry": {
    "noticeSeen": true,
    "anonymousId": "804aba08-2684-4294-9599-f16cd5de791f"
  }
}

One test file is enough to trigger it.

Fix

Set OPENSPEC_TELEMETRY=0 / DO_NOT_TRACK=1 through vitest's test.env. Workers inherit it, and so do the CLI child processes they spawn — verified below rather than assumed. Telemetry's own tests call enableTelemetry(), which deletes these vars before asserting, so they keep their coverage.

Verification

main this branch
global config written under isolated HOME anonymousId persisted none
pnpm test 3969 pass 3969 pass
  • pnpm test — 136/136 files, 3969/3969 tests pass
  • pnpm lint — clean
  • Full suite under an isolated HOME writes no config.json anywhere
  • test/telemetry/* still passes (it manages its own env and mocks fetch)

Note on the interaction with #1666

On main, that same isolated-HOME run also fails spec show > should display spec in text format — the stdout-pollution bug in #1666. This PR silences that failure as a side effect, because a disabled-telemetry run never prints the notice at all.

That is a masking effect, not a fix: #1666 is the user-facing bug (real users have telemetry enabled and no CI set), and it needs to land on its own. I'd suggest treating these as independent — this one for test hygiene, #1666 for the actual stdout contract. #1666 also adds a unit assertion pinning the notice to stderr, so the behavior stays covered even with telemetry off in the suite.

Happy to rebase this on top of #1666, or hold it until that one lands, whichever you prefer.

Summary by CodeRabbit

  • Tests
    • Disabled telemetry during test execution, including in child processes, for improved privacy and more predictable CLI test runs.

34 test files spawn the real CLI across ~62 call sites. Each spawn runs
the preAction hook exactly like a user invocation, so a local `pnpm test`
persisted an anonymousId into the developer's real global config
(~/.config/openspec/config.json) and POSTed a command_executed event per
spawn to the telemetry endpoint.

CI never saw this because CI=<truthy> already disables telemetry; it only
happens on contributor machines, where it also skews the maintainers'
usage data with test traffic.

Set OPENSPEC_TELEMETRY=0 / DO_NOT_TRACK=1 via vitest's env so workers and
the CLI children they spawn are both covered. Telemetry's own tests
delete these vars before asserting, so they are unaffected.
@ryandemelo
ryandemelo requested a review from a team as a code owner August 15, 2026 04:24
@ryandemelo
ryandemelo requested review from clay-good and removed request for a team August 15, 2026 04:24
@coderabbitai

coderabbitai Bot commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 611aaa41-b379-4dbd-8ab8-8c6bc9696936

📥 Commits

Reviewing files that changed from the base of the PR and between 2826b88 and c5c9c6e.

📒 Files selected for processing (1)
  • vitest.config.ts

📝 Walkthrough

Walkthrough

Vitest now sets OPENSPEC_TELEMETRY=0 and DO_NOT_TRACK=1 for tests and inherited CLI child processes.

Changes

Test telemetry isolation

Layer / File(s) Summary
Vitest telemetry environment
vitest.config.ts
Vitest sets telemetry and tracking opt-out variables for the test suite and its child processes.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to c5c9c

The change limits telemetry side effects to test runs without altering normal user invocations, and the reported test and lint checks pass. No actionable merge-blocking risk remains beyond normal review.

Possibly related PRs

Suggested reviewers: clay-good

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: disabling telemetry for the test suite.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant