Fix: Ensure OAuth server is listening before accessing its address

src/client.ts

@@ -21,6 +21,7 @@ import {
   getServerUrlHash,
   connectToRemoteServer,
   TransportStrategy,
+  findAvailablePort,
 } from './lib/utils'
 import { StaticOAuthClientInformationFull, StaticOAuthClientMetadata } from './lib/types'
 import { createLazyAuthCoordinator } from './lib/coordination'
@@ -44,8 +45,8 @@ async function runClient(
   // Get the server URL hash for lockfile operations
   const serverUrlHash = getServerUrlHash(serverUrl)
 
-  // Create a lazy auth coordinator
-  const authCoordinator = createLazyAuthCoordinator(serverUrlHash, callbackPort, events, authTimeoutMs)
+  // Create a lazy auth coordinator with dynamic port support
+  const authCoordinator = createLazyAuthCoordinator(serverUrlHash, callbackPort, events, authTimeoutMs, findAvailablePort)
 
   // Create the OAuth client provider
   const authProvider = new NodeOAuthClientProvider({

src/lib/coordination.ts

@@ -1,10 +1,10 @@
-import { checkLockfile, createLockfile, deleteLockfile, getConfigFilePath, LockfileData } from './mcp-auth-config'
+import { checkLockfile, createLockfile, deleteLockfile, getConfigFilePath, LockfileData, deleteConfigFile } from './mcp-auth-config'
 import { EventEmitter } from 'events'
 import { Server } from 'http'
 import express from 'express'
 import { AddressInfo } from 'net'
 import { unlinkSync } from 'fs'
-import { log, debugLog, setupOAuthCallbackServerWithLongPoll } from './utils'
+import { log, debugLog, setupOAuthCallbackServerWithLongPoll, findAvailablePort } from './utils'
 
 export type AuthCoordinator = {
   initializeAuth: () => Promise<{ server: Server; waitForAuthCode: () => Promise<string>; skipBrowserAuth: boolean }>
@@ -133,8 +133,9 @@ export function createLazyAuthCoordinator(
   callbackPort: number,
   events: EventEmitter,
   authTimeoutMs: number,
-): AuthCoordinator {
-  let authState: { server: Server; waitForAuthCode: () => Promise<string>; skipBrowserAuth: boolean } | null = null
+  findAvailablePortFn?: (preferredPort?: number) => Promise<number>,
+): AuthCoordinator & { getActualPort: () => number | undefined } {
+  let authState: { server: Server; waitForAuthCode: () => Promise<string>; skipBrowserAuth: boolean; actualPort: number } | null = null
 
   return {
     initializeAuth: async () => {
@@ -148,10 +149,17 @@ export function createLazyAuthCoordinator(
       debugLog('Initializing auth coordination on-demand', { serverUrlHash, callbackPort })
 
       // Initialize auth using the existing coordinateAuth logic
-      authState = await coordinateAuth(serverUrlHash, callbackPort, events, authTimeoutMs)
-      debugLog('Auth coordination completed', { skipBrowserAuth: authState.skipBrowserAuth })
-      return authState
+      authState = await coordinateAuth(serverUrlHash, callbackPort, events, authTimeoutMs, findAvailablePortFn)
+      debugLog('Auth coordination completed', { skipBrowserAuth: authState.skipBrowserAuth, actualPort: authState.actualPort })
+
+      // Return without actualPort for compatibility
+      return {
+        server: authState.server,
+        waitForAuthCode: authState.waitForAuthCode,
+        skipBrowserAuth: authState.skipBrowserAuth,
+      }
     },
+    getActualPort: () => authState?.actualPort,
   }
 }
 
@@ -160,14 +168,16 @@ export function createLazyAuthCoordinator(
  * @param serverUrlHash The hash of the server URL
  * @param callbackPort The port to use for the callback server
  * @param events The event emitter to use for signaling
- * @returns An object with the server, waitForAuthCode function, and a flag indicating if browser auth can be skipped
+ * @param findAvailablePortFn Optional function to find an available port
+ * @returns An object with the server, waitForAuthCode function, a flag indicating if browser auth can be skipped, and the actual port used
  */
 export async function coordinateAuth(
   serverUrlHash: string,
   callbackPort: number,
   events: EventEmitter,
   authTimeoutMs: number,
-): Promise<{ server: Server; waitForAuthCode: () => Promise<string>; skipBrowserAuth: boolean }> {
+  findAvailablePortFn?: (preferredPort?: number) => Promise<number>,
+): Promise<{ server: Server; waitForAuthCode: () => Promise<string>; skipBrowserAuth: boolean; actualPort: number }> {
   debugLog('Coordinating authentication', { serverUrlHash, callbackPort })
 
   // Check for a lockfile (disabled on Windows for the time being)
@@ -207,6 +217,7 @@ export async function coordinateAuth(
           server: dummyServer,
           waitForAuthCode: dummyWaitForAuthCode,
           skipBrowserAuth: true,
+          actualPort: callbackPort, // Use original port as we're not actually listening
         }
       } else {
         log('Taking over authentication process...')
@@ -227,16 +238,66 @@ export async function coordinateAuth(
 
   // Create our own lockfile
   debugLog('Setting up OAuth callback server', { port: callbackPort })
-  const { server, waitForAuthCode, authCompletedPromise } = setupOAuthCallbackServerWithLongPoll({
-    port: callbackPort,
-    path: '/oauth/callback',
-    events,
-    authTimeoutMs,
-  })
+
+  // Try to set up the OAuth callback server
+  let server: Server
+  let waitForAuthCode: () => Promise<string>
+  let authCompletedPromise: Promise<string>
+  let actualPort = callbackPort
+
+  try {
+    const result = await setupOAuthCallbackServerWithLongPoll({
+      port: callbackPort,
+      path: '/oauth/callback',
+      events,
+      authTimeoutMs,
+    })
+    server = result.server
+    waitForAuthCode = result.waitForAuthCode
+    authCompletedPromise = result.authCompletedPromise
+  } catch (error: any) {
+    // If we get an EADDRINUSE error, it means another process is using the port
+    if (error.code === 'EADDRINUSE' && findAvailablePortFn) {
+      log(`Port ${callbackPort} is already in use. Finding an alternative port...`)
+      debugLog('Port conflict detected, finding alternative', { originalPort: callbackPort, error: error.message })
+
+      // Find a new available port
+      actualPort = await findAvailablePortFn()
+      log(`Using dynamically assigned port: ${actualPort}`)
+
+      // Delete the old client info since the port has changed
+      // This will force re-registration with the new port
+      await deleteConfigFile(serverUrlHash, 'client_info.json')
+      log('Cleared existing client registration to force re-registration with new port')
+
+      // Try again with the new port
+      const result = await setupOAuthCallbackServerWithLongPoll({
+        port: actualPort,
+        path: '/oauth/callback',
+        events,
+        authTimeoutMs,
+      })
+      server = result.server
+      waitForAuthCode = result.waitForAuthCode
+      authCompletedPromise = result.authCompletedPromise
+    } else if (error.code === 'EADDRINUSE') {
+      // No dynamic port function provided, fail with clear error
+      log(`Fatal error: Port ${callbackPort} is already in use by another process.`)
+      log(`This usually means another instance is already handling OAuth for this server.`)
+      log(`Please wait for the other instance to complete or terminate it.`)
+      debugLog('Port conflict detected, no dynamic port function', { port: callbackPort, error: error.message })
+      throw new Error(`Port ${callbackPort} is already in use. Cannot proceed with OAuth authentication.`)
+    } else {
+      // Re-throw other errors
+      throw error
+    }
+  }
 
-  // Get the actual port the server is running on
+  // Get the actual port the server is running on (in case port 0 was used)
   const address = server.address() as AddressInfo
-  const actualPort = address.port
+  if (actualPort === 0) {
+    actualPort = address.port
+  }
   debugLog('OAuth callback server running', { port: actualPort })
 
   log(`Creating lockfile for server ${serverUrlHash} with process ${process.pid} on port ${actualPort}`)
@@ -275,5 +336,6 @@ export async function coordinateAuth(
     server,
     waitForAuthCode,
     skipBrowserAuth: false,
+    actualPort,
   }
 }

src/lib/utils.test.ts

@@ -887,7 +887,7 @@ describe('setupOAuthCallbackServerWithLongPoll', () => {
 
   it('should use custom timeout when authTimeoutMs is provided', async () => {
     const customTimeout = 5000
-    const result = setupOAuthCallbackServerWithLongPoll({
+    const result = await setupOAuthCallbackServerWithLongPoll({
       port: 0, // Use any available port
       path: '/oauth/callback',
       events,
@@ -902,7 +902,7 @@ describe('setupOAuthCallbackServerWithLongPoll', () => {
   })
 
   it('should use default timeout when authTimeoutMs is not provided', async () => {
-    const result = setupOAuthCallbackServerWithLongPoll({
+    const result = await setupOAuthCallbackServerWithLongPoll({
       port: 0, // Use any available port
       path: '/oauth/callback',
       events,

src/lib/utils.ts

@@ -433,7 +433,7 @@ export async function connectToRemoteServer(
  * @param options The server options
  * @returns An object with the server, authCode, and waitForAuthCode function
  */
-export function setupOAuthCallbackServerWithLongPoll(options: OAuthCallbackServerOptions) {
+export async function setupOAuthCallbackServerWithLongPoll(options: OAuthCallbackServerOptions) {
   let authCode: string | null = null
   const app = express()
 
@@ -510,33 +510,40 @@ export function setupOAuthCallbackServerWithLongPoll(options: OAuthCallbackServe
     options.events.emit('auth-code-received', code)
   })
 
-  const server = app.listen(options.port, () => {
-    log(`OAuth callback server running at http://127.0.0.1:${options.port}`)
-  })
+  // Wrap server.listen in a Promise to ensure it's listening before we return
+  return new Promise((resolve, reject) => {
+    const server = app.listen(options.port, () => {
+      log(`OAuth callback server running at http://127.0.0.1:${options.port}`)
+
+      const waitForAuthCode = (): Promise<string> => {
+        return new Promise((resolve) => {
+          if (authCode) {
+            resolve(authCode)
+            return
+          }
 
-  const waitForAuthCode = (): Promise<string> => {
-    return new Promise((resolve) => {
-      if (authCode) {
-        resolve(authCode)
-        return
+          options.events.once('auth-code-received', (code) => {
+            resolve(code)
+          })
+        })
       }
 
-      options.events.once('auth-code-received', (code) => {
-        resolve(code)
-      })
+      // Resolve with the server and related functions once it's listening
+      resolve({ server, authCode, waitForAuthCode, authCompletedPromise })
     })
-  }
-
-  return { server, authCode, waitForAuthCode, authCompletedPromise }
+
+    // Handle server errors
+    server.on('error', reject)
+  })
 }
 
 /**
  * Sets up an Express server to handle OAuth callbacks
  * @param options The server options
  * @returns An object with the server, authCode, and waitForAuthCode function
  */
-export function setupOAuthCallbackServer(options: OAuthCallbackServerOptions) {
-  const { server, authCode, waitForAuthCode } = setupOAuthCallbackServerWithLongPoll(options)
+export async function setupOAuthCallbackServer(options: OAuthCallbackServerOptions) {
+  const { server, authCode, waitForAuthCode } = await setupOAuthCallbackServerWithLongPoll(options)
   return { server, authCode, waitForAuthCode }
 }
 

src/proxy.ts

@@ -19,6 +19,7 @@ import {
   setupSignalHandlers,
   getServerUrlHash,
   TransportStrategy,
+  findAvailablePort,
 } from './lib/utils'
 import { StaticOAuthClientInformationFull, StaticOAuthClientMetadata } from './lib/types'
 import { NodeOAuthClientProvider } from './lib/node-oauth-client-provider'
@@ -45,8 +46,8 @@ async function runProxy(
   // Get the server URL hash for lockfile operations
   const serverUrlHash = getServerUrlHash(serverUrl)
 
-  // Create a lazy auth coordinator
-  const authCoordinator = createLazyAuthCoordinator(serverUrlHash, callbackPort, events, authTimeoutMs)
+  // Create a lazy auth coordinator with dynamic port support
+  const authCoordinator = createLazyAuthCoordinator(serverUrlHash, callbackPort, events, authTimeoutMs, findAvailablePort)
 
   // Create the OAuth client provider
   const authProvider = new NodeOAuthClientProvider({