Skip to content

round 4: 54 audited fixes across router, cache, RSC, build/vite, browser, theme, server, and testing primitives - #611

Merged
ivogt merged 20 commits into
mainfrom
advisor/r4-improvements
Jun 18, 2026
Merged

ivogt merged 20 commits into
mainfrom
advisor/r4-improvements

Conversation

@ivogt

@ivogt ivogt commented Jun 18, 2026 •

Copy link
Copy Markdown
Collaborator

A fourth improve-skill sweep. A 28-lane adversarial audit (80 raw → 57 confirmed, 0 already-fixed, 23 refuted) of the modules rounds 1–3 had touched only lightly, implemented as 54 fixes in one PR, one commit per batch, each tested red-before-green. The whole branch then went through a multi-agent /simplify + /code-review (62 raw → 33 confirmed → 24 fixes folded in, last commit) — which caught real bugs in the new code (a cache-key collision, an incomplete transform fix, a missing sibling guard) before it landed.

All gates green: pnpm run typecheck (13/13 packages), pnpm run test:unit:all (266 router test files + every consumer dogfood suite + RSC/Flight), pnpm run lint, pnpm run format, pnpm check:e2e-bucketing. New dev+prod e2e for 5 HTTP-observable behaviors.

The two HIGH-severity ones (consumer-visible)

"use cache" never hit on a binary/FormData arg. The cache key for a "use cache" function called with a FormData/Blob arg was derived from the response body of an encodeReply, which embeds a random multipart boundary — so every call computed a different key and always missed.

// A cached function keyed on a binary arg — before: every call recomputed, never hit.
const resizeImage = cache(async (file: Blob) => expensiveResize(file))
// after: identical args -> identical key -> a real cache hit.

DSL loader errors never reached onError. A throwing DSL loader's error never fired the consumer's createRouter({ onError }) callback or loader.error telemetry (the error context was dropped at every wrap site), unlike fetchable loaders.

createRouter({ onError(err, phase) { reportToSentry(err, phase) } })
route("/x", () => { const data = use(ThrowingLoader) })  // before: onError never called; after: fires with phase:"loader"

What's in it (54 fixes, by area)

Area Representative fixes
Router matching/core (A) trie no longer binds a required param to "" for /a//b (404 like the regex); extractStaticPrefix keeps the fast-skip when a static segment has a literal colon; matchMiddleware runs the scope regex once; route(name) validates the name like path()/include(); prefetchCacheTTL: NaN no longer emits max-age=NaN
Segment-resolution / context (B) loader onError context (HIGH); dedup-before-tags(); bounded LoaderCacheEntry; intercept segment cached once; typed ctx.search for action/dispatch loaders; Max-Age=05 not misread as deletion
Cache (C) boundary-independent "use cache" key (HIGH); a throwing cache({ key }) degrades to a miss instead of crashing the render; Cache-Control: no-cache not served as a fresh HIT (RFC 7234); profile ttl/swr validated; normalizeTag trims; chunked base64
RSC (D) malformed action body → 400 not opaque 500; warmupEnabled in the initial payload; falsy error-boundary node used not discarded; thrown redirect from a no-middleware fetchable loader → 302; prod loader error no longer leaks err.name; Vary: Accept emitted once; SSR setup only when HTML is rendered
Build / Vite (E) regex-literal-aware router-file scan; include() of a default+named import resolves; $$id idempotency keyed on the real marker; aliased loader exports share one id; comment-only createHandle args; Node-preset optimizeDeps.exclude; prerender fail-fast; AST-based use client sniff
Browser (F) useLinkStatus releases the pinned spinner on idle; partial-update full-update history-key staleness guard; ScrollRestoration coverage; a stalled prefetch can't strand the inflight key; Link adaptive prefetch reads touch capability per render; LocationState primitive-state guard
Theme / handles (G) no bogus class="system" when enableSystem:false; client setTheme validates like the server; Promise meta descriptors documented append-only; breadcrumbs dedup preserves order; idempotent FOUC script (no double-run / matchMedia leak)
Server / core (H) a loader module-load failure surfaces as 500 + onError not a silent 404; waitUntil sync-throw is fire-and-forget; strict-decimal search coercion (?page= no longer becomes 0); the RSC client entry exports the full hook set; removed a deprecated dead export
Testing primitives (I) seeded ctx.use(Loader) returns a Promise like production; dispatch exercises the response-route cache path; runInRequestContext drains onResponse; renderToFlightString accepts theme/cacheStore/cacheProfiles

Behavior / semantics call-outs

Case Before After
malformed JS action body opaque host 500 controlled 400 / error boundary (PE parity)
cache({ key }) throws page render crashes degrades to a cache miss
Cache-Control: no-cache doc served as fresh HIT not stored as fresh (qualified no-cache="…" still stores)
no-middleware fetchable loader throw redirect() 500 302
prod fetchable-loader error body leaks err.name "Error" (dev unchanged)
?page= (empty) parsed as number 0 omitted → schema default applies
Vary on negotiated routes Accept, Accept Accept once

Test plan

  • pnpm run typecheck (13/13) · pnpm run test:unit:all (266 router files + consumer suites) · pnpm run lint · pnpm run format · pnpm check:e2e-bucketing — all green.
  • Every fix ships a unit/primitive test, red-before-green verified; testing-primitive (dogfood) coverage where the feature is consumer-touchable.
  • dev+prod e2e for 5 HTTP-observable behaviors: empty-segment 404 (A1), no-cache not-fresh (C3), loader thrown-redirect 302 (D4), prod error no name-leak (D5), single Vary: Accept (D6); each run 2–3× locally. router.named-routes.gen.ts regenerated for the one new route.
  • Multi-agent /simplify + /code-review across the full diff; 24 verified findings fixed in the final commit (incl. a cache-key collision, an incomplete countArgs, a missing write() guard, a setTheme "system" gap).

Notes / deferred

  • A cross-package generic-variance error (RequestContext<TEnv> vs a consumer's concrete Env in the new dispatch cache path) was caught by the full turbo typecheck and fixed — the per-package tsc missed it.
  • e2e deferred (both unit-covered, with reasons): D1 (a malformed action body isn't reliably 400 over HTTP because React's decodeReply is lazy) and G1 (theme config is router-global; an e2e would break the existing theme suite or need a second app).

Post-review additions (review-driven, after the original body above)

Beyond the 54 audited fixes, this PR folded in everything from the multi-round review:

  • Cache body-disturb race — the response-cache-serve extraction deferred the cache clone into waitUntil, racing mergeResponse (flaky 500 on cached response routes). Now clones synchronously before returning.
  • Prefetch P2 (body-stall) — the stall timeout now stays armed past headers and evicts a published entry whose body never settles (was: future prefetches deduped against a stuck entry forever).
  • Prefetch P3 (identity-guarded eviction) — removePrefetch deletes only when cache.get(key)?.entry === entry, so a stale stall-timer can't drop a fresh entry republished under the same key.
  • Build scan keyword-context — the regex-vs-division heuristic now uses keyword context (return /re/ etc.), so a regex body's inner quotes can't hide a later createRouter() and drop a router file.
  • CI infra to land it green — dev e2e sharded 3→5 (the heaviest shards were OOM-killing the hosted runner) + the rangojs-router-e2e job timeout 20→30 min. An attempted install-deps per-step timeout/retry was reverted (it killed a slow-but-working apt and the orphaned sudo apt-get held the dpkg lock, failing the retry; the 20-min job timeout already bounds a true hang).

Each fix ships a red-before-green regression test. Final CI: 27/27 green.

ivogt added 11 commits June 18, 2026 11:11
A1 [bug] trie-matching: reject empty required-param segment (/a//b no longer
   matches /a/:s/b binding s=""); matches the regex ([^/]+) contract + parity row.
A2 [perf] pattern-matching: extractStaticPrefix computes the cut per-segment so a
   literal colon in a static segment (/a:b/c/:id) no longer drops the fast-skip.
A3 [perf] middleware: matchMiddleware runs the scope regex once (single match())
   instead of test() + extractParams' second match() per matching entry.
A4 [api] dsl-helpers: route(name,...) now calls validateUserRouteName like
   path()/include(), so a reserved name ($prefix_/$path_) errors instead of
   silently vanishing from reverse()/types.
A5 [bug] router: prefetchCacheTTL NaN/Infinity guarded with Number.isFinite
   (-> 300s default) instead of emitting Cache-Control: max-age=NaN. Extracted to
   prefetch-cache-ttl.ts for unit coverage.
A6 [bug] document the regex-fallback wildcard trailing-slash splat divergence vs
   the canonical trie with a parity row so it cannot silently widen (trie is
   canonical; matcher behavior unchanged).
B1 [error-handling] DSL loader errors now reach the consumer onError callback and
   loader.error telemetry: buildLoaderErrorContext threads errorContext into all
   wrapLoaderPromise call sites (fresh/revalidation/intercept), matching fetchable
   loaders.
B2 [perf] resolveLoaderData short-circuits the per-loaderId dedup BEFORE running
   the user tags()/TTL/SWR/recordRequestTags work, so a deduped second resolution
   no longer re-runs tags().
B3 [memory] segment-loader-promise caps the per-key LoaderCacheEntry array
   (evict-oldest) so it stops growing unbounded across navigations.
B4 [bug] intercept segments are cached once on a fresh intercept navigation
   (dedup state.interceptSegments against allSegments ids), not twice.
B5 [bug] createUseFunction loaders (action/dispatch path) now receive the parsed
   typed ctx.search instead of always {}.
B6 [bug] Max-Age cookie parsing treats only a numeric <= 0 as deletion, so
   Max-Age=05 (zero-prefixed non-zero) is no longer misread as a deletion.
…xes)

C1 [bug] "use cache" key for a FormData arg is now derived from sorted entries
   (boundary-independent), not the random multipart boundary, so identical args
   produce the same key and actually hit the cache (was: always missed).
C2 [error-handling] a consumer cache({ key }) fn that throws now degrades to a
   cache miss (render continues) instead of crashing the page render.
C3 [bug] the document cache no longer stores/serves an unqualified
   Cache-Control: no-cache response as a fresh HIT (RFC 7234); field-qualified
   no-cache="set-cookie" is still storable.
C4 [error-handling] resolveCacheProfiles now rejects non-finite/negative ttl/swr
   at createRouter time instead of creating a permanently-fresh, never-evicting
   entry.
C5 [bug] normalizeTag trims surrounding whitespace so " products " and
   "products" are one cache tag, not two.
C6 [perf] bufferToBase64 builds the latin1 string in 8KB chunks instead of one
   byte at a time (byte-identical output), cheaper for large document payloads.
D1 [error-handling] a malformed JS-path action body now yields a controlled 400
   (error boundary or plain 400) instead of an opaque host 500, converging with
   the no-JS PE path; the decode error is sanitized (no leak).
D2 [bug] the initial full-render payload carries warmupEnabled, so warmup:false
   is respected from first load (was ignored until a navigation).
D3 [bug] a loader error boundary that renders to a falsy node (0/""/false) is
   used instead of being discarded and the original error rethrown (== null
   sentinel, not falsy).
D4 [bug] a thrown Response (redirect/notFound) from a no-middleware fetchable
   loader now produces the real 302/404 instead of a generic 500.
D5 [security] the production fetchable-loader error payload no longer leaks the
   consumer error class name (dev unchanged).
D6 [bug] negotiated response routes and RSC renders emit Vary with Accept once
   (appendVaryAccept dedup helper) instead of a duplicated token.
