diff --git a/CLAUDE.md b/CLAUDE.md index a85214784..ac80f2d25 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -52,6 +52,8 @@ When modifying MCP functionality, changes typically need to be applied across al ## Development Rules +**Read `CONTRIBUTING.md` before opening issues or PRs.** It describes when PRs are appropriate, what we expect from enhancement proposals, and what we'll close without review. + ### Git & CI - Prek hooks are required (run automatically on commits) @@ -101,6 +103,14 @@ The handwritten notes are prepended above the auto-generated changelog and are t - Minor fixes: keep body short and concise - No "test plan" sections or testing summaries +### Code Review Guidelines + +- **Fix causes, not symptoms.** When a PR works around a problem instead of addressing why it occurs, that's a red flag. A side-channel that compensates for a missing step adds permanent complexity. If the fix doesn't change the code path where the bug actually happens, ask why not. +- Focus on API design and naming clarity +- Identify confusing patterns (e.g., parameter values that contradict defaults) or non-idiomatic code (mutable defaults, etc.). Contributed code will need to be maintained indefinitely, and by someone other than the author (unless the author is a maintainer). +- Suggest specific improvements, not generic "add more tests" comments +- Think about API ergonomics from a user perspective + ### Code Standards - Python ≥ 3.10 with full type annotations diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e948f0c0d..6f861c50a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -20,11 +20,13 @@ We encourage you to use LLMs to help identify bugs, write MREs, and prepare cont ## When to open a pull request -**Bug fixes** — PRs are welcome for simple, well-scoped bug fixes where the problem and solution are both straightforward. "The function raises `TypeError` when passed `None` because of a missing guard" is a good candidate. If the fix requires design decisions or touches multiple subsystems, open an issue instead. +An open issue is not an invitation to submit a PR. Issues track problems; whether and how to solve them is a separate decision. If you want to work on something, propose your approach in the issue first — especially for anything beyond a trivial fix. + +**Bug fixes** — PRs are welcome for simple, well-scoped bug fixes where the problem and solution are both straightforward. "The function raises `TypeError` when passed `None` because of a missing guard" is a good candidate. If the fix requires design decisions or touches multiple subsystems, open an issue with a design proposal instead. **Documentation** — Typo fixes, clarifications, and improvements to examples are always welcome as PRs. -**Enhancements and features** — For changes that affect the behavior or design of the framework, please open an issue first. Maintainers will typically implement these themselves. FastMCP is opinionated, and enhancements need to reflect those opinions — not just solve the problem, but solve it in a way that's consistent with the framework's design. That's hard to do from the outside, and it's why a clear problem description is more useful than a proposed solution. +**Enhancements and features** — We welcome enhancement PRs, but our experience is that most contributors — even when using LLMs — implement fixes that address the one instance of a problem they encountered rather than understanding why the framework produces that problem and fixing it at the right layer. This creates branching, patch-style code that's difficult to maintain and makes it impossible to reason about the framework as a coherent system. For this reason, enhancements need a design proposal in the issue before code is written. The proposal doesn't need to be long — just enough to show you've thought about how the change fits into the framework, not just how it solves your immediate case. **Integrations** — FastMCP generally does not accept PRs that add third-party integrations (custom middleware, provider-specific adapters, etc.). If you're building something for your users, ship it as a standalone package — that's a feature, not a limitation. Authentication providers are an exception, since auth is tightly coupled to the framework. @@ -36,6 +38,7 @@ If you do open a PR: - **Keep it focused.** One logical change per PR. Don't bundle unrelated fixes or refactors. - **Match existing patterns.** Follow the code style, type annotation conventions, and test patterns you see in the codebase. Run `uv run prek run --all-files` before submitting. - **Write tests.** Bug fixes should include a test that fails without the fix. Enhancements should include tests for the new behavior. +- **Fix the cause, not the symptom.** If the bug is that a code path skips a step, the fix should make it stop skipping that step — not add compensation elsewhere. Workaround-style fixes will be sent back for revision. - **Don't submit generated boilerplate.** We review every line. PRs that read like unedited LLM output — verbose descriptions, speculative changes, shotgun-style fixes — will be closed. ## What we'll close without review @@ -47,4 +50,4 @@ To keep the project maintainable, we will close PRs that: - Add third-party integrations that belong in a separate package - Are difficult to review due to size, scope, or generated content -This isn't personal. FastMCP receives a high volume of contributions and we need to focus maintainer time where it has the most impact — which is why a good issue is often the best thing you can do for the project. +This isn't personal — contributing to a framework is different from contributing to an application. In an application, a fix that works is a good fix. In a framework, a fix that works but doesn't fit the framework's design creates maintenance burden that compounds over time. Every patch that works around a problem instead of solving it at the right layer makes the system harder for *everyone* to reason about — maintainers, contributors, and users. We hold contributions to this standard because the alternative is a codebase that's a series of patches rather than a coherent system. A good issue is often the best thing you can do for the project. diff --git a/src/fastmcp/server/dependencies.py b/src/fastmcp/server/dependencies.py index 956fce711..4d2c2b1c4 100644 --- a/src/fastmcp/server/dependencies.py +++ b/src/fastmcp/server/dependencies.py @@ -11,6 +11,7 @@ import contextlib import inspect import logging import weakref +from collections import OrderedDict from collections.abc import AsyncGenerator, Callable from contextlib import AsyncExitStack, asynccontextmanager from contextvars import ContextVar, Token @@ -72,6 +73,7 @@ __all__ = [ "get_task_context", "get_task_session", "is_docket_available", + "register_task_server", "register_task_session", "require_docket", "resolve_dependencies", @@ -174,6 +176,32 @@ def get_task_session(session_id: str) -> ServerSession | None: _current_server: ContextVar[weakref.ref[FastMCP] | None] = ContextVar( "server", default=None ) + +# --- Background task server map --- +# Maps task_id → server weakref so background workers can resolve the correct +# server for mounted-child tasks. Follows the same pattern as _task_sessions. +# Populated in submit_to_docket() where the child server is in context; +# consulted in get_server() when running inside a Docket worker. + +_task_server_map: OrderedDict[str, weakref.ref[FastMCP]] = OrderedDict() +_TASK_SERVER_MAP_MAX_SIZE = 10_000 + + +def register_task_server(task_id: str, server: FastMCP) -> None: + """Register the server for a background task. + + Called at task-submission time (inside the child server's call_tool + context) so that background workers can resolve CurrentFastMCP() and + ctx.fastmcp to the child server for mounted tasks. + + The map is bounded to avoid unbounded growth in long-lived servers. + Evicted entries fall back to the ContextVar (parent server). + """ + _task_server_map[task_id] = weakref.ref(server) + while len(_task_server_map) > _TASK_SERVER_MAP_MAX_SIZE: + _task_server_map.popitem(last=False) + + _current_docket: ContextVar[Docket | None] = ContextVar("docket", default=None) _current_worker: ContextVar[Worker | None] = ContextVar("worker", default=None) _task_access_token: ContextVar[AccessToken | None] = ContextVar( @@ -378,12 +406,28 @@ def get_context() -> Context: def get_server() -> FastMCP: """Get the current FastMCP server instance directly. + In a background-task worker, checks the task-server map first so that + mounted-child tasks resolve to the child server (not the parent that + started the worker). + Returns: The active FastMCP server Raises: RuntimeError: If no server in context """ + # In a task context, prefer the task-specific server mapping. + # This handles mounted-child tasks where _current_server is the parent. + task_info = get_task_context() + if task_info is not None: + ref = _task_server_map.get(task_info.task_id) + if ref is not None: + server = ref() + if server is not None: + return server + # Server was garbage collected, clean up + _task_server_map.pop(task_info.task_id, None) + server_ref = _current_server.get() if server_ref is None: raise RuntimeError("No FastMCP server instance in context") @@ -1037,13 +1081,7 @@ class _CurrentFastMCP(Dependency["FastMCP"]): """Async context manager for FastMCP server dependency.""" async def __aenter__(self) -> FastMCP: - server_ref = _current_server.get() - if server_ref is None: - raise RuntimeError("No FastMCP server instance in context") - server = server_ref() - if server is None: - raise RuntimeError("FastMCP server instance is no longer available") - return server + return get_server() async def __aexit__( self, diff --git a/src/fastmcp/server/tasks/handlers.py b/src/fastmcp/server/tasks/handlers.py index b36296f38..034f133dd 100644 --- a/src/fastmcp/server/tasks/handlers.py +++ b/src/fastmcp/server/tasks/handlers.py @@ -14,7 +14,12 @@ import mcp.types from mcp.shared.exceptions import McpError from mcp.types import INTERNAL_ERROR, ErrorData -from fastmcp.server.dependencies import _current_docket, get_access_token, get_context +from fastmcp.server.dependencies import ( + _current_docket, + get_access_token, + get_context, + register_task_server, +) from fastmcp.server.tasks.config import TaskMeta from fastmcp.server.tasks.keys import build_task_key from fastmcp.utilities.logging import get_logger @@ -80,6 +85,12 @@ async def submit_to_docket( ) ) + # Register the current server so background workers resolve + # CurrentFastMCP() / ctx.fastmcp to the correct (child) server + # for mounted tasks. At this point ctx.fastmcp is the child because + # we're inside the child's call_tool dispatch. + register_task_server(server_task_id, ctx.fastmcp) + # Build full task key with embedded metadata task_key = build_task_key(session_id, server_task_id, task_type, key) diff --git a/tests/server/tasks/test_task_mount.py b/tests/server/tasks/test_task_mount.py index d9a83ce2a..da055072a 100644 --- a/tests/server/tasks/test_task_mount.py +++ b/tests/server/tasks/test_task_mount.py @@ -336,10 +336,80 @@ class TestMountedTaskDependencies: task = await client.call_tool("child_tool_with_server", {}, task=True) await task.result() - # The server should be the child server since that's where the tool is defined assert len(received_server) == 1 - # Note: It might be parent or child depending on implementation - assert received_server[0] is not None + assert received_server[0].name == "server-dep-child" + + +class TestMountedTaskServerContext: + """Test that background tasks on mounted servers resolve to the child server (#3571).""" + + async def test_current_fastmcp_resolves_to_child_server(self): + """CurrentFastMCP() inside a mounted background task returns the child server.""" + child = FastMCP("child") + received_server: list[FastMCP] = [] + + @child.tool(task=True) + async def whoami(server: CurrentFastMCP = CurrentFastMCP()) -> str: # type: ignore[invalid-type-form] # ty:ignore[invalid-type-form] + received_server.append(server) + return f"server name: {server.name}" + + parent = FastMCP("parent") + parent.mount(child, namespace="child") + + async with Client(parent) as client: + task = await client.call_tool("child_whoami", {}, task=True) + result = await task.result() + + assert len(received_server) == 1 + assert received_server[0].name == "child" + assert "server name: child" in str(result) + + async def test_context_fastmcp_resolves_to_child_server(self): + """ctx.fastmcp inside a mounted background task returns the child server.""" + from fastmcp import Context + + child = FastMCP("child") + received_server: list[FastMCP] = [] + + @child.tool(task=True) + async def whoami_ctx(ctx: Context) -> str: + received_server.append(ctx.fastmcp) + return f"context server: {ctx.fastmcp.name}" + + parent = FastMCP("parent") + parent.mount(child, namespace="child") + + async with Client(parent) as client: + task = await client.call_tool("child_whoami_ctx", {}, task=True) + result = await task.result() + + assert len(received_server) == 1 + assert received_server[0].name == "child" + assert "context server: child" in str(result) + + async def test_nested_mount_resolves_to_innermost_server(self): + """Doubly-nested mounts resolve to the innermost child server.""" + grandchild = FastMCP("grandchild") + received_server: list[FastMCP] = [] + + @grandchild.tool(task=True) + async def deep_whoami(server: CurrentFastMCP = CurrentFastMCP()) -> str: # type: ignore[invalid-type-form] # ty:ignore[invalid-type-form] + received_server.append(server) + return f"server name: {server.name}" + + child = FastMCP("child") + child.mount(grandchild, namespace="gc") + + parent = FastMCP("parent") + parent.mount(child, namespace="child") + + async with Client(parent) as client: + task = await client.call_tool("child_gc_deep_whoami", {}, task=True) + result = await task.result() + + assert len(received_server) == 1 + assert received_server[0].name == "grandchild" + assert "server name: grandchild" in str(result) class TestMultipleMounts: