From 586ff7b1d47f105e3360440691ee637e766a52ad Mon Sep 17 00:00:00 2001 From: Daniel Visca Date: Mon, 24 Aug 2026 15:27:23 -0700 Subject: [PATCH 1/3] feat(mcp): add opt-in captureModel option for agent-self-reported model id Injects a required `llm_model` parameter into every tool (mirroring the `context` -> `$mcp_intent` mechanism) so the calling agent self-reports the model it runs as, captured as `$mcp_llm_model` with `$mcp_llm_model_source = "self_reported"`. The MCP wire deliberately carries no model identity, so agent self-report is the only capture path: harnesses inject the model id into the agent's system prompt, and agents restate it accurately. Reasoning effort is intentionally not collected - agents don't reliably know it. - Off by default; `captureModel: true` or `{ description }` to enable - Argument is stripped before the tool handler runs - A customer-declared `llm_model` parameter is never stolen or captured - An honest "unknown" from the agent is dropped rather than recorded Generated-By: PostHog Desktop Task-Id: efe2cfaa-b580-4392-bdb4-a18eb2cd325b --- .changeset/model-capture-option.md | 5 + .../src/__tests__/model-parameters.test.ts | 311 ++++++++++++++++++ .../src/extensions/analytics-parameters.ts | 9 +- packages/mcp/src/extensions/capture.ts | 4 + packages/mcp/src/extensions/constants.ts | 4 + .../src/extensions/instrument-highlevel.ts | 2 + .../mcp/src/extensions/instrumentation.ts | 18 + .../mcp/src/extensions/model-parameters.ts | 144 ++++++++ packages/mcp/src/extensions/posthog-events.ts | 8 + packages/mcp/src/extensions/truncation.ts | 3 + packages/mcp/src/index.ts | 2 + packages/mcp/src/types.ts | 28 ++ 12 files changed, 536 insertions(+), 2 deletions(-) create mode 100644 .changeset/model-capture-option.md create mode 100644 packages/mcp/src/__tests__/model-parameters.test.ts create mode 100644 packages/mcp/src/extensions/model-parameters.ts diff --git a/.changeset/model-capture-option.md b/.changeset/model-capture-option.md new file mode 100644 index 0000000000..588582e2b6 --- /dev/null +++ b/.changeset/model-capture-option.md @@ -0,0 +1,5 @@ +--- +'@posthog/mcp': minor +--- + +Add opt-in `captureModel` option: injects a required `llm_model` parameter into every tool so the calling agent self-reports the model it runs as, captured as `$mcp_llm_model` with `$mcp_llm_model_source = "self_reported"`. The argument is stripped before the tool handler runs, a customer-declared `llm_model` parameter is never stolen or captured, and an honest `"unknown"` from the agent is dropped rather than recorded. diff --git a/packages/mcp/src/__tests__/model-parameters.test.ts b/packages/mcp/src/__tests__/model-parameters.test.ts new file mode 100644 index 0000000000..2d10d619ac --- /dev/null +++ b/packages/mcp/src/__tests__/model-parameters.test.ts @@ -0,0 +1,311 @@ +import { Client } from '@modelcontextprotocol/sdk/client/index.js' +import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js' +import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js' +import { CallToolResultSchema, ListToolsResultSchema } from '@modelcontextprotocol/sdk/types.js' +import { z } from 'zod' +import { instrument } from '../index' +import { DEFAULT_MODEL_PARAMETER_DESCRIPTION } from '../extensions/constants' +import { addModelParameterToTool, addModelParameterToTools } from '../extensions/model-parameters' +import { log } from '../extensions/logger' +import { EventCapture, fakePostHog } from './test-utils' +import { resetTodos, setupTestServerAndClient } from './test-utils/client-server-factory' + +jest.mock('../extensions/logger', () => ({ + createLogger: (logger?: (message: string) => void) => logger ?? (() => undefined), + log: jest.fn(), + setLogger: jest.fn(), +})) + +const mockedLog = jest.mocked(log) + +beforeEach(() => { + mockedLog.mockClear() +}) + +afterEach(() => { + jest.restoreAllMocks() +}) + +/** + * --- Unit tests: `addModelParameterToTool` / `addModelParameterToTools` --- + * + * Same pure-function contract as the `context` injection: mutate a JSON-Schema + * tool descriptor to add a required `llm_model` string parameter. + */ +describe('addModelParameterToTool', () => { + it.each([ + ['no inputSchema', { name: 'tool' }], + ['empty inputSchema {}', { name: 'tool', inputSchema: {} }], + [ + 'inputSchema with existing properties + required', + { + name: 'tool', + inputSchema: { + type: 'object', + properties: { text: { type: 'string' } }, + required: ['text'], + }, + }, + ], + ])('injects a string llm_model param + marks it required (%s)', (_, tool) => { + const result = addModelParameterToTool(tool as Parameters[0]) + + expect(result.inputSchema?.properties?.llm_model).toEqual({ + type: 'string', + description: DEFAULT_MODEL_PARAMETER_DESCRIPTION, + }) + expect(result.inputSchema?.required).toContain('llm_model') + }) + + it('preserves existing required fields when adding llm_model', () => { + const result = addModelParameterToTool({ + name: 'tool', + inputSchema: { type: 'object', properties: { text: { type: 'string' } }, required: ['text'] }, + }) + + expect(result.inputSchema?.required).toEqual(expect.arrayContaining(['text', 'llm_model'])) + }) + + it('removes additionalProperties:false (would otherwise reject the injected param)', () => { + const result = addModelParameterToTool({ + name: 'strict-tool', + inputSchema: { + type: 'object', + properties: { name: { type: 'string' } }, + additionalProperties: false, + }, + }) + + expect(result.inputSchema?.properties?.llm_model).toBeDefined() + expect(result.inputSchema?.additionalProperties).toBeUndefined() + }) + + it.each([ + [ + 'tool already has an llm_model property', + { + name: 'has-model', + inputSchema: { type: 'object', properties: { llm_model: { type: 'number', description: 'existing' } } }, + }, + "already has 'llm_model' parameter", + ], + ['schema uses a root $ref', { name: 'referenced-tool', inputSchema: { $ref: '#/$defs/Input' } }, 'complex schema'], + [ + 'schema uses oneOf', + { name: 'union-tool', inputSchema: { oneOf: [{ type: 'object', properties: {} }] } }, + 'complex schema', + ], + ])('skips + warns when %s', (_, tool, expectedWarningSubstring) => { + const before = JSON.parse(JSON.stringify(tool)) + const result = addModelParameterToTool(tool as Parameters[0]) + + expect(result.inputSchema).toEqual(before.inputSchema) + expect(mockedLog).toHaveBeenCalledWith(expect.stringContaining(expectedWarningSubstring)) + }) + + it('uses a custom description when provided', () => { + const customDescription = 'State your model identifier' + const result = addModelParameterToTool({ name: 'tool' }, customDescription) + + expect(result.inputSchema?.properties?.llm_model?.description).toBe(customDescription) + }) +}) + +describe('addModelParameterToTools (batch)', () => { + it('applies the right outcome per tool in a mixed batch', () => { + const result = addModelParameterToTools([ + { name: 'plain', inputSchema: { type: 'object', properties: {} } }, + { name: 'complex', inputSchema: { oneOf: [{ type: 'string' }] } }, + { name: 'collision', inputSchema: { type: 'object', properties: { llm_model: { type: 'number' } } } }, + ]) + + expect(result[0].inputSchema?.properties?.llm_model).toBeDefined() + expect(result[1].inputSchema?.properties).toBeUndefined() + expect(result[2].inputSchema?.properties?.llm_model?.type).toBe('number') + expect(mockedLog).toHaveBeenCalledTimes(2) + }) +}) + +/** + * --- Integration tests: captureModel against a real MCP server --- + */ +describe('Model capture — integration with an instrumented server', () => { + let server: any + let client: any + let cleanup: () => Promise + + beforeEach(async () => { + resetTodos() + const setup = await setupTestServerAndClient() + server = setup.server + client = setup.client + cleanup = setup.cleanup + }) + + afterEach(async () => { + await cleanup() + }) + + it('does not inject llm_model when captureModel is off (default)', async () => { + instrument(server, fakePostHog(), {}) + + const toolsResponse = await client.request({ method: 'tools/list', params: {} }, ListToolsResultSchema) + for (const tool of toolsResponse.tools) { + expect(tool.inputSchema.properties?.llm_model).toBeUndefined() + } + }) + + it.each([ + ['default', true, DEFAULT_MODEL_PARAMETER_DESCRIPTION], + ['custom', { description: 'Which model are you?' }, 'Which model are you?'], + ])('injects the llm_model parameter on every tool in tools/list (%s description)', async (_, option, expected) => { + instrument(server, fakePostHog(), { captureModel: option as any }) + + const toolsResponse = await client.request({ method: 'tools/list', params: {} }, ListToolsResultSchema) + const userTools = toolsResponse.tools.filter((t: any) => + ['add_todo', 'list_todos', 'complete_todo'].includes(t.name) + ) + + expect(userTools).toHaveLength(3) + for (const tool of userTools) { + expect(tool.inputSchema.properties.llm_model).toBeDefined() + expect(tool.inputSchema.properties.llm_model.type).toBe('string') + expect(tool.inputSchema.properties.llm_model.description).toBe(expected) + } + }) + + it('captures llm_model as $mcp_llm_model with source=self_reported and strips it from the tool args', async () => { + const capture = new EventCapture() + await capture.start() + try { + instrument(server, fakePostHog(), { captureModel: true }) + + // Prime ownership the way a real client does: list tools first. + await client.request({ method: 'tools/list', params: {} }, ListToolsResultSchema) + + const result = await client.request( + { + method: 'tools/call', + params: { + name: 'add_todo', + arguments: { text: 'buy milk', llm_model: 'claude-opus-4-8' }, + }, + }, + CallToolResultSchema + ) + + // The tool ran normally — the injected argument never reached the handler. + expect(result.content[0].text).toContain('buy milk') + + const toolCalls = capture.findCapturesByEvent('$mcp_tool_call') + expect(toolCalls).toHaveLength(1) + expect(toolCalls[0].properties.$mcp_llm_model).toBe('claude-opus-4-8') + expect(toolCalls[0].properties.$mcp_llm_model_source).toBe('self_reported') + // Stripped from the captured parameters too — it is analytics metadata, not an argument. + expect((toolCalls[0].properties.$mcp_parameters as any)?.llm_model).toBeUndefined() + } finally { + await capture.stop() + } + }) + + it('omits the property when the agent passes "unknown"', async () => { + const capture = new EventCapture() + await capture.start() + try { + instrument(server, fakePostHog(), { captureModel: true }) + await client.request({ method: 'tools/list', params: {} }, ListToolsResultSchema) + + await client.request( + { + method: 'tools/call', + params: { name: 'add_todo', arguments: { text: 'x', llm_model: 'unknown' } }, + }, + CallToolResultSchema + ) + + const toolCalls = capture.findCapturesByEvent('$mcp_tool_call') + expect(toolCalls).toHaveLength(1) + expect(toolCalls[0].properties.$mcp_llm_model).toBeUndefined() + expect(toolCalls[0].properties.$mcp_llm_model_source).toBeUndefined() + } finally { + await capture.stop() + } + }) + + it('does not capture or strip llm_model when captureModel is off', async () => { + const capture = new EventCapture() + await capture.start() + try { + instrument(server, fakePostHog(), {}) + await client.request({ method: 'tools/list', params: {} }, ListToolsResultSchema) + + await client.request( + { + method: 'tools/call', + params: { name: 'add_todo', arguments: { text: 'x' } }, + }, + CallToolResultSchema + ) + + const toolCalls = capture.findCapturesByEvent('$mcp_tool_call') + expect(toolCalls).toHaveLength(1) + expect(toolCalls[0].properties.$mcp_llm_model).toBeUndefined() + } finally { + await capture.stop() + } + }) +}) + +/** + * --- Ownership: a customer-declared llm_model parameter is never stolen --- + */ +describe('llm_model ownership', () => { + it('passes a customer-declared llm_model argument through to the callback and does not capture it', async () => { + const server = new McpServer({ name: 'model-ownership', version: '1.0.0' }) + let receivedArgs: Record | undefined + + server.registerTool( + 'pick_model', + { + inputSchema: z.object({ + llm_model: z.string(), + prompt: z.string(), + }), + }, + async (args) => { + receivedArgs = { ...args } + return { content: [{ type: 'text', text: 'ok' }] } + } + ) + + const client = new Client({ name: 'model-ownership-client', version: '1.0.0' }) + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair() + await Promise.all([client.connect(clientTransport), server.server.connect(serverTransport)]) + + const capture = new EventCapture() + await capture.start() + try { + instrument(server, fakePostHog(), { captureModel: true }) + await client.request({ method: 'tools/list', params: {} }, ListToolsResultSchema) + + await client.request( + { + method: 'tools/call', + params: { name: 'pick_model', arguments: { llm_model: 'gpt-5', prompt: 'hi' } }, + }, + CallToolResultSchema + ) + + // The application owns this parameter: it must reach the handler untouched... + expect(receivedArgs).toEqual({ llm_model: 'gpt-5', prompt: 'hi' }) + // ...and must not be recorded as the calling agent's model. + const toolCalls = capture.findCapturesByEvent('$mcp_tool_call') + expect(toolCalls).toHaveLength(1) + expect(toolCalls[0].properties.$mcp_llm_model).toBeUndefined() + } finally { + await capture.stop() + await clientTransport.close?.() + await serverTransport.close?.() + } + }) +}) diff --git a/packages/mcp/src/extensions/analytics-parameters.ts b/packages/mcp/src/extensions/analytics-parameters.ts index 8f9a5f5d2e..c4055d4f77 100644 --- a/packages/mcp/src/extensions/analytics-parameters.ts +++ b/packages/mcp/src/extensions/analytics-parameters.ts @@ -76,18 +76,19 @@ export function getAnalyticsParameterOwnership( return { context: analyticsOwnsParameter(inputSchema, 'context'), conversationId: analyticsOwnsParameter(inputSchema, 'conversation_id'), + llmModel: analyticsOwnsParameter(inputSchema, 'llm_model'), outputInstructions: canDeclareOutputInstructions(outputSchema), } } /** * Removes the arguments we injected, leaving the host's own. Input-side only, so - * it takes just those two flags rather than the whole ownership record — callers + * it takes just those flags rather than the whole ownership record — callers * that have no output-schema context should not have to invent a value for it. */ export function stripOwnedAnalyticsArguments( args: unknown, - ownership: Pick + ownership: Pick ): unknown { let cleanedArgs = args if (ownership.context && cleanedArgs && typeof cleanedArgs === 'object' && 'context' in cleanedArgs) { @@ -98,5 +99,9 @@ export function stripOwnedAnalyticsArguments( const { conversation_id: _conversationId, ...rest } = cleanedArgs cleanedArgs = rest } + if (ownership.llmModel && cleanedArgs && typeof cleanedArgs === 'object' && 'llm_model' in cleanedArgs) { + const { llm_model: _llmModel, ...rest } = cleanedArgs + cleanedArgs = rest + } return cleanedArgs } diff --git a/packages/mcp/src/extensions/capture.ts b/packages/mcp/src/extensions/capture.ts index 494f21b429..323eac24e8 100644 --- a/packages/mcp/src/extensions/capture.ts +++ b/packages/mcp/src/extensions/capture.ts @@ -81,6 +81,10 @@ export function captureEvent( response: eventInput.response, userIntent: eventInput.userIntent, userIntentSource: eventInput.userIntentSource, + // Self-reported per request (`captureModel`), so like the header-borne + // fields above there is deliberately no sessionInfo fallback. + llmModel: eventInput.llmModel, + llmModelSource: eventInput.llmModelSource, isError: eventInput.isError, // Two consumers of the same failure: `error` is the full thrown value that // fans out into the separate `$exception` event (stacktrace, error-tracking diff --git a/packages/mcp/src/extensions/constants.ts b/packages/mcp/src/extensions/constants.ts index 80fa3f0468..4f058bf734 100644 --- a/packages/mcp/src/extensions/constants.ts +++ b/packages/mcp/src/extensions/constants.ts @@ -7,6 +7,8 @@ export const INACTIVITY_TIMEOUT_IN_MINUTES = 30 export const DEFAULT_CONTEXT_PARAMETER_DESCRIPTION = `Explain why you are calling this tool and how it fits into the user's overall goal. This parameter is used for analytics and user intent tracking. YOU MUST provide 15-25 words (count carefully). NEVER use first person ('I', 'we', 'you') - maintain third-person perspective. NEVER include sensitive information such as credentials, passwords, or personal data. Example (20 words): "Searching across the organization's repositories to find all open issues related to performance complaints and latency issues for team prioritization."` +export const DEFAULT_MODEL_PARAMETER_DESCRIPTION = `The exact model identifier you (the assistant) are running as, taken from your system prompt or environment (e.g. "claude-opus-4-8", "gpt-5.2"). Used for analytics only. If you do not know your model identifier with certainty, pass "unknown" — never guess.` + export const DEFAULT_CONVERSATION_ID_DESCRIPTION = "Echo the conversation_id from the server's previous response. The server provides it on the first call — never invent one, and do not issue parallel tool calls until you have it." @@ -48,6 +50,8 @@ export const PostHogMCPAnalyticsProperty = { Intent: '$mcp_intent', IntentSource: '$mcp_intent_source', ListedToolNames: '$mcp_listed_tool_names', + LlmModel: '$mcp_llm_model', + LlmModelSource: '$mcp_llm_model_source', Parameters: '$mcp_parameters', ProtocolVersion: '$mcp_protocol_version', ResourceName: '$mcp_resource_name', diff --git a/packages/mcp/src/extensions/instrument-highlevel.ts b/packages/mcp/src/extensions/instrument-highlevel.ts index 9b59751fbf..428713d416 100644 --- a/packages/mcp/src/extensions/instrument-highlevel.ts +++ b/packages/mcp/src/extensions/instrument-highlevel.ts @@ -17,6 +17,7 @@ import { stripOwnedAnalyticsArguments, } from './analytics-parameters' import { isContextEnabled } from './context-parameters' +import { isCaptureModelEnabled } from './model-parameters' import { MCPAnalyticsEventType } from './event-types' import { getServerTrackingData } from './internal' import type { LoggerFn } from './logger' @@ -189,6 +190,7 @@ function addTracingToToolCallbackInternal( const cleanedArgs = stripOwnedAnalyticsArguments(args, { context: isContextEnabled(options?.context) && analyticsOwnsParameter(inputSchema, 'context'), conversationId: options?.enableConversationId === true && analyticsOwnsParameter(inputSchema, 'conversation_id'), + llmModel: isCaptureModelEnabled(options?.captureModel) && analyticsOwnsParameter(inputSchema, 'llm_model'), }) try { if (cleanedArgs === undefined) { diff --git a/packages/mcp/src/extensions/instrumentation.ts b/packages/mcp/src/extensions/instrumentation.ts index 348531e7f0..1347a7cb14 100644 --- a/packages/mcp/src/extensions/instrumentation.ts +++ b/packages/mcp/src/extensions/instrumentation.ts @@ -16,6 +16,13 @@ import type { } from '../types' import { getAnalyticsParameterOwnership, stripOwnedAnalyticsArguments } from './analytics-parameters' import { addContextParameterToTools, getContextDescription, isContextEnabled } from './context-parameters' +import { + addModelParameterToTools, + getModelArgument, + getModelDescription, + isCaptureModelEnabled, + setEventModel, +} from './model-parameters' import { addConversationIdToTools, type ConversationIdResolution, @@ -179,6 +186,7 @@ function getActiveAnalyticsParameterOwnership( contextOwnershipKnown: ownership !== undefined, context: !isMissingCapabilityTool && isContextEnabled(data.options.context) && ownership?.context === true, conversationId: data.options.enableConversationId === true && ownership?.conversationId === true, + llmModel: isCaptureModelEnabled(data.options.captureModel) && ownership?.llmModel === true, // Deliberately read off `listed`, never the override: only the advertised // JSON Schema can say whether `tools/list` declared `_mcp_instructions` (an // override is built from the live registry, which holds Zod on the @@ -263,6 +271,13 @@ async function prepareToolCallEvent( await applyResolvedMetadata(event, data, request, extra) setEventIntent(event, await resolveToolCallIntent(data, request, canCaptureContextIntent, extra)) + // Unlike intent, the model is only read under positive ownership: with + // ownership unresolved, `llm_model` may be the application's own argument, + // and recording a customer value as the calling agent's model is worse + // than a gap in coverage. + if (ownership.llmModel) { + setEventModel(event, getModelArgument(request)) + } return { event, requestAttribution } } catch (error) { data.logger( @@ -646,6 +661,9 @@ async function getTracedToolsList( if (data && isContextEnabled(data.options.context)) { tools = addContextParameterToTools(tools, getContextDescription(data.options.context), data.logger) } + if (data && isCaptureModelEnabled(data.options.captureModel)) { + tools = addModelParameterToTools(tools, getModelDescription(data.options.captureModel), data.logger) + } if (data) { const missingToolName = resolveMissingCapabilityToolName(data.options) diff --git a/packages/mcp/src/extensions/model-parameters.ts b/packages/mcp/src/extensions/model-parameters.ts new file mode 100644 index 0000000000..0b515821d6 --- /dev/null +++ b/packages/mcp/src/extensions/model-parameters.ts @@ -0,0 +1,144 @@ +import type { MCPAnalyticsOptions, MCPRequestLike, McpEvent } from '../types' +import { + canInjectAnalyticsParameter, + hasAnalyticsParameter, + type AnalyticsInjectableJsonSchema, +} from './analytics-parameters' +import { DEFAULT_MODEL_PARAMETER_DESCRIPTION } from './constants' +import { log, type LoggerFn } from './logger' + +/** + * Self-reported model capture (`captureModel`). + * + * MCP keeps servers deliberately model-ignorant: the wire carries client + * name/version and protocol version, never the LLM behind the client, and no + * spec revision changes that. The one place the information exists on the + * server side of the connection is the agent itself — harnesses inject the + * model id into the system prompt, so the agent can state it the same way it + * states its intent through the `context` parameter. + * + * This module injects a required `llm_model` string parameter into every tool + * (mirroring `context-parameters.ts`), strips it before the tool runs, and + * captures it as `$mcp_llm_model` with `$mcp_llm_model_source = "self_reported"`. + * The source property is deliberate: like `clientInfo` in the MCP spec, the + * value is self-reported and unverified — right for degradation analytics + * ("does our MCP get worse on model X?"), never for billing or security. + * + * Reasoning effort is deliberately NOT captured: it never crosses the wire, + * and models cannot reliably self-report it (harnesses apply it as a sampling + * parameter the model never sees), so any captured value would be noise. + */ + +export interface ModelInjectableTool { + inputSchema?: AnalyticsInjectableJsonSchema + name?: string + [key: string]: unknown +} + +/** Unlike `context`, model capture is opt-in: off unless explicitly enabled. */ +export function isCaptureModelEnabled(captureModel: MCPAnalyticsOptions['captureModel']): boolean { + return captureModel === true || (typeof captureModel === 'object' && captureModel !== null) +} + +export function getModelDescription(captureModel: MCPAnalyticsOptions['captureModel']): string | undefined { + return typeof captureModel === 'object' && captureModel !== null ? captureModel.description : undefined +} + +/** + * Adds an `llm_model` parameter to a tool's JSON Schema. Called AFTER the MCP + * SDK has converted Zod schemas to JSON Schema, so only JSON Schema needs + * handling. Skip rules match `addContextParameterToTool`: a tool that already + * declares `llm_model` owns it, and complex schemas can't safely gain keys. + */ +export function addModelParameterToTool( + tool: TTool, + modelDescriptionOverride?: string, + logger: LoggerFn = log +): TTool { + const modifiedTool = { ...tool } + const toolName = tool.name || 'unknown' + const schema = modifiedTool.inputSchema as AnalyticsInjectableJsonSchema | undefined + + if (!canInjectAnalyticsParameter(schema, 'llm_model')) { + if (hasAnalyticsParameter(schema, 'llm_model')) { + logger(`WARN: Tool "${toolName}" already has 'llm_model' parameter. Skipping model injection.`) + } else { + logger(`WARN: Tool "${toolName}" has complex schema (oneOf/allOf/anyOf/$ref). Skipping model injection.`) + } + return modifiedTool + } + + if (!modifiedTool.inputSchema) { + modifiedTool.inputSchema = { + type: 'object', + properties: {}, + required: [], + } + } + + const modelDescription = modelDescriptionOverride || DEFAULT_MODEL_PARAMETER_DESCRIPTION + + // Deep copy: the server may reuse or freeze the schema object it handed us. + modifiedTool.inputSchema = JSON.parse(JSON.stringify(modifiedTool.inputSchema)) as AnalyticsInjectableJsonSchema + + const inputSchema = modifiedTool.inputSchema as AnalyticsInjectableJsonSchema + + if (!inputSchema.properties) { + inputSchema.properties = {} + } + + // The MCP SDK emits `additionalProperties: false` when converting Zod schemas; + // left in place it would make the injected `llm_model` key invalid. + if (inputSchema.additionalProperties === false) { + inputSchema.additionalProperties = undefined + } + + inputSchema.properties.llm_model = { + type: 'string', + description: modelDescription, + } + + if (Array.isArray(inputSchema.required)) { + if (!inputSchema.required.includes('llm_model')) { + inputSchema.required.push('llm_model') + } + } else { + inputSchema.required = ['llm_model'] + } + + return modifiedTool +} + +export function addModelParameterToTools( + tools: TTool[], + modelDescriptionOverride?: string, + logger: LoggerFn = log +): TTool[] { + return tools.map((tool) => addModelParameterToTool(tool, modelDescriptionOverride, logger)) +} + +/** + * Reads the self-reported model off a tool-call request. Returns `undefined` + * for a missing, blank, or `"unknown"` value — the parameter description asks + * agents to pass `unknown` when uncertain, and an honest "I don't know" must + * not become a property value queries would group by. + */ +export function getModelArgument(request: MCPRequestLike): string | undefined { + const model = request.params?.arguments?.llm_model + if (typeof model !== 'string') { + return undefined + } + const trimmed = model.trim() + if (!trimmed || trimmed.toLowerCase() === 'unknown') { + return undefined + } + return trimmed +} + +export function setEventModel(event: McpEvent, model: string | undefined): void { + if (!model) { + return + } + event.llmModel = model + event.llmModelSource = 'self_reported' +} diff --git a/packages/mcp/src/extensions/posthog-events.ts b/packages/mcp/src/extensions/posthog-events.ts index 3f77526290..71008c6169 100644 --- a/packages/mcp/src/extensions/posthog-events.ts +++ b/packages/mcp/src/extensions/posthog-events.ts @@ -170,6 +170,14 @@ function addCommonEventProperties(event: Event, properties: Record(event: T): T { result.clientUserAgent = truncateString(result.clientUserAgent, MAX_METADATA_LENGTH) result.vendorClient = truncateString(result.vendorClient, MAX_METADATA_LENGTH) result.errorType = truncateString(result.errorType, MAX_METADATA_LENGTH) + // Agent-supplied free text, same trust level as a header. Real model ids are + // tens of characters; anything past the metadata cap is junk, not a model. + result.llmModel = truncateString(result.llmModel, MAX_METADATA_LENGTH) // Error field limits — operate on the core `$exception_list` shape if (result.error != null && typeof result.error === 'object') { diff --git a/packages/mcp/src/index.ts b/packages/mcp/src/index.ts index 343c32cf5b..c1e97a13b1 100644 --- a/packages/mcp/src/index.ts +++ b/packages/mcp/src/index.ts @@ -228,6 +228,8 @@ export type { McpCaptureCommon, MCPAnalyticsContextOptions, MCPAnalyticsIntentSource, + MCPAnalyticsModelOptions, + MCPAnalyticsModelSource, MCPAnalyticsOptions, MissingCapabilityCaptureData, PreparedToolCall, diff --git a/packages/mcp/src/types.ts b/packages/mcp/src/types.ts index 8ca5776668..1ba76237bd 100644 --- a/packages/mcp/src/types.ts +++ b/packages/mcp/src/types.ts @@ -129,6 +129,20 @@ export interface MCPAnalyticsOptions { enableExceptionAutocapture?: boolean /** Inject a required `context` parameter on every tool to capture user intent. */ context?: boolean | MCPAnalyticsContextOptions + /** + * Inject a required `llm_model` parameter on every tool so the calling agent + * self-reports the model it runs as, captured as `$mcp_llm_model` with + * `$mcp_llm_model_source = "self_reported"`. Off by default. + * + * The MCP wire deliberately carries no model identity, so self-report is the + * only capture path — harnesses inject the model id into the agent's system + * prompt, and agents restate it accurately. Like `clientInfo` in the MCP + * spec, the value is unverified: use it for per-model quality analytics, not + * billing or security. An honest `"unknown"` from the agent is dropped + * rather than captured. Reasoning effort is intentionally not collected — + * agents don't reliably know it, so it would only ever be noise. + */ + captureModel?: boolean | MCPAnalyticsModelOptions /** * Identify the calling user. Returning a non-null value sets `distinct_id` and `$set` * on subsequent events for the session. Object form is treated as a static identity. @@ -172,8 +186,13 @@ export interface MCPAnalyticsContextOptions { description?: string } +export interface MCPAnalyticsModelOptions { + description?: string +} + export type MaybePromise = T | Promise export type MCPAnalyticsIntentSource = 'context_parameter' | 'inferred' +export type MCPAnalyticsModelSource = 'self_reported' export type ToolCallback = | (( @@ -225,6 +244,14 @@ export interface Event { eventId?: string eventType: MCPAnalyticsEventType groups?: Record + /** + * The calling agent's self-reported model id → `$mcp_llm_model`. Read off + * the SDK-injected `llm_model` argument (`captureModel` option); unverified + * by design, like the MCP spec's own `clientInfo`. + */ + llmModel?: string + /** How the model id was obtained → `$mcp_llm_model_source`. Always `self_reported` today. */ + llmModelSource?: MCPAnalyticsModelSource /** * Explicit PostHog event name. When set (via `capture(server, { event })`) it * overrides the built-in name derived from `eventType`, so callers can emit any @@ -380,6 +407,7 @@ export interface SessionInfo { export interface AnalyticsParameterOwnership { context: boolean conversationId: boolean + llmModel: boolean /** * True when we declared `_mcp_instructions` on this tool's advertised output * schema, so writing that key into `structuredContent` will validate. False From a960d92e7aeb2dddac6d73aa7415133947df72c2 Mon Sep 17 00:00:00 2001 From: Daniel Visca Date: Tue, 25 Aug 2026 10:29:31 -0700 Subject: [PATCH 2/3] fix(mcp): inject llm_model into the virtual tool, share the parameter injector, document captureModel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review feedback on #4633. Blocking: model injection ran before reportMissing appended get_more_tools, so with both options on the advertised virtual tool had no llm_model and its missing-capability reports could never self-report a model — the one call type silently missing the property. Injection now runs after the append. Ownership is still cached from the un-injected descriptor, and context stays excluded because the virtual tool declares its own. Suggestion: addModelParameterToTool duplicated addContextParameterToTool almost verbatim. Both now delegate to a shared addAnalyticsParameterToTool in analytics-parameters.ts, so schema ownership, cloning, and required-field handling cannot drift. Docs: README gains a captureModel section covering the ownership limit (it degrades to silence, unlike context, and the high-level path resolves ownership per request from the registry); ARCHITECTURE gains the §5/§6/§7 entries for captureModel, $mcp_llm_model, and $mcp_llm_model_source; the changeset no longer promises unconditional stripping and capture. Generated-By: PostHog Desktop Task-Id: 44c7be3e-4938-4f9a-bb80-89fd9d74db6d --- .changeset/model-capture-option.md | 2 +- packages/mcp/README.md | 26 ++++++ packages/mcp/docs/ARCHITECTURE.md | 5 +- .../src/__tests__/model-parameters.test.ts | 49 +++++++++++ .../src/extensions/analytics-parameters.ts | 81 +++++++++++++++++++ .../mcp/src/extensions/context-parameters.ts | 69 +++------------- .../mcp/src/extensions/instrumentation.ts | 14 +++- .../mcp/src/extensions/model-parameters.ts | 70 +++------------- 8 files changed, 192 insertions(+), 124 deletions(-) diff --git a/.changeset/model-capture-option.md b/.changeset/model-capture-option.md index 588582e2b6..1548292d07 100644 --- a/.changeset/model-capture-option.md +++ b/.changeset/model-capture-option.md @@ -2,4 +2,4 @@ '@posthog/mcp': minor --- -Add opt-in `captureModel` option: injects a required `llm_model` parameter into every tool so the calling agent self-reports the model it runs as, captured as `$mcp_llm_model` with `$mcp_llm_model_source = "self_reported"`. The argument is stripped before the tool handler runs, a customer-declared `llm_model` parameter is never stolen or captured, and an honest `"unknown"` from the agent is dropped rather than recorded. +Add opt-in `captureModel` option: injects a required `llm_model` parameter into every tool — including the `get_more_tools` virtual tool — so the calling agent self-reports the model it runs as, captured as `$mcp_llm_model` with `$mcp_llm_model_source = "self_reported"`. Stripping the argument before the handler runs and capturing the property both require confirmed SDK ownership of the parameter: a customer-declared `llm_model` is never stolen or captured, and a low-level `Server` that builds a fresh instance per request records nothing (see the README). An honest `"unknown"` from the agent is dropped rather than recorded. diff --git a/packages/mcp/README.md b/packages/mcp/README.md index cb9ec7a66b..0f1652b5e9 100644 --- a/packages/mcp/README.md +++ b/packages/mcp/README.md @@ -149,6 +149,32 @@ instrument(server, posthog, { // keep it, dr `intentFallback` is the third option: supply the intent yourself when the agent did not send one. +### What `$mcp_llm_model` records, and when it stays empty + +`captureModel` is **off** by default. Turn it on and the SDK adds a required `llm_model` parameter to +every tool it advertises — including the `get_more_tools` virtual tool — asks the agent which model +it runs as, and records the answer as `$mcp_llm_model` with `$mcp_llm_model_source = "self_reported"`. + +The value is self-reported and unverified, exactly like `clientInfo` in the MCP spec. Use it to spot +degradation across models ("does our MCP get worse on model X?"), never for billing or access +control. An agent that answers `unknown` is recorded as nothing rather than as a model called +"unknown". + +Unlike `context`, this option degrades to **silence** rather than to a kept argument. Both the strip +and the capture require the SDK to have confirmed the parameter is its own: + +- `instrument(server)` on a high-level `McpServer` resolves ownership per request from the live tool + registry, so it works even on a fresh instance. +- Instrumenting a low-level `Server` learns ownership while serving `tools/list`. On a server that + builds a fresh instance per HTTP request — `createMcpHandler`, or `@rekog/mcp-nest` in its + stateless mode — the instance handling a `tools/call` never served one, so it neither strips + `llm_model` nor records `$mcp_llm_model`. Nothing breaks and no wrong value is stored; the property + is simply absent while agents still pay a token for the extra field. + +As with `context`, what matters is instance lifetime rather than statelessness: a transport-stateless +server (`sessionIdGenerator: undefined`) that keeps one long-lived server object learns ownership +from the first `tools/list` and keeps it. + ### If you switched to `instrument(server.server)` Before v2 support landed, the compatibility gate rejected high-level v2 servers, and the usual diff --git a/packages/mcp/docs/ARCHITECTURE.md b/packages/mcp/docs/ARCHITECTURE.md index b20f90c8d9..a1b8c1e4cc 100644 --- a/packages/mcp/docs/ARCHITECTURE.md +++ b/packages/mcp/docs/ARCHITECTURE.md @@ -102,7 +102,7 @@ All events are emitted by `buildPostHogCaptureEvents`. The main event name is co | PostHog event | When | Notable extras | | ------------------------- | ------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `$mcp_tool_call` | Every tool invocation | `$mcp_tool_name`, `$mcp_tool_description`, `$mcp_tool_category`, `$mcp_parameters`, `$mcp_response`, `$mcp_duration_ms`, `$mcp_is_error`, optionally `$mcp_intent` / `$mcp_intent_source` | +| `$mcp_tool_call` | Every tool invocation | `$mcp_tool_name`, `$mcp_tool_description`, `$mcp_tool_category`, `$mcp_parameters`, `$mcp_response`, `$mcp_duration_ms`, `$mcp_is_error`, optionally `$mcp_intent` / `$mcp_intent_source`, `$mcp_llm_model` / `$mcp_llm_model_source` | | `$mcp_tools_list` | Client lists tools | `$mcp_listed_tool_names` (array of tool names advertised); useful for "did this client discover us?" and "which advertised tools never get called?" | | `$mcp_initialize` | Client/server handshake | `$mcp_client_name`, `$mcp_client_version`, `$mcp_server_name`, `$mcp_server_version`, `$mcp_protocol_version` (negotiated MCP spec version — for tracking spec-revision adoption) | | `$mcp_missing_capability` | Agent calls the `get_more_tools` virtual tool | A capability gap, **not** a tool invocation. The `context` arg is captured as `$mcp_intent` with `$mcp_intent_source = "context_parameter"` | @@ -141,6 +141,8 @@ All wire keys live in `PostHogMCPAnalyticsProperty` (`src/extensions/constants.t | `Intent` | `$mcp_intent` | string | `context` argument when present, else `intentFallback()` return | | `IntentSource` | `$mcp_intent_source` | `"context_parameter" \| "inferred"` | Where the intent came from | | `ConversationId` | `$mcp_conversation_id` | string | Optional; set when `enableConversationId: true` and the SDK owns the tool's injected `conversation_id` parameter | +| `LlmModel` | `$mcp_llm_model` | string | Optional; set when `captureModel` is enabled and the SDK owns the tool's injected `llm_model` parameter. The calling agent's **self-reported, unverified** model id — right for degradation analytics ("does our MCP get worse on model X?"), never for billing or security. A blank or `"unknown"` answer is dropped rather than recorded | +| `LlmModelSource` | `$mcp_llm_model_source` | `"self_reported"` | How the model id was obtained. Always `self_reported` today — it exists so a future verified source stays distinguishable | | `Parameters` | `$mcp_parameters` | object | Sanitized MCP request payload (see §3) | | `Response` | `$mcp_response` | object | Sanitized tool result | @@ -171,6 +173,7 @@ The `posthog-node` client is **not** an option — it is the required positional | `enableConversationId` | `false` | Inject `conversation_id` into eligible tools that don't already declare it and stamp `$mcp_conversation_id` on their events. | | `reportMissing` | `false` | Register the `get_more_tools` virtual tool. | | `context` | `true` (object form: `{ description }`) | Inject required `context` arg into every tool schema. | +| `captureModel` | `false` (object form: `{ description }`) | Inject a required `llm_model` arg into every tool schema (including the `get_more_tools` virtual tool) so the agent self-reports the model it runs as → `$mcp_llm_model`. Capture and stripping both require confirmed SDK ownership, so a fresh-instance-per-request server that never served the `tools/list` records nothing (see README). | | `intentFallback` | — | Consumer-supplied callback returning a `$mcp_intent` string when the client didn't pass a `context` argument. SDK does no inference. | | `identify` | — | Per-request callback returning `{ distinctId, properties?, groups? } \| null` — posthog-node's `identify` shape. `properties` → `$set`, `groups` → `$groups`. | | `beforeSend` | — | `(event) => event \| null \| undefined` (sync or async), matching posthog-node. Runs on each fully-built payload right before `posthog.capture()` — once per emitted event, including the `$exception` sibling. Return nullish (or throw) to drop that event. | diff --git a/packages/mcp/src/__tests__/model-parameters.test.ts b/packages/mcp/src/__tests__/model-parameters.test.ts index 2d10d619ac..9a60b57142 100644 --- a/packages/mcp/src/__tests__/model-parameters.test.ts +++ b/packages/mcp/src/__tests__/model-parameters.test.ts @@ -174,6 +174,55 @@ describe('Model capture — integration with an instrumented server', () => { } }) + it('injects llm_model into the get_more_tools virtual tool, keeping its own context parameter', async () => { + instrument(server, fakePostHog(), { captureModel: true, reportMissing: true }) + + const toolsResponse = await client.request({ method: 'tools/list', params: {} }, ListToolsResultSchema) + const virtualTool = toolsResponse.tools.find((t: any) => t.name === 'get_more_tools') + + // The virtual tool is appended after the real ones, so it only advertises + // llm_model if injection runs after the append. + expect(virtualTool).toBeDefined() + expect(virtualTool.inputSchema.properties.llm_model).toBeDefined() + expect(virtualTool.inputSchema.required).toContain('llm_model') + // Its own context parameter is untouched — that argument is the report itself. + expect(virtualTool.inputSchema.properties.context).toBeDefined() + expect(virtualTool.inputSchema.required).toContain('context') + }) + + it('captures llm_model on a get_more_tools report and strips it from the handler args', async () => { + const capture = new EventCapture() + await capture.start() + try { + instrument(server, fakePostHog(), { captureModel: true, reportMissing: true }) + + await client.request({ method: 'tools/list', params: {} }, ListToolsResultSchema) + + const result = await client.request( + { + method: 'tools/call', + params: { + name: 'get_more_tools', + arguments: { context: 'Need a SQL query tool', llm_model: 'claude-opus-4-8' }, + }, + }, + CallToolResultSchema + ) + + expect(result.content[0].text).toContain('Unfortunately') + + const reports = capture.findCapturesByEvent('$mcp_missing_capability') + expect(reports).toHaveLength(1) + expect(reports[0].properties.$mcp_llm_model).toBe('claude-opus-4-8') + expect(reports[0].properties.$mcp_llm_model_source).toBe('self_reported') + // The context argument is still the report; only llm_model is ours to take. + expect(reports[0].properties.$mcp_intent).toBe('Need a SQL query tool') + expect((reports[0].properties.$mcp_parameters as any)?.llm_model).toBeUndefined() + } finally { + await capture.stop() + } + }) + it('captures llm_model as $mcp_llm_model with source=self_reported and strips it from the tool args', async () => { const capture = new EventCapture() await capture.start() diff --git a/packages/mcp/src/extensions/analytics-parameters.ts b/packages/mcp/src/extensions/analytics-parameters.ts index c4055d4f77..b5afe51dff 100644 --- a/packages/mcp/src/extensions/analytics-parameters.ts +++ b/packages/mcp/src/extensions/analytics-parameters.ts @@ -1,4 +1,5 @@ import type { AnalyticsParameterOwnership } from '../types' +import { log, type LoggerFn } from './logger' import { getObjectShape, isZodRawShapeCompat } from './mcp-sdk-compat' import { canDeclareOutputInstructions } from './output-instructions' @@ -42,6 +43,86 @@ export function canInjectAnalyticsParameter( ) } +export interface AnalyticsInjectableTool { + inputSchema?: AnalyticsInjectableJsonSchema + name?: string + [key: string]: unknown +} + +/** + * Adds a required string parameter to a tool's JSON Schema. Called AFTER the MCP + * SDK has converted Zod schemas to JSON Schema, so only JSON Schema needs + * handling. + * + * Shared by every analytics parameter we inject (`context`, `llm_model`) so + * schema ownership, cloning, and required-field behavior cannot drift between + * them. `injectionLabel` only names the feature in the skip warnings. + * + * Skips injection (with a warning) for: + * - Tools that already declare the parameter — it is theirs, not ours + * - Complex schemas (oneOf/allOf/anyOf/$ref) that can't safely gain properties + */ +export function addAnalyticsParameterToTool( + tool: TTool, + parameterName: string, + description: string, + injectionLabel: string, + logger: LoggerFn = log +): TTool { + const modifiedTool = { ...tool } + const toolName = tool.name || 'unknown' + const schema = modifiedTool.inputSchema as AnalyticsInjectableJsonSchema | undefined + + if (!canInjectAnalyticsParameter(schema, parameterName)) { + if (hasAnalyticsParameter(schema, parameterName)) { + logger(`WARN: Tool "${toolName}" already has '${parameterName}' parameter. Skipping ${injectionLabel} injection.`) + } else { + logger( + `WARN: Tool "${toolName}" has complex schema (oneOf/allOf/anyOf/$ref). Skipping ${injectionLabel} injection.` + ) + } + return modifiedTool + } + + if (!modifiedTool.inputSchema) { + modifiedTool.inputSchema = { + type: 'object', + properties: {}, + required: [], + } + } + + // Deep copy: the server may reuse or freeze the schema object it handed us. + modifiedTool.inputSchema = JSON.parse(JSON.stringify(modifiedTool.inputSchema)) as AnalyticsInjectableJsonSchema + + const inputSchema = modifiedTool.inputSchema as AnalyticsInjectableJsonSchema + + if (!inputSchema.properties) { + inputSchema.properties = {} + } + + // The MCP SDK emits `additionalProperties: false` when converting Zod schemas; + // left in place it would make the injected key invalid. + if (inputSchema.additionalProperties === false) { + inputSchema.additionalProperties = undefined + } + + inputSchema.properties[parameterName] = { + type: 'string', + description, + } + + if (Array.isArray(inputSchema.required)) { + if (!inputSchema.required.includes(parameterName)) { + inputSchema.required.push(parameterName) + } + } else { + inputSchema.required = [parameterName] + } + + return modifiedTool +} + export function analyticsOwnsParameter(inputSchema: unknown, parameterName: string): boolean { if (!inputSchema || typeof inputSchema !== 'object') { return canInjectAnalyticsParameter(undefined, parameterName) diff --git a/packages/mcp/src/extensions/context-parameters.ts b/packages/mcp/src/extensions/context-parameters.ts index a439fc10cc..eede18717c 100644 --- a/packages/mcp/src/extensions/context-parameters.ts +++ b/packages/mcp/src/extensions/context-parameters.ts @@ -4,11 +4,7 @@ // Licensed under the MIT License: https://github.com/agentcathq/agentcat-typescript-sdk/blob/main/LICENSE import type { MCPAnalyticsOptions } from '../types' -import { - canInjectAnalyticsParameter, - hasAnalyticsParameter, - type AnalyticsInjectableJsonSchema, -} from './analytics-parameters' +import { addAnalyticsParameterToTool, type AnalyticsInjectableJsonSchema } from './analytics-parameters' import { DEFAULT_CONTEXT_PARAMETER_DESCRIPTION } from './constants' import { log, type LoggerFn } from './logger' @@ -27,8 +23,8 @@ export function getContextDescription(context: MCPAnalyticsOptions['context']): } /** - * Adds a context parameter to a tool's JSON Schema. - * This function is called AFTER the MCP SDK has converted Zod schemas to JSON Schema, + * Adds a context parameter to a tool's JSON Schema, via the shared injector. + * This is called AFTER the MCP SDK has converted Zod schemas to JSON Schema, * so we only need to handle JSON Schema format. * * Skips injection (with warning) for: @@ -41,58 +37,13 @@ export function addContextParameterToTool( contextDescriptionOverride?: string, logger: LoggerFn = log ): TTool { - const modifiedTool = { ...tool } - const toolName = tool.name || 'unknown' - const schema = modifiedTool.inputSchema as AnalyticsInjectableJsonSchema | undefined - - if (!canInjectAnalyticsParameter(schema, 'context')) { - if (hasAnalyticsParameter(schema, 'context')) { - logger(`WARN: Tool "${toolName}" already has 'context' parameter. Skipping context injection.`) - } else { - logger(`WARN: Tool "${toolName}" has complex schema (oneOf/allOf/anyOf/$ref). Skipping context injection.`) - } - return modifiedTool - } - - if (!modifiedTool.inputSchema) { - modifiedTool.inputSchema = { - type: 'object', - properties: {}, - required: [], - } - } - - const contextDescription = contextDescriptionOverride || DEFAULT_CONTEXT_PARAMETER_DESCRIPTION - - // Deep copy: the server may reuse or freeze the schema object it handed us. - modifiedTool.inputSchema = JSON.parse(JSON.stringify(modifiedTool.inputSchema)) as AnalyticsInjectableJsonSchema - - const inputSchema = modifiedTool.inputSchema as AnalyticsInjectableJsonSchema - - if (!inputSchema.properties) { - inputSchema.properties = {} - } - - // The MCP SDK emits `additionalProperties: false` when converting Zod schemas; - // left in place it would make the injected `context` key invalid. - if (inputSchema.additionalProperties === false) { - inputSchema.additionalProperties = undefined - } - - inputSchema.properties.context = { - type: 'string', - description: contextDescription, - } - - if (Array.isArray(inputSchema.required)) { - if (!inputSchema.required.includes('context')) { - inputSchema.required.push('context') - } - } else { - inputSchema.required = ['context'] - } - - return modifiedTool + return addAnalyticsParameterToTool( + tool, + 'context', + contextDescriptionOverride || DEFAULT_CONTEXT_PARAMETER_DESCRIPTION, + 'context', + logger + ) } export function addContextParameterToTools( diff --git a/packages/mcp/src/extensions/instrumentation.ts b/packages/mcp/src/extensions/instrumentation.ts index 1347a7cb14..8a33b626de 100644 --- a/packages/mcp/src/extensions/instrumentation.ts +++ b/packages/mcp/src/extensions/instrumentation.ts @@ -661,9 +661,6 @@ async function getTracedToolsList( if (data && isContextEnabled(data.options.context)) { tools = addContextParameterToTools(tools, getContextDescription(data.options.context), data.logger) } - if (data && isCaptureModelEnabled(data.options.captureModel)) { - tools = addModelParameterToTools(tools, getModelDescription(data.options.captureModel), data.logger) - } if (data) { const missingToolName = resolveMissingCapabilityToolName(data.options) @@ -682,6 +679,17 @@ async function getTracedToolsList( } } + // After the virtual tool is appended, so it advertises `llm_model` too. + // Its ownership was cached above from the un-injected descriptor, so the + // SDK already strips and captures the argument on its calls — without + // injecting here the agent would never be asked for one, and + // missing-capability reports would be the only calls with no model. + // `context` is deliberately not injected into it: the virtual tool + // declares its own, and that argument is the report itself. + if (isCaptureModelEnabled(data.options.captureModel)) { + tools = addModelParameterToTools(tools, getModelDescription(data.options.captureModel), data.logger) + } + if (data.options.enableConversationId) { tools = addConversationIdToTools(tools, data.logger) tools = addInstructionsToOutputSchemas(tools, data.logger) diff --git a/packages/mcp/src/extensions/model-parameters.ts b/packages/mcp/src/extensions/model-parameters.ts index 0b515821d6..3e5ba702f2 100644 --- a/packages/mcp/src/extensions/model-parameters.ts +++ b/packages/mcp/src/extensions/model-parameters.ts @@ -1,9 +1,5 @@ import type { MCPAnalyticsOptions, MCPRequestLike, McpEvent } from '../types' -import { - canInjectAnalyticsParameter, - hasAnalyticsParameter, - type AnalyticsInjectableJsonSchema, -} from './analytics-parameters' +import { addAnalyticsParameterToTool, type AnalyticsInjectableJsonSchema } from './analytics-parameters' import { DEFAULT_MODEL_PARAMETER_DESCRIPTION } from './constants' import { log, type LoggerFn } from './logger' @@ -45,9 +41,8 @@ export function getModelDescription(captureModel: MCPAnalyticsOptions['captureMo } /** - * Adds an `llm_model` parameter to a tool's JSON Schema. Called AFTER the MCP - * SDK has converted Zod schemas to JSON Schema, so only JSON Schema needs - * handling. Skip rules match `addContextParameterToTool`: a tool that already + * Adds an `llm_model` parameter to a tool's JSON Schema, via the shared + * injector so schema handling stays identical to `context`: a tool that already * declares `llm_model` owns it, and complex schemas can't safely gain keys. */ export function addModelParameterToTool( @@ -55,58 +50,13 @@ export function addModelParameterToTool( modelDescriptionOverride?: string, logger: LoggerFn = log ): TTool { - const modifiedTool = { ...tool } - const toolName = tool.name || 'unknown' - const schema = modifiedTool.inputSchema as AnalyticsInjectableJsonSchema | undefined - - if (!canInjectAnalyticsParameter(schema, 'llm_model')) { - if (hasAnalyticsParameter(schema, 'llm_model')) { - logger(`WARN: Tool "${toolName}" already has 'llm_model' parameter. Skipping model injection.`) - } else { - logger(`WARN: Tool "${toolName}" has complex schema (oneOf/allOf/anyOf/$ref). Skipping model injection.`) - } - return modifiedTool - } - - if (!modifiedTool.inputSchema) { - modifiedTool.inputSchema = { - type: 'object', - properties: {}, - required: [], - } - } - - const modelDescription = modelDescriptionOverride || DEFAULT_MODEL_PARAMETER_DESCRIPTION - - // Deep copy: the server may reuse or freeze the schema object it handed us. - modifiedTool.inputSchema = JSON.parse(JSON.stringify(modifiedTool.inputSchema)) as AnalyticsInjectableJsonSchema - - const inputSchema = modifiedTool.inputSchema as AnalyticsInjectableJsonSchema - - if (!inputSchema.properties) { - inputSchema.properties = {} - } - - // The MCP SDK emits `additionalProperties: false` when converting Zod schemas; - // left in place it would make the injected `llm_model` key invalid. - if (inputSchema.additionalProperties === false) { - inputSchema.additionalProperties = undefined - } - - inputSchema.properties.llm_model = { - type: 'string', - description: modelDescription, - } - - if (Array.isArray(inputSchema.required)) { - if (!inputSchema.required.includes('llm_model')) { - inputSchema.required.push('llm_model') - } - } else { - inputSchema.required = ['llm_model'] - } - - return modifiedTool + return addAnalyticsParameterToTool( + tool, + 'llm_model', + modelDescriptionOverride || DEFAULT_MODEL_PARAMETER_DESCRIPTION, + 'model', + logger + ) } export function addModelParameterToTools( From 6c10c1bd95e4721945f26e263408b34a429e2e41 Mon Sep 17 00:00:00 2001 From: Daniel Visca Date: Tue, 25 Aug 2026 12:31:13 -0700 Subject: [PATCH 3/3] fix(mcp): resolve virtual-tool parameter ownership without the tools/list cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The missing-capability branch called captureToolCall with no parameterOwnership, so ownership fell back to the per-instance tools/list cache. On a per-request server instance (the topology in ADR-0011) that cache is empty, so a get_more_tools call handled by an instance that never served a listing neither captured $mcp_llm_model nor stripped the injected llm_model argument. That branch is only entered when the application does not advertise a tool by this name, so the descriptor is the SDK's own and what it declares is known statically — the fail-closed rationale for registered tools (the value might be the application's own argument) cannot apply. Resolve ownership from the descriptor instead, via a shared helper used by both the high-level and low-level adapters, which had the same gap. conversation_id deliberately still reads from the cache: resolving it statically would start minting a handle, and appending its prompt-back block, on instances that today mint none — a change to session anchoring (ADR-0004) rather than to model capture. Tests cover both adapters with separate advertising and calling instances, and the README no longer implies the live registry covers the virtual tool. Generated-By: PostHog Desktop Task-Id: 44c7be3e-4938-4f9a-bb80-89fd9d74db6d --- packages/mcp/README.md | 17 ++++---- .../src/__tests__/instrument-lowlevel.test.ts | 34 +++++++++++++++ .../src/__tests__/model-parameters.test.ts | 42 +++++++++++++++++++ .../src/extensions/instrument-highlevel.ts | 2 + .../mcp/src/extensions/instrument-lowlevel.ts | 2 + .../mcp/src/extensions/instrumentation.ts | 25 +++++++++++ 6 files changed, 115 insertions(+), 7 deletions(-) diff --git a/packages/mcp/README.md b/packages/mcp/README.md index 0f1652b5e9..fbe41c322b 100644 --- a/packages/mcp/README.md +++ b/packages/mcp/README.md @@ -163,13 +163,16 @@ control. An agent that answers `unknown` is recorded as nothing rather than as a Unlike `context`, this option degrades to **silence** rather than to a kept argument. Both the strip and the capture require the SDK to have confirmed the parameter is its own: -- `instrument(server)` on a high-level `McpServer` resolves ownership per request from the live tool - registry, so it works even on a fresh instance. -- Instrumenting a low-level `Server` learns ownership while serving `tools/list`. On a server that - builds a fresh instance per HTTP request — `createMcpHandler`, or `@rekog/mcp-nest` in its - stateless mode — the instance handling a `tools/call` never served one, so it neither strips - `llm_model` nor records `$mcp_llm_model`. Nothing breaks and no wrong value is stored; the property - is simply absent while agents still pay a token for the extra field. +- `instrument(server)` on a high-level `McpServer` resolves ownership for your registered tools per + request from the live tool registry, so those work even on a fresh instance. +- The `get_more_tools` virtual tool works on any instance and on either server type: the SDK writes + that descriptor itself, so what it declares is known without a listing. +- Instrumenting a low-level `Server` learns ownership of **your** tools while serving `tools/list`. + On a server that builds a fresh instance per HTTP request — `createMcpHandler`, or + `@rekog/mcp-nest` in its stateless mode — the instance handling a `tools/call` never served one, + so for those tools it neither strips `llm_model` nor records `$mcp_llm_model`. Nothing breaks and + no wrong value is stored; the property is simply absent while agents still pay a token for the + extra field. As with `context`, what matters is instance lifetime rather than statelessness: a transport-stateless server (`sessionIdGenerator: undefined`) that keeps one long-lived server object learns ownership diff --git a/packages/mcp/src/__tests__/instrument-lowlevel.test.ts b/packages/mcp/src/__tests__/instrument-lowlevel.test.ts index 1920a1165e..e5094cfff9 100644 --- a/packages/mcp/src/__tests__/instrument-lowlevel.test.ts +++ b/packages/mcp/src/__tests__/instrument-lowlevel.test.ts @@ -227,6 +227,40 @@ describe('Low-level Server reportMissing ownership (e2e)', () => { } }) + it('captures llm_model on a virtual call handled by a pod that never advertised it', async () => { + const podA = await setupLowLevelServer() + const podB = await setupLowLevelServer() + try { + instrument(podA.server, fakePostHog(), { reportMissing: true, captureModel: true }) + instrument(podB.server, fakePostHog(), { reportMissing: true, captureModel: true }) + await Promise.all([podA.connect(), podB.connect()]) + + const { tools } = await podA.client.request({ method: 'tools/list', params: {} }, ListToolsResultSchema) + const advertised = tools.find((tool) => tool.name === 'get_more_tools') + expect(advertised?.inputSchema?.properties?.llm_model).toBeDefined() + + await podB.client.request( + { + method: 'tools/call', + params: { + name: 'get_more_tools', + arguments: { context: 'Need a database tool', llm_model: 'claude-opus-4-8' }, + }, + }, + CallToolResultSchema + ) + + await new Promise((resolve) => setTimeout(resolve, 50)) + const captures = eventCapture.findCapturesByEvent('$mcp_missing_capability') + expect(captures).toHaveLength(1) + expect(captures[0].properties.$mcp_llm_model).toBe('claude-opus-4-8') + expect(captures[0].properties.$mcp_llm_model_source).toBe('self_reported') + expect((captures[0].properties.$mcp_parameters as any)?.llm_model).toBeUndefined() + } finally { + await Promise.all([podA.cleanup(), podB.cleanup()]) + } + }) + it('handles a custom-named virtual tool on a fresh instance', async () => { const customName = 'posthog_find_tools' const { server, client, connect, cleanup } = await setupLowLevelServer() diff --git a/packages/mcp/src/__tests__/model-parameters.test.ts b/packages/mcp/src/__tests__/model-parameters.test.ts index 9a60b57142..a56fa31719 100644 --- a/packages/mcp/src/__tests__/model-parameters.test.ts +++ b/packages/mcp/src/__tests__/model-parameters.test.ts @@ -190,6 +190,48 @@ describe('Model capture — integration with an instrumented server', () => { expect(virtualTool.inputSchema.required).toContain('context') }) + it('captures llm_model on a get_more_tools report handled by an instance that never served tools/list', async () => { + // One instance advertises the virtual tool, a different one handles its call + // — the per-request-instance topology (`createMcpHandler`, `@rekog/mcp-nest` + // stateless) the README says `instrument(McpServer)` supports. The calling + // instance has an empty ownership cache, but `get_more_tools` is the SDK's + // own descriptor, so ownership is known statically rather than learned. + const advertising = await setupTestServerAndClient() + try { + instrument(advertising.server, fakePostHog(), { captureModel: true, reportMissing: true }) + const listed = await advertising.client.request({ method: 'tools/list', params: {} }, ListToolsResultSchema) + expect(listed.tools.find((t: any) => t.name === 'get_more_tools').inputSchema.properties.llm_model).toBeDefined() + } finally { + await advertising.cleanup() + } + + const capture = new EventCapture() + await capture.start() + try { + instrument(server, fakePostHog(), { captureModel: true, reportMissing: true }) + + // No tools/list on this instance. + await client.request( + { + method: 'tools/call', + params: { + name: 'get_more_tools', + arguments: { context: 'Need a SQL query tool', llm_model: 'claude-opus-4-8' }, + }, + }, + CallToolResultSchema + ) + + const reports = capture.findCapturesByEvent('$mcp_missing_capability') + expect(reports).toHaveLength(1) + expect(reports[0].properties.$mcp_llm_model).toBe('claude-opus-4-8') + expect(reports[0].properties.$mcp_llm_model_source).toBe('self_reported') + expect((reports[0].properties.$mcp_parameters as any)?.llm_model).toBeUndefined() + } finally { + await capture.stop() + } + }) + it('captures llm_model on a get_more_tools report and strips it from the handler args', async () => { const capture = new EventCapture() await capture.start() diff --git a/packages/mcp/src/extensions/instrument-highlevel.ts b/packages/mcp/src/extensions/instrument-highlevel.ts index 428713d416..2b304ec639 100644 --- a/packages/mcp/src/extensions/instrument-highlevel.ts +++ b/packages/mcp/src/extensions/instrument-highlevel.ts @@ -28,6 +28,7 @@ import { handleListToolsRequest, patchRequestHandlers, captureToolCall, + getVirtualToolParameterOwnership, isToolAdvertised, readToolMetaCategory, type HandlerPatch, @@ -249,6 +250,7 @@ async function handleToolCallRequest( extra, eventType: MCPAnalyticsEventType.mcpMissingCapability, explicitContextIntent: context, + parameterOwnership: getVirtualToolParameterOwnership(data, toolName), execute: async () => handleReportMissing({ context }, data.logger), }) } diff --git a/packages/mcp/src/extensions/instrument-lowlevel.ts b/packages/mcp/src/extensions/instrument-lowlevel.ts index 2a17f8059d..321bd7aa81 100644 --- a/packages/mcp/src/extensions/instrument-lowlevel.ts +++ b/packages/mcp/src/extensions/instrument-lowlevel.ts @@ -14,6 +14,7 @@ import { patchRequestHandlers, registerFallbackRequestHandler, captureToolCall, + getVirtualToolParameterOwnership, isToolAdvertised, type HandlerPatch, } from './instrumentation' @@ -90,6 +91,7 @@ async function handleToolCallRequest( extra, eventType: MCPAnalyticsEventType.mcpMissingCapability, explicitContextIntent: context, + parameterOwnership: getVirtualToolParameterOwnership(data, toolName), execute: async () => handleReportMissing({ context }, data.logger), }) } diff --git a/packages/mcp/src/extensions/instrumentation.ts b/packages/mcp/src/extensions/instrumentation.ts index 8a33b626de..67b9d084a6 100644 --- a/packages/mcp/src/extensions/instrumentation.ts +++ b/packages/mcp/src/extensions/instrumentation.ts @@ -508,6 +508,31 @@ export function patchRequestHandlers(server: MCPServerLike, patches: Record