Skip to content

feat(mcp): support MCP Python SDK v2 and cross-SDK parity with TS SDK - #881

Merged
gesh merged 17 commits into
mainfrom
posthog-code/mcp-sdk-v2
Aug 21, 2026
Merged

feat(mcp): support MCP Python SDK v2 and cross-SDK parity with TS SDK#881
gesh merged 17 commits into
mainfrom
posthog-code/mcp-sdk-v2

Conversation

@gesh

@gesh gesh commented Aug 20, 2026

Copy link
Copy Markdown
Member

The 2026-07-28 MCP spec revision removed the initialize handshake and Mcp-Session-Id header, and MCP Python SDK v2 (mcp==2.0.0, now the default on pip install mcp) reshaped every seam posthog.mcp hooks. The Python twin of the @posthog/mcp v2 work (PostHog/posthog-js#4449, PostHog/posthog-js#4565).

Before this PR, instrument() crashes on mcp≥2: _compatibility.py imported mcp.server.fastmcp at module scope — gone in v2 — and the ImportError propagated straight into the host app (worse than the JS silent no-op).

Mega issue: PostHog/posthog#64016

Fixed by this PR — was failing before

Capability Before (mcp≥2 / spec 2026-07-28) Now
instrument() on a v2 server 💥 ImportError crashes the host app at startup ✅ Instruments MCPServer and low-level Server; unknown shapes degrade to a logged no-op
Tool call capture ($mcp_tool_call) ❌ Nothing captured on v2 ✅ Captured on both protocol eras, both server levels
Error capture ($mcp_is_error + $exception) ❌ Nothing ✅ Raised errors and is_error results, re-raised untouched
tools/list + intent (context injection/stripping) ❌ Nothing ✅ Same strip-vs-leave policy as the 1.x adapters
Client identity + $mcp_protocol_version ❌ Nothing ✅ From the handshake, the session token, or the per-request 2026-07-28 _meta envelope
Sessions on 2026-07-28 (spec removed the session header) ❌ Every request its own $session_id — also on SDK 1.x ✅ Conversation-anchored $session_id with enable_conversation_id, byte-parity with @posthog/mcp, works across stateless pods — anchored only once the handle is confirmed delivered
Stateless session token on v2 (legacy era) ❌ Dead (crash) ✅ Minted at initialize, carries identity + session across pods (wire-verified)
Handlers registered after instrument() (v2) ❌ Never wrapped (the #4449 pattern) add_request_handler patched; late registrations captured
Invented conversation_id echoes (e.g. conv-1) ⚠️ Accepted verbatim — two unrelated callers could merge into one session once anchored ✅ uuidv7 minted-shape guard; invented values get a fresh mint (JS parity)
Conversation handle on a failing first call ⚠️ Prompt-back dropped on errored results — retry started a new conversation ✅ Prompt-back rides errored results (JS parity)
Conversation handle on tools with structured output ❌ Delivered only as a content text block — clients that read structuredContent never render content, so the agent had nothing to echo (0% echo rate, measured against Claude Code) ✅ Also declared on the tool's outputSchema and mirrored into structuredContent on every response
Reading HTTP headers in identify / event_properties callbacks ⚠️ v1 exposed no way to reach them from extra; v2 exposed only an undocumented path, so one callback body couldn't serve both majors (silent failure → every event anonymous) ✅ Uniform extra["ctx"] on all adapters + exported get_request_headers(extra)
Raw request ctx in captured events ⚠️ The whole extra dict was embedded in $identify parameters and stringified by truncation ✅ Callbacks still get the full ctx; captured events carry only a JSON-safe scalar projection
CI coverage ⚠️ v1 only, buried inside the whole-repo tests matrix ✅ Named checks per SDK major — MCP SDK v1 (Python 3.10/3.14) + MCP SDK v2 (Python 3.10/3.14) — plus dual-era wire tests on every PR

What changed

Test gate first

  • The MCP suite now runs twice in CI: the existing tests matrix stays on mcp>=1.26,<2; a new tests-mcp matrix runs posthog/test/mcp per SDK major — named checks MCP SDK v1/v2 (Python 3.10/3.14); the v2 legs swap in mcp>=2,<3. A conftest.py splits collection by installed major.
  • test_v2_mcpserver.py / test_v2_lowlevel.py — the v2 adapters driven directly, mirroring the v1 files.
  • test_v2_wire_dual_era.py — raw JSON-RPC over the real streamable-http app (deliberately not the SDK Client, which negotiates the legacy era) in both protocol eras, stateless topology: envelope client identity, no session header on 2026-07-28, conversation-anchored $session_id across two fresh server instances, self-encoded token surviving pods on the legacy era.
  • test_conversation_session.py — the cross-SDK derivation contract, with byte-for-byte vectors validated against posthog-js (deriveSessionIdFromConversation).
  • test_no_crash.pyinstrument() degrades to a logged no-op on both majors.
Screenshot 2026-08-20 at 16 00 03

SDK changes

  • New _instrument_v2.py: high-level MCPServer wraps ToolManager.call_tool (every dispatch routes through it; late-registered tools covered) + tools/list at the low-level registry; low-level Server wraps the string-keyed registry via the public add_request_handler/get_request_handler API — late registrations included (the #4449 lesson). Strip-vs-leave policy matches the 1.x pair.
  • Import-tolerant, shape-based detection (JS ADR-0005): probes answer False instead of raising; unsupported servers → logged no-op handle, never a crash.
  • Conversation-anchored sessions (JS ADR-0004, previously missing in Python): with enable_conversation_id, $session_id derives deterministically from the agent's conversation_id — the only correlation that survives 2026-07-28's per-request instances. Includes the minted-shape (uuidv7) guard so two callers inventing conv-1 never merge, lowercased echoes, and per-request (never sticky) resolution. New export: derive_session_id_from_conversation. Anchoring applies on 1.x too — it is not era-gated, matching JS.
    • The session is resolved only after the handle's fate is known. A handle the SDK mints is not a fact until the agent has it, so anchoring on it up front strands events in a session nobody holds whenever the prompt-back can't be delivered (a tool that raises past our seam, a result with nowhere to put it) — one orphan session per call, plus a duplicate $identify each time. Resolving after delivery gives both properties: a delivered handle anchors the call that minted it, so the whole conversation is one session; an undelivered one anchors nothing. The shared per-server session is settled before dispatch so an in-tool analytics.capture() is attributed to the current caller rather than the previous one.
    • The handle travels as data, not an instruction. It is returned as {"conversation_id": "..."} rather than a [SERVER]: Reuse ... sentence: a tool result is untrusted content, so an imperative from the server is exactly what a client's prompt-injection filter strips — and a stripped block breaks the feature silently. It also kept appearing in users' transcripts. Same payload as @posthog/mcp.
  • Prompt-back rides errored results (JS parity): a tool failing on the first call of a conversation is exactly when the agent needs the handle; previously the minted id was dropped on isError results.
  • Dual-shape reads (isError/is_error, inputSchema/input_schema, clientInfo/client_info); captured payloads dump by_alias so both majors emit the camelCase wire shape. Version advisory widened to mcp>=1.26,<3.
  • No new runtime deps; mcp stays a lazily-imported peer dependency. jlowin's fastmcp keeps the 1.x seams (it pins mcp<2).

Follow-ups found while manually testing against real clients

  • structuredContent delivery channel (new _output_instructions.py, ports posthog-js #4430/#4431). Manual testing against Claude Code showed the conversation feature was inert for any tool declaring an output schema: such clients render structuredContent and never see the content text block, so the handle was invisible and every call minted a fresh one. Now an optional _mcp_instructions key is declared on the tool's advertised outputSchema at tools/list and mirrored into structuredContent on every response. The declaration is what makes the write safe — clients validate against the advertised schema, so an undeclared key fails the customer's whole result under additionalProperties: false. Only declared tools are written to; an instance that never served a listing fails closed; composed schemas (oneOf/allOf/anyOf/$ref) and tools owning the key are skipped and stay content-only.
  • Uniform callback context + get_request_headers (new request_headers.py). Adding v2 support had left the callback contract divergent — v2 passed the request context as extra["ctx"], v1 passed only session_id — so an identify() written for one major silently got nothing on the other, and on v1 headers were unreachable from extra entirely. All four adapters now pass the SDK's own per-request context identically, and the exported helper flattens Starlette Headers, plain mappings, or any iterable of pairs into a lowercase-keyed dict (None on stdio, never raises). Mirrors @posthog/mcp's getRequestHeaders and its ADR-0006 stance: hand hosts the raw context rather than synthesising a fake uniform shape.
  • $identify no longer captures the raw request context (review finding). Callbacks still receive the full extra — including the v2 ctx so hosts can read headers — but captured event parameters now carry only a JSON-safe scalar projection, so an opaque context object can't be stringified by truncation into an event.

Deliberate 1.x behavior changes (parity with @posthog/mcp)

  1. An invented (non-uuidv7) conversation_id echo is replaced with a fresh mint instead of accepted verbatim — required before anchoring $session_id to it.
  2. Minted prompt-backs are appended to errored results too (tests updated accordingly).

Review loop (/pr-shepherd, 4 rounds)

An automated review loop found and fixed 8 further defects — 3 HIGH — after CI was already green. Each was reproduced before being fixed and each fix has a regression test verified to fail without it; round 4 came back clean. Detail and per-finding status in the QA Swarm summary comment.

The three HIGHs, all in code added by this PR:

  1. Strict output schemas broke after a tool-cache rebuild (5ed2086) — mcp 1.x validates structuredContent against its cached tool definition, rebuilt from the internal req=None listing pass we skipped injecting on. One call to an unlisted tool name and every later call to a tool with additionalProperties: false came back isError — analytics destroying a customer's successful result.
  2. Cross-caller conversation-handle leak (5ed2086) — the mirror and prompt-back mutated the caller's result object in place, so a tool returning a shared/cached result served one conversation's handle to every later caller; since the handle outranks the transport session, that collapses unrelated clients into one session and identity merging can then write one user's person properties onto another's. Both channels now copy instead of mutating.
  3. The first fix was incomplete (e335876) — the raw low-level adapter had the same cache trap on the input side, rejecting the very arguments the SDK tells agents to send.

Plus: a second tools/list silently disabling the mirror, an unguarded request_context read crashing the public FastMCP.call_tool(), a parameter marked required in the schema we strip it from (strict_input_validation=True → every call failed), a narrowed guard in the tool-call hot path, and a docstring that walked hosts toward putting a bearer token in a person property. A simplify pass (07dd1c8) reused existing helpers in the v2 adapter.

Base was also 3 commits behind, and main had changed before_send semantics (#880) and regenerated the public API snapshot this PR edits — merged and re-verified (d7f2f39).

Validation

  • v1 lane: uv sync --extra test → full posthog/test/mcp green (199 passed), plus the whole-repo suite (only pre-existing, network-dependent posthog/test/ai failures, reproduced on a clean tree).
  • v2 lane: same env with mcp==2.0.0 (fastmcp removed) → 182 passed, 8 skipped.
  • ruff format --check / ruff check, mypy (baseline clean), make public_api_check (snapshot regenerated for the new export), uv build all green.
  • Cross-SDK vectors computed from the TS deterministicPrefixedId and asserted byte-for-byte in test_conversation_session.py.

Out of scope (matching JS): prompts/resources auto-instrumentation, MRTR/input_required, tasks, subscriptions/listen, server/discover capture; posthog.com docs update (the Python example there still shows the v1 import path) is a follow-up in the docs repo.


Created with PostHog Code

gesh added 2 commits August 20, 2026 14:47
…nd v2

The suite now runs twice in CI: the existing `tests` matrix stays on
mcp>=1.26,<2, and a new `tests-mcp-v2` job swaps in mcp>=2,<3 (spec
2026-07-28) and runs posthog/test/mcp. A conftest splits collection by
installed major, since each major's seams fail at import on the other.

New coverage, red until the SDK changes land:
- test_v2_mcpserver / test_v2_lowlevel: the v2 adapters driven directly,
  mirroring the v1 files (capture, errors, intent injection/stripping,
  identify, report_missing, idempotency, late registration)
- test_v2_wire_dual_era: raw JSON-RPC over the real streamable-http app in
  both protocol eras — stateless topology, envelope identity, no session
  header on 2026-07-28, conversation-anchored sessions across two fresh
  instances, and the self-encoded token surviving pods on the legacy era
- test_conversation_session: the cross-SDK session derivation contract
  (byte-for-byte vectors against posthog-js), the minted-shape guard, and
  v1 anchoring flows
- test_no_crash: instrument() degrades to a logged no-op instead of
  crashing, on both majors

Generated-By: PostHog Code
Task-Id: ebafcb71-b03b-443d-b40c-d527ed4a04f4
instrument() now works on mcp 1.x and 2.x:

- New _instrument_v2 adapters: the high-level MCPServer (renamed FastMCP)
  wraps ToolManager.call_tool — the seam every dispatch routes through —
  and the low-level Server wraps the string-keyed handler registry through
  the public add_request_handler/get_request_handler API, late
  registrations included (the posthog-js#4449 lesson). Strip-vs-leave
  policy per entry point matches the 1.x pair: the high-level path strips
  injected context/conversation_id (v2 validates against the function
  signature), the raw low-level path leaves them optional in the schema.
- _compatibility probes are import-tolerant and shape-based (posthog-js
  ADR-0005): the old module-level `from mcp.server.fastmcp import ...`
  raised ImportError straight out of instrument() on mcp>=2, crashing the
  host app. Unsupported servers now degrade to a logged no-op handle.
- Conversation-anchored sessions (posthog-js ADR-0004): with
  enable_conversation_id, $session_id is derived deterministically from
  the agent-echoed conversation_id — the only correlation that survives
  2026-07-28's per-request instances. Byte-for-byte parity with
  @posthog/mcp (new export derive_session_id_from_conversation), the
  minted-shape (uuidv7) guard so invented handles can't merge unrelated
  callers, lowercased echoes, and the prompt-back riding errored results
  so a first-call failure keeps the conversation together. Applies to the
  1.x adapters too — anchoring is not era-gated.
- Dual-shape attribute reads (isError/is_error, inputSchema/input_schema,
  clientInfo/client_info); captured payloads dump by_alias so both majors
  emit the camelCase wire shape.
- Version advisory widened to mcp>=1.26,<3.

No new runtime dependencies; mcp stays a lazily-imported peer dependency.
jlowin's fastmcp keeps the 1.x seams (it pins mcp<2).

Generated-By: PostHog Code
Task-Id: ebafcb71-b03b-443d-b40c-d527ed4a04f4
@gesh
gesh requested a review from a team as a code owner August 20, 2026 11:48
@gesh
gesh removed the request for review from a team August 20, 2026 11:49
Generated-By: PostHog Code
Task-Id: ebafcb71-b03b-443d-b40c-d527ed4a04f4
Comment thread posthog/mcp/_instrument_v2.py
@github-actions

github-actions Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

posthog-python Compliance Report

Date: 2026-08-21 11:08:16 UTC
Duration: 256065ms

✅ All Tests Passed!

111/111 tests passed


Capture_V1 Tests

94/94 tests passed

View Details
Test Status Duration
Endpoint And Method.Targets V1 Endpoint 514ms
Endpoint And Method.Does Not Use Legacy Endpoints 509ms
Required Headers.Has Authorization Bearer Header 508ms
Required Headers.Has Content Type Json 509ms
Required Headers.Has Posthog Sdk Info Format 508ms
Required Headers.Has Posthog Attempt Header 509ms
Required Headers.Has Posthog Request Id 508ms
Required Headers.Has Posthog Request Timestamp 509ms
Required Headers.Has User Agent 508ms
Body Format.Body Has Created At And Batch 508ms
Body Format.No Api Key In Body 509ms
Body Format.No Sent At In Body 508ms
Event Format.Event Has Required Root Fields 508ms
Event Format.Event Uuid Is Valid 510ms
Event Format.Event Timestamp Is Rfc3339 508ms
Event Format.Distinct Id Is String 508ms
Event Format.Distinct Id At Root Not Properties 509ms
Event Format.Custom Properties Preserved 508ms
Event Format.Set Properties Preserved 508ms
Event Format.Set Once Properties Preserved 508ms
Event Format.Groups Properties Preserved 509ms
Event Format.Sdk Generates Uuid If Not Provided 509ms
Event Format.Event Has Required Root Fields Batch 511ms
Event Format.Event Uuid Is Valid Batch 510ms
Event Format.Event Timestamp Is Rfc3339 Batch 511ms
Event Format.Distinct Id Is String Batch 511ms
Event Format.Distinct Id At Root Not Properties Batch 511ms
Event Format.Custom Properties Preserved Batch 511ms
Event Format.Set Properties Preserved Batch 511ms
Event Format.Set Once Properties Preserved Batch 511ms
Event Format.Groups Properties Preserved Batch 511ms
Event Format.Sdk Generates Uuid If Not Provided Batch 511ms
Batch Behavior.Multiple Events In Single Batch 513ms
Batch Behavior.Batch Envelope Smoke 512ms
Batch Behavior.Flush With No Events Sends Nothing 505ms
Batch Behavior.Flush At Triggers Batch 1008ms
Batch Behavior.Created At Reflects Batch Creation Time 509ms
Deduplication.Generates Unique Uuids 514ms
Deduplication.Different Events Same Content Different Uuids 510ms
Deduplication.Preserves Uuid On Retry 6516ms
Deduplication.Preserves Timestamp On Retry 6514ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry 6520ms
Deduplication.No Duplicate Events In Batch 514ms
Header Behavior On Retry.Attempt Header Starts At One 509ms
Header Behavior On Retry.Attempt Header Increments On Retry 13517ms
Header Behavior On Retry.Request Id Preserved On Retry 6516ms
Header Behavior On Retry.Different Requests Have Different Request Ids 3017ms
Header Behavior On Retry.Request Timestamp Changes On Retry 6518ms
Response Format Validation.Success Response Has Uuid Keyed Results 508ms
Response Format Validation.Success Response Has Ok For Each Event 512ms
Response Format Validation.Success No Retry After When All Ok 511ms
Response Format Validation.Success Retry After Present When Retry Events 1513ms
Response Format Validation.Success No Retry After When Drop Only 510ms
Response Format Validation.Response Echoes Request Id 509ms
Retry Behavior.Retries On 408 6516ms
Retry Behavior.Retries On 500 6517ms
Retry Behavior.Retries On 503 8517ms
Retry Behavior.Retries On 504 6518ms
Retry Behavior.Retryable Errors Have Retry After 3514ms
Retry Behavior.Respects Retry After On Retryable Error 11521ms
Retry Behavior.Does Not Retry On 400 2511ms
Retry Behavior.Does Not Retry On 401 2512ms
Retry Behavior.Does Not Retry On 402 2512ms
Retry Behavior.Does Not Retry On 413 2511ms
Retry Behavior.Does Not Retry On 415 2511ms
Retry Behavior.Non Retryable Errors Have No Retry After 2511ms
Retry Behavior.Implements Backoff 22533ms
Retry Behavior.Max Retries Respected 22532ms
Partial Batch Handling.Handles 200 Full Success 2511ms
Partial Batch Handling.Handles 200 With All Ok 3512ms
Partial Batch Handling.Does Not Retry Dropped Events 3513ms
Partial Batch Handling.Does Not Retry Limited Events 3512ms
Partial Batch Handling.Prunes Ok Events On Partial Retry 6515ms
Partial Batch Handling.Prunes Dropped Events On Partial Retry 6518ms
Partial Batch Handling.Retries Only Retry Events From Partial 6520ms
Partial Batch Handling.Partial Retry Preserves Uuids 6520ms
Partial Batch Handling.Partial Retry Attempt Header Increments 6518ms
Partial Batch Handling.Partial Retry Request Id Preserved 6518ms
Partial Batch Handling.Respects Retry After On Partial 8514ms
Partial Batch Handling.Unknown Result Treated As Terminal 3514ms
Partial Batch Handling.Mixed Ok Drop Limited No Retry 3517ms
Compression.Sends Gzip Content Encoding 509ms
Compression.No Content Encoding When Disabled 509ms
Compression.Compressed Body Is Decompressible 508ms
Error Handling.Does Not Retry On Unknown 4Xx 2511ms
Event Options.Cookieless Mode Override 509ms
Event Options.Disable Skew Correction Override 508ms
Event Options.Process Person Profile Override 508ms
Event Options.Product Tour Id Override 509ms
Event Options.Unset Options Omitted 508ms
Event Options.Options Override In Batch 511ms
Geoip And Historical Migration.Geoip Disable Injected Into Properties 508ms
Geoip And Historical Migration.Historical Migration Set In Body 509ms
Geoip And Historical Migration.Historical Migration Absent By Default 508ms

Feature_Flags Tests

17/17 tests passed

View Details
Test Status Duration
Request Payload.Request With Person Properties Device Id 9ms
Request Payload.Flags Request Uses V2 Query Param 7ms
Request Payload.Flags Request Hits Flags Path Not Decide 7ms
Request Payload.Flags Request Omits Authorization Header 7ms
Request Payload.Token In Flags Body Matches Init 7ms
Request Payload.Groups Round Trip 7ms
Request Payload.Groups Default To Empty Object 7ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False 7ms
Request Payload.Disable Geoip Omitted Defaults To False 7ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key 7ms
Request Lifecycle.No Flags Request On Init Alone 4ms
Request Lifecycle.No Flags Request On Normal Capture 508ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests 11ms
Request Lifecycle.Mock Response Value Is Returned To Caller 7ms
Retry Behavior.Retries Flags On 502 310ms
Retry Behavior.Retries Flags On 504 311ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event 509ms

@greptile-apps

greptile-apps Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor
Prompt To Fix All With AI
### Issue 1
posthog/mcp/session.py:77-78
**Untrusted handles override session identity**

If two independent clients send the same UUIDv7-shaped `conversation_id`, this branch accepts it without proof that the SDK minted it and gives it precedence over each client's token and transport session, causing unrelated events and identity attribution to share one deterministic `$session_id`.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "feat(mcp): support MCP Python SDK v2 and..." | Re-trigger Greptile

@veria-ai

veria-ai Bot commented Aug 20, 2026

Copy link
Copy Markdown

PR overview

This pull request updates MCP instrumentation to support the MCP Python SDK v2 and align behavior across the Python and TypeScript SDKs, including FastMCP and low-level request handling.

One session-isolation issue remains open: events captured during tool execution can inherit the previous caller’s session and identity, causing incorrect cross-caller attribution and possible exposure of event properties to the wrong profile. Exploitation depends on shared server reuse and tools emitting events during execution, limiting the blast radius. One other issue has already been addressed.

Open issues (1)

Fixed/addressed: 1 · PR risk: 4/10

Comment thread posthog/mcp/session.py
@gesh gesh changed the title feat(mcp): support MCP Python SDK v2 / spec 2026-07-28, with a dual-major test gate feat(mcp): support MCP Python SDK v2 / spec 2026-07-28 Aug 20, 2026
gesh added 5 commits August 20, 2026 15:13
The v2 adapters hand the raw request ctx to identify/event_properties/
intent_fallback callbacks via `extra` so hosts can read headers — but
handle_identify embedded the whole dict into the captured $identify
parameters, where the sanitizer leaves opaque objects untouched and
truncation stringifies them: whatever the context repr carries (headers,
transport state) would ship to PostHog without key-based redaction.

Callbacks keep the full extra; captured parameters now carry only
JSON-safe scalars (e.g. session_id).

Generated-By: PostHog Code
Task-Id: ebafcb71-b03b-443d-b40c-d527ed4a04f4
Reshape tests-mcp-v2 into a tests-mcp matrix over {v1, v2} x {3.10, 3.14},
so the checks list shows an explicit per-major signal instead of the v1
side being buried inside the whole-repo tests matrix. The v1 leg uses the
lockfile's mcp 1.x as-is; the v2 leg swaps in mcp>=2,<3 and drops jlowin
fastmcp (pins mcp<2). The main tests matrix is unchanged.

Generated-By: PostHog Code
Task-Id: ebafcb71-b03b-443d-b40c-d527ed4a04f4
Manual testing against Claude Code found the conversation feature inert for
any tool that declares an output schema: clients that read structuredContent
never render the content blocks, so the [SERVER]: Reuse conversation_id text
block was invisible and the agent had nothing to echo back. Every call minted
a fresh handle, and each landed in its own session.

Ports the second delivery channel from @posthog/mcp (ADR-0004, posthog-js
 #4430/#4431), which measured the same 0% echo rate before fixing it:

- declare an optional `_mcp_instructions` key on the tool's advertised
  outputSchema at tools/list (never `required`)
- mirror {conversation_id} into the result's structuredContent on *every*
  response, not just the minting one, so an agent that dropped the handle
  can read it back

The declaration is what makes the write safe — clients validate
structuredContent against the advertised schema, so an undeclared key fails
the customer's whole tool result under additionalProperties: false. Only
tools we declared on are ever written to, and an instance that never served
a tools/list fails closed. Composed schemas (oneOf/allOf/anyOf/$ref) and
tools owning the key are skipped; the text block still carries them.

Wired into all four adapters (v1 FastMCP, v1 low-level, jlowin fastmcp, v2)
with shape-tolerant reads: the (content, structured) tuple from FastMCP 1.x's
convert_result path, CallToolResult models (structuredContent on 1.x,
structured_content on 2.x), and plain dicts.

Verified on the live playground server: outputSchema declares the key and
structuredContent carries the handle alongside the tool's own payload.

Generated-By: PostHog Code
Task-Id: ebafcb71-b03b-443d-b40c-d527ed4a04f4
Adding v2 support left the callback contract divergent: the v2 adapters
passed the request context as extra["ctx"] while the v1 adapters passed
only session_id, so an identify() written against one major silently got
nothing on the other — and on v1 there was no way to reach headers from
extra at all. Header-based identification is the main use of identify, and
the failure is invisible: no headers, no distinct id, every event
anonymous.

- all four adapters now pass the SDK's own per-request context as
  extra["ctx"], unchanged and identically shaped across majors
- new exported get_request_headers(extra) flattens Starlette Headers, plain
  mappings, or any iterable of pairs into a lowercase-keyed dict; returns
  None on stdio and never raises
- the context object still never reaches an event: the scalar projection
  added earlier for the $identify capture strips it

Ports the intent of @posthog/mcp's getRequestHeaders (ADR-0006: hand hosts
the raw context rather than synthesising a fake uniform shape, and give
them one helper for the thing they actually need).

Generated-By: PostHog Code
Task-Id: ebafcb71-b03b-443d-b40c-d527ed4a04f4
…tx read

Four defects found by the qa-swarm delegation pass (fable technical lens +
opus security lens), all reproduced before fixing:

HIGH — strict output schemas broke after a tool-cache rebuild. mcp 1.x
validates a call's structuredContent against its *cached* tool definition,
and rebuilds that cache from the internal `req=None` listing pass whenever
an unlisted tool name is called (our own advertised `get_more_tools` does
it). We skipped injecting on that pass, so the cache lost the
`_mcp_instructions` declaration while the mirror kept writing the key:
every later call to a tool with `additionalProperties: false` came back
`isError: "Additional properties are not allowed"`. The internal pass now
gets the same injections; only capture is skipped.

HIGH — cross-caller handle leak. The mirror and the prompt-back mutated the
result object in place, so a tool returning a shared/cached CallToolResult
pinned one conversation's handle onto it and served it to every later
caller — and since the handle outranks the transport session, that
collapses unrelated clients into one session, where identity merging can
write one user's person properties onto another's profile. Both channels
now copy (model_copy) instead of mutating; the low-level path rewraps and
returns the copy.

MEDIUM — a re-listing disabled the mirror. add_instructions_to_output_schema
could not tell its own prior declaration from a customer's, so servers that
return persistent Tool objects flipped ownership to False on the second
tools/list, silently switching off the feature for the schema-reading
clients it exists for (and blaming the customer in the log). It now
recognises our declaration by its description sentinel.

MEDIUM — `getattr(context, "request_context", None)` was unguarded:
FastMCP's property *raises* outside a request, and getattr only swallows
AttributeError, so the public `FastMCP.call_tool()` entry point started
raising ValueError from analytics. Read it through a guarded helper, like
every other context reader here.

Also de-footguns the get_request_headers docstring: its example resolved an
Authorization header to a user id rather than handing back the raw token,
with an explicit warning that person properties and custom event properties
are not redacted.

Regression tests for all four, each verified to fail without its fix.

Generated-By: PostHog Code
Task-Id: ebafcb71-b03b-443d-b40c-d527ed4a04f4
@gesh

gesh commented Aug 20, 2026

Copy link
Copy Markdown
Member Author

Note

🤖 Automated comment by QA Swarm — not written by a human

Multi-perspective review: router (cheap-first pass) + delegated reviewers (technical + security lenses), run as a 4-round quality loop

Verdict: ✅ APPROVE (round 4 @ d7f2f39 — loop converged, round 4 dry)

Four rounds. Eight defects found and fixed — three of them HIGH — on a PR whose CI was already green. Every finding was reproduced before being fixed, and every fix has a regression test verified to fail without it. Round 4 came back clean.

Findings and status

# Severity Finding Status
1 🟠 HIGH Strict output schemas broke after a tool-cache rebuild: mcp 1.x validates structuredContent against its cached tool definition, rebuilt from the internal req=None listing pass we skipped injecting on. One call to an unlisted name — our own get_more_tools will do it — and every later call to a tool with additionalProperties: false returned isError. Analytics destroying a customer's successful result. ✅ Fixed 5ed2086
2 🟠 HIGH Cross-caller handle leak: the mirror and prompt-back mutated the caller's result in place, so a tool returning a shared/cached CallToolResult served one conversation's handle to every later caller. Because the handle outranks the transport session, that collapses unrelated clients into one session — where identity merging writes one user's person properties onto another's profile. Reproduced on both SDK majors. ✅ Fixed 5ed2086
3 🟠 HIGH Fix #1 was incomplete — the raw low-level adapter had the same cache trap on the input side, and worse: it advertises context/conversation_id without stripping them, so a rebuilt cache rejected the very arguments we tell agents to send. ✅ Fixed e335876
4 🟡 MEDIUM A second tools/list silently disabled the mirror: add_instructions_to_output_schema couldn't tell its own prior declaration from a customer's, so servers returning persistent Tool objects flipped ownership True → False — switching the feature off for exactly the clients it exists for, and blaming the customer in the log. ✅ Fixed 5ed2086
5 🟡 MEDIUM Unguarded getattr(context, "request_context", None): FastMCP's property raises outside a request and getattr only swallows AttributeError, so the public FastMCP.call_tool() entry point began raising from analytics. Clean regression against main. ✅ Fixed 5ed2086
6 🟡 MEDIUM We marked context required in the very schema we strip it from before the SDK validates — under FastMCP(strict_input_validation=True) every call failed with 'context' is a required property. Latent on main, made permanent by fix #3's cache injection. ✅ Fixed 45f5630
7 🟢 LOW The simplify pass narrowed an exception guard, so a malformed properties could raise (or answer by substring) in the tool-call hot path. ✅ Fixed e335876
8 ⚪ NIT get_request_headers docstring example read headers.get("authorization") inside identify — one step from a bearer token landing in a person property, the one payload area redaction never touches. ✅ Fixed 5ed2086

Convergence

The in-place-mutation defect (#2) was flagged independently by both delegated reviewers — the technical lens rated it LOW (contingent on result caching), the security lens traced it through to cross-user PII contamination and rated it HIGH. Resolved at the root: the SDK no longer mutates caller-owned objects at all.

Three of the eight (#3, #6, #7) were defects in the previous round's fixes — which is the argument for the loop running to convergence rather than stopping at one pass.

Reviewer summaries

Reviewer Assessment
🧭 router (sonnet) Read every hunk with context, cross-checked the v2 SDK source, ran both lanes green. Danger MEDIUM / confidence HIGH, delegated nothing — the null result was wrong. Escalated anyway per the caller model policy (public API + broad abstraction ⇒ second-model validation); that override is why anything was found.
🔬 qa-team/correctness (fable) Rounds 1–4. Confirmed the wrapper plumbing sound (idempotency, delivered accounting, exception isolation), then found #1, #3, #4, #6, #7 — each with a runnable probe.
🔐 security-audit (opus) Confirmed the capture side clean (by_alias is a pure rename, v1/v2 field parity, the earlier _captured_extra fix holds), then found the leak running the other way: #2, #5, #8.
🧹 simplify (sonnet) Reused existing helpers in the v2 adapter (header/session-id normalisation, schema_has_param, params_to_request_dict); declined to collapse the three adapters, which diverge deliberately. 07dd1c8

Also in this iteration

  • Base updated. The branch was 3 commits behind, and main had changed before_send semantics (fix: Drop events when before_send callbacks raise #880) and regenerated references/public_api_snapshot.txt, which this PR also edits. Merged and re-verified rather than letting it fail after merge.
  • CI: 40 passing, 0 failing, no repair needed.
  • Local at d7f2f39: MCP SDK v1 197 passed · MCP SDK v2 182 passed/6 skipped · ruff, mypy, public-API snapshot clean.
Previous rounds (3)

Automated by QA Swarm — not a human review

gesh added 4 commits August 20, 2026 19:00
Reuse existing helpers in the v2 adapter instead of hand-rolled copies:
- _ctx_mcp_session_id now goes through get_request_headers +
  read_mcp_session_header, which already handle case-insensitive keys,
  list-valued headers and whitespace — strictly better than the raw
  ctx.request.headers read it replaces
- shared schema_has_param() in _context_parameters (was duplicated between
  the lowlevel and v2 adapters)
- shared params_to_request_dict() in _instrumentation (request_to_dict now
  delegates to it)
- one tool-manager lookup per call instead of two when checking ownership
  of both injected parameters

The three adapters stay separate on purpose — they hook different SDK seams
with different semantics; only genuinely identical logic is shared.

Generated-By: PostHog Code
Task-Id: ebafcb71-b03b-443d-b40c-d527ed4a04f4
Round 2 of the review loop caught that the previous commit's cache-rebuild
fix stopped at the FastMCP adapter. The raw low-level v1 adapter has the
identical exposure on the *input* side, and it is worse there: this adapter
advertises `context`/`conversation_id` without stripping them, relying on
the advertised schema doubling as the call's validation schema. So a cache
rebuilt from the un-injected internal listing pass rejects exactly the
arguments the SDK told the agent to send —
"Input validation error: Additional properties are not allowed ('context'
was unexpected)" — for any tool with `additionalProperties: false`. One
unknown tool name (or a fresh stateless instance) triggers it, and the
conversation prompt-back tells the agent to keep sending the argument, so
it persists until the next client-facing tools/list.

The internal pass now gets the same injections here too, via a shared
`_inject_tool_schemas` helper mirroring the FastMCP one. Regression test
verified to fail without the fix.

Also from round 2:
- `_tool_own_properties_v2` fails closed on a malformed schema again — the
  simplify pass had narrowed the guard so a non-dict `properties` would
  raise (or answer by substring) in the tool-call hot path
- finish the `schema_has_param` dedup the simplify pass started: the
  low-level adapter now uses the shared helper instead of its own copy

Generated-By: PostHog Code
Task-Id: ebafcb71-b03b-443d-b40c-d527ed4a04f4
Round 3 of the review loop. On the jlowin-FastMCP path the adapter strips
the injected `context`/`conversation_id` before dispatch, because that SDK
validates tool arguments against the function signature. But the listing
wrapper marked `context` *required* in the advertised schema — which is
also the schema mcp 1.x's low-level server validates the (already stripped)
arguments against. Under `FastMCP(strict_input_validation=True)` every call
therefore failed with "Input validation error: 'context' is a required
property".

Latent on main, where an un-injected cache rebuild intermittently cured it;
the previous commit's internal-pass injection made it permanent. Advertise
`context` as optional on this path instead — requiring a parameter the
validator can never see is self-contradictory, and losing the "required"
nudge only softens intent capture, where the alternative is a hard failure
of the customer's tool call.

Regression test added; the default `strict_input_validation=False` and both
MCP SDK 2.x paths were unaffected either way.

Generated-By: PostHog Code
Task-Id: ebafcb71-b03b-443d-b40c-d527ed4a04f4
@gesh gesh changed the title feat(mcp): support MCP Python SDK v2 / spec 2026-07-28 feat(mcp): support MCP Python SDK v2 and cross-SDK parity with TS SDK Aug 21, 2026
The changeset claimed "SDK 1.x paths are unchanged", which stopped being
true as the PR grew: conversation-anchored sessions, the structuredContent
delivery channel, the uuidv7 handle guard, the prompt-back on errored
results and the callback context all reach v1 servers, and three separately
reproduced bugs on v1 paths were fixed. Since no mainstream client speaks
the 2026-07-28 era yet, that half is what current users actually get.

Restructured into v2 support / cross-SDK parity / fixes affecting existing
1.x users, and calls out the two deliberate behavioural changes.

Generated-By: PostHog Code
Task-Id: ebafcb71-b03b-443d-b40c-d527ed4a04f4
the Python SDK handles initialize in the session layer, not ``request_handlers``.)"""
session_id = await resolve_session_id(data, mcp_session_id, token=token)
session_id = await resolve_session_id(
data, mcp_session_id, token=token, conversation_id=conversation_id

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blocking: Session is anchored before a minted conversation handle is delivered

prepare_request() derives the session from a newly minted conversation_id and immediately emits identify/initialize before either adapter knows whether the result can carry the prompt-back. On an MCPServer/FastMCP exception converted outside ToolManager.call_tool, or a non-injectable result, the client never receives this handle, so a retry mints another session while these events remain in an unreachable conversation-derived session. Please delay anchoring newly minted handles until delivery is confirmed, or wrap the outer error conversion so the handle is always returned; cover official FastMCP/MCPServer error and non-injectable-result paths.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed and fixed in ff81024. Reproduced first: two failing calls produced two orphan sessions, each with its own duplicate $identify and $mcp_initialize — a conversation-derived session nobody holds, and the next call mints another, which is strictly worse than not anchoring at all. Took the first option you offered: an echo is the only proof of delivery, so a freshly minted handle no longer anchors — the minting call stays in the transport/memory session it would have used anyway, and every call after the agent returns the handle joins the conversation's session, so undeliverable prompt-backs degrade to pre-feature behaviour instead of fragmenting. Covers the MCPServer/FastMCP outer-conversion path and non-injectable results alike, since neither can produce an echo. The cross-pod contract is intact where it matters: a pod receiving an echoed handle still derives the same session as any other pod with no shared state — it just starts from the echo rather than the mint. Regression test added (two failed calls must stay in one session), and the three tests that asserted the old mint-anchors behaviour now document the new contract. Worth noting @posthog/mcp anchors mints the same way today, so I'll raise the equivalent fix there.

Comment thread posthog/mcp/request_headers.py Outdated

"""Read HTTP request headers inside a host callback, on either MCP SDK major.

``identify``, ``intent_fallback``, ``event_properties`` and ``before_send``

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blocking: before_send documentation promises an unsupported extra["ctx"] argument

before_send is typed and invoked as before_send(capture) in _sink._apply_before_send; a callback following this documented (capture, extra) contract raises TypeError, which is caught and silently drops each event. Please either remove the before_send claims here, in posthog/mcp/__init__.py, and the changeset, or add backward-compatible request-context plumbing while preserving one-argument callbacks and surfacing callback-signature failures.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct — _apply_before_send calls before_send(capture) with one argument, so a callback following the documented (capture, extra) shape would raise TypeError and have every event silently dropped. Removed the claim in ff81024 from the request_headers docstring, posthog/mcp/__init__.py and the changeset rather than plumbing a request context in: before_send is handed the finished capture payload, not the request, so extra doesn't belong there — identify, intent_fallback and event_properties are the three that genuinely receive it.

@marandaneto
marandaneto requested a review from a team August 21, 2026 09:53
Review finding: prepare_request() anchored the session on a *freshly minted*
handle and emitted identify/initialize immediately — before either adapter
knew whether the prompt-back could ride the result. When it could not (an
exception converted outside ToolManager.call_tool, a result with nothing to
carry it), the client never received that handle, so the events stranded in
a conversation-derived session nobody holds and the next call minted
another. Reproduced: two failed calls produced two orphan sessions, each
with its own duplicate $identify and $mcp_initialize — strictly worse than
not anchoring at all, and the same degradation as an agent that never
echoes.

An echo is the only proof of delivery, so only echoed handles anchor now.
The minting call stays in the transport/memory session it would have used
anyway; every call after the agent returns the handle joins the
conversation's session. Failure therefore degrades to the pre-feature
behaviour instead of fragmenting.

The cross-pod contract is unchanged where it matters: a pod that receives
an echoed handle derives the same session as any other pod without shared
state — it just starts from the echo rather than the mint.

Also drops the `before_send` claim from the get_request_headers docs,
posthog/mcp/__init__.py and the changeset: before_send is invoked as
before_send(capture) with one argument, so a callback written to the
documented (capture, extra) contract would raise TypeError and have every
event silently dropped. It receives the finished payload, not the request,
so `extra` does not belong there.

Generated-By: PostHog Code
Task-Id: ebafcb71-b03b-443d-b40c-d527ed4a04f4
@gesh
gesh requested a review from marandaneto August 21, 2026 10:12
gesh added 2 commits August 21, 2026 13:32
The prompt-back shipped an imperative sentence inside the tool result:

  [SERVER]: Reuse conversation_id=... on every subsequent tool call in this
  conversation. Required for the server to correlate calls and ...

Two problems, both of which @posthog/mcp already moved away from (it emits
JSON.stringify({conversation_id}) with the same reasoning):

- Tool results are untrusted content, so a server telling the model what to
  do on every later call is exactly the shape a client's prompt-injection
  filter looks for. A stripped block means the handle never arrives and
  conversation sessions quietly stop working — the failure is invisible.
- It renders in the user's transcript. Spotted while manually testing the
  stack through Claude Desktop.

Now emits {"conversation_id": "..."} — same channel and cadence, just data.
The earlier parity sweep compared the delivery channels (text block vs
structuredContent) but not the payload inside the block, so this slipped
through.

Generated-By: PostHog Code
Task-Id: ebafcb71-b03b-443d-b40c-d527ed4a04f4
The previous fix was too blunt. It never anchored a freshly minted handle,
which removed the orphan sessions but split every conversation in two: the
call that minted the handle landed in the connection session and only the
echoes joined the conversation's. Observed live — three Claude Desktop
calls sharing one conversation_id produced two sessions.

The check was simply in the wrong place. Delivery is known a few lines
after the tool returns; the session was being resolved before the tool ran.
Resolving it after instead gives both properties at once:

  minted + delivered    -> anchors, so the whole conversation is one session
  minted + undelivered  -> anchors nothing, so no unreachable sessions
  echoed                -> anchors, unchanged

This is also a closer reading of the review comment that started it:
"delay anchoring newly minted handles until delivery is confirmed" — the
handle is now anchored exactly when confirmed, not abandoned.

prepare_request moves behind a local resolver in all four tool-call
adapters, called on each exit path with the handle that actually reached
the agent. The orphan-guard test and the mint-then-echo test now both pass,
which is the point: they were previously mutually exclusive.

Generated-By: PostHog Code
Task-Id: ebafcb71-b03b-443d-b40c-d527ed4a04f4
if not delivered:
delivered_conversation_id = None

session_id = await _session(delivered_conversation_id)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Low: In-tool events use the previous caller's session

prepare_request() now runs after original(), but McpAnalytics.capture() reads data.session_id and its cached identity while the tool is executing. A caller can trigger a tool that captures attacker-controlled or sensitive properties and have that event attributed to the previous caller's profile. Preserve a request-local session and identity before dispatch, then finalize the conversation anchor after delivery. The same ordering occurs in _instrument_lowlevel.py:252 and _instrument_v2.py:348,510.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 80c9c35 — a real regression from the previous commit. Moving prepare_request() after the call left data.session_id holding the prior request's value while the tool body ran, so an in-tool analytics.capture() was attributed to the previous caller (and through the identity cache, their person). prime_session() now settles the transport/memory session before dispatch, with the conversation anchor still finalised after delivery — an in-tool event predates that decision, so the transport session is the correct answer available at that moment. Regression test drives two callers with distinct Mcp-Session-Id headers and asserts B's body sees B's session; verified red without the fix. Applied at all four sites you listed.

Review finding, and a regression from the previous commit: moving
prepare_request() to after the call meant data.session_id still held the
*previous* request's value while the tool executed. McpAnalytics.capture()
reads that field, so a custom event emitted from inside a tool body was
attributed to the previous caller's session — and through the identity
cache, their person.

prime_session() now settles the transport/memory session before dispatch;
the conversation anchor is still finalised afterwards, once delivery is
known. An in-tool event predates that decision, so it belongs to the
transport session, which is the correct answer available at that moment.

Regression test uses two callers with distinct Mcp-Session-Id headers and
asserts B's tool body sees B's session — verified to fail without the fix
(the first version of this test passed either way and was worthless).

Generated-By: PostHog Code
Task-Id: ebafcb71-b03b-443d-b40c-d527ed4a04f4
@gesh
gesh merged commit b0ab12c into main Aug 21, 2026
43 checks passed
@gesh
gesh deleted the posthog-code/mcp-sdk-v2 branch August 21, 2026 12:03
gesh added a commit that referenced this pull request Aug 21, 2026
#881 was squash-merged and released, so sampo turned
mcp-sdk-v2-support.md into the 7.40.0 changelog entry and deleted it from
main. This branch predates that merge and still carried the file; because
the squash gave main no shared history with the branch, git saw "main never
had this file" rather than a modify/delete and silently kept it — which
would have re-emitted the entire v7.40.0 entry in the next release.

Generated-By: PostHog Code
Task-Id: ebafcb71-b03b-443d-b40c-d527ed4a04f4
gesh added a commit that referenced this pull request Aug 21, 2026
* test(mcp): dual-major test setup — run the MCP suite against SDK v1 and v2

The suite now runs twice in CI: the existing `tests` matrix stays on
mcp>=1.26,<2, and a new `tests-mcp-v2` job swaps in mcp>=2,<3 (spec
2026-07-28) and runs posthog/test/mcp. A conftest splits collection by
installed major, since each major's seams fail at import on the other.

New coverage, red until the SDK changes land:
- test_v2_mcpserver / test_v2_lowlevel: the v2 adapters driven directly,
  mirroring the v1 files (capture, errors, intent injection/stripping,
  identify, report_missing, idempotency, late registration)
- test_v2_wire_dual_era: raw JSON-RPC over the real streamable-http app in
  both protocol eras — stateless topology, envelope identity, no session
  header on 2026-07-28, conversation-anchored sessions across two fresh
  instances, and the self-encoded token surviving pods on the legacy era
- test_conversation_session: the cross-SDK session derivation contract
  (byte-for-byte vectors against posthog-js), the minted-shape guard, and
  v1 anchoring flows
- test_no_crash: instrument() degrades to a logged no-op instead of
  crashing, on both majors

Generated-By: PostHog Code
Task-Id: ebafcb71-b03b-443d-b40c-d527ed4a04f4

* feat(mcp): support MCP Python SDK v2 and the 2026-07-28 spec revision

instrument() now works on mcp 1.x and 2.x:

- New _instrument_v2 adapters: the high-level MCPServer (renamed FastMCP)
  wraps ToolManager.call_tool — the seam every dispatch routes through —
  and the low-level Server wraps the string-keyed handler registry through
  the public add_request_handler/get_request_handler API, late
  registrations included (the posthog-js#4449 lesson). Strip-vs-leave
  policy per entry point matches the 1.x pair: the high-level path strips
  injected context/conversation_id (v2 validates against the function
  signature), the raw low-level path leaves them optional in the schema.
- _compatibility probes are import-tolerant and shape-based (posthog-js
  ADR-0005): the old module-level `from mcp.server.fastmcp import ...`
  raised ImportError straight out of instrument() on mcp>=2, crashing the
  host app. Unsupported servers now degrade to a logged no-op handle.
- Conversation-anchored sessions (posthog-js ADR-0004): with
  enable_conversation_id, $session_id is derived deterministically from
  the agent-echoed conversation_id — the only correlation that survives
  2026-07-28's per-request instances. Byte-for-byte parity with
  @posthog/mcp (new export derive_session_id_from_conversation), the
  minted-shape (uuidv7) guard so invented handles can't merge unrelated
  callers, lowercased echoes, and the prompt-back riding errored results
  so a first-call failure keeps the conversation together. Applies to the
  1.x adapters too — anchoring is not era-gated.
- Dual-shape attribute reads (isError/is_error, inputSchema/input_schema,
  clientInfo/client_info); captured payloads dump by_alias so both majors
  emit the camelCase wire shape.
- Version advisory widened to mcp>=1.26,<3.

No new runtime dependencies; mcp stays a lazily-imported peer dependency.
jlowin's fastmcp keeps the 1.x seams (it pins mcp<2).

Generated-By: PostHog Code
Task-Id: ebafcb71-b03b-443d-b40c-d527ed4a04f4

* chore(mcp): refresh public API snapshot for the 0.3.0 sdk-surface bump

Generated-By: PostHog Code
Task-Id: ebafcb71-b03b-443d-b40c-d527ed4a04f4

* fix(mcp): capture only a scalar projection of extra on $identify events

The v2 adapters hand the raw request ctx to identify/event_properties/
intent_fallback callbacks via `extra` so hosts can read headers — but
handle_identify embedded the whole dict into the captured $identify
parameters, where the sanitizer leaves opaque objects untouched and
truncation stringifies them: whatever the context repr carries (headers,
transport state) would ship to PostHog without key-based redaction.

Callbacks keep the full extra; captured parameters now carry only
JSON-safe scalars (e.g. session_id).

Generated-By: PostHog Code
Task-Id: ebafcb71-b03b-443d-b40c-d527ed4a04f4

* ci(mcp): name the MCP gate per SDK major — MCP SDK v1/v2 (Python X.Y)

Reshape tests-mcp-v2 into a tests-mcp matrix over {v1, v2} x {3.10, 3.14},
so the checks list shows an explicit per-major signal instead of the v1
side being buried inside the whole-repo tests matrix. The v1 leg uses the
lockfile's mcp 1.x as-is; the v2 leg swaps in mcp>=2,<3 and drops jlowin
fastmcp (pins mcp<2). The main tests matrix is unchanged.

Generated-By: PostHog Code
Task-Id: ebafcb71-b03b-443d-b40c-d527ed4a04f4

* fix(mcp): deliver the conversation handle via structuredContent too

Manual testing against Claude Code found the conversation feature inert for
any tool that declares an output schema: clients that read structuredContent
never render the content blocks, so the [SERVER]: Reuse conversation_id text
block was invisible and the agent had nothing to echo back. Every call minted
a fresh handle, and each landed in its own session.

Ports the second delivery channel from @posthog/mcp (ADR-0004, posthog-js
 #4430/#4431), which measured the same 0% echo rate before fixing it:

- declare an optional `_mcp_instructions` key on the tool's advertised
  outputSchema at tools/list (never `required`)
- mirror {conversation_id} into the result's structuredContent on *every*
  response, not just the minting one, so an agent that dropped the handle
  can read it back

The declaration is what makes the write safe — clients validate
structuredContent against the advertised schema, so an undeclared key fails
the customer's whole tool result under additionalProperties: false. Only
tools we declared on are ever written to, and an instance that never served
a tools/list fails closed. Composed schemas (oneOf/allOf/anyOf/$ref) and
tools owning the key are skipped; the text block still carries them.

Wired into all four adapters (v1 FastMCP, v1 low-level, jlowin fastmcp, v2)
with shape-tolerant reads: the (content, structured) tuple from FastMCP 1.x's
convert_result path, CallToolResult models (structuredContent on 1.x,
structured_content on 2.x), and plain dicts.

Verified on the live playground server: outputSchema declares the key and
structuredContent carries the handle alongside the tool's own payload.

Generated-By: PostHog Code
Task-Id: ebafcb71-b03b-443d-b40c-d527ed4a04f4

* feat(mcp): uniform ctx in callbacks + exported get_request_headers

Adding v2 support left the callback contract divergent: the v2 adapters
passed the request context as extra["ctx"] while the v1 adapters passed
only session_id, so an identify() written against one major silently got
nothing on the other — and on v1 there was no way to reach headers from
extra at all. Header-based identification is the main use of identify, and
the failure is invisible: no headers, no distinct id, every event
anonymous.

- all four adapters now pass the SDK's own per-request context as
  extra["ctx"], unchanged and identically shaped across majors
- new exported get_request_headers(extra) flattens Starlette Headers, plain
  mappings, or any iterable of pairs into a lowercase-keyed dict; returns
  None on stdio and never raises
- the context object still never reaches an event: the scalar projection
  added earlier for the $identify capture strips it

Ports the intent of @posthog/mcp's getRequestHeaders (ADR-0006: hand hosts
the raw context rather than synthesising a fake uniform shape, and give
them one helper for the thing they actually need).

Generated-By: PostHog Code
Task-Id: ebafcb71-b03b-443d-b40c-d527ed4a04f4

* fix(mcp): review findings — never mutate the caller's result, guard ctx read

Four defects found by the qa-swarm delegation pass (fable technical lens +
opus security lens), all reproduced before fixing:

HIGH — strict output schemas broke after a tool-cache rebuild. mcp 1.x
validates a call's structuredContent against its *cached* tool definition,
and rebuilds that cache from the internal `req=None` listing pass whenever
an unlisted tool name is called (our own advertised `get_more_tools` does
it). We skipped injecting on that pass, so the cache lost the
`_mcp_instructions` declaration while the mirror kept writing the key:
every later call to a tool with `additionalProperties: false` came back
`isError: "Additional properties are not allowed"`. The internal pass now
gets the same injections; only capture is skipped.

HIGH — cross-caller handle leak. The mirror and the prompt-back mutated the
result object in place, so a tool returning a shared/cached CallToolResult
pinned one conversation's handle onto it and served it to every later
caller — and since the handle outranks the transport session, that
collapses unrelated clients into one session, where identity merging can
write one user's person properties onto another's profile. Both channels
now copy (model_copy) instead of mutating; the low-level path rewraps and
returns the copy.

MEDIUM — a re-listing disabled the mirror. add_instructions_to_output_schema
could not tell its own prior declaration from a customer's, so servers that
return persistent Tool objects flipped ownership to False on the second
tools/list, silently switching off the feature for the schema-reading
clients it exists for (and blaming the customer in the log). It now
recognises our declaration by its description sentinel.

MEDIUM — `getattr(context, "request_context", None)` was unguarded:
FastMCP's property *raises* outside a request, and getattr only swallows
AttributeError, so the public `FastMCP.call_tool()` entry point started
raising ValueError from analytics. Read it through a guarded helper, like
every other context reader here.

Also de-footguns the get_request_headers docstring: its example resolved an
Authorization header to a user id rather than handing back the raw token,
with an explicit warning that person properties and custom event properties
are not redacted.

Regression tests for all four, each verified to fail without its fix.

Generated-By: PostHog Code
Task-Id: ebafcb71-b03b-443d-b40c-d527ed4a04f4

* refactor: apply simplify pass

Reuse existing helpers in the v2 adapter instead of hand-rolled copies:
- _ctx_mcp_session_id now goes through get_request_headers +
  read_mcp_session_header, which already handle case-insensitive keys,
  list-valued headers and whitespace — strictly better than the raw
  ctx.request.headers read it replaces
- shared schema_has_param() in _context_parameters (was duplicated between
  the lowlevel and v2 adapters)
- shared params_to_request_dict() in _instrumentation (request_to_dict now
  delegates to it)
- one tool-manager lookup per call instead of two when checking ownership
  of both injected parameters

The three adapters stay separate on purpose — they hook different SDK seams
with different semantics; only genuinely identical logic is shared.

Generated-By: PostHog Code
Task-Id: ebafcb71-b03b-443d-b40c-d527ed4a04f4

* fix(mcp): finish the tool-cache fix on the low-level adapter

Round 2 of the review loop caught that the previous commit's cache-rebuild
fix stopped at the FastMCP adapter. The raw low-level v1 adapter has the
identical exposure on the *input* side, and it is worse there: this adapter
advertises `context`/`conversation_id` without stripping them, relying on
the advertised schema doubling as the call's validation schema. So a cache
rebuilt from the un-injected internal listing pass rejects exactly the
arguments the SDK told the agent to send —
"Input validation error: Additional properties are not allowed ('context'
was unexpected)" — for any tool with `additionalProperties: false`. One
unknown tool name (or a fresh stateless instance) triggers it, and the
conversation prompt-back tells the agent to keep sending the argument, so
it persists until the next client-facing tools/list.

The internal pass now gets the same injections here too, via a shared
`_inject_tool_schemas` helper mirroring the FastMCP one. Regression test
verified to fail without the fix.

Also from round 2:
- `_tool_own_properties_v2` fails closed on a malformed schema again — the
  simplify pass had narrowed the guard so a non-dict `properties` would
  raise (or answer by substring) in the tool-call hot path
- finish the `schema_has_param` dedup the simplify pass started: the
  low-level adapter now uses the shared helper instead of its own copy

Generated-By: PostHog Code
Task-Id: ebafcb71-b03b-443d-b40c-d527ed4a04f4

* fix(mcp): don't require a parameter we strip before the SDK validates

Round 3 of the review loop. On the jlowin-FastMCP path the adapter strips
the injected `context`/`conversation_id` before dispatch, because that SDK
validates tool arguments against the function signature. But the listing
wrapper marked `context` *required* in the advertised schema — which is
also the schema mcp 1.x's low-level server validates the (already stripped)
arguments against. Under `FastMCP(strict_input_validation=True)` every call
therefore failed with "Input validation error: 'context' is a required
property".

Latent on main, where an un-injected cache rebuild intermittently cured it;
the previous commit's internal-pass injection made it permanent. Advertise
`context` as optional on this path instead — requiring a parameter the
validator can never see is self-contradictory, and losing the "required"
nudge only softens intent capture, where the alternative is a hard failure
of the customer's tool call.

Regression test added; the default `strict_input_validation=False` and both
MCP SDK 2.x paths were unaffected either way.

Generated-By: PostHog Code
Task-Id: ebafcb71-b03b-443d-b40c-d527ed4a04f4

* docs(mcp): reframe the changeset around cross-SDK parity

The changeset claimed "SDK 1.x paths are unchanged", which stopped being
true as the PR grew: conversation-anchored sessions, the structuredContent
delivery channel, the uuidv7 handle guard, the prompt-back on errored
results and the callback context all reach v1 servers, and three separately
reproduced bugs on v1 paths were fixed. Since no mainstream client speaks
the 2026-07-28 era yet, that half is what current users actually get.

Restructured into v2 support / cross-SDK parity / fixes affecting existing
1.x users, and calls out the two deliberate behavioural changes.

Generated-By: PostHog Code
Task-Id: ebafcb71-b03b-443d-b40c-d527ed4a04f4

* feat(mcp): emit $mcp_error_message and $mcp_error_type

Why a call failed only ever reached PostHog on the sibling $exception
event. The failures view reads the scalars off the primary event, so every
Python-backed MCP server showed empty error rows — reported from the field
after a team built against MCP Analytics with a hand-rolled dispatcher — and
disabling exception autocapture dropped the reason on the floor entirely.

Both values come from the same $exception_list the sibling carries, so the
two surfaces cannot disagree, and the message inherits the existing
2048-char cap because truncation runs before the event mapping. An explicit
error_type wins over the thrown class name: PostHogMCP.capture_tool_call()
and capture_tools_list() now take one, so a custom dispatcher can send a
coarse category ("validation", "timeout") that means something to the
product where "ValueError" does not.

Parity with @posthog/mcp, which derives both the same way.

Generated-By: PostHog Code
Task-Id: ebafcb71-b03b-443d-b40c-d527ed4a04f4

* fix(mcp): sanitize exception messages before they leave

Review finding on this PR: $mcp_error_message copied the exception value
straight out of $exception_list, so a credential in the failure text ("auth
failed for token phc_...") shipped verbatim.

Valid, and broader than reported — sanitize_event covered response,
parameters and user_intent but never error, so the $exception sibling has
been sending unredacted exception messages since long before this property
existed. @posthog/mcp sanitizes it (sanitizeExceptionValues); Python simply
never ported that, and surfacing the message a second time made the gap
visible.

Redacts the `value` of every $exception_list frame through the same
sanitizer as everything else, leaving type/mechanism intact. Both surfaces
are covered, since the scalar is derived after sanitization runs.

Generated-By: PostHog Code
Task-Id: ebafcb71-b03b-443d-b40c-d527ed4a04f4

* docs(mcp): state the real redaction scope on exception values

A review comment pointed out that a non-PostHog credential (an sk-... key)
in an exception message still ships. Correct, but pre-existing and not
specific to this property: sanitize_captured_value redacts PostHog tokens
and sensitive-looking keys, and applies the same way to response,
parameters and user_intent. @posthog/mcp uses the identical pattern, so
broadening it here alone would diverge the two SDKs.

Not fixing the scope — enumerating every vendor's key format is an arms
race that fails quietly in both directions. Fixing the comment, which
overclaimed by citing "sk-..." as an example of what gets redacted, and
naming before_send as the gate for hosts with strict requirements.

Generated-By: PostHog Code
Task-Id: ebafcb71-b03b-443d-b40c-d527ed4a04f4

* fix(mcp): only anchor a conversation handle the agent has confirmed

Review finding: prepare_request() anchored the session on a *freshly minted*
handle and emitted identify/initialize immediately — before either adapter
knew whether the prompt-back could ride the result. When it could not (an
exception converted outside ToolManager.call_tool, a result with nothing to
carry it), the client never received that handle, so the events stranded in
a conversation-derived session nobody holds and the next call minted
another. Reproduced: two failed calls produced two orphan sessions, each
with its own duplicate $identify and $mcp_initialize — strictly worse than
not anchoring at all, and the same degradation as an agent that never
echoes.

An echo is the only proof of delivery, so only echoed handles anchor now.
The minting call stays in the transport/memory session it would have used
anyway; every call after the agent returns the handle joins the
conversation's session. Failure therefore degrades to the pre-feature
behaviour instead of fragmenting.

The cross-pod contract is unchanged where it matters: a pod that receives
an echoed handle derives the same session as any other pod without shared
state — it just starts from the echo rather than the mint.

Also drops the `before_send` claim from the get_request_headers docs,
posthog/mcp/__init__.py and the changeset: before_send is invoked as
before_send(capture) with one argument, so a callback written to the
documented (capture, extra) contract would raise TypeError and have every
event silently dropped. It receives the finished payload, not the request,
so `extra` does not belong there.

Generated-By: PostHog Code
Task-Id: ebafcb71-b03b-443d-b40c-d527ed4a04f4

* fix(mcp): carry the conversation handle as data, not an instruction

The prompt-back shipped an imperative sentence inside the tool result:

  [SERVER]: Reuse conversation_id=... on every subsequent tool call in this
  conversation. Required for the server to correlate calls and ...

Two problems, both of which @posthog/mcp already moved away from (it emits
JSON.stringify({conversation_id}) with the same reasoning):

- Tool results are untrusted content, so a server telling the model what to
  do on every later call is exactly the shape a client's prompt-injection
  filter looks for. A stripped block means the handle never arrives and
  conversation sessions quietly stop working — the failure is invisible.
- It renders in the user's transcript. Spotted while manually testing the
  stack through Claude Desktop.

Now emits {"conversation_id": "..."} — same channel and cadence, just data.
The earlier parity sweep compared the delivery channels (text block vs
structuredContent) but not the payload inside the block, so this slipped
through.

Generated-By: PostHog Code
Task-Id: ebafcb71-b03b-443d-b40c-d527ed4a04f4

* fix(mcp): anchor the minting call too, once delivery is confirmed

The previous fix was too blunt. It never anchored a freshly minted handle,
which removed the orphan sessions but split every conversation in two: the
call that minted the handle landed in the connection session and only the
echoes joined the conversation's. Observed live — three Claude Desktop
calls sharing one conversation_id produced two sessions.

The check was simply in the wrong place. Delivery is known a few lines
after the tool returns; the session was being resolved before the tool ran.
Resolving it after instead gives both properties at once:

  minted + delivered    -> anchors, so the whole conversation is one session
  minted + undelivered  -> anchors nothing, so no unreachable sessions
  echoed                -> anchors, unchanged

This is also a closer reading of the review comment that started it:
"delay anchoring newly minted handles until delivery is confirmed" — the
handle is now anchored exactly when confirmed, not abandoned.

prepare_request moves behind a local resolver in all four tool-call
adapters, called on each exit path with the handle that actually reached
the agent. The orphan-guard test and the mint-then-echo test now both pass,
which is the point: they were previously mutually exclusive.

Generated-By: PostHog Code
Task-Id: ebafcb71-b03b-443d-b40c-d527ed4a04f4

* fix(mcp): settle the shared session before the tool body runs

Review finding, and a regression from the previous commit: moving
prepare_request() to after the call meant data.session_id still held the
*previous* request's value while the tool executed. McpAnalytics.capture()
reads that field, so a custom event emitted from inside a tool body was
attributed to the previous caller's session — and through the identity
cache, their person.

prime_session() now settles the transport/memory session before dispatch;
the conversation anchor is still finalised afterwards, once delivery is
known. An in-tool event predates that decision, so it belongs to the
transport session, which is the correct answer available at that moment.

Regression test uses two callers with distinct Mcp-Session-Id headers and
asserts B's tool body sees B's session — verified to fail without the fix
(the first version of this test passed either way and was worthless).

Generated-By: PostHog Code
Task-Id: ebafcb71-b03b-443d-b40c-d527ed4a04f4

* fix(mcp): redact non-PostHog credentials in captured strings

Blocking review finding. The sanitizer only knew phc_/phx_ tokens, so an
exception like AuthError("auth failed for sk-proj-...") shipped someone
else's key — and this PR newly exposes that on the primary event when
mcp_exception_autocapture=False, a configuration where no error text left
the process at all before.

I argued against this earlier on the grounds that enumerating vendor key
formats is an arms race. That was wrong for this repo: posthog-python
already ships a general detector for exactly this — exception_utils
._looks_like_secret (entropy, known formats such as AWS key ids, PEM
markers), used by the code-variables path. Reusing it needs no vendor
table.

Applied per whitespace-separated word rather than to the whole string:
redacting an entire exception message would destroy the diagnostic value
$mcp_error_message exists to provide. "auth failed for sk-proj-..." becomes
"auth failed for [redacted]", and ordinary text is untouched because no
single word in it looks like a credential.

Covers every captured string, so parameters and responses get it too, not
just error values. Note this is deliberately broader than @posthog/mcp,
which still only strips PostHog tokens — worth porting there.

Generated-By: PostHog Code
Task-Id: ebafcb71-b03b-443d-b40c-d527ed4a04f4

* chore(mcp): drop the changeset already consumed by v7.40.0

#881 was squash-merged and released, so sampo turned
mcp-sdk-v2-support.md into the 7.40.0 changelog entry and deleted it from
main. This branch predates that merge and still carried the file; because
the squash gave main no shared history with the branch, git saw "main never
had this file" rather than a modify/delete and silently kept it — which
would have re-emitted the entire v7.40.0 entry in the next release.

Generated-By: PostHog Code
Task-Id: ebafcb71-b03b-443d-b40c-d527ed4a04f4
gesh added a commit to PostHog/posthog.com that referenced this pull request Aug 21, 2026
posthog-python's posthog.mcp now supports MCP Python SDK v2 (mcp>=1.26,<3, MCPServer), emits $mcp_error_type/$mcp_error_message, and captures $mcp_client_user_agent/$mcp_vendor_client (PostHog/posthog-python#881, #882, #883). Remove the stale "TypeScript only" / "Python soon" claims and document the new surface.

Generated-By: PostHog Desktop
Task-Id: a5edeffa-82df-4f0e-a565-01611e261806
gesh added a commit to PostHog/posthog.com that referenced this pull request Aug 21, 2026
…19633)

posthog-python's posthog.mcp now supports MCP Python SDK v2 (mcp>=1.26,<3, MCPServer), emits $mcp_error_type/$mcp_error_message, and captures $mcp_client_user_agent/$mcp_vendor_client (PostHog/posthog-python#881, #882, #883). Remove the stale "TypeScript only" / "Python soon" claims and document the new surface.

Generated-By: PostHog Desktop
Task-Id: a5edeffa-82df-4f0e-a565-01611e261806
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants