Kaynağa Gözat

fix(mcp): show auth URL when browser cannot open in remote sessions (#7884)

Dan Lapid 1 ay önce
ebeveyn
işleme
b572c68100

+ 13 - 0
packages/opencode/src/cli/cmd/mcp.ts

@@ -13,6 +13,7 @@ import { Installation } from "../../installation"
 import path from "path"
 import path from "path"
 import { Global } from "../../global"
 import { Global } from "../../global"
 import { modify, applyEdits } from "jsonc-parser"
 import { modify, applyEdits } from "jsonc-parser"
+import { Bus } from "../../bus"
 
 
 function getAuthStatusIcon(status: MCP.AuthStatus): string {
 function getAuthStatusIcon(status: MCP.AuthStatus): string {
   switch (status) {
   switch (status) {
@@ -227,6 +228,16 @@ export const McpAuthCommand = cmd({
         const spinner = prompts.spinner()
         const spinner = prompts.spinner()
         spinner.start("Starting OAuth flow...")
         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 {
         try {
           const status = await MCP.authenticate(serverName)
           const status = await MCP.authenticate(serverName)
 
 
@@ -256,6 +267,8 @@ export const McpAuthCommand = cmd({
         } catch (error) {
         } catch (error) {
           spinner.stop("Authentication failed", 1)
           spinner.stop("Authentication failed", 1)
           prompts.log.error(error instanceof Error ? error.message : String(error))
           prompts.log.error(error instanceof Error ? error.message : String(error))
+        } finally {
+          unsubscribe()
         }
         }
 
 
         prompts.outro("Done")
         prompts.outro("Done")

+ 34 - 1
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(
   export const Failed = NamedError.create(
     "MCPFailed",
     "MCPFailed",
     z.object({
     z.object({
@@ -787,7 +795,32 @@ export namespace MCP {
     // The SDK has already added the state parameter to the authorization URL
     // The SDK has already added the state parameter to the authorization URL
     // We just need to open the browser
     // We just need to open the browser
     log.info("opening browser for oauth", { mcpName, url: authorizationUrl, state: oauthState })
     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<void>((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
     // Wait for callback using the OAuth state parameter
     const code = await McpOAuthCallback.waitForCallback(oauthState)
     const code = await McpOAuthCallback.waitForCallback(oauthState)

+ 261 - 0
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<void> } | undefined
+    constructor(url: URL, options?: { authProvider?: { redirectToAuthorization?: (url: URL) => Promise<void> } }) {
+      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<void> }) {
+      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://")
+    },
+  })
+})