fix(mcp): warn when stateless session middleware never attached

[posthog]

💡 Motivation and Context

A customer on a stateless server lost weeks of analytics sessions because PostHogMcpStatelessSessionMiddleware silently never attached — and nothing in the SDK said so. Two support round trips and an SDK release that didn't fix their issue.

The mint is zero-config only for an ASGI app built after instrument() runs. autowire_stateless_mint monkey-patches the app factories, but FastMCP builds a fresh Starlette app per call and never retains it — so an app built or mounted before instrument() (the common FastAPI-mounts-at-import case) can't be retrofitted and gets no middleware at all. Every session then falls through resolve_session_id to the per-process generated branch, and $session_id fragments across pods with nothing logged. Works-in-dev (mcp.run() calls the patched factory), dark-in-prod (FastAPI mounts its own app).

Two independent signals now surface it — either would have ended the ticket on day one:

Signal	When
instrument() warning	streamable_http_app() was already called before instrument() ran
Runtime, warn-once	first tool call over streamable HTTP whose session still comes from process memory

Both go to the posthog.mcp stdlib logger as well as the MCPAnalyticsOptions(logger=...) sink. That part matters: log() is a no-op unless the host opts in, so routing these through it alone would have left the failure exactly as dark as it was — the customer had no logger configured. Silence with logging.getLogger("posthog.mcp").setLevel(logging.ERROR).

Was stacked on #881 — now rebased onto main

#881 squash-merged (b0ab12c) while this was in review, so GitHub retargeted this PR to main and the squash left the branch conflicting. Rebased the two commits onto main; the conflict was purely the squash-vs-individual-commits history mismatch, and the replay was clean. No longer blocked on anything.

The dependency on #881 was real while it lasted — this needs conversation_id in scope and request_headers.get_request_headers, both of which are now in main.

Rebasing surfaced two things worth calling out.

1. Detection can't read data.session_source. #881's resolve_session_id short-circuits on conversation_id before the lock and never writes that field, so it keeps a stale "generated". A check reading it would warn about conversation-anchored sessions — which are deterministic and stable across pods, i.e. exactly the case #881 exists to support. So resolve_session_id_with_source() now returns the source for this request and the detector uses that. resolve_session_id() is kept as a thin wrapper; no caller changes.

2. The v1/v2 adapter changes were deleted. #881 populates extra["ctx"] at all seven prepare_request call sites, so the HTTP probe derives from get_request_headers(extra) inside prepare_request instead of being threaded through as a parameter. That removes every edit to _instrument_fastmcp.py and _instrument_lowlevel.py and covers _instrument_v2.py for free — which the earlier revision of this PR left unplumbed.

Also excluded: the deprecated SSE transport. It keys sessions off a query param, and the mint sets a response header an SSE client never replays, so recommending the middleware there would be wrong advice.

💚 How did you test it?

Green on both CI legs — mcp>=1.26,<2: 225 passed / 1 skipped; mcp>=2,<3: 209 passed / 13 skipped, with v1-only tests skipping via importorskip("mcp.server.fastmcp") rather than erroring. All 41 GitHub checks pass. ruff check / ruff format clean, public API snapshot regenerated.

Tests exercise the real transport (starlette.testclient), not a http_request=True parameter passed in by the test itself — a wrong getattr in the probe would otherwise leave the signal dead with everything green. Each guard was mutation-tested to confirm it bites:

Mutation	Caught by
read stale data.session_source	test_no_warning_when_session_is_anchored_by_conversation_id
HTTP probe disabled	test_no_warning_for_stdio
SSE suppression removed	test_no_warning_for_sse_transport
warn-once removed	test_runtime_warning_fires_once_per_server
stdlib logging removed	test_warnings_are_visible_without_configuring_a_logger
instrument-time probe disabled	test_runtime_warns_when_app_was_built_before_instrument
warning fires unconditionally	test_runtime_silent_when_correctly_wired + 3

Known gap, documented rather than hidden: jlowin's fastmcp 2.x/3.x keeps its session manager as a local inside http_app(), so there's nothing to probe at instrument time — those servers get the runtime warning only.

