Skip to content

Commit 1be723d

Browse files
Clear the MCP auth banner after mid-session re-authentication succeeds (#433)
Re-emit connected status once interactive OAuth and the retried MCP operation both succeed, so standing needs-auth chrome does not stick around after tools are already registered.
1 parent 826c7ba commit 1be723d

4 files changed

Lines changed: 114 additions & 6 deletions

File tree

src/agent/tools.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,18 @@ export async function createAgentToolset(args: AgentToolsetArgs): Promise<AgentT
353353
const result = await connectMCPServer(config, {
354354
stderr: "ignore",
355355
onAuthURL: (name, url) => callbacks.onStatus({ name, state: "needs-auth", url }),
356+
// Mid-session re-auth fires needs-auth again without a later connected
357+
// event. Re-emit connected only when tools are already registered so
358+
// first-connect still waits for the real post-connect status.
359+
onAuthorized: (name) => {
360+
const client = connectedClients.find((c) => c.serverName === name);
361+
if (client === undefined) return;
362+
callbacks.onStatus({
363+
name,
364+
state: "connected",
365+
tools: client.tools.map((t) => t.name),
366+
});
367+
},
356368
...(signal !== undefined ? { signal } : {}),
357369
});
358370
if (!result.ok) {

src/mcp/client-auth-retry.test.ts

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
import { describe, expect, test } from "bun:test";
2+
import { retryAfterInteractiveAuth } from "./client.js";
3+
4+
describe("retryAfterInteractiveAuth", () => {
5+
test("notifies only after the retried operation succeeds", async () => {
6+
let notified = false;
7+
await expect(
8+
retryAfterInteractiveAuth(
9+
async () => undefined,
10+
async () => {
11+
throw new Error("still unauthorized");
12+
},
13+
() => {
14+
notified = true;
15+
},
16+
),
17+
).rejects.toThrow("still unauthorized");
18+
expect(notified).toBe(false);
19+
20+
const value = await retryAfterInteractiveAuth(
21+
async () => undefined,
22+
async () => "ok",
23+
() => {
24+
notified = true;
25+
},
26+
);
27+
expect(value).toBe("ok");
28+
expect(notified).toBe(true);
29+
});
30+
31+
test("propagates auth completion failures without notifying", async () => {
32+
let notified = false;
33+
let operationRan = false;
34+
await expect(
35+
retryAfterInteractiveAuth(
36+
async () => {
37+
throw new Error("auth aborted");
38+
},
39+
async () => {
40+
operationRan = true;
41+
return "ok";
42+
},
43+
() => {
44+
notified = true;
45+
},
46+
),
47+
).rejects.toThrow("auth aborted");
48+
expect(operationRan).toBe(false);
49+
expect(notified).toBe(false);
50+
});
51+
});

src/mcp/client.ts

Lines changed: 47 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,12 @@ export type MCPConnectResult = { ok: true; client: MCPClient } | { ok: false; se
2222
export type MCPConnectOptions = {
2323
stderr?: "inherit" | "ignore" | "pipe";
2424
onAuthURL?: (serverName: string, authorizationUrl: string) => void;
25+
/**
26+
* Interactive OAuth finished and the retried operation succeeded. Callers
27+
* that already registered tools for this server can re-emit a connected
28+
* status so standing "needs auth" chrome clears mid-session.
29+
*/
30+
onAuthorized?: (serverName: string) => void;
2531
signal?: AbortSignal;
2632
};
2733

@@ -39,7 +45,15 @@ export function unwrapToolContent(content: unknown): string {
3945
}).join("\n");
4046
}
4147

42-
type HTTPAuthContext = { url: URL; authProvider: CorbitsOAuthProvider; callback: CallbackServer; signal?: AbortSignal; interactive: boolean };
48+
type HTTPAuthContext = {
49+
url: URL;
50+
authProvider: CorbitsOAuthProvider;
51+
callback: CallbackServer;
52+
signal?: AbortSignal;
53+
interactive: boolean;
54+
serverName: string;
55+
onAuthorized?: (serverName: string) => void;
56+
};
4357

4458
function isRecoverableAuthError(err: unknown): boolean {
4559
return err instanceof UnauthorizedError || err instanceof OAuthError;
@@ -51,14 +65,35 @@ async function completeInteractiveAuth(context: HTTPAuthContext): Promise<void>
5165
await new StreamableHTTPClientTransport(context.url, { authProvider: context.authProvider }).finishAuth(code);
5266
}
5367

68+
/**
69+
* Run interactive OAuth, retry the failed operation, and notify only when the
70+
* retry itself succeeded — a failed re-auth must leave standing "needs auth"
71+
* chrome alone.
72+
*/
73+
export async function retryAfterInteractiveAuth<T>(
74+
completeAuth: () => Promise<void>,
75+
operation: () => Promise<T>,
76+
onAuthorized: (() => void) | undefined,
77+
): Promise<T> {
78+
await completeAuth();
79+
const value = await operation();
80+
onAuthorized?.();
81+
return value;
82+
}
83+
5484
async function recoverHTTPAuthorization<T>(err: unknown, context: HTTPAuthContext | undefined, operation: () => Promise<T>): Promise<T> {
5585
if (context === undefined || !isRecoverableAuthError(err)) throw err;
5686
let lastErr: unknown = err;
5787
for (let attempt = 0; attempt < 2; attempt += 1) {
5888
if (lastErr instanceof OAuthError) await context.authProvider.resetAuthorization();
5989
if (lastErr instanceof UnauthorizedError) {
60-
await completeInteractiveAuth(context);
61-
return operation();
90+
return retryAfterInteractiveAuth(
91+
() => completeInteractiveAuth(context),
92+
operation,
93+
context.onAuthorized === undefined
94+
? undefined
95+
: () => context.onAuthorized?.(context.serverName),
96+
);
6297
}
6398
try {
6499
return await operation();
@@ -128,7 +163,15 @@ async function connectHttp(config: MCPServerConfig, options: MCPConnectOptions):
128163
});
129164
const makeTransport = (): Transport => new StreamableHTTPClientTransport(url, { authProvider }) as unknown as Transport;
130165
const client = new Client({ name: MCP_CLIENT_NAME, version: "1.0.0" });
131-
const authContext: HTTPAuthContext = { url, authProvider, callback, interactive: options.onAuthURL !== undefined, ...(options.signal !== undefined ? { signal: options.signal } : {}) };
166+
const authContext: HTTPAuthContext = {
167+
url,
168+
authProvider,
169+
callback,
170+
interactive: options.onAuthURL !== undefined,
171+
serverName: config.name,
172+
...(options.onAuthorized !== undefined ? { onAuthorized: options.onAuthorized } : {}),
173+
...(options.signal !== undefined ? { signal: options.signal } : {}),
174+
};
132175
try {
133176
await withHTTPAuthorizationRecovery(authContext, () => client.connect(makeTransport()));
134177
return { ok: true, client: await finishClient(client, config.name, authContext) };

src/tui-opentui/runtime-channels.test.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,13 +115,15 @@ describe("mcp.status channel", () => {
115115
}
116116
})
117117

118-
test("connecting clears the standing auth segment", async () => {
118+
test("connected clears the standing auth segment from state and the painted frame", async () => {
119119
const { host, emitter, frame, cleanup } = await mountHeadless()
120120
try {
121121
emitter.emit("mcp.status", { name: "linear", state: "needs-auth", url: "https://x/a" })
122+
expect(await frame()).toContain("needs auth")
122123
emitter.emit("mcp.status", { name: "linear", state: "connected", tools: ["a"] })
123-
await frame()
124+
const painted = await frame()
124125
expect(host.shell.mcpNeedsAuth).toEqual([])
126+
expect(painted).not.toContain("needs auth")
125127
} finally {
126128
cleanup()
127129
}

0 commit comments

Comments
 (0)