docs: @deprecated roots/sampling/logging (advisory; SEP-2577)#2071
docs: @deprecated roots/sampling/logging (advisory; SEP-2577)#2071felixweinberger wants to merge 1 commit into
Conversation
felixweinberger
added
the
v2-stateless
@modelcontextprotocol/client
@modelcontextprotocol/server
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
|
@claude review |
1 similar comment
|
@claude review |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| * | ||
| * @deprecated SEP-2577 deprecates the MCP Logging feature (advisory; no wire change). | ||
| * Prefer stderr or OpenTelemetry for server diagnostics. |
There was a problem hiding this comment.
🟡 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.mdhas dedicated### Logging(L315-337),### Sampling(L387-401+), and### Roots(L479-493) sections with full code examples callingctx.mcpReq.log(...),ctx.mcpReq.requestSampling(...), andserver.server.listRoots().docs/client.mdhas### Sampling(L432+) and### Roots(L473+) sections, plussetLoggingLevelguidance at L407-410 andclient.sendRootsListChanged()at L488.docs/migration.mdL622/636/651 documentsrequestSampling/logas the v2 way to do these things.CLAUDE.mdL97-99 lists Sampling and Roots as first-class client features; L208/L230-244 documentsrequestSampling/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
- A new user reads
docs/server.md§ Logging and follows the example:await ctx.mcpReq.log('info', 'Processing started'). - They paste this into their IDE. The
logidentifier renders with a strikethrough, and hovering shows "@deprecated SEP-2577 deprecates the MCP Logging feature (advisory; no wire change). Prefer stderr or OpenTelemetry." - The user goes back to
docs/server.mdto 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. - Same flow applies to
requestSampling,listRoots,setLoggingLevel, andsendRootsListChangedviadocs/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
stderror 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.
| /** | ||
| * Sets the minimum severity level for log messages sent by the server. | ||
| * | ||
| * @deprecated SEP-2577 deprecates the MCP Logging feature (advisory; no wire change). | ||
| */ |
There was a problem hiding this comment.
🟡 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
- User has
@modelcontextprotocol/serverat the current version and code containingawait server.createMessage({...}). - They upgrade to the release that includes this PR.
node_modules/@modelcontextprotocol/server/dist/server/server.d.tsnow carries@deprecated SEP-2577 ...abovecreateMessage.- VS Code / tsserver renders the call site with strikethrough; hovering shows the deprecation note.
- 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).
|
@claude review |
There was a problem hiding this comment.
LGTM — description now matches the diff; remaining inline comments are non-blocking nits that can land as follow-ups.
Extended reasoning...
Overview
Adds @deprecated JSDoc tags (referencing SEP-2577) to the roots/sampling/logging method surfaces across four files: Client.setLoggingLevel/sendRootsListChanged, ServerContext.mcpReq.log/requestSampling, Server.createMessage (all overloads)/listRoots/sendLoggingMessage, and McpServer.sendLoggingMessage. Pure JSDoc additions — zero runtime or wire-level change.
What changed since the last review
The previous 🔴 finding (PR description over-promised type-level deprecations that weren't in the diff) has been addressed: the description now explicitly states capability-field types are Zod-derived and not annotated here, and the inline comment is resolved. The code itself is unchanged (3b256a8).
Security risks
None. Comment-only change; no control flow, validation, auth, or transport logic touched.
Level of scrutiny
Low for the mechanics — JSDoc tags are trivially verifiable and the diff is small. The decision to deprecate these features is driven by SEP-2577 upstream, not this PR. The advisory-only nature means nothing breaks for existing consumers.
Other factors
The two open 🟡 nits from the prior round (prose docs in docs/*.md still recommend the deprecated APIs; no changeset) and the new nit (McpServer.sendLoggingMessage lacks the "prefer stderr/OTel" hint its siblings have) are all explicitly non-blocking and reasonable as follow-ups. None affect correctness of what this PR ships.
| * 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). |
There was a problem hiding this comment.
🟡 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@exampleblock 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
- A user has
const server = new McpServer({...})and typesserver.sendLoggingMessage({ level: 'info', data: 'x' }). - tsserver resolves the call to
McpServer.sendLoggingMessageand renders the JSDoc at mcp.ts:1008-1024. - 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.
- By contrast, a user on the low-level API (
new Server(...)) callingserver.sendLoggingMessage(...)sees the additional "Prefer stderr or OpenTelemetry for server diagnostics." line. - 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 paramsNon-blocking; filing as a nit.
|
@claude review |
| * @deprecated SEP-2577 deprecates the MCP Sampling feature (advisory; no wire change). | ||
| */ | ||
| async createMessage(params: CreateMessageRequestParamsBase, options?: RequestOptions): Promise<CreateMessageResult>; |
There was a problem hiding this comment.
🟡 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
- A user wants streaming sampling and writes:
for await (const msg of server.experimental.tasks.createMessageStream({ messages, maxTokens: 100 })) { … }
- tsserver resolves the call to
ExperimentalServerTasks.createMessageStreamand renders the JSDoc at experimental/tasks/server.ts:73-115. - The hover shows the description, the
@example, and@experimental— no strikethrough, no SEP-2577 mention. - 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)." - Result: two SDK methods that issue the identical
sampling/createMessagewire request give the user contradictory signals about whether Sampling is deprecated.
Why this isn't a duplicate of prior comments
- Prior comment Better validation of data coming over the wire #1 / inline #3226630194 (now resolved) was about types in
packages/core/src/types/; the author addressed it by trimming the PR description to explicitly exclude Zod-derived types.createMessageStreamis not a type — it's a method surface in exactly the same category asServer.createMessage, which the PR does tag. - Prior comment Race condition between transport start and server connection #2 is about
docs/*.mdprose, not JSDoc. - Prior comment Squash commits before a public release #3 is about a missing changeset.
- Prior comment Add
SSEServerTransport.handleMessagebypass #4 / inline #3227479148 is about a missing replacement-hint line onMcpServer.sendLoggingMessage, which already has the@deprecatedtag. Here the@deprecatedtag is absent entirely.
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(
|
@claude review |
1 participant
Adds
@deprecatedJSDoc toServer.createMessage,Server.listRoots,ctx.mcpReq.log, andctx.mcpReq.requestSampling. Capability-field types (ClientCapabilities.{roots,sampling},ServerCapabilities.logging) are Zod-derived and not annotated here.Motivation and Context
Implements SEP-2577 (advisory deprecation of roots/sampling/logging). No wire-level change; deprecation is signal-only.
How Has This Been Tested?
typecheck + lint clean.
Breaking Changes
None.
Types of changes
Checklist
Additional context
On
main(independent). Review guide: https://gist.github.com/felixweinberger/5a48e0f14d5aced39ed6a91b61940711.