From 0be196ca04773df5b0b152c382c85438736a63cf Mon Sep 17 00:00:00 2001
From: Coco Sheng <cocosheng@google.com>
Date: Thu, 14 May 2026 13:11:59 -0400
Subject: [PATCH] refactor(core): address code review feedback for OAuth flow
 robustness

---
 packages/core/src/mcp/oauth-provider.test.ts   |  4 ++--
 packages/core/src/utils/oauth-flow-fix.test.ts |  8 ++++----
 packages/core/src/utils/oauth-flow.ts          | 10 +++++-----
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/packages/core/src/mcp/oauth-provider.test.ts b/packages/core/src/mcp/oauth-provider.test.ts
index 3caffe4a73..8934d4d043 100644
--- a/packages/core/src/mcp/oauth-provider.test.ts
+++ b/packages/core/src/mcp/oauth-provider.test.ts
@@ -139,14 +139,14 @@ vi.mock('node:http', () => ({
 
 // Mock startCallbackServer to return what the new implementation returns
 vi.mock('../utils/oauth-flow.js', async (importOriginal) => {
-  const actual = (await importOriginal()) as any;
+  const actual = (await importOriginal()) as typeof import('../utils/oauth-flow.js');
   return {
     ...actual,
     startCallbackServer: vi.fn((expectedState: string, port?: number) => {
       const result = actual.startCallbackServer(expectedState, port);
       // Ensure the mock server is used if createServer is mocked
       if (vi.isMockFunction(http.createServer)) {
-        result.server = mockHttpServer;
+        result.server = mockHttpServer as unknown as http.Server;
       }
       return result;
     }),
diff --git a/packages/core/src/utils/oauth-flow-fix.test.ts b/packages/core/src/utils/oauth-flow-fix.test.ts
index 727e1fe063..a5940613c3 100644
--- a/packages/core/src/utils/oauth-flow-fix.test.ts
+++ b/packages/core/src/utils/oauth-flow-fix.test.ts
@@ -12,8 +12,8 @@ describe('OAuth Flow Repro', () => {
   });
 
   it('should not have an unhandled rejection when close() is called before timeout', async () => {
-    let unhandledRejection: any = null;
-    const handler = (reason: any) => {
+    let unhandledRejection: unknown = null;
+    const handler = (reason: unknown) => {
       unhandledRejection = reason;
     };
     process.on('unhandledRejection', handler);
@@ -39,8 +39,8 @@ describe('OAuth Flow Repro', () => {
   });
 
   it('should not have an unhandled rejection even if NOT closed, due to internal catch', async () => {
-    let unhandledRejection: any = null;
-    const handler = (reason: any) => {
+    let unhandledRejection: unknown = null;
+    const handler = (reason: unknown) => {
       unhandledRejection = reason;
     };
     process.on('unhandledRejection', handler);
diff --git a/packages/core/src/utils/oauth-flow.ts b/packages/core/src/utils/oauth-flow.ts
index 6666bcc610..e8fff4d44c 100644
--- a/packages/core/src/utils/oauth-flow.ts
+++ b/packages/core/src/utils/oauth-flow.ts
@@ -122,7 +122,7 @@ export function startCallbackServer(
   let serverPort: number;
 
   let resolveResponse: (value: OAuthAuthorizationResponse) => void;
-  let rejectResponse: (reason: any) => void;
+  let rejectResponse: (reason: unknown) => void;
   const responsePromise = new Promise<OAuthAuthorizationResponse>(
     (resolve, reject) => {
       resolveResponse = resolve;
@@ -133,7 +133,7 @@ export function startCallbackServer(
   const server = http.createServer(
     async (req: http.IncomingMessage, res: http.ServerResponse) => {
       try {
-        const url = new URL(req.url!, `http://localhost:${serverPort}`);
+        const url = new URL(req.url ?? '', 'http://localhost');
 
         if (url.pathname !== REDIRECT_PATH) {
           res.writeHead(404);
@@ -203,7 +203,7 @@ export function startCallbackServer(
   });
 
   // Determine which port to use (env var, argument, or OS-assigned)
-  let listenPort = 0; // Default to OS-assigned port
+  let listenPort: number | undefined = 0; // Default to OS-assigned port
 
   const portStr = process.env['OAUTH_CALLBACK_PORT'];
   if (portStr) {
@@ -214,7 +214,7 @@ export function startCallbackServer(
       );
       portReject(error);
       rejectResponse(error);
-      // We still return the object, but the promises will be rejected
+      listenPort = undefined;
     } else {
       listenPort = envPort;
     }
@@ -222,7 +222,7 @@ export function startCallbackServer(
     listenPort = port;
   }
 
-  if (listenPort !== undefined || !portStr) {
+  if (listenPort !== undefined) {
     server.listen(listenPort, () => {
       // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
       const address = server.address() as net.AddressInfo;