diff --git a/.sampo/changesets/mcp-analytics-defaults.md b/.sampo/changesets/mcp-analytics-defaults.md new file mode 100644 index 00000000..7dbb9b5d --- /dev/null +++ b/.sampo/changesets/mcp-analytics-defaults.md @@ -0,0 +1,5 @@ +--- +pypi/posthog: minor +--- + +Enable MCP model capture and conversation correlation by default. Advertised tool schemas gain an `llm_model` argument (never enforced at dispatch) and eligible tool results gain a conversation handle; `MCPAnalyticsOptions(capture_model=False, enable_conversation_id=False)` restores the previous shape. Fresh low-level instances now read the self-reported model instead of staying silent. diff --git a/posthog/mcp/README.md b/posthog/mcp/README.md index ece6b576..bc3c68ef 100644 --- a/posthog/mcp/README.md +++ b/posthog/mcp/README.md @@ -36,21 +36,29 @@ Request headers use the same identity and package version, so SDK Health can com Because `$lib` is a client-level identity, `instrument()` relabels every event sent by the client passed to it. Use a client dedicated to MCP analytics if the application also captures unrelated events. -## Capture the calling model +## Defaults and opt-outs -Model capture is off by default. Enable it for an instrumented MCP Python SDK 1.x or -2.x server: +Intent, model capture, conversation correlation, and MCP exception capture are on by default. +Missing-capability reporting and feedback collection remain off. ```python from posthog.mcp import MCPAnalyticsOptions, instrument -analytics = instrument( - server, - posthog, - MCPAnalyticsOptions(capture_model=True), -) +instrument(server, posthog, MCPAnalyticsOptions(capture_model=False, enable_conversation_id=False)) ``` +Model capture adds an `llm_model` argument to compatible tool schemas, required on the official +high-level adapters and optional elsewhere. Dispatch never enforces it, so servers keep working; +strict-schema clients see the new field. Set `capture_model=False` to leave schemas untouched. +Conversation correlation adds an optional `conversation_id` argument and returns a handle in +eligible tool results. Clients must echo it to group later calls; calls without it mint new handles. +Set `enable_conversation_id=False` to retain transport-based session grouping and unchanged +response content. Custom `PostHogMCP` dispatchers enable model capture by default but still +supply their own session IDs. The reasoning is recorded in posthog-js `docs/adr/0013`. + +## Capture the calling model + +Model capture is on by default for instrumented MCP Python SDK 1.x and 2.x servers. The SDK records the best model identifier visible to the server as `$mcp_llm_model`. Recognized client metadata wins and sets `$mcp_llm_model_source` to `client_metadata`. The SDK also adds an `llm_model` @@ -75,13 +83,21 @@ If a tool already declares `llm_model`, or uses a root `$ref`, `oneOf`, `allOf`, `anyOf` schema, PostHog leaves the schema and argument untouched. Client metadata can still be captured in those cases. -For a custom dispatcher, use the same option on `PostHogMCP` and pass request +A raw low-level server learns ownership while serving `tools/list`. A fresh instance that never +served one has no answer, so it records `llm_model` as the self-reported model and strips +nothing (posthog-js ADR-0011: reads fail open, strips fail closed). A tool that declares its own +`llm_model` on such an instance is therefore recorded under `$mcp_llm_model` until a listing says +otherwise; `capture_model=False` or `before_send` are the escapes. High-level adapters and +standalone `fastmcp.FastMCP` read ownership from the registered tool schema, so they are +unaffected and need no prior listing. + +For a custom dispatcher, `PostHogMCP` enables the same option by default; pass request metadata through explicitly: ```python from posthog.mcp import PostHogMCP -posthog = PostHogMCP("phc_...", capture_model=True) +posthog = PostHogMCP("phc_...") tools = posthog.prepare_tool_list(server_tools) original_tool = next(tool for tool in server_tools if tool["name"] == tool_name) call = posthog.prepare_tool_call( @@ -229,7 +245,7 @@ sess = get_mcp_session(request) # sess.session_id, sess.client_name, ... ### Or skip the middleware entirely: conversation ids -`MCPAnalyticsOptions(enable_conversation_id=True)` derives `$session_id` from the +`enable_conversation_id` is on by default and derives `$session_id` from the agent's conversation handle, deterministically and identically on every pod. That 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. diff --git a/posthog/mcp/_instrument_lowlevel.py b/posthog/mcp/_instrument_lowlevel.py index 78659793..0a050046 100644 --- a/posthog/mcp/_instrument_lowlevel.py +++ b/posthog/mcp/_instrument_lowlevel.py @@ -14,13 +14,14 @@ from __future__ import annotations +import functools import inspect import time -from typing import Any, Optional, Tuple +from typing import Any, Dict, List, Optional, Tuple import mcp.types as mcp_types -from ._context_parameters import schema_has_param +from ._context_parameters import is_context_enabled, schema_has_param from ._conversation_id import build_prompt_back from ._event_types import MCPAnalyticsEventType from ._instrumentation import ( @@ -42,6 +43,7 @@ ) from ._internal import MCPAnalyticsData from ._model_parameters import request_meta_from_context +from ._model_parameters import can_inject_model_parameter, is_capture_model_enabled from ._output_instructions import mirror_instructions_into_structured_content from .logger import log from .tools import get_more_tools_result_text, resolve_missing_capability_tool_name @@ -182,6 +184,11 @@ def _wrap_call_tool( async def handler(req: Any) -> Any: name = req.params.name arguments = dict(req.params.arguments or {}) + strip, model_ours = ( + await _standalone_ownership(data, high_level, name, req.params.meta) + if strip_injected + else (set(), data.tool_model_parameter_injected.get(name)) + ) client_name, client_version = _client_info(server) protocol_version = _protocol_version(server) mcp_session_id = _mcp_session_id(server) @@ -195,9 +202,8 @@ async def handler(req: Any) -> Any: name=name, arguments=arguments, request_meta=request_meta_from_context(_request_context(server)), - allow_self_reported_model=data.tool_model_parameter_injected.get( - name, False - ), + # Reads fail open on unknown ownership (posthog-js ADR-0011). + allow_self_reported_model=model_ours is not False, mcp_session_id=mcp_session_id, token=token, client_name=client_name, @@ -237,14 +243,9 @@ async def handler(req: Any) -> Any: # but NOT a key the tool declares itself (that's a real argument). Ownership # is read from the tool's own signature, so it holds with or without a prior # tools/list and across stateless per-request server instances. - if strip_injected and req.params.arguments: - owned = await _tool_owned_injected_keys(high_level, name) - injected_keys = ["context", "conversation_id"] - if data.tool_model_parameter_injected.get(name, False): - injected_keys.append("llm_model") - for key in injected_keys: - if key not in owned: - req.params.arguments.pop(key, None) + if strip and req.params.arguments: + for key in strip: + req.params.arguments.pop(key, None) # 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. @@ -325,6 +326,7 @@ def _inject_tool_schemas( population pass, so the schema the SDK validates against always matches the one we advertised — see the note in ``handler``. """ + verdicts: Dict[str, bool] = {} for tool in tools: schema = getattr(tool, "inputSchema", None) mutate_tool_schema( @@ -334,6 +336,14 @@ def _inject_tool_schemas( owns_context=schema_has_param(schema, "context"), context_required=context_required, ) + verdict = data.tool_model_parameter_injected.get(tool.name) + if verdict is None: + continue + if verdicts.setdefault(tool.name, verdict) != verdict: + # Two advertised tools share this name and disagree (FastMCP 2.x + # lists a middleware tool beside the registered one it shadows). + # Which one dispatches is unknown, so the strip fails closed. + data.tool_model_parameter_injected[tool.name] = False def _wrap_list_tools( @@ -441,20 +451,198 @@ async def _feedback_name_owned_by_real_tool(high_level: Any, name: str) -> bool: return False -async def _tool_owned_injected_keys(high_level: Any, name: str) -> set: - """Which of (``context``, ``conversation_id``) the jlowin FastMCP tool declares - itself, read from its function signature. These are real tool arguments we must - not strip. On any lookup failure, return empty (strip both) — same as the prior - unconditional behaviour, so a flaky introspection never leaks an injected key.""" - if high_level is None: - return set() +_INJECTED_KEYS = ("context", "conversation_id", "llm_model") + + +async def _standalone_ownership( + data: MCPAnalyticsData, high_level: Any, name: str, meta: Any +) -> Tuple[set, Optional[bool]]: + """Ownership of the injected arguments on jlowin's standalone FastMCP: the + keys to strip before it validates the call, and whether ``llm_model`` is + ours (``None`` when nothing can say). + + Only keys injected under the current options are candidates. ``context`` + and ``conversation_id`` are stripped unless the registered schema (or, + without one, the function signature) declares them; a failed lookup strips + both — the prior behaviour, so a flaky introspection never leaks an injected + key into validation. ``llm_model`` is judged by the effective listing first, + because middleware can provide or shadow the tool the registry knows, then + by the registry; with neither witness it stays and is still read — strips + fail closed, reads fail open (posthog-js ADR-0011). + """ + try: + declared, model_injectable = await _registry_view(high_level, name, meta) + registry_trusted = model_injectable is not None and not _dispatch_can_differ( + high_level + ) + except Exception: # noqa: BLE001 - ownership inference must never prevent dispatch + declared, model_injectable, registry_trusted = None, None, False + listed = data.tool_model_parameter_injected.get(name) + if listed is not None: + model_ours: Optional[bool] = listed + elif registry_trusted: + model_ours = model_injectable + else: + model_ours = None + candidates = _injected_keys(data) + strip = {k for k in candidates - {"llm_model"} if k not in (declared or set())} + if "llm_model" in candidates and model_ours: + strip.add("llm_model") + return strip, model_ours + + +def _injected_keys(data: MCPAnalyticsData) -> set: + """The analytics arguments the SDK injects under the current options — the + only ones it may strip. A disabled feature injects nothing, so its key is + the application's even when the schema does not declare it.""" + keys = set() + if is_context_enabled(data.options.context): + keys.add("context") + if data.options.enable_conversation_id: + keys.add("conversation_id") + if is_capture_model_enabled(data.options.capture_model): + keys.add("llm_model") + return keys + + +async def _registry_view( + high_level: Any, name: str, meta: Any +) -> Tuple[Optional[set], Optional[bool]]: + """What the registered tool says about the injected keys: which of + ``_INJECTED_KEYS`` it declares itself, and whether a listing would have + injected ``llm_model`` into its schema (the same test the listing applies, + on the schema as the client would see it). Read from the schema (a ``Tool`` + subclass may have no function) else the signature. The registry is read + directly, never through middleware, so a cold instance answers without a + listing and rate limiters are not charged. ``(None, None)`` when the + registry has no such tool or cannot be read.""" try: - tool = await high_level.get_tool(name) - fn = getattr(tool, "fn", None) - params = set(inspect.signature(fn).parameters) if fn is not None else set() - return {k for k in ("context", "conversation_id") if k in params} + tool = await _registered_tool(high_level, name, meta) except Exception: # noqa: BLE001 - introspection is best-effort - return set() + return None, None + if tool is None: + return None, None + schema = getattr(tool, "parameters", None) + if isinstance(schema, dict): + return _schema_view(schema, dereferenced=_server_dereferences(high_level)) + fn = getattr(tool, "fn", None) + if fn is None: + return set(), True + try: + declared = {k for k in _INJECTED_KEYS if k in inspect.signature(fn).parameters} + except Exception: # noqa: BLE001 - introspection is best-effort + return set(), True + return declared, "llm_model" not in declared + + +async def _registered_tool(high_level: Any, name: str, meta: Any) -> Any: + """The tool version the request pinned in ``_meta.fastmcp.version``, else + the highest one — the same choice FastMCP makes when dispatching.""" + version = _requested_tool_version(meta) + if version is None: + return await high_level.get_tool(name) + from fastmcp.utilities.versions import VersionSpec + + return await high_level.get_tool(name, version=VersionSpec(eq=version)) + + +def _requested_tool_version(meta: Any) -> Optional[str]: + """Ownership must follow dispatch. Only a FastMCP that exposes the + ``_meta`` version extractor its own dispatch uses honours a pinned + version; every earlier release calls the highest version regardless.""" + try: + from fastmcp.server.dependencies import extract_version_spec + except ImportError: + return None + dump = getattr(meta, "model_dump", None) + if callable(dump): + meta = dump(by_alias=True) + return extract_version_spec(meta) if isinstance(meta, dict) else None + + +def _schema_view(schema: Dict[str, Any], *, dereferenced: bool) -> Tuple[set, bool]: + """The injected keys a schema declares as its own, and whether a listing + would inject ``llm_model`` into it. With dereferencing on, every node along + a root ``$ref`` chain counts, as the client sees the merged schema; with it + off the client sees the reference itself, which the listing never injects + into. Nothing is injected into a composed or unresolvable schema either, so + all keys count as declared there and nothing is stripped.""" + nodes = _reference_chain(schema) if dereferenced else [schema] + if nodes is None or any( + node.get(key) for node in nodes for key in ("oneOf", "allOf", "anyOf") + ): + return set(_INJECTED_KEYS), False + declared = { + key + for node in nodes + if isinstance(node.get("properties"), dict) + for key in node["properties"] + if key in _INJECTED_KEYS + } + injectable = can_inject_model_parameter(nodes[-1]) and "llm_model" not in declared + return declared, injectable + + +def _server_dereferences(server: Any) -> bool: + """Whether this FastMCP dereferences schemas before advertising them (its + built-in middleware, on by default from 3.x; absent on 2.x).""" + return any( + type(middleware).__module__.endswith(".dereference") + for middleware in getattr(server, "middleware", ()) + ) + + +_DISPATCH_HOOKS = ("on_message", "on_request", "on_list_tools", "on_call_tool") + + +def _dispatch_can_differ(server: Any) -> bool: + """Whether application middleware can provide, shadow, or reroute a tool, + making the registry an unreliable witness for what actually runs. Any + override of a listing or dispatch hook can; FastMCP's own built-ins are + excluded. Without a trustworthy registry a cold instance keeps ``llm_model`` + (strips fail closed), which is exactly what it did before this path.""" + try: + from fastmcp.server.middleware import Middleware + except ImportError: # FastMCP before middleware existed: nothing can differ + return False + + return any( + type(middleware) not in _builtin_middleware_types() # subclasses are the app's + and any( + getattr(type(middleware), hook) is not getattr(Middleware, hook) + for hook in _DISPATCH_HOOKS + ) + for middleware in getattr(server, "middleware", ()) + ) + + +@functools.lru_cache(maxsize=1) +def _builtin_middleware_types() -> tuple: + """The middleware a bare ``FastMCP()`` installs on its own, probed rather + than named so a new built-in in a later release is still recognised.""" + from fastmcp import FastMCP + + return tuple(type(middleware) for middleware in FastMCP("posthog-probe").middleware) + + +def _reference_chain(schema: Dict[str, Any]) -> Optional[List[Dict[str, Any]]]: + """The schema and each local root ``$ref`` target in turn; ``None`` for a + reference that is external, dangling, or cyclic.""" + nodes = [schema] + while len(nodes) <= 8: + ref = nodes[-1].get("$ref") + if ref is None: + return nodes + if not isinstance(ref, str) or not ref.startswith("#/"): + return None + target: Any = schema + for part in ref[2:].split("/"): + key = part.replace("~1", "/").replace("~0", "~") # JSON Pointer escapes + target = target.get(key) if isinstance(target, dict) else None + if not isinstance(target, dict) or any(target is node for node in nodes): + return None + nodes.append(target) + return None def _request_context(server: Any) -> Any: diff --git a/posthog/mcp/_instrument_v2.py b/posthog/mcp/_instrument_v2.py index 66a28e77..844ca974 100644 --- a/posthog/mcp/_instrument_v2.py +++ b/posthog/mcp/_instrument_v2.py @@ -486,15 +486,18 @@ def _wrap_v2_call_tool(server: Any, data: MCPAnalyticsData) -> None: async def handler(ctx: Any, params: Any) -> Any: name = params.name arguments = dict(params.arguments or {}) - analytics_owns_model = data.tool_model_parameter_injected.get(name, False) + # A raw instance that never served a listing has no ownership answer and + # reads the self-reported model anyway; only a listing that proved the + # application owns `llm_model` stops it (posthog-js ADR-0011). + analytics_owns_model = data.tool_model_parameter_injected.get(name) is not False standalone = data.standalone_fastmcp() if data.standalone_fastmcp else None if standalone is not None: version = _requested_tool_version(ctx) injected = await _standalone_injected_parameters( standalone, data, name, version ) - analytics_owns_model = injected is not None and "llm_model" in injected if injected is not None: + analytics_owns_model = "llm_model" in injected call_arguments = { key: value for key, value in arguments.items() diff --git a/posthog/mcp/posthog_mcp.py b/posthog/mcp/posthog_mcp.py index 614b9615..a461e046 100644 --- a/posthog/mcp/posthog_mcp.py +++ b/posthog/mcp/posthog_mcp.py @@ -71,7 +71,7 @@ def __init__( api_key: str, missing_capability_tool_name: Optional[str] = None, mcp_exception_autocapture: bool = True, - capture_model: Union[bool, MCPAnalyticsModelOptions] = False, + capture_model: Union[bool, MCPAnalyticsModelOptions] = True, collect_feedback: Union[bool, CollectFeedbackOptions] = False, **kwargs: Any, ) -> None: diff --git a/posthog/mcp/types.py b/posthog/mcp/types.py index d2f3afdd..fb37a73e 100644 --- a/posthog/mcp/types.py +++ b/posthog/mcp/types.py @@ -183,7 +183,7 @@ class MCPAnalyticsOptions: logger: Optional[LoggerFn] = None report_missing: bool = False missing_capability_tool_name: Optional[str] = None - enable_conversation_id: bool = False + enable_conversation_id: bool = True enable_exception_autocapture: bool = True # Inject a required `context` parameter on every tool to capture user intent. context: Union[bool, MCPAnalyticsContextOptions] = True @@ -197,8 +197,8 @@ class MCPAnalyticsOptions: # Extra properties merged onto every auto-captured event. event_properties: Optional[EventPropertiesFn] = None # Capture the model from recognized client metadata, falling back to an - # SDK-injected llm_model argument. Off by default. - capture_model: Union[bool, MCPAnalyticsModelOptions] = False + # SDK-injected llm_model argument. On by default; False disables capture. + capture_model: Union[bool, MCPAnalyticsModelOptions] = True # Inject the `send_feedback` virtual tool so agents can send feedback about # this server to its developers — a missing capability (the priority # category), a tool that failed or confused them, or praise. Calls to it emit diff --git a/posthog/test/mcp/test_defaults.py b/posthog/test/mcp/test_defaults.py new file mode 100644 index 00000000..8a7a70a8 --- /dev/null +++ b/posthog/test/mcp/test_defaults.py @@ -0,0 +1,89 @@ +import pytest + +from posthog.mcp.types import MCPAnalyticsOptions +from posthog.test.mcp._helpers import ( + MCP_MAJOR, + FakeClient, + events_named, + flush_background, +) + + +@pytest.mark.skipif(MCP_MAJOR < 2, reason="v2 low-level handler API") +async def test_default_capture_on_fresh_lowlevel_instances(): + from posthog.mcp import instrument + from posthog.test.mcp.test_v2_lowlevel import make_server, _call_tool, _list_tools + + client = FakeClient() + + def fresh(): + server = make_server() + instrument(server, client) + return server + + listing = await _list_tools(fresh()) + assert [tool.name for tool in listing.tools] == ["add"] + assert {"context", "llm_model", "conversation_id"} <= set( + listing.tools[0].input_schema["properties"] + ) + await _call_tool( + fresh(), "add", {"a": 1, "b": 2, "context": "intent", "llm_model": "model-a"} + ) + await flush_background() + first = events_named(client, "$mcp_tool_call")[0]["properties"] + await _call_tool( + fresh(), + "add", + { + "a": 2, + "b": 3, + "context": "intent", + "llm_model": "model-a", + "conversation_id": first["$mcp_conversation_id"], + }, + ) + await flush_background() + calls = events_named(client, "$mcp_tool_call") + assert len(calls) == 2 + assert first["$mcp_llm_model"] == "model-a" + assert first["$session_id"] == calls[1]["properties"]["$session_id"] + assert len(events_named(client, "$mcp_tools_list")) == 1 + + +@pytest.mark.skipif(MCP_MAJOR >= 2, reason="v1 low-level handler API") +@pytest.mark.parametrize("enabled", [True, False]) +async def test_v1_cold_capture_and_opt_out(enabled): + import mcp.types as types + from posthog.mcp import instrument + from posthog.test.mcp.test_lowlevel import make_server, _call_request + + client = FakeClient() + server = make_server() + options = ( + None + if enabled + else MCPAnalyticsOptions(capture_model=False, enable_conversation_id=False) + ) + received = [] + original = server.request_handlers[types.CallToolRequest] + + async def spy(req): + received.append(dict(req.params.arguments or {})) + return await original(req) + + server.request_handlers[types.CallToolRequest] = spy + instrument(server, client, options) + request = _call_request("echo", {"msg": "ok", "llm_model": "model-a"}) + result = await server.request_handlers[types.CallToolRequest](request) + await flush_background() + calls = events_named(client, "$mcp_tool_call") + assert len(calls) == 1 + assert not calls[0]["properties"]["$mcp_is_error"] + assert calls[0]["properties"].get("$mcp_llm_model") == ( + "model-a" if enabled else None + ) + assert len(result.root.content) == (2 if enabled else 1) + assert len(events_named(client, "$mcp_tools_list")) == 0 + # A cold raw instance reads the self-reported model but strips nothing: it + # cannot prove it owns the argument, and raw handlers ignore extra keys. + assert received == [{"msg": "ok", "llm_model": "model-a"}] diff --git a/posthog/test/mcp/test_fastmcp_v2.py b/posthog/test/mcp/test_fastmcp_v2.py index 9ebc1145..f8ed5307 100644 --- a/posthog/test/mcp/test_fastmcp_v2.py +++ b/posthog/test/mcp/test_fastmcp_v2.py @@ -1,6 +1,8 @@ """Tests for jlowin's standalone FastMCP 2.0 (the `fastmcp` package), distinct from the official SDK's mcp.server.fastmcp.FastMCP.""" +import inspect + import pytest pytest.importorskip("fastmcp") @@ -27,6 +29,14 @@ def add(a: int, b: int) -> int: return server +def _tool_result_type(): + try: + from fastmcp.tools.tool import ToolResult + except ImportError: + pytest.skip("this FastMCP predates ToolResult") + return ToolResult + + async def _list(server): handler = server._mcp_server.request_handlers[mcp_types.ListToolsRequest] return await handler(mcp_types.ListToolsRequest(method="tools/list")) @@ -151,3 +161,403 @@ def echo(msg: str) -> str: assert _events(client, "$mcp_tool_call")[0]["properties"]["$mcp_intent"] == ( "strict validation" ) + + +@pytest.mark.parametrize("listed", [True, False], ids=["listed", "cold"]) +async def test_jlowin_call_strips_llm_model_and_records_it(listed): + # Ownership comes from the registered schema, so a cold instance (one that + # never served the listing that advertised llm_model, as in a multi-replica + # deployment) strips it before jlowin validates and still records it. + server = make_server() + client = FakeClient() + instrument(server, client) + + if listed: + await _list(server) + out = await _call( + server, "add", {"a": 2, "b": 3, "context": "sum", "llm_model": "model-a"} + ) + await _flush() + + assert out.root.isError is False + calls = _events(client, "$mcp_tool_call") + assert calls[0]["properties"]["$mcp_llm_model"] == "model-a" + + +_OWN_MODEL = {"llm_model": {"type": "string"}} + + +@pytest.mark.parametrize("listed", [True, False], ids=["listed", "cold"]) +@pytest.mark.parametrize("capture_model", [True, False]) +@pytest.mark.parametrize( + "parameters", + [ + {"type": "object", "properties": _OWN_MODEL, "required": ["llm_model"]}, + { + "allOf": [ + {"type": "object", "properties": _OWN_MODEL, "required": ["llm_model"]} + ] + }, + ], + ids=["properties", "allOf"], +) +async def test_jlowin_schema_declared_llm_model_is_kept( + capture_model, parameters, listed +): + # A Tool subclass declares its arguments in `parameters` and has no `fn`. + # Its own llm_model must reach the tool and must never be read as the + # model, with or without a prior listing. + from fastmcp.tools import Tool + + ToolResult = _tool_result_type() + + class Router(Tool): + async def run(self, arguments): + return ToolResult( + content=[ + mcp_types.TextContent(type="text", text=arguments["llm_model"]) + ] + ) + + server = FastMCP("jlowin-schema-owner") + server.add_tool(Router(name="route", parameters=parameters)) + client = FakeClient() + instrument(server, client, MCPAnalyticsOptions(capture_model=capture_model)) + + if listed: + await _list(server) + out = await _call(server, "route", {"llm_model": "gpt-5", "context": "routing"}) + await _flush() + + assert out.root.isError is False + assert out.root.content[0].text == "gpt-5" + assert "$mcp_llm_model" not in _events(client, "$mcp_tool_call")[0]["properties"] + + +@pytest.mark.parametrize( + "declares_model", + ["referenced", "sibling", "chained", "escaped", None], + ids=["referenced", "sibling", "chained", "escaped", "plain"], +) +async def test_jlowin_root_ref_schema_is_dereferenced_for_ownership(declares_model): + # FastMCP dereferences a root `$ref` before the client sees the listing, so + # ownership must be read from every node along the reference chain and from + # the root's own sibling properties: a key declared in any of them stays, + # and an undeclared llm_model is stripped so validation passes. + from fastmcp.tools import Tool + + ToolResult = _tool_result_type() + + properties = {"a": {"type": "integer"}} + extra = {} + if declares_model == "referenced": + properties["llm_model"] = {"type": "string"} + if declares_model == "sibling": + extra = {"properties": {"llm_model": {"type": "string"}}} + if declares_model == "escaped": + # A JSON Pointer escapes "/" as "~1"; FastMCP resolves it, so must we. + extra = { + "$ref": "#/$defs/Args~1Input", + "$defs": { + "Args/Input": { + "type": "object", + "properties": {**properties, "llm_model": {"type": "string"}}, + } + }, + } + if declares_model == "chained": + extra = { + "$defs": { + "Args": {"$ref": "#/$defs/Real"}, + "Real": { + "type": "object", + "properties": {**properties, "llm_model": {"type": "string"}}, + }, + } + } + + class Echo(Tool): + async def run(self, arguments): + return ToolResult( + content=[ + mcp_types.TextContent(type="text", text=",".join(sorted(arguments))) + ] + ) + + options = {} + if declares_model == "chained": + # FastMCP's own dereferencer recurses forever on a chained reference, + # instrumented or not; the chain case only exists with it turned off. + if "dereference_schemas" not in inspect.signature(FastMCP.__init__).parameters: + pytest.skip("this FastMCP cannot disable schema dereferencing") + options = {"dereference_schemas": False} + server = FastMCP("jlowin-root-ref", **options) + server.add_tool( + Echo( + name="echo", + parameters={ + "$ref": "#/$defs/Args", + "$defs": {"Args": {"type": "object", "properties": properties}}, + **extra, + }, + ) + ) + client = FakeClient() + instrument(server, client) + + out = await _call(server, "echo", {"a": 1, "context": "c", "llm_model": "gpt-5"}) + await _flush() + + assert out.root.isError is False + # A FastMCP that never dereferences advertises the `$ref` itself, which the + # listing never injects into, so the plain case keeps the argument there. + dereferences = ( + "dereference_schemas" in inspect.signature(FastMCP.__init__).parameters + ) + stripped = declares_model is None and dereferences + assert out.root.content[0].text == ("a" if stripped else "a,llm_model") + recorded = _events(client, "$mcp_tool_call")[0]["properties"].get("$mcp_llm_model") + assert recorded == ("gpt-5" if stripped else None) + + +async def test_jlowin_ownership_follows_the_dispatched_tool_version(): + # A request may pin a version in `_meta`; only a FastMCP that exposes + # `extract_version_spec` honours it when dispatching, every other release + # calls the highest version. Ownership must strip for the version that + # actually runs, or the call fails validation. + pytest.importorskip("fastmcp.utilities.versions") + try: + from fastmcp.server.dependencies import extract_version_spec # noqa: F401 + + expected = "v1:gpt-5" + except ImportError: + expected = "v2" + server = FastMCP("jlowin-versions") + + @server.tool(name="route", version="1") + def route_v1(prompt: str, llm_model: str) -> str: + return f"v1:{llm_model}" + + @server.tool(name="route", version="2") + def route_v2(prompt: str) -> str: + return "v2" + + client = FakeClient() + instrument(server, client) + handler = server._mcp_server.request_handlers[mcp_types.CallToolRequest] + + out = await handler( + mcp_types.CallToolRequest( + method="tools/call", + params=mcp_types.CallToolRequestParams( + name="route", + arguments={"prompt": "p", "context": "c", "llm_model": "gpt-5"}, + **{"_meta": {"fastmcp": {"version": "1"}}}, + ), + ) + ) + await _flush() + + assert out.root.isError is False, out.root.content + assert out.root.content[0].text == expected + recorded = _events(client, "$mcp_tool_call")[0]["properties"].get("$mcp_llm_model") + assert recorded == (None if expected.startswith("v1") else "gpt-5") + + +async def test_jlowin_disabled_capture_leaves_llm_model_for_permissive_tools(): + # With capture off nothing injected llm_model, so it is the application's + # even when the schema does not declare it: a Tool accepting arbitrary keys + # must still receive it. + from fastmcp.tools import Tool + + ToolResult = _tool_result_type() + + class Bag(Tool): + async def run(self, arguments): + return ToolResult( + content=[ + mcp_types.TextContent(type="text", text=",".join(sorted(arguments))) + ] + ) + + server = FastMCP("jlowin-permissive") + server.add_tool( + Bag( + name="bag", + parameters={"type": "object", "additionalProperties": {"type": "string"}}, + ) + ) + client = FakeClient() + instrument(server, client, MCPAnalyticsOptions(capture_model=False)) + + out = await _call(server, "bag", {"llm_model": "mine", "context": "c"}) + await _flush() + + assert out.root.isError is False + assert out.root.content[0].text == "llm_model" + assert "$mcp_llm_model" not in _events(client, "$mcp_tool_call")[0]["properties"] + + +@pytest.mark.parametrize("listed", [True, False], ids=["listed", "cold"]) +async def test_jlowin_middleware_provided_tool_keeps_its_llm_model(listed): + # The registry does not know a middleware-provided tool, so the effective + # listing is the witness for llm_model. Cold, with no witness at all, the + # argument stays (strips fail closed) and is read (reads fail open). + pytest.importorskip("fastmcp.server.middleware") + from fastmcp.server.middleware.tool_injection import ToolInjectionMiddleware + from fastmcp.tools import Tool + + def route(prompt: str, llm_model: str) -> str: + return llm_model + + server = FastMCP("jlowin-middleware-tool") + server.add_middleware(ToolInjectionMiddleware(tools=[Tool.from_function(route)])) + client = FakeClient() + instrument(server, client) + + if listed: + await _list(server) + out = await _call( + server, "route", {"prompt": "p", "context": "c", "llm_model": "own"} + ) + await _flush() + + assert out.root.isError is False, out.root.content + assert out.root.content[0].text == "own" + recorded = _events(client, "$mcp_tool_call")[0]["properties"].get("$mcp_llm_model") + assert recorded == (None if listed else "own") + + +@pytest.mark.parametrize( + "shadow, listed", + [ + ("listing", True), + ("listing", False), + ("dispatch", False), + ("builtin-subclass", False), + ], + ids=["listing-listed", "listing-cold", "dispatch-cold", "builtin-subclass-cold"], +) +async def test_jlowin_middleware_shadowed_tool_keeps_its_llm_model(shadow, listed): + # A registered tool without llm_model is shadowed by middleware serving one + # that declares it, either by also advertising it or only at dispatch. The + # registry would answer for the wrong tool, so with such middleware present + # it is not trusted: the argument stays and, cold, is read fail-open; a + # listing that advertised the shadowing tool settles it as the application's. + pytest.importorskip("fastmcp.server.middleware") + from fastmcp.server.middleware import Middleware + from fastmcp.server.middleware.tool_injection import ToolInjectionMiddleware + from fastmcp.tools import Tool + + server = FastMCP("jlowin-shadow") + + @server.tool(name="route") + def registered(prompt: str) -> str: + return "registered" + + def route(prompt: str, llm_model: str) -> str: + return llm_model + + shadowing = Tool.from_function(route) + + class DispatchShadow(Middleware): + async def on_call_tool(self, context, call_next): + if context.message.name == "route": + return await shadowing.run(context.message.arguments or {}) + return await call_next(context) + + if shadow == "builtin-subclass": + # An application subclass of one of FastMCP's own middleware is still + # the application's: its overrides count. + builtins = [type(m) for m in FastMCP("probe").middleware] + if not builtins: + pytest.skip("this FastMCP installs no built-in middleware") + builtin = builtins[0] + + class DispatchShadow(builtin): # type: ignore[no-redef,misc] + async def on_call_tool(self, context, call_next): + if context.message.name == "route": + return await shadowing.run(context.message.arguments or {}) + return await call_next(context) + + server.add_middleware( + ToolInjectionMiddleware(tools=[shadowing]) + if shadow == "listing" + else DispatchShadow() + ) + client = FakeClient() + instrument(server, client) + + if listed: + await _list(server) + out = await _call( + server, "route", {"prompt": "p", "context": "c", "llm_model": "own"} + ) + await _flush() + + assert out.root.isError is False, out.root.content + assert out.root.content[0].text == "own" + recorded = _events(client, "$mcp_tool_call")[0]["properties"].get("$mcp_llm_model") + assert recorded == (None if listed else "own") + + +async def test_jlowin_without_dereferencing_a_root_ref_is_never_injected_into(): + # With dereferencing off the client sees the `$ref` itself, which the + # listing never injects llm_model into, so a cold call must not strip it + # from a permissive tool either. + from fastmcp.tools import Tool + + ToolResult = _tool_result_type() + + if "dereference_schemas" not in inspect.signature(FastMCP.__init__).parameters: + pytest.skip("this FastMCP cannot disable schema dereferencing") + + class Bag(Tool): + async def run(self, arguments): + return ToolResult( + content=[ + mcp_types.TextContent(type="text", text=",".join(sorted(arguments))) + ] + ) + + server = FastMCP("jlowin-raw-ref", dereference_schemas=False) + server.add_tool( + Bag( + name="bag", + parameters={ + "$ref": "#/$defs/Args", + "$defs": {"Args": {"type": "object", "additionalProperties": True}}, + }, + ) + ) + client = FakeClient() + instrument(server, client) + + out = await _call(server, "bag", {"llm_model": "mine", "context": "c"}) + await _flush() + + assert out.root.isError is False + assert out.root.content[0].text == "llm_model" + + +async def test_jlowin_cold_call_survives_a_fastmcp_without_middleware(monkeypatch): + # FastMCP releases before middleware existed have no + # `fastmcp.server.middleware`; ownership inference must not import its way + # into breaking dispatch, and the registry is then simply trusted. + import sys + + monkeypatch.setitem(sys.modules, "fastmcp.server.middleware", None) + server = make_server() + client = FakeClient() + instrument(server, client) + + out = await _call( + server, "add", {"a": 2, "b": 3, "context": "sum", "llm_model": "model-a"} + ) + await _flush() + + assert out.root.isError is False, out.root.content + assert ( + _events(client, "$mcp_tool_call")[0]["properties"]["$mcp_llm_model"] + == "model-a" + ) diff --git a/posthog/test/mcp/test_lowlevel.py b/posthog/test/mcp/test_lowlevel.py index 992c558a..ac84541c 100644 --- a/posthog/test/mcp/test_lowlevel.py +++ b/posthog/test/mcp/test_lowlevel.py @@ -377,7 +377,7 @@ async def test_tool_call_error_captured_from_is_error_result(): async def test_initialize_emitted_once(): server = make_server() client = FakeClient() - instrument(server, client) + instrument(server, client, MCPAnalyticsOptions(enable_conversation_id=False)) handler = server.request_handlers[mcp_types.CallToolRequest] await handler(_call_request("echo", {"msg": "a", "context": "first call"})) diff --git a/posthog/test/mcp/test_posthog_mcp.py b/posthog/test/mcp/test_posthog_mcp.py index 7b9b89fc..9355e810 100644 --- a/posthog/test/mcp/test_posthog_mcp.py +++ b/posthog/test/mcp/test_posthog_mcp.py @@ -222,7 +222,7 @@ def test_prepare_tool_list_can_be_disabled(): ) async def test_prepare_and_capture_model(options: dict[str, bool]) -> None: client, captured = make_client(**options) - enabled = options.get("capture_model", False) + enabled = options.get("capture_model", True) tools = [ { "name": "search", diff --git a/posthog/test/mcp/test_session_token.py b/posthog/test/mcp/test_session_token.py index 10d79cba..4cb0ac81 100644 --- a/posthog/test/mcp/test_session_token.py +++ b/posthog/test/mcp/test_session_token.py @@ -645,7 +645,7 @@ def test_runtime_warns_when_app_was_built_before_instrument(): app = srv.streamable_http_app() # BEFORE instrument() -- the trap with _captured_logs() as logs: - instrument(srv, _Sink()) + instrument(srv, _Sink(), MCPAnalyticsOptions(enable_conversation_id=False)) with TestClient(app) as client: resp = _call_ping(client) assert resp.status_code == 200, resp.text @@ -716,7 +716,7 @@ def test_warnings_are_visible_without_configuring_a_logger(caplog): set_logger(None) # explicitly no `logger` option anywhere with caplog.at_level("WARNING", logger="posthog.mcp"): - instrument(srv, _Sink()) + instrument(srv, _Sink(), MCPAnalyticsOptions(enable_conversation_id=False)) with TestClient(app) as client: _call_ping(client) diff --git a/posthog/test/mcp/test_units.py b/posthog/test/mcp/test_units.py index 998510df..e8312123 100644 --- a/posthog/test/mcp/test_units.py +++ b/posthog/test/mcp/test_units.py @@ -109,7 +109,7 @@ def test_schema_pipeline_does_not_warn_for_owned_conversation_id(monkeypatch): tool = SimpleNamespace(name="t", input_schema=schema) mutate_tool_schema( - _data(context=False, enable_conversation_id=True), + _data(context=False, capture_model=False, enable_conversation_id=True), tool, schema_attribute="input_schema", owns_context=False, diff --git a/posthog/test/mcp/test_v2_lowlevel.py b/posthog/test/mcp/test_v2_lowlevel.py index 4d53407e..e3ad65c7 100644 --- a/posthog/test/mcp/test_v2_lowlevel.py +++ b/posthog/test/mcp/test_v2_lowlevel.py @@ -453,7 +453,7 @@ async def late_read_resource(ctx, params): async def test_initialize_and_session_reuse_across_calls(): server = make_server() client = FakeClient() - instrument(server, client) + instrument(server, client, MCPAnalyticsOptions(enable_conversation_id=False)) await _call_tool(server, "add", {"a": 1, "b": 1, "context": "first"}) await _call_tool(server, "add", {"a": 2, "b": 2, "context": "second"}) diff --git a/posthog/test/mcp/test_v2_mcpserver.py b/posthog/test/mcp/test_v2_mcpserver.py index 80ae82ce..890e342c 100644 --- a/posthog/test/mcp/test_v2_mcpserver.py +++ b/posthog/test/mcp/test_v2_mcpserver.py @@ -175,7 +175,7 @@ async def test_tool_call_error_is_captured_and_converted(): async def test_initialize_emitted_once_per_session(): server = make_server() client = FakeClient() - instrument(server, client) + instrument(server, client, MCPAnalyticsOptions(enable_conversation_id=False)) await _call_tool( server, "add", {"a": 1, "b": 1, "context": "first call to warm up"} diff --git a/posthog/test/mcp/test_v2_wire_dual_era.py b/posthog/test/mcp/test_v2_wire_dual_era.py index 4b7c11b3..0e89b69b 100644 --- a/posthog/test/mcp/test_v2_wire_dual_era.py +++ b/posthog/test/mcp/test_v2_wire_dual_era.py @@ -227,7 +227,9 @@ async def test_modern_result_shape_survives_instrumentation(): bare_response = await modern_call(http, "add", {"a": 4, "b": 5}) instrumented = make_server() - instrument(instrumented, FakeClient()) + instrument( + instrumented, FakeClient(), MCPAnalyticsOptions(enable_conversation_id=False) + ) async with wire(instrumented) as http: response = await modern_call(http, "add", {"a": 4, "b": 5, "context": "alive"}) await _flush() @@ -295,7 +297,7 @@ async def test_legacy_stateless_token_survives_across_instances(): # Pod A mints the token on initialize. server_a = make_server() - instrument(server_a, client) + instrument(server_a, client, MCPAnalyticsOptions(enable_conversation_id=False)) async with wire(server_a) as http: init = rpc( "initialize", @@ -316,7 +318,7 @@ async def test_legacy_stateless_token_survives_across_instances(): # A compliant legacy client replays both the session header and the # negotiated MCP-Protocol-Version on every subsequent request. server_b = make_server() - instrument(server_b, client) + instrument(server_b, client, MCPAnalyticsOptions(enable_conversation_id=False)) async with wire(server_b) as http: headers = { **legacy_headers(), diff --git a/references/public_api_snapshot.txt b/references/public_api_snapshot.txt index bf7211fb..843003ab 100644 --- a/references/public_api_snapshot.txt +++ b/references/public_api_snapshot.txt @@ -818,10 +818,10 @@ attribute posthog.mcp.types.MCPAnalyticsContextOptions.description: Optional[str attribute posthog.mcp.types.MCPAnalyticsModelOptions.description: Optional[str] = None attribute posthog.mcp.types.MCPAnalyticsModelSource = Literal['client_metadata', 'self_reported'] attribute posthog.mcp.types.MCPAnalyticsOptions.before_send: Optional[BeforeSendFn] = None -attribute posthog.mcp.types.MCPAnalyticsOptions.capture_model: Union[bool, MCPAnalyticsModelOptions] = False +attribute posthog.mcp.types.MCPAnalyticsOptions.capture_model: Union[bool, MCPAnalyticsModelOptions] = True attribute posthog.mcp.types.MCPAnalyticsOptions.collect_feedback: Union[bool, CollectFeedbackOptions] = False attribute posthog.mcp.types.MCPAnalyticsOptions.context: Union[bool, MCPAnalyticsContextOptions] = True -attribute posthog.mcp.types.MCPAnalyticsOptions.enable_conversation_id: bool = False +attribute posthog.mcp.types.MCPAnalyticsOptions.enable_conversation_id: bool = True attribute posthog.mcp.types.MCPAnalyticsOptions.enable_exception_autocapture: bool = True attribute posthog.mcp.types.MCPAnalyticsOptions.event_properties: Optional[EventPropertiesFn] = None attribute posthog.mcp.types.MCPAnalyticsOptions.identify: Optional[Union[IdentifyFn, UserIdentity]] = None @@ -1021,14 +1021,14 @@ class posthog.mcp.McpAnalytics(key: Any) class posthog.mcp.asgi.PostHogMcpStatelessSessionMiddleware(app: Any) class posthog.mcp.constants.PostHogMCPAnalyticsEvent class posthog.mcp.constants.PostHogMCPAnalyticsProperty -class posthog.mcp.posthog_mcp.PostHogMCP(api_key: str, missing_capability_tool_name: Optional[str] = None, mcp_exception_autocapture: bool = True, capture_model: Union[bool, MCPAnalyticsModelOptions] = False, collect_feedback: Union[bool, CollectFeedbackOptions] = False, **kwargs: Any) +class posthog.mcp.posthog_mcp.PostHogMCP(api_key: str, missing_capability_tool_name: Optional[str] = None, mcp_exception_autocapture: bool = True, capture_model: Union[bool, MCPAnalyticsModelOptions] = True, collect_feedback: Union[bool, CollectFeedbackOptions] = False, **kwargs: Any) class posthog.mcp.session_token.SessionTokenPayload(session_id: str, client_name: Optional[str] = None, client_version: Optional[str] = None, protocol_version: Optional[str] = None) class posthog.mcp.types.CaptureEventData(event: str, properties: Optional[JsonRecord] = None) class posthog.mcp.types.CollectFeedbackOptions(tool_name: Optional[str] = None, description: Optional[str] = None, extra_properties: Optional[Dict[str, Dict[str, Any]]] = None, extra_required: Optional[List[str]] = None, on_feedback: Optional[OnFeedbackFn] = None) class posthog.mcp.types.FeedbackReport(feedback_type: str = 'other', summary: str = '', sentiment: Optional[str] = None, friction_points: Optional[str] = None, suggested_improvement: Optional[str] = None, details: Optional[str] = None, tool_name: Optional[str] = None, task_completed: Optional[bool] = None, extras: JsonRecord = dict(), raw: JsonRecord = dict()) class posthog.mcp.types.MCPAnalyticsContextOptions(description: Optional[str] = None) class posthog.mcp.types.MCPAnalyticsModelOptions(description: Optional[str] = None) -class posthog.mcp.types.MCPAnalyticsOptions(logger: Optional[LoggerFn] = None, report_missing: bool = False, missing_capability_tool_name: Optional[str] = None, enable_conversation_id: bool = False, enable_exception_autocapture: bool = True, context: Union[bool, MCPAnalyticsContextOptions] = True, identify: Optional[Union[IdentifyFn, UserIdentity]] = None, intent_fallback: Optional[IntentFallbackFn] = None, before_send: Optional[BeforeSendFn] = None, event_properties: Optional[EventPropertiesFn] = None, capture_model: Union[bool, MCPAnalyticsModelOptions] = False, collect_feedback: Union[bool, CollectFeedbackOptions] = False) +class posthog.mcp.types.MCPAnalyticsOptions(logger: Optional[LoggerFn] = None, report_missing: bool = False, missing_capability_tool_name: Optional[str] = None, enable_conversation_id: bool = True, enable_exception_autocapture: bool = True, context: Union[bool, MCPAnalyticsContextOptions] = True, identify: Optional[Union[IdentifyFn, UserIdentity]] = None, intent_fallback: Optional[IntentFallbackFn] = None, before_send: Optional[BeforeSendFn] = None, event_properties: Optional[EventPropertiesFn] = None, capture_model: Union[bool, MCPAnalyticsModelOptions] = True, collect_feedback: Union[bool, CollectFeedbackOptions] = False) class posthog.mcp.types.PreparedToolCall(args: Optional[JsonRecord] = None, intent: Optional[str] = None, intent_source: Optional[str] = None, is_missing_capability: bool = False, llm_model: Optional[str] = None, llm_model_source: Optional[MCPAnalyticsModelSource] = None, is_feedback: bool = False, feedback_report: Optional[FeedbackReport] = None) class posthog.mcp.types.UserIdentity(distinct_id: str, properties: Optional[JsonRecord] = None, groups: Optional[Dict[str, str]] = None) class posthog.metrics_capture.PostHogMetrics(client, config: Optional[dict] = None)