fix(start-server-core): keep response context headers on non-2xx responses

[kdelay]

Fixes #7914

A server function that sets a header through the response context and then serializes a non-2xx response loses that header. Only set-cookie survived, so getResponseHeaders().set('x-custom-header', 'true') next to setResponseStatus(401) reached the client as a bare 401.

Cause

h3 merges the response context headers into the final response only when Response.ok is true (prepareResponse returns early on !val.ok). mergeEventResponseHeaders in packages/start-server-core/src/request-response.ts covers the remaining cases, but it only restored set-cookie and returned early when the event carried no cookie.

Change

mergeEventResponseHeaders now merges every event header, using the same precedence h3 applies to 2xx responses: set-cookie is appended, anything else overrides the header already on the response. A response whose headers are immutable (Response.redirect, a passed-through fetch response) is rebuilt instead of mutated; that path previously threw as soon as the event carried a cookie.

Verification

Added packages/start-server-core/tests/request-response.test.ts, driving requestHandler directly. Three of the five cases fail on main and pass with this change; the other two pin the existing 2xx and set-cookie behaviour.

pnpm nx run-many --target=test:unit,test:types,test:eslint,test:build --projects=@tanstack/start-server-core
pnpm nx affected --target=test:unit --exclude="examples/**,e2e/**"

Both are green.

Summary by CodeRabbit

• Bug Fixes

  • Response headers are now consistently preserved when handling successful, error, and redirect responses.
  • Multiple Set-Cookie values are retained during response processing, including cookies from upstream responses.
  • Context-provided headers correctly override existing response headers when applicable.
  • Immutable responses are safely rebuilt with the merged headers.

• Tests

  • Added coverage for successful, error, redirect, immutable, override, upstream response, and cookie-related scenarios.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 6c81ca95-90df-441a-997c-1fefa3cb3df1

📥 Commits

Reviewing files that changed from the base of the PR and between 5936014 and 0f46b86.

📒 Files selected for processing (1)

• packages/start-server-core/tests/request-response.test.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• packages/start-server-core/tests/request-response.test.ts

---

📝 Walkthrough

Walkthrough

The response pipeline merges response-context headers into responses with any status, preserves multiple Set-Cookie values, rebuilds immutable responses, and returns the merged Response. Tests cover success, error, redirect, upstream, and header-precedence cases.

Changes

Response Header Preservation

Layer / File(s)	Summary
Merge response headers packages/start-server-core/src/request-response.ts	Responses now receive event response headers regardless of status. Multiple Set-Cookie values are appended. Immutable responses are rebuilt with the original body and status. Callers use the returned Response.
Validate response handling packages/start-server-core/tests/request-response.test.ts	Tests verify status preservation, header merging, cookie retention from event, direct, and fetched responses, immutable redirects, and response-context header precedence.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the fix for preserving response-context headers on non-2xx responses.
Linked Issues check	✅ Passed	The changes address non-2xx header merging, cookie preservation, immutable responses, precedence, and regression coverage required by issue #7914.
Out of Scope Changes check	✅ Passed	The source and test changes are directly related to response-context header merging and the requirements in issue #7914.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

packages/start-server-core/tests/request-response.test.ts (1)

54-63: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Add coverage for immutable responses with pre-existing cookies.

The immutable-response test only checks a plain header (x-custom-header). It does not verify that a set-cookie header already present on an immutable response survives the rebuild in mergeEventResponseHeaders alongside an event-set cookie. Since cookie preservation combined with header rebuilding is a core objective of this PR, add a test that combines both.

it('preserves an existing set-cookie header when rebuilding an immutable response', async () => {
  const response = await run(() => {
    setCookie('from-event', 'a')
    const redirect = Response.redirect('http://localhost/next', 302)
    redirect.headers.append('set-cookie', 'from-response=b')
    return redirect
  })

  expect(response.headers.getSetCookie()).toEqual([
    'from-response=b',
    'from-event=a; Path=/',
  ])
})

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/start-server-core/tests/request-response.test.ts` around lines 54 -
63, Add coverage alongside the immutable-response test using run, setCookie, and
Response.redirect: create an event cookie, append a pre-existing set-cookie
header to the immutable redirect, and verify the rebuilt response preserves both
cookies in the expected order via getSetCookie.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/start-server-core/src/request-response.ts`:
- Around line 81-116: Update applyEventHeaders to skip content-length alongside
set-cookie when copying eventHeaders, while continuing to append set-cookie
values and preserve all other header merging behavior.

---

Nitpick comments:
In `@packages/start-server-core/tests/request-response.test.ts`:
- Around line 54-63: Add coverage alongside the immutable-response test using
run, setCookie, and Response.redirect: create an event cookie, append a
pre-existing set-cookie header to the immutable redirect, and verify the rebuilt
response preserves both cookies in the expected order via getSetCookie.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: add351aa-662c-4d0b-b0bd-8303e8999556

📥 Commits

Reviewing files that changed from the base of the PR and between 2cb221c and 1d500af.

📒 Files selected for processing (2)

• packages/start-server-core/src/request-response.ts
• packages/start-server-core/tests/request-response.test.ts

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/start-server-core/tests/request-response.test.ts`:
- Around line 76-78: Update the test server setup around server.listen in
beforeAll to wrap the bind in a Promise that rejects on the server's error event
and resolves on successful listening. Attach the error listener before invoking
server.listen so binding failures propagate to beforeAll instead of leaving the
Promise pending.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 14c15da5-0bdd-4ab7-bdf6-19bb7a710f25

📥 Commits

Reviewing files that changed from the base of the PR and between 1d500af and 5936014.

📒 Files selected for processing (1)

• packages/start-server-core/tests/request-response.test.ts