fix(mcp): decide tool ownership per request, not from the last listing

Capture intent for tools the host owns; skip it only for PostHog's own.
The code asked that question two different ways and they disagreed.

Interception asked a per-request probe, which was right. Intent and the
conversation exemption asked `virtual_tool_collisions`, which only a
served tools/list writes. On a process that had served none -- the
ordinary multi-pod case the probe exists for -- a host tool named
`get_more_tools` dispatched correctly but its $mcp_tool_call carried
`$mcp_intent=None`. Permanent on FastMCP and v2 MCPServer; on raw
low-level it self-healed after one call, because the MCP SDK's own
`req is None` cache pass happens to refresh the state during dispatch.

The intent guard turns out to be dead code for its stated purpose:
PostHog's virtual tools are intercepted before dispatch and captured by
record_missing_capability / record_feedback, so they never reach
record_tool_call. Verified by spying on it. Everything that gets there
is host-dispatched by construction, so the guard could only ever fire
on a host tool sharing the name. Removed.

The same applies to the conversation exemption: the virtual tools'
capture paths discard the resolved handle, so exempting by name only
ever cost the host's tool its handle.

start_tool_call_lifecycle now reads `enabled_virtual_tool_names`
instead of `injectable_`, so the call path stops reading listing state
altogether. That also closes the cross-client report: the collision set
is per-server and rewritten by whichever listing ran last, so reading
it on the call path let one caller's catalogue decide another caller's
call.

Note on the reports: the missing conversation id they also cite is not
name-specific -- an ordinary tool shows the same in that harness, so it
is not a regression here. The new tests assert parity with an ordinary
tool instead, which is the rule being fixed.

The changeset claimed a real `get_more_tools` "keeps its $mcp_intent",
which was only true after a listing on the same instance. Corrected.

Reported by QA Swarm and veria-ai on #962.

Generated-By: PostHog Desktop
Task-Id: 989fd424-fd24-4ae4-8682-548b87f761f6

Author: gesh

.sampo/changesets/mcp-virtual-tool-first-page.md

@@ -5,7 +5,8 @@ pypi/posthog: patch
 Fix MCP analytics virtual tool injection on a paginated `tools/list`. `get_more_tools` was added to every page and `send_feedback` only to the last; both now go on the first page, matching `@posthog/mcp`.
 
 - A real tool of yours named `get_more_tools` is no longer silently swallowed. It wins, and the warning names `missing_capability_tool_name`.
-- Custom tool names are honoured consistently: a renamed `get_more_tools` no longer gets a stray `conversation_id` argument, and a real `get_more_tools` of yours keeps its `context` injection and `$mcp_intent`.
+- Custom tool names are honoured consistently: a renamed `get_more_tools` no longer gets a stray `conversation_id` argument.
+- A real `get_more_tools` of yours is attributed like any other tool of yours, including its `$mcp_intent`. Attribution used to be decided from the last served `tools/list`, so on a process that had not served one — the ordinary multi-pod case — the intent was silently dropped.
 - A server that returns the same `tools/list` result object on every request no longer reads PostHog's own injected tool back as a name collision.
 - Collision warnings now reach the `posthog.mcp` logger too, so they are visible without setting the `logger` option.
 

posthog/mcp/_conversation_id.py

@@ -77,30 +77,27 @@ def extract_conversation_id(args: Any) -> Optional[str]:
 def resolve_conversation_id(
     enabled: bool,
     args: Any,
-    tool_name: Optional[str],
-    missing_capability_tool_name: Optional[str],
+    tool_name: Optional[str] = None,
+    missing_capability_tool_name: Optional[str] = None,
     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)``.