D7 [perf] early SSR setup is only kicked off when the request will render HTML,
   not for no-Accept requests that return RSC (avoids wasted work + an orphan
   rejection window); mayNeedSSR / document-cache keying untouched.
…ts (8 fixes)

E1 [bug] source-scan classifier is now regex-literal-aware (a /.../ literal
   containing a quote or // no longer desyncs the code/string/comment state and
   drops a real router file from discovery).
E2 [bug] include-resolution resolves an include() target imported via a combined
   default+named import (import Foo, { bar }) instead of false-hard-erroring.
E3 [bug] createRouter $$id idempotency guard checks for the actual injected
   $$routeNames marker, not a substring, so args containing the literal "$$id"
   no longer drop named-route wiring.
E4 [bug] aliased loader exports get the same single id (exportNames[0]) on the
   client stub as the server registers, fixing client/server $$id divergence.
E5 [bug] transformHandles branches on binding.argCount (AST-derived) instead of a
   string-trim heuristic, so a comment-prefixed createHandle arg transforms
   correctly.
E6 [bug] the Node preset's rsc environment gets the same optimizeDeps.exclude the
   cloudflare preset and ssr env set.
E7 [perf] prerender runWithConcurrency stops scheduling new work on the first
   error instead of letting remaining workers finish before the build aborts.
E8 [bug] the client-component-hmr 'use client' sniff is AST-based and tolerates a
   leading comment before the directive (hasUseClientDirective).
…nk (7 fixes)

F1 [bug] useLinkStatus releases the pinned useOptimistic value when navigation
   returns to idle, so a Link spinner no longer stays stuck after a
   view-transition navigation.
F2 [concurrency] the partial-update full-update fallback re-checks the history
   key after the stream completes (like the partial branch), so a stale
   stale-revalidation full update can't clobber a page the user navigated to.
F3 [test-gap] adds unit coverage for ScrollRestoration (LRU eviction, MRU
   re-order, sessionStorage quota evict-and-retry ladder, custom getKey).
F4 [memory] a direct (hover) prefetch with no caller AbortSignal now installs an
   internal abort+timeout so a never-settling fetch can't strand the inflight key
   and permanently block re-prefetch of that URL.
F5 [bug] Link evaluates touch capability per render (resolveAdaptiveStrategy)
   instead of once at module load, so prefetch="adaptive" reflects current input
   capability (hybrid device, SSR->hydrate).
F6 [type-safety] LocationState.delete() guards that history.state is an object
   before `key in current`, so a non-null primitive state is a no-op instead of
   throwing.
F7 [test-gap] adds the missing useSearchParams SSR-empty-seed + mount-catch-up
   unit test (mirrors usePathname); no source divergence found.
…s (5 fixes)

G1 [bug] resolveThemeConfig coerces a "system" defaultTheme to a concrete theme
   when enableSystem is false, so <html> never gets a bogus class="system".
G2 [bug] client setTheme validates against the configured theme set (warn+ignore
   an unknown theme) like server ctx.setTheme, preventing SSR drift.
G3 [bug] collectMeta handles Promise meta descriptors explicitly and dev-warns
   when one is pushed under an active title template (async meta is append-only
   and documented; collect is synchronous so it cannot participate in dedup).
G4 [bug] breadcrumbs href-dedup keeps each href's first position but last value,
   so re-pushing a parent crumb refreshes its label in place instead of
   reordering it to the end (parent->child order preserved).
G5 [api] the FOUC theme script is idempotent (a per-storageKey window guard), so
   rendering both MetaTags (auto-inject) and ThemeScript no longer double-runs the
   script or leaks a matchMedia listener; JSDoc steers consumers to pick one.
…sc (5 fixes)

H1 [error-handling] a loader module-load (dynamic import) failure now surfaces as
   a 500 + onError instead of being swallowed into a misleading 404; an
   unregistered loader still 404s.
H2 [error-handling] waitUntil defers the callback (Promise.resolve().then) so a
   synchronously-throwing callback is fire-and-forget (logged) instead of
   propagating into the request flow, on both the EC and no-EC seats.
H3 [bug] parseSearchParams number coercion is strict-decimal: ""/whitespace/
   Infinity/0x10 are treated as missing (omitted, so schema defaults apply) instead
   of becoming 0/Infinity/16; valid decimals and 1e3 still parse.
H4 [api] the RSC client entry (client.rsc.tsx) re-exports the full "use client"
   hook set the default entry exports (useParams/useMount/useSegments/
   useSearchParams/...), matching the surface; navigation/action stay omitted by
   design.
H5 [dead-code] removed the deprecated dead extractUrlsVariableFromRouter (function
   + re-export), per the pre-stable no-deprecated-API rule; its self-test now
   exercises the canonical extractUrlsFromRouter.
I1 [api] a seeded ctx.use(Loader) now returns a Promise (Promise.resolve) like
   production and the real-fn delegate path, instead of the raw value.
I2 [test-gap] dispatch wires the production response-route cache path (getResponse
   / SWR / putResponse / resolveCacheTags), so cached path.json/path.text routes
   are reachable + cache via the primitive; cache-scope is lazily imported so the
   testing barrel doesn't eagerly pull plugin-rsc.
I3 [test-gap] runInRequestContext (and runLoader) drain ctx.onResponse() callbacks
   on both the normal and throw-redirect branches, matching production.
I4 [test-gap] renderToFlightString accepts theme/cacheStore/cacheProfiles and
   threads them into the RequestContext, so a server component reading ctx.theme
   or invoking use-cache works under the primitive instead of crashing.
Adds dev+prod e2e (guard-safe (production) siblings, each run locally) for the
HTTP/browser-observable round-4 fixes:
- A1 empty path segment /item//detail does not bind :itemId="" (cloudflare-basic
  trie-routing).
- C3 an unqualified Cache-Control: no-cache response is never served as a fresh
  document-cache HIT (cloudflare-basic blog-cache + a fixture page).
- D4 a fetchable loader throwing a redirect Response yields the real 302, not 500
  (router e2e, test-app ThrownRedirectLoader).
- D5 in production the fetchable-loader error body does not leak err.name; dev
  does (router e2e, test-app NamedErrorLoader).
- D6 a negotiated response/RSC route emits Vary: Accept exactly once (router e2e
  content-negotiation).

D1 (malformed-action 400) and G1 (theme system+enableSystem:false) are covered by
unit tests but deferred at the e2e layer: D1's 400 path isn't reliably reachable
over HTTP (React decodeReply is lazy, so a malformed body surfaces on the action
-runtime 500 path); G1's theme config is router-global and an e2e would break the
existing theme suite or require a second app.

cloudflare-basic router.named-routes.gen.ts regenerated for the one new route.
A multi-agent review (62 raw -> 33 confirmed) of the 54-fix branch surfaced real
issues in the new code; all verified-to-fix items applied:

Bugs (red-before-green):
- cache-runtime: the C1 Blob/File cache-key token percent-encodes type/name so a
  user-controlled colon in a MIME type or filename can't collide two files onto
  one key.
- expose-id-utils countArgs: a comment-only createHandle/createLoader arg list is
  counted as 0 args at the producer (fixes E5 on the regex-fallback path, not just
  the AST path).
- location-state write(): coerces a primitive/null history.state to a fresh dict
  like delete() (the F6 guard was missing from the sibling writer).
- setTheme rejects "system" when enableSystem is false on BOTH client and server
  (shared isValidTheme/warnInvalidTheme helper), closing the gap the G1 coercion
  left in the setter.
- intercept cache-hit loader revalidation reports onError isPartial:true (matches
  the revalidation path).
- loader-fetch: notFound() (a DataNotFoundError) maps to 404, not a generic 500.
- meta async-descriptor dev-warning no longer false-positives on a normal async
  descriptor (only the title-template collision case).
