fix(mcp): stop idle OAuth callback server

packages/opencode/src/mcp/index.ts

@@ -745,7 +745,7 @@ export const layer = Layer.effect(
       return mcpConfig
     })
 
-    const startAuth = Effect.fn("MCP.startAuth")(function* (mcpName: string) {
+    const startAuth = Effect.fn("MCP.startAuth")(function* (mcpName: string, pendingState?: string) {
       const mcpConfig = yield* requireMcpConfig(mcpName)
       if (mcpConfig.type !== "remote") throw new Error(`MCP server ${mcpName} is not a remote server`)
       if (mcpConfig.oauth === false) throw new Error(`MCP server ${mcpName} has OAuth explicitly disabled`)
@@ -763,9 +763,11 @@ export const layer = Layer.effect(
       // Start the callback server with custom redirectUri if configured
       yield* Effect.promise(() => McpOAuthCallback.ensureRunning(effectiveRedirectUri))
 
-      const oauthState = Array.from(crypto.getRandomValues(new Uint8Array(32)))
-        .map((b) => b.toString(16).padStart(2, "0"))
-        .join("")
+      const oauthState =
+        pendingState ??
+        Array.from(crypto.getRandomValues(new Uint8Array(32)))
+          .map((b) => b.toString(16).padStart(2, "0"))
+          .join("")
       yield* auth.updateOAuthState(mcpName, oauthState)
       let capturedUrl: URL | undefined
       const authProvider = new McpOAuthProvider(
@@ -811,60 +813,66 @@ export const layer = Layer.effect(
     })
 
     const authenticate = Effect.fn("MCP.authenticate")(function* (mcpName: string) {
-      const result = yield* startAuth(mcpName)
-      if (!result.authorizationUrl) {
-        const client = "client" in result ? result.client : undefined
-        const mcpConfig = yield* requireMcpConfig(mcpName).pipe(
-          Effect.tapError(() => Effect.tryPromise(() => client?.close() ?? Promise.resolve()).pipe(Effect.ignore)),
-        )
+      const oauthState = Array.from(crypto.getRandomValues(new Uint8Array(32)))
+        .map((b) => b.toString(16).padStart(2, "0"))
+        .join("")
+      const callbackPromise = McpOAuthCallback.waitForCallback(oauthState, mcpName)
+      void callbackPromise.catch(() => {})
 
-        const listed = client
-          ? client.getServerCapabilities()?.tools
-            ? yield* McpCatalog.defs(client, mcpConfig.timeout)
-            : []
-          : undefined
-        if (!client || !listed) {
-          yield* Effect.tryPromise(() => client?.close() ?? Promise.resolve()).pipe(Effect.ignore)
-          return { status: "failed", error: "Failed to get tools" } satisfies Status
+      return yield* Effect.gen(function* () {
+        const result = yield* startAuth(mcpName, oauthState)
+        if (!result.authorizationUrl) {
+          const client = "client" in result ? result.client : undefined
+          const mcpConfig = yield* requireMcpConfig(mcpName).pipe(
+            Effect.tapError(() => Effect.tryPromise(() => client?.close() ?? Promise.resolve()).pipe(Effect.ignore)),
+          )
+
+          const listed = client
+            ? client.getServerCapabilities()?.tools
+              ? yield* McpCatalog.defs(client, mcpConfig.timeout)
+              : []
+            : undefined
+          if (!client || !listed) {
+            yield* Effect.tryPromise(() => client?.close() ?? Promise.resolve()).pipe(Effect.ignore)
+            return { status: "failed", error: "Failed to get tools" } satisfies Status
+          }
+
+          const s = yield* InstanceState.get(state)
+          yield* auth.clearOAuthState(mcpName)
+          return yield* storeClient(s, mcpName, client, listed, mcpConfig.timeout)
         }
 
-        const s = yield* InstanceState.get(state)
-        yield* auth.clearOAuthState(mcpName)
-        return yield* storeClient(s, mcpName, client, listed, mcpConfig.timeout)
-      }
-
-      const callbackPromise = McpOAuthCallback.waitForCallback(result.oauthState, mcpName)
-
-      yield* Effect.tryPromise(() => open(result.authorizationUrl)).pipe(
-        Effect.flatMap((subprocess) =>
-          Effect.callback<void, Error>((resume) => {
-            const timer = setTimeout(() => resume(Effect.void), 500)
-            subprocess.on("error", (err) => {
-              clearTimeout(timer)
-              resume(Effect.fail(err))
-            })
-            subprocess.on("exit", (code) => {
-              if (code !== null && code !== 0) {
+        yield* Effect.tryPromise(() => open(result.authorizationUrl)).pipe(
+          Effect.flatMap((subprocess) =>
+            Effect.callback<void, Error>((resume) => {
+              const timer = setTimeout(() => resume(Effect.void), 500)
+              subprocess.on("error", (err) => {
                 clearTimeout(timer)
-                resume(Effect.fail(new Error(`Browser open failed with exit code ${code}`)))
-              }
-            })
+                resume(Effect.fail(err))
+              })
+              subprocess.on("exit", (code) => {
+                if (code !== null && code !== 0) {
+                  clearTimeout(timer)
+                  resume(Effect.fail(new Error(`Browser open failed with exit code ${code}`)))
+                }
+              })
+            }),
+          ),
+          Effect.catch(() => {
+            return events.publish(BrowserOpenFailed, { mcpName, url: result.authorizationUrl }).pipe(Effect.ignore)
           }),
-        ),
-        Effect.catch(() => {
-          return events.publish(BrowserOpenFailed, { mcpName, url: result.authorizationUrl }).pipe(Effect.ignore)
-        }),
-      )
+        )
 
-      const code = yield* Effect.promise(() => callbackPromise)
+        const code = yield* Effect.promise(() => callbackPromise)
 
-      const storedState = yield* auth.getOAuthState(mcpName)
-      if (storedState !== result.oauthState) {
+        const storedState = yield* auth.getOAuthState(mcpName)
+        if (storedState !== result.oauthState) {
+          yield* auth.clearOAuthState(mcpName)
+          throw new Error("OAuth state mismatch - potential CSRF attack")
+        }
         yield* auth.clearOAuthState(mcpName)
-        throw new Error("OAuth state mismatch - potential CSRF attack")
-      }
-      yield* auth.clearOAuthState(mcpName)
-      return yield* finishAuth(mcpName, code)
+        return yield* finishAuth(mcpName, code)
+      }).pipe(Effect.ensuring(Effect.sync(() => McpOAuthCallback.cancelPending(mcpName))))
     })
 
     const finishAuth = Effect.fn("MCP.finishAuth")(function* (mcpName: string, authorizationCode: string) {

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

@@ -54,6 +54,9 @@ interface PendingAuth {
 }
 
 let server: ReturnType<typeof createServer> | undefined
+let starting: Promise<void> | undefined
+let stopping: Promise<void> | undefined
+let idleStopRequested = false
 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).
@@ -70,6 +73,23 @@ function cleanupStateIndex(oauthState: string) {
   }
 }
 
+function stopIfIdle() {
+  if (pendingAuths.size > 0) return
+  if (starting || stopping) {
+    idleStopRequested = true
+    return
+  }
+  if (!server) return
+
+  const current = server
+  server = undefined
+  idleStopRequested = false
+  stopping = new Promise<void>((resolve) => current.close(() => resolve())).finally(() => {
+    stopping = undefined
+    if (idleStopRequested) stopIfIdle()
+  })
+}
+
 function handleRequest(req: import("http").IncomingMessage, res: import("http").ServerResponse) {
   const url = new URL(req.url || "/", `http://localhost:${currentPort}`)
 
@@ -103,6 +123,7 @@ function handleRequest(req: import("http").IncomingMessage, res: import("http").
     }
     res.writeHead(200, { "Content-Type": "text/html" })
     res.end(HTML_ERROR(errorMsg))
+    stopIfIdle()
     return
   }
 
@@ -129,12 +150,19 @@ function handleRequest(req: import("http").IncomingMessage, res: import("http").
 
   res.writeHead(200, { "Content-Type": "text/html" })
   res.end(HTML_SUCCESS)
+  stopIfIdle()
 }
 
 export async function ensureRunning(redirectUri?: string): Promise<void> {
   // Parse the redirect URI to get port and path (uses defaults if not provided)
   const { port, path } = parseRedirectUri(redirectUri)
 
+  if (stopping) await stopping
+  if (starting) {
+    await starting
+    return ensureRunning(redirectUri)
+  }
+
   // If server is running on a different port/path, stop it first
   if (server && (currentPort !== port || currentPath !== path)) {
     await stop()
@@ -142,24 +170,31 @@ export async function ensureRunning(redirectUri?: string): Promise<void> {
 
   if (server) return
 
-  const running = await isPortInUse(port)
-  if (running) {
-    return
-  }
+  starting = (async () => {
+    if (await isPortInUse(port)) return
 
-  currentPort = port
-  currentPath = path
+    currentPort = port
+    currentPath = path
 
-  server = createServer(handleRequest)
-  await new Promise<void>((resolve, reject) => {
-    server!.listen(currentPort, () => {
-      resolve()
+    const next = createServer(handleRequest)
+    await new Promise<void>((resolve, reject) => {
+      next.listen(currentPort, () => resolve())
+      next.on("error", reject)
     })
-    server!.on("error", reject)
+    server = next
+  })()
+  await starting.finally(() => {
+    starting = undefined
   })
+  if (idleStopRequested) stopIfIdle()
 }
 
-export function waitForCallback(oauthState: string, mcpName?: string): Promise<string> {
+export function waitForCallback(
+  oauthState: string,
+  mcpName?: string,
+  timeoutMs: number = CALLBACK_TIMEOUT_MS,
+): Promise<string> {
+  idleStopRequested = false
   if (mcpName) mcpNameToState.set(mcpName, oauthState)
   return new Promise((resolve, reject) => {
     const timeout = setTimeout(() => {
@@ -167,8 +202,9 @@ export function waitForCallback(oauthState: string, mcpName?: string): Promise<s
         pendingAuths.delete(oauthState)
         if (mcpName) mcpNameToState.delete(mcpName)
         reject(new Error("OAuth callback timeout - authorization took too long"))
+        stopIfIdle()
       }
-    }, CALLBACK_TIMEOUT_MS)
+    }, timeoutMs)
 
     pendingAuths.set(oauthState, { resolve, reject, timeout })
   })
@@ -184,6 +220,7 @@ export function cancelPending(mcpName: string): void {
     pendingAuths.delete(key)
     mcpNameToState.delete(mcpName)
     pending.reject(new Error("Authorization cancelled"))
+    stopIfIdle()
   }
 }
 
@@ -201,9 +238,13 @@ export async function isPortInUse(port: number = OAUTH_CALLBACK_PORT): Promise<b
 }
 
 export async function stop(): Promise<void> {
+  if (starting) await starting
+  if (stopping) await stopping
+
   if (server) {
-    await new Promise<void>((resolve) => server!.close(() => resolve()))
+    const current = server
     server = undefined
+    await new Promise<void>((resolve) => current.close(() => resolve()))
   }
 
   for (const [_name, pending] of pendingAuths) {
@@ -212,6 +253,7 @@ export async function stop(): Promise<void> {
   }
   pendingAuths.clear()
   mcpNameToState.clear()
+  idleStopRequested = false
 }
 
 export function isRunning(): boolean {

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

@@ -1,4 +1,5 @@
 import { test, expect, describe, afterEach } from "bun:test"
+import { createServer } from "http"
 import { McpOAuthCallback } from "../../src/mcp/oauth-callback"
 import { parseRedirectUri } from "../../src/mcp/oauth-provider"
 
@@ -32,3 +33,117 @@ describe("McpOAuthCallback.ensureRunning", () => {
     expect(McpOAuthCallback.isRunning()).toBe(true)
   })
 })
+
+describe("McpOAuthCallback lifecycle", () => {
+  afterEach(async () => {
+    await McpOAuthCallback.stop()
+  })
+
+  test("stops after a successful callback response completes", async () => {
+    const port = await availablePort()
+    await McpOAuthCallback.ensureRunning(`http://127.0.0.1:${port}/callback`)
+    const callback = McpOAuthCallback.waitForCallback("success")
+
+    const response = await fetch(`http://127.0.0.1:${port}/callback?code=code&state=success`)
+
+    expect(response.status).toBe(200)
+    expect(await callback).toBe("code")
+    await waitForStop()
+  })
+
+  test("stops after a provider error", async () => {
+    const port = await availablePort()
+    await McpOAuthCallback.ensureRunning(`http://127.0.0.1:${port}/callback`)
+    const callback = McpOAuthCallback.waitForCallback("provider-error")
+    const error = callback.catch((cause) => cause)
+
+    const response = await fetch(
+      `http://127.0.0.1:${port}/callback?error=access_denied&error_description=Denied&state=provider-error`,
+    )
+
+    expect(response.status).toBe(200)
+    await response.text()
+    expect(await error).toEqual(new Error("Denied"))
+    await waitForStop()
+  })
+
+  test("stops after cancellation", async () => {
+    const port = await availablePort()
+    await McpOAuthCallback.ensureRunning(`http://127.0.0.1:${port}/callback`)
+    const callback = McpOAuthCallback.waitForCallback("cancelled", "server")
+
+    McpOAuthCallback.cancelPending("server")
+
+    await expect(callback).rejects.toThrow("Authorization cancelled")
+    await waitForStop()
+  })
+
+  test("stops when cancellation races listener startup", async () => {
+    const port = await availablePort()
+    const callback = McpOAuthCallback.waitForCallback("cancelled", "starting-server")
+    const starting = McpOAuthCallback.ensureRunning(`http://127.0.0.1:${port}/callback`)
+
+    McpOAuthCallback.cancelPending("starting-server")
+
+    await expect(callback).rejects.toThrow("Authorization cancelled")
+    await starting
+    await waitForStop()
+  })
+
+  test("stops after timeout", async () => {
+    const port = await availablePort()
+    await McpOAuthCallback.ensureRunning(`http://127.0.0.1:${port}/callback`)
+    const callback = McpOAuthCallback.waitForCallback("timeout", undefined, 10)
+
+    await expect(callback).rejects.toThrow("OAuth callback timeout")
+    await waitForStop()
+  })
+
+  test("stays running until all simultaneous callbacks settle", async () => {
+    const port = await availablePort()
+    const redirectUri = `http://127.0.0.1:${port}/callback`
+    await Promise.all([McpOAuthCallback.ensureRunning(redirectUri), McpOAuthCallback.ensureRunning(redirectUri)])
+    const first = McpOAuthCallback.waitForCallback("first")
+    const second = McpOAuthCallback.waitForCallback("second")
+
+    await fetch(`${redirectUri}?code=one&state=first`)
+    expect(await first).toBe("one")
+    expect(McpOAuthCallback.isRunning()).toBe(true)
+
+    await fetch(`${redirectUri}?code=two&state=second`)
+    expect(await second).toBe("two")
+    await waitForStop()
+  })
+
+  test("restarts when a callback is registered during shutdown", async () => {
+    const port = await availablePort()
+    const redirectUri = `http://127.0.0.1:${port}/callback`
+    await McpOAuthCallback.ensureRunning(redirectUri)
+    const first = McpOAuthCallback.waitForCallback("first")
+
+    await fetch(`${redirectUri}?code=one&state=first`)
+    expect(await first).toBe("one")
+
+    const second = McpOAuthCallback.waitForCallback("second")
+    await McpOAuthCallback.ensureRunning(redirectUri)
+    const response = await fetch(`${redirectUri}?code=two&state=second`)
+
+    expect(response.status).toBe(200)
+    expect(await second).toBe("two")
+    await waitForStop()
+  })
+})
+
+async function availablePort() {
+  const probe = createServer()
+  await new Promise<void>((resolve) => probe.listen(0, "127.0.0.1", resolve))
+  const address = probe.address()
+  if (!address || typeof address === "string") throw new Error("Failed to allocate callback test port")
+  await new Promise<void>((resolve) => probe.close(() => resolve()))
+  return address.port
+}
+
+async function waitForStop() {
+  for (let attempt = 0; attempt < 50 && McpOAuthCallback.isRunning(); attempt++) await Bun.sleep(10)
+  expect(McpOAuthCallback.isRunning()).toBe(false)
+}