Fail closed when the OAuth callback server is disposed

close() must reject waitForCode so toolset disposal cannot leave
interactive authorization hung. Bind failures keep the OS error
rather than telling the user to retry an ephemeral port.

Author: TheGreatAxios

src/mcp/callback-server.test.ts

@@ -73,15 +73,21 @@ describe("MCP callback server", () => {
     }
   });
 
-  test("binds an ephemeral OS-assigned port on 127.0.0.1", async () => {
-    const server = await startCallbackServer();
+  test("binds concurrent servers to distinct loopback ports", async () => {
+    const first = await startCallbackServer();
+    const second = await startCallbackServer();
     try {
-      const url = new URL(server.redirectUrl);
-
-      expect(url.hostname).toBe("127.0.0.1");
-      expect(Number(url.port)).toBeGreaterThan(0);
+      const firstUrl = new URL(first.redirectUrl);
+      const secondUrl = new URL(second.redirectUrl);
+
+      expect(firstUrl.hostname).toBe("127.0.0.1");
+      expect(secondUrl.hostname).toBe("127.0.0.1");
+      expect(firstUrl.port).not.toBe(secondUrl.port);
+      expect(Number(firstUrl.port)).toBeGreaterThan(0);
+      expect(Number(secondUrl.port)).toBeGreaterThan(0);
     } finally {
-      server.close();
+      first.close();
+      second.close();
     }
   });
 
@@ -100,7 +106,24 @@ describe("MCP callback server", () => {
     );
   });
 
-  test("reports a clear actionable error when the server fails to bind", async () => {
+  test("close is idempotent and does not leak a waiter after a late callback", async () => {
+    const server = await startCallbackServer();
+    authorize(server, "expected");
+    server.close();
+    server.close();
+
+    await expect(
+      fetch(`${server.redirectUrl}?code=abc&state=expected`).then(
+        () => "fetched",
+        (err: unknown) => err,
+      ),
+    ).resolves.toBeInstanceOf(Error);
+    await expect(server.waitForCode(new AbortController().signal)).rejects.toThrow(
+      "closed before authorization completed",
+    );
+  });
+
+  test("rejects start when listen fails without rewriting the OS error as a retry", async () => {
     await withMockedModuleDuring(
       import.meta.resolve("node:http"),
       (real: typeof import("node:http")) => ({
@@ -113,23 +136,17 @@ describe("MCP callback server", () => {
               return fake;
             },
             listen: () => {
-              listeners.error?.(new Error("listen EADDRINUSE: address already in use"));
+              listeners.error?.(new Error("listen EACCES: permission denied"));
             },
             address: (): undefined => undefined,
           };
           return fake as unknown as Server;
         }) as typeof real.createServer,
       }),
       async () => {
-        const err: unknown = await startCallbackServer().then(
-          () => undefined,
-          (failure: unknown) => failure,
+        await expect(startCallbackServer()).rejects.toThrow(
+          "Could not start the OAuth callback server: listen EACCES: permission denied",
         );
-        const error = err as Error;
-
-        expect(error.message).toContain("OAuth callback server");
-        expect(error.message).toContain("EADDRINUSE");
-        expect(error.message).toContain("retry");
       },
     );
   });

src/mcp/callback-server.ts

@@ -21,9 +21,9 @@ interface CallbackWaiter {
 const CALLBACK_PATH = "/callback";
 const CLOSED_ERROR = "OAuth callback server closed before authorization completed.";
 
-// The listen port is always 0, so the OS assigns an ephemeral port and
-// concurrent sessions can never collide on a fixed callback port. `serverName`
-// only names the authorization on the page the browser lands on.
+// Start a loopback server to receive the OAuth redirect. close() fail-closes
+// waitForCode so disposing the toolset cannot leave authorization hung.
+// `serverName` only names the authorization on the page the browser lands on.
 export async function startCallbackServer(serverName?: string): Promise<CallbackServer> {
   let closed = false;
   let expectedState: string | undefined;
@@ -36,6 +36,7 @@ export async function startCallbackServer(serverName?: string): Promise<Callback
   };
 
   const deliver = (result: CallbackResult): void => {
+    if (closed) return;
     if (waiter === undefined) {
       pendingResult = result;
       return;
@@ -78,13 +79,7 @@ export async function startCallbackServer(serverName?: string): Promise<Callback
 
   await new Promise<void>((resolve, reject) => {
     server.once("error", (err) => {
-      reject(
-        new Error(
-          `Could not start the OAuth callback server: ${err.message}. ` +
-            "The server always requests an ephemeral port, so this is unexpected — " +
-            "retry connecting to the MCP server.",
-        ),
-      );
+      reject(new Error(`Could not start the OAuth callback server: ${err.message}`));
     });
     server.listen(0, "127.0.0.1", resolve);
   });
@@ -130,6 +125,7 @@ export async function startCallbackServer(serverName?: string): Promise<Callback
     close: () => {
       // Rejecting the waiter here is what unblocks toolset disposal: a server
       // that merely stops listening would leave a pending waitForCode hung.
+      if (closed) return;
       closed = true;
       pendingResult = undefined;
       if (waiter !== undefined) {