fixes for oauth spec - adds github oauth support. Resource paramater. (#6281)

5 files changed, 557 insertions, 173 deletions

diff --git a/packages/core/src/mcp/oauth-provider.test.ts b/packages/core/src/mcp/oauth-provider.test.ts
index 3991aecc..2163d6bc 100644
--- a/packages/core/src/mcp/oauth-provider.test.ts
+++ b/packages/core/src/mcp/oauth-provider.test.ts
@@ -29,6 +29,55 @@ import { MCPOAuthTokenStorage, MCPOAuthToken } from './oauth-token-storage.js';
 const mockFetch = vi.fn();
 global.fetch = mockFetch;
 
+// Helper function to create mock fetch responses with proper headers
+const createMockResponse = (options: {
+  ok: boolean;
+  status?: number;
+  contentType?: string;
+  text?: string | (() => Promise<string>);
+  json?: unknown | (() => Promise<unknown>);
+}) => {
+  const response: {
+    ok: boolean;
+    status?: number;
+    headers: {
+      get: (name: string) => string | null;
+    };
+    text?: () => Promise<string>;
+    json?: () => Promise<unknown>;
+  } = {
+    ok: options.ok,
+    headers: {
+      get: (name: string) => {
+        if (name.toLowerCase() === 'content-type') {
+          return options.contentType || null;
+        }
+        return null;
+      },
+    },
+  };
+
+  if (options.status !== undefined) {
+    response.status = options.status;
+  }
+
+  if (options.text !== undefined) {
+    response.text =
+      typeof options.text === 'string'
+        ? () => Promise.resolve(options.text as string)
+        : (options.text as () => Promise<string>);
+  }
+
+  if (options.json !== undefined) {
+    response.json =
+      typeof options.json === 'function'
+        ? (options.json as () => Promise<unknown>)
+        : () => Promise.resolve(options.json);
+  }
+
+  return response;
+};
+
 // Define a reusable mock server with .listen, .close, and .on methods
 const mockHttpServer = {
   listen: vi.fn(),
@@ -133,10 +182,14 @@ describe('MCPOAuthProvider', () => {
       });
 
       // Mock token exchange
-      mockFetch.mockResolvedValueOnce({
-        ok: true,
-        json: () => Promise.resolve(mockTokenResponse),
-      });
+      mockFetch.mockResolvedValueOnce(
+        createMockResponse({
+          ok: true,
+          contentType: 'application/json',
+          text: JSON.stringify(mockTokenResponse),
+          json: mockTokenResponse,
+        }),
+      );
 
       const result = await MCPOAuthProvider.authenticate(
         'test-server',
@@ -165,7 +218,11 @@ describe('MCPOAuthProvider', () => {
 
     it('should handle OAuth discovery when no authorization URL provided', async () => {
       // Use a mutable config object
-      const configWithoutAuth: MCPOAuthConfig = { ...mockConfig };
+      const configWithoutAuth: MCPOAuthConfig = {
+        ...mockConfig,
+        clientId: 'test-client-id',
+        clientSecret: 'test-client-secret',
+      };
       delete configWithoutAuth.authorizationUrl;
       delete configWithoutAuth.tokenUrl;
 
@@ -179,21 +236,30 @@ describe('MCPOAuthProvider', () => {
         scopes_supported: ['read', 'write'],
       };
 
+      // Mock HEAD request for WWW-Authenticate check
       mockFetch
-        .mockResolvedValueOnce({
-          ok: true,
-          json: () => Promise.resolve(mockResourceMetadata),
-        })
-        .mockResolvedValueOnce({
-          ok: true,
-          json: () => Promise.resolve(mockAuthServerMetadata),
-        });
-
-      // Patch config after discovery
-      configWithoutAuth.authorizationUrl =
-        mockAuthServerMetadata.authorization_endpoint;
-      configWithoutAuth.tokenUrl = mockAuthServerMetadata.token_endpoint;
-      configWithoutAuth.scopes = mockAuthServerMetadata.scopes_supported;
+        .mockResolvedValueOnce(
+          createMockResponse({
+            ok: true,
+            status: 200,
+          }),
+        )
+        .mockResolvedValueOnce(
+          createMockResponse({
+            ok: true,
+            contentType: 'application/json',
+            text: JSON.stringify(mockResourceMetadata),
+            json: mockResourceMetadata,
+          }),
+        )
+        .mockResolvedValueOnce(
+          createMockResponse({
+            ok: true,
+            contentType: 'application/json',
+            text: JSON.stringify(mockAuthServerMetadata),
+            json: mockAuthServerMetadata,
+          }),
+        );
 
       // Setup callback handler
       let callbackHandler: unknown;
@@ -220,10 +286,14 @@ describe('MCPOAuthProvider', () => {
       });
 
       // Mock token exchange with discovered endpoint
-      mockFetch.mockResolvedValueOnce({
-        ok: true,
-        json: () => Promise.resolve(mockTokenResponse),
-      });
+      mockFetch.mockResolvedValueOnce(
+        createMockResponse({
+          ok: true,
+          contentType: 'application/json',
+          text: JSON.stringify(mockTokenResponse),
+          json: mockTokenResponse,
+        }),
+      );
 
       const result = await MCPOAuthProvider.authenticate(
         'test-server',
@@ -236,7 +306,9 @@ describe('MCPOAuthProvider', () => {
         'https://discovered.auth.com/token',
         expect.objectContaining({
           method: 'POST',
-          headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
+          headers: expect.objectContaining({
+            'Content-Type': 'application/x-www-form-urlencoded',
+          }),
         }),
       );
     });
@@ -261,14 +333,22 @@ describe('MCPOAuthProvider', () => {
       };
 
       mockFetch
-        .mockResolvedValueOnce({
-          ok: true,
-          json: () => Promise.resolve(mockAuthServerMetadata),
-        })
-        .mockResolvedValueOnce({
-          ok: true,
-          json: () => Promise.resolve(mockRegistrationResponse),
-        });
+        .mockResolvedValueOnce(
+          createMockResponse({
+            ok: true,
+            contentType: 'application/json',
+            text: JSON.stringify(mockAuthServerMetadata),
+            json: mockAuthServerMetadata,
+          }),
+        )
+        .mockResolvedValueOnce(
+          createMockResponse({
+            ok: true,
+            contentType: 'application/json',
+            text: JSON.stringify(mockRegistrationResponse),
+            json: mockRegistrationResponse,
+          }),
+        );
 
       // Setup callback handler
       let callbackHandler: unknown;
@@ -295,10 +375,14 @@ describe('MCPOAuthProvider', () => {
       });
 
       // Mock token exchange
-      mockFetch.mockResolvedValueOnce({
-        ok: true,
-        json: () => Promise.resolve(mockTokenResponse),
-      });
+      mockFetch.mockResolvedValueOnce(
+        createMockResponse({
+          ok: true,
+          contentType: 'application/json',
+          text: JSON.stringify(mockTokenResponse),
+          json: mockTokenResponse,
+        }),
+      );
 
       const result = await MCPOAuthProvider.authenticate(
         'test-server',
@@ -397,15 +481,18 @@ describe('MCPOAuthProvider', () => {
         }, 10);
       });
 
-      mockFetch.mockResolvedValueOnce({
-        ok: false,
-        status: 400,
-        text: () => Promise.resolve('Invalid grant'),
-      });
+      mockFetch.mockResolvedValueOnce(
+        createMockResponse({
+          ok: false,
+          status: 400,
+          contentType: 'application/x-www-form-urlencoded',
+          text: 'error=invalid_grant&error_description=Invalid grant',
+        }),
+      );
 
       await expect(
         MCPOAuthProvider.authenticate('test-server', mockConfig),
-      ).rejects.toThrow('Token exchange failed: 400 - Invalid grant');
+      ).rejects.toThrow('Token exchange failed: invalid_grant - Invalid grant');
     });
 
     it('should handle callback timeout', async () => {
@@ -445,10 +532,14 @@ describe('MCPOAuthProvider', () => {
         refresh_token: 'new_refresh_token',
       };
 
-      mockFetch.mockResolvedValueOnce({
-        ok: true,
-        json: () => Promise.resolve(refreshResponse),
-      });
+      mockFetch.mockResolvedValueOnce(
+        createMockResponse({
+          ok: true,
+          contentType: 'application/json',
+          text: JSON.stringify(refreshResponse),
+          json: refreshResponse,
+        }),
+      );
 
       const result = await MCPOAuthProvider.refreshAccessToken(
         mockConfig,
@@ -461,17 +552,24 @@ describe('MCPOAuthProvider', () => {
         'https://auth.example.com/token',
         expect.objectContaining({
           method: 'POST',
-          headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
+          headers: {
+            'Content-Type': 'application/x-www-form-urlencoded',
+            Accept: 'application/json, application/x-www-form-urlencoded',
+          },
           body: expect.stringContaining('grant_type=refresh_token'),
         }),
       );
     });
 
     it('should include client secret in refresh request when available', async () => {
-      mockFetch.mockResolvedValueOnce({
-        ok: true,
-        json: () => Promise.resolve(mockTokenResponse),
-      });
+      mockFetch.mockResolvedValueOnce(
+        createMockResponse({
+          ok: true,
+          contentType: 'application/json',
+          text: JSON.stringify(mockTokenResponse),
+          json: mockTokenResponse,
+        }),
+      );
 
       await MCPOAuthProvider.refreshAccessToken(
         mockConfig,
@@ -484,11 +582,14 @@ describe('MCPOAuthProvider', () => {
     });
 
     it('should handle refresh token failure', async () => {
-      mockFetch.mockResolvedValueOnce({
-        ok: false,
-        status: 400,
-        text: () => Promise.resolve('Invalid refresh token'),
-      });
+      mockFetch.mockResolvedValueOnce(
+        createMockResponse({
+          ok: false,
+          status: 400,
+          contentType: 'application/x-www-form-urlencoded',
+          text: 'error=invalid_request&error_description=Invalid refresh token',
+        }),
+      );
 
       await expect(
         MCPOAuthProvider.refreshAccessToken(
@@ -496,7 +597,9 @@ describe('MCPOAuthProvider', () => {
           'invalid_refresh_token',
           'https://auth.example.com/token',
         ),
-      ).rejects.toThrow('Token refresh failed: 400 - Invalid refresh token');
+      ).rejects.toThrow(
+        'Token refresh failed: invalid_request - Invalid refresh token',
+      );
     });
   });
 
@@ -544,10 +647,14 @@ describe('MCPOAuthProvider', () => {
         refresh_token: 'new_refresh_token',
       };
 
-      mockFetch.mockResolvedValueOnce({
-        ok: true,
-        json: () => Promise.resolve(refreshResponse),
-      });
+      mockFetch.mockResolvedValueOnce(
+        createMockResponse({
+          ok: true,
+          contentType: 'application/json',
+          text: JSON.stringify(refreshResponse),
+          json: refreshResponse,
+        }),
+      );
 
       const result = await MCPOAuthProvider.getValidToken(
         'test-server',
@@ -590,11 +697,14 @@ describe('MCPOAuthProvider', () => {
       vi.mocked(MCPOAuthTokenStorage.isTokenExpired).mockReturnValue(true);
       vi.mocked(MCPOAuthTokenStorage.removeToken).mockResolvedValue(undefined);
 
-      mockFetch.mockResolvedValueOnce({
-        ok: false,
-        status: 400,
-        text: () => Promise.resolve('Invalid refresh token'),
-      });
+      mockFetch.mockResolvedValueOnce(
+        createMockResponse({
+          ok: false,
+          status: 400,
+          contentType: 'application/x-www-form-urlencoded',
+          text: 'error=invalid_request&error_description=Invalid refresh token',
+        }),
+      );
 
       const result = await MCPOAuthProvider.getValidToken(
         'test-server',
@@ -664,10 +774,14 @@ describe('MCPOAuthProvider', () => {
         }, 10);
       });
 
-      mockFetch.mockResolvedValueOnce({
-        ok: true,
-        json: () => Promise.resolve(mockTokenResponse),
-      });
+      mockFetch.mockResolvedValueOnce(
+        createMockResponse({
+          ok: true,
+          contentType: 'application/json',
+          text: JSON.stringify(mockTokenResponse),
+          json: mockTokenResponse,
+        }),
+      );
 
       await MCPOAuthProvider.authenticate('test-server', mockConfig);
 
@@ -709,12 +823,20 @@ describe('MCPOAuthProvider', () => {
         }, 10);
       });
 
-      mockFetch.mockResolvedValueOnce({
-        ok: true,
-        json: () => Promise.resolve(mockTokenResponse),
-      });
+      mockFetch.mockResolvedValueOnce(
+        createMockResponse({
+          ok: true,
+          contentType: 'application/json',
+          text: JSON.stringify(mockTokenResponse),
+          json: mockTokenResponse,
+        }),
+      );
 
-      await MCPOAuthProvider.authenticate('test-server', mockConfig);
+      await MCPOAuthProvider.authenticate(
+        'test-server',
+        mockConfig,
+        'https://auth.example.com',
+      );
 
       expect(capturedUrl).toBeDefined();
       expect(capturedUrl!).toContain('response_type=code');
@@ -757,10 +879,14 @@ describe('MCPOAuthProvider', () => {
         }, 10);
       });
 
-      mockFetch.mockResolvedValueOnce({
-        ok: true,
-        json: () => Promise.resolve(mockTokenResponse),
-      });
+      mockFetch.mockResolvedValueOnce(
+        createMockResponse({
+          ok: true,
+          contentType: 'application/json',
+          text: JSON.stringify(mockTokenResponse),
+          json: mockTokenResponse,
+        }),
+      );
 
       const configWithParamsInUrl = {
         ...mockConfig,
@@ -806,10 +932,14 @@ describe('MCPOAuthProvider', () => {
         }, 10);
       });
 
-      mockFetch.mockResolvedValueOnce({
-        ok: true,
-        json: () => Promise.resolve(mockTokenResponse),
-      });
+      mockFetch.mockResolvedValueOnce(
+        createMockResponse({
+          ok: true,
+          contentType: 'application/json',
+          text: JSON.stringify(mockTokenResponse),
+          json: mockTokenResponse,
+        }),
+      );
 
       const configWithFragment = {
         ...mockConfig,
diff --git a/packages/core/src/mcp/oauth-provider.ts b/packages/core/src/mcp/oauth-provider.ts
index eaec5c2e..c2579d1a 100644
--- a/packages/core/src/mcp/oauth-provider.ts
+++ b/packages/core/src/mcp/oauth-provider.ts
@@ -144,8 +144,8 @@ export class MCPOAuthProvider {
   private static async discoverOAuthFromMCPServer(
     mcpServerUrl: string,
   ): Promise<MCPOAuthConfig | null> {
-    const baseUrl = OAuthUtils.extractBaseUrl(mcpServerUrl);
-    return OAuthUtils.discoverOAuthConfig(baseUrl);
+    // Use the full URL with path preserved for OAuth discovery
+    return OAuthUtils.discoverOAuthConfig(mcpServerUrl);
   }
 
   /**
@@ -302,14 +302,18 @@ export class MCPOAuthProvider {
     }
 
     // Add resource parameter for MCP OAuth spec compliance
-    // Use the MCP server URL if provided, otherwise fall back to authorization URL
-    const resourceUrl = mcpServerUrl || config.authorizationUrl!;
-    try {
-      params.append('resource', OAuthUtils.buildResourceParameter(resourceUrl));
-    } catch (error) {
-      throw new Error(
-        `Invalid resource URL: "${resourceUrl}". ${getErrorMessage(error)}`,
-      );
+    // Only add if we have an MCP server URL (indicates MCP OAuth flow, not standard OAuth)
+    if (mcpServerUrl) {
+      try {
+        params.append(
+          'resource',
+          OAuthUtils.buildResourceParameter(mcpServerUrl),
+        );
+      } catch (error) {
+        console.warn(
+          `Could not add resource parameter: ${getErrorMessage(error)}`,
+        );
+      }
     }
 
     const url = new URL(config.authorizationUrl!);
@@ -355,32 +359,93 @@ export class MCPOAuthProvider {
     }
 
     // Add resource parameter for MCP OAuth spec compliance
-    // Use the MCP server URL if provided, otherwise fall back to token URL
-    const resourceUrl = mcpServerUrl || config.tokenUrl!;
-    try {
-      params.append('resource', OAuthUtils.buildResourceParameter(resourceUrl));
-    } catch (error) {
-      throw new Error(
-        `Invalid resource URL: "${resourceUrl}". ${getErrorMessage(error)}`,
-      );
+    // Only add if we have an MCP server URL (indicates MCP OAuth flow, not standard OAuth)
+    if (mcpServerUrl) {
+      const resourceUrl = mcpServerUrl;
+      try {
+        params.append(
+          'resource',
+          OAuthUtils.buildResourceParameter(resourceUrl),
+        );
+      } catch (error) {
+        console.warn(
+          `Could not add resource parameter: ${getErrorMessage(error)}`,
+        );
+      }
     }
 
     const response = await fetch(config.tokenUrl!, {
       method: 'POST',
       headers: {
         'Content-Type': 'application/x-www-form-urlencoded',
+        Accept: 'application/json, application/x-www-form-urlencoded',
       },
       body: params.toString(),
     });
 
+    const responseText = await response.text();
+    const contentType = response.headers.get('content-type') || '';
+
     if (!response.ok) {
-      const errorText = await response.text();
+      // Try to parse error from form-urlencoded response
+      let errorMessage: string | null = null;
+      try {
+        const errorParams = new URLSearchParams(responseText);
+        const error = errorParams.get('error');
+        const errorDescription = errorParams.get('error_description');
+        if (error) {
+          errorMessage = `Token exchange failed: ${error} - ${errorDescription || 'No description'}`;
+        }
+      } catch {
+        // Fall back to raw error
+      }
       throw new Error(
-        `Token exchange failed: ${response.status} - ${errorText}`,
+        errorMessage ||
+          `Token exchange failed: ${response.status} - ${responseText}`,
+      );
+    }
+
+    // Log unexpected content types for debugging
+    if (
+      !contentType.includes('application/json') &&
+      !contentType.includes('application/x-www-form-urlencoded')
+    ) {
+      console.warn(
+        `Token endpoint returned unexpected content-type: ${contentType}. ` +
+          `Expected application/json or application/x-www-form-urlencoded. ` +
+          `Will attempt to parse response.`,
       );
     }
 
-    return (await response.json()) as OAuthTokenResponse;
+    // Try to parse as JSON first, fall back to form-urlencoded
+    try {
+      return JSON.parse(responseText) as OAuthTokenResponse;
+    } catch {
+      // Parse form-urlencoded response
+      const tokenParams = new URLSearchParams(responseText);
+      const accessToken = tokenParams.get('access_token');
+      const tokenType = tokenParams.get('token_type') || 'Bearer';
+      const expiresIn = tokenParams.get('expires_in');
+      const refreshToken = tokenParams.get('refresh_token');
+      const scope = tokenParams.get('scope');
+
+      if (!accessToken) {
+        // Check for error in response
+        const error = tokenParams.get('error');
+        const errorDescription = tokenParams.get('error_description');
+        throw new Error(
+          `Token exchange failed: ${error || 'no_access_token'} - ${errorDescription || responseText}`,
+        );
+      }
+
+      return {
+        access_token: accessToken,
+        token_type: tokenType,
+        expires_in: expiresIn ? parseInt(expiresIn, 10) : undefined,
+        refresh_token: refreshToken || undefined,
+        scope: scope || undefined,
+      } as OAuthTokenResponse;
+    }
   }
 
   /**
@@ -417,32 +482,92 @@ export class MCPOAuthProvider {
     }
 
     // Add resource parameter for MCP OAuth spec compliance
-    // Use the MCP server URL if provided, otherwise fall back to token URL
-    const resourceUrl = mcpServerUrl || tokenUrl;
-    try {
-      params.append('resource', OAuthUtils.buildResourceParameter(resourceUrl));
-    } catch (error) {
-      throw new Error(
-        `Invalid resource URL: "${resourceUrl}". ${getErrorMessage(error)}`,
-      );
+    // Only add if we have an MCP server URL (indicates MCP OAuth flow, not standard OAuth)
+    if (mcpServerUrl) {
+      try {
+        params.append(
+          'resource',
+          OAuthUtils.buildResourceParameter(mcpServerUrl),
+        );
+      } catch (error) {
+        console.warn(
+          `Could not add resource parameter: ${getErrorMessage(error)}`,
+        );
+      }
     }
 
     const response = await fetch(tokenUrl, {
       method: 'POST',
       headers: {
         'Content-Type': 'application/x-www-form-urlencoded',
+        Accept: 'application/json, application/x-www-form-urlencoded',
       },
       body: params.toString(),
     });
 
+    const responseText = await response.text();
+    const contentType = response.headers.get('content-type') || '';
+
     if (!response.ok) {
-      const errorText = await response.text();
+      // Try to parse error from form-urlencoded response
+      let errorMessage: string | null = null;
+      try {
+        const errorParams = new URLSearchParams(responseText);
+        const error = errorParams.get('error');
+        const errorDescription = errorParams.get('error_description');
+        if (error) {
+          errorMessage = `Token refresh failed: ${error} - ${errorDescription || 'No description'}`;
+        }
+      } catch {
+        // Fall back to raw error
+      }
       throw new Error(
-        `Token refresh failed: ${response.status} - ${errorText}`,
+        errorMessage ||
+          `Token refresh failed: ${response.status} - ${responseText}`,
       );
     }
 
-    return (await response.json()) as OAuthTokenResponse;
+    // Log unexpected content types for debugging
+    if (
+      !contentType.includes('application/json') &&
+      !contentType.includes('application/x-www-form-urlencoded')
+    ) {
+      console.warn(
+        `Token refresh endpoint returned unexpected content-type: ${contentType}. ` +
+          `Expected application/json or application/x-www-form-urlencoded. ` +
+          `Will attempt to parse response.`,
+      );
+    }
+
+    // Try to parse as JSON first, fall back to form-urlencoded
+    try {
+      return JSON.parse(responseText) as OAuthTokenResponse;
+    } catch {
+      // Parse form-urlencoded response
+      const tokenParams = new URLSearchParams(responseText);
+      const accessToken = tokenParams.get('access_token');
+      const tokenType = tokenParams.get('token_type') || 'Bearer';
+      const expiresIn = tokenParams.get('expires_in');
+      const refreshToken = tokenParams.get('refresh_token');
+      const scope = tokenParams.get('scope');
+
+      if (!accessToken) {
+        // Check for error in response
+        const error = tokenParams.get('error');
+        const errorDescription = tokenParams.get('error_description');
+        throw new Error(
+          `Token refresh failed: ${error || 'unknown_error'} - ${errorDescription || responseText}`,
+        );
+      }
+
+      return {
+        access_token: accessToken,
+        token_type: tokenType,
+        expires_in: expiresIn ? parseInt(expiresIn, 10) : undefined,
+        refresh_token: refreshToken || undefined,
+        scope: scope || undefined,
+      } as OAuthTokenResponse;
+    }
   }
 
   /**
@@ -464,37 +589,43 @@ export class MCPOAuthProvider {
         'No authorization URL provided, attempting OAuth discovery...',
       );
 
-      // For SSE URLs, first check if authentication is required
-      if (OAuthUtils.isSSEEndpoint(mcpServerUrl)) {
-        try {
-          const response = await fetch(mcpServerUrl, {
-            method: 'HEAD',
-            headers: {
-              Accept: 'text/event-stream',
-            },
-          });
+      // First check if the server requires authentication via WWW-Authenticate header
+      try {
+        const headers: HeadersInit = OAuthUtils.isSSEEndpoint(mcpServerUrl)
+          ? { Accept: 'text/event-stream' }
+          : { Accept: 'application/json' };
+
+        const response = await fetch(mcpServerUrl, {
+          method: 'HEAD',
+          headers,
+        });
+
+        if (response.status === 401 || response.status === 307) {
+          const wwwAuthenticate = response.headers.get('www-authenticate');
 
-          if (response.status === 401 || response.status === 307) {
-            const wwwAuthenticate = response.headers.get('www-authenticate');
-            if (wwwAuthenticate) {
-              const discoveredConfig =
-                await OAuthUtils.discoverOAuthFromWWWAuthenticate(
-                  wwwAuthenticate,
-                );
-              if (discoveredConfig) {
-                config = {
-                  ...config,
-                  ...discoveredConfig,
-                  scopes: discoveredConfig.scopes || config.scopes || [],
-                };
-              }
+          if (wwwAuthenticate) {
+            const discoveredConfig =
+              await OAuthUtils.discoverOAuthFromWWWAuthenticate(
+                wwwAuthenticate,
+              );
+            if (discoveredConfig) {
+              // Merge discovered config with existing config, preserving clientId and clientSecret
+              config = {
+                ...config,
+                authorizationUrl: discoveredConfig.authorizationUrl,
+                tokenUrl: discoveredConfig.tokenUrl,
+                scopes: discoveredConfig.scopes || config.scopes || [],
+                // Preserve existing client credentials
+                clientId: config.clientId,
+                clientSecret: config.clientSecret,
+              };
             }
           }
-        } catch (error) {
-          console.debug(
-            `Failed to check SSE endpoint for authentication requirements: ${getErrorMessage(error)}`,
-          );
         }
+      } catch (error) {
+        console.debug(
+          `Failed to check endpoint for authentication requirements: ${getErrorMessage(error)}`,
+        );
       }
 
       // If we still don't have OAuth config, try the standard discovery
@@ -502,8 +633,16 @@ export class MCPOAuthProvider {
         const discoveredConfig =
           await this.discoverOAuthFromMCPServer(mcpServerUrl);
         if (discoveredConfig) {
-          config = { ...config, ...discoveredConfig };
-          console.log('OAuth configuration discovered successfully');
+          // Merge discovered config with existing config, preserving clientId and clientSecret
+          config = {
+            ...config,
+            authorizationUrl: discoveredConfig.authorizationUrl,
+            tokenUrl: discoveredConfig.tokenUrl,
+            scopes: discoveredConfig.scopes || config.scopes || [],
+            // Preserve existing client credentials
+            clientId: config.clientId,
+            clientSecret: config.clientSecret,
+          };
         } else {
           throw new Error(
             'Failed to discover OAuth configuration from MCP server',
@@ -633,9 +772,13 @@ export class MCPOAuthProvider {
     );
 
     // Convert to our token format
+    if (!tokenResponse.access_token) {
+      throw new Error('No access token received from token endpoint');
+    }
+
     const token: MCPOAuthToken = {
       accessToken: tokenResponse.access_token,
-      tokenType: tokenResponse.token_type,
+      tokenType: tokenResponse.token_type || 'Bearer',
       refreshToken: tokenResponse.refresh_token,
       scope: tokenResponse.scope,
     };
@@ -657,12 +800,16 @@ export class MCPOAuthProvider {
 
       // Verify token was saved
       const savedToken = await MCPOAuthTokenStorage.getToken(serverName);
-      if (savedToken) {
-        console.log(
-          `Token verification successful: ${savedToken.token.accessToken.substring(0, 20)}...`,
-        );
+      if (savedToken && savedToken.token && savedToken.token.accessToken) {
+        const tokenPreview =
+          savedToken.token.accessToken.length > 20
+            ? `${savedToken.token.accessToken.substring(0, 20)}...`
+            : '[token]';
+        console.log(`Token verification successful: ${tokenPreview}`);
       } else {
-        console.error('Token verification failed: token not found after save');
+        console.error(
+          'Token verification failed: token not found or invalid after save',
+        );
       }
     } catch (saveError) {
       console.error(`Failed to save token: ${getErrorMessage(saveError)}`);
diff --git a/packages/core/src/mcp/oauth-utils.test.ts b/packages/core/src/mcp/oauth-utils.test.ts
index 12871ff2..47bfb9ae 100644
--- a/packages/core/src/mcp/oauth-utils.test.ts
+++ b/packages/core/src/mcp/oauth-utils.test.ts
@@ -28,8 +28,8 @@ describe('OAuthUtils', () => {
   });
 
   describe('buildWellKnownUrls', () => {
-    it('should build correct well-known URLs', () => {
-      const urls = OAuthUtils.buildWellKnownUrls('https://example.com/path');
+    it('should build standard root-based URLs by default', () => {
+      const urls = OAuthUtils.buildWellKnownUrls('https://example.com/mcp');
       expect(urls.protectedResource).toBe(
         'https://example.com/.well-known/oauth-protected-resource',
       );
@@ -37,6 +37,42 @@ describe('OAuthUtils', () => {
         'https://example.com/.well-known/oauth-authorization-server',
       );
     });
+
+    it('should build path-based URLs when includePathSuffix is true', () => {
+      const urls = OAuthUtils.buildWellKnownUrls(
+        'https://example.com/mcp',
+        true,
+      );
+      expect(urls.protectedResource).toBe(
+        'https://example.com/.well-known/oauth-protected-resource/mcp',
+      );
+      expect(urls.authorizationServer).toBe(
+        'https://example.com/.well-known/oauth-authorization-server/mcp',
+      );
+    });
+
+    it('should handle root path correctly', () => {
+      const urls = OAuthUtils.buildWellKnownUrls('https://example.com', true);
+      expect(urls.protectedResource).toBe(
+        'https://example.com/.well-known/oauth-protected-resource',
+      );
+      expect(urls.authorizationServer).toBe(
+        'https://example.com/.well-known/oauth-authorization-server',
+      );
+    });
+
+    it('should handle trailing slash in path', () => {
+      const urls = OAuthUtils.buildWellKnownUrls(
+        'https://example.com/mcp/',
+        true,
+      );
+      expect(urls.protectedResource).toBe(
+        'https://example.com/.well-known/oauth-protected-resource/mcp',
+      );
+      expect(urls.authorizationServer).toBe(
+        'https://example.com/.well-known/oauth-authorization-server/mcp',
+      );
+    });
   });
 
   describe('fetchProtectedResourceMetadata', () => {
diff --git a/packages/core/src/mcp/oauth-utils.ts b/packages/core/src/mcp/oauth-utils.ts
index 64fd68be..b926ce0b 100644
--- a/packages/core/src/mcp/oauth-utils.ts
+++ b/packages/core/src/mcp/oauth-utils.ts
@@ -43,18 +43,36 @@ export interface OAuthProtectedResourceMetadata {
 export class OAuthUtils {
   /**
    * Construct well-known OAuth endpoint URLs.
+   * By default, uses standard root-based well-known URLs.
+   * If includePathSuffix is true, appends any path from the base URL to the well-known endpoints.
    */
-  static buildWellKnownUrls(baseUrl: string) {
+  static buildWellKnownUrls(baseUrl: string, includePathSuffix = false) {
     const serverUrl = new URL(baseUrl);
     const base = `${serverUrl.protocol}//${serverUrl.host}`;
 
+    if (!includePathSuffix) {
+      // Standard discovery: use root-based well-known URLs
+      return {
+        protectedResource: new URL(
+          '/.well-known/oauth-protected-resource',
+          base,
+        ).toString(),
+        authorizationServer: new URL(
+          '/.well-known/oauth-authorization-server',
+          base,
+        ).toString(),
+      };
+    }
+
+    // Path-based discovery: append path suffix to well-known URLs
+    const pathSuffix = serverUrl.pathname.replace(/\/$/, ''); // Remove trailing slash
     return {
       protectedResource: new URL(
-        '/.well-known/oauth-protected-resource',
+        `/.well-known/oauth-protected-resource${pathSuffix}`,
         base,
       ).toString(),
       authorizationServer: new URL(
-        '/.well-known/oauth-authorization-server',
+        `/.well-known/oauth-authorization-server${pathSuffix}`,
         base,
       ).toString(),
     };
@@ -132,25 +150,56 @@ export class OAuthUtils {
     serverUrl: string,
   ): Promise<MCPOAuthConfig | null> {
     try {
-      const wellKnownUrls = this.buildWellKnownUrls(serverUrl);
+      // First try standard root-based discovery
+      const wellKnownUrls = this.buildWellKnownUrls(serverUrl, false);
 
-      // First, try to get the protected resource metadata
-      const resourceMetadata = await this.fetchProtectedResourceMetadata(
+      // Try to get the protected resource metadata at root
+      let resourceMetadata = await this.fetchProtectedResourceMetadata(
         wellKnownUrls.protectedResource,
       );
 
+      // If root discovery fails and we have a path, try path-based discovery
+      if (!resourceMetadata) {
+        const url = new URL(serverUrl);
+        if (url.pathname && url.pathname !== '/') {
+          const pathBasedUrls = this.buildWellKnownUrls(serverUrl, true);
+          resourceMetadata = await this.fetchProtectedResourceMetadata(
+            pathBasedUrls.protectedResource,
+          );
+        }
+      }
+
       if (resourceMetadata?.authorization_servers?.length) {
         // Use the first authorization server
         const authServerUrl = resourceMetadata.authorization_servers[0];
-        const authServerMetadataUrl = new URL(
-          '/.well-known/oauth-authorization-server',
-          authServerUrl,
+
+        // The authorization server URL may include a path (e.g., https://github.com/login/oauth)
+        // We need to preserve this path when constructing the metadata URL
+        const authServerUrlObj = new URL(authServerUrl);
+        const authServerPath =
+          authServerUrlObj.pathname === '/' ? '' : authServerUrlObj.pathname;
+
+        // Try with the authorization server's path first
+        let authServerMetadataUrl = new URL(
+          `/.well-known/oauth-authorization-server${authServerPath}`,
+          `${authServerUrlObj.protocol}//${authServerUrlObj.host}`,
         ).toString();
 
-        const authServerMetadata = await this.fetchAuthorizationServerMetadata(
+        let authServerMetadata = await this.fetchAuthorizationServerMetadata(
           authServerMetadataUrl,
         );
 
+        // If that fails, try root as fallback
+        if (!authServerMetadata && authServerPath) {
+          authServerMetadataUrl = new URL(
+            '/.well-known/oauth-authorization-server',
+            `${authServerUrlObj.protocol}//${authServerUrlObj.host}`,
+          ).toString();
+          authServerMetadata = await this.fetchAuthorizationServerMetadata(
+            authServerMetadataUrl,
+          );
+        }
+
         if (authServerMetadata) {
           const config = this.metadataToOAuthConfig(authServerMetadata);
           if (authServerMetadata.registration_endpoint) {
@@ -221,10 +270,6 @@ export class OAuthUtils {
       return null;
     }
 
-    console.log(
-      `Found resource metadata URI from www-authenticate header: ${resourceMetadataUri}`,
-    );
-
     const resourceMetadata =
       await this.fetchProtectedResourceMetadata(resourceMetadataUri);
     if (!resourceMetadata?.authorization_servers?.length) {
@@ -232,19 +277,36 @@ export class OAuthUtils {
     }
 
     const authServerUrl = resourceMetadata.authorization_servers[0];
+
+    // The authorization server URL may include a path (e.g., https://github.com/login/oauth)
+    // We need to preserve this path when constructing the metadata URL
+    const authServerUrlObj = new URL(authServerUrl);
+    const authServerPath =
+      authServerUrlObj.pathname === '/' ? '' : authServerUrlObj.pathname;
+
+    // Build auth server metadata URL with the authorization server's path
     const authServerMetadataUrl = new URL(
-      '/.well-known/oauth-authorization-server',
-      authServerUrl,
+      `/.well-known/oauth-authorization-server${authServerPath}`,
+      `${authServerUrlObj.protocol}//${authServerUrlObj.host}`,
     ).toString();
 
-    const authServerMetadata = await this.fetchAuthorizationServerMetadata(
+    let authServerMetadata = await this.fetchAuthorizationServerMetadata(
       authServerMetadataUrl,
     );
 
-    if (authServerMetadata) {
-      console.log(
-        'OAuth configuration discovered successfully from www-authenticate header',
+    // If that fails and we have a path, also try the root path as a fallback
+    if (!authServerMetadata && authServerPath) {
+      const rootAuthServerMetadataUrl = new URL(
+        '/.well-known/oauth-authorization-server',
+        `${authServerUrlObj.protocol}//${authServerUrlObj.host}`,
+      ).toString();
+
+      authServerMetadata = await this.fetchAuthorizationServerMetadata(
+        rootAuthServerMetadataUrl,
       );
+    }
+
+    if (authServerMetadata) {
       return this.metadataToOAuthConfig(authServerMetadata);
     }
 
diff --git a/packages/core/src/tools/mcp-client.ts b/packages/core/src/tools/mcp-client.ts
index 83bc4024..837b91fc 100644
--- a/packages/core/src/tools/mcp-client.ts
+++ b/packages/core/src/tools/mcp-client.ts
@@ -227,10 +227,16 @@ async function handleAutomaticOAuth(
     };
 
     // Perform OAuth authentication
+    // Pass the server URL for proper discovery
+    const serverUrl = mcpServerConfig.httpUrl || mcpServerConfig.url;
     console.log(
       `Starting OAuth authentication for server '${mcpServerName}'...`,
     );
-    await MCPOAuthProvider.authenticate(mcpServerName, oauthAuthConfig);
+    await MCPOAuthProvider.authenticate(
+      mcpServerName,
+      oauthAuthConfig,
+      serverUrl,
+    );
 
     console.log(
       `OAuth authentication successful for server '${mcpServerName}'`,
@@ -933,12 +939,15 @@ export async function connectToMcpServer(
               };
 
               // Perform OAuth authentication
+              // Pass the server URL for proper discovery
+              const serverUrl = mcpServerConfig.httpUrl || mcpServerConfig.url;
               console.log(
                 `Starting OAuth authentication for server '${mcpServerName}'...`,
               );
               await MCPOAuthProvider.authenticate(
                 mcpServerName,
                 oauthAuthConfig,
+                serverUrl,
               );
 
               // Retry connection with OAuth token