Skip to content

Commit 315f2c0

Browse files
committed
Close MCP recovery abort and cap-clear races
1 parent c08dc62 commit 315f2c0

4 files changed

Lines changed: 230 additions & 35 deletions

File tree

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

Lines changed: 161 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ let refreshGate: Promise<void> | undefined;
3030
let releaseRefresh: (() => void) | undefined;
3131
let callbackGate: Promise<void> | undefined;
3232
let releaseCallback: (() => void) | undefined;
33+
let lastRequestSignal: AbortSignal | undefined;
34+
let retryGate: Promise<void> | undefined;
35+
let releaseRetry: (() => void) | undefined;
3336
let saveGate: Promise<void> | undefined;
3437
let releaseSave: (() => void) | undefined;
3538
interface MockAuthProvider {
@@ -45,7 +48,7 @@ const fakeProvider = {
4548
refreshToken: async () => {
4649
refreshCalls += 1;
4750
authEvents.push("refresh");
48-
await refreshGate;
51+
await waitForOptionalGate(refreshGate, lastRequestSignal);
4952
if (!refreshSucceeds) throw new UnauthorizedError("refresh rejected");
5053
return { access_token: "fresh", refresh_token: "refresh-me" };
5154
},
@@ -98,22 +101,30 @@ async function emitRedirects(provider: MockAuthProvider | undefined): Promise<vo
98101
}
99102
}
100103

