diff --git a/packages/core/src/tool/mcp.ts b/packages/core/src/tool/mcp.ts index 7867fc6571..23d11d06a0 100644 --- a/packages/core/src/tool/mcp.ts +++ b/packages/core/src/tool/mcp.ts @@ -6,6 +6,7 @@ import { Effect, Exit, type JsonSchema, Layer, Scope, Semaphore, Stream } from " import { makeLocationNode } from "../effect/app-node" import { EventV2 } from "../event" import { MCP } from "../mcp" +import { PermissionV2 } from "../permission" import { Tool } from "./tool" import { Tools } from "./tools" import { ToolRegistry } from "./registry" @@ -21,6 +22,7 @@ export const layer = Layer.effectDiscard( const mcp = yield* MCP.Service const tools = yield* Tools.Service const events = yield* EventV2.Service + const permission = yield* PermissionV2.Service const scope = yield* Scope.Scope const lock = Semaphore.makeUnsafe(1) let current: Scope.Closeable | undefined @@ -41,8 +43,21 @@ export const layer = Layer.effectDiscard( properties: schema.properties ?? {}, additionalProperties: false, }, - execute: (input) => + execute: (input, context) => Effect.gen(function* () { + yield* permission.assert({ + action: name(tool.server, tool.name), + resources: ["*"], + save: ["*"], + metadata: {}, + sessionID: context.sessionID, + agent: context.agent, + source: { + type: "tool", + messageID: context.assistantMessageID, + callID: context.toolCallID, + }, + }) const result = yield* mcp .callTool({ server: tool.server, @@ -72,7 +87,13 @@ export const layer = Layer.effectDiscard( : { type: "file" as const, data: part.data, mime: part.mimeType }, ), } - }), + }).pipe( + Effect.mapError((error) => + error instanceof ToolFailure + ? error + : new ToolFailure({ message: `Unable to execute ${name(tool.server, tool.name)}` }), + ), + ), }) groups.set(tool.server, group) } @@ -96,5 +117,5 @@ export const layer = Layer.effectDiscard( export const node = makeLocationNode({ name: "mcp-tools", layer, - deps: [ToolRegistry.toolsNode, MCP.node, EventV2.node], + deps: [ToolRegistry.toolsNode, MCP.node, EventV2.node, PermissionV2.node], }) diff --git a/packages/core/test/mcp.test.ts b/packages/core/test/mcp.test.ts index 7c62032784..f52b223dce 100644 --- a/packages/core/test/mcp.test.ts +++ b/packages/core/test/mcp.test.ts @@ -1,7 +1,61 @@ import { describe, expect, test } from "bun:test" +import { AppNodeBuilder } from "@opencode-ai/core/effect/app-node-builder" +import { LayerNode } from "@opencode-ai/core/effect/layer-node" +import { EventV2 } from "@opencode-ai/core/event" import { MCP } from "@opencode-ai/core/mcp/index" import { MCPClient } from "@opencode-ai/core/mcp/client" +import { PermissionV2 } from "@opencode-ai/core/permission" +import { SessionV2 } from "@opencode-ai/core/session" import { McpTool } from "@opencode-ai/core/tool/mcp" +import { ToolRegistry } from "@opencode-ai/core/tool/registry" +import { ToolOutputStore } from "@opencode-ai/core/tool-output-store" +import { Deferred, Effect, Fiber, Layer, Stream } from "effect" +import { testEffect } from "./lib/effect" +import { settleTool, toolIdentity, waitForTool } from "./lib/tool" + +let assertion: Deferred.Deferred | undefined +let decision: Effect.Effect = Effect.void +let calls = 0 + +const mcp = Layer.mock(MCP.Service, { + tools: () => + Effect.succeed([ + new MCP.Tool({ + server: MCP.ServerName.make("demo"), + name: "search", + description: "Search", + inputSchema: { type: "object", properties: {} }, + }), + ]), + callTool: (input) => + Effect.sync(() => { + calls += 1 + return new MCP.ToolResult({ + server: MCP.ServerName.make(input.server), + tool: input.name, + isError: false, + structured: { ok: true }, + content: [], + }) + }), +}) +const permissions = Layer.mock(PermissionV2.Service, { + assert: (input) => + Effect.gen(function* () { + if (!assertion) return yield* Effect.die("Permission test is not initialized") + yield* Deferred.succeed(assertion, input) + yield* decision + }), +}) +const events = Layer.mock(EventV2.Service, { subscribe: () => Stream.never }) +const it = testEffect( + AppNodeBuilder.build(LayerNode.group([ToolRegistry.node, ToolRegistry.toolsNode, McpTool.node]), [ + [MCP.node, mcp], + [PermissionV2.node, permissions], + [EventV2.node, events], + [ToolOutputStore.node, ToolOutputStore.nodeWithoutConfig], + ]), +) describe("MCP errors", () => { test("expose useful messages", () => { @@ -17,3 +71,57 @@ describe("MCP errors", () => { test("MCP tool names match V1 sanitization", () => { expect(McpTool.name("context 7", "resolve.library/id")).toBe("context_7_resolve_library_id") }) + +it.effect("waits for permission before calling an MCP tool", () => + Effect.gen(function* () { + calls = 0 + assertion = yield* Deferred.make() + const permission = yield* Deferred.make() + decision = Deferred.await(permission) + const registry = yield* ToolRegistry.Service + yield* waitForTool(registry, "demo_search") + + const fiber = yield* settleTool(registry, { + sessionID: SessionV2.ID.make("ses_mcp_permission"), + ...toolIdentity, + call: { type: "tool-call", id: "call_mcp_permission", name: "demo_search", input: {} }, + }).pipe(Effect.forkScoped) + expect(yield* Deferred.await(assertion)).toEqual({ + action: "demo_search", + resources: ["*"], + save: ["*"], + metadata: {}, + sessionID: SessionV2.ID.make("ses_mcp_permission"), + agent: toolIdentity.agent, + source: { + type: "tool", + messageID: toolIdentity.assistantMessageID, + callID: "call_mcp_permission", + }, + }) + expect(calls).toBe(0) + + yield* Deferred.succeed(permission, undefined) + yield* Fiber.join(fiber) + expect(calls).toBe(1) + }), +) + +it.effect("does not call MCP when permission is rejected", () => + Effect.gen(function* () { + calls = 0 + assertion = yield* Deferred.make() + decision = Effect.fail(new PermissionV2.RejectedError()) + const registry = yield* ToolRegistry.Service + yield* waitForTool(registry, "demo_search") + + expect( + yield* settleTool(registry, { + sessionID: SessionV2.ID.make("ses_mcp_rejected"), + ...toolIdentity, + call: { type: "tool-call", id: "call_mcp_rejected", name: "demo_search", input: {} }, + }), + ).toEqual({ result: { type: "error", value: "Unable to execute demo_search" } }) + expect(calls).toBe(0) + }), +)