feat(mcp): cache tool definitions, refresh via ToolListChangedNotification

tools() no longer calls listTools() on every server each turn.
Instead, tool definitions are fetched once at connect time and
cached in state. The ToolListChangedNotification handler refreshes
the cache when a server reports its tools have changed.

- Add defs to State, populated by create() and createAndStore()
- watch() registers per-client notification handler with staleness guards
- tools() reads from s.defs (zero network calls on hot path)
- closeClient() clears defs to keep cache and client lifecycle in sync
- Close old client on ALL failure paths, not just disabled
- Tests verify caching (listToolsCalls stays at 1) and notification refresh

packages/opencode/specs/effect-migration.md

@@ -175,4 +175,4 @@ Still open and likely worth migrating:
 - [ ] `Provider`
 - [x] `Project`
 - [ ] `LSP`
-- [ ] `MCP`
+- [x] `MCP`

packages/opencode/src/mcp/auth.ts

@@ -1,7 +1,9 @@
 import path from "path"
 import z from "zod"
 import { Global } from "../global"
-import { Filesystem } from "../util/filesystem"
+import { Effect, Layer, ServiceMap } from "effect"
+import { AppFileSystem } from "@/filesystem"
+import { makeRunPromise } from "@/effect/run-service"
 
 export namespace McpAuth {
   export const Tokens = z.object({
@@ -25,106 +27,183 @@ export namespace McpAuth {
     clientInfo: ClientInfo.optional(),
     codeVerifier: z.string().optional(),
     oauthState: z.string().optional(),
-    serverUrl: z.string().optional(), // Track the URL these credentials are for
+    serverUrl: z.string().optional(),
   })
   export type Entry = z.infer<typeof Entry>
 
   const filepath = path.join(Global.Path.data, "mcp-auth.json")
 
-  export async function get(mcpName: string): Promise<Entry | undefined> {
-    const data = await all()
-    return data[mcpName]
+  export interface Interface {
+    readonly all: () => Effect.Effect<Record<string, Entry>>
+    readonly get: (mcpName: string) => Effect.Effect<Entry | undefined>
+    readonly getForUrl: (mcpName: string, serverUrl: string) => Effect.Effect<Entry | undefined>
+    readonly set: (mcpName: string, entry: Entry, serverUrl?: string) => Effect.Effect<void>
+    readonly remove: (mcpName: string) => Effect.Effect<void>
+    readonly updateTokens: (mcpName: string, tokens: Tokens, serverUrl?: string) => Effect.Effect<void>
+    readonly updateClientInfo: (mcpName: string, clientInfo: ClientInfo, serverUrl?: string) => Effect.Effect<void>
+    readonly updateCodeVerifier: (mcpName: string, codeVerifier: string) => Effect.Effect<void>
+    readonly clearCodeVerifier: (mcpName: string) => Effect.Effect<void>
+    readonly updateOAuthState: (mcpName: string, oauthState: string) => Effect.Effect<void>
+    readonly getOAuthState: (mcpName: string) => Effect.Effect<string | undefined>
+    readonly clearOAuthState: (mcpName: string) => Effect.Effect<void>
+    readonly isTokenExpired: (mcpName: string) => Effect.Effect<boolean | null>
   }
 
-  /**
-   * Get auth entry and validate it's for the correct URL.
-   * Returns undefined if URL has changed (credentials are invalid).
-   */
-  export async function getForUrl(mcpName: string, serverUrl: string): Promise<Entry | undefined> {
-    const entry = await get(mcpName)
-    if (!entry) return undefined
+  export class Service extends ServiceMap.Service<Service, Interface>()("@opencode/McpAuth") {}
 
-    // If no serverUrl is stored, this is from an old version - consider it invalid
-    if (!entry.serverUrl) return undefined
+  export const layer = Layer.effect(
+    Service,
+    Effect.gen(function* () {
+      const fs = yield* AppFileSystem.Service
 
-    // If URL has changed, credentials are invalid
-    if (entry.serverUrl !== serverUrl) return undefined
+      const all = Effect.fn("McpAuth.all")(function* () {
+        return yield* fs.readJson(filepath).pipe(
+          Effect.map((data) => data as Record<string, Entry>),
+          Effect.catch(() => Effect.succeed({} as Record<string, Entry>)),
+        )
+      })
 
-    return entry
-  }
+      const get = Effect.fn("McpAuth.get")(function* (mcpName: string) {
+        const data = yield* all()
+        return data[mcpName]
+      })
 
-  export async function all(): Promise<Record<string, Entry>> {
-    return Filesystem.readJson<Record<string, Entry>>(filepath).catch(() => ({}))
-  }
+      const getForUrl = Effect.fn("McpAuth.getForUrl")(function* (mcpName: string, serverUrl: string) {
+        const entry = yield* get(mcpName)
+        if (!entry) return undefined
+        if (!entry.serverUrl) return undefined
+        if (entry.serverUrl !== serverUrl) return undefined
+        return entry
+      })
 
-  export async function set(mcpName: string, entry: Entry, serverUrl?: string): Promise<void> {
-    const data = await all()
-    // Always update serverUrl if provided
-    if (serverUrl) {
-      entry.serverUrl = serverUrl
-    }
-    await Filesystem.writeJson(filepath, { ...data, [mcpName]: entry }, 0o600)
-  }
+      const set = Effect.fn("McpAuth.set")(function* (mcpName: string, entry: Entry, serverUrl?: string) {
+        const data = yield* all()
+        if (serverUrl) entry.serverUrl = serverUrl
+        yield* fs.writeJson(filepath, { ...data, [mcpName]: entry }, 0o600).pipe(Effect.orDie)
+      })
 
-  export async function remove(mcpName: string): Promise<void> {
-    const data = await all()
-    delete data[mcpName]
-    await Filesystem.writeJson(filepath, data, 0o600)
-  }
+      const remove = Effect.fn("McpAuth.remove")(function* (mcpName: string) {
+        const data = yield* all()
+        delete data[mcpName]
+        yield* fs.writeJson(filepath, data, 0o600).pipe(Effect.orDie)
+      })
 
-  export async function updateTokens(mcpName: string, tokens: Tokens, serverUrl?: string): Promise<void> {
-    const entry = (await get(mcpName)) ?? {}
-    entry.tokens = tokens
-    await set(mcpName, entry, serverUrl)
-  }
+      const updateTokens = Effect.fn("McpAuth.updateTokens")(function* (
+        mcpName: string,
+        tokens: Tokens,
+        serverUrl?: string,
+      ) {
+        const entry = (yield* get(mcpName)) ?? {}
+        entry.tokens = tokens
+        yield* set(mcpName, entry, serverUrl)
+      })
 
-  export async function updateClientInfo(mcpName: string, clientInfo: ClientInfo, serverUrl?: string): Promise<void> {
-    const entry = (await get(mcpName)) ?? {}
-    entry.clientInfo = clientInfo
-    await set(mcpName, entry, serverUrl)
-  }
+      const updateClientInfo = Effect.fn("McpAuth.updateClientInfo")(function* (
+        mcpName: string,
+        clientInfo: ClientInfo,
+        serverUrl?: string,
+      ) {
+        const entry = (yield* get(mcpName)) ?? {}
+        entry.clientInfo = clientInfo
+        yield* set(mcpName, entry, serverUrl)
+      })
 
-  export async function updateCodeVerifier(mcpName: string, codeVerifier: string): Promise<void> {
-    const entry = (await get(mcpName)) ?? {}
-    entry.codeVerifier = codeVerifier
-    await set(mcpName, entry)
-  }
+      const updateCodeVerifier = Effect.fn("McpAuth.updateCodeVerifier")(function* (
+        mcpName: string,
+        codeVerifier: string,
+      ) {
+        const entry = (yield* get(mcpName)) ?? {}
+        entry.codeVerifier = codeVerifier
+        yield* set(mcpName, entry)
+      })
 
-  export async function clearCodeVerifier(mcpName: string): Promise<void> {
-    const entry = await get(mcpName)
-    if (entry) {
-      delete entry.codeVerifier
-      await set(mcpName, entry)
-    }
-  }
+      const clearCodeVerifier = Effect.fn("McpAuth.clearCodeVerifier")(function* (mcpName: string) {
+        const entry = yield* get(mcpName)
+        if (entry) {
+          delete entry.codeVerifier
+          yield* set(mcpName, entry)
+        }
+      })
 
-  export async function updateOAuthState(mcpName: string, oauthState: string): Promise<void> {
-    const entry = (await get(mcpName)) ?? {}
-    entry.oauthState = oauthState
-    await set(mcpName, entry)
-  }
+      const updateOAuthState = Effect.fn("McpAuth.updateOAuthState")(function* (mcpName: string, oauthState: string) {
+        const entry = (yield* get(mcpName)) ?? {}
+        entry.oauthState = oauthState
+        yield* set(mcpName, entry)
+      })
 
-  export async function getOAuthState(mcpName: string): Promise<string | undefined> {
-    const entry = await get(mcpName)
-    return entry?.oauthState
-  }
+      const getOAuthState = Effect.fn("McpAuth.getOAuthState")(function* (mcpName: string) {
+        const entry = yield* get(mcpName)
+        return entry?.oauthState
+      })
 
-  export async function clearOAuthState(mcpName: string): Promise<void> {
-    const entry = await get(mcpName)
-    if (entry) {
-      delete entry.oauthState
-      await set(mcpName, entry)
-    }
-  }
+      const clearOAuthState = Effect.fn("McpAuth.clearOAuthState")(function* (mcpName: string) {
+        const entry = yield* get(mcpName)
+        if (entry) {
+          delete entry.oauthState
+          yield* set(mcpName, entry)
+        }
+      })
 
-  /**
-   * Check if stored tokens are expired.
-   * Returns null if no tokens exist, false if no expiry or not expired, true if expired.
-   */
-  export async function isTokenExpired(mcpName: string): Promise<boolean | null> {
-    const entry = await get(mcpName)
-    if (!entry?.tokens) return null
-    if (!entry.tokens.expiresAt) return false
-    return entry.tokens.expiresAt < Date.now() / 1000
-  }
+      const isTokenExpired = Effect.fn("McpAuth.isTokenExpired")(function* (mcpName: string) {
+        const entry = yield* get(mcpName)
+        if (!entry?.tokens) return null
+        if (!entry.tokens.expiresAt) return false
+        return entry.tokens.expiresAt < Date.now() / 1000
+      })
+
+      return Service.of({
+        all,
+        get,
+        getForUrl,
+        set,
+        remove,
+        updateTokens,
+        updateClientInfo,
+        updateCodeVerifier,
+        clearCodeVerifier,
+        updateOAuthState,
+        getOAuthState,
+        clearOAuthState,
+        isTokenExpired,
+      })
+    }),
+  )
+
+  const defaultLayer = layer.pipe(Layer.provide(AppFileSystem.defaultLayer))
+
+  const runPromise = makeRunPromise(Service, defaultLayer)
+
+  // Async facades for backward compat (used by McpOAuthProvider, CLI)
+
+  export const get = async (mcpName: string) => runPromise((svc) => svc.get(mcpName))
+
+  export const getForUrl = async (mcpName: string, serverUrl: string) =>
+    runPromise((svc) => svc.getForUrl(mcpName, serverUrl))
+
+  export const all = async () => runPromise((svc) => svc.all())
+
+  export const set = async (mcpName: string, entry: Entry, serverUrl?: string) =>
+    runPromise((svc) => svc.set(mcpName, entry, serverUrl))
+
+  export const remove = async (mcpName: string) => runPromise((svc) => svc.remove(mcpName))
+
+  export const updateTokens = async (mcpName: string, tokens: Tokens, serverUrl?: string) =>
+    runPromise((svc) => svc.updateTokens(mcpName, tokens, serverUrl))
+
+  export const updateClientInfo = async (mcpName: string, clientInfo: ClientInfo, serverUrl?: string) =>
+    runPromise((svc) => svc.updateClientInfo(mcpName, clientInfo, serverUrl))
+
+  export const updateCodeVerifier = async (mcpName: string, codeVerifier: string) =>
+    runPromise((svc) => svc.updateCodeVerifier(mcpName, codeVerifier))
+
+  export const clearCodeVerifier = async (mcpName: string) => runPromise((svc) => svc.clearCodeVerifier(mcpName))
+
+  export const updateOAuthState = async (mcpName: string, oauthState: string) =>
+    runPromise((svc) => svc.updateOAuthState(mcpName, oauthState))
+
+  export const getOAuthState = async (mcpName: string) => runPromise((svc) => svc.getOAuthState(mcpName))
+
+  export const clearOAuthState = async (mcpName: string) => runPromise((svc) => svc.clearOAuthState(mcpName))
+
+  export const isTokenExpired = async (mcpName: string) => runPromise((svc) => svc.isTokenExpired(mcpName))
 }

packages/opencode/src/mcp/oauth-callback.ts

@@ -54,6 +54,9 @@ interface PendingAuth {
 export namespace McpOAuthCallback {
   let server: ReturnType<typeof Bun.serve> | undefined
   const pendingAuths = new Map<string, PendingAuth>()
+  // Reverse index: mcpName → oauthState, so cancelPending(mcpName) can
+  // find the right entry in pendingAuths (which is keyed by oauthState).
+  const mcpNameToState = new Map<string, string>()
 
   const CALLBACK_TIMEOUT_MS = 5 * 60 * 1000 // 5 minutes
 
@@ -98,6 +101,12 @@ export namespace McpOAuthCallback {
             const pending = pendingAuths.get(state)!
             clearTimeout(pending.timeout)
             pendingAuths.delete(state)
+            for (const [name, s] of mcpNameToState) {
+              if (s === state) {
+                mcpNameToState.delete(name)
+                break
+              }
+            }
             pending.reject(new Error(errorMsg))
           }
           return new Response(HTML_ERROR(errorMsg), {
@@ -126,6 +135,13 @@ export namespace McpOAuthCallback {
 
         clearTimeout(pending.timeout)
         pendingAuths.delete(state)
+        // Clean up reverse index
+        for (const [name, s] of mcpNameToState) {
+          if (s === state) {
+            mcpNameToState.delete(name)
+            break
+          }
+        }
         pending.resolve(code)
 
         return new Response(HTML_SUCCESS, {
@@ -137,11 +153,13 @@ export namespace McpOAuthCallback {
     log.info("oauth callback server started", { port: OAUTH_CALLBACK_PORT })
   }
 
-  export function waitForCallback(oauthState: string): Promise<string> {
+  export function waitForCallback(oauthState: string, mcpName?: string): Promise<string> {
+    if (mcpName) mcpNameToState.set(mcpName, oauthState)
     return new Promise((resolve, reject) => {
       const timeout = setTimeout(() => {
         if (pendingAuths.has(oauthState)) {
           pendingAuths.delete(oauthState)
+          if (mcpName) mcpNameToState.delete(mcpName)
           reject(new Error("OAuth callback timeout - authorization took too long"))
         }
       }, CALLBACK_TIMEOUT_MS)
@@ -151,10 +169,14 @@ export namespace McpOAuthCallback {
   }
 
   export function cancelPending(mcpName: string): void {
-    const pending = pendingAuths.get(mcpName)
+    // Look up the oauthState for this mcpName via the reverse index
+    const oauthState = mcpNameToState.get(mcpName)
+    const key = oauthState ?? mcpName
+    const pending = pendingAuths.get(key)
     if (pending) {
       clearTimeout(pending.timeout)
-      pendingAuths.delete(mcpName)
+      pendingAuths.delete(key)
+      mcpNameToState.delete(mcpName)
       pending.reject(new Error("Authorization cancelled"))
     }
   }
@@ -184,6 +206,7 @@ export namespace McpOAuthCallback {
       pending.reject(new Error("OAuth callback server stopped"))
     }
     pendingAuths.clear()
+    mcpNameToState.clear()
   }
 
   export function isRunning(): boolean {

packages/opencode/test/mcp/lifecycle.test.ts

@@ -0,0 +1,660 @@
+import { test, expect, mock, beforeEach } from "bun:test"
+
+// --- Mock infrastructure ---
+
+// Per-client state for controlling mock behavior
+interface MockClientState {
+  tools: Array<{ name: string; description?: string; inputSchema: object }>
+  listToolsCalls: number
+  listToolsShouldFail: boolean
+  listToolsError: string
+  listPromptsShouldFail: boolean
+  listResourcesShouldFail: boolean
+  prompts: Array<{ name: string; description?: string }>
+  resources: Array<{ name: string; uri: string; description?: string }>
+  closed: boolean
+  notificationHandlers: Map<unknown, (...args: any[]) => any>
+}
+
+const clientStates = new Map<string, MockClientState>()
+let lastCreatedClientName: string | undefined
+let connectShouldFail = false
+let connectError = "Mock transport cannot connect"
+// Tracks how many Client instances were created (detects leaks)
+let clientCreateCount = 0
+
+function getOrCreateClientState(name?: string): MockClientState {
+  const key = name ?? "default"
+  let state = clientStates.get(key)
+  if (!state) {
+    state = {
+      tools: [{ name: "test_tool", description: "A test tool", inputSchema: { type: "object", properties: {} } }],
+      listToolsCalls: 0,
+      listToolsShouldFail: false,
+      listToolsError: "listTools failed",
+      listPromptsShouldFail: false,
+      listResourcesShouldFail: false,
+      prompts: [],
+      resources: [],
+      closed: false,
+      notificationHandlers: new Map(),
+    }
+    clientStates.set(key, state)
+  }
+  return state
+}
+
+// Mock transport that succeeds or fails based on connectShouldFail
+class MockStdioTransport {
+  stderr: null = null
+  pid = 12345
+  constructor(_opts: any) {}
+  async start() {
+    if (connectShouldFail) throw new Error(connectError)
+  }
+  async close() {}
+}
+
+class MockStreamableHTTP {
+  constructor(_url: URL, _opts?: any) {}
+  async start() {
+    if (connectShouldFail) throw new Error(connectError)
+  }
+  async close() {}
+  async finishAuth() {}
+}
+
+class MockSSE {
+  constructor(_url: URL, _opts?: any) {}
+  async start() {
+    throw new Error("SSE fallback - not used in these tests")
+  }
+  async close() {}
+}
+
+mock.module("@modelcontextprotocol/sdk/client/stdio.js", () => ({
+  StdioClientTransport: MockStdioTransport,
+}))
+
+mock.module("@modelcontextprotocol/sdk/client/streamableHttp.js", () => ({
+  StreamableHTTPClientTransport: MockStreamableHTTP,
+}))
+
+mock.module("@modelcontextprotocol/sdk/client/sse.js", () => ({
+  SSEClientTransport: MockSSE,
+}))
+
+mock.module("@modelcontextprotocol/sdk/client/auth.js", () => ({
+  UnauthorizedError: class extends Error {
+    constructor() {
+      super("Unauthorized")
+    }
+  },
+}))
+
+// Mock Client that delegates to per-name MockClientState
+mock.module("@modelcontextprotocol/sdk/client/index.js", () => ({
+  Client: class MockClient {
+    _state!: MockClientState
+    transport: any
+
+    constructor(_opts: any) {
+      clientCreateCount++
+    }
+
+    async connect(transport: { start: () => Promise<void> }) {
+      this.transport = transport
+      await transport.start()
+      // After successful connect, bind to the last-created client name
+      this._state = getOrCreateClientState(lastCreatedClientName)
+    }
+
+    setNotificationHandler(schema: unknown, handler: (...args: any[]) => any) {
+      this._state?.notificationHandlers.set(schema, handler)
+    }
+
+    async listTools() {
+      if (this._state) this._state.listToolsCalls++
+      if (this._state?.listToolsShouldFail) {
+        throw new Error(this._state.listToolsError)
+      }
+      return { tools: this._state?.tools ?? [] }
+    }
+
+    async listPrompts() {
+      if (this._state?.listPromptsShouldFail) {
+        throw new Error("listPrompts failed")
+      }
+      return { prompts: this._state?.prompts ?? [] }
+    }
+
+    async listResources() {
+      if (this._state?.listResourcesShouldFail) {
+        throw new Error("listResources failed")
+      }
+      return { resources: this._state?.resources ?? [] }
+    }
+
+    async close() {
+      if (this._state) this._state.closed = true
+    }
+  },
+}))
+
+beforeEach(() => {
+  clientStates.clear()
+  lastCreatedClientName = undefined
+  connectShouldFail = false
+  connectError = "Mock transport cannot connect"
+  clientCreateCount = 0
+})
+
+// Import after mocks
+const { MCP } = await import("../../src/mcp/index")
+const { Instance } = await import("../../src/project/instance")
+const { tmpdir } = await import("../fixture/fixture")
+
+// --- Helper ---
+
+function withInstance(config: Record<string, any>, fn: () => Promise<void>) {
+  return async () => {
+    await using tmp = await tmpdir({
+      init: async (dir) => {
+        await Bun.write(
+          `${dir}/opencode.json`,
+          JSON.stringify({
+            $schema: "https://opencode.ai/config.json",
+            mcp: config,
+          }),
+        )
+      },
+    })
+
+    await Instance.provide({
+      directory: tmp.path,
+      fn: async () => {
+        await fn()
+        // dispose instance to clean up state between tests
+        await Instance.dispose()
+      },
+    })
+  }
+}
+
+// ========================================================================
+// Test: tools() are cached after connect
+// ========================================================================
+
+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: {} } },
+    ]
+
+    // 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")
+
+    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)
+  }),
+)
+
+// ========================================================================
+// Test: tool change notifications refresh the cache
+// ========================================================================
+
+test(
+  "tool change notifications refresh cached tool definitions",
+  withInstance({}, async () => {
+    lastCreatedClientName = "status-server"
+    const serverState = getOrCreateClientState("status-server")
+
+    await 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)
+
+    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 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)
+  }),
+)
+
+// ========================================================================
+// Test: connect() / disconnect() lifecycle
+// ========================================================================
+
+test(
+  "disconnect sets status to disabled and removes client",
+  withInstance(
+    {
+      "disc-server": {
+        type: "local",
+        command: ["echo", "test"],
+      },
+    },
+    async () => {
+      lastCreatedClientName = "disc-server"
+      getOrCreateClientState("disc-server")
+
+      await MCP.add("disc-server", {
+        type: "local",
+        command: ["echo", "test"],
+      })
+
+      const statusBefore = await MCP.status()
+      expect(statusBefore["disc-server"]?.status).toBe("connected")
+
+      await MCP.disconnect("disc-server")
+
+      const statusAfter = await 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)
+    },
+  ),
+)
+
+test(
+  "connect() after disconnect() re-establishes the server",
+  withInstance(
+    {
+      "reconn-server": {
+        type: "local",
+        command: ["echo", "test"],
+      },
+    },
+    async () => {
+      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"],
+      })
+
+      await MCP.disconnect("reconn-server")
+      expect((await MCP.status())["reconn-server"]?.status).toBe("disabled")
+
+      // Reconnect
+      await MCP.connect("reconn-server")
+      expect((await MCP.status())["reconn-server"]?.status).toBe("connected")
+
+      const tools = await MCP.tools()
+      expect(Object.keys(tools).some((k) => k.includes("my_tool"))).toBe(true)
+    },
+  ),
+)
+
+// ========================================================================
+// Test: add() closes existing client before replacing
+// ========================================================================
+
+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")
+
+    await MCP.add("replace-server", {
+      type: "local",
+      command: ["echo", "test"],
+    })
+
+    expect(firstState.closed).toBe(false)
+
+    // 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"],
+    })
+
+    expect(firstState.closed).toBe(true)
+    expect(secondState.closed).toBe(false)
+  }),
+)
+
+// ========================================================================
+// Test: state init with mixed success/failure
+// ========================================================================
+
+test(
+  "init connects available servers even when one fails",
+  withInstance(
+    {
+      "good-server": {
+        type: "local",
+        command: ["echo", "good"],
+      },
+      "bad-server": {
+        type: "local",
+        command: ["echo", "bad"],
+      },
+    },
+    async () => {
+      // 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
+
+      // Add good server first
+      lastCreatedClientName = "good-server"
+      await 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"],
+      })
+
+      const status = await 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)
+    },
+  ),
+)
+
+// ========================================================================
+// Test: disabled server via config
+// ========================================================================
+
+test(
+  "disabled server is marked as disabled without attempting connection",
+  withInstance(
+    {
+      "disabled-server": {
+        type: "local",
+        command: ["echo", "test"],
+        enabled: false,
+      },
+    },
+    async () => {
+      const countBefore = clientCreateCount
+
+      await MCP.add("disabled-server", {
+        type: "local",
+        command: ["echo", "test"],
+        enabled: false,
+      } as any)
+
+      // No client should have been created
+      expect(clientCreateCount).toBe(countBefore)
+
+      const status = await MCP.status()
+      expect(status["disabled-server"]?.status).toBe("disabled")
+    },
+  ),
+)
+
+// ========================================================================
+// Test: prompts() and resources()
+// ========================================================================
+
+test(
+  "prompts() returns prompts from connected servers",
+  withInstance(
+    {
+      "prompt-server": {
+        type: "local",
+        command: ["echo", "test"],
+      },
+    },
+    async () => {
+      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"],
+      })
+
+      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")
+    },
+  ),
+)
+
+test(
+  "resources() returns resources from connected servers",
+  withInstance(
+    {
+      "resource-server": {
+        type: "local",
+        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" }]
+
+      await 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")
+    },
+  ),
+)
+
+test(
+  "prompts() skips disconnected servers",
+  withInstance(
+    {
+      "prompt-disc-server": {
+        type: "local",
+        command: ["echo", "test"],
+      },
+    },
+    async () => {
+      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"],
+      })
+
+      await MCP.disconnect("prompt-disc-server")
+
+      const prompts = await MCP.prompts()
+      expect(Object.keys(prompts).length).toBe(0)
+    },
+  ),
+)
+
+// ========================================================================
+// Test: connect() on nonexistent server
+// ========================================================================
+
+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()
+  }),
+)
+
+// ========================================================================
+// Test: disconnect() on nonexistent server
+// ========================================================================
+
+test(
+  "disconnect() on nonexistent server does not throw",
+  withInstance({}, async () => {
+    await MCP.disconnect("nonexistent")
+    // Should complete without error
+  }),
+)
+
+// ========================================================================
+// Test: tools() with no MCP servers configured
+// ========================================================================
+
+test(
+  "tools() returns empty when no MCP servers are configured",
+  withInstance({}, async () => {
+    const tools = await MCP.tools()
+    expect(Object.keys(tools).length).toBe(0)
+  }),
+)
+
+// ========================================================================
+// Test: connect failure during create()
+// ========================================================================
+
+test(
+  "server that fails to connect is marked as failed",
+  withInstance(
+    {
+      "fail-connect": {
+        type: "local",
+        command: ["echo", "test"],
+      },
+    },
+    async () => {
+      lastCreatedClientName = "fail-connect"
+      getOrCreateClientState("fail-connect")
+      connectShouldFail = true
+      connectError = "Connection refused"
+
+      await 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")
+      }
+
+      // No tools should be available
+      const tools = await MCP.tools()
+      expect(Object.keys(tools).length).toBe(0)
+    },
+  ),
+)
+
+// ========================================================================
+// Bug #5: McpOAuthCallback.cancelPending uses wrong key
+// ========================================================================
+
+test("McpOAuthCallback.cancelPending is keyed by mcpName but pendingAuths uses oauthState", async () => {
+  const { McpOAuthCallback } = await import("../../src/mcp/oauth-callback")
+
+  // Register a pending auth with an oauthState key, associated to an mcpName
+  const oauthState = "abc123hexstate"
+  const callbackPromise = McpOAuthCallback.waitForCallback(oauthState, "my-mcp-server")
+
+  // cancelPending is called with mcpName — should find the entry via reverse index
+  McpOAuthCallback.cancelPending("my-mcp-server")
+
+  // The callback should still be pending because cancelPending looked up
+  // "my-mcp-server" in a map keyed by "abc123hexstate"
+  let resolved = false
+  let rejected = false
+  callbackPromise.then(() => (resolved = true)).catch(() => (rejected = true))
+
+  // Give it a tick
+  await new Promise((r) => setTimeout(r, 50))
+
+  // cancelPending("my-mcp-server") should have rejected the pending callback
+  expect(rejected).toBe(true)
+
+  await McpOAuthCallback.stop()
+})
+
+// ========================================================================
+// Test: multiple tools from same server get correct name prefixes
+// ========================================================================
+
+test(
+  "tools() prefixes tool names with sanitized server name",
+  withInstance(
+    {
+      "my.special-server": {
+        type: "local",
+        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: {} } },
+      ]
+
+      await MCP.add("my.special-server", {
+        type: "local",
+        command: ["echo", "test"],
+      })
+
+      const tools = await 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)
+    },
+  ),
+)