feat(core): deprecate sessionId on base Transport/BaseContext; legacySessionId() helper (SEP-2567)

[felixweinberger]

---

Transport.sessionId and BaseContext.sessionId become optional and @deprecated (the base interfaces are session-free per SEP-2567). Concrete SHTTP transports keep their sessionId field. Adds legacySessionId(t) narrowing helper and LegacySessionTransport type.

Motivation and Context

Implements SEP-2567 (sessionless protocol). Session is a transport-specific concern, not a base-protocol one.

How Has This Been Tested?

pnpm test:all + lint clean. No conformance change.

Breaking Changes

ctx.sessionId is now ctx.http?.sessionId (or legacySessionId(transport)). Documented in docs/migration.md.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update

Checklist

• I have read the MCP Documentation
• My code follows the repository's style guidelines
• New and existing tests pass locally
• I have added appropriate error handling
• I have added or updated documentation as needed

Additional context

Depends on R4. Review guide: https://gist.github.com/felixweinberger/5a48e0f14d5aced39ed6a91b61940711.

---

[changeset-bot]

⚠️ No Changeset found

Latest commit: 3cbbcd0

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

[pkg-pr-new]

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/@modelcontextprotocol/client@2060

@modelcontextprotocol/server

npm i https://pkg.pr.new/@modelcontextprotocol/server@2060

@modelcontextprotocol/express

npm i https://pkg.pr.new/@modelcontextprotocol/express@2060

@modelcontextprotocol/fastify

npm i https://pkg.pr.new/@modelcontextprotocol/fastify@2060

@modelcontextprotocol/hono

npm i https://pkg.pr.new/@modelcontextprotocol/hono@2060

@modelcontextprotocol/node

npm i https://pkg.pr.new/@modelcontextprotocol/node@2060

commit: 3cbbcd0

[felixweinberger]

@claude review

[felixweinberger]

@claude review

[claude]

