fix(mcp): request configured OAuth scopes when server advertises scopes

packages/opencode/test/mcp/oauth-scope-merge.test.ts

@@ -0,0 +1,125 @@
+import { test, expect, describe } from "bun:test"
+import { createRequire } from "node:module"
+
+// Other mcp tests `mock.module("@modelcontextprotocol/sdk/client/auth.js", ...)`
+// (those mocks only export UnauthorizedError) and bun leaks that across files in
+// the suite. Import the REAL module by resolved absolute path — the mock is keyed
+// on the bare specifier and does not intercept an absolute-path import.
+const require = createRequire(import.meta.url)
+const { auth } = await import(require.resolve("@modelcontextprotocol/sdk/client/auth.js"))
+
+// These tests pin the patched SDK scope-selection behavior (see
+// patches/@modelcontextprotocol%2Fsdk@1.29.0.patch): the config `scope` from
+// clientMetadata must be MERGED on top of the server-advertised resource
+// scopes, not used only as a last-resort fallback. This is what lets
+// `offline_access` (a client<->AS scope a resource server never advertises) be
+// requested so the AS issues a refresh token. See opencode issue #34034.
+
+const SERVER_URL = "https://mcp.example.com/mcp"
+const AS_URL = "https://as.example.com/"
+const AUTHORIZE = "https://as.example.com/authorize"
+
+// Drive the real SDK auth() flow with no network: returning discoveryState()
+// makes authInternal skip RFC 9728 discovery, and returning clientInformation()
+// skips dynamic client registration. The flow reaches startAuthorization() and
+// hands the final authorization URL to redirectToAuthorization(), which we
+// capture to inspect the requested `scope`.
+async function capturedAuthorizationScope(opts: {
+  advertisedScopes?: string[]
+  configScope?: string
+}): Promise<string | null> {
+  const captured: { url?: URL } = {}
+
+  const provider = {
+    get redirectUrl() {
+      return "http://127.0.0.1:19876/mcp/oauth/callback"
+    },
+    get clientMetadata() {
+      return {
+        redirect_uris: ["http://127.0.0.1:19876/mcp/oauth/callback"],
+        client_name: "OpenCode",
+        grant_types: ["authorization_code", "refresh_token"],
+        response_types: ["code"],
+        token_endpoint_auth_method: "none",
+        ...(opts.configScope ? { scope: opts.configScope } : {}),
+      }
+    },
+    async clientInformation() {
+      return { client_id: "test-client" }
+    },
+    async tokens() {
+      return undefined
+    },
+    async state() {
+      return "state-123"
+    },
+    async discoveryState() {
+      return {
+        authorizationServerUrl: AS_URL,
+        resourceMetadata: {
+          resource: "https://mcp.example.com/",
+          ...(opts.advertisedScopes ? { scopes_supported: opts.advertisedScopes } : {}),
+        },
+        authorizationServerMetadata: {
+          issuer: AS_URL,
+          authorization_endpoint: AUTHORIZE,
+          token_endpoint: "https://as.example.com/token",
+          response_types_supported: ["code"],
+          code_challenge_methods_supported: ["S256"],
+        },
+      }
+    },
+    // Skip the `resource` param + RFC 8707 compatibility check for the test.
+    async validateResourceURL() {
+      return undefined
+    },
+    async saveCodeVerifier() {},
+    async saveClientInformation() {},
+    async saveTokens() {},
+    async redirectToAuthorization(url: URL) {
+      captured.url = url
+    },
+  }
+
+  const result = await auth(provider, { serverUrl: SERVER_URL })
+  expect(result).toBe("REDIRECT")
+  return captured.url?.searchParams.get("scope") ?? null
+}
+
+const asSet = (scope: string | null) => new Set((scope ?? "").split(/\s+/).filter(Boolean))
+
+describe("MCP SDK scope selection (patched)", () => {
+  test("merges config scope on top of advertised resource scopes", async () => {
+    const scope = await capturedAuthorizationScope({
+      advertisedScopes: ["mcp:read", "mcp:write"],
+      configScope: "openid offline_access mcp:read mcp:write",
+    })
+
+    expect(asSet(scope)).toEqual(new Set(["mcp:read", "mcp:write", "openid", "offline_access"]))
+  })
+
+  test("adds offline_access when config lists only the extra scope", async () => {
+    const scope = await capturedAuthorizationScope({
+      advertisedScopes: ["mcp:read"],
+      configScope: "offline_access",
+    })
+
+    expect(asSet(scope)).toEqual(new Set(["mcp:read", "offline_access"]))
+  })
+
+  test("leaves advertised scopes untouched when no config scope is set", async () => {
+    const scope = await capturedAuthorizationScope({
+      advertisedScopes: ["mcp:read", "mcp:write"],
+    })
+
+    expect(asSet(scope)).toEqual(new Set(["mcp:read", "mcp:write"]))
+  })
+
+  test("falls back to config scope when server advertises none (#28810)", async () => {
+    const scope = await capturedAuthorizationScope({
+      configScope: "openid offline_access",
+    })
+
+    expect(asSet(scope)).toEqual(new Set(["openid", "offline_access"]))
+  })
+})

