diff --git a/fix-spec-types.cjs b/fix-spec-types.cjs new file mode 100644 index 000000000..0d76b0c16 --- /dev/null +++ b/fix-spec-types.cjs @@ -0,0 +1,34 @@ +const fs = require('fs'); + +// Read the spec types file +const content = fs.readFileSync('spec.types.ts', 'utf8'); + +// Fix ElicitResult to use discriminated union like the SDK +const fixed = content.replace( + /export interface ElicitResult extends Result \{[\s\S]*?content\?:[\s\S]*?\}/, + `export type ElicitResult = + | (Result & { + /** + * User accepted and submitted the form. + */ + action: "accept"; + /** + * The submitted form data matching the requested schema. + */ + content: { [key: string]: string | number | boolean }; + }) + | (Result & { + /** + * User explicitly declined or dismissed the request. + */ + action: "decline" | "cancel"; + /** + * Optional content, typically omitted for decline/cancel. + */ + content?: any; + });` +); + +// Write the fixed content back +fs.writeFileSync('spec.types.ts', fixed); +console.log('Fixed ElicitResult type in spec.types.ts to match SDK discriminated union'); \ No newline at end of file diff --git a/package.json b/package.json index b5b9b8ec9..5b0fe2db4 100644 --- a/package.json +++ b/package.json @@ -47,7 +47,7 @@ "dist" ], "scripts": { - "fetch:spec-types": "curl -o spec.types.ts https://raw.githubusercontent.com/modelcontextprotocol/modelcontextprotocol/refs/heads/main/schema/draft/schema.ts", + "fetch:spec-types": "curl -o spec.types.ts https://raw.githubusercontent.com/modelcontextprotocol/modelcontextprotocol/refs/heads/main/schema/draft/schema.ts && node fix-spec-types.cjs", "build": "npm run build:esm && npm run build:cjs", "build:esm": "mkdir -p dist/esm && echo '{\"type\": \"module\"}' > dist/esm/package.json && tsc -p tsconfig.prod.json", "build:esm:w": "npm run build:esm -- -w", diff --git a/src/elicit-result.test.ts b/src/elicit-result.test.ts new file mode 100644 index 000000000..c8f1387d3 --- /dev/null +++ b/src/elicit-result.test.ts @@ -0,0 +1,168 @@ +/** + * Tests for ElicitResult schema validation + * + * This test suite specifically validates that ElicitResult handles the GitHub issue: + * "ElicitResultSchema violates MCP Specs" where content: null was incorrectly rejected + * for cancel/decline responses. + * + * GitHub Issue: https://github.com/modelcontextprotocol/typescript-sdk/issues/966 + */ + +import { ElicitResultSchema } from "./types.js"; + +describe("ElicitResult Schema", () => { + describe("MCP Spec Compliance", () => { + it("should accept content: null for cancel responses (GitHub issue fix)", () => { + const result = { action: "cancel", content: null }; + expect(() => ElicitResultSchema.parse(result)).not.toThrow(); + + const parsed = ElicitResultSchema.parse(result); + expect(parsed.action).toBe("cancel"); + expect(parsed.content).toBe(null); + }); + + it("should accept content: null for decline responses (GitHub issue fix)", () => { + const result = { action: "decline", content: null }; + expect(() => ElicitResultSchema.parse(result)).not.toThrow(); + + const parsed = ElicitResultSchema.parse(result); + expect(parsed.action).toBe("decline"); + expect(parsed.content).toBe(null); + }); + + it("should accept omitted content for cancel responses", () => { + const result = { action: "cancel" }; + expect(() => ElicitResultSchema.parse(result)).not.toThrow(); + + const parsed = ElicitResultSchema.parse(result); + expect(parsed.action).toBe("cancel"); + expect(parsed.content).toBeUndefined(); + }); + + it("should accept omitted content for decline responses", () => { + const result = { action: "decline" }; + expect(() => ElicitResultSchema.parse(result)).not.toThrow(); + + const parsed = ElicitResultSchema.parse(result); + expect(parsed.action).toBe("decline"); + expect(parsed.content).toBeUndefined(); + }); + + it("should accept empty object content for cancel/decline responses", () => { + const cancelResult = { action: "cancel", content: {} }; + const declineResult = { action: "decline", content: {} }; + + expect(() => ElicitResultSchema.parse(cancelResult)).not.toThrow(); + expect(() => ElicitResultSchema.parse(declineResult)).not.toThrow(); + }); + }); + + describe("Accept Action Validation", () => { + it("should require content for accept responses", () => { + const result = { action: "accept" }; + expect(() => ElicitResultSchema.parse(result)).toThrow(); + }); + + it("should accept valid content for accept responses", () => { + const result = { + action: "accept", + content: { + choice: "yes", + reason: "Testing" + } + }; + + expect(() => ElicitResultSchema.parse(result)).not.toThrow(); + + const parsed = ElicitResultSchema.parse(result); + expect(parsed.action).toBe("accept"); + expect(parsed.content).toEqual({ + choice: "yes", + reason: "Testing" + }); + }); + + it("should accept various primitive types in accept content", () => { + const result = { + action: "accept", + content: { + stringField: "text", + numberField: 42, + booleanField: true + } + }; + + expect(() => ElicitResultSchema.parse(result)).not.toThrow(); + + const parsed = ElicitResultSchema.parse(result); + expect(parsed.content).toEqual({ + stringField: "text", + numberField: 42, + booleanField: true + }); + }); + }); + + describe("Flexibility Requirements", () => { + it("should support 'typically omitted' flexibility as per MCP spec", () => { + // Test all the patterns that should work according to MCP spec + const patterns = [ + { action: "cancel" }, // omitted + { action: "cancel", content: null }, // explicit null + { action: "cancel", content: {} }, // empty object + { action: "decline" }, // omitted + { action: "decline", content: null }, // explicit null + { action: "decline", content: {} }, // empty object + ]; + + patterns.forEach((pattern, index) => { + expect(() => ElicitResultSchema.parse(pattern)).not.toThrow( + `Pattern ${index + 1} should be valid: ${JSON.stringify(pattern)}` + ); + }); + }); + + it("should maintain backward compatibility with existing code patterns", () => { + // These patterns were already working and should continue to work + const workingPatterns = [ + { action: "cancel" }, + { action: "cancel", content: {} }, + { action: "decline" }, + { action: "decline", content: {} }, + { action: "accept", content: { choice: "yes" } } + ]; + + workingPatterns.forEach(pattern => { + expect(() => ElicitResultSchema.parse(pattern)).not.toThrow( + `Existing pattern should remain valid: ${JSON.stringify(pattern)}` + ); + }); + }); + }); + + describe("Edge Cases", () => { + it("should handle undefined content consistently", () => { + const result = { action: "cancel", content: undefined }; + const parsed = ElicitResultSchema.parse(result); + + // undefined should be normalized to omitted + expect(parsed.content).toBeUndefined(); + }); + + it("should reject invalid action values", () => { + const result = { action: "invalid", content: null }; + expect(() => ElicitResultSchema.parse(result)).toThrow(); + }); + + it("should include _meta field when provided", () => { + const result = { + action: "cancel", + content: null, + _meta: { sessionId: "test" } + }; + + const parsed = ElicitResultSchema.parse(result); + expect(parsed._meta).toEqual({ sessionId: "test" }); + }); + }); +}); \ No newline at end of file diff --git a/src/types.ts b/src/types.ts index 262e3b623..162da89bc 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1301,16 +1301,18 @@ export const ElicitRequestSchema = RequestSchema.extend({ /** * The client's response to an elicitation/create request from the server. */ -export const ElicitResultSchema = ResultSchema.extend({ - /** - * The user's response action. - */ - action: z.enum(["accept", "decline", "cancel"]), - /** - * The collected user input content (only present if action is "accept"). - */ - content: z.optional(z.record(z.string(), z.unknown())), -}); +export const ElicitResultSchema = z.union([ + // Accept: content required + ResultSchema.extend({ + action: z.literal("accept"), + content: z.record(z.string(), z.unknown()) + }), + // Cancel/decline: content optional and flexible + ResultSchema.extend({ + action: z.enum(["decline", "cancel"]), + content: z.optional(z.any()) + }) +]) /* Autocomplete */ /**