fix(pages): re-enter filesystem routes after rewrites

[james-elicx]

locale: false public rewrite parity investigation

Outcome

Confirmed and fixed a real vinext parity gap from Actions run 27514800656.

Next.js re-enters filesystem routing after config rewrites. Vinext applied the
rewrite destination to page/API dispatch, but only probed public and built
static files before rewrites. Consequently, a locale: false rewrite from a
localized source to /file.txt or /_next/static/... returned 404.

Baseline

• Vinext base: a3d2f921520ff140a826224616df5e0db4ed0186
• Next.js reference tag: v16.2.6
• Next.js source/build commit: ee6e79b1792a4d401ddf2480f40a83549fe8e722
• Worktree: /tmp/vinext-locale-false-public.MmAbBc/vinext
• Branch: codex/fix-locale-false-public-rewrites

Exact upstream assertions

Suite:
test/e2e/i18n-ignore-rewrite-source-locale/rewrites.test.ts

The source config uses a beforeFiles rewrite:

{
  source: '/:locale/rewrite-files/:path*',
  destination: '/:path*',
  locale: false,
}

The deploy run 27514800656 failed the public-file assertion for /, /nl,
and /sv, returning Next 404 HTML instead of hello from file.txt. The suite's
API rewrite was the control and showed that rewrite matching itself worked.

Local reproduction on unmodified main

Added a vinext Pages fixture with locales en, sv, and nl, a public text
file, a page, an API route, and rewrite destinations for public, built-static,
page, and API routes.

Targeted command:

vp test run tests/pages-i18n-public-rewrite.test.ts

Baseline result: 5/5 failed. Both default-locale (/en) and non-default-locale
(/sv) public rewrites returned 404 in development and production. The API and
page controls passed, while the rewritten built-static destination also
returned 404.

Fix

• Replaced the Node-only direct static callback with a runtime-supplied
  filesystem-route callback.
• Re-entered filesystem resolution after successful beforeFiles,
  afterFiles, and fallback rewrites, matching Next.js routing order.
• Preserved direct public/build-static precedence before afterFiles and
  fallback rewrites.
• Re-dispatched API routes after rewritten filesystem misses.
• Used Vite's serving pipeline in development so rewritten files preserve HEAD,
  range, MIME, ETag, and conditional-request behavior.
• Used the production static-file cache in the Node production server.
• Added Worker generated-entry support using env.ASSETS.fetch, only for
  rewritten GET/HEAD non-API destinations.

The fix is not specific to locale parsing. locale: false matching already
produced the correct destination; the missing behavior was post-rewrite
filesystem re-entry.

Independent review follow-up: dev dot-path preflight

Independent review found a separate dev-only gap in the early static-asset
preflight. For file-looking paths such as /download.txt, dev decided whether
to enter the Pages pipeline by fully evaluating rewrite has / missing
conditions against the original request. That skipped middleware entirely when
middleware was responsible for adding the header that enabled the rewrite.

The preflight now checks only whether a rewrite's source pattern can match the
pathname. Request-dependent conditions remain deferred to the Pages pipeline,
after middleware request-header overrides have been applied. Paths that do not
match any rewrite source still bypass Pages handling, so unrelated static
assets continue through Vite's normal filesystem middleware.

Focused dev coverage verifies:

• /download.txt?rewrite reaches middleware, receives an injected request
  header, and rewrites to a Pages route.
• /download.txt remains unmatched when middleware does not inject the header.
• An unrelated real public asset remains served by Vite.

Node production and Worker routing do not have the dev-only file-extension
preflight. Both already enter runPagesRequest, run middleware, apply request
header overrides, and evaluate config rewrites using the post-middleware
request context, so no runtime changes were required there.

Targeted validation

vp test run \
  tests/pages-i18n-public-rewrite.test.ts \
  tests/pages-request-pipeline.test.ts \
  tests/deploy.test.ts

Result: 349 tests passed.

Coverage includes default/non-default locales, dev/prod public files,
built-static assets, API/page controls, beforeFiles/afterFiles/fallback
ordering, Worker asset fetch generation, HEAD/range/ETag behavior, and misses.

