diff --git a/bun.lock b/bun.lock index 7533ce499..53ec43c7c 100644 --- a/bun.lock +++ b/bun.lock @@ -742,6 +742,7 @@ "@executor-js/sdk": "workspace:*", "@modelcontextprotocol/sdk": "^1.29.0", "effect": "catalog:", + "zod": "^4.3.6", }, "devDependencies": { "@effect/atom-react": "catalog:", @@ -754,7 +755,6 @@ "react": "catalog:", "tsup": "catalog:", "vitest": "catalog:", - "zod": "^4.3.6", }, "peerDependencies": { "@effect/atom-react": "catalog:", diff --git a/packages/core/execution/src/tool-invoker.test.ts b/packages/core/execution/src/tool-invoker.test.ts index f688b5d38..4271fcea7 100644 --- a/packages/core/execution/src/tool-invoker.test.ts +++ b/packages/core/execution/src/tool-invoker.test.ts @@ -1,6 +1,5 @@ import { describe, expect, it } from "@effect/vitest"; import { Effect, Fiber, Schema } from "effect"; -import * as ts from "typescript"; import { ElicitationResponse, @@ -10,7 +9,7 @@ import { definePlugin, tool, } from "@executor-js/sdk"; -import { makeTestConfig } from "@executor-js/sdk/testing"; +import { makeTestConfig, typeCheckOutputTypeScript } from "@executor-js/sdk/testing"; import { makeQuickJsExecutor } from "@executor-js/runtime-quickjs"; import { createExecutionEngine } from "./engine"; import { describeTool, makeExecutorToolInvoker, searchTools } from "./tool-invoker"; @@ -44,48 +43,13 @@ const typeCheckDescribedInvocation = ( described: DescribedToolContract, runtimeResult: unknown, consumerSource: string, -): readonly string[] => { - const fileName = "described-tool-contract.ts"; - const source = [ - ...Object.entries(described.typeScriptDefinitions).map(([name, definition]) => { - return `type ${name} = ${definition};`; - }), - `type ToolOutput = ${described.outputTypeScript};`, - `const invokedResult: ToolOutput = ${JSON.stringify(runtimeResult)};`, +): readonly string[] => + typeCheckOutputTypeScript(described, runtimeResult, { consumerSource, - ].join("\n"); - - const options: ts.CompilerOptions = { - module: ts.ModuleKind.ESNext, - noEmit: true, - skipLibCheck: true, - strict: true, - target: ts.ScriptTarget.ES2022, - }; - const host = ts.createCompilerHost(options); - const originalGetSourceFile = host.getSourceFile.bind(host); - const originalReadFile = host.readFile.bind(host); - const originalFileExists = host.fileExists.bind(host); - - host.getSourceFile = (candidate, languageVersion, onError, shouldCreateNewSourceFile) => { - if (candidate === fileName) { - return ts.createSourceFile(candidate, source, languageVersion, true); - } - return originalGetSourceFile(candidate, languageVersion, onError, shouldCreateNewSourceFile); - }; - host.readFile = (candidate) => (candidate === fileName ? source : originalReadFile(candidate)); - host.fileExists = (candidate) => candidate === fileName || originalFileExists(candidate); - - const program = ts.createProgram([fileName], options, host); - return ts.getPreEmitDiagnostics(program).map((diagnostic) => { - const message = ts.flattenDiagnosticMessageText(diagnostic.messageText, "\n"); - if (!diagnostic.file || diagnostic.start === undefined) { - return message; - } - const position = diagnostic.file.getLineAndCharacterOfPosition(diagnostic.start); - return `${diagnostic.file.fileName}:${position.line + 1}:${position.character + 1} ${message}`; + fileName: "described-tool-contract.ts", + typeName: "ToolOutput", + valueName: "invokedResult", }); -}; // --------------------------------------------------------------------------- // Test plugins — each one declares a namespace as a static source with N diff --git a/packages/core/sdk/src/testing.ts b/packages/core/sdk/src/testing.ts index 49c1a4771..59014db2f 100644 --- a/packages/core/sdk/src/testing.ts +++ b/packages/core/sdk/src/testing.ts @@ -32,6 +32,11 @@ export { type OAuthTestServerShape, } from "./testing/oauth-test-server"; export { createSqliteTestFumaDb, type SqliteTestFumaDb } from "./sqlite-test-db"; +export { + typeCheckOutputTypeScript, + type OutputTypeScriptContract, + type TypeCheckOutputTypeScriptOptions, +} from "./testing/tool-output-contract"; export class TestHttpServerAddressError extends Data.TaggedError("TestHttpServerAddressError")<{ readonly address: unknown; diff --git a/packages/core/sdk/src/testing/tool-output-contract.test.ts b/packages/core/sdk/src/testing/tool-output-contract.test.ts new file mode 100644 index 000000000..200bfb4c9 --- /dev/null +++ b/packages/core/sdk/src/testing/tool-output-contract.test.ts @@ -0,0 +1,52 @@ +import { describe, expect, it } from "@effect/vitest"; + +import { typeCheckOutputTypeScript } from "./tool-output-contract"; + +describe("typeCheckOutputTypeScript", () => { + it("accepts runtime output that matches the described TypeScript contract", () => { + const diagnostics = typeCheckOutputTypeScript( + { + outputTypeScript: "{ ok: true; data: ResultData }", + typeScriptDefinitions: { + Payload: "{ answer: string }", + ResultData: + '{ content: readonly { type: "text"; text: string }[]; structuredContent: Payload }', + }, + }, + { + ok: true, + data: { + content: [{ type: "text", text: "done" }], + structuredContent: { answer: "done" }, + }, + }, + { + consumerSource: + "const answer: string = invokedOutput.data.structuredContent.answer; answer;", + }, + ); + + expect(diagnostics).toEqual([]); + }); + + it("reports when the described contract omits the runtime output wrapper", () => { + const diagnostics = typeCheckOutputTypeScript( + { + outputTypeScript: "{ ok: true; data: { answer: string } }", + }, + { + ok: true, + data: { + content: [{ type: "text", text: "done" }], + structuredContent: { answer: "done" }, + }, + }, + ); + + expect(diagnostics.join("\n")).toContain("answer"); + }); + + it("reports missing output TypeScript contracts", () => { + expect(typeCheckOutputTypeScript({}, { ok: true })).toEqual(["missing outputTypeScript"]); + }); +}); diff --git a/packages/core/sdk/src/testing/tool-output-contract.ts b/packages/core/sdk/src/testing/tool-output-contract.ts new file mode 100644 index 000000000..56000e1d3 --- /dev/null +++ b/packages/core/sdk/src/testing/tool-output-contract.ts @@ -0,0 +1,66 @@ +import * as ts from "typescript"; + +export type OutputTypeScriptContract = { + readonly outputTypeScript?: string; + readonly typeScriptDefinitions?: Record; +}; + +export type TypeCheckOutputTypeScriptOptions = { + readonly consumerSource?: string; + readonly fileName?: string; + readonly typeName?: string; + readonly valueName?: string; +}; + +export const typeCheckOutputTypeScript = ( + contract: OutputTypeScriptContract | null | undefined, + runtimeOutput: unknown, + options: TypeCheckOutputTypeScriptOptions = {}, +): readonly string[] => { + if (!contract?.outputTypeScript) { + return ["missing outputTypeScript"]; + } + + const fileName = options.fileName ?? "tool-output-contract.ts"; + const typeName = options.typeName ?? "ToolOutput"; + const valueName = options.valueName ?? "invokedOutput"; + const source = [ + ...Object.entries(contract.typeScriptDefinitions ?? {}).map( + ([name, definition]) => `type ${name} = ${definition};`, + ), + `type ${typeName} = ${contract.outputTypeScript};`, + `const ${valueName}: ${typeName} = ${JSON.stringify(runtimeOutput)};`, + options.consumerSource ?? `${valueName};`, + ].join("\n"); + + const compilerOptions: ts.CompilerOptions = { + module: ts.ModuleKind.ESNext, + noEmit: true, + skipLibCheck: true, + strict: true, + target: ts.ScriptTarget.ES2022, + }; + const host = ts.createCompilerHost(compilerOptions); + const originalGetSourceFile = host.getSourceFile.bind(host); + const originalReadFile = host.readFile.bind(host); + const originalFileExists = host.fileExists.bind(host); + + host.getSourceFile = (candidate, languageVersion, onError, shouldCreateNewSourceFile) => { + if (candidate === fileName) { + return ts.createSourceFile(candidate, source, languageVersion, true); + } + return originalGetSourceFile(candidate, languageVersion, onError, shouldCreateNewSourceFile); + }; + host.readFile = (candidate) => (candidate === fileName ? source : originalReadFile(candidate)); + host.fileExists = (candidate) => candidate === fileName || originalFileExists(candidate); + + const program = ts.createProgram([fileName], compilerOptions, host); + return ts.getPreEmitDiagnostics(program).map((diagnostic) => { + const message = ts.flattenDiagnosticMessageText(diagnostic.messageText, "\n"); + if (!diagnostic.file || diagnostic.start === undefined) { + return message; + } + const position = diagnostic.file.getLineAndCharacterOfPosition(diagnostic.start); + return `${diagnostic.file.fileName}:${position.line + 1}:${position.character + 1} ${message}`; + }); +}; diff --git a/packages/plugins/mcp/package.json b/packages/plugins/mcp/package.json index f63a9d03b..84a79cacb 100644 --- a/packages/plugins/mcp/package.json +++ b/packages/plugins/mcp/package.json @@ -66,7 +66,8 @@ "@executor-js/config": "workspace:*", "@executor-js/sdk": "workspace:*", "@modelcontextprotocol/sdk": "^1.29.0", - "effect": "catalog:" + "effect": "catalog:", + "zod": "^4.3.6" }, "devDependencies": { "@effect/atom-react": "catalog:", @@ -78,8 +79,7 @@ "bun-types": "catalog:", "react": "catalog:", "tsup": "catalog:", - "vitest": "catalog:", - "zod": "^4.3.6" + "vitest": "catalog:" }, "peerDependencies": { "@effect/atom-react": "catalog:", diff --git a/packages/plugins/mcp/src/sdk/elicitation.test.ts b/packages/plugins/mcp/src/sdk/elicitation.test.ts index 34b008f4b..fbb98a4b9 100644 --- a/packages/plugins/mcp/src/sdk/elicitation.test.ts +++ b/packages/plugins/mcp/src/sdk/elicitation.test.ts @@ -1,13 +1,16 @@ import { describe, expect, it } from "@effect/vitest"; import { Effect, Schema } from "effect"; +import { CfWorkerJsonSchemaValidator } from "@modelcontextprotocol/sdk/validation/cfworker"; +import type { JsonSchemaType } from "@modelcontextprotocol/sdk/validation/types"; import { createExecutor, FormElicitation, ElicitationResponse, + isToolResult, type InvokeOptions, } from "@executor-js/sdk"; -import { makeTestConfig } from "@executor-js/sdk/testing"; +import { makeTestConfig, typeCheckOutputTypeScript } from "@executor-js/sdk/testing"; import { mcpPlugin } from "./plugin"; import { makeElicitationMcpServer, serveMcpServer } from "../testing"; @@ -16,6 +19,24 @@ const isFormElicitation = Schema.is(FormElicitation); const serveElicitationTestServer = serveMcpServer(makeElicitationMcpServer); +const schemaValidator = new CfWorkerJsonSchemaValidator({ shortcircuit: false }); + +const expectMatchesOutputSchema = (outputSchema: unknown, value: unknown): void => { + expect(outputSchema).toBeDefined(); + const result = schemaValidator.getValidator(outputSchema as JsonSchemaType)(value); + expect(result).toEqual({ + valid: true, + data: value, + errorMessage: undefined, + }); +}; + +const expectToolResultOkData = (result: unknown): unknown => { + expect(isToolResult(result)).toBe(true); + expect(result).toMatchObject({ ok: true }); + return (result as { readonly ok: true; readonly data: unknown }).data; +}; + // --------------------------------------------------------------------------- // Helper — create executor with MCP plugin pointed at test server // --------------------------------------------------------------------------- @@ -122,12 +143,84 @@ describe("MCP elicitation (end-to-end)", () => { }), ); + it.effect("registered tools without MCP outputSchema still describe CallToolResult", () => + Effect.gen(function* () { + const server = yield* serveElicitationTestServer; + const executor = yield* makeTestExecutor(server.url); + const tools = yield* executor.tools.list(); + const simpleEcho = tools.find((t) => t.name === "simple_echo")!; + const schema = yield* executor.tools.schema(simpleEcho.id); + + expect(schema?.outputSchema).toMatchObject({ + type: "object", + properties: { + content: { type: "array" }, + structuredContent: {}, + isError: { const: false }, + _meta: { type: "object" }, + }, + required: ["content"], + }); + const outputSchema = schema?.outputSchema as { + readonly properties: { + readonly content: { + readonly items: { + readonly anyOf: readonly unknown[]; + }; + }; + }; + }; + expect(outputSchema.properties.content.items.anyOf).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + properties: expect.objectContaining({ + type: { const: "text", type: "string" }, + text: { type: "string" }, + }), + required: ["type", "text"], + }), + ]), + ); + expect(schema?.outputTypeScript).toContain('type: "text"'); + expect(schema?.outputTypeScript).toContain("structuredContent?: { [k: string]: unknown; }"); + + const result = yield* executor.tools.invoke( + simpleEcho.id, + { value: "plain" }, + { onElicitation: "accept-all" }, + ); + + const data = expectToolResultOkData(result); + expectMatchesOutputSchema(schema?.outputSchema, data); + expect(typeCheckOutputTypeScript(schema, data)).toEqual([]); + }), + ); + it.effect("successful tool invocation preserves structured MCP result fields", () => Effect.gen(function* () { const server = yield* serveElicitationTestServer; const executor = yield* makeTestExecutor(server.url); const tools = yield* executor.tools.list(); const structuredEcho = tools.find((t) => t.name === "structured_echo")!; + const schema = yield* executor.tools.schema(structuredEcho.id); + + expect(schema?.outputSchema).toMatchObject({ + type: "object", + properties: { + content: { type: "array" }, + structuredContent: { + type: "object", + properties: { + value: { type: "string" }, + upper: { type: "string" }, + }, + }, + _meta: { type: "object" }, + }, + required: ["content", "structuredContent"], + }); + expect(schema?.outputTypeScript).toContain("structuredContent"); + expect(schema?.outputTypeScript).toContain("value: string"); const result = yield* executor.tools.invoke( structuredEcho.id, @@ -143,6 +236,55 @@ describe("MCP elicitation (end-to-end)", () => { _meta: { trace: "kept" }, }, }); + const data = expectToolResultOkData(result); + expectMatchesOutputSchema(schema?.outputSchema, data); + expect(typeCheckOutputTypeScript(schema, data)).toEqual([]); + }), + ); + + it.effect("refreshSource keeps MCP outputSchema nested under structuredContent", () => + Effect.gen(function* () { + const server = yield* serveElicitationTestServer; + const executor = yield* createExecutor( + makeTestConfig({ + plugins: [mcpPlugin()] as const, + }), + ); + + yield* executor.mcp.addSource({ + transport: "remote", + scope: "test-scope", + name: "test-mcp", + namespace: "schema_refresh", + endpoint: server.url, + }); + yield* executor.mcp.refreshSource("schema_refresh", "test-scope"); + + const schema = yield* executor.tools.schema("schema_refresh.structured_echo"); + expect(schema?.outputSchema).toMatchObject({ + type: "object", + properties: { + content: { type: "array" }, + structuredContent: { + type: "object", + properties: { + value: { type: "string" }, + upper: { type: "string" }, + }, + }, + }, + required: ["content", "structuredContent"], + }); + expect(schema?.outputTypeScript).toContain("structuredContent"); + expect(schema?.outputTypeScript).toContain("upper: string"); + + const result = yield* executor.tools.invoke( + "schema_refresh.structured_echo", + { value: "plain" }, + { onElicitation: "accept-all" }, + ); + const data = expectToolResultOkData(result); + expect(typeCheckOutputTypeScript(schema, data)).toEqual([]); }), ); diff --git a/packages/plugins/mcp/src/sdk/plugin.ts b/packages/plugins/mcp/src/sdk/plugin.ts index bcbf2ea41..f32aada41 100644 --- a/packages/plugins/mcp/src/sdk/plugin.ts +++ b/packages/plugins/mcp/src/sdk/plugin.ts @@ -14,6 +14,8 @@ import { import type { HttpClient } from "effect/unstable/http"; import type { OAuthClientProvider } from "@modelcontextprotocol/sdk/client/auth.js"; +import { CallToolResultSchema } from "@modelcontextprotocol/sdk/types.js"; +import * as z from "zod/v4"; import { type CredentialBindingRef, @@ -319,6 +321,31 @@ const normalizeNamespace = (config: McpSourceConfig): string => command: config.transport === "stdio" ? config.command : undefined, }); +type JsonSchemaObject = Record & { + readonly properties?: Record; +}; + +const McpCallToolResultJsonSchema = z.toJSONSchema(CallToolResultSchema) as JsonSchemaObject; + +const mcpCallToolResultOutputSchema = (structuredContentSchema?: unknown): JsonSchemaObject => { + const defaultStructuredContentSchema = + McpCallToolResultJsonSchema.properties?.structuredContent ?? {}; + + return { + ...McpCallToolResultJsonSchema, + properties: { + ...McpCallToolResultJsonSchema.properties, + structuredContent: + structuredContentSchema === undefined + ? defaultStructuredContentSchema + : structuredContentSchema, + isError: { const: false }, + }, + required: + structuredContentSchema === undefined ? ["content"] : ["content", "structuredContent"], + }; +}; + const toBinding = (entry: McpToolManifestEntry): McpToolBinding => McpToolBinding.make({ toolId: entry.toolId, @@ -1440,7 +1467,7 @@ export const mcpPlugin = definePlugin((options?: McpPluginOptions) => { name: e.toolId, description: e.description ?? `MCP tool: ${e.toolName}`, inputSchema: e.inputSchema, - outputSchema: e.outputSchema, + outputSchema: mcpCallToolResultOutputSchema(e.outputSchema), })), }); if (initialRemote && initialBindings.length > 0) { @@ -1582,7 +1609,7 @@ export const mcpPlugin = definePlugin((options?: McpPluginOptions) => { name: e.toolId, description: e.description ?? `MCP tool: ${e.toolName}`, inputSchema: e.inputSchema, - outputSchema: e.outputSchema, + outputSchema: mcpCallToolResultOutputSchema(e.outputSchema), })), }); }),