fix(desktop): cancel MCP OAuth flows server-side so a retry doesn't 409

This commit is contained in:
Brooklyn Nicholson 2026-08-13 00:10:04 -05:00 committed by brooklyn!
parent 3efce9b98c
commit 6ef0fc4f62
4 changed files with 52 additions and 23 deletions

View File

@ -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')

View File

@ -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 = <T,>(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
}

View File

@ -1151,6 +1151,16 @@ export function getMcpOAuthFlow(flowId: string): Promise<McpOAuthFlow> {
})
}
/** 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<ToolsetInfo[]> {
return window.hermesDesktop.api<ToolsetInfo[]>({
...profileScoped(),

View File

@ -12,10 +12,26 @@ interface CompleteOptions {
start: (name: string) => Promise<McpOAuthFlow>
status: (flowId: string) => Promise<McpOAuthFlow>
openExternal: (url: string) => Promise<void>
/** 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<unknown>
sleep?: (milliseconds: number) => Promise<void>
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<void>(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<McpOAuthFlow> {
@ -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 {