🟡 This PR adds a new public runtime export (legacySessionId) and deprecates Transport.sessionId / BaseContext.sessionId — both changelog-worthy public-API changes — but ships no .changeset/*.md. The changeset-bot notice is neutral (it doesn't decide whether a bump is needed); since this is the terminal PR of the v2-stateless stack and every recent merged PR in this repo has carried a changeset, one should be added here before merge.

Extended reasoning...

What's missing

The diff touches the public API surface in two ways: it re-exports a new runtime symbol legacySessionId from packages/core/src/exports/public/index.ts:73, and it adds @deprecated to Transport.sessionId (transport.ts:121-123) and BaseContext.sessionId / RequestEnv.sessionId (context.ts:114-116, 270-273). The PR is self-labeled a Breaking change and adds a docs/migration.md section, so the author clearly considers this user-facing. Yet git diff --name-only shows no file under .changeset/, and the changeset-bot posted No Changeset found on commit 6122f03.

Why the bot comment alone isn't sufficient

The changeset-bot comment is informational, not prescriptive — it explicitly says "If these changes should not result in a new version, you're good to go." It does not make the judgment call. The reviewer's job here is to make that call: this PR does change the public API (new export + deprecation), so a changeset is warranted. Flagging it in review is not redundant with the bot; it's the human determination the bot defers to.

Why "the stack defers changesets" doesn't hold

It's true that none of the five stacked commits (1916e69 → 6122f03) added a changeset, which could suggest intentional deferral. But this PR is the terminal PR of the stack — its description says "Depends on R4" and nothing depends on it — so this is exactly where a deferred stack-wide changeset would land. Leaving it out here means the entire v2-stateless stack merges with no changelog entry. There is also no repo evidence (CONTRIBUTING.md, prior stacked PRs) that end-of-stack deferral past the final PR is the convention; on the contrary, git log -- .changeset/ shows every recent merged PR (#1855, #1887, #1974, #1901, #1871, #1976, #1907, …) added one, and .changeset/ currently holds ~60 pending entries in alpha pre-mode.

Step-by-step proof

1. ls .changeset/ → 60+ *.md files plus pre.json (pre-release mode active for @modelcontextprotocol/core).
2. git log --oneline -- .changeset/ → every one of the last ~17 merged PRs added a changeset file.
3. git diff main...HEAD --name-only for this PR → docs/migration.md, packages/core/src/exports/public/index.ts, packages/core/src/shared/context.ts, packages/core/src/shared/transport.ts. No .changeset/*.md.
4. packages/core/src/exports/public/index.ts:73 now exports legacySessionId — a net-new public runtime symbol.
5. Therefore the next changesets version run will produce a release of @modelcontextprotocol/core whose CHANGELOG.md has no line for the sessionId deprecation or the new legacySessionId helper, even though docs/migration.md documents it as a breaking change.

Impact

Process-only, not a code-correctness bug. Because pre.json is active and other unreleased changesets exist, the package will still get a version bump regardless — the only loss is the changelog line for this specific change. That's why this is a nit, not a blocker.

Fix

Add a changeset (the bot comment already provides a one-click link), e.g.:

---
"@modelcontextprotocol/core": minor
---

Deprecate `Transport.sessionId` / `BaseContext.sessionId` (SEP-2567 sessionless protocol); add `legacySessionId(transport)` helper for explicit-intent reads during the compat window.

If the intent is one changeset for the whole v2-stateless stack, this terminal PR is the place to add it — covering R1–R5 in one entry is fine, but it needs to land before merge.

[claude]

🟡 The PR description says this "Adds legacySessionId(t) narrowing helper and LegacySessionTransport type", but LegacySessionTransport does not exist anywhere in the repo, and legacySessionId() as implemented is a plain accessor (return t?.sessionId), not a type-predicate narrowing helper. Either the type + predicate signature were intended and got dropped (incomplete implementation), or the PR description is stale and should be corrected.

Extended reasoning...

What the PR description claims vs. what shipped

The PR description states:

Adds legacySessionId(t) narrowing helper and LegacySessionTransport type.

Grepping the entire repo for LegacySessionTransport returns zero matches. Only legacySessionId() was added to packages/core/src/shared/transport.ts:145-147, and it is exported from packages/core/src/exports/public/index.ts. There is no companion type.

Additionally, legacySessionId() is not a narrowing helper. As implemented:

export function legacySessionId(t: Transport | undefined): string | undefined {
    return t?.sessionId;
}

This is a plain accessor that returns string | undefined. A narrowing helper would be a TypeScript type predicate, e.g.:

export type LegacySessionTransport = Transport & { sessionId: string };
export function hasLegacySessionId(t: Transport | undefined): t is LegacySessionTransport {
    return typeof t?.sessionId === 'string';
}

The use of the word "narrowing" together with the mention of a LegacySessionTransport type strongly suggests the type-predicate + companion-type pattern was the original design, and one half was dropped during implementation.

Step-by-step proof

1. PR description (line 3 of the body): "Adds legacySessionId(t) narrowing helper and LegacySessionTransport type."
2. rg LegacySessionTransport across the repo → 0 matches. The type is not defined, not exported, not referenced in migration.md, not mentioned in any JSDoc.
3. packages/core/src/shared/transport.ts:145-147 defines legacySessionId(t: Transport | undefined): string | undefined. The return type is string | undefined, not t is <Something>. Calling legacySessionId(transport) does not narrow transport at the call site — TypeScript still sees transport.sessionId as string | undefined afterwards.
4. The public-exports diff adds only legacySessionId (a value export), no new type export.
5. Therefore the PR description promises two things the diff does not deliver: (a) a LegacySessionTransport type, and (b) narrowing semantics on the helper.

Why existing code doesn't catch this

The shipped JSDoc on legacySessionId() and the docs/migration.md section are internally consistent with what was actually implemented (an "explicit-intent read" accessor) — neither mentions LegacySessionTransport or narrowing. So consumers reading the published docs/types won't be misled. Tests and lint pass because there's nothing wrong with the code that did ship; the gap is between stated intent (PR description) and delivered surface.

Impact

If the accessor-only design is intentional, the impact is limited to PR metadata being inaccurate (PR descriptions don't ship to npm). However, if the type-predicate design was the actual intent for SEP-2567, then this PR is shipping an incomplete public API: downstream code that wants to branch on "is this a session-bearing transport?" has no exported type to narrow to, and the helper doesn't narrow. That would need a follow-up PR (and another public-API addition) to fix.

How to fix

Two valid resolutions — author should pick one:

(a) Ship what was described — add the type and make the helper narrow:

export type LegacySessionTransport = Transport & { readonly sessionId: string };

export function legacySessionId(t: Transport | undefined): t is LegacySessionTransport {
    return typeof t?.sessionId === 'string';
}

…and export LegacySessionTransport from packages/core/src/exports/public/index.ts. (Note this changes the call-site shape from const id = legacySessionId(t) to if (legacySessionId(t)) { t.sessionId }, so migration.md would need a matching update.)

(b) Keep the accessor as-is — update the PR description to drop "narrowing" and "LegacySessionTransport type", e.g. "Adds legacySessionId(t) explicit-intent accessor." The shipped JSDoc/migration docs already match this framing, so no code change is needed.

[felixweinberger]

@claude review

[claude]

🟡 The fix-up commit updated migration-SKILL.md:419 to mark extra.sessionId → ctx.sessionId as (deprecated, SEP-2567) and point to ctx.http?.authInfo / legacySessionId(transport), but the parallel row in migration.md:611 was left as plain | extra.sessionId | ctx.sessionId |. The two mapping tables (which CLAUDE.md:27-30 requires to stay in sync) now diverge on this row, and within migration.md itself the table contradicts the new §"sessionId deprecated" section 19 lines above it — annotate line 611 the same way SKILL.md:419 was annotated.

Extended reasoning...

What's inconsistent

The earlier review on this PR flagged that migration-SKILL.md had not been updated for the SEP-2567 deprecation. Fix-up commit 1127824 addressed that by changing migration-SKILL.md:419 from:

| extra.sessionId | ctx.sessionId |

to:

| extra.sessionId | ctx.sessionId _(deprecated, SEP-2567)_ — prefer ctx.http?.authInfo for identity; for the legacy header value use legacySessionId(transport) |

However, the parallel "Request handler context types" mapping table in docs/migration.md — line 611 — was not given the same treatment. It still reads | extra.sessionId | ctx.sessionId | with no deprecation marker. So the fix-up that was meant to bring the two docs into sync instead moved the divergence from one file to the other.

Why this matters

Two reasons:

1. Cross-file: CLAUDE.md:27-30 requires breaking changes to be documented in both docs/migration.md and docs/migration-SKILL.md, and the two context-field mapping tables are explicit mirrors of each other. After the fix-up they disagree on the extra.sessionId row — the SKILL doc carries the deprecation annotation, the human-readable doc does not.

2. Within-file: migration.md is now internally inconsistent. Lines 576-592 (the new §"sessionId deprecated (SEP-2567)" section added by this PR) say "Do not key application state on sessionId" and point users at ctx.http?.authInfo / legacySessionId(transport). Nineteen lines later, the field-mapping table at line 611 directs v1 users to migrate extra.sessionId → ctx.sessionId with no caveat — the very pattern the section above warns against.

Why existing checks don't catch this

This is a docs-only inconsistency; tsc, lint, and the test suite don't validate that the two migration tables stay in lockstep. The original review comment (inline-comment 3226597186) is now marked resolved because migration-SKILL.md was updated, so the remaining half of the parity requirement is easy to overlook.

Step-by-step proof

1. docs/migration-SKILL.md:419 (post-fix-up) reads: | extra.sessionId | ctx.sessionId _(deprecated, SEP-2567)_ — prefer ctx.http?.authInfo for identity; for the legacy header value use legacySessionId(transport) |
2. docs/migration.md:611 (post-fix-up) reads: | extra.sessionId | ctx.sessionId | — verified by reading the file at HEAD.
3. docs/migration.md:576-592 contains the new §"sessionId deprecated (SEP-2567)" section that says "Do not key application state on sessionId".
4. Therefore: (a) the two parallel tables now differ on the same row, and (b) migration.md:611 maps users onto a field that migration.md:578 deprecates, with no annotation linking the two.

Impact

Low — hence nit, not a blocker. migration.md does contain the full deprecation guidance in the dedicated section immediately above the table, so a sequential reader gets the right information. The table row is also technically correct (the field did move from extra.sessionId to ctx.sessionId; it just happens to also be deprecated now). The only readers misled are those who jump straight to the mapping table without reading the prose section above it.

Fix

One-line change to docs/migration.md:611 to mirror migration-SKILL.md:419:

| extra.sessionId | ctx.sessionId _(deprecated, SEP-2567)_ — prefer ctx.http?.authInfo |

(or the longer form including legacySessionId(transport), matching the SKILL doc verbatim).

[felixweinberger]

@claude review

[felixweinberger]

@claude review

[claude]

🟡 Several shipped server examples still read ctx.sessionId — most notably examples/server/src/simpleTaskInteractive.ts:525,603,623, which keys task state on ctx.sessionId ?? '' (the exact "silently shares state across stateless clients" anti-pattern this PR's new migration section warns against), plus ssePollingExample.ts:47,61,73 and elicitationUrlExample.ts:49,85. Per REVIEW.md's "Tests & docs" checklist (examples should demonstrate the current API on behavior change), these should be moved to legacySessionId(transport) or ctx.http?.authInfo so shipped examples don't contradict the shipped guidance — fine as a follow-up since they still compile and run.

Extended reasoning...

What's inconsistent

This PR marks BaseContext.sessionId as @deprecated (context.ts:114-116) and adds docs/migration.md §"sessionId deprecated (SEP-2567)" which says, in bold: "Do not key application state on sessionId." It also adds legacySessionId(transport) as the explicit-intent compat accessor and points users at ctx.http?.authInfo for identity.

However, three shipped server examples still read ctx.sessionId directly in handlers:

• examples/server/src/simpleTaskInteractive.ts:525,603,623 — passes ctx.sessionId into taskStore.createTask(...), stores { sessionId: ctx.sessionId ?? '' }, and calls taskResultHandler.handle(taskId, server, ctx.sessionId ?? ''). Task state is keyed on the session id.
• examples/server/src/elicitationUrlExample.ts:49,85 — reads ctx.sessionId and throws if absent, keying the elicitation/callback flow on it.
• examples/server/src/ssePollingExample.ts:47,61,73 — console.log keyed on ctx.sessionId (benign, but will show @deprecated strikethrough in IDEs).

Why this belongs in this PR

REVIEW.md's "Tests & docs" checklist says: "Behavior change: assess whether existing examples/ still compile and demonstrate the current API", and Recurring Catches → Completeness says "When a PR replaces a pattern, grep the package for surviving instances of the old form." This PR replaces the ctx.sessionId pattern (→ ctx.http?.authInfo / legacySessionId(transport)) and adds prose telling users not to key state on sessionId, while the repo's own examples continue to demonstrate exactly that pattern. migration.md itself points users at the examples directory under "Need Help?", so the examples are part of the published guidance.

Step-by-step proof (the simpleTaskInteractive case)

1. docs/migration.md:583 (added by this PR) shows the Before anti-pattern as perSession.get(ctx.sessionId) with the comment "silently shares state across stateless clients".
2. examples/server/src/simpleTaskInteractive.ts:603 does sessionId: ctx.sessionId ?? '' — the ?? '' fallback means every stateless (2026-06+) client collapses to the same empty-string key. This is literally the failure mode the new migration section was written to warn about.
3. After this PR, opening that example in an IDE shows ctx.sessionId struck through with the JSDoc "Key application state on ctx.http?.authInfo, not session-id" — yet the surrounding code does precisely the opposite.
4. A user who follows the "Review the examples for updated usage patterns" link in migration.md lands on code that contradicts the section they just read.

Why nothing catches this

tsc and lint pass because the field is deprecated, not removed. There is no test asserting examples are deprecation-free. The four prior review comments on this PR cover migration-SKILL.md sync, the missing changeset, the LegacySessionTransport type, and the migration.md:611 table row — none mention examples/, so this is not a duplicate.

Impact

Low — hence nit. The examples still compile and run correctly against session-mode transports today; the field is deprecated, not removed. The cost is (a) IDE strikethrough noise in shipped sample code, and (b) the SDK's own examples modeling the precise pattern its migration guide tells users to avoid, which undercuts the SEP-2567 guidance this PR is trying to land.

Fix

Update the three examples to match the new guidance:

• For simpleTaskInteractive.ts and elicitationUrlExample.ts (which actually key state): switch to ctx.http?.authInfo for the principal, or — if the example is intentionally demonstrating session-mode compat — to legacySessionId(transport) so the "explicit-intent read" is visible in the sample code.
• For ssePollingExample.ts (log labels only): swap to legacySessionId(transport) or drop the field from the log line.

This is small and could land as a fast-follow, but per REVIEW.md it's worth flagging alongside the deprecation itself.

[claude]

🟡 nit: legacySessionId is a new public runtime export but has no vitest coverage — the sibling helper createFetchWithInit from this same file is covered in packages/core/test/shared/transport.test.ts, so a 2-line smoke test there (legacySessionId(undefined) === undefined; legacySessionId({sessionId:'x'} as Transport) === 'x') would satisfy the REVIEW.md "new behavior has vitest coverage" checklist and lock the public-API contract against accidental removal.

Extended reasoning...

What's missing

legacySessionId is added as a net-new public runtime export at packages/core/src/exports/public/index.ts:76, but rg legacySessionId shows it appears only in transport.ts (definition), exports/public/index.ts (re-export), and the two docs/migration*.md files — zero .test.ts references. REVIEW.md's Tests & docs checklist says "New behavior has vitest coverage including error paths", and this is a public-surface addition.

In-file precedent

The sibling public helper from the same source file, createFetchWithInit, does have vitest coverage in packages/core/test/shared/transport.test.ts. So there is both an established convention for helpers in this module and a ready-made test file to drop the assertions into — no new file needed.

Addressing the counter-argument

It's fair to note that not every public export in this repo carries a smoke test — getDisplayName, isTerminal, takeResult are also untested — so the original claim that "every recent public-API addition ships with a smoke test" overstates the convention. And legacySessionId is genuinely trivial: return t?.sessionId has no error paths and no logic branches beyond optional chaining, so the comparison to createFetchWithInit (which has real header-merge edge cases) is not perfectly apples-to-apples.

That said, the value of a test here is not behavioral verification — it's a public-API contract lock. legacySessionId is exported specifically as a migration affordance that docs/migration.md and docs/migration-SKILL.md now both direct users to call by name; an accidental rename or removal in a future refactor would silently break those docs and downstream code with no test failure. A 2-assertion smoke test costs ~4 lines and catches that. The REVIEW.md checklist item is also explicit, and the natural test home already exists. Hence: worth mentioning, but firmly nit — not a blocker.

Step-by-step proof

1. packages/core/src/shared/transport.ts:145-147 defines export function legacySessionId(t: Transport | undefined): string | undefined { return t?.sessionId; }.
2. packages/core/src/exports/public/index.ts:76 re-exports it: export { createFetchWithInit, legacySessionId } from '../../shared/transport.js'; — net-new public runtime symbol.
3. rg -l legacySessionId → packages/core/src/shared/transport.ts, packages/core/src/exports/public/index.ts, docs/migration.md, docs/migration-SKILL.md. No *.test.ts.
4. rg -l createFetchWithInit packages/core/test → packages/core/test/shared/transport.test.ts (sibling helper from the same source file is tested).
5. REVIEW.md Tests & docs: "New behavior has vitest coverage including error paths".

Fix

Add to packages/core/test/shared/transport.test.ts:

describe('legacySessionId', () => {
    it('returns undefined for undefined transport', () => {
        expect(legacySessionId(undefined)).toBeUndefined();
    });
    it('returns sessionId from a transport that exposes one', () => {
        expect(legacySessionId({ sessionId: 'x' } as Transport)).toBe('x');
    });
});