Extend the interaction suite to the 2026-07-28 spec - #3018
Conversation
There was a problem hiding this comment.
No issues found across 29 files
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Tip: cubic could auto-approve low-risk PRs like this, if it thinks it's safe to merge. Learn more
Re-trigger cubic
There was a problem hiding this comment.
I didn't find any bugs in this change, but it's a large addition (~109 new test functions across 29 files) that pins spec interpretations and 65 documented divergences for the 2026-07-28 revision, so it deserves a human maintainer's review rather than an automated approval.
Extended reasoning...
Overview
This PR extends tests/interaction/ from the 2025-11-25 baseline to the 2026-07-28 spec revision. It touches 29 test files (no src/ changes), adding roughly 109 new test functions covering MRTR (multi-round-trip requests), the modern streamable-HTTP entry, x-mcp-header validation, SEP-2549 caching hints, the RFC 9207 iss validation table, OAuth refresh rotation / AS binding / step-up bounds, JSON Schema dialect handling, and assorted lifecycle/versioning behaviours. It also reworks the requirements manifest (399 → 605 entries) and registers 64 deferred entries plus 65 recorded divergences.
Security risks
None direct: the change is test-only and does not modify any production or security-sensitive code paths. The auth-related tests exercise the OAuth client/server harness but only add coverage; the harness changes (_harness.py, _provider.py) add test-only knobs (code/iss overrides, non-rotating refresh provider) that are scoped to the test suite.
Level of scrutiny
Although the production-risk level is low (tests only), the PR encodes a large number of spec interpretations and intentionally pins current SDK behaviour where it diverges from the 2026-07-28 spec (65 divergences). Whether those interpretations and pins are the right ones is a maintainer judgment call about the project's conformance posture, not something an automated review can sign off on. The sheer size also exceeds what is appropriate for shadow approval.
Other factors
The bug-hunting pass found no bugs. The author reports the suite green over ten consecutive runs with pyright/ruff clean and the manifest↔test coverage gate passing at every commit, and the commits are structured to be individually reviewable. There are no prior reviews or unresolved reviewer comments on the PR.
There was a problem hiding this comment.
I didn't find any bugs, but this is a very large change (~109 new test functions, manifest restructuring, 65 pinned divergences) that warrants a human look — note also that the diff touches src/mcp/server/streamable_http.py (coverage pragma removals), while the description says no src/ changes.
Extended reasoning...
Overview
This PR extends tests/interaction/ from the 2025-11-25 baseline to the 2026-07-28 spec revision: ~109 new test functions across 40 test files, manifest groundwork (requirement id renames, era added_in/removed_in linkage, 64 deferred entries), new test families (MRTR, modern streamable-HTTP entry, x-mcp-header, SEP-2549 caching, RFC 9207 iss validation, AS binding, JSON Schema dialects), and a per-cell coverage gate in test_coverage.py. The only src/ change is removing two coverage pragmas (# pragma: no branch / # pragma: no cover) in streamable_http.py, which is behaviour-neutral but contradicts the PR description's claim that only tests/interaction/ is touched.
Security risks
No production code paths change, so there is no direct security exposure. However, several new tests deliberately pin security-adjacent divergences as green (e.g. the registration application_type echo corruption, the missing capability embed gates, unvalidated Mcp-Param-* headers, the audience-mismatch acceptance) — pinning these is a documented suite policy, but a human should confirm each pinned divergence is intentional rather than accidentally normalising a behaviour the team would prefer to fix first.
Level of scrutiny
Test-only changes are normally low risk, but this PR encodes substantial policy decisions: which spec obligations are deferred, which SDK divergences are accepted and pinned, and how the coverage manifest is structured going forward. Those choices shape future conformance work and the v2 burn-down list, so they merit maintainer review even though the runtime SDK is untouched. The sheer volume (1009 collected cells, 605 manifest entries, a ~440k-character diff) also makes it impractical to certify correctness automatically.
Other factors
The bug-hunting system found no bugs, the author reports ten consecutive green runs with pyright/ruff clean, and the manifest↔test coverage contract is enforced at import. The 15 commits are described as individually reviewable, which should help a human reviewer work through the divergence and deferral decisions commit by commit.
b46806f to
011bbd5
Compare
There was a problem hiding this comment.
I didn't find any bugs in the new tests, but this is a large PR (~109 new test functions across 29 files) that pins many spec divergences as expected behaviour and reshapes the requirements manifest, so it warrants a human maintainer's review of those judgment calls.
Extended reasoning...
Overview
This PR touches only tests/interaction/ — no src/ changes — extending the interaction suite from the 2025-11-25 baseline to the 2026-07-28 spec revision. It adds roughly 109 new test functions across 29 files, plus harness/provider extensions (refresh-token rotation knob, iss/code overrides on the headless OAuth callback, persistent step-up shim) and substantial manifest re-keying (requirement id renames, era links, 64 deferred entries, 65 recorded divergences).
Security risks
There is no production code change, so no direct security exposure. The auth tests do exercise security-relevant client behaviour (RFC 9207 iss validation, refresh-token rotation, AS-binding credential discard, scope step-up), and several tests deliberately pin current SDK behaviour that diverges from the spec (e.g. pre-registered credentials silently replaced rather than erroring, Mcp-Param header values not validated, no embed capability gate). Pinning these as green tests is a policy decision: it makes today's gaps regression-stable, but a maintainer should confirm each pinned divergence is acceptable rather than something that should be fixed before being enshrined in the test suite.
Level of scrutiny
As a tests-only change the blast radius is limited to CI signal, which lowers the bar relative to runtime code. However, the PR is far from mechanical: it encodes interpretations of a draft 2026 spec, decides which behaviours are SDK-defined versus spec-mandated, and restructures the requirements manifest that gates coverage. Those are design/judgment decisions the approval guidelines say a human should weigh in on, and the sheer volume (1000+ added lines of intricate, heavily-documented test logic) exceeds what I can confidently rubber-stamp.
Other factors
The bug-hunting pass found no bugs, the description reports ten consecutive green runs with pyright/ruff clean, and the tests themselves are unusually well documented (each pins a named requirement with rationale). There are no prior reviewer comments to address. The main thing a human reviewer should focus on is the divergence list and the manifest id renames rather than line-by-line test logic.
There was a problem hiding this comment.
I didn't find any bugs in this change — it's test-only (no src/ changes), but at ~109 new test functions across 29 files plus the manifest/divergence bookkeeping it's too large and design-laden for an automated approval, so it deserves a human pass.
Extended reasoning...
Overview
This PR extends tests/interaction/ from the 2025-11-25 baseline to the 2026-07-28 spec revision: new MRTR (multi-round-trip request) coverage, the modern stateless streamable-HTTP entry, x-mcp-header validation, SEP-2549 caching hints, RFC 9207 iss validation, refresh-token rotation, AS-binding migration scenarios, and assorted smaller obligations. It touches 29 files, adds roughly 109 test functions, and registers 64 deferred manifest entries plus 65 recorded divergences. No production (src/) code is modified.
Security risks
None direct — the changes are confined to the test suite and its harnesses. Several tests pin current SDK behaviour that diverges from spec security requirements (e.g. Mcp-Param-* header values not validated against the body, the input_required capability embed gate not enforced, audience mismatch acceptance), which is intentional divergence-pinning rather than a regression introduced here, but a maintainer should confirm those pins match the project's divergence-lifecycle policy and that the referenced burn-down items exist.
Level of scrutiny
Moderate. As test-only code it cannot break production paths, and the bug-hunting pass found no issues. However, the PR encodes many normative judgments — which behaviours are spec-mandated vs SDK-defined, which divergences are acceptable to pin green, and how the manifest's era/supersedes metadata is structured — and those judgments shape future conformance work and issue filing. That is exactly the kind of design-level content a maintainer should sanity-check rather than a bot.
Other factors
The diff is internally consistent with the existing suite's conventions (requirement decorators, snapshot pinning, recording-first assertions), the author reports green runs, pyright/ruff clean, and a coverage-contract check at every commit. There are no prior reviews or unresolved comments on the thread. The size alone (well beyond a simple, mechanical change) is the main reason not to shadow-approve.
b4f5bfb to
1f3b5f0
Compare
There was a problem hiding this comment.
I didn't find any bugs in this change, but it's a large test-suite extension (~109 new test functions across 32 files) that pins 65 recorded divergences from the 2026-07-28 spec — including auth-flow behaviour — so the divergence-pinning decisions deserve a human maintainer's judgment.
Extended reasoning...
Overview
This PR touches only tests/interaction/ — no src/ changes. It extends the interaction suite from the 2025-11-25 baseline to the 2026-07-28 spec revision: MRTR round-trips, the modern streamable-HTTP entry, x-mcp-header handling, SEP-2549 caching hints, server/discover, RFC 9207 iss validation, refresh-token rotation, AS-binding, DCR defaults, and JSON Schema dialect handling. It also updates the requirements manifest and test harnesses (_harness.py, _provider.py, _connect.py).
Security risks
No production code is modified, so there is no direct security exposure. However, several new tests deliberately pin divergent current behaviour green in security-adjacent areas (e.g. audience-mismatch acceptance, pre-registered credentials with a mismatched issuer being silently replaced, the missing capability embed gates, Mcp-Param-* values not validated against the body). Pinning these as passing tests is intentional per the suite's documented divergence lifecycle, but it encodes a policy decision about which spec violations are tolerated for now — a maintainer should confirm those pins match the v2 burn-down plan.
Level of scrutiny
Test-only changes normally warrant lighter scrutiny, but the sheer size (~3,500+ changed lines, ~109 new test functions, 32 files) and the number of behaviour-pinning decisions (65 divergences, era gating of transports, manifest restructuring with supersedes/superseded_by links) make this more than a mechanical addition. The tests themselves look carefully constructed — deterministic, event-driven waits under anyio.fail_after(5), wire-level assertions where the typed API can't observe behaviour — and I found no correctness bugs in the test logic.
Other factors
The bug-hunting pass surfaced no issues, the PR description reports ten consecutive green runs plus clean pyright/ruff, and each divergence pin carries a re-pin instruction in its docstring. The remaining open questions (whether the recorded divergences and deferred entries are the right call for the v2 line) are project-direction decisions rather than code-correctness ones, which is why I'm deferring rather than approving.
1f3b5f0 to
e75603f
Compare
e75603f to
210f9c5
Compare
210f9c5 to
72edf90
Compare
There was a problem hiding this comment.
Beyond the inline findings (both prose-only manifest notes), one other candidate was examined and ruled out this run: the logging:per-request-level:invalid-level deferral's claim that the SDK rejects an unrecognized io.modelcontextprotocol/logLevel value was suspected of being false, but it holds — the 2026-07-28 request _meta model types that key as the LoggingLevel enum (src/mcp-types/mcp_types/_v2026_07_28/__init__.py), so surface validation does reject out-of-range values before any handler runs.
Extended reasoning...
This run's two findings are both nit-severity wording issues in manifest note/deferral prose in tests/interaction/_requirements.py; nothing executes that text and neither blocks merge. The one additional candidate raised — that the logging:per-request-level:invalid-level deferral overstates SDK validation — was verified against the code: the 2026-07-28 request meta model declares io.modelcontextprotocol/logLevel as LoggingLevel | None, so an unrecognized value fails typed parsing as the deferral describes. Recording it here so a later pass does not re-explore it. Given the PR's size (33 files, ~1000 test cells) this stays with human review; no approval decision is implied.
72edf90 to
0042ad8
Compare
0042ad8 to
3a9127d
Compare
| ), | ||
| added_in="2026-07-28", | ||
| note=( | ||
| "The spec scopes the rejection MUST to clients using the Streamable HTTP transport " |
There was a problem hiding this comment.
🟡 [quality] Verbatim note/deferral strings repeated across manifest entries instead of a shared constant sweep:(The spec scopes the rejection MUST to clients using|Era-bound: 2025-era-only legacy interop projection) [also at: tests/interaction/auth/_provider.py:197 - [quality] exchange_refresh_token's new non-rotating branch duplicates the access-token mint and OAuthToken construction ; tests/interaction/transports/test_hosting_http_modern.py:702 - [quality] nit: hand-built `DiscoverResult(supported_versions=[LATEST_MODERN_VERSION], capabilities=ServerCapabilities()); +2 more]
Extended reasoning...
The same ~40-word note ('The spec scopes the rejection MUST to clients using the Streamable HTTP transport ... a deliberate superset, pinned on both cells.') is pasted verbatim into seven client:x-mcp-header:* entries, and the 'Era-bound: 2025-era-only legacy interop projection ...' sentence into five 2025:jsonschema:* entries. The file already establishes the pattern for shared text with the module-level _TASKS_DEFERRAL constant (tests/interaction/_requirements.py top of file); these copies should be hoisted the same way. Concrete cost: when the SDK's gating changes (e.g. the version-gate superset note needs rewording), an author must find and edit 7 (or 5) scattered copies and any missed copy silently goes stale — exactly the stale-note class the prior review already caught elsewhere in this manifest.
Verification: nit — the claim is factually accurate. In /home/claude/python-sdk/tests/interaction/_requirements.py the four-line note "The spec scopes the rejection MUST to clients using the Streamable HTTP transport ... a deliberate superset, pinned on both cells." is pasted verbatim as the note= of seven consecutive client:x-mcp-header:invalid-definition-rejected* entries (lines 1658-1661, 1672-1675, 1686-1
There was a problem hiding this comment.
Partially addressed: the five "Era-bound: 2025-era-only" copies are gone, but the fix is incomplete for the primary site. The x-mcp-header note was reworded yet is still pasted verbatim into all seven client:x-mcp-header:invalid-definition-rejected* entries (tests/interaction/_requirements.py lines 1279, 1292, 1305, 1318, 1331, 1345, 1359) — and that in-place rewording of seven copies is exactly the sync-edit hazard flagged. Hoist the note into a single module-level constant alongside _TASKS_DEFERRAL and reference it from each entry so future rewording touches one place.
3a9127d to
87f0d14
Compare
Take tests/interaction/ from its 2025-11-25 baseline to the 2026-07-28 revision, keeping the suite's rule that every manifest entry is backed by a high-level, public-API scenario test. Manifest groundwork on the existing entries: dead 2026 source URLs repointed at live sections, ids aligned with the typescript-sdk e2e vocabulary, over-claiming behaviour strings narrowed to what their tests prove, and the era pass: behaviours the new revision retired carry removed_in, their replacements carry added_in, and the pairs are linked supersedes / superseded_by (enforced bidirectionally by the coverage gate at import), so no test body branches on a version literal. New coverage, all driven through Client against a server declared inside the test and run across the transport x spec-version matrix where the behaviour allows: multi round-trip requests end to end (the input_required round trip, requestState echo, omission, opacity and the default sealing that rejects a tampered or replayed token, parallel-call isolation, multi-round completion and bounds, all three origin methods, and the MCPServer authoring surface: Resolve with Elicit, Sample and ListRoots dependencies plus input_required pass-through for resources and prompts); the 2026 message-direction rules; server/discover, version modes and the -32022 retry; the per-request envelope; the modern streamable-HTTP entry (response modes, lazy SSE upgrade, the header-validation ladder, cacheable stamping); the x-mcp-header pipeline in both directions; response caching hints; subscriptions/listen (typed events per kind, acknowledgment stamping, per-stream filtering, concurrent demultiplexing, the capacity guard, MCPServer change notifications); stdio serving both eras from the same server; OAuth negotiating the modern era after authenticating, RFC 9207 iss validation, step-up bounds and refresh rotation; resultType handling; JSON Schema 2020-12 output validation; and the era method gate. Where the SDK's current behaviour differs from the spec the test pins today's behaviour and the entry records the divergence with the re-pin instruction in the test docstring. Deferred entries are limited to behaviour the SDK cannot express through its public API, each with a one-line reason. No-Verification-Needed: test-only change
87f0d14 to
611dbaf
Compare
Prove the bare input_required handler actually ran before asserting the rejection; record listen frames unconditionally in the message handler and check the subscription-id stamp outside it, so an unstamped frame fails the test instead of being swallowed by the session's handler guard; run the path-traversal rejection on every era, since the server applies it on handshake connections too; fold the two hand-rolled loud-fail twins into a parametrized fixture test; drop unused client_via_http parameters; say plainly that the max-total-timeout behaviour has no test yet; and make the README's example test one that passes as written. No-Verification-Needed: test-only change
Extends
tests/interaction/from its 2025-11-25 baseline to the 2026-07-28 revision. Onlytests/is touched: the interaction suite itself, a two-paragraph docstring correction intests/_stamp.py, and three one-line model facts intests/test_types.py. Nosrc/changes.Part of #2891.
What's covered
Every new entry is backed by a high-level scenario test: a server declared inside the test, driven through
Client, run across the transport x spec-version matrix wherever the behaviour allows. The 2026-07-28 additions:input_requiredround trip,requestStateecho / omission / opacity and the default sealing that rejects a tampered or replayed token, parallel-call isolation, multi-round completion and bounds, all three origin methods, and the MCPServer authoring surface (ResolvewithElicit,SampleandListRootsdependencies, plusinput_requiredpass-through for resources and prompts) with one handler body serving both eras.server/discover, version modes and the-32022retry, the per-request envelope, the era method gate.x-mcp-headerpipeline in both directions.issvalidation, step-up bounds and refresh rotation,resultTypehandling, and JSON Schema 2020-12 output validation.Groundwork on the existing entries: dead 2026 source URLs repointed at live sections, ids aligned with the typescript-sdk e2e suite vocabulary, over-claiming behaviour strings narrowed to what their tests prove, and the era pass. Behaviours the new revision retired carry
removed_in, their replacements carryadded_in, and the pairs are linkedsupersedes/superseded_by(checked bidirectionally by the coverage gate at import), so the 2025 to 2026 transition is queryable data and no test body branches on a version literal.Numbers
The suite is green over three consecutive runs under
PYTHONWARNDEFAULTENCODING=1, the suite's own files hold 100% line and branch coverage, pyright and ruff are clean, and the manifest / test coverage contract passes.Divergences
Where current SDK behaviour differs from the spec, the suite follows its documented lifecycle: the test pins today's behaviour green, the entry records the divergence, and the test docstring carries the re-pin instruction so the eventual fix is mechanical. Six new divergences are recorded by this branch; the ones most worth knowing about:
input_requiredcapability embed gate is enforced on the MCPServer resolver path only; a hand-built low-levelinput_requiredrequest is transmitted ungated,resultType,scopeparameter,cacheScopethat changes across pages of one paginated listing is accepted rather than refused.Deferred entries are limited to behaviour the SDK cannot express through its public API today, each with a one-line reason; the branch adds no untested promissory entries.
Conformance: behaviours covered both here and in the conformance suite at the CI pin agree.
AI Disclaimer