fix(app-router): recover mismatched prefetch rewrites

[james-elicx]

Summary

• classify automatic prefetches conservatively when middleware may rewrite navigation, including default match-all middleware and encoded/locale/query/cookie matcher cases
• render page-local cache-components Suspense shells through the existing two-pass PPR lifecycle while preserving static siblings and the actually suspended fallback
• validate Flight shell completeness across every selected page/children root, preserve terminal errors for React boundaries, and avoid waiting forever on missing roots

Failure mapping

• GitHub Actions run: https://github.com/cloudflare/vinext/actions/runs/31290819291
• Next.js E2E: test/e2e/app-dir/concurrent-navigations/mismatching-prefetch.test.ts
• This is current-main proof; stale/conflicting PR fix(app-router): recover mismatched prefetch rewrites #2453 was not reused.

Validation

• CI=true REPO="$(pwd)" NEXTJS_DIR="/Users/jamesanderson/Developer/vinext/.nextjs-ref" ./scripts/run-targeted-nextjs-e2e.sh test/e2e/app-dir/concurrent-navigations/mismatching-prefetch.test.ts — 1/1 passed
• vp test run tests/app-optimistic-routing.test.ts tests/app-page-route-wiring.test.ts tests/link-navigation.test.ts tests/middleware-runtime.test.ts tests/app-browser-entry.test.ts — 493/493 passed
• strengthened production fixture: tests/e2e/app-router/nextjs-compat/mismatching-prefetch.browser.spec.ts — 1/1 passed
• vp test run tests/build-report.test.ts — 98/98 passed
• vp check across all 21 touched files — clean
• vp run vinext#build — passed

Independent cumulative review completed with no findings on exact head 161749f0f.

[james-elicx]

/bigbonk review for issues

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/@vinext/cloudflare@2852

npm i https://pkg.pr.new/create-vinext-app@2852

npm i https://pkg.pr.new/@vinext/types@2852

npm i https://pkg.pr.new/vinext@2852

commit: 80b4d6f

[github-actions]

Example	Preview	Production	Original
app-router-cloudflare	preview	production
pages-router-cloudflare	preview	production
app-router-playground	preview	production	original
realworld-api-rest	preview	production
nextra-docs-template	preview	production
benchmarks	preview	production
hackernews	preview	production	original
workers-cache	preview	production
web	preview	production

[james-elicx]

/bigbonk review for issues

Re-requesting on exact head bca01b98b after fixing CI knip failure (the matcher extractor is now internal). Local vp run knip and targeted checks pass.

[james-elicx]

Exact-head validation update for bca01b98b:

• CI=true REPO="$(pwd)" NEXTJS_DIR="/Users/jamesanderson/Developer/vinext/.nextjs-ref" ./scripts/run-targeted-nextjs-e2e.sh test/e2e/app-dir/concurrent-navigations/mismatching-prefetch.test.ts
• Result: 1/1 passed (recovers when a navigation rewrites to a different route than the one that was prefetched).
• vp run knip and focused format/lint/types also pass after the CI fix.

[ask-bonk]

Review: fix(app-router): recover mismatched prefetch rewrites

Reviewed at head 161749f0f. Ran targeted suites — all green:
link-navigation (82), app-optimistic-routing (16), app-page-route-wiring (79), middleware-runtime/app-browser-entry/build-report (414). vp check clean across all 21 touched files.

The core approach is sound: conservatively classify automatic prefetches as loading-shell prefetches whenever middleware may rewrite the navigation, and validate Flight shell completeness before trusting a page-local Suspense shell. The connection() shim change correctly mirrors the existing headers()/cookies() PPR-fallback-shell pattern, and the matcherRequest plumbing correctly evaluates matcher has/missing against the pre-Flight-strip request (matching Next.js, which strips after matching). No dev/prod parity gap — executeMiddleware is only reached via App Router app-middleware.ts, and Pages Router doesn't strip Flight headers before matching.

No blocking issues found. A few non-blocking observations below.

Non-blocking

