Skip to content

fix(server): refuse includeContext thisServer/allServers without sampling.context - #3064

Open
StevenLi-phoenix wants to merge 1 commit into
modelcontextprotocol:mainfrom
StevenLi-phoenix:fix/0-sampling-context-capability-gate
Open

StevenLi-phoenix wants to merge 1 commit into
modelcontextprotocol:mainfrom
StevenLi-phoenix:fix/0-sampling-context-capability-gate

Conversation

@StevenLi-phoenix

Copy link
Copy Markdown

Created by Claude Opus-5-5: the code, tests and this description were written entirely by Claude Opus-5-5. Any reply to review comments that is written by AI will say so.

No GitHub issue: this fixes the e2e knownFailure sampling:context:server-gated-by-capability in test/e2e/requirements.ts. It is a few-line bug fix with tests, which CONTRIBUTING lets skip the issue step.

What was broken

In the 2025-11-25 spec, the includeContext values thisServer and allServers are soft-deprecated, and servers SHOULD only use them when the client declares ClientCapabilities.sampling.context (spec: client/sampling#capabilities, and the JSDoc on CreateMessageRequestParams.includeContext / ClientCapabilities.sampling.context).

Server.createMessage() refuses tools / toolChoice when the client did not declare sampling.tools, but it forwarded includeContext unchecked. So a server could send includeContext: 'thisServer' to a client that declared only sampling: {}, and the request reached the client's sampling handler. The e2e manifest recorded this as a knownFailure on all six inMemory / stdio / streamableHttp cells.

Fix

Server.createMessage() now throws SdkError(SdkErrorCode.CapabilityNotSupported, 'Client does not support sampling context capability.') before sending, when includeContext is thisServer or allServers and the client did not declare sampling.context. This mirrors the existing sampling.tools check right above it. 'none' and an omitted includeContext are unaffected.

The check is skipped when the connection negotiated a revision before 2025-11-25 (2025-06-18 and earlier). Those revisions have no sampling.context capability, so a client there cannot declare it, and under those revisions includeContext needs only sampling. Gating them would break servers that are conformant for the revision they negotiated.

The version check is a plain string compare against '2025-11-25'. That is safe here because Server._negotiatedProtocolVersion is never a raw peer string. The initialize handler sets it to the client's requested version only when that version is in this server's own legacy supported-versions list, and otherwise to the first entry of that list (or LATEST_PROTOCOL_VERSION). connect() sets it only to a modern revision that the SDK implements and this instance lists. Revision identifiers are ISO dates, so string order is date order; isModernProtocolVersion() in core-internal/src/shared/protocolEras.ts relies on the same property. On 2026-07-28-era requests, createMessage already throws before this point (_assertPushApiInServedEra).

ctx.mcpReq.requestSampling() and the legacy inputRequired shim's sampling leg both call createMessage(), so they get the same refusal. On the shim, it surfaces as the shim's usual fulfilment failure.

Other SDKs: python-sdk documents the rule in the ServerSession.create_message docstring but does not enforce it. A code search for IncludeContext in the C# and Go SDKs found no such check. This PR follows the behaviour the e2e manifest already specifies for this SDK.

Behaviour change (stated in the changeset): a server that sends thisServer / allServers on a 2025-11-25 connection to a client without sampling.context now gets an error, and must omit includeContext or pass 'none'. The changeset is a patch: VERSIONING.md lists "bug fixes that correct behavior to match documented intent" as not breaking, and this makes the SDK follow the spec's SHOULD and the e2e requirement.

Out of scope, possible follow-ups:

  • The 2026-07-28 MRTR input-request gate (requiredClientCapabilitiesForInputRequest in core-internal) checks sampling.tools but not sampling.context either.
  • A v1.x backport (same gap in src/server/index.ts, same knownFailure).

Open PR #3049 edits the same createMessage() block, so whichever of the two merges second will need a small rebase.

Changes

  • packages/server/src/server/server.ts: the capability check in createMessage().
  • test/e2e/requirements.ts: removed the knownFailure for sampling:context:server-gated-by-capability.
  • test/e2e/scenarios/sampling.test.ts: the sampling:create:include-context passthrough client now declares sampling: { context: {} }, because it sends thisServer / allServers.
  • test/integration/test/server.test.ts: added a test that thisServer and allServers are rejected without sampling.context, and a test that thisServer still passes through on a 2025-06-18 connection.
  • .changeset/sampling-context-capability-gate.md: a patch changeset for @modelcontextprotocol/server.

How I tested it

With the test changes only, on unmodified source:

  • cd test/e2e && npx vitest run scenarios/sampling.test.ts -t context: 6 failed | 4 passed. Each sampling:context:server-gated-by-capability cell fails with expected { ok: true, … } to match object { ok: false, … }.
  • cd test/integration && npx vitest run test/server.test.ts -t includeContext: the rejection test fails with promise resolved "{ model: 'test-model', … }" instead of rejecting.

With the fix (after pnpm --filter @modelcontextprotocol/server build, because e2e imports dist):

  • e2e -t context: 10 passed. These are the six gated cells plus the four sampling:create:include-context cells (inMemory, stdio, streamableHttp, sse).
  • integration -t includeContext: 2 passed.
  • As a check of the version condition, I made the check ignore the negotiated version. The 2025-06-18 test then failed with SdkError: Client does not support sampling context capability. I reverted that change.
  • pnpm typecheck:all and pnpm lint:all pass.
  • Full pnpm -r test: server 674/674, integration 446/446, and e2e 2699 passed | 135 expected fail. The only failures were protocol:timeout:max-total [sse], which fails the same way with main's server.ts on my heavily loaded machine, and examples/shared, whose better-sqlite3 native binding my local install did not build.

…ling.context

The 2025-11-25 spec soft-deprecates includeContext 'thisServer' and
'allServers': servers SHOULD only use them when the client declares
ClientCapabilities.sampling.context. Server.createMessage already refuses
tools/toolChoice to a client without sampling.tools, but forwarded
includeContext unchecked. It now throws CapabilityNotSupported before
sending, on connections negotiated at 2025-11-25 or later. Older
revisions have no sampling.context capability, so they are not gated.

Removes the e2e knownFailure for sampling:context:server-gated-by-capability
and has the include-context passthrough client declare sampling.context.
@StevenLi-phoenix
StevenLi-phoenix requested a review from a team as a code owner October 22, 2026 04:22
@changeset-bot

changeset-bot Bot commented Oct 22, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: b7dea7f

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

This PR includes changesets to release 6 packages
Name Type
@modelcontextprotocol/server Patch
@modelcontextprotocol/client Patch
@modelcontextprotocol/codemod Patch
@modelcontextprotocol/core Patch
@modelcontextprotocol/server-legacy Patch
@modelcontextprotocol/core-internal Patch

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 Oct 22, 2026

Copy link
Copy Markdown

Open in StackBlitz

@modelcontextprotocol/client

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

@modelcontextprotocol/codemod

npm i https://pkg.pr.new/@modelcontextprotocol/codemod@3064

@modelcontextprotocol/core

npm i https://pkg.pr.new/@modelcontextprotocol/core@3064

@modelcontextprotocol/server

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

@modelcontextprotocol/server-legacy

npm i https://pkg.pr.new/@modelcontextprotocol/server-legacy@3064

@modelcontextprotocol/express

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

@modelcontextprotocol/fastify

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

@modelcontextprotocol/hono

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

@modelcontextprotocol/node

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

commit: b7dea7f

@claude claude Bot added the v2 Ideas, requests and plans for v2 of the SDK which will incorporate major changes and fixes label Oct 22, 2026
@arhancanli

Copy link
Copy Markdown

Read the diff (suite not run). The version gate this._negotiatedProtocolVersion < "2025-11-25" is a plain string comparison. It works for date-shaped versions, but any other format sorts by its first character: a value starting with a letter (a draft label, say) sorts after every digit and so is treated as not predating, and a malformed value from a lenient peer is compared the same way. That fails towards enforcing the capability, which is the safe direction, but a test with a non-date negotiated version would pin it, and a comparison against the SDK list of supported versions by index would not depend on string order. Second, the two new tests cover sampling: {} on the current version and 2025-06-18; a client that declares sampling: { context: {} } and sends includeContext: "none" or leaves it unset would show the new check stays out of the way of the common call.

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

v2 Ideas, requests and plans for v2 of the SDK which will incorporate major changes and fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants