Skip to content

Commit c701aff

Browse files
Stop MCP authorization recovery from looping reconnects (#857)
* Refresh OAuth tokens before browser re-auth and cap prompts Linear MCP servers periodically disconnect and every auth failure fell through to a browser re-auth prompt. The provider now performs a refresh_token grant against discovered authorization server metadata before any browser flow, and browser prompts are capped at three per server with a five minute cooldown, surfacing a clear error through the connect failure path instead of looping forever. * Gate MCP browser prompts at redirect and pass resource on refresh * Share MCP browser authentication episode state * Coordinate MCP authorization recovery per connection * Close leftover MCP authorization recovery races * Close MCP recovery abort and cap-clear races * Reformat MCP recovery completion callback * Treat only AbortError as a cancelled OAuth refresh * Clear MCP auth chrome when close aborts a verified retry * Cap MCP browser re-auth at one prompt and time out ignored login Reconnect during an in-flight browser wait was starting a new coordinator and prompting again. An ignored tab left connect pending until abort.
1 parent df0b168 commit c701aff

6 files changed

Lines changed: 1613 additions & 62 deletions

File tree

‎src/mcp/client-auth-policy.test.ts‎

Lines changed: 70 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ let blockTokenExchange = false;
1717
let tokenExchangeSignals: (AbortSignal | null | undefined)[] = [];
1818
let tokenExchangeAborts = 0;
1919
let lastTransportAuth: (() => Promise<void>) | undefined;
20+
let lastTransportRedirect: (() => Promise<void>) | undefined;
2021
let tokenRefreshSignals: (AbortSignal | null | undefined)[] = [];
2122
let tokenRefreshAborts = 0;
2223

@@ -35,15 +36,22 @@ function hangUntilAbort(
3536
});
3637
}
3738

38-
const authProvider = { resetAuthorization: async () => undefined };
39+
const redirectToAuthorization = () => undefined;
40+
const authProvider = {
41+
resetAuthorization: async () => undefined,
42+
redirectToAuthorization,
43+
};
3944

