Skip to content

Commit 7b9ed3d

Browse files
TheGreatAxiosTheGreatAxios
committed
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 a31c01c commit 7b9ed3d

2 files changed

Lines changed: 100 additions & 57 deletions

File tree

‎src/mcp/client-auth-reauth-cap.test.ts‎

Lines changed: 69 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,7 @@ await withMockedModule(
231231
const {
232232
connectMCPServer,
233233
resetBrowserAuthState,
234+
setBrowserAuthWaitMs,
234235
MAX_BROWSER_AUTH_ATTEMPTS,
235236
BROWSER_AUTH_COOLDOWN_MS,
236237
} = await import("./client.js");
@@ -452,13 +453,11 @@ describe("HTTP MCP re-auth loop prevention", () => {
452453
expect(connected.ok).toBe(true);
453454
if (!connected.ok) return;
454455

455-
for (let episode = 0; episode < 2; episode += 1) {
456-
callFailuresLeft = 1;
457-
await expect(connected.client.call("ping", {}, new AbortController().signal)).rejects.toThrow(
458-
"finishAuth exploded",
459-
);
460-
}
461-
expect(authURLCount).toBe(2);
456+
callFailuresLeft = 1;
457+
await expect(connected.client.call("ping", {}, new AbortController().signal)).rejects.toThrow(
458+
"finishAuth exploded",
459+
);
460+
expect(authURLCount).toBe(1);
462461
expect(authorizedCount).toBe(0);
463462

464463
refreshSucceeds = true;
@@ -467,7 +466,7 @@ describe("HTTP MCP re-auth loop prevention", () => {
467466
callFailuresLeft = 1;
468467
await expect(connected.client.call("ping", {}, new AbortController().signal)).resolves.toBe("");
469468
expect(authorizedCount).toBe(1);
470-
expect(authURLCount).toBe(2);
469+
expect(authURLCount).toBe(1);
471470

472471
refreshSucceeds = false;
473472
finishAuthError = new Error("finishAuth exploded");
@@ -478,13 +477,13 @@ describe("HTTP MCP re-auth loop prevention", () => {
478477
"finishAuth exploded",
479478
);
480479
}
481-
expect(authURLCount).toBe(2 + MAX_BROWSER_AUTH_ATTEMPTS);
480+
expect(authURLCount).toBe(1 + MAX_BROWSER_AUTH_ATTEMPTS);
482481

483482
callFailuresLeft = 1;
484483
await expect(connected.client.call("ping", {}, new AbortController().signal)).rejects.toThrow(
485484
"retrying paused",
486485
);
487-
expect(authURLCount).toBe(2 + MAX_BROWSER_AUTH_ATTEMPTS);
486+
expect(authURLCount).toBe(1 + MAX_BROWSER_AUTH_ATTEMPTS);
488487
});
489488

490489
test("client close aborts the shared callback waiter", async () => {
@@ -559,22 +558,20 @@ describe("HTTP MCP re-auth loop prevention", () => {
559558
const second = connected.client.call("second", {}, new AbortController().signal);
560559
while (callToolCalls < 4) await Promise.resolve();
561560

562-
callbackGate = new Promise((resolve) => {
563-
releaseCallback = resolve;
564-
});
561+
retryGate = undefined;
562+
refreshSucceeds = true;
563+
redirectsPerFailure = 0;
565564
callFailuresLeft = 1;
566565
const third = connected.client.call("third", {}, new AbortController().signal);
567-
while (waitForCodeCalls < 2) await Promise.resolve();
566+
await expect(third).resolves.toBe("");
567+
expect(authorizedCount).toBe(1);
568+
expect(authURLCount).toBe(1);
568569

569570
releaseRetry?.();
570571
await expect(first).resolves.toBe("");
571572
await expect(second).resolves.toBe("");
572-
expect(authorizedCount).toBe(0);
573-
574-
releaseCallback?.();
575-
await expect(third).resolves.toBe("");
576573
expect(authorizedCount).toBe(1);
577-
expect(authURLCount).toBe(2);
574+
expect(waitForCodeCalls).toBe(1);
578575
});
579576

580577
test("does not repeat the SDK refresh after redirecting to authorization", async () => {
@@ -590,7 +587,29 @@ describe("HTTP MCP re-auth loop prevention", () => {
590587
expect(authURLCount).toBe(1);
591588
});
592589

593-
test("live-call auth episodes share redirect state and stop after three prompts", async () => {
590+
test("reconnect during in-flight waitForCode does not emit a second prompt", async () => {
591+
connectFailuresLeft = Number.POSITIVE_INFINITY;
592+
callbackGate = new Promise(() => undefined);
593+
const firstAbort = new AbortController();
594+
const first = connectMCPServer(config, {
595+
onAuthURL: () => (authURLCount += 1),
596+
signal: firstAbort.signal,
597+
});
598+
while (authURLCount === 0 || waitForCodeCalls === 0) await Promise.resolve();
599+
expect(authURLCount).toBe(1);
600+
601+
const second = await connectWithAuthPrompt();
602+
603+
expect(second.ok).toBe(false);
604+
expect(second.error).toContain("retrying paused");
605+
expect(authURLCount).toBe(1);
606+
expect(waitForCodeCalls).toBe(1);
607+
608+
firstAbort.abort();
609+
await expect(first).resolves.toMatchObject({ ok: false });
610+
});
611+
612+
test("live-call auth episodes share redirect state and stop after one prompt", async () => {
594613
const connected = await connectMCPServer(config, {
595614
onAuthURL: () => {
596615
authURLCount += 1;
@@ -650,7 +669,7 @@ describe("HTTP MCP re-auth loop prevention", () => {
650669
const result = await connectWithAuthPrompt();
651670
expect(result.ok).toBe(false);
652671
expect(result.error).toContain(
653-
`MCP authorization for linear failed after ${MAX_BROWSER_AUTH_ATTEMPTS} attempts`,
672+
`MCP authorization for linear failed after ${MAX_BROWSER_AUTH_ATTEMPTS} ${MAX_BROWSER_AUTH_ATTEMPTS === 1 ? "attempt" : "attempts"}`,
654673
);
655674
expect(result.error).toContain("retrying paused for 5 minutes");
656675
expect(result.error).toContain("Retry later after the cooldown");
@@ -663,19 +682,11 @@ describe("HTTP MCP re-auth loop prevention", () => {
663682
});
664683

665684
test("successful interactive auth clears the cap so a later failure can prompt", async () => {
666-
connectFailuresLeft = Number.POSITIVE_INFINITY;
667-
for (let episode = 0; episode < 2; episode += 1) {
668-
const result = await connectWithAuthPrompt();
669-
expect(result.ok).toBe(false);
670-
expect(result.error).toContain("finishAuth exploded");
671-
}
672-
expect(authURLCount).toBe(2);
673-
674685
finishAuthError = undefined;
675686
connectFailuresLeft = 1;
676687
const recovered = await connectWithAuthPrompt();
677688
expect(recovered.ok).toBe(true);
678-
expect(authURLCount).toBe(3);
689+
expect(authURLCount).toBe(1);
679690

680691
finishAuthError = new Error("finishAuth exploded");
681692
connectFailuresLeft = Number.POSITIVE_INFINITY;
@@ -684,15 +695,15 @@ describe("HTTP MCP re-auth loop prevention", () => {
684695
expect(result.ok).toBe(false);
685696
expect(result.error).toContain("finishAuth exploded");
686697
}
687-
expect(authURLCount).toBe(3 + MAX_BROWSER_AUTH_ATTEMPTS);
698+
expect(authURLCount).toBe(1 + MAX_BROWSER_AUTH_ATTEMPTS);
688699

689700
const capped = await connectWithAuthPrompt();
690701
expect(capped.ok).toBe(false);
691702
expect(capped.error).toContain("retrying paused");
692-
expect(authURLCount).toBe(3 + MAX_BROWSER_AUTH_ATTEMPTS);
703+
expect(authURLCount).toBe(1 + MAX_BROWSER_AUTH_ATTEMPTS);
693704
});
694705

695-
test("prompts resume five minutes after the third failed episode", async () => {
706+
test("prompts resume five minutes after the capped failed episode", async () => {
696707
const thirdEpisodeAt = new Date("2026-01-01T00:00:00Z").getTime();
697708
setSystemTime(thirdEpisodeAt);
698709
connectFailuresLeft = Number.POSITIVE_INFINITY;
@@ -847,20 +858,11 @@ describe("HTTP MCP re-auth loop prevention", () => {
847858

848859
finishAuthError = new Error("finishAuth exploded");
849860
callRedirectsLeft = Number.POSITIVE_INFINITY;
850-
for (let episode = 0; episode < 2; episode += 1) {
851-
callFailuresLeft = 1;
852-
await expect(connected.client.call("ping", {}, new AbortController().signal)).rejects.toThrow(
853-
"finishAuth exploded",
854-
);
855-
}
856-
expect(authURLCount).toBe(3);
857-
expect(authorizedCount).toBe(0);
858-
859861
callFailuresLeft = 1;
860862
await expect(connected.client.call("ping", {}, new AbortController().signal)).rejects.toThrow(
861863
"retrying paused",
862864
);
863-
expect(authURLCount).toBe(3);
865+
expect(authURLCount).toBe(1);
864866
expect(authorizedCount).toBe(0);
865867
});
866868

@@ -877,33 +879,46 @@ describe("HTTP MCP re-auth loop prevention", () => {
877879
expect(connected.ok).toBe(true);
878880
if (!connected.ok) return;
879881

880-
callFailuresLeft = 2;
881-
callRedirectsLeft = 1;
882-
await expect(connected.client.call("ping", {}, new AbortController().signal)).rejects.toThrow(
883-
"authorization required",
884-
);
885-
expect(authorizedCount).toBe(0);
886-
887-
callRedirectsLeft = Number.POSITIVE_INFINITY;
888882
callFailuresLeft = 1;
883+
callRedirectsLeft = 1;
889884
await expect(connected.client.call("ping", {}, new AbortController().signal)).resolves.toBe("");
890885
expect(authorizedCount).toBe(1);
891-
expect(authURLCount).toBe(2);
886+
expect(authURLCount).toBe(1);
892887

893888
finishAuthError = new Error("finishAuth exploded");
889+
callRedirectsLeft = Number.POSITIVE_INFINITY;
894890
for (let episode = 0; episode < MAX_BROWSER_AUTH_ATTEMPTS; episode += 1) {
895891
callFailuresLeft = 1;
896892
await expect(connected.client.call("ping", {}, new AbortController().signal)).rejects.toThrow(
897893
"finishAuth exploded",
898894
);
899895
}
900-
expect(authURLCount).toBe(2 + MAX_BROWSER_AUTH_ATTEMPTS);
896+
expect(authURLCount).toBe(1 + MAX_BROWSER_AUTH_ATTEMPTS);
901897

902898
callFailuresLeft = 1;
903899
await expect(connected.client.call("ping", {}, new AbortController().signal)).rejects.toThrow(
904900
"retrying paused",
905901
);
906-
expect(authURLCount).toBe(2 + MAX_BROWSER_AUTH_ATTEMPTS);
902+
expect(authURLCount).toBe(1 + MAX_BROWSER_AUTH_ATTEMPTS);
907903
expect(authorizedCount).toBe(1);
908904
});
905+
906+
test("ignored browser wait times out as disconnected and still counts the prompt", async () => {
907+
setBrowserAuthWaitMs(50);
908+
connectFailuresLeft = Number.POSITIVE_INFINITY;
909+
callbackGate = new Promise(() => undefined);
910+
911+
const result = await connectWithAuthPrompt();
912+
913+
expect(result.ok).toBe(false);
914+
expect(result.error).toContain("timed out waiting for the browser");
915+
expect(result.error).toContain("disconnected");
916+
expect(authURLCount).toBe(1);
917+
expect(waitForCodeCalls).toBe(1);
918+
919+
const capped = await connectWithAuthPrompt();
920+
expect(capped.ok).toBe(false);
921+
expect(capped.error).toContain("retrying paused");
922+
expect(authURLCount).toBe(1);
923+
});
909924
});

‎src/mcp/client.ts‎

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,9 @@ function isAbortError(err: unknown): boolean {
9595
return typeof err === "object" && err !== null && "name" in err && err.name === "AbortError";
9696
}
9797

98-
export const MAX_BROWSER_AUTH_ATTEMPTS = 3;
98+
export const MAX_BROWSER_AUTH_ATTEMPTS = 1;
9999
export const BROWSER_AUTH_COOLDOWN_MS = 5 * 60_000;
100+
export const BROWSER_AUTH_WAIT_MS = 2 * 60_000;
100101

101102
interface BrowserAuthAttempts {
102103
count: number;
@@ -105,19 +106,46 @@ interface BrowserAuthAttempts {
105106
// Keyed by server identity, not provider instance, so the cap survives the
106107
// provider re-creation that every reconnect performs.
107108
const browserAuthAttempts = new Map<string, BrowserAuthAttempts>();
109+
let browserAuthWaitMs = BROWSER_AUTH_WAIT_MS;
108110

109111
export function resetBrowserAuthState(): void {
110112
browserAuthAttempts.clear();
113+
browserAuthWaitMs = BROWSER_AUTH_WAIT_MS;
114+
}
115+
116+
export function setBrowserAuthWaitMs(ms: number): void {
117+
browserAuthWaitMs = ms;
111118
}
112119

113120
function browserAuthCapError(serverName: string): Error {
114121
const minutes = Math.round(BROWSER_AUTH_COOLDOWN_MS / 60_000);
122+
const attempts =
123+
MAX_BROWSER_AUTH_ATTEMPTS === 1 ? "1 attempt" : `${String(MAX_BROWSER_AUTH_ATTEMPTS)} attempts`;
115124
return new Error(
116-
`MCP authorization for ${serverName} failed after ${MAX_BROWSER_AUTH_ATTEMPTS} attempts; ` +
125+
`MCP authorization for ${serverName} failed after ${attempts}; ` +
117126
`retrying paused for ${minutes} minutes. Retry later after the cooldown.`,
118127
);
119128
}
120129

130+
function browserAuthWaitError(serverName: string): Error {
131+
return new Error(
132+
`MCP authorization for ${serverName} timed out waiting for the browser; ` +
133+
`the server is disconnected. Retry later after the cooldown.`,
134+
);
135+
}
136+
137+
async function waitForBrowserAuthCode(context: HTTPAuthContext): Promise<string> {
138+
const lifecycle = context.coordinator.lifecycle.signal;
139+
const deadline = AbortSignal.any([lifecycle, AbortSignal.timeout(browserAuthWaitMs)]);
140+
try {
141+
return await context.callback.waitForCode(deadline);
142+
} catch (err) {
143+
if (lifecycle.aborted) throw err;
144+
if (deadline.aborted) throw browserAuthWaitError(context.serverName);
145+
throw err;
146+
}
147+
}
148+
121149
function browserAuthKey(context: HTTPAuthContext): string {
122150
return `${context.serverName}|${context.url.toString()}`;
123151
}
@@ -294,7 +322,7 @@ async function driveRecovery(err: UnauthorizedError | OAuthError, context: HTTPA
294322

295323
const browserFlow = coordinator.browserFlow;
296324
await browserFlow.promptEmitted;
297-
const code = await context.callback.waitForCode(coordinator.lifecycle.signal);
325+
const code = await waitForBrowserAuthCode(context);
298326
await new StreamableHTTPClientTransport(
299327
context.url,
300328
streamableHTTPTransportOptions(context.authProvider, coordinator.lifecycle.signal),

0 commit comments

Comments
 (0)