fix(test): stop worker test from classifying the whole fork as production - #2348
Conversation
…tion worker.test.ts assigned DEPLOY_ENVIRONMENT="prod" and SENTRY_DSN directly on process.env inside vi.hoisted() and never restored them. The "prod" classification persisted for the entire vitest fork, defeating the production-only guard in initProductionSentry() for every module in that fork — the same failure mode 7eba92d (#1882) fixed for Sentry. Any un-mocked telemetry sink wired to that guard would then ship local test-fixture errors to production error tracking. - Use tracked vi.stubEnv and restore with vi.unstubAllEnvs() in afterAll so the classification cannot outlive the test file within a reused fork. - Add an explicit non-production DEPLOY_ENVIRONMENT="test" default to the shared test env so no test run is ever classified as a production deployment unless it opts in. - Record the recurrence in docs/production-incident-baseline.md. Generated-By: PostHog Code Task-Id: 0fd126a1-18dd-4b16-a4cc-a990b91ce4aa
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
Reviewer's GuideThis PR fixes test-induced production classification leaks by converting worker test env mutations to tracked Vitest env stubs, setting a non-production default deployment environment for all tests, and documenting the incident and mitigation in the production incident baseline. Sequence diagram for vitest worker test env stubbing and production guardsequenceDiagram
actor Developer
participant VitestRunner
participant worker_test_ts
participant worker_ts
participant initProductionSentry
participant TelemetrySink
Developer->>VitestRunner: run worker.test.ts
VitestRunner->>worker_test_ts: execute vi.hoisted
worker_test_ts->>vi: stubEnv DEPLOY_ENVIRONMENT "prod"
worker_test_ts->>vi: stubEnv SENTRY_DSN "https://test@sentry.io/123"
VitestRunner->>worker_ts: import worker.ts
worker_ts->>initProductionSentry: initProductionSentry
initProductionSentry->>initProductionSentry: [DEPLOY_ENVIRONMENT == "prod"]
initProductionSentry-->>TelemetrySink: initialize production client
VitestRunner-->>worker_test_ts: run tests
VitestRunner->>worker_test_ts: run afterAll callback
worker_test_ts->>vi: unstubAllEnvs
initProductionSentry-->>initProductionSentry: [DEPLOY_ENVIRONMENT == "test"]
initProductionSentry-->>TelemetrySink: skip production client initialization
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Storybook previews for This comment updates automatically on each PR push. |
PR Summary by QodoPrevent vitest env leak from marking worker tests as production
AI Description
Diagram
High-Level Assessment
Files changed (3)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
162 rules✅ Skills:
fix-provider, write-tests, cloudflare 1.
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
worker.test.ts, consider callingvi.unstubEnvfor the specific keys (DEPLOY_ENVIRONMENT,SENTRY_DSN) instead ofvi.unstubAllEnvs()so future tests in this file can safely use other env stubs without having them implicitly cleared.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `worker.test.ts`, consider calling `vi.unstubEnv` for the specific keys (`DEPLOY_ENVIRONMENT`, `SENTRY_DSN`) instead of `vi.unstubAllEnvs()` so future tests in this file can safely use other env stubs without having them implicitly cleared.
## Individual Comments
### Comment 1
<location path="docs/production-incident-baseline.md" line_range="21137-21138" />
<code_context>
+ the production-only guard in `initProductionSentry()`
+ ([src/lib/sentry.ts](../src/lib/sentry.ts)) for every module that ran in the
+ same fork. `@sentry/node` is mocked in that test, so Sentry stayed clean, but
+ any un-mocked telemetry sink wired to the same guard emitted the fixture
+ errors as real production events.
+- **Root cause:** A recurrence of the failure mode fixed by commit 7eba92d
</code_context>
<issue_to_address>
**nitpick (typo):** Consider changing "un-mocked" to the more standard "unmocked".
This matches common technical terminology and reads more smoothly.
```suggestion
same fork. `@sentry/node` is mocked in that test, so Sentry stayed clean, but
any unmocked telemetry sink wired to the same guard emitted the fixture
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Bring the branch up to date with main and fix downstream breakages from the report empty-state work and provider sync refactor that were failing lint, typecheck, unit, and mobile tests in CI. Co-authored-by: Cursor <cursoragent@cursor.com>
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
Mobile PreviewScan to open on device:
To test on device:
|
Replace mise trust with explicit MISE_LOCKED=1 on install commands so pinned tool versions are enforced without relying on mise.toml settings. Co-authored-by: Cursor <cursoragent@cursor.com>
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
Align withings timeout expectations with ProviderRequestTimeoutError, pass endDate in monthly report integration tests, lowercase GHCR cache refs for fork repos, and defer HealthKit observer coordinator init to avoid self capture in Swift. Co-authored-by: Cursor <cursoragent@cursor.com>
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
Bring in main's Dockerfile Python pin, HealthKit lazy-init, report schema updates, and workflow fixes while keeping the worker test env leak fix and the mobile polling regression test. Co-authored-by: Cursor <cursoragent@cursor.com>
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
Remove recovery fields from component stories and tests since web UI consumes repository data shapes, not router results. Delete the unused report-data fixture that was breaking Knip, coverage, and Stryker. Co-authored-by: Cursor <cursoragent@cursor.com>
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
Add primary-source links for vi.hoisted, fork worker env sharing, vi.stubEnv, vi.unstubAllEnvs, and module mocking per review feedback. Also use "unmocked". Co-authored-by: Cursor <cursoragent@cursor.com>
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
Keep both 2026-07-30 incident entries and adopt shared empty-state fixtures from main. Co-authored-by: Cursor <cursoragent@cursor.com>
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
Use targeted vi.stubEnv calls instead of vi.unstubAllEnvs so future tests in worker.test.ts can stub other env vars without implicit teardown. Co-authored-by: Cursor <cursoragent@cursor.com>
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
Add official SDK documentation links for @sentry/node and posthog-node claims to satisfy third-party citation compliance in the incident baseline. Co-authored-by: Cursor <cursoragent@cursor.com>
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
Why
Error tracking was flooded with fake exceptions — every ingested
$exceptionwas a unit-test fixture string (test failure,test worker error,invalid FIT, …) from a local vitest run, not a real failure. That noise buries genuine production exceptions and each fixture spawns its own "new issue" alert.Root cause
src/jobs/worker.test.tssetDEPLOY_ENVIRONMENT="prod"andSENTRY_DSNdirectly onprocess.envinsidevi.hoisted()and never tore them down. The "prod" classification persisted for the whole vitest fork and defeated the production-only guard ininitProductionSentry()(src/lib/sentry.ts) for every module that ran in that fork.@sentry/nodeis mocked in the test, so Sentry stayed clean — but any un-mocked telemetry sink wired to the same guard would ship the fixture errors as real production events. This is a recurrence of the failure mode fixed for Sentry in #1882 (commit 7eba92d).Fix
process.envassignments with trackedvi.stubEnvcalls, restored viavi.unstubAllEnvs()in anafterAll, so the classification can't leak across files in a reused fork.DEPLOY_ENVIRONMENT: "test"default to the shared test env invitest.config.tsas a config-level safety net — no test run is classified as a production deployment unless it opts in.docs/production-incident-baseline.md.No production code changed; no
NODE_ENV === "test"branch added.Scope note
The originally-observed events came from a
posthog-nodeexporter, which is not onmain(the reporter noted it lives in an uncommitted local workspace). This PR fixes the root cause present onmain— the env mutation that tricks the production guard — rather than mocking a module that doesn't exist yet. The baseline entry records that mocking such an exporter inworker.test.tsis the follow-up when it lands.Test plan
src/jobs/worker.test.ts,src/lib/sentry.test.ts,packages/server/src/lib/sentry.test.tspass.packages/web/src/routes/training/cycling.test.tsx, a pagination assertion — confirmed failing identically on cleanmain).pnpm biome checkandpnpm tsc --noEmit(root) pass.Created with PostHog Desktop from this inbox report.
Summary by cubic
Prevents local Vitest runs from being treated as production and stabilizes builds across web, server, mobile, and e2e.
Bug Fixes
process.envwrites withvi.stubEnv; inafterAllrestore onlyDEPLOY_ENVIRONMENTandSENTRY_DSNvia targetedvi.stubEnv(notvi.unstubAllEnvs). Set a non-prodDEPLOY_ENVIRONMENT: "test"default invitest.config.ts. Documented indocs/production-incident-baseline.mdwith Vitest links; added citations to@sentry/nodeandposthog-node; kept both 2026-07-30 entries.dofek-server/report-empty-state; adopt shared empty-state fixtures; align web components toWeeklyReportData/MonthlyReportData; removerecoveryfields; delete the unused report-data fixture that broke Knip, coverage, and Stryker.ROUTINE_SYNC_DAYS; update tests..github/workflows/test.yml; retain iOS HealthKit lazy-init; align provider timeout expectations; passendDatein monthly report tests.Dependencies
MISE_LOCKED=1for installs instead ofmise trust.Written for commit 1d65cc4. Summary will update on new commits.
Summary by Sourcery
Prevent test environment configuration from causing local Vitest runs to be classified as production deployments.
Bug Fixes:
Enhancements: