test(router): make the HMR recovery test deterministic and its cleanup failure-safe - #1048

Closes #1040.

Problem

route-types-hmr "recovery: helper-file fix after broken-import error re-runs discovery" failed in about half of local runs, and a failed run left e2e/test-app/src/urls/blog.handlers.tsx with the appended syntax error. The serial retry then died in beforeAll (dirty-tree guard), which is why a failure reported "1 failed, 1 flaky, 19 passed". The suite is local-only (CI skips it).

Cause

Two separate defects, both in the test.

1. The failing assertion was the "still at last-good while discovery is failing" check (line 852 on main), not the sanity check before the edit. The hypothesis in the issue (a rediscovery succeeding between the two writes) is refuted: every captured run has exactly one rediscovery cycle for the test's edits, and it fails. The actual mechanism is in src/vite/router-discovery.ts: the watcher's debounced handler first runs the static route parser and writes the generated file from blog.tsx (writeCombinedRouteTypesWithTracking, "watcher: regenerated gen files"), and only then starts the runtime rediscovery that throws. Polling the generated file every 5ms from the writes on, on main:

   1ms   wrote blog.tsx / wrote handlers
   13ms  genHasRecovered=false failureMarkerSeen=false
   154ms genHasRecovered=true  failureMarkerSeen=false   (static write)
   205ms genHasRecovered=true  failureMarkerSeen=true    (marker; test reads here)
   267ms genHasRecovered=false failureMarkerSeen=true    (reverted by regeneration)
   

   The file holds the new route until a second change event for it triggers regenerateGeneratedRouteFiles() from the last-good manifest (about 65ms after the marker). A test that reads inside that window fails. In some runs the revert does not happen at all: a 4s probe showed the file keeping blog.recovered for the whole window, with the runtime still unable to serve the route. So the generated file's content during the failed state is nondeterministic, and the old assertion encoded a guarantee the product does not give.

2. afterEach restored the four baselines one at a time, waiting for the dev server to apply each. With discovery failed, the first wait could not succeed until blog.handlers.tsx was restored, so the loop threw on blog.tsx and never reached it. The test's own finally restored only blog.tsx, after the assertions that can fail.

Change

packages/rangojs-router/e2e/route-types-hmr.test.ts only; nothing under src/.

• The failed state no longer depends on both writes landing in one debounce window: blog.handlers.tsx is broken first (a change the watcher ignores outside recovery mode, no urls(/createRouter), then blog.tsx gets the route, so the rediscovery that edit triggers can only see the broken helper. (No rediscovery was seen to succeed between the two writes on main; the reorder removes the dependence, it was not the cause.)
• The "last-good" check moves from the generated file to the runtime manifest: while the helper is broken queryReverse(["blog.recovered"]) must not resolve the route (the dev server cannot evaluate the router and answers with an error page; either outcome passes). The final assertions are unchanged: after fixing only the helper, the generated file and the runtime manifest both carry blog.recovered. The runtime check is the one that discriminates recovery from a stale static write.
• afterEach writes every modified baseline first, then waits once (one rewrite retry if the watcher missed the event). The test-local try/finally is gone; afterEach covers every file.
• afterEach then waits for five consecutive good runtime answers. Dropping the page fixture from afterEach removed an accidental delay that was hiding a second race: after the gen-file recreate tests, the dev server reloads the rsc graph and the next test's first queryReverse got an HTML error page ("reverse() should still work after gen file deletion and recreation", seen on the first attempt in 2 of 20 runs before the settle, 0 in 12 after).

Numbers

Full hmr-routes suite, local, other browser suites running on the machine (load 2-12).

Every failure on main left blog.handlers.tsx modified; after the change git status --porcelain was empty after every run, including all failing ones.

Test plan

• Forced failure after the break step (temporary throw, not committed), full suite, --retries=0: 1 failed, 20 passed, git status --porcelain shows no test-app files; an immediate second run starts (no "uncommitted changes" skip) and gives the same 20 passed.
• 12 full-suite runs after the change: 11 clean, 1 flaky test (Notes).
• 5 more full-suite runs after merging main and the review edits: 5 of 5 clean (21 passed each), tree clean after each.
• pnpm run typecheck, pnpm run test:unit:all, pnpm run lint, pnpm run format, pnpm check:e2e-bucketing, pnpm check:e2e-parity --strict.
• Dev-only on purpose (HMR); tools/e2e-parity-allowlist.json untouched.

Notes

• Possible product follow-up, not changed here: during a failed rediscovery the generated route file can keep the statically parsed new route (while the runtime manifest stays last-good), and whether it reverts depends on a second watcher event. Harmless for types, but the "preserved at last-good" debug message only holds for the runtime manifest.
• "an invalid transition({ when }) edit prints the error and shows the Vite overlay" failed once on its first attempt in the 12 final runs (overlay element not found within 10s) and passed on retry. It did not fail in the 10 instrumented runs on main and it is unattributed; not touched here.
• The suite takes about 49 s (21 tests). The settle wait after each test costs roughly half a second per test; narrowing it to the tests that write the generated file was considered and skipped, since the suite is local-only.
• Review: one independent four-angle pass (reuse, simplification, efficiency, altitude). Applied: a comment that claimed a rediscovery could succeed between the two writes is corrected (the measurements show none did), the long comments are trimmed, the step-4 comment says the runtime manifest is the proof because the generated file may already hold the route, and the stale "temp-write + rename" doc of writeFileAndAwaitHmr in tests/shared-e2e/src/index.ts now says what the helper does. Not changed: the "during failure" check still treats an erroring query as "not resolved"; asserting that the query errors would pin how a broken dev server answers, which is not what this test is about.