Skip to content

fix(mcp): restore analytics capture for standalone FastMCP 4 - #936

Merged
lucasheriques merged 5 commits into
mainfrom
codex/mcp-fastmcp-v4-capture
Sep 11, 2026
Merged

lucasheriques merged 5 commits into
mainfrom
codex/mcp-fastmcp-v4-capture

Conversation

@lucasheriques

@lucasheriques lucasheriques commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

💡 Motivation and Context

Standalone FastMCP 4 servers execute tools successfully but emit no MCP analytics after instrumentation.

FastMCP 4 uses the MCP SDK v2 registry; the standalone adapter still assumes the v1 handler dictionary.

  • FastMCP 4 now captures tool calls through the existing v2 adapter without changing the installation call.
  • Mounted and versioned tools retain their application-owned arguments, including names shared with analytics parameters.
  • Each call resolves the effective schema in its request context, including middleware overrides, instead of sharing argument ownership across clients.
  • Instrumenting the wrapper and underlying server works in either order without duplicate events.
  • Native v2 model fields preserve conversation metadata when FastMCP exposes deprecated aliases.
  • The existing CI matrix now covers FastMCP 4; documentation and a Sampo patch changeset accompany the fix.
Adapter routing before and after

Before:

flowchart LR
  A[FastMCP 4] --> B[v1 handler dictionary] --> C[No analytics]
  classDef phBlue fill:#1d4aff,stroke:#1d4aff,color:#fff;
  class A,B,C phBlue;
Loading

After:

flowchart LR
  A[FastMCP 4] --> B[v2 registry adapter] --> C[Tool events captured]
  classDef phBlue fill:#1d4aff,stroke:#1d4aff,color:#fff;
  class A,B,C phBlue;
Loading

💚 How did you test it?

The MCP suite passed locally on the PR branch:

Dependencies Result
MCP 1.30.0 / FastMCP 3.2.0 307 passed
MCP 2.2.0 / FastMCP 4.0.3 305 passed, 13 skipped
  • The new HTTP tests cover both protocol eras, mounted argument ownership, repeated instrumentation, late tool registration, errors, and capture outages.
  • Regression tests reproduce and protect both initialization orders and client-requested tool versions.
  • Middleware tests verify dispatch without get_tool() resolution, stripping all three advertised analytics fields, and preserving application-owned fields before and after listing.
  • Interleaved client tests guard against cross-client argument stripping for providers, middleware, and middleware overrides in both listing orders.
  • A local stdio probe delivered real PostHog HTTP batches under both protocol eras, with exactly one event per tool call.
  • Ruff, filtered mypy, the warning-free import check, and actionlint passed.
  • Fresh-process checks confirm imports work without MCP, and official SDK instrumentation works without standalone FastMCP.
  • Full wizard installation and hosted PostHog ingestion remain unchecked.

Reproduce the regression suite with pytest posthog/test/mcp --timeout=30 in each dependency environment.

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed.
  • No breaking change or entry added to the changelog.

If releasing new changes

  • Ran sampo add to generate a changeset file

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Codex used the MCP Analytics debugging, CI workflow, dataclass, test, code comment, and PR description skills with shell tools and the GitHub CLI.

The fix reuses registry-based SDK detection. Adapter imports share one lazy dependency boundary; FastMCP-specific helpers load only when needed. Existing mutable tracking state holds a weak wrapper reference. Tests exercise public FastMCP HTTP entry points with synthetic tools. Human review is required.

Standalone FastMCP calls perform a schema listing per request to preserve middleware and client-specific argument ownership without a shared cache.

Select the adapter by handler registry so standalone FastMCP 4 captures
tool calls without changing the customer's instrument() invocation.
Preserve application-owned arguments, including mounted tools, and use
native v2 model fields when FastMCP exposes deprecated v1 aliases.

Add HTTP regression coverage for both protocol eras, late tool registration,
repeat instrumentation, failed tools, unavailable capture, and mounted
argument ownership. Include FastMCP 4 in the existing MCP v2 CI matrix,
document adapter behavior, and add a Sampo patch changeset.

Validation on current main: MCP v1 suite 307 passed; MCP v2 suite 291 passed,
13 skipped. Ruff lint/format, filtered mypy (231 source files), import
warning check, and actionlint passed. Real stdio tools emitted HTTP capture
batches to a local receiver under both protocol eras, two events per run.
Authenticated wizard installation and hosted ingestion remain untested.
@lucasheriques
lucasheriques requested a review from a team as a code owner September 9, 2026 21:04
@lucasheriques lucasheriques self-assigned this Sep 9, 2026
@greptile-apps

