fix(codemod): keep request() result schemas the call still needs

.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.

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<string> = 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++;
 

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');
+    });
 });