feedback

Author: edelauna

src/i18n/locales/en/mcp.json

@@ -24,5 +24,17 @@
 		"refreshing_all": "Refreshing all MCP servers...",
 		"all_refreshed": "All MCP servers have been refreshed.",
 		"project_config_deleted": "Project MCP configuration file deleted. All project MCP servers have been disconnected."
+	},
+	"oauth": {
+		"callback_title": "OAuth Callback - Roo Code",
+		"failed": "Failed",
+		"success": "Success!",
+		"auth_failed": "Authentication failed: {{error}}. Please check the MCP server logs.",
+		"auth_success": "MCP server authenticated successfully. You can now close this browser tab.",
+		"connection_complete": "The server connection is complete.",
+		"close_countdown": "This tab will attempt to close in <span id=\"count\">{{seconds}}</span>s...",
+		"manual_close": "If the tab did not close, you can safely close it manually.",
+		"invalid_state": "Error: Invalid state parameter",
+		"auth_failed_title": "OAuth Authentication Failed"
 	}
 }

src/services/mcp/McpHub.ts

@@ -27,6 +27,7 @@ import type {
 	McpTool,
 	McpToolCallResponse,
 } from "@roo-code/types"
+import { TOKEN_EXPIRY_BUFFER_MS } from "./utils/constants"
 
 import { t } from "../../i18n"
 
