From f2b1e69cbc396ffb2900a8163bbde426b771c37b Mon Sep 17 00:00:00 2001 From: Nathan Nguyen <146415969+NathanDrake2406@users.noreply.github.com> Date: Wed, 6 May 2026 19:32:36 +1000 Subject: [PATCH 1/2] fix(router): reject stale same-url server action commits Same-URL server action payloads could commit visible router state after newer visible work had already won. URL equality and activeNavigationId alone did not distinguish two different visible worlds at the same href. The lifecycle gate now compares a pending operation's started visibleCommitVersion against the current router state's visibleCommitVersion at approval time. Stale results keep returning their action value, but they no longer mutate visible state or trigger stale hard-navigation recovery. Tests cover out-of-order same-URL server actions and the visible-version stale path taking priority over root-boundary hard navigation. --- .../app-browser-navigation-controller.ts | 8 +- .../vinext/src/server/app-browser-state.ts | 10 ++ .../src/server/app-browser-visible-commit.ts | 8 +- tests/app-browser-entry.test.ts | 95 +++++++++++++++++++ 4 files changed, 114 insertions(+), 7 deletions(-) diff --git a/packages/vinext/src/server/app-browser-navigation-controller.ts b/packages/vinext/src/server/app-browser-navigation-controller.ts index a46a83df8..a82916bde 100644 --- a/packages/vinext/src/server/app-browser-navigation-controller.ts +++ b/packages/vinext/src/server/app-browser-navigation-controller.ts @@ -417,7 +417,7 @@ export function createAppBrowserNavigationController( const approval = approvePendingNavigationCommit({ activeNavigationId, - currentState, + currentState: getBrowserRouterState(), pending, startedNavigationId: options.navId, }); @@ -479,11 +479,6 @@ export function createAppBrowserNavigationController( ): Promise { const currentState = getBrowserRouterState(); const startedNavigationId = activeNavigationId; - // Known limitation: if a same-URL navigation fully commits while this - // server action is awaiting resolveAndClassifyNavigationCommit(), the action - // can still dispatch its older payload afterward. The old pre-2c code had - // the same race, and Next.js has similar behavior. Tightening this would - // need a stronger commit-version gate than activeNavigationId alone. const { approvedCommit, decision, @@ -496,6 +491,7 @@ export function createAppBrowserNavigationController( currentState, navigationSnapshot, nextElements, + getCurrentStateForApproval: getBrowserRouterState, renderId: allocateRenderId(), operationLane: "server-action", startedNavigationId, diff --git a/packages/vinext/src/server/app-browser-state.ts b/packages/vinext/src/server/app-browser-state.ts index 69faf7776..75a9a23f7 100644 --- a/packages/vinext/src/server/app-browser-state.ts +++ b/packages/vinext/src/server/app-browser-state.ts @@ -277,14 +277,20 @@ export function shouldHardNavigate( export function resolvePendingNavigationCommitDisposition(options: { activeNavigationId: number; + currentVisibleCommitVersion: number; currentRootLayoutTreePath: string | null; nextRootLayoutTreePath: string | null; startedNavigationId: number; + startedVisibleCommitVersion: number; }): PendingNavigationCommitDisposition { if (options.startedNavigationId !== options.activeNavigationId) { return "skip"; } + if (options.startedVisibleCommitVersion !== options.currentVisibleCommitVersion) { + return "skip"; + } + if (shouldHardNavigate(options.currentRootLayoutTreePath, options.nextRootLayoutTreePath)) { return "hard-navigate"; } @@ -294,16 +300,20 @@ export function resolvePendingNavigationCommitDisposition(options: { export function resolvePendingNavigationCommitDispositionDecision(options: { activeNavigationId: number; + currentVisibleCommitVersion: number; currentRootLayoutTreePath: string | null; nextRootLayoutTreePath: string | null; startedNavigationId: number; + startedVisibleCommitVersion: number; }): PendingNavigationCommitDispositionDecision { const disposition = resolvePendingNavigationCommitDisposition(options); const traceFields = { activeNavigationId: options.activeNavigationId, + currentVisibleCommitVersion: options.currentVisibleCommitVersion, currentRootLayoutTreePath: options.currentRootLayoutTreePath, nextRootLayoutTreePath: options.nextRootLayoutTreePath, startedNavigationId: options.startedNavigationId, + startedVisibleCommitVersion: options.startedVisibleCommitVersion, }; return { diff --git a/packages/vinext/src/server/app-browser-visible-commit.ts b/packages/vinext/src/server/app-browser-visible-commit.ts index 2668ddf15..02f4ad61d 100644 --- a/packages/vinext/src/server/app-browser-visible-commit.ts +++ b/packages/vinext/src/server/app-browser-visible-commit.ts @@ -63,9 +63,11 @@ export function applyApprovedVisibleCommit( function resolvePendingNavigationCommitDecision(options: { activeNavigationId: number; + currentVisibleCommitVersion: number; currentRootLayoutTreePath: string | null; nextRootLayoutTreePath: string | null; startedNavigationId: number; + startedVisibleCommitVersion: number; }): CommitDecision { const { disposition, trace } = resolvePendingNavigationCommitDispositionDecision(options); @@ -123,9 +125,11 @@ export function approvePendingNavigationCommit(options: { }): CommitApproval { const decision = resolvePendingNavigationCommitDecision({ activeNavigationId: options.activeNavigationId, + currentVisibleCommitVersion: options.currentState.visibleCommitVersion, currentRootLayoutTreePath: options.currentState.rootLayoutTreePath, nextRootLayoutTreePath: options.pending.rootLayoutTreePath, startedNavigationId: options.startedNavigationId, + startedVisibleCommitVersion: options.pending.action.operation.startedVisibleCommitVersion, }); switch (decision.disposition) { @@ -155,6 +159,7 @@ export async function resolveAndClassifyNavigationCommit(options: { currentState: AppRouterState; navigationSnapshot: ClientNavigationRenderSnapshot; nextElements: Promise; + getCurrentStateForApproval?: () => AppRouterState; operationLane: OperationLane; previousNextUrl?: string | null; renderId: number; @@ -171,9 +176,10 @@ export async function resolveAndClassifyNavigationCommit(options: { type: options.type, }); + const approvalState = options.getCurrentStateForApproval?.() ?? options.currentState; const approval = approvePendingNavigationCommit({ activeNavigationId: options.activeNavigationId, - currentState: options.currentState, + currentState: approvalState, pending, startedNavigationId: options.startedNavigationId, }); diff --git a/tests/app-browser-entry.test.ts b/tests/app-browser-entry.test.ts index 0db3ceb88..7a38ef0a6 100644 --- a/tests/app-browser-entry.test.ts +++ b/tests/app-browser-entry.test.ts @@ -300,9 +300,11 @@ describe("app browser entry state helpers", () => { expect( resolvePendingNavigationCommitDisposition({ activeNavigationId: 3, + currentVisibleCommitVersion: currentState.visibleCommitVersion, currentRootLayoutTreePath: currentState.rootLayoutTreePath, nextRootLayoutTreePath: pending.rootLayoutTreePath, startedNavigationId: 3, + startedVisibleCommitVersion: pending.action.operation.startedVisibleCommitVersion, }), ).toBe("hard-navigate"); }); @@ -391,9 +393,11 @@ describe("app browser entry state helpers", () => { expect( resolvePendingNavigationCommitDisposition({ activeNavigationId: 5, + currentVisibleCommitVersion: currentState.visibleCommitVersion, currentRootLayoutTreePath: currentState.rootLayoutTreePath, nextRootLayoutTreePath: pending.rootLayoutTreePath, startedNavigationId: 4, + startedVisibleCommitVersion: pending.action.operation.startedVisibleCommitVersion, }), ).toBe("skip"); }); @@ -401,9 +405,11 @@ describe("app browser entry state helpers", () => { it("traces stale pending commits with compact reason codes and structured fields", () => { const decision = resolvePendingNavigationCommitDispositionDecision({ activeNavigationId: 5, + currentVisibleCommitVersion: 0, currentRootLayoutTreePath: "/", nextRootLayoutTreePath: "/(dashboard)", startedNavigationId: 4, + startedVisibleCommitVersion: 0, }); expect(decision.disposition).toBe("skip"); @@ -414,21 +420,39 @@ describe("app browser entry state helpers", () => { code: NavigationTraceReasonCodes.staleOperation, fields: { activeNavigationId: 5, + currentVisibleCommitVersion: 0, currentRootLayoutTreePath: "/", nextRootLayoutTreePath: "/(dashboard)", startedNavigationId: 4, + startedVisibleCommitVersion: 0, }, }, ], }); }); + it("treats a visible commit version mismatch as stale before root-boundary decisions", () => { + const decision = resolvePendingNavigationCommitDispositionDecision({ + activeNavigationId: 2, + currentVisibleCommitVersion: 1, + currentRootLayoutTreePath: "/(marketing)", + nextRootLayoutTreePath: "/(dashboard)", + startedNavigationId: 2, + startedVisibleCommitVersion: 0, + }); + + expect(decision.disposition).toBe("skip"); + expect(decision.trace.entries[0]?.code).toBe(NavigationTraceReasonCodes.staleOperation); + }); + it("traces root-boundary hard navigation decisions", () => { const decision = resolvePendingNavigationCommitDispositionDecision({ activeNavigationId: 2, + currentVisibleCommitVersion: 0, currentRootLayoutTreePath: "/(marketing)", nextRootLayoutTreePath: "/(dashboard)", startedNavigationId: 2, + startedVisibleCommitVersion: 0, }); expect(decision.disposition).toBe("hard-navigate"); @@ -437,9 +461,11 @@ describe("app browser entry state helpers", () => { code: NavigationTraceReasonCodes.rootBoundaryChanged, fields: { activeNavigationId: 2, + currentVisibleCommitVersion: 0, currentRootLayoutTreePath: "/(marketing)", nextRootLayoutTreePath: "/(dashboard)", startedNavigationId: 2, + startedVisibleCommitVersion: 0, }, }, ]); @@ -448,9 +474,11 @@ describe("app browser entry state helpers", () => { it("traces unknown root-layout identity as a legacy soft-commit fallback", () => { const decision = resolvePendingNavigationCommitDispositionDecision({ activeNavigationId: 2, + currentVisibleCommitVersion: 0, currentRootLayoutTreePath: "/", nextRootLayoutTreePath: null, startedNavigationId: 2, + startedVisibleCommitVersion: 0, }); expect(decision.disposition).toBe("dispatch"); @@ -460,9 +488,11 @@ describe("app browser entry state helpers", () => { it("traces matching root-layout dispatches as current commits", () => { const decision = resolvePendingNavigationCommitDispositionDecision({ activeNavigationId: 2, + currentVisibleCommitVersion: 0, currentRootLayoutTreePath: "/", nextRootLayoutTreePath: "/", startedNavigationId: 2, + startedVisibleCommitVersion: 0, }); expect(decision.disposition).toBe("dispatch"); @@ -471,9 +501,11 @@ describe("app browser entry state helpers", () => { code: NavigationTraceReasonCodes.commitCurrent, fields: { activeNavigationId: 2, + currentVisibleCommitVersion: 0, currentRootLayoutTreePath: "/", nextRootLayoutTreePath: "/", startedNavigationId: 2, + startedVisibleCommitVersion: 0, }, }, ]); @@ -1020,6 +1052,69 @@ describe("app browser navigation controller", () => { } }); + it("does not let older same-URL server action payloads overwrite newer visible commits", async () => { + const initialState = createState({ + rootLayoutTreePath: "/", + routeId: "route:/settings", + navigationSnapshot: createClientNavigationRenderSnapshot("https://example.com/settings", { + tab: "profile", + }), + }); + const { controller, detach, stateRef } = createControllerHarness(initialState); + const { assign } = stubWindow("https://example.com/settings"); + let resolveOlderPayload!: (elements: AppElements) => void; + const olderPayload = new Promise((resolve) => { + resolveOlderPayload = resolve; + }); + + try { + const olderResult = controller.commitSameUrlNavigatePayload( + olderPayload, + stateRef.current.navigationSnapshot, + { + data: "older-action-result", + ok: true, + }, + ); + + const newerResult = await controller.commitSameUrlNavigatePayload( + Promise.resolve( + createResolvedElements("route:/settings/newer", "/", null, { + "page:/settings/newer": React.createElement("main", null, "newer"), + }), + ), + stateRef.current.navigationSnapshot, + { + data: "newer-action-result", + ok: true, + }, + ); + + expect(newerResult).toBe("newer-action-result"); + expect(stateRef.current.routeId).toBe("route:/settings/newer"); + expect(stateRef.current.visibleCommitVersion).toBe(1); + + resolveOlderPayload( + createResolvedElements("route:/settings/older", "/", null, { + "page:/settings/older": React.createElement("main", null, "older"), + }), + ); + + await expect(olderResult).resolves.toBe("older-action-result"); + expect(assign).not.toHaveBeenCalled(); + expect(stateRef.current.routeId).toBe("route:/settings/newer"); + expect(stateRef.current.visibleCommitVersion).toBe(1); + expect(stateRef.current.activeOperation).toMatchObject({ + lane: "server-action", + startedVisibleCommitVersion: 0, + state: "committed", + visibleCommitVersion: 1, + }); + } finally { + detach(); + } + }); + it("hard-navigates same-URL server action payloads when the root layout changes", async () => { const initialState = createState({ rootLayoutTreePath: "/(marketing)", From 7b84a431f309f1a3064ced5dfd44c5e49d7eef24 Mon Sep 17 00:00:00 2001 From: Nathan Nguyen <146415969+NathanDrake2406@users.noreply.github.com> Date: Wed, 6 May 2026 19:47:31 +1000 Subject: [PATCH 2/2] fix(router): preserve server action initiation version Server action POST handling captured the router state before fetch but the same-URL commit helper previously rebased the pending operation when the response was decoded. If a newer visible commit landed while the POST was in flight, the old action could still look current. Thread the initiation state through same-URL action commit handling so startedVisibleCommitVersion reflects the world that submitted the action. Approval still compares against the live router state after payload decoding, so stale results keep their action value without mutating visible state. Add a hostile race test where the action response is processed after a newer same-URL visible commit. --- .../vinext/src/server/app-browser-entry.ts | 9 +++- .../app-browser-navigation-controller.ts | 4 +- tests/app-browser-entry.test.ts | 53 +++++++++++++++++++ 3 files changed, 64 insertions(+), 2 deletions(-) diff --git a/packages/vinext/src/server/app-browser-entry.ts b/packages/vinext/src/server/app-browser-entry.ts index bbcf9eaa9..15b5d0aa7 100644 --- a/packages/vinext/src/server/app-browser-entry.ts +++ b/packages/vinext/src/server/app-browser-entry.ts @@ -291,6 +291,7 @@ async function renderNavigationPayload( async function commitSameUrlNavigatePayload( nextElements: Promise, returnValue?: ServerActionResult["returnValue"], + actionInitiationState?: AppRouterState, ): Promise { const navigationSnapshot = createClientNavigationRenderSnapshot( window.location.href, @@ -300,6 +301,7 @@ async function commitSameUrlNavigatePayload( nextElements, navigationSnapshot, returnValue, + actionInitiationState, ); } @@ -853,10 +855,15 @@ function registerServerActionCallback(): void { return commitSameUrlNavigatePayload( Promise.resolve(AppElementsWire.decode(result.root)), result.returnValue, + currentState, ); } - return commitSameUrlNavigatePayload(Promise.resolve(AppElementsWire.decode(result))); + return commitSameUrlNavigatePayload( + Promise.resolve(AppElementsWire.decode(result)), + undefined, + currentState, + ); }); } diff --git a/packages/vinext/src/server/app-browser-navigation-controller.ts b/packages/vinext/src/server/app-browser-navigation-controller.ts index a82916bde..b6b93cdfb 100644 --- a/packages/vinext/src/server/app-browser-navigation-controller.ts +++ b/packages/vinext/src/server/app-browser-navigation-controller.ts @@ -73,6 +73,7 @@ type BrowserNavigationController = { nextElements: Promise, navigationSnapshot: ClientNavigationRenderSnapshot, returnValue?: { ok: boolean; data: unknown }, + actionInitiationState?: AppRouterState, ): Promise; hmrReplaceTree( nextElements: Promise, @@ -476,8 +477,9 @@ export function createAppBrowserNavigationController( nextElements: Promise, navigationSnapshot: ClientNavigationRenderSnapshot, returnValue?: { ok: boolean; data: unknown }, + actionInitiationState?: AppRouterState, ): Promise { - const currentState = getBrowserRouterState(); + const currentState = actionInitiationState ?? getBrowserRouterState(); const startedNavigationId = activeNavigationId; const { approvedCommit, diff --git a/tests/app-browser-entry.test.ts b/tests/app-browser-entry.test.ts index 7a38ef0a6..7525dbad2 100644 --- a/tests/app-browser-entry.test.ts +++ b/tests/app-browser-entry.test.ts @@ -1115,6 +1115,59 @@ describe("app browser navigation controller", () => { } }); + it("uses the server-action initiation state when the response is processed after a newer commit", async () => { + const initialState = createState({ + rootLayoutTreePath: "/", + routeId: "route:/settings", + navigationSnapshot: createClientNavigationRenderSnapshot("https://example.com/settings", { + tab: "profile", + }), + }); + const { controller, detach, stateRef } = createControllerHarness(initialState); + const { assign } = stubWindow("https://example.com/settings"); + const actionInitiationState = stateRef.current; + + try { + const newerResult = await controller.commitSameUrlNavigatePayload( + Promise.resolve( + createResolvedElements("route:/settings/newer", "/", null, { + "page:/settings/newer": React.createElement("main", null, "newer"), + }), + ), + stateRef.current.navigationSnapshot, + { + data: "newer-action-result", + ok: true, + }, + ); + + expect(newerResult).toBe("newer-action-result"); + expect(stateRef.current.routeId).toBe("route:/settings/newer"); + expect(stateRef.current.visibleCommitVersion).toBe(1); + + const olderResult = await controller.commitSameUrlNavigatePayload( + Promise.resolve( + createResolvedElements("route:/settings/older", "/", null, { + "page:/settings/older": React.createElement("main", null, "older"), + }), + ), + actionInitiationState.navigationSnapshot, + { + data: "older-action-result", + ok: true, + }, + actionInitiationState, + ); + + expect(olderResult).toBe("older-action-result"); + expect(assign).not.toHaveBeenCalled(); + expect(stateRef.current.routeId).toBe("route:/settings/newer"); + expect(stateRef.current.visibleCommitVersion).toBe(1); + } finally { + detach(); + } + }); + it("hard-navigates same-URL server action payloads when the root layout changes", async () => { const initialState = createState({ rootLayoutTreePath: "/(marketing)",