Repository navigation
feat(mcp): align PostHog::MCP with the MCP analytics spec - #332
Merged
Merged
Conversation
posthog-ruby-sync Compliance ReportDate: 2026-11-06T20:57:10.808242+00:00
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ✅ | 8ms |
| Format Validation.Event Has Uuid | ✅ | 5ms |
| Format Validation.Event Has Lib Properties | ✅ | 5ms |
| Format Validation.Distinct Id Is String | ✅ | 8ms |
| Format Validation.Token Is Present | ✅ | 6ms |
| Format Validation.Custom Properties Preserved | ✅ | 7ms |
| Format Validation.Event Has Timestamp | ✅ | 6ms |
| Format Validation.Non Utc Event Timestamp Is Converted To Utc | ✅ | 6ms |
| Retry Behavior.Retries On 503 | ✅ | 5322ms |
| Retry Behavior.Does Not Retry On 400 | ❌ | 6ms |
| Retry Behavior.Does Not Retry On 401 | ❌ | 4ms |
| Retry Behavior.Respects Retry After Header | ✅ | 8015ms |
| Retry Behavior.Implements Backoff | ❌ | 226ms |
| Retry Behavior.Retries On 500 | ✅ | 5112ms |
| Retry Behavior.Retries On 502 | ✅ | 5113ms |
| Retry Behavior.Retries On 504 | ✅ | 5142ms |
| Retry Behavior.Max Retries Respected | ❌ | 647ms |
| Deduplication.Generates Unique Uuids | ✅ | 14ms |
| Deduplication.Preserves Uuid On Retry | ✅ | 5142ms |
| Deduplication.Preserves Uuid And Timestamp On Retry | ✅ | 10279ms |
| Deduplication.Preserves Uuid And Timestamp On Batch Retry | ✅ | 5119ms |
| Deduplication.No Duplicate Events In Batch | ✅ | 14ms |
| Deduplication.Different Events Have Different Uuids | ✅ | 7ms |
| Compression.Sends Gzip When Enabled | ✅ | 4ms |
| Batch Format.Uses Proper Batch Structure | ✅ | 5ms |
| Batch Format.Flush With No Events Sends Nothing | ✅ | 2ms |
| Batch Format.Multiple Events Batched Together | ❌ | 12ms |
| Error Handling.Does Not Retry On 403 | ❌ | 4ms |
| Error Handling.Does Not Retry On 413 | ❌ | 3ms |
| Error Handling.Retries On 408 | ✅ | 5113ms |
Failures
retry_behavior.does_not_retry_on_400
500, message='Internal Server Error', url='http://sdk-adapter:8080/capture'
retry_behavior.does_not_retry_on_401
500, message='Internal Server Error', url='http://sdk-adapter:8080/capture'
retry_behavior.implements_backoff
500, message='Internal Server Error', url='http://sdk-adapter:8080/capture'
retry_behavior.max_retries_respected
500, message='Internal Server Error', url='http://sdk-adapter:8080/capture'
batch_format.multiple_events_batched_together
Expected 1 requests, got 5
error_handling.does_not_retry_on_403
500, message='Internal Server Error', url='http://sdk-adapter:8080/capture'
error_handling.does_not_retry_on_413
500, message='Internal Server Error', url='http://sdk-adapter:8080/capture'
Feature_Flags Tests
View Details
| Test | Status | Duration |
|---|---|---|
| Request Payload.Request With Person Properties Device Id | ✅ | 8ms |
| Request Payload.Flags Request Uses V2 Query Param | ✅ | 6ms |
| Request Payload.Flags Request Hits Flags Path Not Decide | ✅ | 7ms |
| Request Payload.Flags Request Omits Authorization Header | ✅ | 6ms |
| Request Payload.Token In Flags Body Matches Init | ✅ | 5ms |
| Request Payload.Groups Round Trip | ✅ | 5ms |
| Request Payload.Groups Default To Empty Object | ✅ | 5ms |
| Request Payload.Disable Geoip False Propagates As Geoip Disable False | ✅ | 5ms |
| Request Payload.Disable Geoip Omitted Defaults To False | ❌ | 4ms |
| Request Payload.Flag Keys To Evaluate Contains Only Requested Key | ✅ | 5ms |
| Request Lifecycle.No Flags Request On Init Alone | ✅ | 1ms |
| Request Lifecycle.No Flags Request On Normal Capture | ✅ | 4ms |
| Request Lifecycle.Two Flag Calls Produce Two Remote Requests | ✅ | 7ms |
| Request Lifecycle.Mock Response Value Is Returned To Caller | ✅ | 5ms |
| Retry Behavior.Retries Flags On 502 | ✅ | 134ms |
| Retry Behavior.Retries Flags On 504 | ✅ | 108ms |
| Side Effect Events.Get Feature Flag Captures Feature Flag Called Event | ✅ | 7ms |
Failures
request_payload.disable_geoip_omitted_defaults_to_false
Field 'geoip_disable' not found in /flags request body at path 'geoip_disable'. Available keys: ['distinct_id', 'groups', 'person_properties', 'group_properties', 'flag_keys_to_evaluate', 'token']
posthog-ruby-async Compliance ReportDate: 2026-11-06T20:57:47.324154+00:00
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ✅ | 108ms |
| Format Validation.Event Has Uuid | ✅ | 106ms |
| Format Validation.Event Has Lib Properties | ✅ | 107ms |
| Format Validation.Distinct Id Is String | ✅ | 105ms |
| Format Validation.Token Is Present | ✅ | 105ms |
| Format Validation.Custom Properties Preserved | ✅ | 105ms |
| Format Validation.Event Has Timestamp | ✅ | 105ms |
| Format Validation.Non Utc Event Timestamp Is Converted To Utc | ✅ | 106ms |
| Retry Behavior.Retries On 503 | ✅ | 5412ms |
| Retry Behavior.Does Not Retry On 400 | ✅ | 2109ms |
| Retry Behavior.Does Not Retry On 401 | ✅ | 2108ms |
| Retry Behavior.Respects Retry After Header | ✅ | 8110ms |
| Retry Behavior.Implements Backoff | ✅ | 15616ms |
| Retry Behavior.Retries On 500 | ✅ | 5211ms |
| Retry Behavior.Retries On 502 | ✅ | 5210ms |
| Retry Behavior.Retries On 504 | ✅ | 5211ms |
| Retry Behavior.Max Retries Respected | ✅ | 15516ms |
| Deduplication.Generates Unique Uuids | ✅ | 109ms |
| Deduplication.Preserves Uuid On Retry | ✅ | 5207ms |
| Deduplication.Preserves Uuid And Timestamp On Retry | ✅ | 10414ms |
| Deduplication.Preserves Uuid And Timestamp On Batch Retry | ✅ | 5208ms |
| Deduplication.No Duplicate Events In Batch | ✅ | 110ms |
| Deduplication.Different Events Have Different Uuids | ✅ | 107ms |
| Compression.Sends Gzip When Enabled | ✅ | 107ms |
| Batch Format.Uses Proper Batch Structure | ✅ | 105ms |
| Batch Format.Flush With No Events Sends Nothing | ✅ | 4ms |
| Batch Format.Multiple Events Batched Together | ✅ | 108ms |
| Error Handling.Does Not Retry On 403 | ✅ | 2107ms |
| Error Handling.Does Not Retry On 413 | ✅ | 2108ms |
| Error Handling.Retries On 408 | ✅ | 5211ms |
Feature_Flags Tests
View Details
| Test | Status | Duration |
|---|---|---|
| Request Payload.Request With Person Properties Device Id | ✅ | 106ms |
| Request Payload.Flags Request Uses V2 Query Param | ✅ | 107ms |
| Request Payload.Flags Request Hits Flags Path Not Decide | ✅ | 106ms |
| Request Payload.Flags Request Omits Authorization Header | ✅ | 106ms |
| Request Payload.Token In Flags Body Matches Init | ✅ | 106ms |
| Request Payload.Groups Round Trip | ✅ | 107ms |
| Request Payload.Groups Default To Empty Object | ✅ | 108ms |
| Request Payload.Disable Geoip False Propagates As Geoip Disable False | ✅ | 106ms |
| Request Payload.Disable Geoip Omitted Defaults To False | ❌ | 106ms |
| Request Payload.Flag Keys To Evaluate Contains Only Requested Key | ✅ | 106ms |
| Request Lifecycle.No Flags Request On Init Alone | ✅ | 3ms |
| Request Lifecycle.No Flags Request On Normal Capture | ✅ | 104ms |
| Request Lifecycle.Two Flag Calls Produce Two Remote Requests | ✅ | 109ms |
| Request Lifecycle.Mock Response Value Is Returned To Caller | ✅ | 105ms |
| Retry Behavior.Retries Flags On 502 | ✅ | 245ms |
| Retry Behavior.Retries Flags On 504 | ✅ | 253ms |
| Side Effect Events.Get Feature Flag Captures Feature Flag Called Event | ✅ | 107ms |
Failures
request_payload.disable_geoip_omitted_defaults_to_false
Field 'geoip_disable' not found in /flags request body at path 'geoip_disable'. Available keys: ['distinct_id', 'groups', 'person_properties', 'group_properties', 'flag_keys_to_evaluate', 'token']
SDK performance: posthog-rubyBenchmarks completed. No changes flagged for review. Flags require at least a 5% change in the median and all six pairs agreeing on direction. This is a noise filter, not proof of a performance change or a CI gate. View all results
Values are medians of paired measurements. Not flagged does not mean unchanged. Unavailable means a measurement could not be provided, not zero. Estimated allocation measurements retain their instrumentation uncertainty. RSS measures the whole process, not just SDK allocations; observed peaks are sampled checkpoints. Paired ranges in the diagnostics describe variability, not confidence intervals. Each shard uses its own runner; compare Before PR against With PR within a row, not across shards. Compared 102 metric rows; 102 rows shown in all results. Omitted or shortened data remain in |
lucasheriques
marked this pull request as ready for review
November 6, 2026 17:05
![posthog[bot]](https://avatars.githubusercontent.com/in/1203049?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
PostHog Review · Standard · reviewed 02608a6: 1 Should fix.
See this review and its findings in PostHog Review. Comment @posthog review --deep for a Deep review. A Deep review takes longer and looks deeper than a Standard review.
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
Not approved — this change needs a human reviewer.
Re-adding the stamphog label gives the same result unless the changed files change.
stamphog can't approve this pull request automatically. Two gates refused it: the deny-list gate matched public_api, because public_api_snapshot.txt changes with the new PostHog::MCP::Event::UNKNOWN_TOOL constant. The tier gate classified the change as T2-never (132 lines across 11 files, cross-cutting, a feature), a category that always needs a human reviewer.
The size and prerequisites gates passed, so size is not the problem. Please ask a human maintainer to review this pull request. Splitting it into smaller pieces, such as the get_more_tools annotations, the llm_model default, and the unknown-tool event, may make that review easier, though the public API change will still need a person to look at it.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✗ | matches: public_api |
| size | ✓ | 59L, 5F substantive, 132L/11F incl. docs/generated/snapshots — within ceiling |
| tier | ✗ | classified as T2-never: T2-never (132L, 11F, cross-cutting, feat) |
| stamphog 2.7.0 | .stamphog/policy.yml @ unknown · reviewed head 953d6d2 |
…ered tool A tools/call for a tool the server never registered now emits $mcp_unknown_tool instead of a failed $mcp_tool_call; the client gets the same error as before. The event never triggers $mcp_initialize and keeps a conversation handle the caller echoed. The event name lives in the @api private EventType map, so public_api_snapshot.txt is unchanged. Tested: bundle exec rspec (2169 examples, 0 failures), rubocop clean, rake public_api:check passes.
lucasheriques
force-pushed
the
lucas/mcp-v1-spec-align
branch
from
November 6, 2026 20:55
953d6d2 to
fa6d54f
Compare
There was a problem hiding this comment.
Not approved — escalated to a human reviewer.
Re-add the stamphog label to request another review once you have addressed this.
The analytics contract and capture-path changes lack current-head independent assurance; the older bot review supplies no actionable finding in the provided context. Request review from Rafael Audibert, another teammate, or an agent reviewer covering these changes before re-requesting.
- Author wrote 0% of the modified lines and has 6 merged PRs in these paths (familiarity MODERATE).
- Risky territory without independent assurance: Public API contracts — lib/posthog/mcp/constants.rb and lib/posthog/mcp/event_builder.rb change the externally emitted analytics event and tool-name properties for unregistered tool requests.; Data ingestion or write paths — lib/posthog/mcp/instrumentation.rb routes unregistered tool requests through a new capture path instead of recording failed tool calls and exception events.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 37L, 3F substantive, 95L/5F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (95L, 5F, cross-cutting, feat) |
| stamphog 2.7.0 | .stamphog/policy.yml @ fa6d54f · reviewed head fa6d54f |
Contributor
Author
|
@PostHog review |
Contributor
|
@lucasheriques PostHog Review is reviewing this pull request. |
There was a problem hiding this comment.
Approved.
The analytics write-path and event-contract changes have independent assurance from the current-head agent review. No concrete unresolved review concerns or other refusal grounds are present.
- Author wrote 0% of the modified lines and has 6 merged PRs in these paths (familiarity MODERATE).
- PostHog Review (Standard) reviewed the current head: no findings.
- posthog[bot] reviewed the current head.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 37L, 3F substantive, 95L/5F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (95L, 5F, cross-cutting, feat) |
| stamphog 2.7.0 | .stamphog/policy.yml @ fa6d54f · reviewed head fa6d54f |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
We're aligning every PostHog MCP SDK with the sdk-specs MCP spec before
@posthog/mcp1.0 and a stableposthog.mcp, so customers' insights mean the same thing whichever SDK sent the data.Changes
get_more_toolsannotations are allfalse, per "Virtual tool annotations". The tool records an event in the developer's project, so it isn't read-only, and repeat calls send more events. Thetitleis gone because the spec's acceptance scenario matches the annotations JSON exactly. The Go adapter makes the same change in posthog-go#418.contextdescription is the shared text every SDK now sends: a few words of purpose, people by role, no quoting the user.llm_modelis optional (add_model_parameterdefaults torequired: false), per sdk-specs#189.$mcp_unknown_toolwith$mcp_tool_name, session and identity, instead of a failed$mcp_tool_callplus$exception. The client gets the same error as before. The spec says an unregistered tool is never a tool call. A server whose registry the SDK can't read still records a failed call, as the spec allows.EXPERIMENTAL_NOTICEand the README banner are unchanged.How I tested
bundle exec rspec spec/posthog/mcp: 203 examples, 0 failures, 2 pending (existing auth_info skips).bundle exec rspec: 2111 examples, 0 failures, 40 pending.bundle exec rubocop: no offenses.tools/list$mcp_unknown_tool, with no sibling events and a client response identical to an uninstrumented serverPre-PR gate: a Sonnet adversary found an unknown-tool call also sent
$mcp_initializewith noinitializerequest. Fixed, and the spec now asserts only$mcp_unknown_toolis sent. Its concurrency runs (60 and 100 threads) misclassified nothing. Ordinary tool calls still send$mcp_initializeon a session's first request, as main does today; that's a separate fix.