-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Relax monolith ElicitRequestURLParams.elicitation_id for 2026-07-28 #2913
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1865,13 +1865,15 @@ | |
| class CancelledNotificationParams(NotificationParams): | ||
| request_id: RequestId | None = None | ||
| """ | ||
| The ID of the request to cancel. | ||
|
|
||
| This MUST correspond to the ID of a request previously issued in the same direction. | ||
| Required on the wire through 2025-06-18; optional from 2025-11-25. | ||
| Required on the wire through 2025-06-18; optional at 2025-11-25; required again from | ||
| 2026-07-28, where it must name a request the client previously issued (servers send | ||
| this notification only to terminate a `subscriptions/listen` stream). | ||
| """ | ||
| reason: str | None = None | ||
| """An optional string describing the reason for the cancellation.""" | ||
|
Check warning on line 1876 in src/mcp/types/_types.py
|
||
|
|
||
|
|
||
| class CancelledNotification(Notification[CancelledNotificationParams, Literal["notifications/cancelled"]]): | ||
|
|
@@ -1953,16 +1955,17 @@ | |
| message: str | ||
| """The message to present to the user explaining why the interaction is needed.""" | ||
|
|
||
| url: str | ||
| """The URL that the user should navigate to.""" | ||
|
|
||
| elicitation_id: str | ||
| elicitation_id: str | None = None | ||
| """The ID of the elicitation, which must be unique within the context of the server. | ||
|
|
||
| The client MUST treat this ID as an opaque value. | ||
| The client MUST treat this ID as an opaque value. Required on the wire at | ||
| 2025-11-25; removed at 2026-07-28. | ||
| """ | ||
|
|
||
| task: TaskMetadata | None = None | ||
|
Check notice on line 1968 in src/mcp/types/_types.py
|
||
|
Comment on lines
1958
to
1968
Contributor
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. 🟣 Pre-existing issue (not introduced by this PR, but adjacent to the field being relaxed): Extended reasoning...The bug. Code path / proof. Walk through the URL elicitation flow with this example client:
Note line 72 right above it ( Why nothing catches it. The example is a snippet, not exercised by the test suite for this output, and Why it is worth mentioning on this PR. This PR relaxes Fix. Replace line 73 with |
||
| """If specified, the caller requests task-augmented execution (2025-11-25 only).""" | ||
|
|
||
|
|
||
|
|
||
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 unchanged sentence "This MUST correspond to the ID of a request previously issued in the same direction" now contradicts the new 2026-07-28 clause added directly below it, which says the field must name a request the client previously issued (a server-sent cancellation terminating a
subscriptions/listenstream names a request issued in the opposite direction). Consider qualifying the same-direction sentence as applying through 2025-11-25, matching the vendoredv2026_07_28surface which drops the same-direction wording entirely.Extended reasoning...
The inconsistency. The PR extends the
request_iddocstring onCancelledNotificationParams(src/mcp/types/_types.py:1868-1876) with a 2026-07-28 version note: the field is "required again from 2026-07-28, where it must name a request the client previously issued (servers send this notification only to terminate asubscriptions/listenstream)". The sentence directly above it, untouched by the PR, still states without qualification: "This MUST correspond to the ID of a request previously issued in the same direction." Within the same docstring these two MUST-level statements now disagree for the exact case the new sentence describes.Why they conflict. Through 2025-11-25,
notifications/cancelledmay only target a request the sender issued — each side cancels its own requests, so "same direction" is accurate. At 2026-07-28 the rule changes: the spec narrows the notification so that a server-sentnotifications/cancellednames the client'ssubscriptions/listenrequest to terminate that stream. From the server's (sender's) perspective, that is a request issued in the opposite direction. The vendored 2026-07-28 surface confirms this:src/mcp/types/v2026_07_28/__init__.py(CancelledNotificationParams.request_id, around lines 2177-2189) drops the same-direction wording entirely and reads "This MUST correspond to the ID of a request the client previously issued", and itsClientNotificationdocstring describes the server-sent listen-termination case explicitly.Concrete walk-through. On a 2026-07-28 session: (1) the client sends
subscriptions/listenwith id42; (2) the server later decides to close the stream and sendsnotifications/cancelledwithrequestId: 42. Per the new sentence in this docstring (and the vendored schema), this is the only legitimate server-sent cancellation at 2026-07-28 — yet it violates the unqualified first sentence, since request42was issued client→server while the notification travels server→client. A reader applying the first sentence literally would conclude this spec-mandated message is malformed.Why this matters (and why it doesn't matter much). This is the version-superset monolith whose per-field docstrings are the documented mechanism for recording version availability (see the module docstring), so internal consistency of those notes is the point of the file. That said, the impact is purely documentation-level: no validation logic encodes the same-direction rule, and the new clause does spell out the correct 2026 behavior, so a careful reader is unlikely to be misled — which is essentially the refuting reviewer's point. The refutation is fair that a reader gets the right answer for 2026 from the adjacent clause; the residual problem is that the first sentence is phrased as an unqualified MUST that the very next sentence contradicts, rather than as a general rule with a scoped exception. Since the PR is already touching this docstring, tightening it is nearly free.
Suggested fix. Qualify the first sentence, e.g.: "This MUST correspond to the ID of a request previously issued in the same direction (through 2025-11-25)." — or fold it into the version notes: "Through 2025-11-25 this MUST correspond to the ID of a request previously issued in the same direction. Required on the wire through 2025-06-18; optional at 2025-11-25; required again from 2026-07-28, where it must name a request the client previously issued ...". Either keeps the superset docstring accurate for every version it covers.