-
Notifications
You must be signed in to change notification settings - Fork 1.9k
feat(core,server): SEP-2577 type-stack @deprecated sweep; ResourceNotFoundError -32602; 2026-era + entry-hosting e2e cells #2333
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
Open
felixweinberger
wants to merge
6
commits into
fweinberger/on-codec-conformance
Choose a base branch
from
fweinberger/on-m12
base: fweinberger/on-codec-conformance
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
d86daec
docs(core,client): complete SEP-2577 @deprecated sweep on public type…
felixweinberger 96b38ad
fix(core,server): resources/read miss answers -32602 on every era; Re…
felixweinberger 2ba3c5a
test(server): pin tools/prompts/resources list deterministic ordering…
felixweinberger 5d330a9
test(e2e): author 2026-era sibling rows for push-style sampling/elici…
felixweinberger 8aecce7
test(e2e): dedicated entry-side cells for HTTP-mechanics and bearer-a…
felixweinberger 40670b2
docs(core): qualify ResourceNotFoundError fromError reconstruction; f…
felixweinberger 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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| --- | ||
| "@modelcontextprotocol/core": minor | ||
| "@modelcontextprotocol/server": major | ||
| "@modelcontextprotocol/client": minor | ||
| --- | ||
|
|
||
| `resources/read` for an unknown URI now answers with JSON-RPC error code `-32602` | ||
| (Invalid Params) on every protocol revision, with `error.data.uri` echoing the | ||
| requested URI. The 2026-07-28 specification requires `-32602`; the v1.x SDK already | ||
| emitted `-32602` on earlier revisions, so v1.x peers see no change. | ||
|
|
||
| This supersedes an interim `-32002` emission that shipped in earlier v2 alphas. The | ||
| era-aware encode seam (`WireCodec.encodeErrorCode`) maps any handler-thrown `-32002` | ||
| to `-32602` on the wire; note that a `-32002` thrown without `data.uri` is emitted as | ||
| a bare `-32602` and is no longer recognizable as resource-not-found — throw | ||
| `ResourceNotFoundError` (or include `data: { uri }`) to preserve the classification. | ||
|
|
||
| `ProtocolErrorCode.ResourceNotFound` (`-32002`) remains importable as receive-tolerated | ||
| vocabulary; clients should accept both `-32602` and `-32002` from peers (the | ||
| specification's backwards-compatibility clause). The new typed `ResourceNotFoundError` | ||
| class carries `data.uri`, and `ProtocolError.fromError` reconstructs it from a `-32602` | ||
| only when `error.data` is exactly `{ uri: string }` (and nothing else), and from a | ||
| legacy `-32002` whenever `data.uri` is a string; a bare `-32002` without `data.uri` | ||
| stays a generic `ProtocolError`. |
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 |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| '@modelcontextprotocol/core': patch | ||
| '@modelcontextprotocol/client': patch | ||
| --- | ||
|
|
||
| Complete the SEP-2577 `@deprecated` sweep on the public type surface (SEP-2596 Tier-1 obligation). Marks the full Logging type stack (`LoggingLevel`, `SetLevelRequest`, `LoggingMessageNotification` and params), the full Sampling type stack (`CreateMessageRequest`/`Result`, `SamplingMessage`, `ModelPreferences`/`ModelHint`, `ToolChoice`, `ToolUseContent`/`ToolResultContent`, and the `includeContext` enum values), the full Roots type stack (`Root`, `ListRootsRequest`/`Result`, `RootsListChangedNotification`), and `registerClient` (Dynamic Client Registration; prefer Client ID Metadata Documents per SEP-991). Mirrors the markers already present on the per-revision reference types. JSDoc only — wire behavior is unchanged; everything remains fully functional during the deprecation window (at least twelve months). |
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
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
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.
🟡 The PR description's 'Small additives + doc closeout' section claims 'empty-string cursor pass-through verified', but no test in this diff or anywhere in the repo at the PR head sends an empty-string cursor — the only new test file (listOrdering.test.ts) never uses a cursor, and the pre-existing pagination tests only exercise non-empty cursors. The migration.md bullet itself is accurate (CursorSchema is z.string(), passed through verbatim), so this is a PR-description-accuracy nit: either add the small empty-cursor pass-through pin or drop the 'verified' wording so reviewers/changelog readers don't assume coverage that doesn't exist.
Extended reasoning...
What the issue is. The PR description's "Small additives + doc closeout" section lists two parallel bullets: "tools/list / prompts/list / resources/list deterministic registration-order verified at the wire (test pins existing behavior)" and "empty-string cursor pass-through verified". The first bullet is backed by a real deliverable in this diff (
packages/server/test/server/listOrdering.test.ts). The second is not backed by anything: no test in this PR or anywhere in the repo at the PR head sends an empty-string cursor.Verification. A repo-wide search at the PR head for an empty-string cursor assertion (
cursor: '',\"cursor\": \"\", "empty cursor") acrosspackages/server/test,packages/core/test,packages/client/test,test/integration, andtest/e2ereturns nothing. The existing cursor coverage (test/e2e/scenarios/pagination.test.tsand related cells) only exercises non-empty/opaque cursors plus an invalid-cursor rejection, and is untouched by this PR. The only new test file in this PR,listOrdering.test.ts, pins list ordering and never sends a cursor at all. The.changesetmention of empty-string cursors elsewhere in the repo belongs to a different PR.Step-by-step reading a reviewer would make. (1) The sibling bullet says "verified at the wire (test pins existing behavior)" and ships a test. (2) The cursor bullet, in the same list, says "empty-string cursor pass-through verified". (3) The natural reading is that a similar small pin was added for the cursor case. (4) Searching the diff for any cursor test finds none — the claimed verification was not added. A future contributor or changelog reader relying on the description would believe coverage exists that doesn't, and a later refactor of cursor handling (e.g. a well-meaning
cursor || undefinednormalization) would not be caught by any pin.Why the shipped artifacts are fine (and why this is only a nit). The new
docs/migration.mdbullet at this location is accurate:CursorSchemaisz.string(),PaginatedRequestParamsSchemausesCursorSchema.optional(), and the client only gates oncursor !== undefined, so an empty-string cursor genuinely passes through verbatim and is not treated as end-of-results. Nothing shipped in the diff is wrong, and there is no runtime impact.Addressing the counter-argument. One reviewer noted the bullet does not literally say "test pins" the way its sibling does, so "verified" could be read as the doc-closeout conclusion recorded in migration.md (schema/code inspection rather than a new test cell). That reading is possible, but the bullet sits in a list whose other verification item explicitly means a test pin, and "How Has This Been Tested" doesn't disambiguate — so at minimum the wording is ambiguous in a way this repo's conventions on prose-vs-diff accuracy try to avoid. Either resolution is cheap.
How to fix. Prose-only, pick one: (a) drop or reword the bullet (e.g. "empty-string cursor pass-through confirmed by inspection — CursorSchema is z.string(), no normalization on the path"), or (b) add the small pin the wording implies — a list request with
cursor: ''reaching the handler verbatim (and/or aPaginatedRequestParamsSchema.parse({ cursor: '' })round-trip), a few lines next to the new listOrdering test. Not blocking either way.