-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Deprecate roots, sampling, and logging per SEP-2577 #2922
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,8 +25,8 @@ | |
| JSONRPCMessage, | ||
| PromptReference, | ||
| ResourceTemplateReference, | ||
| SamplingMessage, | ||
| SetLevelRequestParams, | ||
| SamplingMessage, # pyright: ignore[reportDeprecated] | ||
| SetLevelRequestParams, # pyright: ignore[reportDeprecated] | ||
| SubscribeRequestParams, | ||
| TextContent, | ||
| TextResourceContents, | ||
|
|
@@ -177,7 +177,7 @@ async def test_sampling(prompt: str, ctx: Context) -> str: | |
| try: | ||
| # Request sampling from client | ||
| result = await ctx.session.create_message( | ||
| messages=[SamplingMessage(role="user", content=TextContent(type="text", text=prompt))], | ||
| messages=[SamplingMessage(role="user", content=TextContent(type="text", text=prompt))], # pyright: ignore[reportDeprecated] | ||
| max_tokens=100, | ||
| ) | ||
|
|
||
|
|
@@ -397,7 +397,7 @@ def test_prompt_with_image() -> list[UserMessage]: | |
| # Custom request handlers | ||
| # TODO(felix): Add public APIs to MCPServer for subscribe_resource, unsubscribe_resource, | ||
| # and set_logging_level to avoid accessing protected _lowlevel_server attribute. | ||
| async def handle_set_logging_level(ctx: ServerRequestContext, params: SetLevelRequestParams) -> EmptyResult: | ||
| async def handle_set_logging_level(ctx: ServerRequestContext, params: SetLevelRequestParams) -> EmptyResult: # pyright: ignore[reportDeprecated] | ||
| """Handle logging level changes""" | ||
| logger.info(f"Log level set to: {params.level}") | ||
| return EmptyResult() | ||
|
|
@@ -418,7 +418,9 @@ async def handle_unsubscribe(ctx: ServerRequestContext, params: UnsubscribeReque | |
|
|
||
|
|
||
| mcp._lowlevel_server.add_request_handler( # pyright: ignore[reportPrivateUsage] | ||
| "logging/setLevel", SetLevelRequestParams, handle_set_logging_level | ||
| "logging/setLevel", | ||
| SetLevelRequestParams, # pyright: ignore[reportDeprecated] | ||
| handle_set_logging_level, | ||
| ) | ||
|
Comment on lines
420
to
424
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can't we just delete this? |
||
| mcp._lowlevel_server.add_request_handler( # pyright: ignore[reportPrivateUsage] | ||
| "resources/subscribe", SubscribeRequestParams, handle_subscribe | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 After this PR removes the Sampling section, README.v2.md still lists
await ctx.session.create_message(messages, max_tokens=...)in the "Context Properties and Methods" list (line 1068) with no deprecation note, leaving an orphaned pointer to a SEP-2577-deprecated feature, and the retained "Logging and Notifications" section likewise carries no deprecation note. Consider removing or annotating thecreate_messagebullet and adding a brief "Deprecated as of 2026-07-28 (SEP-2577)" note to the Logging section for consistency.Extended reasoning...
What the issue is. This PR implements SEP-2577 by deprecating roots, sampling, and logging, and its documentation strategy (per the PR description) is to drop the sampling-specific docs rather than decorate them with suppressions: the README's entire
### Samplingsection, its TOC entry, andexamples/snippets/servers/sampling.pyare all removed. However, the "Context Properties and Methods" list further down still contains the bulletawait ctx.session.create_message(messages, max_tokens=...) - Request LLM sampling/completion(README.v2.md line 1068), and a search of the post-PR README for "deprecat"/"SEP-2577" returns no hits.How it manifests. A reader scanning the Context API list now finds
create_messagerecommended as a normal context capability, but the section that previously explained sampling (and showed the example it linked to) no longer exists anywhere in the README, and nothing tells the reader the feature is deprecated as of 2026-07-28. Similarly, the### Logging and Notificationssection (line 933) is kept in full even though logging is equally deprecated by SEP-2577, with no note. The only place the deprecation is documented isdocs/migration.md.Step-by-step. (1) Pre-PR, README.v2.md had a
### Samplingsection with a fullcreate_messageexample, plus the line-1068 bullet pointing at the same feature. (2) This PR deletes the section and TOC entry but leaves the bullet untouched. (3) Post-PR, line 1068 is the only sampling documentation left in the README, it carries no deprecation context, and grepping the file for "SEP-2577" or "deprecat" finds nothing. (4) The Logging section at line 933 remains fully documented with the same absence of any note.Why this is worth flagging despite the PR's stated rationale. One verifier argued this follows the PR's deliberate "drop the snippet rather than add suppressions" approach and that nothing left in the README is factually wrong (the features remain functional for ≤ 2025-11-25 sessions). That's fair as far as the code-snippet docs go — the Logging section's backing snippet uses
ctx.debug/info/...and needs no suppression. But the leftovercreate_messagebullet is not consistent with that rationale either way: if the policy is "remove sampling docs," the bullet was missed; if the policy is "keep accurate references," it now lacks the context the deleted section used to provide. Either way the inconsistency is introduced by this PR's own doc edits and is trivially cheap to fix.How to fix. Either delete the
create_messagebullet from the Context Properties and Methods list, or annotate it (and optionally the### Logging and Notificationsheading) with a one-line "Deprecated as of 2026-07-28 (SEP-2577); still functional for sessions negotiating ≤ 2025-11-25" note, mirroring the wording already used indocs/migration.md.Severity. Documentation-only and non-blocking — filed as a nit.