Skip to content

Refactor provider execution: components own their execution#2663

Merged
jlowin merged 8 commits into
mainfrom
provider-refactor-wip
Dec 21, 2025
Merged

Refactor provider execution: components own their execution#2663
jlowin merged 8 commits into
mainfrom
provider-refactor-wip

Conversation

@jlowin
Copy link
Copy Markdown
Member

@jlowin jlowin commented Dec 21, 2025

This refactor moves task routing logic from Provider into the component classes themselves.

Key changes:

  • Providers source, components execute: Provider no longer has call_tool(), read_resource(), etc. - it only lists and looks up components. Components handle their own execution via run(), read(), render(). This is to avoid two potentially-conflicting ways to do the same thing (e.g. run a tool). Execution logic should be contained on a tool subclass.

  • Unified task routing: New check_background_task() in routing.py consolidates the duplicated task mode enforcement logic that was copy-pasted across Tool._run(), Resource._read(), Prompt._render(), and ResourceTemplate._read().

  • convert_result() as instance methods: Moved from standalone functions (convert_to_tool_result(result, tool)) to instance methods (tool.convert_result(result)) on base classes, enabling subclass customization.

  • Wrapper components for middleware chains: FastMCPProviderTool, FastMCPProviderResource, etc. wrap components to delegate execution through nested server middleware.

This closes arguably a significant issue with the previous background implementation: it avoided all middleware. Now, background tasks are appropriately passed through all (potentially nested) middleware stacks.

…nents

- Remove execution methods (call_tool, read_resource, etc.) from Provider base
- Add FastMCPProvider* wrapper classes that delegate to child server middleware
- Move task routing to Tool._run() using contextvars (_task_metadata, _tool_call_key)
- Add convert_to_tool_result(result, output_schema) utility for Docket results
- Add convert_to_prompt_result() utility for prompt task results
- Pass namespaced key via add_to_docket(name=) for mounted tool lookup
All components now use explicit fn_key (function lookup) and task_key
(result storage) parameters instead of relying on implicit key handling.
This fixes mounted component task execution where the MCP-visible key
differs from the Docket-registered function name.
Tests verify middleware runs at parent, child, and grandchild levels
for tools, resources, prompts, and resource templates.
…n progress

Work in progress on refactoring execution to use component _read()/_run()/_render() methods.
Template background tasks not yet working - needs fix for Docket key lookup.
Pass Tool/Prompt/Resource/Template to conversion functions instead of
individual attributes, ensuring access to serializer, output_schema,
mime_type, etc. Also fixes mixed-content output schema validation.
…helper

- Add convert_result() instance methods to all component types (Tool, Prompt, Resource, ResourceTemplate)
- Extract duplicated task routing logic into check_background_task() helper
- Fix type annotations on FastMCPProviderResource.read() and FastMCPProviderPrompt.render()
- Update protocol.py to use component.convert_result() uniformly
@marvin-context-protocol marvin-context-protocol Bot added enhancement Improvement to existing functionality. For issues and smaller PR improvements. server Related to FastMCP server implementation or server-side functionality. provider Related to the FastMCP Provider class labels Dec 21, 2025
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Dec 21, 2025

Walkthrough

This PR implements the SEP-1686 background task protocol across the FastMCP codebase. Changes introduce a unified task submission and routing system by adding convert_result() methods to Tool, Resource, Prompt, and ResourceTemplate classes for standardized value conversion; extending add_to_docket() signatures across components to accept fn_key and task_key parameters; implementing centralized task routing via check_background_task() and submit_to_docket() functions; introducing context variables (_task_metadata, _docket_fn_key) and get_task_metadata() API for task metadata propagation; expanding return type annotations for managers and server handlers to include mcp.types.CreateTaskResult; refactoring the Provider base class to remove direct execution methods and delegate through wrapped component classes; and updating server middleware paths to handle both synchronous and background task execution flows consistently.

