diff --git a/packages/opencode/src/cli/cmd/mcp.ts b/packages/opencode/src/cli/cmd/mcp.ts index fedad9285..95719215e 100644 --- a/packages/opencode/src/cli/cmd/mcp.ts +++ b/packages/opencode/src/cli/cmd/mcp.ts @@ -6,7 +6,6 @@ import * as prompts from "@clack/prompts" import { UI } from "../ui" import { MCP } from "../../mcp" import { McpAuth } from "../../mcp/auth" -import { McpOAuthCallback } from "../../mcp/oauth-callback" import { McpOAuthProvider } from "../../mcp/oauth-provider" import { Config } from "../../config/config" import { Instance } from "../../project/instance" @@ -683,10 +682,6 @@ export const McpDebugCommand = cmd({ // Try to discover OAuth metadata const oauthConfig = typeof serverConfig.oauth === "object" ? serverConfig.oauth : undefined - - // Start callback server - await McpOAuthCallback.ensureRunning(oauthConfig?.redirectUri) - const authProvider = new McpOAuthProvider( serverName, serverConfig.url, @@ -694,7 +689,6 @@ export const McpDebugCommand = cmd({ clientId: oauthConfig?.clientId, clientSecret: oauthConfig?.clientSecret, scope: oauthConfig?.scope, - redirectUri: oauthConfig?.redirectUri, }, { onRedirect: async () => {}, diff --git a/packages/opencode/src/config/config.ts b/packages/opencode/src/config/config.ts index 355b3ba00..1574c644d 100644 --- a/packages/opencode/src/config/config.ts +++ b/packages/opencode/src/config/config.ts @@ -435,10 +435,6 @@ export namespace Config { .describe("OAuth client ID. If not provided, dynamic client registration (RFC 7591) will be attempted."), clientSecret: z.string().optional().describe("OAuth client secret (if required by the authorization server)"), scope: z.string().optional().describe("OAuth scopes to request during authorization"), - redirectUri: z - .string() - .optional() - .describe("OAuth redirect URI (default: http://127.0.0.1:19876/mcp/oauth/callback)."), }) .strict() .meta({ diff --git a/packages/opencode/src/mcp/index.ts b/packages/opencode/src/mcp/index.ts index 7b9a8c207..66843aedc 100644 --- a/packages/opencode/src/mcp/index.ts +++ b/packages/opencode/src/mcp/index.ts @@ -308,8 +308,6 @@ export namespace MCP { let authProvider: McpOAuthProvider | undefined if (!oauthDisabled) { - await McpOAuthCallback.ensureRunning(oauthConfig?.redirectUri) - authProvider = new McpOAuthProvider( key, mcp.url, @@ -317,7 +315,6 @@ export namespace MCP { clientId: oauthConfig?.clientId, clientSecret: oauthConfig?.clientSecret, scope: oauthConfig?.scope, - redirectUri: oauthConfig?.redirectUri, }, { onRedirect: async (url) => { @@ -347,7 +344,6 @@ export namespace MCP { let lastError: Error | undefined const connectTimeout = mcp.timeout ?? DEFAULT_TIMEOUT - for (const { name, transport } of transports) { try { const client = new Client({ @@ -574,8 +570,7 @@ export namespace MCP { for (const [clientName, client] of Object.entries(clientsSnapshot)) { // Only include tools from connected MCPs (skip disabled ones) - const clientStatus = s.status[clientName]?.status - if (clientStatus !== "connected") { + if (s.status[clientName]?.status !== "connected") { continue } @@ -725,10 +720,8 @@ export namespace MCP { throw new Error(`MCP server ${mcpName} has OAuth explicitly disabled`) } - // OAuth config is optional - if not provided, we'll use auto-discovery - const oauthConfig = typeof mcpConfig.oauth === "object" ? mcpConfig.oauth : undefined - - await McpOAuthCallback.ensureRunning(oauthConfig?.redirectUri) + // Start the callback server + await McpOAuthCallback.ensureRunning() // Generate and store a cryptographically secure state parameter BEFORE creating the provider // The SDK will call provider.state() to read this value @@ -738,6 +731,8 @@ export namespace MCP { await McpAuth.updateOAuthState(mcpName, oauthState) // Create a new auth provider for this flow + // OAuth config is optional - if not provided, we'll use auto-discovery + const oauthConfig = typeof mcpConfig.oauth === "object" ? mcpConfig.oauth : undefined let capturedUrl: URL | undefined const authProvider = new McpOAuthProvider( mcpName, @@ -746,7 +741,6 @@ export namespace MCP { clientId: oauthConfig?.clientId, clientSecret: oauthConfig?.clientSecret, scope: oauthConfig?.scope, - redirectUri: oauthConfig?.redirectUri, }, { onRedirect: async (url) => { @@ -775,7 +769,6 @@ export namespace MCP { pendingOAuthTransports.set(mcpName, transport) return { authorizationUrl: capturedUrl.toString() } } - throw error } } @@ -785,9 +778,9 @@ export namespace MCP { * Opens the browser and waits for callback. */ export async function authenticate(mcpName: string): Promise { - const result = await startAuth(mcpName) + const { authorizationUrl } = await startAuth(mcpName) - if (!result.authorizationUrl) { + if (!authorizationUrl) { // Already authenticated const s = await state() return s.status[mcpName] ?? { status: "connected" } @@ -801,9 +794,9 @@ 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: result.authorizationUrl, state: oauthState }) + log.info("opening browser for oauth", { mcpName, url: authorizationUrl, state: oauthState }) try { - const subprocess = await open(result.authorizationUrl) + 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) @@ -826,7 +819,7 @@ export namespace MCP { // 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: result.authorizationUrl }) + Bus.publish(BrowserOpenFailed, { mcpName, url: authorizationUrl }) } // Wait for callback using the OAuth state parameter diff --git a/packages/opencode/src/mcp/oauth-callback.ts b/packages/opencode/src/mcp/oauth-callback.ts index a690ab5e3..bb3b56f2e 100644 --- a/packages/opencode/src/mcp/oauth-callback.ts +++ b/packages/opencode/src/mcp/oauth-callback.ts @@ -1,12 +1,8 @@ import { Log } from "../util/log" -import { OAUTH_CALLBACK_PORT, OAUTH_CALLBACK_PATH, parseRedirectUri } from "./oauth-provider" +import { OAUTH_CALLBACK_PORT, OAUTH_CALLBACK_PATH } from "./oauth-provider" const log = Log.create({ service: "mcp.oauth-callback" }) -// Current callback server configuration (may differ from defaults if custom redirectUri is used) -let currentPort = OAUTH_CALLBACK_PORT -let currentPath = OAUTH_CALLBACK_PATH - const HTML_SUCCESS = ` @@ -60,33 +56,21 @@ export namespace McpOAuthCallback { const CALLBACK_TIMEOUT_MS = 5 * 60 * 1000 // 5 minutes - export async function ensureRunning(redirectUri?: string): Promise { - // Parse the redirect URI to get port and path (uses defaults if not provided) - const { port, path } = parseRedirectUri(redirectUri) - - // If server is running on a different port/path, stop it first - if (server && (currentPort !== port || currentPath !== path)) { - log.info("stopping oauth callback server to reconfigure", { oldPort: currentPort, newPort: port }) - await stop() - } - + export async function ensureRunning(): Promise { if (server) return - const running = await isPortInUse(port) + const running = await isPortInUse() if (running) { - log.info("oauth callback server already running on another instance", { port }) + log.info("oauth callback server already running on another instance", { port: OAUTH_CALLBACK_PORT }) return } - currentPort = port - currentPath = path - server = Bun.serve({ - port: currentPort, + port: OAUTH_CALLBACK_PORT, fetch(req) { const url = new URL(req.url) - if (url.pathname !== currentPath) { + if (url.pathname !== OAUTH_CALLBACK_PATH) { return new Response("Not found", { status: 404 }) } @@ -149,7 +133,7 @@ export namespace McpOAuthCallback { }, }) - log.info("oauth callback server started", { port: currentPort, path: currentPath }) + log.info("oauth callback server started", { port: OAUTH_CALLBACK_PORT }) } export function waitForCallback(oauthState: string): Promise { @@ -174,11 +158,11 @@ export namespace McpOAuthCallback { } } - export async function isPortInUse(port: number = OAUTH_CALLBACK_PORT): Promise { + export async function isPortInUse(): Promise { return new Promise((resolve) => { Bun.connect({ hostname: "127.0.0.1", - port, + port: OAUTH_CALLBACK_PORT, socket: { open(socket) { socket.end() diff --git a/packages/opencode/src/mcp/oauth-provider.ts b/packages/opencode/src/mcp/oauth-provider.ts index 82bad60da..35ead25e8 100644 --- a/packages/opencode/src/mcp/oauth-provider.ts +++ b/packages/opencode/src/mcp/oauth-provider.ts @@ -17,7 +17,6 @@ export interface McpOAuthConfig { clientId?: string clientSecret?: string scope?: string - redirectUri?: string } export interface McpOAuthCallbacks { @@ -33,10 +32,6 @@ export class McpOAuthProvider implements OAuthClientProvider { ) {} get redirectUrl(): string { - // Use configured redirectUri if provided, otherwise use OpenCode defaults - if (this.config.redirectUri) { - return this.config.redirectUri - } return `http://127.0.0.1:${OAUTH_CALLBACK_PORT}${OAUTH_CALLBACK_PATH}` } @@ -157,22 +152,3 @@ export class McpOAuthProvider implements OAuthClientProvider { } export { OAUTH_CALLBACK_PORT, OAUTH_CALLBACK_PATH } - -/** - * Parse a redirect URI to extract port and path for the callback server. - * Returns defaults if the URI can't be parsed. - */ -export function parseRedirectUri(redirectUri?: string): { port: number; path: string } { - if (!redirectUri) { - return { port: OAUTH_CALLBACK_PORT, path: OAUTH_CALLBACK_PATH } - } - - try { - const url = new URL(redirectUri) - const port = url.port ? parseInt(url.port, 10) : url.protocol === "https:" ? 443 : 80 - const path = url.pathname || OAUTH_CALLBACK_PATH - return { port, path } - } catch { - return { port: OAUTH_CALLBACK_PORT, path: OAUTH_CALLBACK_PATH } - } -} diff --git a/packages/opencode/test/mcp/oauth-callback.test.ts b/packages/opencode/test/mcp/oauth-callback.test.ts deleted file mode 100644 index aa23f4dfb..000000000 --- a/packages/opencode/test/mcp/oauth-callback.test.ts +++ /dev/null @@ -1,75 +0,0 @@ -import { test, expect, describe, afterEach } from "bun:test" -import { McpOAuthCallback } from "../../src/mcp/oauth-callback" -import { parseRedirectUri } from "../../src/mcp/oauth-provider" - -describe("McpOAuthCallback.ensureRunning", () => { - afterEach(async () => { - await McpOAuthCallback.stop() - }) - - test("starts server with default config when no redirectUri provided", async () => { - await McpOAuthCallback.ensureRunning() - expect(McpOAuthCallback.isRunning()).toBe(true) - }) - - test("starts server with custom redirectUri", async () => { - await McpOAuthCallback.ensureRunning("http://127.0.0.1:18000/custom/callback") - expect(McpOAuthCallback.isRunning()).toBe(true) - }) - - test("is idempotent when called with same redirectUri", async () => { - await McpOAuthCallback.ensureRunning("http://127.0.0.1:18001/callback") - await McpOAuthCallback.ensureRunning("http://127.0.0.1:18001/callback") - expect(McpOAuthCallback.isRunning()).toBe(true) - }) - - test("restarts server when redirectUri changes", async () => { - await McpOAuthCallback.ensureRunning("http://127.0.0.1:18002/path1") - expect(McpOAuthCallback.isRunning()).toBe(true) - - await McpOAuthCallback.ensureRunning("http://127.0.0.1:18003/path2") - expect(McpOAuthCallback.isRunning()).toBe(true) - }) - - test("isRunning returns false when not started", async () => { - expect(McpOAuthCallback.isRunning()).toBe(false) - }) - - test("isRunning returns false after stop", async () => { - await McpOAuthCallback.ensureRunning() - await McpOAuthCallback.stop() - expect(McpOAuthCallback.isRunning()).toBe(false) - }) -}) - -describe("parseRedirectUri", () => { - test("returns defaults when no URI provided", () => { - const result = parseRedirectUri() - expect(result.port).toBe(19876) - expect(result.path).toBe("/mcp/oauth/callback") - }) - - test("parses port and path from URI", () => { - const result = parseRedirectUri("http://127.0.0.1:8080/oauth/callback") - expect(result.port).toBe(8080) - expect(result.path).toBe("/oauth/callback") - }) - - test("defaults to port 80 for http without explicit port", () => { - const result = parseRedirectUri("http://127.0.0.1/callback") - expect(result.port).toBe(80) - expect(result.path).toBe("/callback") - }) - - test("defaults to port 443 for https without explicit port", () => { - const result = parseRedirectUri("https://127.0.0.1/callback") - expect(result.port).toBe(443) - expect(result.path).toBe("/callback") - }) - - test("returns defaults for invalid URI", () => { - const result = parseRedirectUri("not-a-valid-url") - expect(result.port).toBe(19876) - expect(result.path).toBe("/mcp/oauth/callback") - }) -})