Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions packages/client/src/client/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -718,7 +718,11 @@ export class Client extends Protocol<ClientContext> {
return this._requestWithSchema({ method: 'completion/complete', params }, CompleteResultSchema, options);
}

/** Sets the minimum severity level for log messages sent by the server. */
/**
* Sets the minimum severity level for log messages sent by the server.
*
* @deprecated SEP-2577 deprecates the MCP Logging feature (advisory; no wire change).
*/
Comment on lines +721 to +725
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 nit: Consider adding a patch-level changeset for @modelcontextprotocol/{core,client,server} noting the SEP-2577 advisory deprecation. @deprecated JSDoc is emitted into the published .d.ts, so consumers will see IDE strikethrough on these methods after upgrading — that's a user-visible change worth a CHANGELOG entry. (Precedent: .changeset/register-rawshape-compat.md documented a prior @deprecated addition.) Non-blocking.

Extended reasoning...

What this is

This PR adds @deprecated JSDoc to ~8 public API members across three published packages:

  • @modelcontextprotocol/client: Client.setLoggingLevel, Client.sendRootsListChanged
  • @modelcontextprotocol/server: Server.createMessage (all overloads), Server.listRoots, Server.sendLoggingMessage, McpServer.sendLoggingMessage
  • @modelcontextprotocol/core: ServerContext.mcpReq.log, ServerContext.mcpReq.requestSampling

The diff touches only these four source files; git show --stat 3b256a8 confirms no .changeset/*.md was added.

Why it's user-visible

TypeScript preserves @deprecated tags in emitted .d.ts files. After upgrading, every consumer who calls server.createMessage(...), ctx.mcpReq.log(...), etc. will see IDE strikethrough, and projects with "reportDeprecated": true in tsconfig will get a tsc diagnostic. That's a behaviour change in the consuming developer's experience, even though there is no runtime or wire-level change.

Step-by-step

  1. User has @modelcontextprotocol/server at the current version and code containing await server.createMessage({...}).
  2. They upgrade to the release that includes this PR.
  3. node_modules/@modelcontextprotocol/server/dist/server/server.d.ts now carries @deprecated SEP-2577 ... above createMessage.
  4. VS Code / tsserver renders the call site with strikethrough; hovering shows the deprecation note.
  5. The user checks the package CHANGELOG to understand why → finds nothing, because no changeset was added.

Repo convention

The repo uses changesets as its only changelog mechanism (release.yml runs changesets/action; .changeset/ has 60+ pending entries). Recent merged PRs (#1855 — a type-only fix for exactOptionalPropertyTypes, #1887, #1974) all included changesets. There is also direct precedent for exactly this kind of change: .changeset/register-rawshape-compat.md was added specifically to document a prior @deprecated addition to public overloads.

Caveats / why this is a nit, not a blocker

There is no CI gate enforcing changeset presence, and CONTRIBUTING.md does not mandate one. Some merged commits (e.g. pure refactors) have landed without changesets. So this is a convention/process suggestion, not a hard requirement violation — hence nit, non-blocking.

Suggested fix

---
'@modelcontextprotocol/core': patch
'@modelcontextprotocol/client': patch
'@modelcontextprotocol/server': patch
---

Mark roots/sampling/logging APIs as `@deprecated` per SEP-2577 (advisory only; no wire-level or runtime change).

async setLoggingLevel(level: LoggingLevel, options?: RequestOptions) {
return this._requestWithSchema({ method: 'logging/setLevel', params: { level } }, EmptyResultSchema, options);
}
Expand Down Expand Up @@ -1053,7 +1057,11 @@ export class Client extends Protocol<ClientContext> {
this.setNotificationHandler(notificationMethod, handler);
}

/** Notifies the server that the client's root list has changed. Requires the `roots.listChanged` capability. */
/**
* Notifies the server that the client's root list has changed. Requires the `roots.listChanged` capability.
*
* @deprecated SEP-2577 deprecates the MCP Roots feature (advisory; no wire change).
*/
async sendRootsListChanged() {
return this.notification({ method: 'notifications/roots/list_changed' });
}
Expand Down
5 changes: 5 additions & 0 deletions packages/core/src/shared/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,9 @@ export type ServerContext = BaseContext & {
/**
* Send a log message notification to the client.
* Respects the client's log level filter set via logging/setLevel.
*
* @deprecated SEP-2577 deprecates the MCP Logging feature (advisory; no wire change).
* Prefer stderr or OpenTelemetry for server diagnostics.
Comment on lines +250 to +252
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 The prose docs (docs/server.md §Logging/§Sampling/§Roots, docs/client.md §Sampling/§Roots/setLoggingLevel, docs/migration.md, and CLAUDE.md L97-99) still actively recommend ctx.mcpReq.log, ctx.mcpReq.requestSampling, listRoots(), etc. with no mention of SEP-2577 — after this PR, users following those guides will land on struck-through APIs with no prose explanation. Consider adding a brief "Deprecated (SEP-2577)" callout to the affected sections pointing at the recommended replacements (stderr/OTel for logging, tool params/server config for roots).

Extended reasoning...

What's missing

This PR adds @deprecated JSDoc tags to the runtime API surface for roots/sampling/logging (Server.createMessage, Server.listRoots, Server.sendLoggingMessage, ctx.mcpReq.log, ctx.mcpReq.requestSampling, Client.setLoggingLevel, Client.sendRootsListChanged). However, the prose documentation that teaches users to call these APIs is left untouched. Concretely:

  • docs/server.md has dedicated ### Logging (L315-337), ### Sampling (L387-401+), and ### Roots (L479-493) sections with full code examples calling ctx.mcpReq.log(...), ctx.mcpReq.requestSampling(...), and server.server.listRoots().
  • docs/client.md has ### Sampling (L432+) and ### Roots (L473+) sections, plus setLoggingLevel guidance at L407-410 and client.sendRootsListChanged() at L488.
  • docs/migration.md L622/636/651 documents requestSampling / log as the v2 way to do these things.
  • CLAUDE.md L97-99 lists Sampling and Roots as first-class client features; L208/L230-244 documents requestSampling/createMessage.

A grep for SEP-2577 or deprecat across docs/ returns zero hits related to roots/sampling/logging.

Why this matters

The REVIEW.md "Tests & docs" checklist explicitly says: "Bugfix or behavior change: check whether docs/**/*.md describes the old behavior and needs updating; flag prose that now contradicts the implementation." This PR self-identifies as a "Documentation update" in the checklist, yet the actual prose documentation is the one artifact that wasn't updated.

Step-by-step user impact

  1. A new user reads docs/server.md § Logging and follows the example: await ctx.mcpReq.log('info', 'Processing started').
  2. They paste this into their IDE. The log identifier renders with a strikethrough, and hovering shows "@deprecated SEP-2577 deprecates the MCP Logging feature (advisory; no wire change). Prefer stderr or OpenTelemetry."
  3. The user goes back to docs/server.md to understand why the docs told them to use a deprecated API and what they should do instead — but the prose says nothing about deprecation, SEP-2577, or alternatives.
  4. Same flow applies to requestSampling, listRoots, setLoggingLevel, and sendRootsListChanged via docs/client.md.

The JSDoc tooltip does contain the replacement hint, so users aren't completely stranded — but the prose docs are now actively misleading by recommending deprecated surface without acknowledgement.

Suggested fix

Add a short admonition to each affected section, e.g.:

Deprecated (SEP-2577). The MCP Logging feature is deprecated (advisory; no wire change). Prefer stderr or OpenTelemetry for server diagnostics.

At minimum this should land in docs/server.md (Logging/Sampling/Roots) and docs/client.md (Sampling/Roots/setLoggingLevel). docs/migration.md and CLAUDE.md are lower priority but ideally get the same treatment.

Severity

Filing as a nit rather than blocking: the prose isn't strictly wrong (the features still work; deprecation is advisory-only with no wire change), and the JSDoc tooltips do surface the replacement guidance. It's also plausible this is intentionally scoped for a follow-up PR. But since the PR is labeled as a documentation update and the REVIEW.md checklist explicitly calls for flagging prose that contradicts the implementation, it's worth noting.

This is distinct from any concern about spec.types.ts JSDoc — this is purely about the docs/*.md prose guides.

*/
log: (level: LoggingLevel, data: unknown, logger?: string) => Promise<void>;

Expand All @@ -257,6 +260,8 @@ export type ServerContext = BaseContext & {

/**
* Request LLM sampling from the client.
*
* @deprecated SEP-2577 deprecates the MCP Sampling feature (advisory; no wire change).
*/
requestSampling: (
params: CreateMessageRequest['params'],
Expand Down
1 change: 1 addition & 0 deletions packages/server/src/server/mcp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1010,6 +1010,7 @@ export class McpServer {
* Sends a logging message to the client, if connected.
* Note: You only need to send the parameters object, not the entire JSON-RPC message.
* @see {@linkcode LoggingMessageNotification}
* @deprecated SEP-2577 deprecates the MCP Logging feature (advisory; no wire change).
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 nit: The @deprecated tag on McpServer.sendLoggingMessage omits the "Prefer stderr or OpenTelemetry for server diagnostics." replacement hint that this same PR adds to Server.sendLoggingMessage (server.ts:658-659) and ctx.mcpReq.log (protocol.ts:251-252). Since McpServer is the recommended high-level API (the low-level Server class is itself @deprecated in favour of McpServer), this is the deprecation tooltip most users will actually see — yet it's the only server-side logging surface that doesn't tell them what to use instead. Suggest appending the same line for consistency.

Extended reasoning...

What this is

This PR deprecates three server-side surfaces for the MCP Logging feature, all referencing SEP-2577. Two of them include a one-line migration hint; the third does not:

Surface File Replacement hint?
ctx.mcpReq.log protocol.ts:251-252 ✅ "Prefer stderr or OpenTelemetry for server diagnostics."
Server.sendLoggingMessage server.ts:658-659 ✅ "Prefer stderr or OpenTelemetry for server diagnostics."
McpServer.sendLoggingMessage mcp.ts:1013 ❌ — only "@deprecated SEP-2577 deprecates the MCP Logging feature (advisory; no wire change)."

All three deprecate the same feature with the same SEP reference; only McpServer.sendLoggingMessage lacks the "what to use instead" line.

Why McpServer is the one that matters most

Per server.ts:101, the low-level Server class is itself annotated @deprecated Use McpServer instead for the high-level API. So of the three server-side logging entry points:

  • Server.sendLoggingMessage — on a class users are already steered away from.
  • ctx.mcpReq.log — only reachable inside a request handler.
  • McpServer.sendLoggingMessage — the public method on the recommended server class, with its own @example block in the JSDoc.

The surface most users will hover in their IDE is therefore the only one missing the migration guidance.

Why nothing else covers it

McpServer.sendLoggingMessage is a thin delegate to this.server.sendLoggingMessage(...), but TypeScript/IDE tooltips do not inherit @deprecated text through call delegation — the tooltip shown at the call site comes solely from the JSDoc on McpServer.sendLoggingMessage itself. So the replacement hint on the underlying Server method never reaches McpServer callers.

Step-by-step proof

  1. A user has const server = new McpServer({...}) and types server.sendLoggingMessage({ level: 'info', data: 'x' }).
  2. tsserver resolves the call to McpServer.sendLoggingMessage and renders the JSDoc at mcp.ts:1008-1024.
  3. The hover shows: "Sends a logging message to the client… @deprecated SEP-2577 deprecates the MCP Logging feature (advisory; no wire change)." — strikethrough applied, but no "prefer X instead" guidance.
  4. By contrast, a user on the low-level API (new Server(...)) calling server.sendLoggingMessage(...) sees the additional "Prefer stderr or OpenTelemetry for server diagnostics." line.
  5. Result: the high-level (recommended) API gives less migration help than the low-level (itself-deprecated) API.

Impact

Minor — it's a one-line documentation inconsistency in an advisory-only deprecation. The feature still works; users can still find SEP-2577 if they search for it. But the whole point of advisory @deprecated tags is the IDE tooltip, and this is the one tooltip most users will see. This appears to be an oversight rather than intentional, since the author clearly meant to include replacement guidance for logging (they did so on both other server-side surfaces in this same PR).

Suggested fix

Append the same sentence used on the sibling methods:

      * @see {@linkcode LoggingMessageNotification}
      * @deprecated SEP-2577 deprecates the MCP Logging feature (advisory; no wire change).
+     * Prefer stderr or OpenTelemetry for server diagnostics.
      * @param params

Non-blocking; filing as a nit.

* @param params
* @param sessionId Optional for stateless transports and backward compatibility.
*
Expand Down
15 changes: 15 additions & 0 deletions packages/server/src/server/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -466,18 +466,24 @@
/**
* Request LLM sampling from the client (without tools).
* Returns single content block for backwards compatibility.
*
* @deprecated SEP-2577 deprecates the MCP Sampling feature (advisory; no wire change).
*/
async createMessage(params: CreateMessageRequestParamsBase, options?: RequestOptions): Promise<CreateMessageResult>;

Check warning on line 472 in packages/server/src/server/server.ts

View check run for this annotation

Claude / Claude Code Review

ExperimentalServerTasks.createMessageStream missed by SEP-2577 deprecation sweep

nit: `server.experimental.tasks.createMessageStream(...)` (packages/server/src/experimental/tasks/server.ts:73-168) is a public, documented sampling entry point that issues `sampling/createMessage`, but unlike `Server.createMessage` and `ctx.mcpReq.requestSampling` it carries no `@deprecated SEP-2577` tag — users of the streaming variant get no IDE deprecation signal. Consider adding the same advisory line for consistency. (Distinct from the earlier types-in-core comment, which the author resolv
Comment on lines +470 to 472
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 nit: server.experimental.tasks.createMessageStream(...) (packages/server/src/experimental/tasks/server.ts:73-168) is a public, documented sampling entry point that issues sampling/createMessage, but unlike Server.createMessage and ctx.mcpReq.requestSampling it carries no @deprecated SEP-2577 tag — users of the streaming variant get no IDE deprecation signal. Consider adding the same advisory line for consistency. (Distinct from the earlier types-in-core comment, which the author resolved by scoping types out of the description; this is a method surface in the same category as Server.createMessage, which was tagged.)

Extended reasoning...

What the gap is

This PR's stated scope is to add advisory @deprecated JSDoc to the SDK method surfaces for the SEP-2577-deprecated features (roots / sampling / logging). For Sampling specifically, the diff tags:

Surface File @deprecated?
Server.createMessage (×3 overloads) server.ts:466-491 ✅ added in this PR
ctx.mcpReq.requestSampling protocol.ts:262-264 ✅ added in this PR
ExperimentalServerTasks.createMessageStream experimental/tasks/server.ts:73-168 missing

createMessageStream is the streaming/task-augmented variant of createMessage. It is public, has its own @example block (L80-108), and ultimately issues the same sampling/createMessage wire request (L163). It is reachable via Server.experimental.tasks.createMessageStream(...) (server.ts:194-201) and McpServer.server.experimental.tasks.createMessageStream(...).

Why @experimental doesn't already cover it

The method already carries @experimental (L114), but @experimental and @deprecated are orthogonal signals. @experimental says "this API shape may change without notice"; @deprecated SEP-2577 … says "the underlying protocol feature is being sunset." A user reading the @experimental tag has no reason to infer that Sampling itself is deprecated — they'd reasonably assume only the streaming wrapper is unstable while the feature is fine. The PR clearly treats these as compatible: it tags Server.createMessage, which is also a method on a class that itself carries @deprecated Use McpServer instead, so stacking deprecation reasons is already the pattern here.

Step-by-step proof

  1. A user wants streaming sampling and writes:
    for await (const msg of server.experimental.tasks.createMessageStream({ messages, maxTokens: 100 })) {  }
  2. tsserver resolves the call to ExperimentalServerTasks.createMessageStream and renders the JSDoc at experimental/tasks/server.ts:73-115.
  3. The hover shows the description, the @example, and @experimentalno strikethrough, no SEP-2577 mention.
  4. The same user instead writes the non-streaming form await server.createMessage({ messages, maxTokens: 100 }) → tsserver renders strikethrough and the tooltip "@deprecated SEP-2577 deprecates the MCP Sampling feature (advisory; no wire change)."
  5. Result: two SDK methods that issue the identical sampling/createMessage wire request give the user contradictory signals about whether Sampling is deprecated.

Why this isn't a duplicate of prior comments

Impact

Minor — this is in the experimental namespace (already disclaimed as unstable) and the deprecation is advisory-only with no wire change. But it's a straightforward consistency gap directly within this PR's stated scope, and the repo's review guide explicitly calls out partial migrations ("grep the package for surviving instances of the old form — partial migrations leave sibling code paths with the very gap the PR claims to fix"). Filing as a nit, non-blocking.

Suggested fix

Add the same line used on the sibling sampling surfaces, just below @experimental at L114:

      * @experimental
+     * @deprecated SEP-2577 deprecates the MCP Sampling feature (advisory; no wire change).
      */
     createMessageStream(


/**
* Request LLM sampling from the client with tool support.
* Returns content that may be a single block or array (for parallel tool calls).
*
* @deprecated SEP-2577 deprecates the MCP Sampling feature (advisory; no wire change).
*/
async createMessage(params: CreateMessageRequestParamsWithTools, options?: RequestOptions): Promise<CreateMessageResultWithTools>;

/**
* Request LLM sampling from the client.
* When tools may or may not be present, returns the union type.
*
* @deprecated SEP-2577 deprecates the MCP Sampling feature (advisory; no wire change).
*/
async createMessage(
params: CreateMessageRequest['params'],
Expand Down Expand Up @@ -632,6 +638,12 @@
);
}

/**
* Request the client's filesystem roots.
*
* @deprecated SEP-2577 deprecates the MCP Roots feature (advisory; no wire change).
* Prefer passing scope via tool parameters or server configuration.
*/
Comment thread
felixweinberger marked this conversation as resolved.
async listRoots(params?: ListRootsRequest['params'], options?: RequestOptions) {
return this._requestWithSchema({ method: 'roots/list', params }, ListRootsResultSchema, options);
}
Expand All @@ -642,6 +654,9 @@
* @see {@linkcode LoggingMessageNotification}
* @param params
* @param sessionId Optional for stateless transports and backward compatibility.
*
* @deprecated SEP-2577 deprecates the MCP Logging feature (advisory; no wire change).
* Prefer stderr or OpenTelemetry for server diagnostics.
*/
async sendLoggingMessage(params: LoggingMessageNotification['params'], sessionId?: string) {
if (this._capabilities.logging && !this.isMessageIgnored(params.level, sessionId)) {
Expand Down
Loading