Possibly related PRs

  • jlowin/fastmcp#2378: Implements identical SEP-1686 background-task/Docket integration across matching modules and file changes with aligned signatures and routing logic.
  • jlowin/fastmcp#2657: Adds instance-level docket integration via add_to_docket() and task\_config on component base classes to enable background-task support for custom Tool/Resource/Prompt subclasses.
  • jlowin/fastmcp#2660: Introduces component-level convert_result() methods and refactors background-task handling to propagate mcp.types.CreateTaskResult with unified server/task conversion paths.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description lacks required checklist items and doesn't follow the provided template structure with marked checkboxes. Add the Contributors and Review checklists with checked boxes (- [x]) to demonstrate completion of required steps like testing, self-review, and issue linkage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Refactor provider execution: components own their execution' directly describes the main architectural change - shifting execution responsibility from providers to components themselves.
Docstring Coverage ✅ Passed Docstring coverage is 91.18% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch provider-refactor-wip

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
src/fastmcp/resources/template.py (1)

306-329: Unused uri parameter flagged by static analysis.

The uri parameter is declared but never used in this method. The method uses params directly for the read() call. If uri is intentionally unused (e.g., for API consistency with ResourceTemplate._read), consider prefixing with underscore to suppress the warning.

🔎 Proposed fix
     async def _read(
-        self, uri: str, params: dict[str, Any]
+        self, _uri: str, params: dict[str, Any]
     ) -> ResourceContent | mcp.types.CreateTaskResult:
src/fastmcp/server/providers/fastmcp_provider.py (1)

326-375: Unused uri parameter in _read method.

Similar to FunctionResourceTemplate._read, the uri parameter is unused. The method constructs original_uri from _original_uri_template and params instead.

🔎 Proposed fix
     async def _read(
-        self, uri: str, params: dict[str, Any]
+        self, _uri: str, params: dict[str, Any]
     ) -> ResourceContent | mcp.types.CreateTaskResult:
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fc8ba72 and d91400b.

⛔ Files ignored due to path filters (4)
  • tests/server/providers/test_fastmcp_provider.py is excluded by none and included by none
  • tests/server/tasks/test_custom_subclass_tasks.py is excluded by none and included by none
  • tests/server/tasks/test_task_mount.py is excluded by none and included by none
  • tests/server/test_providers.py is excluded by none and included by none
