Skip to content

MCPServer: content-block returns are unstructured, prompt messages take Image/Audio - #3320

Open
maxisbey wants to merge 10 commits into
mainfrom
mcpserver-content-and-prompt-ergonomics
Open

MCPServer: content-block returns are unstructured, prompt messages take Image/Audio#3320
maxisbey wants to merge 10 commits into
mainfrom
mcpserver-content-and-prompt-ergonomics

Conversation

@maxisbey

@maxisbey maxisbey commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Four small MCPServer/client fixes that remove traps the server docs would otherwise have to explain around. One commit per item so they can be reviewed (or dropped) independently.

Behaviour change to call out in the release notes (item 1): a tool whose return annotation is a content-block type (-> EmbeddedResource, -> list[TextContent], -> tuple[TextContent, ...], -> str | TextContent, ...) or has Image/Audio as its list/tuple items no longer advertises outputSchema and no longer returns structuredContent; its content is unchanged. Pass structured_output=True to keep the old shape.

Motivation and Context

1. Content-block, Image and Audio return annotations are unstructured (func_metadata)

@mcp.tool() def f() -> EmbeddedResource published the pydantic schema of the EmbeddedResource class itself (~2 KB) as the tool's outputSchema and echoed the block into structuredContent a second time. Same for -> ResourceLink, -> TextContent, -> list[ContentBlock], tuple[...], unions. -> Image escaped only because Image is a plain class, and -> list[Image] / -> list[str | Image] / -> Image | Audio didn't register at all (PydanticSchemaGenerationError from create_model, outside the existing try).

In auto-detect mode (structured_output=None), a return annotation that declares content blocks or the Image/Audio helpers — bare, as the items of a list/tuple/Sequence, or as the arms of a union (through Annotated/Optional) — now derives no output schema. That is the annotation-level mirror of what _convert_to_content already does with those values at runtime; mapping values and model fields are data and keep their schema exactly as before. structured_output=True still forces a schema. The check sits right before schema derivation, so there is one rule and one override; Annotated[CallToolResult, list[TextContent]] is covered by the same rule (on main that spelling failed every call unless structured_content was hand-built).

Prompt.from_function and ResourceTemplate.from_function only ever needed the argument model but ran the same auto-detection, so an unschematizable return annotation on a prompt or resource template (-> list[SomePlainClass]) failed registration with a tool structured-output error; they now pass structured_output=False, which also keeps this rule from reaching beyond tools.

2. Prompt messages accept Image / Audio (prompts/base.py)

Tools convert the helpers; UserMessage(Image(...)) was a pydantic validation error (client saw -32603), so you had to write Image(...).to_image_content(). Message.__init__ now does the same conversion str already gets. The dict form ({"role": "user", "content": Image(...)}) works too since validation goes through __init__.

A prompt function may also return bare content the way a tool does — Image(...), a ready-made content block, or a list mixing captions and images — and each item becomes one user message; previously anything that wasn't a str/Message/dict was JSON-dumped (an Image arrived as its repr). That last part is its own commit (Prompt functions may return bare content blocks, Image or Audio) and can be dropped independently; the JSON-dump fallback for other values is untouched, and conversion errors (e.g. an unreadable media file) now reach the existing Error rendering prompt ... handler instead of being re-wrapped as Could not convert prompt result to message: <repr>.

3. Prompt message classes exported from mcp.server.mcpserver (__init__.py)

Message, UserMessage, AssistantMessage are re-exported next to Image/Audio, so the prompt examples import everything from one place. (An earlier revision also let add_prompt() take a function like add_tool(); that was dropped — add_resource()/add_prompt() take built objects today, and changing the imperative registration API deserves its own pass across all three primitives. mcp.add_prompt(Prompt.from_function(fn, ...)) remains the runtime-registration spelling.)

4. Stale TODO in Client.send_roots_list_changed

The comment claimed the server can't handle the notification; the lowlevel Server has on_roots_list_changed and tests/interaction/lowlevel/test_roots.py drives it. (The runtime deprecation warning currently fires once per decorated layer for the Client -> ClientSession delegations and for ctx.info() -> ctx.log() -> send_log_message; that is the same across ~10 call sites and is left for a follow-up rather than special-casing roots here.)

