fix(router): reject stale same-url server action commits

[NathanDrake2406]

What this changes

Implements #726-CORE-04 from #726 by rejecting stale same-URL server-action visible commits. A server action that started from an older visible router state can still return its action value, but it can no longer overwrite newer visible state once another commit has advanced the lifecycle version.

Why

Issue #726 calls out that activeNavigationId is not enough for same-URL refresh and server-action races. Same URL does not mean same visible world. Before this change, two same-URL server actions resolving out of order could let the older RSC payload commit after the newer one had already updated the route.

That violated the lifecycle spine invariant from #726: no stale operation commits visible state, and no older server-action result overwrites newer visible state.

Approach

• Extend the pending commit disposition gate to compare startedVisibleCommitVersion with the current router state visibleCommitVersion.
• Treat visible-version mismatch as stale before root-boundary hard-navigation decisions, so old action payloads do not trigger stale reload recovery.
• Read the current browser router state at approval time after async RSC payload resolution, instead of approving against only the state captured before the await.
• Preserve the existing same-URL server action return-value contract when the visible UI commit is rejected.

Non-goals: this does not model discarded revalidation, redirect lifecycle, cache reuse authority, or the broader NavigationPlanner work from issue #726.

Validation

• vp test run tests/app-browser-entry.test.ts
• vp check packages/vinext/src/server/app-browser-navigation-controller.ts packages/vinext/src/server/app-browser-state.ts packages/vinext/src/server/app-browser-visible-commit.ts tests/app-browser-entry.test.ts
• vp run vinext#build

vp run vinext#build completed with the existing virtual-module external warnings for virtual:vinext-rsc-entry, virtual:vite-rsc/client-references, and private-next-instrumentation-client.

Review note

Bonk: please read issue #726 to see the big picture before reviewing this PR. This is specifically the #726-CORE-04 slice, not the full lifecycle, planner, cache, or compatibility architecture.

[pkg-pr-new]

Open in StackBlitz

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

commit: 7b84a43

[NathanDrake2406]

@codex review

[james-elicx]

/bigbonk review

[ask-bonk]

This is a clean, well-scoped implementation of the #726-CORE-04 stale-action rejection gate. The approach is correct: stamp startedVisibleCommitVersion at operation creation time, re-read the live visibleCommitVersion at approval time, and skip if they diverge. A few observations and one minor nit below — none blocking.

What works well:

• The visible-version check in resolvePendingNavigationCommitDisposition is ordered before the shouldHardNavigate check, which is the right call. A stale action whose root layout happens to differ from the current one should be silently dropped, not trigger a hard reload. The "treats a visible commit version mismatch as stale before root-boundary decisions" test explicitly validates this.
• The getCurrentStateForApproval callback pattern in resolveAndClassifyNavigationCommit cleanly separates the "state at initiation" (used for createPendingNavigationCommit to stamp startedVisibleCommitVersion) from the "state at approval" (re-read after await). This mirrors what renderNavigationPayload already does at line 421 of the controller.
• The return-value contract is correctly preserved: stale actions still resolve their returnValue even when the UI commit is rejected. Both tests validate this.
• The actionInitiationState parameter threading from app-browser-entry.ts through to the controller is correct — currentState is captured before fetch() and before the RSC decode await, which is exactly when the action's "world view" should be frozen.

Test coverage is solid. The two new controller tests cover the two important orderings: (1) concurrent actions where the older payload resolves last (deferred promise pattern), and (2) sequential actions where the caller explicitly passes a stale actionInitiationState. The existing tests are updated to pass the new required fields without changing their semantics.

One minor nit inline.

[ask-bonk]

Nit: The ? makes this optional, which means callers that don't pass it silently fall back to using the stale captured currentState for approval. That's fine for renderNavigationPayload (which calls approvePendingNavigationCommit directly and never goes through this function), but if a future caller forgets to pass it for an async path, the version gate becomes ineffective without any signal.

Consider making it required for async callers or at least adding a doc comment that explains the fallback semantics — something like "If omitted, approval uses the captured currentState, which is only safe for synchronous commit paths."

[ask-bonk]

Review posted. Here's a summary of what I found:

Approved — this is a well-scoped, correct implementation of the #726-CORE-04 stale-action rejection gate.

Key observations:

1. Correct ordering — The visibleCommitVersion check in resolvePendingNavigationCommitDisposition runs before shouldHardNavigate, so a stale action with a different root layout gets silently dropped rather than triggering a spurious hard reload.

2. Clean state separation — The getCurrentStateForApproval callback pattern correctly separates "state at initiation" (used to stamp startedVisibleCommitVersion via createPendingNavigationCommit) from "state at approval" (re-read after the async RSC payload resolves). This mirrors the existing pattern in renderNavigationPayload at controller line 421.

3. Return-value contract preserved — Stale actions still resolve their returnValue to the caller even when the UI commit is rejected, matching the documented contract.

4. Good test coverage — Two new controller tests cover both orderings: concurrent actions with deferred resolution, and sequential actions with an explicitly stale initiation state. All 67 tests pass.

One minor nit posted inline: the optional getCurrentStateForApproval? parameter in resolveAndClassifyNavigationCommit falls back silently to the stale captured state when omitted, which could hide bugs in future async callers. Suggested adding a doc comment about the fallback semantics.

github run