Skip to content

Add a pluggable server extension API with MCP Apps#3003

Open
Kludex wants to merge 10 commits into
mainfrom
extension-api-sep-2133
Open

Add a pluggable server extension API with MCP Apps#3003
Kludex wants to merge 10 commits into
mainfrom
extension-api-sep-2133

Conversation

@Kludex

@Kludex Kludex commented Jun 26, 2026

Copy link
Copy Markdown
Member

Summary

A pluggable, opt-in extension API for MCPServer (SEP-2133), with MCP Apps as the reference extension. (Tasks was dropped from this PR — see below.)

An extension is a narrow base class (HTTPX Transport/Auth style) whose methods default, so it overrides only what it needs:

class Extension:
    identifier: str
    def settings(self) -> dict[str, Any]: ...           # advertised under capabilities.extensions
    def tools(self) -> Sequence[ToolBinding]: ...        # additive
    def resources(self) -> Sequence[ResourceBinding]: ...
    def methods(self) -> Sequence[MethodBinding]: ...     # new request verbs
    async def intercept_tool_call(self, params, ctx, call_next): ...  # wraps tools/call

You opt in declaratively at construction:

mcp = MCPServer("demo", extensions=[Apps()])

The server applies a closed set of contribution kinds and never hands itself to an extension.

What's included

  • Extensions capability map (ServerCapabilities.extensions, SEP-2133): threaded through get_capabilities/create_initialization_options, advertised over server/discover, checked in Connection.check_capability, and advertised client-side via Client(extensions=...).
  • Apps (io.modelcontextprotocol/ui) — additive: @apps.tool(resource_uri=...) binds a tool to a ui:// resource via _meta.ui.resourceUri; client_supports_apps(ctx) drives the SEP-2133 text-only fallback.
  • A runnable apps example story (in-memory + http-asgi) and a migration-guide entry.

Tasks deferred to a follow-up (SEP-2663)

This PR originally included a Tasks extension, but it was built against the 2025-11-25 in-core Tasks design still carried (types-only) in mcp_types, not SEP-2663 — the extension that actually ships in 2026-07-28. They diverge on nearly every wire-observable detail (server-decided augmentation vs params.task; {tasks/get, tasks/update, tasks/cancel} vs tasks/list+tasks/result; CreateTaskResult / resultType: "task" vs CallToolResult + _meta; execution.taskSupport gating; ttlMs). Rather than ship a spec-violating example, Tasks is removed here and returns as a separate PR rewritten to SEP-2663 with the conformance tasks-* scenarios wired in.

Design notes

The shape is the pluggable-interface pattern: declarative wiring (extensions=[...]), a narrow named interface with defaults, behaviour flowing through as plain values — chosen over a generic plugin framework (no other official SDK builds one) and over an app= kwarg on @tool (which would couple core MCPServer to one extension).

Testing

In-memory Client(server) e2e tests, 100% coverage maintained, strict-no-cover clean, pyright + ruff + markdownlint green. The apps story legs pass.

AI Disclaimer

This PR was developed with the assistance of either Claude or Codex. I've reviewed and verified the changes.

Kludex added 5 commits June 26, 2026 20:17
Thread an `extensions` argument through the low-level `Server.get_capabilities`
and `create_initialization_options` (mirroring `experimental`), backed by a
`Server.extensions` attribute so the streamable-HTTP `server/discover` path
advertises it too. Add an `extensions` branch to `Connection.check_capability`
(presence-of-identifier, since settings are negotiated per-extension) and let a
client advertise its own support via `Client(extensions=...)` /
`ClientSession(extensions=...)`, mirrored into `ClientCapabilities.extensions`.
Introduce `Extension`, a narrow base class (HTTPX `Transport`/`Auth` style) whose
methods default so an extension overrides only what it needs: `settings()`,
`tools()`, `resources()`, `methods()`, and `intercept_tool_call()`. `MCPServer`
accepts `extensions=[...]` at construction and `add_extension()` later, applying a
closed set of contributions (tool/resource/method bindings) and composing every
extension's `tools/call` interceptor into one `ServerMiddleware`. The server never
hands itself to an extension; the extension declares what it adds as data.
`Apps` is an additive `Extension`: `@apps.tool(resource_uri=...)` binds a tool to a
`ui://` UI resource via `_meta.ui.resourceUri`, `add_html_resource()` serves the
HTML at `text/html;profile=mcp-app`, and `client_supports_apps(ctx)` gates the
SEP-2133 text-only fallback. Drop the now-exercised `# pragma: no cover` on
`TextResource.read()` (the Apps resource path covers it).
`Tasks` is an interceptive `Extension`: `intercept_tool_call` records a
task-augmented `tools/call` and stamps the task id into
`_meta[io.modelcontextprotocol/related-task]`, while `methods()` serves
`tasks/get`, `tasks/result`, `tasks/cancel`, and `tasks/list` over an in-memory
store. It demonstrates the interceptive seam; the augmented call returns a
`CallToolResult` rather than `CreateTaskResult` because the `tools/call` result
schema admits only `CallToolResult | InputRequiredResult` (TODO L56). Also add
the negotiation-plumbing tests shared by both extensions.
Wire runnable `apps` and `tasks` stories (in-memory + http-asgi) into the manifest
and document the extensions API in the migration guide.
Comment thread src/mcp/server/tasks.py Outdated
Comment thread src/mcp/server/tasks.py Outdated
Drop the public `MCPServer.add_extension`; extensions are fixed at construction
via `extensions=[...]` (the apply logic moves to a private `_apply_extension`,
with the `tools/call` interceptor composed once afterwards). This matches the
declarative design and removes the mid-connection mutation footgun. Rework the
tasks story around a `render_report` tool whose multi-step work motivates running
it as a task, with named `_start_task` / `_get_task` / `_task_result` helpers so
the client reads as a clear lifecycle.

@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.

3 issues found and verified against the latest diff

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/mcp/server/apps.py">

<violation number="1" location="src/mcp/server/apps.py:76">
P2: `@apps.tool` does not merge/strip caller-provided `meta`, so `meta=` triggers duplicate keyword arguments and can crash server setup.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread src/mcp/server/tasks.py Outdated
Comment thread src/mcp/server/apps.py Outdated
Comment thread src/mcp/server/tasks.py Outdated
Kludex added 2 commits June 26, 2026 20:36
Make explicit that a plain tools/call is unchanged - only a call carrying a
`task` field becomes a task - and document that per-tool gating on the declared
`ToolExecution.task_support` is not enforced by this reference extension.
# Conflicts:
#	src/mcp/server/mcpserver/__init__.py
#	src/mcp/server/mcpserver/server.py
Comment thread src/mcp/server/tasks.py Outdated
Comment thread src/mcp/server/apps.py
Comment thread src/mcp/server/tasks.py Outdated
Comment thread src/mcp/server/mcpserver/extension.py Outdated

@maxisbey maxisbey 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.

Went through this against the spec primary sources (SEP-2663, SEP-2133, the ext-apps spec, and schema/draft/schema.ts). The framework shape is good and Apps is basically right, but tasks.py is implementing the 2025-11-25 in-core Tasks design rather than the SEP-2663 extension that actually ships in 2026-07-28 — they diverge on almost every wire-observable detail.

Tasks (src/mcp/server/tasks.py) — implements the wrong spec version

Everything below is a divergence from SEP-2663 / schema/draft/schema.ts, cross-checked against the 2025-11-25 schema where the current behavior comes from.

Method set

  • methods() registers tasks/result and tasks/list. Both were removed by SEP-2663 — clients calling either MUST get -32601, and tasks/list reintroduces the cross-caller enumeration leak the SEP explicitly designed out. The 2026 set is exactly {tasks/get, tasks/update, tasks/cancel}.
  • tasks/update is not registered, so there's no way for a client to deliver inputResponses for the input_required flow.

