mirror of
https://github.com/PrefectHQ/fastmcp.git
synced 2026-08-15 18:19:10 +02:00
* Unify background task context forwarding and fix concurrent dependency bugs We've been getting a steady trickle of edge-case reports around background tasks and contextual dependencies over the last few months (#3654, #3656, #3569). Each one pointed at a different symptom, but they all traced back to the same area: the way context is negotiated between the "frontend" server and Docket workers was grown piecemeal, with each new piece of context (access tokens, HTTP headers, origin request IDs) getting its own Redis key, its own restore function, and its own ContextVar. This made it hard to reason about what state was available where, and the shared-instance Dependency pattern made concurrent tasks stomp on each other's cleanup state. This takes a step back and reworks the whole thing as a single unified system: - Dependency subclasses (_CurrentContext, Progress, _CurrentAccessToken, etc.) are now stateless factories — __aenter__ returns a fresh per-invocation object, so concurrent tasks never share mutable state. Fixes #3654, #3656. - The three individual context-snapshot Redis keys (access_token, http_headers, origin_request_id) are collapsed into a single TaskContextSnapshot stored as one JSON key per task. The three _restore_task_* functions and two ContextVars they populated are gone. - Sync functions like get_http_request() and get_access_token() now find the snapshot transparently in background tasks via a 3-tier sync fallback: ContextVar (set by _CurrentContext for functions with deps) → in-memory dict (same-process workers) → sync Redis GET (out-of-process workers). No function wrapping needed. - The _wrap_for_task_http_headers hack is deleted. FunctionTool registers its raw function with Docket so Docket sees and resolves ALL dependencies, including Docket-native ones like Retry and Timeout. - ProxyTool.from_mcp_tool() now propagates execution.taskSupport metadata from remote tools. Fixes #3569. - Removed redundant _current_docket/_current_worker ContextVar management from Context.__aenter__/__aexit__ (they're only set in the lifespan now). Closes #3654 Closes #3656 Closes #3569 🤖 Generated with Claude Code Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Address code review feedback - _OptionalCurrentContext: guard __aexit__ against cleaning up contexts it didn't create (check is_background_task before delegating) - Narrow except clauses in snapshot loading (OSError, JSONDecodeError, etc. instead of bare Exception) - Fix docstrings on register_with_docket for resources/prompts/templates - Simplify Progress: read ExecutionProgress directly from current_execution instead of creating and manually entering a DocketProgress wrapper 🤖 Generated with Claude Code Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Use pop-on-access transfer buffer instead of bounded LRU cache for snapshots The in-memory snapshot dict is a transfer mechanism, not a cache. Entries go in at submission and come out at the worker's first access. Using pop instead of get means the dict only holds entries during the brief submission-to-execution window, bounded by task concurrency (~10) rather than a 10,000-entry LRU limit. 🤖 Generated with Claude Code Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Drop in-memory transfer buffer, use sync Redis for all backends Instead of maintaining an in-memory dict to bridge the async/sync gap, use a sync Redis client directly. For memory:// backends (fakeredis), shares the same FakeServer instance via docket._redis.get_memory_server() so data written by the async Docket client is visible to sync reads. For real Redis, creates a standard sync connection. No in-process state to manage at all. 🤖 Generated with Claude Code Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Move snapshot operations to TaskContextSnapshot methods capture(), from_json(), to_json(), save() are now classmethod/instance methods on the dataclass instead of free functions. Deduplicates JSON parsing that was copy-pasted between the async and sync load paths. 🤖 Generated with Claude Code Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Trim implementation details from register_with_docket docstrings 🤖 Generated with Claude Code Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Clarify docket lookup comment in submit_to_docket 🤖 Generated with Claude Code Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Restore docket/worker ContextVar bridge in Context.__aenter__ Servers that own the Docket (the parent) re-set _current_docket/_current_worker from their instance attributes when entering a Context. Mounted children skip this (their _docket is None), so they inherit the parent's value. This is needed for ASGI deployments where ContextVars set during the lifespan don't propagate to request handlers. 🤖 Generated with Claude Code Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Key snapshot cache by task_id to prevent cross-task context leakage Docket workers may reuse the same asyncio context for sequential tasks. The ContextVar cache now stores (task_id, snapshot) tuples so stale entries from previous tasks are automatically ignored. 🤖 Generated with Claude Code Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
213 lines
7 KiB
Python
213 lines
7 KiB
Python
"""Tests for concurrent dependency resolution in foreground and background tasks.
|
|
|
|
Regression tests for:
|
|
- #3654: ValueError when concurrent Docket tasks share a Dependency instance
|
|
that stores a ContextVar token on `self`
|
|
- #3656: Progress raises AssertionError when concurrent tasks share `_impl`
|
|
"""
|
|
|
|
import asyncio
|
|
|
|
from fastmcp import FastMCP
|
|
from fastmcp.client import Client
|
|
from fastmcp.dependencies import Progress
|
|
from fastmcp.server.context import Context
|
|
from fastmcp.server.dependencies import (
|
|
get_access_token,
|
|
get_http_headers,
|
|
)
|
|
|
|
|
|
async def test_concurrent_foreground_tools_with_context():
|
|
"""Multiple concurrent tool calls sharing the same CurrentContext() default
|
|
should not raise ValueError from ContextVar token resets (#3654)."""
|
|
mcp = FastMCP("test")
|
|
results: list[str] = []
|
|
|
|
@mcp.tool()
|
|
async def slow_tool(name: str, ctx: Context) -> str:
|
|
await asyncio.sleep(0.05)
|
|
results.append(name)
|
|
return f"done:{name}"
|
|
|
|
async with Client(mcp) as client:
|
|
tasks = [client.call_tool("slow_tool", {"name": f"task-{i}"}) for i in range(4)]
|
|
outcomes = await asyncio.gather(*tasks)
|
|
|
|
assert len(outcomes) == 4
|
|
for outcome in outcomes:
|
|
assert outcome.content[0].text.startswith("done:")
|
|
|
|
|
|
async def test_concurrent_foreground_tools_with_progress():
|
|
"""Multiple concurrent tool calls sharing the same Progress() default
|
|
should not raise AssertionError from _impl being None (#3656)."""
|
|
mcp = FastMCP("test")
|
|
|
|
@mcp.tool()
|
|
async def variable_tool(
|
|
name: str, delay: float, progress: Progress = Progress()
|
|
) -> str:
|
|
await progress.set_total(3)
|
|
await progress.increment()
|
|
await asyncio.sleep(delay)
|
|
await progress.increment()
|
|
await progress.set_message(f"finishing {name}")
|
|
await progress.increment()
|
|
return f"done:{name}"
|
|
|
|
async with Client(mcp) as client:
|
|
tasks = [
|
|
client.call_tool(
|
|
"variable_tool", {"name": f"t-{i}", "delay": 0.01 * (i + 1)}
|
|
)
|
|
for i in range(4)
|
|
]
|
|
outcomes = await asyncio.gather(*tasks)
|
|
|
|
assert len(outcomes) == 4
|
|
for outcome in outcomes:
|
|
assert outcome.content[0].text.startswith("done:")
|
|
|
|
|
|
async def test_concurrent_background_tasks_with_context():
|
|
"""Multiple concurrent background tasks sharing _CurrentContext() should
|
|
not raise ValueError from ContextVar token resets (#3654)."""
|
|
mcp = FastMCP("test")
|
|
|
|
@mcp.tool(task=True)
|
|
async def bg_tool(name: str, ctx: Context) -> str:
|
|
await asyncio.sleep(0.05)
|
|
return f"bg:{name}"
|
|
|
|
async with Client(mcp) as client:
|
|
task_handles = [
|
|
await client.call_tool("bg_tool", {"name": f"bg-{i}"}, task=True)
|
|
for i in range(4)
|
|
]
|
|
results = await asyncio.gather(*[t.result() for t in task_handles])
|
|
|
|
assert len(results) == 4
|
|
for result in results:
|
|
assert result.content[0].text.startswith("bg:")
|
|
|
|
|
|
async def test_concurrent_background_tasks_with_progress():
|
|
"""Multiple concurrent background tasks sharing Progress() should
|
|
not raise AssertionError from _impl being None (#3656)."""
|
|
mcp = FastMCP("test")
|
|
|
|
@mcp.tool(task=True)
|
|
async def bg_progress_tool(
|
|
name: str, delay: float, progress: Progress = Progress()
|
|
) -> str:
|
|
await progress.set_total(3)
|
|
await progress.increment()
|
|
await asyncio.sleep(delay)
|
|
await progress.increment()
|
|
await progress.set_message(f"bg finishing {name}")
|
|
await progress.increment()
|
|
return f"bg:{name}"
|
|
|
|
async with Client(mcp) as client:
|
|
task_handles = [
|
|
await client.call_tool(
|
|
"bg_progress_tool",
|
|
{"name": f"bg-{i}", "delay": 0.01 * (i + 1)},
|
|
task=True,
|
|
)
|
|
for i in range(4)
|
|
]
|
|
results = await asyncio.gather(*[t.result() for t in task_handles])
|
|
|
|
assert len(results) == 4
|
|
for result in results:
|
|
assert result.content[0].text.startswith("bg:")
|
|
|
|
|
|
async def test_dependency_aenter_returns_fresh_instances():
|
|
"""Verify that Dependency.__aenter__ returns independent per-invocation
|
|
objects, not the shared default."""
|
|
mcp = FastMCP("test")
|
|
|
|
instances: list[Context] = []
|
|
|
|
@mcp.tool()
|
|
async def capture_context(ctx: Context) -> str:
|
|
instances.append(ctx)
|
|
return "ok"
|
|
|
|
async with Client(mcp) as client:
|
|
await asyncio.gather(
|
|
client.call_tool("capture_context", {}),
|
|
client.call_tool("capture_context", {}),
|
|
)
|
|
|
|
assert len(instances) == 2
|
|
assert instances[0] is not instances[1]
|
|
|
|
|
|
async def test_progress_aenter_returns_fresh_instances():
|
|
"""Verify that Progress.__aenter__ returns independent per-invocation
|
|
objects, not the shared default."""
|
|
progress_instances: list[Progress] = []
|
|
|
|
mcp = FastMCP("test")
|
|
|
|
@mcp.tool()
|
|
async def capture_progress(progress: Progress = Progress()) -> str:
|
|
progress_instances.append(progress)
|
|
await progress.set_total(1)
|
|
await progress.increment()
|
|
return "ok"
|
|
|
|
async with Client(mcp) as client:
|
|
await asyncio.gather(
|
|
client.call_tool("capture_progress", {}),
|
|
client.call_tool("capture_progress", {}),
|
|
)
|
|
|
|
assert len(progress_instances) == 2
|
|
assert progress_instances[0] is not progress_instances[1]
|
|
assert progress_instances[0]._impl is not progress_instances[1]._impl
|
|
|
|
|
|
async def test_sync_context_functions_work_in_background_without_deps():
|
|
"""Sync functions like get_http_request() should work in background tasks
|
|
even when the tool declares no Context or CurrentRequest dependency.
|
|
|
|
This exercises the sync Redis fallback path (_get_task_snapshot_sync →
|
|
_load_snapshot_sync_redis) which must work with both memory:// (fakeredis)
|
|
and real Redis backends.
|
|
"""
|
|
mcp = FastMCP("test")
|
|
|
|
@mcp.tool(task=True)
|
|
async def bare_sync_access() -> dict[str, str]:
|
|
headers = get_http_headers()
|
|
return {"has_headers": str(bool(headers))}
|
|
|
|
async with Client(mcp) as client:
|
|
task = await client.call_tool("bare_sync_access", {}, task=True)
|
|
result = await task.result()
|
|
assert result.data == {"has_headers": "False"}
|
|
|
|
|
|
async def test_sync_context_functions_work_in_background_with_context():
|
|
"""Sync functions work via ContextVar when _CurrentContext loads the snapshot."""
|
|
mcp = FastMCP("test")
|
|
|
|
@mcp.tool(task=True)
|
|
async def context_sync_access(ctx: Context) -> dict[str, str]:
|
|
headers = get_http_headers()
|
|
token = get_access_token()
|
|
return {
|
|
"has_headers": str(bool(headers)),
|
|
"has_token": str(token is not None),
|
|
"is_background": str(ctx.is_background_task),
|
|
}
|
|
|
|
async with Client(mcp) as client:
|
|
task = await client.call_tool("context_sync_access", {}, task=True)
|
|
result = await task.result()
|
|
assert result.data["is_background"] == "True"
|