📝 Checklist

• I reviewed the submitted code.
• I added tests to verify the changes.
• I updated the docs if needed.
• Ran sampo add to generate a changeset file

🤖 Agent context

Autonomy: Supervised — reviewed and rebuilt by Claude Code (Opus 5) on top of #881 after a human review flagged that the original implementation, authored against a since-closed #830, misfired on #881's conversation-anchored sessions and broke its mcp 2.x CI leg.

[github-actions]

posthog-python Compliance Report

Date: 2026-08-21 13:33:35 UTC
Duration: 256452ms

✅ 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	✅	517ms
Endpoint And Method.Does Not Use Legacy Endpoints	✅	511ms
Required Headers.Has Authorization Bearer Header	✅	510ms
Required Headers.Has Content Type Json	✅	511ms
Required Headers.Has Posthog Sdk Info Format	✅	511ms
Required Headers.Has Posthog Attempt Header	✅	511ms
Required Headers.Has Posthog Request Id	✅	511ms
Required Headers.Has Posthog Request Timestamp	✅	511ms
Required Headers.Has User Agent	✅	511ms
Body Format.Body Has Created At And Batch	✅	511ms
Body Format.No Api Key In Body	✅	510ms
Body Format.No Sent At In Body	✅	511ms
Event Format.Event Has Required Root Fields	✅	510ms
Event Format.Event Uuid Is Valid	✅	513ms
Event Format.Event Timestamp Is Rfc3339	✅	511ms
Event Format.Distinct Id Is String	✅	510ms
Event Format.Distinct Id At Root Not Properties	✅	510ms
Event Format.Custom Properties Preserved	✅	510ms
Event Format.Set Properties Preserved	✅	510ms
Event Format.Set Once Properties Preserved	✅	511ms
Event Format.Groups Properties Preserved	✅	510ms
Event Format.Sdk Generates Uuid If Not Provided	✅	511ms
Event Format.Event Has Required Root Fields Batch	✅	514ms
Event Format.Event Uuid Is Valid Batch	✅	513ms
Event Format.Event Timestamp Is Rfc3339 Batch	✅	514ms
Event Format.Distinct Id Is String Batch	✅	514ms
Event Format.Distinct Id At Root Not Properties Batch	✅	514ms
Event Format.Custom Properties Preserved Batch	✅	513ms
Event Format.Set Properties Preserved Batch	✅	514ms
Event Format.Set Once Properties Preserved Batch	✅	514ms
Event Format.Groups Properties Preserved Batch	✅	514ms
Event Format.Sdk Generates Uuid If Not Provided Batch	✅	516ms
Batch Behavior.Multiple Events In Single Batch	✅	518ms
Batch Behavior.Batch Envelope Smoke	✅	515ms
Batch Behavior.Flush With No Events Sends Nothing	✅	507ms
Batch Behavior.Flush At Triggers Batch	✅	1011ms
Batch Behavior.Created At Reflects Batch Creation Time	✅	512ms
Deduplication.Generates Unique Uuids	✅	518ms
Deduplication.Different Events Same Content Different Uuids	✅	513ms
Deduplication.Preserves Uuid On Retry	✅	6520ms
Deduplication.Preserves Timestamp On Retry	✅	6518ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry	✅	6524ms
Deduplication.No Duplicate Events In Batch	✅	518ms
Header Behavior On Retry.Attempt Header Starts At One	✅	511ms
Header Behavior On Retry.Attempt Header Increments On Retry	✅	13523ms
Header Behavior On Retry.Request Id Preserved On Retry	✅	6517ms
Header Behavior On Retry.Different Requests Have Different Request Ids	✅	3020ms
Header Behavior On Retry.Request Timestamp Changes On Retry	✅	6519ms
Response Format Validation.Success Response Has Uuid Keyed Results	✅	511ms
Response Format Validation.Success Response Has Ok For Each Event	✅	514ms
Response Format Validation.Success No Retry After When All Ok	✅	513ms
Response Format Validation.Success Retry After Present When Retry Events	✅	1515ms
Response Format Validation.Success No Retry After When Drop Only	✅	513ms
Response Format Validation.Response Echoes Request Id	✅	510ms
Retry Behavior.Retries On 408	✅	6519ms
Retry Behavior.Retries On 500	✅	6519ms
Retry Behavior.Retries On 503	✅	8528ms
Retry Behavior.Retries On 504	✅	6521ms
Retry Behavior.Retryable Errors Have Retry After	✅	3516ms
Retry Behavior.Respects Retry After On Retryable Error	✅	11517ms
Retry Behavior.Does Not Retry On 400	✅	2514ms
Retry Behavior.Does Not Retry On 401	✅	2513ms
Retry Behavior.Does Not Retry On 402	✅	2513ms
Retry Behavior.Does Not Retry On 413	✅	2514ms
Retry Behavior.Does Not Retry On 415	✅	2514ms
Retry Behavior.Non Retryable Errors Have No Retry After	✅	2513ms
Retry Behavior.Implements Backoff	✅	22534ms
Retry Behavior.Max Retries Respected	✅	22533ms
Partial Batch Handling.Handles 200 Full Success	✅	2511ms
Partial Batch Handling.Handles 200 With All Ok	✅	3518ms
Partial Batch Handling.Does Not Retry Dropped Events	✅	3518ms
Partial Batch Handling.Does Not Retry Limited Events	✅	3518ms
Partial Batch Handling.Prunes Ok Events On Partial Retry	✅	6521ms
Partial Batch Handling.Prunes Dropped Events On Partial Retry	✅	6521ms
Partial Batch Handling.Retries Only Retry Events From Partial	✅	6523ms
Partial Batch Handling.Partial Retry Preserves Uuids	✅	6521ms
Partial Batch Handling.Partial Retry Attempt Header Increments	✅	6521ms
Partial Batch Handling.Partial Retry Request Id Preserved	✅	6521ms
Partial Batch Handling.Respects Retry After On Partial	✅	8521ms
Partial Batch Handling.Unknown Result Treated As Terminal	✅	3516ms
Partial Batch Handling.Mixed Ok Drop Limited No Retry	✅	3517ms
Compression.Sends Gzip Content Encoding	✅	511ms
Compression.No Content Encoding When Disabled	✅	511ms
Compression.Compressed Body Is Decompressible	✅	510ms
Error Handling.Does Not Retry On Unknown 4Xx	✅	2512ms
Event Options.Cookieless Mode Override	✅	511ms
Event Options.Disable Skew Correction Override	✅	510ms
Event Options.Process Person Profile Override	✅	510ms
Event Options.Product Tour Id Override	✅	510ms
Event Options.Unset Options Omitted	✅	510ms
Event Options.Options Override In Batch	✅	513ms
Geoip And Historical Migration.Geoip Disable Injected Into Properties	✅	511ms
Geoip And Historical Migration.Historical Migration Set In Body	✅	510ms
Geoip And Historical Migration.Historical Migration Absent By Default	✅	510ms