📒 Files selected for processing (15)
  • examples/providers/sqlite/server.py (2 hunks)
  • src/fastmcp/prompts/prompt.py (4 hunks)
  • src/fastmcp/prompts/prompt_manager.py (2 hunks)
  • src/fastmcp/resources/resource.py (3 hunks)
  • src/fastmcp/resources/resource_manager.py (2 hunks)
  • src/fastmcp/resources/template.py (5 hunks)
  • src/fastmcp/server/dependencies.py (3 hunks)
  • src/fastmcp/server/providers/base.py (3 hunks)
  • src/fastmcp/server/providers/fastmcp_provider.py (4 hunks)
  • src/fastmcp/server/proxy.py (2 hunks)
  • src/fastmcp/server/server.py (11 hunks)
  • src/fastmcp/server/tasks/handlers.py (4 hunks)
  • src/fastmcp/server/tasks/protocol.py (2 hunks)
  • src/fastmcp/server/tasks/routing.py (1 hunks)
  • src/fastmcp/tools/tool.py (3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/fastmcp/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

src/fastmcp/**/*.py: Write Python code with ≥3.10 type annotations required throughout
Never use bare except - be specific with exception types

Files:

  • src/fastmcp/prompts/prompt_manager.py
  • src/fastmcp/server/tasks/protocol.py
  • src/fastmcp/server/tasks/handlers.py
  • src/fastmcp/server/tasks/routing.py
  • src/fastmcp/prompts/prompt.py
  • src/fastmcp/server/providers/base.py
  • src/fastmcp/server/server.py
  • src/fastmcp/server/proxy.py
  • src/fastmcp/tools/tool.py
  • src/fastmcp/resources/resource_manager.py
  • src/fastmcp/server/dependencies.py
  • src/fastmcp/server/providers/fastmcp_provider.py
  • src/fastmcp/resources/resource.py
  • src/fastmcp/resources/template.py
🧠 Learnings (4)
📚 Learning: 2025-11-26T21:51:44.174Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: .cursor/rules/core-mcp-objects.mdc:0-0
Timestamp: 2025-11-26T21:51:44.174Z
Learning: Review and update related Manager classes (ToolManager, ResourceManager, PromptManager) when modifying MCP object definitions

Applied to files:

  • src/fastmcp/prompts/prompt_manager.py
  • src/fastmcp/server/providers/base.py
  • src/fastmcp/server/server.py
  • src/fastmcp/server/proxy.py
  • src/fastmcp/server/providers/fastmcp_provider.py
📚 Learning: 2025-12-17T03:06:14.522Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T03:06:14.522Z
Learning: Applies to src/fastmcp/**/*.py : Write Python code with ≥3.10 type annotations required throughout

Applied to files:

  • src/fastmcp/prompts/prompt_manager.py
  • src/fastmcp/server/tasks/routing.py
  • src/fastmcp/resources/resource_manager.py
  • src/fastmcp/resources/template.py
📚 Learning: 2025-12-17T03:06:14.522Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T03:06:14.522Z
Learning: Applies to src/fastmcp/**/__init__.py : Be intentional about re-exports; core types that define a module's purpose should be exported; specialized features can live in submodules; only re-export to fastmcp.* for fundamental types

Applied to files:

  • src/fastmcp/server/tasks/routing.py
📚 Learning: 2025-11-26T21:51:44.174Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: .cursor/rules/core-mcp-objects.mdc:0-0
Timestamp: 2025-11-26T21:51:44.174Z
Learning: Note that Resources and Resource Templates are distinct objects but both handled by ResourceManager, requiring coordinated updates when changes affect either object type

Applied to files:

  • src/fastmcp/resources/template.py
🧬 Code graph analysis (10)
src/fastmcp/prompts/prompt_manager.py (1)
src/fastmcp/prompts/prompt.py (1)
  • PromptResult (73-115)
examples/providers/sqlite/server.py (4)
src/fastmcp/server/proxy.py (1)
  • list_tools (103-106)
src/fastmcp/server/providers/base.py (2)
  • list_tools (151-156)
  • get_tool (158-168)
src/fastmcp/server/server.py (2)
  • get_tool (698-715)
  • name (355-356)
src/fastmcp/tools/tool_manager.py (1)
  • get_tool (65-70)
src/fastmcp/server/tasks/routing.py (6)
src/fastmcp/server/dependencies.py (2)
  • get_task_metadata (280-287)
  • message (426-427)
src/fastmcp/server/tasks/handlers.py (1)
  • submit_to_docket (30-142)
src/fastmcp/prompts/prompt.py (1)
  • Prompt (118-322)
src/fastmcp/server/server.py (1)
  • resource (2034-2177)
src/fastmcp/resources/resource.py (2)
  • Resource (137-331)
  • key (302-304)
src/fastmcp/resources/template.py (2)
  • ResourceTemplate (97-298)
  • key (267-269)
src/fastmcp/prompts/prompt.py (2)
src/fastmcp/exceptions.py (1)
  • PromptError (22-23)
src/fastmcp/server/tasks/routing.py (1)
  • check_background_task (26-75)
src/fastmcp/server/proxy.py (3)
src/fastmcp/client/client.py (3)
  • read_resource (834-839)
  • read_resource (842-849)
  • read_resource (851-889)
src/fastmcp/server/context.py (1)
  • read_resource (297-307)
src/fastmcp/prompts/prompt.py (1)
  • PromptResult (73-115)
src/fastmcp/tools/tool.py (2)
src/fastmcp/utilities/types.py (2)
  • Audio (307-362)
  • File (365-448)
src/fastmcp/server/tasks/routing.py (1)
  • check_background_task (26-75)
src/fastmcp/resources/resource_manager.py (5)
src/fastmcp/server/proxy.py (1)
  • read_resource (188-219)
src/fastmcp/client/client.py (3)
  • read_resource (834-839)
  • read_resource (842-849)
  • read_resource (851-889)
src/fastmcp/server/middleware/tool_injection.py (1)
  • read_resource (98-103)
src/fastmcp/server/context.py (1)
  • read_resource (297-307)
src/fastmcp/resources/resource.py (1)
  • ResourceContent (39-134)
src/fastmcp/server/providers/fastmcp_provider.py (2)
src/fastmcp/server/dependencies.py (1)
  • get_context (271-277)
src/fastmcp/prompts/prompt_manager.py (1)
  • get_prompt (55-60)
src/fastmcp/resources/resource.py (4)
src/fastmcp/resources/template.py (4)
  • convert_result (180-185)
  • _read (187-219)
  • _read (306-329)
  • key (267-269)
src/fastmcp/server/providers/fastmcp_provider.py (2)
  • _read (145-151)
  • _read (326-375)
src/fastmcp/server/tasks/routing.py (1)
  • check_background_task (26-75)
src/fastmcp/client/tasks.py (4)
  • result (202-207)
  • result (336-393)
  • result (427-458)
  • result (497-551)
src/fastmcp/resources/template.py (2)
src/fastmcp/resources/resource.py (7)
  • Resource (137-331)
  • ResourceContent (39-134)
  • convert_result (234-239)
  • from_value (69-108)
  • key (302-304)
  • read (220-232)
  • read (398-416)
src/fastmcp/server/tasks/routing.py (1)
  • check_background_task (26-75)
🪛 Ruff (0.14.8)
src/fastmcp/prompts/prompt.py

246-246: Avoid specifying long messages outside the exception class

(TRY003)

src/fastmcp/server/server.py

1351-1351: Avoid specifying long messages outside the exception class

(TRY003)


1353-1353: Avoid specifying long messages outside the exception class

(TRY003)


1398-1398: Avoid specifying long messages outside the exception class

(TRY003)


1400-1400: Avoid specifying long messages outside the exception class

(TRY003)


1452-1452: Avoid specifying long messages outside the exception class

(TRY003)


1492-1494: Prefer TypeError exception for invalid type

(TRY004)


1492-1494: Avoid specifying long messages outside the exception class

(TRY003)


1495-1495: Consider moving this statement to an else block

(TRY300)


1597-1597: Avoid specifying long messages outside the exception class

(TRY003)


1615-1615: Avoid specifying long messages outside the exception class

(TRY003)


1616-1616: Avoid specifying long messages outside the exception class

(TRY003)


1677-1680: Prefer TypeError exception for invalid type

(TRY004)


1677-1680: Avoid specifying long messages outside the exception class

(TRY003)


1734-1734: Avoid specifying long messages outside the exception class

(TRY003)

src/fastmcp/server/providers/fastmcp_provider.py

256-256: Consider moving this statement to an else block

(TRY300)


327-327: Unused method argument: uri

(ARG002)

src/fastmcp/resources/template.py

307-307: Unused method argument: uri

(ARG002)

⏰ 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). (4)
  • GitHub Check: Run tests with lowest-direct dependencies
  • GitHub Check: Run tests: Python 3.10 on windows-latest
  • GitHub Check: Run tests: Python 3.10 on ubuntu-latest
  • GitHub Check: Run tests: Python 3.13 on ubuntu-latest