101-
function waitForGate(signal: AbortSignal): Promise<void> {
102-
if (callbackGate === undefined) return Promise.resolve();
104+
function waitForOptionalGate(
105+
gate: Promise<void> | undefined,
106+
signal: AbortSignal | undefined,
107+
): Promise<void> {
108+
if (gate === undefined) return Promise.resolve();
109+
if (signal === undefined) return gate;
103110
if (signal.aborted) return Promise.reject(signal.reason);
104111
return new Promise((resolve, reject) => {
105112
const onAbort = () => {
106113
signal.removeEventListener("abort", onAbort);
107114
reject(signal.reason);
108115
};
109116
signal.addEventListener("abort", onAbort, { once: true });
110-
void callbackGate?.then(() => {
117+
void gate.then(() => {
111118
signal.removeEventListener("abort", onAbort);
112119
resolve();
113120
});
114121
});
115122
}
116123

124+
function waitForGate(signal: AbortSignal): Promise<void> {
125+
return waitForOptionalGate(callbackGate, signal);
126+
}
127+
117128
await withMockedModule(
118129
import.meta.resolve("@modelcontextprotocol/sdk/client/index.js"),
119130
(real: typeof import("@modelcontextprotocol/sdk/client/index.js")) => ({
@@ -147,6 +158,7 @@ await withMockedModule(
147158
}
148159
throw new UnauthorizedError("authorization required");
149160
}
161+
await retryGate;
150162
return { content: [] };
151163
}
152164
async close(): Promise<void> {}
@@ -160,8 +172,13 @@ await withMockedModule(
160172
...real,
161173
StreamableHTTPClientTransport: class {
162174
provider?: MockAuthProvider;
163-
constructor(_url: URL, options?: { authProvider?: MockAuthProvider }) {
175+
constructor(
176+
_url: URL,
177+
options?: { authProvider?: MockAuthProvider; requestInit?: RequestInit },
178+
) {
164179
if (options?.authProvider !== undefined) this.provider = options.authProvider;
180+
const signal = options?.requestInit?.signal;
181+
if (signal !== undefined && signal !== null) lastRequestSignal = signal;
165182
}
166183
async finishAuth(): Promise<void> {
167184
finishAuthCalls += 1;
@@ -263,6 +280,9 @@ describe("HTTP MCP re-auth loop prevention", () => {
263280
releaseCallback = undefined;
264281
saveGate = undefined;
265282
releaseSave = undefined;
283+
lastRequestSignal = undefined;
284+
retryGate = undefined;
285+
releaseRetry = undefined;
266286
resetBrowserAuthState();
267287
setSystemTime();
268288
});
@@ -383,6 +403,90 @@ describe("HTTP MCP re-auth loop prevention", () => {
383403
expect(authURLCount).toBe(1);
384404
});
385405

406+
test("aborted waiter still fires onAuthorized when background finishAuth succeeds", async () => {
407+
finishAuthError = undefined;
408+
callbackGate = new Promise((resolve) => {
409+
releaseCallback = resolve;
410+
});
411+
const connected = await connectMCPServer(config, {
412+
onAuthURL: () => (authURLCount += 1),
413+
onAuthorized: () => (authorizedCount += 1),
414+
});
415+
expect(connected.ok).toBe(true);
416+
if (!connected.ok) return;
417+
callFailuresLeft = 1;
418+
const abort = new AbortController();
419+
const call = connected.client.call("ping", {}, abort.signal);
420+
while (authURLCount === 0 || waitForCodeCalls === 0) await Promise.resolve();
421+
422+
abort.abort(new Error("caller stopped"));
423+
await expect(call).rejects.toThrow("caller stopped");
424+
expect(authorizedCount).toBe(0);
425+
426+
releaseCallback?.();
427+
while (finishAuthCalls === 0) await Promise.resolve();
428+
for (let tick = 0; tick < 20 && authorizedCount === 0; tick += 1) await Promise.resolve();
429+
expect(authorizedCount).toBe(1);
430+
expect(finishAuthCalls).toBe(1);
431+
432+
finishAuthError = new Error("finishAuth exploded");
433+
for (let episode = 0; episode < MAX_BROWSER_AUTH_ATTEMPTS; episode += 1) {
434+
callFailuresLeft = 1;
435+
await expect(connected.client.call("ping", {}, new AbortController().signal)).rejects.toThrow(
436+
"finishAuth exploded",
437+
);
438+
}
439+
expect(authURLCount).toBe(1 + MAX_BROWSER_AUTH_ATTEMPTS);
440+
callFailuresLeft = 1;
441+
await expect(connected.client.call("ping", {}, new AbortController().signal)).rejects.toThrow(
442+
"retrying paused",
443+
);
444+
expect(authURLCount).toBe(1 + MAX_BROWSER_AUTH_ATTEMPTS);
445+
});
446+
447+
test("refresh-only recovery clears prior browser-cap counts", async () => {
448+
const connected = await connectMCPServer(config, {
449+
onAuthURL: () => (authURLCount += 1),
450+
onAuthorized: () => (authorizedCount += 1),
451+
});
452+
expect(connected.ok).toBe(true);
453+
if (!connected.ok) return;
454+
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);
462+
expect(authorizedCount).toBe(0);
463+
464+
refreshSucceeds = true;
465+
finishAuthError = undefined;
466+
redirectsPerFailure = 0;
467+
callFailuresLeft = 1;
468+
await expect(connected.client.call("ping", {}, new AbortController().signal)).resolves.toBe("");
469+
expect(authorizedCount).toBe(1);
470+
expect(authURLCount).toBe(2);
471+
472+
refreshSucceeds = false;
473+
finishAuthError = new Error("finishAuth exploded");
474+
redirectsPerFailure = 1;
475+
for (let episode = 0; episode < MAX_BROWSER_AUTH_ATTEMPTS; episode += 1) {
476+
callFailuresLeft = 1;
477+
await expect(connected.client.call("ping", {}, new AbortController().signal)).rejects.toThrow(
478+
"finishAuth exploded",
479+
);
480+
}
481+
expect(authURLCount).toBe(2 + MAX_BROWSER_AUTH_ATTEMPTS);
482+
483+
callFailuresLeft = 1;
484+
await expect(connected.client.call("ping", {}, new AbortController().signal)).rejects.toThrow(
485+
"retrying paused",
486+
);
487+
expect(authURLCount).toBe(2 + MAX_BROWSER_AUTH_ATTEMPTS);
488+
});
489+
386490
test("client close aborts the shared callback waiter", async () => {
387491
finishAuthError = undefined;
388492
callbackGate = new Promise((resolve) => {
@@ -401,6 +505,57 @@ describe("HTTP MCP re-auth loop prevention", () => {
401505
expect(finishAuthCalls).toBe(0);
402506
});
403507

508+
test("client close during hung refresh does not emit a browser prompt", async () => {
509+
const connected = await connectMCPServer(config, { onAuthURL: () => (authURLCount += 1) });
510+
expect(connected.ok).toBe(true);
511+
if (!connected.ok) return;
512+
refreshGate = new Promise(() => undefined);
513+
redirectsPerFailure = 0;
514+
callFailuresLeft = 1;
515+
const call = connected.client.call("ping", {}, new AbortController().signal);
516+
while (refreshCalls === 0) await Promise.resolve();
517+
518+
await connected.client.close();
519+
520+
await expect(call).rejects.toHaveProperty("name", "AbortError");
521+
expect(authURLCount).toBe(0);
522+
expect(waitForCodeCalls).toBe(0);
523+
});
524+
525+
test("late retry success from a prior recovery does not fire onAuthorized after a new recovery starts", async () => {
526+
finishAuthError = undefined;
527+
const connected = await connectMCPServer(config, {
528+
onAuthURL: () => (authURLCount += 1),
529+
onAuthorized: () => (authorizedCount += 1),
530+
});
531+
expect(connected.ok).toBe(true);
532+
if (!connected.ok) return;
533+
retryGate = new Promise((resolve) => {
534+
releaseRetry = resolve;
535+
});
536+
callFailuresLeft = 2;
537+
const first = connected.client.call("first", {}, new AbortController().signal);
538+
const second = connected.client.call("second", {}, new AbortController().signal);
539+
while (callToolCalls < 4) await Promise.resolve();
540+
541+
callbackGate = new Promise((resolve) => {
542+
releaseCallback = resolve;
543+
});
544+
callFailuresLeft = 1;
545+
const third = connected.client.call("third", {}, new AbortController().signal);
546+
while (waitForCodeCalls < 2) await Promise.resolve();
547+
548+
releaseRetry?.();
549+
await expect(first).resolves.toBe("");
550+
await expect(second).resolves.toBe("");
551+
expect(authorizedCount).toBe(0);
552+
553+
releaseCallback?.();
554+
await expect(third).resolves.toBe("");
555+
expect(authorizedCount).toBe(1);
556+
expect(authURLCount).toBe(2);
557+
});
558+
404559
test("does not repeat the SDK refresh after redirecting to authorization", async () => {
405560
finishAuthError = undefined;
406561
connectFailuresLeft = 1;
@@ -516,7 +671,7 @@ describe("HTTP MCP re-auth loop prevention", () => {
516671
expect(authURLCount).toBe(3 + MAX_BROWSER_AUTH_ATTEMPTS);
517672
});
518673

519-
test("prompts resume five minutes after the third failed episode without a fourth probe", async () => {
674+
test("prompts resume five minutes after the third failed episode", async () => {
520675
const thirdEpisodeAt = new Date("2026-01-01T00:00:00Z").getTime();
521676
setSystemTime(thirdEpisodeAt);
522677
connectFailuresLeft = Number.POSITIVE_INFINITY;

0 commit comments

Comments
 (0)