Skip to content

Omit an unset experimental capability from the initialize result - #3614

Merged
maxisbey merged 1 commit into
mainfrom
3254-omit-empty-experimental
Oct 2, 2026
Merged

maxisbey merged 1 commit into
mainfrom
3254-omit-empty-experimental

Conversation

@maxisbey

@maxisbey maxisbey commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Fixes #3254.

A server with no experimental capabilities configured sent "experimental": {} in its initialize result but left the field out of its server/discover result, so the same server looked different depending on how the client connected. This makes initialize omit it too.

What changes on the wire

  • An unconfigured server's initialize result no longer carries "experimental": {}.
    • This applies to the lowlevel Server and to MCPServer, on every transport.
    • v1 has always sent it, so this is a deliberate change to the legacy-path output.
  • server/discover is unchanged.
  • A map passed to create_initialization_options(experimental_capabilities=...) is still sent as given, including an explicit {}.

Why

  • The field is optional in the schema for both results, and an absent field and an empty object both mean "no experimental capabilities".
  • server/discover already omits it, and the docs already show capabilities without it.
  • The other SDKs I checked (TypeScript, C#, Go, Rust, Ruby, PHP) omit it on both paths when nothing is configured.

Who could notice

  • Client code that reads capabilities.experimental from a legacy connection without handling None, for example caps.experimental.get(...).
    • The field has always been typed as optional, and a modern connection already returns None here.
    • caps.experimental or {} works on both.

What this does not do

The change

  • create_initialization_options() passed experimental_capabilities or {} to get_capabilities(); it now passes the argument through unchanged.
  • No signature, default or new name anywhere. get_capabilities() is untouched.

How it was checked

  • New test in tests/server/lowlevel/test_server_discover.py: a bare server connected once through initialize and once through server/discover reports no experimental either way.
  • Updated the three snapshots that pinned experimental={} on the legacy path (tests/client/test_client.py, tests/interaction/lowlevel/test_initialize.py).
  • The new test and the nine updated cases fail on main and pass with the change.
  • ./scripts/test passes with 100% coverage; ruff and pyright are clean.

AI Disclaimer

`create_initialization_options()` turned a missing experimental map into
`{}`, so a server with nothing configured sent `"experimental": {}` on
`initialize` while its `server/discover` result left the field out. Pass
the argument through unchanged so both results omit it.

An experimental map the caller passes explicitly, including an empty
one, is still sent as given.

Fixes #3254

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 4 files

Re-trigger cubic

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline findings, I also checked for in-tree readers of ServerCapabilities.experimental that assume a dict — there are none; the only .experimental consumer in src/ is check_client_capability in src/mcp/server/connection.py, which reads client capabilities and already guards None. The passthrough also matches get_capabilities()'s own None default, so the server/discover and initialize paths now share one code path for the unset case.

Extended reasoning...

One-line change in src/mcp/server/lowlevel/server.py removing the or {} coercion in create_initialization_options, plus three hand-trimmed inline snapshots and one new in-memory Client test; no security-sensitive surface. The inline findings cover the migration-doc drift and the v2 compatibility-contract question, which are maintainer decisions, so a human should weigh those; the ruled-out note records that no SDK code path depends on the old {} value.

capabilities=self.get_capabilities(
notification_options or NotificationOptions(),
experimental_capabilities or {},
experimental_capabilities,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 nit (optional): Readers migrating from v1 are told the v2 initialize result differs only in server_version, which is no longer true after this change. docs/migration.md:1287 says of create_initialization_options() that "the only value that differs is server_version", but server.py:548 now also drops the v1 "experimental": {} from the legacy initialize result when nothing is configured. Fix: update that migration.md sentence (correcting an existing entry is allowed) to also note that an unconfigured server omits experimental instead of sending {}, so caps.experimental can be None on a legacy connection.

Why this was flagged

The PR removes experimental_capabilities or {} at src/mcp/server/lowlevel/server.py:548, so a lowlevel Server or MCPServer with no experimental capabilities now returns ServerCapabilities(experimental=None) from the initialize handshake; on the base branch and in v1 it sent "experimental": {}. docs/migration.md:1287 still states that create_initialization_options() builds the same InitializationOptions as v1 and "the only value that differs is server_version". A v1 user following that page and keeping code like caps.experimental.get(...) on a legacy connection now gets AttributeError on None, and the migration doc does not warn them. AGENTS.md requires docs/ to be updated in the same PR when user-visible behaviour changes, and permits correcting existing migration.md entries; no docs file is touched in this diff.

Verification: nit. Triggering condition: any v1 user reading the migration page for an unconfigured lowlevel Server. Mechanism: the diff changes src/mcp/server/lowlevel/server.py:548 so an unconfigured server now builds experimental=None where the base built experimental={}. docs/migration.md:1287 still reads "the only value that differs is server_version", so that sentence is now inaccurate. The diff touches no file under docs/.

capabilities=self.get_capabilities(
notification_options or NotificationOptions(),
experimental_capabilities or {},
experimental_capabilities,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 nit (optional): AGENTS.md says a change to a released v2 API's observable behaviour is a maintainer design decision that should generally be avoided: dropping or {} here makes create_initialization_options() (and so every legacy initialize result from Server/MCPServer) omit experimental instead of sending {}, which v1 and 2.x have always emitted. Fix: either keep the {} coercion on the legacy path (and let server/discover stay as-is), or have a maintainer explicitly sign off on the wire change and record it where 2.x behaviour changes are noted.

Why this was flagged

Nothing fails in the SDK itself; the guard is the 2.x compatibility contract. A client on a legacy (<=2025-11-25) connection that reads caps.experimental.get(...) or caps.experimental[...] without a None check will now raise AttributeError/TypeError where it previously got an empty dict. Mitigating facts the maintainer may weigh: the field has always been typed dict | None; server/discover already omitted it, so modern connections returned None already; docs/client/index.md already states an absent capability is None; the PR description lists this as a deliberate legacy-path change and cites other SDKs omitting it; the author is a listed project author in pyproject.toml. An explicit experimental_capabilities={} passed by a caller is still sent as {}.

Verification: AGENTS.md (base 17aaf25, "Branching Model") states: "v2 is released; its public API is a compatibility contract for the 2.x line. Removals, renames, or any change to an existing API's signature or observable behaviour ... is a design decision a maintainer makes explicitly, and should generally be avoided."

@maxisbey
maxisbey merged commit 449070c into main Oct 2, 2026
38 checks passed
@maxisbey
maxisbey deleted the 3254-omit-empty-experimental branch October 2, 2026 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[v2] MCPServer reports empty experimental capabilities as {} via initialize but None via server/discover

1 participant