diff --git a/.sampo/changesets/mcp-tools-list-envelope.md b/.sampo/changesets/mcp-tools-list-envelope.md new file mode 100644 index 000000000..89b988977 --- /dev/null +++ b/.sampo/changesets/mcp-tools-list-envelope.md @@ -0,0 +1,5 @@ +--- +pypi/posthog: patch +--- + +`$mcp_tools_list` events no longer copy the tool descriptors into `$mcp_response`, which keeps only the response envelope such as `nextCursor`. The tool names stay in `$mcp_listed_tool_names`. diff --git a/posthog/mcp/_instrument_fastmcp.py b/posthog/mcp/_instrument_fastmcp.py index 7f9af6f15..e0fdd1c48 100644 --- a/posthog/mcp/_instrument_fastmcp.py +++ b/posthog/mcp/_instrument_fastmcp.py @@ -29,7 +29,6 @@ from ._conversation_id import build_prompt_back from ._instrument_lowlevel import _wrap_resource_requests from ._instrumentation import ( - _to_jsonable, apply_virtual_tool_injection, collect_listed_tools, extract_tools, @@ -288,7 +287,7 @@ async def list_handler(req: Any) -> Any: await lifecycle.record_result( names=names, - response=_to_jsonable(result), + result=result, duration_ms=duration_ms, is_empty=empty, ) diff --git a/posthog/mcp/_instrument_lowlevel.py b/posthog/mcp/_instrument_lowlevel.py index ee24748aa..c12dab787 100644 --- a/posthog/mcp/_instrument_lowlevel.py +++ b/posthog/mcp/_instrument_lowlevel.py @@ -26,7 +26,6 @@ from ._conversation_id import build_prompt_back from ._event_types import MCPAnalyticsEventType from ._instrumentation import ( - _to_jsonable, advertised_tool_names, apply_virtual_tool_injection, collect_listed_tools, @@ -490,7 +489,7 @@ async def handler(req: Any) -> Any: await lifecycle.record_result( names=names, - response=_to_jsonable(result), + result=result, duration_ms=duration_ms, is_empty=empty, ) diff --git a/posthog/mcp/_instrument_v2.py b/posthog/mcp/_instrument_v2.py index b6d7ecd99..68c273556 100644 --- a/posthog/mcp/_instrument_v2.py +++ b/posthog/mcp/_instrument_v2.py @@ -40,7 +40,6 @@ from ._conversation_id import build_prompt_back from ._event_types import MCPAnalyticsEventType from ._instrumentation import ( - _to_jsonable, advertised_tool_names, apply_virtual_tool_injection, collect_listed_tools, @@ -723,7 +722,7 @@ async def handler(ctx: Any, params: Any) -> Any: await lifecycle.record_result( names=names, - response=_to_jsonable(result), + result=result, duration_ms=duration_ms, is_empty=empty, ) diff --git a/posthog/mcp/_instrumentation.py b/posthog/mcp/_instrumentation.py index 2355bcdc6..cbf50ab48 100644 --- a/posthog/mcp/_instrumentation.py +++ b/posthog/mcp/_instrumentation.py @@ -692,6 +692,24 @@ def extract_tools(result: Any) -> list: return list(getattr(root, "tools", []) or []) +def tools_list_envelope(result: Any) -> Optional[Dict[str, Any]]: + """The ``tools/list`` result minus its tools, or None when nothing else is set. + + The names already ride ``listed_tool_names``, and a descriptor copy would be + sanitized and truncated on the request path.""" + root = getattr(result, "root", result) + if not hasattr(root, "model_dump"): + return None + envelope = root.model_dump( + mode="json", + by_alias=True, + exclude={"tools"}, + exclude_unset=True, + exclude_none=True, + ) + return envelope or None + + def append_virtual_tool(result: Any, tool: Any) -> Any: """Return a copy of a ``tools/list`` result with ``tool`` added. @@ -1113,7 +1131,7 @@ async def record_result( self, *, names: List[str], - response: Any, + result: Any, duration_ms: float, is_empty: bool, ) -> None: @@ -1122,7 +1140,7 @@ async def record_result( self.session_id, names=names, request=self.request, - response=response, + result=result, duration_ms=duration_ms, is_error=is_empty, error="tools/list returned no tools" if is_empty else None, @@ -1276,7 +1294,7 @@ async def record_tools_list( *, names: List[str], request: Dict[str, Any], - response: Any = None, + result: Any = None, duration_ms: Optional[float] = None, is_error: bool = False, error: Any = None, @@ -1291,7 +1309,7 @@ async def record_tools_list( "session_id": session_id, "listed_tool_names": names, "parameters": build_captured_mcp_parameters(request), - "response": _wrap_response(response) if response is not None else None, + "response": tools_list_envelope(result), "duration": duration_ms, "client_name": client_name, "client_version": client_version, diff --git a/posthog/test/mcp/test_review_fixes.py b/posthog/test/mcp/test_review_fixes.py index 8c2886ff1..c7682e402 100644 --- a/posthog/test/mcp/test_review_fixes.py +++ b/posthog/test/mcp/test_review_fixes.py @@ -2,7 +2,7 @@ - B: a tool that declares its own ``context`` keeps it while an injected ``conversation_id`` is stripped (the two are decoupled). -- E: ``tools/list`` captures ``$mcp_response`` + ``$mcp_duration_ms``, and a +- E: ``tools/list`` captures ``$mcp_duration_ms`` (not the listing), and a list handler that raises is captured as an errored ``$mcp_tools_list``. - F: ``$mcp_initialize`` is emitted on a ``tools/list`` (a client may list but never call a tool). @@ -21,6 +21,11 @@ from posthog.mcp import PostHogMCP, instrument from posthog.mcp.types import MCPAnalyticsOptions, UserIdentity +from posthog.test.mcp._helpers_lowlevel import ( + list_page, + make_paged_lowlevel, + tool_names, +) from posthog.test.mcp._helpers import ( FakeClient, events_named as _events, @@ -99,7 +104,7 @@ def summarize(text: str, context: str) -> str: # --- E: tools/list response + duration, and failure capture ------------------- -async def test_tools_list_captures_response_and_duration(): +async def test_tools_list_captures_duration_without_response(): server = make_lowlevel() client = FakeClient() instrument(server, client) @@ -110,11 +115,40 @@ async def test_tools_list_captures_response_and_duration(): listed = _events(client, "$mcp_tools_list") assert listed props = listed[0]["properties"] - assert props["$mcp_response"] is not None + assert "$mcp_response" not in props assert "$mcp_duration_ms" in props assert props["$mcp_is_error"] is False +@pytest.mark.parametrize( + "pages, expected_response", + [ + ([["alpha", "beta"], ["gamma"]], {"nextCursor": "1"}), + ([["alpha", "beta"]], None), + ], + ids=["paginated", "single-page"], +) +async def test_tools_list_response_is_the_envelope_without_tools( + pages, expected_response +): + server = make_paged_lowlevel( + [ + [mcp_types.Tool(name=name, inputSchema={"type": "object"}) for name in page] + for page in pages + ] + ) + client = FakeClient() + instrument(server, client) + + result = await list_page(server) + await _flush() + + assert tool_names(result) == pages[0] + props = _events(client, "$mcp_tools_list")[0]["properties"] + assert props["$mcp_listed_tool_names"] == pages[0] + assert props.get("$mcp_response") == expected_response + + async def test_tools_list_handler_raise_is_captured(): server = Server("list-raises") diff --git a/posthog/test/mcp/test_units.py b/posthog/test/mcp/test_units.py index 2fff89e6a..c07c85ab6 100644 --- a/posthog/test/mcp/test_units.py +++ b/posthog/test/mcp/test_units.py @@ -5,6 +5,9 @@ from datetime import datetime, timedelta, timezone from types import SimpleNamespace +import pytest +import mcp.types as mcp_types + from posthog.mcp._conversation_id import ( add_conversation_id_to_schema, can_inject_prompt_back, @@ -19,6 +22,7 @@ is_first_listing_page, mutate_tool_schema, resolve_virtual_tool_injection, + tools_list_envelope, ) from posthog.mcp._intent import _get_context_argument, resolve_tool_call_intent from posthog.mcp._internal import ( @@ -408,3 +412,29 @@ def test_identity_cache_evicts_least_recently_used(): cache.get("s1") # touch s1 so s2 becomes the LRU entry cache.set("s3", UserIdentity(distinct_id="u3")) # evicts s2 assert cache.has("s1") and cache.has("s3") and not cache.has("s2") + + +@pytest.mark.parametrize( + "result, expected", + [ + ( + mcp_types.ListToolsResult( + tools=[mcp_types.Tool(name="a", inputSchema={"type": "object"})], + nextCursor="2", + ), + {"nextCursor": "2"}, + ), + ( + mcp_types.ListToolsResult.model_validate( + {"tools": [], "nextCursor": "2", "_meta": {"trace": "t"}} + ), + {"nextCursor": "2", "_meta": {"trace": "t"}}, + ), + (mcp_types.ListToolsResult(tools=[]), None), + (None, None), + ([mcp_types.Tool(name="a", inputSchema={"type": "object"})], None), + ], + ids=["paginated", "meta", "tools-only", "none", "bare-list"], +) +def test_tools_list_envelope(result, expected): + assert tools_list_envelope(result) == expected diff --git a/posthog/test/mcp/test_v2_lowlevel.py b/posthog/test/mcp/test_v2_lowlevel.py index 251cc9865..d923be04d 100644 --- a/posthog/test/mcp/test_v2_lowlevel.py +++ b/posthog/test/mcp/test_v2_lowlevel.py @@ -159,6 +159,36 @@ async def test_list_tools_injects_optional_context_and_captures(): assert listed[0]["properties"]["$mcp_server_name"] == "test-low-v2" +@pytest.mark.parametrize( + "next_cursor, expected_response", + [("page-2", {"nextCursor": "page-2"}), (None, None)], + ids=["paginated", "single-page"], +) +async def test_list_tools_response_is_the_envelope_without_tools( + next_cursor, expected_response +): + async def on_list_tools(ctx, params): + return mcp_types.ListToolsResult( + tools=[ + mcp_types.Tool(name=name, input_schema={"type": "object"}) + for name in ("alpha", "beta") + ], + next_cursor=next_cursor, + ) + + server = Server("envelope-v2", on_list_tools=on_list_tools) + client = FakeClient() + instrument(server, client) + + result = await _list_tools(server) + await _flush() + + assert [t.name for t in result.tools][:2] == ["alpha", "beta"] + props = _events(client, "$mcp_tools_list")[0]["properties"] + assert props["$mcp_listed_tool_names"] == ["alpha", "beta"] + assert props.get("$mcp_response") == expected_response + + @pytest.mark.parametrize( "uri, captured_uri, resource_error", [