feat(McpHub): cache whether server required OAuth or not

src/services/mcp/McpHub.ts

@@ -795,24 +795,32 @@ export class McpHub {
 					throw new Error("SecretStorageService not initialized — call setSecretStorage() before connecting")
 				}
 
-				// Create an OAuth provider for this server.
+				// Decide whether to perform OAuth discovery (RFC 9728 + RFC 8414)
+				// upfront or skip it to avoid a wasted network round-trip for
+				// non-OAuth servers.
 				//
-				// McpOAuthClientProvider.create() performs OAuth discovery (RFC 9728 +
-				// RFC 8414) once and starts the local callback server so the redirect
-				// URI port is stable before any connect attempt.
-				//
-				// If the server already has a stored token the SDK will use it
-				// transparently; the browser is only opened when a 401 forces a new
-				// authorization flow.
-				const authProvider = await McpOAuthClientProvider.create(configInjected.url, this.secretStorage, name)
+				// Three cases:
+				//  1. SecretStorage has OAuth data → known OAuth server → discover
+				//  2. In-memory negative cache says non-OAuth → skip discovery
+				//  3. Unknown server (first connection) → discover once, cache result
+				const hasOAuthData = await this.secretStorage.hasOAuthData(configInjected.url)
+				const knownNonOAuth = McpOAuthClientProvider.isKnownNonOAuth(configInjected.url)
+				const skipDiscovery = !hasOAuthData && knownNonOAuth
+
+				const authProvider = await McpOAuthClientProvider.create(configInjected.url, this.secretStorage, name, {
+					skipDiscovery,
+				})
 
 				// Pre-register the OAuth client so the SDK can skip its own
 				// registration step (broken for path-prefixed issuers — see
 				// utils/oauth.ts for upstream issue links).
-				try {
-					await authProvider.registerClientIfNeeded()
-				} catch {
-					// Registration may not be supported — the SDK will attempt its own.
+				// Skip when discovery was skipped — there's no metadata to register with.
+				if (!skipDiscovery) {
+					try {
+						await authProvider.registerClientIfNeeded()
+					} catch {
+						// Registration may not be supported — the SDK will attempt its own.
+					}
 				}
 
 				transport = new StreamableHTTPClientTransport(new URL(configInjected.url), {
@@ -959,10 +967,23 @@ export class McpHub {
 				await client.connect(transport)
 			} catch (connectError) {
 				if (connectError instanceof UnauthorizedError && streamableHttpAuthProvider && configInjected.url) {
-					// The server requires OAuth. Mark this connection as "connecting" and
-					// detach the toast + browser flow from the initialization path so that
+					// The server requires OAuth.
+					McpOAuthClientProvider.clearNonOAuthCache(configInjected.url)
+
+					// If discovery was skipped (provider has no metadata), we need to
+					// tear down and reconnect with full discovery now that we know
+					// this is an OAuth server. The reconnect will do discovery since
+					// the negative cache was cleared and no SecretStorage data exists yet.
+					if (!streamableHttpAuthProvider.hasMetadata) {
+						await streamableHttpAuthProvider.close()
+						await this.deleteConnection(name, source)
+						void this.connectToServer(name, config, source)
+						return
+					}
+
+					// Mark this connection as "connecting" and detach the toast +
+					// browser flow from the initialization path so that
 					// waitUntilReady() resolves immediately and the MCP panel can load.
-					const serverUrl = configInjected.url
 					connection.server.status = "connecting"
 
 					void this._initiateOAuthFlow(

src/services/mcp/McpOAuthClientProvider.ts

@@ -31,6 +31,35 @@ import { fetchOAuthAuthServerMetadata } from "./utils/oauth"
  *  6. `await authProvider.close()` when done (success or permanent failure)
  */
 export class McpOAuthClientProvider implements OAuthClientProvider {
+	// ── Static negative cache ────────────────────────────────────────────────
+	// Remembers servers that returned no OAuth metadata so we can skip the
+	// discovery probe on subsequent connection attempts (reconnect, restart).
+	private static _nonOAuthCache = new Map<string, number>() // serverUrl → timestamp
+	private static NON_OAUTH_TTL_MS = 30 * 60 * 1000 // 30 minutes
+
+	static isKnownNonOAuth(serverUrl: string): boolean {
+		const ts = McpOAuthClientProvider._nonOAuthCache.get(serverUrl)
+		if (ts === undefined) return false
+		if (Date.now() - ts > McpOAuthClientProvider.NON_OAUTH_TTL_MS) {
+			McpOAuthClientProvider._nonOAuthCache.delete(serverUrl)
+			return false
+		}
+		return true
+	}
+
+	static markNonOAuth(serverUrl: string): void {
+		McpOAuthClientProvider._nonOAuthCache.set(serverUrl, Date.now())
+	}
+
+	static clearNonOAuthCache(serverUrl?: string): void {
+		if (serverUrl) {
+			McpOAuthClientProvider._nonOAuthCache.delete(serverUrl)
+		} else {
+			McpOAuthClientProvider._nonOAuthCache.clear()
+		}
+	}
+
+	// ── Instance fields ──────────────────────────────────────────────────────
 	private _codeVerifier?: string
 	// Client info is kept in-memory only (not persisted) to avoid stale registrations
 	// when the redirect URI port changes between sessions.
@@ -71,14 +100,25 @@ export class McpOAuthClientProvider implements OAuthClientProvider {
 		serverUrl: string,
 		secretStorage: SecretStorageService,
 		serverName?: string,
+		options?: { skipDiscovery?: boolean },
 	): Promise<McpOAuthClientProvider> {
-		// Fetch auth server metadata once.  Reused for:
-		//  - selecting token_endpoint_auth_method / grant_types / scopes
-		//  - pre-registering the client (registration_endpoint)
-		//  - RFC 8707 resource indicator (injected into authorization URL)
-		const discovery = await fetchOAuthAuthServerMetadata(serverUrl)
-		const authServerMeta = discovery?.authServerMeta ?? null
-		const resourceIndicator = discovery?.resourceIndicator ?? null
+		let authServerMeta: Record<string, any> | null = null
+		let resourceIndicator: string | null = null
+
+		if (!options?.skipDiscovery) {
+			// Fetch auth server metadata once.  Reused for:
+			//  - selecting token_endpoint_auth_method / grant_types / scopes
+			//  - pre-registering the client (registration_endpoint)
+			//  - RFC 8707 resource indicator (injected into authorization URL)
+			const discovery = await fetchOAuthAuthServerMetadata(serverUrl)
+			authServerMeta = discovery?.authServerMeta ?? null
+			resourceIndicator = discovery?.resourceIndicator ?? null
+
+			// Cache the result so subsequent connections can skip the probe.
+			if (!authServerMeta) {
+				McpOAuthClientProvider.markNonOAuth(serverUrl)
+			}
+		}
 
 		// Extract auth-method preferences.
 		// Prefer "none" → first supported → "client_secret_post"
@@ -113,6 +153,11 @@ export class McpOAuthClientProvider implements OAuthClientProvider {
 
 	// ── OAuthClientProvider interface ────────────────────────────────────────
 
+	/** Whether this provider was created with OAuth metadata (discovery succeeded). */
+	get hasMetadata(): boolean {
+		return this._authServerMeta !== null
+	}
+
 	get redirectUrl(): string {
 		return `http://localhost:${this._port}/callback`
 	}

src/services/mcp/SecretStorageService.ts

@@ -45,6 +45,11 @@ export class SecretStorageService {
 		await this._storage.store(this._key(serverUrl), JSON.stringify(data))
 	}
 
+	async hasOAuthData(serverUrl: string): Promise<boolean> {
+		const raw = await this._storage.get(this._key(serverUrl))
+		return raw !== undefined
+	}
+
 	async deleteOAuthData(serverUrl: string): Promise<void> {
 		await this._storage.delete(this._key(serverUrl))
 	}

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

@@ -82,6 +82,43 @@ function setupCallbackServerMock(code = "test-auth-code", state?: string) {
 describe("McpOAuthClientProvider", () => {
 	beforeEach(() => {
 		vi.clearAllMocks()
+		McpOAuthClientProvider.clearNonOAuthCache()
+	})
+
+	describe("static negative cache", () => {
+		it("isKnownNonOAuth returns false for unknown servers", () => {
+			expect(McpOAuthClientProvider.isKnownNonOAuth("https://unknown.com/mcp")).toBe(false)
+		})
+
+		it("markNonOAuth makes isKnownNonOAuth return true", () => {
+			McpOAuthClientProvider.markNonOAuth("https://example.com/mcp")
+			expect(McpOAuthClientProvider.isKnownNonOAuth("https://example.com/mcp")).toBe(true)
+		})
+
+		it("clearNonOAuthCache(url) clears a specific entry", () => {
+			McpOAuthClientProvider.markNonOAuth("https://a.com/mcp")
+			McpOAuthClientProvider.markNonOAuth("https://b.com/mcp")
+			McpOAuthClientProvider.clearNonOAuthCache("https://a.com/mcp")
+			expect(McpOAuthClientProvider.isKnownNonOAuth("https://a.com/mcp")).toBe(false)
+			expect(McpOAuthClientProvider.isKnownNonOAuth("https://b.com/mcp")).toBe(true)
+		})
+
+		it("clearNonOAuthCache() with no arg clears all entries", () => {
+			McpOAuthClientProvider.markNonOAuth("https://a.com/mcp")
+			McpOAuthClientProvider.markNonOAuth("https://b.com/mcp")
+			McpOAuthClientProvider.clearNonOAuthCache()
+			expect(McpOAuthClientProvider.isKnownNonOAuth("https://a.com/mcp")).toBe(false)
+			expect(McpOAuthClientProvider.isKnownNonOAuth("https://b.com/mcp")).toBe(false)
+		})
+
+		it("entries expire after the TTL", () => {
+			const realNow = Date.now()
+			McpOAuthClientProvider.markNonOAuth("https://example.com/mcp")
+			// Advance time past the 30-minute TTL
+			const spy = vi.spyOn(Date, "now").mockReturnValue(realNow + 31 * 60 * 1000)
+			expect(McpOAuthClientProvider.isKnownNonOAuth("https://example.com/mcp")).toBe(false)
+			spy.mockRestore()
+		})
 	})
 
 	describe("create", () => {
@@ -95,6 +132,36 @@ describe("McpOAuthClientProvider", () => {
 			expect(provider.redirectUrl).toBe("http://localhost:0/callback")
 			await provider.close()
 		})
+
+		it("should skip discovery when skipDiscovery option is true", async () => {
+			const secretStorage = createMockSecretStorage()
+			const provider = await McpOAuthClientProvider.create("https://example.com/mcp", secretStorage, undefined, {
+				skipDiscovery: true,
+			})
+
+			expect(discoverOAuthProtectedResourceMetadata).not.toHaveBeenCalled()
+			expect(provider.hasMetadata).toBe(false)
+		})
+
+		it("should have hasMetadata true when discovery succeeds", async () => {
+			const secretStorage = createMockSecretStorage()
+			const provider = await McpOAuthClientProvider.create("https://example.com/mcp", secretStorage)
+
+			expect(discoverOAuthProtectedResourceMetadata).toHaveBeenCalled()
+			expect(provider.hasMetadata).toBe(true)
+		})
+
+		it("should cache server as non-OAuth when discovery fails", async () => {
+			// Use mockImplementationOnce to override just this call
+			;(discoverOAuthProtectedResourceMetadata as any).mockImplementationOnce(() => {
+				throw new Error("not found")
+			})
+			const secretStorage = createMockSecretStorage()
+			const provider = await McpOAuthClientProvider.create("https://no-oauth.example.com/mcp", secretStorage)
+
+			expect(provider.hasMetadata).toBe(false)
+			expect(McpOAuthClientProvider.isKnownNonOAuth("https://no-oauth.example.com/mcp")).toBe(true)
+		})
 	})
 
 	describe("redirectUrl (pre-server-start)", () => {

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

@@ -89,6 +89,31 @@ describe("SecretStorageService", () => {
 		})
 	})
 
+	describe("hasOAuthData", () => {
+		it("should return false when no data stored", async () => {
+			expect(await service.hasOAuthData("https://example.com/mcp")).toBe(false)
+		})
+
+		it("should return true when data is stored", async () => {
+			const data: StoredMcpOAuthData = {
+				tokens: { access_token: "tok", token_type: "Bearer" },
+				expires_at: Date.now() + 3600_000,
+			}
+			await service.saveOAuthData("https://example.com/mcp", data)
+			expect(await service.hasOAuthData("https://example.com/mcp")).toBe(true)
+		})
+
+		it("should return false after data is deleted", async () => {
+			const data: StoredMcpOAuthData = {
+				tokens: { access_token: "tok", token_type: "Bearer" },
+				expires_at: Date.now() + 3600_000,
+			}
+			await service.saveOAuthData("https://example.com/mcp", data)
+			await service.deleteOAuthData("https://example.com/mcp")
+			expect(await service.hasOAuthData("https://example.com/mcp")).toBe(false)
+		})
+	})
+
 	describe("deleteOAuthData", () => {
 		it("should delete stored data", async () => {
 			const data: StoredMcpOAuthData = {