@@ -454,7 +455,7 @@ export class McpHub {
 		const projectConnections = this.connections.filter((conn) => conn.server.source === "project")
 
 		for (const conn of projectConnections) {
-			await this.deleteConnection(conn.server.name, "project")
+			await this.deleteConnection(conn.server.name, conn.server.source)
 		}
 
 		// Clear project servers from the connections list
@@ -797,13 +798,8 @@ export class McpHub {
 
 				// Create an OAuth provider for this server.
 				//
-				// 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.
+				// McpOAuthClientProvider.create() returns a provider instance.
+				// Discovery and callback-server startup are deferred until actually needed.
 				const authProvider = await McpOAuthClientProvider.create(configInjected.url, this.secretStorage, name)
 
 				// Pre-register the OAuth client so the SDK can skip its own
@@ -841,10 +837,10 @@ export class McpHub {
 							if (!reauthPromise) {
 								reauthPromise = this._completeOAuthFlow(
 									authProvider,
-									transport as StreamableHTTPClientTransport,
 									connection as ConnectedMcpConnection,
 									name,
 									source,
+									false, // mid-session re-auth needs toast
 								)
 									.catch((err) => {
 										console.error(`OAuth flow failed for "${name}":`, err)
@@ -965,58 +961,61 @@ export class McpHub {
 					connection.server.status = "connecting"
 
 					void (async () => {
-						const TOKEN_EXPIRY_BUFFER_MS = 5 * 60 * 1000
-
-						// Check if another window already saved valid tokens
-						const existing = await this.secretStorage!.getOAuthData(serverUrl)
-						if (existing && Date.now() < existing.expires_at - TOKEN_EXPIRY_BUFFER_MS) {
-							await streamableHttpAuthProvider.close()
-							await this.deleteConnection(name, source)
-							await this.connectToServer(name, config, source)
-							await this.notifyWebviewOfServerChanges()
-							return
-						}
-
-						// Show a confirmation toast so the user can decide whether to authenticate.
-						// This resolves immediately when the user responds — but connectToServer
-						// has already returned so the panel is not blocked.
-						const choice = await vscode.window.showInformationMessage(
-							`MCP server "${name}" requires authentication.`,
-							"Authenticate",
-						)
-
-						if (choice === "Authenticate") {
-							// Check tokens again — another window may have authed while toast was showing
-							const tokens = await this.secretStorage!.getOAuthData(serverUrl)
-							if (tokens && Date.now() < tokens.expires_at - TOKEN_EXPIRY_BUFFER_MS) {
+						try {
+							// Check if another window already saved valid tokens
+							const existing = await this.secretStorage!.getOAuthData(serverUrl)
+							if (existing && Date.now() < existing.expires_at - TOKEN_EXPIRY_BUFFER_MS) {
 								await streamableHttpAuthProvider.close()
 								await this.deleteConnection(name, source)
 								await this.connectToServer(name, config, source)
 								await this.notifyWebviewOfServerChanges()
 								return
 							}
-							void this._completeOAuthFlow(
-								streamableHttpAuthProvider,
-								transport as StreamableHTTPClientTransport,
-								connection,
-								name,
-								source,
+
+							// Show a confirmation toast so the user can decide whether to authenticate.
+							// This resolves immediately when the user responds — but connectToServer
+							// has already returned so the panel is not blocked.
+							const choice = await vscode.window.showInformationMessage(
+								`MCP server "${name}" requires authentication.`,
+								"Authenticate",
 							)
-						} else {
-							// Toast was dismissed or auto-timed-out.
-							// First do an immediate check — another window may have already
-							// completed auth while the toast was showing.
-							const tokens = await this.secretStorage!.getOAuthData(serverUrl)
-							if (tokens && Date.now() < tokens.expires_at - TOKEN_EXPIRY_BUFFER_MS) {
+
+							if (choice === "Authenticate") {
+								// Check tokens again — another window may have authed while toast was showing
+								const tokens = await this.secretStorage!.getOAuthData(serverUrl)
+								if (tokens && Date.now() < tokens.expires_at - TOKEN_EXPIRY_BUFFER_MS) {
+									await streamableHttpAuthProvider.close()
+									await this.deleteConnection(name, source)
+									await this.connectToServer(name, config, source)
+									await this.notifyWebviewOfServerChanges()
+									return
+								}
+								void this._completeOAuthFlow(
+									streamableHttpAuthProvider,
+									connection,
+									name,
+									source,
+									true, // initial connect already showed toast
+								)
+							} else {
+								// Toast was dismissed or auto-timed-out.
+								// First do an immediate check — another window may have already
+								// completed auth while the toast was showing.
+								const tokens = await this.secretStorage!.getOAuthData(serverUrl)
+								if (tokens && Date.now() < tokens.expires_at - TOKEN_EXPIRY_BUFFER_MS) {
+									await streamableHttpAuthProvider.close()
+									await this.deleteConnection(name, source)
+									await this.connectToServer(name, config, source)
+									await this.notifyWebviewOfServerChanges()
+									return
+								}
+								// Tokens not available yet — start watching for them
 								await streamableHttpAuthProvider.close()
-								await this.deleteConnection(name, source)
-								await this.connectToServer(name, config, source)
-								await this.notifyWebviewOfServerChanges()
-								return
+								this._watchForOAuthTokens(name, source, serverUrl, config)
 							}
-							// Tokens not available yet — start watching for them
+						} catch (error) {
+							console.error(`[McpHub] Initial OAuth flow failed for "${name}":`, error)
 							await streamableHttpAuthProvider.close()
-							this._watchForOAuthTokens(name, source, serverUrl, config)
 						}
 					})()
 
@@ -1067,13 +1066,40 @@ export class McpHub {
 	 */
 	private async _completeOAuthFlow(
 		authProvider: McpOAuthClientProvider,
-		transport: StreamableHTTPClientTransport,
 		connection: ConnectedMcpConnection,
 		name: string,
 		source: "global" | "project",
+		skipToast: boolean = false,
 	): Promise<void> {
 		const config = JSON.parse(connection.server.config)
+		const serverUrl = config.url
+
 		try {
+			if (!skipToast) {
+				// Mid-session re-auth: check if tokens were already refreshed by another window
+				const tokens = await this.secretStorage?.getOAuthData(serverUrl)
+				if (tokens && Date.now() < tokens.expires_at - TOKEN_EXPIRY_BUFFER_MS) {
+					// Tokens are already valid, just reconnect
+					const validatedConfig = this.validateServerConfig(config, name)
+					await this.deleteConnection(name, source)
+					await this.connectToServer(name, validatedConfig, source)
+					await this.notifyWebviewOfServerChanges()
+					return
+				}
+
+				const choice = await vscode.window.showInformationMessage(
+					`MCP server "${name}" requires re-authentication.`,
+					"Authenticate",
+				)
+
+				if (choice !== "Authenticate") {
+					// User cancelled or dismissed toast.
+					// We don't start a watcher here because _completeOAuthFlow is already
+					// running as a detached task. We just let it exit.
+					return
+				}
+			}
+
 			// Open the browser now that the user has confirmed the toast.
 			// redirectToAuthorization() was already called by the SDK (which stored
 			// the URL in _pendingAuthorizationUrl), but deliberately did not open it.
@@ -1130,8 +1156,6 @@ export class McpHub {
 			this._oauthWatchers.delete(watcherKey)
 		}
 
-		const TOKEN_EXPIRY_BUFFER_MS = 5 * 60 * 1000
-
 		// Called when SecretStorage fires onDidChange for this server's key.
 		// Runs in all VS Code windows the instant tokens are saved — no polling delay.
 		const onTokensChanged = async () => {
@@ -1581,11 +1605,6 @@ export class McpHub {
 					// Validate the config
 					const validatedConfig = this.validateServerConfig(parsedConfig, serverName)
 
-					// Clear OAuth tokens for streamable-http servers on restart
-					if (validatedConfig.type === "streamable-http" && this.secretStorage) {
-						await this.secretStorage.deleteOAuthData(validatedConfig.url)
-					}
-
 					// Try to connect again using validated config
 					await this.connectToServer(serverName, validatedConfig, connection.server.source || "global")
 					vscode.window.showInformationMessage(t("mcp:info.server_connected", { serverName }))
@@ -2086,10 +2105,10 @@ export class McpHub {
 				if (!reauthPromise) {
 					reauthPromise = this._completeOAuthFlow(
 						connection.authProvider,
-						connection.transport as StreamableHTTPClientTransport,
 						connection,
 						serverName,
 						source || connection.server.source || "global",
+						false, // mid-session re-auth needs toast
 					).finally(() => {
 						this.reauthPromises.delete(reauthKey)
 					})