🔇 Additional comments (44)
examples/providers/sqlite/server.py (4)

17-17: LGTM!

The Sequence import aligns with the updated return type annotation for list_tools.


74-76: LGTM!

Adding super().__init__() ensures proper initialization of the Provider base class, which is a best practice for inheritance.


78-83: LGTM!

The signature change from list_tools(self, context: Context) -> list[Tool] to list_tools(self) -> Sequence[Tool] correctly aligns with the updated Provider base class interface in src/fastmcp/server/providers/base.py (lines 150-155).


85-92: LGTM!

The signature change removing the context parameter aligns with the new provider interface where components own their execution rather than providers. This matches the base class definition in src/fastmcp/server/providers/base.py (lines 157-167).

src/fastmcp/resources/resource_manager.py (2)

10-10: LGTM!

The import is required for the CreateTaskResult type annotation in the read_resource return type.


290-302: LGTM!

The return type extension to ResourceContent | mcp.types.CreateTaskResult enables propagation of background task results through the resource reading path. The docstring accurately documents both execution paths (synchronous and background). This aligns with the broader SEP-1686 background task protocol being implemented across the codebase.

src/fastmcp/prompts/prompt_manager.py (2)

7-8: LGTM!

The import is required for the CreateTaskResult type annotation in the render_prompt return type.


106-120: LGTM!

The return type extension to PromptResult | mcp.types.CreateTaskResult is consistent with the parallel changes in ResourceManager.read_resource. The docstring accurately documents both synchronous and background execution paths. Based on learnings, this is correctly applied across related Manager classes.

