diff --git a/.sampo/changesets/mcp-virtual-tool-conversation-id.md b/.sampo/changesets/mcp-virtual-tool-conversation-id.md new file mode 100644 index 000000000..91610c5bd --- /dev/null +++ b/.sampo/changesets/mcp-virtual-tool-conversation-id.md @@ -0,0 +1,5 @@ +--- +pypi/posthog: patch +--- + +MCP virtual tools now use conversation IDs when `enable_conversation_id` is enabled. diff --git a/posthog/mcp/README.md b/posthog/mcp/README.md index a8c8dd168..ea8399511 100644 --- a/posthog/mcp/README.md +++ b/posthog/mcp/README.md @@ -299,6 +299,9 @@ needs no middleware and no ordering discipline, and it is the only thing that correlates a session under the 2026-07-28 revision's per-request server instances. Prefer it if you're on a recent client. +The `get_more_tools` and `send_feedback` virtual tools also use the conversation +handle when this option is enabled. + ### How the SDK tells you it's misconfigured The failure used to be silent. It now surfaces two ways: diff --git a/posthog/mcp/_conversation_id.py b/posthog/mcp/_conversation_id.py index b1e024b3b..f6fcafb88 100644 --- a/posthog/mcp/_conversation_id.py +++ b/posthog/mcp/_conversation_id.py @@ -74,33 +74,13 @@ def extract_conversation_id(args: Any) -> Optional[str]: return trimmed or None -def resolve_conversation_id( - enabled: bool, - args: Any, - tool_name: Optional[str], - missing_capability_tool_name: Optional[str], - feedback_tool_name: Optional[str] = None, -) -> Tuple[Optional[str], bool]: - """Return ``(conversation_id, minted)``. Disabled, get_more_tools, or - send_feedback → ``(None, False)``; agent echoed a handle we could have minted - → ``(value, False)``; anything else (omitted, or a value the agent made up) - → ``(new uuid, True)``. - - Either virtual tool's name arrives as ``None`` when that tool is disabled, - so a real application tool by the same name mints and echoes a handle like - any other tool's. +def resolve_conversation_id(enabled: bool, args: Any) -> Tuple[Optional[str], bool]: + """Return the conversation id and whether the SDK minted it. Lowercased on the way in: the shape test is case-insensitive but the hash behind ``$session_id`` is not, so an uppercased echo (some hosts normalise uuids) would land in a different session than the call that minted it.""" - if ( - not enabled - or ( - missing_capability_tool_name is not None - and tool_name == missing_capability_tool_name - ) - or (feedback_tool_name is not None and tool_name == feedback_tool_name) - ): + if not enabled: return None, False supplied = extract_conversation_id(args) if supplied and _MINTED_CONVERSATION_ID.match(supplied): diff --git a/posthog/mcp/_instrument_fastmcp.py b/posthog/mcp/_instrument_fastmcp.py index 485a168c7..7f9af6f15 100644 --- a/posthog/mcp/_instrument_fastmcp.py +++ b/posthog/mcp/_instrument_fastmcp.py @@ -118,14 +118,19 @@ async def wrapped( if lifecycle.is_missing_capability and ( _name_owned_by_real_tool(server, name) is False ): - await lifecycle.record_missing_capability() - return [ - mcp_types.TextContent(type="text", text=get_more_tools_result_text()) + virtual_content = [ + mcp_types.TextContent(type="text", text=text) + for text in lifecycle.virtual_result_texts(get_more_tools_result_text()) ] + await lifecycle.record_missing_capability(conversation_id_delivered=True) + return virtual_content if lifecycle.is_feedback and (_name_owned_by_real_tool(server, name) is False): - reply = await lifecycle.record_feedback() - return [mcp_types.TextContent(type="text", text=reply)] + reply = await lifecycle.record_feedback(conversation_id_delivered=True) + return [ + mcp_types.TextContent(type="text", text=text) + for text in lifecycle.virtual_result_texts(reply) + ] # Strip each injected key independently. A tool can declare its own # `context` (kept) while `conversation_id` is still SDK-injected (stripped), diff --git a/posthog/mcp/_instrument_lowlevel.py b/posthog/mcp/_instrument_lowlevel.py index 59a1b2703..30722496f 100644 --- a/posthog/mcp/_instrument_lowlevel.py +++ b/posthog/mcp/_instrument_lowlevel.py @@ -212,14 +212,14 @@ async def handler(req: Any) -> Any: if lifecycle.is_missing_capability and ( await _name_owned_by_real_tool(high_level, data, name, server) is False ): - await lifecycle.record_missing_capability() + virtual_content = [ + mcp_types.TextContent(type="text", text=text) + for text in lifecycle.virtual_result_texts(get_more_tools_result_text()) + ] + await lifecycle.record_missing_capability(conversation_id_delivered=True) return mcp_types.ServerResult( mcp_types.CallToolResult( - content=[ - mcp_types.TextContent( - type="text", text=get_more_tools_result_text() - ) - ], + content=virtual_content, isError=False, ) ) @@ -227,10 +227,14 @@ async def handler(req: Any) -> Any: if lifecycle.is_feedback and ( await _name_owned_by_real_tool(high_level, data, name, server) is False ): - reply = await lifecycle.record_feedback() + reply = await lifecycle.record_feedback(conversation_id_delivered=True) + virtual_content = [ + mcp_types.TextContent(type="text", text=text) + for text in lifecycle.virtual_result_texts(reply) + ] return mcp_types.ServerResult( mcp_types.CallToolResult( - content=[mcp_types.TextContent(type="text", text=reply)], + content=virtual_content, isError=False, ) ) diff --git a/posthog/mcp/_instrument_v2.py b/posthog/mcp/_instrument_v2.py index 857261d9f..716fc1460 100644 --- a/posthog/mcp/_instrument_v2.py +++ b/posthog/mcp/_instrument_v2.py @@ -302,22 +302,22 @@ async def wrapped( if lifecycle.is_missing_capability and ( _name_owned_by_real_tool_v2(server, name) is False ): - await lifecycle.record_missing_capability() - return mcp_types.CallToolResult( - content=[ - mcp_types.TextContent( - type="text", text=get_more_tools_result_text() - ) - ] - ) + virtual_content = [ + mcp_types.TextContent(type="text", text=text) + for text in lifecycle.virtual_result_texts(get_more_tools_result_text()) + ] + await lifecycle.record_missing_capability(conversation_id_delivered=True) + return mcp_types.CallToolResult(content=virtual_content) if lifecycle.is_feedback and ( _name_owned_by_real_tool_v2(server, name) is False ): - reply = await lifecycle.record_feedback() - return mcp_types.CallToolResult( - content=[mcp_types.TextContent(type="text", text=reply)] - ) + reply = await lifecycle.record_feedback(conversation_id_delivered=True) + virtual_content = [ + mcp_types.TextContent(type="text", text=text) + for text in lifecycle.virtual_result_texts(reply) + ] + return mcp_types.CallToolResult(content=virtual_content) # v2 validates against the function signature and rejects unexpected # keys, so injected parameters are stripped before dispatch — but never @@ -526,22 +526,22 @@ async def handler(ctx: Any, params: Any) -> Any: if lifecycle.is_missing_capability and ( await raw_listing_owns_tool_name(data, name, ctx) is False ): - await lifecycle.record_missing_capability() - return mcp_types.CallToolResult( - content=[ - mcp_types.TextContent( - type="text", text=get_more_tools_result_text() - ) - ] - ) + virtual_content = [ + mcp_types.TextContent(type="text", text=text) + for text in lifecycle.virtual_result_texts(get_more_tools_result_text()) + ] + await lifecycle.record_missing_capability(conversation_id_delivered=True) + return mcp_types.CallToolResult(content=virtual_content) if lifecycle.is_feedback and ( await raw_listing_owns_tool_name(data, name, ctx) is False ): - reply = await lifecycle.record_feedback() - return mcp_types.CallToolResult( - content=[mcp_types.TextContent(type="text", text=reply)] - ) + reply = await lifecycle.record_feedback(conversation_id_delivered=True) + virtual_content = [ + mcp_types.TextContent(type="text", text=text) + for text in lifecycle.virtual_result_texts(reply) + ] + return mcp_types.CallToolResult(content=virtual_content) # Settle the shared session before the tool body runs, so an in-tool # `analytics.capture()` is attributed to this caller and not the last one. diff --git a/posthog/mcp/_instrumentation.py b/posthog/mcp/_instrumentation.py index 48a0fd36d..f39afb0a0 100644 --- a/posthog/mcp/_instrumentation.py +++ b/posthog/mcp/_instrumentation.py @@ -23,7 +23,11 @@ is_context_enabled, schema_has_param, ) -from ._conversation_id import add_conversation_id_to_schema, resolve_conversation_id +from ._conversation_id import ( + add_conversation_id_to_schema, + build_prompt_back, + resolve_conversation_id, +) from ._event_types import MCPAnalyticsEventType from ._exceptions import capture_exception from .feedback import ( @@ -470,11 +474,26 @@ async def prime_session(self) -> None: self.data, mcp_session_id=self.mcp_session_id, token=self.token ) - async def record_missing_capability(self) -> None: - session_id = await self.prepare_session(None) + def virtual_result_texts(self, primary_text: str) -> List[str]: + """Build the text payload for an SDK virtual-tool result.""" + if not self.conversation_id or not self.minted_conversation_id: + return [primary_text] + return [primary_text, build_prompt_back(self.conversation_id)["text"]] + + def _anchored_conversation_id(self, delivered: bool) -> Optional[str]: + if self.minted_conversation_id and not delivered: + return None + return self.conversation_id + + async def record_missing_capability( + self, *, conversation_id_delivered: bool = False + ) -> None: + conversation_id = self._anchored_conversation_id(conversation_id_delivered) + session_id = await self.prepare_session(conversation_id) await record_missing_capability( self.data, session_id, + conversation_id=conversation_id, tool_name=self.missing_name or self.name, context=(self.arguments or {}).get("context"), arguments=self.arguments, @@ -486,15 +505,17 @@ async def record_missing_capability(self) -> None: extra=self.extra, ) - async def record_feedback(self) -> str: + async def record_feedback(self, *, conversation_id_delivered: bool = False) -> str: """Capture the ``$mcp_feedback`` event, then run the host's ``on_feedback`` handler and return the reply text for the agent. The event is captured whether or not the handler raises.""" report = parse_feedback_report(self.arguments, self.feedback_options) - session_id = await self.prepare_session(None) + conversation_id = self._anchored_conversation_id(conversation_id_delivered) + session_id = await self.prepare_session(conversation_id) await record_feedback( self.data, session_id, + conversation_id=conversation_id, report=report, tool_name=self.feedback_name or self.name, arguments=self.arguments, @@ -509,7 +530,7 @@ async def record_feedback(self) -> str: async def record_error(self, error: Any, duration_ms: float) -> None: # A freshly minted handle cannot anchor or be captured when dispatch # raised: no adapter had an opportunity to deliver it to the agent. - conversation_id = None if self.minted_conversation_id else self.conversation_id + conversation_id = self._anchored_conversation_id(False) session_id = await self.prepare_session(conversation_id) await record_tool_call( self.data, @@ -530,9 +551,7 @@ async def record_error(self, error: Any, duration_ms: float) -> None: async def record_result( self, result: Any, duration_ms: float, *, conversation_id_delivered: bool ) -> None: - conversation_id = self.conversation_id - if self.minted_conversation_id and not conversation_id_delivered: - conversation_id = None + conversation_id = self._anchored_conversation_id(conversation_id_delivered) session_id = await self.prepare_session(conversation_id) await record_tool_call( self.data, @@ -573,11 +592,7 @@ def start_tool_call_lifecycle( # running the host's `on_feedback` handler read the configured options. feedback_options = resolve_collect_feedback_options(data.options.collect_feedback) conversation_id, minted = resolve_conversation_id( - data.options.enable_conversation_id, - arguments, - name, - missing_name, - feedback_name, + data.options.enable_conversation_id, arguments ) return ToolCallLifecycle( data=data, @@ -1010,10 +1025,8 @@ def mutate_tool_schema( data.tool_model_parameter_injected[tool.name] = ( not app_owns_model and schema_has_param(schema, "llm_model") ) - if ( - not is_sdk_virtual_tool - and data.options.enable_conversation_id - and not schema_has_param(schema, "conversation_id") + if data.options.enable_conversation_id and not schema_has_param( + schema, "conversation_id" ): schema = add_conversation_id_to_schema(schema, tool.name) if schema is not original_schema: @@ -1138,6 +1151,7 @@ async def record_missing_capability( data: MCPAnalyticsData, session_id: str, *, + conversation_id: Optional[str] = None, tool_name: str, context: Optional[str], arguments: Optional[Dict[str, Any]], @@ -1155,6 +1169,7 @@ async def record_missing_capability( event: Dict[str, Any] = { "event_type": MCPAnalyticsEventType.MCP_MISSING_CAPABILITY, "session_id": session_id, + "conversation_id": conversation_id, "resource_name": tool_name, "parameters": build_captured_mcp_parameters( request, strip_llm_model=allow_self_reported_model @@ -1186,6 +1201,7 @@ async def record_feedback( data: MCPAnalyticsData, session_id: str, *, + conversation_id: Optional[str] = None, report: FeedbackReport, tool_name: str, arguments: Optional[Dict[str, Any]], @@ -1206,6 +1222,7 @@ async def record_feedback( event: Dict[str, Any] = { "event_type": MCPAnalyticsEventType.MCP_FEEDBACK, "session_id": session_id, + "conversation_id": conversation_id, "resource_name": tool_name, "client_name": client_name, "client_version": client_version, diff --git a/posthog/test/mcp/test_conversation_session.py b/posthog/test/mcp/test_conversation_session.py index 037888a6f..0e1c04dad 100644 --- a/posthog/test/mcp/test_conversation_session.py +++ b/posthog/test/mcp/test_conversation_session.py @@ -62,7 +62,7 @@ def test_derivation_is_deterministic_and_distinct(): def test_echo_of_a_mintable_handle_is_accepted(): cid, minted = resolve_conversation_id( - True, {"conversation_id": MINTED_SHAPE_HANDLE}, "t", "get_more_tools" + True, {"conversation_id": MINTED_SHAPE_HANDLE} ) assert minted is False assert cid == MINTED_SHAPE_HANDLE @@ -73,7 +73,7 @@ def test_uppercased_echo_is_lowercased_before_hashing(): # case-sensitive, so the echo must be folded back or it lands in a # different session than the call that minted it. cid, minted = resolve_conversation_id( - True, {"conversation_id": MINTED_SHAPE_HANDLE.upper()}, "t", "get_more_tools" + True, {"conversation_id": MINTED_SHAPE_HANDLE.upper()} ) assert minted is False assert cid == MINTED_SHAPE_HANDLE @@ -82,9 +82,7 @@ def test_uppercased_echo_is_lowercased_before_hashing(): def test_invented_handle_is_not_anchored(): # Two unrelated users both sending "conv-1" must NOT share a session, so a # value we could not have minted is replaced with a fresh handle. - cid, minted = resolve_conversation_id( - True, {"conversation_id": "conv-1"}, "t", "get_more_tools" - ) + cid, minted = resolve_conversation_id(True, {"conversation_id": "conv-1"}) assert minted is True assert cid != "conv-1" diff --git a/posthog/test/mcp/test_fastmcp.py b/posthog/test/mcp/test_fastmcp.py index e15391745..63eb0b0fc 100644 --- a/posthog/test/mcp/test_fastmcp.py +++ b/posthog/test/mcp/test_fastmcp.py @@ -85,6 +85,28 @@ async def test_list_tools_injects_model_into_real_and_virtual_tools(): assert "llm_model" in tools[name].inputSchema["required"] +async def test_virtual_tool_uses_conversation_id(): + server = make_server() + client = FakeClient() + instrument( + server, + client, + MCPAnalyticsOptions(report_missing=True, enable_conversation_id=True), + ) + + listed = await _list_tools(server) + tool = next(t for t in listed.root.tools if t.name == "get_more_tools") + assert "conversation_id" in tool.inputSchema["properties"] + + result = await server._tool_manager.call_tool("get_more_tools", {"context": "csv"}) + await _flush() + + handle = _events(client, "$mcp_missing_capability")[0]["properties"][ + "$mcp_conversation_id" + ] + assert any(handle in item.text for item in result) + + # --- tools/call -------------------------------------------------------------- diff --git a/posthog/test/mcp/test_feedback.py b/posthog/test/mcp/test_feedback.py index e418baaa9..3e016299a 100644 --- a/posthog/test/mcp/test_feedback.py +++ b/posthog/test/mcp/test_feedback.py @@ -813,7 +813,7 @@ async def test_collision_warning_is_logged_once_across_repeated_listings(): assert len([m for m in messages if "Cannot inject PostHog's" in m]) == 1 -async def test_feedback_never_mints_conversation_id(): +async def test_feedback_mints_conversation_id(): server = make_lowlevel() client = FakeClient() instrument( @@ -824,15 +824,49 @@ async def test_feedback_never_mints_conversation_id(): result = await _list_tools_lowlevel(server) virtual = [t for t in result.root.tools if t.name == "send_feedback"][0] - assert "conversation_id" not in virtual.inputSchema["properties"] + assert "conversation_id" in virtual.inputSchema["properties"] call_handler = server.request_handlers[mcp_types.CallToolRequest] out = await call_handler(_call_request("send_feedback", dict(_REPORT_ARGS))) await _flush() feedback = _events(client, "$mcp_feedback") - assert "$mcp_conversation_id" not in feedback[0]["properties"] - # No prompt-back block appended to the acknowledgement. + handle = feedback[0]["properties"]["$mcp_conversation_id"] + assert handle in out.root.content[1].text + + +async def test_renamed_feedback_reuses_conversation_id(): + server = make_lowlevel() + client = FakeClient() + instrument( + server, + client, + MCPAnalyticsOptions( + collect_feedback=CollectFeedbackOptions(tool_name="report_feedback"), + enable_conversation_id=True, + ), + ) + + result = await _list_tools_lowlevel(server) + virtual = [t for t in result.root.tools if t.name == "report_feedback"][0] + assert "conversation_id" in virtual.inputSchema["properties"] + + handle = "0198d3a7-1111-7222-8333-444455556666" + call_handler = server.request_handlers[mcp_types.CallToolRequest] + await call_handler(_call_request("echo", {"msg": "hi", "conversation_id": handle})) + out = await call_handler( + _call_request( + "report_feedback", {**dict(_REPORT_ARGS), "conversation_id": handle} + ) + ) + await _flush() + + tool_call = _events(client, "$mcp_tool_call")[0] + feedback = _events(client, "$mcp_feedback")[0] + assert feedback["properties"]["$mcp_conversation_id"] == handle + assert ( + tool_call["properties"]["$session_id"] == feedback["properties"]["$session_id"] + ) assert len(out.root.content) == 1 diff --git a/posthog/test/mcp/test_units.py b/posthog/test/mcp/test_units.py index 663f88cce..59397c949 100644 --- a/posthog/test/mcp/test_units.py +++ b/posthog/test/mcp/test_units.py @@ -282,21 +282,11 @@ def test_extract_conversation_id(): def test_resolve_conversation_id_disabled(): - assert resolve_conversation_id(False, {}, "t", "get_more_tools") == (None, False) + assert resolve_conversation_id(False, {}) == (None, False) -def test_resolve_conversation_id_skips_missing_capability_tool(): - assert resolve_conversation_id(True, {}, "get_more_tools", "get_more_tools") == ( - None, - False, - ) - - -def test_resolve_conversation_id_mints_for_a_shadowed_virtual_tool_name(): - # `None` means the virtual tool is disabled or a real application tool owns - # the name. Either way the call belongs to that real tool, so it mints and - # echoes a handle like any other tool's. - cid, minted = resolve_conversation_id(True, {}, "get_more_tools", None) +def test_resolve_conversation_id_applies_to_virtual_tools(): + cid, minted = resolve_conversation_id(True, {}) assert minted is True and cid @@ -305,20 +295,19 @@ def test_resolve_conversation_id_uses_supplied_when_mintable_shape(): # the handle becomes $session_id, so an invented value ("conv-1") must not # anchor two unrelated callers to one session (parity with posthog-js). handle = "0198d3a7-1111-7222-8333-444455556666" - assert resolve_conversation_id( - True, {"conversation_id": handle}, "t", "get_more_tools" - ) == (handle, False) + assert resolve_conversation_id(True, {"conversation_id": handle}) == ( + handle, + False, + ) def test_resolve_conversation_id_replaces_invented_values(): - cid, minted = resolve_conversation_id( - True, {"conversation_id": "conv-1"}, "t", "get_more_tools" - ) + cid, minted = resolve_conversation_id(True, {"conversation_id": "conv-1"}) assert minted is True and cid != "conv-1" def test_resolve_conversation_id_mints_when_absent(): - cid, minted = resolve_conversation_id(True, {}, "t", "get_more_tools") + cid, minted = resolve_conversation_id(True, {}) assert minted is True and isinstance(cid, str) and cid diff --git a/posthog/test/mcp/test_v2_lowlevel.py b/posthog/test/mcp/test_v2_lowlevel.py index 9b9142725..6bb0091a8 100644 --- a/posthog/test/mcp/test_v2_lowlevel.py +++ b/posthog/test/mcp/test_v2_lowlevel.py @@ -478,11 +478,17 @@ async def test_instrument_is_idempotent(): async def test_report_missing_appends_virtual_tool(): server = make_server() client = FakeClient() - instrument(server, client, MCPAnalyticsOptions(report_missing=True)) + instrument( + server, + client, + MCPAnalyticsOptions(report_missing=True, enable_conversation_id=True), + ) result = await _list_tools(server) names = [t.name for t in result.tools] assert "get_more_tools" in names + virtual = next(t for t in result.tools if t.name == "get_more_tools") + assert "conversation_id" in virtual.input_schema["properties"] call_result = await _call_tool( server, "get_more_tools", {"context": "need an email tool"} @@ -492,15 +498,23 @@ async def test_report_missing_appends_virtual_tool(): assert call_result.is_error is False missing = _events(client, "$mcp_missing_capability") assert missing and missing[0]["properties"]["$mcp_intent"] == "need an email tool" + handle = missing[0]["properties"]["$mcp_conversation_id"] + assert handle in call_result.content[1].text async def test_collect_feedback_appends_virtual_tool(): server = make_server() client = FakeClient() - instrument(server, client, MCPAnalyticsOptions(collect_feedback=True)) + instrument( + server, + client, + MCPAnalyticsOptions(collect_feedback=True, enable_conversation_id=True), + ) result = await _list_tools(server) assert "send_feedback" in [t.name for t in result.tools] + virtual = next(t for t in result.tools if t.name == "send_feedback") + assert "conversation_id" in virtual.input_schema["properties"] call_result = await _call_tool( server, @@ -514,6 +528,8 @@ async def test_collect_feedback_appends_virtual_tool(): assert len(feedback) == 1 assert feedback[0]["properties"]["$mcp_feedback_type"] == "issue" assert "$mcp_parameters" not in feedback[0]["properties"] + handle = feedback[0]["properties"]["$mcp_conversation_id"] + assert handle in call_result.content[1].text assert _events(client, "$mcp_tool_call") == [] diff --git a/posthog/test/mcp/test_v2_mcpserver.py b/posthog/test/mcp/test_v2_mcpserver.py index d39c6162a..0d6187d8f 100644 --- a/posthog/test/mcp/test_v2_mcpserver.py +++ b/posthog/test/mcp/test_v2_mcpserver.py @@ -348,11 +348,17 @@ def totals(event: str) -> dict[str, Any]: async def test_report_missing_advertises_and_captures(): server = make_server() client = FakeClient() - instrument(server, client, MCPAnalyticsOptions(report_missing=True)) + instrument( + server, + client, + MCPAnalyticsOptions(report_missing=True, enable_conversation_id=True), + ) listed = await _list_tools(server) names = [t.name for t in listed.tools] assert "get_more_tools" in names + virtual = next(t for t in listed.tools if t.name == "get_more_tools") + assert "conversation_id" in virtual.input_schema["properties"] result = await _call_tool( server, "get_more_tools", {"context": "need a tool to send emails"} @@ -363,6 +369,8 @@ async def test_report_missing_advertises_and_captures(): missing = _events(client, "$mcp_missing_capability") assert missing assert missing[0]["properties"]["$mcp_intent"] == "need a tool to send emails" + handle = missing[0]["properties"]["$mcp_conversation_id"] + assert handle in result.content[1].text async def test_report_missing_accepts_omitted_arguments(): @@ -404,10 +412,16 @@ def get_more_tools(context: str) -> str: async def test_collect_feedback_advertises_and_captures(): server = make_server() client = FakeClient() - instrument(server, client, MCPAnalyticsOptions(collect_feedback=True)) + instrument( + server, + client, + MCPAnalyticsOptions(collect_feedback=True, enable_conversation_id=True), + ) listed = await _list_tools(server) assert "send_feedback" in [t.name for t in listed.tools] + virtual = next(t for t in listed.tools if t.name == "send_feedback") + assert "conversation_id" in virtual.input_schema["properties"] result = await _call_tool( server, @@ -427,6 +441,8 @@ async def test_collect_feedback_advertises_and_captures(): assert props["$mcp_feedback_type"] == "missing_capability" assert props["$mcp_intent"] == "No email tool.\n\nWanted to notify a teammate." assert "$mcp_parameters" not in props + handle = props["$mcp_conversation_id"] + assert handle in result.content[1].text # A send_feedback call is NOT a normal tool call. assert _events(client, "$mcp_tool_call") == [] diff --git a/posthog/test/mcp/test_virtual_tools.py b/posthog/test/mcp/test_virtual_tools.py index 84481f9ca..978471a0e 100644 --- a/posthog/test/mcp/test_virtual_tools.py +++ b/posthog/test/mcp/test_virtual_tools.py @@ -221,8 +221,6 @@ async def test_both_virtual_tools_configured_with_the_same_name(): @pytest.mark.parametrize("enable_conversation_id", [False, True]) async def test_renamed_tool_carries_its_own_intent(enable_conversation_id): - # A virtual tool states its intent in its own `context` argument, so it gets - # neither an injected `context` nor a `conversation_id` -- renamed or not. server = make_paged_lowlevel([[_ECHO_TOOL]]) instrument( server, @@ -236,10 +234,40 @@ async def test_renamed_tool_carries_its_own_intent(enable_conversation_id): page = await _list_page(server) virtual = [tool for tool in page.root.tools if tool.name == "find_tools"][0] - assert list(virtual.inputSchema["properties"]) == ["context"] + expected = {"context", "conversation_id"} if enable_conversation_id else {"context"} + assert set(virtual.inputSchema["properties"]) == expected assert virtual.inputSchema["required"] == ["context"] +async def test_renamed_tool_uses_the_conversation_id(): + server = make_paged_lowlevel([[_ECHO_TOOL]]) + client = FakeClient() + instrument( + server, + client, + MCPAnalyticsOptions( + report_missing=True, + missing_capability_tool_name="find_tools", + enable_conversation_id=True, + ), + ) + await _list_page(server) + + handle = "0198d3a7-1111-7222-8333-444455556666" + await _call(server, "echo", {"msg": "hi", "conversation_id": handle}) + await _call( + server, "find_tools", {"context": "need csv", "conversation_id": handle} + ) + await _flush() + + tool_call = _events(client, "$mcp_tool_call")[0] + missing = _events(client, "$mcp_missing_capability")[0] + assert missing["properties"]["$mcp_conversation_id"] == handle + assert ( + tool_call["properties"]["$session_id"] == missing["properties"]["$session_id"] + ) + + async def test_renamed_tool_is_intercepted_and_the_default_name_is_not(): server = make_paged_lowlevel([[_ECHO_TOOL]]) client = FakeClient()