How Has This Been Tested?

  • New/updated unit tests: parametrized func_metadata cases (bare block, list[ContentBlock], list[str | Image], tuple[Audio, ...], Annotated[CallToolResult, list[TextContent]]), the structured_output=True override, a model with a content-block field staying structured; Image/Audio in UserMessage/AssistantMessage.
  • Review-round pins: dict[str, TextContent] and a model with a block field stay structured; a prompt and a resource template with an unschematizable return annotation register; dict-form prompt message with an Image; bare Image/EmbeddedResource/[str, Image] prompt returns; tests/docs_src/test_structured_output.py proves the new page section.
  • The existing test_tool_mixed_content flips to structured_content is None; test_tool_mixed_list_with_audio_and_image gets its real annotation back and loses a TODO plus three type: ignores.
  • Exercised a user-style server over a real stdio subprocess before/after: on main the module fails to import (-> list[str | Image]), report/blocks advertise outputSchema, and the image prompt is an internal error; on this branch tools/list shows no outputSchema for the content tools (and still one for structured_output=True and a dict[str, float] control) and the prompt renders text/image/audio.

Breaking Changes

None. No signature, export, or documented behaviour changes; per VERSIONING.md these are bug fixes plus additive API for a minor release, so the migration guide is untouched.

The one observable difference is item 1: tools whose return annotation is a content-block type stop advertising outputSchema and stop returning structuredContent (their content is unchanged). No docs page presented that shape as the intended contract (the media page says such results carry no output schema; the structured-output page enumerates models, TypedDicts, dataclasses, scalars and generics), and -> list[Image] not registering was a plain bug. It does show up in the everything-server's test_image_content / test_audio_content / test_embedded_resource / test_multiple_content_types; the conformance scenarios only assert on content, so they are unaffected. Anyone who wants the old shape passes structured_output=True. docs/servers/structured-output.md gets two sentences so the published page stays accurate.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