src/fastmcp/server/proxy.py (2)

188-219: LGTM!

The return type extension to ResourceContent | mcp.types.CreateTaskResult correctly matches the parent class signature. The proxy fallback path appropriately returns ResourceContent since ProxyResource uses task_config=TaskConfig(mode="forbidden") (line 363), meaning proxy resources cannot be background tasks themselves.


257-276: LGTM!

The return type extension to PromptResult | mcp.types.CreateTaskResult correctly matches the parent class signature. Consistent with the resource proxy, the prompt proxy returns PromptResult directly since ProxyPrompt uses task_config=TaskConfig(mode="forbidden") (line 534).

src/fastmcp/server/dependencies.py (3)

43-51: LGTM!

The two new ContextVar declarations for task metadata propagation are well-documented and appropriately typed. The comments clearly explain their purpose:

  • _task_metadata for propagating task metadata through the async call stack
  • _docket_fn_key for component Docket function lookup with namespace prefix

53-68: LGTM!

The __all__ export list is correctly updated to include get_task_metadata, making the new public API discoverable.


280-288: LGTM!

The get_task_metadata() function provides a clean public API for accessing the task metadata context variable. The docstring clearly explains the return semantics (dict for background task requests, None for normal execution). Note: The AI summary mentioned a duplicate definition, but only one definition exists in this file.

src/fastmcp/server/tasks/protocol.py (1)

265-281: LGTM! Clean delegation to component conversion methods.

The refactored resource and new template task type handling correctly delegate result conversion to the respective component's convert_result() method, then convert to MCP format. This aligns well with the PR objective of having components own their execution/conversion logic.

src/fastmcp/server/tasks/routing.py (1)

26-75: Well-designed centralized task routing logic.

This consolidates the ~30 lines of duplicated background-task enforcement across Tool, Resource, Prompt, and ResourceTemplate into a single reusable function. The mode enforcement is clear and the error messages are descriptive.