1. Duplicated client middleware-matcher logic (maintainability).
shims/link.tsx introduces a second implementation of clientMiddlewareSourceMatches / isClientMiddlewareMatcherObject / stripLocaleForMiddlewareMatcher, duplicating the ones already in shims/internal/pages-data-target.ts. The two now differ in behavior (e.g. the new App Router stripLocaleForMiddlewareMatcher matches locales case-insensitively while the Pages Router one is case-sensitive; the new source matcher returns "unknown" for regex-y sources whereas the Pages version returns a plain boolean). These are deliberate, but two copies of matcher logic will drift silently. Consider extracting a shared internal helper so Pages and App Router stay in sync.

2. Full Flight decode on the common loading-shell path (perf).
app-page-render.ts:759 now calls invalidateIncompletePageSuspenseShell for every PPR fallback-shell render. For the common LoadingBoundary (non-PageSuspense) shell, hasCompletedPageSuspenseShell fully TextDecoder-decodes and JSON.parsees every Flight model row before returning false at the root-marker check (app-page-loading-shell.ts:141), then invalidateIncompletePageSuspenseShell does a full bytes.slice() copy + byte scan for a marker that isn't there. Per AGENTS.md ("keep the common request path lightweight"), consider a cheap early-out — e.g. a quick byte search for the "PageSuspense" marker before the full decode — so only genuine PageSuspense shells pay the parse cost. This path isn't per-request-hot (prefetch/prerender only), so it's minor.

