diff --git a/apps/desktop/src/app/chat/composer/mcp-suggestion-pills.tsx b/apps/desktop/src/app/chat/composer/mcp-suggestion-pills.tsx index 9280d06b8a5b1..d873a21105a5f 100644 --- a/apps/desktop/src/app/chat/composer/mcp-suggestion-pills.tsx +++ b/apps/desktop/src/app/chat/composer/mcp-suggestion-pills.tsx @@ -3,11 +3,11 @@ import { useState } from 'react' import { composerFloatingPill } from '@/components/chat/composer-dock' import { Codicon } from '@/components/ui/codicon' import { Tip } from '@/components/ui/tooltip' -import { addMcpServer, authMcpServer, getMcpOAuthFlow, removeMcpServer } from '@/hermes' +import { addMcpServer, authMcpServer, cancelMcpOAuthFlow, getMcpOAuthFlow, removeMcpServer } from '@/hermes' import { useI18n } from '@/i18n' import { triggerHaptic } from '@/lib/haptics' import { brandFor, brandGlyphStyle } from '@/lib/mcp-brands' -import { completeMcpDesktopOAuth } from '@/lib/mcp-dashboard-oauth' +import { completeMcpDesktopOAuth, McpOAuthCancelled } from '@/lib/mcp-dashboard-oauth' import { directoryEntry } from '@/lib/mcp-directory' import { prettyName } from '@/lib/text' import { useSessionSlice } from '@/lib/use-session-slice' @@ -70,15 +70,9 @@ export function McpSuggestionPills({ sessionId }: { sessionId: null | string }) await completeMcpDesktopOAuth({ serverName: known.name, start: authMcpServer, - status: async flowId => { - const flow = await getMcpOAuthFlow(flowId) - - if (cancels.get(server)) { - throw CANCELLED - } - - return flow - }, + status: getMcpOAuthFlow, + cancelled: () => cancels.get(server) === true, + cancel: cancelMcpOAuthFlow, openExternal: url => window.hermesDesktop.openExternal(url) }) } catch (error) { @@ -102,7 +96,7 @@ export function McpSuggestionPills({ sessionId }: { sessionId: null | string }) } catch (error) { setPhase(server, 'idle') - if (error !== CANCELLED) { + if (!(error instanceof McpOAuthCancelled)) { notifyError(error, copy.connectFailed(prettyName(server))) } } @@ -146,7 +140,3 @@ export function McpSuggestionPills({ sessionId }: { sessionId: null | string }) ) }) } - -// Thrown by the poll wrapper when the user cancels — the rollback has its own -// path, so the catch must swallow this rather than toast it. -const CANCELLED = Symbol('mcp-pill-cancelled') diff --git a/apps/desktop/src/components/assistant-ui/mcp-setup-tool.tsx b/apps/desktop/src/components/assistant-ui/mcp-setup-tool.tsx index 380abe99d8d5e..b84f8a07366ea 100644 --- a/apps/desktop/src/components/assistant-ui/mcp-setup-tool.tsx +++ b/apps/desktop/src/components/assistant-ui/mcp-setup-tool.tsx @@ -13,6 +13,7 @@ import { Input } from '@/components/ui/input' import { addMcpServer, authMcpServer, + cancelMcpOAuthFlow, getActionStatus, getMcpCatalog, getMcpOAuthFlow, @@ -25,7 +26,7 @@ import { useI18n } from '@/i18n' import { triggerHaptic } from '@/lib/haptics' import { AlertCircle, CheckCircle2, Loader2 } from '@/lib/icons' import { brandFor, brandGlyphStyle } from '@/lib/mcp-brands' -import { completeMcpDesktopOAuth } from '@/lib/mcp-dashboard-oauth' +import { completeMcpDesktopOAuth, McpOAuthCancelled } from '@/lib/mcp-dashboard-oauth' import { directoryEntry } from '@/lib/mcp-directory' import { prettyName } from '@/lib/text' import { cn } from '@/lib/utils' @@ -253,9 +254,8 @@ function McpSetupPending({ args }: ToolCallMessagePartProps) { cancelRef.current = false setWorking(true) - // Poll-boundary abort for the two long flows (OAuth browser round-trip, - // background install). Wrapping the status fns keeps the loops themselves - // untouched — they throw the sentinel instead of returning stale progress. + // Poll-boundary abort for the background-install loop; the OAuth flows + // carry their own cancel via completeMcpDesktopOAuth's `cancelled`. const throwIfCancelled = (value: T): T => { if (cancelRef.current) { throw CANCELLED @@ -277,7 +277,9 @@ function McpSetupPending({ args }: ToolCallMessagePartProps) { const flow = await completeMcpDesktopOAuth({ serverName: server, start: authMcpServer, - status: flowId => getMcpOAuthFlow(flowId).then(throwIfCancelled), + status: getMcpOAuthFlow, + cancelled: () => cancelRef.current, + cancel: cancelMcpOAuthFlow, openExternal: url => window.hermesDesktop.openExternal(url) }) @@ -322,7 +324,9 @@ function McpSetupPending({ args }: ToolCallMessagePartProps) { flow = await completeMcpDesktopOAuth({ serverName: known.name, start: authMcpServer, - status: flowId => getMcpOAuthFlow(flowId).then(throwIfCancelled), + status: getMcpOAuthFlow, + cancelled: () => cancelRef.current, + cancel: cancelMcpOAuthFlow, openExternal: url => window.hermesDesktop.openExternal(url) }) } catch (error) { @@ -372,7 +376,7 @@ function McpSetupPending({ args }: ToolCallMessagePartProps) { } catch (error) { // User cancel: the declined respond is already on the wire — the // abandoned flow just stops, nothing to report. - if (error === CANCELLED) { + if (error === CANCELLED || error instanceof McpOAuthCancelled) { return } diff --git a/apps/desktop/src/hermes.ts b/apps/desktop/src/hermes.ts index 80a02d303edf4..b3722926d2786 100644 --- a/apps/desktop/src/hermes.ts +++ b/apps/desktop/src/hermes.ts @@ -1151,6 +1151,16 @@ export function getMcpOAuthFlow(flowId: string): Promise { }) } +/** Cancel an in-flight MCP OAuth flow server-side, freeing the per-server + * "already in progress" slot so a retry doesn't 409. */ +export function cancelMcpOAuthFlow(flowId: string): Promise<{ ok: boolean; status: string }> { + return window.hermesDesktop.api<{ ok: boolean; status: string }>({ + ...profileScoped(), + path: `/api/mcp/oauth/flows/${encodeURIComponent(flowId)}`, + method: 'DELETE' + }) +} + export function getToolsets(): Promise { return window.hermesDesktop.api({ ...profileScoped(), diff --git a/apps/desktop/src/lib/mcp-dashboard-oauth.ts b/apps/desktop/src/lib/mcp-dashboard-oauth.ts index b26f1df11a590..32b56362de88b 100644 --- a/apps/desktop/src/lib/mcp-dashboard-oauth.ts +++ b/apps/desktop/src/lib/mcp-dashboard-oauth.ts @@ -12,10 +12,26 @@ interface CompleteOptions { start: (name: string) => Promise status: (flowId: string) => Promise openExternal: (url: string) => Promise + /** Polled between status checks. Returning true cancels: the flow is + * cancelled SERVER-SIDE (freeing the per-server in-progress slot — without + * it a retry 409s until the backend's callback timeout) and the promise + * rejects with `McpOAuthCancelled`. */ + cancelled?: () => boolean + /** Server-side flow cancel, wired to DELETE /api/mcp/oauth/flows/{id}. */ + cancel?: (flowId: string) => Promise sleep?: (milliseconds: number) => Promise maxPollFailures?: number } +/** Thrown when the caller's `cancelled()` tripped — callers branch on this to + * skip error toasts for a deliberate user cancel. */ +export class McpOAuthCancelled extends Error { + constructor() { + super('OAuth cancelled by user') + this.name = 'McpOAuthCancelled' + } +} + const defaultSleep = (milliseconds: number) => new Promise(resolve => window.setTimeout(resolve, milliseconds)) export async function completeMcpDesktopOAuth({ @@ -23,6 +39,8 @@ export async function completeMcpDesktopOAuth({ start, status, openExternal, + cancelled, + cancel, sleep = defaultSleep, maxPollFailures = 3 }: CompleteOptions): Promise { @@ -41,6 +59,13 @@ export async function completeMcpDesktopOAuth({ let pollFailures = 0 for (;;) { + if (cancelled?.()) { + // Free the backend slot before rejecting; best-effort — the flow also + // dies on its own timeout if this request is lost. + await cancel?.(started.flow_id).catch(() => {}) + throw new McpOAuthCancelled() + } + let current: McpOAuthFlow try {