fix(cache): preserve ISR with request-time middleware

[james-elicx]

Summary

• keep middleware and request-dependent config effects request-scoped when a CDN adapter can serve responses above the origin
• use origin-managed ISR for affected paths while retaining edge caching for matcher-excluded routes
• harden cache adapter registration across router module graphs, runtime environments, and build changes
• cover App and Pages HTML, RSC/data, route handlers, redirects, cookies, rewrites, revalidation, and direct-write static responses

Validation

• vp check
• 2,142 focused unit tests
• Pages production middleware regression across GET, HEAD, 304, range, 405, and image responses
• 17 Cloudflare Workers cache-isolation E2E tests
• vinext and @vinext/cloudflare package builds
• full PR CI, deploy examples, CodeQL, and performance workflows

[pkg-pr-new]

Open in StackBlitz

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

npm i https://pkg.pr.new/create-vinext-app@2803

npm i https://pkg.pr.new/@vinext/types@2803

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

commit: c1e5e15

[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

[github-actions]

Performance benchmarks

Compared c1e5e15 against base dfc979c using alternating same-runner rounds. Next.js was unchanged and skipped.

0 improved · 1 regressed · 5 within ±1.5%

Scenario	Framework	Baseline	Current	Change
Client bundle size (gzip)	vinext	134.6 KB	134.6 KB	⚫ -0.0%
Client entry size (gzip)	vinext	122.2 KB	122.1 KB	⚫ -0.0%
Dev server cold start	vinext	2.20 s	2.18 s	⚫ -0.7%
Production build time	vinext	2.42 s	2.40 s	⚫ -0.6%
RSC entry closure size (gzip)	vinext	113.8 KB	115.2 KB	⚫ +1.2%
Server bundle size (gzip)	vinext	191.3 KB	200.4 KB	🔴 +4.7%

View detailed results and traces

_{🟢 improvement · 🔴 regression · ⚫ change below 1.5% · paired base/head}

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: fix(cache): preserve ISR with request-time middleware

I reviewed all 59 files against the four goals in the PR body (request-scope middleware/config effects, origin-managed ISR for affected paths, hardened adapter registration, broad test coverage). The design is coherent and the test coverage is genuinely strong — I ran the affected suites locally and all pass (cache-adapters-config, pages-request-pipeline, middleware-runtime, app-rsc-*, pages-router, pages-revalidate, cloudflare-cdn-cache, shims, deploy — 2,400+ tests green), and vp check is clean on the core changed source files.

Overall this is a well-executed change. A few observations, none blocking.

Correctness — verified good

• finalizeMiddlewareSafePageCacheHeaderRecord Set-Cookie preservation (cache-control.ts:93) is careful and correct. Because applyCdnResponseHeaders only mutates Cache-Control/CDN headers, Set-Cookie compares equal (afterValue === before.get(name)) and is retained as its original array from the { ...headers } spread. The copy-back only overwrites names the CDN policy actually changed. Good.
• app-rsc-response-finalizer.ts refactor correctly converts the old early-return on configHeadersAlreadyApplied into a skip of config-header application while still reaching the final applyOriginManagedPageCacheResponseHeaders reassertion. Previously an already-applied response returned early and never got the safe policy — this is a real fix, not just a refactor.
• linkHeader threading (app-page-cache-finalizer.ts / app-page-render.ts:1266) fixes a latent bug: the ISR write previously read response.headers.get("link") after middleware headers merged, so a middleware-modified link header could be persisted. It now uses the page-owned linkHeader computed pre-merge. Consistent with the PR's request-scoping goal.
• On-demand revalidate tag purge (pages-revalidate.ts:121) fallback tag encodeCacheTag(\N_T${stem || "/"}`)matches the tagging used inpages-page-handler.ts:164, pages-page-data.ts:494, pages-page-response.ts:696`. Consistent.
• middlewarePathMatched seeding in runPagesRequest — seeding from the pathname matcher for auth'd on-demand revalidation (middleware is skipped there), and updating from config header/redirect/rewrite matches before step-4 config redirects are finalized — is correctly ordered. The closure reads the let at call time, so pre-middleware responses see updated state.
• Generated virtual:vinext-cache-adapters output is syntactically valid for the data-only, cdn-only, and both-configured branches (verified by generating it). Indentation is cosmetically off but braces balance.

Non-blocking notes

1. wrapMiddlewareWithBasePath behavioral change (pages-request-pipeline.ts:230). The old code used addBasePathToPathname, a no-op when the pathname already started with basePath (via hasBasePath). The new code unconditionally prepends basePath. This changes the middleware URL for apps whose application pathname legitimately begins with the basePath segment (/docs/docs/...). The comment justifies it (adapter strips exactly one segment) and it's covered by pages-request-pipeline.test.ts:2549 and the /docs/docs/* cases in pages-router.test.ts. Flagging only because it's a subtle semantics change buried in a large diff — worth a callout in the changelog/commit body if not already there.

2. Per-request cost of registration (cache-adapters-virtual.ts). Removing the __vinextCacheAdaptersRegistered idempotency guard means registerConfiguredCacheAdapters now runs its body on every request instead of returning early after the first. The happy path is cheap (symbol lookup + isConfigured*Active comparison, no factory re-invocation), so this is fine, but it does add a small constant to the common request path — mild tension with the "keep the common request path lightweight" guidance in AGENTS.md. Acceptable given it's needed for cross-module-graph correctness; just noting the tradeoff was made deliberately.

3. CodeQL findings are test-only (tests/e2e/cloudflare-workers/route-handler-draft-cache.spec.ts). Both flagged lines live in the in-test "edge cache stand-in" HTTP server:

   • SSRF (L41): fetch(\${BASE_URL}${cacheKey}`)` — host is a fixed test worker; only the path derives from the request, which is the point of simulating a CDN pass-through. Not a production surface.
   • Stack-trace exposure (L70): outgoing.end(error.stack) on a 502 — test harness diagnostics.
     Neither is a production concern. If you want to keep CodeQL quiet you could return a generic 502 body and/or assert the host, but I would not block on it.

4. app-pages-bridge.ts API-route ctx change (now passing executionContext ?? undefined instead of undefined) is a correctness improvement — it wires cacheAdapterEnv into the pages API path under the App Router bridge, which previously got no binding env. Just confirm the pages API handler tolerates a non-undefined ctx here (tests pass, so it appears to).

Nice work — the request-scoping model, the "bypassesOriginOnCacheHit" adapter contract, and the fail-safe defaults (assume in-scope on malformed URL / custom adapters default to true) are all the conservative-by-default choices you want for a cache-correctness change.

[ask-bonk]

Review posted to PR #2803.

Summary

I reviewed all 59 files in PR #2803 (fix(cache): preserve ISR with request-time middleware), ran the affected test suites locally (2,400+ tests, all green), and confirmed vp check is clean on the core changed source.

Verdict: well-executed, no blocking issues. The design (request-scoped middleware/config effects + origin-managed ISR fallback when a CDN adapter would bypass the origin on HITs) is coherent with conservative fail-safe defaults.

Key findings:

Verified correct

• finalizeMiddlewareSafePageCacheHeaderRecord preserves Set-Cookie arrays correctly (CDN policy never touches those names).
• app-rsc-response-finalizer.ts refactor fixes a latent bug — already-applied responses previously returned early and skipped the safe policy; now they always reach the reassertion.
• linkHeader threading fixes caching of a middleware-modified link header into ISR.
• On-demand revalidate tag-purge fallback matches the existing _N_T_ tagging convention.

Non-blocking notes I flagged

1. wrapMiddlewareWithBasePath now unconditionally prepends basePath (was a no-op when already present) — intentional, tested, but a subtle semantics change worth a changelog callout.
2. Removing the registration idempotency guard adds a small per-request cost (mild tension with the lightweight-hot-path guideline; happy path stays cheap).
3. The two CodeQL findings (SSRF + stack-trace exposure) are both in a test-only edge-cache stand-in server — not production surfaces.
4. The app-pages-bridge API-route ctx change is a correctness improvement (wires binding env into the pages API path).

github run

[james-elicx]

/bigbonk review for issues

[ask-bonk]

@james-elicx Bonk workflow failed. Check the logs for details.

View workflow run · To retry, trigger Bonk again.

[james-elicx]

/bigbonk review for issues

[ask-bonk]

UnknownError: ProviderInitError

github run

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: fix(cache): preserve ISR with request-time middleware

I re-reviewed the full diff (59 files, 2758/-186), traced the request-scoping model end to end, generated the virtual:vinext-cache-adapters output to check it, and ran the affected suites locally — all green:

• cache-adapters-config + cloudflare-cdn-cache (50)
• pages-request-pipeline + middleware-runtime + app-rsc-handler + app-rsc-response-finalizer (330)
• pages-router + pages-revalidate + app-page-cache + app-pages-bridge + deploy (771)

vp check is clean on the core changed source (cache-control, pages-request-pipeline, app-rsc-handler, app-rsc-response-finalizer, cdn-cache, cache-adapters-virtual).

Verdict: no blocking issues. The core idea — decide cache-safety at the pathname level (middleware matcher scope + header/cookie-dependent config rules), keep it stable per URL, and fall back to origin-managed ISR whenever a HIT-above-origin adapter would skip request-time composition — is the right model and is applied consistently across App and Pages, dev and prod.

Verified correct

• Read/write symmetry holds. originManagedPageCache is derived from pathname scope, not from per-request has/missing inputs (see matchesMiddlewarePathname, matchesRequestDependentHeaderSource/Redirect/Rewrite). So isrGet/isrSet/triggerBackgroundRegeneration all resolve getCdnCacheStorageAdapter() to the same store for every request to a given URL — no split-brain between edge and origin stores.
• Generated adapter module is syntactically valid for the data-only, cdn-only, both, and none branches (generated and eyeballed — braces balance despite the cosmetic indentation drift inside registerConfiguredCacheAdapters).
• finalizeMiddlewareSafePageCacheResponse catch path correctly rebuilds only when headers are immutable (redirects), and bails on status-0 responses that can't be reconstructed.
• finalizeMiddlewareSafePageCacheHeaderRecord Set-Cookie preservation is correct: the CDN policy only touches Cache-Control/CDN headers, so Set-Cookie compares equal and is retained from the { ...headers } spread as its original array.
• app-rsc-response-finalizer.ts now reaches applyOriginManagedPageCacheResponseHeaders even for already-applied config headers (the old early return skipped the safe policy) — a real fix, not just a refactor.
• app-pages-bridge ctx wiring (executionContext ?? undefined into the 4th positional) lands in renderPage's ctx slot (pages-server-entry.ts:501), which is exactly what registerConfiguredCacheAdapters(ctx?.cacheAdapterEnv) + _runWithExecutionContext(ctx, …) consume. Previously undefined starved the Pages API/render path of binding env. Correct improvement.
• linkHeader threading persists the page-owned pre-merge Link value into ISR instead of the post-middleware-merged one.
• On-demand revalidate tag purge falls back to _N_T_${stem} which matches the existing tagging convention, and only fires for bypassesOriginOnCacheHit adapters.
• Failed-registration tracking uses a WeakMap for object envs (GC-safe) and a Map only for rare primitive envs, bounded by the number of configured adapters — no practical memory growth.

Non-blocking notes

1. wrapMiddlewareWithBasePath semantics change (pages-request-pipeline.ts): now unconditionally re-prepends basePath instead of the old addBasePathToPathname no-op-when-present. Intentional (the adapter strips exactly one segment) and covered by the /docs/docs/* cases, but it's a subtle behavior change worth a commit-body/changelog callout.
2. Per-request registration cost (cache-adapters-virtual.ts): dropping __vinextCacheAdaptersRegistered means the body runs every request. The happy path is cheap (isConfigured*Active symbol compare, no factory re-invocation), but it's a small constant added to the common path — mild tension with the "keep the hot path lightweight" guidance, accepted deliberately for cross-module-graph correctness.
3. pages-page-handler.ts:615 default options?.originManagedPageCache ?? hasMiddleware: fail-safe (conservative) for direct renderPage callers that bypass the pipeline, but it means a direct call for an out-of-scope path in a middleware app would still be treated as origin-managed. The pipeline always passes an explicit boolean, so this only affects non-pipeline callers — acceptable, just noting it.
4. CodeQL findings are test-only. Both are in the in-test edge-cache stand-in (route-handler-draft-cache.spec.ts). SSRF: upstreamUrl is only ever ${BASE_URL}${pathname} via a pre-registered map — host is fixed. Stack-trace: the current code returns a generic 502 body and only console.errors the error, so that finding looks already addressed in this commit. Not production surfaces.

Nice work — the bypassesOriginOnCacheHit contract with custom adapters defaulting to true, the malformed-URL fail-safe (normalizedPathname instanceof Response → true), and pathname-level scoping are the conservative-by-default choices you want for a cache-correctness change.

[ask-bonk]

Review posted to PR #2803.

Summary

I reviewed the full diff (59 files, +2758/-186), traced the request-scoping model end to end, verified the generated cache-adapters virtual module is syntactically valid, ran the affected test suites locally (1,151+ tests, all green), and confirmed vp check is clean on the core changed source.

Verdict: no blocking issues. The design is sound and consistently applied.

Key finding that makes the design correct

The whole approach hinges on cache-safety being decided at the pathname level (middleware matcher scope + header/cookie-dependent config rules), not per-request has/missing inputs. This keeps originManagedPageCache stable for a given URL, so isrGet/isrSet/background regeneration all resolve to the same store on every request — no edge/origin split-brain. I verified this holds.

Verified correct

• Generated adapter module (all four config branches) is valid.
• finalizeMiddlewareSafePageCacheResponse catch path rebuilds only immutable-header responses, bails on status-0.
• Set-Cookie preservation in finalizeMiddlewareSafePageCacheHeaderRecord.
• app-rsc-response-finalizer now always reaches the safe policy (fixes a latent early-return bug).
• app-pages-bridge ctx wiring correctly lands in renderPage's ctx slot (fixes missing binding env on the Pages API/render path).
• Failed-registration tracking is GC-safe (WeakMap for object envs).

Non-blocking notes flagged

1. wrapMiddlewareWithBasePath now unconditionally re-prepends basePath (intentional, tested, worth a changelog callout).
2. Per-request registration cost from dropping the idempotency guard (cheap happy path, accepted tradeoff).
3. pages-page-handler.ts ?? hasMiddleware default only affects non-pipeline direct callers (fail-safe).
4. Both CodeQL findings are in a test-only edge-cache stand-in — not production surfaces; the stack-trace one appears already addressed in this commit.

github run