Uh oh!
There was an error while loading. Please reload this page.
fix: preserve typed Context in resource and prompt handlers - #3236
fix: preserve typed Context in resource and prompt handlers#3236morluto wants to merge 5 commits into
Conversation
There was a problem hiding this comment.
2 issues found across 8 files
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/handlers/lifespan.md">
<violation number="1" location="docs/handlers/lifespan.md:44">
P2: Static resources still cannot declare `ctx`: decoration raises `ValueError`, so this statement leads readers to a non-starting server. Limit the resource claim to URI-template resources and call out the static-resource restriction.</violation>
</file>
<file name="tests/server/mcpserver/test_server.py">
<violation number="1" location="tests/server/mcpserver/test_server.py:1200">
P1: The `ctx: Context[None]` subscripted-generic annotation is not detected by `find_context_parameter`, which only checks `inspect.isclass(annotation)` or matches a class argument in `get_args` (here `(NoneType,)`, not a Context subclass). For this annotation context_kwarg resolves to None, so the context parameter is not skipped from the schema and never injected — breaking the test's `template.context_kwarg == "ctx"` assertion and the typed-context behavior the PR intends. Either use a plain `Context` annotation, or update `find_context_parameter` to detect the generic origin (e.g. `issubclass(get_origin(annotation), Context)`) so typed `Context[...]` annotations are preserved.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
2 issues found across 8 files
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/handlers/lifespan.md">
<violation number="1" location="docs/handlers/lifespan.md:44">
P3: Users reading the Context page are still told `Context` works on any resource, then hit a decoration-time error on static resources. Update that page to distinguish URI-template resources while documenting this restriction here.</violation>
</file>
<file name="tests/server/mcpserver/resources/test_resource_template.py">
<violation number="1" location="tests/server/mcpserver/resources/test_resource_template.py:229">
P3: As written, this regression test won't actually guard the fix for issue #3235: `ctx` is unannotated, so Pydantic's validator treats it as `Any` and forwards the exact same object even under the old `validate_call(fn)` behavior — meaning `assert ctx is context` passes with or without the SkipValidation change. To make the identity assertion a real regression guard, annotate the parameter as `Context` (ideally `Context[LifespanState]`) so the pre-fix reconstruction breaks the identity check; `Context` is already imported and `context_kwarg="ctx"` keeps it out of the generated schema.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
1 issue found across 9 files
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="tests/server/mcpserver/test_server.py">
<violation number="1" location="tests/server/mcpserver/test_server.py:1200">
P2: The new tests annotate the context parameter as `Context[None]`, but `find_context_parameter` cannot detect a parameterized `Context[...]` annotation: it only matches annotations that are a direct `Context` subclass or whose `get_args()` entries are `Context` subclasses. For `Context[None]` the origin is `Context` but the arg is `NoneType`, so it returns `None`. That leaves `context_kwarg` unset, so context is never injected and these assertions (`ctx.request_context is not None`, `template.context_kwarg == 'ctx'`) are testing injected context that won't be present. Consider extending `find_context_parameter` to also check `get_origin(annotation)` (and `typing.get_origin`'s origin) before/alongside the arg loop, so `Context[LifespanState]`/`Context[None]` parameterized handlers are recognized.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Uh oh!
There was an error while loading. Please reload this page.
Fixes#3235.
Problem
The server creates one live
Contextobject for each request. That object carries private state linking it to the active request and session.Resource-template and prompt handlers are wrapped with Pydantic's
validate_call. When a handler uses the preciseContext[LifespanState]annotation, Pydantic reconstructs the context as a new model instead of passing through the server-created object. Private request state is not part of that reconstructed model, soctx.request_context,ctx.session, and related properties raise:Bare
Contexthappened to pass through unchanged, so the failure appeared only when resource or prompt handlers usedContext[LifespanState]. Tool handlers were unaffected because they use a different argument-validation path.Change
The validation wrapper now marks only the SDK-injected context parameter with Pydantic's
SkipValidation. Caller-supplied arguments still receive the existing validation and coercion, including parameters collected through**kwargs, while the original live context reaches synchronous and asynchronous handlers unchanged.The wrapper restores the handler's public signature after Pydantic builds its validator, so introspection and generated schemas continue to expose the original types.
The lifespan and migration documentation now describes
Context[LifespanState]as supported across tools, resource templates, and prompts; static resources do not support context injection.Regression coverage
The context-injection tests now use parameterized
Contextand access live request state through an asynchronous resource handler and a synchronous prompt handler. A resource-template regression also verifies that the exact context object is preserved while URI parameters continue to flow through**kwargs.On the affected base revision, these tests fail when the reconstructed context first accesses its request state.
Validation
uv run coverage run -m pytest -n auto— 5,585 passed, 8 skipped, 1 xfaileduv run coverage report— 100.00% statement and branch coverageuv run ruff format --check .— passeduv run ruff check .— passeduv run pyright— passeduvx --from pre-commit pre-commit run markdownlint --files docs/handlers/lifespan.md docs/migration.md— passedAI assistance was used to investigate and prepare this contribution; the final diff and validation results were reviewed before submission.