refactor(core): simplify mcp tool wiring

2026-07-03 19:08:22 -05:00
parent 319f14952f
commit 0da23bb4f0
5 changed files with 140 additions and 255 deletions
@@ -5,7 +5,6 @@ import { AgentV2 } from "../agent"
 import { makeLocationNode } from "../effect/app-node"
 import { PermissionV2 } from "../permission"
 import { SystemContext } from "../system-context/index"
-import { McpTool } from "../tool/mcp"
 import { MCP } from "./index"

 const Summary = Schema.Struct({
@@ -75,8 +74,7 @@ export const layer = Layer.effect(
              owned.length === 0 ||
              owned.some(
                (tool) =>
-                  PermissionV2.evaluate(McpTool.permissionAction(tool.server, tool.name), "*", agent.permissions)
-                    .effect !== "deny",
+                  PermissionV2.evaluate(`mcp:${tool.server}:${tool.name}`, "*", agent.permissions).effect !== "deny",
              )
            )
          })
@@ -1,22 +1,11 @@
 export * as ExecuteTool from "./execute"

-import {
-  CodeMode,
-  Tool,
-  toolError,
-  type ExecuteResult,
-  type JsonSchema,
-  type ToolCallEnded,
-  type ToolCallStarted,
-  type ToolDefinition,
-} from "@opencode-ai/codemode"
+import { CodeMode, Tool, toolError, type ToolDefinition } from "@opencode-ai/codemode"
 import { Effect, Schema } from "effect"
 import { MCP } from "../mcp"
 import { PermissionV2 } from "../permission"
 import { make, type Context } from "./tool"

-const LIMITS = { timeoutMs: 5 * 60_000, maxToolCalls: 100 } as const
-
 export const Input = Schema.Struct({
  code: Schema.String.annotate({ description: "Code to execute using the available MCP tools" }),
 })
@@ -39,22 +28,9 @@ export const Output = Schema.Struct({
  attachments: Schema.Array(Attachment),
 })

-const Structured = Schema.Struct({
-  output: Output.fields.output,
-  toolCalls: Output.fields.toolCalls,
-  error: Output.fields.error,
-})
-
 type ExecuteCall = typeof Call.Type

