diff --git a/.changeset/lucky-poets-refuse.md b/.changeset/lucky-poets-refuse.md new file mode 100644 index 000000000..92f005285 --- /dev/null +++ b/.changeset/lucky-poets-refuse.md @@ -0,0 +1,5 @@ +--- +'@modelcontextprotocol/codemod': patch +--- + +Keep the result-schema argument on `request()` calls unless the method is a literal spec method, and keep the generic passthrough `ResultSchema` even then. Schema-less v2 `request()` enforces the spec result schema for spec methods and throws a `TypeError` for non-spec methods, so dropping the schema from a dynamic-method call site (the proxy/forwarder shape, `request({ method, params }, ResultSchema)`) or from a custom-method call broke the call. `callTool()` is unaffected — v2 `callTool()` has no schema parameter. diff --git a/packages/codemod/src/migrations/v1-to-v2/transforms/schemaParamRemoval.ts b/packages/codemod/src/migrations/v1-to-v2/transforms/schemaParamRemoval.ts index 66143b253..c8fd439eb 100644 --- a/packages/codemod/src/migrations/v1-to-v2/transforms/schemaParamRemoval.ts +++ b/packages/codemod/src/migrations/v1-to-v2/transforms/schemaParamRemoval.ts @@ -3,9 +3,40 @@ import { Node, SyntaxKind } from 'ts-morph'; import type { Transform, TransformContext, TransformResult } from '../../../types'; import { hasMcpImports, isImportedFromMcp, removeUnusedImport, resolveOriginalImportName } from '../../../utils/importUtils'; +import { SCHEMA_TO_METHOD } from '../mappings/schemaToMethodMap'; const TARGET_METHODS = new Set(['request', 'callTool']); +/** Spec request methods — the only methods whose result schema v2 resolves by name. */ +const SPEC_REQUEST_METHODS: ReadonlySet = new Set(Object.values(SCHEMA_TO_METHOD)); + +// `request()` keeps its result-schema parameter in v2 (custom methods, passthrough +// forwarding); only calls whose method the codemod can PROVE is a literal spec method +// may safely lose it. Schema-less v2 `request()` enforces the spec result schema for +// spec methods and throws a TypeError for non-spec methods, so dropping the schema +// from a dynamic-method call (`request({ method, params }, schema)` in a +// proxy/forwarder) or from a custom-method call breaks the call site. +function literalMethodOf(arg: Node): string | undefined { + if (!Node.isObjectLiteralExpression(arg)) return undefined; + const prop = arg.getProperty('method'); + if (!prop || !Node.isPropertyAssignment(prop)) return undefined; + // A spread after the `method` property can override it at runtime — not provably literal. + const props = arg.getProperties(); + const spreadAfterMethod = props.slice(props.indexOf(prop) + 1).some(p => Node.isSpreadAssignment(p)); + if (spreadAfterMethod) return undefined; + let initializer = prop.getInitializer(); + // Unwrap `'tools/call' as const` / `satisfies` / parenthesized forms. + while ( + initializer !== undefined && + (Node.isAsExpression(initializer) || Node.isSatisfiesExpression(initializer) || Node.isParenthesizedExpression(initializer)) + ) { + initializer = initializer.getExpression(); + } + if (initializer === undefined) return undefined; + if (!Node.isStringLiteral(initializer) && !Node.isNoSubstitutionTemplateLiteral(initializer)) return undefined; + return initializer.getLiteralValue(); +} + export const schemaParamRemovalTransform: Transform = { name: 'Schema parameter removal', id: 'schema-params', @@ -50,6 +81,18 @@ export const schemaParamRemovalTransform: Transform = { if (!originalName.endsWith('Schema')) continue; if (!isImportedFromMcp(sourceFile, schemaName)) continue; + // v2 `callTool()` has no schema parameter at all, so the argument always goes. + // v2 `request()` still accepts one — only drop it when the method is a literal + // SPEC method (schema-less request() throws a TypeError for anything else) and + // the schema is method-specific: the generic `ResultSchema` is the v1 + // passthrough idiom, and dropping it would silently switch the call from + // passthrough to spec-schema enforcement. + if (methodName === 'request') { + const literal = literalMethodOf(args[0]!); + if (literal === undefined || !SPEC_REQUEST_METHODS.has(literal)) continue; + if (originalName === 'ResultSchema') continue; + } + call.removeArgument(1); changesCount++; diff --git a/packages/codemod/test/v1-to-v2/transforms/schemaParamRemoval.test.ts b/packages/codemod/test/v1-to-v2/transforms/schemaParamRemoval.test.ts index 5f65411b5..93a52ecae 100644 --- a/packages/codemod/test/v1-to-v2/transforms/schemaParamRemoval.test.ts +++ b/packages/codemod/test/v1-to-v2/transforms/schemaParamRemoval.test.ts @@ -166,4 +166,113 @@ describe('schema-param-removal transform', () => { const result = applyTransform(input); expect(result).toContain('undefined'); }); + + it('keeps the schema (and its import) when request() has a shorthand dynamic method', () => { + // The proxy/forwarder shape: schema-less v2 request() would enforce spec result + // schemas and resolve `undefined` for unknown methods — the schema must survive. + const input = [ + `import { ResultSchema } from '@modelcontextprotocol/sdk/types.js';`, + `const result = await upstream.request({ method, params }, ResultSchema);`, + '' + ].join('\n'); + const result = applyTransform(input); + expect(result).toContain('request({ method, params }, ResultSchema)'); + expect(result).toMatch(/import.*ResultSchema/); + }); + + it('keeps the schema when request() has a variable method property', () => { + const input = [ + `import { CallToolResultSchema } from '@modelcontextprotocol/sdk/types.js';`, + `const result = await client.request({ method: m, params: {} }, CallToolResultSchema);`, + '' + ].join('\n'); + const result = applyTransform(input); + expect(result).toContain('CallToolResultSchema)'); + }); + + it('keeps the schema when the request object is not an object literal', () => { + const input = [ + `import { CallToolResultSchema } from '@modelcontextprotocol/sdk/types.js';`, + `const result = await relay.request(inbound, CallToolResultSchema);`, + '' + ].join('\n'); + const result = applyTransform(input); + expect(result).toContain('relay.request(inbound, CallToolResultSchema)'); + }); + + it('keeps the generic passthrough ResultSchema even for a literal spec method', () => { + // Dropping it would switch the call from passthrough validation to spec-schema + // enforcement — a silent semantic change. + const input = [ + `import { ResultSchema } from '@modelcontextprotocol/sdk/types.js';`, + `const result = await client.request({ method: 'ping' }, ResultSchema);`, + '' + ].join('\n'); + const result = applyTransform(input); + expect(result).toContain("request({ method: 'ping' }, ResultSchema)"); + }); + + it('keeps an aliased passthrough ResultSchema', () => { + const input = [ + `import { ResultSchema as RS } from '@modelcontextprotocol/sdk/types.js';`, + `const result = await client.request({ method: 'tools/call' }, RS);`, + '' + ].join('\n'); + const result = applyTransform(input); + expect(result).toContain("request({ method: 'tools/call' }, RS)"); + }); + + it('keeps the schema for a literal NON-spec method (custom methods need their schema)', () => { + // Schema-less v2 request() throws a TypeError for non-spec methods. + const input = [ + `import { CallToolResultSchema } from '@modelcontextprotocol/sdk/types.js';`, + `const result = await client.request({ method: 'acme/search', params: {} }, CallToolResultSchema);`, + '' + ].join('\n'); + const result = applyTransform(input); + expect(result).toContain("request({ method: 'acme/search', params: {} }, CallToolResultSchema)"); + }); + + it('still removes the schema for a no-substitution template-literal method', () => { + const input = [ + `import { CallToolResultSchema } from '@modelcontextprotocol/sdk/types.js';`, + 'const result = await client.request({ method: `tools/call`, params: {} }, CallToolResultSchema);', + '' + ].join('\n'); + const result = applyTransform(input); + expect(result).not.toContain('CallToolResultSchema)'); + }); + + it('still removes the schema when the literal method is wrapped in `as const`', () => { + const input = [ + `import { CallToolResultSchema } from '@modelcontextprotocol/sdk/types.js';`, + `const result = await client.request({ method: 'tools/call' as const, params: {} }, CallToolResultSchema);`, + '' + ].join('\n'); + const result = applyTransform(input); + expect(result).not.toContain('CallToolResultSchema)'); + }); + + it('keeps the schema when a spread follows the method property', () => { + // `{ method: 'tools/call', ...rest }` — rest can override method at runtime. + const input = [ + `import { CallToolResultSchema } from '@modelcontextprotocol/sdk/types.js';`, + `const result = await client.request({ method: 'tools/call', ...rest }, CallToolResultSchema);`, + '' + ].join('\n'); + const result = applyTransform(input); + expect(result).toContain('CallToolResultSchema)'); + }); + + it('still removes the schema from callTool() with a variable params object', () => { + // v2 callTool() has no schema parameter — the argument goes regardless of shape. + const input = [ + `import { CallToolResultSchema } from '@modelcontextprotocol/sdk/types.js';`, + `const result = await client.callTool(callParams, CallToolResultSchema);`, + '' + ].join('\n'); + const result = applyTransform(input); + expect(result).toContain('client.callTool(callParams)'); + expect(result).not.toContain('CallToolResultSchema'); + }); });