fix(tests): isolate process.env to stop flaky auth/pairs failures

[therealbibson]

Overview

Fixes intermittent vitest failures (notably auth.test.ts and pairs.test.ts, ~1 in 5 full runs) caused by shared process.env across reused workers / parallel files, plus a pairs-test anti-pattern that cleared ADMIN_API_KEY before request-time auth checks.

Related Issue

Closes #151

Changes

Root cause

• Shared process.env across the suite. Vitest reuses worker processes between files. Suites such as x402-quota, usage, pairIssuerMatch, and auth mutate env vars (REQUIRE_API_KEY, ADMIN_TOKEN, WATCHED_PAIRS_*, STELLAR_NETWORK, …) and either never restore them or restore incorrectly (process.env.X = undefined becomes the string "undefined"). That cross-file state is why failures vanish in isolation and rotate between victims.
• pairs.test.ts restored/deleted ADMIN_API_KEY inside buildApp() after app.ready(). routes/pairs.ts reads process.env.ADMIN_API_KEY at request time, not registration time — so a race/clear left the happy-path POST returning 401 instead of 201.
• Default 5s testTimeout under parallel import load. Cold Fastify boots and vi.resetModules() + config re-imports occasionally exceeded 5s when many files transformed in parallel (same symptom class as the issue: full run flakes, isolation passes). Fixed by raising timeout without disabling fileParallelism.

Fix

• [ADD] vitest.setup.ts — snapshot process.env once per worker; restore after every test.
• [MODIFY] vitest.config.ts — register setup file; set testTimeout: 20_000; keep file parallelism enabled.
• [MODIFY] src/__tests__/pairs.test.ts — keep ADMIN_API_KEY for the request lifetime; restore in afterEach.
• [MODIFY] src/__tests__/auth.test.ts — delete ADMIN_TOKEN on restore when originally unset (avoid "undefined" string).
• [MODIFY] src/__tests__/pairIssuerMatch.test.ts — move env + config import into beforeEach so global restore cannot wipe sticky beforeAll state.
• [MODIFY] src/__tests__/x402-quota.test.ts / usage.test.ts — restore mutated env keys after each test.

Verification Results

npx vitest run   # 20 consecutive full-suite runs
20/20 passed — Tests  400 passed | 1 skipped (401) each run

Acceptance Criteria	Status
Root cause identified and named (which state, shared how)	Shared process.env via worker reuse + pairs ADMIN_API_KEY cleared before request-time read; timeout contention under parallel imports
20 consecutive full runs pass	20/20 — 400 passed / 1 skipped each
Fix does not simply disable parallelism	fileParallelism left on; isolation via env snapshot/restore
Tests mutating process.env restore it	Global setup restore + per-file restores for known mutators

Closes #151

[drips-wave]

@therealbibson Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

[Miracle656]

Flagging rather than closing, since it is yours — but I have asked for #161 instead, and the two cannot both land: they conflict with each other (both add vitest.setup.ts and edit vitest.config.ts).

The deciding factor is when vitest.setup.ts snapshots process.env. This PR snapshots once at module load and restores after every test, which wipes anything a file sets in its own beforeAll from test 2 onward:

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

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

That is also why this PR had to convert pairIssuerMatch.test.ts from beforeAll to beforeEach — it was repairing damage its own setup file caused. As an always-on shared file it would do that to every future test.

This PR is the better one in every other respect, and I have asked #161 to take three things from it: the 20s testTimeout (its 60s and 120s ceilings would hide a real hang), and the extra per-file restores in usage.test.ts and x402-quota.test.ts. Your root-cause phrasing is also closer to correct — #161 says the default pool is threads, which is wrong; vitest 4.1.5 already defaults to forks. The real mechanism is env persisting across files in a reused fork, which is nearer what you wrote.

Worth knowing: the flake is real. I reproduced it three times, always in a full run, always passing in isolation, with rotating victims. An earlier check of mine got seven green runs and wrongly called it fixed — that was luck.

Neither PR reaches the root hazard, though: src/config.ts:245-253 memoises each NetworkConfig in a module-level Map, and restoring process.env does not invalidate it — only vi.resetModules() does. If you want the follow-up that actually closes #151, that is it, and I would rather you took it than someone starting cold.

[Miracle656]

This one lands awkwardly and I want to be straight about why: #161 shipped as 8b66b44 and it cherry-picked most of this PR. Your diagnosis was right and it's now on main — but the credit landed in another PR's commit, and that's on us, not you.

Here is the file-by-file comparison against current origin/main:

