fix(oauth+errors): OAuth silent refresh, quota STOP patterns, compaction loop cap

Bug fixes: 1. OAuth token refresh (#3149): buildHttpRequestInit() now attempts silent refresh via refresh_token before triggering full browser re-auth. Added refresh() method to McpOAuthProvider. Includes test isolation fix for discovery mock. 2. Quota error STOP (#3126): Added STOP_MESSAGE_PATTERNS in model-error-classifier that take precedence over RETRYABLE_MESSAGE_PATTERNS. Message-only quota errors now non-retryable. Runtime-fallback: quota_exceeded with 'retrying in' signal still triggers fallback (provider-managed auto-retry). Restored removed patterns. 3. Compaction loop (#3127): MAX_RECOVERY_ATTEMPTS=3 cap + additional suppression guard from opencode session in degradation monitor. Also: refactored extractAutoRetrySignal to auto-retry-signal.ts, new regression tests for quota classifier and compaction degradation monitor.
2026-04-06 17:38:37 +09:00
parent 6c4e0b69a5
commit 61083d499d
15 changed files with 611 additions and 531 deletions
@@ -1,4 +1,4 @@
-import { describe, expect, it, beforeEach, afterEach, mock } from "bun:test"
+import { describe, expect, it, beforeEach, afterEach, mock, afterAll } from "bun:test"
 import { createHash, randomBytes } from "node:crypto"
 import type { OAuthTokenData } from "./storage"

@@ -226,6 +226,90 @@ describe("McpOAuthProvider", () => {
    })
  })

+  describe("refresh", () => {
+    let originalFetch: typeof globalThis.fetch
+    let originalEnv: string | undefined
+
+    beforeEach(() => {
+      originalFetch = globalThis.fetch
+      originalEnv = process.env.OPENCODE_CONFIG_DIR
+      const { mkdirSync } = require("node:fs")
+      const { tmpdir } = require("node:os")
+      const { join } = require("node:path")
+      const testDir = join(tmpdir(), `mcp-oauth-provider-refresh-test-${Date.now()}`)
+      mkdirSync(testDir, { recursive: true })
+      process.env.OPENCODE_CONFIG_DIR = testDir
+    })
+
+    afterEach(() => {
+      globalThis.fetch = originalFetch
+      if (originalEnv === undefined) {
+        delete process.env.OPENCODE_CONFIG_DIR
+      } else {
+        process.env.OPENCODE_CONFIG_DIR = originalEnv
+      }
+    })
+
+    it("exchanges refresh token and preserves it when the response omits a new one", async () => {
+      // Stub fetch to handle both discovery (well-known) and token exchange
+      const fetchStub = mock(async (input: RequestInfo | URL, init?: RequestInit) => {
+        const url = input.toString()
+        if (url.includes("oauth-protected-resource")) {
+          // PRM: return authorization_servers pointing to auth server
+          return new Response(
+            JSON.stringify({ authorization_servers: ["https://auth.example.com"] }),
+            { status: 200, headers: { "content-type": "application/json" } },
+          )
+        }
+        if (url.includes(".well-known")) {
+          // AS metadata
+          return new Response(
+            JSON.stringify({
+              issuer: "https://auth.example.com",
+              authorization_endpoint: "https://auth.example.com/authorize",
+              token_endpoint: "https://auth.example.com/token",
+            }),
+            { status: 200, headers: { "content-type": "application/json" } },
+          )
+        }
+        // Token exchange
+        const body = init?.body?.toString() ?? ""
+        expect(body).toContain("grant_type=refresh_token")
+        expect(body).toContain("refresh_token=refresh-token-456")
+        expect(body).toContain("client_id=my-client")
+        return new Response(
+          JSON.stringify({ access_token: "refreshed-access-token", expires_in: 3600 }),
+          { status: 200, headers: { "content-type": "application/json" } },
+        )
+      })
+      const fetchMock = Object.assign(
+        async (...args: Parameters<typeof fetch>): ReturnType<typeof fetch> => fetchStub(...args),
+        { preconnect: originalFetch.preconnect.bind(originalFetch) },
+      ) satisfies typeof fetch
+      globalThis.fetch = fetchMock
+
+      // given
+      const providerModule = await importFreshProviderModule()
+      const provider = new providerModule.McpOAuthProvider({
+        serverUrl: "https://mcp.example.com",
+        clientId: "my-client",
+      })
+      provider.saveTokens({
+        accessToken: "old-access-token",
+        refreshToken: "refresh-token-456",
+        expiresAt: Math.floor(Date.now() / 1000) - 60,
+        clientInfo: { clientId: "my-client" },
+      })
+
+      // when
+      const result = await provider.refresh("refresh-token-456")
+
+      // then
+      expect(result.accessToken).toBe("refreshed-access-token")
+      expect(result.refreshToken).toBe("refresh-token-456") // preserved from input when absent in response
+    })
+  })
+
  describe("redirectUrl", () => {
    it("returns localhost callback URL with default port", () => {
      // given
@@ -19,6 +19,48 @@ export type McpOAuthProviderOptions = {
  scopes?: string[]
 }

+async function parseTokenResponse(tokenResponse: Response): Promise<Record<string, unknown>> {
+  if (!tokenResponse.ok) {
+    let errorDetail = `${tokenResponse.status}`
+    try {
+      const body = (await tokenResponse.json()) as Record<string, unknown>
+      if (body.error) {
+        errorDetail = `${tokenResponse.status} ${body.error}`
+        if (body.error_description) {
+          errorDetail += `: ${body.error_description}`
+        }
+      }
+    } catch {
+      // Response body not JSON
+    }
+    throw new Error(`Token exchange failed: ${errorDetail}`)
+  }
+
+  return (await tokenResponse.json()) as Record<string, unknown>
+}
+
+function buildOAuthTokenData(
+  tokenData: Record<string, unknown>,
+  clientInfo: ClientCredentials,
+  fallbackRefreshToken?: string,
+): OAuthTokenData {
+  const accessToken = tokenData.access_token
+  if (typeof accessToken !== "string") {
+    throw new Error("Token response missing access_token")
+  }
+
+  return {
+    accessToken,
+    refreshToken: typeof tokenData.refresh_token === "string" ? tokenData.refresh_token : fallbackRefreshToken,
+    expiresAt:
+      typeof tokenData.expires_in === "number" ? Math.floor(Date.now() / 1000) + tokenData.expires_in : undefined,
+    clientInfo: {
+      clientId: clientInfo.clientId,
+      ...(clientInfo.clientSecret ? { clientSecret: clientInfo.clientSecret } : {}),
+    },
+  }
+}
+
 export class McpOAuthProvider {
  private readonly serverUrl: string
  private readonly configClientId: string | undefined
@@ -131,38 +173,38 @@ export class McpOAuthProvider {
      }).toString(),
    })

-    if (!tokenResponse.ok) {
-      let errorDetail = `${tokenResponse.status}`
-      try {
-        const body = (await tokenResponse.json()) as Record<string, unknown>
-        if (body.error) {
-          errorDetail = `${tokenResponse.status} ${body.error}`
-          if (body.error_description) {
-            errorDetail += `: ${body.error_description}`
-          }
-        }
-      } catch {
-        // Response body not JSON
-      }
-      throw new Error(`Token exchange failed: ${errorDetail}`)
+    const tokenData = await parseTokenResponse(tokenResponse)
+    const oauthTokenData = buildOAuthTokenData(tokenData, clientInfo)
+
+    this.saveTokens(oauthTokenData)
+    return oauthTokenData
+  }
+
+  async refresh(refreshToken: string): Promise<OAuthTokenData> {
+    const metadata = await discoverOAuthServerMetadata(this.serverUrl)
+    const clientInfo = this.clientInformation()
+    const clientId = clientInfo?.clientId ?? this.configClientId
+    if (!clientId) {
+      throw new Error("No client information available. Run login() or register a client first.")
    }

-    const tokenData = (await tokenResponse.json()) as Record<string, unknown>
-    const accessToken = tokenData.access_token
-    if (typeof accessToken !== "string") {
-      throw new Error("Token response missing access_token")
-    }
+    const tokenResponse = await fetch(metadata.tokenEndpoint, {
+      method: "POST",
+      headers: { "content-type": "application/x-www-form-urlencoded" },
+      body: new URLSearchParams({
+        grant_type: "refresh_token",
+        refresh_token: refreshToken,
+        client_id: clientId,
+        ...(clientInfo?.clientSecret ? { client_secret: clientInfo.clientSecret } : {}),
+        ...(metadata.resource ? { resource: metadata.resource } : {}),
+      }).toString(),
+    })

-    const oauthTokenData: OAuthTokenData = {
-      accessToken,
-      refreshToken: typeof tokenData.refresh_token === "string" ? tokenData.refresh_token : undefined,
-      expiresAt:
-        typeof tokenData.expires_in === "number" ? Math.floor(Date.now() / 1000) + tokenData.expires_in : undefined,
-      clientInfo: {
-        clientId: clientInfo.clientId,
-        clientSecret: clientInfo.clientSecret,
-      },
-    }
+    const tokenData = await parseTokenResponse(tokenResponse)
+    const oauthTokenData = buildOAuthTokenData(tokenData, {
+      clientId,
+      ...(clientInfo?.clientSecret ? { clientSecret: clientInfo.clientSecret } : {}),
+    }, refreshToken)

    this.saveTokens(oauthTokenData)
    return oauthTokenData
@@ -1,14 +1,16 @@
 import { describe, it, expect, beforeEach, afterEach, afterAll, mock, spyOn } from "bun:test"
 import type { SkillMcpClientInfo, SkillMcpServerContext } from "./types"
 import type { ClaudeCodeMcpServer } from "../claude-code-mcp-loader/types"
+import type { OAuthTokenData } from "../mcp-oauth/storage"

 // Mock the MCP SDK transports to avoid network calls
 const mockHttpConnect = mock(() => Promise.reject(new Error("Mocked HTTP connection failure")))
 const mockHttpClose = mock(() => Promise.resolve())
 let lastTransportInstance: { url?: URL; options?: { requestInit?: RequestInit } } = {}

-const mockTokens = mock(() => null as { accessToken: string } | null)
-const mockLogin = mock(() => Promise.resolve({ accessToken: "test-token" }) as Promise<{ accessToken: string } | null>)
+const mockTokens = mock(() => null as OAuthTokenData | null)
+const mockLogin = mock(() => Promise.resolve({ accessToken: "test-token" } satisfies OAuthTokenData))
+const mockRefresh = mock((_: string) => Promise.resolve({ accessToken: "refreshed-token" } satisfies OAuthTokenData))

 async function importFreshManagerModule(): Promise<typeof import("./manager")> {
  mock.module("@modelcontextprotocol/sdk/client/streamableHttp.js", () => ({
@@ -41,12 +43,14 @@ describe("SkillMcpManager", () => {
      createOAuthProvider: () => ({
        tokens: () => mockTokens(),
        login: () => mockLogin(),
+        refresh: (refreshToken: string) => mockRefresh(refreshToken),
      }),
    })
    mockHttpConnect.mockClear()
    mockHttpClose.mockClear()
    mockTokens.mockClear()
    mockLogin.mockClear()
+    mockRefresh.mockClear()
  })

  afterEach(async () => {
@@ -724,6 +728,71 @@ describe("SkillMcpManager", () => {
      expect(headers?.Authorization).toBe("Bearer oauth-token")
    })

+    it("attempts silent refresh for expired stored tokens before login", async () => {
+      // given
+      const info: SkillMcpClientInfo = {
+        serverName: "oauth-refresh",
+        skillName: "oauth-skill",
+        sessionID: "session-oauth-refresh",
+      }
+      const config: ClaudeCodeMcpServer = {
+        url: "https://mcp.example.com/mcp",
+        oauth: {
+          clientId: "my-client",
+        },
+      }
+      mockTokens.mockReturnValue({
+        accessToken: "expired-token",
+        refreshToken: "refresh-token",
+        expiresAt: Math.floor(Date.now() / 1000) - 60,
+      })
+      mockRefresh.mockResolvedValue({ accessToken: "refreshed-token" })
+
+      // when
+      try {
+        await manager.getOrCreateClient(info, config)
+      } catch { /* connection fails in test */ }
+
+      // then
+      const headers = lastTransportInstance.options?.requestInit?.headers as Record<string, string> | undefined
+      expect(headers?.Authorization).toBe("Bearer refreshed-token")
+      expect(mockRefresh).toHaveBeenCalledWith("refresh-token")
+      expect(mockLogin).not.toHaveBeenCalled()
+    })
+
+    it("falls back to login when silent refresh fails", async () => {
+      // given
+      const info: SkillMcpClientInfo = {
+        serverName: "oauth-refresh-fallback",
+        skillName: "oauth-skill",
+        sessionID: "session-oauth-refresh-fallback",
+      }
+      const config: ClaudeCodeMcpServer = {
+        url: "https://mcp.example.com/mcp",
+        oauth: {
+          clientId: "my-client",
+        },
+      }
+      mockTokens.mockReturnValue({
+        accessToken: "expired-token",
+        refreshToken: "refresh-token",
+        expiresAt: Math.floor(Date.now() / 1000) - 60,
+      })
+      mockRefresh.mockRejectedValue(new Error("Refresh failed"))
+      mockLogin.mockResolvedValue({ accessToken: "login-token" })
+
+      // when
+      try {
+        await manager.getOrCreateClient(info, config)
+      } catch { /* connection fails in test */ }
+
+      // then
+      const headers = lastTransportInstance.options?.requestInit?.headers as Record<string, string> | undefined
+      expect(headers?.Authorization).toBe("Bearer login-token")
+      expect(mockRefresh).toHaveBeenCalledWith("refresh-token")
+      expect(mockLogin).toHaveBeenCalled()
+    })
+
    it("does not create auth provider when oauth config is absent", async () => {
      // given
      const info: SkillMcpClientInfo = {
@@ -44,7 +44,7 @@ export async function buildHttpRequestInit(
    const provider = getOrCreateAuthProvider(authProviders, config.url, config.oauth, createOAuthProvider)
    let tokenData = provider.tokens()

-    if (!tokenData || isTokenExpired(tokenData)) {
+    if (!tokenData) {
      try {
        tokenData = await provider.login()
      } catch {
@@ -52,6 +52,20 @@ export async function buildHttpRequestInit(
      }
    }

+    if (tokenData && isTokenExpired(tokenData)) {
+      try {
+        tokenData = tokenData.refreshToken
+          ? await provider.refresh(tokenData.refreshToken)
+          : await provider.login()
+      } catch {
+        try {
+          tokenData = await provider.login()
+        } catch {
+          tokenData = null
+        }
+      }
+    }
+
    if (tokenData) {
      headers.Authorization = `Bearer ${tokenData.accessToken}`
    }
@@ -50,7 +50,7 @@ export interface ProcessCleanupHandler {

 export type OAuthProviderLike = Pick<
  McpOAuthProvider,
-  "tokens" | "login"
+  "tokens" | "login" | "refresh"
 >

 export type OAuthProviderFactory = (options: {