mirror of
https://github.com/PrefectHQ/fastmcp.git
synced 2026-08-20 20:44:17 +02:00
fix: elicitation scalar return, resource auto-serialization, Client.new() state, prompt errors (#3859)
* fix: elicitation scalar return, resource auto-serialization, Client.new() state, prompt errors
- Auto-wrap scalar elicitation responses for ScalarElicitationType schemas
so handlers can return T directly for ctx.elicit("msg", str/int/float)
- Auto-serialize dict/int/float/bool/None resource returns to JSON text
instead of crashing with TypeError
- Reset _task_registry and _submitted_task_ids in Client.new() so cloned
clients have independent task tracking state
- Include original error message in prompt render errors (matching tool
error behavior)
Fixes #3856
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Fix misleading comment and add list/tuple auto-serialization for resources
The comment said "list/tuple of primitives" but the isinstance check
didn't include list or tuple. Now it does, and the comment matches.
🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Fix TaskNotificationHandler binding in Client.new() and meta forwarding for JSON resources
Two review-identified bugs:
1. Client.new() shallow-copies _session_kwargs, so the cloned client's
TaskNotificationHandler still dispatches to the original client.
Fix: create a fresh _session_kwargs dict with a new handler bound
to the new client.
2. convert_result() for dict/int/float/bool/None fell through to
ResourceResult(raw_value) which lost component meta (CSP, permissions).
The str/bytes path correctly wrapped in ResourceContent with meta.
Fix: explicitly serialize JSON-native types and wrap with meta,
matching the str/bytes path. Other types still fall through for
error handling.
🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Preserve custom message handlers in Client.new()
Only replace the message handler with a new TaskNotificationHandler
if the current handler IS a TaskNotificationHandler. If the user
provided a custom message_handler, preserve it in the clone.
🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix client.new and add regression tests
* Honor declared MIME type for auto-serialized JSON resources
* Fix static analysis: remove unused StdioTransport import, fix ty:ignore comment
* Exclude list[ResourceContent] from JSON auto-serialization path
A bare list[ResourceContent] would match the isinstance(list) check
and get JSON-serialized instead of passing through to ResourceResult
normalization. Check for ResourceContent items first.
🤖 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>
This commit is contained in:
parent
9a063963f4
commit
db6d7a8a61
9 changed files with 140 additions and 20 deletions
|
|
@ -70,7 +70,6 @@ from .transports import (
|
|||
PythonStdioTransport,
|
||||
SessionKwargs,
|
||||
SSETransport,
|
||||
StdioTransport,
|
||||
StreamableHttpTransport,
|
||||
infer_transport,
|
||||
)
|
||||
|
|
@ -433,9 +432,25 @@ class Client(
|
|||
"""
|
||||
new_client = copy.copy(self)
|
||||
|
||||
if not isinstance(self.transport, StdioTransport):
|
||||
# Reset session state to fresh state
|
||||
new_client._session_state = ClientSessionState()
|
||||
# Always reset session state so cloned clients start disconnected and do not
|
||||
# share lifecycle state with the original instance.
|
||||
new_client._session_state = ClientSessionState()
|
||||
|
||||
# Reset mutable task tracking state so new client is independent
|
||||
new_client._task_registry = {}
|
||||
new_client._submitted_task_ids = set()
|
||||
|
||||
# Create a fresh session kwargs dict so the clone doesn't share
|
||||
# the original's mutable dict. Rebind the task notification handler
|
||||
# to the new client if the default handler is in use; preserve any
|
||||
# custom message handler the user may have set.
|
||||
new_client._session_kwargs = {**self._session_kwargs} # type: ignore[typeddict-item]
|
||||
if isinstance(
|
||||
self._session_kwargs.get("message_handler"), TaskNotificationHandler
|
||||
):
|
||||
new_client._session_kwargs["message_handler"] = TaskNotificationHandler(
|
||||
new_client
|
||||
)
|
||||
|
||||
new_client.name += f":{secrets.token_hex(2)}"
|
||||
|
||||
|
|
|
|||
|
|
@ -61,10 +61,18 @@ def create_elicitation_callback(
|
|||
result = ElicitResult(action="accept", content=result)
|
||||
content = to_jsonable_python(result.content)
|
||||
if not isinstance(content, dict | None):
|
||||
raise ValueError(
|
||||
"Elicitation responses must be serializable as a JSON object (dict). Received: "
|
||||
f"{result.content!r}"
|
||||
)
|
||||
# Auto-wrap scalar values for ScalarElicitationType schemas
|
||||
# (single "value" property). This lets handlers return T directly
|
||||
# for ctx.elicit("msg", str/int/float/bool).
|
||||
if isinstance(params, ElicitRequestFormParams) and set(
|
||||
params.requestedSchema.get("properties", {}).keys()
|
||||
) == {"value"}:
|
||||
content = {"value": content}
|
||||
else:
|
||||
raise ValueError(
|
||||
"Elicitation responses must be serializable as a JSON object (dict). Received: "
|
||||
f"{result.content!r}"
|
||||
)
|
||||
return MCPElicitResult(
|
||||
_meta=result.meta, # type: ignore[call-arg] # _meta is Pydantic alias for meta field # ty:ignore[unknown-argument]
|
||||
action=result.action,
|
||||
|
|
|
|||
|
|
@ -363,7 +363,7 @@ class FunctionPrompt(Prompt):
|
|||
return self.convert_result(result)
|
||||
except Exception as e:
|
||||
logger.exception(f"Error rendering prompt {self.name}")
|
||||
raise PromptError(f"Error rendering prompt {self.name}.") from e
|
||||
raise PromptError(f"Error rendering prompt {self.name!r}: {e}") from e
|
||||
|
||||
def register_with_docket(self, docket: Docket) -> None:
|
||||
"""Register this prompt with docket for background execution."""
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@
|
|||
from __future__ import annotations
|
||||
|
||||
import base64
|
||||
import json
|
||||
from collections.abc import Callable
|
||||
from typing import TYPE_CHECKING, Annotated, Any, ClassVar, overload
|
||||
|
||||
|
|
@ -188,6 +189,12 @@ class ResourceResult(pydantic.BaseModel):
|
|||
f"Use ResourceContent({item!r}) to wrap the value."
|
||||
)
|
||||
return contents
|
||||
# Auto-serialize JSON-native types to JSON text
|
||||
if (
|
||||
isinstance(contents, dict | list | tuple | int | float | bool)
|
||||
or contents is None
|
||||
):
|
||||
return [ResourceContent(json.dumps(contents), mime_type="application/json")]
|
||||
raise TypeError(
|
||||
f"contents must be str, bytes, or list[ResourceContent], got {type(contents).__name__}"
|
||||
)
|
||||
|
|
@ -329,7 +336,30 @@ class Resource(FastMCPComponent):
|
|||
[ResourceContent(raw_value, mime_type=self.mime_type, meta=self.meta)]
|
||||
)
|
||||
|
||||
# ResourceResult.__init__ handles all other normalization
|
||||
# For JSON-native types (dict, list, tuple, int, float, bool, None),
|
||||
# serialize and wrap in ResourceContent with the component's meta,
|
||||
# matching the str/bytes path above so CSP/permissions propagate.
|
||||
# Exclude list[ResourceContent] which should go through ResourceResult
|
||||
# normalization below.
|
||||
if (
|
||||
isinstance(raw_value, dict | list | tuple | int | float | bool)
|
||||
or raw_value is None
|
||||
) and not (
|
||||
isinstance(raw_value, list)
|
||||
and raw_value
|
||||
and isinstance(raw_value[0], ResourceContent)
|
||||
):
|
||||
return ResourceResult(
|
||||
[
|
||||
ResourceContent(
|
||||
json.dumps(raw_value),
|
||||
mime_type=self.mime_type or "application/json",
|
||||
meta=self.meta,
|
||||
)
|
||||
]
|
||||
)
|
||||
|
||||
# All other types fall through to ResourceResult for error handling
|
||||
return ResourceResult(raw_value)
|
||||
|
||||
@overload
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@ from pydantic import AnyUrl
|
|||
|
||||
import fastmcp
|
||||
from fastmcp.client import Client
|
||||
from fastmcp.client.tasks import TaskNotificationHandler
|
||||
from fastmcp.client.transports import (
|
||||
ClientTransport,
|
||||
FastMCPTransport,
|
||||
|
|
@ -832,3 +833,34 @@ async def test_client_list_dict_return_type():
|
|||
async with client:
|
||||
result = await client.call_tool("get_temperatures", {})
|
||||
assert result.data == [{"city": "NYC", "temp": 72}, {"city": "LA", "temp": 85}]
|
||||
|
||||
|
||||
def test_client_new_resets_mutable_task_state(fastmcp_server):
|
||||
"""Client.new() should not share mutable task tracking structures."""
|
||||
client = Client(transport=FastMCPTransport(fastmcp_server))
|
||||
|
||||
client._task_registry["task-1"] = lambda: None # type: ignore[assignment] # ty:ignore[invalid-assignment]
|
||||
client._submitted_task_ids.add("task-1")
|
||||
|
||||
clone = client.new()
|
||||
|
||||
assert clone is not client
|
||||
assert clone._task_registry == {}
|
||||
assert clone._submitted_task_ids == set()
|
||||
assert clone._task_registry is not client._task_registry
|
||||
assert clone._submitted_task_ids is not client._submitted_task_ids
|
||||
|
||||
|
||||
def test_client_new_rebinds_default_task_notification_handler(fastmcp_server):
|
||||
"""Client.new() should bind the default task handler to the cloned client."""
|
||||
client = Client(transport=FastMCPTransport(fastmcp_server))
|
||||
|
||||
handler = client._session_kwargs.get("message_handler")
|
||||
assert isinstance(handler, TaskNotificationHandler)
|
||||
|
||||
clone = client.new()
|
||||
|
||||
clone_handler = clone._session_kwargs.get("message_handler")
|
||||
assert isinstance(clone_handler, TaskNotificationHandler)
|
||||
assert clone_handler is not handler
|
||||
assert clone_handler._client_ref() is clone
|
||||
|
|
|
|||
|
|
@ -144,6 +144,24 @@ async def test_elicitation_cancel_action():
|
|||
|
||||
|
||||
class TestScalarResponseTypes:
|
||||
async def test_scalar_handler_return_is_auto_wrapped(self):
|
||||
"""Scalar handler returns are wrapped as {"value": ...} for scalar schemas."""
|
||||
mcp = FastMCP("TestServer")
|
||||
|
||||
@mcp.tool
|
||||
async def my_tool(context: Context) -> str:
|
||||
result = await context.elicit(message="", response_type=str)
|
||||
assert isinstance(result, AcceptedElicitation)
|
||||
assert isinstance(result.data, str)
|
||||
return result.data
|
||||
|
||||
async def elicitation_handler(message, response_type, params, ctx):
|
||||
return ElicitResult(action="accept", content="Alice")
|
||||
|
||||
async with Client(mcp, elicitation_handler=elicitation_handler) as client:
|
||||
result = await client.call_tool("my_tool", {})
|
||||
assert result.data == "Alice"
|
||||
|
||||
async def test_elicitation_no_response(self):
|
||||
"""Test elicitation with no response type."""
|
||||
mcp = FastMCP("TestServer")
|
||||
|
|
|
|||
|
|
@ -277,7 +277,7 @@ class TestPromptTypeConversion:
|
|||
with pytest.raises(PromptError) as exc_info:
|
||||
await prompt.render(arguments={"numbers": "not valid json"})
|
||||
|
||||
assert f"Error rendering prompt {prompt.name}" in str(exc_info.value)
|
||||
assert f"Error rendering prompt {prompt.name!r}" in str(exc_info.value)
|
||||
|
||||
async def test_json_parsing_fallback(self):
|
||||
"""Test that JSON parsing falls back to direct validation when needed."""
|
||||
|
|
|
|||
|
|
@ -66,8 +66,8 @@ class TestFunctionResource:
|
|||
result = await resource._read()
|
||||
assert result.contents[0].content == b"Hello, world!"
|
||||
|
||||
async def test_dict_return_raises_type_error(self):
|
||||
"""Returning dict from read() raises TypeError - use ResourceResult."""
|
||||
async def test_dict_return_auto_serializes(self):
|
||||
"""Returning dict from read() auto-serializes to JSON."""
|
||||
|
||||
def get_data() -> dict:
|
||||
return {"key": "value"}
|
||||
|
|
@ -81,9 +81,10 @@ class TestFunctionResource:
|
|||
result = await resource.read()
|
||||
assert result == {"key": "value"}
|
||||
|
||||
# _read() raises TypeError - must return str, bytes, or ResourceResult
|
||||
with pytest.raises(TypeError, match="must be str, bytes, or list"):
|
||||
await resource._read()
|
||||
# _read() auto-serializes dict to JSON text
|
||||
resource_result = await resource._read()
|
||||
assert len(resource_result.contents) == 1
|
||||
assert '"key"' in str(resource_result.contents[0].content)
|
||||
|
||||
async def test_error_handling(self):
|
||||
"""Test error handling in FunctionResource."""
|
||||
|
|
|
|||
|
|
@ -208,10 +208,13 @@ class TestResourceResult:
|
|||
assert result.contents[0].content == b"\xff\xfe"
|
||||
assert result.contents[0].mime_type == "application/octet-stream"
|
||||
|
||||
def test_init_from_dict_raises_type_error(self):
|
||||
"""Dict input raises TypeError - must use ResourceContent for serialization."""
|
||||
with pytest.raises(TypeError, match="must be str, bytes, or list"):
|
||||
ResourceResult({"page": 1, "total": 100}) # type: ignore[arg-type] # ty:ignore[invalid-argument-type]
|
||||
def test_init_from_dict_auto_serializes(self):
|
||||
"""Dict input is auto-serialized to JSON text."""
|
||||
result = ResourceResult({"page": 1, "total": 100}) # type: ignore[arg-type] # ty:ignore[invalid-argument-type]
|
||||
assert len(result.contents) == 1
|
||||
text = str(result.contents[0].content)
|
||||
assert '"page"' in text
|
||||
assert '"total"' in text
|
||||
|
||||
def test_init_from_single_resource_content_raises_type_error(self):
|
||||
"""Single ResourceContent raises TypeError - must be in a list."""
|
||||
|
|
@ -354,3 +357,16 @@ class TestResourceMetaPropagation:
|
|||
result = await client.read_resource_mcp("test://both-meta")
|
||||
assert result.meta == {"result_key": "result_val"}
|
||||
assert result.contents[0].meta == {"item_key": "item_val"}
|
||||
|
||||
async def test_json_native_return_preserves_component_meta(self):
|
||||
"""JSON-native returns should propagate component-level meta to content."""
|
||||
mcp = FastMCP()
|
||||
|
||||
@mcp.resource("test://json-meta", meta={"csp": "default-src 'none'"})
|
||||
def json_resource() -> dict[str, str]:
|
||||
return {"hello": "world"}
|
||||
|
||||
async with Client(mcp) as client:
|
||||
result = await client.read_resource_mcp("test://json-meta")
|
||||
assert len(result.contents) == 1
|
||||
assert result.contents[0].meta == {"csp": "default-src 'none'"}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue