[PR #1090] [MERGED] refactor(app-router): gate browser state writes with approved visible commits #1086

New issue

Closed

opened 2026-05-06 13:11:54 +02:00 by BreizhHardware · 0 comments

BreizhHardware commented

2026-05-06 13:11:54 +02:00

Owner

📋 Pull Request Information

Original PR: https://github.com/cloudflare/vinext/pull/1090
Author: @NathanDrake2406
Created: 5/6/2026
Status: ✅ Merged
Merged: 5/6/2026
Merged by: @james-elicx

Base: main ← Head: nathan/726-core-02-approved-visible-commit

📝 Commits (5)

673d157 refactor(app-router): gate browser state writes with approved visible commits
6aa9dbd refactor(app-router): close visible commit approval escape hatch
ad26049 ci: rerun checks
19a388a refactor(app-router): isolate visible commit approval authority
8e2fb7a refactor(app-router): remove legacy commit disposition mirror

📊 Changes

4 files changed (+430 additions, -130 deletions)

View changed files

📝 packages/vinext/src/server/app-browser-navigation-controller.ts (+35 -85)
📝 packages/vinext/src/server/app-browser-state.ts (+1 -41)
➕ packages/vinext/src/server/app-browser-visible-commit.ts (+187 -0)
📝 tests/app-browser-entry.test.ts (+207 -4)

📄 Description

What this changes

Implements #726-CORE-02/03 from #726.

This introduces CommitDecision and ApprovedVisibleCommit as the explicit proof object for visible App Router browser commits. Navigation and same-URL server-action state writes now flow through lifecycle approval before visible browser state can be reduced, and HMR uses a named recovery approval path instead of generic proof construction.

Why

Issue #726 is moving the browser navigation lifecycle toward explicit commit authority. The current code already had operation records and visibleCommitVersion, but each browser writer still held its own implicit approval check before calling the reducer. That made future stale-result gates and planner work easier to bypass accidentally.

This PR is the intended thin spine step: no proof, no visible commit transaction.

Approach

Add app-browser-visible-commit.ts as the visible browser mutation authority module.
Keep app-browser-state.ts focused on state shape, reducer mechanics, pending commit construction, history helpers, server-action request headers, and legacy disposition tracing.
Add a commit decision layer that maps the existing dispatch, skip, and hard-navigate semantics to commit, no-commit, and hard-navigate decisions.
Add ApprovedVisibleCommit as a nominal commit proof and route visible state reduction through applyApprovedVisibleCommit.
Keep the generic proof constructors private inside app-browser-visible-commit.ts.
Expose approvePendingNavigationCommit() as the navigation lifecycle gate.
Expose approveHmrVisibleCommit() as the named trusted HMR recovery path, with a lane guard so non-HMR pending commits cannot use it as a generic approval escape hatch.
Replace the controller's raw dispatchBrowserTree path with dispatchApprovedVisibleCommit for navigation, same-URL action payloads, and HMR.
Have same-URL action payload handling read the authoritative CommitDecision directly, with the legacy disposition shape kept private to the older state-level classifier.
Preserve current behavior.

Non-goals for this PR, matching #726-CORE-02/03:

stale same-URL server-action rejection
NavigationPlanner
cache semantics

Validation

vp check packages/vinext/src/server/app-browser-state.ts packages/vinext/src/server/app-browser-visible-commit.ts packages/vinext/src/server/app-browser-navigation-controller.ts tests/app-browser-entry.test.ts
vp test run tests/app-browser-entry.test.ts tests/app-route-graph.test.ts
vp run vinext#build
Commit hook ran vp check --fix and knip --no-progress

Bonk

Bonk, please read issue #726 before reviewing so the ApprovedVisibleCommit boundary is evaluated in the larger lifecycle and planner roadmap context, not as an isolated refactor.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/cloudflare/vinext/pull/1090 **Author:** [@NathanDrake2406](https://github.com/NathanDrake2406) **Created:** 5/6/2026 **Status:** ✅ Merged **Merged:** 5/6/2026 **Merged by:** [@james-elicx](https://github.com/james-elicx) **Base:** `main` ← **Head:** `nathan/726-core-02-approved-visible-commit` --- ### 📝 Commits (5) - [`673d157`](https://github.com/cloudflare/vinext/commit/673d1576e031eb6099eab65963aabc0d2b7d2744) refactor(app-router): gate browser state writes with approved visible commits - [`6aa9dbd`](https://github.com/cloudflare/vinext/commit/6aa9dbdfce46513573cdba092d111a6c49fe5441) refactor(app-router): close visible commit approval escape hatch - [`ad26049`](https://github.com/cloudflare/vinext/commit/ad2604924cc0a2788e5d974213e10079d719ff58) ci: rerun checks - [`19a388a`](https://github.com/cloudflare/vinext/commit/19a388aeaee4782ca06b7b1d8b807b4935a4a4a4) refactor(app-router): isolate visible commit approval authority - [`8e2fb7a`](https://github.com/cloudflare/vinext/commit/8e2fb7ad11fd62cbcc6b2dc5c5767844b283b09c) refactor(app-router): remove legacy commit disposition mirror ### 📊 Changes **4 files changed** (+430 additions, -130 deletions) <details> <summary>View changed files</summary> 📝 `packages/vinext/src/server/app-browser-navigation-controller.ts` (+35 -85) 📝 `packages/vinext/src/server/app-browser-state.ts` (+1 -41) ➕ `packages/vinext/src/server/app-browser-visible-commit.ts` (+187 -0) 📝 `tests/app-browser-entry.test.ts` (+207 -4) </details> ### 📄 Description ## What this changes Implements `#726-CORE-02/03` from #726. This introduces `CommitDecision` and `ApprovedVisibleCommit` as the explicit proof object for visible App Router browser commits. Navigation and same-URL server-action state writes now flow through lifecycle approval before visible browser state can be reduced, and HMR uses a named recovery approval path instead of generic proof construction. ## Why Issue #726 is moving the browser navigation lifecycle toward explicit commit authority. The current code already had operation records and `visibleCommitVersion`, but each browser writer still held its own implicit approval check before calling the reducer. That made future stale-result gates and planner work easier to bypass accidentally. This PR is the intended thin spine step: no proof, no visible commit transaction. ## Approach - Add `app-browser-visible-commit.ts` as the visible browser mutation authority module. - Keep `app-browser-state.ts` focused on state shape, reducer mechanics, pending commit construction, history helpers, server-action request headers, and legacy disposition tracing. - Add a commit decision layer that maps the existing `dispatch`, `skip`, and `hard-navigate` semantics to `commit`, `no-commit`, and `hard-navigate` decisions. - Add `ApprovedVisibleCommit` as a nominal commit proof and route visible state reduction through `applyApprovedVisibleCommit`. - Keep the generic proof constructors private inside `app-browser-visible-commit.ts`. - Expose `approvePendingNavigationCommit()` as the navigation lifecycle gate. - Expose `approveHmrVisibleCommit()` as the named trusted HMR recovery path, with a lane guard so non-HMR pending commits cannot use it as a generic approval escape hatch. - Replace the controller's raw `dispatchBrowserTree` path with `dispatchApprovedVisibleCommit` for navigation, same-URL action payloads, and HMR. - Have same-URL action payload handling read the authoritative `CommitDecision` directly, with the legacy disposition shape kept private to the older state-level classifier. - Preserve current behavior. Non-goals for this PR, matching `#726-CORE-02/03`: - stale same-URL server-action rejection - `NavigationPlanner` - cache semantics ## Validation - `vp check packages/vinext/src/server/app-browser-state.ts packages/vinext/src/server/app-browser-visible-commit.ts packages/vinext/src/server/app-browser-navigation-controller.ts tests/app-browser-entry.test.ts` - `vp test run tests/app-browser-entry.test.ts tests/app-route-graph.test.ts` - `vp run vinext#build` - Commit hook ran `vp check --fix` and `knip --no-progress` ## Bonk Bonk, please read issue #726 before reviewing so the `ApprovedVisibleCommit` boundary is evaluated in the larger lifecycle and planner roadmap context, not as an isolated refactor. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>