Skip to content

[v1.x] fix(shared): stop referencing the DOM-only global HeadersInit in normalizeHeaders - #2807

Open
nikolas-sapa wants to merge 1 commit into
modelcontextprotocol:v1.xfrom
nikolas-sapa:fix/headersinit-node-consumers
Open

nikolas-sapa wants to merge 1 commit into
modelcontextprotocol:v1.xfrom
nikolas-sapa:fix/headersinit-node-consumers

Conversation

@nikolas-sapa

Copy link
Copy Markdown

Summary

Fixes #2568 (marked ready for work).

shared/transport.d.ts exports normalizeHeaders(headers: HeadersInit | undefined). HeadersInit is a web-IDL type alias that @types/node structurally cannot bind globally — it binds Headers/Request/RequestInit/Response from undici-types, but a typedef cannot be declaration-merged the way the interface-shaped ones can (see the DefinitelyTyped PR #74414 review noted in the issue). So Node-only consumers compiling with "lib": ["ES2023"], "types": ["node"], "skipLibCheck": false fail on the published declaration:

node_modules/@modelcontextprotocol/sdk/dist/esm/shared/transport.d.ts(7,51): error TS2304: Cannot find name 'HeadersInit'.

This changes the signature to RequestInit['headers'] | undefined — same-behaviour, dependent only on a global @types/node does bind, and exactly what the v2 rewrite already does (#1279). Runtime code is unchanged. This is the 1.x backport the issue asks for (v2 is outside the peer range @anthropic-ai/claude-agent-sdk pins).

Verification

Consumer repro (lib: ["ES2023"], types: ["node"], moduleResolution: "bundler", skipLibCheck: false, importing client/index.js and shared/transport.js):

  • against @modelcontextprotocol/sdk@1.30.0 from npm: dist/esm/shared/transport.d.ts(7,51): error TS2304: Cannot find name 'HeadersInit'. — exactly the issue's error
  • against this branch's emitted dist/esm: tsc --noEmit exits 0, including passing a Headers instance and tuple arrays through normalizeHeaders

Repo typecheck error set is byte-identical with and without this change (verified by diffing tsgo --noEmit output on the clean tree vs the fix — the 8 pre-existing errors are unrelated test-file drift in this environment).

Type of change

  • Bug fix (non-breaking change that fixes an issue)

Checklist

  • Bug fix with the consumer reproduction documented above; changeset included (@modelcontextprotocol/sdk: patch)
  • No runtime behaviour change; single signature + doc comment touched, per the issue's stated blast radius

@nikolas-sapa
nikolas-sapa requested a review from a team as a code owner September 13, 2026 12:34
@changeset-bot

changeset-bot Bot commented Sep 13, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: b606294

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new

pkg-pr-new Bot commented Sep 13, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/@modelcontextprotocol/sdk@2807

commit: b606294

@nikolas-sapa

Copy link
Copy Markdown
Author

Rebased onto the current v3.7 tip (4fb60c4e) — the branch is now 0 behind / 4 ahead, which should clear the BEHIND state.

While rebase-conflicting I reworked the branch to be a single commit, and dropped some noise that had crept in across the previous rebase attempts:

  • 3 commits were local-install artifacts (a 3384-line pnpm-lock.yaml added then deleted, a pnpm-workspace.yaml artifact, two net-empty cleanup commits) — none of that belongs on a maintenance branch.
  • A commit merged upstream v1.x in, and the next commit reverted that merge, which rolled package.json / package-lock.json from 1.30.1 back to 1.30.0. Upstream has since moved to 1.31.0, so that was the only thing actually conflicting. Dropping the revert via rebase --skip cleared both conflicts and removed the version regression. package.json and package-lock.json are now byte-identical to upstream.

The diff against v1.x is now 2 files, +9/−3: the .changeset file, and src/shared/transport.ts:9 changing HeadersInit → RequestInit['headers'] (plus a doc-comment reword so it stops naming the DOM type).

I confirmed the fix is still needed on the current tip — git grep HeadersInit on v1.x still hits the exact line this changes, and #2568 is still open.

On a maintenance branch specifically, this seemed like the low-risk shape of v1.x change:

  • It's type-only. diff -r of the upstream build against this build shows every emitted .js identical except shared/transport.js, and that diff is only the JSDoc comment carried into the output. No runtime behaviour change, no API change.
  • It's a patch bump, and the emitted .d.ts is currently broken. I reproduced normalizeHeaders references the bare global HeadersInit, breaking Node-only consumers with skipLibCheck: false #2568's exact consumer setup (@types/node@22.20.1, lib: ["ES2023"], types: ["node"], skipLibCheck: false) against both builds: upstream's dist/esm/shared/transport.d.ts(7,51) fails with TS2304: Cannot find name 'HeadersInit', this branch's build is clean. So the only workarounds today are an ambient shim, adding "DOM" to lib (which drags every browser global into a Node service), or skipLibCheck: true (which stops checking all declaration files).
  • v2 already made this change via [v2] Мonorepo, sdk-client, sdk-server, sdk-core package split #1279, so this is backporting upstream's eventual answer rather than proposing a new approach.

Checks: npm run build, npm run typecheck, and npm run lint (eslint + prettier --check) all exit 0. npm test is 1692 passed / 1 failed, and that one failure (test/integration-tests/processCleanup.test.ts > should exit cleanly for a server that hangs) is pre-existing — I checked out v1.x detached and it fails there too, alongside a second failure. Full CI on this head is green across build, test on Node 18 and 24, test-e2e on both, and both conformance suites.

One thing I deliberately left alone: src/client/sse.ts:117 and src/client/streamableHttp.ts:184 still annotate locals as HeadersInit & Record<string, string>. Those are erased local annotations and don't reach the published .d.ts (confirmed: zero HeadersInit in dist/esm/*.d.ts after the change), so fixing them would be unrelated scope on a maintenance branch — but happy to do it if you'd rather have them cleaned up.

@nikolas-sapa
nikolas-sapa force-pushed the fix/headersinit-node-consumers branch from 88d7af0 to 0a76b73 Compare October 3, 2026 09:18
@nikolas-sapa

Copy link
Copy Markdown
Author

Two corrections to my earlier comment on this PR, plus a rebase.

The rebase. My previous message said this was rebased onto the v3.7 tip at 4fb60c4e and was "0 behind / 4 ahead". That was wrong in a way worth correcting rather than leaving: 4fb60c4e is not an ancestor of v1.x at all, so that was not a meaningful base for this PR. Rebased onto the actual v1.x tip a8cf5036. It had drifted to 10 behind / 1 ahead by the time I picked it up; it is now 0 behind / 1 ahead.

I also added the [v1.x] prefix to the commit subject, matching the other functional commits on that branch and the PR title. Message only — the tree is byte-identical, verified with HEAD^{tree} across the amend.

The red check was not this PR. pkg-publish failed on a network timeout, not on anything in the diff:

14:15:06  Run npx pkg-pr-new publish
14:21:37  [error] fetch failed
            [cause]: Headers Timeout Error

6m31s wall against a 300s undici headers timeout, in the publish step only. In the same job npm ci, build:esm and build:cjs all completed clean, and the re-run on the new head finished in 13s with ⚡️ Your npm packages are published. So it was a transient pkg.pr.new outage. It's also not a required check on v1.x, so it was never merge-blocking.

Still needed, and confirmed still needed. On the v1.x tip, src/shared/transport.ts:9 still reads headers: HeadersInit | undefined, so the fix is live rather than moot.

Verified on the rebased head: npm ci, npm run build, npm run typecheck, npm run lint all exit 0; npm test → 1852 passed / 1852. The fix is load-bearing — against the same harness and the same @types/node, the base tip fails with TS2304: Cannot find name 'HeadersInit' in the emitted transport.d.ts, while this head exits 0. HeadersInit occurrences in the emitted .d.ts go 2 → 0.

mergeable_state is blocked rather than behind now, and not because of CI — all required contexts are green. The v1.x ruleset requires one approving code-owner review, which is a maintainer action.

@nikolas-sapa
nikolas-sapa force-pushed the fix/headersinit-node-consumers branch from 0a76b73 to b606294 Compare October 5, 2026 19:07
@nikolas-sapa

Copy link
Copy Markdown
Author

Rebased onto the current v1.x tip — b6062944, base fa56d815 ("allow registering after connect()..."). It had drifted 4 behind. 0 conflicts, 1 commit replayed, commit message preserved verbatim.

Base branch re-confirmed as v1.x before and after, so no retargeting question arises for a [v1.x] PR. Before rewriting I verified nothing human could be lost: 0 reviews and 0 inline review comments on this PR. The 4 issue comments are 2 bots (changeset-bot, pkg-pr-new) and 2 of mine, so the rebase could not orphan anyone's feedback.

Worth noting because it is easy to miss: upstream's 4 new commits include #2814, which edits the same file (src/shared/transport.ts) and adds | undefined to the Transport/TransportSendOptions optionals. Git auto-merged it because the hunks are far apart, which is exactly the case where "no conflict" means nothing. I verified both changes survive in the rebased file — normalizeHeaders(headers: RequestInit['headers'] | undefined) at line 9, all 8 | undefined; occurrences from #2814 intact, and setToolRequestHandlers() still present in mcp.ts. Both changesets coexist: normalize-headers-node-consumers.md and fix-transport-exact-optional-property-types.md.

Verified on the rebased head (npm, matching CI's npm ci — pnpm-lock.yaml is main-only on this lineage):

  • npm ci → exit 0, 307 packages
  • npm run build → exit 0
  • npm run typecheck → exit 0
  • npm run lint → exit 0, All matched files use Prettier code style!
  • npm test → exit 0, 56 files / 1858 tests passed, 0 failures, 0 skips

The fix is still load-bearing, and I corrected how I proved it. My previous verification reported "the base tip fails with TS2304, this head exits 0" — but that test only reproduces when DOM is excluded from lib. With TypeScript's default lib set, HeadersInit resolves from lib.dom.d.ts and both sides go green. So the earlier check could have passed against a broken target without detecting anything, which is the failure mode worth being suspicious of.

Re-run with lib pinned to ["ES2022"], which is what a Node consumer actually compiles against:

  • base fa56d815 → transport.d.ts(7,51): error TS2304: Cannot find name 'HeadersInit'. exit 2
  • rebased head b6062944 → exit 0

And I added a positive control — const h: HeadersInit = {} in consumer code, which fails TS2304 in both harnesses. That is what demonstrates the instrument can detect the failure mode, so the green side is a real green rather than a harness that simply never fires.

CI on the new head is green: skipped=1 success=9, matching the pre-drift state. The skip is publish, expected on a PR. mergeable_state is blocked because v1.x requires one approving code-owner review, not because of any check — this repo's workflows do run on fork PRs, so no approval is needed for the runs themselves.

Pushed with --force-with-lease only, lease pinned to 0a76b73d and confirmed via both the API and ls-remote first. No amend, no --no-verify.

I am not going to claim this stays off v1.x — that branch moves too. If it drifts again before review, I will rebase rather than re-comment.

@nikolas-sapa

Copy link
Copy Markdown
Author

Verification run for this PR. Confirmed the base is v1.x (not main): baseRefName=v1.x, and the branch is already current with upstream/v1.x — git rev-list --count HEAD..upstream/v1.x is 0, so git rebase upstream/v1.x reports "Current branch is up to date." (6 ahead of main, which is the wrong base for this PR.)

The fix is load-bearing — verified with a control that fails

The claim is that the published declaration references HeadersInit, which is DOM-only, and breaks Node-only consumers with skipLibCheck: false and no "DOM" lib.

I built the package and compiled a minimal Node-only consumer against the emitted .d.ts — lib: ["ES2022"] (no DOM), types: ["node"], strict: true, skipLibCheck: false, moduleResolution: Node16.

A) This branch (fixed):

$ tsc -p tsconfig.json
EXIT=0

Emitted declaration:

export declare function normalizeHeaders(headers: RequestInit['headers'] | undefined): Record<string, string>;

HeadersInit no longer appears in dist/esm/shared/transport.d.ts.

B) Control — same consumer against the pre-fix declaration (the only edit being the type annotation, reverted and rebuilt):

export declare function normalizeHeaders(headers: HeadersInit | undefined): Record<string, string>;
$ tsc -p tsconfig.json
dist/esm/shared/transport.d.ts(7,51): error TS2304: Cannot find name 'HeadersInit'.
EXIT=2

That is exactly the TS2304: Cannot find name 'HeadersInit' failure reported in #2568, so the control reproduces the reported bug and the branch fixes it. RequestInit is available via @types/node, which is why this resolves in a non-DOM project.

Build / test output

Command Result
npm ci --ignore-scripts added 307 packages
npm run typecheck (tsgo --noEmit) clean, no errors
npm run build (build:esm + build:cjs) both completed, no errors
npm test (vitest, e2e excluded) 1857 passed, 1 failed (56 files)

The one failure is test/integration-tests/processCleanup.test.ts > Process cleanup > should exit cleanly for a server that hangs, plus intermittent [vitest-pool]: Timeout starting forks runner errors on this machine while other jobs were running.

This failure is pre-existing and unrelated. Control — reverting only src/shared/transport.ts to upstream/v1.x, rebuilding, and rerunning that file in isolation:

Code processCleanup.test.ts
This branch (fixed) 2 passed, 1 failed
Pre-fix upstream/v1.x 1 passed, 2 failed

The pre-fix baseline fails more of that file, and the file contains no reference to normalizeHeaders (grep returns nothing). These are load-sensitive child-process timing tests, consistent with the concurrent jobs on this host. Note CI on this PR is green.

Note on scope

The change is type-only: it alters a published .d.ts signature and its doc comment, plus the changeset. No runtime code path changes. HeadersInit and RequestInit['headers'] are the same type in a DOM environment, so DOM consumers are unaffected.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v1 Issues / PRs related to v1.x

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant