Skip to content

fix(mcpserver): ignore the return annotation when finding the context parameter - #3301

Open
hishammoizuddin wants to merge 1 commit into
modelcontextprotocol:mainfrom
hishammoizuddin:fix-context-return-annotation
Open

fix(mcpserver): ignore the return annotation when finding the context parameter#3301
hishammoizuddin wants to merge 1 commit into
modelcontextprotocol:mainfrom
hishammoizuddin:fix-context-return-annotation

Conversation

@hishammoizuddin

Copy link
Copy Markdown

Fixes #3298

What's wrong

find_context_parameter() loops over everything typing.get_type_hints() returns. That
mapping includes the function's return annotation under the special "return" key, which
isn't a parameter.

So a handler annotated -> Context (or a union containing it) gets recorded as having a
context parameter named "return", and the context is injected under that name when the
handler is called. return is a keyword and can't be declared as a parameter, so the call
always fails — registration succeeds and the breakage only shows up at invocation time.

It isn't only tools: prompts and resource templates resolve their context through the same
function. Reverting just context_injection.py and exercising one of each:

TOOL     is_error: True | Error executing tool tool_fn: tool_fn() got an unexpected keyword
argument 'return'
PROMPT   RAISED: MCPError Internal server error
RESOURCE RAISED: MCPError Error creating resource from template res://x

The fix

Drop the "return" key before scanning the hints, so only real parameters are considered.

Why this should be safe

Every path that previously resolved to "return" failed at invocation, so nothing could
have been depending on it — this only turns a guaranteed failure into working behaviour.

Handlers that have both a real Context parameter and a Context return annotation were
already fine, because get_type_hints() yields parameters before return (you get
['a', 'ctx', 'b', 'return'] for def f(a, ctx, b) -> Context). I've pinned that ordering
in a test so a later refactor can't quietly break it.

Tests

New tests/server/mcpserver/test_context_injection.py:

  • the positive path (a parameter annotated Context is found)
  • -> Context isn't reported as a parameter
  • -> Context | None isn't reported as a parameter
  • a real Context parameter still wins when the return annotation is also Context
  • end-to-end through Client(server) for a tool, a prompt and a resource template

Five of the seven fail without the src/ change.

./scripts/test passes (100% branch coverage, strict-no-cover clean), ruff and pyright
are clean on the changed files, and the new tests pass on 3.10 and 3.14.

Related PRs

No overlap with #3236 or #2623, which touch the same module for unrelated reasons. I
test-merged both against this branch and neither conflicts.


Disclosure: I used AI assistance (Claude Code) for this change. I've reviewed it myself
and can explain it.

… parameter

`find_context_parameter` iterated every entry from `typing.get_type_hints`,
which includes the return annotation under the special `"return"` key. A tool,
prompt, or resource function annotated `-> Context` (or a union containing it)
was therefore recorded as having a context parameter named `"return"`, and the
handler failed at invocation with `got an unexpected keyword argument 'return'`.

Drop the `"return"` key before scanning, so only real parameters are considered.

Fixes modelcontextprotocol#3298

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@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 2 files

Re-trigger cubic

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.

find_context_parameter treats a Context return annotation as a parameter

1 participant