From ecf550c88cdadce1f8bb4bd6bb47a09c9899a867 Mon Sep 17 00:00:00 2001 From: Aiden Cline Date: Sat, 13 Jun 2026 11:28:54 -0500 Subject: [PATCH] fix(mcp): cancel timed out oauth refresh --- packages/opencode/src/mcp/index.ts | 8 ++++++-- packages/opencode/src/mcp/oauth-provider.ts | 9 ++++++--- packages/opencode/test/mcp/lifecycle.test.ts | 18 +++++++++++++++--- 3 files changed, 27 insertions(+), 8 deletions(-) diff --git a/packages/opencode/src/mcp/index.ts b/packages/opencode/src/mcp/index.ts index 7a838c9754..08ea8d3fa7 100644 --- a/packages/opencode/src/mcp/index.ts +++ b/packages/opencode/src/mcp/index.ts @@ -230,11 +230,15 @@ export const layer = Layer.effect( auth, ) authProvider = provider + const controller = new AbortController() yield* Effect.tryPromise(() => withTimeout( - provider.refreshTokensIfExpired(mcp.headers ? createFetchWithInit(fetch, { headers: mcp.headers }) : undefined), + provider.refreshTokensIfExpired( + mcp.headers ? createFetchWithInit(fetch, { headers: mcp.headers }) : undefined, + controller.signal, + ), connectTimeout, - ), + ).finally(() => controller.abort()), ).pipe(Effect.ignore) } diff --git a/packages/opencode/src/mcp/oauth-provider.ts b/packages/opencode/src/mcp/oauth-provider.ts index da7f02ea67..9c4bcf3a02 100644 --- a/packages/opencode/src/mcp/oauth-provider.ts +++ b/packages/opencode/src/mcp/oauth-provider.ts @@ -128,7 +128,7 @@ export class McpOAuthProvider implements OAuthClientProvider { ) } - async refreshTokensIfExpired(fetchFn?: FetchLike): Promise { + async refreshTokensIfExpired(fetchFn?: FetchLike, signal?: AbortSignal): Promise { const entry = await Effect.runPromise(this.auth.getForUrl(this.mcpName, this.serverUrl)) if (!entry?.tokens?.refreshToken) return false if (!entry.tokens.expiresAt) return false @@ -137,14 +137,17 @@ export class McpOAuthProvider implements OAuthClientProvider { const clientInformation = await this.clientInformation() if (!clientInformation) return false - const info = await discoverOAuthServerInfo(this.serverUrl, { fetchFn }) + const request = signal + ? (url: string | URL, init?: RequestInit) => (fetchFn ?? fetch)(url, { ...init, signal }) + : fetchFn + const info = await discoverOAuthServerInfo(this.serverUrl, { fetchFn: request }) await this.saveTokens( await refreshAuthorization(info.authorizationServerUrl, { metadata: info.authorizationServerMetadata, clientInformation, refreshToken: entry.tokens.refreshToken, resource: await selectResourceURL(this.serverUrl, this, info.resourceMetadata), - fetchFn, + fetchFn: request, }), ) return true diff --git a/packages/opencode/test/mcp/lifecycle.test.ts b/packages/opencode/test/mcp/lifecycle.test.ts index a111e13632..a7777f71d8 100644 --- a/packages/opencode/test/mcp/lifecycle.test.ts +++ b/packages/opencode/test/mcp/lifecycle.test.ts @@ -54,6 +54,7 @@ let transportCloseCount = 0 // Captures the opts passed to each MockStdioTransport, keyed by lastCreatedClientName const stdioOptsByName = new Map() let refreshAuthorizationCalls = 0 +let refreshAborted = false function getOrCreateClientState(name?: string): MockClientState { const key = name ?? "default" @@ -253,6 +254,7 @@ beforeEach(() => { clientCreateCount = 0 transportCloseCount = 0 refreshAuthorizationCalls = 0 + refreshAborted = false }) // Import after mocks @@ -307,14 +309,23 @@ it.live("McpOAuthProvider refreshes expired stored tokens", () => ) it.instance( - "remote connect bounds expired token refresh by mcp timeout", + "remote connect cancels expired token refresh after mcp timeout", () => Effect.acquireUseRelease( Effect.sync(() => { const original = McpOAuthProvider.prototype.refreshTokensIfExpired - McpOAuthProvider.prototype.refreshTokensIfExpired = () => { + McpOAuthProvider.prototype.refreshTokensIfExpired = (_fetchFn, signal) => { refreshAuthorizationCalls++ - return new Promise(() => {}) + return new Promise((_, reject) => { + signal?.addEventListener( + "abort", + () => { + refreshAborted = true + reject(signal.reason) + }, + { once: true }, + ) + }) } return original }), @@ -331,6 +342,7 @@ it.instance( expect(statusName(result.status, "remote-timeout")).toBe("connected") expect(refreshAuthorizationCalls).toBe(1) + expect(refreshAborted).toBe(true) }), ), (original) =>