Not done here, noted as follow-ups:

  • Deprecated public methods delegating to other deprecated public methods emit MCPDeprecationWarning once per layer (Client.set_logging_level/subscribe_resource/unsubscribe_resource/send_progress_notification/send_roots_list_changed, and Context.debug/info/warning/error -> log -> ServerSession.send_log_message). The clean fix is undecorated private bodies that both public layers call, plus a "one warning per call, attributed to the caller" regression test. (The roots deprecation text cites SEP-2577; the notification's removal at 2026-07-28 is SEP-2575 — same pass.)
  • Typing indirections: type X = ... (PEP 695) and NewType return annotations are not unwrapped by the content rule, by the existing InputRequiredResult stripping, or by Annotated[CallToolResult, ...] detection (baseline behaviour, not a regression). One alias-resolution step on the inspected return type feeding all three is the right place.
  • _try_create_model_and_schema builds its wrapper models outside the try, so -> list[SomePlainClass] on a tool (and structured_output=True with -> list[Image], or dict[str, Image]) still raises a raw PydanticSchemaGenerationError instead of falling back / raising InvalidSignature.
  • Path-backed Image/Audio are read and base64-encoded on the event loop wherever they are converted: Tool.run -> convert_result -> _convert_to_content (since v1), Message.__init__ inside an async prompt function, and now render's bare-content arm. Fix once, in the helpers or inside the existing worker-thread hop, for tools and prompts together.
  • Prompt message union: UserMessage and AssistantMessage both declare role: Literal["user", "assistant"], so the dict form always yields a UserMessage (the validator is now left-to-right, so at least the content is converted once). Distinct role literals would let it discriminate but are an API-visible change.

The docs pages that motivated this (media, prompts) are being rewritten separately; the doc edits here are only the ones needed to keep currently published statements true.

AI Disclaimer

The lowlevel Server has handled roots/list_changed via on_roots_list_changed
for a while (see tests/interaction/lowlevel/test_roots.py); the comment was
left behind when the pragma next to it was removed.
Tools already convert the Image/Audio helpers to ImageContent/AudioContent;
prompt messages rejected them with a pydantic validation error, forcing
UserMessage(Image(...).to_image_content()). Message.__init__ now performs the
same conversion, so UserMessage(Image(...)) works, including via the dict form.
…ed tool output

A tool annotated to return a content block (-> EmbeddedResource, -> TextContent,
-> list[ContentBlock], ...) had the block model's own pydantic schema published as
its output_schema and every block echoed into structured_content a second time,
while Image/Audio inside a generic (-> list[Image], -> Image | Audio) failed to
register at all. -> Image escaped only because Image is a plain class.

In auto-detect mode, an annotation that mentions a content block class or the
Image/Audio helpers anywhere in its type tree now derives no output schema,
matching what _convert_to_content already does with those values at runtime.
structured_output=True still forces a schema. Behaviour change vs v1/2.0, so it
is documented in the migration guide and the structured-output page.
add_tool(fn) registers a function but add_prompt() only took a ready-made Prompt,
so registering a prompt outside the decorator meant importing Prompt from a
subpackage and calling Prompt.from_function yourself. add_prompt() now also
accepts the function with the same keyword options as @prompt(); the Prompt form
(including add_prompt(prompt=...)) is unchanged and @prompt() still hands
add_prompt a Prompt, so subclass overrides keep intercepting registrations.

Message, UserMessage and AssistantMessage are re-exported from
mcp.server.mcpserver next to Image and Audio.
@github-actions

github-actions Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

📚 Documentation preview

Preview https://pr-3320.mcp-python-docs.pages.dev
Deployment https://47c9dd2b.mcp-python-docs.pages.dev
Commit 6015f97
Triggered by @maxisbey
Updated 2026-08-17 11:03:51 UTC

The migration guide documents breaking changes between majors. Nothing here
changes a signature or documented behaviour, so the notes belong in the release
notes, not the guide.

No-Verification-Needed: docs-only revert
Comment thread src/mcp/server/mcpserver/utilities/func_metadata.py Outdated

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

2 issues found across 1 file (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="docs/migration.md">

<violation number="1">
P2: This line now says tool return handling is unchanged, but content-block/Image/Audio return annotations are no longer auto-structured in auto-detect mode. Document that exception here (or keep the dedicated migration note) so users who relied on `output_schema`/`structured_content` understand the behavior change and override path (`structured_output=True`).</violation>

<violation number="2">
P2: `add_prompt()` is not unchanged: it now accepts a plain function plus `name/title/description/icons`, while v1 only accepted a `Prompt`. Keep this bullet aligned with the current API so migration readers see the supported registration form.</violation>
</file>

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

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.

Additional findings (outside current diff — PR may have been updated during review):

  • 🟡 src/mcp/server/mcpserver/server.py — [quality] Now that add_prompt() accepts a plain function with name/title/description/icons, the @ prompt() decorator should delegate to it (self.add_prompt(func, name=name, title=title, description=description, icons=icons)) instead of duplicating the Prompt.from_function(...) construction, matching how @ tool() delegates to add_tool(fn, ...) at server.py:677.

    Extended reasoning...

    Concrete cost: two separate code paths construct a Prompt from a function (server.py:1004 in the decorator and server.py:931 in add_prompt), so any future change to registration (extra validation, new kwargs, duplicate-name policy) must be applied in both places or the decorator and add_prompt drift apart. The sibling @ tool() decorator already uses the delegation form (add_tool(fn, ...)), so the prompt decorator is now the odd one out for no benefit; delegating removes the duplicated Prompt.from_function call added alongside this PR's new add_prompt function form.

    Verification: nit — the claim is factually true. This diff added a callable overload to add_prompt (src/mcp/server/mcpserver/server.py:893-933) whose body does prompt = Prompt.from_function(prompt, name=name, title=title, description=description, icons=icons) — exactly the same construction the @ prompt() decorator still performs itself at lines 1003-1005: `prompt = Prompt.from_function(func, name=name, title

  • 🟣 src/mcp/server/mcpserver/prompts/base.py — Prompt functions returning a bare Image/Audio (the shape tools accept, and which this PR now advertises for prompt message content) are silently stringified to the object's repr instead of converting to ImageContent/AudioContent: Message.__init__ gained the conversion but Prompt.render's per-item dispatch (Message/dict/str, else JSON-dump with fallback=str) was not extended.

    Extended reasoning...

    A user reads the new Message docstring ('content may be ... an Image or Audio helper') or is used to tools, and writes @ mcp.prompt()\ndef p(): return ["look at this", Image(path)] (or return Image(path)). render() hits the else branch at lines 199-201: pydantic_core.to_json(Image_instance, fallback=str) produces a JSON string like '"<mcp.server.mcpserver.utilities.types.Image object at 0x7f...>"', which is sent to the client as a TextContent message — silent garbage, no error. Inconsistently, the dict form {"role": "user", "content": Image(path)} DOES work, because pydantic's custom_init routes message_validator dict validation through the new init. The else branch is pre-existing (and marked pragma: no cover), but the PR's widening of the prompt content surface to Image/Audio is what makes this path a realistic user trigger; the fix is adding the same isinstance(Image/Audio) conversion in render's dispatch.

    Verification: pre-existing — the failure path is real and reachable, though the dispatch lines themselves predate this diff; the PR extends the same feature and makes the mistake more likely. This PR adds Image/Audio conversion only to Message.__init__ (src/mcp/server/mcpserver/prompts/base.py:38-41, new in this diff) and advertises it in the new docstring at lines 28-29 ("content may be ... an Image or

Comment thread src/mcp/server/mcpserver/utilities/func_metadata.py Outdated
Comment thread docs/migration.md Outdated
Comment thread src/mcp/server/mcpserver/__init__.py
Comment thread src/mcp/server/mcpserver/prompts/base.py
add_tool takes a function while add_resource and add_prompt take built objects;
letting add_prompt accept both would be a third shape rather than consistency, and
changing the imperative registration API deserves its own design pass across all
three primitives. mcp.add_prompt(Prompt.from_function(fn, ...)) remains the spelling
for runtime registration.
@maxisbey maxisbey changed the title MCPServer: content-block returns are unstructured, prompt messages take Image/Audio, add_prompt() takes a function MCPServer: content-block returns are unstructured, prompt messages take Image/Audio Aug 16, 2026

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

Additional findings (outside current diff — PR may have been updated during review):

  • 🟡 src/mcp/server/mcpserver/server.py — [quality] nit: the structured_output docstring block is duplicated in three places, and only the func_metadata copy was updated with the new content-block rule — MCPServer.add_tool() (server.py:594-597) and MCPServer.tool() (server.py:644-647) still describe plain auto-detection with no mention that content-block/Image/Audio annotations now opt out of structured output.

    Extended reasoning...

    Concrete cost: divergent duplicated documentation on the public API surface. The diff changes what "auto-detects based on the function's return type annotation" means (a -> list[TextContent] tool now silently gets no outputSchema/structuredContent), and documents that only in the internal func_metadata() docstring (func_metadata.py:235-237). A user reading help(mcp.tool) or the IDE hover for add_tool/tool — the only docstrings users actually see — gets the pre-change semantics and has no pointer to the structured_output=True override; the three copies of this bullet list will keep drifting. Fix: extend the bullet in both public docstrings (or reference the one canonical description) in the same PR that changed the behavior.

    Verification: nit — the claim is factually true. The diff updates only the internal func_metadata() docstring (src/mcp/server/mcpserver/utilities/func_metadata.py, new bullet: "Content blocks (TextContent, EmbeddedResource, ...), Image and Audio, anywhere in the annotation - unstructured when auto-detecting; structured_output=True bypasses this rule"), while the two public copies of the same `structured_out

  • 🟣 src/mcp/server/mcpserver/prompts/base.py — Pre-existing, made more visible by this change: Prompt.render's fallback branch JSON-dumps a bare Image/Audio helper returned from a prompt function into a garbage text block, while the same helper is now converted properly everywhere else (Message content, tool returns).

    Extended reasoning...

    This PR teaches Message/UserMessage/AssistantMessage to convert Image/Audio helpers (base.py lines 35-42) and documents that 'prompt messages accept the same Image/Audio helpers tools return'. A user then naturally writes @ mcp.prompt()\ndef pic() -> Image: return Image(path) (or returns [Image(path), "caption"]). render() hits the else branch at lines 199-201: pydantic_core.to_json(Image_instance, fallback=str) produces the object's repr, so the client receives a text message containing '"<mcp.server.mcpserver.utilities.types.Image object at 0x7f...>"' instead of an ImageContent block (or an error). No exception is raised, so the broken prompt ships silently. The fix is one more dispatch arm (convert Image/Audio — and arguably bare ContentBlock — into a UserMessage) at the render level where str already gets special-cased; the PR applied the conversion only at Message.init depth. Author lists this as a follow-up in the PR description; filed so it is tracked against the code that merges.

    Verification: pre-existing — src/mcp/server/mcpserver/prompts/base.py:199-201: in Prompt.render(), a bare Image/Audio returned from a prompt function falls to the else branch content = pydantic_core.to_json(msg, fallback=str, indent=2).decode(); since Image/Audio are plain non-pydantic classes (src/mcp/server/mcpserver/utilities/types.py:9,57), fallback=str yields the object repr, which line 201 wraps as a us

Comment thread src/mcp/server/mcpserver/utilities/func_metadata.py Outdated
Comment thread docs/servers/structured-output.md Outdated

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

Additional findings (outside current diff — PR may have been updated during review):

  • 🟣 src/mcp/server/mcpserver/prompts/base.py — Pre-existing, made far more likely by this PR: a prompt function that returns a bare Image/Audio helper (or a bare content block) — instead of wrapping it in UserMessage — hits Prompt.render's fallback else branch, which JSON-dumps the object with fallback=str, so the client silently receives a text message containing the helper's repr. The PR teaches prompts to accept Image/Audio (Message.init converts them) and exports Message classes publicly, but only when the helper is wrapped in a message; the symmetric spelling tools use (return Image(...)) still degrades to garbage text instead of being converted via the same to_image_content()/to_audio_content() path that Message.init now has three lines above.

    Extended reasoning...

    A user reads the new docs/exports showing prompts accept Image/Audio, and — mirroring the tool pattern def tool() -> Image: return Image(path) — writes @ mcp.prompt() def logo_prompt(): return Image("logo.png"). render() reaches the else branch at prompts/base.py:199-201: pydantic_core.to_json(Image_instance, fallback=str) yields '"<mcp.server.mcpserver.utilities.types.Image object at 0x7f...>"', which is wrapped in a user TextContent message. The client's get_prompt succeeds and the LLM is fed a Python object repr instead of the image — no error, no warning (the branch is # pragma: no cover, so no test would catch it either). Fix at the same depth as the tool path: convert Image/Audio (and pass through ContentBlock) in the render fallback, or raise a clear error.

    Verification: pre-existing — the defective fallback predates this PR, but the diff extends the adjacent conversion code and makes the trap likelier. At src/mcp/server/mcpserver/prompts/base.py:199-201, a prompt result that is not Message/dict/str hits content = pydantic_core.to_json(msg, fallback=str, indent=2).decode(); Image/Audio (plain classes, utilities/types.py:9/57, no serializer or str) theref

Comment thread src/mcp/server/mcpserver/prompts/base.py
Comment thread src/mcp/server/mcpserver/utilities/func_metadata.py
…ompts and templates out of it

- The predicate now recurses only where _convert_to_content renders blocks: through
  Annotated, unions, and list/tuple/Sequence/Iterable items. Mapping values, generic
  TypedDicts/dataclasses parameterised by a block, and type[...] are data again and
  keep their schema, so the docs sentence (now under its own heading, with tuple) and
  the code describe the same rule. Renamed to _returns_content(annotation).
- Prompt.from_function and ResourceTemplate.from_function only ever read arg_model, so
  they pass structured_output=False instead of running tool output-schema derivation;
  an unschematizable return annotation on a prompt or template no longer decides
  whether it registers.
- Tests: dict/model-field cases stay structured; prompt and template registration with
  an unschematizable return annotation; dict-form prompt message with an Image; the
  docs_src pin for the new structured-output section; prompt tests import the message
  classes from mcp.server.mcpserver.
render() special-cased str and JSON-dumped anything else that was not a Message or
dict, so a prompt returning Image(...) or a ready-made content block (or a list mixing
captions and images) reached the client as the object's repr or a JSON blob. Bare
content now becomes one user message via UserMessage(msg), making Message.__init__
the single place prompt content is coerced; the JSON-dump fallback for other values is
unchanged. SyncPromptResult is widened to match.
Comment thread src/mcp/server/mcpserver/utilities/func_metadata.py Outdated
Comment thread src/mcp/server/mcpserver/utilities/func_metadata.py Outdated
Comment thread src/mcp/server/mcpserver/prompts/base.py
@maxisbey

Copy link
Copy Markdown
Contributor Author

Dispositions for the summary-level findings: the @prompt()-should-delegate note and the cubic add_prompt() note are moot since b1f7a29 dropped the function overload; the cubic migration.md note is declined (per VERSIONING.md the guide records breaks between majors — the behaviour-change paragraph is at the top of the PR body for the release notes instead); the add_tool()/tool() docstring copies still say "auto-detects from the return annotation", which remains true, and the rule itself lives on the structured-output page; the three "bare Image from a prompt function → repr" findings are fixed by 2251112 (bare content blocks/Image/Audio, alone or in a list, become user messages; the JSON-dump fallback for other values is unchanged).

AI Disclaimer

…e from content origins; prompt() docstring

- Annotated[X, meta...]: only X is a type, so recurse into it alone (and cover the
  nested-Annotated shape with a test).
- Iterable[...] values are typically generators, which _convert_to_content does not
  unroll, so the annotation no longer counts as content; Sequence stays because its
  runtime value is a list or tuple.
- @mcp.prompt() docstring lists the bare content forms render() now accepts.
Comment thread src/mcp/server/mcpserver/prompts/base.py Outdated
Comment thread docs/servers/structured-output.md Outdated
…once

- Drop the per-item try/except in render(): it only re-raised as 'Could not convert
  prompt result to message: <repr>' and hid the real error (e.g. a missing media file)
  one level deeper; the outer handler already reports 'Error rendering prompt X: ...'.
- message_validator validates the UserMessage | AssistantMessage union left to right.
  Both classes accept either role, so smart mode always landed on the first arm anyway,
  after converting (and, for path-backed Image/Audio, reading) the content for both.
- Docs: the content rule sentence names Sequence alongside list/tuple.
Comment on lines +63 to +67
# Both classes accept either role, so the first arm always matches: validate left to right rather than
# trying both (which converted - and for path-backed Image/Audio, read - the content twice).
message_validator: TypeAdapter[UserMessage | AssistantMessage] = TypeAdapter(
Annotated[UserMessage | AssistantMessage, Field(union_mode="left_to_right")]
)

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.

🟡 [quality] The left_to_right union + explanatory comment is a bandaid on the real design issue: UserMessage/AssistantMessage keep role: Literal["user", "assistant"] instead of narrowing to their own role, so the AssistantMessage arm of message_validator is permanently dead and a dict {"role": "assistant", ...} materializes as a UserMessage instance whose role field is "assistant".

Extended reasoning...

Concrete cost: now that Message/UserMessage/AssistantMessage are exported as public API from mcp.server.mcpserver (this PR's item 3), users will naturally write isinstance(msg, AssistantMessage) dispatch over Prompt.render() results or over prompt middleware — and it silently returns False for every assistant message that was supplied in dict form, because message_validator's first arm (UserMessage) accepts role="assistant". The validator also carries a dead second arm plus a three-line comment explaining why it is dead. Narrowing each subclass to Literal["user"] / Literal["assistant"] (or using a discriminated union on role) would make the union self-describing, classify dict messages as the right class, and remove the need for the union_mode override and comment entirely. Wire output is unaffected (role serializes from the field value), so this is nit-level maintainability/API-shape cost, not a correctness bug.

Verification: nit — the claim is factually true. src/mcp/server/mcpserver/prompts/base.py:48 and :57 keep role: Literal["user", "assistant"] on both subclasses (only defaults differ), so UserMessage accepts role="assistant". The diff's own comment at lines 63-64 concedes the dead arm: "Both classes accept either role, so the first arm always matches: validate left to right rather than trying both"

Comment on lines +26 to +30
"""Base class for all prompt messages.

`content` may be a plain string (wrapped in `TextContent`), an `Image` or `Audio`
helper (converted to `ImageContent` / `AudioContent`), or any ready-made content block.
"""

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.

🟡 [quality] Newly public Message/UserMessage/AssistantMessage docstrings omit a Raises: section even though init now performs file I/O: passing Image(path=...)/Audio(path=...) triggers to_image_content()/to_audio_content(), which open() the file and can raise OSError/FileNotFoundError at message construction time. AGENTS.md requires: "When a public API raises exceptions a caller would reasonably catch, document them in a Raises: section." These classes were promoted to public API in this diff (added to src/mcp/server/mcpserver/init.py all), and the Image/Audio acceptance is also new in this diff, so the catchable-exception surface is new.

Extended reasoning...

Concrete cost (convention, /home/claude/python-sdk/AGENTS.md "Code Quality" rule on Raises: sections): a user building prompt messages with UserMessage(Image(path=...)) has no documented signal that construction reads the file and can raise OSError for a missing/unreadable path — they discover it as an unhandled exception in production instead of a documented, catchable error, and nothing in the class docstring distinguishes this from the pure in-memory data= form. One-line Raises: OSError note on the Message docstring (which UserMessage/AssistantMessage inherit contextually) fixes it.

Verification: nit — the claim is factually accurate on all three legs. (1) The classes were promoted to public API in this diff: src/mcp/server/mcpserver/init.py now imports AssistantMessage, Message, UserMessage from .prompts.base and adds all three to __all__. (2) Construction really does file I/O that can raise OSError: Message.__init__ (src/mcp/server/mcpserver/prompts/base.py:38-41) calls `cont

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.

1 participant