fix(test): isolate process.env to stop flaky auth/pairs suite #151

[therealbibson]

Overview

Fixes intermittent Vitest failures (~1 in 5 full runs) where auth.test.ts and pairs.test.ts (and rotating victims such as networkVenueConfig.test.ts) fail only in the full suite — never in isolation.

Root cause

process.env leaks between files that share a test worker. Vitest 4's default pool is already forks, so files never run concurrently in a single process against each other; the mechanism is that a reused fork child process runs several files sequentially, and an env key set by one file is still set when the next file starts. POST /pairs reads ADMIN_API_KEY at request time, so a preceding file that left the key cleared yields a spurious 401; other suites inherit stale venue flags the same way. That is why the failures rotate and always pass in isolation. networkVenueConfig additionally saw the 5s default exceeded under fork load when resetModules+SDK import ran.

Related Issue

Closes #151

Changes

Isolation (the real fix)

• [ADD] vitest.setup.ts — snapshot process.env in beforeEach, restore in afterEach (enforced, not convention).
• [MODIFY] vitest.config.ts — pool: 'forks' pins the existing Vitest default (kept explicit, not a behavioural change); setupFiles; testTimeout: 20_000 — tight enough that a genuine hang fails rather than passing slowly.

Test hygiene

• [MODIFY] src/__tests__/pairs.test.ts — stop restoring ADMIN_API_KEY before inject (route reads env at request time); clear in afterEach.
• [MODIFY] src/__tests__/auth.test.ts — restore ADMIN_TOKEN with delete when originally unset (Node stringifies undefined → "undefined").
• [MODIFY] src/__tests__/usage.test.ts, src/__tests__/x402-quota.test.ts — per-file env restores adopted from fix(tests): isolate process.env to stop flaky auth/pairs failures #160 (additive, compatible with the shared setup file).
• [MODIFY] tests/aggregator.property.test.ts — property-case timeout 120_000 → 30_000.

Verification Results

npx vitest run   # full suite, 20 consecutive runs
✅ 20/20 passed — Tests  400 passed | 1 skipped (401) each run
EVIDENCE_20 pass=20 fail=0

Pre-fix baseline on the same machine: ~1/10 full runs failed (networkVenueConfig / property timeout / env races). After fix: 20 consecutive greens.

Acceptance Criteria	Status
Root cause named (which state, shared how)	✅ process.env leaking across sequentially-reused fork workers
20 consecutive full runs pass	✅ 20/20 evidence above
Does not simply disable parallelism	✅ pool: 'forks' keeps file parallelism (and is the default anyway)
Env mutation restore enforced via shared setup	✅ vitest.setup.ts

[Miracle656]

Take this one over #160 — you opened two PRs for #151 nine minutes apart, they conflict with each other (both add vitest.setup.ts and edit vitest.config.ts), and only one can land. This is the better of the two, for a specific reason.

Why this one

The decisive difference is when vitest.setup.ts snapshots process.env. This PR snapshots in beforeEach; #160 snapshots once at module load and restores after every test — which wipes anything a file set in its own beforeAll from test 2 onward. Demonstrated with an identical probe under each setup:

### under #160's setup
 ✓ test 1 sees it
   → expected undefined to be 'set-in-beforeAll'
      Tests  1 failed | 1 passed (2)

### under this PR's setup
 ✓ test 1 sees it
 ✓ test 2 still sees it
      Tests  2 passed (2)

That is also why #160 had to convert pairIssuerMatch.test.ts from beforeAll to beforeEach — it was patching damage its own setup file caused. As an always-on shared setup file, that is a landmine for every future test.

Three changes before I merge

1. The stated root cause is wrong. The header comment says "Vitest's default threads pool runs multiple files concurrently inside one Node process". Vitest 4.1.5's default pool is already forks — I probed inside a test and got isMainThread = true, i.e. a child process. Files do not run concurrently in one process, and process.env cannot race concurrently across them. (Env does persist across files run sequentially in a reused fork, which is the real mechanism — #160's phrasing is closer.) Please correct the comment and the PR body; a wrong explanation in a shared setup file will mislead whoever touches it next.

2. pool: 'forks' in vitest.config.ts is a no-op — it is already the default. Keep it as an explicit pin if you like, but the comment should say "pinning the existing default", not imply a change.

3. The timeouts are too loose. testTimeout: 60_000 and a 120s property timeout will hide a genuine hang as a slow pass. #160 used 20s; take that, and 30s for the property test.

Then cherry-pick #160's extra per-file restores in src/__tests__/usage.test.ts and src/__tests__/x402-quota.test.ts — strictly additive and compatible with your setup file.

Two things to know

The flake is real — I reproduced it three times, always in a full run, always passing in isolation, with rotating victims exactly as #151 describes. An earlier check of mine got seven green runs and wrongly concluded it was fixed; it was luck. So this PR is worth having.

But neither PR touches the root hazard. src/config.ts:245-253 memoises each NetworkConfig in a module-level Map on first access. Restoring process.env does not invalidate _networkCache — only vi.resetModules() does. Both PRs are test-side only, so the memoisation footgun survives either merge. I will keep #151 open with that noted, or open a follow-up.

Also: both PR bodies claim 20/20 passed. With no CI on fork PRs that is unverifiable, and I managed four runs per branch — not enough to confirm a roughly 1-in-10 flake either way. Worth being precise about what you actually observed.

[therealbibson]

Corrected the write-up and tightened the ceilings, @Miracle656 — specifically the three points from your review:

1. Root cause corrected in both vitest.setup.ts and the PR body. You were right that the threads-pool explanation was wrong: Vitest 4's default pool is already forks, so files don't share a process concurrently. The real mechanism is process.env persisting inside a reused fork that runs files sequentially — which is also why the victims rotate with scheduling order. The body now says that instead of the wrong story.
2. pool: 'forks' comment now reads as pinning the existing default, not implying a behavioural change.
3. Timeouts tightened: testTimeout 60_000 → 20_000, and the 10k-iteration property case 120_000 → 30_000 (your fix(tests): isolate process.env to stop flaky auth/pairs failures #160 values).

I also cherry-picked #160's per-file restores in src/__tests__/usage.test.ts and src/__tests__/x402-quota.test.ts — both strictly additive and compatible with the shared setup file.

Pushed as a single commit on the head branch. Could you take another look when you have a moment?

[Miracle656]

Re-reviewed at d713f17. All three changes are in, and I verified the fix does what it claims rather than taking the green on trust. Merging.

The three points

#	Point	Status
1	Stated root cause is wrong (threads vs forks)	Fixed
2	pool: 'forks' presented as a change	Fixed
3	60s/120s timeouts too loose	Fixed
—	Cherry-pick #160's per-file restores	Done

On 1, the new header in vitest.setup.ts says it correctly — "Vitest 4's default pool is already forks, so files do not run concurrently in one process. What leaks instead is process.env within a fork — a reused child process runs several files sequentially". That is the real mechanism. On 2, the comment now reads "This pins Vitest's existing default rather than changing it". On 3, testTimeout: 20_000 and the property test at 30_000 — and you also dropped the stray second }, 15000) argument that was silently overriding the inline option. Good catch, I had not spotted that one.

src/__tests__/usage.test.ts and src/__tests__/x402-quota.test.ts both carry #160's per-file restores, and auth.test.ts now deletes rather than assigning undefined (which Node stringifies to "undefined"). All strictly additive.

Verification

The branch is 21 commits behind main, so I tested the merge result, not the tip — the tip alone runs 47 files / 401 tests and would have told me nothing about main's nine newer suites.

Baseline, origin/main, cold cache:

 Test Files  4 failed | 51 passed | 1 skipped (56)
      Tests  3 failed | 529 passed | 6 skipped (538)
 FAIL  src/__tests__/middleware/x402.test.ts > x402 middleware > returns 200 when payment header is valid
   Error: Test timed out in 5000ms.

main + this PR, merged locally, four runs (three warm, one with node_modules/.vite deleted to force a cold transform):

 Test Files  55 passed | 1 skipped (56)
      Tests  537 passed | 1 skipped (538)      ×4

The cold run is the interesting one — 69s wall, 226s of import work, and nothing timed out. The 5s default was a live flake source independent of env leakage, and testTimeout: 20_000 closes it. The merge itself is clean; your changes to tests/aggregator.property.test.ts and vitest.config.ts do not collide with main's.

Still open, as a follow-up and not against you

src/config.ts memoises each NetworkConfig in a module-level _networkCache on first access, and restoring process.env does not invalidate it — only vi.resetModules() does. Your setup file cannot reach that, and neither could #160. I am keeping #151 open with that noted so the memoisation footgun is tracked separately.

Thanks for the turnaround, and for correcting the explanation rather than just the symptom — a wrong comment in an always-on setup file would have cost someone real time later.