Feature_Flags Tests

✅ 17/17 tests passed

View Details

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

[github-actions]

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week.

[marandaneto]

@PostHog/mcp-analytics

[gesh]

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 @ 5cf9629)}

Round 3 covers the delta since round 2 (65d6575..5cf9629): a rebase onto a newer main (picking up $mcp_error_message/$mcp_error_type and $mcp_client_user_agent/$mcp_vendor_client, one import conflict resolved by keeping both sides) plus a genuine refactor, share the request unwrap with get_request_headers. The router traced all four flagged questions to concrete conclusions and found no behavior change and no correctness bugs; confidence was high enough that no delegation was needed.

Key findings

None. Zero actionable findings this round.

Convergence

Only one reviewer ran this round (router); no cross-reviewer convergence to report.

Reviewer summaries

Reviewer

Assessment

🧭 router (sonnet)

LOW danger / HIGH confidence. Verified the rebase resolution against origin/main's _instrumentation.py: all of #882/#883's hunks survive intact (stamp_transport_identity calls at all 4 event-emission sites, the _transport_identity import, resolve_session_id_with_source) — nothing lost. On the refactor: (1) widening _is_sse_request to drop its isinstance(extra, dict) guard is safe — it has exactly one call site, inside prepare_request, and all 3 internal callers (_instrument_v2.py, _instrument_lowlevel.py, _instrument_fastmcp.py) always build extra as a Dict[str, Any] with a "ctx" key, so get_request's own dict-or-bare-ctx handling makes the removed check dead code, not a narrowing loss; the function also stays wrapped in its own broad except Exception. (2) Leaving get_request out of __all__ and out of posthog/mcp/__init__.py while not underscoring it is a defensible, precedented call — it mirrors the module's existing private-by-convention _to_header_bag and the docstring already frames get_request_headers as the one supported entry point; verified get_request is absent from both the __init__.py export list and references/public_api_snapshot.txt. (3) Traced get_request_headers's new body (getattr(get_request(extra), "headers", None)) against the old get_request_headers(extra) is not None probe: on every real and test-simulated transport shape a constructed request object always carries a .headers attribute, so get_request(extra) is not None and the old get_request_headers(extra) is not None are equivalent — no behavior change on any transport in this codebase. (4) No correctness bugs in the refactor; existing test_request_headers.py coverage (raw-ctx, stdio-none, hostile-header-object, dict-wrapping) still exercises the unchanged externally-visible behavior of get_request_headers through the new indirection. Independently re-ran posthog/test/mcp on the 1.x env: 225 passed / 1 skipped, matching the PR's own claim. Delegated nothing — a small, mechanically-verifiable refactor with grep-confirmed call-site enumeration didn't warrant a stronger model.

