Skip to content

fix(router): cloudflare dev startup races runtime discovery on factory routes - #485

Merged
ivogt merged 2 commits into
mainfrom
fix/cf-dev-runtime-discovery-flag-and-race
Apr 26, 2026
Merged

ivogt merged 2 commits into
mainfrom
fix/cf-dev-runtime-discovery-flag-and-race

Conversation

@ivogt

@ivogt ivogt commented Apr 26, 2026

Copy link
Copy Markdown
Collaborator

Summary

pnpm dev for any cloudflare app whose RSC entry calls router.reverse() at module load and whose routes are partly factory-generated (e.g. Array.from({ length: N }, (_, i) => path(...)) or any non-AST-visible loop) crashed at startup with Unknown route: <name> whenever router.named-routes.gen.ts was wiped, stale, or fresh-from-clone. Discovery was supposed to populate the gen file at startup, but the dev path had two cooperating bugs that prevented it.

cloudflare-stress-demo (14k+ factory routes) was the canonical repro: ~98% of named routes were missing from the dev gen file and worker.rsc.tsx couldn't import ./router.js. The stack ended at:

Error: Unknown route: shop.product.item42
    at Object.reverse (.../reverse.ts:302:13)
    at eval (.../router.tsx:23:10)              ← module-level reverse() call
    ...
    at async eval (.../worker.rsc.tsx:2:1)
ELIFECYCLE  Command failed with exit code 1.

Before vs. after

// Consumer pattern that hit the bug — entry calls reverse() at module
// load, and the named-routes file is partly factory-generated.

// router.tsx
export const router = createRouter({...}).routes(urlpatterns);
console.log("ssr-pre-warm:", router.reverse("shop.product.item42")); // module-level

// urls.tsx
const productPatterns = urls(({ path }) =>
  Array.from({ length: 100 }, (_, i) =>
    path(`/${i + 1}`, ProductPage, { name: `item${i + 1}` }),  // <- factory loop
  ),
);

// worker.rsc.tsx
import { router } from "./router.js";                           // <- evaluates router.tsx
import "virtual:rsc-router/routes-manifest";                    // <- gate fires too late
Sequence: rm router.named-routes.gen.ts && pnpm dev

  BEFORE
    [rsc-router] Generated route types (19 routes) -> ...      ← static parser only sees literal-named routes
    [rsc-router] Failed to create temp runner: Unknown route: shop.product.item42
    Error: Unknown route: shop.product.item42                  ← workerd evaluates against 19-route gen
    ELIFECYCLE  Command failed with exit code 1.

  AFTER
    [rsc-router] Generated route types (19 routes) -> ...      ← static parser pass
    rango:discovery getOrCreateTempServer: __rscRouterDiscoveryActive=true   ← layer 1 fix
    rango:discovery getOrCreateTempServer: importing entry (flag=true)
    [rsc-router] Router "..." -> 14219 routes (1215 static, 13004 dynamic)   ← runtime discovery succeeds
    [rsc-router] Generated route types -> ...                  ← gen rewritten to 14219 routes
    rango:routes await discoveryDone (manifest) (384.8ms)      ← layer 2 fix: workerd's manifest import gate
    reverse shop.product.item42: /shop/product/42              ← workerd reverse() succeeds
    VITE v7.3.0  ready in 2463 ms

What changed

The fix is two layers of the same handshake, both small and complementary. The PR narrative is "make the existing manifest readiness gate evaluate before user router modules" — see the file-by-file detail below.

Layer 1 — temp-runner discovery flag

packages/rangojs-router/src/vite/router-discovery.ts (getOrCreateTempServer)

Build's path at :874 already sets globalThis.__rscRouterDiscoveryActive = true before its temp Vite server's module runner imports the user entry. With the flag set, reverse(name) returns /__unresolved_reverse/<name> (placeholder) instead of throwing (reverse.ts:296-303), so factory-route apps survive the bootstrap import. The dev cloudflare branch was missing the same guard, so the temp runner crashed on the first module-level reverse() call and discovery was silently skipped.

const flagAlreadySet = !!(globalThis as any).__rscRouterDiscoveryActive;
if (!flagAlreadySet) (globalThis as any).__rscRouterDiscoveryActive = true;
try {
  /* import entry, read RouterRegistry */
} finally {
  if (!flagAlreadySet) delete (globalThis as any).__rscRouterDiscoveryActive;
}

Mirrors the build path's set/clear in a try/finally. Verified by DEBUG=rango:* pnpm dev against a wiped gen file: temp runner now completes at 14219 routes (1215 static, 13004 dynamic).

Layer 2 — version-injector import ordering

packages/rangojs-router/src/vite/plugins/version-injector.ts

The router already has the right gate: the manifest virtual module's load() hook awaits s.discoveryDone (router-discovery.ts:1017), and runtime discovery resolves that promise once the gen file is rewritten. But the gate only fires when import "virtual:rsc-router/routes-manifest" is evaluated, and ES module evaluation of side-effect imports follows source order in the dependency graph (NOT just declaration hoisting). When a consumer writes:

import { router } from "./router.js";        // line 2 — evaluates first
import "virtual:rsc-router/routes-manifest";  // line 7 — gate fires too late

./router.js evaluates first, runs router.reverse() at module level, and throws against a stale gen file before the gate ever blocks.

Old version-injector at :55-57 skipped prepending when the user already had the import anywhere in the source. New behavior: always prepend, ESM dedups the user's later duplicate so module init still runs once. Inserted after any leading /// <reference ... /> triple-slash directives so we don't silently demote TypeScript directives to plain comments. The misleading "ES imports are hoisted" comment was rewritten to explain side-effect ordering.

Diagnostics — preserved

The rango:discovery debug lines added in router-discovery.ts (entry path, flag state, import attempts, registry size, failure mode) were how the chain was traced. Kept inline so the next regression in this area is one DEBUG=rango:* pnpm dev away from being diagnosable. Reverted research-only console.logs in reverse.ts to keep that file (which runs in workerd/browser too) process.env-free.

Tests

Unit — packages/rangojs-router/src/vite/__tests__/version-injector.test.ts (3 cases):

Case Pins
User already imports manifest after router import Transformed code has manifest first; user's late import becomes harmless ESM duplicate
Leading /// <reference ... /> directive Imports go after the directive so TS triple-slash semantics survive
createRSCHandler auto-version injection Auto-injected VERSION import lands after the manifest gate

E2E — tests/cloudflare-stress-demo/e2e/dev-vs-build-named-routes.test.ts:

Wipes router.named-routes.gen.ts, spawns pnpm dev, waits for the runtime-discovery completion log line, then asserts dev's gen file matches the build's byte-for-byte. If either layer regresses, the runtime-discovery line never fires (timeout) or dev exits non-zero (caught by an exit listener). Hardened against process leaks (detached:true + process.kill(-pid, ...) for the whole pnpm → vite → workerd/miniflare tree, taskkill fallback on Windows) and against split-chunk log misses (line-aware buffered scan over accumulated stdout+stderr). Added at the recommendation of review.

Test plan

  • pnpm --filter @rangojs/router exec tsc --noEmit clean.
  • pnpm --filter @rangojs/router run test:unit — 3050 pass / 3 todo (+3 from new version-injector tests).
  • pnpm -w run lint — 0/0.
  • pnpm -w run format clean.
  • pnpm --filter @rangojs/router exec playwright test --project=smoke — 20/20.
  • pnpm --filter @rangojs/router exec playwright test --project=dev — 854 passed, 11 flaky (passed on retry), 0 failed.
  • pnpm --filter @rangojs/router exec playwright test --project=production — 596 passed, 0 failed.
  • pnpm --filter cloudflare-stress-demo exec playwright test — 7/7 (incl. new parity test).
  • Manual repro: rm tests/cloudflare-stress-demo/src/router.named-routes.gen.ts && pnpm --filter cloudflare-stress-demo run dev → VITE ready in 2463 ms, reverse shop.product.item42: /shop/product/42, gen file ends at 14228 lines / 14219 routes.

Notes

  • The two-layer framing matters: shipping layer 1 alone converts "temp discovery fails" into "temp discovery succeeds but workerd still throws on its own import." Both must land together.
  • The rango:discovery debug lines stay even after the fix lands. They're cheap (debug package, off by default) and the next time this area regresses they'll be the first thing a maintainer reaches for.
  • CI: cloudflare-stress-demo-e2e job (.github/workflows/e2e.yml:514) already runs on every non-draft PR, so the new parity test gates each PR without workflow changes.

ivogt added 2 commits April 26, 2026 17:52
…y routes

Fixes two layers of the same handshake bug where cloudflare-stress-demo
(or any cloudflare app with module-level `router.reverse()` and
factory-generated routes) crashed `pnpm dev` from a fresh / stale
`router.named-routes.gen.ts` with `Unknown route: shop.product.item42`.

## Layer 1 — temp runner runtime discovery never set the global flag

`router-discovery.ts:874` (build path) sets
`globalThis.__rscRouterDiscoveryActive = true` before the temp Vite
server's module runner imports the user entry, so module-level
`router.reverse()` calls return `/__unresolved_reverse/<name>` placeholders
(see `reverse.ts:299`) instead of throwing. The dev cloudflare branch
in `getOrCreateTempServer` was missing the same guard, so:

  1. configResolved's static parser found 19 literal routes and wrote
     them over a stale gen file.
  2. configureServer's temp runner imported `worker.rsc.tsx` →
     `router.tsx` → `reverse("shop.product.item42")` → throw (flag
     unset) → temp runner crashed → discovery skipped → gen file stayed
     at 19 routes for the rest of the dev session.

Mirroring the build path's flag set/clear in a try/finally heals the
temp runner. Verified by `DEBUG=rango:* pnpm dev` against a wiped gen
file: trace shows `getOrCreateTempServer: __rscRouterDiscoveryActive=true`,
the placeholder branch fires, registry size=1, `Router "c26945ad" -> 14219
routes` (1215 static, 13004 dynamic), gen rewritten end-to-end.

## Layer 2 — version-injector skipped the manifest import when user wrote it themselves

The manifest virtual module's `load()` hook in `router-discovery.ts:1017`
already awaits `s.discoveryDone` so the rest of the entry sees the fully
discovered route table. But the gate only fires from the load() of
`virtual:rsc-router/routes-manifest`, and ES module evaluation of
side-effect imports follows source order in the dependency graph (NOT
just declaration hoisting). So when a user writes:

    import { router } from "./router.js";        // line 2
    import "virtual:rsc-router/routes-manifest";  // line 7

`./router.js` (and its `module-level router.reverse(...)`) evaluates
first; the gate fires too late.

`version-injector.ts:55-57` previously skipped prepending when the user
already had the import anywhere in the file, deferring to the late
position. Now we always prepend (ESM dedups the user's late duplicate so
module init still runs once), and insert *after* any leading `///
<reference ... />` triple-slash directives so we don't silently demote
TypeScript directives to plain comments.

Together the two layers restore the readiness handshake the dev
cloudflare path was already designed for: temp runner populates routes
under the discovery flag, manifest virtual module's load() awaits the
resulting promise, workerd's eager `worker.rsc.tsx` import sees the
fully populated `router.named-routes.gen.ts` and `router.reverse(name)`
returns real URLs.

## Diagnostics — preserved

The `rango:discovery` debug lines added in this PR (entry path, flag
state, import attempts, registry size, failure mode) were how the chain
was traced. Kept inline so the next regression in this area is one
`DEBUG=rango:* pnpm dev` away from being diagnosable. Sample post-fix
trace excerpt:

    rango:discovery getOrCreateTempServer: __rscRouterDiscoveryActive=true
    rango:discovery getOrCreateTempServer: importing entry (flag=true)
    rango:discovery getOrCreateTempServer: registry size=1
    rango:discovery getOrCreateTempServer: cleared __rscRouterDiscoveryActive
    [rsc-router] Router "c26945ad" -> 14219 routes (1215 static, 13004 dynamic)
    rango:routes await discoveryDone (manifest) (384.8ms)
    rango:routes manifest module emitted (691 bytes)
    reverse shop.product.item42: /shop/product/42

## Tests

Adds three vitest cases pinning `version-injector.transform`:

  1. Always prepends the manifest import even when the entry already has
     it later in source (the cf-stress regression case).
  2. Inserts after leading `/// <reference ... />` directives so
     TypeScript triple-slash semantics are preserved.
  3. Keeps the auto-injected `VERSION` import, in order, after the
     manifest gate.

Verified manually against cloudflare-stress-demo with a wiped gen file:
`pnpm dev` reaches `VITE ready` in ~2.6s; `worker.rsc.tsx`'s module-level
`router.reverse("shop.product.item42")` returns `/shop/product/42`
post-discovery.

Full unit suite: 3050 pass / 3 todo (+3 from new tests).
Typecheck/lint/format clean.
…build

Adds an e2e regression test that wipes router.named-routes.gen.ts,
spawns `pnpm dev`, waits for the runtime-discovery completion log line,
then asserts dev's gen file matches the build's byte-for-byte.

Pre-fix the cloudflare-stress-demo crashed `pnpm dev` from a stale gen
file with `Unknown route: shop.product.item42`. The two-layer fix in
the same branch heals it; this test guards against either layer
regressing — a missing flag in the temp runner or an out-of-order
`virtual:rsc-router/routes-manifest` import would surface here as
either an unexpected dev exit or a gen-file mismatch.

Implementation notes (both incorporated from review):

  * Spawned with detached:true and torn down via process.kill(-pid)
    to target the whole process group (pnpm → vite → workerd/miniflare).
    A direct SIGTERM on pnpm doesn't propagate to descendants and
    leaks port 5002, breaking subsequent tests. Mirrors the existing
    fixture pattern in packages/rangojs-router/e2e/fixture.ts.

  * Stdout/stderr accumulated into a buffer and scanned line-aware
    rather than per-chunk. Node stream chunks are not line-aligned —
    a single chunk can carry both the static-parser line and the
    later runtime-discovery line, so per-chunk scanning would silently
    skip the second match and time out.

  * Falls back to taskkill /pid /t /f on Windows.
@pkg-pr-new

pkg-pr-new Bot commented Apr 26, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/@rangojs/router@485

commit: cc7a2f9

@ivogt
ivogt marked this pull request as draft April 26, 2026 15:43
@ivogt
ivogt marked this pull request as ready for review April 26, 2026 15:49
@ivogt
ivogt merged commit 40ef0fc into main Apr 26, 2026
25 checks passed
@ivogt
ivogt deleted the fix/cf-dev-runtime-discovery-flag-and-race branch April 26, 2026 15:51
ivogt added a commit that referenced this pull request Apr 26, 2026
…ediscovery

Stacked on top of the cold-start fix (#485). Pre-fix, after a successful
cloudflare dev cold start, any source-file change collapsed
`router.named-routes.gen.ts` from the full runtime manifest back to the
~19-route static-parser view, then propagated that partial map into
workerd via the manifest virtual module — `worker.rsc.tsx`'s module-level
`router.reverse("shop.product.item42")` then threw `Unknown route` and
killed the dev server.

The mechanism had three independent contributors and the fix addresses
each:

1. **Static-parser write was authoritative on HMR.** `scheduleRouteRegeneration`
   ran `writeCombinedRouteTypesWithTracking(s)` first; for cf-stress the
   static parser only sees 19 literal-named routes. The supplement helper
   (`supplementGenFilesWithRuntimeRoutes`) only restores `factoryOnlyPrefixes`
   — dot-prefixes entirely absent from static output — and cf-stress has
   `shop.home`, `api.benchFirst`, etc. as static names, so the
   `shop.`/`api.` prefixes are not factory-only and the factory siblings
   under those prefixes (`shop.product.item42` etc.) were dropped.

   Fix: when `!rscEnv?.runner && s.perRouterManifests.length > 0` (cloudflare
   dev with cold-start runtime state), skip the static write entirely. The
   runtime rediscovery below owns the gen file in this environment.

2. **Runtime rediscovery never ran in cloudflare HMR.** `refreshRuntimeDiscovery`
   bailed early when `rscEnv.runner` was unavailable, which is the entire
   cloudflare-dev case (workerd is a separate runtime). Even with #1
   skipping the static write, no fresh runtime discovery would happen,
   so newly-added factory routes wouldn't appear until restart.

   Fix: add a no-main-runner branch that mirrors cold start — close+null
   both `prerenderTempServer` AND `prerenderNodeRegistry`
   (`getOrCreateTempServer`'s cached-registry short-circuit at line ~387
   would otherwise keep the stale module graph), recreate via
   `getOrCreateTempServer()` under the `__rscRouterDiscoveryActive` flag,
   re-import the entry, refresh `s.perRouterManifests`, write via
   `writeRouteTypesFiles(s)`. ~700ms per save — acceptable, optimizable
   later via finer-grained module-graph invalidation.

3. **Manifest readiness gate didn't reset on HMR.** `s.discoveryDone` is
   the promise the manifest virtual module's `load()` awaits; once
   resolved at cold-start completion it stays resolved. So during HMR,
   any workerd reload triggered by the source change cascaded straight
   through the gate against whatever the current gen file said — even
   if rediscovery was about to overwrite it 700ms later.

   Fix: extract `beginDiscoveryGate()` / `resolveDiscoveryGate()` helpers
   that own the promise lifecycle and replace ad-hoc construction at
   both cold-start and HMR sites. Reset the gate IMMEDIATELY in
   `handleRouteFileChange` (before the 100ms debounce — refinement from
   review), so a workerd reload firing within the debounce window blocks
   on the new pending promise. Resolve the gate only after rediscovery's
   `writeRouteTypesFiles` completes (or in the `catch` so reloads don't
   hang on errors).

## Order of operations on HMR (cloudflare)

  source file change (chokidar)
    ↓ handleRouteFileChange()
  beginDiscoveryGate()  ← new pending promise, before any async / fanout
    ↓
  scheduleRouteRegeneration()  (100ms debounce)
    ↓ (skip static write — owned by rediscovery)
  refreshRuntimeDiscovery() — cloudflare branch:
    close+null prerenderTempServer + prerenderNodeRegistry
    ↓
    getOrCreateTempServer()  (sets __rscRouterDiscoveryActive)
    ↓
    discoverRouters(s, tempRscEnv) — refresh perRouterManifests
    ↓
    writeRouteTypesFiles(s) — gen file rewritten (or unchanged bytes)
    ↓
  resolveDiscoveryGate()  ← workerd's pending manifest load() unblocks
    ↓
  workerd evaluates router.tsx with the fresh gen file — reverse() works

## Tests

Extends `tests/cloudflare-stress-demo/e2e/dev-vs-build-named-routes.test.ts`
with a second case that covers the cold → touch → post-HMR scenario. Counts
`[rsc-router] Router "X" -> N routes` log lines (fired unconditionally per
discoverRouters cycle) rather than the gen-file-write log (only fires when
content changes, but HMR rediscovery often writes identical bytes). Pins:

  - cold runtime discovery completes
  - settle 2.5s (vite stdout buffering on non-TTY swallows "VITE ready" so
    we use a fixed pause)
  - touch a route source file (writeFile with content+"\n", because
    chokidar/vite ignore mtime-only bumps in some setups)
  - second runtime discovery completes (the regression signal — pre-fix
    this never fired in cf dev, the test would time out)
  - process is still alive (no `Unknown route` exit)
  - gen file matches build byte-for-byte

Process-tree cleanup and chunk-safe log scanning carried over from the
cold-start parity test (detached process group, `process.kill(-pid)`,
buffer-then-line-scan).

## What stayed the same

Node dev path is unchanged — its `rscEnv.runner` is real, so it continues
to use the existing in-process discoverRouters branch. The static-parser-
then-supplement dance is preserved for any environment where
`s.perRouterManifests.length === 0` (degraded mode where cold-start
runtime discovery never populated state, e.g. an unrelated startup error).

## Verification

  pnpm --filter @rangojs/router exec tsc --noEmit  — clean
  pnpm --filter @rangojs/router run test:unit       — 3050 pass / 3 todo
  pnpm -w run lint                                  — 0/0
  pnpm -w run format                                — clean
  pnpm --filter cloudflare-stress-demo exec playwright test
                                                    — 8/8 pass

Manual repro: rm gen file, pnpm dev → cold-start writes 14228 routes,
touch shop-patterns.tsx → second discovery cycle logs
`Router "c26945ad" -> 14219 routes (1215 static, 13004 dynamic)`,
gen file stays at 14228 lines, workerd reverses `/shop/product/42`,
no `Unknown route`.
ivogt added a commit that referenced this pull request Apr 27, 2026
…ediscovery (#486)

* fix(router): cloudflare HMR resettable discovery gate + temp-runner rediscovery

Stacked on top of the cold-start fix (#485). Pre-fix, after a successful
cloudflare dev cold start, any source-file change collapsed
`router.named-routes.gen.ts` from the full runtime manifest back to the
~19-route static-parser view, then propagated that partial map into
workerd via the manifest virtual module — `worker.rsc.tsx`'s module-level
`router.reverse("shop.product.item42")` then threw `Unknown route` and
killed the dev server.

The mechanism had three independent contributors and the fix addresses
each:

1. **Static-parser write was authoritative on HMR.** `scheduleRouteRegeneration`
   ran `writeCombinedRouteTypesWithTracking(s)` first; for cf-stress the
   static parser only sees 19 literal-named routes. The supplement helper
   (`supplementGenFilesWithRuntimeRoutes`) only restores `factoryOnlyPrefixes`
   — dot-prefixes entirely absent from static output — and cf-stress has
   `shop.home`, `api.benchFirst`, etc. as static names, so the
   `shop.`/`api.` prefixes are not factory-only and the factory siblings
   under those prefixes (`shop.product.item42` etc.) were dropped.

   Fix: when `!rscEnv?.runner && s.perRouterManifests.length > 0` (cloudflare
   dev with cold-start runtime state), skip the static write entirely. The
   runtime rediscovery below owns the gen file in this environment.

2. **Runtime rediscovery never ran in cloudflare HMR.** `refreshRuntimeDiscovery`
   bailed early when `rscEnv.runner` was unavailable, which is the entire
   cloudflare-dev case (workerd is a separate runtime). Even with #1
   skipping the static write, no fresh runtime discovery would happen,
   so newly-added factory routes wouldn't appear until restart.

   Fix: add a no-main-runner branch that mirrors cold start — close+null
   both `prerenderTempServer` AND `prerenderNodeRegistry`
   (`getOrCreateTempServer`'s cached-registry short-circuit at line ~387
   would otherwise keep the stale module graph), recreate via
   `getOrCreateTempServer()` under the `__rscRouterDiscoveryActive` flag,
   re-import the entry, refresh `s.perRouterManifests`, write via
   `writeRouteTypesFiles(s)`. ~700ms per save — acceptable, optimizable
   later via finer-grained module-graph invalidation.

3. **Manifest readiness gate didn't reset on HMR.** `s.discoveryDone` is
   the promise the manifest virtual module's `load()` awaits; once
   resolved at cold-start completion it stays resolved. So during HMR,
   any workerd reload triggered by the source change cascaded straight
   through the gate against whatever the current gen file said — even
   if rediscovery was about to overwrite it 700ms later.

   Fix: extract `beginDiscoveryGate()` / `resolveDiscoveryGate()` helpers
   that own the promise lifecycle and replace ad-hoc construction at
   both cold-start and HMR sites. Reset the gate IMMEDIATELY in
   `handleRouteFileChange` (before the 100ms debounce — refinement from
   review), so a workerd reload firing within the debounce window blocks
   on the new pending promise. Resolve the gate only after rediscovery's
   `writeRouteTypesFiles` completes (or in the `catch` so reloads don't
   hang on errors).

## Order of operations on HMR (cloudflare)

  source file change (chokidar)
    ↓ handleRouteFileChange()
  beginDiscoveryGate()  ← new pending promise, before any async / fanout
    ↓
  scheduleRouteRegeneration()  (100ms debounce)
    ↓ (skip static write — owned by rediscovery)
  refreshRuntimeDiscovery() — cloudflare branch:
    close+null prerenderTempServer + prerenderNodeRegistry
    ↓
    getOrCreateTempServer()  (sets __rscRouterDiscoveryActive)
    ↓
    discoverRouters(s, tempRscEnv) — refresh perRouterManifests
    ↓
    writeRouteTypesFiles(s) — gen file rewritten (or unchanged bytes)
    ↓
  resolveDiscoveryGate()  ← workerd's pending manifest load() unblocks
    ↓
  workerd evaluates router.tsx with the fresh gen file — reverse() works

## Tests

Extends `tests/cloudflare-stress-demo/e2e/dev-vs-build-named-routes.test.ts`
with a second case that covers the cold → touch → post-HMR scenario. Counts
`[rsc-router] Router "X" -> N routes` log lines (fired unconditionally per
discoverRouters cycle) rather than the gen-file-write log (only fires when
content changes, but HMR rediscovery often writes identical bytes). Pins:

  - cold runtime discovery completes
  - settle 2.5s (vite stdout buffering on non-TTY swallows "VITE ready" so
    we use a fixed pause)
  - touch a route source file (writeFile with content+"\n", because
    chokidar/vite ignore mtime-only bumps in some setups)
  - second runtime discovery completes (the regression signal — pre-fix
    this never fired in cf dev, the test would time out)
  - process is still alive (no `Unknown route` exit)
  - gen file matches build byte-for-byte

Process-tree cleanup and chunk-safe log scanning carried over from the
cold-start parity test (detached process group, `process.kill(-pid)`,
buffer-then-line-scan).

## What stayed the same

Node dev path is unchanged — its `rscEnv.runner` is real, so it continues
to use the existing in-process discoverRouters branch. The static-parser-
then-supplement dance is preserved for any environment where
`s.perRouterManifests.length === 0` (degraded mode where cold-start
runtime discovery never populated state, e.g. an unrelated startup error).

## Verification

  pnpm --filter @rangojs/router exec tsc --noEmit  — clean
  pnpm --filter @rangojs/router run test:unit       — 3050 pass / 3 todo
  pnpm -w run lint                                  — 0/0
  pnpm -w run format                                — clean
  pnpm --filter cloudflare-stress-demo exec playwright test
                                                    — 8/8 pass

Manual repro: rm gen file, pnpm dev → cold-start writes 14228 routes,
touch shop-patterns.tsx → second discovery cycle logs
`Router "c26945ad" -> 14219 routes (1215 static, 13004 dynamic)`,
gen file stays at 14228 lines, workerd reverses `/shop/product/42`,
no `Unknown route`.

* fix(router): make HMR discovery gate idempotent + queue overlapping rediscoveries

Two race conditions on top of the previous HMR fix, both of which could
leave workerd hanging on the manifest virtual module's `s.discoveryDone`
await with no path to resolution. Surfaced by code review of the previous
commit on this branch.

## Bug 1 — beginDiscoveryGate() orphans the in-flight promise

The previous version overwrote `s.discoveryDone` and `gateResolver` on
every call. File watchers can fire multiple change/add events for a
single save (chokidar's polling mode on CI Linux, atomic-save flows that
unlink+rename instead of writing in-place). When the second event fires
beginDiscoveryGate() while workerd is already awaiting the first
promise, the resolver pointer moves to the new promise — the original is
no longer reachable, never resolves, and workerd's load() hangs forever.

Fix: track `gatePending` and make `beginDiscoveryGate()` a no-op while
pending. The same single promise covers all events that arrive within a
single rediscovery cycle. resolveDiscoveryGate() is similarly idempotent.

## Bug 2 — runtimeRediscoveryInProgress early-return orphans a fresh gate

`refreshRuntimeDiscovery()` returned immediately when a previous run was
in flight. handleRouteFileChange (which fires before the debounce) had
already reset the gate to a new pending promise — but the early-return
path didn't resolve it, and the in-flight run's `finally` only resolved
the gate it was originally tied to. Result: a second gate gets created
and never resolved.

Fix: queue a follow-up cycle instead of dropping the event. If a
rediscovery is in flight, set `runtimeRediscoveryQueued = true` and
return. The active run's `finally` consumes the queue (recurses) and
ONLY resolves the gate at the tail of the chain — when no further work
is queued. Belt-and-suspenders: the queue's outer .catch() also resolves
the gate so a thrown queued cycle can't hang.

## Order of operations (cf dev, two events overlapping)

  t=0    Touch 1 fires
         handleRouteFileChange → beginDiscoveryGate() (gate1 pending)
         scheduleRouteRegeneration debounces 100ms

  t=100  Refresh #1 begins (~700ms work)
         runtimeRediscoveryInProgress = true

  t=200  Touch 2 fires
         handleRouteFileChange → beginDiscoveryGate() (no-op: gate1 still pending)
         scheduleRouteRegeneration debounces 100ms (timer reset)

  t=300  Refresh #2 called → in flight → runtimeRediscoveryQueued = true, return
         (gate1 still pending — does NOT get replaced)

  t=800  Refresh #1 finally
         runtimeRediscoveryInProgress = false
         queued = true → consume: recursive refresh()
         (do NOT resolve gate1 yet)

  t=800  Recursive refresh begins (~700ms)

  t=1500 Recursive refresh finally
         queued = false → resolveDiscoveryGate() → gate1 resolves
         workerd's manifest load() unblocks against the freshly written gen

## Tests

Adds a third e2e case to cf-stress: cold start, then two source-file
edits with a 200ms gap (tuned to land the second edit during the first
refresh's ~700ms work window). Asserts:

  - dev process stays alive
  - gen file equals build byte-for-byte
  - the queue path actually fired by matching the new rango:discovery
    debug lines: "rediscovery in flight — queued for a follow-up cycle"
    and "consuming queued rediscovery". Without this assertion, a
    timing-collapse where both events fall in the same debounce window
    would silently skip the bug surface and pass.

DEBUG=rango:discovery is set explicitly on this test's spawn so the
queue debug lines surface.

## Verification

  pnpm --filter @rangojs/router run test:unit       — 3050 pass / 3 todo
  pnpm --filter @rangojs/router exec tsc --noEmit   — clean
  pnpm -w run lint                                   — 0/0
  pnpm -w run format                                 — clean
  pnpm --filter cloudflare-stress-demo exec playwright test
                                                     — 9/9 pass (incl. 3 dev/build/HMR + 1 rapid-events)

Manual repro under DEBUG=rango:* with a 200ms gap between touches:

  Router "c26945ad" -> 14219 routes              # cold-start
  hmr: rediscovery in flight — queued for a follow-up cycle
  Router "c26945ad" -> 14219 routes              # HMR cycle #1
  hmr: consuming queued rediscovery
  Router "c26945ad" -> 14219 routes              # queued cycle
  reverse shop.product.item42: /shop/product/42  # workerd unblocks

* fix(router): close the HMR tail-race window — pendingRouteEvents flag

Stacks on the previous commit. Code review surfaced a third state-tracking
bug: the queue mechanism only handles overlap during refresh A's WORK
phase, not the brief window between A's writeRouteTypesFiles and A's
finally — events arriving in that window mark `gatePending=true` (no-op
on the still-pending gate1) and arm a fresh debounce, but their
`refreshRuntimeDiscovery()` call hasn't happened yet, so
`runtimeRediscoveryQueued` is still false. A's finally then resolves
gate1, and the workerd window between resolve and refresh B's start can
read stale gen.

## Repro window

  t=0    Touch 1 → handleRouteFileChange → beginDiscoveryGate (gate1)
                 → debounce 100ms
  t=100  Refresh A begins (~700ms work)
  t=750  Touch 2 (during A's TAIL) → handleRouteFileChange:
           - beginDiscoveryGate (no-op: gatePending=true)
           - schedule debounce, fires at t=850
         queued is still false (debounce hasn't called refresh yet)
  t=800  Refresh A's finally:
           queued=false  → pre-fix: RESOLVES gate1
           ┌─── workerd reload window (~50ms) ───┐
           │  manifest load() awaits resolved    │
           │  gate → returns IMMEDIATELY against │
           │  the post-A gen file, which doesn't │
           │  yet reflect Touch 2's change       │
           └─────────────────────────────────────┘
  t=850  Refresh B starts — but no fresh gate to bind to

## Fix

Add `pendingRouteEvents` flag, set in handleRouteFileChange at event
time (alongside the gate reset). Cleared at the start of refresh's main
work path (the events that triggered THIS run are now being consumed).
Refresh's finally checks the flag and KEEPS the gate pending if it's
true, with no need to recurse — the debounced timer that armed for the
late event is still scheduled and will fire on its own. The next refresh
cycle's finally then clears the flag and resolves.

Three branches in the finally now:

  1. queued=true   → recurse (queue overlap; gate stays pending)
  2. pending=true  → don't resolve, don't recurse — debounce will fire
                     and trigger the next refresh; gate stays pending
  3. neither       → resolve gate

## Diagnostic log lines

Added two for traceability:

  - `hmr: discoveryDone resolved` — fires only when gatePending was true
    and we actually transitioned it to false. Useful for ordering
    assertions in tests and debug sessions.
  - `hmr: holding gate for pending events (debounce not yet fired)` —
    fires when the new pendingRouteEvents branch keeps the gate alive.

## Tests

  - The existing rapid-events test now uses `waitForLog` for deterministic
    timing: waits for `hmr: closing cached temp server` (refresh A
    in-flight) before writing touch 2. That guarantees we exercise the
    queue path. Asserts the queue debug pair AND the gate-resolution log
    line appears at the trailing end (proving idempotent gate behavior).

  - New `rapid edits near refresh tail keep gen file consistent` test:
    waits for `hmr writeRouteTypesFiles` (refresh A's last sync work
    step) before writing touch 2. Asserts dev stays alive and gen ends
    up matching build. Doesn't assert on the specific code path
    (pendingRouteEvents vs queued) because chokidar latency on macOS/
    Linux/CI varies ~50–200ms and can push the event into either window;
    the user-visible contract (dev alive + gen correct + 3 discoveries)
    is what we pin.

## Verification

  pnpm --filter @rangojs/router exec tsc --noEmit  — clean
  pnpm --filter @rangojs/router run test:unit       — 3050 pass / 3 todo
  pnpm -w run lint                                   — 0/0
  pnpm -w run format                                 — clean
  pnpm --filter cloudflare-stress-demo exec playwright test
                                                     — 10/10 pass

Manual repro under DEBUG=rango:* with touches around the refresh tail:

  Router "c26945ad" -> 14219 routes              # cold-start
  hmr: closing cached temp server                # refresh A starts
  hmr: rediscovery in flight — queued for a follow-up cycle
  Router "c26945ad" -> 14219 routes              # refresh A
  hmr: consuming queued rediscovery
  Router "c26945ad" -> 14219 routes              # queued cycle
  hmr: discoveryDone resolved                    # gate held until tail
  reverse shop.product.item42: /shop/product/42  # workerd reads fresh gen

* test(cf-stress): tighten HMR tests with route-meaningful edits + gen-file polling

Two refinements from code review on the HMR/rapid-events tests:

## 1. Whitespace-only edits couldn't distinguish stale-A from fresh-B

Previously both touches in the rapid-events and tail-race tests appended
whitespace. Refresh A and refresh B produced byte-identical gen files,
so the assertion `postGen === buildGen` would pass even if the gate had
been resolved against stale A and refresh B never observed touch 2 —
the test was mostly proving "dev survives two HMR cycles," not "the
gate-state fix protects a route-changing edit."

Touch 2 in both tests now changes `Array.from({ length: 100 }` to
`{ length: 101 }` in shop-patterns.tsx, adding a `shop.product.item101`
route that ONLY exists if refresh B successfully re-imports the edited
source. The assertion is now `postGen.toContain('shop.product.item101')`
which directly proves refresh B's output reached disk.

## 2. `discoveryDone resolved` waits matched cold-start

`resolveDiscoveryGate()` logs the same line on cold-start completion
and on every subsequent HMR resolve. The previous `waitForLog(...)` calls
searched the whole buffer, which short-circuited on the cold-start
match and didn't pin the post-HMR resolution.

Switching from `waitForLog(/discoveryDone resolved/)` to
`waitForGenFileContains(genFilePath, 'shop.product.item101')` is more
robust on two axes: it ignores cold-start log entries entirely (the
content check is a direct end-state probe), and it doesn't depend on
log-line ordering (refresh A's gate resolution can race with chokidar
latency — touch 2's handler sometimes fires before A's finally and
sometimes after; either path leads to the same correct end state).

For the in-flight `hmr: rediscovery in flight — queued for a follow-up
cycle` and `hmr: closing cached temp server` markers, the existing
`waitForLogFromIdx(..., preHmrIdx)` is preserved because those lines
genuinely don't fire pre-HMR.

## What the tests now prove

  Test 1 ("rapid duplicate route-file events"):
    - cold-start writes 14219 routes
    - touch 1 (whitespace) starts refresh A; we wait for "closing cached
      temp server" to confirm A is in-flight
    - touch 2 (length 100→101) lands in queue path
    - assert: queue debug pair fires
    - assert: gen file ends up containing item101
       → proves refresh B observed the edited source

  Test 2 ("rapid edits near refresh tail"):
    - same flow, but touch 2 fires after `hmr writeRouteTypesFiles`
      (refresh A's last sync step) → debounce can fire before/after A's
      finally depending on chokidar latency
    - assert: dev process stays alive
    - assert: gen file ends up containing item101
       → proves refresh B observed the edited source regardless of which
         code path (queue vs pendingRouteEvents vs new gate) handled it

Both tests now correctly distinguish stale-A from fresh-B.

## Verification

  pnpm --filter @rangojs/router exec tsc --noEmit  — clean
  pnpm --filter @rangojs/router run test:unit       — 3050 pass / 3 todo
  pnpm -w run lint                                   — 0/0
  pnpm -w run format                                 — clean
  pnpm --filter cloudflare-stress-demo exec playwright test
                                                     — 10/10 pass

Helper added: `waitForGenFileContains` polls the gen file on disk for a
target substring, with a 30s timeout. Cleaner than log-line scraping
because it tests the user-visible contract directly.

Removed: the inline `waitForLog` helpers in both tests — replaced by
`waitForLogFromIdx` (offset-anchored, defined once at module scope) for
log markers and `waitForGenFileContains` for end-state assertions.

* refactor(router): extract discovery gate state machine + add deterministic unit tests

The gate/queue/pending state machine that keeps `s.discoveryDone`
consistent across HMR fan-out is now in its own module
(`./discovery/gate-state.ts`) with a focused factory function and a
documented four-flag invariant. router-discovery.ts shrinks by ~80
lines; the env-specific work (close+recreate temp server, discoverRouters,
writeRouteTypesFiles, propagateDiscoveryState) is the only thing it
still owns — the state transitions are delegated.

## Why now

Code review flagged that the cf-stress e2e tail-race test was probabilistic:

  > It waits for the child process to print `hmr writeRouteTypesFiles`,
  > then the parent writes touch 2. Since stdout delivery is async, the
  > dev process may already have run refresh A's `finally` before the
  > parent observes that log. In that common path, touch 2 creates a
  > normal fresh gate after A, so pre-fix code could still pass.

Three options were on the table: a test-only delay between the write
and the finally, asserting the `holding gate for pending events` log
under that delay, or extracting the state machine into a unit test.
Extraction is the longest-lived option: the state machine becomes
unit-testable in isolation, the e2e tests stop carrying the burden of
proving timing-sensitive invariants, and router-discovery.ts gets
notably more readable.

## What's in gate-state.ts

  createDiscoveryGate(s, debug?) → DiscoveryGate

  beginGate()        idempotent: reset s.discoveryDone, no-op if pending
  resolveGate()      idempotent: settle the pending gate
  noteRouteEvent()   set pendingEvents=true + beginGate()
  runRefreshCycle()  manage queued/pending around the work callback
  state()            test-only state snapshot

The module's JSDoc spells out the four flags and the t=0..t=1500 timeline
diagram for the cf-stress repro. Three terminal cases in the cycle's
finally:

  - queued        → recurse with the work fn (gate stays pending)
  - pendingEvents → hold gate (next debounced cycle resolves)
  - neither       → resolveGate

Belt-and-suspenders: the queued-cycle's recursion is fire-and-forget
with a .catch that calls resolveGate, so a thrown work fn can't hang
workerd indefinitely.

## Unit tests (9 cases, all deterministic)

  - beginGate creates a pending promise; resolveGate settles it
  - beginGate is a no-op when already pending — preserves the existing
    resolver (regression: chokidar-fired duplicate events)
  - resolveGate is a no-op when no gate is pending
  - clean run with no events resolves the gate at the tail
  - **noteRouteEvent during work does NOT prematurely resolve the gate**
    (the exact tail-race window the cf-stress e2e couldn't deterministically
    cover; this test drives state transitions synchronously)
  - overlap (queued path) — second call during in-flight is queued and
    consumed at tail
  - combined queued + pendingEvents flows through correctly
  - thrown work in a top-level cycle still releases the gate
  - thrown work in a queued cycle still releases the gate
    (fire-and-forget recursion's .catch belt-and-suspenders)

3050 → 3059 unit tests after this commit.

## E2E test refinements

The cf-stress `waitForGenFileContains` helper now accepts an optional
`getBuffer` so timeouts append the captured dev stdout/stderr — without
that, gen-file-polling failures lost the most useful diagnostic
(reviewer flagged this on the previous round). Both call sites updated
to pass `() => buffer`.

The two tail-race / rapid-events e2e tests are kept (they prove the
integrated path under real chokidar/Vite/workerd timing) but the
gate-state invariants are now pinned by the unit tests above. The
e2es' role is "the wired-up plumbing works"; the unit tests' role is
"the state machine cannot regress."

## What didn't change

- Public surface of router-discovery.ts (the gate is still exposed via
  `s.discoveryDone` for the manifest virtual module's load() hook).
- HMR semantics, debug log lines, or the env-specific work paths.
- Cold-start: still uses beginGate + setTimeout(discover, 0) + resolveGate.

## Verification

  pnpm --filter @rangojs/router exec tsc --noEmit  — clean
  pnpm --filter @rangojs/router run test:unit       — 3059 pass / 3 todo
  pnpm -w run lint                                   — 0/0
  pnpm -w run format                                 — clean
  pnpm --filter cloudflare-stress-demo exec playwright test
                                                     — 10/10 pass

* docs(internal): list gate-state.ts in the Vite plugin ecosystem feature map

Per the repo convention (each new production source file should appear in
the internal feature/file ownership map), `vite/discovery/gate-state.ts`
gets added to the "Vite plugin ecosystem" row. The note column also
calls out the four-flag state machine and that it's owned by
`gate-state.ts` and consumed by `router-discovery.ts`, so future
maintainers can find the contract docs (and the unit tests) by following
the file reference.

Pure docs change — no code or tests affected.

* refactor(router): cf HMR uses module-graph invalidation instead of close+recreate

Closes the first-edit transport-disconnect window the user observed and
hardens the temp-server refresh against staleness via direct e2e proof.

## Why

Pre-fix HMR rediscovery did `prerenderTempServer.close() +
createTempRscServer(...)` — closing one Vite instance and creating
another while the main dev server's transport to workerd was still
mid-reconnect (workerd's first post-cold-start module-fetch window).
The user-visible symptom was, on the FIRST edit of a top-of-graph file
like `urls.tsx`:

  [vite] An error happened during full reload
  transport was disconnected, cannot call "fetchModule"
    at /packages/rangojs-router/src/router/manifest.ts:9:31
    at /packages/rangojs-router/src/router/match-api.ts:10:31

The system recovered (workerd reconnected, retried, eventually rendered
the right routes), but the error in the log was alarming and pointed at
real transport disruption. Subsequent edits were fine because workerd's
module cache was warm by then — its eval finished before our close()
ran.

## What changed

`vite/router-discovery.ts`:

  1. Extracted `importEntryAndRegistry(tempRscEnv)` — owns the
     `__rscRouterDiscoveryActive` flag handshake + entry import +
     RouterRegistry read. Used by both cold-start (`getOrCreateTempServer`)
     and HMR (`refreshTempRscEnv`).

  2. Added `refreshTempRscEnv()` — keep the same temp Vite instance
     alive across HMR cycles. Steps:

       a. Ensure server exists (create on first call, reuse otherwise).
       b. Pick the module graph: env-level (Vite 6+ environments API,
          where the module-runner cache lives) preferred; server-level
          fallback. If neither exposes `invalidateAll()`, fall back to
          close+recreate so we never lose the freshness contract.
       c. Invalidate the chosen graph.
       d. Drop cached `prerenderNodeRegistry`.
       e. `importEntryAndRegistry` re-fetches source through the
          now-invalidated runner, refreshes registry.

  3. `refreshRuntimeDiscovery`'s cloudflare branch now calls
     `refreshTempRscEnv()` instead of close+recreate.

The same Vite instance staying alive means workerd's transport is never
torn down at HMR time. Verified manually: edited urls.tsx multiple
times from cold start, 0 `transport was disconnected` errors. Add /
remove / rename of factory routes all flow through correctly (env-level
`invalidateAll()` actually re-reads source on disk — proven by the
remove case below).

## Tests

Three new e2e cases on `dev-vs-build-named-routes.test.ts`:

  - **HMR remove factory route — invalidation drops it from gen**:
    bumps `length: 100 → 101` (item101 appears) then back to `100`
    (item101 must disappear). If the runner served cached source on
    the second edit, the gen file would still contain item101 and the
    `waitForGenFileMissing` would time out. Pre-fix close+recreate
    handled this correctly because the new Vite instance had no
    cache; with module-graph invalidation we have to prove it
    actively re-reads disk on each cycle.

  - **HMR rename factory route — old names disappear, new appear**:
    rename `name: \`item${i + 1}\`` → `\`product${i + 1}\``. Asserts
    `product1` appears AND `item42` disappears. Catches "additive"
    staleness (where both versions persist) and "no-update" staleness
    (where only old persists).

  - **first urls.tsx edit after cold start does NOT log
    `transport was disconnected`**: directly pins the absence of the
    error string in the post-edit buffer slice. Also warns (not fails)
    if the close+recreate fallback path was taken instead of
    invalidation, so an API drift in Vite's `moduleGraph.invalidateAll`
    would surface in test logs without hard-failing the suite.

The existing `rapid duplicate route-file events` test was updated to
wait for the new `refreshTempRscEnv: invalidating module graph` log
line instead of the now-removed `hmr: closing cached temp server`.

Tests also added a `runDevAndWaitForRediscovery` shared helper to
deduplicate the spawn+watcher+cold-discovery scaffolding across the
three new cases (eliminates ~120 lines of repetition).

## Verification

  pnpm --filter @rangojs/router exec tsc --noEmit  — clean
  pnpm --filter @rangojs/router run test:unit       — 3059 pass / 3 todo
  pnpm -w run lint                                   — 0/0
  pnpm -w run format                                 — clean
  pnpm --filter cloudflare-stress-demo exec playwright test
                                                     — 13/13 pass

Manual repro under DEBUG=rango:* — multiple urls.tsx edits from cold
start, all clean:

  refreshTempRscEnv: invalidating module graph (env)
  hmr re-discovery done (756ms)
  Router "c26945ad" -> 14219 routes (1215 static, 13004 dynamic)
  reverse shop.product.item42: /shop/product/42
  # ... no `transport was disconnected` ...

Plus add (100→101 → 14220 routes, item101 in gen) and remove
(101→100 → 14219 routes, item101 gone).

* fix(router): close two cf HMR lifecycle edges (orphan temp server + ghost router registry)

Reviewer flagged two lifecycle edges in the new module-graph
invalidation path:

## 1. Persistent temp server retains removed routers in RouterRegistry

`createRouter()` does `RouterRegistry.set(routerId, router)` on a
package-level singleton Map. `moduleGraph.invalidateAll()` clears
Vite's module cache but NOT this Map. For an edit that removes an
entire `createRouter()` call or changes a router id, the OLD entry
persists in the registry alongside whatever the freshly re-imported
source declares — discoverRouters then emits ghost routes from the
removed/renamed router.

Fix: new `clearTempRegistries(tempRscEnv)` helper that imports
`@rangojs/router/server` through the runner (same module instance the
re-imported entry will populate) and calls `.clear()` on both
`RouterRegistry` and `HostRouterRegistry`. Defensive: optional-chained
`.clear?.()` so the helper survives a non-Map registry shape.

Wired into `refreshTempRscEnv` between `target.invalidateAll()` and
`importEntryAndRegistry()`. The cf-stress fixture only has one router
so the multi-router scenario isn't directly exercised — but the
existing "HMR remove factory route" test now asserts the new
`clearTempRegistries: cleared RouterRegistry + HostRouterRegistry`
debug log fires post-edit, pinning the lifecycle code path.

## 2. Failed refresh leaves an orphaned temp server

`getOrCreateTempServer()` used `prerenderNodeRegistry` as the cache
sentinel. `refreshTempRscEnv()` flow was:

  1. getOrCreateTempServer (returns existing env)
  2. invalidateAll
  3. prerenderNodeRegistry = null
  4. importEntryAndRegistry — IF THIS THROWS

…the previous `prerenderTempServer` was still alive but with no
registry. The next `getOrCreateTempServer()` call saw no registry,
fell through to the create path, and overwrote the handle — the
previous server became orphaned (still alive, no references, never
closed).

Fix: `getOrCreateTempServer()` now checks for an existing temp
server FIRST. Three branches:

  - server alive + registry cached → return immediately (steady-state)
  - server alive + registry missing → re-import on the existing runner;
    on success, return; on failure, close+null the orphan and fall
    through to create-new
  - server alive but rsc.runner missing → close+null, create new
  - no server → create new

This guarantees we never silently leak a temp Vite instance after a
failed refresh.

## What didn't change

- `refreshTempRscEnv`'s happy path still goes invalidateAll →
  clearTempRegistries → importEntryAndRegistry. Both lifecycle
  fixes layer cleanly on top.
- close+recreate fallback is still present for Vite versions where
  neither env nor server graph exposes `invalidateAll()`.
- Node dev path is unchanged.

## Verification

  pnpm --filter @rangojs/router exec tsc --noEmit  — clean
  pnpm --filter cloudflare-stress-demo exec playwright test
                                                     — 13/13 pass
  pnpm --filter @rangojs/router run test:unit       — 3059 pass / 3 todo
  pnpm -w run lint / format                          — clean

Manual repro under `DEBUG=rango:* pnpm dev` confirms the lifecycle log
fires post-edit:

  refreshTempRscEnv: invalidating module graph (env)
  clearTempRegistries: cleared RouterRegistry + HostRouterRegistry
  hmr re-discovery done (619ms)

## Follow-up not in this PR

The reviewer also suggested an e2e test for the multi-router /
removed-router scenario specifically. The cf-stress fixture only
declares one router, so adding that case requires a separate fixture
app (or a temporary second `createRouter()` in cf-stress that the
test harness adds and removes). Tracked as future work; for this PR
the lifecycle code path is pinned by the debug-log assertion.

* fix(router): cold-start resolveGate must defer when HMR cycle is in flight

ultrareview caught a real race I missed in the gate state machine:

  beginDiscoveryGate();
  setTimeout(() => discover().then(resolveDiscoveryGate, resolveDiscoveryGate), 0);

`resolveGate` only checked `gatePending`, ignoring `inProgress` /
`pendingEvents`. If a user edits a route file between
`discoverRouters` committing `s.perRouterManifests` and cold-start's
`discover()` returning:

  1. handleRouteFileChange → noteRouteEvent (pendingEvents=true)
  2. 100ms debounce fires → runRefreshCycle → inProgress=true, work starts
  3. cold-start's discover() resolves
  4. its `.then(resolveGate)` fires → naively settles s.discoveryDone
  5. workerd's manifest virtual module load() unblocks against the
     stale cold-start gen file (HMR cycle's writeRouteTypesFiles
     hasn't run yet)

The new e2e suite's 2.5s settle pause masked it — every existing test
waited until cold-start was fully past the window before editing.

## Fix

`resolveGate` now defers when ANY in-flight signal is set
(`inProgress | queued | pendingEvents`). The runRefreshCycle's
`finally` is the canonical resolver — it calls resolveGate at the
tail when all work is drained. Cold-start's resolveGate becomes a
no-op if it fires while a cycle is in progress; the cycle's finally
handles resolution.

```ts
const resolveGate = (): void => {
  if (!gatePending) return;
  if (inProgress || queued || pendingEvents) {
    debug?.("hmr: resolveGate deferred — work in flight ...");
    return;
  }
  gatePending = false;
  debug?.("hmr: discoveryDone resolved");
  gateResolver();
};
```

## Why this over routing cold-start through runRefreshCycle

The reviewer's suggested fix was to make cold-start use
`runRefreshCycle(discover)`, so cold-start participates in the same
queue contract. That's architecturally cleaner, but has a subtle
correctness issue: when the queue recurses, it re-runs the SAME work
fn — for cold-start that's `discover()`, which uses
`getOrCreateTempServer()` (no module-graph invalidation). The HMR
work fn that triggered the queue uses `refreshTempRscEnv()` (with
invalidation). Reusing cold-start's work would skip invalidation and
re-introduce stale-source risk.

The state-aware resolveGate is more surgical:

  - cold-start retains its raw beginGate/discover/resolveGate lifecycle
  - resolveGate becomes a no-op when work is queued, so cold-start
    can't prematurely settle the gate
  - HMR's runRefreshCycle is the canonical resolver, with the
    correct (invalidating) work fn

Same correctness, no work-fn ownership question.

## Tests

Two new gate-state unit tests cover both shapes:

  - **resolveGate is deferred when runRefreshCycle is in flight (cold-start race)**:
    beginGate → noteRouteEvent → runRefreshCycle (work pending) →
    resolveGate fires → assert gate STILL pending → finish work →
    cycle's finally resolves.

  - **resolveGate is deferred when pendingEvents is true**: beginGate
    → noteRouteEvent → resolveGate fires BEFORE debounce expires →
    assert deferred → debounce → cycle resolves.

Verified by temporarily reverting the guard: both new tests fail.
With guard restored: 11/11 pass.

## Verification

  pnpm --filter @rangojs/router exec tsc --noEmit  — clean
  pnpm --filter @rangojs/router exec vitest run gate-state.test.ts
                                                     — 11/11 (+2 new)
  pnpm --filter @rangojs/router run test:unit       — 3061 pass / 3 todo
  pnpm --filter cloudflare-stress-demo exec playwright test
                                                     — 13/13 pass
  pnpm -w run lint / format                          — clean

## Note on the 2.5s settle

The reviewer also suggested dropping the 2.5s settle pause in
dev-vs-build-named-routes.test.ts now that the gate handles the race.
Keeping it for now — the settle also covers chokidar watcher-readiness
and vite's full-reload fan-out, which are independent concerns from
the gate. Reducing it can land in a follow-up after observing CI
stability across a few runs.

* chore: re-trigger CI

* fix(router): suppress duplicate cf HMR cycle from gen-file self-write

Every cf HMR cycle was producing two `[vite] program reload` events
instead of one — the second came from vite's chokidar detecting the
gen-file write that `refreshTempRscEnv → writeRouteTypesFiles` performs
at the tail of the cycle. The chokidar watcher was already
short-circuited via `consumeSelfGenWrite` in our own
`handleRouteFileChange`, but vite's independent HMR pipeline still
processed the change and re-evaluated importers (visible client-side as
a duplicate "HMR: version changed" event).

Add a `handleHotUpdate` plugin hook that returns `[]` when the changed
file is in `selfWrittenGenFiles`, dropping the cascade. Use a new
non-consuming `peekSelfGenWrite` so the chokidar handler that fires
later can still consume the same entry.

The lookup is keyed by absolute path, not filename, so it works for any
configured gen-file naming scheme — no hardcoded suffix list.
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