Repository navigation
feat(mcp): tell directory reviews what get_more_tools records - #343
Merged
Merged
Conversation
What: get_more_tools annotations now declare readOnlyHint, destructiveHint, idempotentHint and openWorldHint as false (no title). The injected `context` description is shorter and asks for a few words by role. `llm_model` is optional (add_model_parameter defaults to required: false). Why: promised to OpenAI's plugin directory review. A get_more_tools call records an event, so it is not read-only or idempotent. Split from posthog-ruby#332, which adds a public constant and needs a human reviewer; this change touches no public API. Tested with: bundle exec rspec spec/posthog/mcp, bundle exec rspec, bundle exec rubocop, LANG=en_US.UTF-8 LC_ALL=en_US.UTF-8 bundle exec rake public_api:check (snapshot unchanged). Reviewer note: published tool definitions change, so directory listings may re-review. Changeset is minor.
posthog-ruby-sync Compliance ReportDate: 2026-11-06T20:26:47.478792+00:00
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ✅ | 9ms |
| Format Validation.Event Has Uuid | ✅ | 7ms |
| Format Validation.Event Has Lib Properties | ✅ | 6ms |
| Format Validation.Distinct Id Is String | ✅ | 9ms |
| Format Validation.Token Is Present | ✅ | 6ms |
| Format Validation.Custom Properties Preserved | ✅ | 8ms |
| Format Validation.Event Has Timestamp | ✅ | 7ms |
| Format Validation.Non Utc Event Timestamp Is Converted To Utc | ✅ | 7ms |
| Retry Behavior.Retries On 503 | ✅ | 5269ms |
| Retry Behavior.Does Not Retry On 400 | ❌ | 7ms |
| Retry Behavior.Does Not Retry On 401 | ❌ | 5ms |
| Retry Behavior.Respects Retry After Header | ✅ | 8015ms |
| Retry Behavior.Implements Backoff | ❌ | 257ms |
| Retry Behavior.Retries On 500 | ✅ | 5113ms |
| Retry Behavior.Retries On 502 | ✅ | 5116ms |
| Retry Behavior.Retries On 504 | ✅ | 5118ms |
| Retry Behavior.Max Retries Respected | ❌ | 513ms |
| Deduplication.Generates Unique Uuids | ✅ | 17ms |
| Deduplication.Preserves Uuid On Retry | ✅ | 5139ms |
| Deduplication.Preserves Uuid And Timestamp On Retry | ✅ | 10344ms |
| Deduplication.Preserves Uuid And Timestamp On Batch Retry | ✅ | 5145ms |
| Deduplication.No Duplicate Events In Batch | ✅ | 19ms |
| Deduplication.Different Events Have Different Uuids | ✅ | 9ms |
| Compression.Sends Gzip When Enabled | ✅ | 7ms |
| Batch Format.Uses Proper Batch Structure | ✅ | 6ms |
| Batch Format.Flush With No Events Sends Nothing | ✅ | 3ms |
| Batch Format.Multiple Events Batched Together | ❌ | 16ms |
| Error Handling.Does Not Retry On 403 | ❌ | 5ms |
| Error Handling.Does Not Retry On 413 | ❌ | 5ms |
| Error Handling.Retries On 408 | ✅ | 5114ms |
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 | ✅ | 9ms |
| Request Payload.Flags Request Uses V2 Query Param | ✅ | 8ms |
| Request Payload.Flags Request Hits Flags Path Not Decide | ✅ | 8ms |
| Request Payload.Flags Request Omits Authorization Header | ✅ | 7ms |
| Request Payload.Token In Flags Body Matches Init | ✅ | 8ms |
| Request Payload.Groups Round Trip | ✅ | 6ms |
| Request Payload.Groups Default To Empty Object | ✅ | 7ms |
| Request Payload.Disable Geoip False Propagates As Geoip Disable False | ✅ | 6ms |
| 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 | ✅ | 2ms |
| Request Lifecycle.No Flags Request On Normal Capture | ✅ | 6ms |
| Request Lifecycle.Two Flag Calls Produce Two Remote Requests | ✅ | 9ms |
| Request Lifecycle.Mock Response Value Is Returned To Caller | ✅ | 7ms |
| Retry Behavior.Retries Flags On 502 | ✅ | 110ms |
| Retry Behavior.Retries Flags On 504 | ✅ | 142ms |
| Side Effect Events.Get Feature Flag Captures Feature Flag Called Event | ✅ | 10ms |
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']
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approved.
The disclosed changes enter public-contract and analytics-privacy territory, with independent current-head assurance reported by the trusted context. No unresolved review concerns or concrete production or security showstoppers were found.
- PostHog Review (Standard) reviewed the current head: no findings.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 21L, 3F substantive, 45L/8F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (45L, 8F, cross-cutting, feat) |
| stamphog 2.7.0 | .stamphog/policy.yml @ 7b8fe43 · reviewed head 7b8fe43 |
posthog-ruby-async Compliance ReportDate: 2026-11-06T20:27:27.523553+00:00
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ✅ | 10ms |
| Format Validation.Event Has Uuid | ✅ | 106ms |
| Format Validation.Event Has Lib Properties | ✅ | 109ms |
| Format Validation.Distinct Id Is String | ✅ | 107ms |
| Format Validation.Token Is Present | ✅ | 113ms |
| Format Validation.Custom Properties Preserved | ✅ | 108ms |
| Format Validation.Event Has Timestamp | ✅ | 108ms |
| Format Validation.Non Utc Event Timestamp Is Converted To Utc | ✅ | 9ms |
| Retry Behavior.Retries On 503 | ✅ | 5307ms |
| Retry Behavior.Does Not Retry On 400 | ✅ | 2109ms |
| Retry Behavior.Does Not Retry On 401 | ✅ | 2109ms |
| Retry Behavior.Respects Retry After Header | ✅ | 8117ms |
| Retry Behavior.Implements Backoff | ✅ | 15623ms |
| Retry Behavior.Retries On 500 | ✅ | 5212ms |
| Retry Behavior.Retries On 502 | ✅ | 5212ms |
| Retry Behavior.Retries On 504 | ✅ | 5213ms |
| Retry Behavior.Max Retries Respected | ✅ | 15711ms |
| Deduplication.Generates Unique Uuids | ✅ | 112ms |
| Deduplication.Preserves Uuid On Retry | ✅ | 5212ms |
| Deduplication.Preserves Uuid And Timestamp On Retry | ✅ | 10417ms |
| Deduplication.Preserves Uuid And Timestamp On Batch Retry | ✅ | 5216ms |
| Deduplication.No Duplicate Events In Batch | ✅ | 112ms |
| Deduplication.Different Events Have Different Uuids | ✅ | 107ms |
| Compression.Sends Gzip When Enabled | ✅ | 107ms |
| Batch Format.Uses Proper Batch Structure | ✅ | 106ms |
| Batch Format.Flush With No Events Sends Nothing | ✅ | 4ms |
| Batch Format.Multiple Events Batched Together | ✅ | 111ms |
| Error Handling.Does Not Retry On 403 | ✅ | 2110ms |
| Error Handling.Does Not Retry On 413 | ✅ | 2108ms |
| Error Handling.Retries On 408 | ✅ | 5213ms |
Feature_Flags Tests
View Details
| Test | Status | Duration |
|---|---|---|
| Request Payload.Request With Person Properties Device Id | ✅ | 108ms |
| Request Payload.Flags Request Uses V2 Query Param | ✅ | 107ms |
| Request Payload.Flags Request Hits Flags Path Not Decide | ✅ | 108ms |
| Request Payload.Flags Request Omits Authorization Header | ✅ | 108ms |
| Request Payload.Token In Flags Body Matches Init | ✅ | 108ms |
| Request Payload.Groups Round Trip | ✅ | 107ms |
| Request Payload.Groups Default To Empty Object | ✅ | 107ms |
| Request Payload.Disable Geoip False Propagates As Geoip Disable False | ✅ | 107ms |
| Request Payload.Disable Geoip Omitted Defaults To False | ❌ | 107ms |
| Request Payload.Flag Keys To Evaluate Contains Only Requested Key | ✅ | 108ms |
| Request Lifecycle.No Flags Request On Init Alone | ✅ | 3ms |
| Request Lifecycle.No Flags Request On Normal Capture | ✅ | 106ms |
| Request Lifecycle.Two Flag Calls Produce Two Remote Requests | ✅ | 119ms |
| Request Lifecycle.Mock Response Value Is Returned To Caller | ✅ | 109ms |
| Retry Behavior.Retries Flags On 502 | ✅ | 212ms |
| Retry Behavior.Retries Flags On 504 | ✅ | 210ms |
| Side Effect Events.Get Feature Flag Captures Feature Flag Called Event | ✅ | 108ms |
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 |
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.
Problem
ChatGPT's and Claude's plugin directory reviews flag servers instrumented with
PostHog::MCP.get_more_toolsclaims to be read-only, idempotent and open-world, though it records an event in the server owner's PostHog project, and the injectedcontextdescription asks for more text than review allows. The MCP analytics spec (sdk-specs, "Virtual tool annotations" and "Arguments the SDK injects") now requires them.Changes
get_more_toolsannotations:readOnlyHint,destructiveHint,idempotentHintandopenWorldHintare allfalse, with notitle.contextdescription is the shared text every PostHog MCP SDK sends: a few words of purpose, people by role, no quoting the user.llm_modelis no longer required. A model the client sends in_metastill wins.This is the listing-only part of posthog-ruby#332, split out because it adds no public API. The unknown-tool event stays in #332.
How I tested
bundle exec rspec spec/posthog/mcp: 200 examples, 0 failures.bundle exec rspec: 2152 examples, 0 failures.bundle exec rubocopis clean.rake public_api:checkpasses with the snapshot unchanged.