fix: preserve multi-valued Set-Cookie headers in route handler ISR cache

[NathanDrake2406]

Summary

• buildAppRouteCacheValue() used headers.forEach() which overwrites same-key entries, dropping all but the last Set-Cookie header from cached responses
• The restore path (buildRouteHandlerCachedResponse) already handles string[] correctly — only the store path was broken
• Uses getSetCookie() to preserve all Set-Cookie headers as a string[], matching the pattern in worker-utils.ts and prod-server.ts

Test plan

• Test: route handler with multiple Set-Cookie headers — all preserved after cache round-trip
• Test: route handler with zero Set-Cookie headers — no regression
• Existing app-router tests pass (275 tests)
• Existing ISR cache tests pass (42 tests)

[pkg-pr-new]

Open in StackBlitz

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

commit: 95b28b2

[james-elicx]

/bigbonk review

[ask-bonk]

Clean fix. The bug analysis is correct: Headers.forEach() joins multi-valued headers into a single comma-separated string for each key, which is the wrong thing for Set-Cookie (cookies with ; attributes get mangled). The fix correctly uses getSetCookie() to preserve each cookie as a separate array entry, matching the existing pattern throughout the codebase (worker-utils.ts, prod-server.ts, deploy.ts).

The type is already Record<string, string | string[]> so the string[] storage is type-safe, the restore path in buildRouteHandlerCachedResponse already handles Array.isArray(value) with append(), and the KV serialization layer round-trips string[] through JSON without issue.

Tests cover the three key scenarios (multiple cookies preserved, zero cookies = no key, full round-trip through store/restore). LGTM.

[ask-bonk]

Nit: the optional chaining (getSetCookie?.()) is defensive against environments where Headers doesn't have getSetCookie (pre-Node 20, some older polyfills). This matches the existing convention in worker-utils.ts:57 and prod-server.ts:173, so it's consistent — just noting it's fine as-is.

[ask-bonk]

Approved the PR. Here's a summary of the review:

Verdict: APPROVE

The fix is correct and well-scoped. Headers.forEach() joins multi-valued headers into a single comma-separated string per key, which corrupts Set-Cookie headers (they contain ; in attribute values). The fix uses getSetCookie() to preserve each cookie individually as a string[], which:

• Is type-safe (CachedRouteValue["headers"] is Record<string, string | string[]>)
• Round-trips through the KV cache serialization layer (JSON handles string[] fine)
• Is already handled by the restore path (buildRouteHandlerCachedResponse uses Array.isArray + append)
• Matches the established pattern in worker-utils.ts, prod-server.ts, and deploy.ts

Tests cover multiple cookies, zero cookies, and a full store/restore round-trip.

github run