diff --git a/.changeset/tidy-models-report.md b/.changeset/tidy-models-report.md new file mode 100644 index 0000000..2a981a8 --- /dev/null +++ b/.changeset/tidy-models-report.md @@ -0,0 +1,5 @@ +--- +'posthog-ruby': minor +--- + +Enable MCP model capture and conversation ids by default, matching the JavaScript and Python MCP SDKs. Pass `capture_model: false` or `enable_conversation_id: false` to opt out. diff --git a/lib/posthog/mcp/client.rb b/lib/posthog/mcp/client.rb index 266426b..efaed13 100644 --- a/lib/posthog/mcp/client.rb +++ b/lib/posthog/mcp/client.rb @@ -17,10 +17,13 @@ class Client < PostHog::Client # @param opts [Hash] {PostHog::Client} options plus: # @option opts [String] :missing_capability_tool_name name of the virtual tool (default `get_more_tools`) # @option opts [Boolean] :mcp_exception_autocapture emit a sibling `$exception` for failed calls (default true) + # @option opts [Boolean, ModelOptions] :capture_model capture the calling model (default true) def initialize(opts = {}) opts = opts.transform_keys(&:to_sym) @missing_capability_tool_name = opts.delete(:missing_capability_tool_name) || Tools::GET_MORE_TOOLS_NAME @mcp_exception_autocapture = opts.delete(:mcp_exception_autocapture) != false + capture_model = opts.key?(:capture_model) ? opts.delete(:capture_model) : true + @capture_model_option = Options.new(capture_model: capture_model).capture_model super @mcp_sink = Sink.new(self) @mcp_options = Options.new( @@ -113,16 +116,20 @@ def capture_missing_capability(context: nil, parameters: nil, protocol_version: emit(event) end - # Inject the `context` argument (and, with `capture_model`, `llm_model`) into + # Inject the `context` and `llm_model` arguments into # every tool descriptor (Hash with `inputSchema`) so agents state their intent, # and optionally append the `get_more_tools` virtual tool. Returns a new Array # of new Hashes. A tool whose schema is composed (oneOf/allOf/anyOf) or a - # `$ref` is passed through untouched. + # `$ref` is passed through untouched. Pass `capture_model: false` here or + # to {#initialize} to disable model capture for this client, including + # extraction from later tool calls. # # @param tools [Array] `tools/list` entries # @return [Array] - def prepare_tool_list(tools, context: true, report_missing: false, capture_model: false) + def prepare_tool_list(tools, context: true, report_missing: false, capture_model: @capture_model_option) options = Options.new(context: context, capture_model: capture_model) + @capture_model_option = false unless options.capture_model_enabled? + options = Options.new(context: context, capture_model: false) if @capture_model_option == false prepared = tools.map do |tool| next tool unless tool.is_a?(Hash) && (options.context_enabled? || options.capture_model_enabled?) @@ -165,7 +172,7 @@ def prepare_tool_list(tools, context: true, report_missing: false, capture_model # @return [PreparedToolCall] def prepare_tool_call(name, args = nil, input_schema: nil) intent = tool_declares?(input_schema, 'context') ? nil : Intent.normalize(argument(args, 'context')) - model = if tool_declares?(input_schema, ModelCapture::PARAM_NAME) + model = if @capture_model_option == false || tool_declares?(input_schema, ModelCapture::PARAM_NAME) nil else ModelCapture.normalize(argument(args, ModelCapture::PARAM_NAME)) @@ -199,6 +206,8 @@ def base_event(event_type, distinct_id, session_id, set_properties, groups, prop end def apply_model(event, llm_model, source) + return if @capture_model_option == false + model = ModelCapture.normalize(llm_model) return unless model @@ -236,9 +245,11 @@ def argument(args, param) def strip_injected(args, input_schema) return args unless args.is_a?(Hash) - keys = ['context', ModelCapture::PARAM_NAME].reject { |param| tool_declares?(input_schema, param) } - .flat_map { |param| [param.to_sym, param] } - .select { |key| args.key?(key) } + params = ['context'] + params << ModelCapture::PARAM_NAME unless @capture_model_option == false + keys = params.reject { |param| tool_declares?(input_schema, param) } + .flat_map { |param| [param.to_sym, param] } + .select { |key| args.key?(key) } keys.empty? ? args : args.except(*keys) end end diff --git a/lib/posthog/mcp/instrumentation.rb b/lib/posthog/mcp/instrumentation.rb index 3736fac..9a9b7ed 100644 --- a/lib/posthog/mcp/instrumentation.rb +++ b/lib/posthog/mcp/instrumentation.rb @@ -506,9 +506,8 @@ def warn_stateless_session_not_wired 'Warning: an MCP request arrived over streamable HTTP with no session id, so PostHog generated a ' \ 'per-process $session_id that will fragment across requests and pods. In stateless mode the ' \ 'client must replay the Mcp-Session-Id header PostHog::MCP mints at initialize; for a custom Rack ' \ - 'stack add PostHog::MCP::RackMiddleware. Enabling conversation ids ' \ - '(PostHog::MCP.instrument(server, enable_conversation_id: true)) also anchors the session without ' \ - 'any middleware. See https://posthog.com/docs/mcp-analytics/installation#ruby.' + 'stack add PostHog::MCP::RackMiddleware. Conversation ids, which are enabled by default, also anchor ' \ + 'the session without any middleware. See https://posthog.com/docs/mcp-analytics/installation#ruby.' ) end diff --git a/lib/posthog/mcp/options.rb b/lib/posthog/mcp/options.rb index ac472ec..c606fdf 100644 --- a/lib/posthog/mcp/options.rb +++ b/lib/posthog/mcp/options.rb @@ -52,13 +52,13 @@ class Options attr_reader :report_missing # @return [String] Name of the virtual tool. Default `get_more_tools`. attr_reader :missing_capability_tool_name - # @return [Boolean] Inject `conversation_id` and anchor `$session_id` on it. Default false. + # @return [Boolean] Inject `conversation_id` and anchor `$session_id` on it. Default true. attr_reader :enable_conversation_id # @return [Boolean] Emit a sibling `$exception` event for failed calls. Default true. attr_reader :enable_exception_autocapture # @return [Boolean, ContextOptions] Inject the required `context` argument. Default true. attr_reader :context - # @return [Boolean, ModelOptions] Capture `$mcp_llm_model`. Default false. + # @return [Boolean, ModelOptions] Capture `$mcp_llm_model`. Default true. attr_reader :capture_model # @return [#call, UserIdentity, Hash, nil] `(request, extra) -> UserIdentity | Hash | nil`, or a static identity. attr_reader :identify @@ -70,8 +70,8 @@ class Options attr_reader :event_properties def initialize(logger: nil, report_missing: false, missing_capability_tool_name: nil, - enable_conversation_id: false, enable_exception_autocapture: true, context: true, - capture_model: false, identify: nil, intent_fallback: nil, before_send: nil, + enable_conversation_id: true, enable_exception_autocapture: true, context: true, + capture_model: true, identify: nil, intent_fallback: nil, before_send: nil, event_properties: nil) @logger = logger @report_missing = report_missing == true diff --git a/spec/posthog/mcp/client_spec.rb b/spec/posthog/mcp/client_spec.rb index a7d9bd8..a371e3d 100644 --- a/spec/posthog/mcp/client_spec.rb +++ b/spec/posthog/mcp/client_spec.rb @@ -96,19 +96,54 @@ expect(client.prepare_tool_call('get_more_tools').is_missing_capability).to be(true) end - it 'advertises llm_model on every tool and on the virtual one when capture_model is on' do + it 'advertises llm_model on every tool and on the virtual one by default' do tools = [{ name: 'a', inputSchema: { type: 'object', properties: {} } }] - prepared = client.prepare_tool_list(tools, capture_model: true, report_missing: true) + prepared = client.prepare_tool_list(tools, report_missing: true) expect(prepared[0][:inputSchema][:properties].keys).to eq(%i[context llm_model]) expect(prepared[1][:name]).to eq('get_more_tools') expect(prepared[1][:inputSchema][:properties].keys).to eq(%i[context llm_model]) expect(prepared[1][:inputSchema][:required]).to contain_exactly('context', 'llm_model') - without = client.prepare_tool_list(tools, report_missing: true) + without = client.prepare_tool_list(tools, capture_model: false, report_missing: true) expect(without[0][:inputSchema][:properties].keys).to eq([:context]) expect(without[1][:inputSchema][:properties].keys).to eq([:context]) end + it 'does not extract or strip a model after opting out during tool preparation' do + tools = [{ name: 'search', inputSchema: { type: 'object', properties: {} } }] + client.prepare_tool_list(tools, capture_model: false) + + call = client.prepare_tool_call('search', { llm_model: 'private-model' }, input_schema: tools[0][:inputSchema]) + expect(call.args).to eq(llm_model: 'private-model') + expect(call.llm_model).to be_nil + expect(call.llm_model_source).to be_nil + + client.capture_tool_call('search', llm_model: call.llm_model, llm_model_source: call.llm_model_source) + expect(client.dequeue_last_message[:properties]).not_to have_key('$mcp_llm_model') + + client.capture_tool_call('search', llm_model: 'explicit-model') + expect(client.dequeue_last_message[:properties]).not_to have_key('$mcp_llm_model') + expect(client.prepare_tool_list(tools)[0][:inputSchema][:properties]).not_to have_key(:llm_model) + expect(client.prepare_tool_list(tools, capture_model: true)[0][:inputSchema][:properties]) + .not_to have_key(:llm_model) + end + + it 'honours a constructor model opt-out even before preparing a tool list' do + quiet = described_class.new(api_key: 'phc_test', test_mode: true, capture_model: false) + call = quiet.prepare_tool_call('search', { llm_model: 'private-model' }, input_schema: { properties: {} }) + expect(call.args).to eq(llm_model: 'private-model') + expect(call.llm_model).to be_nil + prepared = quiet.prepare_tool_list([{ name: 'search', inputSchema: { properties: {} } }]) + expect(prepared[0][:inputSchema][:properties]).not_to have_key(:llm_model) + end + + it 'uses the constructor model description for prepared tools' do + model = PostHog::MCP::ModelOptions.new(description: 'Which model?') + configured = described_class.new(api_key: 'phc_test', test_mode: true, capture_model: model) + prepared = configured.prepare_tool_list([{ name: 'search', inputSchema: { properties: {} } }]) + expect(prepared[0][:inputSchema][:properties][:llm_model][:description]).to eq('Which model?') + end + it 'round-trips an injected llm_model and leaves a tool-declared one in args' do injected = { type: 'object', properties: { title: { type: 'string' } } } call = client.prepare_tool_call('add', { title: 'x', llm_model: ' claude-opus-4-8 ' }, input_schema: injected) @@ -144,7 +179,8 @@ it 'strips only the context argument it injected when given the tool schema' do own = { type: 'object', properties: { context: { type: 'string' } }, required: ['context'] } - expect(client.prepare_tool_list([{ name: 'search', inputSchema: own }])[0][:inputSchema]).to eq(own) + expect(client.prepare_tool_list([{ name: 'search', inputSchema: own }], capture_model: false)[0][:inputSchema]) + .to eq(own) kept = client.prepare_tool_call('search', { context: 'application data' }, input_schema: own) expect(kept.args).to eq(context: 'application data') # The tool declares `context`, so its value is the tool's own data: it stays in diff --git a/spec/posthog/mcp/http_transport_spec.rb b/spec/posthog/mcp/http_transport_spec.rb index eb96449..64d60ff 100644 --- a/spec/posthog/mcp/http_transport_spec.rb +++ b/spec/posthog/mcp/http_transport_spec.rb @@ -59,7 +59,7 @@ def parse(response) let(:transport) { MCP::Server::Transports::StreamableHTTPTransport.new(server, stateless: true, enable_json_response: true) } it 'mints a session token on initialize, recovers it on replay, and stamps transport identity' do - PostHog::MCP.instrument(server, client) + PostHog::MCP.instrument(server, client, enable_conversation_id: false) response = transport.call(env_for(initialize_body, 'user-agent' => 'claude-code/2.1.0 (cli)', 'x-anthropic-client' => 'cli')) expect(response[0]).to eq(200) token = response[1]['mcp-session-id'] @@ -107,7 +107,7 @@ def parse(response) let(:transport) { MCP::Server::Transports::StreamableHTTPTransport.new(server, enable_json_response: true) } it 'hashes the transport session id deterministically across requests' do - PostHog::MCP.instrument(server, client) + PostHog::MCP.instrument(server, client, enable_conversation_id: false) response = transport.call(env_for(initialize_body)) session_id = response[1]['mcp-session-id'] expect(PostHog::MCP.decode_session_id(session_id)).to be_nil @@ -121,7 +121,9 @@ def parse(response) end it 'runs get_more_tools through the real request lifecycle so in-flight entries are released' do - PostHog::MCP.instrument(server, client, report_missing: true) + PostHog::MCP.instrument( + server, client, report_missing: true, capture_model: false, enable_conversation_id: false + ) session_id = transport.call(env_for(initialize_body))[1]['mcp-session-id'] 3.times do |i| response = transport.call(env_for(rpc(i + 2, 'tools/call', { name: 'get_more_tools', arguments: { context: 'csv export' } }), @@ -145,7 +147,9 @@ def parse(response) let(:identify) { ->(_request, extra) { { distinct_id: extra['headers']['user-agent'] } } } before do - PostHogMcpHttpSpecCaptureTool.analytics = PostHog::MCP.instrument(server, client, identify: identify) + PostHogMcpHttpSpecCaptureTool.analytics = PostHog::MCP.instrument( + server, client, identify: identify, enable_conversation_id: false + ) PostHogMcpHttpSpecCaptureTool.before_capture = nil end @@ -229,7 +233,7 @@ def dispatching_app(status: 200, body: nil) end it 'mints from the instrumented server without reading the request body' do - PostHog::MCP.instrument(server, client) + PostHog::MCP.instrument(server, client, enable_conversation_id: false) env = { 'REQUEST_METHOD' => 'POST', 'rack.input' => PostHogMcpUnreadableInput.new } app = ->(_e) { [200, { 'content-type' => 'application/json' }, [server.handle_json(initialize_json)]] } status, headers, = described_class.new(app).call(env) @@ -245,7 +249,7 @@ def dispatching_app(status: 200, body: nil) end it 'publishes the request headers so a server below it sees the HTTP context' do - PostHog::MCP.instrument(server, client) + PostHog::MCP.instrument(server, client, enable_conversation_id: false) env = env_for(initialize_json, 'user-agent' => 'claude-code/2.1.0 (cli)', 'x-anthropic-client' => 'cli') _, headers, = described_class.new(dispatching_app).call(env) token = headers['mcp-session-id'] diff --git a/spec/posthog/mcp/instrument_spec.rb b/spec/posthog/mcp/instrument_spec.rb index e6a46cf..96e211c 100644 --- a/spec/posthog/mcp/instrument_spec.rb +++ b/spec/posthog/mcp/instrument_spec.rb @@ -166,7 +166,7 @@ def initialize_request(id = 1, version = '2025-06-18') seen << data[:method] handler.call }) - described_class.instrument(server, client) + described_class.instrument(server, client, capture_model: false, enable_conversation_id: false) server.handle(rpc(1, 'tools/list')) expect(seen).to eq(['tools/list']) expect(drain_events(client).map { |e| e[:event] }).to include('$mcp_tools_list') @@ -177,7 +177,7 @@ def initialize_request(id = 1, version = '2025-06-18') before { allow(Kernel).to receive(:warn) } it 'captures initialize, tools/list and a successful tool call with $lib override' do - described_class.instrument(server, client) + described_class.instrument(server, client, capture_model: false, enable_conversation_id: false) response = server.handle(initialize_request) expect(response[:result][:protocolVersion]).to eq('2025-06-18') list = server.handle(rpc(2, 'tools/list')) @@ -217,7 +217,7 @@ def initialize_request(id = 1, version = '2025-06-18') end it 'strips the injected context before the tool sees it but keeps a tool-owned context' do - described_class.instrument(server, client) + described_class.instrument(server, client, capture_model: false, enable_conversation_id: false) result = server.handle(rpc(1, 'tools/call', { name: 'owns_context', arguments: { context: 'mine' } })) expect(result[:result][:content][0][:text]).to eq('ctx=mine') list = server.handle(rpc(2, 'tools/list')) @@ -247,7 +247,7 @@ def initialize_request(id = 1, version = '2025-06-18') end it 'treats isError results as errors and honours enable_exception_autocapture: false' do - described_class.instrument(server, client, enable_exception_autocapture: false) + described_class.instrument(server, client, enable_exception_autocapture: false, enable_conversation_id: false) server.handle(rpc(1, 'tools/call', { name: 'soft_fail', arguments: {} })) events = drain_events(client) expect(events.map { |e| e[:event] }).to eq(['$mcp_initialize', '$mcp_tool_call']) @@ -342,9 +342,9 @@ def initialize_request(id = 1, version = '2025-06-18') calls += 1 { distinct_id: 'user-1', properties: { name: 'Alice' }, groups: { organization: 'org_123' } } end - described_class.instrument(server, client, identify: identify, event_properties: lambda { |_r, _e| - { env: 'production' } - }) + event_properties = ->(_request, _extra) { { env: 'production' } } + described_class.instrument(server, client, identify: identify, enable_conversation_id: false, + event_properties: event_properties) server.handle(initialize_request) 3.times { |i| server.handle(rpc(i + 2, 'tools/call', { name: 'echo', arguments: { message: 'x' } })) } events = drain_events(client) @@ -384,7 +384,9 @@ def initialize_request(id = 1, version = '2025-06-18') end shared = MCP::Server.new(name: 'spec-server', version: '9.9.9', tools: [PostHogMcpSpecParkedTool, PostHogMcpSpecEchoTool]) - PostHogMcpSpecParkedTool.analytics = described_class.instrument(shared, client, identify: identify) + PostHogMcpSpecParkedTool.analytics = described_class.instrument( + shared, client, identify: identify, enable_conversation_id: false + ) alice = Thread.new { shared.handle(rpc(1, 'tools/call', { name: 'parked', arguments: { user: 'alice' } })) } PostHogMcpSpecParkedTool.entered.pop @@ -420,7 +422,7 @@ def initialize_request(id = 1, version = '2025-06-18') payload['properties']['bloat'] = 'x' * 40_000 payload end - described_class.instrument(server, client, before_send: before_send) + described_class.instrument(server, client, before_send: before_send, enable_conversation_id: false) server.handle(rpc(1, 'tools/call', { name: 'echo', arguments: { message: 'hi' } })) events = drain_events(client) # The hook runs after truncation, so without a second pass the batch would @@ -453,13 +455,13 @@ def initialize_request(id = 1, version = '2025-06-18') describe 'conversation ids, get_more_tools and llm_model' do before { allow(Kernel).to receive(:warn) } - it 'mints, prompts back, mirrors into structuredContent and anchors the session on an echo' do - described_class.instrument(server, client, enable_conversation_id: true) + it 'mints, prompts back, mirrors into structuredContent and anchors the session on an echo by default' do + described_class.instrument(server, client) list = server.handle(rpc(1, 'tools/list')) structured = list[:result][:tools].find { |t| t[:name] == 'structured' } expect(structured[:inputSchema][:properties]).to have_key(:conversation_id) expect(structured[:outputSchema][:properties]).to have_key(:_mcp_instructions) - expect(structured[:inputSchema][:required]).to eq(['context']) + expect(structured[:inputSchema][:required]).to contain_exactly('context', 'llm_model') first = server.handle(rpc(2, 'tools/call', { name: 'echo', arguments: { message: 'hi', context: 'c' } })) prompt_back = JSON.parse(first[:result][:content][1][:text]) @@ -524,7 +526,7 @@ def initialize_request(id = 1, version = '2025-06-18') end it 'advertises and intercepts get_more_tools as $mcp_missing_capability' do - described_class.instrument(server, client, report_missing: true) + described_class.instrument(server, client, report_missing: true, capture_model: false) list = server.handle(rpc(1, 'tools/list')) virtual = list[:result][:tools].last expect(virtual[:name]).to eq('get_more_tools') @@ -585,8 +587,8 @@ def initialize_request(id = 1, version = '2025-06-18') '$mcp_llm_model_source' => 'self_reported') end - it 'captures llm_model from the injected argument or client metadata' do - described_class.instrument(server, client, capture_model: true) + it 'captures llm_model from the injected argument or client metadata by default' do + described_class.instrument(server, client) server.handle(rpc(1, 'tools/call', { name: 'echo', arguments: { message: 'hi', llm_model: ' claude-opus-4-8 ' } })) server.handle(rpc(2, 'tools/call', { name: 'echo', arguments: { message: 'hi', llm_model: 'unknown' } })) diff --git a/spec/posthog/mcp/options_spec.rb b/spec/posthog/mcp/options_spec.rb index 8443fe6..58867e5 100644 --- a/spec/posthog/mcp/options_spec.rb +++ b/spec/posthog/mcp/options_spec.rb @@ -6,14 +6,20 @@ it 'has JS/Python defaults' do options = described_class.new expect(options.report_missing).to be(false) - expect(options.enable_conversation_id).to be(false) + expect(options.enable_conversation_id).to be(true) expect(options.enable_exception_autocapture).to be(true) expect(options.context_enabled?).to be(true) expect(options.context_description).to be_nil - expect(options.capture_model_enabled?).to be(false) + expect(options.capture_model_enabled?).to be(true) expect(PostHog::MCP::Tools.missing_capability_tool_name(options)).to eq('get_more_tools') end + it 'allows conversation and model capture to be disabled' do + options = described_class.new(enable_conversation_id: false, capture_model: false) + expect(options.enable_conversation_id).to be(false) + expect(options.capture_model_enabled?).to be(false) + end + it 'normalises hash forms' do options = described_class.new(context: { description: 'why' }, capture_model: { description: 'which' }, missing_capability_tool_name: 'find_tools')