fix(McpHub): not blocking on authprovider flow

Author: edelauna

apps/vscode-e2e/src/suite/mcp-oauth.test.ts

@@ -128,7 +128,13 @@ suite("Roo Code MCP OAuth", function () {
 
 			// SDK constructs: new URL("/.well-known/oauth-authorization-server", "http://host/auth")
 			// which resolves to http://host/.well-known/oauth-authorization-server (origin-relative)
-			if (url === "/.well-known/oauth-authorization-server") {
+			// Our custom fetchOAuthAuthServerMetadata constructs the RFC 8414 URL with issuer path:
+			//   /.well-known/oauth-authorization-server/auth  (with issuer path)
+			// Handle BOTH forms so our provider gets _authServerMeta.
+			if (
+				url === "/.well-known/oauth-authorization-server" ||
+				url === "/.well-known/oauth-authorization-server/auth"
+			) {
 				endpointsHit.add("auth-metadata")
 				res.writeHead(200, { "Content-Type": "application/json" })
 				res.end(

src/services/mcp/McpHub.ts

@@ -985,7 +985,7 @@ export class McpHub {
 			// and hits the same broken URL for path-prefixed issuers (see
 			// utils/oauth.ts for upstream issue links).
 			await authProvider.exchangeCodeForTokens(code)
-			await authProvider.close()
+			authProvider.close().catch(console.error)
 
 			// Recover the validated server config stored on the connection so we
 			// can pass it directly to connectToServer without re-reading the file.

src/services/mcp/McpOAuthClientProvider.ts

@@ -156,6 +156,21 @@ export class McpOAuthClientProvider implements OAuthClientProvider {
 	 */
 	async registerClientIfNeeded(): Promise<void> {
 		if (this._clientInfo) return // already registered
+
+		// Check if we have a cached client_id from previous registration
+		const cachedData = await this._secretStorage.getOAuthData(this._serverUrl)
+		if (cachedData?.client_id && cachedData.redirect_uri === this.redirectUrl) {
+			this._clientInfo = {
+				client_id: cachedData.client_id,
+				redirect_uris: [this.redirectUrl],
+				client_name: this._clientName,
+				grant_types: this._grantTypes,
+				response_types: ["code"],
+				token_endpoint_auth_method: this._tokenEndpointAuthMethod,
+			}
+			return
+		}
+
 		if (!this._authServerMeta?.registration_endpoint) return // DCR not supported
 
 		const response = await fetch(this._authServerMeta.registration_endpoint as string, {
@@ -185,7 +200,12 @@ export class McpOAuthClientProvider implements OAuthClientProvider {
 
 	async saveTokens(tokens: OAuthTokens): Promise<void> {
 		const expires_at = tokens.expires_in ? Date.now() + tokens.expires_in * 1000 : Date.now() + 3600 * 1000 // default 1 hour when server omits expires_in
-		await this._secretStorage.saveOAuthData(this._serverUrl, { tokens, expires_at })
+		await this._secretStorage.saveOAuthData(this._serverUrl, {
+			tokens,
+			expires_at,
+			client_id: this._clientInfo?.client_id,
+			redirect_uri: this.redirectUrl,
+		})
 	}
 
 	async redirectToAuthorization(authorizationUrl: URL): Promise<void> {

src/services/mcp/SecretStorageService.ts

@@ -5,6 +5,10 @@ export interface StoredMcpOAuthData {
 	tokens: OAuthTokens
 	/** Unix ms timestamp after which the access token should be considered expired. */
 	expires_at: number
+	/** The client_id used to obtain these tokens (for token reuse without re-registration). */
+	client_id?: string
+	/** The redirect_uri used during client registration (to detect port changes). */
+	redirect_uri?: string
 }
 
 /**

src/services/mcp/__tests__/McpOAuthClientProvider.spec.ts

@@ -39,6 +39,7 @@ mockFetch.mockResolvedValue({
 			issuer: "https://auth.example.com",
 			authorization_endpoint: "https://auth.example.com/authorize",
 			token_endpoint: "https://auth.example.com/token",
+			registration_endpoint: "https://auth.example.com/register",
 			response_types_supported: ["code"],
 			token_endpoint_auth_methods_supported: ["none"],
 			grant_types_supported: ["authorization_code", "refresh_token"],
@@ -543,4 +544,72 @@ describe("McpOAuthClientProvider", () => {
 			expect(stopCallbackServer).toHaveBeenCalledTimes(1)
 		})
 	})
+
+	describe("registerClientIfNeeded", () => {
+		it("should reuse cached client_id when redirect_uri matches", async () => {
+			setupCallbackServerMock()
+			const secretStorage = createMockSecretStorage()
+
+			// Pre-populate storage with cached data
+			await secretStorage.saveOAuthData("https://example.com/mcp", {
+				tokens: { access_token: "cached-token", token_type: "Bearer" },
+				expires_at: Date.now() + 3600000,
+				client_id: "cached-client-id",
+				redirect_uri: "http://localhost:12345/callback",
+			})
+
+			const provider = await McpOAuthClientProvider.create("https://example.com/mcp", secretStorage)
+			await provider.registerClientIfNeeded()
+
+			expect((await provider.clientInformation())?.client_id).toBe("cached-client-id")
+			await provider.close()
+		})
+
+		it("should not reuse cached client_id when redirect_uri does not match", async () => {
+			setupCallbackServerMock()
+			const secretStorage = createMockSecretStorage()
+
+			// Clear previous mocks and set up for this test
+			mockFetch.mockClear()
+			mockFetch.mockResolvedValueOnce({
+				ok: true,
+				json: () =>
+					Promise.resolve({
+						issuer: "https://auth.example.com",
+						authorization_endpoint: "https://auth.example.com/authorize",
+						token_endpoint: "https://auth.example.com/token",
+						registration_endpoint: "https://auth.example.com/register",
+						response_types_supported: ["code"],
+						token_endpoint_auth_methods_supported: ["none"],
+						grant_types_supported: ["authorization_code", "refresh_token"],
+					}),
+			})
+			mockFetch.mockResolvedValueOnce({
+				ok: true,
+				json: () =>
+					Promise.resolve({
+						client_id: "new-client-id",
+						redirect_uris: ["http://localhost:12345/callback"],
+						client_name: "Roo Code",
+						grant_types: ["authorization_code", "refresh_token"],
+						response_types: ["code"],
+						token_endpoint_auth_method: "none",
+					}),
+			})
+
+			// Pre-populate storage with cached data with different redirect_uri
+			await secretStorage.saveOAuthData("https://example.com/mcp", {
+				tokens: { access_token: "cached-token", token_type: "Bearer" },
+				expires_at: Date.now() + 3600000,
+				client_id: "cached-client-id",
+				redirect_uri: "http://localhost:99999/callback", // different port
+			})
+
+			const provider = await McpOAuthClientProvider.create("https://example.com/mcp", secretStorage)
+			await provider.registerClientIfNeeded()
+
+			expect((await provider.clientInformation())?.client_id).toBe("new-client-id")
+			await provider.close()
+		})
+	})
 })