vp check \
  packages/vinext/src/deploy.ts \
  packages/vinext/src/index.ts \
  packages/vinext/src/server/pages-request-pipeline.ts \
  packages/vinext/src/server/prod-server.ts \
  packages/vinext/src/server/static-file-cache.ts \
  tests/deploy.test.ts \
  tests/pages-i18n-public-rewrite.test.ts \
  tests/pages-request-pipeline.test.ts

Result: formatting, lint, and type checks passed. git diff --check passed.

Independent review follow-up validation:

vp test run tests/pages-router.test.ts \
  -t "Pages Router dev dot-path rewrite preflight"

vp check \
  packages/vinext/src/config/config-matchers.ts \
  packages/vinext/src/index.ts \
  tests/pages-router.test.ts

Result: 3 focused integration tests passed; scoped formatting, lint, and type
checks passed; git diff --check passed. The upstream deploy suite was not
rerun for this dev-only follow-up.

Exact pinned Next.js deploy validation

NEXTJS_PREPARE=0 \
NEXT_TEST_CONCURRENCY=1 \
vp env exec --node 24 \
  ./scripts/run-nextjs-deploy-suite.sh \
  /Users/jamesanderson/Developer/vinext/.nextjs-ref \
  --retries 0 -c 1 --debug \
  test/e2e/i18n-ignore-rewrite-source-locale/rewrites.test.ts

The custom deployment succeeded at a local URL and reached assertions.

Result: 8/8 passed:

• Public-file rewrites: empty/default path, /en, /sv, /nl
• API rewrite controls: empty/default path, /en, /sv, /nl

Scope

No cacheComponents, use cache, PPR shell, or resume code was changed. No
lockfile change was made. The branch was not pushed and no PR was opened.

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/@vinext/cloudflare@2041

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

commit: 7d22155

[github-actions]

Example	Preview	Production	Original
app-router-cloudflare	preview	production
pages-router-cloudflare	preview	production
app-router-playground	preview	production	original
realworld-api-rest	preview	production
nextra-docs-template	preview	production
benchmarks	preview	production
hackernews	preview	production	original
workers-cache	preview	production
web	preview	production

[james-elicx]

/bigbonk review for issues

[ask-bonk]

@james-elicx Bonk workflow was cancelled.

View workflow run · To retry, trigger Bonk again.

[james-elicx]

/bigbonk review for issues

[ask-bonk]

/bigbonk review — PR #2041

Reviewed the post-rewrite filesystem re-entry change across the pipeline, all three runtime adapters (Node prod, Worker, Vite dev), and the new dev dot-path preflight. I checked dev/prod/worker parity, traced the worst-case "public file whose path also matches a rewrite source but the condition is unmet" scenario, and ran the targeted suites.

Verification performed

• vp test run tests/pages-request-pipeline.test.ts → 56 passed
• vp test run tests/pages-i18n-public-rewrite.test.ts → 13 passed (dev + prod)
• vp check on index.ts, pages-request-pipeline.ts, deploy.ts → clean
• Wrote a throwaway repro for the regression I suspected (real public/download.txt that matches the rewrite source, condition unmet) — it correctly returns the real file (200), so the preflight widening does not shadow real public assets. Good.

Overall this looks correct and the test coverage is strong. One concrete issue and a couple of notes.

---

1. Stale/incorrect comment contradicts the code (dev handled path)

packages/vinext/src/index.ts:3953-3954

// The dev static adapter returns a Response rather than writing directly,
// so `handled` remains a Node-production-only result here.
if (pipelineResult.type === "handled") {
  return;
}

This comment is now wrong. The dev adapter's serveRewrittenViteFilesystemRoute returns Promise<boolean> (true when a Vite filesystem middleware wrote the response directly to res), not a Response. The pipeline maps that true to { type: "handled" } (pages-request-pipeline.ts:315), so handled is reachable in dev now — it's exercised by the new tests/pages-i18n-public-rewrite.test.ts "development" cases that rewrite to public/built-static files.

