-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Bring Docket's dependency injection to FastMCP #2318
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughIntegrates Docket-based dependency injection throughout FastMCP by replacing manual Context kwarg handling with wrapper-based parameter filtering and centralized dependency resolution. Adds new dependency utilities and comprehensive tests while maintaining backward compatibility. Changes
Sequence DiagramsequenceDiagram
participant User
participant Tool
participant Wrapper
participant DependencyResolver
participant OriginalFn
User->>Tool: Invoke with user args
Note over Tool: Old flow: manual Context kwarg extraction
Tool->>Tool: Build Context
Tool->>OriginalFn: Call with Context kwarg + user args
OriginalFn-->>Tool: Result
User->>Tool: Invoke with user args
Note over Tool: New flow: dependency resolution
Tool->>Wrapper: Get user-facing signature
Tool->>DependencyResolver: resolve_dependencies(OriginalFn, user_args)
DependencyResolver->>DependencyResolver: Resolve Docket deps
DependencyResolver->>DependencyResolver: Inject Context if needed
DependencyResolver-->>Tool: Merged kwargs
Tool->>OriginalFn: Call with resolved kwargs
OriginalFn-->>Tool: Result
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
This introduces Docket's `Depends()` to all FastMCP tools, resources, and prompts, laying the groundwork for [MCP SEP-1686](modelcontextprotocol/modelcontextprotocol#1686) background task support. The implementation uses a wrapper-based approach that completely hides dependency injection from Pydantic while maintaining full validation of user arguments. The wrapper function (`without_injected_parameters()`) excludes Context and Docket dependencies, making functions safe for Pydantic's TypeAdapter without requiring schemas for things like database connections. Key outcomes: - Users can write dependency functions and inject them with `Depends()` - Works across tools, resources, templates, and prompts - Dependencies properly excluded from MCP schemas - Context managers stay open during async execution - External callers can't override dependencies (security) - 100% backward compatible with existing Context injection Next step: Wire up Docket's task system for background execution. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
75062fb to
d7fdbdf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
pyproject.toml(1 hunks)src/fastmcp/__init__.py(2 hunks)src/fastmcp/prompts/prompt.py(4 hunks)src/fastmcp/resources/resource.py(1 hunks)src/fastmcp/resources/template.py(4 hunks)src/fastmcp/server/dependencies.py(3 hunks)src/fastmcp/server/server.py(1 hunks)src/fastmcp/tools/tool.py(3 hunks)src/fastmcp/utilities/types.py(1 hunks)tests/server/test_dependencies.py(1 hunks)tests/utilities/test_types.py(0 hunks)
💤 Files with no reviewable changes (1)
- tests/utilities/test_types.py
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use Python ≥ 3.10 and provide full type annotations for library code
Files:
src/fastmcp/utilities/types.pysrc/fastmcp/server/server.pysrc/fastmcp/tools/tool.pysrc/fastmcp/__init__.pysrc/fastmcp/resources/template.pysrc/fastmcp/resources/resource.pysrc/fastmcp/server/dependencies.pysrc/fastmcp/prompts/prompt.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Never use bare except; always catch specific exception types
Files:
src/fastmcp/utilities/types.pysrc/fastmcp/server/server.pysrc/fastmcp/tools/tool.pysrc/fastmcp/__init__.pysrc/fastmcp/resources/template.pytests/server/test_dependencies.pysrc/fastmcp/resources/resource.pysrc/fastmcp/server/dependencies.pysrc/fastmcp/prompts/prompt.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/**/*.py: Tests must be atomic, self-contained, and cover a single functionality
Use pytest parameterization for multiple examples of the same functionality
Use separate tests for distinct pieces of functionality
Always put imports at the top of test files; do not import inside test bodies
Do not add @pytest.mark.asyncio; asyncio_mode = "auto" is set globally
Prefer in-memory transport for tests; use HTTP transport only when explicitly testing networking
For slow/long-running tests, mark them as integration or optimize (default timeout is 5s)
In tests, use # type: ignore[attr-defined] for MCP results instead of type assertions
Files:
tests/server/test_dependencies.py
🧬 Code graph analysis (8)
src/fastmcp/utilities/types.py (2)
src/fastmcp/server/dependencies.py (3)
without_injected_parameters(62-116)wrapper(99-104)resolve_dependencies(176-212)src/fastmcp/server/context.py (2)
fastmcp(155-160)Context(103-692)
src/fastmcp/server/server.py (1)
src/fastmcp/server/dependencies.py (1)
without_injected_parameters(62-116)
src/fastmcp/tools/tool.py (2)
src/fastmcp/server/dependencies.py (2)
get_context(215-221)without_injected_parameters(62-116)src/fastmcp/utilities/types.py (2)
without_injected_parameters(45-106)get_cached_typeadapter(110-182)
src/fastmcp/resources/template.py (2)
src/fastmcp/server/dependencies.py (3)
get_context(215-221)without_injected_parameters(62-116)resolve_dependencies(176-212)src/fastmcp/utilities/types.py (2)
without_injected_parameters(45-106)get_cached_typeadapter(110-182)
tests/server/test_dependencies.py (2)
src/fastmcp/server/server.py (10)
FastMCP(148-2690)tool(1364-1378)tool(1381-1395)tool(1397-1529)name(359-360)resource(1613-1748)prompt(1773-1784)prompt(1787-1798)prompt(1800-1942)get_prompt(608-612)src/fastmcp/server/context.py (2)
Context(103-692)request_id(300-302)
src/fastmcp/resources/resource.py (1)
src/fastmcp/server/dependencies.py (1)
resolve_dependencies(176-212)
src/fastmcp/server/dependencies.py (2)
src/fastmcp/server/context.py (2)
fastmcp(155-160)Context(103-692)src/fastmcp/utilities/types.py (3)
is_class_member_of_type(195-214)without_injected_parameters(45-106)wrapper(87-94)
src/fastmcp/prompts/prompt.py (3)
src/fastmcp/server/dependencies.py (3)
get_context(215-221)without_injected_parameters(62-116)resolve_dependencies(176-212)src/fastmcp/utilities/types.py (2)
without_injected_parameters(45-106)get_cached_typeadapter(110-182)src/fastmcp/utilities/json_schema.py (1)
compress_schema(200-230)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run tests with lowest-direct dependencies
- GitHub Check: Run tests: Python 3.10 on windows-latest
Use the wrapper's signature in _convert_string_arguments() to ensure we're only operating on user-facing parameters. This prevents theoretical issues where dependency parameter types could interfere with conversion logic. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Capture and reset the tokens from _Depends.cache and _Depends.stack to avoid leaving stale contextvar state. This prevents issues with reentrant calls or sequential invocations where closed AsyncExitStacks or old cache dicts could cause errors. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/fastmcp/prompts/prompt.py(5 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use Python ≥ 3.10 and provide full type annotations for library code
Files:
src/fastmcp/prompts/prompt.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Never use bare except; always catch specific exception types
Files:
src/fastmcp/prompts/prompt.py
🧬 Code graph analysis (1)
src/fastmcp/prompts/prompt.py (3)
src/fastmcp/server/dependencies.py (3)
get_context(215-221)without_injected_parameters(62-116)resolve_dependencies(176-212)src/fastmcp/utilities/types.py (1)
get_cached_typeadapter(45-117)src/fastmcp/utilities/json_schema.py (1)
compress_schema(200-230)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run tests with lowest-direct dependencies
- GitHub Check: Run tests: Python 3.10 on windows-latest
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/fastmcp/server/dependencies.py(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use Python ≥ 3.10 and provide full type annotations for library code
Files:
src/fastmcp/server/dependencies.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Never use bare except; always catch specific exception types
Files:
src/fastmcp/server/dependencies.py
🧬 Code graph analysis (1)
src/fastmcp/server/dependencies.py (2)
src/fastmcp/server/context.py (3)
fastmcp(155-160)Context(103-692)error(402-416)src/fastmcp/utilities/types.py (1)
is_class_member_of_type(130-149)
The resolved_dependencies function was setting contextvars without capturing reset tokens, which left stale references after the context manager exited. This could cause issues in reentrant or sequential calls, like "stack is closed" errors when the AsyncExitStack was already closed but still referenced. Now we capture tokens from all contextvar.set() calls and reset them in finally blocks to restore prior state. This follows the same pattern suggested in the fastmcp PR review: jlowin/fastmcp#2318 Added tests that verify contextvars are properly isolated and cleaned up between task executions, including a test for reentrant calls to resolved_dependencies that would previously fail with stale context. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Add security filtering in resolve_dependencies() to strip out any dependency parameter names from user arguments. This prevents external callers from injecting values for dependencies by providing keys that match parameter names. Particularly important for prompts and resource templates which don't validate against strict schemas. The filtering provides defense-in-depth across all component types. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/fastmcp/server/dependencies.py (1)
99-105: Critical: Context managers close before generators are consumed (unresolved from past review).The current implementation exits the
resolve_dependenciescontext before the returned generator is consumed. Iffnreturns an async or sync generator, the dependencies (including any database connections, file handles, etc.) will be closed immediately after the generator object is created but before any iteration occurs. This will cause "stack closed" errors for streaming responses.The previous review comment on these lines flagged this exact issue but was never marked as addressed. The suggested fix requires:
- Manually entering the context manager (
__aenter__)- Detecting if the result is a generator (sync or async)
- If it's a generator, wrapping it to keep dependencies alive during iteration
- Ensuring
__aexit__is called exactly once after consumption completesExample of the required pattern:
async def wrapper(**user_kwargs: Any) -> Any: - async with resolve_dependencies(fn, user_kwargs) as resolved_kwargs: - result = fn(**resolved_kwargs) - if inspect.isawaitable(result): - result = await result - return result + # Manually manage the context to keep it alive for generators + ctx = resolve_dependencies(fn, user_kwargs) + resolved_kwargs = await ctx.__aenter__() + + try: + result = fn(**resolved_kwargs) + + # Handle async generators + if inspect.isasyncgen(result): + async def wrapper_async_gen(): + try: + async for item in result: + yield item + finally: + await ctx.__aexit__(None, None, None) + return wrapper_async_gen() + + # Handle sync generators + elif inspect.isgenerator(result): + async def wrapper_sync_gen(): + try: + for item in result: + yield item + finally: + await ctx.__aexit__(None, None, None) + return wrapper_sync_gen() + + # Handle awaitables + elif inspect.isawaitable(result): + result = await result + await ctx.__aexit__(None, None, None) + return result + + # Handle plain values + else: + await ctx.__aexit__(None, None, None) + return result + + except Exception: + # On exception, pass it to __aexit__ + import sys + await ctx.__aexit__(*sys.exc_info()) + raise
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/fastmcp/server/dependencies.py(3 hunks)tests/server/test_dependencies.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Never use bare except; always catch specific exception types
Files:
tests/server/test_dependencies.pysrc/fastmcp/server/dependencies.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/**/*.py: Tests must be atomic, self-contained, and cover a single functionality
Use pytest parameterization for multiple examples of the same functionality
Use separate tests for distinct pieces of functionality
Always put imports at the top of test files; do not import inside test bodies
Do not add @pytest.mark.asyncio; asyncio_mode = "auto" is set globally
Prefer in-memory transport for tests; use HTTP transport only when explicitly testing networking
For slow/long-running tests, mark them as integration or optimize (default timeout is 5s)
In tests, use # type: ignore[attr-defined] for MCP results instead of type assertions
Files:
tests/server/test_dependencies.py
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use Python ≥ 3.10 and provide full type annotations for library code
Files:
src/fastmcp/server/dependencies.py
🧬 Code graph analysis (2)
tests/server/test_dependencies.py (2)
src/fastmcp/server/server.py (10)
FastMCP(148-2690)tool(1364-1378)tool(1381-1395)tool(1397-1529)name(359-360)resource(1613-1748)prompt(1773-1784)prompt(1787-1798)prompt(1800-1942)get_prompt(608-612)src/fastmcp/server/context.py (2)
Context(103-692)request_id(300-302)
src/fastmcp/server/dependencies.py (1)
src/fastmcp/utilities/types.py (1)
is_class_member_of_type(130-149)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run tests: Python 3.10 on windows-latest
- GitHub Check: Run tests with lowest-direct dependencies
🔇 Additional comments (11)
src/fastmcp/server/dependencies.py (5)
1-33: LGTM: Imports and public API properly declared.The imports are appropriate for the dependency injection functionality, and the
__all__declaration correctly exposes the new public utilities alongside existing ones.
36-59: LGTM: Type-based parameter lookup is robust.The function correctly handles methods, type hints with fallback, and supports union/annotated types through
is_class_member_of_type.
147-178: LGTM: Contextvar hygiene properly implemented.The contextvar reset issue from the previous review has been correctly addressed. Both
cache_tokenandstack_tokenare properly captured and reset infinallyblocks, ensuring contextvars are always restored even on exceptions.
181-227: LGTM: Security filtering prevents dependency override.The function correctly filters dependency parameters from user arguments before resolution, preventing external callers from overriding injected dependencies. This is a critical security feature that's well-documented in the code.
229-327: No changes to existing utilities.The existing utility functions (
get_context,get_http_request,get_http_headers,get_access_token) remain unchanged and are outside the scope of this dependency injection feature.tests/server/test_dependencies.py (6)
1-38: LGTM: Well-structured test setup.Imports are properly organized at the top, the
Connectiontest utility effectively tracks context manager lifecycle, and the fixture follows best practices.
40-147: LGTM: Comprehensive basic dependency tests.Tests are atomic, self-contained, and cover sync/async dependencies, context managers, nested dependencies, and schema exclusion. All follow the coding guidelines.
149-231: LGTM: Validates backward compatibility and advanced features.These tests confirm 100% backward compatibility with existing Context injection and validate important features like dependency caching and mixed injection modes.
233-310: LGTM: Dependencies work across all FastMCP entity types.Tests confirm that dependency injection works consistently across tools, resources, and prompts, with proper support for Context injection in all cases.
464-569: LGTM: Sync context managers properly tested.Tests validate that synchronous context managers work correctly across all entity types, with proper cleanup after execution.
423-446: LGTM: Comprehensive security validation.Tests thoroughly validate that external callers cannot override injected dependencies through any vector (tool arguments, prompt arguments, resource URIs). The validation test confirms that Pydantic validation still works for user-provided arguments.
Also applies to: 571-701
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
@chrisguidry this is looking great! So the headline on this PR is an overhaul of FastMCP's existing (hacked up) DI system, but no additional functionality (yet) |
|
The big addition here is that you get a So no background execution just yet but definitely a nice new feature (IMO)! https://chrisguidry.github.io/docket/dependencies/#custom-dependencies |
The `resolved_dependencies` function was setting contextvars without capturing reset tokens, which left stale references after the context manager exited. This could cause issues in reentrant or sequential calls, like "stack is closed" errors when the AsyncExitStack was already closed but still referenced. Now we capture tokens from all `contextvar.set()` calls and reset them in finally blocks to restore prior state. This follows the same pattern suggested in the fastmcp PR review: jlowin/fastmcp#2318 ## Changes - Modified `resolved_dependencies` to capture reset tokens from all 5 contextvar sets - Added nested try/finally blocks to ensure proper cleanup even on exceptions - Added 5 new tests that verify contextvar isolation and cleanup ## Testing Added comprehensive tests including: - `test_contextvar_reset_on_reentrant_call` - Demonstrates the stale contextvar issue (fails without fix, passes with fix) - `test_contextvar_not_leaked_to_caller` - Verifies no leakage outside context - `test_contextvar_isolation_between_tasks` - Confirms isolation between sequential tasks - `test_contextvar_cleanup_after_task` - Ensures cleanup after task completion - `test_async_exit_stack_cleanup` - Verifies AsyncExitStack cleanup All 272 tests pass with 100% coverage maintained in `src/docket/dependencies.py`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude <[email protected]>
Add a dedicated `fastmcp.dependencies` module to prevent namespace pollution in the top-level `fastmcp` package. This moves `Depends` (and future Docket exports like `CurrentDocket`) into a semantic grouping that's easier to extend. Changes: - New `src/fastmcp/dependencies.py` re-exports `Depends` from Docket - Remove `Depends` from top-level `fastmcp.__init__` - Update imports in `tests/server/test_dependencies.py` All tests pass, no breaking changes for existing code. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/fastmcp/dependencies.py(1 hunks)tests/server/test_dependencies.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use Python ≥ 3.10 and provide full type annotations for library code
Files:
src/fastmcp/dependencies.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Never use bare except; always catch specific exception types
Files:
src/fastmcp/dependencies.pytests/server/test_dependencies.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/**/*.py: Tests must be atomic, self-contained, and cover a single functionality
Use pytest parameterization for multiple examples of the same functionality
Use separate tests for distinct pieces of functionality
Always put imports at the top of test files; do not import inside test bodies
Do not add @pytest.mark.asyncio; asyncio_mode = "auto" is set globally
Prefer in-memory transport for tests; use HTTP transport only when explicitly testing networking
For slow/long-running tests, mark them as integration or optimize (default timeout is 5s)
In tests, use # type: ignore[attr-defined] for MCP results instead of type assertions
Files:
tests/server/test_dependencies.py
🧬 Code graph analysis (1)
tests/server/test_dependencies.py (2)
src/fastmcp/server/server.py (11)
FastMCP(148-2690)tool(1364-1378)tool(1381-1395)tool(1397-1529)name(359-360)_list_tools_mcp(682-697)resource(1613-1748)prompt(1773-1784)prompt(1787-1798)prompt(1800-1942)get_prompt(608-612)src/fastmcp/server/context.py (2)
Context(103-692)request_id(300-302)
🔇 Additional comments (6)
src/fastmcp/dependencies.py (1)
1-10: LGTM!This module provides a clean abstraction layer for dependency injection by re-exporting
Dependsfrom the docket package. The implementation is minimal and correct, with appropriate documentation.tests/server/test_dependencies.py (5)
1-38: LGTM!The imports, test fixtures, and helper classes are well-organized and follow the coding guidelines. Imports are at the top, and the
Connectiontest class appropriately models a resource with lifecycle management.
465-570: Good test organization for distinct APIs.While these four tests (sync context managers for tools, resources, resource templates, and prompts) follow similar patterns, keeping them separate is appropriate since they test distinct decorators and code paths. This aligns with the guideline to use separate tests for distinct functionality.
572-702: Excellent security test coverage.These tests provide comprehensive validation that external callers cannot override internal dependencies across all FastMCP primitives (tools, prompts, resources, resource templates). This is critical for preventing injection attacks and maintaining security boundaries. The inclusion of URI parameter collision testing (lines 686-702) demonstrates thorough consideration of edge cases.
129-148: Thorough validation of schema exclusion.The tests effectively verify that dependency parameters are excluded from MCP schemas, which is essential for keeping schemas clean and preventing exposure of internal types. Testing both simple dict dependencies and complex Connection types ensures comprehensive coverage.
Also applies to: 449-463
150-163: Good backward compatibility coverage.These tests ensure that existing
Contextinjection via type annotation remains functional while also working alongside the newDependsAPI. This is critical for maintaining 100% backward compatibility as stated in the PR objectives.Also applies to: 208-232
| async def test_async_tool_context_manager_stays_open(mcp: FastMCP): | ||
| """Test that context manager dependencies stay open during async tool execution. | ||
| Context managers must remain open while the async function executes, not just | ||
| while it's being called (which only returns a coroutine). | ||
| """ | ||
|
|
||
| @mcp.tool() | ||
| async def query_data( | ||
| query: str, connection: Connection = Depends(get_connection) | ||
| ) -> str: | ||
| assert connection.is_open | ||
| return f"open={connection.is_open}" | ||
|
|
||
| async with Client(mcp) as client: | ||
| result = await client.call_tool("query_data", {"query": "test"}) | ||
| content = result.content[0] | ||
| assert isinstance(content, TextContent) | ||
| assert content.text == "open=True" | ||
|
|
||
|
|
||
| async def test_async_resource_context_manager_stays_open(mcp: FastMCP): | ||
| """Test that context manager dependencies stay open during async resource execution.""" | ||
|
|
||
| @mcp.resource("data://config") | ||
| async def load_config(connection: Connection = Depends(get_connection)) -> str: | ||
| assert connection.is_open | ||
| return f"open={connection.is_open}" | ||
|
|
||
| async with Client(mcp) as client: | ||
| result = await client.read_resource("data://config") | ||
| content = result[0] | ||
| assert isinstance(content, TextResourceContents) | ||
| assert content.text == "open=True" | ||
|
|
||
|
|
||
| async def test_async_resource_template_context_manager_stays_open(mcp: FastMCP): | ||
| """Test that context manager dependencies stay open during async resource template execution.""" | ||
|
|
||
| @mcp.resource("user://{user_id}") | ||
| async def get_user( | ||
| user_id: str, connection: Connection = Depends(get_connection) | ||
| ) -> str: | ||
| assert connection.is_open | ||
| return f"open={connection.is_open},user={user_id}" | ||
|
|
||
| async with Client(mcp) as client: | ||
| result = await client.read_resource("user://123") | ||
| content = result[0] | ||
| assert isinstance(content, TextResourceContents) | ||
| assert "open=True" in content.text | ||
|
|
||
|
|
||
| async def test_async_prompt_context_manager_stays_open(mcp: FastMCP): | ||
| """Test that context manager dependencies stay open during async prompt execution.""" | ||
|
|
||
| @mcp.prompt() | ||
| async def research_prompt( | ||
| topic: str, connection: Connection = Depends(get_connection) | ||
| ) -> str: | ||
| assert connection.is_open | ||
| return f"open={connection.is_open},topic={topic}" | ||
|
|
||
| async with Client(mcp) as client: | ||
| result = await client.get_prompt("research_prompt", {"topic": "AI"}) | ||
| message = result.messages[0] | ||
| content = message.content | ||
| assert isinstance(content, TextContent) | ||
| assert "open=True" in content.text | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Address the outstanding generator/streaming test coverage.
The previous review comment correctly identifies a gap in context manager lifecycle testing. The current tests only validate simple return values, but generator/streaming scenarios are critical for catching lifecycle bugs where dependencies might close prematurely during iteration.
The past review provides specific test examples for both async and sync generators. These tests are essential because:
- Tools that stream results are a common FastMCP pattern
- Context managers must stay open during the entire streaming period
- The bug mentioned in
without_injected_parameterswould only surface with generators
Please implement the generator/streaming tests suggested in the previous review, or if they've been addressed elsewhere, mark that comment as resolved.
| # Invalid argument type should fail validation | ||
| with pytest.raises(Exception): # Will be ToolError wrapping validation error | ||
| await client.call_tool("validated_tool", {"age": "not a number"}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace bare Exception with specific exception type.
The coding guidelines require catching specific exception types rather than bare Exception. Based on the comment, this should catch ToolError or a validation error type.
Apply this diff:
- # Invalid argument type should fail validation
- with pytest.raises(Exception): # Will be ToolError wrapping validation error
- await client.call_tool("validated_tool", {"age": "not a number"})
+ # Invalid argument type should fail validation
+ from mcp.types import ToolError
+ with pytest.raises(ToolError):
+ await client.call_tool("validated_tool", {"age": "not a number"})Based on coding guidelines.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In tests/server/test_dependencies.py around lines 444 to 446, replace the bare
pytest.raises(Exception) with a specific exception type (e.g.,
pytest.raises(ToolError) or the concrete validation error used by the tool) to
follow the guideline of catching specific exceptions; update or add the
necessary import for ToolError (or the actual validation exception class) at the
top of the test file and change the context manager to
pytest.raises(TheSpecificException) so the test asserts the correct error is
raised.
| # Try to override dependency - rejected (not in schema) | ||
| with pytest.raises(Exception): | ||
| await client.call_tool( | ||
| "check_permission", {"action": "read", "admin": "hacker"} | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace bare Exception with specific exception type.
The coding guidelines require catching specific exception types. This test expects validation to reject the extra admin parameter.
Apply this diff to use a specific exception type:
- # Try to override dependency - rejected (not in schema)
- with pytest.raises(Exception):
- await client.call_tool(
- "check_permission", {"action": "read", "admin": "hacker"}
- )
+ # Try to override dependency - rejected (not in schema)
+ from mcp.types import ToolError
+ with pytest.raises(ToolError):
+ await client.call_tool(
+ "check_permission", {"action": "read", "admin": "hacker"}
+ )Based on coding guidelines.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In tests/server/test_dependencies.py around lines 596 to 600, the test currently
uses a bare `Exception` when expecting parameter validation to reject the extra
`admin` field; replace the broad `with pytest.raises(Exception):` with `with
pytest.raises(pydantic.error_wrappers.ValidationError):` (or the concrete
ValidationError class used by your validation library) and add the corresponding
import at the top of the test file (e.g., `from pydantic.error_wrappers import
ValidationError`) so the test asserts the specific validation error type.
Brings FastAPI-style background task execution to FastMCP as the next step
toward MCP SEP-1686 support. When `FASTMCP_EXPERIMENTAL_ENABLE_DOCKET=true`,
FastMCP automatically creates a Docket instance with an in-process Worker,
enabling tools to schedule background tasks via dependency injection.
```python
from fastmcp.dependencies import CurrentDocket
@mcp.tool()
async def analyze_data(query: str, docket: Docket = CurrentDocket()) -> str:
# Schedule work to run in background
await docket.add(long_running_analysis)(query)
return "Analysis started"
```
This provides Starlette-like background task capabilities without requiring
distributed infrastructure. All tasks run in the same process via memory://
broker. Distributed workers will come in a future phase.
Also adds `CurrentContext()` and `CurrentWorker()` dependency functions
following Docket's pattern (no `Depends()` wrapper needed).
New `fastmcp.dependencies` module centralizes all dependency injection exports
to keep the top-level namespace clean.
Related: https://linear.app/prefect/document/fastmcp-sep-1686-tasks-7c4711171324
Based on: #2318
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
|
@chrisguidry let's hold merging this and #2326 as I think they may be appropriate for a fully documented 2.14 release and there's a lot in flight that will go out with 2.13.1. Can also set up a tracking branch like #2329 |
|
Sounds great I can set up a feature branch for sep-1686 and land all these there |
Brings FastAPI-style background task execution to FastMCP as the next step
toward MCP SEP-1686 support. When `FASTMCP_EXPERIMENTAL_ENABLE_DOCKET=true`,
FastMCP automatically creates a Docket instance with an in-process Worker,
enabling tools to schedule background tasks via dependency injection.
```python
from fastmcp.dependencies import CurrentDocket
@mcp.tool()
async def analyze_data(query: str, docket: Docket = CurrentDocket()) -> str:
# Schedule work to run in background
await docket.add(long_running_analysis)(query)
return "Analysis started"
```
This provides Starlette-like background task capabilities without requiring
distributed infrastructure. All tasks run in the same process via memory://
broker. Distributed workers will come in a future phase.
Also adds `CurrentContext()` and `CurrentWorker()` dependency functions
following Docket's pattern (no `Depends()` wrapper needed).
New `fastmcp.dependencies` module centralizes all dependency injection exports
to keep the top-level namespace clean.
Related: https://linear.app/prefect/document/fastmcp-sep-1686-tasks-7c4711171324
Based on: #2318
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
This introduces Docket's
Depends()to all FastMCP tools, resources, and prompts, laying the groundwork for MCP SEP-1686 background task support.The implementation uses a wrapper-based approach that completely hides dependency injection from Pydantic while maintaining full validation of user arguments. The wrapper function (
without_injected_parameters()) excludes Context and Docket dependencies, making functions safe for Pydantic's TypeAdapter without requiring schemas for things like database connections.Key outcomes:
Depends()Example:
Next step: Wire up Docket's task system for background execution.
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Dependspattern, enabling cleaner injection of dependencies into tools, prompts, and resources with automatic lifecycle management and async context manager support.Chores
pydocket>=0.12.0as a new dependency to support advanced dependency injection capabilities.