patches/@modelcontextprotocol%2Fsdk@1.29.0.patch

@@ -1,31 +1,49 @@
+diff --git a/dist/cjs/client/auth.js b/dist/cjs/client/auth.js
+index c2e4fa91d26f5336889f6afa416147db75fc4872..179a6152ab5d354ac7e5275b8cb50d4e9330ede3 100644
+--- a/dist/cjs/client/auth.js
++++ b/dist/cjs/client/auth.js
+@@ -241,12 +241,21 @@ async function authInternal(provider, { serverUrl, authorizationCode, scope, res
+         });
+     }
+     const resource = await selectResourceURL(serverUrl, provider, resourceMetadata);
+-    // Apply scope selection strategy (SEP-835):
+-    // 1. WWW-Authenticate scope (passed via `scope` param)
+-    // 2. PRM scopes_supported
+-    // 3. Client metadata scope (user-configured fallback)
++    // Apply scope selection strategy (SEP-835 + opencode #34034):
++    // Server-advertised resource scopes (WWW-Authenticate, then PRM
++    // scopes_supported) are MERGED with the user-configured
++    // clientMetadata.scope instead of the config scope being a last-resort
++    // fallback. This lets client<->AS scopes like `offline_access` (which a
++    // resource server never advertises) be requested on top of the resource
++    // scopes, so the AS issues a refresh token. Order preserved, deduped.
+     // The resolved scope is used consistently for both DCR and the authorization request.
+-    const resolvedScope = scope || resourceMetadata?.scopes_supported?.join(' ') || provider.clientMetadata.scope;
++    const advertisedScope = scope || resourceMetadata?.scopes_supported?.join(' ');
++    const resolvedScope = [advertisedScope, provider.clientMetadata.scope]
++        .filter(Boolean)
++        .flatMap((s) => s.split(/\s+/))
++        .filter(Boolean)
++        .filter((value, index, all) => all.indexOf(value) === index)
++        .join(' ') || undefined;
+     // Handle client registration if needed
+     let clientInformation = await Promise.resolve(provider.clientInformation());
+     if (!clientInformation) {
 diff --git a/dist/cjs/client/index.d.ts b/dist/cjs/client/index.d.ts
-index 1822bf749aec71d2bb295083d832114ee187bb67..58b859a7b32222fb5cb9f2011fdc5d010f3d05fb 100644
+index 6f567a193626587a2730b5a49293ca5dfd4181ea..5b7c841c000508e389ce617f559f7c2a5126ca9f 100644
 --- a/dist/cjs/client/index.d.ts
 +++ b/dist/cjs/client/index.d.ts
 @@ -428,6 +428,8 @@ export declare class Client<RequestT extends Request = Request, NotificationT ex
       *
       * For task-based execution with streaming behavior, use client.experimental.tasks.callToolStream() instead.
       */
 +    callTool(params: CallToolRequest['params'], resultSchema?: undefined, options?: RequestOptions): Promise<SchemaOutput<typeof CallToolResultSchema>>;
-+    callTool<T extends typeof CallToolResultSchema | typeof CompatibilityCallToolResultSchema>(params: CallToolRequest['params'], resultSchema: T, options?: RequestOptions): Promise<SchemaOutput<T>>;
-     callTool(params: CallToolRequest['params'], resultSchema?: typeof CallToolResultSchema | typeof CompatibilityCallToolResultSchema, options?: RequestOptions): Promise<{
-         [x: string]: unknown;
-         content: ({
-diff --git a/dist/esm/client/index.d.ts b/dist/esm/client/index.d.ts
-index 1822bf749aec71d2bb295083d832114ee187bb67..58b859a7b32222fb5cb9f2011fdc5d010f3d05fb 100644
---- a/dist/esm/client/index.d.ts
-+++ b/dist/esm/client/index.d.ts
-@@ -428,6 +428,8 @@ export declare class Client<RequestT extends Request = Request, NotificationT ex
-      *
-      * For task-based execution with streaming behavior, use client.experimental.tasks.callToolStream() instead.
-      */
-+    callTool(params: CallToolRequest['params'], resultSchema?: undefined, options?: RequestOptions): Promise<SchemaOutput<typeof CallToolResultSchema>>;
 +    callTool<T extends typeof CallToolResultSchema | typeof CompatibilityCallToolResultSchema>(params: CallToolRequest['params'], resultSchema: T, options?: RequestOptions): Promise<SchemaOutput<T>>;
      callTool(params: CallToolRequest['params'], resultSchema?: typeof CallToolResultSchema | typeof CompatibilityCallToolResultSchema, options?: RequestOptions): Promise<{
          [x: string]: unknown;
          content: ({
 diff --git a/dist/cjs/client/index.js b/dist/cjs/client/index.js
-index 6ac1da14dc7f6211ae70f7711c124b76098816d8..adb5b7bd45514a406a0f7e40b64631c101584c84 100644
+index 6ac1da14dc7f6211ae70f7711c124b76098816d8..88e58b90b673cb0f9c60920edec7d2ebb16f112b 100644
 --- a/dist/cjs/client/index.js
 +++ b/dist/cjs/client/index.js
 @@ -288,41 +288,16 @@ class Client extends protocol_js_1.Protocol {
@@ -113,7 +131,7 @@ index 6ac1da14dc7f6211ae70f7711c124b76098816d8..adb5b7bd45514a406a0f7e40b64631c1
       * After initialization has completed, this will be populated with the server's reported capabilities.
       */
 diff --git a/dist/cjs/client/streamableHttp.js b/dist/cjs/client/streamableHttp.js
-index a29a7d3a0f14d9cd800ef5b296485237350c666f..c362ae5fe6c62c8c8eae7e2e61de1eedff5443c9 100644
+index a29a7d3a0f14d9cd800ef5b296485237350c666f..6c7cbbca4777def85c4bcfc573717c0685857bbb 100644
 --- a/dist/cjs/client/streamableHttp.js
 +++ b/dist/cjs/client/streamableHttp.js
 @@ -290,7 +290,38 @@ class StreamableHTTPClientTransport {
@@ -204,7 +222,7 @@ index a29a7d3a0f14d9cd800ef5b296485237350c666f..c362ae5fe6c62c8c8eae7e2e61de1eed
                  }
                  throw new StreamableHTTPError(response.status, `Error POSTing to endpoint: ${text}`);
 diff --git a/dist/cjs/shared/protocol.js b/dist/cjs/shared/protocol.js
-index 3617e787f0ba70447c99501aee7aa67584d89758..4a96d6a0328fa348b96f3869ab7e0bb77538182b 100644
+index 3617e787f0ba70447c99501aee7aa67584d89758..4ee4d158391558fdc1f977f5134b7cacfc45e8c3 100644
 --- a/dist/cjs/shared/protocol.js
 +++ b/dist/cjs/shared/protocol.js
 @@ -744,7 +744,12 @@ class Protocol {
@@ -221,8 +239,52 @@ index 3617e787f0ba70447c99501aee7aa67584d89758..4a96d6a0328fa348b96f3869ab7e0bb7
                      this._cleanupTimeout(messageId);
                      reject(error);
                  });
+diff --git a/dist/esm/client/auth.js b/dist/esm/client/auth.js
+index e183040fc2bba22ca1ccc784984f3310854403b7..fefc74a3f405b649639c3d69969349a23a5f3df2 100644
+--- a/dist/esm/client/auth.js
++++ b/dist/esm/client/auth.js
+@@ -216,12 +216,21 @@ async function authInternal(provider, { serverUrl, authorizationCode, scope, res
+         });
+     }
+     const resource = await selectResourceURL(serverUrl, provider, resourceMetadata);
+-    // Apply scope selection strategy (SEP-835):
+-    // 1. WWW-Authenticate scope (passed via `scope` param)
+-    // 2. PRM scopes_supported
+-    // 3. Client metadata scope (user-configured fallback)
++    // Apply scope selection strategy (SEP-835 + opencode #34034):
++    // Server-advertised resource scopes (WWW-Authenticate, then PRM
++    // scopes_supported) are MERGED with the user-configured
++    // clientMetadata.scope instead of the config scope being a last-resort
++    // fallback. This lets client<->AS scopes like `offline_access` (which a
++    // resource server never advertises) be requested on top of the resource
++    // scopes, so the AS issues a refresh token. Order preserved, deduped.
+     // The resolved scope is used consistently for both DCR and the authorization request.
+-    const resolvedScope = scope || resourceMetadata?.scopes_supported?.join(' ') || provider.clientMetadata.scope;
++    const advertisedScope = scope || resourceMetadata?.scopes_supported?.join(' ');
++    const resolvedScope = [advertisedScope, provider.clientMetadata.scope]
++        .filter(Boolean)
++        .flatMap((s) => s.split(/\s+/))
++        .filter(Boolean)
++        .filter((value, index, all) => all.indexOf(value) === index)
++        .join(' ') || undefined;
+     // Handle client registration if needed
+     let clientInformation = await Promise.resolve(provider.clientInformation());
+     if (!clientInformation) {
+diff --git a/dist/esm/client/index.d.ts b/dist/esm/client/index.d.ts
+index 6f567a193626587a2730b5a49293ca5dfd4181ea..5b7c841c000508e389ce617f559f7c2a5126ca9f 100644
+--- a/dist/esm/client/index.d.ts
++++ b/dist/esm/client/index.d.ts
+@@ -428,6 +428,8 @@ export declare class Client<RequestT extends Request = Request, NotificationT ex
+      *
+      * For task-based execution with streaming behavior, use client.experimental.tasks.callToolStream() instead.
+      */
++    callTool(params: CallToolRequest['params'], resultSchema?: undefined, options?: RequestOptions): Promise<SchemaOutput<typeof CallToolResultSchema>>;
++    callTool<T extends typeof CallToolResultSchema | typeof CompatibilityCallToolResultSchema>(params: CallToolRequest['params'], resultSchema: T, options?: RequestOptions): Promise<SchemaOutput<T>>;
+     callTool(params: CallToolRequest['params'], resultSchema?: typeof CallToolResultSchema | typeof CompatibilityCallToolResultSchema, options?: RequestOptions): Promise<{
+         [x: string]: unknown;
+         content: ({
 diff --git a/dist/esm/client/index.js b/dist/esm/client/index.js
-index 49b12c6cd918c457420fef7ad5528a9443d1a191..2afe2e22e960f26c9d516ef135d89f8eb9e4caff 100644
+index 49b12c6cd918c457420fef7ad5528a9443d1a191..339153cb7e9299b7a9bdec0e56e41cedf425fb31 100644
 --- a/dist/esm/client/index.js
 +++ b/dist/esm/client/index.js
 @@ -284,41 +284,16 @@ export class Client extends Protocol {
@@ -310,7 +372,7 @@ index 49b12c6cd918c457420fef7ad5528a9443d1a191..2afe2e22e960f26c9d516ef135d89f8e
       * After initialization has completed, this will be populated with the server's reported capabilities.
       */
 diff --git a/dist/esm/client/streamableHttp.js b/dist/esm/client/streamableHttp.js
-index 624172aa24ae255a67c083f9c19053343e4a0581..ac75b14545fda44aff7ff4d97cc5da884fcc627a 100644
+index 624172aa24ae255a67c083f9c19053343e4a0581..0a08610045189f33ab029d5dec60262a9c5e859d 100644
 --- a/dist/esm/client/streamableHttp.js
 +++ b/dist/esm/client/streamableHttp.js
 @@ -1,5 +1,5 @@
@@ -408,7 +470,7 @@ index 624172aa24ae255a67c083f9c19053343e4a0581..ac75b14545fda44aff7ff4d97cc5da88
                  }
                  throw new StreamableHTTPError(response.status, `Error POSTing to endpoint: ${text}`);
 diff --git a/dist/esm/shared/protocol.js b/dist/esm/shared/protocol.js
-index bfa2b7120a0f50c569364ea5264e6f811076f44f..abd8dfd707c155f71dae7aeeeeaf7547368ac749 100644
+index bfa2b7120a0f50c569364ea5264e6f811076f44f..dec477d16a0fd796854542c1144279a6e86567f2 100644
 --- a/dist/esm/shared/protocol.js
 +++ b/dist/esm/shared/protocol.js
 @@ -740,7 +740,12 @@ export class Protocol {