4045
await withMockedModule(
4146
import.meta.resolve("@modelcontextprotocol/sdk/client/index.js"),
4247
(real: typeof import("@modelcontextprotocol/sdk/client/index.js")) => ({
4348
...real,
4449
Client: class {
4550
async connect(): Promise<void> {
46-
if (clientConnectError !== undefined) throw clientConnectError;
51+
if (clientConnectError !== undefined) {
52+
if (clientConnectError instanceof UnauthorizedError) await lastTransportRedirect?.();
53+
throw clientConnectError;
54+
}
4755
}
4856
async listTools(
4957
_params?: unknown,
@@ -83,12 +91,18 @@ await withMockedModule(
8391
constructor(
8492
_url: URL,
8593
private readonly options?: {
94+
authProvider?: { redirectToAuthorization?: (url: URL) => void | Promise<void> };
8695
requestInit?: RequestInit;
8796
fetch?: (url: string | URL, init?: RequestInit) => Promise<Response>;
8897
},
8998
) {
9099
transportOptions.push(options);
91100
lastTransportAuth = () => this.auth();
101+
lastTransportRedirect = async () => {
102+
await this.options?.authProvider?.redirectToAuthorization?.(
103+
new URL("https://auth.test/authorize"),
104+
);
105+
};
92106
}
93107
async finishAuth(): Promise<void> {
94108
const signal = this.options?.requestInit?.signal;
@@ -174,8 +188,10 @@ describe("HTTP MCP auth policy", () => {
174188
tokenExchangeSignals = [];
175189
tokenExchangeAborts = 0;
176190
lastTransportAuth = undefined;
191+
lastTransportRedirect = undefined;
177192
tokenRefreshSignals = [];
178193
tokenRefreshAborts = 0;
194+
authProvider.redirectToAuthorization = redirectToAuthorization;
179195
});
180196

181197
test("built-in anonymous Exa treats 401 as a normal failure without OAuth machinery", async () => {
@@ -243,7 +259,8 @@ describe("HTTP MCP auth policy", () => {
243259
);
244260
while (tokenExchangeSignals.length === 0) await Promise.resolve();
245261

246-
expect(tokenExchangeSignals[0]).toBe(abort.signal);
262+
expect(tokenExchangeSignals[0]).toBeDefined();
263+
expect(tokenExchangeSignals[0]?.aborted).toBe(false);
247264
abort.abort(new Error("toolset disposed"));
248265
const result = await connection;
249266

@@ -264,12 +281,17 @@ describe("HTTP MCP auth policy", () => {
264281
expect(result.ok).toBe(true);
265282
if (!result.ok) return;
266283
expect(transportOptions).toEqual([
267-
{ authProvider, requestInit: { signal: abort.signal }, fetch: expect.any(Function) },
284+
{
285+
authProvider,
286+
requestInit: { signal: expect.any(AbortSignal) },
287+
fetch: expect.any(Function),
288+
},
268289
]);
269290

270291
const call = result.client.call("ping", {}, abort.signal);
271292
while (tokenRefreshSignals.length === 0) await Promise.resolve();
272-
expect(tokenRefreshSignals[0]).toBe(abort.signal);
293+
expect(tokenRefreshSignals[0]).toBeDefined();
294+
expect(tokenRefreshSignals[0]?.aborted).toBe(false);
273295
abort.abort(new Error("toolset disposed"));
274296
await expect(call).rejects.toThrow("toolset disposed");
275297
expect(tokenRefreshAborts).toBe(1);
@@ -294,7 +316,49 @@ describe("HTTP MCP auth policy", () => {
294316
expect(callbackStarts).toBe(1);
295317
expect(providerCreates).toBe(1);
296318
expect(providerServerURL).toBe("https://custom.example/mcp?mode=full");
297-
expect(transportOptions).toEqual([{ authProvider }]);
319+
expect(transportOptions).toEqual([
320+
{
321+
authProvider,
322+
requestInit: { signal: expect.any(AbortSignal) },
323+
fetch: expect.any(Function),
324+
},
325+
]);
326+
});
327+
328+
test("client close aborts in-flight OAuth without aborting the connect signal", async () => {
329+
const connectAbort = new AbortController();
330+
const result = await connectMCPServer(
331+
{ name: "linear", type: "http", url: "https://mcp.linear.app/mcp" },
332+
{ signal: connectAbort.signal },
333+
);
334+
expect(result.ok).toBe(true);
335+
if (!result.ok) return;
336+
337+
const transport = transportOptions[0] as {
338+
requestInit?: { signal?: AbortSignal };
339+
fetch?: (url: string | URL, init?: RequestInit) => Promise<Response>;
340+
};
341+
expect(transport.requestInit?.signal).toBeDefined();
342+
expect(transport.requestInit?.signal).not.toBe(connectAbort.signal);
343+
expect(transport.fetch).toBeTypeOf("function");
344+
345+
const call = result.client.call("ping", {}, new AbortController().signal);
346+
while (tokenRefreshSignals.length === 0) await Promise.resolve();
347+
expect(tokenRefreshSignals[0]).not.toBe(connectAbort.signal);
348+
expect(tokenRefreshSignals[0]?.aborted).toBe(false);
349+
expect(connectAbort.signal.aborted).toBe(false);
350+
351+
await result.client.close();
352+
353+
await expect(call).rejects.toThrow();
354+
expect(tokenRefreshAborts).toBe(1);
355+
expect(connectAbort.signal.aborted).toBe(false);
356+
expect(transport.requestInit?.signal?.aborted).toBe(true);
357+
const fetchFn = transport.fetch;
358+
expect(fetchFn).toBeTypeOf("function");
359+
if (fetchFn === undefined) return;
360+
await expect(fetchFn("https://auth.test/token")).rejects.toThrow();
361+
expect(connectAbort.signal.aborted).toBe(false);
298362
});
299363
});
300364

0 commit comments

Comments
 (0)