The runtime behavior is fine (it returns correctly), but given how much this repo leans on dev/prod parity reasoning, the comment is actively misleading. Suggest something like:

// `handled` means an adapter wrote the response directly: Node prod via res,
// or dev via Vite's filesystem middleware (rewritten public/static files).

---

2. (Note, not blocking) Worker serveFilesystemRoute depends on not_found_handling: "none"

fetchWorkerFilesystemRoute treats any non-404 from env.ASSETS.fetch as a served asset. This is correct for the generated wrangler config (deploy.ts writes not_found_handling: "none"), but if a user overrides it to single-page-application, the asset binding returns 200 for unmatched paths and would shadow page rendering for every rewritten GET/HEAD destination. This matches how the existing image-optimization path already trusts the binding, so it's pre-existing behavior — just flagging it as a sharp edge for anyone customizing asset handling.

---

3. (Note) Worker asset request uses the pre-middleware request snapshot

In deploy.ts, the serveFilesystemRoute closure captures the outer request (basePath-stripped, but pre-middleware), and fetchWorkerFilesystemRoute builds the asset request from it via new Request(assetUrl, request). The pipeline reassigns its own local request to the post-middleware one, but the worker closure doesn't see that. For static asset fetches only path + GET/HEAD matter, so this is harmless today — but worth a one-line comment so a future reader doesn't assume middleware request-header overrides reach the asset fetch.

---

Nice work on the upstream-test porting and the dev preflight follow-up. The matchesRewriteSource preflight (conditions deferred until after middleware) is the right call and is well covered by the three new pages-router.test.ts cases.

[ask-bonk]

Review posted to PR #2041.

Summary

I reviewed the post-rewrite filesystem re-entry change in PR #2041 across all four request handlers (pipeline, Node prod, Worker, Vite dev), verified dev/prod/worker parity, and ran the targeted suites.

Verification:

• tests/pages-request-pipeline.test.ts → 56 passed
• tests/pages-i18n-public-rewrite.test.ts → 13 passed (dev + prod)
• vp check on the three main source files → clean
• Wrote a throwaway repro for the scenario I suspected was a regression (a real public/download.txt that matches a rewrite source but with the condition unmet) — it correctly serves the real file, so the dev preflight widening does not shadow real public assets.

Findings (1 concrete, 2 notes):

1. Stale/incorrect comment at index.ts:3953-3954 — it claims the dev adapter returns a Response so handled is "Node-production-only", but serveRewrittenViteFilesystemRoute returns a boolean, the pipeline maps true → { type: "handled" }, and the new dev tests exercise exactly that path. Runtime is fine; the comment is misleading given the repo's parity focus.

2. Note: Worker serveFilesystemRoute trusts any non-404 from env.ASSETS, which is correct for the generated not_found_handling: "none" config but would shadow page rendering if a user sets SPA mode (pre-existing pattern, just a sharp edge).

3. Note: The Worker asset fetch uses the pre-middleware request snapshot — harmless for static assets but worth a clarifying comment.

The core fix is correct and well-tested; the upstream-test porting and the matchesRewriteSource dev preflight (deferring has/missing until after middleware) are the right approach.

github run

[james-elicx]

/bigbonk review for issues

[ask-bonk]

/bigbonk review — PR #2041 (re-review)

Independently re-reviewed the post-rewrite filesystem re-entry change across the pipeline, all three runtime adapters (Node prod, Worker, Vite dev), and the dev dot-path preflight. The prior review's finding #1 (the stale index.ts comment) was already fixed in 652943a, so that's resolved.

Verification performed

• vp test run tests/pages-request-pipeline.test.ts → 56 passed
• vp test run tests/pages-i18n-public-rewrite.test.ts → 13 passed (dev + prod)
• vp test run tests/pages-router.test.ts -t "preflight" → 3 passed
• vp check on index.ts, pages-request-pipeline.ts, deploy.ts, config-matchers.ts, prod-server.ts → clean
• Wrote a throwaway repro for the "real public/download.txt that matches a rewrite source but condition unmet" scenario. It correctly serves the real file (200, real body). Confirmed why: vinext's dev middleware is registered after Vite's viteServePublicMiddleware, so real public files are served by Vite before the Pages pipeline ever sees them. The preflight widening therefore only affects file-looking paths that are not real public files but do match a rewrite source — which is exactly the case that should enter the pipeline. No regression here.

Overall the fix is correct, matches Next.js routing order, and is well-covered. One new note below.

---

1. (Note) Dev rewrite-to-static-file path ignores middlewareStatus — minor dev/prod parity gap

In the pipeline, the Node-prod / worker filesystem path merges middleware state with the served response including the middleware status override:

pages-request-pipeline.ts:308-313

const served = await deps.serveFilesystemRoute(requestPathname, middlewareHeaders, phase);
if (served instanceof Response) {
  return { type: "response", response: mergeHeaders(served, middlewareHeaders, middlewareStatus) };
}

mergeHeaders applies statusOverride (worker-utils.ts:58), so a middleware-set status flows onto the served static asset in prod/worker.

The dev adapter (index.ts serveRewrittenViteFilesystemRoute) instead lets Vite's filesystem middleware write the response directly and only applies the staged middleware headers to res — middlewareStatus is never applied, and the pipeline returns { type: "handled" } without a status merge. So a middleware that both sets a custom status (e.g. via NextResponse.next({ status })) and rewrites to a public/static file would yield that status in prod/worker but the asset's natural 200 in dev.

This is an edge case (status override + rewrite-to-static-file is rare) and the runtime is otherwise correct, but given how much this repo leans on dev/prod parity, it's worth either a short comment documenting the intentional difference or applying middlewareStatus in the dev handled path.

---

2. (Note, carry-over) Worker asset fetch + basePath

fetchWorkerFilesystemRoute queries env.ASSETS.fetch with the basePath-stripped rewrite destination (e.g. /file.txt). For a basePath app, public assets are served at <basePath>/file.txt and direct serving happens at the CF edge before the worker runs. A rewrite-to-static-file destination resolved without the basePath prefix may 404 against the asset binding. This is a niche combination (basePath + config rewrite → static asset on Workers) and isn't covered by the new tests; worth a follow-up test or a clarifying comment. This compounds the prior review's note that the worker closure captures the pre-middleware request snapshot — fine for path + GET/HEAD today, but undocumented.

---

Nice work on the upstream-test porting and the matchesRewriteSource preflight (deferring has/missing until after middleware is the right call and is well covered). The core re-entry logic is sound and the prod/dev/worker ordering is consistent.

[ask-bonk]

Review posted to PR #2041.

Summary

I independently re-reviewed PR #2041's post-rewrite filesystem re-entry change across the pipeline and all three runtime adapters (Node prod, Worker, Vite dev), plus the dev dot-path preflight.

Verification:

• tests/pages-request-pipeline.test.ts → 56 passed
• tests/pages-i18n-public-rewrite.test.ts → 13 passed
• tests/pages-router.test.ts -t "preflight" → 3 passed
• vp check on all 5 changed source files → clean
• Wrote a throwaway repro for the "real public/download.txt matching a rewrite source, condition unmet" scenario — it correctly serves the real file. Confirmed why: vinext's dev middleware runs after Vite's public-file middleware, so real public files are served before the Pages pipeline sees them. The preflight widening only catches non-real-file paths that match a rewrite source. No regression.

Findings:

1. The prior review's finding Virtual module imports break esbuild dependency optimization when vinext is installed from npm #1 (stale index.ts comment) was already fixed in 652943a.

2. New note: Dev rewrite-to-static-file ignores middlewareStatus. The prod/worker path applies it via mergeHeaders(served, middlewareHeaders, middlewareStatus), but the dev handled path only applies staged headers, never the status. Edge case (middleware status override + rewrite to static file), but a real dev/prod parity gap worth documenting or fixing.

3. Note (carry-over): Worker fetchWorkerFilesystemRoute queries env.ASSETS with the basePath-stripped destination; a basePath app could 404 on rewrite-to-static-file. Niche, uncovered by tests.

The core fix is correct, matches Next.js routing order, and is well-tested.

github run