Merge PR #6425: fix(mcp): bound oauth discovery and smithery poll fetches with abort timeouts (@roboomp)

packages/coding-agent/CHANGELOG.md

@@ -31,6 +31,7 @@
 
 - Fixed discarded `Settings` instances keeping debounced save timers and chained background saves armed; discarding an instance now cancels its pending writes so they cannot race a successor's file locks.
 - Fixed startup status messages (warnings, errors, extension/tool errors, status lines) keeping the dark-mode color after auto-theme detection later switched the active theme to light — e.g. `dark-catppuccin`/`light-catppuccin` warnings rendered in Mocha yellow on the Latte background. Transient status presenters now resolve their color lazily at render time so a theme swap re-shapes them ([#6337](https://github.com/can1357/oh-my-pi/issues/6337)).
+- Fixed MCP OAuth endpoint discovery and Smithery browser-login polling hanging indefinitely against an endpoint that accepts the TCP connection but never responds. `discoverOAuthEndpoints`/`fetchResourceMetadataScopes` now bound every metadata/well-known/authorization-server fetch with a per-request `AbortSignal.timeout`, and `pollSmitheryCliAuthSession` bounds each poll so the loop reaches its 5-minute deadline instead of stalling ([#4103](https://github.com/can1357/oh-my-pi/issues/4103)).
 - Fixed `error.notify` raising a "Stopped with error" toast for provider failures while an auto-retry or async-delivery continuation was pending; the toast now waits for the true terminal settle.
 - Fixed concurrent MCP config mutations losing updates and racing on a shared temp path: every `mcp.json` read-modify-write (add/update/remove server, disabled/force-enabled lists) is now serialized under a per-file lock, and each atomic write uses a unique temp file so overlapping writers no longer rename each other's `.tmp` out from under them (ENOENT or clobbered config) — reachable in-process via the fire-and-forget extensions-dashboard toggle and across processes on a shared `~/.omp/mcp.json` ([#4104](https://github.com/can1357/oh-my-pi/issues/4104)).
 - Fixed terminal `yield` results racing post-turn maintenance, which could trigger an unnecessary automatic handoff or compaction.

packages/coding-agent/src/mcp/oauth-discovery.ts

@@ -6,6 +6,10 @@
  */
 import * as AIError from "@oh-my-pi/pi-ai/error";
 import type { FetchImpl } from "@oh-my-pi/pi-ai/types";
+import { withTimeoutSignal } from "../utils/fetch-timeout";
+
+/** Per-request abort deadline for each OAuth discovery metadata fetch. */
+const DISCOVERY_FETCH_TIMEOUT_MS = 10_000;
 
 export interface OAuthEndpoints {
 	authorizationUrl: string;
@@ -346,7 +350,7 @@ function readMetadataScopes(metadata: Record<string, unknown>): string | undefin
  */
 export async function fetchResourceMetadataScopes(
 	resourceMetadataUrl: string,
-	opts?: { fetch?: FetchImpl },
+	opts?: { fetch?: FetchImpl; signal?: AbortSignal },
 ): Promise<string | undefined> {
 	const fetchImpl: FetchImpl = opts?.fetch ?? fetch;
 	try {
@@ -354,6 +358,7 @@ export async function fetchResourceMetadataScopes(
 			method: "GET",
 			headers: { Accept: "application/json" },
 			redirect: "follow",
+			signal: withTimeoutSignal(DISCOVERY_FETCH_TIMEOUT_MS, opts?.signal),
 		});
 		if (!resp.ok) return undefined;
 		const meta = (await resp.json()) as Record<string, unknown>;
@@ -371,7 +376,7 @@ export async function discoverOAuthEndpoints(
 	serverUrl: string,
 	authServerUrl?: string,
 	resourceMetadataUrl?: string,
-	opts?: { fetch?: FetchImpl; protectedResource?: string; protectedScopes?: string },
+	opts?: { fetch?: FetchImpl; protectedResource?: string; protectedScopes?: string; signal?: AbortSignal },
 ): Promise<OAuthEndpoints | null> {
 	const fetchImpl: FetchImpl = opts?.fetch ?? fetch;
 	const wellKnownPaths = [
@@ -402,6 +407,7 @@ export async function discoverOAuthEndpoints(
 				method: "GET",
 				headers: { Accept: "application/json" },
 				redirect: "follow",
+				signal: withTimeoutSignal(DISCOVERY_FETCH_TIMEOUT_MS, opts?.signal),
 			});
 			if (metaResp.ok) {
 				const meta = (await metaResp.json()) as Record<string, unknown>;
@@ -486,6 +492,7 @@ export async function discoverOAuthEndpoints(
 						method: "GET",
 						headers: { Accept: "application/json" },
 						redirect: "follow",
+						signal: withTimeoutSignal(DISCOVERY_FETCH_TIMEOUT_MS, opts?.signal),
 					});
 
 					if (response.ok) {
@@ -521,6 +528,7 @@ export async function discoverOAuthEndpoints(
 									fetch: fetchImpl,
 									protectedResource: discoveredProtectedResource,
 									protectedScopes: readMetadataScopes(metadata) ?? protectedScopes,
+									signal: opts?.signal,
 								});
 								if (discovered) return discovered;
 							}

packages/coding-agent/src/mcp/smithery-auth.test.ts

@@ -0,0 +1,29 @@
+import { afterEach, describe, expect, it, vi } from "bun:test";
+import { pollSmitheryCliAuthSession } from "./smithery-auth";
+
+type FetchInput = string | URL | Request;
+type FetchInit = RequestInit | BunFetchRequestInit;
+
+describe("pollSmitheryCliAuthSession fetch cancellation", () => {
+	afterEach(() => {
+		vi.restoreAllMocks();
+	});
+
+	it("bounds each poll request with a timeout abort signal even without a caller signal", async () => {
+		const signals: AbortSignal[] = [];
+		const fetchStub = Object.assign(
+			async (_input: FetchInput, init?: FetchInit) => {
+				if (init?.signal instanceof AbortSignal) signals.push(init.signal);
+				return Response.json({ status: "pending" });
+			},
+			{ preconnect: globalThis.fetch.preconnect },
+		);
+		vi.spyOn(globalThis, "fetch").mockImplementation(fetchStub);
+
+		const result = await pollSmitheryCliAuthSession("sess-123");
+
+		expect(result.status).toBe("pending");
+		expect(signals).toHaveLength(1);
+		expect(signals[0]?.aborted).toBe(false);
+	});
+});

packages/coding-agent/src/mcp/smithery-auth.ts

@@ -7,6 +7,7 @@ import { withTimeoutSignal } from "../utils/fetch-timeout";
 const SMITHERY_AUTH_FILENAME = "smithery.json";
 const SMITHERY_URL = process.env.SMITHERY_URL || "https://smithery.ai";
 const SMITHERY_AUTH_TIMEOUT_MS = 10_000;
+const SMITHERY_POLL_TIMEOUT_MS = 30_000;
 
 type SmitheryCliAuthSession = {
 	sessionId: string;
@@ -53,7 +54,7 @@ export async function pollSmitheryCliAuthSession(
 	signal?: AbortSignal,
 ): Promise<SmitheryCliPollResponse> {
 	const response = await fetch(`${SMITHERY_URL}/api/auth/cli/poll/${sessionId}`, {
-		signal,
+		signal: withTimeoutSignal(SMITHERY_POLL_TIMEOUT_MS, signal),
 	});
 	if (!response.ok) {
 		if (response.status === 404 || response.status === 410) {

packages/coding-agent/test/oauth-discovery.test.ts

@@ -1,4 +1,5 @@
 import { describe, expect, it } from "bun:test";
+import type { FetchImpl } from "@oh-my-pi/pi-ai/types";
 import {
 	analyzeAuthError,
 	discoverOAuthEndpoints,
@@ -641,3 +642,34 @@ describe("RFC 8414 §3.3 issuer validation", () => {
 		});
 	});
 });
+
+describe("bounded discovery fetches", () => {
+	// A fetch that never resolves on its own; it settles only when its
+	// AbortSignal fires. Pre-fix, discovery passed no signal, so this hung forever.
+	const hangingFetch: FetchImpl = (_input, init) =>
+		new Promise<Response>((_resolve, reject) => {
+			const signal = init?.signal;
+			const abort = () => reject(new DOMException("aborted", "AbortError"));
+			if (signal?.aborted) {
+				abort();
+				return;
+			}
+			signal?.addEventListener("abort", abort, { once: true });
+		});
+
+	it("aborts hanging well-known discovery fetches instead of stalling", async () => {
+		const oauth = await discoverOAuthEndpoints("https://mcp.example.test/mcp", undefined, undefined, {
+			fetch: hangingFetch,
+			signal: AbortSignal.timeout(50),
+		});
+		expect(oauth).toBeNull();
+	});
+
+	it("aborts a hanging resource_metadata fetch and returns undefined", async () => {
+		const scopes = await fetchResourceMetadataScopes(
+			"https://mcp.example.test/.well-known/oauth-protected-resource",
+			{ fetch: hangingFetch, signal: AbortSignal.timeout(50) },
+		);
+		expect(scopes).toBeUndefined();
+	});
+});