fix(router): refresh retained parallel routes

[james-elicx]

Summary

• refresh every mounted App Router branch when router refresh or a revalidating server action has retained parallel routes
• preserve source-page query and history route evidence across interception and traversal
• make supplemental refresh fan-out atomic, cancellable, and recoverable on failure or timeout
• promote an unambiguous supplemental source page into its mounted nested children carrier
• write history route and BFCache metadata from the approved merged visible state
• avoid refreshing retained descendant children routes after navigation returns to an ancestor
• compare supplemental refresh route identities canonically across basePath, trailing slashes, and encoded path segments

Deploy-suite failures fixed

• Source: run 31439707085, test report job 93624401572, Next.js v16.2.6.

1. router.refresh regular, searchParams false: previously intercepted modal and active page slot
2. router.refresh regular, searchParams true: intercepted route and previously active page slot
3. router.refresh regular, searchParams true: previously intercepted modal and active page slot
4. router.refresh dynamic, searchParams false: previously intercepted modal and active page slot
5. router.refresh dynamic, searchParams true: intercepted route and previously active page slot
6. router.refresh dynamic, searchParams true: previously intercepted modal and active page slot
7. server action revalidation: multiple active parallel slots

Validation

Validated at a50c0c7:

• exact Next.js v16.2.6 parallel-routes-revalidation suite: 16 passed, 1 upstream skip
• app-router-client-cache Playwright project: 15 passed, including the refresh regression
• focused vinext tests: 670 passed
• full vp check: clean
• independent review and re-review: no findings

[james-elicx]

/bigbonk review for issues

[pkg-pr-new]

Open in StackBlitz

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

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

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

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

commit: b3b0d50

