fix: next/headers readonly semantics and legacy sync compatibility

[JaredStowell]

Fix next/headers compatibility gaps so vinext matches Next.js more closely for request APIs.

This changes headers() and cookies() to support the legacy sync access pattern while preserving their Promise-based API, makes headers() read-only, and restricts cookies() mutation to route handlers and server actions instead of allowing writes during normal render paths.

What changed

• Added decorated Promise return values for headers() and cookies() so legacy sync access like headers().get(...) and cookies().get(...) works.
• Made the public headers() result read-only and throw on set, append, and delete.
• Split cookie behavior by phase:
  • render: read-only
  • route handler: mutable
  • server action: mutable
• Added phase tracking to the next/headers shim state.
• Updated the App Router runtime to switch request API phase around route handlers and server actions.
• Kept the internal mutable request header object for framework use, while sealing the public user-facing API.

Tests

Added substantial regression coverage across unit, integration, and browser layers.

Unit / shim coverage

Expanded tests/shims.test.ts to cover:

• legacy sync access for headers() and cookies()
• read-only headers() for sync and awaited access
• read-only cookies() during render
• mutable cookies() only in route-handler and action phases
• mutable cookie references becoming read-only again after phase changes
• validation behavior for writable cookie paths in route-handler phase

HTTP integration coverage

Added tests/nextjs-compat/request-apis.test.ts and new fixture routes/pages to verify:

• sync headers() and cookies() access in real App Router page requests
• render-path readonly behavior for both APIs
• route-handler mutability for cookies()
• route-handler readonly behavior for headers()
• mixed sync and awaited access within the same route handler

New fixtures:

• tests/fixtures/app-basic/app/nextjs-compat/request-api-sync/page.tsx
• tests/fixtures/app-basic/app/nextjs-compat/request-api-readonly/page.tsx
• tests/fixtures/app-basic/app/nextjs-compat/api/request-api-sync/route.ts
• tests/fixtures/app-basic/app/nextjs-compat/api/headers-readonly/route.ts
• tests/fixtures/app-basic/app/nextjs-compat/api/request-api-repeat/route.ts

Browser / server-action coverage

Added a Playwright regression in tests/e2e/app-router/server-actions.spec.ts plus a new fixture page/action:

• tests/fixtures/app-basic/app/nextjs-compat/action-cookie-phase/page.tsx
• tests/fixtures/app-basic/app/nextjs-compat/action-cookie-phase/actions.ts

This verifies that:

• a server action can set cookies successfully
• the subsequent render path still treats cookies() as read-only
• the cookie persists and is visible on reload

Validation

Ran:

• pnpm test tests/shims.test.ts tests/nextjs-compat/request-apis.test.ts tests/nextjs-compat/set-cookies.test.ts tests/nextjs-compat/draft-mode.test.ts
• pnpm test tests/nextjs-compat/request-apis.test.ts
• PLAYWRIGHT_PROJECT=app-router pnpm run test:e2e tests/e2e/app-router/server-actions.spec.ts
• pnpm run typecheck
• pnpm run lint

Notes

This preserves vinext’s internal lazy mutable request header optimization for middleware forwarding, but seals the public API so user code can no longer mutate request headers or write cookies from normal render contexts.

[pkg-pr-new]

Open in StackBlitz

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

commit: 275c4ee

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1657d7e3b4

ℹ️ 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".

[james-elicx]

/bigbonk review

[ask-bonk]

Review Summary

Well-structured PR that brings headers() and cookies() significantly closer to Next.js semantics. The decorated Promise approach for sync/async compatibility, phase-aware cookie mutability, and the accessError consolidation for dynamic = "error" are all clean designs. The test coverage is thorough across unit, integration, and E2E layers.

A few items to consider below — one correctness concern with draftMode(), one minor proxy observation, and a nit.

Re: the Codex review concern about error-path sync access — the _decorateRejectedRequestApiPromise implementation correctly handles this. The throwingTarget proxy throws normalizedError on any property access except then/catch/finally, so headers().get("x-test") outside a request context properly throws the context error, not .get is not a function. The test at line 451 of shims.test.ts confirms this.

[ask-bonk]

Very minor: ReadonlyHeadersError.callable is returned as a function reference from the _sealHeaders proxy get trap. This means typeof headers().set returns "function" rather than throwing on property access. Calling .set() correctly throws. This is fine for practical purposes; just noting it differs slightly from Next.js where even accessing the method reference may throw. Not blocking.

[ask-bonk]

The review was submitted successfully. Here's a summary of what I reviewed:

