fix(codemod): keep request() result schemas the call still needs (#2383)
This commit is contained in:
parent
ba27d201e1
commit
9f8ba61279
3 changed files with 157 additions and 0 deletions
5
.changeset/lucky-poets-refuse.md
Normal file
5
.changeset/lucky-poets-refuse.md
Normal file
|
|
@ -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.
|
||||
|
|
@ -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++;
|
||||
|
||||
|
|
|
|||
|
|
@ -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');
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue