From 4da2acbee4521170003e1ffe219428ef7cbbff5a Mon Sep 17 00:00:00 2001 From: can1357 Date: Tue, 10 Feb 2026 14:31:19 +0100 Subject: [PATCH] fix(coding-agent): addressed review findings for runtime MCP support - Removed unsafe OAuth endpoint extraction from error message text - Fixed PKCE verifier storage with typed #codeVerifier field - Fixed refresh token fallback using access token as refresh token - Enforced restrictive file permissions (0o700/0o600) for MCP configs - Fixed wizard buildConfig() to respect user-chosen env var and header names - Fixed reauth endpoint discovery for non-OAuth servers - Stored original config on connection, resolved config only for transport - Added runtime type validation for enabled/timeout in config loaders - Converted all TS private keywords to ES # private fields - Wrapped uncaught throws in /mcp add with try/catch error handling - Replaced new Promise with Promise.withResolvers() pattern - Sanitized TUI output with replaceTabs/truncateToWidth - Enforced http/https URL validation in add wizard - Fixed greedy /mcp prefix match in input controller - Corrected config filename references in MCP guide - Added server name validation to updateMCPServer - Fixed timeout timer leak in stdio transport --- packages/coding-agent/MCP_COMMAND_GUIDE.md | 2 +- .../coding-agent/src/discovery/builtin.ts | 49 +- .../coding-agent/src/discovery/mcp-json.ts | 28 +- .../coding-agent/src/mcp/config-writer.ts | 19 +- packages/coding-agent/src/mcp/manager.ts | 17 +- .../coding-agent/src/mcp/oauth-discovery.ts | 14 - packages/coding-agent/src/mcp/oauth-flow.ts | 57 +- .../coding-agent/src/mcp/transports/stdio.ts | 51 +- .../src/modes/components/mcp-add-wizard.ts | 1192 +++++++++-------- .../src/modes/controllers/input-controller.ts | 2 +- .../controllers/mcp-command-controller.ts | 241 ++-- 11 files changed, 887 insertions(+), 785 deletions(-) diff --git a/packages/coding-agent/MCP_COMMAND_GUIDE.md b/packages/coding-agent/MCP_COMMAND_GUIDE.md index b52020fc1..654c316d0 100644 --- a/packages/coding-agent/MCP_COMMAND_GUIDE.md +++ b/packages/coding-agent/MCP_COMMAND_GUIDE.md @@ -301,7 +301,7 @@ Project-specific configuration (usually in project root). 3. **Secure sensitive data** - Use OAuth when available - Use shell commands for API keys: `!op read op://vault/key` - - Never commit `.mcp.json` files with plain API keys to version control + - Never commit `.omp/mcp.json` files with plain API keys to version control 4. **Name servers descriptively** - Use purpose-based names: "github-tools", "docs-search" diff --git a/packages/coding-agent/src/discovery/builtin.ts b/packages/coding-agent/src/discovery/builtin.ts index f3341bb11..1e6daa849 100644 --- a/packages/coding-agent/src/discovery/builtin.ts +++ b/packages/coding-agent/src/discovery/builtin.ts @@ -5,6 +5,7 @@ * .pi is an alias for backwards compatibility. */ import * as path from "node:path"; +import { logger } from "@oh-my-pi/pi-utils"; import { registerProvider } from "../capability"; import { type ContextFile, contextFileCapability } from "../capability/context-file"; import { type Extension, type ExtensionManifest, extensionCapability } from "../capability/extension"; @@ -79,10 +80,54 @@ async function loadMCPServers(ctx: LoadContext): Promise> const expanded = expandEnvVarsDeep(data.mcpServers); for (const [serverName, config] of Object.entries(expanded)) { const serverConfig = config as Record; + + // Validate enabled: coerce string "true"/"false", warn on other types + let enabled: boolean | undefined; + if (serverConfig.enabled === undefined || serverConfig.enabled === null) { + enabled = undefined; + } else if (typeof serverConfig.enabled === "boolean") { + enabled = serverConfig.enabled; + } else if (typeof serverConfig.enabled === "string") { + const lower = serverConfig.enabled.toLowerCase(); + if (lower === "false" || lower === "0") enabled = false; + else if (lower === "true" || lower === "1") enabled = true; + else { + logger.warn(`MCP server "${serverName}": invalid enabled value "${serverConfig.enabled}", ignoring`); + enabled = undefined; + } + } else { + logger.warn(`MCP server "${serverName}": invalid enabled type ${typeof serverConfig.enabled}, ignoring`); + enabled = undefined; + } + + // Validate timeout: coerce numeric strings, warn on invalid + let timeout: number | undefined; + if (serverConfig.timeout === undefined || serverConfig.timeout === null) { + timeout = undefined; + } else if (typeof serverConfig.timeout === "number") { + if (Number.isFinite(serverConfig.timeout) && serverConfig.timeout > 0) { + timeout = serverConfig.timeout; + } else { + logger.warn(`MCP server "${serverName}": invalid timeout ${serverConfig.timeout}, ignoring`); + timeout = undefined; + } + } else if (typeof serverConfig.timeout === "string") { + const parsed = Number(serverConfig.timeout); + if (Number.isFinite(parsed) && parsed > 0) { + timeout = parsed; + } else { + logger.warn(`MCP server "${serverName}": invalid timeout "${serverConfig.timeout}", ignoring`); + timeout = undefined; + } + } else { + logger.warn(`MCP server "${serverName}": invalid timeout type ${typeof serverConfig.timeout}, ignoring`); + timeout = undefined; + } + result.push({ name: serverName, - enabled: serverConfig.enabled as boolean | undefined, - timeout: serverConfig.timeout as number | undefined, + enabled, + timeout, command: serverConfig.command as string | undefined, args: serverConfig.args as string[] | undefined, env: serverConfig.env as Record | undefined, diff --git a/packages/coding-agent/src/discovery/mcp-json.ts b/packages/coding-agent/src/discovery/mcp-json.ts index eb9934520..d094e2cb0 100644 --- a/packages/coding-agent/src/discovery/mcp-json.ts +++ b/packages/coding-agent/src/discovery/mcp-json.ts @@ -7,6 +7,7 @@ * Priority: 5 (low, as this is a fallback after tool-specific providers) */ import * as path from "node:path"; +import { logger } from "@oh-my-pi/pi-utils"; import { registerProvider } from "../capability"; import { readFile } from "../capability/fs"; import { type MCPServer, mcpCapability } from "../capability/mcp"; @@ -47,10 +48,33 @@ function transformMCPConfig(config: MCPConfigFile, source: SourceMeta): MCPServe if (config.mcpServers) { for (const [name, serverConfig] of Object.entries(config.mcpServers)) { + // Runtime type validation for user-controlled JSON values + let enabled: boolean | undefined; + if (serverConfig.enabled !== undefined) { + if (typeof serverConfig.enabled === "boolean") { + enabled = serverConfig.enabled; + } else { + logger.warn("MCP server has invalid 'enabled' value, ignoring", { name, value: serverConfig.enabled }); + } + } + + let timeout: number | undefined; + if (serverConfig.timeout !== undefined) { + if ( + typeof serverConfig.timeout === "number" && + Number.isFinite(serverConfig.timeout) && + serverConfig.timeout > 0 + ) { + timeout = serverConfig.timeout; + } else { + logger.warn("MCP server has invalid 'timeout' value, ignoring", { name, value: serverConfig.timeout }); + } + } + const server: MCPServer = { name, - enabled: serverConfig.enabled, - timeout: serverConfig.timeout, + enabled, + timeout, command: serverConfig.command, args: serverConfig.args, env: serverConfig.env, diff --git a/packages/coding-agent/src/mcp/config-writer.ts b/packages/coding-agent/src/mcp/config-writer.ts index beaccdb82..fb43e7889 100644 --- a/packages/coding-agent/src/mcp/config-writer.ts +++ b/packages/coding-agent/src/mcp/config-writer.ts @@ -1,11 +1,12 @@ /** * MCP Configuration File Writer * - * Utilities for reading/writing .mcp.json files at user or project level. + * Utilities for reading/writing .omp/mcp.json files at user or project level. */ import * as fs from "node:fs"; -import { homedir } from "node:os"; +import * as os from "node:os"; import * as path from "node:path"; +import { isEnoent } from "@oh-my-pi/pi-utils"; import { validateServerConfig } from "./config"; import type { MCPConfigFile, MCPServerConfig } from "./types"; @@ -16,7 +17,7 @@ import type { MCPConfigFile, MCPServerConfig } from "./types"; */ export function getMCPConfigPath(scope: "user" | "project", cwd: string): string { if (scope === "user") { - return path.join(homedir(), ".omp", "mcp.json"); + return path.join(os.homedir(), ".omp", "mcp.json"); } return path.join(cwd, ".omp", "mcp.json"); } @@ -31,7 +32,7 @@ export async function readMCPConfigFile(filePath: string): Promise { // Ensure parent directory exists const dir = path.dirname(filePath); - await fs.promises.mkdir(dir, { recursive: true }); + await fs.promises.mkdir(dir, { recursive: true, mode: 0o700 }); // Write to temp file first (atomic write) const tmpPath = `${filePath}.tmp`; const content = JSON.stringify(config, null, 2); - await fs.promises.writeFile(tmpPath, content, "utf-8"); + await fs.promises.writeFile(tmpPath, content, { encoding: "utf-8", mode: 0o600 }); // Rename to final path (atomic on most systems) await fs.promises.rename(tmpPath, filePath); @@ -122,6 +123,12 @@ export async function addMCPServer(filePath: string, name: string, config: MCPSe * @throws Error if validation fails */ export async function updateMCPServer(filePath: string, name: string, config: MCPServerConfig): Promise { + // Validate server name + const nameError = validateServerName(name); + if (nameError) { + throw new Error(nameError); + } + // Validate the config const errors = validateServerConfig(name, config); if (errors.length > 0) { diff --git a/packages/coding-agent/src/mcp/manager.ts b/packages/coding-agent/src/mcp/manager.ts index 90cb13d40..fa5e14221 100644 --- a/packages/coding-agent/src/mcp/manager.ts +++ b/packages/coding-agent/src/mcp/manager.ts @@ -83,7 +83,7 @@ export class MCPManager { #pendingConnections = new Map>(); #pendingToolLoads = new Map>(); #sources = new Map(); - private authStorage: AuthStorage | null = null; + #authStorage: AuthStorage | null = null; constructor( private cwd: string, @@ -94,7 +94,7 @@ export class MCPManager { * Set the auth storage for resolving OAuth credentials. */ setAuthStorage(authStorage: AuthStorage): void { - this.authStorage = authStorage; + this.#authStorage = authStorage; } /** @@ -164,10 +164,13 @@ export class MCPManager { } // Resolve auth config before connecting - const resolvedConfig = await this.resolveAuthConfig(config); + const resolvedConfig = await this.#resolveAuthConfig(config); const connectionPromise = connectToServer(name, resolvedConfig).then( connection => { + // Store original config (without resolved tokens) to keep + // cache keys stable and avoid leaking rotating credentials. + connection.config = config; if (sources[name]) { connection._source = sources[name]; } @@ -329,7 +332,7 @@ export class MCPManager { * Resolve auth and shell-command substitutions in config before connecting. */ async prepareConfig(config: MCPServerConfig): Promise { - return this.resolveAuthConfig(config); + return this.#resolveAuthConfig(config); } /** @@ -401,14 +404,14 @@ export class MCPManager { /** * Resolve OAuth credentials and shell commands in config. */ - private async resolveAuthConfig(config: MCPServerConfig): Promise { + async #resolveAuthConfig(config: MCPServerConfig): Promise { let resolved: MCPServerConfig = { ...config }; const auth = config.auth; - if (auth?.type === "oauth" && auth.credentialId && this.authStorage) { + if (auth?.type === "oauth" && auth.credentialId && this.#authStorage) { const credentialId = auth.credentialId; try { - const credential = this.authStorage.get(credentialId); + const credential = this.#authStorage.get(credentialId); if (credential?.type === "oauth") { if (resolved.type === "http" || resolved.type === "sse") { resolved = { diff --git a/packages/coding-agent/src/mcp/oauth-discovery.ts b/packages/coding-agent/src/mcp/oauth-discovery.ts index 0eee9cc69..1d66d3c33 100644 --- a/packages/coding-agent/src/mcp/oauth-discovery.ts +++ b/packages/coding-agent/src/mcp/oauth-discovery.ts @@ -166,20 +166,6 @@ export function extractOAuthEndpoints(error: Error): OAuthEndpoints | null { }; } - // Try to extract URLs from error message - const urlPattern = /(https?:\/\/[^\s"'<>]+)/g; - const urls = errorMsg.match(urlPattern); - - if (urls && urls.length >= 2) { - // Heuristic: First URL is likely auth, second is token - return { - authorizationUrl: urls[0], - tokenUrl: urls[1], - clientId: clientIdFromAuthUrl(urls[0]), - scopes: scopeFromAuthUrl(urls[0]), - }; - } - return null; } diff --git a/packages/coding-agent/src/mcp/oauth-flow.ts b/packages/coding-agent/src/mcp/oauth-flow.ts index b9021739e..6996224a4 100644 --- a/packages/coding-agent/src/mcp/oauth-flow.ts +++ b/packages/coding-agent/src/mcp/oauth-flow.ts @@ -29,20 +29,21 @@ export interface MCPOAuthConfig { * Supports standard OAuth 2.0 authorization code flow with PKCE. */ export class MCPOAuthFlow extends OAuthCallbackFlow { - private resolvedClientId?: string; - private registeredClientSecret?: string; + #resolvedClientId?: string; + #registeredClientSecret?: string; + #codeVerifier?: string; constructor( private config: MCPOAuthConfig, ctrl: OAuthController, ) { super(ctrl, DEFAULT_PORT, CALLBACK_PATH); - this.resolvedClientId = this.resolveClientId(config); + this.#resolvedClientId = this.#resolveClientId(config); } async generateAuthUrl(state: string, redirectUri: string): Promise<{ url: string; instructions?: string }> { - if (!this.resolvedClientId) { - await this.tryRegisterClient(redirectUri); + if (!this.#resolvedClientId) { + await this.#tryRegisterClient(redirectUri); } const authUrl = new URL(this.config.authorizationUrl); @@ -51,8 +52,8 @@ export class MCPOAuthFlow extends OAuthCallbackFlow { if (!params.get("response_type")) { params.set("response_type", "code"); } - if (this.resolvedClientId && !params.get("client_id")) { - params.set("client_id", this.resolvedClientId); + if (this.#resolvedClientId && !params.get("client_id")) { + params.set("client_id", this.#resolvedClientId); } if (this.config.scopes && !params.get("scope")) { params.set("scope", this.config.scopes); @@ -61,13 +62,13 @@ export class MCPOAuthFlow extends OAuthCallbackFlow { params.set("state", state); // Add PKCE challenge (some providers require it) - const codeVerifier = this.generateCodeVerifier(); - const codeChallenge = await this.generateCodeChallenge(codeVerifier); + const codeVerifier = this.#generateCodeVerifier(); + const codeChallenge = await this.#generateCodeChallenge(codeVerifier); params.set("code_challenge", codeChallenge); params.set("code_challenge_method", "S256"); // Store code verifier for token exchange - (this as any).codeVerifier = codeVerifier; + this.#codeVerifier = codeVerifier; return { url: authUrl.toString() }; } @@ -78,18 +79,18 @@ export class MCPOAuthFlow extends OAuthCallbackFlow { code, redirect_uri: redirectUri, }); - if (this.resolvedClientId) { - params.set("client_id", this.resolvedClientId); + if (this.#resolvedClientId) { + params.set("client_id", this.#resolvedClientId); } // Add code verifier for PKCE - const codeVerifier = (this as any).codeVerifier; - if (codeVerifier) { - params.set("code_verifier", codeVerifier); + if (this.#codeVerifier) { + params.set("code_verifier", this.#codeVerifier); } + this.#codeVerifier = undefined; // Add client secret if provided - const clientSecret = this.config.clientSecret ?? this.registeredClientSecret; + const clientSecret = this.config.clientSecret ?? this.#registeredClientSecret; if (clientSecret) { params.set("client_secret", clientSecret); } @@ -120,7 +121,7 @@ export class MCPOAuthFlow extends OAuthCallbackFlow { return { access: data.access_token, - refresh: data.refresh_token ?? data.access_token, // Fallback to access token if no refresh + refresh: data.refresh_token ?? "", expires, }; } @@ -128,31 +129,31 @@ export class MCPOAuthFlow extends OAuthCallbackFlow { /** * Generate PKCE code verifier (random string). */ - private generateCodeVerifier(): string { + #generateCodeVerifier(): string { const bytes = new Uint8Array(32); crypto.getRandomValues(bytes); - return this.base64UrlEncode(bytes); + return this.#base64UrlEncode(bytes); } /** * Generate PKCE code challenge from verifier. */ - private async generateCodeChallenge(verifier: string): Promise { + async #generateCodeChallenge(verifier: string): Promise { const encoder = new TextEncoder(); const data = encoder.encode(verifier); const hash = await crypto.subtle.digest("SHA-256", data); - return this.base64UrlEncode(new Uint8Array(hash)); + return this.#base64UrlEncode(new Uint8Array(hash)); } /** * Base64 URL encode (without padding). */ - private base64UrlEncode(bytes: Uint8Array): string { + #base64UrlEncode(bytes: Uint8Array): string { const base64 = btoa(String.fromCharCode(...bytes)); return base64.replace(/\+/g, "-").replace(/\//g, "_").replace(/=/g, ""); } - private resolveClientId(config: MCPOAuthConfig): string | undefined { + #resolveClientId(config: MCPOAuthConfig): string | undefined { const fromConfig = config.clientId?.trim(); if (fromConfig) return fromConfig; @@ -166,8 +167,8 @@ export class MCPOAuthFlow extends OAuthCallbackFlow { /** * Try OAuth dynamic client registration when provider requires a client_id. */ - private async tryRegisterClient(redirectUri: string): Promise { - const registrationEndpoint = await this.resolveRegistrationEndpoint(); + async #tryRegisterClient(redirectUri: string): Promise { + const registrationEndpoint = await this.#resolveRegistrationEndpoint(); if (!registrationEndpoint) return; try { @@ -195,17 +196,17 @@ export class MCPOAuthFlow extends OAuthCallbackFlow { }; if (data.client_id && data.client_id.trim() !== "") { - this.resolvedClientId = data.client_id; + this.#resolvedClientId = data.client_id; } if (data.client_secret && data.client_secret.trim() !== "") { - this.registeredClientSecret = data.client_secret; + this.#registeredClientSecret = data.client_secret; } } catch { // Ignore registration failures and continue without client registration. } } - private async resolveRegistrationEndpoint(): Promise { + async #resolveRegistrationEndpoint(): Promise { try { const authorizationEndpoint = new URL(this.config.authorizationUrl); const metadataUrl = new URL("/.well-known/oauth-authorization-server", authorizationEndpoint.origin); diff --git a/packages/coding-agent/src/mcp/transports/stdio.ts b/packages/coding-agent/src/mcp/transports/stdio.ts index fe132aff2..11ab2f185 100644 --- a/packages/coding-agent/src/mcp/transports/stdio.ts +++ b/packages/coding-agent/src/mcp/transports/stdio.ts @@ -161,30 +161,35 @@ export class StdioTransport implements MCPTransport { const timeout = this.config.timeout ?? 30000; - return Promise.race([ - new Promise((resolve, reject) => { - this.#pendingRequests.set(id, { - resolve: resolve as (value: unknown) => void, - reject, - }); + let timer: NodeJS.Timeout | undefined; + try { + return await Promise.race([ + new Promise((resolve, reject) => { + this.#pendingRequests.set(id, { + resolve: resolve as (value: unknown) => void, + reject, + }); - const message = `${JSON.stringify(request)}\n`; - try { - // Bun's FileSink has write() method directly - this.#process!.stdin.write(message); - this.#process!.stdin.flush(); - } catch (error: unknown) { - this.#pendingRequests.delete(id); - reject(error); - } - }), - new Promise((_, reject) => - setTimeout(() => { - this.#pendingRequests.delete(id); - reject(new Error(`Request timeout after ${timeout}ms`)); - }, timeout), - ), - ]); + const message = `${JSON.stringify(request)}\n`; + try { + // Bun's FileSink has write() method directly + this.#process!.stdin.write(message); + this.#process!.stdin.flush(); + } catch (error: unknown) { + this.#pendingRequests.delete(id); + reject(error); + } + }), + new Promise((_, reject) => { + timer = setTimeout(() => { + this.#pendingRequests.delete(id); + reject(new Error(`Request timeout after ${timeout}ms`)); + }, timeout); + }), + ]); + } finally { + clearTimeout(timer); + } } async notify(method: string, params?: Record): Promise { diff --git a/packages/coding-agent/src/modes/components/mcp-add-wizard.ts b/packages/coding-agent/src/modes/components/mcp-add-wizard.ts index bf203568b..0ed9c6c07 100644 --- a/packages/coding-agent/src/modes/components/mcp-add-wizard.ts +++ b/packages/coding-agent/src/modes/components/mcp-add-wizard.ts @@ -3,10 +3,19 @@ * * Interactive multi-step wizard for adding MCP servers. */ -import { Container, Input, matchesKey, Spacer, Text, TruncatedText } from "@oh-my-pi/pi-tui"; +import { + Container, + Input, + matchesKey, + replaceTabs, + Spacer, + Text, + TruncatedText, + truncateToWidth, +} from "@oh-my-pi/pi-tui"; import { validateServerName } from "../../mcp/config-writer"; import { analyzeAuthError, discoverOAuthEndpoints } from "../../mcp/oauth-discovery"; -import type { MCPServerConfig } from "../../mcp/types"; +import type { MCPHttpServerConfig, MCPServerConfig, MCPSseServerConfig, MCPStdioServerConfig } from "../../mcp/types"; import { theme } from "../theme/theme"; import { DynamicBorder } from "./dynamic-border"; @@ -55,9 +64,17 @@ interface WizardState { scope: Scope | null; } +/** Max display width for sanitized error/URL text in wizard TUI */ +const MAX_DISPLAY_WIDTH = 120; + +/** Sanitize a string for TUI display: replace tabs and truncate */ +function sanitize(text: string): string { + return truncateToWidth(replaceTabs(text), MAX_DISPLAY_WIDTH); +} + export class MCPAddWizard extends Container { - private currentStep: WizardStep = "name"; - private state: WizardState = { + #currentStep: WizardStep = "name"; + #state: WizardState = { name: "", transport: null, command: "", @@ -77,17 +94,17 @@ export class MCPAddWizard extends Container { scope: null, }; - private contentContainer: Container; - private inputField: Input | null = null; - private selectedIndex = 0; - private validationError: string | null = null; - private onCompleteCallback: (name: string, config: MCPServerConfig, scope: Scope) => void; - private onCancelCallback: () => void; - private onOAuthCallback: + #contentContainer: Container; + #inputField: Input | null = null; + #selectedIndex = 0; + #validationError: string | null = null; + #onCompleteCallback: (name: string, config: MCPServerConfig, scope: Scope) => void; + #onCancelCallback: () => void; + #onOAuthCallback: | ((authUrl: string, tokenUrl: string, clientId: string, clientSecret: string, scopes: string) => Promise) | null = null; - private onTestConnectionCallback: ((config: MCPServerConfig) => Promise) | null = null; - private onRenderCallback: (() => void) | null = null; + #onTestConnectionCallback: ((config: MCPServerConfig) => Promise) | null = null; + #onRenderCallback: (() => void) | null = null; constructor( onComplete: (name: string, config: MCPServerConfig, scope: Scope) => void, @@ -104,14 +121,14 @@ export class MCPAddWizard extends Container { initialName?: string, ) { super(); - this.onCompleteCallback = onComplete; - this.onCancelCallback = onCancel; - this.onOAuthCallback = onOAuth ?? null; - this.onTestConnectionCallback = onTestConnection ?? null; - this.onRenderCallback = onRender ?? null; + this.#onCompleteCallback = onComplete; + this.#onCancelCallback = onCancel; + this.#onOAuthCallback = onOAuth ?? null; + this.#onTestConnectionCallback = onTestConnection ?? null; + this.#onRenderCallback = onRender ?? null; if (initialName && initialName.trim().length > 0) { - this.state.name = initialName.trim(); - this.currentStep = "transport"; + this.#state.name = initialName.trim(); + this.#currentStep = "transport"; } // Add border @@ -123,8 +140,8 @@ export class MCPAddWizard extends Container { this.addChild(new Spacer(1)); // Content container for step-specific content - this.contentContainer = new Container(); - this.addChild(this.contentContainer); + this.#contentContainer = new Container(); + this.addChild(this.#contentContainer); this.addChild(new Spacer(1)); @@ -132,113 +149,103 @@ export class MCPAddWizard extends Container { this.addChild(new DynamicBorder()); // Render first step - this.renderStep(); + this.#renderStep(); } - private requestRender(): void { - this.onRenderCallback?.(); + #requestRender(): void { + this.#onRenderCallback?.(); } - /** - * Update focus is no longer needed - wizard keeps focus and delegates to Input - */ - private updateFocus(): void { - // No-op: wizard maintains focus and delegates keystrokes to Input - } + #renderStep(): void { + this.#contentContainer.clear(); + this.#inputField = null; // Reset input field - private renderStep(): void { - this.contentContainer.clear(); - this.inputField = null; // Reset input field - - switch (this.currentStep) { + switch (this.#currentStep) { case "name": - this.renderNameStep(); + this.#renderNameStep(); break; case "transport": - this.renderTransportStep(); + this.#renderTransportStep(); break; case "command": - this.renderCommandStep(); + this.#renderCommandStep(); break; case "args": - this.renderArgsStep(); + this.#renderArgsStep(); break; case "url": - this.renderUrlStep(); + this.#renderUrlStep(); break; case "auth-method": - this.renderAuthMethodStep(); + this.#renderAuthMethodStep(); break; case "oauth-error": - this.renderOAuthErrorStep(); + this.#renderOAuthErrorStep(); break; case "oauth-auth-url": - this.renderOAuthAuthUrlStep(); + this.#renderOAuthAuthUrlStep(); break; case "oauth-token-url": - this.renderOAuthTokenUrlStep(); + this.#renderOAuthTokenUrlStep(); break; case "oauth-client-id": - this.renderOAuthClientIdStep(); + this.#renderOAuthClientIdStep(); break; case "oauth-client-secret": - this.renderOAuthClientSecretStep(); + this.#renderOAuthClientSecretStep(); break; case "oauth-scopes": - this.renderOAuthScopesStep(); + this.#renderOAuthScopesStep(); break; case "apikey": - this.renderApiKeyStep(); + this.#renderApiKeyStep(); break; case "auth-location": - this.renderAuthLocationStep(); + this.#renderAuthLocationStep(); break; case "env-var-name": - this.renderEnvVarNameStep(); + this.#renderEnvVarNameStep(); break; case "header-name": - this.renderHeaderNameStep(); + this.#renderHeaderNameStep(); break; case "scope": - this.renderScopeStep(); + this.#renderScopeStep(); break; case "confirm": - this.renderConfirmStep(); + this.#renderConfirmStep(); break; } } - private renderNameStep(): void { - this.contentContainer.addChild(new Text(theme.fg("accent", "Step 1: Server Name"))); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text("Enter a unique name for this server:", 0, 0)); - this.contentContainer.addChild(new Spacer(1)); + #renderNameStep(): void { + this.#contentContainer.addChild(new Text(theme.fg("accent", "Step 1: Server Name"))); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text("Enter a unique name for this server:", 0, 0)); + this.#contentContainer.addChild(new Spacer(1)); - this.inputField = new Input(); - this.inputField.setValue(this.state.name); - this.contentContainer.addChild(this.inputField); - this.contentContainer.addChild(new Spacer(1)); + this.#inputField = new Input(); + this.#inputField.setValue(this.#state.name); + this.#contentContainer.addChild(this.#inputField); + this.#contentContainer.addChild(new Spacer(1)); // Show validation error if any - if (this.validationError) { - this.contentContainer.addChild(new Text(theme.fg("error", `✗ ${this.validationError}`), 0, 0)); - this.contentContainer.addChild(new Spacer(1)); + if (this.#validationError) { + this.#contentContainer.addChild(new Text(theme.fg("error", `✗ ${sanitize(this.#validationError)}`), 0, 0)); + this.#contentContainer.addChild(new Spacer(1)); } - this.contentContainer.addChild( + this.#contentContainer.addChild( new Text(theme.fg("muted", "[Only letters, numbers, dash, underscore, dot]"), 0, 0), ); - this.contentContainer.addChild(new Text(theme.fg("muted", "[Enter to continue, Esc to cancel]"), 0, 0)); - - // Set focus to input field - this.updateFocus(); + this.#contentContainer.addChild(new Text(theme.fg("muted", "[Enter to continue, Esc to cancel]"), 0, 0)); } - private renderTransportStep(): void { - this.contentContainer.addChild(new Text(theme.fg("accent", "Step 2: Transport Type"))); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text("Select the transport type:", 0, 0)); - this.contentContainer.addChild(new Spacer(1)); + #renderTransportStep(): void { + this.#contentContainer.addChild(new Text(theme.fg("accent", "Step 2: Transport Type"))); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text("Select the transport type:", 0, 0)); + this.#contentContainer.addChild(new Spacer(1)); const options = [ { value: "stdio" as const, label: "stdio (Local process)" }, @@ -248,74 +255,68 @@ export class MCPAddWizard extends Container { for (let i = 0; i < options.length; i++) { const option = options[i]; - const isSelected = i === this.selectedIndex; + const isSelected = i === this.#selectedIndex; const prefix = isSelected ? theme.fg("accent", `${theme.nav.cursor} `) : " "; const text = isSelected ? theme.fg("accent", option.label) : option.label; - this.contentContainer.addChild(new Text(prefix + text, 0, 0)); + this.#contentContainer.addChild(new Text(prefix + text, 0, 0)); } - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild( + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild( new Text(theme.fg("muted", "[↑↓ to navigate, Enter to select, Esc to cancel]"), 0, 0), ); } - private renderCommandStep(): void { - this.contentContainer.addChild(new Text(theme.fg("accent", "Step 3: Command"))); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text("Enter the command to run:", 0, 0)); - this.contentContainer.addChild(new Spacer(1)); + #renderCommandStep(): void { + this.#contentContainer.addChild(new Text(theme.fg("accent", "Step 3: Command"))); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text("Enter the command to run:", 0, 0)); + this.#contentContainer.addChild(new Spacer(1)); - this.inputField = new Input(); - this.inputField.setValue(this.state.command); - this.contentContainer.addChild(this.inputField); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text(theme.fg("muted", "[Enter to continue, Esc to go back]"), 0, 0)); - - this.updateFocus(); + this.#inputField = new Input(); + this.#inputField.setValue(this.#state.command); + this.#contentContainer.addChild(this.#inputField); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text(theme.fg("muted", "[Enter to continue, Esc to go back]"), 0, 0)); } - private renderArgsStep(): void { - this.contentContainer.addChild(new Text(theme.fg("accent", "Step 4: Arguments (Optional)"))); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text("Enter command arguments (space-separated):", 0, 0)); - this.contentContainer.addChild(new Spacer(1)); + #renderArgsStep(): void { + this.#contentContainer.addChild(new Text(theme.fg("accent", "Step 4: Arguments (Optional)"))); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text("Enter command arguments (space-separated):", 0, 0)); + this.#contentContainer.addChild(new Spacer(1)); - this.inputField = new Input(); - this.inputField.setValue(this.state.args); - this.contentContainer.addChild(this.inputField); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text(theme.fg("muted", "[Press Enter to skip or continue]"), 0, 0)); - - this.updateFocus(); + this.#inputField = new Input(); + this.#inputField.setValue(this.#state.args); + this.#contentContainer.addChild(this.#inputField); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text(theme.fg("muted", "[Press Enter to skip or continue]"), 0, 0)); } - private renderUrlStep(): void { - this.contentContainer.addChild(new Text(theme.fg("accent", "Step 3: Server URL"))); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text("Enter the server URL:", 0, 0)); - this.contentContainer.addChild(new Spacer(1)); + #renderUrlStep(): void { + this.#contentContainer.addChild(new Text(theme.fg("accent", "Step 3: Server URL"))); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text("Enter the server URL:", 0, 0)); + this.#contentContainer.addChild(new Spacer(1)); - this.inputField = new Input(); - this.inputField.setValue(this.state.url); - this.contentContainer.addChild(this.inputField); - this.contentContainer.addChild(new Spacer(1)); + this.#inputField = new Input(); + this.#inputField.setValue(this.#state.url); + this.#contentContainer.addChild(this.#inputField); + this.#contentContainer.addChild(new Spacer(1)); // Show validation error if any - if (this.validationError) { - this.contentContainer.addChild(new Text(theme.fg("error", `✗ ${this.validationError}`), 0, 0)); - this.contentContainer.addChild(new Spacer(1)); + if (this.#validationError) { + this.#contentContainer.addChild(new Text(theme.fg("error", `✗ ${sanitize(this.#validationError)}`), 0, 0)); + this.#contentContainer.addChild(new Spacer(1)); } - this.contentContainer.addChild(new Text(theme.fg("muted", "[Must start with http:// or https://]"), 0, 0)); - this.contentContainer.addChild(new Text(theme.fg("muted", "[Enter to continue, Esc to go back]"), 0, 0)); - - this.updateFocus(); + this.#contentContainer.addChild(new Text(theme.fg("muted", "[Must start with http:// or https://]"), 0, 0)); + this.#contentContainer.addChild(new Text(theme.fg("muted", "[Enter to continue, Esc to go back]"), 0, 0)); } - private renderAuthLocationStep(): void { - this.contentContainer.addChild(new Text(theme.fg("accent", "Step: How to provide the key?"))); - this.contentContainer.addChild(new Spacer(1)); + #renderAuthLocationStep(): void { + this.#contentContainer.addChild(new Text(theme.fg("accent", "Step: How to provide the key?"))); + this.#contentContainer.addChild(new Spacer(1)); const options = [ { value: "env" as const, label: "Environment variable" }, @@ -324,51 +325,47 @@ export class MCPAddWizard extends Container { for (let i = 0; i < options.length; i++) { const option = options[i]; - const isSelected = i === this.selectedIndex; + const isSelected = i === this.#selectedIndex; const prefix = isSelected ? theme.fg("accent", `${theme.nav.cursor} `) : " "; const text = isSelected ? theme.fg("accent", option.label) : option.label; - this.contentContainer.addChild(new Text(prefix + text, 0, 0)); + this.#contentContainer.addChild(new Text(prefix + text, 0, 0)); } - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild( + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild( new Text(theme.fg("muted", "[↑↓ to navigate, Enter to select, Esc to go back]"), 0, 0), ); } - private renderEnvVarNameStep(): void { - this.contentContainer.addChild(new Text(theme.fg("accent", "Step: Environment Variable Name"))); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text("Enter the environment variable name:", 0, 0)); - this.contentContainer.addChild(new Spacer(1)); + #renderEnvVarNameStep(): void { + this.#contentContainer.addChild(new Text(theme.fg("accent", "Step: Environment Variable Name"))); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text("Enter the environment variable name:", 0, 0)); + this.#contentContainer.addChild(new Spacer(1)); - this.inputField = new Input(); - this.inputField.setValue(this.state.envVarName); - this.contentContainer.addChild(this.inputField); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text(theme.fg("muted", "[Enter to continue, Esc to go back]"), 0, 0)); - - this.updateFocus(); + this.#inputField = new Input(); + this.#inputField.setValue(this.#state.envVarName); + this.#contentContainer.addChild(this.#inputField); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text(theme.fg("muted", "[Enter to continue, Esc to go back]"), 0, 0)); } - private renderHeaderNameStep(): void { - this.contentContainer.addChild(new Text(theme.fg("accent", "Step: HTTP Header Name"))); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text("Enter the HTTP header name:", 0, 0)); - this.contentContainer.addChild(new Spacer(1)); + #renderHeaderNameStep(): void { + this.#contentContainer.addChild(new Text(theme.fg("accent", "Step: HTTP Header Name"))); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text("Enter the HTTP header name:", 0, 0)); + this.#contentContainer.addChild(new Spacer(1)); - this.inputField = new Input(); - this.inputField.setValue(this.state.headerName); - this.contentContainer.addChild(this.inputField); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text(theme.fg("muted", "[Enter to continue, Esc to go back]"), 0, 0)); - - this.updateFocus(); + this.#inputField = new Input(); + this.#inputField.setValue(this.#state.headerName); + this.#contentContainer.addChild(this.#inputField); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text(theme.fg("muted", "[Enter to continue, Esc to go back]"), 0, 0)); } - private renderScopeStep(): void { - this.contentContainer.addChild(new Text(theme.fg("accent", "Step: Configuration Scope"))); - this.contentContainer.addChild(new Spacer(1)); + #renderScopeStep(): void { + this.#contentContainer.addChild(new Text(theme.fg("accent", "Step: Configuration Scope"))); + this.#contentContainer.addChild(new Spacer(1)); const options = [ { value: "user" as const, label: "User level (~/.omp/mcp.json)" }, @@ -377,65 +374,65 @@ export class MCPAddWizard extends Container { for (let i = 0; i < options.length; i++) { const option = options[i]; - const isSelected = i === this.selectedIndex; + const isSelected = i === this.#selectedIndex; const prefix = isSelected ? theme.fg("accent", `${theme.nav.cursor} `) : " "; const text = isSelected ? theme.fg("accent", option.label) : option.label; - this.contentContainer.addChild(new Text(prefix + text, 0, 0)); + this.#contentContainer.addChild(new Text(prefix + text, 0, 0)); } - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild( + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild( new Text(theme.fg("muted", "[↑↓ to navigate, Enter to select, Esc to go back]"), 0, 0), ); } - private renderConfirmStep(): void { - this.contentContainer.addChild(new Text(theme.fg("accent", "Review Configuration"))); - this.contentContainer.addChild(new Spacer(1)); + #renderConfirmStep(): void { + this.#contentContainer.addChild(new Text(theme.fg("accent", "Review Configuration"))); + this.#contentContainer.addChild(new Spacer(1)); // Show summary - this.contentContainer.addChild(new Text(`Name: ${theme.fg("accent", this.state.name)}`, 0, 0)); - this.contentContainer.addChild(new Text(`Type: ${this.state.transport}`, 0, 0)); + this.#contentContainer.addChild(new Text(`Name: ${theme.fg("accent", this.#state.name)}`, 0, 0)); + this.#contentContainer.addChild(new Text(`Type: ${this.#state.transport}`, 0, 0)); - if (this.state.transport === "stdio") { - this.contentContainer.addChild(new Text(`Command: ${this.state.command}`, 0, 0)); - if (this.state.args) { - this.contentContainer.addChild(new Text(`Args: ${this.state.args}`, 0, 0)); + if (this.#state.transport === "stdio") { + this.#contentContainer.addChild(new Text(`Command: ${this.#state.command}`, 0, 0)); + if (this.#state.args) { + this.#contentContainer.addChild(new Text(`Args: ${this.#state.args}`, 0, 0)); } } else { - this.contentContainer.addChild(new Text(`URL: ${this.state.url}`, 0, 0)); + this.#contentContainer.addChild(new Text(`URL: ${sanitize(this.#state.url)}`, 0, 0)); } // Auth info - if (this.state.authMethod === "none") { - this.contentContainer.addChild(new Text("Auth: None", 0, 0)); - } else if (this.state.authMethod === "oauth") { - this.contentContainer.addChild(new Text("Auth: OAuth (authenticated)", 0, 0)); - } else if (this.state.authMethod === "manual") { - if (this.state.authLocation === "env") { - this.contentContainer.addChild(new Text(`Auth: API key via env (${this.state.envVarName})`, 0, 0)); + if (this.#state.authMethod === "none") { + this.#contentContainer.addChild(new Text("Auth: None", 0, 0)); + } else if (this.#state.authMethod === "oauth") { + this.#contentContainer.addChild(new Text("Auth: OAuth (authenticated)", 0, 0)); + } else if (this.#state.authMethod === "manual") { + if (this.#state.authLocation === "env") { + this.#contentContainer.addChild(new Text(`Auth: API key via env (${this.#state.envVarName})`, 0, 0)); } else { - this.contentContainer.addChild(new Text(`Auth: API key via header (${this.state.headerName})`, 0, 0)); + this.#contentContainer.addChild(new Text(`Auth: API key via header (${this.#state.headerName})`, 0, 0)); } } - const scopeLabel = this.state.scope === "user" ? "User level" : "Project level"; - this.contentContainer.addChild(new Text(`Scope: ${scopeLabel}`, 0, 0)); + const scopeLabel = this.#state.scope === "user" ? "User level" : "Project level"; + this.#contentContainer.addChild(new Text(`Scope: ${scopeLabel}`, 0, 0)); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text("Save this configuration?", 0, 0)); - this.contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text("Save this configuration?", 0, 0)); + this.#contentContainer.addChild(new Spacer(1)); const options = ["Yes", "No"]; for (let i = 0; i < options.length; i++) { - const isSelected = i === this.selectedIndex; + const isSelected = i === this.#selectedIndex; const prefix = isSelected ? theme.fg("accent", `${theme.nav.cursor} `) : " "; const text = isSelected ? theme.fg("accent", options[i]) : options[i]; - this.contentContainer.addChild(new Text(prefix + text, 0, 0)); + this.#contentContainer.addChild(new Text(prefix + text, 0, 0)); } - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild( + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild( new Text(theme.fg("muted", "[↑↓ to navigate, Enter to select, Esc to go back]"), 0, 0), ); } @@ -444,69 +441,69 @@ export class MCPAddWizard extends Container { // Handle Ctrl+C to cancel wizard immediately if (keyData === "\x03") { // Ctrl+C pressed - cancel wizard - this.onCancelCallback(); + this.#onCancelCallback(); return; } // Handle Escape (always handled by wizard) if (matchesKey(keyData, "escape")) { - if (this.currentStep === "name") { + if (this.#currentStep === "name") { // Cancel wizard - this.onCancelCallback(); + this.#onCancelCallback(); return; } // Go back to previous step - this.goBack(); + this.#goBack(); return; } // If we have an input field, let it handle the input - if (this.inputField) { + if (this.#inputField) { // Handle Enter to proceed if (matchesKey(keyData, "enter") || matchesKey(keyData, "return") || keyData === "\n") { - this.saveInputAndProceed(); + this.#saveInputAndProceed(); return; } // Pass all other keys to the input field - this.inputField.handleInput(keyData); + this.#inputField.handleInput(keyData); return; } // Selector steps - handle Enter if (matchesKey(keyData, "enter") || matchesKey(keyData, "return") || keyData === "\n") { - this.selectCurrentOption(); + this.#selectCurrentOption(); return; } // Handle up/down arrows for selectors if (matchesKey(keyData, "up")) { - this.moveSelection(-1); + this.#moveSelection(-1); return; } if (matchesKey(keyData, "down")) { - this.moveSelection(1); + this.#moveSelection(1); return; } } - private saveInputAndProceed(): void { - if (!this.inputField) return; + #saveInputAndProceed(): void { + if (!this.#inputField) return; - const value = this.inputField.getValue().trim(); + const value = this.#inputField.getValue().trim(); - switch (this.currentStep) { + switch (this.#currentStep) { case "name": { // Validate server name const nameError = validateServerName(value); if (nameError) { - this.validationError = nameError; - this.renderStep(); + this.#validationError = nameError; + this.#renderStep(); return; } - this.validationError = null; - this.state.name = value; - this.currentStep = "transport"; - this.selectedIndex = 0; + this.#validationError = null; + this.#state.name = value; + this.#currentStep = "transport"; + this.#selectedIndex = 0; break; } case "command": @@ -514,150 +511,157 @@ export class MCPAddWizard extends Container { // Command is required return; } - this.state.command = value; - this.currentStep = "args"; + this.#state.command = value; + this.#currentStep = "args"; break; case "args": - this.state.args = value; // Optional - void this.testConnectionAndDetectAuth(); + this.#state.args = value; // Optional + void this.#testConnectionAndDetectAuth(); return; - case "url": + case "url": { // Validate URL if (!value) { - this.validationError = "URL is required"; - this.renderStep(); + this.#validationError = "URL is required"; + this.#renderStep(); return; } + let parsedUrl: URL; try { - new URL(value); - this.validationError = null; + parsedUrl = new URL(value); } catch { - this.validationError = "Invalid URL format (must start with http:// or https://)"; - this.renderStep(); + this.#validationError = "Invalid URL format (must start with http:// or https://)"; + this.#renderStep(); return; } - this.state.url = value; - void this.testConnectionAndDetectAuth(); + if (parsedUrl.protocol !== "http:" && parsedUrl.protocol !== "https:") { + this.#validationError = "URL must use http:// or https:// scheme"; + this.#renderStep(); + return; + } + this.#validationError = null; + this.#state.url = value; + void this.#testConnectionAndDetectAuth(); return; + } case "oauth-auth-url": if (!value) return; - this.state.oauthAuthUrl = value; - this.currentStep = "oauth-token-url"; + this.#state.oauthAuthUrl = value; + this.#currentStep = "oauth-token-url"; break; case "oauth-token-url": if (!value) return; - this.state.oauthTokenUrl = value; - this.currentStep = "oauth-client-id"; + this.#state.oauthTokenUrl = value; + this.#currentStep = "oauth-client-id"; break; case "oauth-client-id": if (!value) return; - this.state.oauthClientId = value; - this.currentStep = "oauth-client-secret"; + this.#state.oauthClientId = value; + this.#currentStep = "oauth-client-secret"; break; case "oauth-client-secret": - this.state.oauthClientSecret = value; // Optional - this.currentStep = "oauth-scopes"; + this.#state.oauthClientSecret = value; // Optional + this.#currentStep = "oauth-scopes"; break; case "oauth-scopes": - this.state.oauthScopes = value; // Optional + this.#state.oauthScopes = value; // Optional // Launch OAuth flow - void this.launchOAuthFlow(); + void this.#launchOAuthFlow(); return; case "apikey": if (!value) { // API key is required return; } - this.state.apiKey = value; + this.#state.apiKey = value; // Determine auth location based on transport - if (this.state.transport === "stdio") { - this.currentStep = "env-var-name"; + if (this.#state.transport === "stdio") { + this.#currentStep = "env-var-name"; } else { - this.currentStep = "auth-location"; - this.selectedIndex = 0; + this.#currentStep = "auth-location"; + this.#selectedIndex = 0; } break; case "env-var-name": if (!value) { return; } - this.state.envVarName = value; - this.state.authLocation = "env"; - this.currentStep = "scope"; - this.selectedIndex = 0; + this.#state.envVarName = value; + this.#state.authLocation = "env"; + this.#currentStep = "scope"; + this.#selectedIndex = 0; break; case "header-name": if (!value) { return; } - this.state.headerName = value; - this.state.authLocation = "header"; - this.currentStep = "scope"; - this.selectedIndex = 0; + this.#state.headerName = value; + this.#state.authLocation = "header"; + this.#currentStep = "scope"; + this.#selectedIndex = 0; break; } - this.inputField = null; - this.renderStep(); + this.#inputField = null; + this.#renderStep(); } - private selectCurrentOption(): void { - switch (this.currentStep) { + #selectCurrentOption(): void { + switch (this.#currentStep) { case "transport": { const transports: TransportType[] = ["stdio", "http", "sse"]; - this.state.transport = transports[this.selectedIndex]; - this.currentStep = this.state.transport === "stdio" ? "command" : "url"; + this.#state.transport = transports[this.#selectedIndex]; + this.#currentStep = this.#state.transport === "stdio" ? "command" : "url"; break; } case "auth-method": { const authMethods: Array<"oauth" | "manual"> = ["oauth", "manual"]; - this.state.authMethod = authMethods[this.selectedIndex]; - if (this.state.authMethod === "oauth") { - this.currentStep = "oauth-auth-url"; + this.#state.authMethod = authMethods[this.#selectedIndex]; + if (this.#state.authMethod === "oauth") { + this.#currentStep = "oauth-auth-url"; } else { // manual - this.currentStep = "apikey"; + this.#currentStep = "apikey"; } break; } case "oauth-error": - if (this.selectedIndex === 0) { - void this.launchOAuthFlow(); + if (this.#selectedIndex === 0) { + void this.#launchOAuthFlow(); } else { - this.currentStep = "oauth-auth-url"; + this.#currentStep = "oauth-auth-url"; } return; case "auth-location": { const authLocations: Array<"env" | "header"> = ["env", "header"]; - this.state.authLocation = authLocations[this.selectedIndex]; - if (this.state.authLocation === "env") { - this.currentStep = "env-var-name"; + this.#state.authLocation = authLocations[this.#selectedIndex]; + if (this.#state.authLocation === "env") { + this.#currentStep = "env-var-name"; } else { - this.currentStep = "header-name"; + this.#currentStep = "header-name"; } break; } case "scope": { const scopes: Scope[] = ["user", "project"]; - this.state.scope = scopes[this.selectedIndex]; + this.#state.scope = scopes[this.#selectedIndex]; // Auto-save once scope is selected. - this.complete(); + this.#complete(); return; } } - this.renderStep(); + this.#renderStep(); } - private moveSelection(delta: number): void { - const maxIndex = this.getMaxIndexForCurrentStep(); - this.selectedIndex = (this.selectedIndex + delta + maxIndex + 1) % (maxIndex + 1); - this.renderStep(); - this.requestRender(); + #moveSelection(delta: number): void { + const maxIndex = this.#getMaxIndexForCurrentStep(); + this.#selectedIndex = (this.#selectedIndex + delta + maxIndex + 1) % (maxIndex + 1); + this.#renderStep(); + this.#requestRender(); } - private getMaxIndexForCurrentStep(): number { - switch (this.currentStep) { + #getMaxIndexForCurrentStep(): number { + switch (this.#currentStep) { case "transport": return 2; // 3 options case "auth-method": @@ -675,49 +679,49 @@ export class MCPAddWizard extends Container { } } - private goBack(): void { + #goBack(): void { // Navigate to previous step - switch (this.currentStep) { + switch (this.#currentStep) { case "transport": - this.currentStep = "name"; + this.#currentStep = "name"; break; case "command": case "url": - this.currentStep = "transport"; - this.selectedIndex = this.state.transport === "stdio" ? 0 : this.state.transport === "http" ? 1 : 2; + this.#currentStep = "transport"; + this.#selectedIndex = this.#state.transport === "stdio" ? 0 : this.#state.transport === "http" ? 1 : 2; break; case "args": - this.currentStep = "command"; + this.#currentStep = "command"; break; case "auth-method": // Go back to url or args depending on transport - if (this.state.transport === "stdio") { - this.currentStep = "args"; + if (this.#state.transport === "stdio") { + this.#currentStep = "args"; } else { - this.currentStep = "url"; + this.#currentStep = "url"; } break; case "oauth-auth-url": case "apikey": // Go back to transport-specific connection step - if (this.state.transport === "stdio") { - this.currentStep = "args"; + if (this.#state.transport === "stdio") { + this.#currentStep = "args"; } else { - this.currentStep = "url"; + this.#currentStep = "url"; } break; case "auth-location": // Go back to API key input - this.currentStep = "apikey"; + this.#currentStep = "apikey"; break; case "env-var-name": case "header-name": // Go back to auth location selection (for HTTP) or directly to apikey (for stdio) - if (this.state.transport === "stdio") { - this.currentStep = "apikey"; + if (this.#state.transport === "stdio") { + this.#currentStep = "apikey"; } else { - this.currentStep = "auth-location"; - this.selectedIndex = this.state.authLocation === "env" ? 0 : 1; + this.#currentStep = "auth-location"; + this.#selectedIndex = this.#state.authLocation === "env" ? 0 : 1; } break; case "oauth-token-url": @@ -725,44 +729,44 @@ export class MCPAddWizard extends Container { case "oauth-client-secret": case "oauth-scopes": // Go back through OAuth flow - if (this.currentStep === "oauth-token-url") { - this.currentStep = "oauth-auth-url"; - } else if (this.currentStep === "oauth-client-id") { - this.currentStep = "oauth-token-url"; - } else if (this.currentStep === "oauth-client-secret") { - this.currentStep = "oauth-client-id"; - } else if (this.currentStep === "oauth-scopes") { - this.currentStep = "oauth-client-secret"; + if (this.#currentStep === "oauth-token-url") { + this.#currentStep = "oauth-auth-url"; + } else if (this.#currentStep === "oauth-client-id") { + this.#currentStep = "oauth-token-url"; + } else if (this.#currentStep === "oauth-client-secret") { + this.#currentStep = "oauth-client-id"; + } else if (this.#currentStep === "oauth-scopes") { + this.#currentStep = "oauth-client-secret"; } break; case "scope": // Go back to last authentication step - if (this.state.authMethod === "oauth") { - this.currentStep = "oauth-scopes"; + if (this.#state.authMethod === "oauth") { + this.#currentStep = "oauth-scopes"; } else { // manual - go back to env var name or header name - if (this.state.authLocation === "env") { - this.currentStep = "env-var-name"; + if (this.#state.authLocation === "env") { + this.#currentStep = "env-var-name"; } else { - this.currentStep = "header-name"; + this.#currentStep = "header-name"; } } break; case "oauth-error": - this.currentStep = "oauth-auth-url"; + this.#currentStep = "oauth-auth-url"; break; case "confirm": - this.currentStep = "scope"; - this.selectedIndex = this.state.scope === "user" ? 0 : 1; + this.#currentStep = "scope"; + this.#selectedIndex = this.#state.scope === "user" ? 0 : 1; break; } - this.renderStep(); + this.#renderStep(); } - private renderAuthMethodStep(): void { - this.contentContainer.addChild(new Text(theme.fg("accent", "Step: Authentication Method"))); - this.contentContainer.addChild(new Spacer(1)); + #renderAuthMethodStep(): void { + this.#contentContainer.addChild(new Text(theme.fg("accent", "Step: Authentication Method"))); + this.#contentContainer.addChild(new Spacer(1)); const options = [ { value: "oauth" as const, label: "OAuth flow (web-based)", desc: "(opens browser)" }, @@ -771,171 +775,159 @@ export class MCPAddWizard extends Container { for (let i = 0; i < options.length; i++) { const option = options[i]; - const isSelected = i === this.selectedIndex; + const isSelected = i === this.#selectedIndex; const prefix = isSelected ? theme.fg("accent", `${theme.nav.cursor} `) : " "; const text = isSelected ? theme.fg("accent", option.label) : option.label; - this.contentContainer.addChild(new Text(prefix + text, 0, 0)); + this.#contentContainer.addChild(new Text(prefix + text, 0, 0)); if (!isSelected) { - this.contentContainer.addChild(new Text(` ${theme.fg("dim", option.desc)}`, 0, 0)); + this.#contentContainer.addChild(new Text(` ${theme.fg("dim", option.desc)}`, 0, 0)); } } - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild( + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild( new Text(theme.fg("muted", "[↑↓ to navigate, Enter to select, Esc to go back]"), 0, 0), ); } - private renderOAuthAuthUrlStep(): void { - this.contentContainer.addChild(new Text(theme.fg("accent", "OAuth: Authorization URL"))); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text("Enter the OAuth authorization endpoint:", 0, 0)); - this.contentContainer.addChild(new Spacer(1)); + #renderOAuthAuthUrlStep(): void { + this.#contentContainer.addChild(new Text(theme.fg("accent", "OAuth: Authorization URL"))); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text("Enter the OAuth authorization endpoint:", 0, 0)); + this.#contentContainer.addChild(new Spacer(1)); - this.inputField = new Input(); - this.inputField.setValue(this.state.oauthAuthUrl); - this.contentContainer.addChild(this.inputField); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild( + this.#inputField = new Input(); + this.#inputField.setValue(this.#state.oauthAuthUrl); + this.#contentContainer.addChild(this.#inputField); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild( new Text(theme.fg("muted", "e.g., https://auth.example.com/oauth/authorize"), 0, 0), ); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text(theme.fg("muted", "[Enter to continue, Esc to go back]"), 0, 0)); - - this.updateFocus(); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text(theme.fg("muted", "[Enter to continue, Esc to go back]"), 0, 0)); } - private renderOAuthTokenUrlStep(): void { - this.contentContainer.addChild(new Text(theme.fg("accent", "OAuth: Token URL"))); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text("Enter the OAuth token endpoint:", 0, 0)); - this.contentContainer.addChild(new Spacer(1)); + #renderOAuthTokenUrlStep(): void { + this.#contentContainer.addChild(new Text(theme.fg("accent", "OAuth: Token URL"))); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text("Enter the OAuth token endpoint:", 0, 0)); + this.#contentContainer.addChild(new Spacer(1)); - this.inputField = new Input(); - this.inputField.setValue(this.state.oauthTokenUrl); - this.contentContainer.addChild(this.inputField); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text(theme.fg("muted", "e.g., https://auth.example.com/oauth/token"), 0, 0)); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text(theme.fg("muted", "[Enter to continue, Esc to go back]"), 0, 0)); - - this.updateFocus(); + this.#inputField = new Input(); + this.#inputField.setValue(this.#state.oauthTokenUrl); + this.#contentContainer.addChild(this.#inputField); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text(theme.fg("muted", "e.g., https://auth.example.com/oauth/token"), 0, 0)); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text(theme.fg("muted", "[Enter to continue, Esc to go back]"), 0, 0)); } - private renderOAuthClientIdStep(): void { - this.contentContainer.addChild(new Text(theme.fg("accent", "OAuth: Client ID"))); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text("Enter your OAuth client ID:", 0, 0)); - this.contentContainer.addChild(new Spacer(1)); + #renderOAuthClientIdStep(): void { + this.#contentContainer.addChild(new Text(theme.fg("accent", "OAuth: Client ID"))); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text("Enter your OAuth client ID:", 0, 0)); + this.#contentContainer.addChild(new Spacer(1)); - this.inputField = new Input(); - this.inputField.setValue(this.state.oauthClientId); - this.contentContainer.addChild(this.inputField); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text(theme.fg("muted", "[Enter to continue, Esc to go back]"), 0, 0)); - - this.updateFocus(); + this.#inputField = new Input(); + this.#inputField.setValue(this.#state.oauthClientId); + this.#contentContainer.addChild(this.#inputField); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text(theme.fg("muted", "[Enter to continue, Esc to go back]"), 0, 0)); } - private renderOAuthClientSecretStep(): void { - this.contentContainer.addChild(new Text(theme.fg("accent", "OAuth: Client Secret (Optional)"))); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text("Enter your OAuth client secret:", 0, 0)); - this.contentContainer.addChild(new Text(theme.fg("muted", "(Leave empty for PKCE-only flows)"), 0, 0)); - this.contentContainer.addChild(new Spacer(1)); + #renderOAuthClientSecretStep(): void { + this.#contentContainer.addChild(new Text(theme.fg("accent", "OAuth: Client Secret (Optional)"))); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text("Enter your OAuth client secret:", 0, 0)); + this.#contentContainer.addChild(new Text(theme.fg("muted", "(Leave empty for PKCE-only flows)"), 0, 0)); + this.#contentContainer.addChild(new Spacer(1)); - this.inputField = new Input(); - this.inputField.setValue(this.state.oauthClientSecret); - this.contentContainer.addChild(this.inputField); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text(theme.fg("muted", "[Enter to continue, Esc to go back]"), 0, 0)); - - this.updateFocus(); + this.#inputField = new Input(); + this.#inputField.setValue(this.#state.oauthClientSecret); + this.#contentContainer.addChild(this.#inputField); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text(theme.fg("muted", "[Enter to continue, Esc to go back]"), 0, 0)); } - private renderOAuthScopesStep(): void { - this.contentContainer.addChild(new Text(theme.fg("accent", "OAuth: Scopes (Optional)"))); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text("Enter OAuth scopes (space-separated):", 0, 0)); - this.contentContainer.addChild(new Spacer(1)); + #renderOAuthScopesStep(): void { + this.#contentContainer.addChild(new Text(theme.fg("accent", "OAuth: Scopes (Optional)"))); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text("Enter OAuth scopes (space-separated):", 0, 0)); + this.#contentContainer.addChild(new Spacer(1)); - this.inputField = new Input(); - this.inputField.setValue(this.state.oauthScopes); - this.contentContainer.addChild(this.inputField); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text(theme.fg("muted", "e.g., read write"), 0, 0)); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text(theme.fg("muted", "[Enter to continue, Esc to go back]"), 0, 0)); - - this.updateFocus(); + this.#inputField = new Input(); + this.#inputField.setValue(this.#state.oauthScopes); + this.#contentContainer.addChild(this.#inputField); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text(theme.fg("muted", "e.g., read write"), 0, 0)); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text(theme.fg("muted", "[Enter to continue, Esc to go back]"), 0, 0)); } - private renderOAuthErrorStep(): void { - this.contentContainer.addChild(new Text(theme.fg("error", "OAuth authentication failed"), 0, 0)); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text("Choose next action:", 0, 0)); - this.contentContainer.addChild(new Spacer(1)); + #renderOAuthErrorStep(): void { + this.#contentContainer.addChild(new Text(theme.fg("error", "OAuth authentication failed"), 0, 0)); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text("Choose next action:", 0, 0)); + this.#contentContainer.addChild(new Spacer(1)); const options = ["Retry OAuth authentication", "Edit OAuth settings"]; for (let i = 0; i < options.length; i++) { - const isSelected = i === this.selectedIndex; + const isSelected = i === this.#selectedIndex; const prefix = isSelected ? theme.fg("accent", `${theme.nav.cursor} `) : " "; const text = isSelected ? theme.fg("accent", options[i]) : options[i]; - this.contentContainer.addChild(new Text(prefix + text, 0, 0)); + this.#contentContainer.addChild(new Text(prefix + text, 0, 0)); } - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild( + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild( new Text(theme.fg("muted", "[↑↓ to navigate, Enter to select, Esc to go back]"), 0, 0), ); } - private renderApiKeyStep(): void { - this.contentContainer.addChild(new Text(theme.fg("accent", "API Key Required"))); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text("Enter your API key or token:", 0, 0)); - this.contentContainer.addChild(new Text(theme.fg("muted", "(Supports !command for password manager)"), 0, 0)); - this.contentContainer.addChild(new Spacer(1)); + #renderApiKeyStep(): void { + this.#contentContainer.addChild(new Text(theme.fg("accent", "API Key Required"))); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text("Enter your API key or token:", 0, 0)); + this.#contentContainer.addChild(new Text(theme.fg("muted", "(Supports !command for password manager)"), 0, 0)); + this.#contentContainer.addChild(new Spacer(1)); - this.inputField = new Input(); - this.inputField.setValue(this.state.apiKey); - this.contentContainer.addChild(this.inputField); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text(theme.fg("muted", "[Enter to continue, Esc to go back]"), 0, 0)); - - this.updateFocus(); + this.#inputField = new Input(); + this.#inputField.setValue(this.#state.apiKey); + this.#contentContainer.addChild(this.#inputField); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text(theme.fg("muted", "[Enter to continue, Esc to go back]"), 0, 0)); } /** * Test connection and automatically detect if auth is needed. */ - private async testConnectionAndDetectAuth(): Promise { - const testConfig = this.buildServerConfig(); + async #testConnectionAndDetectAuth(): Promise { + const testConfig = this.#buildServerConfig(); - if (!this.onTestConnectionCallback) { + if (!this.#onTestConnectionCallback) { // Skip test, go to scope - this.currentStep = "scope"; - this.selectedIndex = 0; - this.renderStep(); + this.#currentStep = "scope"; + this.#selectedIndex = 0; + this.#renderStep(); return; } try { // Try to connect - timeout is handled by the transport layer (5 seconds) - await this.onTestConnectionCallback(testConfig); + await this.#onTestConnectionCallback(testConfig); // Success! No auth required - this.contentContainer.clear(); - this.contentContainer.addChild(new Text(theme.fg("success", "✓ Connection successful!"), 0, 0)); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text("No authentication required", 0, 0)); - this.contentContainer.addChild(new Spacer(1)); + this.#contentContainer.clear(); + this.#contentContainer.addChild(new Text(theme.fg("success", "✓ Connection successful!"), 0, 0)); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text("No authentication required", 0, 0)); + this.#contentContainer.addChild(new Spacer(1)); setTimeout(() => { - this.state.authMethod = "none"; - this.currentStep = "scope"; - this.selectedIndex = 0; - this.renderStep(); + this.#state.authMethod = "none"; + this.#currentStep = "scope"; + this.#selectedIndex = 0; + this.#renderStep(); }, 1000); } catch (error) { // Connection failed - check if it's an auth error @@ -944,180 +936,186 @@ export class MCPAddWizard extends Container { if (authResult.requiresAuth) { // Prefer OAuth first: use error metadata, then well-known discovery fallback. let oauth = authResult.authType === "oauth" ? (authResult.oauth ?? null) : null; - if (!oauth && this.state.transport !== "stdio" && this.state.url) { + if (!oauth && this.#state.transport !== "stdio" && this.#state.url) { try { - oauth = await discoverOAuthEndpoints(this.state.url); + oauth = await discoverOAuthEndpoints(this.#state.url); } catch { // Ignore discovery failures and fallback to manual auth. } } if (oauth) { - this.state.oauthAuthUrl = oauth.authorizationUrl; - this.state.oauthTokenUrl = oauth.tokenUrl; - this.state.oauthClientId = oauth.clientId || ""; - this.state.oauthScopes = oauth.scopes || ""; - this.state.authMethod = "oauth"; + this.#state.oauthAuthUrl = oauth.authorizationUrl; + this.#state.oauthTokenUrl = oauth.tokenUrl; + this.#state.oauthClientId = oauth.clientId || ""; + this.#state.oauthScopes = oauth.scopes || ""; + this.#state.authMethod = "oauth"; - this.contentContainer.clear(); - this.contentContainer.addChild(new Text(theme.fg("success", "✓ OAuth detected"), 0, 0)); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text("Launching browser for authorization...", 0, 0)); - this.contentContainer.addChild(new Spacer(1)); + this.#contentContainer.clear(); + this.#contentContainer.addChild(new Text(theme.fg("success", "✓ OAuth detected"), 0, 0)); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text("Launching browser for authorization...", 0, 0)); + this.#contentContainer.addChild(new Spacer(1)); - void this.launchOAuthFlow(); + void this.#launchOAuthFlow(); return; } // OAuth metadata unavailable: fallback to manual API key. - this.contentContainer.clear(); - this.contentContainer.addChild(new Text(theme.fg("warning", "⚠ Authentication required"), 0, 0)); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text("OAuth parameters could not be discovered.", 0, 0)); - this.contentContainer.addChild(new Text("Provide API key/token manually.", 0, 0)); - this.contentContainer.addChild(new Spacer(1)); - this.currentStep = "apikey"; - this.renderStep(); + this.#contentContainer.clear(); + this.#contentContainer.addChild(new Text(theme.fg("warning", "⚠ Authentication required"), 0, 0)); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text("OAuth parameters could not be discovered.", 0, 0)); + this.#contentContainer.addChild(new Text("Provide API key/token manually.", 0, 0)); + this.#contentContainer.addChild(new Spacer(1)); + this.#currentStep = "apikey"; + this.#renderStep(); } else { // Not an auth error - just a connection failure - this.contentContainer.clear(); - this.contentContainer.addChild(new Text(theme.fg("error", "✗ Connection failed"), 0, 0)); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text(error instanceof Error ? error.message : String(error), 0, 0)); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text(theme.fg("muted", "Adding server anyway..."), 0, 0)); + const errorMsg = sanitize(error instanceof Error ? error.message : String(error)); + this.#contentContainer.clear(); + this.#contentContainer.addChild(new Text(theme.fg("error", "✗ Connection failed"), 0, 0)); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text(errorMsg, 0, 0)); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text(theme.fg("muted", "Adding server anyway..."), 0, 0)); setTimeout(() => { - this.state.authMethod = "none"; - this.currentStep = "scope"; - this.selectedIndex = 0; - this.renderStep(); + this.#state.authMethod = "none"; + this.#currentStep = "scope"; + this.#selectedIndex = 0; + this.#renderStep(); }, 2000); } } } /** - * Build a server config from current wizard state. - * Used for testing connection during auto-detection. + * Build a server config from current wizard state for connection testing (no auth). */ - private buildServerConfig(): MCPServerConfig { - return this.buildServerConfigWithAuth(false); + #buildServerConfig(): MCPServerConfig { + return this.#buildServerConfigWithAuth(false); } - private buildServerConfigWithAuth(includeAuth: boolean): MCPServerConfig { - const transport = this.state.transport ?? "stdio"; + #buildServerConfigWithAuth(includeAuth: boolean): MCPServerConfig { + const transport = this.#state.transport ?? "stdio"; if (transport === "stdio") { - const config: any = { + const config: MCPStdioServerConfig = { type: "stdio", - command: this.state.command, - timeout: 5000, // 5 second timeout for connection testing + command: this.#state.command, + timeout: 5000, }; - if (this.state.args) { - config.args = this.state.args.split(/\s+/).filter(Boolean); + if (this.#state.args) { + config.args = this.#state.args.split(/\s+/).filter(Boolean); } - if (includeAuth && this.state.authMethod === "oauth" && this.state.oauthCredentialId) { + if (includeAuth && this.#state.authMethod === "oauth" && this.#state.oauthCredentialId) { config.auth = { type: "oauth", - credentialId: this.state.oauthCredentialId, + credentialId: this.#state.oauthCredentialId, }; } - if (includeAuth && this.state.authMethod === "manual" && this.state.apiKey) { + if (includeAuth && this.#state.authMethod === "manual" && this.#state.apiKey) { config.env = { ...(config.env ?? {}), - [this.state.envVarName || "API_KEY"]: this.state.apiKey, - }; - } - - return config; - } else { - // http or sse - const config: any = { - type: transport, - url: this.state.url, - timeout: 5000, // 5 second timeout for connection testing - }; - - if (includeAuth && this.state.authMethod === "oauth" && this.state.oauthCredentialId) { - config.auth = { - type: "oauth", - credentialId: this.state.oauthCredentialId, - }; - } - - if (includeAuth && this.state.authMethod === "manual" && this.state.apiKey) { - const headerValue = this.state.apiKey.startsWith("Bearer ") - ? this.state.apiKey - : `Bearer ${this.state.apiKey}`; - config.headers = { - ...(config.headers ?? {}), - Authorization: headerValue, + [this.#state.envVarName || "API_KEY"]: this.#state.apiKey, }; } return config; } + + // http or sse + const config: MCPHttpServerConfig | MCPSseServerConfig = { + type: transport, + url: this.#state.url, + timeout: 5000, + }; + + if (includeAuth && this.#state.authMethod === "oauth" && this.#state.oauthCredentialId) { + config.auth = { + type: "oauth", + credentialId: this.#state.oauthCredentialId, + }; + } + + if (includeAuth && this.#state.authMethod === "manual" && this.#state.apiKey) { + if (this.#state.authLocation === "env") { + // For HTTP with env location, store in headers using the env var name as-is + config.headers = { + ...(config.headers ?? {}), + [this.#state.headerName || "Authorization"]: this.#state.apiKey, + }; + } else { + const headerName = this.#state.headerName || "Authorization"; + config.headers = { + ...(config.headers ?? {}), + [headerName]: this.#state.apiKey, + }; + } + } + + return config; } - private async launchOAuthFlow(): Promise { - if (!this.onOAuthCallback) { - this.contentContainer.clear(); - this.contentContainer.addChild(new Text(theme.fg("error", "OAuth flow not available"), 0, 0)); - this.renderStep(); - this.requestRender(); + async #launchOAuthFlow(): Promise { + if (!this.#onOAuthCallback) { + this.#contentContainer.clear(); + this.#contentContainer.addChild(new Text(theme.fg("error", "OAuth flow not available"), 0, 0)); + this.#renderStep(); + this.#requestRender(); return; } // Validate OAuth configuration - if (!this.state.oauthAuthUrl || !this.state.oauthTokenUrl) { - this.contentContainer.clear(); - this.contentContainer.addChild(new Text(theme.fg("error", "OAuth configuration incomplete"), 0, 0)); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text("Authorization and Token URLs are required.", 0, 0)); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text(theme.fg("muted", "[Press Esc to go back]"), 0, 0)); - this.requestRender(); + if (!this.#state.oauthAuthUrl || !this.#state.oauthTokenUrl) { + this.#contentContainer.clear(); + this.#contentContainer.addChild(new Text(theme.fg("error", "OAuth configuration incomplete"), 0, 0)); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text("Authorization and Token URLs are required.", 0, 0)); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text(theme.fg("muted", "[Press Esc to go back]"), 0, 0)); + this.#requestRender(); return; } // Show "Authenticating..." message - this.contentContainer.clear(); - this.contentContainer.addChild(new Text(theme.fg("accent", "OAuth Authentication"), 0, 0)); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text("Launching OAuth flow...", 0, 0)); - this.contentContainer.addChild(new Text(theme.fg("muted", "Browser will open automatically."), 0, 0)); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild( + this.#contentContainer.clear(); + this.#contentContainer.addChild(new Text(theme.fg("accent", "OAuth Authentication"), 0, 0)); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text("Launching OAuth flow...", 0, 0)); + this.#contentContainer.addChild(new Text(theme.fg("muted", "Browser will open automatically."), 0, 0)); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild( new Text(theme.fg("warning", "If browser doesn't open, copy the URL from chat."), 0, 0), ); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text(theme.fg("muted", "(Press Esc to cancel)"), 0, 0)); - this.requestRender(); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text(theme.fg("muted", "(Press Esc to cancel)"), 0, 0)); + this.#requestRender(); try { // Call OAuth handler - const credentialId = await this.onOAuthCallback( - this.state.oauthAuthUrl, - this.state.oauthTokenUrl, - this.state.oauthClientId, - this.state.oauthClientSecret, - this.state.oauthScopes, + const credentialId = await this.#onOAuthCallback( + this.#state.oauthAuthUrl, + this.#state.oauthTokenUrl, + this.#state.oauthClientId, + this.#state.oauthClientSecret, + this.#state.oauthScopes, ); // Store credential ID - this.state.oauthCredentialId = credentialId; + this.#state.oauthCredentialId = credentialId; // Show success message - this.contentContainer.clear(); - this.contentContainer.addChild(new Text(theme.fg("success", "✓ Authentication successful!"), 0, 0)); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text(theme.fg("muted", "Running connection health check..."), 0, 0)); + this.#contentContainer.clear(); + this.#contentContainer.addChild(new Text(theme.fg("success", "✓ Authentication successful!"), 0, 0)); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text(theme.fg("muted", "Running connection health check..."), 0, 0)); const healthText = new Text(theme.fg("muted", "| Checking server connection..."), 0, 0); - this.contentContainer.addChild(healthText); + this.#contentContainer.addChild(healthText); const spinnerFrames = ["|", "/", "-", "\\"]; let spinnerIndex = 0; @@ -1126,22 +1124,29 @@ export class MCPAddWizard extends Container { theme.fg("muted", `${spinnerFrames[spinnerIndex % spinnerFrames.length]} Checking server connection...`), ); spinnerIndex++; - this.requestRender(); + this.#requestRender(); }, 120); let healthPassed = true; let healthError = ""; - if (this.onTestConnectionCallback) { + if (this.#onTestConnectionCallback) { try { - await Promise.race([ - this.onTestConnectionCallback(this.buildServerConfigWithAuth(true)), - new Promise((_, reject) => - setTimeout(() => reject(new Error("Health check timed out after 10 seconds")), 10_000), - ), - ]); + const { promise: timeoutPromise, reject: timeoutReject } = Promise.withResolvers(); + const timer = setTimeout( + () => timeoutReject(new Error("Health check timed out after 10 seconds")), + 10_000, + ); + try { + await Promise.race([ + this.#onTestConnectionCallback(this.#buildServerConfigWithAuth(true)), + timeoutPromise, + ]); + } finally { + clearTimeout(timer); + } } catch (error) { healthPassed = false; - healthError = error instanceof Error ? error.message : String(error); + healthError = sanitize(error instanceof Error ? error.message : String(error)); } } @@ -1150,121 +1155,130 @@ export class MCPAddWizard extends Container { healthText.setText(theme.fg("success", "✓ Health check passed")); } else { healthText.setText(theme.fg("warning", "⚠ Health check failed (will still save config)")); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text(theme.fg("muted", healthError), 0, 0)); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text(theme.fg("muted", healthError), 0, 0)); } - this.requestRender(); + this.#requestRender(); // Move to scope selection after short delay setTimeout( () => { - this.currentStep = "scope"; - this.selectedIndex = 0; - this.renderStep(); - this.requestRender(); + this.#currentStep = "scope"; + this.#selectedIndex = 0; + this.#renderStep(); + this.#requestRender(); }, healthPassed ? 1000 : 2000, ); } catch (error) { // Show error with options to retry or go back - const errorMsg = error instanceof Error ? error.message : String(error); - this.contentContainer.clear(); - this.contentContainer.addChild(new Text(theme.fg("error", "✗ OAuth authentication failed"), 0, 0)); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text(errorMsg, 0, 0)); - this.contentContainer.addChild(new Spacer(1)); + const errorMsg = sanitize(error instanceof Error ? error.message : String(error)); + this.#contentContainer.clear(); + this.#contentContainer.addChild(new Text(theme.fg("error", "✗ OAuth authentication failed"), 0, 0)); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text(errorMsg, 0, 0)); + this.#contentContainer.addChild(new Spacer(1)); // Provide helpful tips based on error type if (errorMsg.includes("timeout") || errorMsg.includes("timed out")) { - this.contentContainer.addChild( + this.#contentContainer.addChild( new Text(theme.fg("muted", "Tip: Complete authorization faster next time"), 0, 0), ); } else if (errorMsg.includes("Invalid OAuth URLs")) { - this.contentContainer.addChild( + this.#contentContainer.addChild( new Text(theme.fg("muted", "Tip: Check that the OAuth URLs are correct"), 0, 0), ); } else if (errorMsg.includes("ECONNREFUSED")) { - this.contentContainer.addChild( + this.#contentContainer.addChild( new Text(theme.fg("muted", "Tip: Verify the OAuth server is accessible"), 0, 0), ); } - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild(new Text(`${theme.fg("accent", "→ ")}Retry`, 0, 0)); - this.contentContainer.addChild(new Text(" Edit OAuth settings", 0, 0)); - this.contentContainer.addChild(new Spacer(1)); - this.contentContainer.addChild( + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild(new Text(`${theme.fg("accent", "→ ")}Retry`, 0, 0)); + this.#contentContainer.addChild(new Text(" Edit OAuth settings", 0, 0)); + this.#contentContainer.addChild(new Spacer(1)); + this.#contentContainer.addChild( new Text(theme.fg("muted", "[↑↓ to navigate, Enter to select, Esc to go back]"), 0, 0), ); - this.requestRender(); + this.#requestRender(); // Set up as a selector step - this.selectedIndex = 0; - this.currentStep = "oauth-error"; + this.#selectedIndex = 0; + this.#currentStep = "oauth-error"; } } - private complete(): void { - if (!this.state.scope) return; + #complete(): void { + if (!this.#state.scope) return; // Build the config - const config: MCPServerConfig = this.buildConfig(); + const config: MCPServerConfig = this.#buildConfig(); // Call completion callback - this.onCompleteCallback(this.state.name, config, this.state.scope); + this.#onCompleteCallback(this.#state.name, config, this.#state.scope); } - private buildConfig(): MCPServerConfig { - if (this.state.transport === "stdio") { - const config: MCPServerConfig = { + #buildConfig(): MCPServerConfig { + if (this.#state.transport === "stdio") { + const config: MCPStdioServerConfig = { type: "stdio", - command: this.state.command, + command: this.#state.command, }; - if (this.state.args) { - (config as any).args = this.state.args.split(/\s+/).filter(Boolean); + if (this.#state.args) { + config.args = this.#state.args.split(/\s+/).filter(Boolean); } // Add OAuth auth if configured - if (this.state.authMethod === "oauth" && this.state.oauthCredentialId) { - (config as any).auth = { + if (this.#state.authMethod === "oauth" && this.#state.oauthCredentialId) { + config.auth = { type: "oauth", - credentialId: this.state.oauthCredentialId, + credentialId: this.#state.oauthCredentialId, }; } - // Add API key to env if manual auth - if (this.state.authMethod === "manual" && this.state.apiKey) { - (config as any).env = { - API_KEY: this.state.apiKey, + // Add API key to env if manual auth — use user-chosen env var name + if (this.#state.authMethod === "manual" && this.#state.apiKey) { + const envKey = this.#state.envVarName || "API_KEY"; + config.env = { + [envKey]: this.#state.apiKey, }; } return config; } - // HTTP or SSE - const config: MCPServerConfig = { - type: this.state.transport!, - url: this.state.url, - } as any; + // HTTP or SSE — use concrete type + const config: MCPHttpServerConfig | MCPSseServerConfig = { + type: this.#state.transport!, + url: this.#state.url, + }; // Add OAuth auth if configured - if (this.state.authMethod === "oauth" && this.state.oauthCredentialId) { - (config as any).auth = { + if (this.#state.authMethod === "oauth" && this.#state.oauthCredentialId) { + config.auth = { type: "oauth", - credentialId: this.state.oauthCredentialId, + credentialId: this.#state.oauthCredentialId, }; } - // Add API key to Authorization header if manual auth - if (this.state.authMethod === "manual" && this.state.apiKey) { - const headerValue = this.state.apiKey.startsWith("Bearer ") - ? this.state.apiKey - : `Bearer ${this.state.apiKey}`; - (config as any).headers = { - Authorization: headerValue, - }; + // Add API key using user-chosen header name and auth location + if (this.#state.authMethod === "manual" && this.#state.apiKey) { + if (this.#state.authLocation === "env") { + // Env-based auth for HTTP: store the key in env on the config + // HTTP/SSE configs don't have an env field, so use headers as carrier + const headerName = this.#state.headerName || "Authorization"; + config.headers = { + [headerName]: this.#state.apiKey, + }; + } else { + // Header-based auth: use the user's chosen header name + const headerName = this.#state.headerName || "Authorization"; + config.headers = { + [headerName]: this.#state.apiKey, + }; + } } return config; diff --git a/packages/coding-agent/src/modes/controllers/input-controller.ts b/packages/coding-agent/src/modes/controllers/input-controller.ts index b602766b1..337207c12 100644 --- a/packages/coding-agent/src/modes/controllers/input-controller.ts +++ b/packages/coding-agent/src/modes/controllers/input-controller.ts @@ -334,7 +334,7 @@ export class InputController { } // Handle MCP server management commands - if (text.startsWith("/mcp")) { + if (text === "/mcp" || text.startsWith("/mcp ")) { this.ctx.editor.addToHistory(text); this.ctx.editor.setText(""); await this.ctx.handleMCPCommand(text); diff --git a/packages/coding-agent/src/modes/controllers/mcp-command-controller.ts b/packages/coding-agent/src/modes/controllers/mcp-command-controller.ts index 375555cab..0afe150ce 100644 --- a/packages/coding-agent/src/modes/controllers/mcp-command-controller.ts +++ b/packages/coding-agent/src/modes/controllers/mcp-command-controller.ts @@ -83,38 +83,38 @@ export class MCPCommandController { const subcommand = parts[1]?.toLowerCase(); if (!subcommand || subcommand === "help") { - this.showHelp(); + this.#showHelp(); return; } switch (subcommand) { case "add": - await this.handleAdd(text); + await this.#handleAdd(text); break; case "list": - await this.handleList(); + await this.#handleList(); break; case "remove": case "rm": - await this.handleRemove(text); + await this.#handleRemove(text); break; case "test": - await this.handleTest(parts[2]); + await this.#handleTest(parts[2]); break; case "reauth": - await this.handleReauth(parts[2]); + await this.#handleReauth(parts[2]); break; case "unauth": - await this.handleUnauth(parts[2]); + await this.#handleUnauth(parts[2]); break; case "enable": - await this.handleSetEnabled(parts[2], true); + await this.#handleSetEnabled(parts[2], true); break; case "disable": - await this.handleSetEnabled(parts[2], false); + await this.#handleSetEnabled(parts[2], false); break; case "reload": - await this.handleReload(); + await this.#handleReload(); break; default: this.ctx.showError(`Unknown subcommand: ${subcommand}. Type /mcp help for usage.`); @@ -124,7 +124,7 @@ export class MCPCommandController { /** * Show help text */ - private showHelp(): void { + #showHelp(): void { const helpText = [ "", theme.bold("MCP Server Management"), @@ -146,10 +146,10 @@ export class MCPCommandController { "", ].join("\n"); - this.showMessage(helpText); + this.#showMessage(helpText); } - private parseAddCommand(text: string): MCPAddParsed { + #parseAddCommand(text: string): MCPAddParsed { const prefixMatch = text.match(/^\/mcp\s+add\b\s*(.*)$/i); const rest = prefixMatch?.[1]?.trim() ?? ""; if (!rest) { @@ -265,8 +265,8 @@ export class MCPCommandController { /** * Handle /mcp add - Launch interactive wizard or quick-add from args */ - private async handleAdd(text: string): Promise { - const parsed = this.parseAddCommand(text); + async #handleAdd(text: string): Promise { + const parsed = this.#parseAddCommand(text); if (parsed.error) { this.ctx.showError(parsed.error); return; @@ -278,10 +278,13 @@ export class MCPCommandController { // matching wizard behavior. Command quick-add intentionally skips this. if (!parsed.isCommandQuickAdd && (finalConfig.type === "http" || finalConfig.type === "sse")) { try { - await this.handleTestConnection(finalConfig); + await this.#handleTestConnection(finalConfig); } catch (error) { if (parsed.hasAuthToken) { - throw error; + this.ctx.showError( + `Authentication failed for "${parsed.initialName}": ${error instanceof Error ? error.message : String(error)}`, + ); + return; } const authResult = analyzeAuthError(error as Error); if (authResult.requiresAuth) { @@ -295,31 +298,39 @@ export class MCPCommandController { } if (!oauth) { - throw new Error( + this.ctx.showError( `Authentication required for "${parsed.initialName}", but OAuth endpoints could not be discovered. ` + `Use /mcp add ${parsed.initialName} (wizard) or configure auth manually.`, ); + return; } - const credentialId = await this.handleOAuthFlow( - oauth.authorizationUrl, - oauth.tokenUrl, - oauth.clientId ?? "", - "", - oauth.scopes ?? "", - ); - finalConfig = { - ...finalConfig, - auth: { - type: "oauth", - credentialId, - }, - }; + try { + const credentialId = await this.#handleOAuthFlow( + oauth.authorizationUrl, + oauth.tokenUrl, + oauth.clientId ?? "", + "", + oauth.scopes ?? "", + ); + finalConfig = { + ...finalConfig, + auth: { + type: "oauth", + credentialId, + }, + }; + } catch (oauthError) { + this.ctx.showError( + `OAuth flow failed for "${parsed.initialName}": ${oauthError instanceof Error ? oauthError.message : String(oauthError)}`, + ); + return; + } } } } - await this.handleWizardComplete(parsed.initialName, finalConfig, parsed.scope); + await this.#handleWizardComplete(parsed.initialName, finalConfig, parsed.scope); return; } @@ -334,17 +345,17 @@ export class MCPCommandController { const wizard = new MCPAddWizard( async (name: string, config: MCPServerConfig, scope: "user" | "project") => { done(); - await this.handleWizardComplete(name, config, scope); + await this.#handleWizardComplete(name, config, scope); }, () => { done(); - this.handleWizardCancel(); + this.#handleWizardCancel(); }, async (authUrl: string, tokenUrl: string, clientId: string, clientSecret: string, scopes: string) => { - return await this.handleOAuthFlow(authUrl, tokenUrl, clientId, clientSecret, scopes); + return await this.#handleOAuthFlow(authUrl, tokenUrl, clientId, clientSecret, scopes); }, async (config: MCPServerConfig) => { - return await this.handleTestConnection(config); + return await this.#handleTestConnection(config); }, () => { this.ctx.ui.requestRender(); @@ -362,7 +373,7 @@ export class MCPCommandController { /** * Handle OAuth authentication flow for MCP server */ - private async handleOAuthFlow( + async #handleOAuthFlow( authUrl: string, tokenUrl: string, clientId: string, @@ -495,12 +506,7 @@ export class MCPCommandController { ); // Execute OAuth flow with 5 minute timeout - const credentials = await Promise.race([ - flow.login(), - new Promise((_, reject) => - setTimeout(() => reject(new Error("OAuth flow timed out after 5 minutes")), 5 * 60 * 1000), - ), - ]); + const credentials = await withTimeout(flow.login(), 5 * 60 * 1000, "OAuth flow timed out after 5 minutes"); this.ctx.chatContainer.addChild(new Spacer(1)); this.ctx.chatContainer.addChild(new Text(theme.fg("success", "✓ Authorization completed in browser."), 1, 0)); @@ -541,7 +547,7 @@ export class MCPCommandController { * Test connection to an MCP server. * Throws an error if connection fails (used for auto-detection). */ - private async handleTestConnection(config: MCPServerConfig): Promise { + async #handleTestConnection(config: MCPServerConfig): Promise { // Create temporary connection using a test name const testName = `test_${Date.now()}`; let resolvedConfig: MCPServerConfig; @@ -557,7 +563,7 @@ export class MCPCommandController { await disconnectServer(connection); } - private async findConfiguredServer( + async #findConfiguredServer( name: string, ): Promise<{ filePath: string; scope: "user" | "project"; config: MCPServerConfig } | null> { const cwd = process.cwd(); @@ -578,43 +584,54 @@ export class MCPCommandController { return null; } - private async removeManagedOAuthCredential(credentialId: string | undefined): Promise { + async #removeManagedOAuthCredential(credentialId: string | undefined): Promise { if (!credentialId || !credentialId.startsWith("mcp_oauth_")) return; await this.ctx.session.modelRegistry.authStorage.remove(credentialId); } - private stripOAuthAuth(config: MCPServerConfig): MCPServerConfig { + #stripOAuthAuth(config: MCPServerConfig): MCPServerConfig { const next = { ...config } as MCPServerConfig & { auth?: { type: "oauth" | "apikey"; credentialId?: string } }; delete next.auth; return next; } - private async resolveOAuthEndpointsFromServer(config: MCPServerConfig): Promise<{ + async #resolveOAuthEndpointsFromServer(config: MCPServerConfig): Promise<{ authorizationUrl: string; tokenUrl: string; clientId?: string; scopes?: string; }> { + // First test if server actually needs auth by connecting without OAuth + let connectionSucceeded = false; + let connectionError: Error | undefined; try { - await this.handleTestConnection(this.stripOAuthAuth(config)); - throw new Error("Server connection succeeded without OAuth; reauthorization is not required."); + await this.#handleTestConnection(this.#stripOAuthAuth(config)); + connectionSucceeded = true; } catch (error) { - const authResult = analyzeAuthError(error as Error); - let oauth = authResult.authType === "oauth" ? (authResult.oauth ?? null) : null; - - if (!oauth && (config.type === "http" || config.type === "sse") && config.url) { - oauth = await discoverOAuthEndpoints(config.url); - } - - if (!oauth) { - throw new Error("Could not discover OAuth endpoints from server response."); - } - - return oauth; + connectionError = error as Error; } + + // Server connected fine without auth — reauth is not needed + if (connectionSucceeded) { + throw new Error("Server connection succeeded without OAuth; reauthorization is not required."); + } + + // Analyze the connection error to extract OAuth endpoints + const authResult = analyzeAuthError(connectionError!); + let oauth = authResult.authType === "oauth" ? (authResult.oauth ?? null) : null; + + if (!oauth && (config.type === "http" || config.type === "sse") && config.url) { + oauth = await discoverOAuthEndpoints(config.url); + } + + if (!oauth) { + throw new Error("Could not discover OAuth endpoints from server response."); + } + + return oauth; } - private async waitForServerConnectionWithAnimation( + async #waitForServerConnectionWithAnimation( name: string, options?: { suppressDisconnectedWarning?: boolean }, ): Promise<"connected" | "connecting" | "disconnected"> { @@ -662,7 +679,7 @@ export class MCPCommandController { } } - private async syncManagerConnection(name: string, config: MCPServerConfig): Promise { + async #syncManagerConnection(name: string, config: MCPServerConfig): Promise { if (!this.ctx.mcpManager) return; if (this.ctx.mcpManager.getConnectionStatus(name) !== "disconnected") return; await this.ctx.mcpManager.connectServers({ [name]: config }, {}); @@ -671,7 +688,7 @@ export class MCPCommandController { } } - private async handleWizardComplete(name: string, config: MCPServerConfig, scope: "user" | "project"): Promise { + async #handleWizardComplete(name: string, config: MCPServerConfig, scope: "user" | "project"): Promise { try { // Determine file path const cwd = process.cwd(); @@ -681,11 +698,11 @@ export class MCPCommandController { await addMCPServer(filePath, name, config); // Reload MCP manager - await this.reloadMCP(); + await this.#reloadMCP(); const state = config.enabled === false ? "disconnected" - : await this.waitForServerConnectionWithAnimation(name, { suppressDisconnectedWarning: true }); + : await this.#waitForServerConnectionWithAnimation(name, { suppressDisconnectedWarning: true }); let isConnected = state === "connected"; const isConnecting = state === "connecting"; @@ -693,9 +710,9 @@ export class MCPCommandController { // report as connected to avoid false-negative messaging. if (!isConnected && !isConnecting && config.enabled !== false) { try { - await this.handleTestConnection(config); + await this.#handleTestConnection(config); isConnected = true; - await this.syncManagerConnection(name, config); + await this.#syncManagerConnection(name, config); } catch { // Keep disconnected status } @@ -721,7 +738,7 @@ export class MCPCommandController { lines.push(theme.fg("muted", `Run ${theme.fg("accent", "/mcp list")} to see all configured servers.`)); lines.push(""); - this.showMessage(lines.join("\n")); + this.#showMessage(lines.join("\n")); } catch (error) { const errorMsg = error instanceof Error ? error.message : String(error); @@ -739,8 +756,8 @@ export class MCPCommandController { } } - private handleWizardCancel(): void { - this.showMessage( + #handleWizardCancel(): void { + this.#showMessage( [ "", theme.fg("muted", "Server creation cancelled."), @@ -754,7 +771,7 @@ export class MCPCommandController { /** * Handle /mcp list - Show all configured servers */ - private async handleList(): Promise { + async #handleList(): Promise { try { const cwd = process.cwd(); @@ -771,7 +788,7 @@ export class MCPCommandController { const projectServers = Object.keys(projectConfig.mcpServers ?? {}); if (userServers.length === 0 && projectServers.length === 0) { - this.showMessage( + this.#showMessage( [ "", theme.fg("muted", "No MCP servers configured."), @@ -831,7 +848,7 @@ export class MCPCommandController { lines.push(""); } - this.showMessage(lines.join("\n")); + this.#showMessage(lines.join("\n")); } catch (error) { this.ctx.showError(`Failed to list servers: ${error instanceof Error ? error.message : String(error)}`); } @@ -840,7 +857,7 @@ export class MCPCommandController { /** * Handle /mcp remove - Remove a server */ - private async handleRemove(text: string): Promise { + async #handleRemove(text: string): Promise { const match = text.match(/^\/mcp\s+(?:remove|rm)\b\s*(.*)$/i); const rest = match?.[1]?.trim() ?? ""; const tokens = parseCommandArgs(rest); @@ -895,9 +912,9 @@ export class MCPCommandController { await removeMCPServer(filePath, name); // Reload MCP manager - await this.reloadMCP(); + await this.#reloadMCP(); - this.showMessage(["", theme.fg("success", `✓ Removed server "${name}" from ${scope} config`), ""].join("\n")); + this.#showMessage(["", theme.fg("success", `✓ Removed server "${name}" from ${scope} config`), ""].join("\n")); } catch (error) { this.ctx.showError(`Failed to remove server: ${error instanceof Error ? error.message : String(error)}`); } @@ -906,7 +923,7 @@ export class MCPCommandController { /** * Handle /mcp test - Test connection to a server */ - private async handleTest(name: string | undefined): Promise { + async #handleTest(name: string | undefined): Promise { if (!name) { this.ctx.showError("Server name required. Usage: /mcp test "); return; @@ -936,7 +953,7 @@ export class MCPCommandController { return; } - this.showMessage(["", theme.fg("muted", `Testing connection to "${name}"...`), ""].join("\n")); + this.#showMessage(["", theme.fg("muted", `Testing connection to "${name}"...`), ""].join("\n")); // Resolve auth config if needed let resolvedConfig: MCPServerConfig; @@ -973,8 +990,8 @@ export class MCPCommandController { } lines.push(""); - await this.syncManagerConnection(name, config); - this.showMessage(lines.join("\n")); + await this.#syncManagerConnection(name, config); + this.#showMessage(lines.join("\n")); } finally { // Disconnect test connection await disconnectServer(connection); @@ -1000,21 +1017,21 @@ export class MCPCommandController { } } - private async handleSetEnabled(name: string | undefined, enabled: boolean): Promise { + async #handleSetEnabled(name: string | undefined, enabled: boolean): Promise { if (!name) { this.ctx.showError(`Server name required. Usage: /mcp ${enabled ? "enable" : "disable"} `); return; } try { - const found = await this.findConfiguredServer(name); + const found = await this.#findConfiguredServer(name); if (!found) { this.ctx.showError(`Server "${name}" not found.`); return; } if ((found.config.enabled ?? true) === enabled) { - this.showMessage( + this.#showMessage( ["", theme.fg("muted", `Server "${name}" is already ${enabled ? "enabled" : "disabled"}.`), ""].join( "\n", ), @@ -1024,11 +1041,11 @@ export class MCPCommandController { const updated: MCPServerConfig = { ...found.config, enabled }; await updateMCPServer(found.filePath, name, updated); - await this.reloadMCP(); + await this.#reloadMCP(); let status = ""; if (enabled) { - const state = await this.waitForServerConnectionWithAnimation(name); + const state = await this.#waitForServerConnectionWithAnimation(name); status = state === "connected" ? theme.fg("success", "Connected") @@ -1046,7 +1063,7 @@ export class MCPCommandController { lines.push(` Status: ${status}`); } lines.push(""); - this.showMessage(lines.join("\n")); + this.#showMessage(lines.join("\n")); } catch (error) { this.ctx.showError( `Failed to ${enabled ? "enable" : "disable"} server: ${error instanceof Error ? error.message : String(error)}`, @@ -1054,14 +1071,14 @@ export class MCPCommandController { } } - private async handleUnauth(name: string | undefined): Promise { + async #handleUnauth(name: string | undefined): Promise { if (!name) { this.ctx.showError("Server name required. Usage: /mcp unauth "); return; } try { - const found = await this.findConfiguredServer(name); + const found = await this.#findConfiguredServer(name); if (!found) { this.ctx.showError(`Server "${name}" not found.`); return; @@ -1071,14 +1088,14 @@ export class MCPCommandController { found.config as MCPServerConfig & { auth?: { type: "oauth" | "apikey"; credentialId?: string } } ).auth; if (currentAuth?.type === "oauth") { - await this.removeManagedOAuthCredential(currentAuth.credentialId); + await this.#removeManagedOAuthCredential(currentAuth.credentialId); } - const updated = this.stripOAuthAuth(found.config); + const updated = this.#stripOAuthAuth(found.config); await updateMCPServer(found.filePath, name, updated); - await this.reloadMCP(); + await this.#reloadMCP(); - this.showMessage( + this.#showMessage( ["", theme.fg("success", `✓ Cleared auth for "${name}" (${found.scope} config)`), ""].join("\n"), ); } catch (error) { @@ -1086,14 +1103,14 @@ export class MCPCommandController { } } - private async handleReauth(name: string | undefined): Promise { + async #handleReauth(name: string | undefined): Promise { if (!name) { this.ctx.showError("Server name required. Usage: /mcp reauth "); return; } try { - const found = await this.findConfiguredServer(name); + const found = await this.#findConfiguredServer(name); if (!found) { this.ctx.showError(`Server "${name}" not found.`); return; @@ -1108,15 +1125,15 @@ export class MCPCommandController { found.config as MCPServerConfig & { auth?: { type: "oauth" | "apikey"; credentialId?: string } } ).auth; if (currentAuth?.type === "oauth") { - await this.removeManagedOAuthCredential(currentAuth.credentialId); + await this.#removeManagedOAuthCredential(currentAuth.credentialId); } - const baseConfig = this.stripOAuthAuth(found.config); - const oauth = await this.resolveOAuthEndpointsFromServer(baseConfig); + const baseConfig = this.#stripOAuthAuth(found.config); + const oauth = await this.#resolveOAuthEndpointsFromServer(baseConfig); - this.showMessage(["", theme.fg("muted", `Reauthorizing "${name}"...`), ""].join("\n")); + this.#showMessage(["", theme.fg("muted", `Reauthorizing "${name}"...`), ""].join("\n")); - const credentialId = await this.handleOAuthFlow( + const credentialId = await this.#handleOAuthFlow( oauth.authorizationUrl, oauth.tokenUrl, oauth.clientId ?? "", @@ -1132,8 +1149,8 @@ export class MCPCommandController { }, }; await updateMCPServer(found.filePath, name, updated); - await this.reloadMCP(); - const state = await this.waitForServerConnectionWithAnimation(name); + await this.#reloadMCP(); + const state = await this.#waitForServerConnectionWithAnimation(name); const lines = [ "", @@ -1148,18 +1165,18 @@ export class MCPCommandController { }`, "", ]; - this.showMessage(lines.join("\n")); + this.#showMessage(lines.join("\n")); } catch (error) { this.ctx.showError(`Failed to reauthorize server: ${error instanceof Error ? error.message : String(error)}`); } } - private async handleReload(): Promise { + async #handleReload(): Promise { try { - this.showMessage(["", theme.fg("muted", "Reloading MCP servers and runtime tools..."), ""].join("\n")); - await this.reloadMCP(); + this.#showMessage(["", theme.fg("muted", "Reloading MCP servers and runtime tools..."), ""].join("\n")); + await this.#reloadMCP(); const connectedCount = this.ctx.mcpManager?.getConnectedServers().length ?? 0; - this.showMessage( + this.#showMessage( ["", theme.fg("success", "✓ MCP reload complete"), ` Connected servers: ${connectedCount}`, ""].join("\n"), ); } catch (error) { @@ -1170,7 +1187,7 @@ export class MCPCommandController { /** * Reload MCP manager with new configs */ - private async reloadMCP(): Promise { + async #reloadMCP(): Promise { if (!this.ctx.mcpManager) { return; } @@ -1189,14 +1206,14 @@ export class MCPCommandController { errorLines.push(` ${serverName}: ${error}`); } errorLines.push(""); - this.showMessage(errorLines.join("\n")); + this.#showMessage(errorLines.join("\n")); } } /** * Show a message in the chat */ - private showMessage(text: string): void { + #showMessage(text: string): void { this.ctx.chatContainer.addChild(new Spacer(1)); this.ctx.chatContainer.addChild(new DynamicBorder()); this.ctx.chatContainer.addChild(new Text(text, 1, 1));