greptile-apps Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor
Prompt To Fix All With AI
### Issue 1
posthog/mcp/__init__.py:294-296
**Instrumentation remains order-dependent**

If a standalone FastMCP 4 server's low-level server is instrumented before its wrapper, both calls use the same tracking key, so the wrapper call returns without installing FastMCP-specific argument stripping. The low-level adapter then advertises analytics parameters but forwards them unchanged, causing tools that do not declare those parameters to reject otherwise valid calls. The existing installation needs to support adding the wrapper-specific context, with coverage for this reverse order.

---

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

Reviews (1): Last reviewed commit: "fix(mcp): capture standalone FastMCP on ..." | Re-trigger Greptile

Comment thread posthog/mcp/__init__.py Outdated
Comment on lines +294 to +296
instrument_lowlevel_v2(
server._mcp_server, data, strip_injected_for=server
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Instrumentation remains order-dependent

If a standalone FastMCP 4 server's low-level server is instrumented before its wrapper, both calls use the same tracking key, so the wrapper call returns without installing FastMCP-specific argument stripping. The low-level adapter then advertises analytics parameters but forwards them unchanged, causing tools that do not declare those parameters to reject otherwise valid calls. The existing installation needs to support adding the wrapper-specific context, with coverage for this reverse order.

Prompt To Fix With AI
This is a comment left during a code review.
Path: posthog/mcp/__init__.py
Line: 294-296

Comment:
**Instrumentation remains order-dependent**

If a standalone FastMCP 4 server's low-level server is instrumented before its wrapper, both calls use the same tracking key, so the wrapper call returns without installing FastMCP-specific argument stripping. The low-level adapter then advertises analytics parameters but forwards them unchanged, causing tools that do not declare those parameters to reject otherwise valid calls. The existing installation needs to support adding the wrapper-specific context, with coverage for this reverse order.

---

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

@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

posthog-python Compliance Report

Date: 2026-09-11 16:43:25 UTC
Duration: 256514ms

✅ 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 519ms
Endpoint And Method.Does Not Use Legacy Endpoints 512ms
Required Headers.Has Authorization Bearer Header 512ms
Required Headers.Has Content Type Json 512ms
Required Headers.Has Posthog Sdk Info Format 511ms
Required Headers.Has Posthog Attempt Header 512ms
Required Headers.Has Posthog Request Id 511ms
Required Headers.Has Posthog Request Timestamp 510ms
Required Headers.Has User Agent 511ms
Body Format.Body Has Created At And Batch 510ms
Body Format.No Api Key In Body 512ms
Body Format.No Sent At In Body 511ms
Event Format.Event Has Required Root Fields 510ms
Event Format.Event Uuid Is Valid 510ms
Event Format.Event Timestamp Is Rfc3339 511ms
Event Format.Distinct Id Is String 511ms
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 510ms
Event Format.Groups Properties Preserved 510ms
Event Format.Sdk Generates Uuid If Not Provided 513ms
Event Format.Event Has Required Root Fields Batch 514ms
Event Format.Event Uuid Is Valid Batch 514ms
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 516ms
Event Format.Custom Properties Preserved Batch 515ms
Event Format.Set Properties Preserved Batch 513ms
Event Format.Set Once Properties Preserved Batch 516ms
Event Format.Groups Properties Preserved Batch 515ms
Event Format.Sdk Generates Uuid If Not Provided Batch 514ms
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 1012ms
Batch Behavior.Created At Reflects Batch Creation Time 511ms
Deduplication.Generates Unique Uuids 521ms
Deduplication.Different Events Same Content Different Uuids 513ms
Deduplication.Preserves Uuid On Retry 6520ms
Deduplication.Preserves Timestamp On Retry 6519ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry 6523ms
Deduplication.No Duplicate Events In Batch 519ms
Header Behavior On Retry.Attempt Header Starts At One 511ms
Header Behavior On Retry.Attempt Header Increments On Retry 13520ms
Header Behavior On Retry.Request Id Preserved On Retry 6521ms
Header Behavior On Retry.Different Requests Have Different Request Ids 3022ms
Header Behavior On Retry.Request Timestamp Changes On Retry 6521ms
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 1516ms
Response Format Validation.Success No Retry After When Drop Only 513ms
Response Format Validation.Response Echoes Request Id 512ms
Retry Behavior.Retries On 408 6516ms
Retry Behavior.Retries On 500 6517ms
Retry Behavior.Retries On 503 8523ms
Retry Behavior.Retries On 504 6521ms
Retry Behavior.Retryable Errors Have Retry After 3517ms
Retry Behavior.Respects Retry After On Retryable Error 11525ms
Retry Behavior.Does Not Retry On 400 2514ms
Retry Behavior.Does Not Retry On 401 2513ms
Retry Behavior.Does Not Retry On 402 2514ms
Retry Behavior.Does Not Retry On 413 2513ms
Retry Behavior.Does Not Retry On 415 2515ms
Retry Behavior.Non Retryable Errors Have No Retry After 2514ms
Retry Behavior.Implements Backoff 22539ms
Retry Behavior.Max Retries Respected 22533ms
Partial Batch Handling.Handles 200 Full Success 2517ms
Partial Batch Handling.Handles 200 With All Ok 3516ms
Partial Batch Handling.Does Not Retry Dropped Events 3515ms
Partial Batch Handling.Does Not Retry Limited Events 3514ms
Partial Batch Handling.Prunes Ok Events On Partial Retry 6522ms
Partial Batch Handling.Prunes Dropped Events On Partial Retry 6521ms
Partial Batch Handling.Retries Only Retry Events From Partial 6524ms
Partial Batch Handling.Partial Retry Preserves Uuids 6518ms
Partial Batch Handling.Partial Retry Attempt Header Increments 6522ms
Partial Batch Handling.Partial Retry Request Id Preserved 6518ms
Partial Batch Handling.Respects Retry After On Partial 8517ms
Partial Batch Handling.Unknown Result Treated As Terminal 3517ms
Partial Batch Handling.Mixed Ok Drop Limited No Retry 3520ms
Compression.Sends Gzip Content Encoding 512ms
Compression.No Content Encoding When Disabled 511ms
Compression.Compressed Body Is Decompressible 512ms
Error Handling.Does Not Retry On Unknown 4Xx 2513ms
Event Options.Cookieless Mode Override 511ms
Event Options.Disable Skew Correction Override 511ms
Event Options.Process Person Profile Override 510ms
Event Options.Product Tour Id Override 511ms
Event Options.Unset Options Omitted 511ms
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 511ms

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 9ms
Request Payload.Flags Request Hits Flags Path Not Decide 9ms
Request Payload.Flags Request Omits Authorization Header 10ms
Request Payload.Token In Flags Body Matches Init 9ms
Request Payload.Groups Round Trip 9ms
Request Payload.Groups Default To Empty Object 10ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False 9ms
Request Payload.Disable Geoip Omitted Defaults To False 9ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key 10ms
Request Lifecycle.No Flags Request On Init Alone 3ms
Request Lifecycle.No Flags Request On Normal Capture 509ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests 18ms
Request Lifecycle.Mock Response Value Is Returned To Caller 11ms
Retry Behavior.Retries Flags On 502 316ms
Retry Behavior.Retries Flags On 504 315ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event 514ms

Keep a weak reference to the standalone FastMCP wrapper in shared tracking
state so a later wrapper install enriches existing low-level instrumentation
without wrapping handlers twice. Resolve schemas for the client-requested
tool version before stripping analytics-owned arguments.

Consolidate adapter imports at the optional MCP dependency boundary instead
of repeating imports in dispatch branches. Keep FastMCP-specific imports
lazy so the official SDK remains usable without that optional package.

Validation: reproduced both setup-order failures and the versioned-tool
argument failure before fixing them. MCP v1 suite 307 passed; MCP v2 suite
296 passed, 13 skipped. Ruff format/lint and filtered mypy passed.
Fresh-process checks passed with MCP absent and with FastMCP absent.
Decide which analytics parameters to remove before dispatch from the tools/list
the process served, recorded per tool in mutate_tool_schema, instead of asking
FastMCP for the tool schema on every call. The live lookup remains only for tools
never listed here or when the client pins a tool version, and now logs when it
fails. Tools supplied by ToolInjectionMiddleware are listed but not resolvable via
get_tool(), so the per-call lookup advertised `context` and then forwarded it,
failing validation.

Tested: posthog/test/mcp under mcp 2.2.0 + fastmcp 4.0.3 (297 passed, 13
skipped, new middleware test red before the fix) and under mcp 1.30.0 + fastmcp
3.2.0 (307 passed); ruff, mypy-baseline clean.

Reviewer note: tool_model_parameter_injected still exists alongside the new
tool_injected_parameters set because four adapters read it; folding it in is a
follow-up.

Claude-Session: https://claude.ai/code/session_015wZCNFdfPNK9k4utwJ5bMu
@lucasheriques
lucasheriques requested review from a team and removed request for a team September 10, 2026 16:44

@gesh gesh left a comment

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.

Note

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

QA Swarm review complete. See the inline comment.

return None


async def _standalone_injected_parameters(

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.

Note

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

[router] 🟡 MEDIUM

When server.get_tool() fails, or returns an object whose .parameters is not a dict, this function returns None. _wrap_v2_call_tool then skips the strip step and sends the raw arguments to the real dispatch. The arguments still contain the injected analytics keys (context, conversation_id, llm_model).

FastMCP binds tool arguments strictly. A direct call to fastmcp.FastMCP.call_tool with one unexpected keyword raises a pydantic ValidationError (verified in a fastmcp 4.0.3 environment). So this fail-open path can break dispatch — the opposite of the invariant the other except blocks in this PR protect.

The common case looks safe: FastMCP's own call_tool() resolves the tool through self.get_tool(name, version=...) at the same point, so a lookup failure usually breaks the underlying dispatch too. The residual risk is narrower: a middleware whose on_call_tool hook short-circuits before the manager stage and dispatches to a bound function directly. Dispatch then succeeds, but the PostHog lookup fails and the unstripped keys go through.

None of the new tests in test_fastmcp_v4.py build that shape, so the path is untested. This is plausible, not confirmed.

Suggested action: either add a test for a tool that dispatches but does not resolve through get_tool(), or confirm that the path is unreachable in the supported FastMCP middleware patterns and record that in the docstring.

gesh commented Sep 11, 2026

Copy link
Copy Markdown
Member

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 WITH NITS (round 1 @ 7027977)

The change does what it says: it restores analytics capture for standalone FastMCP 4, it keeps the behaviour stable across the three instrumentation orders, and the new tests cover those orders. One fail-open path stays untested.

Key findings

🟡 MEDIUM

  • posthog/mcp/_instrument_v2.py:412 — when the tool schema does not resolve through server.get_tool(), the code strips nothing and sends the injected analytics keys into the real dispatch. FastMCP binds arguments strictly, so an unexpected keyword raises a ValidationError. The common path is safe, because FastMCP resolves the tool the same way. The risk is a middleware that short-circuits before the manager stage. Plausible, not confirmed, and untested.

Convergence

No convergent findings — the router pass covered the diff alone. Danger was assessed MEDIUM with HIGH confidence, so no stronger model was needed.

Reviewer summaries

Reviewer Assessment
🧭 router Read the full diff with surrounding context, then built two environments (mcp 1.28 + fastmcp 3.2, and mcp 2.2 + fastmcp 4.0.3) and ran posthog/test/mcp/ on both — all tests pass. Verified _canonical_server() detection, instrument() idempotency and the weakref on all three orders, and the schema-strip bookkeeping. Also reproduced FastMCP's strict argument binding to confirm the risk model. Danger MEDIUM, confidence HIGH, delegated nothing.

Note: qa-team was not available in this environment (skill files not on disk). The router judged that no delegation was needed, so no other lens ran.


Automated by QA Swarm — not a human review

@gesh gesh left a comment

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.

Looks good! Left just a small comment

Extend FastMCP HTTP coverage to prove middleware tools dispatch when get_tool returns None. Exercise calls before listing, all advertised analytics arguments after listing, and application-owned arguments in both paths. Document why unknown arguments remain intact.

Validation: MCP v1 suite 307 passed; MCP v2 suite 299 passed, 13 skipped. Ruff check and format checks passed for MCP source and tests. Repository-wide filtered mypy passed for 231 source files.
Comment thread posthog/mcp/_instrument_v2.py Outdated
@veria-ai

veria-ai Bot commented Sep 11, 2026

Copy link
Copy Markdown

PR overview

All previously flagged issues have been addressed. No open security concerns remain on this pull request.

Security review

No open security issues remain on this pull request.

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

Remove the server-wide injected-parameter cache. Resolve effective schemas with the current request context, middleware, and requested tool version before stripping analytics arguments. Avoid using cached model ownership when schema resolution fails.

Add interleaved-client regression cases for dynamic providers, middleware tools, and middleware overrides in both listing orders. The provider and middleware cases reproduced a missing application-owned llm_model argument before the fix. Document the per-call schema lookup.

Validation: MCP v1 suite 307 passed; MCP v2 suite 305 passed, 13 skipped. Ruff check and format passed for MCP source and tests. Repository-wide filtered mypy passed for 231 files.
@lucasheriques
lucasheriques merged commit 9d965dd into main Sep 11, 2026
43 checks passed
@lucasheriques
lucasheriques deleted the codex/mcp-fastmcp-v4-capture branch September 11, 2026 17:07
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