Repository navigation
Conversation
The Python MCP SDK answers initialize itself, so posthog.mcp sent the event on the first request of each session, once per process. A stateless fleet sent one event per pod. A client that sent only initialize sent no event. Now MCP SDK 2.x reports the event from Server.middleware, on every transport. MCP SDK 1.x over HTTP reports it from PostHogMcpStatelessSessionMiddleware, which takes a new optional server= argument. MCP SDK 1.x over stdio keeps the first-request fallback. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
posthog-python Compliance ReportDate: 2026-10-22T15:57:21.774376+00:00 ✅ All Tests Passed!121/121 tests passed Capture_V1 Tests✅ 95/95 tests passed View Details
Capture_Ai Tests✅ 5/5 tests passed View Details
Feature_Flags Tests✅ 17/17 tests passed View Details
Feature_Flags_Local_Evaluation Tests✅ 4/4 tests passed View Details
|
There was a problem hiding this comment.
PostHog Review · Standard · reviewed 57d8dc6: 3 Should fix.
This is the only automatic review while this pull request is a draft. Comment @posthog review to review new changes sooner. Otherwise, a review of the new changes runs when you mark it ready for review.
The mcp 1.x hook now keeps only the status and the Mcp-Session-Id header of the initialize response. It no longer parses the JSON or SSE body. The event uses the protocol version that the client asked for. record_initialize now reads the client name, version, and protocol from the initialize params and reply, so both hooks pass them through. The new tests share their request helpers and drop the SSE body case. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- SSE transport: the initialize event now lands in the session that the SSE calls use. On SDK 1.x the middleware leaves SSE requests to the first-request fallback. On SDK 2.x the hook uses the process session. - The bound middleware marks each request that it passes on. The fallback now stops only for marked requests, not for every HTTP request. - The hooks compute the handshake session without a write to the shared session that McpAnalytics.capture() reads. - A server= value that cannot be bound no longer breaks requests. - Remove the unused _mint_token_if_initialize. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Both handshake hooks run record_initialize in a tracked background task. It started the capture as a second task, after flush() took its snapshot of pending tasks. With a slow before_send, flush() returned early and shutdown dropped $mcp_initialize. The handshake task now awaits the capture itself. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
On MCP SDK 2.x, a stateless client can send initialize with an existing Mcp-Session-Id and keep sending it. Its later calls resolve their session from that header, but the handshake hook read only the connection id and the minted token. The event landed in a generated session. The hook now falls back to the incoming header. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
On MCP SDK 1.x, the middleware read the body only of a POST with no Mcp-Session-Id, because only those need a token. A client that sent initialize with an existing session id got no event: the middleware marked the request, so the first-request fallback did not run either. A middleware bound to a server now reads those bodies too, behind a cheap check for "initialize". It still mints only without a header. The event uses the header that the client keeps sending. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PostHog Review · Standard · couldn't review
|
PostHog Review · Standard · couldn't review
|
When the client disconnected while it sent the body, the middleware stopped its read and replayed the partial body as a complete request. The app parsed half a request and never saw the disconnect. The middleware now replays the partial body with more_body set, then hands the app the disconnect. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A bound middleware reads the body of a POST that carries a session id, to catch an initialize that reuses one. That held every tool call in memory, up to 256 KB, before the app saw it. The middleware now stops after 4 KB when "initialize" is not in it, and streams the rest. JSON-RPC clients put "method" before "params", so a handshake shows up in that window. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The v2 handshake hook read the Mcp-Session-Id header and decoded the token itself. Every later request gets the same pair from _resolve_ctx. The hook now calls it, so the handshake and later requests follow one rule and cannot drift apart into two sessions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dustinbyrne
left a comment
There was a problem hiding this comment.
The handshake-count fixes are well covered and reproduce correctly against the base. One optional resource-safety follow-up is noted inline.
AI-assisted review.
| return await call_next(ctx) | ||
| started = datetime.now(timezone.utc) | ||
| result = await call_next(ctx) | ||
| fire_and_forget(_record_v2_initialize(data, ctx, result, started), data) |
There was a problem hiding this comment.
[P2] Optional follow-up: bound detached handshake recorder admission
Consider adding a finite, nonblocking per-server admission cap before creating handshake recorder tasks, including the equivalent v1 HTTP path in asgi.py:186–194.
This matters when a configured async identify or event_properties callback stays pending—for example, an identity lookup using httpx.AsyncClient(timeout=None) against a service that accepts the connection but never replies. Each successful handshake can then leave another analytics task waiting after the request finishes and the client disconnects.
With the same gated-callback test on base e17a8be and head bb21c6a, both MCP majors retained 0 tasks on base versus 16, 32, and 1,056 on head after that many completed handshakes. Head flush waited for callback release; releasing the gate drained everything to zero. The regression used an illustrative 1,024-task threshold, not a specification-prescribed number; unconditional scheduling in source establishes the missing bound.
The MCP observer-only contract explicitly calls for count-bounded pending state. Sensible caller-side timeouts and cached lookups substantially mitigate this, though, so I’m treating it as optional hardening rather than a merge requirement. This is not an ordinary-operation leak or a demonstrated production OOM.
A saturation regression should exceed the chosen cap, verify successful unchanged handshake responses and bounded work after disconnect, then verify recording resumes once callbacks finish. An unlimited semaphore wait queue would not bound retained tasks. An admission cap also would not, by itself, guarantee flush termination for a callback that never returns; callback-deadline policy can remain separate.
There was a problem hiding this comment.
kind of a stretch, fwiw, given this is only truly unbounded if the caller allows an infinite timeout period - i left this in considering the reference to sdk-specs, but it seems pretty benign to me overall
💡 Motivation and Context
initializeitself, soposthog.mcpcannot wrap it the way@posthog/mcpdoes. Until now, the SDK sent$mcp_initializeon the first request of each session, once per process.initialize.$mcp_initializeonly for aninitializerequest that the client sent.Changes
instrument()adds a hook toServer.middleware. The hook sees eachinitialize, on every transport.PostHogMcpStatelessSessionMiddlewarealready reads theinitializebody to mint the session token. Now it also sends the event after the response. The event uses theMcp-Session-Idthat the client sends back, so the event and the later requests share one$session_id.instrument()autowires bind the middleware to the server. If you mount the middleware by hand, pass the new optionalserver=argument. Without it, nothing changes: each pod sends the event on the first request.initialize. On SDK 2.x, the hook puts the event in the session that the SSE calls use.McpAnalytics.capture()reads.initializethat carries an existingMcp-Session-Idgets an event in that session. On SDK 1.x, a bound middleware reads up to 4 KB of such a POST. If"initialize"is not in it, the rest of the body streams to the app unread.analytics.flush()waits for the handshake capture.initialize. A stdio server has one client, so the count stays correct.conversation_idanchors no longer gets its own$mcp_initialize.$mcp_parameterswith the handshake request. On MCP SDK 2.x, it also has$mcp_responsewith the server reply, the same as@posthog/mcp.Check in review
ServerSession._connection, which is private. The code reads it with a guard, and a test covers it.Mcp-Session-Idheader of the response, not the body. So the event has the protocol version that the client asked for, and no$mcp_response. Aninitializethat fails with a JSON-RPC error still counts, because that error comes back as HTTP 200.identifyandevent_propertiescallbacks for the handshake get anextra["ctx"]that has only.request. The SDK has no request context at that point.get_request_headers(extra)works as before.$mcp_initializecount goes down for stateless Python fleets. The default MCP dashboard counts users and sessions from this event.server=keeps the old per-pod event.initializeand then nothing else gets no event.initializegets no event. The spec does not allow one.conversation_idgets no event.💚 How did you test it?
test_initialize_handshake.pysends raw JSON-RPC over HTTP to real apps and counts the events. Two pods with round-robin requests stand in for a fleet.FastMCPorMCPServer, standalone fastmcp (3.x on SDK 1.x, 4.x on SDK 2.x), and a low-levelServer.initialize(SDK 2.x), an echoed conversation, and the 2026-07-28 spec.server=value that cannot be bound.flush()with a slowbefore_send, and aninitializewith an existing session header.PostHogMCPdispatcher behind a middleware withoutserver=sends only its own event.main, 19 of the new tests fail on SDK 1.x and 20 fail on SDK 2.x.mcp1.30.0 withfastmcp3.x. v2 usesmcp2.3.0 withfastmcp4.1.0.$mcp_initializecount. Themaincolumn is posthog 7.75.4 on SDK 1.x.tools/listtools/listtools/list, and 2 tool callsOn this PR, the counts are the same for stateful and stateless servers, on both SDK majors. Each event is in the same
$session_idas the tool calls of its connection.📝 Checklist
If releasing new changes
sampo addto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
claude-opus-5-5).Server.middlewareon SDK 2.x and the ASGI middleware on SDK 1.x. SDK 1.x over stdio has no such hook, so it keeps the first-request fallback.🤖 Generated with Claude Code