One consideration: METHOD_NOT_FOUND is used for mode violations. This seems intentional per SEP-1686 semantics (the "method" doesn't exist in the requested mode), but verify this aligns with client expectations.

src/fastmcp/tools/tool.py (3)

243-281: Comprehensive result conversion with good fallback handling.

The convert_result() implementation correctly:

  1. Passes through existing ToolResult instances
  2. Handles special content types (ContentBlock, Audio, Image, File) without forcing structured content when no schema exists
  3. Gracefully falls back to content-only on serialization failures
  4. Respects x-fastmcp-wrap-result for schema-driven wrapping

The logic flow is clear and handles edge cases appropriately.


283-306: Clean server entry point with proper task routing delegation.

The _run() method correctly integrates with the centralized check_background_task() helper, resolving the function key from context or falling back to self.key. This pattern enables any Tool subclass to support background execution without code duplication.


314-335: Consistent add_to_docket signature extension across Tool and FunctionTool.

Both implementations correctly:

  • Accept new fn_key and task_key keyword-only parameters
  • Resolve lookup_key with fallback to self.key
  • Inject task_key as "key" in kwargs when provided

The difference in argument passing (positional vs splatted) between Tool and FunctionTool is intentional and correctly documented—FunctionTool.fn expects **kwargs while the base Tool.run expects a dict.

Also applies to: 490-513

src/fastmcp/server/providers/base.py (2)

51-62: TaskComponents field types updated but get_tasks() filter preserved.

The field type widening from FunctionTool/etc to Tool/etc in TaskComponents improves API flexibility while get_tasks() (lines 244-275) correctly continues to filter for function-based implementations that can be registered with Docket. This is a sensible design.


72-81: Clear documentation of the new execution model.

The updated docstring accurately reflects the refactored architecture: "Components execute themselves via run()/read()/render() - providers just source them." This aligns with the PR objective of moving execution responsibility to components.

src/fastmcp/prompts/prompt.py (3)

212-252: Solid convert_result() implementation with proper error handling.

The conversion logic handles all expected input types (PromptResult, strings, PromptMessage, and JSON-serializable objects) with appropriate fallbacks.

Regarding the static analysis hint (TRY003): The inline message in PromptError on line 246 is acceptable here since PromptError is a simple exception class and the message provides useful context about the conversion failure. This is a minor style consideration.


254-293: Correct task routing integration in _render().

The method properly:

  1. Resolves the function key from context variable or falls back to self.key
  2. Delegates to check_background_task() for mode enforcement
  3. Returns early with CreateTaskResult for background execution
  4. Falls back to synchronous render() with deprecation warning handling

The return type annotation PromptResult | mcp.types.CreateTaskResult accurately reflects both execution paths.


301-322: Consistent add_to_docket extensions for Prompt and FunctionPrompt.

Both implementations follow the same pattern as Tool:

  • Accept fn_key and task_key keyword-only parameters
  • Resolve lookup key with fallback
  • Inject task_key into kwargs

FunctionPrompt correctly splats arguments as **kwargs to match its function's signature, while base Prompt passes arguments as a single positional argument.

Also applies to: 526-549

src/fastmcp/resources/resource.py (4)

234-239: LGTM!

The convert_result method correctly delegates to ResourceContent.from_value with the resource's mime_type, providing a consistent conversion API that can be overridden by subclasses.


241-275: Well-structured task routing entry point.

The _read method cleanly separates concerns:

  1. Retrieves the docket function key from contextvar with fallback to self.key
  2. Delegates to check_background_task for mode validation and background submission
  3. Falls back to synchronous execution with deprecation handling

The deprecation warning for returning str/bytes from read() is preserved correctly.


312-331: LGTM!

The add_to_docket signature extension with fn_key and task_key parameters provides the flexibility needed for namespace-aware task scheduling. The logic correctly:

  • Uses fn_key or falls back to self.key for function lookup
  • Passes task_key to docket via kwargs when provided

398-416: LGTM!

FunctionResource.read correctly calls self.convert_result(result) after handling awaitable results and recursive Resource reads, ensuring consistent output conversion.

src/fastmcp/resources/template.py (4)

180-185: LGTM!

The convert_result method provides consistent conversion semantics matching Resource.convert_result, enabling template-specific mime_type handling.


187-219: Clean task routing implementation for ResourceTemplate.

The method correctly:

  1. Uses a pattern check ("{" not in key) to determine whether to use the contextvar or fall back to self.key
  2. Delegates to check_background_task for mode enforcement and background submission
  3. Creates the resource and reads it synchronously when not backgrounded
  4. Uses resource.convert_result for consistent output conversion

277-298: LGTM!

The add_to_docket signature extension properly handles fn_key/task_key and passes params as a positional argument, matching the template's expected calling convention.


391-414: LGTM!

FunctionResourceTemplate.add_to_docket correctly splats params as **kwargs since the underlying function expects keyword arguments. This differs from the base ResourceTemplate which passes params as a single positional dict.

src/fastmcp/server/tasks/handlers.py (3)

30-56: Well-documented unified submission handler.

The function signature and docstring clearly describe the generalized approach for all component types. The SEP-1686 compliance notes are helpful for understanding the design rationale.


77-88: Robust task key and metadata storage.

The implementation correctly:

  1. Builds a structured task key with session/task/type/component info
  2. Stores the mapping in Redis with appropriate TTL buffer
  3. Stores creation timestamp separately for retrieval

The 15-minute TTL buffer ensures mappings outlive task executions.


106-114: Correct differentiation between Resource and other component types.

Resources don't take arguments, so the branching at lines 111-114 correctly handles the different add_to_docket signatures. The type: ignore comments are appropriate given the varying method signatures across component types.

src/fastmcp/server/server.py (5)

1245-1297: Clean task metadata propagation pattern.

The implementation correctly:

  1. Extracts task metadata from the MCP request context
  2. Sets both _task_metadata and _docket_fn_key contextvars before middleware execution
  3. Resets contextvars in a finally block to prevent leakage
  4. Checks for CreateTaskResult and returns it directly for background execution

This establishes a consistent pattern used throughout the handlers.


1304-1353: Proper custom handler for resource task support.

Since the SDK's read_resource decorator doesn't support CreateTaskResult returns, this custom handler correctly bypasses it while maintaining the same structure. The contextvar management mirrors _call_tool_mcp.


1486-1495: Defensive check for unexpected CreateTaskResult.

The RuntimeError at lines 1492-1494 guards against an invariant violation where CreateTaskResult appears without task metadata being set. This is appropriate defensive coding since the direct _read_resource_mcp path shouldn't produce background tasks.


1599-1638: Well-factored execute methods for resources and templates.

The separation of _execute_resource and _execute_template provides clear error handling boundaries while delegating task routing to the component's _read method. The error masking logic respects the server's _mask_error_details setting.


1665-1681: Appropriate RuntimeError for unexpected CreateTaskResult.

The check at lines 1676-1680 correctly enforces that _get_prompt_middleware is only called in synchronous contexts. For task-augmented execution, callers should use _get_prompt_content_middleware directly.

src/fastmcp/server/providers/fastmcp_provider.py (5)

50-105: LGTM!

FastMCPProviderTool correctly wraps a tool and delegates execution to the child server's middleware via _call_tool_middleware. The _run method appropriately skips task handling at this layer since the underlying tool handles it.


153-183: Robust context handling for Docket worker scenarios.

The read method correctly handles two scenarios:

  1. Normal execution with existing context - delegates directly
  2. Docket worker execution without context - creates a temporary context

The comment about preserving _docket_fn_key is important for correct Docket lookup in nested mounts.


559-601: Correct approach: Return actual components for Docket registration.

Returning the child's actual components (not wrapped) ensures their underlying functions get registered with Docket. The TransformingProvider.get_tasks() handles namespace transformation of keys, which is the right separation of concerns.


480-493: LGTM!

The list_tools and get_tool methods correctly wrap returned tools as FastMCPProviderTool instances, ensuring middleware execution when tools are called through the provider.


34-42: URI template expansion lacks protection against backslash sequences in values

In Python's re.sub(), backslash escapes in the replacement string are processed, and backreferences such as \1, \2 are replaced with substrings matched by groups in the pattern. The implementation re.sub(rf"\{{{key}\}}", str(value), result) passes str(value) directly as the replacement without escaping. Since Python 3.5, unmatched groups are replaced with an empty string, but if a value contains backslash followed by digits (e.g., \1), it will still be interpreted as a backreference attempt. While this particular pattern has no capture groups, the approach is fragile.

Consider using a replacement function with re.sub() or escaping backslashes in values to prevent unintended escape sequence processing.

@jlowin jlowin added this to the 2.15 milestone Dec 21, 2025
@jlowin jlowin merged commit 19fdac7 into main Dec 21, 2025
13 checks passed
@jlowin jlowin deleted the provider-refactor-wip branch December 21, 2025 20:03
@marvin-context-protocol
Copy link
Copy Markdown
Contributor

Test Failure Analysis

Summary: Test times out on Windows due to SQLite locking when creating OAuthProxy without isolated storage.

Root Cause: The test creates an instance without providing a client_storage parameter (line 1387 in tests/server/auth/test_oauth_proxy.py). When client_storage is None, OAuthProxy defaults to creating a DiskStore at settings.home / "oauth-proxy". On Windows, when tests run in parallel, multiple tests trying to access the same SQLite database causes locking/timeout issues during the SQLite connection initialization.

The stack trace shows the timeout occurs at:

File "diskcache/core.py", line 623, in _con
    con = self._local.con = sqlite3.connect(

Suggested Solution: Provide an isolated in-memory storage for this test to avoid SQLite file contention:

  1. Modify test_callback_error_returns_html_page to use an in-memory key-value store
  2. Add client_storage parameter to the OAuthProxy initialization

Example fix:

from key_value.aio.stores.memory import MemoryStore

async def test_callback_error_returns_html_page(self):
    """Test that OAuth callback errors return styled HTML instead of data: URLs."""
    from unittest.mock import Mock
    
    from starlette.requests import Request
    from starlette.responses import HTMLResponse
    
    from fastmcp.server.auth.oauth_proxy import OAuthProxy
    from fastmcp.server.auth.providers.jwt import JWTVerifier
    
    # Create a minimal OAuth proxy with isolated storage
    provider = OAuthProxy(
        upstream_authorization_endpoint="https://idp.example.com/authorize",
        upstream_token_endpoint="https://idp.example.com/token",
        upstream_client_id="test-client",
        upstream_client_secret="test-secret",
        token_verifier=JWTVerifier(
            jwks_uri="https://idp.example.com/.well-known/jwks.json",
            issuer="https://idp.example.com",
            audience="test-client",
        ),
        base_url="http://localhost:8000",
        jwt_signing_key="test-signing-key",
        client_storage=MemoryStore(),  # Add this line
    )
    # ... rest of test
Detailed Analysis

The test was added/modified in this PR and exposed a Windows-specific issue that wasn't caught because:

  1. Linux/macOS handle SQLite file locking differently than Windows
  2. The test runs without --numprocesses on Windows (see workflow), so it's not parallelized but still conflicts with other tests from previous runs or shared state

Other tests in the file use the oauth_proxy fixture which also doesn't provide isolated storage, but this particular test might be more susceptible due to timing or ordering.

Related Files
  • tests/server/auth/test_oauth_proxy.py:1376 - Failing test that needs isolated storage
  • src/fastmcp/server/auth/oauth_proxy.py:821 - Where DiskStore is created by default
  • tests/server/auth/test_oauth_proxy.py:312 - oauth_proxy fixture that could also benefit from isolated storage

@marvin-context-protocol
Copy link
Copy Markdown
Contributor

Test Failure Analysis

Summary: Test test_callback_error_returns_html_page times out on Windows due to SQLite locking when creating OAuthProxy without isolated storage.

Root Cause: The test creates an OAuthProxy instance without providing a client_storage parameter (line 1387 in tests/server/auth/test_oauth_proxy.py). When client_storage is None, OAuthProxy defaults to creating a DiskStore at settings.home / "oauth-proxy". On Windows, when tests run in parallel, multiple tests trying to access the same SQLite database causes locking/timeout issues during the SQLite connection initialization.

The stack trace shows the timeout occurs at:

File "diskcache/core.py", line 623, in _con
    con = self._local.con = sqlite3.connect(

Suggested Solution: Provide an isolated in-memory storage for this test to avoid SQLite file contention:

  1. Modify test_callback_error_returns_html_page to use an in-memory key-value store
  2. Add client_storage parameter to the OAuthProxy initialization

Example fix:

from key_value.aio.stores.memory import MemoryStore

async def test_callback_error_returns_html_page(self):
    """Test that OAuth callback errors return styled HTML instead of data: URLs."""
    from unittest.mock import Mock

    from starlette.requests import Request
    from starlette.responses import HTMLResponse

    from fastmcp.server.auth.oauth_proxy import OAuthProxy
    from fastmcp.server.auth.providers.jwt import JWTVerifier

    # Create a minimal OAuth proxy with isolated storage
    provider = OAuthProxy(
        upstream_authorization_endpoint="https://idp.example.com/authorize",
        upstream_token_endpoint="https://idp.example.com/token",
        upstream_client_id="test-client",
        upstream_client_secret="test-secret",
        token_verifier=JWTVerifier(
            jwks_uri="https://idp.example.com/.well-known/jwks.json",
            issuer="https://idp.example.com",
            audience="test-client",
        ),
        base_url="http://localhost:8000",
        jwt_signing_key="test-signing-key",
        client_storage=MemoryStore(),  # Add this line
    )
    # ... rest of test
Detailed Analysis

The test was added/modified in this PR and exposed a Windows-specific issue that wasn't caught because:

  1. Linux/macOS handle SQLite file locking differently than Windows
  2. The test runs without --numprocesses on Windows (see workflow), so it's not parallelized but still conflicts with other tests from previous runs or shared state

Other tests in the file use the oauth_proxy fixture which also doesn't provide isolated storage, but this particular test might be more susceptible due to timing or ordering.

Related Files
  • tests/server/auth/test_oauth_proxy.py:1376 - Failing test that needs isolated storage
  • src/fastmcp/server/auth/oauth_proxy.py:821 - Where DiskStore is created by default
  • tests/server/auth/test_oauth_proxy.py:312 - oauth_proxy fixture that could also benefit from isolated storage

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Improvement to existing functionality. For issues and smaller PR improvements. provider Related to the FastMCP Provider class server Related to FastMCP server implementation or server-side functionality.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant