Review: restore intercepted history slots

Reviewed the fix end-to-end. The change is small, correctly scoped, and well-tested. The core fix is sound: during traverse (browser back/forward), Next.js restores the exact router tree stored in history, so requiring the proof's interception source to match the currently visible route (which may legitimately be the catch-all sibling) is wrong. Gating only the source-identity check on lane !== "traverse" while leaving every other validation in place is the right surgical fix.

What I verified

• Bypass is correctly scoped. Only the interceptedRejectedUnknownSource source-identity check is skipped for traverse. All structural guards still run on traverse: missing-proof, targetMatchedUrl match, manifest-declared interception, layout-ancestor persistence, and the active/owned slot-binding check. So a traverse can't restore a structurally invalid intercepted world — it can only trust the stored proof's source, which matches Next.js "restore the exact stored tree" semantics.
• Forward navigation unchanged. The existing "rejects intercepted preservation when the visible source route is stale" test (navigation lane) still rejects with interceptedRejectedUnknownSource. Confirmed all 43 planner tests pass.
• Single source of truth. validateInterceptedPreservation is the only place this logic lives; it's invoked from the environment-agnostic planner, so there's no dev/prod parity gap to mirror.
• Type check + targeted tests pass. tsc --noEmit clean; vitest run tests/navigation-planner.test.ts → 43 passed.
• Fixtures/e2e consistent. Port 4174 + waitForAppRouterHydration match the rest of the suite; the modal → catch-all page → back → catch-all-sibling flow maps to the new unit test scenario.

Suggestions (non-blocking)

1. Add an inline comment explaining the lane gate. options.lane !== "traverse" is the entire fix and is non-obvious to a future reader. Per this repo's convention of documenting deliberate Next.js parity decisions, a one-line comment would help, e.g.:

   // Browser history traversal restores the exact router tree stored in history.
   // The currently visible route may be the catch-all sibling rather than the
   // original interception source, so the source-identity match (which only
   // applies to forward navigation) must be skipped here. Structural proof
   // (manifest topology + slot bindings) is still enforced below.

2. Minor: getVisibleInterceptionSourceIdentity is still computed on the traverse path where its result is unused. It's a cheap pure call so not worth restructuring, but you could move it inside the if for clarity.

Overall this looks correct and matches the cited Next.js v16.2.6 test. LGTM with the comment suggestion.

Review: restore intercepted history slots

Reviewed end-to-end against the cited Next.js v16.2.6 test. The fix is correct, narrowly scoped, and well-tested. LGTM.

Note: this is the second commit (8f75dae, "scope interception restore proof") and the implementation now differs meaningfully from how the earlier ask-bonk review described it. The earlier review said the gate is options.lane !== "traverse". The actual code does not gate on the lane — it threads a new restoredHistorySnapshot flag from the history-restore commit producer through to the planner. This is a strictly better design, and the difference matters (see below).

Why the restoredHistorySnapshot flag is the right approach (not a lane gate)

A lane === "traverse" gate would have bypassed the source-identity check for every traverse, including non-history-restore traverse flows that legitimately should still validate the visible source. The actual fix only sets the flag in createRestoredHistorySnapshotCommit (app-browser-navigation-controller.ts:621), which is the single producer of a restored-history-snapshot commit. So the bypass applies precisely to "browser restored the exact stored router tree," which is the only case where requiring the proof's source to match the currently-visible catch-all sibling is wrong.

The new unit test rejects a fresh traverse response from an unrelated interception source (uses lane: "traverse" without restoredHistorySnapshot) proves exactly this distinction: a traverse that is not a restored history snapshot still rejects with interceptedRejectedUnknownSource. This is a sharper guarantee than a lane-based gate would give.

What I verified

• Bypass scope. Only the interceptedRejectedUnknownSource source-identity check is skipped (navigation-planner.ts:1310). Every structural guard still runs on the restored snapshot: missing-proof, targetMatchedUrl match, manifest-declared interception, layout-ancestor persistence, the active/owned slot-binding check, and the declared-interception owner match. A structurally invalid restored tree is still rejected; when approvePendingNavigationCommit returns approvedCommit === null, restoreHistorySnapshotVisibleState returns false and the caller falls back to document navigation (app-browser-navigation-controller.ts:651). So the worst case for an invalid snapshot is unchanged (hard nav), and the best case (valid stored modal world) is now correctly restored.
• Flag wiring is clean end-to-end. PendingNavigationCommit.restoredHistorySnapshot (app-browser-state.ts:130) → propagated into FlightResult in planPendingRootBoundaryFlightResponse (app-browser-state.ts:469) → consumed in planFlightResponseArrived via options.event.result.restoredHistorySnapshot === true (navigation-planner.ts:1470). Optional field, defaults to falsy, no behavior change for existing callers.
• Single source of truth. validateInterceptedPreservation is the only place this logic lives and is invoked from the environment-agnostic planner — no dev/prod parity gap to mirror across dev-server/prod-server/worker-entry.
• Tests pass locally. vitest run tests/navigation-planner.test.ts → 44 passed; tests/app-browser-history-controller.test.ts → 15 passed.

Fixture parity note (not a bug — flagging for the record)

The original Next.js fixture places comments, u, trending at the app root and uses the @modal/(...)comments interception convention (intercept-from-root). The vinext fixture nests everything under app/parallel-catchall-specificity/ (since app-basic is a shared app tree) and therefore correctly uses @modal/(.)comments (intercept-same-segment). The convention change from (...) to (.) is the correct consequence of the nesting, not a divergence — comments is now a sibling of @modal in the same segment. Semantics are preserved.

The e2e spec also adds one assertion the upstream test lacks: after goBack() it asserts the Modal heading is visible (line 21). That extra assertion is the actual regression guard for this fix, so it's a welcome strengthening.

Non-blocking suggestions

1. Lazy-evaluate the unused source identity. getVisibleInterceptionSourceIdentity (navigation-planner.ts:1308) is now computed unconditionally even when restoredHistorySnapshot is true, where its result is unused. It's a cheap pure call, but per this repo's hot-path guidance ("avoid repeated hot-path work"), moving it inside the if (!options.restoredHistorySnapshot && ...) would both avoid the dead compute and read more clearly.
2. Add an inline comment on the bypass. The !options.restoredHistorySnapshot condition is the entire fix and is non-obvious. Per this repo's convention of documenting deliberate Next.js-parity decisions, a one-liner pointing at "history traversal restores the exact stored tree; the visible route may be the catch-all sibling, so source-identity is skipped here while structural proof is still enforced below" would help future readers.

Overall: correct, surgical, and matches the cited Next.js v16.2.6 behavior.