PR #402: fix: next/headers readonly semantics and legacy sync compatibility

I submitted a review with 4 inline comments:

1. draftMode() missing accessError guard (correctness) — headers() and cookies() both check accessError and fail when dynamic = "error" is set, but draftMode() itself succeeds and returns { isEnabled: false }. Only enable()/disable() throw. A page reading just (await draftMode()).isEnabled in a dynamic = "error" context would silently succeed.

2. Double proxy nesting for readonly cookies (optimization nit) — _getReadonlyCookies wraps the mutable cookie proxy in a sealed proxy, creating two proxy layers for reads. Suggested sealing a fresh RequestCookies directly to avoid the extra indirection.

3. ReadonlyHeadersError.callable as property vs accessor (informational) — typeof headers().set returns "function" rather than throwing, which differs slightly from Next.js. Not blocking.

4. Phase tracking pattern (positive) — The try/finally save-and-restore pattern for setHeadersAccessPhase in the RSC entry is well designed and correctly handles throws from redirect().

I also confirmed in the review summary that the Codex reviewer's concern about error-path sync access is properly handled by the _decorateRejectedRequestApiPromise implementation.

github run

[james-elicx]

/bigbonk re-review

[ask-bonk]

Review: next/headers readonly semantics and legacy sync compatibility

Well-designed PR. The decorated Promise approach for sync/async compatibility, phase-gated cookie mutability, and the accessError consolidation are all clean patterns that bring vinext meaningfully closer to Next.js behavior. Test coverage is thorough across unit, integration, and E2E layers.

I verified there's no dev/prod parity concern here — both the Node.js prod server (App Router mode) and the Cloudflare Worker entry delegate entirely to the generated RSC handler from app-rsc-entry.ts, so the setHeadersAccessPhase and accessError changes propagate to all deployment targets automatically.

A few items to address, one of which is a real correctness bug:

Issues

1. [Bug] draftMode() calls markDynamicUsage() before accessError check — In headers() and cookies(), markDynamicUsage() is called after the accessError guard, which is correct (error path = no side effects). But draftMode() calls markDynamicUsage() at line 631 before checking accessError at line 634. This means a dynamic = "error" page still gets its dynamic usage flag set even though the API call will throw. The flag itself is harmless since the page will error out anyway, but it's inconsistent with the headers()/cookies() ordering and could matter if the error is caught.

2. [Stale comment] A comment in app-rsc-entry.ts at line 2434 still references the old "throwing Proxy" pattern. The implementation now uses accessError on the context instead.

3. [Nit] Double proxy for readonly cookies — _getReadonlyCookies wraps _getMutableCookies's result... wait, actually looking again, it doesn't — it creates its own new RequestCookies(ctx.cookies) and seals it directly. This is fine.

Positive notes

• The _decorateRejectedRequestApiPromise implementation correctly handles the Codex reviewer's concern — the throwingTarget proxy throws normalizedError on any property access except then/catch/finally, so headers().get("x-test") outside a request context properly throws the context error rather than .get is not a function.
• The try/finally save-and-restore pattern for phase tracking is correct and handles redirect() throws properly.
• The E2E test for server action cookie writes (action-cookie-phase) tests the full lifecycle including the retry logic for cold compilation, which is pragmatic.
• Caching readonly proxies on the context (ctx.readonlyHeaders, ctx.readonlyCookies, etc.) is a good optimization — avoids creating new Proxy wrappers on every headers()/cookies() call within the same request.

[ask-bonk]

Nit: _getReadonlyCookies creates a new RequestCookies(ctx.cookies) and seals it, while _getMutableCookies creates a different new RequestCookies(ctx.cookies) and wraps it with phase checks. Both share the same underlying ctx.cookies Map, so mutations through the mutable proxy are visible to the readonly one on subsequent reads. This is correct.

However, if _getMutableCookies was already called (e.g., during an action phase), and then the phase reverts to render, cookies() will return _getReadonlyCookies. A user who held a reference to the mutable proxy from the action phase would see their writes reflected in the readonly proxy because they share the Map. This is the intended behavior (the test at line 652 of shims.test.ts confirms it), but worth a brief comment on the shared-Map design to prevent future confusion.

[ask-bonk]

Review submitted successfully. Here's a summary of what I reviewed:

PR #402: fix: next/headers readonly semantics and legacy sync compatibility

I submitted a review with 4 inline comments covering:

1. [Bug] markDynamicUsage() ordering in draftMode() — draftMode() calls markDynamicUsage() before the accessError check (line 631), while headers() and cookies() correctly call it after the guard. Provided a suggestion to reorder so the error path has no side effects.

2. [Nit] _sealHeaders returns a function reference for mutating methods — typeof headers().set returns "function" instead of throwing on access. Only calling .set() throws. Not blocking, but differs from Next.js behavior.

3. [Informational] Shared Map between mutable and readonly cookie proxies — Both _getMutableCookies and _getReadonlyCookies wrap the same ctx.cookies Map, so mutations are visible across both. This is correct and tested, but suggested adding a comment for future maintainability.

4. [Positive] Phase tracking pattern — The try/finally save-and-restore for setHeadersAccessPhase is well-implemented, handles redirect() throws, and is properly isolated via ALS.

I also confirmed there's no dev/prod parity concern — both the Node.js prod server and Cloudflare Worker entry delegate entirely to the RSC handler, so changes propagate automatically. Additionally noted a stale comment at app-rsc-entry.ts:2434 that still references the old "throwing Proxy" pattern.

github run

[james-elicx]

/bigbonk re-review

[ask-bonk]

Review Summary

This is a well-structured PR that meaningfully improves Next.js compatibility for headers() and cookies(). The decorated Promise pattern for sync/async dual access, the phase-gated cookie mutability, and the accessError consolidation for dynamic = "error" are all clean designs.

The test coverage is thorough — unit tests for the proxy behavior, integration tests via real dev-server requests, and an E2E Playwright test for the full server-action cookie lifecycle. I verified there's no dev/prod parity concern since both the Node.js prod server and Cloudflare Worker entry delegate to the generated RSC handler.

Two minor items below, neither blocking.

Notes

• The _decorateRejectedRequestApiPromise correctly handles the error-path sync access concern (raised by the Codex reviewer). The throwingTarget proxy throws normalizedError on any property access except then/catch/finally, so headers().get("x-test") outside a request context throws the proper context error.

• The _sealHeaders and _sealCookies proxies correctly throw on property access (not just on invocation), which matches the tests at lines 258-260 and 322-327 of shims.test.ts. This is the right behavior.

• The try/finally phase tracking with save-and-restore in app-rsc-entry.ts is well-implemented. It handles redirect() throws correctly and isolates concurrent requests via ALS.

• The E2E test retry logic for cold compilation is pragmatic and appropriate for dev-mode testing.

[ask-bonk]

Nit: Both _getMutableCookies and _getReadonlyCookies create separate RequestCookies instances wrapping the same ctx.cookies Map. This is correct — mutations through the mutable proxy are visible to the readonly one — but the shared-Map design is non-obvious. The existing comment at line 399-400 partially explains this. Consider expanding it slightly to note that the shared backing Map is intentional for cross-proxy visibility:

Suggested change

	function _getReadonlyCookies(ctx: HeadersContext): RequestCookies {
	// Keep a separate readonly wrapper so render-path reads avoid the
	// mutable phase-checking proxy while still reflecting the shared
	// cookie map. Both _getMutableCookies and _getReadonlyCookies wrap
	// the same ctx.cookies Map, so mutations made during an action/
	// route-handler phase are visible to subsequent readonly reads.

[ask-bonk]

Very minor: ReadonlyHeadersError.callable and ReadonlyRequestCookiesError.callable are defined but never referenced anywhere in this file. They're dead code — _sealHeaders and _sealCookies both throw directly in their proxy get traps rather than returning these callable sentinels. Consider removing them to avoid confusion.

(Not blocking — they're harmless, but their presence suggests a discarded approach that could mislead future readers.)

[ask-bonk]

Review submitted successfully. Here's a summary:

Approved PR #402 with two minor non-blocking comments:

1. Shared-Map comment enhancement (headers.ts:397) — The _getReadonlyCookies comment partially explains the design, but suggested expanding it to explicitly note that the shared backing ctx.cookies Map between mutable and readonly proxies is intentional for cross-proxy visibility.

2. Dead code: ReadonlyHeadersError.callable / ReadonlyRequestCookiesError.callable (headers.ts:276) — These static methods are defined but never referenced. Both _sealHeaders and _sealCookies throw directly in their proxy get traps rather than returning these callable sentinels. Suggested removing them to avoid confusion.

The overall assessment is positive — the decorated Promise pattern, phase-gated cookie mutability, accessError consolidation, and the thorough test coverage across unit/integration/E2E layers are all well-designed. No dev/prod parity concerns since both production server paths delegate to the RSC handler.

github run