Opt-in and result envelope

  • intercept_tool_call gates on params.task as the client opt-in. SEP-2663 says servers MUST ignore params.task (it's the legacy 2025 field) — the server is the sole decider, gated only on whether the client declared io.modelcontextprotocol/tasks in its per-request capabilities.
  • A task-augmented tools/call returns a CallToolResult with _meta["io.modelcontextprotocol/related-task"]. That _meta key is 2025-only (it's in schema/2025-11-25/schema.ts:1330 and absent from schema/draft). The 2026 shape is a CreateTaskResult discriminated by resultType: "task" with taskId/status/createdAt flat on the result.
  • settings() returns {"list": {}, "cancel": {}} — that's the removed legacy capabilities.tasks sub-shape leaking into extension settings. Per SEP-2663 and schema/draft/examples/ServerCapabilities/extensions-tasks.json the settings object is {}.

Lifecycle and shapes

  • tasks/get returns only the flat Task snapshot. For completed/failed/input_required it has to inline result/error/inputRequests per the DetailedTask discriminated union — without that the client has no way to retrieve the tool output through the 2026 method set at all.
  • isError: true from the tool routes to status: "failed". SEP-2663 says an isError: true CallToolResult is a completed task whose result is that CallToolResult; failed is reserved for JSON-RPC Error objects only. Relatedly, TaskStore.fail() records no error payload, so there'd be nothing to surface for failed.error even after fixing tasks/get.
  • tasks/cancel returns the full Task body. Spec says it's an empty Result ack (resultType: "complete").
  • Wire field names: emits ttl/pollInterval (2025 model fields); SEP-2663 renamed these to ttlMs/pollIntervalMs.
  • No input_required state path anywhere — TaskStore has no transition into it and no inputRequests storage, so MRTR-over-tasks isn't implementable on this store.

Security / robustness

  • Task IDs are sequential f"task-{n}". The spec requires sufficient entropy because the ID is a bearer capability for tasks/get/tasks/cancel.
  • _require() returns any task by ID with no principal check; the spec says servers MUST authn/authz each task-related request.
  • tasks/get/tasks/cancel don't check the per-request client extension capability; spec says non-declaring clients MUST get -32021 with data.requiredCapabilities.
  • If await call_next(ctx) raises, the task stays permanently "working" (no try/except around the call).
  • payload = result if isinstance(result, dict) else {} silently drops a non-dict downstream result (e.g. a pydantic model from another middleware) — the response becomes {"_meta": {...}} with the tool's content gone.

Construction

  • TaskStore is hard-instantiated in Tasks.__init__ with no injection seam, which contradicts the module's own docstring example Tasks(store) and means the in-memory store is the only option (problem for stateless HTTP).
  • The default clock is _fixed_clock returning the constant "1970-01-01T00:00:00Z", so out of the box every task's createdAt/lastUpdatedAt is the Unix epoch.

Tests and the example story lock the wrong shape in. tests/server/test_tasks.py asserts tasks/result/tasks/list are routable, asserts _meta[related-task], uses params.task as opt-in, asserts isErrorfailed, asserts a body on cancel, asserts ttl not ttlMs, and hard-codes taskId == "task-1". examples/stories/tasks/ does the same and the README's "Caveats" section frames returning CallToolResult+_meta instead of CreateTaskResult as a "deliberate simplification" — that's a spec violation in user-facing docs, not a simplification. A spec-conformant server would -32601 the example client.

Extension framework — four structural things

The declarative Extension shape itself is nice (and there's no precedent for it in the other SDKs, so this is the reference). Four things I'd want fixed before it lands:

  1. Layering. Extension is defined in mcp/server/mcpserver/extension.py, so helper-tier mcp/server/apps.py and mcp/server/tasks.py import upward from mcp.server.mcpserver.*. The base class belongs at mcp/server/ with MCPServer composing it, so the dependency arrow points the right way (and so third-party extensions don't depend on the composition tier).
  2. identifier enforcement. It's a bare class annotation; a subclass that forgets it constructs fine and only blows up with AttributeError inside _apply_extension. A __init_subclass__ check (and ideally _meta-key grammar validation, since the spec says extension IDs MUST carry a prefix) would make it fail at class-definition time.
  3. MethodBinding is version-blind. It carries no protocol-version field and registers into the flat _request_handlers[method] dict, so extension methods bypass the (method, version) boundary table the runner uses for core methods. An extension can't declaratively say "this method exists only at 2026-07-28" — it'd have to if version == ... inside the handler.
  4. No -32021 raise seam. Connection.check_capability gets a boolean extensions branch, but there's no require_client_extension(id) (or similar) that raises MissingRequiredClientCapabilityError with data.requiredCapabilities.extensions = {id: {}}. The error type already exists in mcp_types; without the helper every extension author has to hand-construct it.

Apps (src/mcp/server/apps.py) — looks right, a few additive gaps

The fundamentals match the ext-apps spec: EXTENSION_ID = "io.modelcontextprotocol/ui", text/html;profile=mcp-app, nested _meta.ui.resourceUri, server auto-advertises under capabilities.extensions. Nothing blocking. Smaller things:

  • client_supports_apps() only checks key presence, never mimeTypes, so a client advertising {"mimeTypes": ["application/x-something-else"]} reads as HTML-capable. The ts reference checks mimeTypes.includes(RESOURCE_MIME_TYPE).
  • @apps.tool() has no visibility kwarg, so you can't set _meta.ui.visibility: ["app"].
  • add_html_resource() has no way to set UIResourceMeta (csp/permissions/domain/prefersBorder) on the registered resource.
  • Passing meta= through @apps.tool(..., meta={...}) is accepted by **tool_kwargs but raises TypeError: got multiple values for keyword argument 'meta' at server construction (_apply_extension calls add_tool(tool.fn, meta=tool.meta, **tool.kwargs)).
  • Nits: no typed McpUi*Meta models (untyped dicts), no cross-check that every resource_uri actually has a registered resource, no opt-out from resources/list for UI-only resources.
  • One open question: ts-sdk and csharp both ship Apps in a separate package (@modelcontextprotocol/ext-apps, ModelContextProtocol.Extensions.Apps). Is in-core mcp.server.apps deliberate? If so worth a line in the module docstring.

Validating it's right

Neither extension currently has external proof-of-correctness:

  • Conformance. The harness has 10 SEP-2663 tasks-* server scenarios but they're in the pending list and no python-sdk CI leg selects them; mcp-everything-server doesn't mount Tasks(). To make this PR externally verifiable: add a --scenario 'tasks-*' leg to conformance.yml, add an --extensions tasks flag to the everything-server fixture, and seed an expected-failures.tasks.yml. After the SEP-2663 rewrite, these seven should be green: tasks-dispatch-and-envelope, tasks-capability-negotiation, tasks-required-task-error, tasks-wire-fields, tasks-lifecycle, tasks-mrtr-input, tasks-request-state-removal. There are zero Apps scenarios in the harness — worth proposing upstream.
  • Interaction tests. I'd write the spec-derived tests/interaction/mcpserver/test_tasks.py first (one @requirement per SEP clause: settings shape {}, non-declaring client gets inline result, non-declaring tasks/get-32021, resultType:"task" flat, isErrorcompleted, tasks/cancel empty ack, tasks/result/tasks/list-32601, ttlMs field name, ID entropy, tasks/update ack, store injectable). They'll be red on this branch — that's the bar the rewrite hits.
  • Stories smoke. The tests/examples/test_stories{,_smoke}.py infrastructure already runs both stories over in-memory + real HTTP. Once examples/stories/tasks/ is rewritten to the 2026 shape it becomes an end-to-end smoke test for free. Adding a wire-shape assertion (raw response has "resultType": "task" and no related-task _meta key) would have caught every Tasks issue above.
  • Cross-SDK. The conformance leg covers py-server↔ts-client. For py-client, csharp-sdk is currently the only other SDK with a working Tasks runtime (McpServerImpl.cs returns resultType:"task"), so that's the interop peer for the polymorphic-result handling.

Suggested split

Given the size of the Tasks delta, I'd land framework + Apps here (after the four framework fixes and the Apps minors) and take Tasks as a follow-up PR rewritten to SEP-2663, with the interaction tests landing red first and the conformance leg wired in the same PR.

AI Disclaimer

The Tasks implementation was built against the 2025-11-25 in-core design still
carried (types-only) in mcp_types, not SEP-2663 (the extension that ships in
2026-07-28). They diverge on nearly every wire-observable detail: SEP-2663 makes
the server the sole decider (ignoring the legacy params.task), uses the
{tasks/get, tasks/update, tasks/cancel} method set (no tasks/list or
tasks/result), returns a CreateTaskResult discriminated by resultType: "task"
(not a CallToolResult with _meta), advertises {} settings, gates on
execution.taskSupport, and renames ttl/pollInterval to ttlMs/pollIntervalMs.

Remove the extension, its tests, and its story rather than ship a spec-violating
example; restore tasks to the deferred manifest list with a SEP-2663 pointer. The
generic Extension API and the Apps reference extension are unaffected and still
at 100% coverage. Tasks returns as a separate PR rewritten to SEP-2663 with the
conformance tasks-* scenarios wired in.
@Kludex Kludex changed the title Add a pluggable server extension API with MCP Apps and Tasks Add a pluggable server extension API with MCP Apps Jun 26, 2026
Comment thread src/mcp/server/mcpserver/extension.py Outdated
…Apps fixes

Framework:
- Move the Extension base class from mcp/server/mcpserver/extension.py to
  mcp/server/extension.py so helper-tier modules (apps.py) and third-party
  extensions depend on the base, not the composition tier.
- Enforce a vendor-prefix/name identifier via __init_subclass__ (and at apply
  time for per-instance identifiers), failing at class-definition rather than
  late with AttributeError.
- Add MethodBinding.protocol_versions so an extension method can be scoped to
  specific wire versions; out-of-range requests get METHOD_NOT_FOUND.
- Add require_client_extension(ctx, identifier) raising the -32021 missing
  required client capability error with a requiredCapabilities payload.

Apps:
- client_supports_apps now checks the client advertised the
  text/html;profile=mcp-app MIME type, not just the extension key.
- Add a visibility kwarg to @apps.tool (_meta.ui.visibility).
- Let add_html_resource set csp/permissions/domain/prefers_border on the
  resource _meta via typed ResourceCsp/ResourcePermissions models.
- Fix the meta= double-keyword TypeError by making meta an explicit param
  merged with the ui entry instead of passing through **tool_kwargs.

@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.

4 issues found across 8 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/mcp/server/apps.py">

<violation number="1" location="src/mcp/server/apps.py:191">
P2: `client_supports_apps` incorrectly returns True when `mimeTypes` is missing. Require explicit `text/html;profile=mcp-app` presence to avoid false-positive Apps support.</violation>
</file>

<file name="docs/migration.md">

<violation number="1" location="docs/migration.md:417">
P3: Migration docs overstate when extension identifier validation runs. Instance-assigned identifiers are validated at extension registration, not only at subclass definition.</violation>

<violation number="2" location="docs/migration.md:430">
P3: Apps fallback docs describe MIME-type gating too narrowly. Current behavior also treats missing `mimeTypes` as supported.</violation>
</file>

<file name="src/mcp/server/extension.py">

<violation number="1" location="src/mcp/server/extension.py:39">
P2: `validate_extension_identifier` uses an overly permissive regex and accepts malformed vendor prefixes that do not satisfy the `_meta` label grammar.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic

Comment thread src/mcp/server/apps.py
if settings is None:
return False
mime_types = settings.get("mimeTypes")
return mime_types is None or APP_MIME_TYPE in mime_types

@cubic-dev-ai cubic-dev-ai Bot Jun 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: client_supports_apps incorrectly returns True when mimeTypes is missing. Require explicit text/html;profile=mcp-app presence to avoid false-positive Apps support.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/mcp/server/apps.py, line 191:

<comment>`client_supports_apps` incorrectly returns True when `mimeTypes` is missing. Require explicit `text/html;profile=mcp-app` presence to avoid false-positive Apps support.</comment>

<file context>
@@ -118,12 +178,17 @@ def resources(self) -> Sequence[ResourceBinding]:
+    if settings is None:
+        return False
+    mime_types = settings.get("mimeTypes")
+    return mime_types is None or APP_MIME_TYPE in mime_types
 
 
</file context>
Suggested change
return mime_types is None or APP_MIME_TYPE in mime_types
return isinstance(mime_types, list) and APP_MIME_TYPE in mime_types
Fix with cubic


# Extension identifiers follow the `_meta` key grammar: a mandatory reverse-DNS
# prefix, a slash, then the extension name (SEP-2133 / the spec's _meta rules).
_IDENTIFIER_RE = re.compile(r"^[A-Za-z0-9.-]+/[A-Za-z0-9._-]+$")

@cubic-dev-ai cubic-dev-ai Bot Jun 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: validate_extension_identifier uses an overly permissive regex and accepts malformed vendor prefixes that do not satisfy the _meta label grammar.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/mcp/server/extension.py, line 39:

<comment>`validate_extension_identifier` uses an overly permissive regex and accepts malformed vendor prefixes that do not satisfy the `_meta` label grammar.</comment>

<file context>
@@ -1,32 +1,55 @@
 
+# Extension identifiers follow the `_meta` key grammar: a mandatory reverse-DNS
+# prefix, a slash, then the extension name (SEP-2133 / the spec's _meta rules).
+_IDENTIFIER_RE = re.compile(r"^[A-Za-z0-9.-]+/[A-Za-z0-9._-]+$")
+
+
</file context>
Suggested change
_IDENTIFIER_RE = re.compile(r"^[A-Za-z0-9.-]+/[A-Za-z0-9._-]+$")
_IDENTIFIER_RE = re.compile(r"^[A-Za-z](?:[A-Za-z0-9-]*[A-Za-z0-9])?(?:\.[A-Za-z](?:[A-Za-z0-9-]*[A-Za-z0-9])?)*/[A-Za-z0-9](?:[A-Za-z0-9._-]*[A-Za-z0-9])?$")
Fix with cubic

Comment thread docs/migration.md
The reference extension is `mcp.server.apps.Apps` (`io.modelcontextprotocol/ui`):
it binds a tool to a `ui://` UI resource via `_meta.ui.resourceUri`, and
`client_supports_apps(ctx)` gates the SEP-2133 text-only fallback (checking the
client advertised the `text/html;profile=mcp-app` MIME type).

@cubic-dev-ai cubic-dev-ai Bot Jun 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: Apps fallback docs describe MIME-type gating too narrowly. Current behavior also treats missing mimeTypes as supported.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/migration.md, line 430:

<comment>Apps fallback docs describe MIME-type gating too narrowly. Current behavior also treats missing `mimeTypes` as supported.</comment>

<file context>
@@ -425,7 +426,14 @@ mcp = MCPServer("demo", extensions=[Apps()])
 it binds a tool to a `ui://` UI resource via `_meta.ui.resourceUri`, and
-`client_supports_apps(ctx)` gates the SEP-2133 text-only fallback.
+`client_supports_apps(ctx)` gates the SEP-2133 text-only fallback (checking the
+client advertised the `text/html;profile=mcp-app` MIME type).
+
+A `MethodBinding` may set `protocol_versions` to scope an extension method to
</file context>
Suggested change
client advertised the `text/html;profile=mcp-app` MIME type).
client declared the Apps extension and either omitted `mimeTypes` or included `text/html;profile=mcp-app`).
Fix with cubic

Comment thread docs/migration.md
(the 2026-07-28 capability map). An extension subclasses `mcp.server.extension.Extension`
and overrides only the contribution methods it needs: `tools()`/`resources()`/`methods()`
(additive) and `intercept_tool_call()` (wraps `tools/call`). The `identifier` must be a
`vendor-prefix/name` string, enforced when the subclass is defined. Pass instances at

@cubic-dev-ai cubic-dev-ai Bot Jun 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: Migration docs overstate when extension identifier validation runs. Instance-assigned identifiers are validated at extension registration, not only at subclass definition.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/migration.md, line 417:

<comment>Migration docs overstate when extension identifier validation runs. Instance-assigned identifiers are validated at extension registration, not only at subclass definition.</comment>

<file context>
@@ -411,9 +411,10 @@ For protocol 2026-07-28 over Streamable HTTP, a tool's input-schema property may
 and overrides only the contribution methods it needs: `tools()`/`resources()`/`methods()`
-(additive) and `intercept_tool_call()` (wraps `tools/call`). Pass instances at
+(additive) and `intercept_tool_call()` (wraps `tools/call`). The `identifier` must be a
+`vendor-prefix/name` string, enforced when the subclass is defined. Pass instances at
 construction:
 
</file context>
Suggested change
`vendor-prefix/name` string, enforced when the subclass is defined. Pass instances at
`vendor-prefix/name` string. Class-level identifiers are validated when the subclass is defined; instance-assigned identifiers are validated when the extension is registered. Pass instances at
Fix with cubic

Comment on lines +290 to +297
self.add_tool(tool.fn, meta=tool.meta, **tool.kwargs)
for resource in extension.resources():
self.add_resource(resource.resource)
for method in extension.methods():
handler = _version_gated(method) if method.protocol_versions is not None else method.handler
self._lowlevel_server.add_request_handler(method.method, method.params_type, handler)

self._lowlevel_server.extensions[extension.identifier] = extension.settings()

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.

🟡 An extension's MethodBinding named after an existing method (e.g. 'tools/list', 'resources/read', or another extension's verb) silently replaces the registered handler in _apply_extension, since add_request_handler replaces by design and only 'initialize' is reserved — even though Extension.methods() is documented as additive and duplicate extension identifiers are rejected with ValueError just above. Consider raising ValueError when method.method already has a registered handler, mirroring the duplicate-identifier check.

Extended reasoning...

What happens. _apply_extension registers each MethodBinding via self._lowlevel_server.add_request_handler(method.method, ...) (src/mcp/server/mcpserver/server.py:293-295). add_request_handler is documented to replace any existing handler for the same method; only initialize is reserved. Nothing on the extension path checks that the contributed method name is actually new, so a binding named tools/list, resources/read, completion/complete, etc. silently overwrites MCPServer's own core handler at construction, and two extensions contributing the same vendor verb silently last-write-wins.\n\nWhy this is an inconsistency rather than a design choice. The PR's own contract says contributions are additive and closed: Extension.methods() is documented as "New request methods this extension serves (additive)", the extension.py module docstring stresses that intercept_tool_call is the one sanctioned way to wrap core behaviour, and the surrounding code enforces its constraints loudly — duplicate extension identifiers raise ValueError four lines above, malformed identifiers raise TypeError, initialize is reserved in add_request_handler. A method-name collision is the one constraint in this set that goes unenforced.\n\nStep-by-step.\n1. class SomeExt(Extension) whose methods() returns [MethodBinding("tools/list", PaginatedRequestParams, my_handler)] — perhaps a typo for a vendor verb, or a copy-paste mistake.\n2. MCPServer("demo", extensions=[SomeExt()])_apply_extension calls add_request_handler("tools/list", ...), which replaces the _handle_list_tools entry registered by the lowlevel Server constructor.\n3. Construction succeeds with no error or warning. Every @mcp.tool() registered on the server is now invisible to tools/list; the replacement also bypasses the intercept_tool_call seam entirely.\n4. The same silent last-write-wins applies when two installed extensions contribute the same method name (e.g. two tasks-style extensions both binding tasks/get).\n\nOn the counter-argument. A refuting reviewer noted this only triggers when an extension author names a binding contrary to the documented contract, that extensions are construction-time author-controlled inputs, and that a malicious extension could subvert the server in other ways anyway — all true, which is why this is filed as a non-blocking nit rather than a correctness bug. But the same is true of the duplicate-identifier case the PR does guard with ValueError; the value of the check is failing loudly at construction for an authoring mistake (or a collision between two third-party extensions) instead of shipping a server whose core listing/read methods are silently gone. The lowlevel replace-on-reregister semantics are sanctioned for direct add_request_handler users, but the extension layer presents a stricter additive contract and should enforce it the way it enforces its other constraints.\n\nFix. A one-line guard in _apply_extension, mirroring the duplicate-identifier check:\n\npython\nfor method in extension.methods():\n if self._lowlevel_server.get_request_handler(method.method) is not None:\n raise ValueError(\n f"Extension {identifier!r} contributes method {method.method!r}, which is already registered"\n )\n ...\n\n\nplus a small test alongside test_duplicate_extension_identifier_raises.

Comment thread src/mcp/server/apps.py
Comment on lines +178 to +191
def client_supports_apps(ctx: Context[Any] | ServerRequestContext[Any, Any]) -> bool:
"""Whether the connected client negotiated MCP Apps support.

Returns `True` only when the client advertised the extension AND listed the
`text/html;profile=mcp-app` MIME type in its settings, so a UI-enabled tool
can fall back to text-only output otherwise.
"""
capabilities = _client_capabilities(ctx)
extensions = capabilities.extensions if capabilities else None
settings = extensions.get(EXTENSION_ID) if extensions else None
if settings is None:
return False
mime_types = settings.get("mimeTypes")
return mime_types is None or APP_MIME_TYPE in mime_types

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.

🟡 client_supports_apps's docstring (and the README/migration.md) says it returns True only when the client both advertised the io.modelcontextprotocol/ui extension AND listed the text/html;profile=mcp-app MIME type, but the implementation ends with return mime_types is None or APP_MIME_TYPE in mime_types, so a client that advertises the extension with no mimeTypes key is treated as fully Apps-capable and gets the rich UI result instead of the documented text-only fallback. Either tighten the predicate to mime_types is not None and APP_MIME_TYPE in mime_types or update the docstring/README/migration text to say an absent mimeTypes list counts as supporting all types — and add a test pinning the absent-mimeTypes case either way.

Extended reasoning...

What the bug is. client_supports_apps in src/mcp/server/apps.py documents a strict contract: "Returns True only when the client advertised the extension AND listed the text/html;profile=mcp-app MIME type in its settings". The apps example README and docs/migration.md repeat this framing — the client "negotiates it by advertising the text/html;profile=mcp-app MIME type" / "checking the client advertised the text/html;profile=mcp-app MIME type". The implementation, however, ends with return mime_types is None or APP_MIME_TYPE in mime_types: when the client declared the extension but supplied no mimeTypes key at all, the function returns True.\n\nThe code path that triggers it. A client connects with Client(target, extensions={"io.modelcontextprotocol/ui": {}}) — extension declared, settings empty. Server-side, a UI-bound tool calls client_supports_apps(ctx): extensions.get(EXTENSION_ID) returns {} (not None, so the early-return is skipped), settings.get("mimeTypes") returns None, and the final line evaluates None is None or ...True. The tool takes the rich/UI branch even though the client never listed the app MIME type.\n\nStep-by-step proof (mirrors the existing tests in tests/server/test_apps.py, which cover only the present-and-matching → True and present-but-wrong → False cases):\n1. Build _clock_server() from the test file (the get_time tool branches on client_supports_apps(ctx)).\n2. Connect with Client(server, extensions={"io.modelcontextprotocol/ui": {}}) — note: no mimeTypes.\n3. Call get_time.\n4. Result is "2026-06-26T00:00:00Z" (the rich path), not "The time is 2026-06-26T00:00:00Z." (the documented text-only fallback for a client that did not list the MIME type).\n\nWhy nothing prevents it. The absent-mimeTypes case is untested: test_apps_tool_returns_rich_output_when_client_negotiated_apps always sends mimeTypes: [APP_MIME_TYPE], and test_client_supports_apps_false_when_mime_type_not_offered sends a wrong-but-present list. So the divergence between code and docs is unpinned and could drift either way in a refactor.\n\nImpact. Small in practice — a client that bothers to declare the UI extension can almost certainly render the app MIME type, and the failure mode is just rich output going to a slightly under-declared client rather than a crash. But the documented contract and the actual predicate disagree, and this function is the SEP-2133 graceful-degradation gate that downstream servers will copy from the example, so the ambiguity is worth resolving before merge.\n\nHow to fix. Pick one side: (a) tighten the predicate to return mime_types is not None and APP_MIME_TYPE in mime_types so the docstring/README/migration text stay accurate, or (b) keep the lenient "absent means all types" interpretation and reword the docstring (and the two doc mentions) to say so explicitly. Either way, add a test for extensions={EXTENSION_ID: {}} so the chosen behavior is pinned.

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.

2 participants