feat(mcp): capture agents feedback - #939
Conversation
Add an opt-in collect_feedback option that injects a send_feedback virtual tool and captures every call as a $mcp_feedback event, porting PostHog/posthog-js#4870 to the Python MCP analytics SDK. Generated-By: PostHog Desktop Task-Id: 0b5063cb-6fcf-4364-be5e-de945b1448f0
posthog-python Compliance ReportDate: 2026-09-11 15:02:00 UTC ✅ All Tests Passed!111/111 tests passed Capture_V1 Tests✅ 94/94 tests passed View Details
Feature_Flags Tests✅ 17/17 tests passed View Details
|
- redact structured PII in tool_name like the other free-text fields - enforce declared type/enum on extras: mismatches stay out of extras and the captured properties (raw keeps everything for the handler) - warn when on_feedback is set on the PostHogMCP path, where it is ignored - align capture_feedback's property precedence with the instrument() path (feedback properties win over caller-supplied ones) - correct the reserved-keys comment: SDK-injected arguments do reach raw because the report is parsed before stripping Generated-By: PostHog Desktop Task-Id: 0b5063cb-6fcf-4364-be5e-de945b1448f0
The TS SDK's sanitizer re-serializes URLs before PII redaction, which percent-encodes "@" and hides emails from the email pattern. The Python sanitizer does no URL rewriting, so the case already redacts correctly; this test keeps it that way if URL scrubbing is ever added. Generated-By: PostHog Desktop Task-Id: 0b5063cb-6fcf-4364-be5e-de945b1448f0
…back-tool # Conflicts: # posthog/mcp/_instrument_lowlevel.py # posthog/mcp/_instrument_v2.py
…k capture Main's URL-credential sanitizer (#928) percent-encodes the @ the email pattern anchors on, so sanitize-then-redact let PII inside URLs through the feedback fields. Free text (summary, details, friction_points, suggested_improvement, tool_name) now uses the $mcp_intent pass (credentials -> PII -> URLs), and extras use a new sanitize_free_text_value walker that applies it per string leaf while keeping key-based redaction. sanitize_intent is renamed sanitize_free_text: the pass is no longer intent-specific. Ports posthog-js#4870 commits 9d3b3933, f4a69ea8, 270c4630. Generated-By: PostHog Desktop Task-Id: 0b5063cb-6fcf-4364-be5e-de945b1448f0
gesh
left a comment
There was a problem hiding this comment.
Note
🤖 Automated comment by QA Swarm — not written by a human
QA Swarm review complete. See inline comments.
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 1 · PR risk: 0/10 |
|
Note 🤖 Automated comment by QA Swarm — not written by a human Multi-perspective review: router (cheap-first pass) + delegated reviewers (qa-team, paul-reviewer, xp-reviewer, security-audit as warranted) Verdict: ✅ APPROVE (round 3 @ 899a219)Round 3 reviewed the review-fix commit (899a219: JSON-Schema-correct integer check for declared extras, exception-type-only logging in the on_feedback catch, and the conversation-id shadow guard) and found no issues: the bool-vs-integer classification order holds, Key findingsNone this round. ConvergenceNone — single reviewer per round (low-danger increments, nothing delegated). Reviewer summaries
Previous rounds (2)round 1 @ be63255 — ✅ APPROVE: faithful, well-tested port of posthog-js#4870; 1 LOW (raw low-level v2 pre-listing collision window, documented accepted limitation); 469 tests, ruff, mypy clean. Automated by QA Swarm — not a human review |
|
Compute _is_sdk_virtual_tool once per mutate_tool_schema call instead of twice per tool per tools/list dispatch. Generated-By: PostHog Desktop Task-Id: 0b5063cb-6fcf-4364-be5e-de945b1448f0
- _matches_extra_schema: a declared "integer" extra accepted any float, including fractional ones (3.5), since Python's numeric tower conflates int and float; now requires the value to be whole. - handle_feedback: an on_feedback exception was logged with str(error) verbatim, letting agent-controlled report text (PII, credentials, log-forging newlines) an error message echoes reach host logs; now logs only the exception type, matching the report log beside it. - start_tool_call_lifecycle: conversation-id resolution skipped every call named like the feedback tool regardless of the listing-derived shadow flag, so a real tool that collided with the configured feedback name never got a conversation id even once ownership was known; now mirrors ToolCallLifecycle.is_feedback's fail-open guard. Addresses greptile-apps findings on PR #939. Generated-By: PostHog Desktop Task-Id: 0b5063cb-6fcf-4364-be5e-de945b1448f0
… prepare_tool_call On the custom-dispatcher path prepare_tool_list skips injecting the virtual tool when a real tool owns the feedback name, but prepare_tool_call still flagged every call by that name as feedback, so the documented dispatch flow suppressed the real tool. A host-supplied original_tool is stateless proof a real tool owns the name (the virtual tool never exists in the host's own list), so it now wins. Without original_tool the name match stands and the documented remedy is a non-colliding tool_name. Generated-By: PostHog Desktop Task-Id: 0b5063cb-6fcf-4364-be5e-de945b1448f0
…pages Each page of a paginated listing recomputed feedback_tool_shadowed from that page's tools alone, so a real send_feedback tool listed on page 1 was forgotten by page 2 — the SDK appended its virtual tool and then swallowed the real tool's calls. The flag is now sticky for the instrumentation instance's lifetime, and the virtual tool is appended only to the final page (no nextCursor), so an early page can't advertise it before a later page reveals the real tool. Generated-By: PostHog Desktop Task-Id: 0b5063cb-6fcf-4364-be5e-de945b1448f0
Problem
Agents constantly hit gaps our analytics never see: a tool they needed that does not exist, a tool that confused them, an error message that did not help. The existing
get_more_toolstool only covers capability gaps and promises "more tools" it never returns. Server owners need one honest, structured channel for agent feedback.This is the Python port of PostHog/posthog-js#4870, which shipped the same feature in
@posthog/mcp.Changes
New opt-in
collect_feedbackoption inposthog.mcp. It injects asend_feedbackvirtual tool and captures every call as a$mcp_feedbackevent. Additive:report_missing/get_more_tools/$mcp_missing_capabilityare unchanged.Enable it
Works on all instrumented server types (MCP SDK 1.x FastMCP, 2.x MCPServer, both low-level servers, standalone fastmcp).
What the agent sends
The tool description makes missing capabilities the priority category and says plainly that the call records feedback and adds no tools. The reply is an honest acknowledgement.
What lands in PostHog
One
$mcp_feedbackevent (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,$mcp_feedback_task_completed. The summary + details also map to$mcp_intent. Free-text fields get the$mcp_intentpipeline: sanitize, structured-PII redaction, length bound — and the event carries no$mcp_parameters, so the raw arguments are never captured. Invalidfeedback_typefalls back toother.Extra properties + a real backend
Declared extras merge into the advertised schema and are captured as
$mcp_feedback_<key>. Arguments the agent invents are never captured (handler-only viareport.raw). A key that collides with a core field raises at setup. A raisedon_feedbacklogs and falls back to the default reply; the event is captured either way.Custom dispatcher (
PostHogMCP)Same mechanics as
get_more_tools: fail-open on real-tool name collisions (a listing-derived shadow flag plus a live registry probe on the high-level adapters), nocontext/conversation_idinjection on the virtual tool,llm_modelcapture still works on it, and no conversation handle is minted for feedback calls.How did you test this code?
New
posthog/test/mcp/test_feedback.py(37 tests): descriptor build + reserved-key/extra_requiredvalidation, deep-copy independence, parse fallbacks,$mcp_feedbackcapture with all properties, PII redaction + length bounds, extras capture (declared only), intent building,instrument()end-to-end on FastMCP + low-level, collision fail-open (with and without a prior listing),on_feedbackcustom/async/raising handlers, custom tool name, coexistence withreport_missing, no conversation-id minting, and the fullPostHogMCPpath. Plus four mirror tests in the MCP SDK 2.x suites (test_v2_mcpserver.py,test_v2_lowlevel.py).Full runs:
pytest posthog/test/mcpgreen on the mcp 1.x leg (344 passed) and the mcp 2.x leg (289 passed), full repo suite,ruff format --check,ruff check,mypy(baseline-filtered, no new issues), and the public-API snapshot regenerated + checked.Created with PostHog Desktop