- prefetch stalled-fetch timeout is layered even when the caller passes a signal
  (queue-driven prefetches), so a stalled server can't strand the inflight key.

Simplify / reuse (no behavior change):
- shared leading-directive sniff (rango.ts + version-plugin), shared isThenable
  predicate (meta/MetaTags/breadcrumbs), shared loader-error response builder,
  shared response-cache-serve leaf used by both production and the dispatch
  primitive (plugin-rsc-free via injected deps), drainOnResponseCallbacks reused
  from rsc/helpers instead of inlined, de-duplicated TTL-guard comments.

Coverage / docs:
- binary-arg use-cache HIT test, warmupEnabled-via-router.fetch primitive test,
  a real Max-Age=00 deletion assertion, corrected scroll-restoration JSDoc.
@pkg-pr-new

pkg-pr-new Bot commented Jun 18, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

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

commit: dd05920

ivogt added 9 commits June 18, 2026 15:20
P1 [security] response-cache-serve: a configured cache({ key }) or store
   keyGenerator that THROWS now degrades to a cache miss (runs uncached, no
   read/write) instead of falling back to the broad response:type:host+path key
   and caching under it — which could serve one user's personalized output to
   another when the key encodes tenant/user/auth state. The no-key default path
   is unchanged.
P2a cache-runtime: replyToCacheKey type-tags string FormData entries ("s:") like
   it already tags Blob/File entries ("b:..."), so a user string equal to a blob
   token can no longer collide two distinct arg lists onto one "use cache" key.
P2b cache-scope + handle-snapshot: segment-codec (which pulls @vitejs/plugin-rsc)
   is now imported lazily at its async use sites, so cache-scope is plugin-rsc-free
   at module load. The dispatch() testing primitive's cached-response-route path is
   now usable in a plain Node/Vitest consumer test without mocking plugin-rsc
   (covered by a new no-mock dispatch test); segment-caching behavior is unchanged.
P3 theme: the inline FOUC boot script and the cross-tab storage event handler now
   apply the same validity rule as setTheme — a stored/received "system" with
   enableSystem:false falls back to defaultTheme instead of leaving a bogus
   class="system"/colorScheme="system" on <html>.
…he write

