test(mcp): avoid fixed oauth callback ports

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

@@ -1,4 +1,3 @@
-import { createConnection } from "net"
 import { createServer } from "http"
 import { escapeHtml } from "@/util/html"
 import { OAUTH_CALLBACK_PORT, OAUTH_CALLBACK_PATH, parseRedirectUri } from "./oauth-provider"
@@ -57,6 +56,7 @@ interface PendingAuth {
 }
 
 let server: ReturnType<typeof createServer> | undefined
+let starting: Promise<void> | 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).
@@ -144,31 +144,49 @@ function handleRequest(req: import("http").IncomingMessage, res: import("http").
 }
 
 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 server is running on a different port/path, stop it first
-  if (server && (currentPort !== port || currentPath !== path)) {
-    await stop()
+  if (starting) {
+    await starting
+    return ensureRunning(redirectUri)
   }
 
-  if (server) return
+  const operation = (async () => {
+    // Parse the redirect URI to get port and path (uses defaults if not provided)
+    const { port, path } = parseRedirectUri(redirectUri)
 
-  const running = await isPortInUse(port)
-  if (running) {
-    return
+    // If server is running on a different port/path, stop it first
+    if (server && (currentPort !== port || currentPath !== path)) await stop()
+    if (server) return
+
+    const candidate = createServer(handleRequest)
+    try {
+      await new Promise<void>((resolve, reject) => {
+        candidate.once("error", reject)
+        candidate.listen(port, OAUTH_CALLBACK_HOST, resolve)
+      })
+    } catch (error) {
+      candidate.close()
+      if ((error as NodeJS.ErrnoException).code === "EADDRINUSE") {
+        throw new Error(
+          `OAuth callback port ${port} is already in use. ` +
+            `Set "oauth.callbackPort" (or "oauth.redirectUri") on the MCP server ` +
+            `entry in your opencode config to use a different port.`,
+          { cause: error },
+        )
+      }
+      throw error
+    }
+
+    currentPort = port
+    currentPath = path
+    server = candidate
+  })()
+
+  starting = operation
+  try {
+    await operation
+  } finally {
+    starting = undefined
   }
-
-  currentPort = port
-  currentPath = path
-
-  server = createServer(handleRequest)
-  await new Promise<void>((resolve, reject) => {
-    server!.listen(currentPort, OAUTH_CALLBACK_HOST, () => {
-      resolve()
-    })
-    server!.on("error", reject)
-  })
 }
 
 export function waitForCallback(oauthState: string, mcpName?: string): Promise<string> {
@@ -201,19 +219,6 @@ export function cancelPending(mcpName: string): void {
   }
 }
 
-export async function isPortInUse(port: number = OAUTH_CALLBACK_PORT): Promise<boolean> {
-  return new Promise((resolve) => {
-    const socket = createConnection(port, "127.0.0.1")
-    socket.on("connect", () => {
-      socket.destroy()
-      resolve(true)
-    })
-    socket.on("error", () => {
-      resolve(false)
-    })
-  })
-}
-
 export async function stop(): Promise<void> {
   if (server) {
     await new Promise<void>((resolve) => server!.close(() => resolve()))

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

@@ -20,6 +20,26 @@ async function getFreeLoopbackPort(): Promise<number> {
   })
 }
 
+async function listen(server: ReturnType<typeof createNetServer>, port = 0): Promise<number> {
+  return new Promise((resolve, reject) => {
+    server.once("error", reject)
+    server.listen(port, "127.0.0.1", () => {
+      const address = server.address()
+      if (typeof address === "object" && address) return resolve(address.port)
+      reject(new Error("Could not listen on loopback"))
+    })
+  })
+}
+
+async function close(server: ReturnType<typeof createNetServer>): Promise<void> {
+  return new Promise((resolve, reject) => {
+    server.close((error) => {
+      if (error) return reject(error)
+      resolve()
+    })
+  })
+}
+
 async function canConnect(host: string, port: number): Promise<boolean> {
   return new Promise((resolve) => {
     const socket = createConnection({ host, port })
@@ -62,12 +82,13 @@ describe("McpOAuthCallback.ensureRunning", () => {
   })
 
   test("starts server with custom redirectUri port and path", async () => {
-    await McpOAuthCallback.ensureRunning("http://127.0.0.1:18000/custom/callback")
+    const port = await getFreeLoopbackPort()
+    await McpOAuthCallback.ensureRunning(`http://127.0.0.1:${port}/custom/callback`)
     expect(McpOAuthCallback.isRunning()).toBe(true)
   })
 
   test("stops after the callback completes", async () => {
-    const redirectUri = "http://127.0.0.1:18003/custom/callback"
+    const redirectUri = `http://127.0.0.1:${await getFreeLoopbackPort()}/custom/callback`
     await McpOAuthCallback.ensureRunning(redirectUri)
     const callback = McpOAuthCallback.waitForCallback("success")
 
@@ -79,7 +100,7 @@ describe("McpOAuthCallback.ensureRunning", () => {
   })
 
   test("escapes provider error markup in callback HTML", async () => {
-    const redirectUri = "http://127.0.0.1:18001/custom/callback"
+    const redirectUri = `http://127.0.0.1:${await getFreeLoopbackPort()}/custom/callback`
     await McpOAuthCallback.ensureRunning(redirectUri)
 
     const error = `<script>alert("xss" & 'more')</script>`
@@ -94,7 +115,7 @@ describe("McpOAuthCallback.ensureRunning", () => {
   })
 
   test("keeps normal provider errors readable", async () => {
-    const redirectUri = "http://127.0.0.1:18002/custom/callback"
+    const redirectUri = `http://127.0.0.1:${await getFreeLoopbackPort()}/custom/callback`
     await McpOAuthCallback.ensureRunning(redirectUri)
 
     const response = await fetch(
@@ -111,4 +132,44 @@ describe("McpOAuthCallback.ensureRunning", () => {
     expect(await canConnect("127.0.0.1", port)).toBe(true)
     expect(await canConnect("::1", port)).toBe(false)
   })
+
+  test("rejects an occupied port and starts there after it is released", async () => {
+    const blocker = createNetServer()
+    const port = await listen(blocker)
+    const redirectUri = `http://127.0.0.1:${port}/custom/callback`
+
+    await expect(McpOAuthCallback.ensureRunning(redirectUri)).rejects.toThrow(
+      new RegExp(`OAuth callback port ${port} is already in use.*oauth\\.callbackPort.*oauth\\.redirectUri`),
+    )
+    expect(McpOAuthCallback.isRunning()).toBe(false)
+
+    await close(blocker)
+    await McpOAuthCallback.ensureRunning(redirectUri)
+    expect(McpOAuthCallback.isRunning()).toBe(true)
+  })
+
+  test("serializes concurrent starts for the same endpoint", async () => {
+    const port = await getFreeLoopbackPort()
+    const redirectUri = `http://127.0.0.1:${port}/custom/callback`
+
+    await Promise.all([
+      McpOAuthCallback.ensureRunning(redirectUri),
+      McpOAuthCallback.ensureRunning(redirectUri),
+      McpOAuthCallback.ensureRunning(redirectUri),
+    ])
+
+    expect(McpOAuthCallback.isRunning()).toBe(true)
+  })
+
+  test("releases the callback port when stopped", async () => {
+    const port = await getFreeLoopbackPort()
+    await McpOAuthCallback.ensureRunning(`http://127.0.0.1:${port}/custom/callback`)
+
+    await McpOAuthCallback.stop()
+
+    const replacement = createNetServer()
+    await listen(replacement, port)
+    await close(replacement)
+    expect(McpOAuthCallback.isRunning()).toBe(false)
+  })
 })