[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

[ask-bonk]

Review: fix(router): refresh retained parallel routes

Reviewed all 29 changed files. This is a well-structured change: heavy runtime logic lives in the new typed app-browser-supplemental-refresh.ts helper (with direct unit tests), the generated entry stays thin, and dev/prod parity holds because the App Router prod server delegates to the built RSC entry. vp check is clean on the touched source files and the focused test suites pass (supplemental-refresh, browser-entry, history-controller, elements, navigation-planner, cache-busting, rsc-handler, entry-templates, shims).

Verified as correct

• Cancellation/ordering. commitSameUrlNavigatePayload sets shouldRetrySupplementalRefresh inside a .then() on nextElements, and the controller (resolveAndClassifyNavigationCommit) awaits nextElements before the outer await resolves, so the retry flag is always observed. Refresh fan-out is cancelled via navigationAbortController.signal (through abortSupersededNavigation()); server-action fan-out via the coordinator's abortAll() at the top of bootstrapHydration. Both supersession paths are covered.
• History-state hardening. readHistoryStateActiveRoutePaths rejects any entry that is not a clean absolute path (blocks ?, #, \, \0, //-prefix, missing leading /). Since those paths flow into new URL(pathname, refreshUrl) + fetch, this is the right SSRF/path-traversal defense for forgeable history state.
• Interception source URL. interceptionSourceMatchedUrl is validated through parseInterceptionMatchedUrl (path-shaped only) before it becomes the interceptionContext on a supplemental fetch — good, since it is attacker-influenceable via the wire payload.
• Cache-busting rollout. Adding X-Vinext-Interception-Id to the hash input and Vary header is a breaking key change; the added computePrevious*InterceptionId* accepted-hash variants only relax validation to hashes that were already valid before the header existed. Correct backward-compat handling.

Non-blocking observations

1. resolveSupplementalRefreshes catch re-awaits a possibly-rejected primary (app-browser-supplemental-refresh.ts:308). If options.primary rejects, await options.primary in the catch re-throws and the whole function rejects instead of returning a degraded result. This is acceptable — a failed primary payload should fail the navigation on both the refresh (requireCompleteSupplementalRefresh) and server-action paths — but it means the degraded contract only holds when the primary succeeds. Worth a one-line comment so a future reader does not assume this always resolves.

2. Interception-id header is not null-byte-stripped (app-rsc-request-normalization.ts:238-245), unlike the sibling X-Vinext-Interception-Context header which explicitly strips null bytes (Step 8). Here the value is only length-bounded and required to start with interception:. Because it is inbound-only (used for exact equality against graph ids and as a cache-busting input, never reflected to a response), this is not exploitable, but stripping \0 for consistency with the neighboring header would remove one avenue of cache-key value fan-out and matches the documented F-PROD-1 pattern.

No correctness blockers found.

[ask-bonk]

Review posted to PR #2880.

Summary

I reviewed all 29 changed files in PR #2880, ran vp check (clean) and the focused test suites (all passing), and traced the critical logic paths.

No correctness blockers found. The change is well-architected: runtime logic lives in the new typed app-browser-supplemental-refresh.ts helper with direct unit tests, generated entries stay thin, and dev/prod parity holds (App Router prod delegates to the built RSC entry).

Things I verified as correct:

• Cancellation/ordering — the shouldRetrySupplementalRefresh flag is reliably observed because the controller awaits nextElements before the outer await resolves; both supersession paths (nav abort controller + coordinator abortAll) are covered.
• History-state hardening — readHistoryStateActiveRoutePaths properly rejects unclean paths before they reach fetch, a solid SSRF/traversal defense on forgeable state.
• Interception source URL validated via parseInterceptionMatchedUrl before becoming a fetch context.
• Cache-busting rollout — the new accepted-hash variants only relax to hashes that were valid pre-header.

Two non-blocking observations I posted:

1. resolveSupplementalRefreshes re-awaits a possibly-rejected primary in its catch, so the degraded contract only holds when primary succeeds (acceptable, but worth a comment).
2. The new interception-id header isn't null-byte-stripped unlike its sibling context header — not exploitable (inbound-only, exact-match/hash), but stripping for consistency would remove one cache-key fan-out avenue.

github run

[github-actions]

Performance benchmarks

Compared b3b0d50 against base 92ff5b4 using alternating same-runner rounds. Next.js was unchanged and skipped.

0 improved · 2 regressed · 4 within ±1.5%

Scenario	Framework	Baseline	Current	Change
Client bundle size (gzip)	vinext	137.8 KB	140.3 KB	🔴 +1.8%
Client entry size (gzip)	vinext	125.2 KB	127.7 KB	🔴 +2.0%
Dev server cold start	vinext	3.06 s	3.10 s	⚫ +1.3%
Production build time	vinext	3.50 s	3.54 s	⚫ +1.2%
RSC entry closure size (gzip)	vinext	114.9 KB	115.2 KB	⚫ +0.3%
Server bundle size (gzip)	vinext	195.4 KB	196.2 KB	⚫ +0.4%

View detailed results and traces

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

[james-elicx]

/bigbonk review for issues

[ask-bonk]

UnknownError: ProviderInitError

github run

[ask-bonk]

@james-elicx Bonk workflow failed. Check the logs for details.

View workflow run · To retry, trigger Bonk again.

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Re-review: fix(router): refresh retained parallel routes

Re-reviewed all 29 files after the retry. Ran vp check on the five heaviest touched source files (clean) and the focused suites app-browser-supplemental-refresh + app-browser-entry (256 passed). Traced the identity round-trip, SSRF surface, abort/timeout handling, and dev/prod parity. No correctness blockers.

Verified correct

• Interception-id identity round-trip is consistent. The slot-binding interceptionId written server-side in app-page-route-wiring.tsx originates from the matcher, which for the slots path computes createAppRouteGraphInterceptionId(slotModule.id, sourceMatchPattern, targetPattern) (app-rsc-route-matching.ts:498) — the exact key the graph uses when populating segmentGraph.interceptions (app-route-graph.ts:658,764). So the client's routeManifest.segmentGraph.interceptions.get(binding.interceptionId) lookup and the server's findIntercept(..., interceptionId) equality filter resolve the same declaration. Exporting createAppRouteGraphInterceptionId to share the one formatter is the right call over duplicating the string shape.
• No SSRF via previousNextUrl. resolvePersistedSourcePageRefreshes only forwards pathname + search, and createRscRequestUrl reconstructs against http://vinext.local returning a relative pathname+search — an absolute/off-origin previousNextUrl cannot escape same-origin. The traverse path is additionally gated by readHistoryStateActiveRoutePaths's clean-absolute-path rejection.
• Supersession vs. timeout abort are distinguished. On the refresh path the rejected rscPayload surfaces through renderNavigationPayload; the outer catch checks isCurrentNavigation(navId) first, so a supersession abort returns silently while a real timeout/failed degradation (still-current nav) falls through to performHardNavigationForScrollIntent — a full reload that recovers fresh state. Matches the PR's "recoverable on failure or timeout" intent.
• Server-action degradation contract holds. resolveServerActionSupplementalRefresh returns the current elements on any non-abort degrade, so the reassigned nextElements.then(...) only rejects when options.primary itself rejects — i.e. the same action-failure path that existed before this PR. Graceful degrade when primary succeeds; unchanged failure semantics when it doesn't.

Non-blocking observations

1. resolveSupplementalRefreshes catch re-awaits a possibly-rejected primary (app-browser-supplemental-refresh.ts:326). If options.primary rejects, await options.primary in the catch re-throws, so the degraded result contract only holds when the primary resolves. This is the correct behavior for both callers (a failed primary should fail the navigation), but a one-line comment documenting it would help a future reader who assumes this always resolves.

2. The new interception-id header is length-bounded and prefix-checked but not null-byte-stripped (app-rsc-request-normalization.ts:238-245), unlike the sibling X-Vinext-Interception-Context header (Step 8). Not exploitable — the value is inbound-only, used solely for exact equality against graph ids (a \0 simply never matches) and as a cache-key input, never reflected — but stripping \0 for parity with the neighboring header would remove one cache-key fan-out avenue.

Non-blocking, ship-ready.

[ask-bonk]

Review posted to PR #2880.

Summary

I re-reviewed all 29 changed files, ran vp check on the five heaviest touched source files (clean), and ran the focused suites app-browser-supplemental-refresh + app-browser-entry (256 passed). No correctness blockers found.

Things I verified as correct:

• Interception-id identity round-trip — the slot-binding id written server-side, the client's interceptions.get() lookup, and the server's findIntercept equality filter all key off the same createAppRouteGraphInterceptionId(slotId, sourcePattern, targetPattern) formatter (now exported and shared). Consistent.
• No SSRF — refresh targets forward only pathname+search, and createRscRequestUrl reconstructs against a local origin, so an absolute/off-origin previousNextUrl can't escape same-origin. Traverse paths are additionally gated by the clean-absolute-path rejection in readHistoryStateActiveRoutePaths.
• Abort vs. timeout — supersession aborts return silently (isCurrentNavigation guard in the outer catch), while a real timeout/failure on a still-current nav falls through to a hard-nav reload, matching the PR's stated recovery intent.
• Server-action degradation — degrades to current elements on non-abort failure; only rejects when the primary payload itself rejects, i.e. the pre-existing action-failure path.

Two non-blocking observations (same spirit as the earlier bonk pass):

1. resolveSupplementalRefreshes re-awaits a possibly-rejected primary in its catch — correct for both callers, worth a clarifying comment.
2. The new interception-id header isn't null-byte-stripped unlike its sibling context header — not exploitable (inbound-only, exact-match/hash), but stripping would match the F-PROD-1 pattern.

Ship-ready.

github run

[james-elicx]

Backlog ownership check: I believe this PR fixes the seven non-cache failures from run 31439707085 / job 93624401572 in test/e2e/app-dir/parallel-routes-revalidation/parallel-routes-revalidation.test.ts: the six regular/dynamic router.refresh() retained intercepted/page-slot cases (with and without search params) plus the server-action revalidation case with multiple active parallel slots. The live head a50c0c71fb945ddea9b13559f0025059a1ad76b5 is conflict-free, so I am adopting this existing james-elicx PR for independent exact-head verification rather than starting a duplicate.

[james-elicx]

Independent review at exact head a50c0c71fb945ddea9b13559f0025059a1ad76b5: NO FINDINGS.

Reviewed the full 29-file diff, including supplemental refresh atomicity/abort/timeout behavior, persisted source/interception identity, history metadata, visible-commit merging, exact server-side interception disambiguation, request validation, cache-busting compatibility, and generated-entry wiring.

Verification:

• vp test run across 14 focused changed/adjacent suites: 2,249 tests passed
• vp check across all 29 touched files: format, lint, and types clean
• targeted upstream test/e2e/app-dir/parallel-routes-revalidation/parallel-routes-revalidation.test.ts: 16 passed, 1 upstream skip, 0 failed (retry 0/0)

The change is parity-complete and ship-ready at this SHA.

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: fix(router): refresh retained parallel routes

Reviewed all 29 files at head. Ran the focused suites — app-browser-supplemental-refresh (18), app-browser-entry + navigation-planner + app-elements (371), and app-rsc-cache-busting + app-browser-history-controller + app-rsc-handler + shims + entry-templates (1,590) — all green. vp check clean on the heaviest touched source files. Traced the identity round-trip, SSRF surface, abort/timeout handling, visible-commit merge, and dev/prod parity. No correctness blockers.

Verified correct

• Pure double-reduce is safe. resolveAndClassifyNavigationCommit now calls applyApprovedVisibleCommit an extra time (app-browser-navigation-controller.ts:842) to derive history metadata from the approved visible state rather than the raw action. applyApprovedVisibleCommit is a pure reducer returning a fresh object (it only advances visibleCommitVersion on the returned copy, never mutates state), so recomputing it and then committing again is correct — the history write now describes the tree that actually becomes visible (planner-retained slots + BFCache ids), fixing the case where the raw action marks retained slots default/unmatched.
• No SSRF via refresh fan-out. resolvePersistedSourcePageRefreshes forwards only pathname + search, and every supplemental fetch routes through createRscRequestUrl, which reconstructs against http://vinext.local and returns a relative pathname+search — an absolute/off-origin previousNextUrl cannot escape same-origin. The traverse path is further gated by readHistoryStateActiveRoutePaths's clean-absolute-path rejection (blocks ?, #, \, \0, //-prefix, missing leading /).
• Interception-id identity round-trip is consistent. The slot-binding id written server-side (app-page-route-wiring.tsx), the client lookup routeManifest.segmentGraph.interceptions.get(binding.interceptionId), and the server-side findIntercept(..., interceptionId) equality filter (app-rsc-route-matching.ts:280) all key off the same exported createAppRouteGraphInterceptionId(slotId, sourcePattern, targetPattern) formatter. Exporting the single formatter over duplicating the string shape is the right call.
• Abort vs. timeout are distinguished and recoverable. In resolveSupplementalRefreshes, supersession aborts (options.signal) return reason: "aborted"; the 10s timeout raises a TimeoutError DOMException → reason: "timeout"; anything else → "failed". Refresh path calls requireCompleteSupplementalRefresh (throws on degrade), whose rejection surfaces through the nav error handler — but only falls through to a hard-nav reload when the nav is still current (isCurrentNavigation guard), so superseded navs abort silently. Server-action path degrades to current elements and only re-schedules a discarded-revalidation retry on non-abort degrade. Both supersession entrypoints are covered: navigationAbortController.signal for refresh, serverActionSupplementalRefreshCoordinator.abortAll() at the top of bootstrapHydration for actions.
• Wire validation for the new slot-binding fields. parseSlotBindings rejects empty/non-string interceptionId and runs interceptionSourceMatchedUrl through parseInterceptionMatchedUrl (path-shaped only) before it can become a supplemental fetch context — good, since it is attacker-influenceable via the payload.
• Cache-busting rollout is backward-compatible. Adding X-Vinext-Interception-Id to both the hash input and the Vary header is a breaking key change; the added includeInterceptionIdHeader: false compatibility variants only relax validation to hashes that were already valid before the header existed. Correct.
• Ancestor/descendant retention filter. The activeRoutePath.startsWith(refreshRoutePathname + "/") prefix check uses the + "/" segment-boundary suffix, so /foo does not falsely match /foobar. Deeper cache-retained children of an ancestor are correctly excluded from the fan-out.

Non-blocking observations (unchanged from prior passes)

1. resolveSupplementalRefreshes catch re-awaits a possibly-rejected primary (app-browser-supplemental-refresh.ts:326). If options.primary rejects, await options.primary in the catch re-throws, so the degraded result contract only holds when the primary resolves. This is the correct behavior for both callers (a failed primary should fail the navigation), but a one-line comment documenting it would help a future reader who assumes this always resolves.

2. The new interception-id header is length-bounded (4096) and prefix-checked but not null-byte-stripped or whitespace-rejected (app-rsc-request-normalization.ts:238-245), unlike the sibling X-Vinext-Interception-Context header, which strips \0, rejects whitespace, and caps at 1024. The id header is a positional cache-key input and is in the Vary list, so an attacker with a valid interception: prefix can fan out up to ~4KB of distinct values into the RSC cache key — the same F-PROD-1 fragmentation class the context header guards against, just with a looser bound. Not a correctness bug (the value is inbound-only, exact-matched against graph ids and never reflected, so a \0/junk value simply never matches), but tightening the bound and stripping \0 for parity with the neighboring header would close the cache-key fan-out avenue.

Non-blocking, ship-ready.

[ask-bonk]

Review posted to PR #2880.

Summary

I reviewed all 29 changed files, ran the focused test suites (18 + 371 + 1,590 = all passing), and confirmed vp check is clean on the heaviest touched source files. No correctness blockers found.

Key things I verified as correct:

• Pure double-reduce is safe — the extra applyApprovedVisibleCommit call in the nav controller derives history metadata from the approved visible state (planner-retained slots + BFCache ids). The reducer is pure and returns a fresh object, so recomputing then committing is correct, and it fixes the case where the raw action marked retained slots default/unmatched.
• No SSRF — refresh targets forward only pathname+search, createRscRequestUrl reconstructs against a local origin, and the traverse path is gated by clean-absolute-path rejection.
• Interception-id identity round-trip — server write, client lookup, and server findIntercept filter all key off the same exported createAppRouteGraphInterceptionId formatter.
• Abort vs. timeout distinguished and recoverable — supersession aborts return silently, real timeouts/failures on a still-current nav fall through to a hard-nav reload; both supersession entrypoints (nav abort controller + coordinator abortAll) are covered.
• Wire validation, cache-busting rollout, and ancestor/descendant retention all check out.

Two non-blocking observations (consistent with the earlier bonk passes):

1. resolveSupplementalRefreshes re-awaits a possibly-rejected primary in its catch — correct for both callers, worth a one-line comment.
2. The new interception-id header is length-bounded (4096) and prefix-checked but not \0-stripped or whitespace-rejected unlike its sibling context header (stripped, whitespace-rejected, 1024 cap). Not exploitable — inbound-only, exact-matched, never reflected — but tightening for parity would close a cache-key fan-out avenue.

Ship-ready.

github run

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: fix(router): refresh retained parallel routes

Independent pass over all 31 changed files at head. Ran the focused suites — app-browser-supplemental-refresh + app-browser-navigation-abort (21), app-browser-entry + app-browser-history-controller + navigation-planner + app-elements + app-rsc-cache-busting (431) — all green. Traced the identity round-trip, SSRF surface, abort/timeout handling, the double-reduce, dev/prod parity, and the new wire fields. No correctness blockers.

Verified correct

• Dev/prod parity for the new interceptionId filter. Both the generated dev entry (entries/app-rsc-entry.ts findIntercept(pathname, sourcePathname, interceptionId)) and the prod handler (app-rsc-handler.ts:994 matchInterceptRoute(..., interceptionIdHeader)) thread the same third argument into createAppRscRouteMatcher.findIntercept, which equality-filters on entry.interceptionId (app-rsc-route-matching.ts:282). The server writes the graph-owned entry.interceptionId (via toInterceptOptions → createAppPageSlotBindings), never the raw attacker header, so the header is used only for exact-match selection and is never reflected.
• Interception-id identity round-trip is consistent. The slot-binding id written server-side originates from createAppRouteGraphInterceptionId(slotModule.id, sourceMatchPattern, targetPattern) (app-rsc-route-matching.ts:498) — the same exported formatter the graph uses to key segmentGraph.interceptions. The client's interceptions.get(binding.interceptionId) lookup (app-browser-entry.ts:491) and the server's findIntercept filter resolve the same declaration.
• No SSRF via the refresh fan-out. resolvePersistedSourcePageRefreshes forwards only pathname + search, resolvePersistedRefreshInterceptions rebuilds targets against refreshUrl and keeps only pathname+search, and every supplemental fetch routes through createRscRequestUrl, which reconstructs against http://vinext.local. An absolute/off-origin previousNextUrl cannot escape same-origin. The traverse path is additionally gated by readHistoryStateActiveRoutePaths's clean-absolute-path rejection (?, #, \, \0, //-prefix, missing leading /), and the shim-preserved __vinext_activeRoutePaths is re-validated on read.
• Pure double-reduce is safe. resolveAndClassifyNavigationCommit calls applyApprovedVisibleCommit a second time to derive history metadata from the approved visible state. The reducer returns a fresh object and never mutates state, so recomputing then committing is correct and fixes the case where the raw action marks retained slots default/unmatched.
• Cancellation/ordering. shouldRetrySupplementalRefresh is set inside a .then() on nextElements, and the controller awaits nextElements (createPendingNavigationCommit) before the outer await resolves, so the flag is always observed. Both supersession entrypoints are covered: navigationAbortCoordinator (refresh transport) and serverActionSupplementalRefreshCoordinator.abortAll() (actions). The new abort coordinator correctly releases ownership once the Flight response is accepted so a newer nav does not tear down a decoding body.
• Abort vs. timeout distinguished and recoverable. Supersession aborts → reason: "aborted" (silent when superseded); the 10s TimeoutError → "timeout"; else "failed". Refresh path throws via requireCompleteSupplementalRefresh and only hard-navigates when isCurrentNavigation still holds; server-action path degrades to current elements and re-schedules a discarded-revalidation retry on non-abort degrade.
• Wire validation. parseSlotBindings rejects empty/non-string interceptionId and runs interceptionSourceMatchedUrl through parseInterceptionMatchedUrl (path-shaped only) before it can become a supplemental fetch context.
• Cache-busting rollout is backward-compatible. Adding X-Vinext-Interception-Id to the hash input and Vary list is a breaking key change; the includeInterceptionIdHeader: false compatibility variants only relax to hashes that were valid pre-header.
• Ancestor/descendant retention filter uses the + "/" segment-boundary suffix, so /foo does not match /foobar (covered by the "shared pathname prefix" test).

Non-blocking observations (unchanged from prior passes)

1. resolveSupplementalRefreshes catch re-awaits a possibly-rejected primary (app-browser-supplemental-refresh.ts:326). If options.primary rejects, await options.primary in the catch re-throws, so the degraded result contract only holds when the primary resolves. Correct for both callers (a failed primary should fail the navigation), but a one-line comment would help a future reader who assumes it always resolves.

2. The new interception-id header is length-bounded (4096) and prefix-checked but not \0-stripped or whitespace-rejected (app-rsc-request-normalization.ts:238-245), unlike the sibling X-Vinext-Interception-Context header (strips \0, rejects whitespace, caps at 1024). It is a positional cache-key input and in the Vary list, so a client with a valid interception: prefix can fan out up to ~4KB of distinct values into the RSC cache key — the same F-PROD-1 fragmentation class the context header guards. Not exploitable (inbound-only, exact-matched against graph ids, never reflected), but tightening the bound and stripping \0 for parity would close the fan-out avenue.

No correctness blockers found. Ship-ready.

[ask-bonk]

Review posted to PR #2880.

Summary

I independently reviewed all 31 changed files, ran the focused test suites (21 + 431 = all passing), and traced the critical paths. No correctness blockers found.

Key things I verified as correct:

• Dev/prod parity for the new interceptionId filter — both the generated dev entry and the prod handler thread the same argument into findIntercept, which equality-filters on entry.interceptionId. The server writes the graph-owned id (via toInterceptOptions → createAppPageSlotBindings), never the raw attacker header, so the header is only used for exact-match selection and is never reflected.
• Interception-id identity round-trip — server write, client interceptions.get() lookup, and server findIntercept filter all key off the same exported createAppRouteGraphInterceptionId formatter.
• No SSRF — refresh fan-out forwards only pathname+search, reconstructed against a local origin; traverse paths are gated by clean-absolute-path rejection, and shim-preserved active route paths are re-validated on read.
• Pure double-reduce is safe — the extra applyApprovedVisibleCommit derives history metadata from the approved visible state; the reducer is pure/non-mutating.
• Cancellation/ordering, abort vs. timeout, wire validation, cache-busting rollout, and ancestor/descendant retention all check out.

Two non-blocking observations (consistent with the prior bonk passes, and I confirmed neither is exploitable):

1. resolveSupplementalRefreshes re-awaits a possibly-rejected primary in its catch — correct for both callers, worth a clarifying comment.
2. The new interception-id header is length-bounded (4096) and prefix-checked but not \0-stripped/whitespace-rejected unlike its sibling context header — not exploitable, but tightening for parity would close a cache-key fan-out avenue.

Ship-ready.

github run

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: fix(router): refresh retained parallel routes

Independent pass over all 31 changed files at head. Ran the focused suites — app-browser-supplemental-refresh (18), app-browser-navigation-abort + app-elements (91) — all green. I re-traced the critical paths myself rather than relying on the prior passes: the interception-id identity round-trip, the SSRF surface on the refresh fan-out, abort/timeout distinction, the double-reduce in the nav controller, and the new inbound wire fields. No correctness blockers.

Verified correct

• Inbound interception-id header is a lookup key, never reflected. interceptionIdHeader (app-rsc-request-normalization.ts:238-245) is threaded only into findIntercept(..., interceptionId), which equality-filters entry.interceptionId (app-rsc-route-matching.ts:282). The value serialized back to the client is intercept.interceptionId sourced from the matched graph entry (app-page-dispatch.ts:591 via toInterceptOptions), never the raw header. So a forged header can only fail to match; it cannot poison a serialized binding.
• Interception-id identity round-trip is consistent. The graph-owned id written server-side originates from createAppRouteGraphInterceptionId(slotModule.id, sourceMatchPattern, targetPattern) (app-rsc-route-matching.ts:498) — the same exported formatter that keys segmentGraph.interceptions. The client's interceptions.get(binding.interceptionId) lookup and the server's findIntercept filter resolve the same declaration. Exporting the single formatter (app-route-graph.ts:434) over duplicating the string shape is the right call.
• No SSRF via the refresh fan-out. resolvePersistedSourcePageRefreshes forwards only pathname + search; resolvePersistedRefreshInterceptions derives targets from server-validated state.interception.targetMatchedUrl/activeRoute.path, rebuilds against refreshUrl, and keeps only pathname+search; every supplemental fetch routes through createRscRequestUrl, which reconstructs against http://vinext.local. An absolute/off-origin previousNextUrl cannot escape same-origin. The traverse path is additionally gated by readHistoryStateActiveRoutePaths's clean-absolute-path rejection (?, #, \, \0, //-prefix, missing leading /), and interceptionSourceMatchedUrl is re-validated through parseInterceptionMatchedUrl on parse (app-elements-wire.ts).
• Pure double-reduce is safe. applyApprovedVisibleCommit is a pure reducer — it only reads state and spreads into fresh objects, advancing visibleCommitVersion solely on the returned copy (app-browser-visible-commit.ts:130). Calling it once to derive history metadata from the approved visible state and again to commit is correct, and it fixes the case where the raw action marks planner-retained slots default/unmatched.
• Abort ownership released at the right moment. createAppBrowserNavigationAbortCoordinator releases ownership once the Flight response is accepted (and for non-supplemental navs, explicitly right after acceptance at app-browser-entry.ts:2441) so a newer nav does not tear down a decoding body. Supplemental refreshes retain the signal until commit because their branch fetches share it.
• Abort vs. timeout distinguished and recoverable. resolveSupplementalRefreshes maps supersession → "aborted", the 10s TimeoutError → "timeout", else "failed". The refresh path throws via requireCompleteSupplementalRefresh and only hard-navigates when isCurrentNavigation still holds; the server-action path degrades to current elements and re-schedules a discarded-revalidation retry on non-abort degrade (the flag is set inside the .then() on nextElements, which the controller awaits before the outer await resolves, so it is always observed).
• Cache-busting rollout is backward-compatible. Adding X-Vinext-Interception-Id to the hash input and Vary list is a breaking key change; the includeInterceptionIdHeader: false compatibility variants only relax validation to hashes that were valid pre-header.
• Ancestor/descendant retention filter uses the + "/" segment-boundary suffix (app-browser-supplemental-refresh.ts:235), so /foo does not falsely match /foobar; deeper cache-retained descendants of an ancestor are correctly excluded from the fan-out.

Non-blocking observations

1. resolveSupplementalRefreshes catch re-awaits a possibly-rejected primary (app-browser-supplemental-refresh.ts:326). If options.primary rejects, await options.primary in the catch re-throws, so the degraded contract only holds when the primary resolves. This is the correct behavior for both callers (a failed primary should fail the navigation), but a one-line comment documenting it would help a future reader who assumes it always resolves.

2. The new interception-id header is length-bounded (4096) and prefix-checked but not \0-stripped or whitespace-rejected (app-rsc-request-normalization.ts:238-245), unlike the sibling X-Vinext-Interception-Context header, which strips \0, rejects whitespace, and caps at 1024 (app-interception-context-header.ts). It is a positional cache-key input and in the Vary list, so a client with a valid interception: prefix can fan out up to ~4KB of arbitrary distinct values (including \0/whitespace) into the RSC cache key — the same F-PROD-1 fragmentation class the sibling header guards against, just with a looser bound. I confirmed this is not exploitable: the value is inbound-only, exact-matched against graph ids (a junk value simply never matches), and never reflected. I also confirmed legitimate values are always interception:<slotId>:<pattern>-><pattern> (no whitespace/null bytes), so tightening the bound and stripping \0 for parity with the neighboring header would close the fan-out avenue with zero risk to real clients.

No correctness blockers found. Ship-ready.

[ask-bonk]

Review posted to PR #2880.

Summary

I independently reviewed all 31 changed files, ran the focused suites (app-browser-supplemental-refresh 18, app-browser-navigation-abort + app-elements 91 — all passing), and re-traced the critical paths myself. No correctness blockers found.

Key things I verified as correct:

• Inbound interception-id header is a lookup key, never reflected — it's threaded only into findIntercept's equality filter; the value serialized back to the client comes from the matched graph entry (toInterceptOptions), not the raw header, so a forged header can only fail to match.
• Interception-id identity round-trip — server write, client interceptions.get() lookup, and server findIntercept all key off the same exported createAppRouteGraphInterceptionId formatter.
• No SSRF — refresh fan-out forwards only pathname+search, reconstructed against a local origin; traverse paths gated by clean-absolute-path rejection; interceptionSourceMatchedUrl re-validated on parse.
• Pure double-reduce is safe — applyApprovedVisibleCommit only reads state and spreads into fresh objects, advancing the version on the returned copy.
• Abort ownership, abort-vs-timeout recovery, cache-busting rollout, and ancestor/descendant retention all check out.

Two non-blocking observations (consistent with the prior bonk passes, and I confirmed neither is exploitable):

1. resolveSupplementalRefreshes re-awaits a possibly-rejected primary in its catch — correct for both callers, worth a one-line comment.
2. The new interception-id header is length-bounded (4096) and prefix-checked but not \0-stripped/whitespace-rejected unlike its sibling context header (1024 cap, stripped, whitespace-rejected). I confirmed legitimate values never contain whitespace/null bytes, so tightening for parity would close a cache-key fan-out avenue with zero risk to real clients.

Ship-ready.

github run

[james-elicx]

/bigbonk review for issues

[ask-bonk]

github run

[james-elicx]

/bigbonk review for issues

[ask-bonk]

github run

[james-elicx]

/bigbonk review for issues

[ask-bonk]

github run