fix(mcp): reject conflicting exclusions on shared operations

docs/published/handbook/engineering/ai/implementing-mcp-tools.md

@@ -356,6 +356,10 @@ Product teams own their definitions and control which operations are exposed as
    For a PATCH action with required request fields, set `param_overrides.<field>.required: true`.
    The MCP tool then requires the field, even when the generated PATCH body marks it optional.
 
+   Enabled tools that share one `operation` in the same YAML file must list the same fields in `exclude_params`.
+   An omitted list counts as empty; order and duplicate entries do not matter. Disabled tools do not count.
+   Conflicting lists fail the build, because those tools share one generated request body schema.
+
    Unknown keys are rejected at build time (Zod `.strict()`) to catch typos early.
 
    For list tools with a UI app, set `response.text_include` to the dot-path fields an agent

services/mcp/scripts/generate-orval-schemas.mjs

@@ -16,7 +16,6 @@ import fs from 'node:fs'
 import os from 'node:os'
 import path from 'node:path'
 import { fileURLToPath } from 'node:url'
-import { parse as parseYaml } from 'yaml'
 
 import {
     applyNestedExclusions,
@@ -25,7 +24,7 @@ import {
     runOrvalParallel,
 } from '@posthog/openapi-codegen'
 
-import { discoverDefinitions, resolveSchemaPath } from './lib/definitions.mjs'
+import { discoverDefinitions, parseToolDefinition, resolveSchemaPath } from './lib/definitions.mjs'
 import { lazifyZodSchemas } from './lib/lazy-zod-schemas.mjs'
 import { stripEnumMinLength, stripUuidFormat } from './lib/schema-transforms.mjs'
 
@@ -43,33 +42,6 @@ if (!fs.existsSync(schemaPath)) {
     process.exit(1)
 }
 
-/**
- * Parse a YAML tool definition and return operationIds plus all exclude_params
- * grouped by operationId for schema-level exclusion before Orval runs.
- */
-function parseToolDefinition(filePath) {
-    const content = fs.readFileSync(filePath, 'utf-8')
-    const parsed = parseYaml(content)
-    const operationIds = new Set()
-    /** @type {Map<string, string[]>} */
-    const schemaExclusions = new Map()
-
-    if (parsed?.tools) {
-        for (const tool of Object.values(parsed.tools)) {
-            if (!tool?.enabled || !tool?.operation) {
-                continue
-            }
-
-            operationIds.add(tool.operation)
-            const excludeParams = tool.exclude_params ?? []
-            if (excludeParams.length > 0) {
-                schemaExclusions.set(tool.operation, excludeParams)
-            }
-        }
-    }
-    return { operationIds, schemaExclusions }
-}
-
 // ------------------------------------------------------------------
 // OpenAPI post-processing (MCP-specific)
 // ------------------------------------------------------------------

services/mcp/scripts/lib/definitions.mjs

@@ -6,6 +6,7 @@
  */
 import fs from 'node:fs'
 import path from 'node:path'
+import { parse as parseYaml } from 'yaml'
 
 /**
  * Resolve the OpenAPI schema path, respecting OPENAPI_SCHEMA_PATH env override.
@@ -86,3 +87,34 @@ export function isQueryWrappersConfig(parsed) {
 export function isToolsConfig(parsed) {
     return typeof parsed === 'object' && parsed !== null && 'tools' in parsed
 }
+
+/**
+ * Parse a YAML tool definition and return operationIds plus all exclude_params
+ * grouped by operationId for schema-level exclusion before Orval runs.
+ */
+export function parseToolDefinition(filePath) {
+    const content = fs.readFileSync(filePath, 'utf-8')
+    const parsed = parseYaml(content)
+    const operationIds = new Set()
+    /** @type {Map<string, string[]>} */
+    const schemaExclusions = new Map()
+
+    if (parsed?.tools) {
+        for (const tool of Object.values(parsed.tools)) {
+            if (!tool?.enabled || !tool?.operation) {
+                continue
+            }
+
+            operationIds.add(tool.operation)
+            const excludeParams = [...new Set(tool.exclude_params ?? [])].sort()
+            const previous = schemaExclusions.get(tool.operation)
+            if (previous && JSON.stringify(previous) !== JSON.stringify(excludeParams)) {
+                throw new Error(
+                    `Tools on "${tool.operation}" in ${filePath} exclude different params. Give them the same exclude_params.`
+                )
+            }
+            schemaExclusions.set(tool.operation, excludeParams)
+        }
+    }
+    return { operationIds, schemaExclusions }
+}

services/mcp/tests/unit/tool-definitions.test.ts

@@ -0,0 +1,85 @@
+import * as fs from 'node:fs'
+import * as os from 'node:os'
+import * as path from 'node:path'
+import { afterEach, beforeEach, describe, expect, it } from 'vitest'
+import { stringify } from 'yaml'
+
+import { parseToolDefinition } from '../../scripts/lib/definitions.mjs'
+
+describe('parseToolDefinition', () => {
+    let directory: string
+    let filePath: string
+
+    beforeEach(() => {
+        directory = fs.mkdtempSync(path.join(os.tmpdir(), 'mcp-definitions-'))
+        filePath = path.join(directory, 'tools.yaml')
+    })
+
+    afterEach(() => {
+        fs.rmSync(directory, { recursive: true, force: true })
+    })
+
+    it.each([
+        { first: undefined, second: ['kind'] },
+        { first: ['kind'], second: undefined },
+        { first: ['kind'], second: ['name'] },
+        { first: ['config.secret'], second: [] },
+    ])('rejects conflicting exclusions $first and $second on a shared operation', ({ first, second }) => {
+        fs.writeFileSync(
+            filePath,
+            stringify({
+                tools: {
+                    'things-create': { operation: 'things_create', enabled: true, exclude_params: first },
+                    'things-email-create': { operation: 'things_create', enabled: true, exclude_params: second },
+                },
+            })
+        )
+
+        expect(() => parseToolDefinition(filePath)).toThrow(
+            `Tools on "things_create" in ${filePath} exclude different params. Give them the same exclude_params.`
+        )
+    })
+
+    it.each([
+        { first: undefined, second: [], expected: [] },
+        { first: ['kind'], second: ['kind'], expected: ['kind'] },
+        { first: ['name', 'kind', 'kind'], second: ['kind', 'name'], expected: ['kind', 'name'] },
+    ])('accepts equivalent exclusions $first and $second', ({ first, second, expected }) => {
+        fs.writeFileSync(
+            filePath,
+            stringify({
+                tools: {
+                    'things-create': { operation: 'things_create', enabled: true, exclude_params: first },
+                    'things-email-create': { operation: 'things_create', enabled: true, exclude_params: second },
+                },
+            })
+        )
+
+        expect(parseToolDefinition(filePath)).toEqual({
+            operationIds: new Set(['things_create']),
+            schemaExclusions: new Map([['things_create', expected]]),
+        })
+    })
+
+    it('ignores disabled tools and keeps exclusions independent across operations', () => {
+        fs.writeFileSync(
+            filePath,
+            stringify({
+                tools: {
+                    'things-create': { operation: 'things_create', enabled: false },
+                    'things-email-create': { operation: 'things_create', enabled: true, exclude_params: ['kind'] },
+                    'things-sms-create': { operation: 'things_create', enabled: false, exclude_params: ['name'] },
+                    'widgets-create': { operation: 'widgets_create', enabled: true },
+                },
+            })
+        )
+
+        expect(parseToolDefinition(filePath)).toEqual({
+            operationIds: new Set(['things_create', 'widgets_create']),
+            schemaExclusions: new Map([
+                ['things_create', ['kind']],
+                ['widgets_create', []],
+            ]),
+        })
+    })
+})