mcp: concurrent OAuth refreshes ignore individual caller cancellation #59109

Activity

1. opencode-agent commented on Oct 26, 2026

   

   opencode-agentBot

   on Oct 26, 2026 – with OpenCode Agent

   Contributor

   More actions

   Thanks for the clear report. I reproduced this from source on the latest dev (08ac673). The MCP OAuth code is unchanged since v2.1.4.

   I made two concurrent refresh requests through McpOAuth.loggedFetch with the same URL, client ID and refresh token. Each had its own AbortController, and the loopback token endpoint held its response until released. The endpoint was hit only once, as intended. Cancellation is not handled per caller:

   • Aborting only the second caller: both callers resolved with 200, so the cancelled caller still received the token.
   • Aborting only the first caller: both callers rejected with AbortError, so the caller that wasn't cancelled failed too.

   The cause is that send in packages/core/src/mcp/oauth.ts passes only the first caller's init.signal to the shared request. Later callers just get a clone of that shared promise. The script below reproduces both cases.

   Reproduction script (repro.ts)

   // Reproduces #59109: shared MCP OAuth refreshes ignore per-caller AbortSignals.
   // Run: REPO=/path/to/opencode bun /workspace/repro/repro.ts
   const REPO = process.env.REPO ?? "/workspace/repo"
   const { McpOAuth } = await import(`${REPO}/packages/core/src/mcp/oauth.ts`)
   const { Effect } = await import(`${REPO}/packages/core/node_modules/effect/dist/index.js`).catch(
     () => import(`${REPO}/node_modules/effect/dist/index.js`),
   )
   
   let hits = 0
   let release: () => void = () => {}
   const server = Bun.serve({
     port: 0,
     async fetch() {
       hits++
       await new Promise<void>((r) => (release = r))
       return Response.json({ access_token: "new", token_type: "Bearer", refresh_token: "rotated" })
     },
   })
   const url = `http://127.0.0.1:${server.port}/token`
   const fetcher = await Effect.runPromise(McpOAuth.loggedFetch({ server: "repro" }))
   
   async function scenario(name: string, abortIndex: 0 | 1, token: string) {
     hits = 0
     const controllers = [new AbortController(), new AbortController()]
     const body = () => new URLSearchParams({ grant_type: "refresh_token", client_id: "c", refresh_token: token })
     const calls = controllers.map((c) =>
       fetcher(url, { method: "POST", body: body(), signal: c.signal }).then(
         async (r: Response) => `resolved ${r.status} ${await r.text()}`,
         (e: unknown) => `rejected (${(e as Error)?.name ?? e})`,
       ),
     )
     await Bun.sleep(100)
     controllers[abortIndex].abort()
     await Bun.sleep(50)
     release()
     const [a, b] = await Promise.all(calls)
     console.log(`\n${name}  (endpoint hits: ${hits})`)
     console.log(`  caller 1 ${abortIndex === 0 ? "(aborted)" : "(not aborted)"}: ${a}`)
     console.log(`  caller 2 ${abortIndex === 1 ? "(aborted)" : "(not aborted)"}: ${b}`)
   }
   
   await scenario("Scenario 1: abort only caller 2", 1, "t1")
   await scenario("Scenario 2: abort only caller 1", 0, "t2")
   server.stop(true)
   process.exit(0)

   Run it: bun repro.ts
2. opencode-agent

   added

   reproduced

   on Oct 26, 2026
3. r266-tech commented on Nov 1, 2026

   

   r266-tech

   on Nov 1, 2026

   Contributor

   More actions

   I can take this issue if it is not already assigned.

   I confirmed the current dev implementation still passes only the first caller's refresh signal to the shared request: aborting a follower does nothing, while aborting the first caller rejects every follower. I have a focused two-file fix ready in a worktree that keeps one underlying refresh request, removes its signal from the shared transport, and races each caller's returned clone against that caller's own AbortSignal.

   The regression covers both abort directions with one loopback token endpoint. cd packages/core && bun run script/test.ts test/mcp-oauth.test.ts --rerun-each 3 passed (69 tests per run), bun typecheck passed, and git diff --check passed. The change is limited to packages/core/src/mcp/oauth.ts and its test.

   This is separate from the cross-location rotating-token issue in #60612. Please confirm that this caller-scoped cancellation fix fits the intended boundary before I open a PR.
4. r266-tech commented on Nov 9, 2026

   

   r266-tech

   on Nov 9, 2026

   Contributor

   More actions

   Follow-up after #62827, at dev 12a661ea40 (Bun 1.4.4, MCP SDK 2.3.1):

   The direct loggedFetch reproducer still shows the signal coupling, but SDK refreshAuthorization/executeTokenRequest do not pass a caller init.signal into the token fetch.

   I checked the real McpClient.connect path with the token endpoint held open. Interrupting the caller settles before its response is released; the shared refresh then persists the rotated tokens. A second SDK refresh with the old token receives the saved result, with one token request. Unmodified dev and a caller-signal patch behave identically, consistent with #59114's withdrawal.

   I'm withdrawing my earlier proposal to fix this only inside loggedFetch; it needs a failing reproduction through the affected MCP entry point first. Loopback fixtures only; no live providers. AI-assisted investigation.