Your change	Status on main
src/__tests__/usage.test.ts — ORIGINAL_ADMIN_TOKEN + afterEach delete/restore	already there, identical
src/__tests__/x402-quota.test.ts — ENV_KEYS snapshot/restore	already there, identical
src/__tests__/auth.test.ts — delete instead of assigning undefined	already there; your diff is now a comment reword only
src/__tests__/pairs.test.ts — drop the restore in buildApp, add afterEach delete	already there, same rationale comment
vitest.config.ts — setupFiles + testTimeout: 20_000	already there
vitest.setup.ts — env snapshot/restore	already there, different snapshot point (below)
src/__tests__/pairIssuerMatch.test.ts — beforeAll → beforeEach	only genuinely new line

So the unique remainder is one line, and the two differences that are left both go the wrong way:

1. vitest.setup.ts — snapshot timing. You snapshot once at module load (const ENV_SNAPSHOT = {...process.env}); main snapshots in a beforeEach. main's version is the more robust of the two, because a global beforeEach registered from setupFiles runs after a file's beforeAll, so env a file sets once for the whole file is captured and survives. With a load-time snapshot it gets wiped after the first test — which is exactly why you then had to convert pairIssuerMatch's beforeAll to beforeEach. main doesn't need that conversion at all. Your own comment ("suites that need a sticky env for the whole file should set it in beforeEach, not only beforeAll") is the constraint this choice imposes, and it's avoidable.

2. vitest.config.ts — pool: 'forks'. You remove it. main pins it deliberately (it's already Vitest's default; pinning stops a future default change from silently reintroducing cross-file sharing). Relatedly, your comment says "with --pool=threads files can even share one process concurrently" — that reads as the diagnosis, but Vitest 4's default is forks, so the leak that was actually biting was sequential reuse of one fork across files, not concurrent sharing. main's comment says that.

3. pairIssuerMatch.test.ts:18. This is the only new line, and it doesn't do what it looks like. src/config.ts:245-253 memoises each NetworkConfig in a module-level Map, so once test 1 calls getNetworkConfig('testnet') the result is cached and later tests don't read process.env at all — only vi.resetModules() invalidates it. That's why all 5 tests pass on main with beforeAll (5 passed, verified locally both standalone and in the full run). Converting to beforeEach makes vi.resetModules() + await import('../config') run five times instead of once, inside a hook — and hooks still use the 10s hookTimeout default, because #161 raised testTimeout only. I actually caught that beforeAll hook failing on one full local run of main, so this makes the exposure worse, not better.

My read: no unique value left, and the two remaining deltas are small regressions. I'd rather not merge it in this shape.

If you want something real to land from this, there is a genuine open bug sitting right next to your work and you're the best-placed person to fix it: hookTimeout was never raised alongside testTimeout, so every beforeAll/beforeEach that does resetModules() + a cold module import is still on a 10s budget. A two-line vitest.config.ts change (hookTimeout: 20_000) plus a note explaining why hooks need the same headroom as tests would be a clean, correct PR, and I'd merge it. Repoint this branch at that if you're willing.

Verification I ran: origin/main baseline 538 passed, 1 skipped (56 files), tsc --noEmit clean; npx vitest run src/__tests__/pairIssuerMatch.test.ts on main → 5 passed; git diff of this branch's merge-base diff against each file on main.

Sorry this is the second of two in a row — the work was real and correct, main just moved under it.

[therealbibson]

@Miracle656 I've resolved the merge conflicts with main by merging the current main into this branch.

All other changes from main are brought in too — nothing from the base branch is reverted.

Merge commit: 67f8b418ba

Could you take another look when you have a moment? Thanks!

[gitguardian]

⚠️ GitGuardian has uncovered 1 secret following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

Since your pull request originates from a forked repository, GitGuardian is not able to associate the secrets uncovered with secret incidents on your GitGuardian dashboard.
Skipping this check run and merging your pull request will create secret incidents on your GitGuardian dashboard.

🔎 Detected hardcoded secret in your pull request

GitGuardian id	GitGuardian status	Secret	Commit	Filename
37768935	Triggered	Generic High Entropy Secret	67f8b41	src/tests/toid.test.ts	View secret

🛠 Guidelines to remediate hardcoded secrets

1. Understand the implications of revoking this secret by investigating where it is used in your code.
2. Replace and store your secret safely. Learn here the best practices.
3. Revoke and rotate this secret.
4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider

• following these best practices for managing and storing secrets including API keys and other credentials
• install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.

---

^{🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.}