From 690a13aa4ffa9a24e29cc3d68eb46f9b03c23e87 Mon Sep 17 00:00:00 2001 From: Edward Bramanti Date: Sat, 25 Jul 2026 01:27:54 -0600 Subject: [PATCH 1/2] Prevent MCP tool errors from exposing internals --- .changeset/quiet-tools-smile.md | 5 + packages/effect/src/unstable/ai/McpServer.ts | 61 ++++-- .../effect/test/unstable/ai/McpServer.test.ts | 198 +++++++++++++++++- 3 files changed, 233 insertions(+), 31 deletions(-) create mode 100644 .changeset/quiet-tools-smile.md diff --git a/.changeset/quiet-tools-smile.md b/.changeset/quiet-tools-smile.md new file mode 100644 index 00000000000..d14443e6e0a --- /dev/null +++ b/.changeset/quiet-tools-smile.md @@ -0,0 +1,5 @@ +--- +"effect": patch +--- + +Prevent MCP tool failures from exposing Cause rendering, stack traces, and internal paths while preserving actionable validation messages. diff --git a/packages/effect/src/unstable/ai/McpServer.ts b/packages/effect/src/unstable/ai/McpServer.ts index 7f998cbb0bc..a91cb49b99b 100644 --- a/packages/effect/src/unstable/ai/McpServer.ts +++ b/packages/effect/src/unstable/ai/McpServer.ts @@ -39,6 +39,7 @@ import type * as RpcGroup from "../rpc/RpcGroup.ts" import * as RpcMessage from "../rpc/RpcMessage.ts" import * as RpcSerialization from "../rpc/RpcSerialization.ts" import * as RpcServer from "../rpc/RpcServer.ts" +import * as AiError from "./AiError.ts" import { CallToolResult, ClientNotificationRpcs, @@ -664,6 +665,14 @@ export const layerHttp = (options: { Layer.provide(RpcSerialization.layerJsonRpc()) ) +const INTERNAL_TOOL_ERROR_MESSAGE = "Tool execution failed due to an internal server error." + +const toolErrorResult = (message: string): CallToolResult => + new CallToolResult({ + isError: true, + content: [{ type: "text", text: message }] + }) + /** * Registers a `Toolkit` with the `McpServer`. * @@ -689,6 +698,7 @@ export const registerToolkit: >( for (const tool of Object.values(built.tools)) { const annotations = tool.annotations const toolMeta = Context.getOrUndefined(annotations, Tool.Meta) + const isDeclaredFailure = Schema.is(tool.failureSchema) const mcpTool = new McpTool({ name: tool.name, description: Tool.getDescription(tool), @@ -709,32 +719,41 @@ export const registerToolkit: >( tool: mcpTool, annotations, handle(payload) { - return built.handle(tool.name as any, payload).pipe( + return built.handle(tool.name as keyof Tools, payload).pipe( Stream.unwrap, Stream.run(Sink.last()), Effect.flatMap(Effect.fromOption), - Effect.provideContext(services as Context.Context), - Effect.matchCause({ - onFailure: (cause) => - new CallToolResult({ - isError: true, - content: [{ - type: "text", - text: Cause.pretty(cause) - }] - }), - onSuccess: (result: any) => - new CallToolResult({ - isError: false, - structuredContent: typeof result.encodedResult === "object" ? result.encodedResult : undefined, - content: [{ - type: "text", - text: JSON.stringify(result.encodedResult) - }] - }) + Effect.provideContext( + services as Context.Context> + ), + Effect.map((result) => + new CallToolResult({ + isError: false, + structuredContent: typeof result.encodedResult === "object" ? result.encodedResult : undefined, + content: [{ + type: "text", + text: JSON.stringify(result.encodedResult) + }] + }) + ), + Effect.catchIf(AiError.isAiError, (error) => + Effect.fail(error).pipe( + Effect.catchReason( + "AiError", + "ToolParameterValidationError", + (reason) => Effect.succeed(toolErrorResult(reason.message)) + ), + Effect.catchTag("AiError", () => Effect.succeed(toolErrorResult(INTERNAL_TOOL_ERROR_MESSAGE))) + )), + Effect.catch((failure) => { + const message = isDeclaredFailure(failure) && failure instanceof Error + ? failure.message + : INTERNAL_TOOL_ERROR_MESSAGE + return Effect.succeed(toolErrorResult(message)) }), + Effect.catchCause(() => Effect.succeed(toolErrorResult(INTERNAL_TOOL_ERROR_MESSAGE))), Effect.tapCause(Effect.log) - ) as any + ) } }) } diff --git a/packages/effect/test/unstable/ai/McpServer.test.ts b/packages/effect/test/unstable/ai/McpServer.test.ts index 3f3edf20ac2..6d11fd18349 100644 --- a/packages/effect/test/unstable/ai/McpServer.test.ts +++ b/packages/effect/test/unstable/ai/McpServer.test.ts @@ -1,8 +1,11 @@ -import { describe, it } from "@effect/vitest" -import { strictEqual } from "@effect/vitest/utils" -import { Effect, Layer } from "effect" +import { assert, describe, it } from "@effect/vitest" +import { assertTrue, strictEqual } from "@effect/vitest/utils" +import { Effect, Layer, Schema } from "effect" +import * as AiError from "effect/unstable/ai/AiError" import * as McpSchema from "effect/unstable/ai/McpSchema" import * as McpServer from "effect/unstable/ai/McpServer" +import * as Tool from "effect/unstable/ai/Tool" +import * as Toolkit from "effect/unstable/ai/Toolkit" import * as FetchHttpClient from "effect/unstable/http/FetchHttpClient" import * as HttpClient from "effect/unstable/http/HttpClient" import * as HttpClientRequest from "effect/unstable/http/HttpClientRequest" @@ -10,14 +13,46 @@ import * as HttpRouter from "effect/unstable/http/HttpRouter" import { RpcSerialization } from "effect/unstable/rpc" import * as RpcClient from "effect/unstable/rpc/RpcClient" -const makeTestClient = Effect.gen(function*() { - const responses: Array = [] +const OptionalStringTool = Tool.make("OptionalStringTool", { + parameters: Schema.Struct({ signature: Schema.optional(Schema.String) }), + success: Schema.String +}) - const serverLayer = McpServer.layerHttp({ - name: "TestServer", - version: "1.0.0", - path: "/mcp" - }) +const PublicFailureTool = Tool.make("PublicFailureTool", { + success: Schema.String, + failure: Schema.Error() +}) + +const InternalAiErrorTool = Tool.make("InternalAiErrorTool", { + success: Schema.String +}) + +const DefectTool = Tool.make("DefectTool", { + success: Schema.String +}) + +const TestToolkit = Toolkit.make(OptionalStringTool, PublicFailureTool, InternalAiErrorTool, DefectTool) +type TestToolkitHandlers = Toolkit.HandlersFrom> + +const testToolkitHandlers = TestToolkit.of({ + OptionalStringTool: ({ signature }) => Effect.succeed(signature ?? "omitted"), + PublicFailureTool: () => Effect.fail(new Error("Public failure")), + InternalAiErrorTool: () => Effect.fail(new AiError.RateLimitError({})), + DefectTool: () => Effect.die("private defect details") +}) + +const INTERNAL_TOOL_ERROR_MESSAGE = "Tool execution failed due to an internal server error." + +const TestServerLayer = McpServer.layerHttp({ + name: "TestServer", + version: "1.0.0", + path: "/mcp" +}) + +const makeTestClientWith = Effect.fnUntraced(function*( + serverLayer: Layer.Layer +) { + const responses: Array = [] const { handler, dispose } = HttpRouter.toWebHandler(serverLayer, { disableLogger: true }) yield* Effect.addFinalizer(() => Effect.promise(() => dispose())) @@ -48,6 +83,31 @@ const makeTestClient = Effect.gen(function*() { return { client, responses, httpClient } }) +const makeTestClient = makeTestClientWith(TestServerLayer) + +const makeToolkitTestClient = Effect.fnUntraced(function*(handlers: TestToolkitHandlers = testToolkitHandlers) { + const serverLayer = McpServer.toolkit(TestToolkit).pipe( + Layer.provideMerge(TestToolkit.toLayer(handlers)), + Layer.provide(TestServerLayer) + ) + const { client } = yield* makeTestClientWith(serverLayer) + yield* client.initialize({ + protocolVersion: "9999-01-01", + capabilities: {}, + clientInfo: { + name: "TestClient", + version: "1.0.0" + } + }) + return client +}) + +const toolResultText = (result: McpSchema.CallToolResult): string => { + const content = result.content[0] + assertTrue(content?.type === "text", "Expected text tool-result content") + return content.text +} + describe("McpServer", () => { it.effect("replays MCP session and negotiated protocol headers after initialize", () => Effect.gen(function*() { @@ -79,4 +139,122 @@ describe("McpServer", () => { strictEqual(response.status, 404) })) + + describe("registerToolkit", () => { + it.effect("returns concise parameter-validation errors without invoking the handler", () => + Effect.gen(function*() { + let handlerInvoked = false + const client = yield* makeToolkitTestClient(TestToolkit.of({ + ...testToolkitHandlers, + OptionalStringTool: ({ signature }) => { + handlerInvoked = true + return Effect.succeed(signature ?? "omitted") + } + })) + + const result = yield* client["tools/call"]({ + name: "OptionalStringTool", + arguments: { signature: null } + }) + + assert.isFalse(handlerInvoked) + assert.strictEqual(result.isError, true) + + const text = toolResultText(result) + assert.match(text, /Invalid parameters for tool 'OptionalStringTool'/) + assert.match(text, /Expected string \| undefined, got null/) + assert.match(text, /at \["signature"\]/) + assert.isFalse(/effect\/ai\//.test(text)) + assert.isFalse(/\[cause\]/.test(text)) + assert.isFalse(/\n {4}at /.test(text)) + assert.isFalse(/(?:file:\/\/)?\/(?:[^/\s]+\/)+[^/\s]+\.ts:\d+:\d+/.test(text)) + })) + + it.effect("preserves successful results when optional parameters are omitted", () => + Effect.gen(function*() { + let handlerInvoked = false + const client = yield* makeToolkitTestClient(TestToolkit.of({ + ...testToolkitHandlers, + OptionalStringTool: ({ signature }) => { + handlerInvoked = true + return Effect.succeed(signature ?? "omitted") + } + })) + + const result = yield* client["tools/call"]({ + name: "OptionalStringTool", + arguments: {} + }) + + assert.isTrue(handlerInvoked) + assert.deepStrictEqual( + result, + new McpSchema.CallToolResult({ + isError: false, + content: [{ type: "text", text: JSON.stringify("omitted") }] + }) + ) + })) + + it.effect("returns schema-validated messages for declared handler failures", () => + Effect.gen(function*() { + const client = yield* makeToolkitTestClient() + + const result = yield* client["tools/call"]({ + name: "PublicFailureTool", + arguments: {} + }) + + assert.strictEqual(result.isError, true) + const text = toolResultText(result) + assert.strictEqual(text, "Public failure") + assert.isFalse(/\n {4}at /.test(text)) + })) + + it.effect("returns a generic message for non-validation AiError failures", () => + Effect.gen(function*() { + const client = yield* makeToolkitTestClient() + + const result = yield* client["tools/call"]({ + name: "InternalAiErrorTool", + arguments: {} + }) + + assert.strictEqual(result.isError, true) + const text = toolResultText(result) + assert.strictEqual(text, INTERNAL_TOOL_ERROR_MESSAGE) + assert.isFalse(/RateLimitError/.test(text)) + assert.isFalse(/\n {4}at /.test(text)) + })) + + it.effect("returns a generic message for handler defects", () => + Effect.gen(function*() { + const client = yield* makeToolkitTestClient() + + const result = yield* client["tools/call"]({ + name: "DefectTool", + arguments: {} + }) + + assert.strictEqual(result.isError, true) + const text = toolResultText(result) + assert.strictEqual(text, INTERNAL_TOOL_ERROR_MESSAGE) + assert.isFalse(/private defect details/.test(text)) + assert.isFalse(/\n {4}at /.test(text)) + })) + + it.effect("keeps unknown tools as protocol errors", () => + Effect.gen(function*() { + const client = yield* makeToolkitTestClient() + + const error = yield* client["tools/call"]({ + name: "UnknownTool", + arguments: {} + }).pipe(Effect.flip) + + assert.instanceOf(error, McpSchema.InvalidParams) + assert.strictEqual(error.code, McpSchema.INVALID_PARAMS_ERROR_CODE) + assert.strictEqual(error.message, "Tool 'UnknownTool' not found") + })) + }) }) From b86a941ade2d343f9d6be7df7cf200a65870bce6 Mon Sep 17 00:00:00 2001 From: Maxwell Brown Date: Sat, 25 Jul 2026 07:58:51 -0400 Subject: [PATCH 2/2] cleanup tests and error handling --- packages/effect/src/unstable/ai/McpServer.ts | 32 +++++++++---------- .../effect/test/unstable/ai/McpServer.test.ts | 18 +++++++---- 2 files changed, 27 insertions(+), 23 deletions(-) diff --git a/packages/effect/src/unstable/ai/McpServer.ts b/packages/effect/src/unstable/ai/McpServer.ts index a91cb49b99b..f602c0227df 100644 --- a/packages/effect/src/unstable/ai/McpServer.ts +++ b/packages/effect/src/unstable/ai/McpServer.ts @@ -736,23 +736,23 @@ export const registerToolkit: >( }] }) ), - Effect.catchIf(AiError.isAiError, (error) => - Effect.fail(error).pipe( - Effect.catchReason( - "AiError", - "ToolParameterValidationError", - (reason) => Effect.succeed(toolErrorResult(reason.message)) - ), - Effect.catchTag("AiError", () => Effect.succeed(toolErrorResult(INTERNAL_TOOL_ERROR_MESSAGE))) - )), - Effect.catch((failure) => { - const message = isDeclaredFailure(failure) && failure instanceof Error - ? failure.message - : INTERNAL_TOOL_ERROR_MESSAGE - return Effect.succeed(toolErrorResult(message)) + Effect.tapCause(Effect.logError), + Effect.catch((error) => { + if (AiError.isAiError(error)) { + const reason = (error as AiError.AiError).reason + return reason._tag === "ToolParameterValidationError" + ? Effect.succeed(toolErrorResult(reason.message)) + : Effect.succeed(toolErrorResult(INTERNAL_TOOL_ERROR_MESSAGE)) + } + if (isDeclaredFailure(error)) { + const message = error instanceof Error + ? error.message + : INTERNAL_TOOL_ERROR_MESSAGE + return Effect.succeed(toolErrorResult(message)) + } + return Effect.succeed(toolErrorResult(INTERNAL_TOOL_ERROR_MESSAGE)) }), - Effect.catchCause(() => Effect.succeed(toolErrorResult(INTERNAL_TOOL_ERROR_MESSAGE))), - Effect.tapCause(Effect.log) + Effect.catchDefect(() => Effect.succeed(toolErrorResult(INTERNAL_TOOL_ERROR_MESSAGE))) ) } }) diff --git a/packages/effect/test/unstable/ai/McpServer.test.ts b/packages/effect/test/unstable/ai/McpServer.test.ts index 6d11fd18349..a900cbae215 100644 --- a/packages/effect/test/unstable/ai/McpServer.test.ts +++ b/packages/effect/test/unstable/ai/McpServer.test.ts @@ -1,6 +1,11 @@ import { assert, describe, it } from "@effect/vitest" import { assertTrue, strictEqual } from "@effect/vitest/utils" -import { Effect, Layer, Schema } from "effect" +import * as Effect from "effect/Effect" +import { constVoid } from "effect/Function" +import * as Layer from "effect/Layer" +import * as Logger from "effect/Logger" +import * as References from "effect/References" +import * as Schema from "effect/Schema" import * as AiError from "effect/unstable/ai/AiError" import * as McpSchema from "effect/unstable/ai/McpSchema" import * as McpServer from "effect/unstable/ai/McpServer" @@ -43,11 +48,15 @@ const testToolkitHandlers = TestToolkit.of({ const INTERNAL_TOOL_ERROR_MESSAGE = "Tool execution failed due to an internal server error." +const noopLogger = Logger.make(constVoid) + const TestServerLayer = McpServer.layerHttp({ name: "TestServer", version: "1.0.0", path: "/mcp" -}) +}).pipe( + Layer.provideMerge(Layer.succeed(References.CurrentLoggers, new Set([noopLogger]))) +) const makeTestClientWith = Effect.fnUntraced(function*( serverLayer: Layer.Layer @@ -208,7 +217,6 @@ describe("McpServer", () => { assert.strictEqual(result.isError, true) const text = toolResultText(result) assert.strictEqual(text, "Public failure") - assert.isFalse(/\n {4}at /.test(text)) })) it.effect("returns a generic message for non-validation AiError failures", () => @@ -223,8 +231,6 @@ describe("McpServer", () => { assert.strictEqual(result.isError, true) const text = toolResultText(result) assert.strictEqual(text, INTERNAL_TOOL_ERROR_MESSAGE) - assert.isFalse(/RateLimitError/.test(text)) - assert.isFalse(/\n {4}at /.test(text)) })) it.effect("returns a generic message for handler defects", () => @@ -239,8 +245,6 @@ describe("McpServer", () => { assert.strictEqual(result.isError, true) const text = toolResultText(result) assert.strictEqual(text, INTERNAL_TOOL_ERROR_MESSAGE) - assert.isFalse(/private defect details/.test(text)) - assert.isFalse(/\n {4}at /.test(text)) })) it.effect("keeps unknown tools as protocol errors", () =>