-export interface Item {
-  readonly action: string
-  readonly tool: MCP.Tool
-  readonly namespace?: string
-  readonly member?: string
-}
-
-const create = Effect.fn("ExecuteTool.make")(function* (items: ReadonlyArray<Item>) {
+const create = Effect.fn("ExecuteTool.make")(function* (items: ReadonlyArray<MCP.Tool>) {
  const mcp = yield* MCP.Service
  const permission = yield* PermissionV2.Service

@@ -62,23 +38,26 @@ const create = Effect.fn("ExecuteTool.make")(function* (items: ReadonlyArray<Ite
    const tools: Record<string, Record<string, ToolDefinition>> = Object.create(null)
    const names = new Map<string, string>()
    for (const item of items) {
-      const namespace = item.namespace ?? item.tool.server.toString()
-      const member = item.member ?? item.tool.name
+      const namespace = item.server.toString().replace(/[^A-Za-z0-9_-]/g, "_")
+      const member = item.name.replace(/[^A-Za-z0-9_-]/g, "_")
      tools[namespace] ??= Object.create(null)
      tools[namespace][member] = Tool.make({
-        description: item.tool.description ?? item.tool.name,
-        input: jsonSchema(item.tool.inputSchema),
+        description: item.description ?? item.name,
+        input:
+          typeof item.inputSchema === "object" && item.inputSchema !== null && !Array.isArray(item.inputSchema)
+            ? { ...item.inputSchema }
+            : { type: "object", properties: {} },
        run: (input) => {
          if (!context) return Effect.die(new Error("Execute tool context is unavailable"))
-          const args = recordInput(input)
+          const args = typeof input === "object" && input !== null && !Array.isArray(input) ? { ...input } : {}
          return permission
            .assert({
              sessionID: context.sessionID,
              agent: context.agent,
-              action: item.action,
+              action: `mcp:${item.server}:${item.name}`,
              resources: ["*"],
              save: ["*"],
-              metadata: { server: item.tool.server, tool: item.tool.name, arguments: args },
+              metadata: { server: item.server, tool: item.name, arguments: args },
              source: {
                type: "tool",
                messageID: context.assistantMessageID,
@@ -89,33 +68,43 @@ const create = Effect.fn("ExecuteTool.make")(function* (items: ReadonlyArray<Ite
              Effect.mapError((error) =>
                toolError(error instanceof PermissionV2.CorrectedError ? error.feedback : "Permission denied"),
              ),
-              Effect.flatMap(() => mcp.callTool({ server: item.tool.server, name: item.tool.name, args })),
+              Effect.flatMap(() => mcp.callTool({ server: item.server, name: item.name, args })),
              Effect.catchTags({
                "MCP.NotFoundError": (error) => Effect.fail(toolError(`MCP server "${error.server}" is not available`)),
                "MCP.ToolCallError": (error) => Effect.fail(toolError(error.message)),
              }),
              Effect.flatMap((result) => {
-                if (result.isError)
-                  return Effect.fail(toolError(errorText(result.content) || "MCP tool returned an error"))
-                for (const part of result.content) {
-                  if (part.type === "media") attachments.push({ data: part.data, mime: part.mimeType })
-                }
-                return Effect.succeed(projectResult(result))
+                const text = result.content
+                  .flatMap((part) => (part.type === "text" ? [part.text] : []))
+                  .join("\n")
+                  .trim()
+                if (result.isError) return Effect.fail(toolError(text || "MCP tool returned an error"))
+                attachments.push(
+                  ...result.content.flatMap((part) =>
+                    part.type === "media" ? [{ data: part.data, mime: part.mimeType }] : [],
+                  ),
+                )
+                if (result.structured !== undefined) return Effect.succeed(result.structured)
+                const media = result.content.filter((part) => part.type === "media").length
+                if (media === 0) return Effect.succeed(text)
+                return Effect.succeed(
+                  [text, `[${media} media attachment${media === 1 ? "" : "s"}]`].filter(Boolean).join("\n"),
+                )
              }),
            )
        },
      })
-      names.set(`${namespace}.${member}`, `${item.tool.server}.${item.tool.name}`)
+      names.set(`${namespace}.${member}`, `${item.server}.${item.name}`)
    }

    return CodeMode.make({
-      limits: LIMITS,
+      limits: { timeoutMs: 5 * 60_000, maxToolCalls: 100 },
      tools,
-      onToolCallStart: (call: ToolCallStarted) =>
+      onToolCallStart: (call) =>
        Effect.sync(() => {
          calls[call.index] = { tool: names.get(call.name) ?? call.name, status: "running", input: call.input }
        }),
-      onToolCallEnd: (call: ToolCallEnded) =>
+      onToolCallEnd: (call) =>
        Effect.sync(() => {
          calls[call.index] = {
            tool: names.get(call.name) ?? call.name,
@@ -130,7 +119,11 @@ const create = Effect.fn("ExecuteTool.make")(function* (items: ReadonlyArray<Ite
    description: createRuntime([], []).instructions(),
    input: Input,
    output: Output,
-    structured: Structured,
+    structured: Schema.Struct({
+      output: Output.fields.output,
+      toolCalls: Output.fields.toolCalls,
+      error: Output.fields.error,
+    }),
    toStructuredOutput: ({ output }) => ({
      output: output.output,
      toolCalls: output.toolCalls,
@@ -145,8 +138,13 @@ const create = Effect.fn("ExecuteTool.make")(function* (items: ReadonlyArray<Ite
        const calls: ExecuteCall[] = []
        const attachments: Array<typeof Attachment.Type> = []
        const result = yield* createRuntime(calls, attachments, context).execute(input.code)
+        const logs = result.logs?.length ? `\n\nLogs:\n${result.logs.join("\n")}` : ""
        return {
-          output: formatResult(result),
+          output: !result.ok
+            ? `${result.error.message}${logs}`
+            : typeof result.value === "string"
+              ? result.value + logs
+              : `${JSON.stringify(result.value, null, 2) ?? "null"}${logs}`,
          toolCalls: calls,
          ...(result.ok ? {} : { error: true as const }),
          attachments,
@@ -156,37 +154,3 @@ const create = Effect.fn("ExecuteTool.make")(function* (items: ReadonlyArray<Ite
 })

 export { create as make }
-
-function recordInput(input: unknown): Record<string, unknown> {
-  return isRecord(input) ? input : {}
-}
-
-function isRecord(input: unknown): input is Record<string, unknown> {
-  return typeof input === "object" && input !== null && !Array.isArray(input)
-}
-
-function jsonSchema(input: unknown): JsonSchema {
-  return isRecord(input) ? (input as JsonSchema) : { type: "object", properties: {} }
-}
-
-function errorText(content: ReadonlyArray<MCP.ToolResultContent>) {
-  return content
-    .flatMap((part) => (part.type === "text" ? [part.text] : []))
-    .join("\n")
-    .trim()
-}
-
-function projectResult(result: MCP.ToolResult) {
-  if (result.structured !== undefined) return result.structured
-  const text = errorText(result.content)
-  const media = result.content.filter((part) => part.type === "media").length
-  if (media === 0) return text
-  return [text, `[${media} media attachment${media === 1 ? "" : "s"}]`].filter(Boolean).join("\n")
-}
-
-function formatResult(result: ExecuteResult) {
-  const logs = result.logs?.length ? `\n\nLogs:\n${result.logs.join("\n")}` : ""
-  if (!result.ok) return `${result.error.message}${logs}`
-  if (typeof result.value === "string") return result.value + logs
-  return `${JSON.stringify(result.value, null, 2) ?? "null"}${logs}`
-}
@@ -1,10 +1,9 @@
 export * as McpTool from "./mcp"

-import { createHash } from "node:crypto"
 import { ToolFailure } from "@opencode-ai/llm"
 import type { PluginContext } from "@opencode-ai/plugin/v2/effect"
 import { McpEvent } from "@opencode-ai/schema/mcp-event"
-import { Effect, Exit, type JsonSchema, Scope, Semaphore, Stream } from "effect"
+import { Effect, Exit, Scope, Semaphore, Stream } from "effect"
 import { EventV2 } from "../event"
 import { Flag } from "../flag/flag"
 import { MCP } from "../mcp"
@@ -12,30 +11,6 @@ import { PermissionV2 } from "../permission"
 import { ExecuteTool } from "./execute"
 import { Tool } from "./tool"

-const MAX_NAME_LENGTH = 64
-const HASH_LENGTH = 8
-
-const sanitize = (value: string) => value.replace(/[^A-Za-z0-9_-]/g, "_")
-const hashSuffix = (raw: string) => "_" + createHash("sha1").update(raw).digest("hex").slice(0, HASH_LENGTH)
-const fit = (base: string, raw: string) => base.slice(0, MAX_NAME_LENGTH - HASH_LENGTH - 1) + hashSuffix(raw)
-
-const registrationName = (server: string, tool: string) => {
-  const joined = sanitize(server) + "_" + sanitize(tool)
-  const base = /^[A-Za-z]/.test(joined) ? joined : "mcp_" + joined
-  return base.length > MAX_NAME_LENGTH ? fit(base, `${server}\u0000${tool}`) : base
-}
-
-export const permissionAction = (server: string, tool: string) => `mcp:${server}:${tool}`
-
-const toContent = (part: MCP.ToolResultContent): Tool.Content =>
-  part.type === "text" ? { type: "text", text: part.text } : { type: "file", data: part.data, mime: part.mimeType }
-
-const errorText = (content: ReadonlyArray<MCP.ToolResultContent>) =>
-  content
-    .flatMap((part) => (part.type === "text" ? [part.text] : []))
-    .join("\n")
-    .trim()
-
 export const Plugin = {
  id: "core-mcp-tools",
  effect: Effect.fn("McpTool.Plugin")(function* (ctx: PluginContext) {
@@ -46,58 +21,88 @@ export const Plugin = {
    const lock = Semaphore.makeUnsafe(1)
    let current: Scope.Closeable | undefined

-    const direct = (item: ExecuteTool.Item) =>
-      Tool.make({
-        description: item.tool.description ?? "",
-        jsonSchema: jsonSchema(item.tool.inputSchema),
-        execute: (input, context) =>
-          Effect.gen(function* () {
-            const args = recordInput(input)
-            yield* permission
-              .assert({
-                sessionID: context.sessionID,
-                agent: context.agent,
-                action: item.action,
-                resources: ["*"],
-                save: ["*"],
-                metadata: { server: item.tool.server, tool: item.tool.name, arguments: args },
-                source: {
-                  type: "tool",
-                  messageID: context.assistantMessageID,
-                  callID: context.toolCallID,
-                },
-              })
-              .pipe(
-                Effect.mapError(
-                  (error) =>
-                    new ToolFailure({
-                      message: error instanceof PermissionV2.CorrectedError ? error.feedback : "Permission denied",
-                    }),
-                ),
-              )
-            const result = yield* mcp.callTool({ server: item.tool.server, name: item.tool.name, args }).pipe(
-              Effect.catchTags({
-                "MCP.NotFoundError": (error) =>
-                  new ToolFailure({ message: `MCP server "${error.server}" is not available` }),
-                "MCP.ToolCallError": (error) => new ToolFailure({ message: error.message }),
-              }),
-            )
-            if (result.isError)
-              return yield* new ToolFailure({ message: errorText(result.content) || "MCP tool returned an error" })
-            return { structured: result.structured ?? {}, content: result.content.map(toContent) }
-          }),
-      })
-
    const reconcile = lock.withPermit(
      Effect.gen(function* () {
-        const items = entries(yield* mcp.tools())
-        const record = Flag.OPENCODE_CODE_MODE
-          ? items.length === 0
-            ? {}
-            : { execute: yield* ExecuteTool.make(items) }
-          : Object.fromEntries(items.map((item) => [item.registration, direct(item)]))
+        const items = yield* mcp.tools()
        const next = yield* Scope.fork(scope)
-        yield* ctx.tool.register(record).pipe(Scope.provide(next), Effect.orDie)
+        yield* ctx.tool
+          .register(
+            Flag.OPENCODE_CODE_MODE
+              ? items.length === 0
+                ? {}
+                : { execute: yield* ExecuteTool.make(items) }
+              : Object.fromEntries(
+                  items.map((item) => [
+                    `${item.server}_${item.name}`,
+                    Tool.withPermission(
+                      Tool.make({
+                        description: item.description ?? "",
+                        jsonSchema:
+                          typeof item.inputSchema === "object" &&
+                          item.inputSchema !== null &&
+                          !Array.isArray(item.inputSchema)
+                            ? { ...item.inputSchema }
+                            : { type: "object", properties: {} },
+                        execute: (input, context) =>
+                          Effect.gen(function* () {
+                            const args =
+                              typeof input === "object" && input !== null && !Array.isArray(input) ? { ...input } : {}
+                            yield* permission
+                              .assert({
+                                sessionID: context.sessionID,
+                                agent: context.agent,
+                                action: `mcp:${item.server}:${item.name}`,
+                                resources: ["*"],
+                                save: ["*"],
+                                metadata: { server: item.server, tool: item.name, arguments: args },
+                                source: {
+                                  type: "tool",
+                                  messageID: context.assistantMessageID,
+                                  callID: context.toolCallID,
+                                },
+                              })
+                              .pipe(
+                                Effect.mapError(
+                                  (error) =>
+                                    new ToolFailure({
+                                      message:
+                                        error instanceof PermissionV2.CorrectedError
+                                          ? error.feedback
+                                          : "Permission denied",
+                                    }),
+                                ),
+                              )
+                            const result = yield* mcp.callTool({ server: item.server, name: item.name, args }).pipe(
+                              Effect.catchTags({
+                                "MCP.NotFoundError": (error) =>
+                                  new ToolFailure({ message: `MCP server "${error.server}" is not available` }),
+                                "MCP.ToolCallError": (error) => new ToolFailure({ message: error.message }),
+                              }),
+                            )
+                            if (result.isError)
+                              return yield* new ToolFailure({
+                                message:
+                                  result.content
+                                    .flatMap((part) => (part.type === "text" ? [part.text] : []))
+                                    .join("\n")
+                                    .trim() || "MCP tool returned an error",
+                              })
+                            return {
+                              structured: result.structured ?? {},
+                              content: result.content.map((part) =>
+                                part.type === "text"
+                                  ? { type: "text" as const, text: part.text }
+                                  : { type: "file" as const, data: part.data, mime: part.mimeType },
+                              ),
+                            }
+                          }),
+                      }),
+                      `mcp:${item.server}:${item.name}`,
+                    ),
+                  ]),
+                ),
+          )
+          .pipe(Scope.provide(next), Effect.orDie)
        if (current) yield* Scope.close(current, Exit.void)
        current = next
      }),
@@ -110,75 +115,3 @@ export const Plugin = {
    yield* reconcile
  }),
 }
-
-type Item = ExecuteTool.Item & { readonly registration: string }
-
-function entries(tools: ReadonlyArray<MCP.Tool>): Item[] {
-  const used = new Set<string>()
-  const namespaces = codeModeNames(
-    tools.map((tool) => tool.server.toString()),
-    true,
-  )
-  const members = new Map(
-    Array.from(new Set(tools.map((tool) => tool.server.toString()))).map((server) => [
-      server,
-      codeModeNames(
-        tools.filter((tool) => tool.server === server).map((tool) => tool.name),
-        false,
-      ),
-    ]),
-  )
-  return tools.map((tool) => {
-    const initial = registrationName(tool.server, tool.name)
-    const item = {
-      action: permissionAction(tool.server, tool.name),
-      namespace: namespaces.get(tool.server)!,
-      member: members.get(tool.server)!.get(tool.name)!,
-      tool,
-    }
-    if (!used.has(initial)) {
-      used.add(initial)
-      return { registration: initial, ...item }
-    }
-    const raw = `${tool.server}\u0000${tool.name}`
-    let collision = 0
-    let registration = fit(initial, raw)
-    while (used.has(registration)) registration = fit(initial, `${raw}\u0000${++collision}`)
-    used.add(registration)
-    return { registration, ...item }
-  })
-}
-
-function codeModeNames(values: ReadonlyArray<string>, namespace: boolean) {
-  const unique = Array.from(new Set(values))
-  const valid = (value: string) =>
-    value.length > 0 &&
-    !value.includes(".") &&
-    value !== "__proto__" &&
-    value !== "constructor" &&
-    value !== "prototype" &&
-    (!namespace || value !== "$codemode")
-  const safe = unique.filter(valid)
-  const used = new Set(safe)
-  const result = new Map(safe.map((value) => [value, value]))
-  for (const value of unique.filter((value) => !valid(value))) {
-    let collision = 0
-    let alias = `mcp_${sanitize(value).slice(0, 40)}${hashSuffix(value)}`
-    while (used.has(alias)) alias = `mcp_${sanitize(value).slice(0, 40)}${hashSuffix(`${value}\u0000${++collision}`)}`
-    used.add(alias)
-    result.set(value, alias)
-  }
-  return result
-}
-
-function recordInput(input: unknown): Record<string, unknown> {
-  return isRecord(input) ? input : {}
-}
-
-function jsonSchema(input: unknown): JsonSchema.JsonSchema {
-  return isRecord(input) ? input : { type: "object", properties: {} }
-}
-
-function isRecord(input: unknown): input is Record<string, unknown> {
-  return typeof input === "object" && input !== null && !Array.isArray(input)
-}
@@ -29,13 +29,13 @@ const tool = new MCP.Tool({
 })

 function make(
-  items: ReadonlyArray<ExecuteTool.Item>,
+  items: ReadonlyArray<MCP.Tool>,
  callTool: MCP.Interface["callTool"],
  assert: PermissionV2.Interface["assert"] = () => Effect.void,
 ) {
  const mcp = MCP.Service.of({
    servers: () => Effect.succeed([]),
-    tools: () => Effect.succeed(items.map((item) => item.tool)),
+    tools: () => Effect.succeed([...items]),
    callTool,
    instructions: () => Effect.succeed([]),
    prompts: () => Effect.succeed([]),
@@ -65,7 +65,7 @@ describe("execute tool", () => {
      const assertions: PermissionV2.AssertInput[] = []
      yield* registry.register({
        execute: yield* make(
-          [{ action: "mcp:context7:resolve-library-id", tool }],
+          [tool],
          (input) =>
            Effect.sync(() => {
              calls.push({ server: input.server.toString(), name: input.name, args: input.args })
@@ -164,19 +164,16 @@ describe("execute tool", () => {
      yield* registry.register({
        execute: yield* make(
          [
-            {
-              action: "mcp:github:search_issues",
-              tool: new MCP.Tool({
-                server: MCP.ServerName.make("github"),
-                name: "search_issues",
-                description: "Search issues",
-                inputSchema: {
-                  type: "object",
-                  properties: { query: { type: "string" } },
-                  required: ["query"],
-                },
-              }),
-            },
+            new MCP.Tool({
+              server: MCP.ServerName.make("github"),
+              name: "search_issues",
+              description: "Search issues",
+              inputSchema: {
+                type: "object",
+                properties: { query: { type: "string" } },
+                required: ["query"],
+              },
+            }),
          ],
          () => Effect.die("callTool should not run when permission fails"),
          () =>
@@ -160,14 +160,7 @@ describe("MCP tool plugin", () => {
        const registry = yield* ToolRegistry.Service
        yield* waitForTool(registry, "execute")
        const definition = (yield* toolDefinitions(registry))[0]
-        const path = definition.description.match(/tools\.(mcp__codemode_[a-f0-9]{8})\.(mcp_foo_bar_[a-f0-9]{8})/)
-        expect(path).not.toBeNull()
-        const namespace = path?.[1]
-        const member = path?.[2]
-        if (!namespace || !member) {
-          yield* Effect.die("MCP aliases were not advertised")
-          return
-        }
+        expect(definition.description).toContain("tools._codemode.foo_bar")

        const settlement = yield* settleTool(registry, {
          sessionID,
@@ -176,7 +169,7 @@ describe("MCP tool plugin", () => {
            type: "tool-call",
            id: "call_mcp_aliased",
            name: "execute",
-            input: { code: `return await tools.${namespace}.${member}({ query: "react" })` },
+            input: { code: 'return await tools._codemode.foo_bar({ query: "react" })' },
          },
        })