fix(coding-agent): retry MCP tool auth challenges

This commit is contained in:
slee1996
2026-07-22 23:44:38 -06:00
parent 7504d4c24d
commit 2145ab8f8e
8 changed files with 325 additions and 56 deletions
+1
View File
@@ -4,6 +4,7 @@
### Fixed
- Fixed the setup wizard hiding the selected row on short terminals (e.g. 24x80): the provider sign-in, theme, and web-search lists now fit their windows to the visible height, and decorative chrome (sign-in hint, theme mock preview) yields to the list when space is tight.
- Fixed MCP tool calls that return a `WWW-Authenticate` challenge by preserving the structured metadata, completing the configured OAuth flow, and retrying the call once on the refreshed connection.
## [17.0.8] - 2026-07-22
+39 -8
View File
@@ -38,6 +38,7 @@ import type { MCPToolDetails } from "./tool-bridge";
import { DeferredMCPTool, MCPTool } from "./tool-bridge";
import type { MCPToolCache } from "./tool-cache";
import type {
MCPAuthChallenge,
MCPGetPromptResult,
MCPPrompt,
MCPRequestOptions,
@@ -160,6 +161,9 @@ export interface MCPDiscoverOptions {
onStatus?: (event: McpConnectionStatusEvent) => void;
}
/** Handles an MCP `WWW-Authenticate` challenge and returns refreshed config. */
export type MCPAuthHandler = (serverName: string, challenge: MCPAuthChallenge) => Promise<MCPServerConfig | undefined>;
/**
* MCP Server Manager.
*
@@ -189,6 +193,7 @@ export class MCPManager {
#pendingToolLoads = new Map<string, Promise<ToolLoadResult>>();
#sources = new Map<string, SourceMeta>();
#authStorage: AuthStorage | null = null;
#authHandler?: MCPAuthHandler;
#onNotification?: (serverName: string, method: string, params: unknown) => void;
#onToolsChanged?: (tools: CustomTool<TSchema, MCPToolDetails>[]) => void;
#onResourcesChanged?: (serverName: string, uri: string) => void;
@@ -201,7 +206,7 @@ export class MCPManager {
/** Preserved configs for reconnection after connection loss. */
#serverConfigs = new Map<string, MCPServerConfig>();
/**
* Timestamps of recent `reconnectServer` invocations per server, used by the
* Timestamps of recent reconnectServer invocations per server, used by the
* crash-storm circuit breaker (see {@link RECONNECT_BURST_LIMIT}).
*/
#reconnectHistory = new Map<string, number[]>();
@@ -312,6 +317,11 @@ export class MCPManager {
this.#authStorage = authStorage;
}
/** Set the callback used to complete OAuth after a tool-level auth challenge. */
setAuthHandler(handler: MCPAuthHandler | undefined): void {
this.#authHandler = handler;
}
/**
* Discover and connect to all MCP servers from .mcp.json files.
* Returns tools and any connection errors.
@@ -474,7 +484,8 @@ export class MCPManager {
.then(async ({ connection, serverTools }) => {
if (this.#pendingToolLoads.get(name) !== toolsPromise) return;
this.#pendingToolLoads.delete(name);
const reconnect = () => this.reconnectServer(name);
const reconnect = (options?: { authChallenge?: MCPAuthChallenge }) =>
this.reconnectServer(name, options);
const customTools = MCPTool.fromTools(connection, serverTools, reconnect);
this.#replaceServerTools(name, customTools);
this.#onToolsChanged?.(this.#tools);
@@ -810,13 +821,15 @@ export class MCPManager {
* the same server share one reconnection attempt. Returns the new
* connection, or `null` if reconnection failed or the per-server crash
* burst limit (see {@link RECONNECT_BURST_LIMIT}) is exceeded.
*
* @param options.manual - When `true`, resets the crash-burst window so a
* user-driven retry (e.g. `/mcp reconnect`) is never blocked by an
* earlier storm. Defaults to `false`; the transport `onClose` callback
* and the per-tool-call retry path in `tool-bridge` MUST NOT set it.
*/
async reconnectServer(name: string, options?: { manual?: boolean }): Promise<MCPServerConnection | null> {
async reconnectServer(
name: string,
options?: { manual?: boolean; authChallenge?: MCPAuthChallenge },
): Promise<MCPServerConnection | null> {
if (options?.manual) {
this.#reconnectHistory.delete(name);
}
@@ -828,7 +841,7 @@ export class MCPManager {
return null;
}
const attempt = this.#doReconnect(name);
const attempt = this.#doReconnect(name, options?.authChallenge);
this.#pendingReconnections.set(name, attempt);
return attempt.finally(() => this.#pendingReconnections.delete(name));
}
@@ -873,12 +886,30 @@ export class MCPManager {
return false;
}
async #doReconnect(name: string): Promise<MCPServerConnection | null> {
async #doReconnect(name: string, authChallenge?: MCPAuthChallenge): Promise<MCPServerConnection | null> {
const oldConnection = this.#connections.get(name);
const config = oldConnection?.config ?? this.#serverConfigs.get(name);
let config = oldConnection?.config ?? this.#serverConfigs.get(name);
const source = this.#sources.get(name) ?? oldConnection?._source;
if (!config) return null;
if (authChallenge) {
if (!this.#authHandler) {
logger.error("MCP auth challenge cannot be handled; no auth handler is configured", {
path: `mcp:${name}`,
});
return null;
}
try {
const refreshedConfig = await this.#authHandler(name, authChallenge);
if (!refreshedConfig) return null;
config = refreshedConfig;
this.#serverConfigs.set(name, config);
} catch (error) {
logger.error("MCP auth challenge handling failed", { path: `mcp:${name}`, error });
return null;
}
}
logger.debug("MCP reconnecting", { path: `mcp:${name}` });
// Close the old transport without removing tools or notifying consumers.
@@ -987,7 +1018,7 @@ export class MCPManager {
};
try {
const serverTools = await listTools(connection);
const reconnect = () => this.reconnectServer(name);
const reconnect = (options?: { authChallenge?: MCPAuthChallenge }) => this.reconnectServer(name, options);
const customTools = MCPTool.fromTools(connection, serverTools, reconnect);
void this.toolCache?.set(name, config, serverTools);
this.#replaceServerTools(name, customTools);
+107 -11
View File
@@ -22,10 +22,17 @@ import { normalizeLocalScheme } from "../tools/path-utils";
import { ToolAbortError, throwIfAborted } from "../tools/tool-errors";
import { callTool } from "./client";
import { renderMCPCall, renderMCPResult } from "./render";
import type { MCPContent, MCPServerConnection, MCPToolCallParams, MCPToolCallResult, MCPToolDefinition } from "./types";
import type {
MCPAuthChallenge,
MCPContent,
MCPServerConnection,
MCPToolCallParams,
MCPToolCallResult,
MCPToolDefinition,
} from "./types";
/** Reconnect callback: tears down stale connection, returns new one or null. */
export type MCPReconnect = () => Promise<MCPServerConnection | null>;
/** Reconnect callback: tears down a stale connection, optionally authorizing first. */
export type MCPReconnect = (options?: { authChallenge?: MCPAuthChallenge }) => Promise<MCPServerConnection | null>;
/**
* Network-level and stale-session errors that warrant a reconnect + single retry.
@@ -174,6 +181,8 @@ export interface MCPToolDetails {
isError?: boolean;
/** Raw content from MCP response */
rawContent?: MCPContent[];
/** Structured metadata from the MCP response */
mcpMeta?: Record<string, unknown>;
/** Provider ID (e.g., "claude", "mcp-json") */
provider?: string;
/** Provider display name (e.g., "Claude Code", "MCP Config") */
@@ -222,6 +231,7 @@ function buildResult(
mcpToolName,
isError: result.isError,
rawContent: result.content,
mcpMeta: result._meta,
provider,
providerName,
};
@@ -249,6 +259,51 @@ function buildErrorResult(
};
}
type MCPToolCallAttempt = {
connection: MCPServerConnection;
result?: MCPToolCallResult;
error?: unknown;
};
function getMcpAuthChallenge(result: MCPToolCallResult): MCPAuthChallenge | undefined {
if (!result.isError) return undefined;
const values = result._meta?.["mcp/www_authenticate"];
if (!Array.isArray(values)) return undefined;
const wwwAuthenticate = values.filter((value): value is string => typeof value === "string" && value.trim() !== "");
return wwwAuthenticate.length > 0 ? { wwwAuthenticate } : undefined;
}
async function callToolWithAuthRetry(
connection: MCPServerConnection,
toolName: string,
args: MCPToolArgs,
reconnect: MCPReconnect | undefined,
signal?: AbortSignal,
): Promise<MCPToolCallAttempt> {
const result = await callTool(connection, toolName, args, { signal });
const authChallenge = getMcpAuthChallenge(result);
if (!authChallenge || !reconnect) return { connection, result };
let newConnection: MCPServerConnection | null;
try {
newConnection = await reconnectWithAbort(reconnect, signal, { authChallenge });
} catch (error) {
rethrowIfAborted(error, signal);
return { connection, error };
}
if (!newConnection) return { connection, result };
try {
return {
connection: newConnection,
result: await callTool(newConnection, toolName, args, { signal }),
};
} catch (error) {
rethrowIfAborted(error, signal);
return { connection: newConnection, error };
}
}
/** Re-throw abort-related errors so they bypass error-result handling. */
function rethrowIfAborted(error: unknown, signal?: AbortSignal): void {
if (error instanceof ToolAbortError) throw error;
@@ -256,9 +311,13 @@ function rethrowIfAborted(error: unknown, signal?: AbortSignal): void {
if (signal?.aborted) throw new ToolAbortError();
}
async function reconnectWithAbort(reconnect: MCPReconnect, signal?: AbortSignal): Promise<MCPServerConnection | null> {
async function reconnectWithAbort(
reconnect: MCPReconnect,
signal?: AbortSignal,
options?: { authChallenge?: MCPAuthChallenge },
): Promise<MCPServerConnection | null> {
try {
return await untilAborted(signal, reconnect);
return await untilAborted(signal, () => reconnect(options));
} catch (error) {
rethrowIfAborted(error, signal);
return null;
@@ -379,8 +438,27 @@ export class MCPTool implements CustomTool<TSchema, MCPToolDetails> {
const providerName = this.connection._source?.providerName;
try {
const result = await callTool(this.connection, this.tool.name, args, { signal });
return buildResult(result, this.connection.name, this.tool.name, provider, providerName);
const attempt = await callToolWithAuthRetry(this.connection, this.tool.name, args, this.reconnect, signal);
if (attempt.error !== undefined) {
return buildErrorResult(attempt.error, this.connection.name, this.tool.name, provider, providerName);
}
if (!attempt.result) {
return buildErrorResult(
new Error("MCP tool call returned no result"),
this.connection.name,
this.tool.name,
provider,
providerName,
);
}
this.connection = attempt.connection;
return buildResult(
attempt.result,
attempt.connection.name,
this.tool.name,
attempt.connection._source?.provider ?? provider,
attempt.connection._source?.providerName ?? providerName,
);
} catch (error) {
rethrowIfAborted(error, signal);
if (this.reconnect && isRetriableConnectionError(error)) {
@@ -483,13 +561,31 @@ export class DeferredMCPTool implements CustomTool<TSchema, MCPToolDetails> {
const connection = await untilAborted(signal, () => this.getConnection());
throwIfAborted(signal);
try {
const result = await callTool(connection, this.tool.name, args, { signal });
const attempt = await callToolWithAuthRetry(connection, this.tool.name, args, this.reconnect, signal);
if (attempt.error !== undefined) {
return buildErrorResult(
attempt.error,
this.serverName,
this.tool.name,
attempt.connection._source?.provider ?? provider,
attempt.connection._source?.providerName ?? providerName,
);
}
if (!attempt.result) {
return buildErrorResult(
new Error("MCP tool call returned no result"),
this.serverName,
this.tool.name,
provider,
providerName,
);
}
return buildResult(
result,
attempt.result,
this.serverName,
this.tool.name,
connection._source?.provider ?? provider,
connection._source?.providerName ?? providerName,
attempt.connection._source?.provider ?? provider,
attempt.connection._source?.providerName ?? providerName,
);
} catch (callError) {
rethrowIfAborted(callError, signal);
+7
View File
@@ -206,10 +206,17 @@ export interface MCPResourceContent {
export type MCPContent = MCPTextContent | MCPImageContent | MCPResourceContent;
/** Structured authentication challenge returned in a tool result. */
export interface MCPAuthChallenge {
/** Values from `_meta["mcp/www_authenticate"]`. */
readonly wwwAuthenticate: readonly string[];
}
/** tools/call response */
export interface MCPToolCallResult {
content: MCPContent[];
isError?: boolean;
_meta?: Record<string, unknown>;
}
// =============================================================================
@@ -47,7 +47,7 @@ import {
searchSmitheryRegistry,
toConfigName,
} from "../../mcp/smithery-registry";
import type { MCPAuthConfig, MCPServerConfig, MCPServerConnection } from "../../mcp/types";
import type { MCPAuthChallenge, MCPAuthConfig, MCPServerConfig, MCPServerConnection } from "../../mcp/types";
import { shortenPath } from "../../tools/render-utils";
import { urlHyperlinkAlways } from "../../tui";
import { copyToClipboard } from "../../utils/clipboard";
@@ -1042,7 +1042,10 @@ export class MCPCommandController {
return next;
}
async #resolveOAuthEndpointsFromServer(config: MCPServerConfig): Promise<OAuthEndpoints> {
async #resolveOAuthEndpointsFromServer(
config: MCPServerConfig,
authChallenge?: MCPAuthChallenge,
): Promise<OAuthEndpoints> {
// Stdio servers manage credentials inside the child process; OMP's OAuth
// flow only applies to http/sse transports. Without this guard the
// unauthenticated preflight below spawns the child, which happily reuses
@@ -1073,8 +1076,13 @@ export class MCPCommandController {
throw new Error("Server connection succeeded without OAuth; reauthorization is not required.");
}
// Analyze the connection error to extract OAuth endpoints
const authResult = analyzeAuthError(connectionError!, "url" in config ? config.url : undefined);
// Tool calls can carry richer RFC 6750/RFC 9728 hints than the original
// connection error. Feed those hints through the same analyzer so
// resource_metadata and scope reach protected-resource discovery.
const authError = authChallenge
? new Error(`${connectionError?.message ?? "HTTP 401"}\n${authChallenge.wwwAuthenticate.join("\n")}`)
: connectionError!;
const authResult = analyzeAuthError(authError, "url" in config ? config.url : undefined);
let oauth = authResult.authType === "oauth" ? (authResult.oauth ?? null) : null;
if (!oauth && (config.type === "http" || config.type === "sse") && config.url) {
@@ -1678,21 +1686,29 @@ export class MCPCommandController {
}
}
async #handleReauth(name: string | undefined): Promise<void> {
/** Reauthorize a server after a tool-level OAuth challenge. */
async handleMCPAuthChallenge(name: string, challenge: MCPAuthChallenge): Promise<MCPServerConfig | undefined> {
return this.#handleReauth(name, { silent: true, reload: false, authChallenge: challenge });
}
async #handleReauth(
name: string | undefined,
options: { silent?: boolean; reload?: boolean; authChallenge?: MCPAuthChallenge } = {},
): Promise<MCPServerConfig | undefined> {
if (!name) {
this.ctx.showError("Server name required. Usage: /mcp reauth <name>");
if (!options.silent) this.ctx.showError("Server name required. Usage: /mcp reauth <name>");
return;
}
try {
const found = await this.#resolveServerForAuth(name);
if (!found) {
this.ctx.showError(`Server "${name}" not found.`);
if (!options.silent) this.ctx.showError(`Server "${name}" not found.`);
return;
}
if (found.config.enabled === false) {
this.ctx.showError(`Server "${name}" is disabled. Run /mcp enable ${name} first.`);
if (!options.silent) this.ctx.showError(`Server "${name}" is disabled. Run /mcp enable ${name} first.`);
return;
}
@@ -1705,7 +1721,7 @@ export class MCPCommandController {
// happened yet if the server turns out not to need (or support) OAuth.
// Use the same env-expanded config shape runtime discovery passes to
// MCPManager; the raw file value may contain `${...}` placeholders.
const oauth = await this.#resolveOAuthEndpointsFromServer(runtimeBaseConfig);
const oauth = await this.#resolveOAuthEndpointsFromServer(runtimeBaseConfig, options.authChallenge);
const serverUrl =
runtimeBaseConfig.type === "http" || runtimeBaseConfig.type === "sse" ? runtimeBaseConfig.url : undefined;
// A user-supplied client secret may live in either block (the wizard
@@ -1719,7 +1735,9 @@ export class MCPCommandController {
const userClientSecret = found.config.oauth?.clientSecret ?? currentAuth?.clientSecret;
const flowClientSecret = userClientSecret ?? storedClientSecret ?? "";
this.#showMessage(["", theme.fg("muted", `Reauthorizing "${name}"...`), ""].join("\n"));
if (!options.silent) {
this.#showMessage(["", theme.fg("muted", `Reauthorizing "${name}"...`), ""].join("\n"));
}
const currentAuthResource = currentAuth?.resource ? expandEnvVarsDeep(currentAuth.resource) : undefined;
const oauthResource =
@@ -1755,39 +1773,49 @@ export class MCPCommandController {
// Definition-only entries resolve through the url-keyed binding alone;
// skip the write-back so a committed project mcp.json stays clean.
const urlKeyedId = serverUrl ? mcpOAuthCredentialId(serverUrl) : undefined;
if (currentAuth || oauthResult.credentialId !== urlKeyedId) {
const updated = this.#persistOAuthResult(baseConfig, oauthResult, {
tokenUrl: oauth.tokenUrl,
clientId: oauth.clientId,
userClientSecret,
resource: oauthResource,
stripSameOriginResource: oauthResourceIsFallback,
});
await updateMCPServer(found.filePath, name, updated);
const shouldPersist = currentAuth || oauthResult.credentialId !== urlKeyedId;
const updatedConfig = shouldPersist
? this.#persistOAuthResult(baseConfig, oauthResult, {
tokenUrl: oauth.tokenUrl,
clientId: oauth.clientId,
userClientSecret,
resource: oauthResource,
stripSameOriginResource: oauthResourceIsFallback,
})
: baseConfig;
if (shouldPersist) {
await updateMCPServer(found.filePath, name, updatedConfig);
}
await this.#reloadMCP();
const state = await this.#waitForServerConnectionWithAnimation(name);
if (options.reload !== false) {
await this.#reloadMCP();
const state = await this.#waitForServerConnectionWithAnimation(name);
const lines = [
"",
theme.fg("success", `✓ Reauthorized "${name}" (${found.scope} config)`),
"",
` Status: ${
state === "connected"
? theme.fg("success", "connected")
: state === "connecting"
? theme.fg("muted", "connecting")
: theme.fg("warning", "not connected")
}`,
"",
];
this.#showMessage(lines.join("\n"));
const lines = [
"",
theme.fg("success", `✓ Reauthorized "${name}" (${found.scope} config)`),
"",
` Status: ${
state === "connected"
? theme.fg("success", "connected")
: state === "connecting"
? theme.fg("muted", "connecting")
: theme.fg("warning", "not connected")
}`,
"",
];
this.#showMessage(lines.join("\n"));
}
return updatedConfig;
} catch (error) {
if (error instanceof MCPOAuthCancelledError) {
this.ctx.showStatus(`Reauthorization cancelled for "${name}"`);
if (!options.silent) this.ctx.showStatus(`Reauthorization cancelled for "${name}"`);
return;
}
this.ctx.showError(`Failed to reauthorize server: ${error instanceof Error ? error.message : String(error)}`);
if (!options.silent) {
this.ctx.showError(
`Failed to reauthorize server: ${error instanceof Error ? error.message : String(error)}`,
);
}
}
}
@@ -656,6 +656,9 @@ export class InteractiveMode implements InteractiveModeContext {
this.#toolUiContextSetter = setToolUIContext;
this.lspServers = lspServers;
this.mcpManager = mcpManager;
this.mcpManager?.setAuthHandler((serverName, challenge) =>
new MCPCommandController(this).handleMCPAuthChallenge(serverName, challenge),
);
this.#eventBus = eventBus;
if (eventBus) {
this.#eventBusUnsubscribers.push(
@@ -244,6 +244,62 @@ describe("/mcp auth commands", () => {
});
});
test("uses tool challenge resource metadata and scopes during reauth", async () => {
const authStorage = freshAuthStorage();
await authStorage.reload();
vi.spyOn(mcpClient, "connectToServer").mockRejectedValue(new Error("HTTP 401: Unauthorized"));
const resourceMetadataUrl = "https://gateway.example.com/.well-known/oauth-protected-resource";
const fetchMock = Object.assign(
async (input: string | URL | Request): Promise<Response> => {
const url = String(input);
if (url === resourceMetadataUrl) {
return new Response(
JSON.stringify({
resource: "https://gateway.example.com/mcp",
authorization_servers: ["https://auth.example.com"],
}),
{ status: 200, headers: { "Content-Type": "application/json" } },
);
}
if (url === "https://auth.example.com/.well-known/oauth-authorization-server") {
return new Response(
JSON.stringify({
authorization_endpoint: "https://auth.example.com/authorize",
token_endpoint: "https://auth.example.com/token",
client_id: "challenge-client",
}),
{ status: 200, headers: { "Content-Type": "application/json" } },
);
}
return new Response("not found", { status: 404 });
},
{ preconnect: globalThis.fetch.preconnect },
);
vi.spyOn(globalThis, "fetch").mockImplementation(fetchMock);
let authorizationUrl = "";
vi.spyOn(oauthFlow.MCPOAuthFlow.prototype, "login").mockImplementation(async function (
this: oauthFlow.MCPOAuthFlow,
) {
authorizationUrl = (await this.generateAuthUrl("state", "http://127.0.0.1:53192/callback")).url;
return {
access: "fresh-access",
refresh: "fresh-refresh",
expires: Date.now() + 3_600_000,
};
});
const { controller, showError } = createController(authStorage);
const updated = await controller.handleMCPAuthChallenge("envserver", {
wwwAuthenticate: [`Bearer resource_metadata="${resourceMetadataUrl}" scope="orders.read"`],
});
expect(updated).toEqual(expect.objectContaining({ type: "http", url: RAW_SERVER_URL }));
expect(showError).not.toHaveBeenCalled();
expect(new URL(authorizationUrl).searchParams.get("scope")).toBe("orders.read");
expect(new URL(authorizationUrl).searchParams.get("resource")).toBe("https://gateway.example.com/mcp");
});
test("reuses embedded DCR client secret during reauth token exchange", async () => {
const authStorage = freshAuthStorage();
await authStorage.reload();
@@ -271,6 +271,53 @@ describe("MCPTool.execute retry on connection error", () => {
expect(result.details?.provider).toBe("orig");
expect(result.details?.providerName).toBe("Original");
});
it("reconnects once when a tool result carries an OAuth challenge", async () => {
let oldCalls = 0;
let newCalls = 0;
let challenge: unknown;
const oldTransport = mockTransport(async () => {
oldCalls++;
return {
...toolCallResult("authorize me", true),
_meta: { "mcp/www_authenticate": ['Bearer resource_metadata="https://mcp.example/meta"'] },
};
});
const newTransport = mockTransport(async () => {
newCalls++;
return toolCallResult("authorized");
});
const newConn = makeConnection(newTransport, "test-server-authorized");
const reconnect: MCPReconnect = async options => {
challenge = options?.authChallenge;
return newConn;
};
const tool = new MCPTool(makeConnection(oldTransport), TOOL_DEF, reconnect);
const result = await tool.execute("call-1", {}, noop, noCtx);
expect(oldCalls).toBe(1);
expect(newCalls).toBe(1);
expect(challenge).toEqual({
wwwAuthenticate: ['Bearer resource_metadata="https://mcp.example/meta"'],
});
expect(result.details?.isError).toBeFalsy();
expect(result.content[0]).toEqual({ type: "text", text: "authorized" });
});
it("preserves the OAuth challenge metadata when no reconnect handler exists", async () => {
const result = await new MCPTool(
makeConnection(
mockTransport(async () => ({
...toolCallResult("authorize me", true),
_meta: { "mcp/www_authenticate": ["Bearer"] },
})),
),
TOOL_DEF,
).execute("call-1", {}, noop, noCtx);
expect(result.details?.isError).toBe(true);
expect(result.details?.mcpMeta).toEqual({ "mcp/www_authenticate": ["Bearer"] });
});
});
describe("reconnect abort propagation", () => {