From 7a769dab3a77ad1fbea62c6c01adacea4a71ac12 Mon Sep 17 00:00:00 2001 From: Sebin Date: Fri, 22 May 2026 13:40:38 +0200 Subject: [PATCH] fix(mcp): include scope in clientMetadata and add callbackPort option (#28810) Co-authored-by: Shoubhit Dash --- packages/opencode/src/config/mcp.ts | 4 ++ packages/opencode/src/mcp/index.ts | 14 ++++- packages/opencode/src/mcp/oauth-provider.ts | 5 +- .../opencode/test/mcp/oauth-provider.test.ts | 61 +++++++++++++++++++ 4 files changed, 80 insertions(+), 4 deletions(-) create mode 100644 packages/opencode/test/mcp/oauth-provider.test.ts diff --git a/packages/opencode/src/config/mcp.ts b/packages/opencode/src/config/mcp.ts index cf170b95fc..2a49745dd8 100644 --- a/packages/opencode/src/config/mcp.ts +++ b/packages/opencode/src/config/mcp.ts @@ -26,6 +26,10 @@ export const OAuth = Schema.Struct({ description: "OAuth client secret (if required by the authorization server)", }), scope: Schema.optional(Schema.String).annotate({ description: "OAuth scopes to request during authorization" }), + callbackPort: Schema.optional(Schema.Int.check(Schema.isBetween({ minimum: 1, maximum: 65535 }))).annotate({ + description: + "Port for the local OAuth callback server (default: 19876). Shorthand for redirectUri when only the port needs changing. Ignored if redirectUri is set.", + }), redirectUri: Schema.optional(Schema.String).annotate({ description: "OAuth redirect URI (default: http://127.0.0.1:19876/mcp/oauth/callback).", }), diff --git a/packages/opencode/src/mcp/index.ts b/packages/opencode/src/mcp/index.ts index 70d1019573..e72b49f343 100644 --- a/packages/opencode/src/mcp/index.ts +++ b/packages/opencode/src/mcp/index.ts @@ -19,7 +19,7 @@ import { NamedError } from "@opencode-ai/core/util/error" import { InstallationVersion } from "@opencode-ai/core/installation/version" import { withTimeout } from "@/util/timeout" import { AppFileSystem } from "@opencode-ai/core/filesystem" -import { McpOAuthProvider } from "./oauth-provider" +import { McpOAuthProvider, OAUTH_CALLBACK_PATH } from "./oauth-provider" import { McpOAuthCallback } from "./oauth-callback" import { McpAuth } from "./auth" import { BusEvent } from "../bus/bus-event" @@ -318,6 +318,7 @@ export const layer = Layer.effect( clientId: oauthConfig?.clientId, clientSecret: oauthConfig?.clientSecret, scope: oauthConfig?.scope, + callbackPort: oauthConfig?.callbackPort, redirectUri: oauthConfig?.redirectUri, }, { @@ -769,8 +770,15 @@ export const layer = Layer.effect( // OAuth config is optional - if not provided, we'll use auto-discovery const oauthConfig = typeof mcpConfig.oauth === "object" ? mcpConfig.oauth : undefined + // Resolve effective redirect URI: explicit redirectUri > callbackPort shorthand > default + const effectiveRedirectUri = + oauthConfig?.redirectUri ?? + (oauthConfig?.callbackPort + ? `http://127.0.0.1:${oauthConfig.callbackPort}${OAUTH_CALLBACK_PATH}` + : undefined) + // Start the callback server with custom redirectUri if configured - yield* Effect.promise(() => McpOAuthCallback.ensureRunning(oauthConfig?.redirectUri)) + yield* Effect.promise(() => McpOAuthCallback.ensureRunning(effectiveRedirectUri)) const oauthState = Array.from(crypto.getRandomValues(new Uint8Array(32))) .map((b) => b.toString(16).padStart(2, "0")) @@ -784,7 +792,7 @@ export const layer = Layer.effect( clientId: oauthConfig?.clientId, clientSecret: oauthConfig?.clientSecret, scope: oauthConfig?.scope, - redirectUri: oauthConfig?.redirectUri, + redirectUri: effectiveRedirectUri, }, { onRedirect: async (url) => { diff --git a/packages/opencode/src/mcp/oauth-provider.ts b/packages/opencode/src/mcp/oauth-provider.ts index 45dcff50f0..c1a52639c8 100644 --- a/packages/opencode/src/mcp/oauth-provider.ts +++ b/packages/opencode/src/mcp/oauth-provider.ts @@ -18,6 +18,7 @@ export interface McpOAuthConfig { clientId?: string clientSecret?: string scope?: string + callbackPort?: number redirectUri?: string } @@ -38,7 +39,8 @@ export class McpOAuthProvider implements OAuthClientProvider { if (this.config.redirectUri) { return this.config.redirectUri } - return `http://127.0.0.1:${OAUTH_CALLBACK_PORT}${OAUTH_CALLBACK_PATH}` + const port = this.config.callbackPort ?? OAUTH_CALLBACK_PORT + return `http://127.0.0.1:${port}${OAUTH_CALLBACK_PATH}` } get clientMetadata(): OAuthClientMetadata { @@ -49,6 +51,7 @@ export class McpOAuthProvider implements OAuthClientProvider { grant_types: ["authorization_code", "refresh_token"], response_types: ["code"], token_endpoint_auth_method: this.config.clientSecret ? "client_secret_post" : "none", + ...(this.config.scope ? { scope: this.config.scope } : {}), } } diff --git a/packages/opencode/test/mcp/oauth-provider.test.ts b/packages/opencode/test/mcp/oauth-provider.test.ts new file mode 100644 index 0000000000..64c2cb6687 --- /dev/null +++ b/packages/opencode/test/mcp/oauth-provider.test.ts @@ -0,0 +1,61 @@ +import { test, expect, describe } from "bun:test" +import { McpOAuthProvider, OAUTH_CALLBACK_PORT, OAUTH_CALLBACK_PATH } from "../../src/mcp/oauth-provider" +import type { McpAuth } from "../../src/mcp/auth" + +// Stub auth — only synchronous getters are exercised in these tests +const stubAuth = {} as McpAuth.Interface + +const makeProvider = (config: ConstructorParameters[2]) => + new McpOAuthProvider("test-server", "https://mcp.example.com/mcp", config, { onRedirect: async () => {} }, stubAuth) + +describe("McpOAuthProvider.redirectUrl", () => { + test("defaults to 127.0.0.1:19876/mcp/oauth/callback", () => { + const provider = makeProvider({}) + expect(provider.redirectUrl).toBe(`http://127.0.0.1:${OAUTH_CALLBACK_PORT}${OAUTH_CALLBACK_PATH}`) + }) + + test("uses callbackPort when set", () => { + const provider = makeProvider({ callbackPort: 6620 }) + expect(provider.redirectUrl).toBe(`http://127.0.0.1:6620${OAUTH_CALLBACK_PATH}`) + }) + + test("redirectUri takes precedence over callbackPort", () => { + const provider = makeProvider({ + callbackPort: 6620, + redirectUri: "http://127.0.0.1:9999/custom/callback", + }) + expect(provider.redirectUrl).toBe("http://127.0.0.1:9999/custom/callback") + }) + + test("uses explicit redirectUri when set without callbackPort", () => { + const provider = makeProvider({ redirectUri: "http://127.0.0.1:8080/oauth/callback" }) + expect(provider.redirectUrl).toBe("http://127.0.0.1:8080/oauth/callback") + }) +}) + +describe("McpOAuthProvider.clientMetadata", () => { + test("includes redirect_uris from redirectUrl", () => { + const provider = makeProvider({ callbackPort: 6620 }) + expect(provider.clientMetadata.redirect_uris).toEqual([`http://127.0.0.1:6620${OAUTH_CALLBACK_PATH}`]) + }) + + test("includes scope when set in config", () => { + const provider = makeProvider({ scope: "openid offline_access" }) + expect(provider.clientMetadata.scope).toBe("openid offline_access") + }) + + test("omits scope when not set in config", () => { + const provider = makeProvider({}) + expect(provider.clientMetadata.scope).toBeUndefined() + }) + + test("sets token_endpoint_auth_method to client_secret_post when clientSecret provided", () => { + const provider = makeProvider({ clientSecret: "secret" }) + expect(provider.clientMetadata.token_endpoint_auth_method).toBe("client_secret_post") + }) + + test("sets token_endpoint_auth_method to none when no clientSecret", () => { + const provider = makeProvider({}) + expect(provider.clientMetadata.token_endpoint_auth_method).toBe("none") + }) +})