-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Adds meta parameter and method for LLM hidden info #1376
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
6fbbaa6
5c23510
0a471e5
dc8ebff
2625a05
c199d93
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
import inspect | ||
from typing import Any | ||
|
||
import anyio | ||
|
@@ -497,3 +498,75 @@ async def mock_server(): | |
assert received_capabilities.roots is not None # Custom list_roots callback provided | ||
assert isinstance(received_capabilities.roots, types.RootsCapability) | ||
assert received_capabilities.roots.listChanged is True # Should be True for custom callback | ||
|
||
|
||
def test_call_tool_method_signature(): | ||
"""Test that call_tool method accepts meta parameter in its signature.""" | ||
|
||
signature = inspect.signature(ClientSession.call_tool) | ||
|
||
assert "meta" in signature.parameters, "call_tool method should have 'meta' parameter" | ||
|
||
meta_param = signature.parameters["meta"] | ||
assert meta_param.default is None, "meta parameter should default to None" | ||
Comment on lines
+503
to
+511
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove this test, it shouldn't be needed :) This is a fragile test which relies entirely on implementation details rather than functionality. This should be tested via other unit tests, such as one that passes a "meta" parameter. If "meta" didn't exist then that test would fail. Similarly, there should be a unit tests which test the default case instead of doing inspection on the signature. |
||
|
||
|
||
def test_call_tool_request_params_construction(): | ||
"""Test that CallToolRequestParams can be constructed with metadata.""" | ||
from mcp.types import CallToolRequestParams, RequestParams | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. move imports to top of file |
||
|
||
params_no_meta = CallToolRequestParams(name="test_tool", arguments={"param": "value"}) | ||
assert params_no_meta.name == "test_tool" | ||
assert params_no_meta.arguments == {"param": "value"} | ||
assert params_no_meta.meta is None | ||
|
||
meta_data = { | ||
"progressToken": None, | ||
"user_id": "test_user", | ||
"session_id": "test_session", | ||
"custom_field": "custom_value", | ||
} | ||
test_meta = RequestParams.Meta.model_validate(meta_data) | ||
|
||
params_with_meta = CallToolRequestParams( | ||
name="test_tool", | ||
arguments={"param": "value"}, | ||
**{"_meta": test_meta}, # Using alias | ||
) | ||
|
||
assert params_with_meta.name == "test_tool" | ||
assert params_with_meta.arguments == {"param": "value"} | ||
assert params_with_meta.meta is not None | ||
|
||
dumped = params_with_meta.meta.model_dump() | ||
assert dumped["user_id"] == "test_user" | ||
assert dumped["session_id"] == "test_session" | ||
assert dumped["custom_field"] == "custom_value" | ||
|
||
|
||
def test_metadata_serialization(): | ||
"""Test that metadata is properly serialized with _meta alias.""" | ||
from mcp.types import CallToolRequest, CallToolRequestParams, RequestParams | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. move imports to top of file |
||
|
||
meta_data = {"progressToken": None, "user_id": "alice", "api_key": "secret_123", "permissions": ["read", "write"]} | ||
test_meta = RequestParams.Meta.model_validate(meta_data) | ||
|
||
request = CallToolRequest( | ||
method="tools/call", | ||
params=CallToolRequestParams(name="secure_tool", arguments={"query": "sensitive_data"}, **{"_meta": test_meta}), | ||
) | ||
|
||
serialized = request.model_dump(by_alias=True) | ||
|
||
assert serialized["method"] == "tools/call" | ||
assert serialized["params"]["name"] == "secure_tool" | ||
assert serialized["params"]["arguments"]["query"] == "sensitive_data" | ||
|
||
assert "_meta" in serialized["params"] | ||
meta_data_serialized = serialized["params"]["_meta"] | ||
assert meta_data_serialized["user_id"] == "alice" | ||
assert meta_data_serialized["api_key"] == "secret_123" | ||
assert meta_data_serialized["permissions"] == ["read", "write"] | ||
assert meta_data["user_id"] == "alice" | ||
assert meta_data["api_key"] == "secret_123" | ||
assert meta_data["permissions"] == ["read", "write"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add unit tests for this