diff --git a/.sampo/changesets/mcp-collect-feedback.md b/.sampo/changesets/mcp-collect-feedback.md new file mode 100644 index 000000000..a43dfb186 --- /dev/null +++ b/.sampo/changesets/mcp-collect-feedback.md @@ -0,0 +1,6 @@ +--- +pypi/posthog: minor +--- + +Add an opt-in `collect_feedback` option to MCP analytics. It injects a `send_feedback` virtual tool and captures every call as a `$mcp_feedback` event, so agents can report a missing capability, a tool problem, or praise. +The option supports a custom tool name and description, host-declared extra schema fields, and an `on_feedback` handler that routes reports to a real backend. `PostHogMCP` gains the same option plus `capture_feedback` for custom dispatchers. diff --git a/posthog/mcp/README.md b/posthog/mcp/README.md index a5c38d645..b6482762b 100644 --- a/posthog/mcp/README.md +++ b/posthog/mcp/README.md @@ -105,6 +105,85 @@ Model injection copies tool objects instead of changing their original schemas. Always advertise the returned list and pass the original application tool to `prepare_tool_call()`. Repeatedly preparing the original list preserves ownership. +## Collect agent feedback + +Feedback collection is off by default. Enable it to advertise a `send_feedback` +virtual tool. Agents use it to report a missing capability (the priority +category), a tool that failed or confused them, or praise: + +```python +from posthog.mcp import MCPAnalyticsOptions, instrument + +analytics = instrument( + server, + posthog, + MCPAnalyticsOptions(collect_feedback=True), +) +``` + +Each call emits one `$mcp_feedback` event (never a `$mcp_tool_call`) with +`$mcp_feedback_type`, `$mcp_feedback_summary`, `$mcp_feedback_details`, +`$mcp_feedback_friction_points`, `$mcp_feedback_suggested_improvement`, +`$mcp_feedback_tool`, `$mcp_feedback_sentiment`, and +`$mcp_feedback_task_completed`. The summary and details also map to +`$mcp_intent`. Free-text fields are sanitized, PII-redacted, and length-bounded; +the raw arguments are never captured. An invalid `feedback_type` falls back to +`other`. The agent receives an honest acknowledgement: the call records feedback +and adds no tools. + +The tool covers what `report_missing` covers (as feedback_type +`missing_capability`), so new integrations should enable only one of the two. + +If a real tool already uses the name, the SDK logs a warning, does not inject +the virtual tool, and never intercepts the real tool. + +Use the object form to rename the tool, declare host-specific fields, or route +reports to a real backend: + +```python +from posthog.mcp import CollectFeedbackOptions, MCPAnalyticsOptions + +options = MCPAnalyticsOptions( + collect_feedback=CollectFeedbackOptions( + extra_properties={ + "product_area": { + "type": "string", + "description": "The product the feedback is about.", + }, + }, + extra_required=["product_area"], + on_feedback=lambda report: feedback_backend.record(report), + ), +) +``` + +Declared extras merge into the advertised schema and are captured as +`$mcp_feedback_`. Arguments the agent invents are never captured; the +handler reads them from `report.raw`. A key that collides with a core field +raises at configuration time. `on_feedback` may be sync or async; a returned +non-blank string replaces the default reply, and a raised handler is logged and +falls back to it. The event is captured either way. + +For a custom dispatcher, use the same option on `PostHogMCP`: + +```python +from posthog.mcp import PostHogMCP, send_feedback_result + +posthog = PostHogMCP("phc_...", collect_feedback=True) + +# tools/list handler +tools = posthog.prepare_tool_list(server_tools, collect_feedback=True) + +# tools/call dispatcher +call = posthog.prepare_tool_call(tool_name, raw_args) +if call.is_feedback: + posthog.capture_feedback(report=call.feedback_report) # emits $mcp_feedback + return send_feedback_result() # replies to the agent and stops dispatch +``` + +`on_feedback` is ignored on this path — the dispatcher routes reports itself via +`call.feedback_report`. + ## Stateless / multi-pod servers A stateless MCP server issues no session id, so `$session_id` fragments across pods diff --git a/posthog/mcp/__init__.py b/posthog/mcp/__init__.py index e22a8a1ab..f7c47a2e8 100644 --- a/posthog/mcp/__init__.py +++ b/posthog/mcp/__init__.py @@ -69,9 +69,17 @@ get_mcp_session, ) from ._sink import McpEventSink +from .feedback import ( + SEND_FEEDBACK_TOOL_NAME, + get_feedback_tool_descriptor, + resolve_collect_feedback_options, + send_feedback_result, +) from .tools import get_more_tools_result from .types import ( CaptureEventData, + CollectFeedbackOptions, + FeedbackReport, MCPAnalyticsContextOptions, MCPAnalyticsModelOptions, MCPAnalyticsModelSource, @@ -91,8 +99,12 @@ "MCPAnalyticsModelSource", "UserIdentity", "CaptureEventData", + "CollectFeedbackOptions", + "FeedbackReport", "PreparedToolCall", "get_more_tools_result", + "send_feedback_result", + "SEND_FEEDBACK_TOOL_NAME", # Read HTTP headers inside identify / intent_fallback / # event_properties callbacks on either SDK major: the per-request context # arrives as extra["ctx"] and its shape differs between them. @@ -254,6 +266,13 @@ def instrument( ) _warn_if_unsupported_mcp_version() + # Fail fast on a `collect_feedback` config error (reserved extra key, + # undeclared extra_required) — before the try below, so it raises instead of + # degrading to the no-op handle and first surfacing at tools/list time. + feedback_options = resolve_collect_feedback_options(opts.collect_feedback) + if feedback_options is not None: + get_feedback_tool_descriptor(feedback_options) + key = _canonical_server(server) try: diff --git a/posthog/mcp/_conversation_id.py b/posthog/mcp/_conversation_id.py index 1f44f3c63..4602897ad 100644 --- a/posthog/mcp/_conversation_id.py +++ b/posthog/mcp/_conversation_id.py @@ -79,15 +79,21 @@ def resolve_conversation_id( args: Any, tool_name: Optional[str], missing_capability_tool_name: str, + feedback_tool_name: Optional[str] = None, ) -> Tuple[Optional[str], bool]: - """Return ``(conversation_id, minted)``. Disabled or get_more_tools → ``(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, 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)``. 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 tool_name == missing_capability_tool_name: + if ( + not enabled + or tool_name == missing_capability_tool_name + or (feedback_tool_name is not None and tool_name == feedback_tool_name) + ): return None, False supplied = extract_conversation_id(args) if supplied and _MINTED_CONVERSATION_ID.match(supplied): diff --git a/posthog/mcp/_event_types.py b/posthog/mcp/_event_types.py index 32e63576d..75778dcd8 100644 --- a/posthog/mcp/_event_types.py +++ b/posthog/mcp/_event_types.py @@ -15,6 +15,7 @@ class MCPAnalyticsEventType: IDENTIFY = "posthog:identify" CUSTOM = "posthog:custom" + MCP_FEEDBACK = "mcp:feedback" MCP_MISSING_CAPABILITY = "mcp:missing_capability" MCP_INITIALIZE = "mcp:initialize" MCP_PROMPTS_GET = "mcp:prompts/get" diff --git a/posthog/mcp/_instrument_fastmcp.py b/posthog/mcp/_instrument_fastmcp.py index 0b6e7f7bc..61d33c27a 100644 --- a/posthog/mcp/_instrument_fastmcp.py +++ b/posthog/mcp/_instrument_fastmcp.py @@ -31,9 +31,12 @@ from ._instrumentation import ( _to_jsonable, append_get_more_tools, + append_send_feedback, collect_listed_tools, extract_tools, + listing_has_next_page, mutate_tool_schema, + refresh_feedback_shadow, request_to_dict, resolve_session_and_client, start_tool_call_lifecycle, @@ -118,6 +121,12 @@ async def wrapped( mcp_types.TextContent(type="text", text=get_more_tools_result_text()) ] + if lifecycle.is_feedback and not _feedback_name_owned_by_real_tool( + server, name + ): + reply = await lifecycle.record_feedback() + return [mcp_types.TextContent(type="text", text=reply)] + # Strip each injected key independently. A tool can declare its own # `context` (kept) while `conversation_id` is still SDK-injected (stripped), # so coupling both to context-ownership leaked conversation_id into the tool. @@ -219,7 +228,12 @@ async def list_handler(req: Any) -> Any: # advertise and write, the SDK rejects the customer's own tool result. if req is None: result = await original(req) - _inject_tool_schemas(server, data, extract_tools(result)) + tools = extract_tools(result) + # Refresh the collision flag here too: this pass sees the real tool + # registry, so a real tool named like the feedback tool is detected + # before any client-facing listing. + refresh_feedback_shadow(data, tools) + _inject_tool_schemas(server, data, tools) return result client_name, client_version = _low_level_client_info(server) @@ -258,6 +272,7 @@ async def list_handler(req: Any) -> Any: tools = extract_tools(result) # Empty is computed before adding the virtual missing-capability tool. names, empty = collect_listed_tools(data, tools) + feedback_name = refresh_feedback_shadow(data, tools) _inject_tool_schemas(server, data, tools) @@ -267,6 +282,10 @@ async def list_handler(req: Any) -> Any: append_get_more_tools(result, missing_name, data) names.append(missing_name) + if feedback_name is not None and not listing_has_next_page(result): + append_send_feedback(result, data) + names.append(feedback_name) + await lifecycle.record_result( names=names, response=_to_jsonable(result), @@ -303,6 +322,16 @@ def _inject_prompt_back(result: Any, conversation_id: str) -> Any: return result +def _feedback_name_owned_by_real_tool(server: Any, name: str) -> bool: + """Live registry probe so a real tool by the feedback tool's name is never + shadowed even before the first listing refreshes the collision flag.""" + try: + tool_manager = getattr(server, "_tool_manager", None) + return tool_manager is not None and tool_manager.get_tool(name) is not None + except Exception: # noqa: BLE001 - unknown tool -> the name is not owned + return False + + def _tool_owns_param(server: Any, name: str, param: str) -> bool: """True when the tool's own function declares ``param`` — then it's a real tool argument we must neither inject nor strip (the agent's value belongs to the tool).""" diff --git a/posthog/mcp/_instrument_lowlevel.py b/posthog/mcp/_instrument_lowlevel.py index d66337e63..786597930 100644 --- a/posthog/mcp/_instrument_lowlevel.py +++ b/posthog/mcp/_instrument_lowlevel.py @@ -26,11 +26,14 @@ from ._instrumentation import ( _to_jsonable, append_get_more_tools, + append_send_feedback, collect_listed_tools, extract_tools, + listing_has_next_page, mutate_tool_schema, prepare_request, record_resource_request, + refresh_feedback_shadow, request_to_dict, resource_listing_response, resolve_session_and_client, @@ -216,6 +219,17 @@ async def handler(req: Any) -> Any: ) ) + if lifecycle.is_feedback and not await _feedback_name_owned_by_real_tool( + high_level, name + ): + reply = await lifecycle.record_feedback() + return mcp_types.ServerResult( + mcp_types.CallToolResult( + content=[mcp_types.TextContent(type="text", text=reply)], + isError=False, + ) + ) + # On raw low-level servers `context`/`conversation_id` are injected as # *optional* schema properties and left in place (a (name, arguments) # handler ignores extra keys). FastMCP 2.0 validates against the function @@ -340,9 +354,12 @@ async def handler(req: Any) -> Any: # `additionalProperties: false`. if req is None: result = await original(req) - _inject_tool_schemas( - data, extract_tools(result), context_required=context_required - ) + tools = extract_tools(result) + # Refresh the collision flag here too: this pass sees the real tool + # registry, so a real tool named like the feedback tool is detected + # before any client-facing listing. + refresh_feedback_shadow(data, tools) + _inject_tool_schemas(data, tools, context_required=context_required) return result client_name, client_version = _client_info(server) @@ -384,6 +401,7 @@ async def handler(req: Any) -> Any: # Zero advertised tools is treated as an errored tools/list before the # virtual missing-capability tool is appended. names, empty = collect_listed_tools(data, tools) + feedback_name = refresh_feedback_shadow(data, tools) _inject_tool_schemas(data, tools, context_required=context_required) @@ -393,6 +411,10 @@ async def handler(req: Any) -> Any: append_get_more_tools(result, missing_name, data) names.append(missing_name) + if feedback_name is not None and not listing_has_next_page(result): + append_send_feedback(result, data) + names.append(feedback_name) + await lifecycle.record_result( names=names, response=_to_jsonable(result), @@ -406,6 +428,19 @@ async def handler(req: Any) -> Any: handlers[mcp_types.ListToolsRequest] = handler +async def _feedback_name_owned_by_real_tool(high_level: Any, name: str) -> bool: + """Live registry probe on the standalone-fastmcp path, so a real tool by the + feedback tool's name is never shadowed even before the first listing refreshes + the collision flag. Raw low-level servers have no registry to probe; they rely + on the listing-derived flag alone.""" + if high_level is None: + return False + try: + return await high_level.get_tool(name) is not None + except Exception: # noqa: BLE001 - unknown tool -> the name is not owned + 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 diff --git a/posthog/mcp/_instrument_v2.py b/posthog/mcp/_instrument_v2.py index 649bc606b..a523a8788 100644 --- a/posthog/mcp/_instrument_v2.py +++ b/posthog/mcp/_instrument_v2.py @@ -41,15 +41,18 @@ from ._instrumentation import ( _to_jsonable, collect_listed_tools, + listing_has_next_page, mutate_tool_schema, params_to_request_dict, prepare_request, record_resource_request, + refresh_feedback_shadow, resource_listing_response, resolve_session_and_client, start_tool_call_lifecycle, start_tools_list_lifecycle, ) +from .feedback import get_feedback_tool_descriptor, resolve_collect_feedback_options from ._internal import MCPAnalyticsData from ._model_parameters import ( can_inject_model_parameter, @@ -303,6 +306,14 @@ async def wrapped( ] ) + if lifecycle.is_feedback and not _feedback_name_owned_by_real_tool_v2( + server, name + ): + reply = await lifecycle.record_feedback() + return mcp_types.CallToolResult( + content=[mcp_types.TextContent(type="text", text=reply)] + ) + # v2 validates against the function signature and rejects unexpected # keys, so injected parameters are stripped before dispatch — but never # one the tool's own schema declares (that's a real argument). @@ -449,6 +460,14 @@ async def handler(ctx: Any, params: Any) -> Any: ] ) + # No registry to probe on a raw low-level server; interception relies on + # the listing-derived collision flag alone. + if lifecycle.is_feedback: + reply = await lifecycle.record_feedback() + return mcp_types.CallToolResult( + content=[mcp_types.TextContent(type="text", text=reply)] + ) + # 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. await lifecycle.prime_session() @@ -592,6 +611,7 @@ async def handler(ctx: Any, params: Any) -> Any: tools = list(getattr(result, "tools", []) or []) # Empty is computed before adding the virtual missing-capability tool. names, empty = collect_listed_tools(data, tools) + feedback_name = refresh_feedback_shadow(data, tools) for tool in tools: schema = getattr(tool, "input_schema", None) @@ -614,6 +634,10 @@ async def handler(ctx: Any, params: Any) -> Any: _append_get_more_tools_v2(result, missing_name, data) names.append(missing_name) + if feedback_name is not None and not listing_has_next_page(result): + _append_send_feedback_v2(result, data) + names.append(feedback_name) + await lifecycle.record_result( names=names, response=_to_jsonable(result), @@ -627,6 +651,43 @@ async def handler(ctx: Any, params: Any) -> Any: _replace_handler(server, _LIST_METHOD, handler, entry.params_type) +def _feedback_name_owned_by_real_tool_v2(high_level: Any, name: str) -> bool: + """Live registry probe so a real tool by the feedback tool's name is never + shadowed even before the first listing refreshes the collision flag.""" + try: + return high_level._tool_manager.get_tool(name) is not None + except Exception: # noqa: BLE001 - unknown tool -> the name is not owned + return False + + +def _append_send_feedback_v2(result: Any, data: MCPAnalyticsData) -> None: + """Append the send_feedback virtual tool to a v2 ListToolsResult. Callers gate + on :func:`refresh_feedback_shadow` returning a name.""" + options = resolve_collect_feedback_options(data.options.collect_feedback) + if options is None: + return + descriptor = get_feedback_tool_descriptor(options) + tool = mcp_types.Tool( + name=descriptor["name"], + description=descriptor["description"], + input_schema=descriptor["inputSchema"], + annotations=descriptor["annotations"], + ) + # `owns_context=True`: the tool carries its intent in its own summary / + # details arguments, so no `context` parameter is injected — but the + # capture_model pass still runs, so it advertises `llm_model` too. + mutate_tool_schema( + data, + tool, + schema_attribute="input_schema", + owns_context=True, + context_required=True, + ) + tools_list = getattr(result, "tools", None) + if isinstance(tools_list, list): + tools_list.append(tool) + + def _append_get_more_tools_v2(result: Any, name: str, data: MCPAnalyticsData) -> None: descriptor = build_report_missing_descriptor(name) tool = mcp_types.Tool( diff --git a/posthog/mcp/_instrumentation.py b/posthog/mcp/_instrumentation.py index 52f2b4e71..4a01bcf2c 100644 --- a/posthog/mcp/_instrumentation.py +++ b/posthog/mcp/_instrumentation.py @@ -26,6 +26,15 @@ from ._conversation_id import add_conversation_id_to_schema, resolve_conversation_id from ._event_types import MCPAnalyticsEventType from ._exceptions import capture_exception +from .feedback import ( + build_feedback_event_properties, + build_feedback_intent, + get_feedback_tool_descriptor, + handle_feedback, + parse_feedback_report, + resolve_collect_feedback_options, + resolve_send_feedback_tool_name, +) from ._intent import resolve_tool_call_intent, set_event_intent from ._internal import MCPAnalyticsData, handle_identify, resolve_event_properties from ._model_parameters import ( @@ -42,6 +51,7 @@ from .session import resolve_session_id, resolve_session_id_with_source from .session_token import SessionTokenPayload, decode_session_id from .tools import GET_MORE_TOOLS_NAME, resolve_missing_capability_tool_name +from .types import CollectFeedbackOptions, FeedbackReport # Keep strong refs to in-flight capture tasks/futures and their lifecycle owners so # they aren't GC'd mid-flight and lifecycle drains can select only their own work. @@ -406,6 +416,8 @@ class ToolCallLifecycle: client_version: Optional[str] protocol_version: Optional[str] missing_name: str + feedback_options: Optional[CollectFeedbackOptions] + feedback_name: Optional[str] conversation_id: Optional[str] minted_conversation_id: bool @@ -413,6 +425,16 @@ class ToolCallLifecycle: def is_missing_capability(self) -> bool: return self.data.options.report_missing and self.name == self.missing_name + @property + def is_feedback(self) -> bool: + # Never intercept a name a real application tool owns (fail-open): the + # listing pass records the collision on `feedback_tool_shadowed`. + return ( + self.feedback_name is not None + and self.name == self.feedback_name + and not self.data.feedback_tool_shadowed + ) + async def prepare_session(self, conversation_id: Optional[str]) -> str: return await prepare_request( self.data, @@ -447,6 +469,26 @@ async def record_missing_capability(self) -> None: extra=self.extra, ) + async def record_feedback(self) -> 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) + await record_feedback( + self.data, + session_id, + report=report, + tool_name=self.feedback_name or self.name, + arguments=self.arguments, + request_meta=self.request_meta, + client_name=self.client_name, + client_version=self.client_version, + protocol_version=self.protocol_version, + extra=self.extra, + ) + return await handle_feedback(report, self.feedback_options) + 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. @@ -508,8 +550,22 @@ def start_tool_call_lifecycle( ) -> ToolCallLifecycle: """Resolve adapter-independent policy for a tool call without dispatching it.""" missing_name = resolve_missing_capability_tool_name(data.options) + feedback_options = resolve_collect_feedback_options(data.options.collect_feedback) + feedback_name = ( + resolve_send_feedback_tool_name(feedback_options) + # Mirrors `ToolCallLifecycle.is_feedback`'s fail-open guard below: once a + # real application tool is known to own this name, conversation-id + # resolution must treat calls to it like any other tool too, not skip + # them as if they were the (shadowed) virtual feedback tool. + if feedback_options is not None and not data.feedback_tool_shadowed + else None + ) conversation_id, minted = resolve_conversation_id( - data.options.enable_conversation_id, arguments, name, missing_name + data.options.enable_conversation_id, + arguments, + name, + missing_name, + feedback_name, ) return ToolCallLifecycle( data=data, @@ -525,6 +581,8 @@ def start_tool_call_lifecycle( client_version=client_version, protocol_version=protocol_version, missing_name=missing_name, + feedback_options=feedback_options, + feedback_name=feedback_name, conversation_id=conversation_id, minted_conversation_id=minted, ) @@ -630,6 +688,74 @@ def append_get_more_tools(result: Any, name: str, data: MCPAnalyticsData) -> Non tools_list.append(tool) +def refresh_feedback_shadow(data: MCPAnalyticsData, tools: list) -> Optional[str]: + """Refresh the collision flag from this listing's tools. Returns the resolved + feedback tool name when the virtual tool may be appended, ``None`` when the + feature is off or a real application tool owns the name (fail-open: the real + tool is advertised and dispatched untouched). Run before the schema-injection + pass so it reads the fresh flag. + + Sticky for the instrumentation instance's lifetime: a paginated ``tools/list`` + delivers one page per request, so a collision seen on an earlier page must + survive a later page that doesn't list the real tool — otherwise that page + would re-arm interception and swallow the real tool's calls. The trade-off is + deliberate: un-shadowing after the host removes the real tool requires + re-instrumentation.""" + options = resolve_collect_feedback_options(data.options.collect_feedback) + if options is None: + return None + name = resolve_send_feedback_tool_name(options) + if any(getattr(tool, "name", None) == name for tool in tools): + if not data.feedback_tool_shadowed: + log( + f'Warning: Cannot inject agent-feedback tool "{name}" because a real tool ' + "already uses that name. The real tool will not be intercepted." + ) + data.feedback_tool_shadowed = True + return None if data.feedback_tool_shadowed else name + + +def listing_has_next_page(result: Any) -> bool: + """Whether this ``tools/list`` result is a non-final page of a paginated + listing. The virtual feedback tool is only appended to the final page: an + earlier page could advertise it before a later page reveals a real tool by + the same name. Reads both SDK majors' cursor spelling (1.x models expose + ``nextCursor``, 2.x ``next_cursor``).""" + root = getattr(result, "root", result) + return bool(getattr(root, "nextCursor", None) or getattr(root, "next_cursor", None)) + + +def append_send_feedback(result: Any, data: MCPAnalyticsData) -> None: + """Append the send_feedback virtual tool to the real ListToolsResult.tools + list. Callers gate on :func:`refresh_feedback_shadow` returning a name.""" + import mcp.types as mcp_types + + options = resolve_collect_feedback_options(data.options.collect_feedback) + if options is None: + return + descriptor = get_feedback_tool_descriptor(options) + tool = mcp_types.Tool( + name=descriptor["name"], + description=descriptor["description"], + inputSchema=descriptor["inputSchema"], + annotations=descriptor["annotations"], + ) + root = getattr(result, "root", result) + tools_list = getattr(root, "tools", None) + if isinstance(tools_list, list): + # `owns_context=True`: the tool carries its intent in its own summary / + # details arguments, so no `context` parameter is injected — but the + # capture_model pass still runs, so it advertises `llm_model` too. + mutate_tool_schema( + data, + tool, + schema_attribute="inputSchema", + owns_context=True, + context_required=True, + ) + tools_list.append(tool) + + def read_tool_category(tool: Any) -> Optional[str]: """Read a tool's product category from its ``_meta.category``.""" meta = getattr(tool, "meta", None) @@ -653,6 +779,19 @@ def collect_listed_tools(data: MCPAnalyticsData, tools: list) -> tuple[List[str] return names, not tools +def _is_sdk_virtual_tool(data: MCPAnalyticsData, tool_name: Any) -> bool: + """Whether this name is one of the SDK's own virtual tools (``get_more_tools``, + ``send_feedback``) — those carry their intent in their own arguments, so they + never get ``context``/``conversation_id`` injected. A shadowed feedback name + belongs to a real application tool and keeps normal injection.""" + if tool_name == GET_MORE_TOOLS_NAME: + return True + options = resolve_collect_feedback_options(data.options.collect_feedback) + if options is None or data.feedback_tool_shadowed: + return False + return tool_name == resolve_send_feedback_tool_name(options) + + def mutate_tool_schema( data: MCPAnalyticsData, tool: Any, @@ -669,8 +808,9 @@ def mutate_tool_schema( """ schema = getattr(tool, schema_attribute, None) original_schema = schema + is_sdk_virtual_tool = _is_sdk_virtual_tool(data, tool.name) if ( - tool.name != GET_MORE_TOOLS_NAME + not is_sdk_virtual_tool and is_context_enabled(data.options.context) and not owns_context ): @@ -696,7 +836,7 @@ def mutate_tool_schema( not app_owns_model and schema_has_param(schema, "llm_model") ) if ( - tool.name != GET_MORE_TOOLS_NAME + not is_sdk_virtual_tool and data.options.enable_conversation_id and not schema_has_param(schema, "conversation_id") ): @@ -867,6 +1007,59 @@ async def record_missing_capability( log(f"record_missing_capability failed (event dropped): {err}") +async def record_feedback( + data: MCPAnalyticsData, + session_id: str, + *, + report: FeedbackReport, + tool_name: str, + arguments: Optional[Dict[str, Any]], + request_meta: Optional[Dict[str, Any]] = None, + client_name: Optional[str] = None, + client_version: Optional[str] = None, + protocol_version: Optional[str] = None, + extra: Optional[Dict[str, Any]] = None, +) -> None: + """Record a ``send_feedback`` call as ``$mcp_feedback``, with the report's + summary and details as ``$mcp_intent``. + + Deliberately no ``parameters``: the arguments are agent-narrated free text, + and the PII-redacted ``$mcp_feedback_*`` properties are the captured surface — + the raw arguments would bypass that redaction and record undeclared fields.""" + try: + request = build_tool_call_request(tool_name, arguments) + event: Dict[str, Any] = { + "event_type": MCPAnalyticsEventType.MCP_FEEDBACK, + "session_id": session_id, + "resource_name": tool_name, + "client_name": client_name, + "client_version": client_version, + "protocol_version": protocol_version, + } + intent = build_feedback_intent(report) + if intent: + event["user_intent"] = intent + event["user_intent_source"] = "context_parameter" + if is_capture_model_enabled(data.options.capture_model): + model, source = resolve_model( + request_meta, arguments, allow_self_reported=True + ) + if model: + event["llm_model"] = model + event["llm_model_source"] = source + # Merged by hand (not `_apply_event_properties`, which assigns) so the + # customer's event_properties callback can't clobber the feedback fields. + props = await resolve_event_properties(data, request, extra) + event["properties"] = { + **(props or {}), + **build_feedback_event_properties(report), + } + stamp_transport_identity(event, extra) + fire_and_forget(capture_event(data, event), data) + except Exception as err: # noqa: BLE001 - isolate analytics from the tool path + log(f"record_feedback failed (event dropped): {err}") + + async def record_tools_list( data: MCPAnalyticsData, session_id: str, diff --git a/posthog/mcp/_internal.py b/posthog/mcp/_internal.py index aa16cc687..96bb4db8d 100644 --- a/posthog/mcp/_internal.py +++ b/posthog/mcp/_internal.py @@ -66,6 +66,11 @@ class MCPAnalyticsData: # signature of a stateless server whose mint middleware never attached. Warned # a single time per server so the log isn't flooded on every request. warned_no_stateless_session: bool = False + # True when the last tools/list showed a real application tool using the + # feedback tool's name. Calls to that name then dispatch normally instead of + # being intercepted (fail-open), and the tool keeps its normal analytics + # schema injection. Refreshed at every listing pass. + feedback_tool_shadowed: bool = False last_activity: datetime = field(default_factory=lambda: datetime.now(timezone.utc)) identified_sessions: IdentityCache = field(default_factory=IdentityCache) tool_categories: Dict[str, str] = field(default_factory=dict) diff --git a/posthog/mcp/_posthog_events.py b/posthog/mcp/_posthog_events.py index e380d598b..2ea292093 100644 --- a/posthog/mcp/_posthog_events.py +++ b/posthog/mcp/_posthog_events.py @@ -21,6 +21,7 @@ _BUILT_IN_EVENT_NAME_BY_TYPE = { MCPAnalyticsEventType.CUSTOM: PostHogMCPAnalyticsEvent.CUSTOM, MCPAnalyticsEventType.IDENTIFY: PostHogMCPAnalyticsEvent.IDENTIFY, + MCPAnalyticsEventType.MCP_FEEDBACK: PostHogMCPAnalyticsEvent.FEEDBACK, MCPAnalyticsEventType.MCP_MISSING_CAPABILITY: PostHogMCPAnalyticsEvent.MISSING_CAPABILITY, MCPAnalyticsEventType.MCP_INITIALIZE: PostHogMCPAnalyticsEvent.INITIALIZE, MCPAnalyticsEventType.MCP_PROMPTS_GET: PostHogMCPAnalyticsEvent.PROMPT_GET, diff --git a/posthog/mcp/_sanitization.py b/posthog/mcp/_sanitization.py index 90db4ebd0..0600ab69a 100644 --- a/posthog/mcp/_sanitization.py +++ b/posthog/mcp/_sanitization.py @@ -443,9 +443,9 @@ def _redact_credentials(value: str) -> str: return _redact_secret_tokens(_POSTHOG_TOKEN_PATTERN.sub(_REDACTED_VALUE, value)) -def sanitize_intent(value: Any) -> Any: - """Sanitize the agent-narrated intent: the binary gate, the credential passes, - structured PII, then URLs. +def sanitize_free_text(value: Any) -> Any: + """Sanitize agent-narrated free text (the intent, the send_feedback fields): + the binary gate, the credential passes, structured PII, then URLs. Every step sits where it does for a reason. The binary gate runs first because splicing a redaction into a base64 blob stops it looking like base64, @@ -586,12 +586,23 @@ def redact_pii(value: Any) -> Any: def sanitize_captured_value(value: Any) -> Any: + return _sanitize_value_with(value, _sanitize_string) + + +def sanitize_free_text_value(value: Any) -> Any: + """:func:`sanitize_captured_value` with the free-text string pass + (:func:`sanitize_free_text`) on every string leaf, so nested agent-narrated + values get structured-PII redaction in the load-bearing order too.""" + return _sanitize_value_with(value, sanitize_free_text) + + +def _sanitize_value_with(value: Any, sanitize_string_fn: Any) -> Any: if value is None: return value if isinstance(value, str): - return _sanitize_string(value) + return sanitize_string_fn(value) if isinstance(value, list): - return [sanitize_captured_value(item) for item in value] + return [_sanitize_value_with(item, sanitize_string_fn) for item in value] # bool is an int subclass; both pass through unchanged. if not isinstance(value, dict): return value @@ -601,7 +612,7 @@ def sanitize_captured_value(value: Any) -> Any: result[key] = ( _REDACTED_VALUE if _should_redact_key(str(key)) - else sanitize_captured_value(nested) + else _sanitize_value_with(nested, sanitize_string_fn) ) return result @@ -622,12 +633,13 @@ def sanitize_event(event: Dict[str, Any]) -> Dict[str, Any]: # The intent comes straight from an agent-narrated `context` string, so it # can contain a secret the LLM read aloud or personal data it narrated about - # the user. `sanitize_intent` redacts it like any other captured value and + # the user. `sanitize_free_text` redacts it like any other captured value and # additionally strips structured PII (emails, phone numbers, IPs, cards, - # SSNs). PII redaction is scoped to the intent only — structured tool - # parameters and responses often hold the same shapes as legitimate data. + # SSNs). PII redaction is scoped to agent-narrated free text only — + # structured tool parameters and responses often hold the same shapes as + # legitimate data. if result.get("user_intent") is not None: - result["user_intent"] = sanitize_intent(result["user_intent"]) + result["user_intent"] = sanitize_free_text(result["user_intent"]) if result.get("llm_model") is not None: result["llm_model"] = sanitize_captured_value(result["llm_model"]) diff --git a/posthog/mcp/constants.py b/posthog/mcp/constants.py index bbf1a81b1..75dff21b8 100644 --- a/posthog/mcp/constants.py +++ b/posthog/mcp/constants.py @@ -49,6 +49,7 @@ class PostHogMCPAnalyticsEvent: CUSTOM = "$mcp_custom" EXCEPTION = "$exception" + FEEDBACK = "$mcp_feedback" IDENTIFY = "$identify" INITIALIZE = "$mcp_initialize" MISSING_CAPABILITY = "$mcp_missing_capability" @@ -72,6 +73,14 @@ class PostHogMCPAnalyticsProperty: DURATION_MS = "$mcp_duration_ms" ERROR_MESSAGE = "$mcp_error_message" ERROR_TYPE = "$mcp_error_type" + FEEDBACK_DETAILS = "$mcp_feedback_details" + FEEDBACK_FRICTION_POINTS = "$mcp_feedback_friction_points" + FEEDBACK_SENTIMENT = "$mcp_feedback_sentiment" + FEEDBACK_SUGGESTED_IMPROVEMENT = "$mcp_feedback_suggested_improvement" + FEEDBACK_SUMMARY = "$mcp_feedback_summary" + FEEDBACK_TASK_COMPLETED = "$mcp_feedback_task_completed" + FEEDBACK_TOOL = "$mcp_feedback_tool" + FEEDBACK_TYPE = "$mcp_feedback_type" IS_ERROR = "$mcp_is_error" INTENT = "$mcp_intent" INTENT_SOURCE = "$mcp_intent_source" diff --git a/posthog/mcp/feedback.py b/posthog/mcp/feedback.py new file mode 100644 index 000000000..fcba7fad1 --- /dev/null +++ b/posthog/mcp/feedback.py @@ -0,0 +1,372 @@ +"""The ``send_feedback`` virtual tool: an opt-in tool advertised to agents so they +can send feedback about the server to its developers — a missing capability (the +priority category), a tool that failed or confused them, or praise. Calling it +emits ``$mcp_feedback`` (not ``$mcp_tool_call``).""" + +from __future__ import annotations + +import copy +import json +from typing import Any, Dict, Optional, Union + +from ._internal import _maybe_await +from ._sanitization import sanitize_free_text, sanitize_free_text_value +from .constants import PostHogMCPAnalyticsProperty +from .logger import log +from .types import CollectFeedbackOptions, FeedbackReport, JsonRecord + +__all__ = ["SEND_FEEDBACK_TOOL_NAME", "send_feedback_result"] + +SEND_FEEDBACK_TOOL_NAME = "send_feedback" + +_FEEDBACK_TYPES = ("missing_capability", "issue", "praise", "other") +_SENTIMENTS = ("positive", "neutral", "negative", "mixed") + +# Free-text fields are agent-narrated, like `$mcp_intent`; bound them the same way. +_MAX_FEEDBACK_TEXT_LENGTH = 2048 +_MAX_FEEDBACK_TOOL_NAME_LENGTH = 256 +_TRUNCATION_SUFFIX = "..." + +_DEFAULT_FEEDBACK_DESCRIPTION = ( + "Send feedback about this server to its developers. Most important: report a missing " + "capability whenever no available tool fits your task, even if you can work around it " + '(feedback_type "missing_capability"). Also welcome: a tool that failed or confused you, ' + "an unhelpful error, or something that worked well. This records the feedback; it does " + "not add or change tools. Do not include user PII or sensitive content." +) + +_SEND_FEEDBACK_RESULT_TEXT = ( + "Your feedback was recorded for the server's developers. No additional tools are " + "available - continue with the tools already listed." +) + +_CORE_FEEDBACK_SCHEMA_PROPERTIES: Dict[str, Dict[str, Any]] = { + "feedback_type": { + "type": "string", + "enum": list(_FEEDBACK_TYPES), + "description": ( + "What kind of feedback this is. Use 'missing_capability' when the tool you " + "needed does not exist in the tool list - nothing failed, the capability is " + "absent (this is the most valuable report; send it even if you found a " + "workaround). Use 'issue' when an existing tool behaved badly: it failed, " + "returned a confusing error, or its description or schema misled you. Use " + "'praise' when something worked notably well. Use 'other' for anything else." + ), + }, + "summary": { + "type": "string", + "description": ( + "One self-contained sentence. For 'missing_capability': the capability you " + "needed, e.g. 'No tool to delete multiple cohorts in one call.' For 'issue': " + "the tool and the problem, e.g. 'query-trends rejects relative date ranges " + "with an unclear error.'" + ), + }, + "details": { + "type": "string", + "description": ( + "Optional longer context: what you tried, exact parameter values, the error " + "text you saw, and any workaround you used. Omit when the summary says it all." + ), + }, + "friction_points": { + "type": "string", + "description": ( + "Optional: the specific moments that slowed you down, as short bullet-like " + "sentences, quoting exact tool names, parameters, or error text." + ), + }, + "suggested_improvement": { + "type": "string", + "description": ( + "Optional: the concrete change that would have helped, e.g. the tool to add, " + "the description to reword, or the error message to improve." + ), + }, + "tool_name": { + "type": "string", + "description": ( + "Optional: the existing tool this feedback is about (for 'issue' or 'praise'). " + "Leave empty for 'missing_capability' - the point is that no tool fits." + ), + }, + "sentiment": { + "type": "string", + "enum": list(_SENTIMENTS), + "description": "Optional: how the experience felt overall.", + }, + "task_completed": { + "type": "boolean", + "description": "Optional: whether you still completed the user's task despite the problem.", + }, +} + +# Extra-property names a host may not declare: the core fields themselves, the +# names whose `$mcp_feedback_` property would collide with a core property +# (`type` -> `$mcp_feedback_type`, `tool` -> `$mcp_feedback_tool`), and the +# SDK-injected analytics arguments — the report is parsed from the raw arguments +# before those are stripped, so an extra by the same name would capture an +# SDK-owned value. +_RESERVED_EXTRA_PROPERTY_KEYS = frozenset(_CORE_FEEDBACK_SCHEMA_PROPERTIES) | { + "type", + "tool", + "context", + "conversation_id", + "llm_model", +} + + +def resolve_collect_feedback_options( + config: Union[bool, CollectFeedbackOptions, None], +) -> Optional[CollectFeedbackOptions]: + """``collect_feedback`` normalized to its object form; ``None`` when the + feature is off.""" + if not config: + return None + return CollectFeedbackOptions() if config is True else config + + +def resolve_send_feedback_tool_name(options: Optional[CollectFeedbackOptions]) -> str: + """The configured name of the virtual tool, falling back to the default. + Resolve through here everywhere (inject + detect) so a custom name can't drift.""" + name = options.tool_name if options is not None else None + return name or SEND_FEEDBACK_TOOL_NAME + + +def get_feedback_tool_descriptor( + options: Optional[CollectFeedbackOptions] = None, +) -> Dict[str, Any]: + """The advertised descriptor: the core feedback schema plus the host's declared + ``extra_properties`` (plain dict; adapters build the framework's Tool object + from it). Raises ``ValueError`` on a config error (a reserved extra key, or an + ``extra_required`` entry that was never declared) so a bad setup fails at + configuration time instead of silently corrupting the advertised schema.""" + options = options or CollectFeedbackOptions() + extra_properties = options.extra_properties or {} + extra_required = options.extra_required or [] + + for key in extra_properties: + if key in _RESERVED_EXTRA_PROPERTY_KEYS: + raise ValueError( + f'collect_feedback extra_properties key "{key}" collides with a core or ' + "SDK-reserved send_feedback field. Rename it." + ) + for key in extra_required: + if key not in extra_properties: + raise ValueError( + f'collect_feedback extra_required key "{key}" is not declared in extra_properties.' + ) + + # Deep copies: the host may reuse or mutate the fragments it handed us, and + # schema injection mutates the advertised descriptor. + properties: Dict[str, Any] = copy.deepcopy(_CORE_FEEDBACK_SCHEMA_PROPERTIES) + if extra_properties: + properties.update(copy.deepcopy(extra_properties)) + + return { + "name": resolve_send_feedback_tool_name(options), + "description": options.description or _DEFAULT_FEEDBACK_DESCRIPTION, + "inputSchema": { + "type": "object", + "properties": properties, + "required": ["feedback_type", "summary", *extra_required], + }, + "annotations": { + "title": "Send feedback", + "readOnlyHint": True, + # Interacts with an external entity: the report lands in analytics. + "openWorldHint": True, + # Only records the feedback, so repeat calls are harmless — and + # advertising it as idempotent makes agents more willing to call it + # proactively. + "idempotentHint": True, + "destructiveHint": False, + }, + } + + +def _read_string(value: Any) -> Optional[str]: + return value if isinstance(value, str) and value.strip() else None + + +def _matches_extra_schema(value: Any, schema: Dict[str, Any]) -> bool: + """True when the value conforms to the declared fragment's ``type`` and + ``enum`` — the same advisory-schema enforcement the core fields get, so + ``extras`` only ever holds schema-conforming values and a misbehaving agent + shows up as absence rather than as an unexpected shape in the host's handler.""" + if isinstance(value, list): + actual = "array" + elif value is None: + actual = "null" + elif isinstance(value, bool): + actual = "boolean" + elif isinstance(value, (int, float)): + actual = "number" + elif isinstance(value, str): + actual = "string" + else: + actual = "object" + declared = schema.get("type") + if declared == "integer" and actual == "number": + # A JSON Schema `integer` also accepts a whole-valued float (`3.0`), but + # not a fractional one (`3.5`) - Python's numeric tower conflates int and + # float here, so the type name alone can't tell them apart. + if not ( + isinstance(value, int) or (isinstance(value, float) and value.is_integer()) + ): + return False + elif declared != actual: + return False + enum = schema.get("enum") + return not isinstance(enum, list) or value in enum + + +def parse_feedback_report( + args: Optional[Dict[str, Any]], + options: Optional[CollectFeedbackOptions] = None, +) -> FeedbackReport: + """Parse the raw ``send_feedback`` arguments into a typed report. Never raises: + an invalid ``feedback_type`` falls back to ``other``, missing fields stay + ``None``, and only **declared** extras whose values match their declared + ``type``/``enum`` are lifted into ``extras`` — mismatches and anything the + agent invented reach the handler via ``raw`` only and are never captured.""" + raw = args or {} + declared = (options.extra_properties if options is not None else None) or {} + extras = { + key: raw[key] + for key, schema in declared.items() + if raw.get(key) is not None and _matches_extra_schema(raw[key], schema) + } + feedback_type = raw.get("feedback_type") + sentiment = raw.get("sentiment") + task_completed = raw.get("task_completed") + return FeedbackReport( + feedback_type=feedback_type if feedback_type in _FEEDBACK_TYPES else "other", + summary=_read_string(raw.get("summary")) or "", + sentiment=sentiment if sentiment in _SENTIMENTS else None, + friction_points=_read_string(raw.get("friction_points")), + suggested_improvement=_read_string(raw.get("suggested_improvement")), + details=_read_string(raw.get("details")), + tool_name=_read_string(raw.get("tool_name")), + task_completed=task_completed if isinstance(task_completed, bool) else None, + extras=extras, + raw=dict(raw), + ) + + +def build_feedback_intent(report: FeedbackReport) -> str: + """The report's free text, used as the event's ``$mcp_intent``.""" + return "\n\n".join(part for part in (report.summary, report.details) if part) + + +def _truncate_feedback_text(value: str, max_length: int) -> str: + return value[:max_length] + _TRUNCATION_SUFFIX if len(value) > max_length else value + + +def _capture_free_text(value: str) -> str: + """Agent-narrated free text can contain a secret the LLM read aloud or personal + data it narrated, so it gets exactly the ``$mcp_intent`` pass + (``sanitize_free_text``: credentials -> structured PII -> URLs — the order is + load-bearing, the URL rewrite would percent-encode the ``@`` the email pattern + anchors on), then a length bound. The event pipeline does not process + ``event["properties"]``, so this happens here.""" + return _truncate_feedback_text(sanitize_free_text(value), _MAX_FEEDBACK_TEXT_LENGTH) + + +def _capture_extra_value(value: Any) -> Any: + """A declared extra is agent-supplied like the core free-text fields, so its + string leaves get the same free-text pass (with the key-based redaction + ``sanitize_free_text_value`` keeps for nested objects), then non-scalars are + JSON-stringified and everything is bounded. Unserializable values are dropped.""" + sanitized = sanitize_free_text_value(value) + if isinstance(sanitized, str): + return _truncate_feedback_text(sanitized, _MAX_FEEDBACK_TEXT_LENGTH) + if sanitized is None or isinstance(sanitized, (bool, int, float)): + return sanitized + try: + return _truncate_feedback_text(json.dumps(sanitized), _MAX_FEEDBACK_TEXT_LENGTH) + except Exception: # noqa: BLE001 - capture must never raise into the tool path + return None + + +def build_feedback_event_properties(report: FeedbackReport) -> JsonRecord: + """The ``$mcp_feedback_*`` event properties for one report, declared extras + included.""" + properties: JsonRecord = { + PostHogMCPAnalyticsProperty.FEEDBACK_TYPE: report.feedback_type + } + if report.summary: + properties[PostHogMCPAnalyticsProperty.FEEDBACK_SUMMARY] = _capture_free_text( + report.summary + ) + if report.sentiment: + properties[PostHogMCPAnalyticsProperty.FEEDBACK_SENTIMENT] = report.sentiment + if report.friction_points: + properties[PostHogMCPAnalyticsProperty.FEEDBACK_FRICTION_POINTS] = ( + _capture_free_text(report.friction_points) + ) + if report.suggested_improvement: + properties[PostHogMCPAnalyticsProperty.FEEDBACK_SUGGESTED_IMPROVEMENT] = ( + _capture_free_text(report.suggested_improvement) + ) + if report.details: + properties[PostHogMCPAnalyticsProperty.FEEDBACK_DETAILS] = _capture_free_text( + report.details + ) + if report.tool_name: + # Nominally an identifier, but the schema can't stop an agent from + # writing prose into it — so it gets the same free-text pass as the + # other fields. + properties[PostHogMCPAnalyticsProperty.FEEDBACK_TOOL] = _truncate_feedback_text( + sanitize_free_text(report.tool_name), + _MAX_FEEDBACK_TOOL_NAME_LENGTH, + ) + if report.task_completed is not None: + properties[PostHogMCPAnalyticsProperty.FEEDBACK_TASK_COMPLETED] = ( + report.task_completed + ) + for key, value in report.extras.items(): + captured = _capture_extra_value(value) + if captured is not None: + properties[f"$mcp_feedback_{key}"] = captured + return properties + + +def send_feedback_result() -> Dict[str, Any]: + """The canned acknowledgement returned to the agent after it calls + ``send_feedback``. Reply with this from a custom dispatcher; the + ``instrument()`` path returns it automatically, or the string your + ``on_feedback`` handler returned instead.""" + return {"content": [{"type": "text", "text": _SEND_FEEDBACK_RESULT_TEXT}]} + + +def send_feedback_result_text() -> str: + return _SEND_FEEDBACK_RESULT_TEXT + + +async def handle_feedback( + report: FeedbackReport, options: Optional[CollectFeedbackOptions] = None +) -> str: + """Run the host's ``on_feedback`` handler (when configured) and return the + reply text. A returned non-blank string replaces the default acknowledgement; + a raised handler is logged and falls back to it — feedback capture must never + break the agent's turn.""" + # Only the type: the summary is agent-narrated free text (possible PII, + # newlines for log forging, unbounded length) and does not belong in host logs. + log(f"Agent feedback reported ({report.feedback_type})") + if options is not None and options.on_feedback is not None: + try: + reply = await _maybe_await(options.on_feedback(report)) + if isinstance(reply, str) and reply.strip(): + return reply + except Exception as error: # noqa: BLE001 - never break the agent's turn + # Only the exception's type, matching the report log above: a + # handler can echo the unsanitized report (PII, credentials, + # log-forging newlines, unbounded length) into its error message, + # and that agent-controlled text does not belong in host logs + # any more than `report.summary` does. + log( + "Warning: on_feedback handler threw " + f"({type(error).__name__}); returning the default acknowledgement" + ) + return _SEND_FEEDBACK_RESULT_TEXT diff --git a/posthog/mcp/posthog_mcp.py b/posthog/mcp/posthog_mcp.py index 8aac5a8b8..614b96157 100644 --- a/posthog/mcp/posthog_mcp.py +++ b/posthog/mcp/posthog_mcp.py @@ -26,6 +26,7 @@ from ._exceptions import capture_exception from ._instrumentation import drain_pending_sync, fire_and_forget from ._lib_identity import apply_mcp_lib_identity +from .logger import log from ._model_parameters import ( add_model_parameter_to_schema, can_inject_model_parameter, @@ -35,8 +36,18 @@ resolve_model, ) from ._sink import McpCaptureOptions, McpEventSink +from .feedback import ( + build_feedback_event_properties, + build_feedback_intent, + get_feedback_tool_descriptor, + parse_feedback_report, + resolve_collect_feedback_options, + resolve_send_feedback_tool_name, +) from .tools import build_report_missing_descriptor from .types import ( + CollectFeedbackOptions, + FeedbackReport, JsonRecord, MCPAnalyticsContextOptions, MCPAnalyticsModelOptions, @@ -51,9 +62,9 @@ class PostHogMCP(Client): """A drop-in posthog ``Client`` with ``capture_tool_call`` / ``capture_initialize`` - / ``capture_tools_list`` / ``capture_missing_capability`` plus ``prepare_tool_list`` - and ``prepare_tool_call`` helpers. ``capture``, ``flush``, ``shutdown``, feature - flags, etc. all work unchanged.""" + / ``capture_tools_list`` / ``capture_missing_capability`` / ``capture_feedback`` + plus ``prepare_tool_list`` and ``prepare_tool_call`` helpers. ``capture``, + ``flush``, ``shutdown``, feature flags, etc. all work unchanged.""" def __init__( self, @@ -61,6 +72,7 @@ def __init__( 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, ) -> None: super().__init__(api_key, **kwargs) @@ -69,6 +81,25 @@ def __init__( self._missing_capability_tool_name = ( missing_capability_tool_name or _GET_MORE_TOOLS_NAME ) + # `None` is the enable switch's off state: without it, prepare_tool_call + # must never claim a call named like the virtual tool — the host may have + # a real tool by that name, and flagging it would shadow the real handler. + # `on_feedback` is ignored on this path: the host dispatcher routes + # reports itself via PreparedToolCall.feedback_report. + self._collect_feedback = resolve_collect_feedback_options(collect_feedback) + self._feedback_tool_name = resolve_send_feedback_tool_name( + self._collect_feedback + ) + # Fail fast on a config error (reserved extra key, undeclared + # extra_required) instead of first surfacing it when a tools/list is served. + if self._collect_feedback is not None: + get_feedback_tool_descriptor(self._collect_feedback) + if self._collect_feedback.on_feedback is not None: + log( + "Warning: collect_feedback.on_feedback is ignored on the PostHogMCP " + "path - route reports from your dispatcher via " + "prepare_tool_call().feedback_report instead." + ) # Whether a failed tool call fans out an `$exception` sibling event. Distinct # from the inherited Client.enable_exception_autocapture (global uncaught-error # hook); this mirrors instrument()'s enable_exception_autocapture, default on. @@ -268,6 +299,52 @@ def capture_missing_capability( _apply_model(event, llm_model, llm_model_source) self._emit(event) + def capture_feedback( + self, + *, + report: FeedbackReport, + llm_model: Optional[str] = None, + llm_model_source: Optional[MCPAnalyticsModelSource] = None, + protocol_version: Optional[str] = None, + distinct_id: Optional[str] = None, + session_id: Optional[str] = None, + client_user_agent: Optional[str] = None, + vendor_client: Optional[str] = None, + set_properties: Optional[JsonRecord] = None, + groups: Optional[Dict[str, str]] = None, + properties: Optional[JsonRecord] = None, + timestamp: Optional[datetime] = None, + ) -> None: + """Capture a ``send_feedback`` call as an agent-feedback report. Emits + ``$mcp_feedback`` with the report's ``$mcp_feedback_*`` properties and its + summary/details as ``$mcp_intent``. Reply to the agent with + ``send_feedback_result()`` (or a custom text) after routing the report to + your own feedback backend.""" + event = self._base_event( + MCPAnalyticsEventType.MCP_FEEDBACK, + distinct_id, + session_id, + set_properties, + groups, + properties, + timestamp, + client_user_agent, + vendor_client, + ) + event["resource_name"] = self._feedback_tool_name + event["protocol_version"] = protocol_version + # Deliberately no `parameters`: the arguments are agent-narrated free + # text, and the PII-redacted `$mcp_feedback_*` properties are the captured + # surface. Raw arguments would bypass that redaction. Feedback properties + # win over the caller's, matching the instrument() path's merge order. + event["properties"] = { + **(properties or {}), + **build_feedback_event_properties(report), + } + _apply_intent(event, build_feedback_intent(report), "context_parameter") + _apply_model(event, llm_model, llm_model_source) + self._emit(event) + # --- prepare helpers ----------------------------------------------------- def prepare_tool_list( @@ -275,12 +352,16 @@ def prepare_tool_list( tools: List[Any], context: Union[bool, MCPAnalyticsContextOptions] = True, report_missing: bool = False, + collect_feedback: bool = False, ) -> List[Any]: """Inject the ``context`` argument into every tool so agents state their intent (captured as ``$mcp_intent``), and optionally append the - ``get_more_tools`` virtual tool (``report_missing=True``). Returns a new - list; dict tools are copied, context injection mutates tool objects in - place, and model injection copies them to preserve field ownership.""" + ``get_more_tools`` virtual tool (``report_missing=True``) and the + ``send_feedback`` virtual tool (``collect_feedback=True``, which also + requires the constructor's ``collect_feedback`` option — the enable switch + that gates detection in :meth:`prepare_tool_call`). Returns a new list; + dict tools are copied, context injection mutates tool objects in place, + and model injection copies them to preserve field ownership.""" prepared = [] context_description = get_context_description(context) for tool in tools: @@ -297,6 +378,12 @@ def prepare_tool_list( prepared.append( build_report_missing_descriptor(self._missing_capability_tool_name) ) + if ( + collect_feedback + and self._collect_feedback is not None + and not any(_tool_name(t) == self._feedback_tool_name for t in prepared) + ): + prepared.append(get_feedback_tool_descriptor(self._collect_feedback)) prepared = self._inject_models(prepared) return prepared @@ -309,9 +396,16 @@ def prepare_tool_call( original_tool: Any = None, ) -> PreparedToolCall: """Pull the agent's intent off the injected ``context`` argument, strip - ``context`` from the arguments, and flag the ``get_more_tools`` virtual tool. - When model capture is enabled, resolve its value and source and strip - the SDK-owned ``llm_model`` argument before dispatch.""" + ``context`` from the arguments, and flag the ``get_more_tools`` and + ``send_feedback`` virtual tools (the latter only with the constructor's + ``collect_feedback`` opt-in, so a real tool by that name is never + shadowed). When model capture is enabled, resolve its value and source and + strip the SDK-owned ``llm_model`` argument before dispatch. + + ``original_tool`` is the application's own tool for ``name``, from the + host's un-prepared list (the virtual tools never exist there). Passing it + also disambiguates a name collision: a real tool by the feedback tool's + name is dispatched normally instead of being flagged as feedback.""" raw_context = (args or {}).get("context") intent = ( raw_context.strip() @@ -334,6 +428,16 @@ def prepare_tool_call( prepared_args = _strip_context(args) if analytics_owns_model: prepared_args = _strip_model(prepared_args) + # A supplied `original_tool` is a real application tool by this name (it + # comes from the host's own list, which never holds the virtual tool), so + # the real tool wins — the stateless twin of instrument()'s listing-derived + # shadow flag. Without it the name match stands, and the documented remedy + # for a collision is configuring a non-colliding `tool_name`. + is_feedback = ( + self._collect_feedback is not None + and name == self._feedback_tool_name + and original_tool is None + ) return PreparedToolCall( args=prepared_args, intent=intent, @@ -341,6 +445,12 @@ def prepare_tool_call( llm_model=llm_model, llm_model_source=llm_model_source, is_missing_capability=name == self._missing_capability_tool_name, + is_feedback=is_feedback, + feedback_report=( + parse_feedback_report(args, self._collect_feedback) + if is_feedback + else None + ), ) # --- internals ----------------------------------------------------------- @@ -385,10 +495,19 @@ def _emit(self, event: Dict[str, Any]) -> None: # flush()/shutdown() able to drain without blocking their own event loop's tasks. fire_and_forget(self._mcp_sink.capture(event, options), self, background=True) + def _is_virtual_tool_name(self, name: Any) -> bool: + """The SDK's own virtual tools carry their intent in their own arguments, + so they never get the ``context`` parameter injected. The feedback name + only counts with the constructor opt-in — without it a real tool by that + name is an ordinary tool.""" + if name == self._missing_capability_tool_name: + return True + return self._collect_feedback is not None and name == self._feedback_tool_name + def _inject_context(self, tool: Any, description: Optional[str]) -> Any: if isinstance(tool, dict): name = tool.get("name", "unknown") - if name == self._missing_capability_tool_name: + if self._is_virtual_tool_name(name): return tool new_schema = add_context_parameter_to_schema( tool.get("inputSchema"), name, description @@ -396,7 +515,7 @@ def _inject_context(self, tool: Any, description: Optional[str]) -> Any: return {**tool, "inputSchema": new_schema} name = getattr(tool, "name", "unknown") - if name == self._missing_capability_tool_name: + if self._is_virtual_tool_name(name): return tool new_schema = add_context_parameter_to_schema( getattr(tool, "inputSchema", None), name, description diff --git a/posthog/mcp/types.py b/posthog/mcp/types.py index c97581200..d2f3afdd3 100644 --- a/posthog/mcp/types.py +++ b/posthog/mcp/types.py @@ -17,7 +17,17 @@ from dataclasses import dataclass, field from datetime import datetime -from typing import Any, Awaitable, Callable, Dict, Literal, Optional, TypedDict, Union +from typing import ( + Any, + Awaitable, + Callable, + Dict, + List, + Literal, + Optional, + TypedDict, + Union, +) from .logger import LoggerFn @@ -28,6 +38,11 @@ "MCPAnalyticsModelSource", "UserIdentity", "CaptureEventData", + "CollectFeedbackConfig", + "CollectFeedbackOptions", + "FeedbackReport", + "FeedbackSentiment", + "FeedbackType", "PreparedToolCall", ] @@ -92,6 +107,67 @@ class MCPAnalyticsModelOptions: description: Optional[str] = None +FeedbackType = Literal["missing_capability", "issue", "praise", "other"] +FeedbackSentiment = Literal["positive", "neutral", "negative", "mixed"] + +# Route each report to a real backend. Return a string (sync or async) to replace +# the default acknowledgement text; a raise is logged and falls back to it. +OnFeedbackFn = Callable[["FeedbackReport"], Any] # -> Optional[str] | awaitable + + +@dataclass +class CollectFeedbackOptions: + """Object form of the ``collect_feedback`` option (``True`` uses the defaults).""" + + # Rename the ``send_feedback`` virtual tool. Set once so the tool is + # advertised and detected under the same name. + tool_name: Optional[str] = None + # Replace the default tool description. + description: Optional[str] = None + # Host-specific fields merged into the tool's advertised input schema (plain + # JSON Schema fragments, keyed by property name). Each declared key is + # captured as a ``$mcp_feedback_`` event property through the standard + # sanitize/redact/truncate pipeline; arguments the agent invents beyond the + # schema are never captured. A key that collides with a core field or an + # SDK-injected argument raises at configuration time. + extra_properties: Optional[Dict[str, Dict[str, Any]]] = None + # Keys of ``extra_properties`` to advertise as required. + extra_required: Optional[List[str]] = None + # ``instrument()`` path only — a custom dispatcher routes reports itself via + # :attr:`PreparedToolCall.feedback_report`. The ``$mcp_feedback`` event is + # captured whether or not the handler raises. + on_feedback: Optional[OnFeedbackFn] = None + + +# The ``collect_feedback`` option: ``True``/``False`` or the object form. +CollectFeedbackConfig = Union[bool, CollectFeedbackOptions] + + +@dataclass +class FeedbackReport: + """One parsed ``send_feedback`` call, as handed to ``on_feedback`` and the + custom dispatcher.""" + + # Invalid or missing values fall back to ``other``. + feedback_type: str = "other" + # One-sentence summary; empty string when the agent omitted it. + summary: str = "" + sentiment: Optional[str] = None + friction_points: Optional[str] = None + suggested_improvement: Optional[str] = None + details: Optional[str] = None + # The existing tool the feedback is about (the ``tool_name`` argument). + tool_name: Optional[str] = None + task_completed: Optional[bool] = None + # Values of the declared ``extra_properties`` fields that match their + # declared ``type``/``enum``. A value the agent sent with the wrong shape is + # left out (find it in ``raw`` if you need it), so these are safe to trust + # as declared. + extras: JsonRecord = field(default_factory=dict) + # The full raw arguments, for the handler only — never captured. + raw: JsonRecord = field(default_factory=dict) + + # request is a JSON-RPC-shaped dict; extra carries session_id / headers. IdentifyFn = Callable[ ..., Any @@ -123,6 +199,16 @@ class MCPAnalyticsOptions: # 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 + # 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 + # `$mcp_feedback` (never a `$mcp_tool_call`). Off by default. `True` uses the + # defaults; the object form renames the tool, replaces its description, + # declares host-specific extra_properties, or wires an on_feedback handler. + # Covers what `report_missing` covers (as feedback_type "missing_capability"), + # so new integrations should enable only one of the two. New field appended + # last: positional construction of the earlier fields must keep working. + collect_feedback: Union[bool, CollectFeedbackOptions] = False @dataclass @@ -145,6 +231,14 @@ class PreparedToolCall: is_missing_capability: bool = False llm_model: Optional[str] = None llm_model_source: Optional[MCPAnalyticsModelSource] = None + # True when the call targeted the ``send_feedback`` virtual tool AND the + # constructor's ``collect_feedback`` option is set. Always False without that + # opt-in, so a real tool that happens to use the name is never shadowed. + is_feedback: bool = False + # The parsed report, set only when ``is_feedback`` is True. Pass it to + # ``PostHogMCP.capture_feedback`` and to your own feedback backend, then + # reply with ``send_feedback_result()`` or a custom text. + feedback_report: Optional[FeedbackReport] = None @dataclass diff --git a/posthog/test/mcp/conftest.py b/posthog/test/mcp/conftest.py index 65c0e887d..c28407858 100644 --- a/posthog/test/mcp/conftest.py +++ b/posthog/test/mcp/conftest.py @@ -16,6 +16,7 @@ "test_fastmcp.py", "test_fastmcp_v2.py", "test_features_m4.py", + "test_feedback.py", "test_lowlevel.py", "test_review_fixes.py", ] diff --git a/posthog/test/mcp/test_feedback.py b/posthog/test/mcp/test_feedback.py new file mode 100644 index 000000000..0e9c02ae1 --- /dev/null +++ b/posthog/test/mcp/test_feedback.py @@ -0,0 +1,942 @@ +"""Tests for the ``send_feedback`` virtual tool (the ``collect_feedback`` option).""" + +import mcp.types as mcp_types +import pytest +from mcp.server.fastmcp import FastMCP +from mcp.server.lowlevel import Server + +from posthog.mcp import ( + SEND_FEEDBACK_TOOL_NAME, + CollectFeedbackOptions, + PostHogMCP, + instrument, + send_feedback_result, +) +from posthog.mcp.feedback import ( + build_feedback_event_properties, + build_feedback_intent, + get_feedback_tool_descriptor, + parse_feedback_report, + send_feedback_result_text, +) +from posthog.mcp.types import MCPAnalyticsOptions +from posthog.test.mcp._helpers import ( + FakeClient, + events_named as _events, + flush_background as _flush, +) + +_REPORT_ARGS = { + "feedback_type": "missing_capability", + "summary": "No tool to delete multiple cohorts in one call.", + "details": "Deleted 20 cohorts one by one via cohort-delete.", + "friction_points": "cohort-delete accepts a single id; no batch variant.", + "suggested_improvement": "Add a bulk delete tool.", + "sentiment": "negative", + "task_completed": True, +} + + +def make_fastmcp(): + server = FastMCP("feedback-fastmcp") + + @server.tool() + def add(a: int, b: int) -> str: + return f"sum is {a + b}" + + return server + + +def make_lowlevel(): + server = Server("feedback-lowlevel") + + @server.list_tools() + async def list_tools(): + return [ + mcp_types.Tool( + name="echo", + description="Echo", + inputSchema={ + "type": "object", + "properties": {"msg": {"type": "string"}}, + "required": ["msg"], + }, + ) + ] + + @server.call_tool() + async def call_tool(name, arguments): + return [mcp_types.TextContent(type="text", text=str(arguments.get("msg")))] + + return server + + +def _call_request(name, arguments): + return mcp_types.CallToolRequest( + method="tools/call", + params=mcp_types.CallToolRequestParams(name=name, arguments=arguments), + ) + + +async def _list_tools_lowlevel(server): + handler = server.request_handlers[mcp_types.ListToolsRequest] + return await handler(mcp_types.ListToolsRequest(method="tools/list")) + + +# --- descriptor + config validation ------------------------------------------- + + +def test_descriptor_defaults(): + descriptor = get_feedback_tool_descriptor() + assert descriptor["name"] == SEND_FEEDBACK_TOOL_NAME + assert "missing capability" in descriptor["description"] + assert descriptor["inputSchema"]["required"] == ["feedback_type", "summary"] + assert set(descriptor["inputSchema"]["properties"]) == { + "feedback_type", + "summary", + "details", + "friction_points", + "suggested_improvement", + "tool_name", + "sentiment", + "task_completed", + } + assert descriptor["annotations"] == { + "title": "Send feedback", + "readOnlyHint": True, + "openWorldHint": True, + "idempotentHint": True, + "destructiveHint": False, + } + + +def test_descriptor_merges_extras_and_custom_name(): + options = CollectFeedbackOptions( + tool_name="report_feedback", + description="Tell us.", + extra_properties={"product_area": {"type": "string", "description": "Area."}}, + extra_required=["product_area"], + ) + descriptor = get_feedback_tool_descriptor(options) + assert descriptor["name"] == "report_feedback" + assert descriptor["description"] == "Tell us." + assert descriptor["inputSchema"]["properties"]["product_area"]["type"] == "string" + assert descriptor["inputSchema"]["required"] == [ + "feedback_type", + "summary", + "product_area", + ] + + +def test_descriptor_deep_copies_host_fragments(): + fragment = {"type": "string", "enum": ["a", "b"]} + options = CollectFeedbackOptions(extra_properties={"area": fragment}) + descriptor = get_feedback_tool_descriptor(options) + fragment["enum"].append("mutated") + descriptor["inputSchema"]["properties"]["feedback_type"]["enum"].append("bogus") + assert get_feedback_tool_descriptor(options)["inputSchema"]["properties"]["area"][ + "enum" + ] == ["a", "b", "mutated"] + assert ( + "bogus" + not in get_feedback_tool_descriptor()["inputSchema"]["properties"][ + "feedback_type" + ]["enum"] + ) + + +@pytest.mark.parametrize( + "key", ["summary", "type", "tool", "context", "conversation_id", "llm_model"] +) +def test_descriptor_rejects_reserved_extra_keys(key): + options = CollectFeedbackOptions(extra_properties={key: {"type": "string"}}) + with pytest.raises(ValueError, match="collides"): + get_feedback_tool_descriptor(options) + + +def test_descriptor_rejects_undeclared_extra_required(): + options = CollectFeedbackOptions(extra_required=["ghost"]) + with pytest.raises(ValueError, match="not declared"): + get_feedback_tool_descriptor(options) + + +def test_instrument_fails_fast_on_config_error(): + server = make_fastmcp() + options = MCPAnalyticsOptions( + collect_feedback=CollectFeedbackOptions( + extra_properties={"context": {"type": "string"}} + ) + ) + with pytest.raises(ValueError, match="collides"): + instrument(server, FakeClient(), options) + + +# --- parsing ------------------------------------------------------------------- + + +def test_parse_falls_back_and_keeps_only_declared_extras(): + options = CollectFeedbackOptions(extra_properties={"area": {"type": "string"}}) + report = parse_feedback_report( + { + "feedback_type": "bogus", + "summary": " ", + "sentiment": "angry", + "task_completed": "yes", + "area": "cohorts", + "invented": "never captured", + }, + options, + ) + assert report.feedback_type == "other" + assert report.summary == "" + assert report.sentiment is None + assert report.task_completed is None + assert report.extras == {"area": "cohorts"} + assert report.raw["invented"] == "never captured" + + +def test_parse_handles_missing_arguments(): + report = parse_feedback_report(None) + assert report.feedback_type == "other" + assert report.summary == "" + assert report.extras == {} and report.raw == {} + + +def test_parse_reads_all_core_fields(): + report = parse_feedback_report({**_REPORT_ARGS, "tool_name": "cohort-delete"}) + assert report.feedback_type == "missing_capability" + assert report.summary == _REPORT_ARGS["summary"] + assert report.details == _REPORT_ARGS["details"] + assert report.friction_points == _REPORT_ARGS["friction_points"] + assert report.suggested_improvement == _REPORT_ARGS["suggested_improvement"] + assert report.tool_name == "cohort-delete" + assert report.sentiment == "negative" + assert report.task_completed is True + + +# --- event properties + intent --------------------------------------------------- + + +def test_properties_carry_all_fields_and_redact_pii(): + report = parse_feedback_report( + { + **_REPORT_ARGS, + "details": "Reach me at jane@example.com about it.", + "tool_name": "cohort-delete", + } + ) + props = build_feedback_event_properties(report) + assert props["$mcp_feedback_type"] == "missing_capability" + assert props["$mcp_feedback_summary"] == _REPORT_ARGS["summary"] + assert props["$mcp_feedback_details"] == "Reach me at [redacted] about it." + assert props["$mcp_feedback_friction_points"] == _REPORT_ARGS["friction_points"] + assert ( + props["$mcp_feedback_suggested_improvement"] + == _REPORT_ARGS["suggested_improvement"] + ) + assert props["$mcp_feedback_tool"] == "cohort-delete" + assert props["$mcp_feedback_sentiment"] == "negative" + assert props["$mcp_feedback_task_completed"] is True + + +def test_properties_bound_free_text_and_tool_name(): + report = parse_feedback_report( + {"feedback_type": "issue", "summary": "s" * 5000, "tool_name": "t" * 500} + ) + props = build_feedback_event_properties(report) + assert len(props["$mcp_feedback_summary"]) == 2048 + 3 + assert props["$mcp_feedback_summary"].endswith("...") + assert len(props["$mcp_feedback_tool"]) == 256 + 3 + + +def test_properties_capture_declared_extras_only(): + options = CollectFeedbackOptions( + extra_properties={ + "area": {"type": "string"}, + "score": {"type": "number"}, + "tags": {"type": "array"}, + } + ) + report = parse_feedback_report( + { + "feedback_type": "praise", + "summary": "Great tools.", + "area": "mail me: jane@example.com", + "score": 9, + "tags": ["a", "b"], + "invented": "nope", + }, + options, + ) + props = build_feedback_event_properties(report) + assert props["$mcp_feedback_area"] == "mail me: [redacted]" + assert props["$mcp_feedback_score"] == 9 + assert props["$mcp_feedback_tags"] == '["a", "b"]' + assert "$mcp_feedback_invented" not in props + + +def test_extras_must_match_declared_type_and_enum(): + options = CollectFeedbackOptions( + extra_properties={ + "score": {"type": "integer"}, + "channel": {"type": "string", "enum": ["web", "app"]}, + } + ) + report = parse_feedback_report( + { + "feedback_type": "praise", + "summary": "Great tools.", + "score": "very high", + "channel": "email", + }, + options, + ) + # Mismatches stay out of extras and the captured properties; raw keeps them. + assert report.extras == {} + assert report.raw["score"] == "very high" and report.raw["channel"] == "email" + props = build_feedback_event_properties(report) + assert "$mcp_feedback_score" not in props and "$mcp_feedback_channel" not in props + + conforming = parse_feedback_report( + {"feedback_type": "praise", "summary": "s", "score": 9, "channel": "web"}, + options, + ) + assert conforming.extras == {"score": 9, "channel": "web"} + + +def test_extras_declared_integer_rejects_fractional_float(): + # `isinstance(value, (int, float))` alone can't tell `3` from `3.5` - both + # are Python `number`s - so a declared `integer` extra must additionally + # check the value has no fractional part before it's trusted downstream. + options = CollectFeedbackOptions(extra_properties={"score": {"type": "integer"}}) + + fractional = parse_feedback_report( + {"feedback_type": "praise", "summary": "s", "score": 3.5}, options + ) + assert fractional.extras == {} + + whole_float = parse_feedback_report( + {"feedback_type": "praise", "summary": "s", "score": 3.0}, options + ) + assert whole_float.extras == {"score": 3.0} + + whole_int = parse_feedback_report( + {"feedback_type": "praise", "summary": "s", "score": 3}, options + ) + assert whole_int.extras == {"score": 3} + + +def test_tool_name_gets_pii_redaction(): + report = parse_feedback_report( + {"feedback_type": "issue", "summary": "s", "tool_name": "ask jane@example.com"} + ) + props = build_feedback_event_properties(report) + assert props["$mcp_feedback_tool"] == "ask [redacted]" + + +def test_pii_inside_urls_is_redacted_on_every_surface(): + # Guards the ordering of the free-text pass: running the URL rewrite before + # PII redaction percent-encodes the "@" the email pattern anchors on, letting + # the email through (the TS SDK's veria finding on this feature). + url = "https://example.com/?email=jane@example.com" + options = CollectFeedbackOptions(extra_properties={"area": {"type": "string"}}) + report = parse_feedback_report( + { + "feedback_type": "issue", + "summary": f"Login fails at {url}", + "details": f"see {url} too", + "friction_points": f"url {url} slow", + "suggested_improvement": f"fix {url}", + "tool_name": url, + "area": url, + }, + options, + ) + props = build_feedback_event_properties(report) + for key in ( + "$mcp_feedback_summary", + "$mcp_feedback_details", + "$mcp_feedback_friction_points", + "$mcp_feedback_suggested_improvement", + "$mcp_feedback_tool", + "$mcp_feedback_area", + ): + assert "jane@example.com" not in props[key], key + assert "jane%40example.com" not in props[key], key + assert "[redacted]" in props[key], key + + +def test_nested_extras_keep_key_based_redaction(): + # The feedback path walks extras with the free-text pass; nothing else + # asserts that credential-named keys inside a nested extra still redact by + # key name, so a stringify-first refactor could drop that protection silently. + options = CollectFeedbackOptions(extra_properties={"meta": {"type": "object"}}) + report = parse_feedback_report( + { + "feedback_type": "issue", + "summary": "s", + "meta": {"note": "ping jane@example.com", "password": "hunter2"}, + }, + options, + ) + props = build_feedback_event_properties(report) + assert '"password": "[redacted]"' in props["$mcp_feedback_meta"] + assert "hunter2" not in props["$mcp_feedback_meta"] + assert '"note": "ping [redacted]"' in props["$mcp_feedback_meta"] + + +def test_intent_joins_summary_and_details(): + report = parse_feedback_report(_REPORT_ARGS) + assert ( + build_feedback_intent(report) + == f"{_REPORT_ARGS['summary']}\n\n{_REPORT_ARGS['details']}" + ) + summary_only = parse_feedback_report({"summary": "Just this."}) + assert build_feedback_intent(summary_only) == "Just this." + assert build_feedback_intent(parse_feedback_report(None)) == "" + + +# --- instrument(): FastMCP ------------------------------------------------------- + + +async def test_fastmcp_advertises_and_captures_feedback(): + server = make_fastmcp() + client = FakeClient() + instrument( + server, client, MCPAnalyticsOptions(collect_feedback=True, capture_model=True) + ) + + list_handler = server._mcp_server.request_handlers[mcp_types.ListToolsRequest] + result = await list_handler(mcp_types.ListToolsRequest(method="tools/list")) + virtual = [t for t in result.root.tools if t.name == "send_feedback"] + assert virtual + schema_props = virtual[0].inputSchema["properties"] + # Its intent rides its own arguments; the model argument is still advertised. + assert "context" not in schema_props and "conversation_id" not in schema_props + assert "llm_model" in schema_props + + canned = await server._tool_manager.call_tool("send_feedback", dict(_REPORT_ARGS)) + await _flush() + + assert canned[0].text == send_feedback_result_text() + feedback = _events(client, "$mcp_feedback") + assert len(feedback) == 1 + props = feedback[0]["properties"] + assert props["$mcp_feedback_type"] == "missing_capability" + assert props["$mcp_feedback_task_completed"] is True + assert ( + props["$mcp_intent"] + == f"{_REPORT_ARGS['summary']}\n\n{_REPORT_ARGS['details']}" + ) + assert props["$mcp_intent_source"] == "context_parameter" + # The raw arguments are agent-narrated free text — never captured. + assert "$mcp_parameters" not in props + # A send_feedback call is NOT a normal tool call. + assert _events(client, "$mcp_tool_call") == [] + + +async def test_fastmcp_invalid_type_falls_back_to_other(): + server = make_fastmcp() + client = FakeClient() + instrument(server, client, MCPAnalyticsOptions(collect_feedback=True)) + + await server._tool_manager.call_tool("send_feedback", {"feedback_type": "bogus"}) + await _flush() + + feedback = _events(client, "$mcp_feedback") + assert feedback[0]["properties"]["$mcp_feedback_type"] == "other" + assert "$mcp_intent" not in feedback[0]["properties"] + + +async def test_fastmcp_collision_fails_open(): + server = make_fastmcp() + + @server.tool() + def send_feedback(note: str) -> str: + return f"real tool got {note}" + + client = FakeClient() + instrument(server, client, MCPAnalyticsOptions(collect_feedback=True)) + + list_handler = server._mcp_server.request_handlers[mcp_types.ListToolsRequest] + result = await list_handler(mcp_types.ListToolsRequest(method="tools/list")) + named = [t for t in result.root.tools if t.name == "send_feedback"] + # Advertised once — the real tool, with normal context injection. + assert len(named) == 1 + assert "context" in named[0].inputSchema["properties"] + + out = await server._tool_manager.call_tool( + "send_feedback", {"note": "hi", "context": "using the real tool"} + ) + await _flush() + + assert "real tool got hi" in str(out) + assert _events(client, "$mcp_feedback") == [] + assert _events(client, "$mcp_tool_call") + + +async def test_fastmcp_collision_fails_open_before_any_listing(): + server = make_fastmcp() + + @server.tool() + def send_feedback(note: str) -> str: + return f"real tool got {note}" + + client = FakeClient() + instrument(server, client, MCPAnalyticsOptions(collect_feedback=True)) + + # No tools/list served yet — the live registry probe must protect the tool. + out = await server._tool_manager.call_tool("send_feedback", {"note": "early"}) + await _flush() + + assert "real tool got early" in str(out) + assert _events(client, "$mcp_feedback") == [] + assert _events(client, "$mcp_tool_call") + + +async def test_fastmcp_custom_tool_name(): + server = make_fastmcp() + client = FakeClient() + instrument( + server, + client, + MCPAnalyticsOptions( + collect_feedback=CollectFeedbackOptions(tool_name="report_feedback") + ), + ) + + list_handler = server._mcp_server.request_handlers[mcp_types.ListToolsRequest] + result = await list_handler(mcp_types.ListToolsRequest(method="tools/list")) + names = [t.name for t in result.root.tools] + assert "report_feedback" in names and "send_feedback" not in names + + await server._tool_manager.call_tool( + "report_feedback", {"feedback_type": "praise", "summary": "Nice."} + ) + await _flush() + assert _events(client, "$mcp_feedback") + + +async def test_fastmcp_coexists_with_report_missing(): + server = make_fastmcp() + client = FakeClient() + instrument( + server, client, MCPAnalyticsOptions(report_missing=True, collect_feedback=True) + ) + + list_handler = server._mcp_server.request_handlers[mcp_types.ListToolsRequest] + result = await list_handler(mcp_types.ListToolsRequest(method="tools/list")) + names = [t.name for t in result.root.tools] + assert "get_more_tools" in names and "send_feedback" in names + + await server._tool_manager.call_tool("get_more_tools", {"context": "need csv"}) + await server._tool_manager.call_tool( + "send_feedback", {"feedback_type": "praise", "summary": "Nice."} + ) + await _flush() + + assert len(_events(client, "$mcp_missing_capability")) == 1 + assert len(_events(client, "$mcp_feedback")) == 1 + assert _events(client, "$mcp_tool_call") == [] + + +# --- on_feedback ----------------------------------------------------------------- + + +async def test_on_feedback_custom_reply(): + server = make_fastmcp() + client = FakeClient() + seen = [] + + def on_feedback(report): + seen.append(report) + return "Thanks - your feedback reached the team." + + instrument( + server, + client, + MCPAnalyticsOptions( + collect_feedback=CollectFeedbackOptions(on_feedback=on_feedback) + ), + ) + + canned = await server._tool_manager.call_tool("send_feedback", dict(_REPORT_ARGS)) + await _flush() + + assert canned[0].text == "Thanks - your feedback reached the team." + assert seen and seen[0].feedback_type == "missing_capability" + assert _events(client, "$mcp_feedback") + + +async def test_on_feedback_async_handler(): + server = make_fastmcp() + client = FakeClient() + + async def on_feedback(report): + return "async thanks" + + instrument( + server, + client, + MCPAnalyticsOptions( + collect_feedback=CollectFeedbackOptions(on_feedback=on_feedback) + ), + ) + + canned = await server._tool_manager.call_tool("send_feedback", dict(_REPORT_ARGS)) + await _flush() + assert canned[0].text == "async thanks" + + +async def test_on_feedback_raise_falls_back_and_still_captures(): + server = make_fastmcp() + client = FakeClient() + + def on_feedback(report): + raise RuntimeError("backend down") + + instrument( + server, + client, + MCPAnalyticsOptions( + collect_feedback=CollectFeedbackOptions(on_feedback=on_feedback) + ), + ) + + canned = await server._tool_manager.call_tool("send_feedback", dict(_REPORT_ARGS)) + await _flush() + + assert canned[0].text == send_feedback_result_text() + assert _events(client, "$mcp_feedback") + + +async def test_on_feedback_raise_does_not_log_agent_text(): + # A raising backend can echo the unsanitized report (PII, credentials, + # forged newlines) into its exception message; the warning log must not + # repeat it, mirroring the report log just above it that logs only the type. + from posthog.mcp import set_logger + + server = make_fastmcp() + client = FakeClient() + secret_summary = "credit card 4242-4242-4242-4242 jane@example.com" + + def on_feedback(report): + raise RuntimeError(f"backend rejected: {report.summary}") + + instrument( + server, + client, + MCPAnalyticsOptions( + collect_feedback=CollectFeedbackOptions(on_feedback=on_feedback) + ), + ) + + messages = [] + set_logger(messages.append) + try: + canned = await server._tool_manager.call_tool( + "send_feedback", {**dict(_REPORT_ARGS), "summary": secret_summary} + ) + await _flush() + finally: + set_logger(None) + + assert canned[0].text == send_feedback_result_text() + assert any("on_feedback handler threw" in message for message in messages) + assert not any(secret_summary in message for message in messages) + + +# --- instrument(): low-level v1 ---------------------------------------------------- + + +async def test_lowlevel_advertises_and_captures_feedback(): + server = make_lowlevel() + client = FakeClient() + instrument(server, client, MCPAnalyticsOptions(collect_feedback=True)) + + result = await _list_tools_lowlevel(server) + assert "send_feedback" in [t.name for t in result.root.tools] + + call_handler = server.request_handlers[mcp_types.CallToolRequest] + out = await call_handler(_call_request("send_feedback", dict(_REPORT_ARGS))) + await _flush() + + assert out.root.isError is False + assert out.root.content[0].text == send_feedback_result_text() + feedback = _events(client, "$mcp_feedback") + assert len(feedback) == 1 + assert "$mcp_parameters" not in feedback[0]["properties"] + assert _events(client, "$mcp_tool_call") == [] + + +async def test_lowlevel_collision_fails_open_after_listing(): + server = Server("feedback-lowlevel-collision") + + @server.list_tools() + async def list_tools(): + return [ + mcp_types.Tool( + name="send_feedback", + description="A real application tool", + inputSchema={ + "type": "object", + "properties": {"note": {"type": "string"}}, + }, + ) + ] + + @server.call_tool() + async def call_tool(name, arguments): + return [mcp_types.TextContent(type="text", text="real tool ran")] + + client = FakeClient() + instrument(server, client, MCPAnalyticsOptions(collect_feedback=True)) + + result = await _list_tools_lowlevel(server) + assert [t.name for t in result.root.tools].count("send_feedback") == 1 + + call_handler = server.request_handlers[mcp_types.CallToolRequest] + out = await call_handler(_call_request("send_feedback", {"note": "hi"})) + await _flush() + + assert out.root.content[0].text == "real tool ran" + assert _events(client, "$mcp_feedback") == [] + assert _events(client, "$mcp_tool_call") + + +def _make_paged_lowlevel(pages): + """A raw low-level server whose tools/list handler serves ``pages`` (a list of + tool lists) one page per request, chained by ``nextCursor``. The paged handler + is registered directly into ``request_handlers`` so the wire pagination shape + is exact; the real ``send_feedback`` handler answers ``real tool ran``.""" + server = Server("feedback-lowlevel-paged") + + @server.call_tool() + async def call_tool(name, arguments): + return [mcp_types.TextContent(type="text", text="real tool ran")] + + async def paged_list(req): + cursor = getattr(getattr(req, "params", None), "cursor", None) if req else None + index = int(cursor) if cursor else 0 + next_cursor = str(index + 1) if index + 1 < len(pages) else None + return mcp_types.ServerResult( + mcp_types.ListToolsResult(tools=list(pages[index]), nextCursor=next_cursor) + ) + + server.request_handlers[mcp_types.ListToolsRequest] = paged_list + return server + + +def _list_page(server, cursor=None): + handler = server.request_handlers[mcp_types.ListToolsRequest] + params = mcp_types.PaginatedRequestParams(cursor=cursor) if cursor else None + return handler(mcp_types.ListToolsRequest(method="tools/list", params=params)) + + +_REAL_SEND_FEEDBACK = mcp_types.Tool( + name="send_feedback", + description="A real application tool", + inputSchema={"type": "object", "properties": {"note": {"type": "string"}}}, +) +_ECHO_TOOL = mcp_types.Tool( + name="echo", + description="Echo", + inputSchema={"type": "object", "properties": {"msg": {"type": "string"}}}, +) + + +@pytest.mark.parametrize("real_tool_page", [0, 1]) +async def test_paginated_listing_keeps_collision_across_pages(real_tool_page): + # A collision seen on any page must survive the other pages: recomputing the + # flag from one page alone would re-arm interception and swallow the real + # tool's calls (and an early page must not advertise the virtual tool before + # a later page reveals the real one). + pages = [[_ECHO_TOOL], [_ECHO_TOOL]] + pages[real_tool_page] = [_REAL_SEND_FEEDBACK] + server = _make_paged_lowlevel(pages) + client = FakeClient() + instrument(server, client, MCPAnalyticsOptions(collect_feedback=True)) + + page_one = await _list_page(server) + page_two = await _list_page(server, cursor="1") + listed = [t.name for t in page_one.root.tools] + [ + t.name for t in page_two.root.tools + ] + assert listed.count("send_feedback") == 1 # the real tool only, never appended + + call_handler = server.request_handlers[mcp_types.CallToolRequest] + out = await call_handler(_call_request("send_feedback", {"note": "hi"})) + await _flush() + + assert out.root.content[0].text == "real tool ran" + assert _events(client, "$mcp_feedback") == [] + assert _events(client, "$mcp_tool_call") + + +async def test_paginated_listing_appends_virtual_tool_once_on_final_page(): + server = _make_paged_lowlevel([[_ECHO_TOOL], [_ECHO_TOOL]]) + client = FakeClient() + instrument(server, client, MCPAnalyticsOptions(collect_feedback=True)) + + page_one = await _list_page(server) + page_two = await _list_page(server, cursor="1") + assert [t.name for t in page_one.root.tools] == ["echo"] # non-final: no append + assert [t.name for t in page_two.root.tools] == ["echo", "send_feedback"] + + +async def test_feedback_never_mints_conversation_id(): + server = make_lowlevel() + client = FakeClient() + instrument( + server, + client, + MCPAnalyticsOptions(collect_feedback=True, enable_conversation_id=True), + ) + + 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"] + + 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. + assert len(out.root.content) == 1 + + +# --- PostHogMCP custom dispatcher --------------------------------------------------- + + +def make_client(**kwargs): + client = PostHogMCP("phc_test", host="https://us.i.posthog.com", **kwargs) + captured = [] + client.capture = lambda event, **kw: captured.append({"event": event, **kw}) + return client, captured + + +def test_posthogmcp_constructor_fails_fast_on_config_error(): + with pytest.raises(ValueError, match="collides"): + PostHogMCP( + "phc_test", + collect_feedback=CollectFeedbackOptions( + extra_properties={"llm_model": {"type": "string"}} + ), + ) + + +async def test_posthogmcp_prepare_tool_list_appends_descriptor(): + client, _ = make_client(collect_feedback=True) + tools = [{"name": "search", "inputSchema": {"type": "object", "properties": {}}}] + + prepared = client.prepare_tool_list(tools, collect_feedback=True) + names = [t["name"] for t in prepared] + assert names == ["search", "send_feedback"] + virtual = prepared[-1] + # The virtual tool never gets the injected context argument. + assert "context" not in virtual["inputSchema"]["properties"] + + # Not appended without the per-call flag, nor over a real tool by the name. + assert len(client.prepare_tool_list(tools)) == 1 + collided = client.prepare_tool_list( + [ + { + "name": "send_feedback", + "inputSchema": {"type": "object", "properties": {}}, + } + ], + collect_feedback=True, + ) + assert [t["name"] for t in collided] == ["send_feedback"] + + +async def test_posthogmcp_prepare_tool_list_requires_constructor_option(): + client, _ = make_client() + tools = [{"name": "search", "inputSchema": {"type": "object", "properties": {}}}] + prepared = client.prepare_tool_list(tools, collect_feedback=True) + assert [t["name"] for t in prepared] == ["search"] + + +async def test_posthogmcp_prepare_tool_call_flags_feedback(): + client, _ = make_client(collect_feedback=True) + call = client.prepare_tool_call("send_feedback", dict(_REPORT_ARGS)) + assert call.is_feedback is True + assert call.feedback_report is not None + assert call.feedback_report.feedback_type == "missing_capability" + + ordinary = client.prepare_tool_call("search", {"q": "x"}) + assert ordinary.is_feedback is False and ordinary.feedback_report is None + + +async def test_posthogmcp_prepare_tool_call_without_opt_in_never_flags(): + client, _ = make_client() + call = client.prepare_tool_call("send_feedback", dict(_REPORT_ARGS)) + assert call.is_feedback is False and call.feedback_report is None + + +async def test_posthogmcp_original_tool_wins_name_collision(): + # A host whose own list holds a real `send_feedback` tool passes it as + # `original_tool`; the call then dispatches as a real tool call instead of + # being swallowed as feedback — the stateless twin of instrument()'s + # listing-derived shadow flag. + client, _ = make_client(collect_feedback=True) + real_tool = { + "name": "send_feedback", + "inputSchema": {"type": "object", "properties": {"note": {"type": "string"}}}, + } + call = client.prepare_tool_call( + "send_feedback", {"note": "hi"}, original_tool=real_tool + ) + assert call.is_feedback is False and call.feedback_report is None + + # Without `original_tool` the name match stands (TS parity). + virtual = client.prepare_tool_call("send_feedback", dict(_REPORT_ARGS)) + assert virtual.is_feedback is True and virtual.feedback_report is not None + + +async def test_posthogmcp_capture_feedback_event_shape(): + client, captured = make_client(collect_feedback=True) + call = client.prepare_tool_call("send_feedback", dict(_REPORT_ARGS)) + client.capture_feedback( + report=call.feedback_report, + distinct_id="user_1", + session_id="s1", + llm_model="claude-opus-4-8", + llm_model_source="self_reported", + properties={"host_prop": "kept", "$mcp_feedback_type": "spoofed"}, + ) + await _flush() + + events = _events(captured, "$mcp_feedback") + assert len(events) == 1 + props = events[0]["properties"] + assert props["$mcp_resource_name"] == "send_feedback" + # Feedback properties win over the caller's, matching the instrument() path. + assert props["$mcp_feedback_type"] == "missing_capability" + assert props["$mcp_intent"].startswith(_REPORT_ARGS["summary"]) + assert props["$mcp_llm_model"] == "claude-opus-4-8" + assert props["host_prop"] == "kept" + assert "$mcp_parameters" not in props + assert events[0]["distinct_id"] == "user_1" + + +def test_posthogmcp_warns_when_on_feedback_is_set(): + from posthog.mcp import set_logger + + messages = [] + set_logger(messages.append) + try: + make_client( + collect_feedback=CollectFeedbackOptions(on_feedback=lambda report: None) + ) + finally: + set_logger(None) + assert any("on_feedback is ignored" in message for message in messages) + + +def test_send_feedback_result_shape(): + result = send_feedback_result() + assert result["content"][0]["type"] == "text" + assert result["content"][0]["text"] == send_feedback_result_text() diff --git a/posthog/test/mcp/test_v2_lowlevel.py b/posthog/test/mcp/test_v2_lowlevel.py index f55b95f95..4d53407ed 100644 --- a/posthog/test/mcp/test_v2_lowlevel.py +++ b/posthog/test/mcp/test_v2_lowlevel.py @@ -493,6 +493,68 @@ async def test_report_missing_appends_virtual_tool(): assert missing and missing[0]["properties"]["$mcp_intent"] == "need an email tool" +async def test_collect_feedback_appends_virtual_tool(): + server = make_server() + client = FakeClient() + instrument(server, client, MCPAnalyticsOptions(collect_feedback=True)) + + result = await _list_tools(server) + assert "send_feedback" in [t.name for t in result.tools] + + call_result = await _call_tool( + server, + "send_feedback", + {"feedback_type": "issue", "summary": "add rejects floats."}, + ) + await _flush() + + assert call_result.is_error is False + feedback = _events(client, "$mcp_feedback") + assert len(feedback) == 1 + assert feedback[0]["properties"]["$mcp_feedback_type"] == "issue" + assert "$mcp_parameters" not in feedback[0]["properties"] + assert _events(client, "$mcp_tool_call") == [] + + +async def test_collect_feedback_collision_fails_open_after_listing(): + async def on_call_tool(ctx, params): + return mcp_types.CallToolResult( + content=[mcp_types.TextContent(type="text", text="real tool ran")] + ) + + async def on_list_tools(ctx, params): + return mcp_types.ListToolsResult( + tools=[ + mcp_types.Tool( + name="send_feedback", + description="A real application tool", + input_schema={ + "type": "object", + "properties": {"note": {"type": "string"}}, + }, + ) + ] + ) + + server = Server( + "low-v2-feedback-collision", + on_call_tool=on_call_tool, + on_list_tools=on_list_tools, + ) + client = FakeClient() + instrument(server, client, MCPAnalyticsOptions(collect_feedback=True)) + + result = await _list_tools(server) + assert [t.name for t in result.tools].count("send_feedback") == 1 + + out = await _call_tool(server, "send_feedback", {"note": "hi"}) + await _flush() + + assert out.content[0].text == "real tool ran" + assert _events(client, "$mcp_feedback") == [] + assert _events(client, "$mcp_tool_call") + + async def test_callbacks_can_read_headers_through_the_helper(): """The same `identify` body must work on both SDK majors: `extra["ctx"]` is the SDK's own context and `get_request_headers` normalises the read.""" diff --git a/posthog/test/mcp/test_v2_mcpserver.py b/posthog/test/mcp/test_v2_mcpserver.py index fb938fb26..80ae82ce6 100644 --- a/posthog/test/mcp/test_v2_mcpserver.py +++ b/posthog/test/mcp/test_v2_mcpserver.py @@ -379,6 +379,83 @@ async def test_report_missing_accepts_omitted_arguments(): assert "$mcp_intent" not in missing[0]["properties"] +async def test_collect_feedback_advertises_and_captures(): + server = make_server() + client = FakeClient() + instrument(server, client, MCPAnalyticsOptions(collect_feedback=True)) + + listed = await _list_tools(server) + assert "send_feedback" in [t.name for t in listed.tools] + + result = await _call_tool( + server, + "send_feedback", + { + "feedback_type": "missing_capability", + "summary": "No email tool.", + "details": "Wanted to notify a teammate.", + }, + ) + await _flush() + + assert result.is_error is False + feedback = _events(client, "$mcp_feedback") + assert len(feedback) == 1 + props = feedback[0]["properties"] + 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 + # A send_feedback call is NOT a normal tool call. + assert _events(client, "$mcp_tool_call") == [] + + +async def test_collect_feedback_collision_fails_open(): + server = make_server() + + @server.tool() + def send_feedback(note: str) -> str: + return f"real tool got {note}" + + client = FakeClient() + instrument(server, client, MCPAnalyticsOptions(collect_feedback=True)) + + listed = await _list_tools(server) + assert [t.name for t in listed.tools].count("send_feedback") == 1 + + await _call_tool(server, "send_feedback", {"note": "hi", "context": "real tool"}) + await _flush() + + assert _events(client, "$mcp_feedback") == [] + assert _events(client, "$mcp_tool_call") + + +async def test_collect_feedback_collision_keeps_conversation_id(): + # The name collision must fail open for every feature keyed off the + # feedback tool name, not just dispatch - conversation-id resolution used + # to keep skipping the real tool because it checked the configured name + # alone, ignoring the listing-derived shadow flag. + server = make_server() + + @server.tool() + def send_feedback(note: str) -> str: + return f"real tool got {note}" + + client = FakeClient() + instrument( + server, + client, + MCPAnalyticsOptions(collect_feedback=True, enable_conversation_id=True), + ) + + await _list_tools(server) + await _call_tool(server, "send_feedback", {"note": "hi", "context": "real tool"}) + await _flush() + + calls = _events(client, "$mcp_tool_call") + assert len(calls) == 1 + assert calls[0]["properties"].get("$mcp_conversation_id") + + async def test_instrument_is_idempotent(): server = make_server() client = FakeClient() diff --git a/references/public_api_snapshot.txt b/references/public_api_snapshot.txt index 503e324b2..bf7211fb2 100644 --- a/references/public_api_snapshot.txt +++ b/references/public_api_snapshot.txt @@ -332,6 +332,8 @@ alias posthog.inner_tag -> posthog.contexts.tag alias posthog.integrations.django.Client -> posthog.client.Client alias posthog.integrations.django.contexts -> posthog.contexts alias posthog.mcp.CaptureEventData -> posthog.mcp.types.CaptureEventData +alias posthog.mcp.CollectFeedbackOptions -> posthog.mcp.types.CollectFeedbackOptions +alias posthog.mcp.FeedbackReport -> posthog.mcp.types.FeedbackReport alias posthog.mcp.MCPAnalyticsContextOptions -> posthog.mcp.types.MCPAnalyticsContextOptions alias posthog.mcp.MCPAnalyticsModelOptions -> posthog.mcp.types.MCPAnalyticsModelOptions alias posthog.mcp.MCPAnalyticsModelSource -> posthog.mcp.types.MCPAnalyticsModelSource @@ -343,6 +345,7 @@ alias posthog.mcp.PostHogMCPAnalyticsEvent -> posthog.mcp.constants.PostHogMCPAn alias posthog.mcp.PostHogMCPAnalyticsProperty -> posthog.mcp.constants.PostHogMCPAnalyticsProperty alias posthog.mcp.PostHogMcpStatelessSessionMiddleware -> posthog.mcp.asgi.PostHogMcpStatelessSessionMiddleware alias posthog.mcp.PreparedToolCall -> posthog.mcp.types.PreparedToolCall +alias posthog.mcp.SEND_FEEDBACK_TOOL_NAME -> posthog.mcp.feedback.SEND_FEEDBACK_TOOL_NAME alias posthog.mcp.SessionTokenPayload -> posthog.mcp.session_token.SessionTokenPayload alias posthog.mcp.UserIdentity -> posthog.mcp.types.UserIdentity alias posthog.mcp.__version__ -> posthog.mcp.version.__version__ @@ -361,6 +364,7 @@ alias posthog.mcp.encode_session_id -> posthog.mcp.session_token.encode_session_ alias posthog.mcp.get_mcp_session -> posthog.mcp.asgi.get_mcp_session alias posthog.mcp.get_more_tools_result -> posthog.mcp.tools.get_more_tools_result alias posthog.mcp.get_request_headers -> posthog.mcp.request_headers.get_request_headers +alias posthog.mcp.send_feedback_result -> posthog.mcp.feedback.send_feedback_result alias posthog.mcp.set_logger -> posthog.mcp.logger.set_logger alias posthog.metrics_capture.VERSION -> posthog.version.VERSION alias posthog.metrics_capture.remove_trailing_slash -> posthog.utils.remove_trailing_slash @@ -741,6 +745,7 @@ attribute posthog.mcp.asgi.PostHogMcpStatelessSessionMiddleware.app = app attribute posthog.mcp.constants.POSTHOG_MCP_ANALYTICS_SOURCE = 'posthog_mcp_analytics' attribute posthog.mcp.constants.PostHogMCPAnalyticsEvent.CUSTOM = '$mcp_custom' attribute posthog.mcp.constants.PostHogMCPAnalyticsEvent.EXCEPTION = '$exception' +attribute posthog.mcp.constants.PostHogMCPAnalyticsEvent.FEEDBACK = '$mcp_feedback' attribute posthog.mcp.constants.PostHogMCPAnalyticsEvent.IDENTIFY = '$identify' attribute posthog.mcp.constants.PostHogMCPAnalyticsEvent.INITIALIZE = '$mcp_initialize' attribute posthog.mcp.constants.PostHogMCPAnalyticsEvent.MISSING_CAPABILITY = '$mcp_missing_capability' @@ -757,6 +762,14 @@ attribute posthog.mcp.constants.PostHogMCPAnalyticsProperty.CONVERSATION_ID = '$ attribute posthog.mcp.constants.PostHogMCPAnalyticsProperty.DURATION_MS = '$mcp_duration_ms' attribute posthog.mcp.constants.PostHogMCPAnalyticsProperty.ERROR_MESSAGE = '$mcp_error_message' attribute posthog.mcp.constants.PostHogMCPAnalyticsProperty.ERROR_TYPE = '$mcp_error_type' +attribute posthog.mcp.constants.PostHogMCPAnalyticsProperty.FEEDBACK_DETAILS = '$mcp_feedback_details' +attribute posthog.mcp.constants.PostHogMCPAnalyticsProperty.FEEDBACK_FRICTION_POINTS = '$mcp_feedback_friction_points' +attribute posthog.mcp.constants.PostHogMCPAnalyticsProperty.FEEDBACK_SENTIMENT = '$mcp_feedback_sentiment' +attribute posthog.mcp.constants.PostHogMCPAnalyticsProperty.FEEDBACK_SUGGESTED_IMPROVEMENT = '$mcp_feedback_suggested_improvement' +attribute posthog.mcp.constants.PostHogMCPAnalyticsProperty.FEEDBACK_SUMMARY = '$mcp_feedback_summary' +attribute posthog.mcp.constants.PostHogMCPAnalyticsProperty.FEEDBACK_TASK_COMPLETED = '$mcp_feedback_task_completed' +attribute posthog.mcp.constants.PostHogMCPAnalyticsProperty.FEEDBACK_TOOL = '$mcp_feedback_tool' +attribute posthog.mcp.constants.PostHogMCPAnalyticsProperty.FEEDBACK_TYPE = '$mcp_feedback_type' attribute posthog.mcp.constants.PostHogMCPAnalyticsProperty.INTENT = '$mcp_intent' attribute posthog.mcp.constants.PostHogMCPAnalyticsProperty.INTENT_SOURCE = '$mcp_intent_source' attribute posthog.mcp.constants.PostHogMCPAnalyticsProperty.IS_ERROR = '$mcp_is_error' @@ -775,6 +788,7 @@ attribute posthog.mcp.constants.PostHogMCPAnalyticsProperty.TOOL_CATEGORY = '$mc attribute posthog.mcp.constants.PostHogMCPAnalyticsProperty.TOOL_DESCRIPTION = '$mcp_tool_description' attribute posthog.mcp.constants.PostHogMCPAnalyticsProperty.TOOL_NAME = '$mcp_tool_name' attribute posthog.mcp.constants.PostHogMCPAnalyticsProperty.VENDOR_CLIENT = '$mcp_vendor_client' +attribute posthog.mcp.feedback.SEND_FEEDBACK_TOOL_NAME = 'send_feedback' attribute posthog.mcp.session_token.MCP_SESSION_HEADER = 'mcp-session-id' attribute posthog.mcp.session_token.SessionTokenPayload.client_name: Optional[str] = None attribute posthog.mcp.session_token.SessionTokenPayload.client_version: Optional[str] = None @@ -782,11 +796,30 @@ attribute posthog.mcp.session_token.SessionTokenPayload.protocol_version: Option attribute posthog.mcp.session_token.SessionTokenPayload.session_id: str attribute posthog.mcp.types.CaptureEventData.event: str attribute posthog.mcp.types.CaptureEventData.properties: Optional[JsonRecord] = None +attribute posthog.mcp.types.CollectFeedbackConfig = Union[bool, CollectFeedbackOptions] +attribute posthog.mcp.types.CollectFeedbackOptions.description: Optional[str] = None +attribute posthog.mcp.types.CollectFeedbackOptions.extra_properties: Optional[Dict[str, Dict[str, Any]]] = None +attribute posthog.mcp.types.CollectFeedbackOptions.extra_required: Optional[List[str]] = None +attribute posthog.mcp.types.CollectFeedbackOptions.on_feedback: Optional[OnFeedbackFn] = None +attribute posthog.mcp.types.CollectFeedbackOptions.tool_name: Optional[str] = None +attribute posthog.mcp.types.FeedbackReport.details: Optional[str] = None +attribute posthog.mcp.types.FeedbackReport.extras: JsonRecord = field(default_factory=dict) +attribute posthog.mcp.types.FeedbackReport.feedback_type: str = 'other' +attribute posthog.mcp.types.FeedbackReport.friction_points: Optional[str] = None +attribute posthog.mcp.types.FeedbackReport.raw: JsonRecord = field(default_factory=dict) +attribute posthog.mcp.types.FeedbackReport.sentiment: Optional[str] = None +attribute posthog.mcp.types.FeedbackReport.suggested_improvement: Optional[str] = None +attribute posthog.mcp.types.FeedbackReport.summary: str = '' +attribute posthog.mcp.types.FeedbackReport.task_completed: Optional[bool] = None +attribute posthog.mcp.types.FeedbackReport.tool_name: Optional[str] = None +attribute posthog.mcp.types.FeedbackSentiment = Literal['positive', 'neutral', 'negative', 'mixed'] +attribute posthog.mcp.types.FeedbackType = Literal['missing_capability', 'issue', 'praise', 'other'] attribute posthog.mcp.types.MCPAnalyticsContextOptions.description: Optional[str] = None 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.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_exception_autocapture: bool = True @@ -797,8 +830,10 @@ attribute posthog.mcp.types.MCPAnalyticsOptions.logger: Optional[LoggerFn] = Non attribute posthog.mcp.types.MCPAnalyticsOptions.missing_capability_tool_name: Optional[str] = None attribute posthog.mcp.types.MCPAnalyticsOptions.report_missing: bool = False attribute posthog.mcp.types.PreparedToolCall.args: Optional[JsonRecord] = None +attribute posthog.mcp.types.PreparedToolCall.feedback_report: Optional[FeedbackReport] = None attribute posthog.mcp.types.PreparedToolCall.intent: Optional[str] = None attribute posthog.mcp.types.PreparedToolCall.intent_source: Optional[str] = None +attribute posthog.mcp.types.PreparedToolCall.is_feedback: bool = False attribute posthog.mcp.types.PreparedToolCall.is_missing_capability: bool = False attribute posthog.mcp.types.PreparedToolCall.llm_model: Optional[str] = None attribute posthog.mcp.types.PreparedToolCall.llm_model_source: Optional[MCPAnalyticsModelSource] = None @@ -986,13 +1021,15 @@ 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, **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] = False, 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) -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) +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.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) class posthog.poller.Poller(interval, execute, *args, **kwargs) @@ -1182,6 +1219,7 @@ function posthog.join() -> None function posthog.load_feature_flags() function posthog.mcp.asgi.autowire_stateless_mint(server: Any) -> None function posthog.mcp.asgi.get_mcp_session(request_or_scope: Any) -> Optional[SessionTokenPayload] +function posthog.mcp.feedback.send_feedback_result() -> Dict[str, Any] function posthog.mcp.instrument(server: Any, posthog_client: Optional[Client] = None, options: Optional[MCPAnalyticsOptions] = None) -> McpAnalytics function posthog.mcp.logger.set_logger(logger: Optional[LoggerFn]) -> None function posthog.mcp.request_headers.get_request_headers(extra: Any) -> Optional[RequestHeaderBag] @@ -1397,13 +1435,14 @@ method posthog.integrations.django.PosthogContextMiddleware.extract_tags(request method posthog.integrations.django.PosthogContextMiddleware.process_exception(request, exception) method posthog.mcp.McpAnalytics.capture(event: str, properties: Optional[dict] = None) -> None method posthog.mcp.McpAnalytics.flush() -> None +method posthog.mcp.posthog_mcp.PostHogMCP.capture_feedback(*, report: FeedbackReport, llm_model: Optional[str] = None, llm_model_source: Optional[MCPAnalyticsModelSource] = None, protocol_version: Optional[str] = None, distinct_id: Optional[str] = None, session_id: Optional[str] = None, client_user_agent: Optional[str] = None, vendor_client: Optional[str] = None, set_properties: Optional[JsonRecord] = None, groups: Optional[Dict[str, str]] = None, properties: Optional[JsonRecord] = None, timestamp: Optional[datetime] = None) -> None method posthog.mcp.posthog_mcp.PostHogMCP.capture_initialize(*, client_name: Optional[str] = None, client_version: Optional[str] = None, protocol_version: Optional[str] = None, parameters: Any = None, response: Any = None, duration_ms: Optional[float] = None, distinct_id: Optional[str] = None, session_id: Optional[str] = None, client_user_agent: Optional[str] = None, vendor_client: Optional[str] = None, set_properties: Optional[JsonRecord] = None, groups: Optional[Dict[str, str]] = None, properties: Optional[JsonRecord] = None, timestamp: Optional[datetime] = None) -> None method posthog.mcp.posthog_mcp.PostHogMCP.capture_missing_capability(*, context: Optional[str] = None, llm_model: Optional[str] = None, llm_model_source: Optional[MCPAnalyticsModelSource] = None, parameters: Any = None, protocol_version: Optional[str] = None, distinct_id: Optional[str] = None, session_id: Optional[str] = None, client_user_agent: Optional[str] = None, vendor_client: Optional[str] = None, set_properties: Optional[JsonRecord] = None, groups: Optional[Dict[str, str]] = None, properties: Optional[JsonRecord] = None, timestamp: Optional[datetime] = None) -> None method posthog.mcp.posthog_mcp.PostHogMCP.capture_tool_call(tool_name: str, *, intent: Optional[str] = None, intent_source: Optional[str] = None, parameters: Any = None, response: Any = None, duration_ms: Optional[float] = None, is_error: bool = False, error: Any = None, error_type: Optional[str] = None, category: Optional[str] = None, tool_description: Optional[str] = None, llm_model: Optional[str] = None, llm_model_source: Optional[MCPAnalyticsModelSource] = None, protocol_version: Optional[str] = None, distinct_id: Optional[str] = None, session_id: Optional[str] = None, client_user_agent: Optional[str] = None, vendor_client: Optional[str] = None, set_properties: Optional[JsonRecord] = None, groups: Optional[Dict[str, str]] = None, properties: Optional[JsonRecord] = None, timestamp: Optional[datetime] = None) -> None method posthog.mcp.posthog_mcp.PostHogMCP.capture_tools_list(*, tool_names: Optional[List[str]] = None, parameters: Any = None, response: Any = None, duration_ms: Optional[float] = None, is_error: bool = False, error: Any = None, error_type: Optional[str] = None, protocol_version: Optional[str] = None, distinct_id: Optional[str] = None, session_id: Optional[str] = None, client_user_agent: Optional[str] = None, vendor_client: Optional[str] = None, set_properties: Optional[JsonRecord] = None, groups: Optional[Dict[str, str]] = None, properties: Optional[JsonRecord] = None, timestamp: Optional[datetime] = None) -> None method posthog.mcp.posthog_mcp.PostHogMCP.flush(timeout_seconds: Optional[float] = 10) -> None method posthog.mcp.posthog_mcp.PostHogMCP.prepare_tool_call(name: str, args: Optional[JsonRecord] = None, *, request_meta: Optional[JsonRecord] = None, original_tool: Any = None) -> PreparedToolCall -method posthog.mcp.posthog_mcp.PostHogMCP.prepare_tool_list(tools: List[Any], context: Union[bool, MCPAnalyticsContextOptions] = True, report_missing: bool = False) -> List[Any] +method posthog.mcp.posthog_mcp.PostHogMCP.prepare_tool_list(tools: List[Any], context: Union[bool, MCPAnalyticsContextOptions] = True, report_missing: bool = False, collect_feedback: bool = False) -> List[Any] method posthog.mcp.posthog_mcp.PostHogMCP.shutdown() -> None method posthog.metrics_capture.PostHogMetrics.count(name: str, value: float = 1, unit: Optional[str] = None, attributes: Optional[dict] = None) -> None method posthog.metrics_capture.PostHogMetrics.flush() -> None @@ -1491,6 +1530,7 @@ module posthog.integrations.django module posthog.mcp module posthog.mcp.asgi module posthog.mcp.constants +module posthog.mcp.feedback module posthog.mcp.logger module posthog.mcp.posthog_mcp module posthog.mcp.request_headers