Previous rounds (2)

round 2 @ 65d6575 — ✅ APPROVE: single-commit delta (2.x attribute-probe fix), zero actionable findings, empirical claims reproduced directly against both SDK majors.
round 1 @ e331445 — ✅ APPROVE: zero actionable findings; one out-of-scope NIT on pre-existing code outside the diff, not posted inline.

---

Automated by QA Swarm — not a human review

[posthog]

🦔 ReviewHog reviewed this pull request

Found 0 must fix, 1 should fix, 0 consider.

Published 1 finding (view the review).

[posthog]

ReviewHog Alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏

[posthog]

ReviewHog Report

Found 1 should fix.

[posthog]

Stateful servers receive an incorrect stateless-session warning

Why we think it's a valid issue

• Checked: _app_was_already_built at posthog/mcp/asgi.py:259-283, the FastMCP source it probes, and the runtime twin _warn_stateless_session_not_wired at posthog/mcp/_instrumentation.py:287.
• Found: mcp/server/fastmcp/server.py:950-961 creates self._session_manager on the first streamable_http_app() call for every server, and passes stateless=self.settings.stateless_http into it. The constructor default is stateless_http: bool = False (server.py:168), so a stateful server sets the exact attribute the probe reads.
• Found: I reproduced it. A default FastMCP("stateful-default") with settings.stateless_http = False, whose app is built before instrument(), makes _app_was_already_built return True and emits the ordering warning to the posthog.mcp stdlib logger. The same object exposes _session_manager.stateless = False, so a correct check is available at the probe site.
• Found: I then ran a full transport test on that stateful server (initialize → initialized → tools/call over starlette.testclient). The transport issued its own Mcp-Session-Id (c1cf19ee...), resolve_session_id_with_source returned source "mcp", and the runtime warning stayed silent. The session is healthy and the mint middleware is not needed there.
• Found: The two signals disagree. The runtime warning is guarded on session_source == "generated" (_instrumentation.py:357-362), so it correctly skips stateful servers. The instrument-time probe has no equivalent guard, so only it misfires.
• Impact: The warning fires on the SDK's default configuration in exactly the scenario this PR targets — an app mounted before instrument(). It goes to the stdlib logger at WARNING level by design, so every default-configured stateful host sees text that says "stateless sessions will not be captured" when the sessions are captured correctly. The two remedies it recommends (reorder instrument(), or add PostHogMcpStatelessSessionMiddleware) change nothing for a stateful server. This contradicts the PR's own stated bar in test_runtime_silent_when_correctly_wired: "A diagnostic that cries wolf on healthy servers is worse than no diagnostic."
• Impact: Note for the fix — read stateless off the discovered session manager rather than settings.stateless_http alone. On mcp 2.x, MCPServer.streamable_http_app() takes stateless_http per call, so there is no server-level setting to read, but the manager object is a StreamableHTTPSessionManager in both majors and stores self.stateless (mcp/server/streamable_http_manager.py:72). Also note that test_app_built_probe_fires_on_the_installed_sdk_major builds a stateful MCPServer on the 2.x leg, so that test needs stateless_http=True once the guard lands.