The response-cache-serve extraction (review dedup #10/#13) deferred the cache
clone into the waitUntil callback (putFresh(response) cloned inside the async
body). On a cache MISS the same response is also returned to the middleware
chain, where mergeResponse rebuilds it as `new Response(response.body, ...)`. The
background clone() and that foreground body read could interleave and throw
"Response body object should not be disturbed or locked" — a flaky 500 on cached
response routes (cache-isolation e2e). In dev this surfaced as a Vite dev-server
"Internal server error" that crashed/restarted the server under Playwright
retries, cascading into the dev e2e shards being killed.

Fix: clone synchronously at the miss site (before returning), so the returned
body is teed independently of the cache write. The SWR path is unaffected (its
fresh response is created inside the background callback and never returned).

Verified: cache-isolation onResponse-freshness 3x reliably green in dev (was
flaky 500); 67 cache-isolation + cache dev e2e green at retries=0; cache/dispatch
unit suites 706 green; typecheck 13/13.
The rangojs-router dev e2e runs in dev mode (a live Vite dev server + esbuild +
Chromium held resident across the shard) over ~300 tests. At 3-way sharding the
two heaviest shards exhausted the GitHub-hosted runner's memory and the runner
was terminated mid-run ("received a shutdown signal" at 5-8 min, well under the
20-minute job cap) — surfacing as flaky dev-shard failures while the same shard
passes locally (more RAM). Splitting dev across 5 smaller shards keeps each
within the runner's memory budget and shortens each shard. Production sharding
(2) and the HMR jobs are unchanged.
Complements the 3->5 dev sharding. The smaller shards removed the runner-OOM, but
the shard that draws the timing-sensitive dev tests (handle-breadcrumbs late-push
+ cache param-change soft-navs) spends real wall-clock on per-test timeouts and
their Playwright retries under CI load, which brushed the old 20-minute cap. 30
minutes gives that headroom; production/HMR jobs are unchanged.
`playwright install-deps chromium` shells out to apt-get to install browser
system libraries, which can hang on a slow/transient package mirror and stall an
e2e job. Wrap all 8 install-deps steps so each attempt is bounded to 60s
(coreutils `timeout`, available on ubuntu-latest) and retried once on failure or
timeout (`timeout 60 ... || timeout 60 ...`), so a flaky mirror costs at most ~2
minutes and self-heals instead of hanging to the job cap.
…evious char

The E1 regex-literal skip decided "regex vs division" from the previous
significant CHAR only. After a keyword like `return` (which ends in a word char)
it read `/` as division, so a regex body's inner quotes opened a phantom string
and swallowed a later real createRouter() -> the router file was dropped from
discovery / HMR / typegen. Example: `return /it's a "x"/g; createRouter()`.

Now, when the previous char ends an identifier, the scanner walks back over any
whitespace + the identifier and treats `/` as a regex iff that identifier is a
regex-preceding keyword (return/typeof/instanceof/in/of/new/delete/void/do/else/
yield/await/case/throw). A value-named identifier (`returnValue / b`) stays
division. Red-before-green: the `return`/`typeof`/`case`/`throw` cases fail
without the keyword lookback and pass with it; division-after-a-value is
unaffected.
The 60s cap was too aggressive: `playwright install-deps chromium` runs apt-get,
which legitimately takes ~1-2 min on some runners, so a 60s timeout killed it
mid-install (both the attempt and the retry) and failed 3 e2e shards on a clean
run. 180s per attempt still catches a genuine hang (a stuck mirror would far
exceed it) while allowing a normal/slow-but-real install; the single retry is
kept.
…y by identity

Two reviewer findings on the hover/queued prefetch path:

P2 (body stall) — the stall timeout only covered "no response ever arrives". Once
headers arrived the timer was cleared in `.finally()`, so a body that stalled
after headers left a published entry whose payload/streamComplete never settle:
future prefetches dedupe against it and navigation awaits it forever. The timer
now stays armed past headers; it is cleared when the stream completes (or in
`.finally()` only for paths that publish no streaming entry), and on fire it
aborts the fetch/stream and evicts the published entry.

P3 (identity-guarded eviction) — evicting by generation alone could delete a
fresh entry republished under the same key in the same generation (stalled entry
consumed, then a new prefetch publishes, then the stale timer fires).
removePrefetch now takes the published entry and deletes only when
cache.get(key)?.entry === entry, so it can only ever drop its own stalled entry.

Regression tests (red-before-green): a response whose body never closes is
evicted after the timeout and re-prefetchable; and a stalled entry's timer firing
does NOT drop a fresh entry republished under the same key after the stalled one
was consumed.
The `timeout 180 ... || timeout 180 ...` wrapper was net-harmful and failed ~4
e2e jobs per run:
- `playwright install-deps` (apt-get downloading + installing ~21MB of browser
  system libs) legitimately exceeds 180s on some runners, so `timeout` killed it
  mid-install.
- `timeout` only kills the pnpm wrapper; the sudo'd apt-get grandchild keeps
  running and holds /var/lib/dpkg/lock-frontend, so the retry fails with
  "E: Could not get lock ... held by process N (apt-get)".

The job-level 20-minute timeout already bounds a genuinely hung install, so the
per-step timeout/retry added failure modes (killed-but-working apt, lock
contention on retry) without real benefit. Restore the plain install-deps step.
@ivogt
ivogt merged commit 1c3eb56 into main Jun 18, 2026
27 checks passed
@ivogt
ivogt deleted the advisor/r4-improvements branch June 18, 2026 16:36
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