Merge main into feature/apps#3187
Conversation
Add Redis-backed notification queue (LPUSH/BRPOP) enabling the MCP server to notify clients about background task events like elicitation requests. - notifications.py: subscriber management with weakref tracking, retry logic, TTL expiration, and graceful shutdown - __init__.py: export ensure_subscriber_running, push_notification, stop_subscriber
- context.py: report_progress uses delta tracking via increment() instead of set_current() (which doesn't exist), stores progress in Redis for background tasks - elicitation.py: replace polling with BLPOP for efficient blocking wait, fail-fast on notification push failure, use get_task_context() for authoritative session_id - handlers.py: subscriber cleanup on session disconnect via _exit_stack.push_async_callback()
Replace 1300+ lines of mock-heavy unit tests with 391 lines of integration tests using real Client(mcp) connections and memory:// Docket backend. - test_context_background_task.py: 17 tests covering report_progress delta tracking, elicitation flow, edge cases, and fail-fast on push failure - test_notifications.py: 2 E2E tests for notification queue lifecycle
🤖 Generated with Codex
Co-authored-by: Bill Easton <strawgate@users.noreply.github.com> Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Jeremiah Lowin <153965+jlowin@users.noreply.github.com>
Co-authored-by: marvin-context-protocol[bot] <225465937+marvin-context-protocol[bot]@users.noreply.github.com>
Co-authored-by: Bill Easton <strawgate@users.noreply.github.com> Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Jeremiah Lowin <153965+jlowin@users.noreply.github.com>
Co-authored-by: marvin-context-protocol[bot] <225465937+marvin-context-protocol[bot]@users.noreply.github.com>
Co-authored-by: marvin-context-protocol[bot] <225465937+marvin-context-protocol[bot]@users.noreply.github.com>
When a background task calls ctx.elicit(), the notification subscriber now detects the input_required notification and sends a standard elicitation/create request to the client via session.elicit(). The client's elicitation_handler fires, and the relay pushes the response to Redis for the blocked worker. This means clients can respond to background task elicitation using the same elicitation_handler they'd use for any other elicitation — no need to interact with Redis or call handle_task_input() directly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
feat: distributed notification queue + BLPOP elicitation for background tasks
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Moves relay_elicitation() into elicitation.py so it can reuse handle_task_input() for the Redis push instead of duplicating that logic. notifications.py just detects the trigger and calls it. Also fixes the related-task metadata key from modelcontextprotocol.io/ to io.modelcontextprotocol/ to match the current spec: https://modelcontextprotocol.io/specification/2025-11-25/basic/utilities/tasks Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Relay task elicitation through standard MCP protocol
Co-authored-by: Bill Easton <strawgate@users.noreply.github.com> Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: marvin-context-protocol[bot] <225465937+marvin-context-protocol[bot]@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: marvin-context-protocol[bot] <225465937+marvin-context-protocol[bot]@users.noreply.github.com>
Co-authored-by: marvin-context-protocol[bot] <225465937+marvin-context-protocol[bot]@users.noreply.github.com>
Co-authored-by: marvin-context-protocol[bot] <225465937+marvin-context-protocol[bot]@users.noreply.github.com>
…3151) Co-authored-by: marvin-context-protocol[bot] <225465937+marvin-context-protocol[bot]@users.noreply.github.com>
Resolve conflict: remove old martian-issue-triage.yml (replaced by martian-triage-issue.yml) Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: marvin-context-protocol[bot] <225465937+marvin-context-protocol[bot]@users.noreply.github.com>
…ngTool (#3062) Co-authored-by: Bill Easton <strawgate@users.noreply.github.com> Co-authored-by: Jeremiah Lowin <jlowin@users.noreply.github.com> Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Jeremiah Lowin <153965+jlowin@users.noreply.github.com> Co-authored-by: marvin-context-protocol[bot] <225465937+marvin-context-protocol[bot]@users.noreply.github.com>
* fix: guard client pagination loops against misbehaving servers Treat empty/falsy nextCursor as end-of-pagination and detect cursor cycles across all client list methods and the server context proxy helper. * chore: Update SDK documentation --------- Co-authored-by: marvin-context-protocol[bot] <225465937+marvin-context-protocol[bot]@users.noreply.github.com>
Updates to github actions / workflows for claude
Co-authored-by: marvin-context-protocol[bot] <225465937+marvin-context-protocol[bot]@users.noreply.github.com>
* fix: restore request context in StatefulProxyClient handlers StatefulProxyClient reuses sessions across requests, so its receive-loop task inherits a stale request_ctx ContextVar from the first request. Server-initiated messages (elicitation, sampling, etc.) that depend on related_request_id routing get sent to a closed stream and hang forever. Closes #3169 * chore: Update SDK documentation --------- Co-authored-by: marvin-context-protocol[bot] <225465937+marvin-context-protocol[bot]@users.noreply.github.com>
) Bumps the uv group with 1 update in the / directory: [cryptography](https://github.com/pyca/cryptography). Updates `cryptography` from 46.0.4 to 46.0.5 - [Changelog](https://github.com/pyca/cryptography/blob/main/CHANGELOG.rst) - [Commits](pyca/cryptography@46.0.4...46.0.5) --- updated-dependencies: - dependency-name: cryptography dependency-version: 46.0.5 dependency-type: indirect dependency-group: uv ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* Fix include_tags/exclude_tags ignored without tools in MCPConfig * chore: Update SDK documentation --------- Co-authored-by: marvin-context-protocol[bot] <225465937+marvin-context-protocol[bot]@users.noreply.github.com>
Test Failure AnalysisSummary: All test jobs are failing during dependency installation because the build cannot find the Root Cause: The PR includes a local editable dependency configuration in [tool.uv.sources]
prefab-ui = { path = "../prefab", editable = true }This configuration works in local development where the
Suggested Solution: Update the CI workflow to either:
Detailed AnalysisError Log ExcerptAffected JobsAll test jobs are failing:
pyproject.toml ChangesThe PR adds: [project.optional-dependencies]
apps = ["prefab-ui>=0.1.0"]
[tool.uv.sources]
prefab-ui = { path = "../prefab", editable = true }
[dependency-groups]
dev = [
"fastmcp[anthropic,apps,azure,openai,tasks]",
# ...
]Related Files
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ccb74da45c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| async def __aenter__(self) -> str: | ||
| token = get_access_token() | ||
| if token is None: | ||
| raise RuntimeError( |
There was a problem hiding this comment.
Restore task token before resolving TokenClaim in workers
_TokenClaim.__aenter__ reads get_access_token() directly, but in Docket workers that returns None unless _task_access_token has first been restored by CurrentContext or CurrentAccessToken. A task-enabled tool that relies only on user_id: str = TokenClaim("sub") (without ctx/CurrentAccessToken) will therefore fail with RuntimeError even though submit_to_docket persisted the access token snapshot for that task.
Useful? React with 👍 / 👎.
| access_token = get_access_token() | ||
| if access_token is None: | ||
| raise RuntimeError( | ||
| "No access token available. Cannot perform OBO exchange." |
There was a problem hiding this comment.
Rehydrate task token before OBO exchange in background tasks
_EntraOBOToken.__aenter__ also calls get_access_token() without restoring the task token snapshot first, so OBO-based dependencies fail in background workers unless another dependency already restored _task_access_token. This breaks task tools that depend on EntraOBOToken(...) directly, despite the token being stored at submission time.
Useful? React with 👍 / 👎.
WalkthroughThis pull request introduces the v3.0.0 release candidate for FastMCP, featuring multiple breaking changes and new capabilities. Key changes include method renames (get\_tools/get\_resources → list\_tools/list\_resources with list return types), removal of 16 deprecated FastMCP constructor kwargs, parameter rename (ui= → app= in decorators, ToolUI/ResourceUI → AppConfig), and async authorization checks with support for sync and async callables. New features include Azure On-Behalf-Of token exchange, request-scoped non-serializable state storage, concurrent tool execution via tool\_concurrency parameter, pre-registered OAuth client credentials, background task notifications infrastructure via Redis, JSON Schema \$ref dereferencing middleware, SKILL.md file generation from CLI, and local provider exposure for direct component management. Supporting changes include updates to OAuth client implementation, context dependency injection (TokenClaim), OpenAPI output validation toggle, and comprehensive documentation including changelog entries and upgrade guides. Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/fastmcp/server/tasks/requests.py (1)
464-472:⚠️ Potential issue | 🟠 MajorInconsistent
fromisoformathandling — will raiseValueErroron Python 3.10 ifcreated_atcontains a trailing"Z".
tasks_get_handler(line 204) applies.replace("Z", "+00:00")before parsing, but this call site does not. Since the same Redis key is the source in both cases, the"Z"suffix is equally likely here. On Python <3.11,fromisoformatdoesn't accept"Z", so this will throw an unhandledValueErrorand surface as an internal error.Proposed fix
return CancelTaskResult( taskId=client_task_id, status="cancelled", - createdAt=datetime.fromisoformat(created_at) + createdAt=datetime.fromisoformat(created_at.replace("Z", "+00:00")) if created_at else datetime.now(timezone.utc),docs/python-sdk/fastmcp-server-tasks-elicitation.mdx (1)
15-75:⚠️ Potential issue | 🟠 MajorAvoid manual edits in bot‑managed docs/python‑sdk pages.
This path is auto‑generated, so manual edits will be overwritten. Please revert these changes and update the source that drives the bot instead.
As per coding guidelines: Do not manually modify docs/python-sdk/** - a bot automatically updates these files via commits added to PRs.
docs/development/v3-notes/v3-features.mdx (2)
1-3:⚠️ Potential issue | 🟡 MinorMissing
descriptionfield in YAML frontmatter.As per coding guidelines, every MDX documentation page must begin with YAML frontmatter containing both
titleanddescription.✏️ Suggested fix
--- title: v3.0 Feature Tracking +description: Tracks major features, breaking changes, and migration notes for FastMCP v3.0 releases. ---
325-328:⚠️ Potential issue | 🟡 MinorBeta1 section still references
notifications/tasks/updated, but RC1 changed this tonotifications/tasks/status.Line 327 says the task "sends a
notifications/tasks/updatednotification with elicitation metadata", but the RC1 changes (line 8, and the elicitation.py code) usenotifications/tasks/status. This inconsistency could confuse readers tracking the notification protocol evolution.✏️ Suggested fix
-Previously, tools running as background tasks couldn't use `ctx.elicit()` because there was no active request context. Now, when a tool executes in a Docket worker, `Context` detects this via its `task_id` and routes elicitation through Redis-based coordination: the task sets its status to `input_required`, sends a `notifications/tasks/updated` notification with elicitation metadata, and waits for the client to respond via `tasks/sendInput`. +Previously, tools running as background tasks couldn't use `ctx.elicit()` because there was no active request context. Now, when a tool executes in a Docket worker, `Context` detects this via its `task_id` and routes elicitation through Redis-based coordination: the task sets its status to `input_required`, sends a `notifications/tasks/status` notification with elicitation metadata, and waits for the client to respond via `tasks/sendInput`.
🧹 Nitpick comments (17)
src/fastmcp/server/providers/proxy.py (1)
814-826: Consider usingfunctools.wrapsto preserve handler metadata.The wrapper replaces the original handler's
__name__,__qualname__, and__doc__, which could make debugging/logging harder when tracing handler invocations. Adding@functools.wraps(handler)towrapperpreserves these attributes without affecting theinspect.isfunction()check mentioned in the docstring.♻️ Suggested improvement
+import functools + def _make_restoring_handler(handler: Callable, rc_ref: list[Any]) -> Callable: """Wrap a proxy handler to restore request_ctx before delegating.""" + `@functools.wraps`(handler) async def wrapper(*args: Any, **kwargs: Any) -> Any: _restore_request_context(rc_ref) return await handler(*args, **kwargs) return wrapperdocs/updates.mdx (1)
34-34: Minor grammar nit: "people that" → "people who".When referring to people, "who" is preferred over "that."
src/fastmcp/server/dependencies.py (2)
743-773: Token restoration silently swallows all exceptions — consider narrowing.The broad
except Exceptionat line 766 catches everything includingredis.ConnectionError, serialization bugs, and programming errors. Since this is a best-effort restoration for background tasks, swallowing is intentional, and theexc_info=Truelogging is good. However, you might consider narrowing to(OSError, ValueError, TypeError)to avoid masking unexpected bugs (e.g.,AttributeErrorfrom an API change indocket.redis()).This is a minor observation given the logging; feel free to keep as-is.
1262-1274:TokenClaimcoerces all claim values tostr— may surprise callers with list/dict claims.
str(value)on line 1274 will produce"['read', 'write']"for list-valued claims or"{'key': 'val'}"for dicts. The return type annotation saysstr, so this is technically consistent, but callers expecting something useful from a list claim will get a Python repr string. Consider documenting that only scalar claims work well withTokenClaim, or raising aTypeErrorfor non-scalar values.docs/servers/dependency-injection.mdx (1)
244-258: Inconsistent import path forTokenClaimvs other dependencies on this page.Other dependencies on this page (e.g.,
CurrentAccessTokenon line 209,CurrentHeaderson line 176) import fromfastmcp.dependencies, butTokenClaimimports fromfastmcp.server.dependencies. SinceTokenClaimis re-exported insrc/fastmcp/dependencies.py, consider using the shorter path for consistency:Suggested diff
-from fastmcp.server.dependencies import TokenClaim +from fastmcp.dependencies import TokenClaimsrc/fastmcp/server/tasks/notifications.py (3)
103-115: Redis connection acquired per loop iteration.Each iteration of the
while Trueloop opens a new Redis context viaasync with docket.redis() as redis. With a 30-second BRPOP timeout, this means a new connection checkout every 30 seconds at minimum. Ifdocket.redis()returns a pooled connection this is fine, but if it creates a fresh connection each time, it adds unnecessary overhead. Consider acquiring the connection outside the inner try block and reconnecting only on error.
165-221: Unuseddocketparameter in_send_mcp_notification.The
docketparameter (line 169) is never used in the function body. Since this is a private helper, either remove it and update the call site at line 125, or add a brief comment indicating it's reserved for future use.Option A: Remove unused parameter
async def _send_mcp_notification( session: ServerSession, notification_dict: dict[str, Any], session_id: str, - docket: Docket, fastmcp: FastMCP, ) -> None:And update the call site:
await _send_mcp_notification( - session, notification_dict, session_id, docket, fastmcp + session, notification_dict, session_id, fastmcp )
48-73:expireresets TTL on every push, extending lifetime for the entire queue.Each call to
push_notificationrunsredis.expire(key, NOTIFICATION_TTL_SECONDS), which resets the TTL for all messages in the queue — not just the newly pushed one. In a steady stream of notifications, old undelivered messages would never expire on their own. This is acceptable if the subscriber is actively draining the queue, but if the subscriber is down, stale messages accumulate beyond the intended 5-minute window.For the current use case (elicitation with an active subscriber), this is likely fine. Just noting the behavior.
src/fastmcp/server/tasks/elicitation.py (1)
164-172: Bareexcept: passswallows cleanup failures silently.When the notification queue fails and we attempt best-effort cleanup, silently swallowing the cleanup exception loses diagnostic information. A
logger.debugwould be consistent with the analogous cleanup block at lines 224-229.🔧 Suggested improvement
# Best-effort cleanup try: async with docket.redis() as redis: await redis.delete( docket.key(request_key), docket.key(status_key), ) - except Exception: - pass # Keys will expire via TTL + except Exception as cleanup_error: + logger.debug( + "Failed to clean up elicitation keys for task %s (will expire via TTL): %s", + task_id, + cleanup_error, + )src/fastmcp/server/tasks/handlers.py (2)
186-204: Monkey-patching_notification_cleanup_registeredon the session object is fragile.Setting
ctx.session._notification_cleanup_registered = Truerelies on theServerSessionobject tolerating arbitrary attribute assignment. IfServerSessionuses__slots__or Pydantic model restrictions, this will fail at runtime. The# type: ignore[attr-defined]suppresses the static check but doesn't protect against runtime failure.Consider using a module-level
setkeyed bysession_idto track which sessions have registered cleanup, similar to how_active_subscribersworks innotifications.py.♻️ Suggested alternative using a module-level set
+# Track sessions that have registered notification cleanup +_cleanup_registered: set[str] = set() + async def submit_to_docket( ...Then replace the monkey-patch check:
- if ( - hasattr(ctx.session, "_exit_stack") - and ctx.session._exit_stack is not None - and not getattr(ctx.session, "_notification_cleanup_registered", False) - ): - - async def _cleanup_subscriber() -> None: - await stop_subscriber(session_id) - - ctx.session._exit_stack.push_async_callback(_cleanup_subscriber) - ctx.session._notification_cleanup_registered = True # type: ignore[attr-defined] + if ( + hasattr(ctx.session, "_exit_stack") + and ctx.session._exit_stack is not None + and session_id not in _cleanup_registered + ): + + async def _cleanup_subscriber() -> None: + _cleanup_registered.discard(session_id) + await stop_subscriber(session_id) + + ctx.session._exit_stack.push_async_callback(_cleanup_subscriber) + _cleanup_registered.add(session_id)
128-150: Consider using the more idiomatic TaskStatusNotification pattern fromsubscriptions.py.The
model_validatedict approach withmethod,params, and_metaat the top level appears to work (identical pattern innotifications.py:189). However, the codebase has two inconsistent patterns:
- Current approach (handlers.py, notifications.py):
TaskStatusNotification.model_validate({...})- Alternative approach (subscriptions.py):
TaskStatusNotification(params=TaskStatusNotificationParams.model_validate(...))The subscriptions.py pattern is more type-safe and idiomatic for Pydantic models. Consider aligning handlers.py to use the direct constructor pattern:
from mcp.types import TaskStatusNotification, TaskStatusNotificationParams notification = TaskStatusNotification( params=TaskStatusNotificationParams.model_validate({ "taskId": server_task_id, "status": "working", "statusMessage": "Task submitted", "createdAt": created_at, "lastUpdatedAt": created_at, "ttl": ttl_ms, "pollInterval": poll_interval_ms, }) ) server_notification = mcp.types.ServerNotification(notification)This eliminates the need to manually specify the
methodfield and ensures proper type validation.examples/task_elicitation.py (1)
63-67: Add type annotations to the elicitation handler for clarity.As an example that users will copy, adding type annotations would improve discoverability and make the expected callback signature explicit. The unused parameters (
response_type,params,context) are required by the handler protocol, so prefixing with_would also silence the Ruff warnings.✏️ Suggested improvement
-async def handle_elicitation(message, response_type, params, context): +async def handle_elicitation( + message: str, response_type: type, _params: dict, _context: object +) -> DinnerPrefs: """Handle elicitation requests from background tasks.""" print(f" Server asks: {message}") print(" Responding with: cuisine=Thai, vegetarian=True") return DinnerPrefs(cuisine="Thai", vegetarian=True)docs/servers/authorization.mdx (1)
140-162: Async auth checks section is well-structured and clearly demonstrates the feature.One minor documentation nit:
fetch_user_permissionson line 156 is referenced but not defined or imported. Per coding guidelines, code examples should be complete and runnable. Consider adding a brief stub or comment indicating it's a placeholder for a user-defined function.Suggested clarification
async def check_user_permissions(ctx: AuthContext) -> bool: """Async auth check that reads server state.""" if ctx.token is None: return False user_id = ctx.token.claims.get("sub") - # Async operations work naturally in auth checks + # Replace with your async lookup (e.g., database query, HTTP call) permissions = await fetch_user_permissions(user_id) return "admin" in permissionssrc/fastmcp/server/auth/providers/azure.py (3)
643-648: Considertry/finallyinstead ofexcept BaseExceptionfor credential cleanup.The intent is to close the credential on any failure, then re-raise. A
try/finallyexpresses this more idiomatically and avoids catchingSystemExit/KeyboardInterruptexplicitly:♻️ Suggested refactor
- try: - result = await self._credential.get_token(*self.scopes) - except BaseException: - await self._credential.close() - self._credential = None - raise - - return result.token + ok = False + try: + result = await self._credential.get_token(*self.scopes) + ok = True + return result.token + finally: + if not ok: + await self._credential.close() + self._credential = None
599-607: Minor static analysis nits.Line 602: Ruff flags the
# noqa: F401as unnecessary (rule not enabled). Remove it.Line 604-607: Ruff suggests avoiding long messages outside exception classes (TRY003). This is a low-priority style concern that can be deferred.
🧹 Remove unused noqa
- import azure.identity # noqa: F401 + import azure.identity
632-637:TypeErrormay be more appropriate thanRuntimeErrorfor provider type mismatch.The check validates the type of
server.auth. Per convention (and Ruff TRY004),TypeErrorbetter communicates "wrong type" thanRuntimeError.🧹 Suggested change
- raise RuntimeError( + raise TypeError( "EntraOBOToken requires an AzureProvider as the auth provider. " f"Current provider: {type(server.auth).__name__}" )src/fastmcp/server/sampling/sampling_tool.py (1)
122-183:from_callable_tooldoes not exposesequentialparameter.Unlike
from_function, thefrom_callable_toolfactory doesn't accept asequentialparameter — the resultingSamplingToolalways defaults tosequential=False. If a server-side tool needs sequential execution when used in sampling, there's no way to set it through this path.This may be intentional (server tools don't carry this concept), but worth confirming.
| ```python | ||
| from fastmcp.server.auth.providers.azure import EntraOBOToken | ||
|
|
||
| @mcp.tool() | ||
| async def get_emails( | ||
| graph_token: str = EntraOBOToken(["https://graph.microsoft.com/Mail.Read"]), | ||
| ): | ||
| # OBO exchange already happened — just use the token | ||
| ... | ||
| ``` |
There was a problem hiding this comment.
Make the Entra OBO snippet runnable (remove the ellipsis).
Lines 17-26 show a code example with ... and no minimal FastMCP setup, so it isn’t copy‑pasteable. Please provide a complete, runnable snippet with the necessary setup and a concrete return value.
As per coding guidelines: Always include complete, runnable code examples that users can copy and execute in MDX documentation.
| <Step title="Client Registration"> | ||
| If the OAuth server supports it and the client isn't already registered (or credentials aren't cached), the client performs dynamic client registration according to RFC 7591. Alternatively, if a `client_metadata_url` is configured and the server supports CIMD, the client uses its metadata URL as its identity instead of registering. | ||
| If a `client_id` is provided, the client uses those pre-registered credentials directly and skips this step entirely. Otherwise, if a `client_metadata_url` is configured and the server supports CIMD, the client uses its metadata URL as its identity. As a fallback, the client performs Dynamic Client Registration (RFC 7591) if the server supports it. | ||
| </Step> |
There was a problem hiding this comment.
Use second‑person wording in the OAuth Flow step.
Line 79 reads in third person (“the client uses…”). Please rephrase to address the reader directly (e.g., “If you provide a client_id, you skip registration…”).
As per coding guidelines: Write in second person ('you') for instructions and procedures in MDX documentation.
| ```python | ||
| from fastmcp import Client | ||
| from fastmcp.client.auth import OAuth | ||
|
|
||
| async with Client( | ||
| "https://mcp-server.example.com/mcp", | ||
| auth=OAuth( | ||
| client_id="my-registered-client-id", | ||
| client_secret="my-client-secret", | ||
| ), | ||
| ) as client: | ||
| await client.ping() | ||
| ``` | ||
|
|
||
| Public clients that rely on PKCE for security can omit `client_secret`: | ||
|
|
||
| ```python | ||
| oauth = OAuth(client_id="my-public-client-id") | ||
| ``` |
There was a problem hiding this comment.
Pre‑registered client examples aren’t runnable.
Lines 164-182 show partial snippets (the second snippet never executes a client call) and omit minimal error handling/expected outcomes, so they aren’t copy‑pasteable. Please provide a complete, runnable example with a full Client usage block and expected result.
As per coding guidelines: Always include complete, runnable code examples that users can copy and execute in MDX documentation.
| #### Listing Methods Renamed and Return Lists | ||
|
|
||
| `get_tools()`, `get_resources()`, `get_prompts()`, and `get_resource_templates()` now return lists instead of dicts: | ||
| `get_tools()`, `get_resources()`, `get_prompts()`, and `get_resource_templates()` have been replaced by `list_tools()`, `list_resources()`, `list_prompts()`, and `list_resource_templates()`. The new methods return lists instead of dicts: | ||
|
|
||
| ```python | ||
| # Before | ||
| tools = await server.get_tools() | ||
| tool = tools["my_tool"] | ||
|
|
||
| # After | ||
| tools = await server.get_tools() | ||
| tools = await server.list_tools() | ||
| tool = next((t for t in tools if t.name == "my_tool"), None) | ||
| ``` |
There was a problem hiding this comment.
Make the new list_ and serializable=False examples executable.*
Lines 46-58 and 100-106 reference undefined server/ctx and omit minimal setup/error handling, so they aren’t runnable. Please expand to full, copy‑pasteable snippets with minimal setup and expected outcomes.
As per coding guidelines: Always include complete, runnable code examples that users can copy and execute in MDX documentation.
Also applies to: 100-106
|
|
||
| ```bash | ||
| pip install "fastmcp[tasks]==3.0.0b2" | ||
| pip install "fastmcp[tasks]==3.0.0rc1" |
There was a problem hiding this comment.
Inconsistent version specifier: == here vs >= in the install commands above.
Lines 15 and 21 use >=3.0.0rc1 so users pick up newer pre-releases and the eventual stable release, but this extras example pins to exactly ==3.0.0rc1. A user copying this line will be stuck on rc1 even after 3.0.0 stable ships. Consider using >= here too for consistency, or add a brief note explaining the difference.
-pip install "fastmcp[tasks]==3.0.0rc1"
+pip install "fastmcp[tasks]>=3.0.0rc1"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pip install "fastmcp[tasks]==3.0.0rc1" | |
| pip install "fastmcp[tasks]>=3.0.0rc1" |
| class _TokenClaim(Dependency): # type: ignore[misc] | ||
| """Dependency that extracts a specific claim from the access token.""" | ||
|
|
||
| def __init__(self, claim_name: str): | ||
| self.claim_name = claim_name | ||
|
|
||
| async def __aenter__(self) -> str: | ||
| token = get_access_token() | ||
| if token is None: | ||
| raise RuntimeError( | ||
| f"No access token available. Cannot extract claim '{self.claim_name}'." | ||
| ) | ||
| value = token.claims.get(self.claim_name) | ||
| if value is None: | ||
| raise RuntimeError( | ||
| f"Claim '{self.claim_name}' not found in access token. " | ||
| f"Available claims: {list(token.claims.keys())}" | ||
| ) | ||
| return str(value) | ||
|
|
||
| async def __aexit__(self, *args: object) -> None: | ||
| pass |
There was a problem hiding this comment.
_TokenClaim.__aenter__ does not attempt background-task token restoration.
Unlike _CurrentAccessToken, _TokenClaim calls get_access_token() directly without falling back to _restore_task_access_token() when the token is None in a Docket worker. If a function uses TokenClaim("sub") but does not declare ctx: Context or CurrentAccessToken(), and runs as a background task, get_access_token() will return None (the ContextVar hasn't been populated yet), and the user gets a confusing RuntimeError instead of a restored token.
Consider adding the same Docket-worker fallback that _CurrentAccessToken has, or documenting this limitation.
Proposed fix: add background-task restoration fallback
class _TokenClaim(Dependency): # type: ignore[misc]
"""Dependency that extracts a specific claim from the access token."""
def __init__(self, claim_name: str):
self.claim_name = claim_name
+ self._access_token_cv_token: Token[AccessToken | None] | None = None
async def __aenter__(self) -> str:
token = get_access_token()
+
+ # Fall back to Redis snapshot in background tasks (same as _CurrentAccessToken)
+ if token is None:
+ task_info = get_task_context()
+ if task_info is not None:
+ self._access_token_cv_token = await _restore_task_access_token(
+ task_info.session_id, task_info.task_id
+ )
+ token = get_access_token()
+
if token is None:
raise RuntimeError(
f"No access token available. Cannot extract claim '{self.claim_name}'."
)
value = token.claims.get(self.claim_name)
if value is None:
raise RuntimeError(
f"Claim '{self.claim_name}' not found in access token. "
f"Available claims: {list(token.claims.keys())}"
)
return str(value)
async def __aexit__(self, *args: object) -> None:
- pass
+ if self._access_token_cv_token is not None:
+ _task_access_token.reset(self._access_token_cv_token)
+ self._access_token_cv_token = None🧰 Tools
🪛 Ruff (0.15.0)
[warning] 1265-1267: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 1270-1273: Avoid specifying long messages outside the exception class
(TRY003)
| stateless_http = fastmcp.settings.stateless_http | ||
|
|
||
| # SSE doesn't support stateless mode | ||
| if stateless_http and transport == "sse": | ||
| raise ValueError("SSE transport does not support stateless mode") | ||
|
|
||
| host = host or self._deprecated_settings.host | ||
| port = port or self._deprecated_settings.port | ||
| default_log_level_to_use = ( | ||
| log_level or self._deprecated_settings.log_level | ||
| ).lower() | ||
| host = host or fastmcp.settings.host | ||
| port = port or fastmcp.settings.port | ||
| default_log_level_to_use = (log_level or fastmcp.settings.log_level).lower() |
There was a problem hiding this comment.
port=0 would be silently ignored due to or instead of is None check.
Line 241 uses port or fastmcp.settings.port, but port=0 is a valid value (OS-assigned port) that is falsy. The same pattern for stateless_http on line 233–234 correctly uses an explicit is None check. Consider aligning port (and optionally host) with the same pattern for consistency and correctness.
Proposed fix
- host = host or fastmcp.settings.host
- port = port or fastmcp.settings.port
+ host = host if host is not None else fastmcp.settings.host
+ port = port if port is not None else fastmcp.settings.port🧰 Tools
🪛 Ruff (0.15.0)
[warning] 238-238: Avoid specifying long messages outside the exception class
(TRY003)
| # Mutable list shared across copies (Client.new() uses copy.copy, | ||
| # which preserves references to mutable containers). ProxyTool.run | ||
| # writes [0] before each backend call; handlers read it to detect | ||
| # stale ContextVars and restore the correct request_ctx. | ||
| # | ||
| # We store the concrete RequestContext (not fastmcp's Context) because | ||
| # Context properties are themselves ContextVar-dependent and resolve | ||
| # in the caller's async context — which is stale in the receive loop. | ||
| _proxy_rc_ref: list[Any] | ||
|
|
||
| def __init__(self, *args: Any, **kwargs: Any): | ||
| # Install context-restoring handler wrappers BEFORE super().__init__ | ||
| # registers them with the Client's session kwargs. | ||
| self._proxy_rc_ref = [None] | ||
| for key, default_fn in ( | ||
| ("roots", default_proxy_roots_handler), | ||
| ("sampling_handler", default_proxy_sampling_handler), | ||
| ("elicitation_handler", default_proxy_elicitation_handler), | ||
| ("log_handler", default_proxy_log_handler), | ||
| ("progress_handler", default_proxy_progress_handler), | ||
| ): | ||
| if key not in kwargs: | ||
| kwargs[key] = _make_restoring_handler(default_fn, self._proxy_rc_ref) | ||
|
|
||
| super().__init__(*args, **kwargs) |
There was a problem hiding this comment.
Potential last-write-wins race on shared _proxy_rc_ref under concurrent same-session tool calls.
The single-element mutable list is shared across all copy.copy clones. If the server enables concurrent tool execution (tool_concurrency > 1) and two tool calls on the same session both reach line 130, the second write to rc_ref[0] silently overwrites the first. A subsequent handler callback for the first call would then restore the wrong RequestContext.
The cross-session case is correctly guarded by the current_rc.session is rc.session identity check. The same-session concurrent case is unlikely with typical MCP request serialization, but worth documenting as a known limitation or guarding with a per-request-id dict if concurrent tool execution becomes common for stateful proxies.
| async def _execute_single_tool(tool_use: ToolUseContent) -> ToolResultContent: | ||
| """Execute a single tool and return its result.""" | ||
| tool = tool_map.get(tool_use.name) | ||
| if tool is None: | ||
| tool_results.append( | ||
| ToolResultContent( | ||
| type="tool_result", | ||
| toolUseId=tool_use.id, | ||
| content=[ | ||
| TextContent( | ||
| type="text", | ||
| text=f"Error: Unknown tool '{tool_use.name}'", | ||
| ) | ||
| ], | ||
| isError=True, | ||
| ) | ||
| ) | ||
| else: | ||
| try: | ||
| result_value = await tool.run(tool_use.input) | ||
| tool_results.append( | ||
| ToolResultContent( | ||
| type="tool_result", | ||
| toolUseId=tool_use.id, | ||
| content=[TextContent(type="text", text=str(result_value))], | ||
| ) | ||
| ) | ||
| except ToolError as e: | ||
| # ToolError is the escape hatch - always pass message through | ||
| logger.exception(f"Error calling sampling tool '{tool_use.name}'") | ||
| tool_results.append( | ||
| ToolResultContent( | ||
| type="tool_result", | ||
| toolUseId=tool_use.id, | ||
| content=[TextContent(type="text", text=str(e))], | ||
| isError=True, | ||
| ) | ||
| ) | ||
| except Exception as e: | ||
| # Generic exceptions - mask based on setting | ||
| logger.exception(f"Error calling sampling tool '{tool_use.name}'") | ||
| if mask_error_details: | ||
| error_text = f"Error executing tool '{tool_use.name}'" | ||
| else: | ||
| error_text = f"Error executing tool '{tool_use.name}': {e}" | ||
| tool_results.append( | ||
| ToolResultContent( | ||
| type="tool_result", | ||
| toolUseId=tool_use.id, | ||
| content=[TextContent(type="text", text=error_text)], | ||
| isError=True, | ||
| return ToolResultContent( | ||
| type="tool_result", | ||
| toolUseId=tool_use.id, | ||
| content=[ | ||
| TextContent( | ||
| type="text", | ||
| text=f"Error: Unknown tool '{tool_use.name}'", | ||
| ) | ||
| ) | ||
| ], | ||
| isError=True, | ||
| ) | ||
|
|
||
| try: | ||
| result_value = await tool.run(tool_use.input) | ||
| return ToolResultContent( | ||
| type="tool_result", | ||
| toolUseId=tool_use.id, | ||
| content=[TextContent(type="text", text=str(result_value))], | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -a 'sampling_tool.py$'Repository: jlowin/fastmcp
Length of output: 184
🏁 Script executed:
wc -l /home/jailuser/git/src/fastmcp/server/sampling/sampling_tool.pyRepository: jlowin/fastmcp
Length of output: 125
🏁 Script executed:
cat -n /home/jailuser/git/src/fastmcp/server/sampling/sampling_tool.pyRepository: jlowin/fastmcp
Length of output: 8032
🏁 Script executed:
wc -l /home/jailuser/git/src/fastmcp/server/sampling/run.pyRepository: jlowin/fastmcp
Length of output: 115
🏁 Script executed:
sed -n '260,310p' /home/jailuser/git/src/fastmcp/server/sampling/run.pyRepository: jlowin/fastmcp
Length of output: 2098
🏁 Script executed:
sed -n '1,50p' /home/jailuser/git/src/fastmcp/server/sampling/run.pyRepository: jlowin/fastmcp
Length of output: 1576
🏁 Script executed:
rg -n "from_function\|from_callable_tool" /home/jailuser/git/src/fastmcp/server/sampling/run.py -A 3Repository: jlowin/fastmcp
Length of output: 40
🏁 Script executed:
sed -n '200,260p' /home/jailuser/git/src/fastmcp/server/sampling/run.pyRepository: jlowin/fastmcp
Length of output: 2316
🏁 Script executed:
sed -n '1,100p' /home/jailuser/git/tests/server/sampling/test_sampling_tool.pyRepository: jlowin/fastmcp
Length of output: 3186
🏁 Script executed:
rg -n "str(result" /home/jailuser/git/src/fastmcp/server/sampling/run.py -B 3 -A 3Repository: jlowin/fastmcp
Length of output: 122
🏁 Script executed:
rg -n "TextContent.*str\(" /home/jailuser/git/src/fastmcp/server/sampling/run.py -B 2 -A 2Repository: jlowin/fastmcp
Length of output: 480
🏁 Script executed:
rg -n "SamplingTool\|sampling_tool" /home/jailuser/git/src/fastmcp/server/sampling/run.py | head -20Repository: jlowin/fastmcp
Length of output: 40
🏁 Script executed:
sed -n '100,200p' /home/jailuser/git/src/fastmcp/server/sampling/run.pyRepository: jlowin/fastmcp
Length of output: 3915
🏁 Script executed:
rg -n "json\|JSON" /home/jailuser/git/src/fastmcp/server/sampling/run.py | head -15Repository: jlowin/fastmcp
Length of output: 40
🏁 Script executed:
sed -n '300,400p' /home/jailuser/git/src/fastmcp/server/sampling/run.pyRepository: jlowin/fastmcp
Length of output: 3671
🏁 Script executed:
rg -n "structured_content\|TextContent" /home/jailuser/git/src/fastmcp/server/sampling/sampling_tool.py -B 2 -A 2Repository: jlowin/fastmcp
Length of output: 40
Use JSON serialization instead of str() for tool results to preserve structured content.
When SamplingTool.run() returns structured objects (e.g., from from_callable_tool() which extracts structured_content), calling str() flattens them to their string representation rather than JSON. Use json.dumps() or a similar serializer to preserve structure in the LLM response.
| if dereference: | ||
| schema = dereference_refs(schema) | ||
|
|
||
| # Resolve root-level $ref for MCP spec compliance (requires type: object at root) | ||
| schema = resolve_root_ref(schema) |
There was a problem hiding this comment.
Root $ref resolution can drop root‑level annotations.
Line 389 now always calls resolve_root_ref(). That function inlines the $defs entry but discards root‑level siblings like title, description, or default, which can silently strip metadata when schemas use a root $ref. Consider merging root‑level siblings into the resolved schema before returning.
🛠️ Suggested fix to preserve root‑level siblings
def resolve_root_ref(schema: dict[str, Any]) -> dict[str, Any]:
@@
if def_name in defs:
# Create a new schema by copying the referenced definition
resolved = dict(defs[def_name])
+ # Preserve root-level sibling keywords (e.g., title/description/default)
+ siblings = {
+ k: v for k, v in schema.items() if k not in ("$ref", "$defs")
+ }
+ if siblings:
+ resolved.update(siblings)
# Preserve $defs for nested references (other fields may still use them)
resolved["$defs"] = defs
return resolved
Brings feature/apps up to date with main. Resolves three merge conflicts:
pyproject.toml: main addedazureoptional dep alongside ourappsdep — kept bothtools/tool.py: preserved prefab imports andAuthCheckCallablealias that main had removeduv.lock: regeneratedAlso updates
UIResponseimports across the codebase to useprefab_ui.response.UIResponse(prefab stopped re-exporting from the top-level__init__).