From 3fd1d4da7a58aa41e15ba27eeef892a0bbd317b9 Mon Sep 17 00:00:00 2001 From: Aiden Cline Date: Tue, 9 Jun 2026 13:45:07 -0500 Subject: [PATCH 1/3] refactor(mcp): simplify service helpers --- packages/opencode/src/mcp/index.ts | 139 ++++++++++------------------- 1 file changed, 45 insertions(+), 94 deletions(-) diff --git a/packages/opencode/src/mcp/index.ts b/packages/opencode/src/mcp/index.ts index 9a5a9e999391..c8764a6b5b89 100644 --- a/packages/opencode/src/mcp/index.ts +++ b/packages/opencode/src/mcp/index.ts @@ -114,7 +114,7 @@ function isMcpConfigured(entry: McpEntry): entry is ConfigMCPV1.Info { const sanitize = (s: string) => s.replace(/[^a-zA-Z0-9_-]/g, "_") const MAX_LIST_PAGES = 1_000 -function remoteURL(key: string, value: string) { +function remoteURL(value: string) { if (URL.canParse(value)) return new URL(value) } @@ -196,25 +196,15 @@ function convertMcpTool(mcpTool: MCPToolDef, client: MCPClient, timeout?: number } function defs(client: MCPClient, timeout?: number) { - return listTools(client, timeout ?? DEFAULT_TIMEOUT).pipe( - Effect.catch((err) => { - return Effect.succeed(undefined) - }), - ) + return listTools(client, timeout ?? DEFAULT_TIMEOUT).pipe(Effect.catch(() => Effect.void)) } function fetchFromClient( clientName: string, client: Client, listFn: (c: Client) => Promise, - label: string, ) { - return Effect.tryPromise({ - try: () => listFn(client), - catch: (e: any) => { - return e - }, - }).pipe( + return Effect.tryPromise(() => listFn(client)).pipe( Effect.map((items) => { const out: Record = {} const sanitizedClient = sanitize(clientName) @@ -316,7 +306,7 @@ export const layer = Layer.effect( ) { const oauthDisabled = mcp.oauth === false const oauthConfig = typeof mcp.oauth === "object" ? mcp.oauth : undefined - const url = remoteURL(key, mcp.url) + const url = remoteURL(mcp.url) if (!url) { return { client: undefined as MCPClient | undefined, @@ -400,12 +390,10 @@ export const layer = Layer.effect( } lastStatus = { status: "failed" as const, error: lastError.message } - return Effect.succeed(undefined) + return Effect.void }), ) - if (result) { - return { client: result.client as MCPClient | undefined, status: { status: "connected" } as Status } - } + if (result) return { client: result.client, status: { status: "connected" } as Status } // If this was an auth error, stop trying other transports if (lastStatus?.status === "needs_auth" || lastStatus?.status === "needs_client_registration") break } @@ -479,8 +467,8 @@ export const layer = Layer.effect( if (process.platform === "win32") return [] as number[] const pids: number[] = [] const queue = [pid] - while (queue.length > 0) { - const current = queue.shift()! + for (let index = 0; index < queue.length; index++) { + const current = queue[index] const handle = yield* spawner.spawn(ChildProcess.make("pgrep", ["-P", String(current)], { stdin: "ignore" })) const text = yield* Stream.mkString(Stream.decodeText(handle.stdout)) yield* handle.exitCode @@ -666,92 +654,59 @@ export const layer = Layer.effect( const config = cfg.mcp ?? {} const defaultTimeout = cfg.experimental?.mcp_timeout - const connectedClients = Object.entries(s.clients).filter( - ([clientName]) => s.status[clientName]?.status === "connected", - ) - - yield* Effect.forEach( - connectedClients, - ([clientName, client]) => - Effect.gen(function* () { - const mcpConfig = config[clientName] - const entry = mcpConfig && isMcpConfigured(mcpConfig) ? mcpConfig : s.config[clientName] - - const listed = s.defs[clientName] - if (!listed) { - return - } + for (const [clientName, client] of Object.entries(s.clients)) { + if (s.status[clientName]?.status !== "connected") continue + const mcpConfig = config[clientName] + const entry = mcpConfig && isMcpConfigured(mcpConfig) ? mcpConfig : s.config[clientName] + const listed = s.defs[clientName] + if (!listed) continue - const timeout = entry?.timeout ?? defaultTimeout - for (const mcpTool of listed) { - result[sanitize(clientName) + "_" + sanitize(mcpTool.name)] = convertMcpTool(mcpTool, client, timeout) - } - }), - { concurrency: "unbounded" }, - ) + const timeout = entry?.timeout ?? defaultTimeout + for (const mcpTool of listed) { + result[sanitize(clientName) + "_" + sanitize(mcpTool.name)] = convertMcpTool(mcpTool, client, timeout) + } + } return result }) - function collectFromConnected( - s: State, - listFn: (c: Client) => Promise, - label: string, - ) { + function collectFromConnected(s: State, listFn: (c: Client) => Promise) { return Effect.forEach( Object.entries(s.clients).filter(([name]) => s.status[name]?.status === "connected"), ([clientName, client]) => - fetchFromClient(clientName, client, listFn, label).pipe(Effect.map((items) => Object.entries(items ?? {}))), + fetchFromClient(clientName, client, listFn).pipe(Effect.map((items) => Object.entries(items ?? {}))), { concurrency: "unbounded" }, ).pipe(Effect.map((results) => Object.fromEntries(results.flat()))) } const prompts = Effect.fn("MCP.prompts")(function* () { const s = yield* InstanceState.get(state) - return yield* collectFromConnected( - s, - (c) => - c.getServerCapabilities()?.prompts - ? paginate( - (cursor) => c.listPrompts(cursor === undefined ? undefined : { cursor }), - (result) => result.prompts, - ) - : Promise.resolve([]), - "prompts", + return yield* collectFromConnected(s, (c) => + c.getServerCapabilities()?.prompts + ? paginate( + (cursor) => c.listPrompts(cursor === undefined ? undefined : { cursor }), + (result) => result.prompts, + ) + : Promise.resolve([]), ) }) const resources = Effect.fn("MCP.resources")(function* () { const s = yield* InstanceState.get(state) - return yield* collectFromConnected( - s, - (c) => - c.getServerCapabilities()?.resources - ? paginate( - (cursor) => c.listResources(cursor === undefined ? undefined : { cursor }), - (result) => result.resources, - ) - : Promise.resolve([]), - "resources", + return yield* collectFromConnected(s, (c) => + c.getServerCapabilities()?.resources + ? paginate( + (cursor) => c.listResources(cursor === undefined ? undefined : { cursor }), + (result) => result.resources, + ) + : Promise.resolve([]), ) }) - const withClient = Effect.fnUntraced(function* ( - clientName: string, - fn: (client: MCPClient) => Promise, - label: string, - meta?: Record, - ) { + const withClient = Effect.fnUntraced(function* (clientName: string, fn: (client: MCPClient) => Promise) { const s = yield* InstanceState.get(state) const client = s.clients[clientName] - if (!client) { - return undefined - } - return yield* Effect.tryPromise({ - try: () => fn(client), - catch: (e: any) => { - return e - }, - }).pipe(Effect.orElseSucceed(() => undefined)) + if (!client) return undefined + return yield* Effect.tryPromise(() => fn(client)).pipe(Effect.orElseSucceed(() => undefined)) }) const getPrompt = Effect.fn("MCP.getPrompt")(function* ( @@ -759,15 +714,11 @@ export const layer = Layer.effect( name: string, args?: Record, ) { - return yield* withClient(clientName, (client) => client.getPrompt({ name, arguments: args }), "getPrompt", { - promptName: name, - }) + return yield* withClient(clientName, (client) => client.getPrompt({ name, arguments: args })) }) const readResource = Effect.fn("MCP.readResource")(function* (clientName: string, resourceUri: string) { - return yield* withClient(clientName, (client) => client.readResource({ uri: resourceUri }), "readResource", { - resourceUri, - }) + return yield* withClient(clientName, (client) => client.readResource({ uri: resourceUri })) }) const getMcpConfig = Effect.fnUntraced(function* (mcpName: string) { @@ -790,7 +741,7 @@ export const layer = Layer.effect( const mcpConfig = yield* requireMcpConfig(mcpName) if (mcpConfig.type !== "remote") throw new Error(`MCP server ${mcpName} is not a remote server`) if (mcpConfig.oauth === false) throw new Error(`MCP server ${mcpName} has OAuth explicitly disabled`) - const url = remoteURL(mcpName, mcpConfig.url) + const url = remoteURL(mcpConfig.url) if (!url) throw new Error(`Invalid MCP URL for "${mcpName}"`) // OAuth config is optional - if not provided, we'll use auto-discovery @@ -862,7 +813,7 @@ export const layer = Layer.effect( : undefined if (!client || !listed) { yield* Effect.tryPromise(() => client?.close() ?? Promise.resolve()).pipe(Effect.ignore) - return { status: "failed", error: "Failed to get tools" } as Status + return { status: "failed", error: "Failed to get tools" } satisfies Status } const s = yield* InstanceState.get(state) @@ -917,7 +868,7 @@ export const layer = Layer.effect( }).pipe(Effect.option) if (Option.isNone(result)) { - return { status: "failed", error: "OAuth completion failed" } as Status + return { status: "failed", error: "OAuth completion failed" } satisfies Status } yield* auth.clearCodeVerifier(mcpName) @@ -946,9 +897,9 @@ export const layer = Layer.effect( const getAuthStatus = Effect.fn("MCP.getAuthStatus")(function* (mcpName: string) { const entry = yield* auth.get(mcpName) - if (!entry?.tokens) return "not_authenticated" as AuthStatus + if (!entry?.tokens) return "not_authenticated" const expired = yield* auth.isTokenExpired(mcpName) - return (expired ? "expired" : "authenticated") as AuthStatus + return expired ? "expired" : "authenticated" }) return Service.of({ From d3f2ab1d816108392b3979571d33f09b19304c32 Mon Sep 17 00:00:00 2001 From: Aiden Cline Date: Tue, 9 Jun 2026 16:35:43 -0500 Subject: [PATCH 2/3] fix(mcp): preserve logged rejection errors --- packages/opencode/src/mcp/index.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/opencode/src/mcp/index.ts b/packages/opencode/src/mcp/index.ts index 088ca071dec2..6ef175e46cd9 100644 --- a/packages/opencode/src/mcp/index.ts +++ b/packages/opencode/src/mcp/index.ts @@ -206,7 +206,10 @@ function fetchFromClient( listFn: (c: Client) => Promise, label: string, ) { - return Effect.tryPromise(() => listFn(client)).pipe( + return Effect.tryPromise({ + try: () => listFn(client), + catch: (error) => (error instanceof Error ? error : new Error(String(error))), + }).pipe( Effect.tapError((error) => Effect.logWarning(`failed to get ${label}`, { clientName, @@ -735,7 +738,10 @@ export const layer = Layer.effect( yield* Effect.logWarning(`client not found for ${label}`, { clientName }) return undefined } - return yield* Effect.tryPromise(() => fn(client)).pipe( + return yield* Effect.tryPromise({ + try: () => fn(client), + catch: (error) => (error instanceof Error ? error : new Error(String(error))), + }).pipe( Effect.tapError((error) => Effect.logError(`failed to ${label}`, { clientName, From 449191efbe52b074f89a526132f7b48b8e2afa41 Mon Sep 17 00:00:00 2001 From: Aiden Cline Date: Tue, 9 Jun 2026 16:55:44 -0500 Subject: [PATCH 3/3] refactor(mcp): preserve promise rejections directly --- packages/opencode/src/mcp/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/opencode/src/mcp/index.ts b/packages/opencode/src/mcp/index.ts index 6ef175e46cd9..63a91ffe4da1 100644 --- a/packages/opencode/src/mcp/index.ts +++ b/packages/opencode/src/mcp/index.ts @@ -208,7 +208,7 @@ function fetchFromClient( ) { return Effect.tryPromise({ try: () => listFn(client), - catch: (error) => (error instanceof Error ? error : new Error(String(error))), + catch: (error) => error, }).pipe( Effect.tapError((error) => Effect.logWarning(`failed to get ${label}`, { @@ -740,7 +740,7 @@ export const layer = Layer.effect( } return yield* Effect.tryPromise({ try: () => fn(client), - catch: (error) => (error instanceof Error ? error : new Error(String(error))), + catch: (error) => error, }).pipe( Effect.tapError((error) => Effect.logError(`failed to ${label}`, {