Skip to content

Commit 89d26e0

Browse files
committed
refactor(mcp): let the SDK finish HTTP auth and SSE reconnect
Drop the extra probe on the ordinary path, finish auth on the transport that already dialed, and remove MCPClient.call. After an OAuth error that arrives before redirect, wipe credentials and dial once: a silent connect lets the real client retry, and a redirect prompts once.
1 parent 0a2de14 commit 89d26e0

12 files changed

Lines changed: 218 additions & 174 deletions

‎docs/MCP.md‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -130,14 +130,19 @@ list is in effect, these actions edit that local file; add remains blocked.
130130

131131
## Dropped Transports Reconnect in the Background
132132

133-
When a connected server's transport dies unexpectedly, the row goes
134-
**reconnecting** instead of failed: the redial attempt and the retained tool
135-
count stay on the row (for example, `acme — reconnecting · attempt 3 · 2
136-
tools`), and backoff redials behind the scenes with a jittered delay that
137-
grows per attempt and caps at 30 seconds. While reconnecting, the server's
138-
tools stay mounted but fail fast — calling one returns "MCP server is
139-
reconnecting; retry the call once it reports connected." rather than hanging
140-
or dispatching against a dead transport.
133+
Stdio and HTTP do not share a redial. A stdio child that exits is respawned
134+
here: the row goes **reconnecting** instead of failed, the redial attempt and
135+
the retained tool count stay on the row (for example, `acme — reconnecting ·
136+
attempt 3 · 2 tools`), and backoff redials behind the scenes with a jittered
137+
delay that grows per attempt and caps at 30 seconds. While that redial is in
138+
progress, the server's tools stay mounted but fail fast — calling one returns
139+
"MCP server is reconnecting; retry the call once it reports connected." rather
140+
than hanging or dispatching against a dead transport.
141+
142+
An HTTP SSE drop does not start that second dial. The Streamable HTTP
143+
transport resumes the stream itself (`reconnectionOptions`: 1s start, doubling,
144+
30s cap). The row stays connected and tools stay live; only a real stdio death
145+
uses the fail-fast stubs and attempt count above.
141146

142147
Pressing Enter on a failed or reconnecting row retries now with a single dial
143148
instead of waiting for backoff, reusing the saved settings row — it never

‎e2e/mcp-late-dispatch.test.ts‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,13 @@ function permissionGate() {
4343

4444
function linearClient(opts: {
4545
inputSchema?: MCPClient["tools"][number]["inputSchema"];
46-
call?: MCPClient["call"];
46+
call?: (
47+
toolName: string,
48+
args: Record<string, unknown>,
49+
signal: AbortSignal,
50+
) => Promise<string>;
4751
}): MCPClient {
52+
const invoke = opts.call ?? (async () => "ISSUE-1");
4853
return {
4954
serverName: "linear",
5055
tools: [
@@ -54,13 +59,8 @@ function linearClient(opts: {
5459
inputSchema: opts.inputSchema ?? LATE_MCP_SCHEMA,
5560
},
5661
],
57-
call: opts.call ?? (async () => "ISSUE-1"),
5862
async callResult(toolName, args, signal) {
59-
const text = await (opts.call ?? (async () => "ISSUE-1"))(
60-
toolName,
61-
args,
62-
signal,
63-
);
63+
const text = await invoke(toolName, args, signal);
6464
return {
6565
blocks: [{ type: "text", text }],
6666
isError: false,

‎e2e/subagent-permission.test.ts‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,6 @@ test.serial(
296296
inputSchema: { type: "object", properties: {} },
297297
},
298298
],
299-
call: async () => "changed",
300299
callResult: async () => {
301300
calls++;
302301
return {
@@ -354,7 +353,6 @@ test.serial(
354353
inputSchema: { type: "object", properties: {} },
355354
},
356355
],
357-
call: async () => "changed",
358356
callResult: async () => {
359357
calls++;
360358
return {
@@ -488,7 +486,6 @@ test.serial(
488486
inputSchema: { type: "object", properties: {} },
489487
},
490488
],
491-
call: async () => "changed",
492489
callResult: async () => {
493490
calls++;
494491
return {
@@ -573,7 +570,6 @@ test("storing parent-gated MCP tools then wrapping again denies without requestA
573570
inputSchema: { type: "object", properties: {} },
574571
},
575572
],
576-
call: async () => "changed",
577573
callResult: async () => {
578574
calls++;
579575
return {

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

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,8 @@ await mockMcpOAuthProviderModule(async (options: { serverURL: string }) => {
143143
return authProvider;
144144
});
145145

146-
const { connectMCPServer, fetchWithConnectAbort } = await import("./client.js");
146+
const { connectMCPServer, fetchWithConnectAbort, MCP_HTTP_RECONNECTION_OPTIONS } =
147+
await import("./client.js");
147148

148149
describe("HTTP MCP auth policy", () => {
149150
beforeEach(() => {
@@ -186,7 +187,9 @@ describe("HTTP MCP auth policy", () => {
186187
expect(providerCreates).toBe(0);
187188
// No signal and no provider: leave redirectPolicy unset so the SDK
188189
// default `same-origin` applies. Do not pin `follow`.
189-
expect(transportOptions).toEqual([{}]);
190+
expect(transportOptions).toEqual([
191+
{ reconnectionOptions: MCP_HTTP_RECONNECTION_OPTIONS },
192+
]);
190193
});
191194

192195
test("closes a bound OAuth callback server when provider setup fails", async () => {
@@ -262,13 +265,14 @@ describe("HTTP MCP auth policy", () => {
262265
if (!result.ok) return;
263266
expect(transportOptions).toEqual([
264267
{
268+
reconnectionOptions: MCP_HTTP_RECONNECTION_OPTIONS,
265269
authProvider,
266270
requestInit: { signal: expect.any(AbortSignal) },
267271
fetch: expect.any(Function),
268272
},
269273
]);
270274

271-
const call = result.client.call("ping", {}, abort.signal);
275+
const call = result.client.callResult("ping", {}, abort.signal);
272276
while (tokenRefreshSignals.length === 0) await Promise.resolve();
273277
expect(tokenRefreshSignals[0]).toBeDefined();
274278
expect(tokenRefreshSignals[0]?.aborted).toBe(false);
@@ -298,6 +302,7 @@ describe("HTTP MCP auth policy", () => {
298302
expect(providerServerURL).toBe("https://custom.example/mcp?mode=full");
299303
expect(transportOptions).toEqual([
300304
{
305+
reconnectionOptions: MCP_HTTP_RECONNECTION_OPTIONS,
301306
authProvider,
302307
requestInit: { signal: expect.any(AbortSignal) },
303308
fetch: expect.any(Function),
@@ -322,7 +327,11 @@ describe("HTTP MCP auth policy", () => {
322327
expect(transport.requestInit?.signal).not.toBe(connectAbort.signal);
323328
expect(transport.fetch).toBeTypeOf("function");
324329

325-
const call = result.client.call("ping", {}, new AbortController().signal);
330+
const call = result.client.callResult(
331+
"ping",
332+
{},
333+
new AbortController().signal,
334+
);
326335
while (tokenRefreshSignals.length === 0) await Promise.resolve();
327336
expect(tokenRefreshSignals[0]).not.toBe(connectAbort.signal);
328337
expect(tokenRefreshSignals[0]?.aborted).toBe(false);

0 commit comments

Comments
 (0)