From bdec85c9ed8d8c522bebce4acf849b95a0aa43ea Mon Sep 17 00:00:00 2001 From: Lucas Faria Date: Wed, 30 Sep 2026 13:37:18 -0300 Subject: [PATCH 1/3] fix(mcp): record only the tools/list response envelope $mcp_tools_list events no longer copy the tool descriptors into $mcp_response. Only the result's non-tools keys with a value (nextCursor, ttlMs, cacheScope, _meta) are recorded, and nothing when there are none. Names stay in $mcp_listed_tool_names. Avoids sanitizing and repeatedly re-truncating large catalogues. Mirrors posthog-js#5160. Tested: bundle exec rspec spec/posthog/mcp (178 examples, 0 failures); bundle exec rspec (1252 examples, 0 failures, 2 pending); bundle exec rubocop (129 files, no offenses); LANG=en_US.UTF-8 bundle exec rake public_api:check (passed). Notes: new specs fail without the change (4 failures). Manual Client#capture_tools_list unchanged. No lib/posthog/mcp/README.md exists and no in-repo doc claims descriptors are captured. --- .changeset/lean-tools-list-envelope.md | 5 +++ lib/posthog/mcp/instrumentation.rb | 10 +++++- spec/posthog/mcp/instrument_spec.rb | 46 ++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 .changeset/lean-tools-list-envelope.md diff --git a/.changeset/lean-tools-list-envelope.md b/.changeset/lean-tools-list-envelope.md new file mode 100644 index 0000000..14d9678 --- /dev/null +++ b/.changeset/lean-tools-list-envelope.md @@ -0,0 +1,5 @@ +--- +'posthog-ruby': patch +--- + +Stop copying the tool descriptors into `$mcp_response` on `$mcp_tools_list` events. The response keeps only the envelope (`nextCursor`, `ttlMs`, and so on) and the tool names stay in `$mcp_listed_tool_names`. diff --git a/lib/posthog/mcp/instrumentation.rb b/lib/posthog/mcp/instrumentation.rb index 9a9b7ed..ded2ddb 100644 --- a/lib/posthog/mcp/instrumentation.rb +++ b/lib/posthog/mcp/instrumentation.rb @@ -226,11 +226,19 @@ def dispatch_tools_list safely do session_id = prepare_request(@request) - record_tools_list(session_id, names: names, response: result, empty: empty) + record_tools_list(session_id, names: names, response: tools_list_envelope(result), empty: empty) end result end + def tools_list_envelope(result) + return nil unless result.is_a?(Hash) + + tools_key = SchemaMutation.key_for(result, :tools) + envelope = result.reject { |key, value| key == tools_key || value.nil? } + envelope unless envelope.empty? + end + def dispatch_initialize client_info = fetch(@params, :clientInfo) client_name = client_info.is_a?(Hash) ? fetch(client_info, :name) : nil diff --git a/spec/posthog/mcp/instrument_spec.rb b/spec/posthog/mcp/instrument_spec.rb index 66561aa..2bf8966 100644 --- a/spec/posthog/mcp/instrument_spec.rb +++ b/spec/posthog/mcp/instrument_spec.rb @@ -363,6 +363,52 @@ def initialize_request(id = 1, version = '2025-06-18') expect(listing['$mcp_error_message']).to eq('tools/list returned no tools') expect(events.map { |e| e[:event] }).to include('$exception') end + + describe 'tools/list response capture' do + let(:names) { %w[echo boom owns_context structured soft_fail] } + + def stringify_keys_via_around_request(target) + target.configuration = MCP::Configuration.new(around_request: lambda { |_data, &handler| + handler.call.transform_keys(&:to_s) + }) + end + + { 'symbol keys' => false, 'string keys' => true }.each do |label, stringify| + it "records only the envelope with #{label}" do + list_server = MCP::Server.new(name: 'spec-server', tools: tools, ttl_ms: 5000, cache_scope: 'public') + stringify_keys_via_around_request(list_server) if stringify + described_class.instrument(list_server, client) + result = list_server.handle(rpc(1, 'tools/list'))[:result] + + properties = events_named(drain_events(client), '$mcp_tools_list').first[:properties] + expect(properties['$mcp_response']).to eq('ttlMs' => 5000, 'cacheScope' => 'public') + expect(properties['$mcp_listed_tool_names']).to eq(names) + expect((result[:tools] || result['tools']).length).to eq(5) + end + end + + it 'records the cursor of a paginated result and no tool descriptors' do + paged = MCP::Server.new(name: 'spec-server', tools: tools, page_size: 2) + described_class.instrument(paged, client) + result = paged.handle(rpc(1, 'tools/list'))[:result] + + properties = events_named(drain_events(client), '$mcp_tools_list').first[:properties] + expect(result[:nextCursor]).to be_a(String) + expect(properties['$mcp_response']).to eq('nextCursor' => result[:nextCursor]) + expect(properties['$mcp_listed_tool_names']).to eq(names.first(2)) + expect(result[:tools].map { |tool| tool[:name] }).to eq(names.first(2)) + end + + it 'omits $mcp_response when the result has only tools' do + described_class.instrument(server, client) + result = server.handle(rpc(1, 'tools/list'))[:result] + + properties = events_named(drain_events(client), '$mcp_tools_list').first[:properties] + expect(properties).not_to have_key('$mcp_response') + expect(properties['$mcp_listed_tool_names']).to eq(names) + expect(result[:tools].length).to eq(5) + end + end end describe 'identify, event_properties and before_send' do From bd0d50641d08f3d430110963f8164289eb734030 Mon Sep 17 00:00:00 2001 From: Lucas Faria Date: Wed, 30 Sep 2026 13:50:54 -0300 Subject: [PATCH 2/3] fix(mcp): drop resultType from the tools/list envelope and table the specs Review follow-up. On the 2026-07-28 wire the gem stamps resultType (a lifecycle discriminator) on every reply before PostHog's hook sees it, so it reached $mcp_response on every event. It is now dropped with tools. Key matching uses key.to_s, which also covers a hash holding both :tools and 'tools'. The four tools/list specs are one table: cache hints with symbol and string keys, only tools, and the modern wire, where the gem's ttlMs/cacheScope defaults stay because they are what the client receives. Tested: bundle exec rspec (see PR), rubocop clean, public_api:check passed. --- lib/posthog/mcp/instrumentation.rb | 4 +-- spec/posthog/mcp/instrument_spec.rb | 48 ++++++++++++----------------- 2 files changed, 21 insertions(+), 31 deletions(-) diff --git a/lib/posthog/mcp/instrumentation.rb b/lib/posthog/mcp/instrumentation.rb index ded2ddb..09ed4e3 100644 --- a/lib/posthog/mcp/instrumentation.rb +++ b/lib/posthog/mcp/instrumentation.rb @@ -231,11 +231,11 @@ def dispatch_tools_list result end + # `resultType` is a lifecycle discriminator the gem stamps on every modern-wire reply, not envelope data. def tools_list_envelope(result) return nil unless result.is_a?(Hash) - tools_key = SchemaMutation.key_for(result, :tools) - envelope = result.reject { |key, value| key == tools_key || value.nil? } + envelope = result.reject { |key, value| %w[tools resultType].include?(key.to_s) || value.nil? } envelope unless envelope.empty? end diff --git a/spec/posthog/mcp/instrument_spec.rb b/spec/posthog/mcp/instrument_spec.rb index 2bf8966..4645320 100644 --- a/spec/posthog/mcp/instrument_spec.rb +++ b/spec/posthog/mcp/instrument_spec.rb @@ -366,48 +366,38 @@ def initialize_request(id = 1, version = '2025-06-18') describe 'tools/list response capture' do let(:names) { %w[echo boom owns_context structured soft_fail] } + let(:modern_meta) do + { _meta: { 'io.modelcontextprotocol/protocolVersion' => '2026-07-28', + 'io.modelcontextprotocol/clientCapabilities' => {} } } + end - def stringify_keys_via_around_request(target) + def stringify_result_keys(target) target.configuration = MCP::Configuration.new(around_request: lambda { |_data, &handler| handler.call.transform_keys(&:to_s) }) end - { 'symbol keys' => false, 'string keys' => true }.each do |label, stringify| - it "records only the envelope with #{label}" do - list_server = MCP::Server.new(name: 'spec-server', tools: tools, ttl_ms: 5000, cache_scope: 'public') - stringify_keys_via_around_request(list_server) if stringify + [ + ['cache hints, symbol keys', { ttl_ms: 5000, cache_scope: 'public' }, false, false, + { 'ttlMs' => 5000, 'cacheScope' => 'public' }], + ['cache hints, string keys', { ttl_ms: 5000, cache_scope: 'public' }, true, false, + { 'ttlMs' => 5000, 'cacheScope' => 'public' }], + ['only tools', {}, false, false, nil], + ['modern wire, gem defaults', {}, false, true, { 'ttlMs' => 0, 'cacheScope' => 'private' }] + ].each do |label, server_options, string_keys, modern, expected_response| + it "records only the envelope: #{label}" do + list_server = MCP::Server.new(name: 'spec-server', tools: tools, **server_options) + stringify_result_keys(list_server) if string_keys described_class.instrument(list_server, client) - result = list_server.handle(rpc(1, 'tools/list'))[:result] + result = list_server.handle(rpc(1, 'tools/list', modern ? modern_meta : nil))[:result] properties = events_named(drain_events(client), '$mcp_tools_list').first[:properties] - expect(properties['$mcp_response']).to eq('ttlMs' => 5000, 'cacheScope' => 'public') + expect(properties['$mcp_response']).to eq(expected_response) + expect(properties.key?('$mcp_response')).to eq(!expected_response.nil?) expect(properties['$mcp_listed_tool_names']).to eq(names) expect((result[:tools] || result['tools']).length).to eq(5) end end - - it 'records the cursor of a paginated result and no tool descriptors' do - paged = MCP::Server.new(name: 'spec-server', tools: tools, page_size: 2) - described_class.instrument(paged, client) - result = paged.handle(rpc(1, 'tools/list'))[:result] - - properties = events_named(drain_events(client), '$mcp_tools_list').first[:properties] - expect(result[:nextCursor]).to be_a(String) - expect(properties['$mcp_response']).to eq('nextCursor' => result[:nextCursor]) - expect(properties['$mcp_listed_tool_names']).to eq(names.first(2)) - expect(result[:tools].map { |tool| tool[:name] }).to eq(names.first(2)) - end - - it 'omits $mcp_response when the result has only tools' do - described_class.instrument(server, client) - result = server.handle(rpc(1, 'tools/list'))[:result] - - properties = events_named(drain_events(client), '$mcp_tools_list').first[:properties] - expect(properties).not_to have_key('$mcp_response') - expect(properties['$mcp_listed_tool_names']).to eq(names) - expect(result[:tools].length).to eq(5) - end end end From 64b97aad6dd3f74a6b2b1c74130bea7747cdd906 Mon Sep 17 00:00:00 2001 From: Lucas Faria Date: Wed, 30 Sep 2026 14:51:06 -0300 Subject: [PATCH 3/3] test(mcp): restore tools/list pagination coverage Review follow-up (Greptile). The spec table lost the only check that a paginated tools/list records its nextCursor and lists only the current page's names. Added it back as a row. Tested: bundle exec rspec spec/posthog/mcp passes; rubocop clean. --- spec/posthog/mcp/instrument_spec.rb | 33 ++++++++++++++++------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/spec/posthog/mcp/instrument_spec.rb b/spec/posthog/mcp/instrument_spec.rb index 4645320..9598ecb 100644 --- a/spec/posthog/mcp/instrument_spec.rb +++ b/spec/posthog/mcp/instrument_spec.rb @@ -378,24 +378,27 @@ def stringify_result_keys(target) end [ - ['cache hints, symbol keys', { ttl_ms: 5000, cache_scope: 'public' }, false, false, - { 'ttlMs' => 5000, 'cacheScope' => 'public' }], - ['cache hints, string keys', { ttl_ms: 5000, cache_scope: 'public' }, true, false, - { 'ttlMs' => 5000, 'cacheScope' => 'public' }], - ['only tools', {}, false, false, nil], - ['modern wire, gem defaults', {}, false, true, { 'ttlMs' => 0, 'cacheScope' => 'private' }] - ].each do |label, server_options, string_keys, modern, expected_response| - it "records only the envelope: #{label}" do - list_server = MCP::Server.new(name: 'spec-server', tools: tools, **server_options) - stringify_result_keys(list_server) if string_keys + { label: 'cache hints, symbol keys', options: { ttl_ms: 5000, cache_scope: 'public' }, + response: { 'ttlMs' => 5000, 'cacheScope' => 'public' } }, + { label: 'cache hints, string keys', options: { ttl_ms: 5000, cache_scope: 'public' }, string_keys: true, + response: { 'ttlMs' => 5000, 'cacheScope' => 'public' } }, + { label: 'only tools', response: nil }, + { label: 'first page', options: { page_size: 2 }, listed: 2, response: :next_cursor }, + { label: 'modern wire, gem defaults', modern: true, response: { 'ttlMs' => 0, 'cacheScope' => 'private' } } + ].each do |row| + it "records only the envelope: #{row[:label]}" do + list_server = MCP::Server.new(name: 'spec-server', tools: tools, **row.fetch(:options, {})) + stringify_result_keys(list_server) if row[:string_keys] described_class.instrument(list_server, client) - result = list_server.handle(rpc(1, 'tools/list', modern ? modern_meta : nil))[:result] + result = list_server.handle(rpc(1, 'tools/list', row[:modern] ? modern_meta : nil))[:result] + listed = names.first(row.fetch(:listed, names.length)) + expected = row[:response] == :next_cursor ? { 'nextCursor' => result[:nextCursor] } : row[:response] properties = events_named(drain_events(client), '$mcp_tools_list').first[:properties] - expect(properties['$mcp_response']).to eq(expected_response) - expect(properties.key?('$mcp_response')).to eq(!expected_response.nil?) - expect(properties['$mcp_listed_tool_names']).to eq(names) - expect((result[:tools] || result['tools']).length).to eq(5) + expect(properties['$mcp_response']).to eq(expected) + expect(properties.key?('$mcp_response')).to eq(!expected.nil?) + expect(properties['$mcp_listed_tool_names']).to eq(listed) + expect((result[:tools] || result['tools']).map { |tool| tool[:name] || tool['name'] }).to eq(listed) end end end