Relax monolith ElicitRequestURLParams.elicitation_id for 2026-07-28#2913
Conversation
…2026-07-28 Follow-up to #2912. The re-vendored 2026-07-28 schema dropped elicitationId from ElicitRequestURLParams, but the version-superset monolith model still required it, so parse_server_result rejected a spec-valid 2026 InputRequiredResult embedding a URL elicitation at the monolith step. The v2025_11_25 surface still enforces requiredness for 2025 callers. Also updates the CancelledNotificationParams.request_id docstring to note that 2026-07-28 makes the field required again.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — small, well-tested follow-up that brings the monolith in line with the re-vendored 2026-07-28 schema using the existing superset pattern; the inline comments are a docstring nit and a pre-existing example issue, neither blocking.
Extended reasoning...
Overview
This PR relaxes ElicitRequestURLParams.elicitation_id from str to str | None = None in the version-superset monolith (src/mcp/types/_types.py), updates the CancelledNotificationParams.request_id docstring with a 2026-07-28 version note, adds a test pinning that a 2026-07-28 InputRequiredResult carrying a URL-mode elicitation without elicitationId parses through both the surface and monolith steps, and widens a capture-list type annotation in an interaction test. It is a focused follow-up to #2912, addressing review comments that landed after that merge.
Security risks
None identified. The change loosens a field on the superset model only; the v2025_11_25 surface package still requires elicitationId on the wire, so 2025-11-25 wire validation is unchanged. All SDK-side producers (elicit_url, send_elicit_complete, the elicitation helpers in server/session.py, shared/peer.py, server/mcpserver/context.py) still take a required str and pass it explicitly, so no None can flow out from the typed API.
Level of scrutiny
Moderate-to-low. Although protocol type definitions are shared infrastructure, this change follows the exact pattern already established for CancelledNotificationParams.request_id (required on some wire versions, optional in the monolith superset), is spec-driven (the 2026-07-28 schema dropped the field), and is covered by a new test that reproduces the original review-comment failure. The PR description accurately matches the diff, and the deliberately-deferred helper-signature work is called out with issue references.
Other factors
The two inline findings are a pre-existing example-snippet attribute lookup issue (not introduced here) and a docstring-wording nit about the unqualified same-direction sentence; neither affects runtime behavior or blocks merging. The author reports the full suite, pyright, and ruff passing with 100% coverage.
| 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.""" |
There was a problem hiding this comment.
🟡 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/listen stream names a request issued in the opposite direction). Consider qualifying the same-direction sentence as applying through 2025-11-25, matching the vendored v2026_07_28 surface which drops the same-direction wording entirely.
Extended reasoning...
The inconsistency. The PR extends the request_id docstring on CancelledNotificationParams (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 a subscriptions/listen stream)". 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/cancelled may 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-sent notifications/cancelled names the client's subscriptions/listen request 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 its ClientNotification docstring describes the server-sent listen-termination case explicitly.
Concrete walk-through. On a 2026-07-28 session: (1) the client sends subscriptions/listen with id 42; (2) the server later decides to close the stream and sends notifications/cancelled with requestId: 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 request 42 was 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.
| 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 |
There was a problem hiding this comment.
🟣 Pre-existing issue (not introduced by this PR, but adjacent to the field being relaxed): examples/snippets/clients/url_elicitation_client.py:73 reads getattr(params, "elicitationId", None), but pydantic only exposes the snake_case attribute elicitation_id, so the example always prints Elicitation ID: None even when the server supplied one. Now that elicitation_id can also legitimately be None at 2026-07-28, the example should read params.elicitation_id and handle a real None explicitly.
Extended reasoning...
The bug. examples/snippets/clients/url_elicitation_client.py:73 extracts the elicitation id with elicitation_id = getattr(params, "elicitationId", None). The params object delivered to the client elicitation callback is the monolith mcp.types.ElicitRequestURLParams, a pydantic v2 model whose MCPModel config sets alias_generator=to_camel and populate_by_name=True. In pydantic v2, the camelCase alias is a wire/validation name only — it is never exposed as an instance attribute. The only attribute is the Python field name elicitation_id, so the getattr lookup for "elicitationId" always misses and falls through to its None default.
Code path / proof. Walk through the URL elicitation flow with this example client:
- The server calls
session.elicit_url("Authorize…", "https://example.com/oauth", "auth-001"), which sendselicitation/createwith wire params{"mode": "url", "url": "…", "elicitationId": "auth-001", …}. - The client session validates those params into
ElicitRequestURLParams; the wire keyelicitationIdpopulates the model fieldelicitation_id(value"auth-001"). After validation the instance has the attributeparams.elicitation_id == "auth-001"— and no attribute namedelicitationId. - Line 73:
getattr(params, "elicitationId", None)→ attribute does not exist → returnsNone. - Line 98:
print(f"\n Elicitation ID: {elicitation_id}")→ always printsElicitation ID: None, even though the server suppliedauth-001.
Note line 72 right above it (url = getattr(params, "url", None)) works only because url has no snake/camel divergence; line 73 is simply the wrong attribute name, not intentional defensiveness.
Why nothing catches it. The example is a snippet, not exercised by the test suite for this output, and getattr with a default silently swallows the missing attribute, so there is no error — just a permanently wrong display value in a security-warning prompt that is supposed to show the user what they are consenting to.
Why it is worth mentioning on this PR. This PR relaxes ElicitRequestURLParams.elicitation_id to str | None so the monolith superset accepts 2026-07-28 bodies that omit elicitationId. After this change the correct attribute can also legitimately be None, so the example's unconditional Elicitation ID: None becomes ambiguous: a reader can no longer tell whether the id was genuinely absent (2026-07-28) or just dropped by the wrong attribute lookup (2025-11-25).
Fix. Replace line 73 with elicitation_id = params.elicitation_id if isinstance(params, types.ElicitRequestURLParams) else None (or simply getattr(params, "elicitation_id", None) to keep the existing defensive style), and ideally only print the Elicitation ID line when the value is not None. This is display-only in an example client and not introduced by this diff, so it should not block the PR.
2 participants
Follow-up to #2912, addressing the two review comments that landed after merge.
Motivation and Context
#2912 re-vendored the 2026-07-28 draft schema, which dropped
elicitationIdfromElicitRequestURLParamsand madeCancelledNotificationParams.requestIdrequired again. The regeneratedv2026_07_28surface package picked both up correctly, but the version-superset monolith in_types.pywasn't relaxed to match. As a resultparse_server_result("tools/call", "2026-07-28", ...)would reject a spec-validInputRequiredResultcarrying a URL-mode elicitation at the monolith step (theElicitRequestarm fails for missingelicitationId).What changed
src/mcp/types/_types.py:ElicitRequestURLParams.elicitation_idis nowstr | None = Nonewith a version note. Thev2025_11_25surface package still requires it on the wire, so 2025 callers are unchanged. Same superset patternCancelledNotificationParams.request_idalready uses.src/mcp/types/_types.py:CancelledNotificationParams.request_iddocstring updated to note that 2026-07-28 makes the field required again and narrows it to client-issued requests.tests/types/test_methods.py: new test pinning that a 2026InputRequiredResultwith a URL elicit (noelicitationId) parses through both steps.tests/interaction/lowlevel/test_elicitation.py: capture-list type widened tolist[str | None]to match the relaxed field.The
elicit_url/send_elicit_completehelper signatures inpeer.py/server/session.py/server/mcpserver/context.pyare intentionally not touched here; those are 2025-11-25 API and version-gating them belongs with #2898 / #2904.How Has This Been Tested?
pyright,ruff,./scripts/test(full suite, 100% coverage,strict-no-coverclean). The new test intests/types/test_methods.pyis the repro from the review comment.Breaking Changes
mcp.types.ElicitRequestURLParamscan now be constructed withoutelicitation_id. Existing constructors that pass it are unaffected.Types of changes
Checklist
AI Disclaimer