diff --git a/packages/opencode/src/cli/cmd/mcp.ts b/packages/opencode/src/cli/cmd/mcp.ts index cdd741fbc75..95719215e32 100644 --- a/packages/opencode/src/cli/cmd/mcp.ts +++ b/packages/opencode/src/cli/cmd/mcp.ts @@ -13,6 +13,7 @@ import { Installation } from "../../installation" import path from "path" import { Global } from "../../global" import { modify, applyEdits } from "jsonc-parser" +import { Bus } from "../../bus" function getAuthStatusIcon(status: MCP.AuthStatus): string { switch (status) { @@ -227,6 +228,16 @@ export const McpAuthCommand = cmd({ const spinner = prompts.spinner() spinner.start("Starting OAuth flow...") + // Subscribe to browser open failure events to show URL for manual opening + const unsubscribe = Bus.subscribe(MCP.BrowserOpenFailed, (evt) => { + if (evt.properties.mcpName === serverName) { + spinner.stop("Could not open browser automatically") + prompts.log.warn("Please open this URL in your browser to authenticate:") + prompts.log.info(evt.properties.url) + spinner.start("Waiting for authorization...") + } + }) + try { const status = await MCP.authenticate(serverName) @@ -256,6 +267,8 @@ export const McpAuthCommand = cmd({ } catch (error) { spinner.stop("Authentication failed", 1) prompts.log.error(error instanceof Error ? error.message : String(error)) + } finally { + unsubscribe() } prompts.outro("Done") diff --git a/packages/opencode/src/mcp/index.ts b/packages/opencode/src/mcp/index.ts index b0790d93e27..66843aedc11 100644 --- a/packages/opencode/src/mcp/index.ts +++ b/packages/opencode/src/mcp/index.ts @@ -46,6 +46,14 @@ export namespace MCP { }), ) + export const BrowserOpenFailed = BusEvent.define( + "mcp.browser.open.failed", + z.object({ + mcpName: z.string(), + url: z.string(), + }), + ) + export const Failed = NamedError.create( "MCPFailed", z.object({ @@ -787,7 +795,32 @@ export namespace MCP { // The SDK has already added the state parameter to the authorization URL // We just need to open the browser log.info("opening browser for oauth", { mcpName, url: authorizationUrl, state: oauthState }) - await open(authorizationUrl) + try { + const subprocess = await open(authorizationUrl) + // The open package spawns a detached process and returns immediately. + // We need to listen for errors which fire asynchronously: + // - "error" event: command not found (ENOENT) + // - "exit" with non-zero code: command exists but failed (e.g., no display) + await new Promise((resolve, reject) => { + // Give the process a moment to fail if it's going to + const timeout = setTimeout(() => resolve(), 500) + subprocess.on("error", (error) => { + clearTimeout(timeout) + reject(error) + }) + subprocess.on("exit", (code) => { + if (code !== null && code !== 0) { + clearTimeout(timeout) + reject(new Error(`Browser open failed with exit code ${code}`)) + } + }) + }) + } catch (error) { + // Browser opening failed (e.g., in remote/headless sessions like SSH, devcontainers) + // Emit event so CLI can display the URL for manual opening + log.warn("failed to open browser, user must open URL manually", { mcpName, error }) + Bus.publish(BrowserOpenFailed, { mcpName, url: authorizationUrl }) + } // Wait for callback using the OAuth state parameter const code = await McpOAuthCallback.waitForCallback(oauthState) diff --git a/packages/opencode/test/mcp/oauth-browser.test.ts b/packages/opencode/test/mcp/oauth-browser.test.ts new file mode 100644 index 00000000000..598a0315ef8 --- /dev/null +++ b/packages/opencode/test/mcp/oauth-browser.test.ts @@ -0,0 +1,261 @@ +import { test, expect, mock, beforeEach } from "bun:test" +import { EventEmitter } from "events" + +// Track open() calls and control failure behavior +let openShouldFail = false +let openCalledWith: string | undefined + +mock.module("open", () => ({ + default: async (url: string) => { + openCalledWith = url + // Return a mock subprocess that emits an error if openShouldFail is true + const subprocess = new EventEmitter() + if (openShouldFail) { + // Emit error asynchronously like a real subprocess would + setTimeout(() => { + subprocess.emit("error", new Error("spawn xdg-open ENOENT")) + }, 10) + } + return subprocess + }, +})) + +// Mock UnauthorizedError +class MockUnauthorizedError extends Error { + constructor() { + super("Unauthorized") + this.name = "UnauthorizedError" + } +} + +// Track what options were passed to each transport constructor +const transportCalls: Array<{ + type: "streamable" | "sse" + url: string + options: { authProvider?: unknown } +}> = [] + +// Mock the transport constructors +mock.module("@modelcontextprotocol/sdk/client/streamableHttp.js", () => ({ + StreamableHTTPClientTransport: class MockStreamableHTTP { + url: string + authProvider: { redirectToAuthorization?: (url: URL) => Promise } | undefined + constructor(url: URL, options?: { authProvider?: { redirectToAuthorization?: (url: URL) => Promise } }) { + this.url = url.toString() + this.authProvider = options?.authProvider + transportCalls.push({ + type: "streamable", + url: url.toString(), + options: options ?? {}, + }) + } + async start() { + // Simulate OAuth redirect by calling the authProvider's redirectToAuthorization + if (this.authProvider?.redirectToAuthorization) { + await this.authProvider.redirectToAuthorization(new URL("https://auth.example.com/authorize?client_id=test")) + } + throw new MockUnauthorizedError() + } + async finishAuth(_code: string) { + // Mock successful auth completion + } + }, +})) + +mock.module("@modelcontextprotocol/sdk/client/sse.js", () => ({ + SSEClientTransport: class MockSSE { + constructor(url: URL) { + transportCalls.push({ + type: "sse", + url: url.toString(), + options: {}, + }) + } + async start() { + throw new Error("Mock SSE transport cannot connect") + } + }, +})) + +// Mock the MCP SDK Client to trigger OAuth flow +mock.module("@modelcontextprotocol/sdk/client/index.js", () => ({ + Client: class MockClient { + async connect(transport: { start: () => Promise }) { + await transport.start() + } + }, +})) + +// Mock UnauthorizedError in the auth module +mock.module("@modelcontextprotocol/sdk/client/auth.js", () => ({ + UnauthorizedError: MockUnauthorizedError, +})) + +beforeEach(() => { + openShouldFail = false + openCalledWith = undefined + transportCalls.length = 0 +}) + +// Import modules after mocking +const { MCP } = await import("../../src/mcp/index") +const { Bus } = await import("../../src/bus") +const { McpOAuthCallback } = await import("../../src/mcp/oauth-callback") +const { Instance } = await import("../../src/project/instance") +const { tmpdir } = await import("../fixture/fixture") + +test("BrowserOpenFailed event is published when open() throws", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + await Bun.write( + `${dir}/opencode.json`, + JSON.stringify({ + $schema: "https://opencode.ai/config.json", + mcp: { + "test-oauth-server": { + type: "remote", + url: "https://example.com/mcp", + }, + }, + }), + ) + }, + }) + + await Instance.provide({ + directory: tmp.path, + fn: async () => { + openShouldFail = true + + const events: Array<{ mcpName: string; url: string }> = [] + const unsubscribe = Bus.subscribe(MCP.BrowserOpenFailed, (evt) => { + events.push(evt.properties) + }) + + // Run authenticate with a timeout to avoid waiting forever for the callback + const authPromise = MCP.authenticate("test-oauth-server") + + // Wait for the browser open attempt (error fires at 10ms, but we wait for event to be published) + await new Promise((resolve) => setTimeout(resolve, 200)) + + // Stop the callback server and cancel any pending auth + await McpOAuthCallback.stop() + + // Wait for authenticate to reject (due to server stopping) + try { + await authPromise + } catch { + // Expected to fail + } + + unsubscribe() + + // Verify the BrowserOpenFailed event was published + expect(events.length).toBe(1) + expect(events[0].mcpName).toBe("test-oauth-server") + expect(events[0].url).toContain("https://") + }, + }) +}) + +test("BrowserOpenFailed event is NOT published when open() succeeds", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + await Bun.write( + `${dir}/opencode.json`, + JSON.stringify({ + $schema: "https://opencode.ai/config.json", + mcp: { + "test-oauth-server-2": { + type: "remote", + url: "https://example.com/mcp", + }, + }, + }), + ) + }, + }) + + await Instance.provide({ + directory: tmp.path, + fn: async () => { + openShouldFail = false + + const events: Array<{ mcpName: string; url: string }> = [] + const unsubscribe = Bus.subscribe(MCP.BrowserOpenFailed, (evt) => { + events.push(evt.properties) + }) + + // Run authenticate with a timeout to avoid waiting forever for the callback + const authPromise = MCP.authenticate("test-oauth-server-2") + + // Wait for the browser open attempt and the 500ms error detection timeout + await new Promise((resolve) => setTimeout(resolve, 700)) + + // Stop the callback server and cancel any pending auth + await McpOAuthCallback.stop() + + // Wait for authenticate to reject (due to server stopping) + try { + await authPromise + } catch { + // Expected to fail + } + + unsubscribe() + + // Verify NO BrowserOpenFailed event was published + expect(events.length).toBe(0) + // Verify open() was still called + expect(openCalledWith).toBeDefined() + }, + }) +}) + +test("open() is called with the authorization URL", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + await Bun.write( + `${dir}/opencode.json`, + JSON.stringify({ + $schema: "https://opencode.ai/config.json", + mcp: { + "test-oauth-server-3": { + type: "remote", + url: "https://example.com/mcp", + }, + }, + }), + ) + }, + }) + + await Instance.provide({ + directory: tmp.path, + fn: async () => { + openShouldFail = false + openCalledWith = undefined + + // Run authenticate with a timeout to avoid waiting forever for the callback + const authPromise = MCP.authenticate("test-oauth-server-3") + + // Wait for the browser open attempt and the 500ms error detection timeout + await new Promise((resolve) => setTimeout(resolve, 700)) + + // Stop the callback server and cancel any pending auth + await McpOAuthCallback.stop() + + // Wait for authenticate to reject (due to server stopping) + try { + await authPromise + } catch { + // Expected to fail + } + + // Verify open was called with a URL + expect(openCalledWith).toBeDefined() + expect(typeof openCalledWith).toBe("string") + expect(openCalledWith!).toContain("https://") + }, + }) +})