fix(app-router): preserve forwarded action redirect wrappers#1582
Open
NathanDrake2406 wants to merge 4 commits into
Open
fix(app-router): preserve forwarded action redirect wrappers#1582NathanDrake2406 wants to merge 4 commits into
NathanDrake2406 wants to merge 4 commits into
Conversation
Server action redirects could fall back to hard HTTP redirects when the action POST represented stale visible-route state or a cross-runtime worker hop. That diverges from Next.js, where forwarded action redirects return an RSC wrapper response that the client can apply without treating the action as a failed network redirect. The action pipeline now renders same-origin redirect targets into the POST response, preserves action cookies for the target render, and uses the wrapper status for forwarded, stale child-route, and cross-runtime redirect shapes. Browser action handling now posts to the visible route with the public next-action header and only runs RSC compatibility checks for real Flight responses. Regression coverage ports the relevant Next.js action cases across helper tests and app-router E2E fixtures.
commit: |
Forwarded non-redirect actions could skip page rendering without preserving Set-Cookie or x-action-revalidated headers, and same-origin redirect rendering treated any App route match as a page render target. Redirect Flight responses also bypassed the RSC compatibility guard. Gate inline redirect rendering to actual page routes, propagate forwarded action side-effect headers on the no-rerender path, and hard-navigate redirect responses when their Flight compatibility ID does not match the client. Text/plain action failures now accept charset-qualified content types. Regression coverage exercises route-handler redirect fallback, forwarded cookie and revalidation propagation, redirect Flight compatibility fallback, and text/plain;charset=utf-8 action errors.
The zizmor online impostor-commit audit failed before auditing because the job token had no contents read scope. The audit needs to query GitHub tag and commit metadata for referenced actions such as voidzero-dev/setup-vp. Grant contents: read to the zizmor job while keeping checkout credentials unpersisted and security-events scoped to the existing upload path.
Contributor
There was a problem hiding this comment.
Pull request overview
Aligns Vinext App Router server-action behavior with Next.js for redirecting actions, especially when the visible route is stale or the action is effectively “forwarded”, by ensuring the client receives a consumable Flight wrapper (and correct wrapper-vs-redirect status semantics) and by improving client-side error/redirect handling.
Changes:
- Server: render same-origin redirect targets into the action POST Flight response when possible, preserve cookies/revalidation headers, and select wrapper status semantics for forwarded/stale/cross-runtime redirects.
- Browser: keep action POSTs on the visible route URL, send the public
next-actionheader, improve handling of non-RSC action error responses, and hard-navigate when redirect Flight is incompatible. - Routing/metadata/tests: thread segment
runtimeinto route config, refine root boundary derivation in the generated manifest, and add/extend unit + e2e fixtures/tests for the new Next.js-compat scenarios.
Reviewed changes
Copilot reviewed 36 out of 36 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/fixtures/app-basic/app/nextjs-compat/action-response-semantics/page.tsx | Adds compat fixture page for action response semantics. |
| tests/fixtures/app-basic/app/nextjs-compat/action-response-semantics/not-found.tsx | Adds compat not-found boundary for action notFound assertions. |
| tests/fixtures/app-basic/app/nextjs-compat/action-response-semantics/error.tsx | Adds compat error boundary for surfaced action errors. |
| tests/fixtures/app-basic/app/nextjs-compat/action-response-semantics/client.tsx | Client UI to trigger complete/redirect/notFound actions. |
| tests/fixtures/app-basic/app/nextjs-compat/action-response-semantics/actions.ts | Server actions to drive semantics tests (return/redirect/notFound). |
| tests/fixtures/app-basic/app/nextjs-compat/action-redirect-cookies/target/page.tsx | Target page asserts cookies merged and next-action stripped. |
| tests/fixtures/app-basic/app/nextjs-compat/action-redirect-cookies/page.tsx | Source page action mutates cookies then redirects. |
| tests/fixtures/app-basic/app/nextjs-compat/action-forwarded-redirect/page.tsx | Fixture page for forwarded redirect scenario. |
| tests/fixtures/app-basic/app/nextjs-compat/action-forwarded-redirect/other/page.tsx | Fixture “other” page for stale child-route redirect test. |
| tests/fixtures/app-basic/app/nextjs-compat/action-forwarded-redirect/node/page.tsx | Fixture node target page for cross-runtime redirect test. |
| tests/fixtures/app-basic/app/nextjs-compat/action-forwarded-redirect/edge/page.tsx | Fixture edge page using cross-runtime client. |
| tests/fixtures/app-basic/app/nextjs-compat/action-forwarded-redirect/edge/other/page.tsx | Fixture edge “other” page for cross-runtime stale route test. |
| tests/fixtures/app-basic/app/nextjs-compat/action-forwarded-redirect/edge/layout.tsx | Sets runtime = "edge" for cross-runtime route metadata. |
| tests/fixtures/app-basic/app/nextjs-compat/action-forwarded-redirect/cross-runtime-client.tsx | Client triggers delayed cross-runtime redirect action. |
| tests/fixtures/app-basic/app/nextjs-compat/action-forwarded-redirect/client.tsx | Client triggers delayed forwarded redirect action. |
| tests/fixtures/app-basic/app/nextjs-compat/action-forwarded-redirect/actions.ts | Redirecting server actions for forwarded/cross-runtime cases. |
| tests/fixtures/app-basic/app/action-redirect-test/redirect-form.tsx | Updates redirect trigger from form submit to button click transition. |
| tests/entry-templates.test.ts | Adds regression test for root boundary derivation without root page. |
| tests/e2e/app-router/server-actions.spec.ts | Adds e2e coverage for action visible-route POSTs, redirects, stale/cross-runtime wrapper semantics, and invalid responses. |
| tests/e2e/app-router/nextjs-compat/actions-revalidate.spec.ts | Adjusts POST URL matching for visible-route action requests. |
| tests/app-server-action-execution.test.ts | Extends unit coverage for redirect rendering, cookie replay, forwarded wrapper semantics, and redirected Flight behavior. |
| tests/app-segment-config.test.ts | Adds tests for segment runtime resolution/unknown runtime ignoring. |
| tests/app-rsc-cache-busting.test.ts | Adds test ensuring server action POST URLs stay on visible route (strip hash). |
| tests/app-route-handler-response.test.ts | Adds test for returned Set-Cookie normalization when overriding fallbacks. |
| tests/app-browser-entry.test.ts | Adds unit tests for new action response parsing/compatibility/error behavior and next-action header inclusion. |
| packages/vinext/src/shims/error-boundary.tsx | Formatting-only state object changes. |
| packages/vinext/src/server/app-server-action-execution.ts | Core server-side redirect rendering, wrapper status selection, cookie replay into redirected GET render, soft tag threading, forwarded rerender suppression changes. |
| packages/vinext/src/server/app-segment-config.ts | Parses and threads supported runtime values into effective segment config. |
| packages/vinext/src/server/app-rsc-cache-busting.ts | Adds createServerActionRequestUrl() helper to keep action POSTs on visible route URL. |
| packages/vinext/src/server/app-route-handler-response.ts | Normalizes returned Set-Cookie values by ensuring Path=/ when missing. |
| packages/vinext/src/server/app-browser-state.ts | Adds public next-action header to action request state. |
| packages/vinext/src/server/app-browser-entry.ts | Updates browser action dispatch to visible-route URLs, improves redirect Flight handling, non-RSC error surfacing, compatibility hard-nav logic, and 404 head metadata syncing. |
| packages/vinext/src/server/app-browser-action-result.ts | Adds helpers for non-RSC action error extraction, compatibility checks, and HTTP fallback normalization. |
| packages/vinext/src/entries/app-rsc-manifest.ts | Improves root boundary (notFound/forbidden/unauthorized/layout) derivation when no root page exists. |
| packages/vinext/src/entries/app-rsc-entry.ts | Threads route runtime resolution into server action execution options. |
| .github/workflows/zizmor.yml | Adds contents: read permission for zizmor workflow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Author
|
@codex review |
Forwarded action redirects treated a missing segment runtime as incomparable instead of the implicit Node runtime. That let implicit Node to explicit Edge redirects use 303 semantics even though they cross runtime boundaries and need the wrapper response path. Server action dispatch also left action-owned HTTP fallback head metadata in place until a response reached the status sync path. Invalid non-RSC responses could throw first and leave stale noindex metadata behind. Normalize null route runtimes to nodejs at the comparison boundary, clear action fallback head metadata before each action fetch, and cover both regressions. The redirect render cookie replay now reuses the existing request cookie parser instead of carrying a local parser clone.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12f041d28a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1481
Overview
app-server-action-execution.ts,app-browser-entry.ts,app-segment-config.ts,server-actions.spec.tsWhy
For App Router actions, the POST URL is the visible route, while
next-actionidentifies the server reference. If the visible route is stale or belongs to a different action worker, Next.js forwards the action and still returns a client-consumable Flight wrapper for redirecting actions. Vinext has one deployed worker, but the client observable contract is the same: the action response must carry redirect headers plus a valid Flight body when the action redirect can be rendered internally.next-actionheader and keeps action POST URLs on the visible route.What Changed
route.tstarget could be treated as a page render target.Set-Cookieandx-action-revalidated.Maintainer review path
packages/vinext/src/server/app-server-action-execution.tsfor action redirect target rendering, cookie replay into the redirected GET render, and wrapper status selection.packages/vinext/src/server/app-browser-entry.tsandapp-browser-action-result.tsfor visible-route action POSTs, non-RSC error handling, and redirect Flight consumption.packages/vinext/src/server/app-segment-config.tsplusentries/app-rsc-entry.tsfor runtime metadata threading.tests/app-server-action-execution.test.tsandtests/e2e/app-router/server-actions.spec.tsfor the low-boundary and browser-level regression coverage.Validation
vp env exec --node 24 ./scripts/run-nextjs-deploy-suite.sh /Users/nathan/Projects/vinext/.refs/nextjs-v16.2.6 --retries 0 -c 1 --debug test/e2e/app-dir/actions/app-action-node-middleware.test.tsvp test run tests/app-server-action-execution.test.ts tests/app-browser-entry.test.ts tests/app-segment-config.test.ts tests/app-route-handler-response.test.ts tests/app-rsc-cache-busting.test.ts tests/entry-templates.test.tsCI=1 PLAYWRIGHT_PROJECT=app-router npx playwright test tests/e2e/app-router/server-actions.spec.ts -g "server action redirects|stale child-route|cross-runtime|server action notFound|text/plain|invalid action response" --retries=0vp checkknip --no-progress; the first commit hook also rerantests/entry-templates.test.ts.Risk and compatibility
runtimeis parsed only from supported Next.js values.Non-goals
References