From 91a696da0b37baaeb859fd9aff5ef1e97d39ca54 Mon Sep 17 00:00:00 2001 From: Aiden Cline Date: Tue, 9 Jun 2026 13:46:50 -0500 Subject: [PATCH 1/3] refactor(mcp): tighten connection result types --- packages/opencode/src/mcp/index.ts | 67 +++++++++++++----------------- 1 file changed, 29 insertions(+), 38 deletions(-) diff --git a/packages/opencode/src/mcp/index.ts b/packages/opencode/src/mcp/index.ts index c8764a6b5b89..fb7a0bcf051c 100644 --- a/packages/opencode/src/mcp/index.ts +++ b/packages/opencode/src/mcp/index.ts @@ -217,11 +217,13 @@ function fetchFromClient( ) } -interface CreateResult { - mcpClient?: MCPClient - status: Status - defs?: MCPToolDef[] -} +type ConnectedStatus = Extract +type DisconnectedStatus = Exclude +type ConnectionFailedStatus = Exclude +type ConnectResult = { client: MCPClient; status: ConnectedStatus } | { status: ConnectionFailedStatus } +type CreateResult = + | { mcpClient: MCPClient; status: ConnectedStatus; defs: MCPToolDef[] } + | { status: DisconnectedStatus } interface AuthResult { authorizationUrl: string @@ -302,15 +304,14 @@ export const layer = Layer.effect( const connectRemote = Effect.fn("MCP.connectRemote")(function* ( key: string, - mcp: ConfigMCPV1.Info & { type: "remote" }, - ) { + mcp: ConfigMCPV1.Remote, + ): Effect.fn.Return { const oauthDisabled = mcp.oauth === false const oauthConfig = typeof mcp.oauth === "object" ? mcp.oauth : undefined const url = remoteURL(mcp.url) if (!url) { return { - client: undefined as MCPClient | undefined, - status: { status: "failed" as const, error: `Invalid MCP URL for "${key}"` }, + status: { status: "failed", error: `Invalid MCP URL for "${key}"` }, } } let authProvider: McpOAuthProvider | undefined @@ -351,7 +352,7 @@ export const layer = Layer.effect( ] const connectTimeout = mcp.timeout ?? DEFAULT_TIMEOUT - let lastStatus: Status | undefined + let lastStatus: ConnectionFailedStatus | undefined for (const { name, transport } of transports) { const result = yield* connectTransport(transport, connectTimeout).pipe( @@ -393,21 +394,19 @@ export const layer = Layer.effect( return Effect.void }), ) - if (result) return { client: result.client, status: { status: "connected" } as Status } + if (result) return { client: result.client, status: { status: "connected" } } // If this was an auth error, stop trying other transports if (lastStatus?.status === "needs_auth" || lastStatus?.status === "needs_client_registration") break } return { - client: undefined as MCPClient | undefined, - status: (lastStatus ?? { status: "failed", error: "Unknown error" }) as Status, + status: lastStatus ?? { status: "failed", error: "Unknown error" }, } }) const connectLocal = Effect.fn("MCP.connectLocal")(function* ( - key: string, - mcp: ConfigMCPV1.Info & { type: "local" }, - ) { + mcp: ConfigMCPV1.Local, + ): Effect.fn.Return { const [cmd, ...args] = mcp.command const cwd = yield* InstanceState.directory const transport = new StdioClientTransport({ @@ -424,13 +423,10 @@ export const layer = Layer.effect( const connectTimeout = mcp.timeout ?? DEFAULT_TIMEOUT return yield* connectTransport(transport, connectTimeout).pipe( - Effect.map((client): { client: MCPClient | undefined; status: Status } => ({ - client, - status: { status: "connected" }, - })), - Effect.catch((error): Effect.Effect<{ client: MCPClient | undefined; status: Status }> => { + Effect.map((client) => ({ client, status: { status: "connected" } }) satisfies ConnectResult), + Effect.catch((error) => { const msg = error instanceof Error ? error.message : String(error) - return Effect.succeed({ client: undefined, status: { status: "failed", error: msg } }) + return Effect.succeed({ status: { status: "failed", error: msg } } satisfies ConnectResult) }), ) }) @@ -440,25 +436,20 @@ export const layer = Layer.effect( return DISABLED_RESULT } - const { client: mcpClient, status } = - mcp.type === "remote" - ? yield* connectRemote(key, mcp as ConfigMCPV1.Info & { type: "remote" }) - : yield* connectLocal(key, mcp as ConfigMCPV1.Info & { type: "local" }) + const result = mcp.type === "remote" ? yield* connectRemote(key, mcp) : yield* connectLocal(mcp) - if (!mcpClient) { - if (status.status !== "connected" && status.status !== "disabled") { - yield* Effect.logWarning("server unavailable", { key, type: mcp.type, status: status.status }) - } - return { status } satisfies CreateResult + if (!("client" in result)) { + yield* Effect.logWarning("server unavailable", { key, type: mcp.type, status: result.status.status }) + return result } - const listed = mcpClient.getServerCapabilities()?.tools ? yield* defs(mcpClient, mcp.timeout) : [] + const listed = result.client.getServerCapabilities()?.tools ? yield* defs(result.client, mcp.timeout) : [] if (!listed) { - yield* Effect.tryPromise(() => mcpClient.close()).pipe(Effect.ignore) + yield* Effect.tryPromise(() => result.client.close()).pipe(Effect.ignore) return { status: { status: "failed", error: "Failed to get tools" } } satisfies CreateResult } - return { mcpClient, status, defs: listed } satisfies CreateResult + return { mcpClient: result.client, status: result.status, defs: listed } satisfies CreateResult }) const cfgSvc = yield* Config.Service @@ -529,9 +520,9 @@ export const layer = Layer.effect( if (!result) return s.status[key] = result.status - if (result.mcpClient) { + if ("mcpClient" in result) { s.clients[key] = result.mcpClient - s.defs[key] = result.defs! + s.defs[key] = result.defs watch(s, key, result.mcpClient, bridge, mcp.timeout) } }), @@ -617,13 +608,13 @@ export const layer = Layer.effect( const result = yield* create(name, mcp) s.status[name] = result.status - if (!result.mcpClient) { + if (!("mcpClient" in result)) { yield* closeClient(s, name) delete s.clients[name] return result.status } - return yield* storeClient(s, name, result.mcpClient, result.defs!, mcp.timeout) + return yield* storeClient(s, name, result.mcpClient, result.defs, mcp.timeout) }) const add = Effect.fn("MCP.add")(function* (name: string, mcp: ConfigMCPV1.Info) { From deebc338c8407358977dfa283234ed6f5d3ab8ea Mon Sep 17 00:00:00 2001 From: Aiden Cline Date: Tue, 9 Jun 2026 13:49:07 -0500 Subject: [PATCH 2/3] refactor(mcp): simplify connection result flow --- packages/opencode/src/mcp/index.ts | 55 +++++++++++++----------------- 1 file changed, 23 insertions(+), 32 deletions(-) diff --git a/packages/opencode/src/mcp/index.ts b/packages/opencode/src/mcp/index.ts index fb7a0bcf051c..baf89837e2b4 100644 --- a/packages/opencode/src/mcp/index.ts +++ b/packages/opencode/src/mcp/index.ts @@ -217,13 +217,7 @@ function fetchFromClient( ) } -type ConnectedStatus = Extract -type DisconnectedStatus = Exclude -type ConnectionFailedStatus = Exclude -type ConnectResult = { client: MCPClient; status: ConnectedStatus } | { status: ConnectionFailedStatus } -type CreateResult = - | { mcpClient: MCPClient; status: ConnectedStatus; defs: MCPToolDef[] } - | { status: DisconnectedStatus } +type UnavailableStatus = Exclude interface AuthResult { authorizationUrl: string @@ -300,18 +294,15 @@ export const layer = Layer.effect( (t, exit) => (Exit.isFailure(exit) ? Effect.tryPromise(() => t.close()).pipe(Effect.ignore) : Effect.void), ) - const DISABLED_RESULT: CreateResult = { status: { status: "disabled" } } + const DISABLED_RESULT = { status: { status: "disabled" as const } } - const connectRemote = Effect.fn("MCP.connectRemote")(function* ( - key: string, - mcp: ConfigMCPV1.Remote, - ): Effect.fn.Return { + const connectRemote = Effect.fn("MCP.connectRemote")(function* (key: string, mcp: ConfigMCPV1.Remote) { const oauthDisabled = mcp.oauth === false const oauthConfig = typeof mcp.oauth === "object" ? mcp.oauth : undefined const url = remoteURL(mcp.url) if (!url) { return { - status: { status: "failed", error: `Invalid MCP URL for "${key}"` }, + status: { status: "failed" as const, error: `Invalid MCP URL for "${key}"` }, } } let authProvider: McpOAuthProvider | undefined @@ -352,7 +343,7 @@ export const layer = Layer.effect( ] const connectTimeout = mcp.timeout ?? DEFAULT_TIMEOUT - let lastStatus: ConnectionFailedStatus | undefined + let lastStatus: UnavailableStatus | undefined for (const { name, transport } of transports) { const result = yield* connectTransport(transport, connectTimeout).pipe( @@ -394,19 +385,17 @@ export const layer = Layer.effect( return Effect.void }), ) - if (result) return { client: result.client, status: { status: "connected" } } + if (result) return { client: result.client } // If this was an auth error, stop trying other transports if (lastStatus?.status === "needs_auth" || lastStatus?.status === "needs_client_registration") break } return { - status: lastStatus ?? { status: "failed", error: "Unknown error" }, + status: lastStatus ?? { status: "failed" as const, error: "Unknown error" }, } }) - const connectLocal = Effect.fn("MCP.connectLocal")(function* ( - mcp: ConfigMCPV1.Local, - ): Effect.fn.Return { + const connectLocal = Effect.fn("MCP.connectLocal")(function* (mcp: ConfigMCPV1.Local) { const [cmd, ...args] = mcp.command const cwd = yield* InstanceState.directory const transport = new StdioClientTransport({ @@ -423,10 +412,10 @@ export const layer = Layer.effect( const connectTimeout = mcp.timeout ?? DEFAULT_TIMEOUT return yield* connectTransport(transport, connectTimeout).pipe( - Effect.map((client) => ({ client, status: { status: "connected" } }) satisfies ConnectResult), + Effect.map((client) => ({ client })), Effect.catch((error) => { const msg = error instanceof Error ? error.message : String(error) - return Effect.succeed({ status: { status: "failed", error: msg } } satisfies ConnectResult) + return Effect.succeed({ status: { status: "failed" as const, error: msg } }) }), ) }) @@ -438,7 +427,7 @@ export const layer = Layer.effect( const result = mcp.type === "remote" ? yield* connectRemote(key, mcp) : yield* connectLocal(mcp) - if (!("client" in result)) { + if ("status" in result) { yield* Effect.logWarning("server unavailable", { key, type: mcp.type, status: result.status.status }) return result } @@ -446,10 +435,10 @@ export const layer = Layer.effect( const listed = result.client.getServerCapabilities()?.tools ? yield* defs(result.client, mcp.timeout) : [] if (!listed) { yield* Effect.tryPromise(() => result.client.close()).pipe(Effect.ignore) - return { status: { status: "failed", error: "Failed to get tools" } } satisfies CreateResult + return { status: { status: "failed" as const, error: "Failed to get tools" } } } - return { mcpClient: result.client, status: result.status, defs: listed } satisfies CreateResult + return { client: result.client, defs: listed } }) const cfgSvc = yield* Config.Service @@ -519,12 +508,14 @@ export const layer = Layer.effect( const result = yield* create(key, mcp).pipe(Effect.catch(() => Effect.void)) if (!result) return - s.status[key] = result.status - if ("mcpClient" in result) { - s.clients[key] = result.mcpClient - s.defs[key] = result.defs - watch(s, key, result.mcpClient, bridge, mcp.timeout) + if ("status" in result) { + s.status[key] = result.status + return } + s.status[key] = { status: "connected" } + s.clients[key] = result.client + s.defs[key] = result.defs + watch(s, key, result.client, bridge, mcp.timeout) }), { concurrency: "unbounded" }, ) @@ -607,14 +598,14 @@ export const layer = Layer.effect( const s = yield* InstanceState.get(state) const result = yield* create(name, mcp) - s.status[name] = result.status - if (!("mcpClient" in result)) { + if ("status" in result) { + s.status[name] = result.status yield* closeClient(s, name) delete s.clients[name] return result.status } - return yield* storeClient(s, name, result.mcpClient, result.defs, mcp.timeout) + return yield* storeClient(s, name, result.client, result.defs, mcp.timeout) }) const add = Effect.fn("MCP.add")(function* (name: string, mcp: ConfigMCPV1.Info) { From 2a25bdce74b9429bac8581c286cd7ba343a876e4 Mon Sep 17 00:00:00 2001 From: Aiden Cline Date: Tue, 9 Jun 2026 13:54:05 -0500 Subject: [PATCH 3/3] refactor(mcp): use tagged connection results --- packages/opencode/src/mcp/index.ts | 71 +++++++++++++++++++----------- 1 file changed, 45 insertions(+), 26 deletions(-) diff --git a/packages/opencode/src/mcp/index.ts b/packages/opencode/src/mcp/index.ts index baf89837e2b4..c2ac9b95350c 100644 --- a/packages/opencode/src/mcp/index.ts +++ b/packages/opencode/src/mcp/index.ts @@ -10,7 +10,6 @@ import { CallToolResultSchema, ListToolsResultSchema, ToolSchema, - type Tool as MCPToolDef, ToolListChangedNotificationSchema, } from "@modelcontextprotocol/sdk/types.js" import { Config } from "@/config/config" @@ -71,6 +70,7 @@ export class NotFoundError extends Schema.TaggedErrorClass()("MCP }) {} type MCPClient = Client +type MCPToolDef = Awaited>["tools"][number] const StatusConnected = Schema.Struct({ status: Schema.Literal("connected") }).annotate({ identifier: "MCPStatusConnected", @@ -124,6 +124,10 @@ function isOutputSchemaValidationError(error: Error) { ) } +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value) +} + async function paginate( list: (cursor?: string) => Promise, items: (result: R) => T[], @@ -168,12 +172,12 @@ function convertMcpTool(mcpTool: MCPToolDef, client: MCPClient, timeout?: number const inputSchema = mcpTool.inputSchema // Spread first, then override type to ensure it's always "object" - const schema: JSONSchema7 = { - ...(inputSchema as JSONSchema7), + const schema = { + ...inputSchema, type: "object", - properties: (inputSchema.properties ?? {}) as JSONSchema7["properties"], + properties: inputSchema.properties ?? {}, additionalProperties: false, - } + } satisfies JSONSchema7 return dynamicTool({ description: mcpTool.description ?? "", @@ -182,7 +186,7 @@ function convertMcpTool(mcpTool: MCPToolDef, client: MCPClient, timeout?: number return client.callTool( { name: mcpTool.name, - arguments: (args || {}) as Record, + arguments: isRecord(args) ? args : {}, }, CallToolResultSchema, { @@ -218,6 +222,10 @@ function fetchFromClient( } type UnavailableStatus = Exclude +type ConnectResult = { type: "connected"; client: MCPClient } | { type: "unavailable"; status: UnavailableStatus } +type CreateResult = + | { type: "connected"; client: MCPClient; defs: MCPToolDef[] } + | { type: "unavailable"; status: UnavailableStatus } interface AuthResult { authorizationUrl: string @@ -294,7 +302,10 @@ export const layer = Layer.effect( (t, exit) => (Exit.isFailure(exit) ? Effect.tryPromise(() => t.close()).pipe(Effect.ignore) : Effect.void), ) - const DISABLED_RESULT = { status: { status: "disabled" as const } } + const DISABLED_RESULT = { + type: "unavailable", + status: { status: "disabled" }, + } satisfies CreateResult const connectRemote = Effect.fn("MCP.connectRemote")(function* (key: string, mcp: ConfigMCPV1.Remote) { const oauthDisabled = mcp.oauth === false @@ -302,8 +313,9 @@ export const layer = Layer.effect( const url = remoteURL(mcp.url) if (!url) { return { - status: { status: "failed" as const, error: `Invalid MCP URL for "${key}"` }, - } + type: "unavailable", + status: { status: "failed", error: `Invalid MCP URL for "${key}"` }, + } satisfies ConnectResult } let authProvider: McpOAuthProvider | undefined @@ -356,7 +368,7 @@ export const layer = Layer.effect( if (isAuthError) { if (lastError.message.includes("registration") || lastError.message.includes("client_id")) { lastStatus = { - status: "needs_client_registration" as const, + status: "needs_client_registration", error: "Server does not support dynamic client registration. Please provide clientId in config.", } return events @@ -369,7 +381,7 @@ export const layer = Layer.effect( .pipe(Effect.ignore, Effect.as(undefined)) } else { pendingOAuthTransports.set(key, transport) - lastStatus = { status: "needs_auth" as const } + lastStatus = { status: "needs_auth" } return events .publish(TuiEvent.ToastShow, { title: "MCP Authentication Required", @@ -381,18 +393,19 @@ export const layer = Layer.effect( } } - lastStatus = { status: "failed" as const, error: lastError.message } + lastStatus = { status: "failed", error: lastError.message } return Effect.void }), ) - if (result) return { client: result.client } + if (result) return { type: "connected", client: result.client } satisfies ConnectResult // If this was an auth error, stop trying other transports if (lastStatus?.status === "needs_auth" || lastStatus?.status === "needs_client_registration") break } return { - status: lastStatus ?? { status: "failed" as const, error: "Unknown error" }, - } + type: "unavailable", + status: lastStatus ?? { status: "failed", error: "Unknown error" }, + } satisfies ConnectResult }) const connectLocal = Effect.fn("MCP.connectLocal")(function* (mcp: ConfigMCPV1.Local) { @@ -412,10 +425,13 @@ export const layer = Layer.effect( const connectTimeout = mcp.timeout ?? DEFAULT_TIMEOUT return yield* connectTransport(transport, connectTimeout).pipe( - Effect.map((client) => ({ client })), + Effect.map((client) => ({ type: "connected", client }) satisfies ConnectResult), Effect.catch((error) => { const msg = error instanceof Error ? error.message : String(error) - return Effect.succeed({ status: { status: "failed" as const, error: msg } }) + return Effect.succeed({ + type: "unavailable", + status: { status: "failed", error: msg }, + } satisfies ConnectResult) }), ) }) @@ -427,24 +443,27 @@ export const layer = Layer.effect( const result = mcp.type === "remote" ? yield* connectRemote(key, mcp) : yield* connectLocal(mcp) - if ("status" in result) { + if (result.type === "unavailable") { yield* Effect.logWarning("server unavailable", { key, type: mcp.type, status: result.status.status }) - return result + return result satisfies CreateResult } const listed = result.client.getServerCapabilities()?.tools ? yield* defs(result.client, mcp.timeout) : [] if (!listed) { yield* Effect.tryPromise(() => result.client.close()).pipe(Effect.ignore) - return { status: { status: "failed" as const, error: "Failed to get tools" } } + return { + type: "unavailable", + status: { status: "failed", error: "Failed to get tools" }, + } satisfies CreateResult } - return { client: result.client, defs: listed } + return { type: "connected", client: result.client, defs: listed } satisfies CreateResult }) const cfgSvc = yield* Config.Service const descendants = Effect.fnUntraced( function* (pid: number) { - if (process.platform === "win32") return [] as number[] + if (process.platform === "win32") return Array() const pids: number[] = [] const queue = [pid] for (let index = 0; index < queue.length; index++) { @@ -463,7 +482,7 @@ export const layer = Layer.effect( return pids }, Effect.scoped, - Effect.catch(() => Effect.succeed([] as number[])), + Effect.catch(() => Effect.succeed(Array())), ) function watch(s: State, name: string, client: MCPClient, bridge: EffectBridge.Shape, timeout?: number) { @@ -508,7 +527,7 @@ export const layer = Layer.effect( const result = yield* create(key, mcp).pipe(Effect.catch(() => Effect.void)) if (!result) return - if ("status" in result) { + if (result.type === "unavailable") { s.status[key] = result.status return } @@ -598,7 +617,7 @@ export const layer = Layer.effect( const s = yield* InstanceState.get(state) const result = yield* create(name, mcp) - if ("status" in result) { + if (result.type === "unavailable") { s.status[name] = result.status yield* closeClient(s, name) delete s.clients[name] @@ -843,7 +862,7 @@ export const layer = Layer.effect( if (!transport) throw new Error(`No pending OAuth flow for MCP server: ${mcpName}`) const result = yield* Effect.tryPromise({ - try: () => transport.finishAuth(authorizationCode).then(() => true as const), + try: () => transport.finishAuth(authorizationCode).then((): true => true), catch: (error) => { return error },