From 1dc69359d5351c4fb4dfbefef9e87a34c7c15b12 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Mon, 13 Apr 2026 13:45:34 -0400 Subject: [PATCH] refactor(mcp): remove async facade exports (#22324) --- packages/opencode/src/cli/cmd/mcp.ts | 127 ++-- packages/opencode/src/mcp/index.ts | 34 -- .../src/server/instance/experimental.ts | 9 +- packages/opencode/src/server/instance/mcp.ts | 48 +- packages/opencode/test/mcp/headers.test.ts | 65 +- packages/opencode/test/mcp/lifecycle.test.ts | 568 ++++++++++-------- .../test/mcp/oauth-auto-connect.test.ts | 14 +- .../opencode/test/mcp/oauth-browser.test.ts | 25 +- 8 files changed, 501 insertions(+), 389 deletions(-) diff --git a/packages/opencode/src/cli/cmd/mcp.ts b/packages/opencode/src/cli/cmd/mcp.ts index e33ab6a35c..4904db1c31 100644 --- a/packages/opencode/src/cli/cmd/mcp.ts +++ b/packages/opencode/src/cli/cmd/mcp.ts @@ -15,7 +15,8 @@ import { Global } from "../../global" import { modify, applyEdits } from "jsonc-parser" import { Filesystem } from "../../util/filesystem" import { Bus } from "../../bus" -import { AppRuntime } from "@/effect/app-runtime" +import { AppRuntime } from "../../effect/app-runtime" +import { Effect } from "effect" function getAuthStatusIcon(status: MCP.AuthStatus): string { switch (status) { @@ -51,6 +52,47 @@ function isMcpRemote(config: McpEntry): config is McpRemote { return isMcpConfigured(config) && config.type === "remote" } +function configuredServers(config: Config.Info) { + return Object.entries(config.mcp ?? {}).filter((entry): entry is [string, McpConfigured] => isMcpConfigured(entry[1])) +} + +function oauthServers(config: Config.Info) { + return configuredServers(config).filter( + (entry): entry is [string, McpRemote] => isMcpRemote(entry[1]) && entry[1].oauth !== false, + ) +} + +async function listState() { + return AppRuntime.runPromise( + Effect.gen(function* () { + const cfg = yield* Config.Service + const mcp = yield* MCP.Service + const config = yield* cfg.get() + const statuses = yield* mcp.status() + const stored = yield* Effect.all( + Object.fromEntries(configuredServers(config).map(([name]) => [name, mcp.hasStoredTokens(name)])), + { concurrency: "unbounded" }, + ) + return { config, statuses, stored } + }), + ) +} + +async function authState() { + return AppRuntime.runPromise( + Effect.gen(function* () { + const cfg = yield* Config.Service + const mcp = yield* MCP.Service + const config = yield* cfg.get() + const auth = yield* Effect.all( + Object.fromEntries(oauthServers(config).map(([name]) => [name, mcp.getAuthStatus(name)])), + { concurrency: "unbounded" }, + ) + return { config, auth } + }), + ) +} + export const McpCommand = cmd({ command: "mcp", describe: "manage MCP (Model Context Protocol) servers", @@ -76,13 +118,8 @@ export const McpListCommand = cmd({ UI.empty() prompts.intro("MCP Servers") - const config = await AppRuntime.runPromise(Config.Service.use((cfg) => cfg.get())) - const mcpServers = config.mcp ?? {} - const statuses = await MCP.status() - - const servers = Object.entries(mcpServers).filter((entry): entry is [string, McpConfigured] => - isMcpConfigured(entry[1]), - ) + const { config, statuses, stored } = await listState() + const servers = configuredServers(config) if (servers.length === 0) { prompts.log.warn("No MCP servers configured") @@ -93,7 +130,7 @@ export const McpListCommand = cmd({ for (const [name, serverConfig] of servers) { const status = statuses[name] const hasOAuth = isMcpRemote(serverConfig) && !!serverConfig.oauth - const hasStoredTokens = await MCP.hasStoredTokens(name) + const hasStoredTokens = stored[name] let statusIcon: string let statusText: string @@ -153,15 +190,11 @@ export const McpAuthCommand = cmd({ UI.empty() prompts.intro("MCP OAuth Authentication") - const config = await AppRuntime.runPromise(Config.Service.use((cfg) => cfg.get())) + const { config, auth } = await authState() const mcpServers = config.mcp ?? {} + const servers = oauthServers(config) - // Get OAuth-capable servers (remote servers with oauth not explicitly disabled) - const oauthServers = Object.entries(mcpServers).filter( - (entry): entry is [string, McpRemote] => isMcpRemote(entry[1]) && entry[1].oauth !== false, - ) - - if (oauthServers.length === 0) { + if (servers.length === 0) { prompts.log.warn("No OAuth-capable MCP servers configured") prompts.log.info("Remote MCP servers support OAuth by default. Add a remote server in opencode.json:") prompts.log.info(` @@ -178,19 +211,17 @@ export const McpAuthCommand = cmd({ let serverName = args.name if (!serverName) { // Build options with auth status - const options = await Promise.all( - oauthServers.map(async ([name, cfg]) => { - const authStatus = await MCP.getAuthStatus(name) - const icon = getAuthStatusIcon(authStatus) - const statusText = getAuthStatusText(authStatus) - const url = cfg.url - return { - label: `${icon} ${name} (${statusText})`, - value: name, - hint: url, - } - }), - ) + const options = servers.map(([name, cfg]) => { + const authStatus = auth[name] + const icon = getAuthStatusIcon(authStatus) + const statusText = getAuthStatusText(authStatus) + const url = cfg.url + return { + label: `${icon} ${name} (${statusText})`, + value: name, + hint: url, + } + }) const selected = await prompts.select({ message: "Select MCP server to authenticate", @@ -214,7 +245,8 @@ export const McpAuthCommand = cmd({ } // Check if already authenticated - const authStatus = await MCP.getAuthStatus(serverName) + const authStatus = + auth[serverName] ?? (await AppRuntime.runPromise(MCP.Service.use((mcp) => mcp.getAuthStatus(serverName)))) if (authStatus === "authenticated") { const confirm = await prompts.confirm({ message: `${serverName} already has valid credentials. Re-authenticate?`, @@ -241,7 +273,7 @@ export const McpAuthCommand = cmd({ }) try { - const status = await MCP.authenticate(serverName) + const status = await AppRuntime.runPromise(MCP.Service.use((mcp) => mcp.authenticate(serverName))) if (status.status === "connected") { spinner.stop("Authentication successful!") @@ -290,22 +322,17 @@ export const McpAuthListCommand = cmd({ UI.empty() prompts.intro("MCP OAuth Status") - const config = await AppRuntime.runPromise(Config.Service.use((cfg) => cfg.get())) - const mcpServers = config.mcp ?? {} + const { config, auth } = await authState() + const servers = oauthServers(config) - // Get OAuth-capable servers - const oauthServers = Object.entries(mcpServers).filter( - (entry): entry is [string, McpRemote] => isMcpRemote(entry[1]) && entry[1].oauth !== false, - ) - - if (oauthServers.length === 0) { + if (servers.length === 0) { prompts.log.warn("No OAuth-capable MCP servers configured") prompts.outro("Done") return } - for (const [name, serverConfig] of oauthServers) { - const authStatus = await MCP.getAuthStatus(name) + for (const [name, serverConfig] of servers) { + const authStatus = auth[name] const icon = getAuthStatusIcon(authStatus) const statusText = getAuthStatusText(authStatus) const url = serverConfig.url @@ -313,7 +340,7 @@ export const McpAuthListCommand = cmd({ prompts.log.info(`${icon} ${name} ${UI.Style.TEXT_DIM}${statusText}\n ${UI.Style.TEXT_DIM}${url}`) } - prompts.outro(`${oauthServers.length} OAuth-capable server(s)`) + prompts.outro(`${servers.length} OAuth-capable server(s)`) }, }) }, @@ -335,7 +362,7 @@ export const McpLogoutCommand = cmd({ prompts.intro("MCP OAuth Logout") const authPath = path.join(Global.Path.data, "mcp-auth.json") - const credentials = await McpAuth.all() + const credentials = await AppRuntime.runPromise(McpAuth.Service.use((auth) => auth.all())) const serverNames = Object.keys(credentials) if (serverNames.length === 0) { @@ -373,7 +400,7 @@ export const McpLogoutCommand = cmd({ return } - await MCP.removeAuth(serverName) + await AppRuntime.runPromise(MCP.Service.use((mcp) => mcp.removeAuth(serverName))) prompts.log.success(`Removed OAuth credentials for ${serverName}`) prompts.outro("Done") }, @@ -623,10 +650,18 @@ export const McpDebugCommand = cmd({ prompts.log.info(`URL: ${serverConfig.url}`) // Check stored auth status - const authStatus = await MCP.getAuthStatus(serverName) + const { authStatus, entry } = await AppRuntime.runPromise( + Effect.gen(function* () { + const mcp = yield* MCP.Service + const auth = yield* McpAuth.Service + return { + authStatus: yield* mcp.getAuthStatus(serverName), + entry: yield* auth.get(serverName), + } + }), + ) prompts.log.info(`Auth status: ${getAuthStatusIcon(authStatus)} ${getAuthStatusText(authStatus)}`) - const entry = await McpAuth.get(serverName) if (entry?.tokens) { prompts.log.info(` Access token: ${entry.tokens.accessToken.substring(0, 20)}...`) if (entry.tokens.expiresAt) { diff --git a/packages/opencode/src/mcp/index.ts b/packages/opencode/src/mcp/index.ts index 696a662c12..fc34143a2c 100644 --- a/packages/opencode/src/mcp/index.ts +++ b/packages/opencode/src/mcp/index.ts @@ -27,7 +27,6 @@ import open from "open" import { Effect, Exit, Layer, Option, Context, Stream } from "effect" import { EffectLogger } from "@/effect/logger" import { InstanceState } from "@/effect/instance-state" -import { makeRuntime } from "@/effect/run-service" import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process" import * as CrossSpawnSpawner from "@/effect/cross-spawn-spawner" @@ -890,37 +889,4 @@ export namespace MCP { Layer.provide(CrossSpawnSpawner.defaultLayer), Layer.provide(AppFileSystem.defaultLayer), ) - - const { runPromise } = makeRuntime(Service, defaultLayer) - - // --- Async facade functions --- - - export const status = async () => runPromise((svc) => svc.status()) - - export const tools = async () => runPromise((svc) => svc.tools()) - - export const prompts = async () => runPromise((svc) => svc.prompts()) - - export const resources = async () => runPromise((svc) => svc.resources()) - - export const add = async (name: string, mcp: Config.Mcp) => runPromise((svc) => svc.add(name, mcp)) - - export const connect = async (name: string) => runPromise((svc) => svc.connect(name)) - - export const disconnect = async (name: string) => runPromise((svc) => svc.disconnect(name)) - - export const startAuth = async (mcpName: string) => runPromise((svc) => svc.startAuth(mcpName)) - - export const authenticate = async (mcpName: string) => runPromise((svc) => svc.authenticate(mcpName)) - - export const finishAuth = async (mcpName: string, authorizationCode: string) => - runPromise((svc) => svc.finishAuth(mcpName, authorizationCode)) - - export const removeAuth = async (mcpName: string) => runPromise((svc) => svc.removeAuth(mcpName)) - - export const supportsOAuth = async (mcpName: string) => runPromise((svc) => svc.supportsOAuth(mcpName)) - - export const hasStoredTokens = async (mcpName: string) => runPromise((svc) => svc.hasStoredTokens(mcpName)) - - export const getAuthStatus = async (mcpName: string) => runPromise((svc) => svc.getAuthStatus(mcpName)) } diff --git a/packages/opencode/src/server/instance/experimental.ts b/packages/opencode/src/server/instance/experimental.ts index 978aa03a99..ca8b89fa6a 100644 --- a/packages/opencode/src/server/instance/experimental.ts +++ b/packages/opencode/src/server/instance/experimental.ts @@ -408,7 +408,14 @@ export const ExperimentalRoutes = lazy(() => }, }), async (c) => { - return c.json(await MCP.resources()) + return c.json( + await AppRuntime.runPromise( + Effect.gen(function* () { + const mcp = yield* MCP.Service + return yield* mcp.resources() + }), + ), + ) }, ), ) diff --git a/packages/opencode/src/server/instance/mcp.ts b/packages/opencode/src/server/instance/mcp.ts index 1e604c9918..f1c8701c4e 100644 --- a/packages/opencode/src/server/instance/mcp.ts +++ b/packages/opencode/src/server/instance/mcp.ts @@ -3,8 +3,10 @@ import { describeRoute, validator, resolver } from "hono-openapi" import z from "zod" import { MCP } from "../../mcp" import { Config } from "../../config/config" +import { AppRuntime } from "../../effect/app-runtime" import { errors } from "../error" import { lazy } from "../../util/lazy" +import { Effect } from "effect" export const McpRoutes = lazy(() => new Hono() @@ -26,7 +28,7 @@ export const McpRoutes = lazy(() => }, }), async (c) => { - return c.json(await MCP.status()) + return c.json(await AppRuntime.runPromise(MCP.Service.use((mcp) => mcp.status()))) }, ) .post( @@ -56,7 +58,7 @@ export const McpRoutes = lazy(() => ), async (c) => { const { name, config } = c.req.valid("json") - const result = await MCP.add(name, config) + const result = await AppRuntime.runPromise(MCP.Service.use((mcp) => mcp.add(name, config))) return c.json(result.status) }, ) @@ -84,12 +86,21 @@ export const McpRoutes = lazy(() => }), async (c) => { const name = c.req.param("name") - const supportsOAuth = await MCP.supportsOAuth(name) - if (!supportsOAuth) { + const result = await AppRuntime.runPromise( + Effect.gen(function* () { + const mcp = yield* MCP.Service + const supports = yield* mcp.supportsOAuth(name) + if (!supports) return { supports } + return { + supports, + auth: yield* mcp.startAuth(name), + } + }), + ) + if (!result.supports) { return c.json({ error: `MCP server ${name} does not support OAuth` }, 400) } - const result = await MCP.startAuth(name) - return c.json(result) + return c.json(result.auth) }, ) .post( @@ -120,7 +131,7 @@ export const McpRoutes = lazy(() => async (c) => { const name = c.req.param("name") const { code } = c.req.valid("json") - const status = await MCP.finishAuth(name, code) + const status = await AppRuntime.runPromise(MCP.Service.use((mcp) => mcp.finishAuth(name, code))) return c.json(status) }, ) @@ -144,12 +155,21 @@ export const McpRoutes = lazy(() => }), async (c) => { const name = c.req.param("name") - const supportsOAuth = await MCP.supportsOAuth(name) - if (!supportsOAuth) { + const result = await AppRuntime.runPromise( + Effect.gen(function* () { + const mcp = yield* MCP.Service + const supports = yield* mcp.supportsOAuth(name) + if (!supports) return { supports } + return { + supports, + status: yield* mcp.authenticate(name), + } + }), + ) + if (!result.supports) { return c.json({ error: `MCP server ${name} does not support OAuth` }, 400) } - const status = await MCP.authenticate(name) - return c.json(status) + return c.json(result.status) }, ) .delete( @@ -172,7 +192,7 @@ export const McpRoutes = lazy(() => }), async (c) => { const name = c.req.param("name") - await MCP.removeAuth(name) + await AppRuntime.runPromise(MCP.Service.use((mcp) => mcp.removeAuth(name))) return c.json({ success: true as const }) }, ) @@ -195,7 +215,7 @@ export const McpRoutes = lazy(() => validator("param", z.object({ name: z.string() })), async (c) => { const { name } = c.req.valid("param") - await MCP.connect(name) + await AppRuntime.runPromise(MCP.Service.use((mcp) => mcp.connect(name))) return c.json(true) }, ) @@ -218,7 +238,7 @@ export const McpRoutes = lazy(() => validator("param", z.object({ name: z.string() })), async (c) => { const { name } = c.req.valid("param") - await MCP.disconnect(name) + await AppRuntime.runPromise(MCP.Service.use((mcp) => mcp.disconnect(name))) return c.json(true) }, ), diff --git a/packages/opencode/test/mcp/headers.test.ts b/packages/opencode/test/mcp/headers.test.ts index 69998aaaa8..14c08e3036 100644 --- a/packages/opencode/test/mcp/headers.test.ts +++ b/packages/opencode/test/mcp/headers.test.ts @@ -1,4 +1,6 @@ import { test, expect, mock, beforeEach } from "bun:test" +import { Effect } from "effect" +import type { MCP as MCPNS } from "../../src/mcp/index" // Track what options were passed to each transport constructor const transportCalls: Array<{ @@ -44,8 +46,10 @@ beforeEach(() => { // Import MCP after mocking const { MCP } = await import("../../src/mcp/index") +const { AppRuntime } = await import("../../src/effect/app-runtime") const { Instance } = await import("../../src/project/instance") const { tmpdir } = await import("../fixture/fixture") +const service = MCP.Service as unknown as Effect.Effect test("headers are passed to transports when oauth is enabled (default)", async () => { await using tmp = await tmpdir({ @@ -73,14 +77,21 @@ test("headers are passed to transports when oauth is enabled (default)", async ( directory: tmp.path, fn: async () => { // Trigger MCP initialization - it will fail to connect but we can check the transport options - await MCP.add("test-server", { - type: "remote", - url: "https://example.com/mcp", - headers: { - Authorization: "Bearer test-token", - "X-Custom-Header": "custom-value", - }, - }).catch(() => {}) + await AppRuntime.runPromise( + Effect.gen(function* () { + const mcp = yield* service + yield* mcp + .add("test-server", { + type: "remote", + url: "https://example.com/mcp", + headers: { + Authorization: "Bearer test-token", + "X-Custom-Header": "custom-value", + }, + }) + .pipe(Effect.catch(() => Effect.void)) + }), + ) // Both transports should have been created with headers expect(transportCalls.length).toBeGreaterThanOrEqual(1) @@ -106,14 +117,21 @@ test("headers are passed to transports when oauth is explicitly disabled", async fn: async () => { transportCalls.length = 0 - await MCP.add("test-server-no-oauth", { - type: "remote", - url: "https://example.com/mcp", - oauth: false, - headers: { - Authorization: "Bearer test-token", - }, - }).catch(() => {}) + await AppRuntime.runPromise( + Effect.gen(function* () { + const mcp = yield* service + yield* mcp + .add("test-server-no-oauth", { + type: "remote", + url: "https://example.com/mcp", + oauth: false, + headers: { + Authorization: "Bearer test-token", + }, + }) + .pipe(Effect.catch(() => Effect.void)) + }), + ) expect(transportCalls.length).toBeGreaterThanOrEqual(1) @@ -137,10 +155,17 @@ test("no requestInit when headers are not provided", async () => { fn: async () => { transportCalls.length = 0 - await MCP.add("test-server-no-headers", { - type: "remote", - url: "https://example.com/mcp", - }).catch(() => {}) + await AppRuntime.runPromise( + Effect.gen(function* () { + const mcp = yield* service + yield* mcp + .add("test-server-no-headers", { + type: "remote", + url: "https://example.com/mcp", + }) + .pipe(Effect.catch(() => Effect.void)) + }), + ) expect(transportCalls.length).toBeGreaterThanOrEqual(1) diff --git a/packages/opencode/test/mcp/lifecycle.test.ts b/packages/opencode/test/mcp/lifecycle.test.ts index 3d17d3a0d0..09caa1cd8a 100644 --- a/packages/opencode/test/mcp/lifecycle.test.ts +++ b/packages/opencode/test/mcp/lifecycle.test.ts @@ -1,4 +1,6 @@ import { test, expect, mock, beforeEach } from "bun:test" +import { Effect } from "effect" +import type { MCP as MCPNS } from "../../src/mcp/index" // --- Mock infrastructure --- @@ -170,7 +172,10 @@ const { tmpdir } = await import("../fixture/fixture") // --- Helper --- -function withInstance(config: Record, fn: () => Promise) { +function withInstance( + config: Record, + fn: (mcp: MCPNS.Interface) => Effect.Effect, +) { return async () => { await using tmp = await tmpdir({ init: async (dir) => { @@ -187,7 +192,7 @@ function withInstance(config: Record, fn: () => Promise) { await Instance.provide({ directory: tmp.path, fn: async () => { - await fn() + await Effect.runPromise(MCP.Service.use(fn).pipe(Effect.provide(MCP.defaultLayer))) // dispose instance to clean up state between tests await Instance.dispose() }, @@ -201,28 +206,30 @@ function withInstance(config: Record, fn: () => Promise) { test( "tools() reuses cached tool definitions after connect", - withInstance({}, async () => { - lastCreatedClientName = "my-server" - const serverState = getOrCreateClientState("my-server") - serverState.tools = [ - { name: "do_thing", description: "does a thing", inputSchema: { type: "object", properties: {} } }, - ] + withInstance({}, (mcp) => + Effect.gen(function* () { + lastCreatedClientName = "my-server" + const serverState = getOrCreateClientState("my-server") + serverState.tools = [ + { name: "do_thing", description: "does a thing", inputSchema: { type: "object", properties: {} } }, + ] - // First: add the server successfully - const addResult = await MCP.add("my-server", { - type: "local", - command: ["echo", "test"], - }) - expect((addResult.status as any)["my-server"]?.status ?? (addResult.status as any).status).toBe("connected") + // First: add the server successfully + const addResult = yield* mcp.add("my-server", { + type: "local", + command: ["echo", "test"], + }) + expect((addResult.status as any)["my-server"]?.status ?? (addResult.status as any).status).toBe("connected") - expect(serverState.listToolsCalls).toBe(1) + expect(serverState.listToolsCalls).toBe(1) - const toolsA = await MCP.tools() - const toolsB = await MCP.tools() - expect(Object.keys(toolsA).length).toBeGreaterThan(0) - expect(Object.keys(toolsB).length).toBeGreaterThan(0) - expect(serverState.listToolsCalls).toBe(1) - }), + const toolsA = yield* mcp.tools() + const toolsB = yield* mcp.tools() + expect(Object.keys(toolsA).length).toBeGreaterThan(0) + expect(Object.keys(toolsB).length).toBeGreaterThan(0) + expect(serverState.listToolsCalls).toBe(1) + }), + ), ) // ======================================================================== @@ -231,30 +238,32 @@ test( test( "tool change notifications refresh cached tool definitions", - withInstance({}, async () => { - lastCreatedClientName = "status-server" - const serverState = getOrCreateClientState("status-server") + withInstance({}, (mcp) => + Effect.gen(function* () { + lastCreatedClientName = "status-server" + const serverState = getOrCreateClientState("status-server") - await MCP.add("status-server", { - type: "local", - command: ["echo", "test"], - }) + yield* mcp.add("status-server", { + type: "local", + command: ["echo", "test"], + }) - const before = await MCP.tools() - expect(Object.keys(before).some((key) => key.includes("test_tool"))).toBe(true) - expect(serverState.listToolsCalls).toBe(1) + const before = yield* mcp.tools() + expect(Object.keys(before).some((key) => key.includes("test_tool"))).toBe(true) + expect(serverState.listToolsCalls).toBe(1) - serverState.tools = [{ name: "next_tool", description: "next", inputSchema: { type: "object", properties: {} } }] + serverState.tools = [{ name: "next_tool", description: "next", inputSchema: { type: "object", properties: {} } }] - const handler = Array.from(serverState.notificationHandlers.values())[0] - expect(handler).toBeDefined() - await handler?.() + const handler = Array.from(serverState.notificationHandlers.values())[0] + expect(handler).toBeDefined() + yield* Effect.promise(() => handler?.()) - const after = await MCP.tools() - expect(Object.keys(after).some((key) => key.includes("next_tool"))).toBe(true) - expect(Object.keys(after).some((key) => key.includes("test_tool"))).toBe(false) - expect(serverState.listToolsCalls).toBe(2) - }), + const after = yield* mcp.tools() + expect(Object.keys(after).some((key) => key.includes("next_tool"))).toBe(true) + expect(Object.keys(after).some((key) => key.includes("test_tool"))).toBe(false) + expect(serverState.listToolsCalls).toBe(2) + }), + ), ) // ======================================================================== @@ -270,28 +279,29 @@ test( command: ["echo", "test"], }, }, - async () => { - lastCreatedClientName = "disc-server" - getOrCreateClientState("disc-server") + (mcp) => + Effect.gen(function* () { + lastCreatedClientName = "disc-server" + getOrCreateClientState("disc-server") - await MCP.add("disc-server", { - type: "local", - command: ["echo", "test"], - }) + yield* mcp.add("disc-server", { + type: "local", + command: ["echo", "test"], + }) - const statusBefore = await MCP.status() - expect(statusBefore["disc-server"]?.status).toBe("connected") + const statusBefore = yield* mcp.status() + expect(statusBefore["disc-server"]?.status).toBe("connected") - await MCP.disconnect("disc-server") + yield* mcp.disconnect("disc-server") - const statusAfter = await MCP.status() - expect(statusAfter["disc-server"]?.status).toBe("disabled") + const statusAfter = yield* mcp.status() + expect(statusAfter["disc-server"]?.status).toBe("disabled") - // Tools should be empty after disconnect - const tools = await MCP.tools() - const serverTools = Object.keys(tools).filter((k) => k.startsWith("disc-server")) - expect(serverTools.length).toBe(0) - }, + // Tools should be empty after disconnect + const tools = yield* mcp.tools() + const serverTools = Object.keys(tools).filter((k) => k.startsWith("disc-server")) + expect(serverTools.length).toBe(0) + }), ), ) @@ -304,26 +314,29 @@ test( command: ["echo", "test"], }, }, - async () => { - lastCreatedClientName = "reconn-server" - const serverState = getOrCreateClientState("reconn-server") - serverState.tools = [{ name: "my_tool", description: "a tool", inputSchema: { type: "object", properties: {} } }] + (mcp) => + Effect.gen(function* () { + lastCreatedClientName = "reconn-server" + const serverState = getOrCreateClientState("reconn-server") + serverState.tools = [ + { name: "my_tool", description: "a tool", inputSchema: { type: "object", properties: {} } }, + ] - await MCP.add("reconn-server", { - type: "local", - command: ["echo", "test"], - }) + yield* mcp.add("reconn-server", { + type: "local", + command: ["echo", "test"], + }) - await MCP.disconnect("reconn-server") - expect((await MCP.status())["reconn-server"]?.status).toBe("disabled") + yield* mcp.disconnect("reconn-server") + expect((yield* mcp.status())["reconn-server"]?.status).toBe("disabled") - // Reconnect - await MCP.connect("reconn-server") - expect((await MCP.status())["reconn-server"]?.status).toBe("connected") + // Reconnect + yield* mcp.connect("reconn-server") + expect((yield* mcp.status())["reconn-server"]?.status).toBe("connected") - const tools = await MCP.tools() - expect(Object.keys(tools).some((k) => k.includes("my_tool"))).toBe(true) - }, + const tools = yield* mcp.tools() + expect(Object.keys(tools).some((k) => k.includes("my_tool"))).toBe(true) + }), ), ) @@ -335,30 +348,32 @@ test( "add() closes the old client when replacing a server", // Don't put the server in config — add it dynamically so we control // exactly which client instance is "first" vs "second". - withInstance({}, async () => { - lastCreatedClientName = "replace-server" - const firstState = getOrCreateClientState("replace-server") + withInstance({}, (mcp) => + Effect.gen(function* () { + lastCreatedClientName = "replace-server" + const firstState = getOrCreateClientState("replace-server") - await MCP.add("replace-server", { - type: "local", - command: ["echo", "test"], - }) + yield* mcp.add("replace-server", { + type: "local", + command: ["echo", "test"], + }) - expect(firstState.closed).toBe(false) + expect(firstState.closed).toBe(false) - // Create new state for second client - clientStates.delete("replace-server") - const secondState = getOrCreateClientState("replace-server") + // Create new state for second client + clientStates.delete("replace-server") + const secondState = getOrCreateClientState("replace-server") - // Re-add should close the first client - await MCP.add("replace-server", { - type: "local", - command: ["echo", "test"], - }) + // Re-add should close the first client + yield* mcp.add("replace-server", { + type: "local", + command: ["echo", "test"], + }) - expect(firstState.closed).toBe(true) - expect(secondState.closed).toBe(false) - }), + expect(firstState.closed).toBe(true) + expect(secondState.closed).toBe(false) + }), + ), ) // ======================================================================== @@ -378,37 +393,38 @@ test( command: ["echo", "bad"], }, }, - async () => { - // Set up good server - const goodState = getOrCreateClientState("good-server") - goodState.tools = [{ name: "good_tool", description: "works", inputSchema: { type: "object", properties: {} } }] + (mcp) => + Effect.gen(function* () { + // Set up good server + const goodState = getOrCreateClientState("good-server") + goodState.tools = [{ name: "good_tool", description: "works", inputSchema: { type: "object", properties: {} } }] - // Set up bad server - will fail on listTools during create() - const badState = getOrCreateClientState("bad-server") - badState.listToolsShouldFail = true + // Set up bad server - will fail on listTools during create() + const badState = getOrCreateClientState("bad-server") + badState.listToolsShouldFail = true - // Add good server first - lastCreatedClientName = "good-server" - await MCP.add("good-server", { - type: "local", - command: ["echo", "good"], - }) + // Add good server first + lastCreatedClientName = "good-server" + yield* mcp.add("good-server", { + type: "local", + command: ["echo", "good"], + }) - // Add bad server - should fail but not affect good server - lastCreatedClientName = "bad-server" - await MCP.add("bad-server", { - type: "local", - command: ["echo", "bad"], - }) + // Add bad server - should fail but not affect good server + lastCreatedClientName = "bad-server" + yield* mcp.add("bad-server", { + type: "local", + command: ["echo", "bad"], + }) - const status = await MCP.status() - expect(status["good-server"]?.status).toBe("connected") - expect(status["bad-server"]?.status).toBe("failed") + const status = yield* mcp.status() + expect(status["good-server"]?.status).toBe("connected") + expect(status["bad-server"]?.status).toBe("failed") - // Good server's tools should still be available - const tools = await MCP.tools() - expect(Object.keys(tools).some((k) => k.includes("good_tool"))).toBe(true) - }, + // Good server's tools should still be available + const tools = yield* mcp.tools() + expect(Object.keys(tools).some((k) => k.includes("good_tool"))).toBe(true) + }), ), ) @@ -426,21 +442,22 @@ test( enabled: false, }, }, - async () => { - const countBefore = clientCreateCount + (mcp) => + Effect.gen(function* () { + const countBefore = clientCreateCount - await MCP.add("disabled-server", { - type: "local", - command: ["echo", "test"], - enabled: false, - } as any) + yield* mcp.add("disabled-server", { + type: "local", + command: ["echo", "test"], + enabled: false, + } as any) - // No client should have been created - expect(clientCreateCount).toBe(countBefore) + // No client should have been created + expect(clientCreateCount).toBe(countBefore) - const status = await MCP.status() - expect(status["disabled-server"]?.status).toBe("disabled") - }, + const status = yield* mcp.status() + expect(status["disabled-server"]?.status).toBe("disabled") + }), ), ) @@ -457,22 +474,23 @@ test( command: ["echo", "test"], }, }, - async () => { - lastCreatedClientName = "prompt-server" - const serverState = getOrCreateClientState("prompt-server") - serverState.prompts = [{ name: "my-prompt", description: "A test prompt" }] + (mcp) => + Effect.gen(function* () { + lastCreatedClientName = "prompt-server" + const serverState = getOrCreateClientState("prompt-server") + serverState.prompts = [{ name: "my-prompt", description: "A test prompt" }] - await MCP.add("prompt-server", { - type: "local", - command: ["echo", "test"], - }) + yield* mcp.add("prompt-server", { + type: "local", + command: ["echo", "test"], + }) - const prompts = await MCP.prompts() - expect(Object.keys(prompts).length).toBe(1) - const key = Object.keys(prompts)[0] - expect(key).toContain("prompt-server") - expect(key).toContain("my-prompt") - }, + const prompts = yield* mcp.prompts() + expect(Object.keys(prompts).length).toBe(1) + const key = Object.keys(prompts)[0] + expect(key).toContain("prompt-server") + expect(key).toContain("my-prompt") + }), ), ) @@ -485,22 +503,23 @@ test( command: ["echo", "test"], }, }, - async () => { - lastCreatedClientName = "resource-server" - const serverState = getOrCreateClientState("resource-server") - serverState.resources = [{ name: "my-resource", uri: "file:///test.txt", description: "A test resource" }] + (mcp) => + Effect.gen(function* () { + lastCreatedClientName = "resource-server" + const serverState = getOrCreateClientState("resource-server") + serverState.resources = [{ name: "my-resource", uri: "file:///test.txt", description: "A test resource" }] - await MCP.add("resource-server", { - type: "local", - command: ["echo", "test"], - }) + yield* mcp.add("resource-server", { + type: "local", + command: ["echo", "test"], + }) - const resources = await MCP.resources() - expect(Object.keys(resources).length).toBe(1) - const key = Object.keys(resources)[0] - expect(key).toContain("resource-server") - expect(key).toContain("my-resource") - }, + const resources = yield* mcp.resources() + expect(Object.keys(resources).length).toBe(1) + const key = Object.keys(resources)[0] + expect(key).toContain("resource-server") + expect(key).toContain("my-resource") + }), ), ) @@ -513,21 +532,22 @@ test( command: ["echo", "test"], }, }, - async () => { - lastCreatedClientName = "prompt-disc-server" - const serverState = getOrCreateClientState("prompt-disc-server") - serverState.prompts = [{ name: "hidden-prompt", description: "Should not appear" }] + (mcp) => + Effect.gen(function* () { + lastCreatedClientName = "prompt-disc-server" + const serverState = getOrCreateClientState("prompt-disc-server") + serverState.prompts = [{ name: "hidden-prompt", description: "Should not appear" }] - await MCP.add("prompt-disc-server", { - type: "local", - command: ["echo", "test"], - }) + yield* mcp.add("prompt-disc-server", { + type: "local", + command: ["echo", "test"], + }) - await MCP.disconnect("prompt-disc-server") + yield* mcp.disconnect("prompt-disc-server") - const prompts = await MCP.prompts() - expect(Object.keys(prompts).length).toBe(0) - }, + const prompts = yield* mcp.prompts() + expect(Object.keys(prompts).length).toBe(0) + }), ), ) @@ -537,12 +557,14 @@ test( test( "connect() on nonexistent server does not throw", - withInstance({}, async () => { - // Should not throw - await MCP.connect("nonexistent") - const status = await MCP.status() - expect(status["nonexistent"]).toBeUndefined() - }), + withInstance({}, (mcp) => + Effect.gen(function* () { + // Should not throw + yield* mcp.connect("nonexistent") + const status = yield* mcp.status() + expect(status["nonexistent"]).toBeUndefined() + }), + ), ) // ======================================================================== @@ -551,10 +573,12 @@ test( test( "disconnect() on nonexistent server does not throw", - withInstance({}, async () => { - await MCP.disconnect("nonexistent") - // Should complete without error - }), + withInstance({}, (mcp) => + Effect.gen(function* () { + yield* mcp.disconnect("nonexistent") + // Should complete without error + }), + ), ) // ======================================================================== @@ -563,10 +587,12 @@ test( test( "tools() returns empty when no MCP servers are configured", - withInstance({}, async () => { - const tools = await MCP.tools() - expect(Object.keys(tools).length).toBe(0) - }), + withInstance({}, (mcp) => + Effect.gen(function* () { + const tools = yield* mcp.tools() + expect(Object.keys(tools).length).toBe(0) + }), + ), ) // ======================================================================== @@ -582,27 +608,28 @@ test( command: ["echo", "test"], }, }, - async () => { - lastCreatedClientName = "fail-connect" - getOrCreateClientState("fail-connect") - connectShouldFail = true - connectError = "Connection refused" + (mcp) => + Effect.gen(function* () { + lastCreatedClientName = "fail-connect" + getOrCreateClientState("fail-connect") + connectShouldFail = true + connectError = "Connection refused" - await MCP.add("fail-connect", { - type: "local", - command: ["echo", "test"], - }) + yield* mcp.add("fail-connect", { + type: "local", + command: ["echo", "test"], + }) - const status = await MCP.status() - expect(status["fail-connect"]?.status).toBe("failed") - if (status["fail-connect"]?.status === "failed") { - expect(status["fail-connect"].error).toContain("Connection refused") - } + const status = yield* mcp.status() + expect(status["fail-connect"]?.status).toBe("failed") + if (status["fail-connect"]?.status === "failed") { + expect(status["fail-connect"].error).toContain("Connection refused") + } - // No tools should be available - const tools = await MCP.tools() - expect(Object.keys(tools).length).toBe(0) - }, + // No tools should be available + const tools = yield* mcp.tools() + expect(Object.keys(tools).length).toBe(0) + }), ), ) @@ -648,28 +675,29 @@ test( command: ["echo", "test"], }, }, - async () => { - lastCreatedClientName = "my.special-server" - const serverState = getOrCreateClientState("my.special-server") - serverState.tools = [ - { name: "tool-a", description: "Tool A", inputSchema: { type: "object", properties: {} } }, - { name: "tool.b", description: "Tool B", inputSchema: { type: "object", properties: {} } }, - ] + (mcp) => + Effect.gen(function* () { + lastCreatedClientName = "my.special-server" + const serverState = getOrCreateClientState("my.special-server") + serverState.tools = [ + { name: "tool-a", description: "Tool A", inputSchema: { type: "object", properties: {} } }, + { name: "tool.b", description: "Tool B", inputSchema: { type: "object", properties: {} } }, + ] - await MCP.add("my.special-server", { - type: "local", - command: ["echo", "test"], - }) + yield* mcp.add("my.special-server", { + type: "local", + command: ["echo", "test"], + }) - const tools = await MCP.tools() - const keys = Object.keys(tools) + const tools = yield* mcp.tools() + const keys = Object.keys(tools) - // Server name dots should be replaced with underscores - expect(keys.some((k) => k.startsWith("my_special-server_"))).toBe(true) - // Tool name dots should be replaced with underscores - expect(keys.some((k) => k.endsWith("tool_b"))).toBe(true) - expect(keys.length).toBe(2) - }, + // Server name dots should be replaced with underscores + expect(keys.some((k) => k.startsWith("my_special-server_"))).toBe(true) + // Tool name dots should be replaced with underscores + expect(keys.some((k) => k.endsWith("tool_b"))).toBe(true) + expect(keys.length).toBe(2) + }), ), ) @@ -679,23 +707,25 @@ test( test( "local stdio transport is closed when connect times out (no process leak)", - withInstance({}, async () => { - lastCreatedClientName = "hanging-server" - getOrCreateClientState("hanging-server") - connectShouldHang = true + withInstance({}, (mcp) => + Effect.gen(function* () { + lastCreatedClientName = "hanging-server" + getOrCreateClientState("hanging-server") + connectShouldHang = true - const addResult = await MCP.add("hanging-server", { - type: "local", - command: ["node", "fake.js"], - timeout: 100, - }) + const addResult = yield* mcp.add("hanging-server", { + type: "local", + command: ["node", "fake.js"], + timeout: 100, + }) - const serverStatus = (addResult.status as any)["hanging-server"] ?? addResult.status - expect(serverStatus.status).toBe("failed") - expect(serverStatus.error).toContain("timed out") - // Transport must be closed to avoid orphaned child process - expect(transportCloseCount).toBeGreaterThanOrEqual(1) - }), + const serverStatus = (addResult.status as any)["hanging-server"] ?? addResult.status + expect(serverStatus.status).toBe("failed") + expect(serverStatus.error).toContain("timed out") + // Transport must be closed to avoid orphaned child process + expect(transportCloseCount).toBeGreaterThanOrEqual(1) + }), + ), ) // ======================================================================== @@ -704,23 +734,25 @@ test( test( "remote transport is closed when connect times out", - withInstance({}, async () => { - lastCreatedClientName = "hanging-remote" - getOrCreateClientState("hanging-remote") - connectShouldHang = true + withInstance({}, (mcp) => + Effect.gen(function* () { + lastCreatedClientName = "hanging-remote" + getOrCreateClientState("hanging-remote") + connectShouldHang = true - const addResult = await MCP.add("hanging-remote", { - type: "remote", - url: "http://localhost:9999/mcp", - timeout: 100, - oauth: false, - }) + const addResult = yield* mcp.add("hanging-remote", { + type: "remote", + url: "http://localhost:9999/mcp", + timeout: 100, + oauth: false, + }) - const serverStatus = (addResult.status as any)["hanging-remote"] ?? addResult.status - expect(serverStatus.status).toBe("failed") - // Transport must be closed to avoid leaked HTTP connections - expect(transportCloseCount).toBeGreaterThanOrEqual(1) - }), + const serverStatus = (addResult.status as any)["hanging-remote"] ?? addResult.status + expect(serverStatus.status).toBe("failed") + // Transport must be closed to avoid leaked HTTP connections + expect(transportCloseCount).toBeGreaterThanOrEqual(1) + }), + ), ) // ======================================================================== @@ -729,22 +761,24 @@ test( test( "failed remote transport is closed before trying next transport", - withInstance({}, async () => { - lastCreatedClientName = "fail-remote" - getOrCreateClientState("fail-remote") - connectShouldFail = true - connectError = "Connection refused" + withInstance({}, (mcp) => + Effect.gen(function* () { + lastCreatedClientName = "fail-remote" + getOrCreateClientState("fail-remote") + connectShouldFail = true + connectError = "Connection refused" - const addResult = await MCP.add("fail-remote", { - type: "remote", - url: "http://localhost:9999/mcp", - timeout: 5000, - oauth: false, - }) + const addResult = yield* mcp.add("fail-remote", { + type: "remote", + url: "http://localhost:9999/mcp", + timeout: 5000, + oauth: false, + }) - const serverStatus = (addResult.status as any)["fail-remote"] ?? addResult.status - expect(serverStatus.status).toBe("failed") - // Both StreamableHTTP and SSE transports should be closed - expect(transportCloseCount).toBeGreaterThanOrEqual(2) - }), + const serverStatus = (addResult.status as any)["fail-remote"] ?? addResult.status + expect(serverStatus.status).toBe("failed") + // Both StreamableHTTP and SSE transports should be closed + expect(transportCloseCount).toBeGreaterThanOrEqual(2) + }), + ), ) diff --git a/packages/opencode/test/mcp/oauth-auto-connect.test.ts b/packages/opencode/test/mcp/oauth-auto-connect.test.ts index 76f825247c..786f1fb464 100644 --- a/packages/opencode/test/mcp/oauth-auto-connect.test.ts +++ b/packages/opencode/test/mcp/oauth-auto-connect.test.ts @@ -1,4 +1,6 @@ import { test, expect, mock, beforeEach } from "bun:test" +import { Effect } from "effect" +import type { MCP as MCPNS } from "../../src/mcp/index" // Mock UnauthorizedError to match the SDK's class class MockUnauthorizedError extends Error { @@ -122,10 +124,14 @@ test("first connect to OAuth server shows needs_auth instead of failed", async ( await Instance.provide({ directory: tmp.path, fn: async () => { - const result = await MCP.add("test-oauth", { - type: "remote", - url: "https://example.com/mcp", - }) + const result = await Effect.runPromise( + MCP.Service.use((mcp) => + mcp.add("test-oauth", { + type: "remote", + url: "https://example.com/mcp", + }), + ).pipe(Effect.provide(MCP.defaultLayer)), + ) const serverStatus = result.status as Record diff --git a/packages/opencode/test/mcp/oauth-browser.test.ts b/packages/opencode/test/mcp/oauth-browser.test.ts index ee4429be75..b6a32b1e1b 100644 --- a/packages/opencode/test/mcp/oauth-browser.test.ts +++ b/packages/opencode/test/mcp/oauth-browser.test.ts @@ -1,5 +1,7 @@ import { test, expect, mock, beforeEach } from "bun:test" import { EventEmitter } from "events" +import { Effect } from "effect" +import type { MCP as MCPNS } from "../../src/mcp/index" // Track open() calls and control failure behavior let openShouldFail = false @@ -100,10 +102,12 @@ beforeEach(() => { // Import modules after mocking const { MCP } = await import("../../src/mcp/index") +const { AppRuntime } = await import("../../src/effect/app-runtime") const { Bus } = await import("../../src/bus") const { McpOAuthCallback } = await import("../../src/mcp/oauth-callback") const { Instance } = await import("../../src/project/instance") const { tmpdir } = await import("../fixture/fixture") +const service = MCP.Service as unknown as Effect.Effect test("BrowserOpenFailed event is published when open() throws", async () => { await using tmp = await tmpdir({ @@ -136,7 +140,12 @@ test("BrowserOpenFailed event is published when open() throws", async () => { // Run authenticate with a timeout to avoid waiting forever for the callback // Attach a handler immediately so callback shutdown rejections // don't show up as unhandled between tests. - const authPromise = MCP.authenticate("test-oauth-server").catch(() => undefined) + const authPromise = AppRuntime.runPromise( + Effect.gen(function* () { + const mcp = yield* service + return yield* mcp.authenticate("test-oauth-server") + }), + ).catch(() => undefined) // Config.get() can be slow in tests, so give it plenty of time. await new Promise((resolve) => setTimeout(resolve, 2_000)) @@ -185,7 +194,12 @@ test("BrowserOpenFailed event is NOT published when open() succeeds", async () = }) // Run authenticate with a timeout to avoid waiting forever for the callback - const authPromise = MCP.authenticate("test-oauth-server-2").catch(() => undefined) + const authPromise = AppRuntime.runPromise( + Effect.gen(function* () { + const mcp = yield* service + return yield* mcp.authenticate("test-oauth-server-2") + }), + ).catch(() => undefined) // Config.get() can be slow in tests; also covers the ~500ms open() error-detection window. await new Promise((resolve) => setTimeout(resolve, 2_000)) @@ -230,7 +244,12 @@ test("open() is called with the authorization URL", async () => { openCalledWith = undefined // Run authenticate with a timeout to avoid waiting forever for the callback - const authPromise = MCP.authenticate("test-oauth-server-3").catch(() => undefined) + const authPromise = AppRuntime.runPromise( + Effect.gen(function* () { + const mcp = yield* service + return yield* mcp.authenticate("test-oauth-server-3") + }), + ).catch(() => undefined) // Config.get() can be slow in tests; also covers the ~500ms open() error-detection window. await new Promise((resolve) => setTimeout(resolve, 2_000))