+    """Return ``(conversation_id, minted)``. Disabled → ``(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 it is disabled or when a
-    real application tool owns it. A shadowed name belongs to that real tool, so
-    it mints and echoes a handle like any other tool's.
+    No tool is exempt by name. A name-based exemption cannot tell PostHog's
+    virtual tool from a *host* tool that shares the name, and got it wrong in the
+    direction that matters: the host's tool lost its handle. PostHog's own
+    virtual tools discard the resolved handle on their own capture paths, so
+    resolving one for them costs nothing.
+
+    The three tool-name parameters are accepted and ignored for backwards
+    compatibility with callers that still pass them positionally.
 
     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):

posthog/mcp/_instrumentation.py

@@ -564,13 +564,14 @@ def start_tool_call_lifecycle(
     extra: Dict[str, Any],
 ) -> ToolCallLifecycle:
     """Resolve adapter-independent policy for a tool call without dispatching it."""
-    # A name a real application tool is known to own resolves to None here, so
-    # every downstream decision treats calls to it like any other tool's:
-    # interception is skipped, and conversation-id resolution stops exempting it
-    # as if it were the (shadowed) virtual tool.
-    injectable = injectable_virtual_tool_names(data)
-    missing_name = injectable.get(VIRTUAL_TOOL_MISSING_CAPABILITY)
-    feedback_name = injectable.get(VIRTUAL_TOOL_FEEDBACK)
+    # Options only -- deliberately NOT the listing-derived collision state.
+    # That state is per-server and rewritten by whichever listing ran last, so a
+    # server with caller-specific catalogues would answer one caller from
+    # another's listing. Ownership on this path is settled per request by the
+    # adapters' probes, which run right after this and are never stale.
+    enabled = enabled_virtual_tool_names(data)
+    missing_name = enabled.get(VIRTUAL_TOOL_MISSING_CAPABILITY)
+    feedback_name = enabled.get(VIRTUAL_TOOL_FEEDBACK)
     # Still needed whatever the name resolution says: parsing the report and
     # running the host's `on_feedback` handler read the configured options.
     feedback_options = resolve_collect_feedback_options(data.options.collect_feedback)

posthog/mcp/_intent.py

@@ -56,25 +56,16 @@ async def resolve_tool_call_intent(
     request: Dict[str, Any],
     extra: Optional[Dict[str, Any]] = None,
 ) -> Optional[ResolvedIntent]:
-    from ._instrumentation import (
-        VIRTUAL_TOOL_MISSING_CAPABILITY,
-        injectable_virtual_tool_names,
-    )
-
+    # Only tools the host actually dispatched get here. PostHog's own virtual
+    # tools are intercepted before dispatch and captured by
+    # `record_missing_capability` / `record_feedback`, which set the event's
+    # intent from their own arguments -- they never reach `record_tool_call`.
+    #
+    # So there is no virtual tool to exempt here, and a name-based exemption
+    # could only ever fire on a *host* tool that happens to share the name,
+    # silently dropping its `$mcp_intent`.
     context_argument = _get_context_argument(request)
-    name = (request.get("params") or {}).get("name")
-    # The virtual tool carries its intent in its own `context` argument, which
-    # is captured as the event's own field rather than as `$mcp_intent`. Resolved
-    # through the injectable map, so a *real* application tool that owns the name
-    # keeps its `context` captured as intent like any other tool's.
-    missing_name = injectable_virtual_tool_names(data).get(
-        VIRTUAL_TOOL_MISSING_CAPABILITY
-    )
-    if (
-        is_context_enabled(data.options.context)
-        and (missing_name is None or name != missing_name)
-        and context_argument
-    ):
+    if is_context_enabled(data.options.context) and context_argument:
         return (context_argument, "context_parameter")
     return await _run_intent_fallback(data, request, extra)
 

posthog/test/mcp/test_units.py

@@ -2,6 +2,8 @@
 schema/loop-back, session-id rollover, and the identity cache. These complement the
 end-to-end adapter tests by exercising edge branches directly."""
 
+import pytest
+
 from datetime import datetime, timedelta, timezone
 from types import SimpleNamespace
 
@@ -58,29 +60,27 @@ async def test_intent_from_context_argument():
     assert out == ("do the thing", "context_parameter")
 
 
-async def test_intent_skips_context_for_missing_capability_tool():
-    # a get_more_tools call's context is a capability report, not a tool-call
-    # intent — but only while the SDK actually owns that name
+@pytest.mark.parametrize("report_missing", [False, True])
+async def test_intent_captures_context_whatever_the_tool_is_called(report_missing):
+    # Only host-dispatched tools reach intent resolution: PostHog's own virtual
+    # tools are intercepted first and set their event's intent from their own
+    # arguments. So a call named `get_more_tools` arriving here is the *host's*
+    # tool, and its context is an ordinary intent.
+    #
+    # A name-based exemption used to live here and could only fire on that host
+    # tool, silently dropping its `$mcp_intent` -- and only on an instance that
+    # had not yet served a tools/list, which made it look intermittent.
     out = await resolve_tool_call_intent(
-        _data(report_missing=True),
+        _data(report_missing=report_missing),
         _call(name="get_more_tools", args={"context": "need csv export"}),
     )
-    assert out is None
-
-
-async def test_intent_keeps_context_for_a_real_tool_named_get_more_tools():
-    # With report_missing off the SDK advertises no such tool, so one by that
-    # name is the host's own and its context is an ordinary intent. Interception
-    # has always been gated on report_missing, so dropping the intent here left
-    # a real tool dispatched but unattributed.
-    out = await resolve_tool_call_intent(
-        _data(), _call(name="get_more_tools", args={"context": "need csv export"})
-    )
     assert out == ("need csv export", "context_parameter")
 
 
-async def test_intent_keeps_context_when_a_real_tool_owns_the_name():
-    # Same, via the listing-derived collision state rather than the switch.
+async def test_intent_is_independent_of_listing_derived_collision_state():
+    # Per-request by construction: nothing here reads state a tools/list wrote,
+    # so a server with caller-specific catalogues cannot attribute one caller's
+    # call from another caller's listing.
     data = _data(report_missing=True)
     data.virtual_tool_collisions.add(VIRTUAL_TOOL_MISSING_CAPABILITY)
     out = await resolve_tool_call_intent(
@@ -311,11 +311,13 @@ def test_resolve_conversation_id_disabled():
     assert resolve_conversation_id(False, {}, "t", "get_more_tools") == (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_exempts_no_tool_by_name():
+    # A name-based exemption cannot tell PostHog's virtual tool from a host tool
+    # sharing the name, and got it wrong in the direction that matters: the
+    # host's tool lost its handle. PostHog's virtual tools discard the resolved
+    # handle on their own capture paths, so resolving one for them costs nothing.
+    cid, minted = resolve_conversation_id(True, {}, "get_more_tools", "get_more_tools")
+    assert minted is True and cid
 
 
 def test_resolve_conversation_id_mints_for_a_shadowed_virtual_tool_name():

posthog/test/mcp/test_virtual_tools.py

@@ -201,28 +201,45 @@ async def test_collision_un_shadows_once_the_real_tool_is_dropped():
 
 async def test_registry_probe_blocks_interception_before_any_listing():
     # The multi-pod case: a call reaching a process that never served a
-    # tools/list has empty collision state, so the registry is the only signal.
+    # tools/list has no listing-derived state, so the registry is the only
+    # signal. The host's tool must run AND be attributed exactly like any other
+    # tool of theirs -- attribution used to be decided from listing state
+    # instead, so $mcp_intent silently went missing on these instances only.
     server = FastMCP("virtual-tools-fastmcp")
 
     @server.tool()
     def get_more_tools(context: str) -> str:
         return "real tool ran"
 
+    @server.tool()
+    def ordinary(context: str) -> str:
+        return "ordinary ran"
+
     client = FakeClient()
     instrument(server, client, MCPAnalyticsOptions(report_missing=True))
 
     out = await server._tool_manager.call_tool(
         "get_more_tools", {"context": "need csv export"}
     )
+    await server._tool_manager.call_tool("ordinary", {"context": "need csv export"})
     await _flush()
 
     assert "real tool ran" in str(out)
     assert _events(client, "$mcp_missing_capability") == []
 
+    colliding, sibling = _events(client, "$mcp_tool_call")
+    assert colliding["properties"]["$mcp_intent"] == "need csv export"
+    # Same treatment as an ordinary tool, which is the whole rule: intent is
+    # captured for tools the host owns, skipped only for PostHog's own.
+    assert (
+        colliding["properties"]["$mcp_intent_source"]
+        == sibling["properties"]["$mcp_intent_source"]
+    )
+
 
 async def test_raw_list_probe_blocks_interception_before_any_listing():
-    # A raw low-level server has no tool registry, so ownership is settled by
-    # asking the host's own tools/list handler.
+    # Same, on a raw low-level server: no tool registry, so ownership is settled
+    # by asking the host's own tools/list handler.
     server = _make_paged_lowlevel([[_REAL_GET_MORE_TOOLS]])
     client = FakeClient()
     instrument(server, client, MCPAnalyticsOptions(report_missing=True))
@@ -232,6 +249,31 @@ async def test_raw_list_probe_blocks_interception_before_any_listing():
 
     assert out.root.content[0].text == "real tool ran"
     assert _events(client, "$mcp_missing_capability") == []
+    call = _events(client, "$mcp_tool_call")[0]
+    assert call["properties"]["$mcp_intent"] == "need csv export"
+
+
+async def test_another_clients_listing_cannot_suppress_interception():
+    # The collision set is per-server and rewritten by whichever listing ran
+    # last, so reading it on the call path would let one caller's catalogue
+    # decide another caller's call. The call path reads the per-request probe
+    # instead, so a stale collision cannot suppress PostHog's virtual tool.
+    server = _make_paged_lowlevel([[_ECHO_TOOL]])
+    client = FakeClient()
+    instrument(server, client, MCPAnalyticsOptions(report_missing=True))
+
+    from posthog.mcp._instrumentation import VIRTUAL_TOOL_MISSING_CAPABILITY
+    from posthog.mcp._internal import get_server_tracking_data
+
+    get_server_tracking_data(server).virtual_tool_collisions.add(
+        VIRTUAL_TOOL_MISSING_CAPABILITY
+    )
+
+    out = await _call(server, "get_more_tools", {"context": "need csv export"})
+    await _flush()
+
+    assert out.root.content[0].text == get_more_tools_result_text()
+    assert _events(client, "$mcp_missing_capability")
 
 
 async def test_both_virtual_tools_configured_with_the_same_name():