Ver código fonte

fix(mcp): honor callback port in debug (#39259)

Aiden Cline 2 semanas atrás
pai
commit
c3be6c4965

+ 1 - 0
packages/opencode/src/cli/cmd/mcp.ts

@@ -737,6 +737,7 @@ export const McpDebugCommand = effectCmd({
           clientId: oauthConfig?.clientId,
           clientSecret: oauthConfig?.clientSecret,
           scope: oauthConfig?.scope,
+          callbackPort: oauthConfig?.callbackPort,
           redirectUri: oauthConfig?.redirectUri,
         },
         {

+ 38 - 25
packages/opencode/src/mcp/catalog.ts

@@ -1,44 +1,57 @@
-import { Client, type Tool as MCPToolDef } from "@modelcontextprotocol/client"
+import { Client, type CallToolResult, type Tool as MCPToolDef } from "@modelcontextprotocol/client"
 import { dynamicTool, jsonSchema, type JSONSchema7, type Tool } from "ai"
 import { Effect } from "effect"
 
 const DEFAULT_TIMEOUT = 30_000
+
+export interface McpTool {
+  readonly def: MCPToolDef
+  readonly client: Client
+  readonly timeout?: number
+}
+
+export async function callTool(
+  tool: McpTool,
+  args: Record<string, unknown>,
+  signal?: AbortSignal,
+): Promise<CallToolResult> {
+  const result = await tool.client.callTool(
+    { name: tool.def.name, arguments: args },
+    {
+      resetTimeoutOnProgress: true,
+      signal,
+      timeout: tool.timeout,
+      // The MCP SDK only sends a progress token when this hook is present, enabling timeout resets.
+      onprogress: () => {},
+    },
+  )
+  if (result.isError)
+    throw new Error(
+      result.content
+        .flatMap((item) => (item.type === "text" ? [item.text] : []))
+        .filter((text) => text.trim())
+        .join("\n\n") || "MCP tool returned an error",
+    )
+  return result
+}
+
 export function defs(client: Client, timeout?: number) {
   return listTools(client, timeout ?? DEFAULT_TIMEOUT).pipe(Effect.catch(() => Effect.void))
 }
 
-export function convertTool(mcpTool: MCPToolDef, client: Client, timeout?: number): Tool {
+export function convertTool(tool: McpTool): Tool {
   const inputSchema: JSONSchema7 = {
-    ...(mcpTool.inputSchema as JSONSchema7),
+    ...(tool.def.inputSchema as JSONSchema7),
     type: "object",
-    properties: (mcpTool.inputSchema.properties ?? {}) as JSONSchema7["properties"],
+    properties: (tool.def.inputSchema.properties ?? {}) as JSONSchema7["properties"],
     additionalProperties: false,
   }
 
   return dynamicTool({
-    description: mcpTool.description ?? "",
+    description: tool.def.description ?? "",
     inputSchema: jsonSchema(inputSchema),
     execute: async (args: unknown, options) => {
-      const result = await client.callTool(
-        {
-          name: mcpTool.name,
-          arguments: (args || {}) as Record<string, unknown>,
-        },
-        {
-          resetTimeoutOnProgress: true,
-          signal: options.abortSignal,
-          timeout,
-          // The MCP SDK only sends a progress token when this hook is present, enabling timeout resets.
-          onprogress: () => {},
-        },
-      )
-      if (result.isError)
-        throw new Error(
-          result.content
-            .flatMap((item) => (item.type === "text" ? [item.text] : []))
-            .filter((text) => text.trim())
-            .join("\n\n") || "MCP tool returned an error",
-        )
+      const result = await callTool(tool, (args || {}) as Record<string, unknown>, options.abortSignal)
       if (result.content.length > 0 || result.structuredContent === undefined || result.structuredContent === null)
         return result
       return {

+ 1 - 6
packages/opencode/src/mcp/index.ts

@@ -162,12 +162,7 @@ export interface ServerInstructions {
 }
 
 /** An MCP tool in its native shape; consumers adapt it to their own tool format. */
-export interface McpTool {
-  /** Shared cached definition; consumers must copy rather than mutate it. */
-  readonly def: MCPToolDef
-  readonly client: MCPClient
-  readonly timeout?: number
-}
+export type McpTool = McpCatalog.McpTool
 
 export interface Interface {
   readonly status: () => Effect.Effect<Record<string, Status>>

+ 1 - 1
packages/opencode/src/session/tools.ts

@@ -388,7 +388,7 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: {
   if (flags.experimentalCodeMode) return tools
 
   for (const [key, entry] of Object.entries(yield* mcp.tools())) {
-    const item = McpCatalog.convertTool(entry.def, entry.client, entry.timeout)
+    const item = McpCatalog.convertTool(entry)
     const execute = item.execute
     if (!execute) continue
 

+ 1 - 21
packages/opencode/src/tool/code-mode.ts

@@ -145,27 +145,7 @@ const invokeChildTool = Effect.fn("CodeMode.invokeChildTool")(function* (input:
   )
   const result: CallToolResult = yield* Effect.gen(function* () {
     yield* input.ctx.ask({ permission: input.entry.key, metadata: {}, patterns: ["*"], always: ["*"] })
-    // Deliberately mirrors McpCatalog.convertTool's transport call so the MCP service stays free of tool-loop concerns.
-    return yield* Effect.promise(async () => {
-      const raw = await input.entry.tool.client.callTool(
-        { name: input.entry.tool.def.name, arguments: input.args },
-        {
-          resetTimeoutOnProgress: true,
-          signal: input.ctx.abort,
-          timeout: input.entry.tool.timeout,
-          // The MCP SDK only sends a progress token when this hook is present, enabling timeout resets.
-          onprogress: () => {},
-        },
-      )
-      if (raw.isError)
-        throw new Error(
-          raw.content
-            .flatMap((item) => (item.type === "text" ? [item.text] : []))
-            .filter((text) => text.trim())
-            .join("\n\n") || "MCP tool returned an error",
-        )
-      return raw
-    })
+    return yield* Effect.promise(() => McpCatalog.callTool(input.entry.tool, input.args, input.ctx.abort))
   }).pipe(
     Effect.withSpan("Tool.execute", {
       attributes: {

+ 42 - 2
packages/opencode/test/mcp/catalog.test.ts

@@ -28,7 +28,10 @@ describe("McpCatalog.convertTool", () => {
   test("preserves content when structuredContent is also present", async () => {
     const content = [{ type: "image" as const, mimeType: "image/png", data: "AAAA" }]
     const structuredContent = { image: { mimeType: "image/png", data: "AAAA" } }
-    const converted = McpCatalog.convertTool(mcpTool(), clientReturning({ content, structuredContent }))
+    const converted = McpCatalog.convertTool({
+      def: mcpTool(),
+      client: clientReturning({ content, structuredContent }),
+    })
 
     const output = await converted.execute?.({}, options)
 
@@ -37,7 +40,10 @@ describe("McpCatalog.convertTool", () => {
 
   test("falls back to structuredContent only when content is absent", async () => {
     const structuredContent = { results: [{ title: "one" }] }
-    const converted = McpCatalog.convertTool(mcpTool(), clientReturning({ content: [], structuredContent }))
+    const converted = McpCatalog.convertTool({
+      def: mcpTool(),
+      client: clientReturning({ content: [], structuredContent }),
+    })
 
     const output = await converted.execute?.({}, options)
 
@@ -48,6 +54,40 @@ describe("McpCatalog.convertTool", () => {
   })
 })
 
+describe("McpCatalog.callTool", () => {
+  test("forwards the request options", async () => {
+    const controller = new AbortController()
+    let request: unknown
+    let options: unknown
+    const client = {
+      callTool: async (input: unknown, config: unknown) => {
+        request = input
+        options = config
+        return { content: [] }
+      },
+    } as unknown as Client
+
+    await McpCatalog.callTool({ def: mcpTool(), client, timeout: 123 }, { value: true }, controller.signal)
+
+    expect(request).toEqual({ name: "screenshot", arguments: { value: true } })
+    expect(options).toMatchObject({ resetTimeoutOnProgress: true, signal: controller.signal, timeout: 123 })
+    expect(typeof (options as { onprogress?: unknown }).onprogress).toBe("function")
+  })
+
+  test("throws text returned by an MCP tool error", async () => {
+    const client = clientReturning({
+      isError: true,
+      content: [
+        { type: "image", data: "AAAA", mimeType: "image/png" },
+        { type: "text", text: "first" },
+        { type: "text", text: "second" },
+      ],
+    })
+
+    await expect(McpCatalog.callTool({ def: mcpTool(), client }, {})).rejects.toThrow("first\n\nsecond")
+  })
+})
+
 test("preserves output schema validation across paginated tool discovery", async () => {
   const server = new Server({ name: "pagination", version: "1.0.0" }, { capabilities: { tools: {} } })
   server.setRequestHandler("tools/list", ({ params }) =>