fix(app-router): track searchParams access for static bailout

[NathanDrake2406]

Scope

Fixes 3 failing assertions from the upstream Next.js deploy-suite candidate in test/e2e/app-dir/searchparams-static-bailout/searchparams-static-bailout.test.ts. With this patch, the full upstream file passes: 5/5 tests under the Vinext deploy harness.

Refs #1524. This intentionally does not close #1524 because that issue also tracks the separate app-static / Pages useSearchParams() failure, which is outside this standalone PR.

CI report triage

The old #1788 blobs were not flaky. They showed branch regressions in static/ISR classification:

File	Failure shape
tests/app-router-dev-server.test.ts	non-force-dynamic and revalidate pages emitted no-store instead of static/ISR cache headers.
tests/features.test.ts	App Router ISR dev page emitted no-store.
tests/app-router-production-server.test.ts	page ISR + searchParams cache behavior regressed.
tests/prerender.test.ts	cache-life prerender metadata regressed.

A clean origin/main baseline passed the prerender cases, so the report pointed at this PR’s dynamic-marking/cache-key behavior rather than CI noise.

What changed

Area	Change
Static-cache bailout	Replaced source-token classification with access observation. Vinext marks dynamic only when searchParams is actually accessed/awaited/used.
Page probe coverage	The generated RSC entry probes the primary page, active parallel slot pages, and active intercepting pages during the normal pre-render probe path. The probe also walks returned server-component subtrees, so Page(props) { return <Child {...props} /> } is observed when Child consumes searchParams.
Cache-hit invariant	Fresh production app-page cache HITs do not execute probePage() or user page/server-component code before serving cached HTML/RSC. Query-bearing requests only serve cached app-page artifacts that carry a complete negative searchParams render observation; older/unproofed artifacts fall through to a fresh render.
Loading boundary render path	Real app-page render searchParams thenables observe actual access, so loading.tsx routes that skip the early page probe still cannot write a false query-invariant cache proof. Passive RSC serialization of the prop remains inert.
dynamic = "force-static"	force-static remains an explicit override and still allows static page cache reads even if the request has a query.
"use cache" page keys	App Router page default exports carry explicit transform metadata into registerCachedFunction(), so page-default cache keys omit only the root searchParams prop. This is consumed at the cached-function call boundary and does not depend on React preserving a non-enumerable marker through createElement().
searchParams inside public cache scopes	Observed App Router page searchParams access now uses the same cache-scope guard as headers() / cookies() / connection(). Access inside public "use cache" rejects and records invalid dynamic usage on the active cache context; if user code catches that throw, the cached wrapper still rethrows before storing a shared data-cache entry.
Userland cache keys	Arbitrary cached functions with { params, searchParams } still key by searchParams; page cache semantics are not inferred from object shape or cache id strings.

This remains unstacked from the harness fix. The deploy-suite validation below was run with the #1786 harness packaging fix applied temporarily in a separate verification worktree; this PR does not include that harness change.

Follow-up

• App Router: replace prerender HTML buffering with SSR metadata readiness #1795 tracks replacing the prerender-only HTML buffering in renderAppPageLifecycle() with an explicit SSR metadata-readiness contract. That work is intentionally not included here: this PR keeps the correctness fix local to searchParams static bailout, while the follow-up can change the SSR/RSC lifecycle boundary deliberately.

Regression coverage

• Fresh production cache HITs do not invoke the generated page probe or page code.
• Query-bearing requests treat unproofed cached app-page artifacts as misses instead of serving possibly empty-query output.
• Query-bearing requests may serve cached app-page artifacts with a complete negative searchParams proof.
• loading.tsx + revalidate + await searchParams does not write a query-invariant cache entry and does not serve one query’s HTML for another query.
• Unused/comment-only searchParams mention stays static and can read cached production HTML.
• Access-observed page probe bypasses cached production HTML for query-sensitive pages.
• force-static overrides observed searchParams access for cache reads.
• Returned server-component subtree consumption, including prop-spread forwarding, marks dynamic.
• Active parallel slot and intercepting pages receive page searchParams.
• Cached App Router primary page, active slot page, and active intercept page render through buildPageElements() with two queries while reusing the same "use cache" entry and not observing searchParams.
• App page default "use cache" rejects when it reads observed searchParams, including the dangerous two-query case where searchParams is omitted from the page-default cache key.
• Caught searchParams access inside page default "use cache" is still treated as invalid dynamic usage and is not stored under the omitted-searchParams key.
• Arbitrary cached functions with { params, searchParams } keep distinct cache keys by searchParams.
• App-page cache-key omission requires explicit page-default transform metadata; cache id strings alone do not trigger page-prop treatment.

Validation

• vp check
  Result: passed.
• vp run knip
  Result: passed.
• vp test run tests/shims.test.ts -t "app page searchParams access inside page default|caught searchParams access|page default export searchParams|cache id alone|params/searchParams objects"
  Result: 5 passed.
• vp test run tests/app-page-element-builder.test.ts tests/shims.test.ts tests/app-page-probe.test.ts tests/app-page-dispatch.test.ts tests/app-page-render.test.ts
  Result: 1133 passed.
• 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/searchparams-static-bailout/searchparams-static-bailout.test.ts
  Result: 5 passed, 0 failed. Run in a separate verification worktree with fix(test): package workspace deps in deploy harness #1786 applied temporarily for the deploy-harness packaging fix.

References

Reference	Why it matters
Next.js searchparams-static-bailout test	Upstream observable contract.
Server Component fixture	await searchParams must bail out from static output.
Client Component page fixture	use(searchParams) in a client page must bail out.
Server page passing searchParams to client component	Forwarded searchParams prop must preserve the same behavior.
Client component consumer	Shows the forwarded prop is consumed with React use().
Next.js page prop creation	Source reference for App Router page prop wiring.
Next.js searchParams instrumentation	Source reference for instrumenting searchParams access during prerender.
Next.js unstable cache wrapper	Source reference for page-segment cache-key treatment and cache-scope handling.
Next.js page searchParams docs	Public API contract for the page prop.

[pkg-pr-new]

Open in StackBlitz

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

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

commit: df0fd12

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: searchParams static-bailout via access observation

I reviewed the full diff and ran the affected test files locally — all pass:

• tests/app-page-dispatch.test.ts (16) + tests/app-page-probe.test.ts (21)
• tests/shims.test.ts (1027) + tests/app-page-element-builder.test.ts + tests/app-page-cache.test.ts + tests/app-page-render.test.ts (1126 total)
• tests/app-router-production-server.test.ts + tests/app-router-dev-server.test.ts (202)
• tests/prerender.test.ts (79)
• vp check on the new/changed source files: clean.

What's solid

The overall direction is correct and matches Next.js semantics well:

• The cache-proof guard is positioned correctly. readAppPageCacheResponse checks hasRequestSearchParams && !hasQueryInvariantAppPageProof before both the HIT (app-page-cache.ts:373) and STALE (app-page-cache.ts:410) serving paths, so a query-bearing request can never be served an unproven entry from either path. It also applies to both RSC and HTML reads since it runs before buildAppPageCachedResponse branches on isRscRequest.
• The loading.tsx false-proof defense is real. When the observed thenable is awaited during render, markDynamicUsage() fires, and finalizeAppPageHtmlCacheResponse skips the write entirely (app-page-cache.ts:561-567). So no query-invariant entry is written for a page that actually reads searchParams behind a loading boundary. The dispatch test correctly allows either "nothing written" or "written-with-searchParams-observed proof."
• The observeReactPromiseStatus split is a nice detail. Probe path opts in (app-page-probe.ts:259) so a React .status read counts as access; the real render-tree thenable does not (so React's use() caching .status reads don't spuriously bail out). The element-builder tests pin this distinction.
• force-static override correctly disables the proof requirement (app-page-dispatch.ts:530), matching Next.js where force-static yields an empty searchParams.
• Defense-in-depth on cache keys: both the transform metadata (appPageDefaultExport) and the non-enumerable markAppPagePropsForUseCache marker are honored in unwrapThenableObjects, and throwIfInsideCacheScope now records invalidDynamicUsageError on the cache context so a caught throw still prevents storing under the omitted-searchParams key.

Concerns (non-blocking)

1. Slot/intercept probe may over-bail (false dynamic). The probe walks all route.slots, not just the slots mounted/active for this request. At render time, a slot with both page.tsx and default.tsx may render default (see app-page-route-wiring.tsx:741-750), but the probe unconditionally invokes __slot?.page?.default. A route whose inactive @modal/page.tsx awaits searchParams would mark the whole request dynamic on a request where that page never renders, defeating static caching for an otherwise-static page. This fails safe (toward dynamic), so it's a caching-efficiency regression rather than a correctness/security issue, but it diverges from the PR description's "active parallel slot pages" wording.

2. Prerender HTML is now fully buffered in memory. renderAppPageLifecycle buffers the entire HTML via readStreamAsText then re-emits (app-page-render.ts:902-903). This is build-time-only and #1795 already tracks replacing it, but worth calling out: large prerendered pages now hold the full HTML in memory before responding.

3. Generated-entry thickness. Per AGENTS.md, entries/app-rsc-entry.ts should stay thin. The probePage() body now hand-rolls a slot/intercept probe loop inline. Consider extracting this into a server/* helper (e.g. buildAppPageProbes(...)) so it's unit-testable directly and the entry stays codegen glue. The probe helper itself (probeAppPage) is already extracted; the fan-out over slots/intercepts is the part that grew inline.

None of these block merge. (1) is the one I'd most want a follow-up on, since it can silently disable caching for routes with parallel slots.

[ask-bonk]

Review posted successfully. Here's a summary of my review of PR #1788.

Summary

I reviewed the full diff for fix(app-router): track searchParams access for static bailout, read the key source files (the new observation module, cache-runtime, headers shim, probe, dispatch, render, element-builder, cache-proof), and ran all affected test suites locally — everything passes (1444+ tests across the changed files, plus vp check clean).

Verdict: Sound, well-tested, non-blocking concerns only

What's correct:

• The query-proof guard in readAppPageCacheResponse is positioned before both HIT and STALE serving paths and covers both RSC and HTML reads, so a query-bearing request can never be served an unproven cache entry.
• The loading.tsx false-proof defense genuinely works: awaiting the observed thenable fires markDynamicUsage(), which causes finalizeAppPageHtmlCacheResponse to skip the cache write entirely.
• The observeReactPromiseStatus distinction (probe opts in, real render doesn't) correctly avoids treating React's internal .status reads as user access.
• force-static override and the cache-key defense-in-depth (transform metadata + marker + invalidDynamicUsageError recording on caught throws) are all correct.

Three non-blocking concerns flagged:

1. Slot/intercept probe over-bail (the one worth a follow-up): the probe walks all route.slots, not just active ones, so an inactive @modal/page.tsx that awaits searchParams can mark a request dynamic even when that page never renders — fails safe toward dynamic, but can silently disable caching.
2. Prerender HTML is now fully buffered in memory (build-time only; App Router: replace prerender HTML buffering with SSR metadata readiness #1795 tracks it).
3. Generated-entry thickness: the inline slot/intercept probe loop in app-rsc-entry.ts would be better as an extracted, unit-testable server/* helper per AGENTS.md.

github run

[james-elicx]

Thanks for the thorough review. Addressed the actionable concerns:

Concern #3 (generated-entry thickness) — fixed. Extracted the inline slot/intercept probe fan-out out of entries/app-rsc-entry.ts into a unit-testable buildAppPageProbes() helper in server/app-page-probe.ts. The entry's probePage() is now a single delegation call, and the fan-out (matched page + parallel slot pages + interception page) is covered by focused unit tests in tests/app-page-probe.test.ts (probe coverage, interception omission/fallback params, slots without a page export).

Concern #1 (slot probe over-bail) — tracked as follow-up. Filed #1798. As you noted this fails safe (toward dynamic — it never serves a stale query-bearing response), so it's a caching-efficiency regression rather than a correctness issue. A correct narrowing needs to mirror the render-time active-slot resolution (slot overrides from interception + mounted-slot state) currently computed in app-page-route-wiring.tsx, which isn't readily available at probe time — better done as its own change than rushed here. Documented the behavior in the buildAppPageProbes doc comment with a link to #1798.

Concern #2 (prerender HTML buffering) — already tracked. Build-time-only and covered by #1795, as you noted.

Also merged latest main and resolved the conflict in server/app-page-probe.ts (kept both the makeObservedAppPageSearchParamsThenable and isPromiseLike imports). vp check and the affected test files pass locally.

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Re-review: searchParams static-bailout via access observation

Verified the full diff, read every changed source module (observation helper, buildAppPageProbes, dispatch, cache-proof guard, cache-runtime, headers shim, element-builder, render lifecycle, entry wiring), and ran the affected suites locally:

• tests/app-page-probe.test.ts + tests/app-page-dispatch.test.ts — 41 passed
• tests/app-page-element-builder.test.ts + tests/app-page-cache.test.ts + tests/app-page-render.test.ts + tests/shims.test.ts — 1126 passed
• tests/app-router-production-server.test.ts + tests/app-router-dev-server.test.ts + tests/prerender.test.ts + tests/features.test.ts — 592 passed
• vp check on the changed source files — clean (format + lint + types)

Verdict: correct and well-tested; no blocking issues

Confirmed correct:

• The query-proof guard (readAppPageCacheResponse, app-page-cache.ts:358) runs before both the HIT (:373) and STALE (:410) serving paths and before the RSC/HTML branch in buildAppPageCachedResponse, so a query-bearing request can never be served an unproven entry from either path.
• markAppPageSearchParamsAccess() calls throwIfInsideCacheScope("searchParams") before markDynamicUsage(), mirroring headers()/cookies(). Inside a public "use cache" the throw records invalidDynamicUsageError on the cache context (headers.ts:274), and runCachedFunctionWithContext rethrows it (cache-runtime.ts:745) so a caught throw still blocks the shared write. The two shims.test.ts cases pin this.
• Cache-key omission is gated on explicit transform metadata (appPageDefaultExport) or the non-enumerable APP_PAGE_PROPS_CACHE_KEY_MARKER, not on object shape or cache-id strings — the "does not infer page props from the cache id alone" and "keeps arbitrary params/searchParams objects distinct" tests confirm userland functions are unaffected.
• force-static correctly bypasses the proof requirement: hasRequestSearchParams: !isForceStatic && hasSearchParams(...) (app-page-dispatch.ts:530).
• The observeReactPromiseStatus split (probe opts in via app-page-probe.ts:250, render-tree thenable does not) keeps React's internal .status reads from spuriously bailing out, while the loading-boundary render-tree thenable still observes a real await. Element-builder tests pin both directions.
• findIntercept is a pure route-graph lookup, so the extra call inside probePage() is redundant work only on the probe path, not a correctness/side-effect concern. The new fan-out also strictly improves on the old probeAppPage call, which never probed slots or intercepts.

Follow-ups (already tracked, non-blocking):

• Slot/intercept over-bail: buildAppPageProbes probes every declared slot.page.default plus the intercept page rather than only the render-time active slot, which can over-bail an otherwise-static page. This fails safe (toward dynamic, never serving a stale query response) and is documented in the buildAppPageProbes doc comment with a link to #1798.
• Prerender HTML is fully buffered in memory (app-page-render.ts:902); build-time only, tracked by #1795.

Concern #3 from the prior review (generated-entry thickness) is resolved — the fan-out is now an extracted, unit-tested buildAppPageProbes() helper and the entry's probePage() is a single delegation. Looks good to merge once CI is green.

[ask-bonk]

Review posted successfully. Here's a summary of my re-review of PR #1788.

Summary

I reviewed the full diff for fix(app-router): track searchParams access for static bailout, read every changed source module, and ran all affected suites locally — all passing (41 + 1126 + 592 tests, plus vp check clean on the changed source files).

Verdict: correct and well-tested, no blocking issues

What I verified as correct:

• Query-proof guard placement — readAppPageCacheResponse checks hasRequestSearchParams && !hasQueryInvariantAppPageProof before both the HIT and STALE serving paths and before the RSC/HTML branch, so a query-bearing request can never be served an unproven entry.
• Cache-scope rejection ordering — markAppPageSearchParamsAccess() calls throwIfInsideCacheScope("searchParams") first, mirroring headers()/cookies(). The recorded invalidDynamicUsageError is rethrown by runCachedFunctionWithContext, so a caught throw still blocks the shared write.
• Cache-key omission gating — keyed on explicit appPageDefaultExport transform metadata or the non-enumerable marker, never on object shape or cache-id strings (userland functions unaffected, confirmed by tests).
• force-static override, the observeReactPromiseStatus probe/render split, and the intercept fan-out all behave correctly. The redundant findIntercept call inside probePage() is a pure lookup with no side effects.

Follow-ups (already tracked, non-blocking):

• Slot/intercept over-bail (App Router: searchParams probe over-bails for inactive parallel-route slot pages #1798) — fails safe toward dynamic, documented in the helper's doc comment.
• Prerender HTML buffering (App Router: replace prerender HTML buffering with SSR metadata readiness #1795) — build-time only.

The prior review's actionable concern (generated-entry thickness) is resolved via the extracted, unit-tested buildAppPageProbes() helper. Recommended to merge once CI is green.

github run

[james-elicx]

Update: I addressed concern #1 directly on the PR instead of punting it to a follow-up (closed #1798).

Concern #1 (slot probe over-bail) — fixed in 08a86dc. The probe now only invokes page components that actually render for the request. The over-bail was specifically the interception-override case: when an interception matches, the element builder replaces the page of the slot named by intercept.slotKey (overrides[slotKey].pageModule = interceptPage, which wins over slot.page in app-page-route-wiring.tsx). buildAppPageProbes now probes the interception page in place of that overridden slot's own page rather than probing both, so an otherwise-static request is no longer marked dynamic for a component that never renders. This is a precise narrowing — the overridden slot page provably doesn't render, and the interception page that does render is still probed, so there's no risk of under-bailing. Added a unit test pinning the behavior (tests/app-page-probe.test.ts).

Concern #3 (generated-entry thickness) — fixed (prior commit). Fan-out extracted into the unit-testable buildAppPageProbes() helper; the entry's probePage() is now a single delegation call.

Concern #2 (prerender HTML buffering) — build-time-only, tracked by #1795 as you noted.

vp check clean; app-page-probe/app-page-dispatch/entry-templates (67) and app-router-production-server/dev-server (202) pass locally.

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: searchParams static bailout via access observation

I read the full diff and the key source modules (the new observation helper, buildAppPageProbes, the dispatch query-proof guard, the cache-runtime page-prop key handling, throwIfInsideCacheScope, the element-builder render-tree wiring, and the prerender buffering in renderAppPageLifecycle). I also ran tests/app-page-probe.test.ts + tests/app-page-dispatch.test.ts locally — 42 passed.

Verdict: correct and well-tested; no blocking issues

The approach (observe actual searchParams access instead of source-token classification) is sound and matches Next.js semantics. The pieces I verified as correct:

• Query-proof guard placement — readAppPageCacheResponse checks hasRequestSearchParams && !hasQueryInvariantAppPageProof(cachedValue) (app-page-cache.ts:358-371) before both the HIT (:373) and STALE (:410) serving paths, and before buildAppPageCachedResponse branches on RSC vs HTML, so a query-bearing request can never be served an unproven entry from either path.
• Cache-scope rejection ordering — markAppPageSearchParamsAccess() calls throwIfInsideCacheScope("searchParams") before markDynamicUsage() (app-page-search-params-observation.ts:13-17), mirroring headers()/cookies(). The recorded invalidDynamicUsageError is rethrown by runCachedFunctionWithContext (cache-runtime.ts:745-747) so a caught throw still blocks the shared write.
• Cache-key omission gating — keyed on explicit appPageDefaultExport transform metadata or the non-enumerable APP_PAGE_PROPS_CACHE_KEY_MARKER (cache-runtime.ts:937-951), never on object shape or cache-id strings; the userland-distinct and cache-id-alone tests confirm arbitrary cached functions are unaffected.
• force-static override — hasRequestSearchParams: !isForceStatic && hasSearchParams(...) (app-page-dispatch.ts:530) correctly disables the proof requirement.
• observeReactPromiseStatus split — probe opts in (app-page-probe.ts:250-252) so a .status read counts as access; the render-tree observed thenable does not, so React's internal use() status reads don't spuriously bail. Element-builder tests pin both directions.
• App page module detection — isAppPageModule (index.ts) is correctly scoped via isInsideDirectory(appDir, ...) so Pages Router files and non-page basenames are excluded.

Two non-blocking observations below.

[ask-bonk]

Non-blocking: the comment block above (and the PR thread) state the probe "only invokes page components that actually render for this request." That's true for the interception-override case you fixed, but this loop still probes slot.page.default for every non-overridden slot, even when that slot will render its default.tsx instead of its page.tsx at render time.

At render time, buildAppPageElements picks overrideOrPageComponent ?? defaultComponent and additionally skips a slot entirely on soft-nav when only default.tsx exists and the slot is already mounted (app-page-route-wiring.tsx:741-750). So a mounted slot rendering default on an RSC soft-nav can still have its page.tsx probed here. This fails safe (toward dynamic — never serves a stale query response), so it's a caching-efficiency edge rather than a correctness bug, but the doc comment slightly overstates the precision. Worth either tightening the wording or tracking the soft-nav/mounted-default case as a follow-up.

[james-elicx]

Good nudge on the wording — tightened the doc comment in bb51b70.

On the soft-nav/mounted-default case specifically: it can't actually over-bail. The continue at app-page-route-wiring.tsx:741 is gated on !overrideOrPageComponent, i.e. the slot has no page.tsx (and no override). In that case slot.page?.default is undefined, so probeAppPage short-circuits to null and probes nothing — a no-op, not a dynamic bailout. Conversely, when a slot does have a page.tsx, overrideOrPageComponent is truthy so that page renders (overrideOrPageComponent ?? defaultComponent), meaning probing slot.page.default is exactly correct.

So after the interception-override fix the fan-out is precise: page-bearing slots render their page (probed), default-only slots have nothing to probe. The doc comment now spells this out, plus notes that a default.tsx awaiting searchParams is backstopped by the real render's observation (so it can't under-bail either).

[ask-bonk]

Non-blocking (already tracked by #1795): the prerender path now fully buffers the rendered HTML into a string via readStreamAsText and re-emits it through createBufferedHtmlStream. This is build-time-only, but for very large prerendered pages it holds the entire HTML in memory before responding. Fine to defer to the #1795 SSR metadata-readiness contract; flagging so it isn't lost.

[james-elicx]

Acknowledged — this is build-time-only (prerender path) and intentionally deferred to the SSR metadata-readiness contract tracked in #1795, which is the right place to remove the full-HTML buffer. Keeping it as-is in this PR so the searchParams change stays focused; flagging here that it's tracked and not lost.

[ask-bonk]

Review posted successfully. Here's a summary of my review of PR #1788.

Summary

I reviewed the full diff for fix(app-router): track searchParams access for static bailout, read the key source modules (observation helper, buildAppPageProbes, dispatch query-proof guard, cache-runtime page-prop key handling, throwIfInsideCacheScope, element-builder render-tree wiring, prerender buffering), and ran tests/app-page-probe.test.ts + tests/app-page-dispatch.test.ts locally — 42 passed.

Verdict: correct and well-tested; no blocking issues

Verified correct:

• Query-proof guard runs before both HIT and STALE paths and before the RSC/HTML branch, so a query-bearing request can never be served an unproven entry.
• Cache-scope rejection ordering mirrors headers()/cookies(); the recorded invalidDynamicUsageError is rethrown so a caught throw still blocks the shared write.
• Cache-key omission is gated on explicit transform metadata or the non-enumerable marker, never on object shape or cache-id strings (userland functions unaffected).
• force-static override and the observeReactPromiseStatus probe/render split behave correctly.
• isAppPageModule detection in index.ts is correctly scoped to the app/ directory and page basenames.

Two non-blocking comments posted inline:

1. The buildAppPageProbes slot loop still probes every non-overridden slot's page.default, even when a mounted slot renders its default.tsx on soft-nav. This fails safe (toward dynamic) but the doc comment slightly overstates "only invokes components that actually render." Worth tightening wording or a follow-up.
2. The prerender HTML buffering (app-page-render.ts:903) fully buffers HTML in memory — build-time only, already tracked by App Router: replace prerender HTML buffering with SSR metadata readiness #1795.

The PR's earlier-flagged actionable concerns (generated-entry thickness, interception over-bail) were both addressed in prior commits. Recommended to merge once CI is green.

github run

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: searchParams static bailout via access observation

I read the full diff and the key source modules (the new app-page-search-params-observation helper, buildAppPageProbes, the dispatch query-proof guard, the cache-runtime page-prop key handling + invalidDynamicUsageError rethrow, throwIfInsideCacheScope, the element-builder/route-wiring searchParams forwarding, and the prerender buffering in renderAppPageLifecycle). Ran the affected suites locally — all green:

• tests/app-page-probe.test.ts + tests/app-page-dispatch.test.ts + tests/app-page-cache.test.ts (72)
• tests/app-page-element-builder.test.ts + tests/app-page-render.test.ts (69)
• tests/shims.test.ts -t "searchParams|page default|cache id|params/searchParams" (7)

Verdict: correct and well-tested; no blocking issues

The shift from source-token classification to access observation is the right call and matches Next.js semantics. Confirmed correct:

• Query-proof guard placement — readAppPageCacheResponse (app-page-cache.ts:358-371) checks hasRequestSearchParams && !hasQueryInvariantAppPageProof before both the HIT (:373) and STALE (:410) serving paths and before buildAppPageCachedResponse branches on RSC vs HTML, so a query-bearing request can never be served an unproven entry from either artifact.
• Caught-throw still blocks the shared write — runCachedFunctionWithContext rethrows ctx.invalidDynamicUsageError (cache-runtime.ts:745-747) immediately after fn returns and before any cache-write logic, so catching the searchParams throw inside a page-default "use cache" cannot store request-specific output under the omitted-searchParams key. The two shims.test.ts cases pin this.
• Cache-key omission gating — keyed on explicit appPageDefaultExport transform metadata or the non-enumerable APP_PAGE_PROPS_CACHE_KEY_MARKER (cache-runtime.ts), never on object shape or cache-id strings. isAppPageModule (index.ts) is correctly scoped via isInsideDirectory(appDir, ...) + path.parse(name).name === "page" + extensionRegex, so Pages Router files and route.ts/layout.tsx are excluded. Userland-distinct + cache-id-alone tests confirm arbitrary cached functions are unaffected.
• force-static override — hasRequestSearchParams: !isForceStatic && hasSearchParams(...) (app-page-dispatch.ts:530) correctly disables the proof requirement.
• observeReactPromiseStatus split — probe opts in (app-page-probe.ts) so a .status read counts as access; the render-tree thenable does not, so React's internal use() status caching doesn't spuriously bail. The loading-boundary render thenable still observes a real await. Element-builder tests pin both directions.

One non-blocking observation inline. The previously-flagged concerns (generated-entry thickness, interception override over-bail) were addressed in prior commits, and prerender HTML buffering is tracked by #1795.

[ask-bonk]

Non-blocking (fails safe toward dynamic): buildAppPageProbes fans out over the matched route's route.slots, but interception can re-target a different route. resolveAppPageInterceptState (app-page-request.ts:333-342) returns a source-route kind whose rendered tree is sourceRoute, not options.currentRoute — so the slots that actually render in that case are sourceRoute.slots, while the probe still walks the matched route's slots.

The interception page itself is still probed (so the modal that renders is covered), and a searchParams-reading slot under the source route is backstopped by the real render's observation skipping the cache write, so this can't under-bail or serve a stale query response. But it can probe a matched-route slot that doesn't render (over-bail) or miss a source-route slot at probe time (handled by the render backstop). Worth a line in the buildAppPageProbes doc comment noting the source-route interception case, or folding it into the #1798-style follow-up — the current comment only describes the current-route override case.

Also note interception only fires for isRscRequest (app-page-request.ts:324), but findIntercept in probePage() isn't gated on that, so HTML requests probe the interception page unconditionally. Same fail-safe direction.

[james-elicx]

Fixed both points in df0fd12.

isRscRequest gating — buildAppPageProbes now ignores the interception match when !isRscRequest, matching resolveAppPageInterceptState (returns kind: "none" for non-RSC, app-page-request.ts:324). So HTML requests probe every slot's own page and skip the interception probe — no more skipping a slot that actually renders or probing an interception page that doesn't. Added a unit test pinning this.

Source-route interception — verified this case never reaches the probe: in dispatchAppPageInner, resolveAppPageIntercept returns interceptResult.response for the source-route kind and dispatch returns early (app-page-dispatch.ts:763), before probePage is invoked (:866). So by the time buildAppPageProbes runs, any matched interception is the current-route override case (sourceRoute === currentRoute), where intercept.slotKey is a slot in the matched route and probing the matched route's slots is correct. The over-bail/miss you described would require probePage to run in the source-route case, which it can't. Documented this in the buildAppPageProbes doc comment so the invariant is explicit.

vp check clean; probe/dispatch/entry-templates (68) and prod/dev server (202) pass locally.

[ask-bonk]

Review posted successfully. Here's a summary of my review of PR #1788.

Summary

I reviewed the full diff for fix(app-router): track searchParams access for static bailout, read the key source modules (the new observation helper, buildAppPageProbes, the dispatch query-proof guard, cache-runtime page-prop key handling + the invalidDynamicUsageError rethrow, throwIfInsideCacheScope, the element-builder/route-wiring searchParams forwarding, and the prerender buffering), and ran the affected suites locally — all green (72 + 69 + 7 tests).

Verdict: correct and well-tested; no blocking issues

Confirmed correct:

• Query-proof guard runs before both HIT and STALE paths and before the RSC/HTML branch, so a query-bearing request can never be served an unproven entry.
• Caught-throw blocks the shared write — runCachedFunctionWithContext rethrows invalidDynamicUsageError immediately after fn returns and before any cache-write logic.
• Cache-key omission gating is on explicit transform metadata / non-enumerable marker, never on object shape or cache-id strings. isAppPageModule is correctly scoped to app/ and page.* basenames.
• force-static override and the observeReactPromiseStatus probe/render split behave correctly.

One non-blocking inline comment: I flagged the source-route interception case in buildAppPageProbes — the probe fans out over the matched route's slots, but resolveAppPageInterceptState can re-target a different sourceRoute, and interception only fires for RSC requests (the probe's findIntercept isn't gated on that). Both directions fail safe toward dynamic and are backstopped by the real render's observation, so it's a caching-efficiency edge rather than a correctness bug — worth a doc-comment note or folding into the #1798-style follow-up, since the current comment only describes the current-route override case.

The PR's previously-flagged concerns (generated-entry thickness, interception override over-bail) were addressed in prior commits, and prerender HTML buffering is tracked by #1795. Recommended to merge once CI is green.

github run

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: searchParams static bailout via access observation

Reviewed the full diff independently, read the core source modules (app-page-search-params-observation, thenable-params, buildAppPageProbes, the dispatch query-proof guard, cache-runtime key handling + invalidDynamicUsageError rethrow, throwIfInsideCacheScope, cache-proof, the element-builder/route-wiring forwarding, and the prerender buffering), and ran the affected suites locally — all green (probe+dispatch 43, shims cache-key cases 5).

Verdict: correct and well-tested; no blocking issues

The shift from source-token classification to access observation is the right approach and matches Next.js semantics. Independently confirmed:

• Query-proof guard placement — readAppPageCacheResponse checks hasRequestSearchParams && !hasQueryInvariantAppPageProof before both the HIT and STALE serving paths and before buildAppPageCachedResponse branches on RSC vs HTML, so a query-bearing request can never be served an unproven entry. The proof requires completeness: "complete" + searchParams: notObserved via hasCompleteNegativeRequestApiProof.
• Cache-scope rejection ordering — markAppPageSearchParamsAccess() calls throwIfInsideCacheScope("searchParams") first, mirroring headers()/cookies(). The recorded invalidDynamicUsageError is rethrown by runCachedFunctionWithContext immediately after fn returns and before any cache-write logic, so a caught throw inside a page-default "use cache" still blocks the shared write. For the "use cache" + query case the cache read is independently protected by the query-proof guard (the unproven entry is a miss regardless of dynamic marking).
• Cache-key omission gating — keyed on explicit appPageDefaultExport transform metadata or the non-enumerable APP_PAGE_PROPS_CACHE_KEY_MARKER, never on object shape or cache-id strings. isAppPageModule in index.ts is correctly scoped via isInsideDirectory(appDir, ...) + path.parse(name).name === "page" + extensionRegex, so Pages Router files, route.ts/layout.tsx, and userland functions are unaffected.
• force-static override — hasRequestSearchParams: !isForceStatic && hasSearchParams(...) correctly disables the proof requirement.
• observeReactPromiseStatus split — the only thenable-params change is the opt-in status observation; the has/getOwnPropertyDescriptor/ownKeys observation already existed pre-PR, so no key-derivation regression. The probe opts in; the render-tree thenable does not.
• Source-route interception — verified dispatchAppPageInner returns interceptResult.response (line 763) before buildAppPageElement/probePage runs, so buildAppPageProbes only ever sees the current-route override case where probing the matched route's slots is correct. markAppPagePropsForUseCache creates a fresh props object per probe call, so the configurable: false marker can't double-define.

Follow-ups (already tracked, non-blocking)

• Prerender HTML is fully buffered in memory (app-page-render.ts) — build-time only, tracked by #1795.
• The generated-entry thickness and interception over-bail concerns from earlier rounds were both addressed in prior commits (buildAppPageProbes extracted + unit-tested; interception probed in place of the overridden slot).

Recommend merging once CI is green.

[ask-bonk]

Review posted successfully.

Summary

I independently reviewed PR #1788 (fix(app-router): track searchParams access for static bailout) by reading the full diff, the core source modules, and running the affected test suites locally (all passing).

Verdict: correct and well-tested; no blocking issues.

Key things I verified independently rather than trusting the PR description:

1. Query-proof guard runs before both HIT and STALE serving paths and before the RSC/HTML branch — a query-bearing request can never be served an unproven cache entry. The proof requires completeness: complete + searchParams: notObserved.

2. Cache-scope rejection ordering — throwIfInsideCacheScope("searchParams") fires before markDynamicUsage(), and the recorded invalidDynamicUsageError is rethrown before any cache-write logic, so a caught throw inside a page-default "use cache" still blocks the shared write. The query-proof guard independently backstops the "use cache" + query case regardless of dynamic marking.

3. Cache-key omission is gated on explicit transform metadata or a non-enumerable marker, never on object shape or cache-id strings. isAppPageModule is correctly scoped to app/ + page.* basenames.

4. thenable-params change is minimal — only the opt-in status observation was added; the has/getOwnPropertyDescriptor/ownKeys observation already existed, so there's no cache-key-derivation regression.

5. Source-route interception — confirmed dispatchAppPageInner returns the intercepted response before probePage runs, so buildAppPageProbes only sees the current-route override case where probing the matched route's slots is correct.

The earlier-round actionable concerns (generated-entry thickness, interception over-bail) were addressed in prior commits, and the remaining item (prerender HTML buffering) is build-time-only and tracked by #1795.

github run