Skip to content

Commit 25c5172

Browse files
CL-8990: fix MCP HTTP auth recovery ignoring abort on callBlocks (#1174)
* test(mcp): cover abort during HTTP auth recovery on callBlocks * fix(mcp): pass abort signal through callBlocks auth recovery
1 parent aac1b61 commit 25c5172

3 files changed

Lines changed: 125 additions & 6 deletions

File tree

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

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -465,6 +465,48 @@ describe("HTTP MCP re-auth loop prevention", () => {
465465
expect(authURLCount).toBe(1);
466466
});
467467

468+
test("block-path caller abort does not cancel shared recovery for another call", async () => {
469+
finishAuthError = undefined;
470+
callbackGate = new Promise((resolve) => {
471+
releaseCallback = resolve;
472+
});
473+
const connected = await connectMCPServer(config, {
474+
onAuthURL: () => (authURLCount += 1),
475+
});
476+
expect(connected.ok).toBe(true);
477+
if (!connected.ok) return;
478+
callFailuresLeft = 2;
479+
const callBlocks = connected.client.callBlocks;
480+
expect(callBlocks).toBeDefined();
481+
if (callBlocks === undefined) return;
482+
const firstAbort = new AbortController();
483+
const first = callBlocks("first", {}, firstAbort.signal);
484+
const second = callBlocks("second", {}, new AbortController().signal);
485+
while (waitForCodeCalls === 0) await Promise.resolve();
486+
487+
let abortTimer: ReturnType<typeof setTimeout> | undefined;
488+
const abortTimeout = new Promise<never>((_, reject) => {
489+
abortTimer = setTimeout(
490+
() => reject(new Error("timed out waiting for caller abort")),
491+
1000,
492+
);
493+
});
494+
try {
495+
firstAbort.abort(new Error("caller stopped"));
496+
await expect(Promise.race([first, abortTimeout])).rejects.toThrow(
497+
"caller stopped",
498+
);
499+
} finally {
500+
if (abortTimer !== undefined) clearTimeout(abortTimer);
501+
releaseCallback?.();
502+
}
503+
504+
await expect(second).resolves.toEqual([]);
505+
expect(waitForCodeCalls).toBe(1);
506+
expect(finishAuthCalls).toBe(1);
507+
expect(authURLCount).toBe(1);
508+
});
509+
468510
test("aborted waiter still fires onAuthorized when background finishAuth succeeds", async () => {
469511
finishAuthError = undefined;
470512
callbackGate = new Promise((resolve) => {
@@ -508,6 +550,50 @@ describe("HTTP MCP re-auth loop prevention", () => {
508550
expect(authURLCount).toBe(1 + MAX_BROWSER_AUTH_ATTEMPTS);
509551
});
510552

553+
test("block-path aborted waiter still fires onAuthorized when background finishAuth succeeds", async () => {
554+
finishAuthError = undefined;
555+
callbackGate = new Promise((resolve) => {
556+
releaseCallback = resolve;
557+
});
558+
const connected = await connectMCPServer(config, {
559+
onAuthURL: () => (authURLCount += 1),
560+
onAuthorized: () => (authorizedCount += 1),
561+
});
562+
expect(connected.ok).toBe(true);
563+
if (!connected.ok) return;
564+
const callBlocks = connected.client.callBlocks;
565+
expect(callBlocks).toBeDefined();
566+
if (callBlocks === undefined) return;
567+
callFailuresLeft = 1;
568+
const abort = new AbortController();
569+
const call = callBlocks("ping", {}, abort.signal);
570+
while (authURLCount === 0 || waitForCodeCalls === 0)
571+
await Promise.resolve();
572+
573+
let abortTimer: ReturnType<typeof setTimeout> | undefined;
574+
const abortTimeout = new Promise<never>((_, reject) => {
575+
abortTimer = setTimeout(
576+
() => reject(new Error("timed out waiting for caller abort")),
577+
1000,
578+
);
579+
});
580+
try {
581+
abort.abort(new Error("caller stopped"));
582+
await expect(Promise.race([call, abortTimeout])).rejects.toThrow(
583+
"caller stopped",
584+
);
585+
} finally {
586+
if (abortTimer !== undefined) clearTimeout(abortTimer);
587+
releaseCallback?.();
588+
}
589+
expect(authorizedCount).toBe(0);
590+
while (finishAuthCalls === 0) await Promise.resolve();
591+
for (let tick = 0; tick < 20 && authorizedCount === 0; tick += 1)
592+
await Promise.resolve();
593+
expect(authorizedCount).toBe(1);
594+
expect(finishAuthCalls).toBe(1);
595+
});
596+
511597
test("refresh-only recovery clears prior browser-cap counts", async () => {
512598
const connected = await connectMCPServer(config, {
513599
onAuthURL: () => (authURLCount += 1),

‎src/mcp/client.ts‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -613,12 +613,13 @@ async function finishClient(
613613
return envelope;
614614
},
615615
async callBlocks(toolName, args, signal) {
616-
const context =
617-
authContext === undefined ? undefined : { ...authContext, signal };
618-
const result = await withHTTPAuthorizationRecovery(context, () =>
619-
client.callTool({ name: toolName, arguments: args }, undefined, {
620-
signal,
621-
}),
616+
const result = await withHTTPAuthorizationRecovery(
617+
authContext,
618+
() =>
619+
client.callTool({ name: toolName, arguments: args }, undefined, {
620+
signal,
621+
}),
622+
signal,
622623
);
623624
return validateMcpContentBlocks(result.content);
624625
},

‎src/mcp/plugin.test.ts‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -635,4 +635,36 @@ describe("mcpClientToAgentTools", () => {
635635
expect(result.isError).toBe(true);
636636
expect(result.content).toContain("transport exploded");
637637
});
638+
639+
test("a parked block-path call aborted mid-recovery surfaces a failed result", async () => {
640+
const gate = skipGate();
641+
const client: MCPClient = {
642+
...fakeClient("unused"),
643+
callBlocks: (_tool, _args, signal) =>
644+
new Promise<never>((_resolve, reject) => {
645+
if (signal.aborted) {
646+
reject(signal.reason ?? new Error("aborted"));
647+
return;
648+
}
649+
signal.addEventListener(
650+
"abort",
651+
() => reject(signal.reason ?? new Error("aborted")),
652+
{ once: true },
653+
);
654+
}),
655+
};
656+
const [tool] = mcpClientToAgentTools(client, gate);
657+
if (tool?.kind !== "full") throw new Error("expected full tool");
658+
659+
const controller = new AbortController();
660+
const pending = tool.handler(
661+
{ id: "c-mcp-abort", name: "mcp__acme__fetch_secret", arguments: {} },
662+
controller.signal,
663+
);
664+
controller.abort(new Error("caller stopped"));
665+
const result = await pending;
666+
667+
expect(result.isError).toBe(true);
668+
expect(result.content).toContain("caller stopped");
669+
});
638670
});

0 commit comments

Comments
 (0)