Issue description

The probe only checks whether _session_manager exists. Stateful servers also create this manager. They already issue stable MCP session IDs and do not need the mint middleware. The new code warns these users that sessions will fragment.

Suggested fix

Check that the session manager is stateless before returning True. For example, inspect its stateless value or the server's settings.stateless_http value. Add a test that builds a stateful app before instrument() and expects no warning.

Prompt to fix with AI (copy-paste)

## Context
@posthog/mcp/asgi.py#L279-280

<issue_description>
The probe only checks whether `_session_manager` exists. Stateful servers also create this manager. They already issue stable MCP session IDs and do not need the mint middleware. The new code warns these users that sessions will fragment.
</issue_description>

<issue_validation>
- **Checked:** `_app_was_already_built` at `posthog/mcp/asgi.py:259-283`, the FastMCP source it probes, and the runtime twin `_warn_stateless_session_not_wired` at `posthog/mcp/_instrumentation.py:287`.
- **Found:** `mcp/server/fastmcp/server.py:950-961` creates `self._session_manager` on the first `streamable_http_app()` call for every server, and passes `stateless=self.settings.stateless_http` into it. The constructor default is `stateless_http: bool = False` (`server.py:168`), so a stateful server sets the exact attribute the probe reads.
- **Found:** I reproduced it. A default `FastMCP("stateful-default")` with `settings.stateless_http = False`, whose app is built before `instrument()`, makes `_app_was_already_built` return `True` and emits the ordering warning to the `posthog.mcp` stdlib logger. The same object exposes `_session_manager.stateless = False`, so a correct check is available at the probe site.
- **Found:** I then ran a full transport test on that stateful server (initialize → initialized → tools/call over `starlette.testclient`). The transport issued its own `Mcp-Session-Id` (`c1cf19ee...`), `resolve_session_id_with_source` returned source `"mcp"`, and the runtime warning stayed silent. The session is healthy and the mint middleware is not needed there.
- **Found:** The two signals disagree. The runtime warning is guarded on `session_source == "generated"` (`_instrumentation.py:357-362`), so it correctly skips stateful servers. The instrument-time probe has no equivalent guard, so only it misfires.
- **Impact:** The warning fires on the SDK's default configuration in exactly the scenario this PR targets — an app mounted before `instrument()`. It goes to the stdlib logger at WARNING level by design, so every default-configured stateful host sees text that says "stateless sessions will not be captured" when the sessions are captured correctly. The two remedies it recommends (reorder `instrument()`, or add `PostHogMcpStatelessSessionMiddleware`) change nothing for a stateful server. This contradicts the PR's own stated bar in `test_runtime_silent_when_correctly_wired`: "A diagnostic that cries wolf on healthy servers is worse than no diagnostic."
- **Impact:** Note for the fix — read `stateless` off the discovered session manager rather than `settings.stateless_http` alone. On mcp 2.x, `MCPServer.streamable_http_app()` takes `stateless_http` per call, so there is no server-level setting to read, but the manager object is a `StreamableHTTPSessionManager` in both majors and stores `self.stateless` (`mcp/server/streamable_http_manager.py:72`). Also note that `test_app_built_probe_fires_on_the_installed_sdk_major` builds a stateful `MCPServer` on the 2.x leg, so that test needs `stateless_http=True` once the guard lands.
</issue_validation>

## Task
Investigate the issue and solve it

<potential_solution>
Check that the session manager is stateless before returning `True`. For example, inspect its `stateless` value or the server's `settings.stateless_http` value. Add a test that builds a stateful app before `instrument()` and expects no warning.
</potential_solution>