3. IPv6 host matcher edge case (very minor).
clientMatcherConditionMatches host case uses target.host.split(":", 1)[0], which yields [ for IPv6 literals like [::1]:3000. target.hostname would be correct. This only affects has/missing host conditions with IPv6 authorities on a conservative may-match path, so impact is negligible.

Overall this is a well-scoped, well-tested fix with strong conservative defaults (un-analyzable middleware falls back to ["/:path*"] match-all, unknown header/cookie conditions treated as may-match, incomplete shells invalidated). Ship-able as-is; items above are cleanup follow-ups.

[ask-bonk]

Review posted to PR #2852.

Summary

I reviewed PR #2852 (fix(app-router): recover mismatched prefetch rewrites) at head 161749f0f. Ran the targeted test suites (all green: 82 + 16 + 79 + 414 tests) and vp check on all 21 touched files (clean).

Verdict: no blocking issues. The core approach is sound and well-tested:

• Conservative prefetch classification when middleware may rewrite navigation (un-analyzable middleware → default ["/:path*"] match-all; unknown header/HttpOnly-cookie conditions → treated as may-match).
• The connection() shim change correctly mirrors the existing headers()/cookies() PPR-fallback-shell pattern.
• matcherRequest plumbing correctly evaluates matcher has/missing against the pre-Flight-strip request (matches Next.js).
• No dev/prod parity gap — executeMiddleware is App-Router-only, and Pages Router doesn't strip Flight headers before matching.
• Incomplete page-Suspense shells are invalidated (PageSuspense → NotSuspended) so the client won't build a broken optimistic tree.

Three non-blocking follow-ups flagged:

1. Duplicated matcher logic — link.tsx reimplements matcher functions already in pages-data-target.ts, and they now differ (case-insensitive vs case-sensitive locale matching). Risk of silent drift; suggest extracting a shared helper.
2. Perf on common path — invalidateIncompletePageSuspenseShell fully decodes + JSON-parses every Flight row even for common LoadingBoundary shells. Suggest a cheap byte-search early-out before the full parse.
3. IPv6 host matcher — host.split(":")[0] mishandles IPv6 literals; hostname would be correct. Negligible impact.

github run

[github-actions]

Performance benchmarks

Compared 80b4d6f against base 9962354 using alternating same-runner rounds. Next.js was unchanged and skipped.

0 improved · 0 regressed · 6 within ±1.5%

Scenario	Framework	Baseline	Current	Change
Client bundle size (gzip)	vinext	134.7 KB	136.0 KB	⚫ +1.0%
Client entry size (gzip)	vinext	122.2 KB	122.8 KB	⚫ +0.5%
Dev server cold start	vinext	3.03 s	3.04 s	⚫ +0.6%
Production build time	vinext	3.35 s	3.36 s	⚫ +0.4%
RSC entry closure size (gzip)	vinext	114.2 KB	115.1 KB	⚫ +0.7%
Server bundle size (gzip)	vinext	191.8 KB	193.8 KB	⚫ +1.0%

View detailed results and traces

_{🟢 improvement · 🔴 regression · ⚫ change below 1.5% · paired base/head}

[james-elicx]

/bigbonk review for issues

Exact-head re-review requested for 0ff276073 after fixing the CI raw AppElements key violation and preserving dynamic metadata in page-local fallback probes. Local validation: Next.js targeted 1/1, production fixture 1/1, metadata reuse 2/2, focused units 663/663, Knip/check/build clean.

[ask-bonk]

Re-review at exact head 0ff276073

Reviewed the two delta commits since my prior review (161749f0f → bca01b98b → 0ff276073):

• fix(build): keep middleware matcher extractor internal (knip fix)
• fix(app-router): preserve metadata in page fallback probes (metadata reuse + raw AppElements key fix)

Ran the relevant suites at head — all green:
ppr-fallback-shell (17), app-optimistic-routing (16), app-page-route-wiring (79), build-report (98), app-page-head (28), streaming-metadata (3), link-navigation (82), middleware-runtime + app-browser-entry (398 combined). vp check clean on all 6 delta-touched files, vp run knip clean.

Verdict: no blocking issues. Both fixes are correct.

1. Raw AppElements key violation fix (app-page-loading-shell.ts) — correct

Replacing the raw key.startsWith("page:") || key.startsWith("slot:children:") string checks with AppElementsWire.parseElementKey(key) and matching parsed?.kind === "page" || (parsed?.kind === "slot" && parsed.name === "children") is behavior-preserving:

• slot:children:/tree parses to { kind: "slot", name: "children", treePath: "/tree" } — matches the old prefix check exactly (the old slot:children: prefix already required a literal : after children, so slot:childrenX:... was excluded by both old and new logic).
• All keys on this path originate from AppElementsWire codecs, so every real key parses; the parser can only reject malformed keys the raw check would have (incorrectly) matched. This is the intended canonical-accessor cleanup.

2. Metadata preservation in page-local fallback probes — correct and well-scoped

The new runWithPprFallbackShellMetadataResolution ALS flag makes getPprFallbackShellState() return null only while resolving metadata/viewport (resolveModuleMetadata/resolveModuleViewport in app-page-head.ts) and only when the active shell has preserveDynamicMetadata === true. This lets generateMetadata resolve dynamic APIs (headers()/cookies()/connection()) normally instead of suspending against the fallback shell.

Verified the scope is safe:

• preserveDynamicMetadata: true is set in exactly one place (app-page-dispatch.ts:639), and always with fallbackParamNames: []. So even though makeThenableParams reads getPprFallbackShellState() inside the metadata scope (metadata.tsx:597) and would see null, there are no fallback param names to suspend on anyway — no risk of leaking raw [slug] placeholders. Build fallback shells (non-empty fallbackParamNames, no preserveDynamicMetadata) are unaffected and still defer, as the second new test asserts.
• The flag is strictly ALS-scoped to the metadata/viewport resolver. The two pprRuntime.getState() reads in app-page-dispatch.ts (1070, 1117) and the page-body dynamic-API calls run outside this scope, so they still observe the real (non-null) shell state — the loading shell continues to defer dynamic APIs in the page body correctly.
• ALS propagation across the unawaited promise returned by resolveModuleViewport (line 45) is correct; async continuations inherit the store.

Both new tests in ppr-fallback-shell.test.ts pin exactly this contract (live metadata for the preserveDynamicMetadata shell, continued deferral otherwise), and state.hasDynamicBoundary correctly stays false for the preserve case.

Carry-over (unchanged, still non-blocking)

The three follow-ups from my prior review (duplicated client middleware-matcher logic in link.tsx, full Flight decode on the common loading-shell path, and the IPv6 host.split(":") edge case) were not touched by these delta commits and remain as cleanup follow-ups. None block this PR.

Ship-able as-is.

github run

[ask-bonk]

Review posted to PR #2852.

Summary

Re-reviewed at exact head 0ff276073, focusing on the two delta commits since my prior review. Ran all relevant suites (green) and vp check / vp run knip (clean).

Verdict: no blocking issues. Both fixes are correct.

1. Raw AppElements key violation fix (app-page-loading-shell.ts) — Replacing raw startsWith("page:") / startsWith("slot:children:") string checks with AppElementsWire.parseElementKey is behavior-preserving. Verified slot:children:/tree parses to {kind:"slot", name:"children", treePath:"/tree"}, matching the old prefix logic exactly, and all keys originate from wire codecs so real keys always parse.

2. Metadata preservation in page-local fallback probes — The new runWithPprFallbackShellMetadataResolution ALS flag makes getPprFallbackShellState() return null only during metadata/viewport resolution and only when the shell has preserveDynamicMetadata === true. I verified:

• preserveDynamicMetadata: true is set in exactly one place, always with empty fallbackParamNames, so there's no risk of leaking raw [slug] placeholders via makeThenableParams.
• The flag is strictly scoped to the metadata resolver; the two pprRuntime.getState() reads and page-body dynamic-API calls run outside it and still observe real shell state — the shell continues deferring dynamic APIs in the page body.
• ALS propagation across the unawaited viewport promise is correct.

Both new tests pin the exact contract. The three prior non-blocking follow-ups (duplicated matcher logic, Flight decode perf, IPv6 host) were untouched by these commits and remain cleanup items. Ship-able as-is.

github run

[james-elicx]

/bigbonk review for issues

Exact-head re-review requested for 80b4d6f9c after addressing both independent-review P1s: recursive mixed bounded/unbounded Flight shell validation, and conservative prefetch-header/default-locale middleware divergence. Local gates: Next.js 1/1, production browser 3/3, focused units 665/665, Knip/check/build clean.

[ask-bonk]

Re-review at exact head 80b4d6f9c

Reviewed the single delta commit fix(app-router): harden deferred prefetch validation (0ff276073 → 80b4d6f9c), which addresses the two independent-review P1s. Ran the relevant suites at head — all green: app-optimistic-routing (16), link-navigation (84), ppr-fallback-shell (17). vp check clean on both touched source files.

Verdict: no blocking issues. Both P1 fixes are correct.

P1a — recursive mixed bounded/unbounded Flight shell validation (app-page-loading-shell.ts) — correct

The old isInspectableSelectedRoot only walked a single $L/$@ string-reference chain and treated any model === undefined as an unconditional failure. It could not descend into element trees, so a bare unbounded reference sitting as a sibling inside a non-Suspense element (e.g. ["$","div",null,{"children":"$Lb"}] next to a properly bounded Suspense) went unvalidated and the shell was wrongly published as complete. The rewrite fixes this:

• Array/object recursion now walks the whole selected-root subtree, so an unbounded reference anywhere outside a Suspense boundary is caught.
• model === undefined → return insideSuspense is the correct semantic: a missing (still-suspended) reference is acceptable only under a Suspense boundary (the fallback renders); at top level it means an incomplete shell. Traced the new mixedBoundedAndUnboundedShell test: the div-wrapped $Lb resolves with insideSuspense === false → false → shell invalidated to NotSuspended. Matches the expectation.
• Suspense-element detection (["$","$<hex>",null,props] with suspenseTypeIds + non-null fallback) recurses fallback with the inherited flag and children with insideSuspense = true, mirroring the existing hasPostponedValueWithinSuspense structure exactly.
• Multi-root coverage: selectedRoots.some(...!isInspectable) runs the recursion per page/children root. Traced missingModalRoot (missing slot:children:/modal root → false → invalidated) and terminalModalRoot (c:E{...} terminal → inspectable → preserved). Both correct.
• visitedIds add/delete on unwind is correct DFS bookkeeping — a node reachable from two branches is no longer falsely rejected as a cycle — and the depth > 100 guard bounds genuine cycles. Both overflow returns (false here, false in the postponed pass) fail conservatively toward invalidation, so a pathological payload can never falsely publish PageSuspense.

isNonNullFallback correctly excludes "$undefined", so a Suspense with no real fallback isn't miscounted. This path is behind the lazy import() on the PPR fallback-shell render (prefetch/prerender only), not the per-request hot path.

P1b — conservative prefetch-header / default-locale middleware divergence (link.tsx) — correct

Prefetch-header matcher. Removing if (condition.key?.toLowerCase() === "next-router-prefetch") return false; and always returning "unknown" for header conditions is the right conservative direction. The old return false asserted "this matcher won't match the navigation, so middleware won't affect it → trust the prefetch shell." But a has: [{header: Next-Router-Prefetch}] matcher does run on the prefetch request and can rewrite it, so the prefetched payload is for a rewritten route; building an optimistic tree from it is exactly the mismatch this PR targets. "unknown" flows through middlewareMatcherConditionsMayMatchNavigation without an early false, so the navigation is conservatively classified as may-be-affected and the loading-shell prefetch is not trusted. The new prefetch-only present header matcher test pins this. No Pages Router parity gap — pages-data-target.ts has no equivalent has/missing header-condition matcher, so the latent bug was App-Router-only.

Default-locale prefix for locale: false matchers. For a locale: false matcher whose source carries an explicit locale prefix (/en/...), a navigation to the default-locale URL (/viewport-prefetch-target, no visible prefix) must still be tested against the prefixed candidate. The new branch adds /${defaultLocale}${...} only when a default locale exists and the pathname has no existing locale prefix (stripLocaleForMiddlewareMatcher(p) === p), avoiding double-prefixing. Verified __VINEXT_DEFAULT_LOCALE__/__VINEXT_LOCALES__ are populated on the client entries, and the candidatePathname === "/" case yields /en (not /en/). The new locale-false default-locale prefix test pins this.

Carry-over (unchanged, still non-blocking)

The three prior follow-ups — duplicated client middleware-matcher logic in link.tsx vs pages-data-target.ts, full Flight decode on the common loading-shell path, and the IPv6 host.split(":") edge case — were not touched by this commit and remain cleanup items. None block this PR.

Ship-able as-is.

[ask-bonk]

Review posted to PR #2852.

Summary

Re-reviewed at exact head 80b4d6f9c, focusing on the single delta commit fix(app-router): harden deferred prefetch validation that addresses the two independent-review P1s. Ran the relevant suites (all green: app-optimistic-routing 16, link-navigation 84, ppr-fallback-shell 17) and vp check (clean on both touched files).

Verdict: no blocking issues. Both P1 fixes are correct.

• P1a (recursive mixed bounded/unbounded Flight shell validation) — The rewritten isInspectableSelectedRoot now walks the full selected-root subtree instead of a single reference chain, and the model === undefined → return insideSuspense semantic correctly distinguishes an acceptable suspended child (under a Suspense boundary, fallback renders) from an incomplete top-level shell. Traced all four new/existing shell fixtures (valid, mixed-error, mixed bounded/unbounded, missing/terminal modal root) — each classifies correctly. DFS visitedIds add/delete and the depth > 100 conservative-fail guards are sound; overflow always fails toward invalidation.

• P1b (conservative prefetch-header / default-locale divergence) — Dropping the Next-Router-Prefetch → return false special-case (now "unknown") is the right conservative direction: a prefetch-header matcher does rewrite the prefetch request, so its shell must not be trusted for optimistic routing. The default-locale-prefix candidate for locale: false matchers is correctly guarded against double-prefixing and the / root case. Both pinned by new tests. No Pages Router parity gap.

The three prior non-blocking follow-ups (matcher duplication, Flight decode perf, IPv6 host) were untouched and remain cleanup items. Ship-able as-is.

github run

[james-elicx]

Backlog mapping for Actions run 31290819291: this PR appears to fix test/e2e/app-dir/concurrent-navigations/mismatching-prefetch.test.ts (recovers when a navigation rewrites to a different route than the one that was prefetched). The exact targeted Next.js E2E passes 1/1 at head 80b4d6f9c5106ab213a80b8e2273124a1357c1c3.