diff --git a/agent/inline_tool_executors.py b/agent/inline_tool_executors.py index 2b5f5f75cb..10d22c7d9a 100644 --- a/agent/inline_tool_executors.py +++ b/agent/inline_tool_executors.py @@ -156,11 +156,34 @@ def _manage_connections(agent, args: dict, ctx: InlineToolContext) -> Any: from tools.connectors import manage_connections from tools.connectors.gateway import config as gateway_config - return manage_connections( + result = manage_connections( args, session_id=getattr(agent, "session_id", None), tool_call_id=ctx.tool_call_id, connection_callback=getattr(agent, "connection_callback", None), connectors_available=gateway_config.connectors_available, ) + _scope_in_connected_mcp_servers(agent, result) + return result + + +def _scope_in_connected_mcp_servers(agent, result: Any) -> None: + """Add the MCP servers this call connected to the agent's toolset selection. + + ``tool_describe``/``tool_call`` resolve names inside that selection, and it was fixed when the + agent was built, so a server registered a moment ago is otherwise "not found" for the rest of + the turn the result calls it available in. Only the selection changes; ``agent.tools`` does + not, so the sent tool schema bytes stay the same.""" + enabled = getattr(agent, "enabled_toolsets", None) + if enabled is None or "no_mcp" in enabled: # None already means every toolset + return + try: + targets = json.loads(result).get("targets") or [] + except (AttributeError, TypeError, ValueError): + return + connected = [str(t.get("name")) for t in targets if isinstance(t, dict) + and t.get("kind") == "mcp" and t.get("state") == "connected" and t.get("tools")] + added = [name for name in connected if name not in enabled] + if added: + agent.enabled_toolsets = [*enabled, *added] def _setup_mcp_shim(agent, args: dict, ctx: InlineToolContext) -> Any: diff --git a/agent/turn_context.py b/agent/turn_context.py index 64f02e7c9a..1a86573d53 100644 --- a/agent/turn_context.py +++ b/agent/turn_context.py @@ -503,6 +503,11 @@ def _refresh_mcp_tools_between_turns(agent: Any) -> None: call assembles ``tools=``. ``preserve_prefix`` keeps the tool array append-only so a flapping ``check_fn`` can't fork the cache.""" try: + # An authorization that committed after its connection card closed: same import-cost gate, + # the module is loaded only in a process that ran a connection operation. + if "tools.connectors.mcp" in sys.modules: + from tools.connectors.mcp import adopt_late_connections + adopt_late_connections(agent) # Import-cost gate: MCP tools are only registered by code that already imported # ``tools.mcp_tool`` (~0.4s); not in sys.modules => nothing to do. if not getattr(agent, "_skip_mcp_refresh", False) and "tools.mcp_tool" in sys.modules: diff --git a/apps/desktop/src/app/chat/composer/index.tsx b/apps/desktop/src/app/chat/composer/index.tsx index 9dd5c3c034..b94fd02d6f 100644 --- a/apps/desktop/src/app/chat/composer/index.tsx +++ b/apps/desktop/src/app/chat/composer/index.tsx @@ -7,7 +7,6 @@ import { useHudComposerDrag } from '@/app/hud/composer-drag' import { composerFloatingStrip, composerInputBacking } from '@/components/chat/composer-dock' import { $chatOnboardingSolo, $chatOnboardingThreadIds } from '@/components/onboarding-chat/assembly' import { OnboardingSkip } from '@/components/onboarding-chat/skip' -import { OnboardingStart } from '@/components/onboarding-chat/start' import { Button } from '@/components/ui/button' import { Slot as ContribSlot } from '@/contrib/react/slot' import { useI18n } from '@/i18n' @@ -1294,7 +1293,6 @@ export function ChatBar({ - {/* Session-scoped status stack (todos, subagents, background tasks, queue). An in-flow dock child: the dock is bottom-anchored, so it diff --git a/apps/desktop/src/app/contrib/handoff-leg.ts b/apps/desktop/src/app/contrib/handoff-leg.ts index 9412450136..cb7ea93711 100644 --- a/apps/desktop/src/app/contrib/handoff-leg.ts +++ b/apps/desktop/src/app/contrib/handoff-leg.ts @@ -6,7 +6,6 @@ import type { ClientSessionState } from '@/app/types' import type { HandoffPlan } from '@/components/onboarding-chat/setup-profile' import type { SessionMessage } from '@/types/hermes' -import { markFirstBuildSession } from './handoff-receipt' import type { AmbientGatewayRequest } from './session-rpc-dispatcher' export const BUILD_PROFILE = 'default' @@ -77,7 +76,6 @@ export async function startHandoff(deps: HandoffDeps, task: HandoffTask, recover const identity = await deps.create() receipt = { ...task, ...identity, status: 'created' } deps.save(receipt) - markFirstBuildSession(receipt.storedId) } else { const snapshot = await deps.request(receipt.owner, 'session.resume', { session_id: receipt.storedId, diff --git a/apps/desktop/src/app/contrib/handoff-receipt.ts b/apps/desktop/src/app/contrib/handoff-receipt.ts index a938697470..ae81992428 100644 --- a/apps/desktop/src/app/contrib/handoff-receipt.ts +++ b/apps/desktop/src/app/contrib/handoff-receipt.ts @@ -6,22 +6,6 @@ import type { HandoffReceipt } from './handoff-leg' // saveHandoffReceipt throws when the write does not read back, so nothing is submitted without a saved receipt. const unsavedReceipts = new Map() -export function markFirstBuildSession(storedId: string): void { - writeKey('hermes.onboarding.first-build.v1', storedId) -} - -export function endFirstBuildConnect(storedId: string): void { - writeKey('hermes.onboarding.first-build.done.v1', storedId) -} - -export function isFirstBuildSession(storedId: string | null | undefined): boolean { - return ( - !!storedId && - readKey('hermes.onboarding.first-build.v1') === storedId && - readKey('hermes.onboarding.first-build.done.v1') !== storedId - ) -} - export function handoffReceiptKey(connection: null | string, guideStoredId: string): string { return `hermes.onboarding.handoff.v1.connection.${encodeURIComponent(connection ?? 'ambient')}.profile.default.guide.${encodeURIComponent(guideStoredId)}` } diff --git a/apps/desktop/src/app/contrib/hooks/use-desktop-integrations.ts b/apps/desktop/src/app/contrib/hooks/use-desktop-integrations.ts index d398e8d4c9..4af2e45e72 100644 --- a/apps/desktop/src/app/contrib/hooks/use-desktop-integrations.ts +++ b/apps/desktop/src/app/contrib/hooks/use-desktop-integrations.ts @@ -4,6 +4,7 @@ import { useEffect, useRef } from 'react' import { closeActiveTab } from '@/app/chat/close-tab' import { commandFocusedPreview } from '@/app/chat/right-rail/preview-nav' import { openSession } from '@/app/open-session' +import { openConnectionDoneLink } from '@/components/assistant-ui/connector-tool' import { $diskPluginsScanPending } from '@/contrib/runtime-loader' import { resolveDeepLinkAction } from '@/lib/deeplink-routes' import { pathFromHermesDeepLink, resolveHermesOpenPath } from '@/lib/hermes-open-target' @@ -318,6 +319,18 @@ export function useDesktopIntegrations({ const action = resolveDeepLinkAction(payload) + // The user finished a sign-in in their browser and the portal sent them back. Show the card + // and wake its watcher; the link's status is not allowed to move any row. + if (action.type === 'connection-done') { + void openConnectionDoneLink(action.op, navigate, runtimeId => { + const viaLocalMap = storedSessionIdForNotification(runtimeId, runtimeIdByStoredSessionId.current) + + return viaLocalMap !== runtimeId ? viaLocalMap : (storedSessionIdForRuntimeId(runtimeId) ?? runtimeId) + }) + + return + } + if (action.type === 'composer-blueprint') { const slots = Object.entries(action.params || {}) .map(([k, v]) => { @@ -364,7 +377,7 @@ export function useDesktopIntegrations({ void window.hermesDesktop?.signalDeepLinkReady?.() return () => unsubscribe?.() - }, [navigate]) + }, [navigate, runtimeIdByStoredSessionId]) // ⌘W via the macOS menu accelerator → close the focused tab; if nothing is // closeable, fall back to closing the window (so ⌘W still works as the diff --git a/apps/desktop/src/app/contrib/onboarding-handoff.ts b/apps/desktop/src/app/contrib/onboarding-handoff.ts index f7e249f454..9245448247 100644 --- a/apps/desktop/src/app/contrib/onboarding-handoff.ts +++ b/apps/desktop/src/app/contrib/onboarding-handoff.ts @@ -182,8 +182,7 @@ export function useOnboardingHandoff({ const seed = await buildFirstTaskSeedMessages( setupHandoff.task, $onboardingAnswers.get(), - setupHandoff.plan, - owner + setupHandoff.plan ) const runtimeId = await runCreatePinnedTo(BUILD_PROFILE, () => diff --git a/apps/desktop/src/app/session/hooks/use-session-actions/restore-pending-connection.ts b/apps/desktop/src/app/session/hooks/use-session-actions/restore-pending-connection.ts index 7e88599326..a064f4b55d 100644 --- a/apps/desktop/src/app/session/hooks/use-session-actions/restore-pending-connection.ts +++ b/apps/desktop/src/app/session/hooks/use-session-actions/restore-pending-connection.ts @@ -39,6 +39,17 @@ export function restorePendingConnectionFromSnapshot( return { authoritativeAbsent: true, cleared: null, request: null } } + const current = $connectionRequests.get()[sessionId] + + // A resume snapshot is read once and can land after the live frames it predates. It must not + // revive a card the operation already settled, nor put back a row a newer frame has moved. + // Only the same operation can refuse it: a settled cache says nothing about the next operation + // the session opened. A settled refusal is no pending card; a newer live frame is still the + // pending card, and the caller must keep treating the session as waiting on it. + if (current?.opId === request.opId && (current.settled || current.seq > request.seq)) { + return { authoritativeAbsent: false, cleared: null, request: current.settled ? null : current } + } + setConnectionRequest(request) return { authoritativeAbsent: false, cleared: null, request } diff --git a/apps/desktop/src/components/assistant-ui/connector-tool.test.tsx b/apps/desktop/src/components/assistant-ui/connector-tool.test.tsx index 381166d359..bf48c576df 100644 --- a/apps/desktop/src/components/assistant-ui/connector-tool.test.tsx +++ b/apps/desktop/src/components/assistant-ui/connector-tool.test.tsx @@ -24,9 +24,13 @@ const PRIMARY_OWNER = { connectionId: null, profile: 'default' } const GMAIL: ConnectionTarget = { action: 'connect', connectUrl: 'https://connect.example/gmail', + connectionId: '', detail: '', + discoveryError: null, kind: 'connector', + instructions: null, name: 'gmail', + requiredEnv: [], state: 'pending', tools: [] } @@ -34,6 +38,7 @@ const GMAIL: ConnectionTarget = { const REQUEST: ConnectionRequest = { deadlineAt: 1_800_000_000, opId: 'operation-1', + seq: 0, toolCallId: 'connector-call-1', sessionId: SESSION_ID, settled: false, @@ -156,7 +161,7 @@ describe('ConnectorTool operation card', () => { expect(request).not.toHaveBeenCalledWith('connectors.connect', expect.anything()) }) - it('Try again mints a fresh link on the open operation and opens it at once', async () => { + it('Try again mints a fresh link on the open operation and never opens a browser by itself', async () => { const openExternal = vi.fn() // SAFETY: the card reads only `openExternal` from the preload bridge. window.hermesDesktop = { openExternal } as never @@ -172,8 +177,9 @@ describe('ConnectorTool operation card', () => { fireEvent.click(screen.getByRole('button', { name: 'Try again' })) await waitFor(() => { - expect(openExternal).toHaveBeenCalledWith('https://connect.example/gmail-2') + expect(request).toHaveBeenCalledTimes(1) }) + expect(openExternal).not.toHaveBeenCalled() expect(request).toHaveBeenCalledWith( 'connectors.connect', { connectors: ['gmail'], reconnect: true, session_id: SESSION_ID }, diff --git a/apps/desktop/src/components/assistant-ui/connector-tool.tsx b/apps/desktop/src/components/assistant-ui/connector-tool.tsx index 19222547d3..5cd582a9bf 100644 --- a/apps/desktop/src/components/assistant-ui/connector-tool.tsx +++ b/apps/desktop/src/components/assistant-ui/connector-tool.tsx @@ -1,9 +1,10 @@ import type { ToolCallMessagePartProps } from '@assistant-ui/react' import type { ConnectionTargetState } from '@hermes/shared' import { useStore } from '@nanostores/react' -import { useEffect, useMemo, useState } from 'react' +import { type RefObject, useEffect, useMemo, useRef, useState } from 'react' import { useSessionView } from '@/app/chat/session-view' +import { sessionRoute } from '@/app/routes' import { resolveSessionOwner } from '@/app/session/hooks/use-session-actions/utils' import { ToolFallback } from '@/components/assistant-ui/tool/fallback' import { Button } from '@/components/ui/button' @@ -12,12 +13,14 @@ import { useI18n } from '@/i18n' import { connectorAuthorizationUrl, connectorCalls, + connectorIconUrl, connectorText, connectorTitle, connectorToolName, recordOf } from '@/lib/connector-tools' import { + $connectionRequests, type ConnectionRequest, type ConnectionTarget, continueConnectionRequest, @@ -29,11 +32,119 @@ import { $activeGatewayProfile } from '@/store/profile' import { assertSessionOwnerResolved } from '@/store/session-owner-resolution' import { isSessionOwnerRoute } from '@/store/session-request-router' -interface ConnectorOwner { +/** Which backend owns the session whose operation this card drives. */ +export interface ConnectorOwner { connectionId: null | string profile: string } +/** Resolve that owner for one stored session, or null when it cannot be resolved. */ +async function connectionOwnerFor(sessionId: string, method: string): Promise { + const ambientProfile = $activeGatewayProfile.get() + + try { + const scope = await resolveSessionOwner(sessionId) + assertSessionOwnerResolved(scope, { method, sessionId }) + + return { + connectionId: isSessionOwnerRoute(scope) ? scope.connectionId : null, + profile: isSessionOwnerRoute(scope) ? scope.profile : scope || ambientProfile + } + } catch { + return null + } +} + +/** Resolve that owner. Null until it resolves and null when it cannot: a card RPC must reach the + * gateway that holds the operation, never whichever one the window happens to have in front. */ +export function useConnectionOwner(sessionId: null | string, active: boolean): ConnectorOwner | null { + const [owner, setOwner] = useState(null) + + useEffect(() => { + if (!sessionId || !active) { + setOwner(null) + + return + } + + let cancelled = false + + void connectionOwnerFor(sessionId, 'connectors.connect').then(resolved => { + if (!cancelled) { + setOwner(resolved) + } + }) + + return () => { + cancelled = true + } + }, [active, sessionId]) + + return owner +} + +/** The browser leg of a connection came back through `hermes://connections/done`. Show the session + * that opened the operation and tell its backend to read the account now instead of at its next + * tick. Nothing in the link is trusted to move a row: the op id only names which card to show, and + * the backend reads the account itself. An operation this window holds no card for, or one that + * already settled, is ignored: the tab can come back long after Continue, and a stale link must + * not pull the user away from where they are. */ +export async function openConnectionDoneLink( + op: string, + navigate: (to: string) => void, + storedSessionIdFor: (runtimeSessionId: string) => string +): Promise { + const request = Object.values($connectionRequests.get()).find(entry => entry.opId === op) + + if (!request?.sessionId || request.settled) { + return + } + + const storedId = storedSessionIdFor(request.sessionId) + navigate(sessionRoute(storedId)) + + const owner = await connectionOwnerFor(storedId, 'connectors.operation.wake') + + if (!owner) { + return + } + + try { + await requestGatewayForAgent(owner.connectionId, owner.profile, 'connectors.operation.wake', { + op_id: op, + session_id: request.sessionId + }) + } catch { + // The wake only shortens the wait. The operation can settle and leave the live registry between + // the link and this RPC (4004); the watcher reads the account at its next tick regardless. + } +} + +/** Try again for one target of the open operation: one RPC, and the fresh link when the backend + * minted one. The backend re-mints only what is actually dead. */ +export async function reissueConnectionTarget( + owner: ConnectorOwner, + request: ConnectionRequest, + name: string +): Promise { + const reply = await requestGatewayForAgent( + owner.connectionId, + owner.profile, + 'connectors.connect', + { + connectors: [name], + reconnect: true, + session_id: request.sessionId + }, + 45000 + ) + + const rows = recordOf(reply).targets + const minted = Array.isArray(rows) ? rows.map(recordOf).find(row => connectorText(row.name) === name) : undefined + + return connectorAuthorizationUrl(minted?.connect_url) +} + /** Names requested by a manage_connections part, including an event-projected row. */ function requestedConnectorNames(args: ToolCallMessagePartProps['args']): string[] { const connectors = recordOf(args).connectors @@ -79,40 +190,7 @@ export function ConnectorTool(props: ToolCallMessagePartProps) { const live = !untargetedStatus && connectionRequestOwnsPart(props, request) // Owner routes and hints are keyed by the stored id, not the runtime id the events carry. - const ownerSessionId = storedId - const [owner, setOwner] = useState(null) - - useEffect(() => { - if (!ownerSessionId || !live) { - setOwner(null) - - return - } - - let cancelled = false - const ambientProfile = $activeGatewayProfile.get() - - void resolveSessionOwner(ownerSessionId) - .then(scope => { - assertSessionOwnerResolved(scope, { method: 'connectors.connect', sessionId: ownerSessionId }) - - if (!cancelled) { - setOwner({ - connectionId: isSessionOwnerRoute(scope) ? scope.connectionId : null, - profile: isSessionOwnerRoute(scope) ? scope.profile : scope || ambientProfile - }) - } - }) - .catch(() => { - if (!cancelled) { - setOwner(null) - } - }) - - return () => { - cancelled = true - } - }, [live, ownerSessionId]) + const owner = useConnectionOwner(storedId, live) if (!live || !request) { return @@ -141,7 +219,7 @@ const connected = (copy: ConnectorCopy): SettledWord => ({ meta: copy.connected, const notConnected = (copy: ConnectorCopy): SettledWord => ({ meta: copy.notConnected }) const skipped = (copy: ConnectorCopy): SettledWord => ({ meta: copy.skipped }) -const CONNECTOR_CARD_PHASES = { +export const CONNECTOR_CARD_PHASES = { connected: { mark: 'connected', resolved: true, settled: connected, verb: 'none' }, expired: { mark: 'idle', resolved: false, settled: notConnected, verb: 'reissue' }, failed: { mark: 'idle', resolved: false, settled: notConnected, verb: 'reissue' }, @@ -152,7 +230,58 @@ const CONNECTOR_CARD_PHASES = { unavailable: { mark: 'idle', resolved: true, settled: notConnected, verb: 'none' } } satisfies Record -const MARK_LABEL = { +// A disabled verb (a working row, a waiting row with no link yet) refuses focus, and the keyboard +// would land on the document body; so the first control that can take it, else the row itself. +const FOCUSABLE_IN_ROW = 'button:not([disabled]), [href], input:not([disabled])' +// The user is typing a credential; a row moving elsewhere on the card must not take the keyboard. +const EDITABLE = 'input, textarea, select, [contenteditable]:not([contenteditable="false"])' + +function focusChangedRow(card: HTMLElement, name: string): void { + const row = [...card.querySelectorAll('[data-connector-row]')].find( + node => node.dataset.connectorRow === name + ) + + ;(row?.querySelector(FOCUSABLE_IN_ROW) ?? row)?.focus() +} + +/** Move focus to the row the backend changed. Only while the card already holds focus, and never + * out of a field the user is typing in — a transition the user is not looking at must not take + * the keyboard away from wherever they are. */ +export function useConnectorFocusHandoff( + targets: readonly ConnectionTarget[], + cardRef: RefObject +): void { + const seen = useRef | null>(null) + const states = targets.map(target => `${target.name}=${target.state}`).join('|') + + // The ref holds what the last frame said, for comparison only: nothing renders from it, so it + // cannot lag a render the way a mirrored atom would. + // eslint-disable-next-line no-restricted-syntax + useEffect(() => { + const previous = seen.current + seen.current = new Map(targets.map(target => [target.name, target.state])) + + const card = cardRef.current + + const moved = targets.find(target => { + const before = previous?.get(target.name) + + return before !== undefined && before !== target.state + }) + + const active = document.activeElement + + if (!previous || !moved || !card?.contains(active) || active?.matches(EDITABLE)) { + return + } + + focusChangedRow(card, moved.name) + // The target states are the whole input; `states` changes exactly when one of them moves. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [states]) +} + +export const MARK_LABEL = { connected: (copy: ConnectorCopy) => copy.connected, idle: (copy: ConnectorCopy) => copy.notConnected, waiting: (copy: ConnectorCopy) => copy.waiting @@ -168,33 +297,18 @@ export function ConnectorOffer({ owner, request }: ConnectorOfferProps) { const copy = t.connectors const [reissuing, setReissuing] = useState>(new Set()) const unresolved = request.targets.some(target => !CONNECTOR_CARD_PHASES[target.state].resolved) + // A DOM handle for the focus handoff, never rendered state. + const cardRef = useRef(null) - // Try again is one RPC on the open operation; the backend re-mints only a dead link. The fresh link - // opens at once, and the update frame then paints the row as waiting. A refused re-mint is a click - // that changed nothing, so it gets a toast; the row stays as it was. + useConnectorFocusHandoff(request.targets, cardRef) + + // The update frame paints the row as waiting with the fresh link; the user opens it from the row. + // A refused re-mint is a click that changed nothing, so it gets a toast; the row stays as it was. const reissue = async (target: ConnectionTarget): Promise => { setReissuing(current => new Set(current).add(target.name)) try { - const reply = await requestGatewayForAgent( - owner.connectionId, - owner.profile, - 'connectors.connect', - { - connectors: [target.name], - reconnect: true, - session_id: request.sessionId - }, - 45000 - ) - - const rows = recordOf(reply).targets - const minted = Array.isArray(rows) ? rows.map(recordOf).find(row => connectorText(row.name) === target.name) : undefined - const url = connectorAuthorizationUrl(minted?.connect_url) - - if (url) { - void window.hermesDesktop?.openExternal?.(url) - } + await reissueConnectionTarget(owner, request, target.name) } catch (error) { notifyError(error, copy.connectErrorFor(connectorTitle(target.name))) } finally { @@ -216,7 +330,7 @@ export function ConnectorOffer({ owner, request }: ConnectorOfferProps) { return ( +
{request.targets.map(target => { const phase = CONNECTOR_CARD_PHASES[target.state] @@ -256,7 +370,7 @@ export function ConnectorOffer({ owner, request }: ConnectorOfferProps) { return ( { - void openFirstBuildLinks( - storedId, - { toolCallId, toolName, args, result }, - { open: window.hermesDesktop?.openExternal ? url => window.hermesDesktop.openExternal(url) : undefined } - ) - }, [storedId, toolCallId, toolName, args, result]) - - useEffect(() => { - flushFirstBuildNote(storedId, newestToolCallId, busy, text => - requestComposerSubmit(text, { displayKind: 'hidden', target }) - ) - }, [storedId, newestToolCallId, busy, pendingNote, target]) - - useEffect(() => { - if (newestToolCallId !== toolCallId) { - return - } - - return watchFirstBuildRows(storedId, runtimeId, { toolCallId, toolName, args, result }, (method, params) => - requestGatewayForAgent(connectionId, profile, method, params, 45000) - ) - }, [storedId, runtimeId, toolCallId, toolName, args, result, connectionId, profile, newestToolCallId]) - - return ( -
- {connections[storedId]?.rows.map(row => ( -
- {connectorTitle(row.connector)} - - {row.phase === 'connected' ? ( - <> - ✓ - {t.connectors.connected} - - ) : row.phase === 'timeout' ? ( - t.connectors.notConnected - ) : row.phase === 'error' ? ( - row.error === 'unavailable' ? ( - t.connectors.notAvailable - ) : ( - t.connectors.statusError - ) - ) : ( - t.connectors.waitingSignIn - )} - - {row.connectUrl && row.phase !== 'connected' && !window.hermesDesktop?.openExternal ? ( - - ) : null} -
- ))} -
- ) -} diff --git a/apps/desktop/src/components/assistant-ui/mcp-setup-tool.test.tsx b/apps/desktop/src/components/assistant-ui/mcp-setup-tool.test.tsx index ad7ef6b646..e419db7fb8 100644 --- a/apps/desktop/src/components/assistant-ui/mcp-setup-tool.test.tsx +++ b/apps/desktop/src/components/assistant-ui/mcp-setup-tool.test.tsx @@ -1,26 +1,44 @@ import type { ToolCallMessagePartProps } from '@assistant-ui/react' -import { cleanup, render, screen } from '@testing-library/react' +import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react' import { atom } from 'nanostores' import { afterEach, describe, expect, it, vi } from 'vitest' import { type SessionView, SessionViewProvider } from '@/app/chat/session-view' import { McpSetupPending, McpSetupTool } from '@/components/assistant-ui/mcp-setup-tool' import { I18nProvider } from '@/i18n' -import { $connectionRequests, type ConnectionRequest, setConnectionRequest } from '@/store/connection-request' +import { + $connectionRequests, + type ConnectionRequest, + type ConnectionTarget, + setConnectionRequest +} from '@/store/connection-request' +import { $gateway, setPrimaryGateway } from '@/store/gateway' const SESSION_ID = 'session-1' +const LINEAR: ConnectionTarget = { + action: 'install', + connectUrl: null, + connectionId: '', + detail: '', + discoveryError: null, + instructions: null, + kind: 'mcp', + name: 'linear', + requiredEnv: [], + state: 'pending', + tools: [] +} + const REQUEST: ConnectionRequest = { deadlineAt: 1_800_000_000, opId: 'operation-1', + seq: 0, toolCallId: 'mcp-call-1', sessionId: SESSION_ID, settled: false, settledBy: null, - targets: [ - { action: 'install', connectUrl: null, detail: '', kind: 'mcp', name: 'linear', state: 'pending', tools: [] }, - { action: 'install', connectUrl: null, detail: '', kind: 'mcp', name: 'postgres', state: 'pending', tools: [] } - ] + targets: [LINEAR, { ...LINEAR, name: 'postgres' }] } const ARGS = { @@ -83,21 +101,37 @@ function renderTool(result?: ToolCallMessagePartProps['result']) { afterEach(() => { cleanup() $connectionRequests.set({}) + $gateway.set(null) + setPrimaryGateway(null) vi.clearAllMocks() }) describe('the MCP setup card', () => { - it('shows one row per target with its verb, and Continue below', () => { - setConnectionRequest(REQUEST) + it('opens required details from the row action and sends the approved environment', async () => { + const rpc = vi.fn().mockResolvedValue({ status: 'ok', settled: false }) + const target = { + ...LINEAR, + instructions: 'Create a Linear API key.', + requiredEnv: [{ default: 'workspace', name: 'LINEAR_TEAM', prompt: 'Team', required: true, secret: false }] + } + // SAFETY: the card calls only `request`; no other gateway client surface is exercised here. + // respondToConnectionRequest reads $gateway, which only applyActive publishes; set it directly. + $gateway.set({ request: rpc } as never) + setConnectionRequest({ ...REQUEST, targets: [target] }) renderTool() + fireEvent.click(screen.getByRole('button', { name: 'Install' })) - expect(screen.getByText('Add MCP servers')).toBeTruthy() - expect(screen.getByText('Linear', { selector: 'span' })).toBeTruthy() - expect(screen.getByText('Postgres', { selector: 'span' })).toBeTruthy() - expect(screen.getAllByRole('button', { name: 'Install' })).toHaveLength(2) - expect(screen.getByRole('button', { name: 'Continue' })).toBeTruthy() - expect(screen.queryByRole('button', { name: 'Not now' })).toBeNull() + expect(screen.getByText('Set up Linear')).toBeTruthy() + expect(screen.getByText('Create a Linear API key.')).toBeTruthy() + expect(screen.getByLabelText('Team').getAttribute('value')).toBe('workspace') + fireEvent.click(screen.getByRole('button', { name: 'Connect' })) + await waitFor(() => expect(rpc).toHaveBeenCalledTimes(1)) + expect(rpc).toHaveBeenCalledWith('connection.respond', { + op_id: 'operation-1', + result: { targets: [{ env: { LINEAR_TEAM: 'workspace' }, name: 'linear', status: 'approved' }] }, + session_id: SESSION_ID + }) }) it('lists every target once settled, in the same three words as the connector card', () => { 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 7fbc323a10..5e762f07b6 100644 --- a/apps/desktop/src/components/assistant-ui/mcp-setup-tool.tsx +++ b/apps/desktop/src/components/assistant-ui/mcp-setup-tool.tsx @@ -2,33 +2,36 @@ import { type ToolCallMessagePartProps, useAuiState } from '@assistant-ui/react' import { useStore } from '@nanostores/react' -import { useEffect, useMemo, useState } from 'react' +import { useEffect, useMemo, useRef, useState } from 'react' -import { capabilityScoped } from '@/api/client' import { useSessionView } from '@/app/chat/session-view' +import { + connectionRequestOwnsPart, + CONNECTOR_CARD_PHASES, + type ConnectorOwner, + MARK_LABEL, + reissueConnectionTarget, + useConnectionOwner, + useConnectorFocusHandoff +} from '@/components/assistant-ui/connector-tool' import { ToolFallback } from '@/components/assistant-ui/tool/fallback' import { WIDGET_SHELL_CLASS } from '@/components/chat/widget-shell' import { Button } from '@/components/ui/button' -import { ConnectorCard, ConnectorRow, type ConnectorRowMark, ConnectorSummary } from '@/components/ui/connector-card' -import { getActionStatus, getMcpCatalog, installMcpCatalogEntry, type McpCatalogEntry, setMcpServerEnabled } from '@/hermes' +import { ConnectorCard, ConnectorRow, type ConnectorRowAction, ConnectorSummary } from '@/components/ui/connector-card' +import { SetupFormDialog } from '@/components/ui/setup-form-dialog' import { useI18n } from '@/i18n' import { connectorText, type McpTarget, mcpTargets } from '@/lib/connector-tools' -import { triggerHaptic } from '@/lib/haptics' import { Loader2 } from '@/lib/icons' -import { isSubmitEnter } from '@/lib/ime' -import { completeMcpDesktopOAuth, McpOAuthCancelled } from '@/lib/mcp-dashboard-oauth' import { prettyName } from '@/lib/text' import { cn } from '@/lib/utils' import { type ConnectionRequest, type ConnectionTarget, - type ConnectionTargetOutcome, + type ConnectionTargetState, continueConnectionRequest, respondToConnectionRequest, sessionConnectionRequest } from '@/store/connection-request' -import { $gateway } from '@/store/gateway' -import { reconnectAction } from '@/store/gateway-reconnect' import { notifyError } from '@/store/notifications' import { invalidateMcpSuggestionIndex } from '@/store/suggestion-providers/mcp' @@ -38,8 +41,6 @@ import { parseMaybeObject } from './tool/fallback-model/format' type SetupAction = McpTarget['action'] type SetupCopy = ReturnType['t']['assistant']['mcpSetup'] -const CATALOG_INSTALL_POLL_MS = 1500 - const SHELL_CLASS = `${WIDGET_SHELL_CLASS} text-[length:var(--conversation-text-font-size)] text-(--ui-text-primary)` const TITLE = { @@ -60,9 +61,36 @@ const DONE = { install: (copy: SetupCopy, server: string) => copy.installed(server) } satisfies Record string> -// Mirrors `RESOLVED_STATES` in tools/connectors/contract.py. -const resolved = (target: ConnectionTarget): boolean => - target.state === 'connected' || target.state === 'skipped' || target.state === 'unavailable' +/** The row's one verb. `approve` is the user's consent, `working` is the backend acting on it, `open` + * is a sign-in link the backend already minted, `reissue` asks for a fresh attempt. */ +type McpVerb = 'approve' | 'none' | 'open' | 'reissue' | 'working' + +const MCP_VERBS = { + connected: 'none', + expired: 'reissue', + failed: 'reissue', + initiated: 'open', + not_connected: 'none', + pending: 'approve', + skipped: 'none', + unavailable: 'none' +} satisfies Record + +// Two states read differently per action. A pending authorize is the backend still minting the link, +// so there is nothing for the user to consent to. An initiated row with a link is that link waiting +// to be opened, whatever the action: an install of an OAuth entry reaches it too. An initiated row +// with no link is the backend working. +const rowVerb = (target: ConnectionTarget, action: SetupAction): McpVerb => { + if (action === 'authorize') { + return target.state === 'pending' ? 'none' : MCP_VERBS[target.state] + } + + if (target.state === 'initiated') { + return target.connectUrl ? 'open' : 'working' + } + + return MCP_VERBS[target.state] +} function readSetupAction(args: unknown): SetupAction { const [target] = mcpTargets('manage_connections', parseMaybeObject(args)) @@ -74,6 +102,39 @@ interface SettledTarget { name: string state: string tools: number + toolsUnavailable: boolean +} + +/** A settled operation is a static per-target summary: one word per row, no controls. */ +function McpSetupSummary({ action, rows }: { action: SetupAction; rows: SettledTarget[] }) { + const { t } = useI18n() + const copy = t.assistant.mcpSetup + + return ( +
+ {rows.map(row => { + const title = prettyName(row.name) + const connected = row.state === 'connected' + + const line = connected + ? row.toolsUnavailable + ? `${DONE[action](copy, title)} · ${t.connectors.authorizedToolsUnavailable}` + : DONE[action](copy, title) + : row.state === 'skipped' + ? t.connectors.skipped + : t.connectors.notConnected + + return ( + 0 ? `${line} · ${copy.toolCount(row.tools)}` : line} + tone={connected ? 'ok' : undefined} + /> + ) + })} +
+ ) } function readSetupResult(result: unknown): SettledTarget[] { @@ -84,7 +145,14 @@ function readSetupResult(result: unknown): SettledTarget[] { const name = connectorText(target.name) return name - ? [{ name, state: connectorText(target.state) ?? '', tools: Array.isArray(target.tools) ? target.tools.length : 0 }] + ? [ + { + name, + state: connectorText(target.state) ?? '', + tools: Array.isArray(target.tools) ? target.tools.length : 0, + toolsUnavailable: Boolean(connectorText(target.discovery_error)) + } + ] : [] }) } @@ -108,73 +176,110 @@ const McpSetupLive = (props: ToolCallMessagePartProps) => { } function McpSetupSettled({ args, result }: ToolCallMessagePartProps) { - const { t } = useI18n() - const copy = t.assistant.mcpSetup const action = useMemo(() => readSetupAction(args), [args]) - const targets = useMemo(() => readSetupResult(result), [result]) + const rows = useMemo(() => readSetupResult(result), [result]) - return ( -
- {targets.map(target => { - const title = prettyName(target.name) - const connected = target.state === 'connected' - - const line = connected - ? DONE[action](copy, title) - : target.state === 'skipped' - ? t.connectors.skipped - : t.connectors.notConnected - - return ( - 0 ? `${line} · ${copy.toolCount(target.tools)}` : line} - tone={connected ? 'ok' : undefined} - /> - ) - })} -
- ) + return } -export function McpSetupPending({ args }: ToolCallMessagePartProps) { +export function McpSetupPending(props: ToolCallMessagePartProps) { const { t } = useI18n() const copy = t.assistant.mcpSetup + const view = useSessionView() // Use the rendering transcript's session, not the globally active one. - const sessionId = useStore(useSessionView().$runtimeId) + const sessionId = useStore(view.$runtimeId) + // Owner routes and hints are keyed by the stored id, not the runtime id the events carry. + const storedId = useStore(view.$storedId) const $request = useMemo(() => sessionConnectionRequest(sessionId), [sessionId]) const request = useStore($request) - const action = useMemo(() => readSetupAction(args), [args]) - const title = TITLE[action](copy) + const action = useMemo(() => readSetupAction(props.args), [props.args]) + // The session's operation belongs to one tool call; another call's request never paints here. + const live = connectionRequestOwnsPart(props, request) + const owner = useConnectionOwner(storedId, live) // `tool.start` arrives before `connection.request`. - if (!request) { + if (!live || !request) { return (
- {title} + {TITLE[action](copy)}
) } - const open = request.targets.filter(target => !resolved(target)) + return +} + +interface McpSetupOfferProps { + action: SetupAction + /** Null until the session's owner resolves; only Try again needs it, so the rest of the card works. */ + owner: ConnectorOwner | null + request: ConnectionRequest +} + +/** The card is a projection of the operation: one row per target, one verb per row, Continue below. */ +export function McpSetupOffer({ action, owner, request }: McpSetupOfferProps) { + const { t } = useI18n() + const copy = t.assistant.mcpSetup + const [reissuing, setReissuing] = useState>(new Set()) + const unresolved = request.targets.some(target => !CONNECTOR_CARD_PHASES[target.state].resolved) + + const settledRows = request.targets.map(target => ({ + name: target.name, + state: target.state, + tools: target.tools.length, + toolsUnavailable: Boolean(target.discoveryError) + })) + + // A DOM handle for the focus handoff, never rendered state. + const cardRef = useRef(null) + + useConnectorFocusHandoff(request.targets, cardRef) + + // Try again is one RPC on the open operation. An authorize target comes back with a fresh link, + // which opens at once; install and enable simply run again and report through connection.update. + const reissue = async (name: string): Promise => { + if (!owner) { + return + } + + setReissuing(current => new Set(current).add(name)) + + try { + // The re-minted link reaches the row through connection.update; the user opens it from the row. + await reissueConnectionTarget(owner, request, name) + } catch (error) { + notifyError(error, copy.failed(prettyName(name))) + } finally { + setReissuing(current => { + const next = new Set(current) + next.delete(name) + + return next + }) + } + } + + if (request.settled) { + return + } return ( -
- +
+ {request.targets.map(target => ( void reissue(target.name)} + reissueBlocked={!owner || reissuing.size > 0} + reissuing={reissuing.has(target.name)} request={request} - single={open.length === 1 && open[0] === target} target={target} /> ))} - {open.length > 0 ? ( + {unresolved ? (
- ) -} diff --git a/apps/desktop/src/components/ui/connector-card.test.tsx b/apps/desktop/src/components/ui/connector-card.test.tsx index e5ae10b903..02ef3c2934 100644 --- a/apps/desktop/src/components/ui/connector-card.test.tsx +++ b/apps/desktop/src/components/ui/connector-card.test.tsx @@ -3,6 +3,7 @@ import { afterEach, describe, expect, it, vi } from 'vitest' import { ConnectorCard, ConnectorRow, type ConnectorRowProps } from './connector-card' import { connectorLogoSource } from './connector-logo' +import { SetupFormDialog } from './setup-form-dialog' afterEach(cleanup) @@ -39,7 +40,12 @@ describe('a row in the card', () => { markLabel: 'Waiting for your browser…' }) - expect(screen.getByRole('img', { name: 'Waiting for your browser…' })).toBeTruthy() + const announced = screen.getByRole('status') + + // A row that flips while the user reads the card has to be heard, not just seen. + expect(announced.getAttribute('aria-live')).toBe('polite') + // The cue repeats the mark's own word here; it is announced once. + expect(announced.textContent).toBe('Waiting for your browser…') expect(screen.getByRole('button', { name: 'Connect' }).hasAttribute('disabled')).toBe(false) }) @@ -56,29 +62,42 @@ describe('a row in the card', () => { renderRow({ mark: 'connected', markLabel: 'Connected' }) expect(screen.queryAllByRole('button')).toHaveLength(0) - expect(screen.getByRole('img', { name: 'Connected' })).toBeTruthy() + expect(screen.getByRole('status').textContent).toBe('Connected') }) }) describe('credentials under a row', () => { - const envFields = [{ name: 'LINEAR_API_KEY', prompt: 'API key', required: true }] + const fields = [ + { default: 'https://api.linear.app', name: 'LINEAR_URL', prompt: 'API URL', required: true, secret: false }, + { default: '', name: 'LINEAR_API_KEY', prompt: 'API key', required: true, secret: true } + ] + const copy = { + cancel: 'Cancel', + connect: 'Connect', + openInBrowser: 'Open in browser', + setup: (server: string) => `Set up ${server}` + } it('stay out of the way until the row asks for them', () => { - renderRow({ envFields }) + render() - expect(screen.queryByLabelText('API key *')).toBeNull() + expect(screen.queryByLabelText('API key')).toBeNull() }) - it('report each keystroke so the caller owns the draft, and mask the value', () => { - const onEnvChange = vi.fn() + it('renders plain and masked inputs, prefills plain defaults, and reports the complete draft', () => { + const onConnect = vi.fn() - renderRow({ envFields, envOpen: true, envRequired: 'Fill in the required credentials first', onEnvChange }) + render() - const input = screen.getByLabelText('API key *') + const plain = screen.getByLabelText('API URL') + const secret = screen.getByLabelText('API key') - fireEvent.change(input, { target: { value: 'lin_abc' } }) - expect(onEnvChange).toHaveBeenCalledWith('LINEAR_API_KEY', 'lin_abc') - expect(input.getAttribute('type')).toBe('password') + expect(plain.getAttribute('type')).toBe('text') + expect(plain.getAttribute('value')).toBe('https://api.linear.app') + expect(secret.getAttribute('type')).toBe('password') + fireEvent.change(secret, { target: { value: 'lin_abc' } }) + fireEvent.click(screen.getByRole('button', { name: 'Connect' })) + expect(onConnect).toHaveBeenCalledWith({ LINEAR_API_KEY: 'lin_abc', LINEAR_URL: 'https://api.linear.app' }) }) }) diff --git a/apps/desktop/src/components/ui/connector-card.tsx b/apps/desktop/src/components/ui/connector-card.tsx index 1483656d00..54ae4cc4dd 100644 --- a/apps/desktop/src/components/ui/connector-card.tsx +++ b/apps/desktop/src/components/ui/connector-card.tsx @@ -4,7 +4,6 @@ import { SCAFFOLD_META_CLASS, ScaffoldRow } from '@/components/chat/scaffold-row import { WIDGET_SHELL_CLASS } from '@/components/chat/widget-shell' import { Button } from '@/components/ui/button' import { ConnectorLogo, type ConnectorLogoSubject } from '@/components/ui/connector-logo' -import { Input } from '@/components/ui/input' import { Check, CircleIcon, Loader2 } from '@/lib/icons' import { cn } from '@/lib/utils' @@ -24,27 +23,15 @@ export interface ConnectorRowAction { onClick: () => void } -export interface ConnectorCardField { - name: string - prompt?: string - required?: boolean -} - export interface ConnectorRowProps { /** Absent once the row is resolved: there is nothing left to offer. */ action?: ConnectorRowAction connector: ConnectorLogoSubject /** The one quiet line after the name; the waiting row's "in your browser" cue. */ cue?: string - envDraft?: Record - envFields?: ConnectorCardField[] - /** The caller reveals fields after a refused credential. */ - envOpen?: boolean - envRequired?: string mark: ConnectorRowMark /** What a screen reader gets for the mark. */ markLabel: string - onEnvChange?: (key: string, value: string) => void } const SHELL_CLASS = `${WIDGET_SHELL_CLASS} text-[length:var(--conversation-text-font-size)] text-(--ui-text-primary)` @@ -52,7 +39,8 @@ const SHELL_CLASS = `${WIDGET_SHELL_CLASS} text-[length:var(--conversation-text- const MARKS = { connected: { Icon: Check, className: 'text-emerald-600 dark:text-emerald-400' }, idle: { Icon: CircleIcon, className: 'text-(--ui-text-quaternary)' }, - waiting: { Icon: Loader2, className: 'animate-spin text-primary' } + // The spin is the only motion in the card; a reduced-motion reader keeps the mark, without it. + waiting: { Icon: Loader2, className: 'animate-spin text-primary motion-reduce:animate-none' } } satisfies Record export function ConnectorCard({ children, title }: { children: ReactNode; title: string }) { @@ -64,30 +52,24 @@ export function ConnectorCard({ children, title }: { children: ReactNode; title: ) } -export function ConnectorRow({ - action, - connector, - cue, - envDraft = {}, - envFields = [], - envOpen = false, - envRequired, - mark, - markLabel, - onEnvChange -}: ConnectorRowProps) { +export function ConnectorRow({ action, connector, cue, mark, markLabel }: ConnectorRowProps) { const { Icon, className } = MARKS[mark] - const fields = envOpen ? envFields : [] + // The mark and the cue are one live region, so a row that flips is announced instead of only seen. + // The cue often repeats the mark's own word; then it is said once. + const announcement = cue && cue !== markLabel ? `${markLabel}. ${cue}` : markLabel return ( -
+
- + + {announcement} {connector.title || connector.name} - {cue} + + {cue} + {action ? ( @@ -106,25 +88,6 @@ export function ConnectorRow({
- {fields.length > 0 && ( -
- {envRequired ?

{envRequired}

: null} - {fields.map(env => ( - - ))} -
- )}
) } diff --git a/apps/desktop/src/components/ui/connector-logo.tsx b/apps/desktop/src/components/ui/connector-logo.tsx index f571f806c0..8a9506cd50 100644 --- a/apps/desktop/src/components/ui/connector-logo.tsx +++ b/apps/desktop/src/components/ui/connector-logo.tsx @@ -10,6 +10,8 @@ import { Favicon } from './favicon' export interface ConnectorLogoSubject { docs?: string homepage?: string + /** The vendor's own mark, served as a public SVG; connector rows always carry one. */ + iconUrl?: string name: string title?: string url?: null | string @@ -58,20 +60,34 @@ export function connectorLogoSource(subject: ConnectorLogoSubject): string { /** * A connector's mark, resolved as far as it goes. * - * Curated brand glyph → the product's own favicon → the monogram every other - * unknown name in the app falls back to. The middle rung is what keeps the - * long tail from all looking alike: a curated icon set is a couple dozen names - * and a public registry is thousands, so something we ship no icon for still - * arrives wearing its own logo. + * Curated brand glyph → the vendor's icon → the product's own favicon → the + * monogram every other unknown name in the app falls back to. The middle rungs + * are what keep the long tail from all looking alike: a curated icon set is a + * couple dozen names and a public registry is thousands, so something we ship + * no icon for still arrives wearing its own logo. + * + * The vendor icon is a plain image: its host sends no CORS header, so nothing + * here may read it, and an unknown slug answers a grey placeholder with a 200, + * so a load event never tells us whether a logo exists. */ export function ConnectorLogo({ className, connector }: { className?: string; connector: ConnectorLogoSubject }) { const label = connector.title || connector.name const brand = brandFor(connector.name) - const site = brand ? '' : connectorLogoSource(connector) + const icon = brand ? '' : (connector.iconUrl ?? '') + const site = brand || icon ? '' : connectorLogoSource(connector) return ( - - {site ? : undefined} + + {icon ? ( + + ) : site ? ( + + ) : undefined} ) } diff --git a/apps/desktop/src/components/ui/setup-field-list.tsx b/apps/desktop/src/components/ui/setup-field-list.tsx new file mode 100644 index 0000000000..ae19a1448c --- /dev/null +++ b/apps/desktop/src/components/ui/setup-field-list.tsx @@ -0,0 +1,43 @@ +import { Field, FieldHint } from '@/components/ui/field' +import { Input } from '@/components/ui/input' +import { useI18n } from '@/i18n' + +export interface SetupField { + name: string + prompt?: string + required: boolean + secret: boolean + default: string +} + +interface SetupFieldListProps { + fields: SetupField[] + draft: Record + onChange: (name: string, value: string) => void + disabled?: boolean +} + +export function SetupFieldList({ disabled, draft, fields, onChange }: SetupFieldListProps) { + const { t } = useI18n() + + return ( +
+ {fields.map(field => { + const id = `setup-field-${field.name}` + + return ( + + onChange(field.name, event.currentTarget.value)} + type={field.secret ? 'password' : 'text'} + value={draft[field.name] ?? ''} + /> + {field.required ? {t.connectors.required} : null} + + ) + })} +
+ ) +} diff --git a/apps/desktop/src/components/ui/setup-form-dialog.tsx b/apps/desktop/src/components/ui/setup-form-dialog.tsx new file mode 100644 index 0000000000..4e7c111484 --- /dev/null +++ b/apps/desktop/src/components/ui/setup-form-dialog.tsx @@ -0,0 +1,108 @@ +import type { ConnectionTargetState } from '@hermes/shared' +import { useEffect, useState } from 'react' + +import { Button } from '@/components/ui/button' +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle +} from '@/components/ui/dialog' +import { FieldHint } from '@/components/ui/field' +import { type SetupField, SetupFieldList } from '@/components/ui/setup-field-list' + +interface SetupFormDialogCopy { + cancel: string + connect: string + openInBrowser: string + setup: (server: string) => string +} + +interface SetupFormDialogProps { + copy: SetupFormDialogCopy + detail?: string + fields: SetupField[] + instructions?: null | string + onCancel: () => void + onConnect: (env: Record) => void + onOpenBrowser: () => void + open: boolean + pending: boolean + server: string + status: ConnectionTargetState + url?: null | string +} + +const initialDraft = (fields: SetupField[]): Record => + Object.fromEntries(fields.map(field => [field.name, field.secret ? '' : field.default])) + +export function SetupFormDialog({ + copy, + detail, + fields, + instructions, + onCancel, + onConnect, + onOpenBrowser, + open, + pending, + server, + status, + url +}: SetupFormDialogProps) { + const [draft, setDraft] = useState>({}) + + // The backend sends a fresh field list with every frame (and an empty one while an attempt runs). + // Fields only fill in what the draft lacks, so a failed Connect keeps what was typed. Closing the + // dialog drops the draft: a typed secret does not outlive the form. + useEffect(() => { + setDraft(current => (open ? { ...initialDraft(fields), ...current } : {})) + }, [fields, open]) + + const missingRequired = fields.some(field => field.required && !draft[field.name]?.trim()) + const awaitingBrowser = status === 'initiated' && Boolean(url) + + return ( + !nextOpen && onCancel()} open={open}> + + + {copy.setup(server)} + {instructions ? ( + + {instructions} + + ) : null} + + + setDraft(current => ({ ...current, [name]: value }))} + /> + + {status === 'failed' && detail ? {detail} : null} + + {awaitingBrowser ? ( +
+

{url}

+ +
+ ) : null} + + + + + +
+
+ ) +} diff --git a/apps/desktop/src/i18n/en.ts b/apps/desktop/src/i18n/en.ts index 97118f8119..63ce4d2693 100644 --- a/apps/desktop/src/i18n/en.ts +++ b/apps/desktop/src/i18n/en.ts @@ -12,11 +12,7 @@ export const en: Translations = { grant: 'Reconnect', connected: 'Connected', checking: 'Checking your apps…', - waitingSignIn: 'Waiting for you to finish signing in…', notConnected: 'Not connected', - notAvailable: 'Not available', - startWith: count => `Start the task with ${count} ${count === 1 ? 'app' : 'apps'} connected`, - startWithout: 'Start without connections', skipped: 'Skipped', disabled: 'Unavailable', failed: 'Could not connect', @@ -25,7 +21,6 @@ export const en: Translations = { waiting: 'Waiting for your browser…', timeout: 'Still waiting for authorization.', refresh: 'Refresh status', - statusError: 'Could not check connections. Try refreshing.', connectError: 'Could not start authorization. Try again.', connectErrorFor: (app: string) => `Could not start authorization for ${app}.`, unavailable: 'Connectors are unavailable for this session.', @@ -33,7 +28,12 @@ export const en: Translations = { search: 'Find an app', empty: 'No matching apps', disclaimer: 'Connecting is optional. Only authorize the apps you want Hermes to use.', - execution: 'Connector tools' + execution: 'Connector tools', + setup: server => `Set up ${server}`, + openInBrowser: 'Open in browser', + setupCancel: 'Cancel', + authorizedToolsUnavailable: 'Authorized. Tools unavailable.', + required: 'Required' }, sessionImport: { @@ -4316,7 +4316,6 @@ export const en: Translations = { authorized: server => `Authorized ${server}`, failed: server => `Setup failed for ${server}`, toolCount: count => (count === 1 ? '1 tool' : `${count} tools`), - notInCatalog: server => `“${server}” is not in the MCP catalog`, envRequired: 'Fill in the required credentials first', sendFailed: 'Could not send MCP setup response', reloadFailed: 'Server saved, but reloading MCP tools failed — they load next session', diff --git a/apps/desktop/src/i18n/ru.ts b/apps/desktop/src/i18n/ru.ts index 945b88d8c5..85cc93f402 100644 --- a/apps/desktop/src/i18n/ru.ts +++ b/apps/desktop/src/i18n/ru.ts @@ -3762,7 +3762,6 @@ export const ru = defineLocale({ authorized: server => `${server} авторизован`, failed: server => `Настройка не удалась для ${server}`, toolCount: count => `${count} ${RU_NOUN(count, 'инструмент', 'инструмента', 'инструментов')}`, - notInCatalog: server => `«${server}» нет в MCP-каталоге`, envRequired: 'Сначала заполните обязательные учётные данные', sendFailed: 'Не удалось отправить ответ на настройку MCP', reloadFailed: 'Сервер сохранён, но перезагрузка MCP-инструментов не удалась — они загрузятся в следующем сеансе', diff --git a/apps/desktop/src/i18n/types.ts b/apps/desktop/src/i18n/types.ts index 9795733f69..164167a041 100644 --- a/apps/desktop/src/i18n/types.ts +++ b/apps/desktop/src/i18n/types.ts @@ -68,11 +68,7 @@ export interface Translations { grant: string connected: string checking: string - waitingSignIn: string notConnected: string - notAvailable: string - startWith: (count: number) => string - startWithout: string skipped: string disabled: string failed: string @@ -81,7 +77,6 @@ export interface Translations { waiting: string timeout: string refresh: string - statusError: string connectError: string connectErrorFor: (app: string) => string unavailable: string @@ -90,6 +85,11 @@ export interface Translations { empty: string disclaimer: string execution: string + setup: (server: string) => string + openInBrowser: string + setupCancel: string + authorizedToolsUnavailable: string + required: string } sessionImport: { title: string @@ -3657,7 +3657,6 @@ export interface Translations { authorized: (server: string) => string failed: (server: string) => string toolCount: (count: number) => string - notInCatalog: (server: string) => string envRequired: string sendFailed: string reloadFailed: string diff --git a/apps/desktop/src/i18n/zh.ts b/apps/desktop/src/i18n/zh.ts index a52552616d..568e397e9f 100644 --- a/apps/desktop/src/i18n/zh.ts +++ b/apps/desktop/src/i18n/zh.ts @@ -20,7 +20,6 @@ export const zh = defineLocale({ notConnected: '未连接', timeout: '仍在等待授权。', refresh: '刷新状态', - statusError: '无法检查连接,请刷新重试。', connectError: '无法开始授权,请重试。', connectErrorFor: app => `无法为 ${app} 开始授权。`, unavailable: '此会话暂时无法使用连接器。', @@ -28,7 +27,12 @@ export const zh = defineLocale({ search: '查找应用', empty: '没有匹配的应用', disclaimer: '连接为可选操作。请仅授权你希望 Hermes 使用的应用。', - execution: '连接器工具' + execution: '连接器工具', + setup: server => `设置 ${server}`, + openInBrowser: '在浏览器中打开', + setupCancel: '取消', + authorizedToolsUnavailable: '已授权。工具不可用。', + required: '必填' }, sessionImport: { @@ -4230,7 +4234,6 @@ export const zh = defineLocale({ authorized: server => `已授权 ${server}`, failed: server => `${server} 设置失败`, toolCount: count => `${count} 个工具`, - notInCatalog: server => `“${server}”不在 MCP 目录中`, envRequired: '请先填写所需凭据', sendFailed: '无法发送 MCP 设置响应', reloadFailed: '服务器已保存,但重新加载 MCP 工具失败 — 将在下个会话加载', diff --git a/apps/desktop/src/lib/connector-tools.ts b/apps/desktop/src/lib/connector-tools.ts index 447602f1ec..3c55d291f8 100644 --- a/apps/desktop/src/lib/connector-tools.ts +++ b/apps/desktop/src/lib/connector-tools.ts @@ -1,29 +1,6 @@ import { isRecord } from '@assistant-ui/core/internal' import type { ToolCallMessagePart } from '@assistant-ui/react' -import type { ChatMessage } from '@/lib/chat-messages' - -export function latestConnectorPart(messages: ChatMessage[]) { - return messages - .flatMap(message => message.parts) - .filter(part => { - if (part.type !== 'tool-call') { - return false - } - - if (part.toolName === 'manage_connections') { - const input = recordOf(part.args) - - return ( - (input.action ?? 'status') !== 'status' || (Array.isArray(input.connectors) && input.connectors.length > 0) - ) - } - - return connectorCalls(part.toolName, part.args).length > 0 - }) - .at(-1) -} - export interface McpTarget { name: string action: 'authorize' | 'enable' | 'install' @@ -51,31 +28,24 @@ export function mcpTargets(toolName: string, args: ToolCallMessagePart['result'] }) } -export type ConnectionStatus = 'active' | 'initiated' | 'failed' | 'expired' | 'revoked' | 'inactive' | 'initializing' +/** The gateway's six-state account status; `pending` covers the vendor's INITIALIZING and INITIATED. */ +export type ConnectionStatus = 'active' | 'expired' | 'failed' | 'inactive' | 'pending' | 'revoked' -const CONNECTION_STATUSES: readonly ConnectionStatus[] = [ - 'active', - 'initiated', - 'failed', - 'expired', - 'revoked', - 'inactive', - 'initializing' -] - -const isConnectionStatus = (value: string): value is ConnectionStatus => - CONNECTION_STATUSES.some(status => status === value) - -/** Display-only; these fields never grant access. */ +/** One `GET /v1/connectors` item as the gateway sends it. Display-only; these fields never grant access. */ export interface ConnectorRow { - connector: string - connected?: boolean - enabled?: boolean + connected: boolean connectionStatus?: ConnectionStatus - name?: string - description?: string + connector: string + disabledTools?: string[] + enabled: boolean + statusReason?: string } +/** The vendor's public logo for a toolkit, keyed by its slug (the gateway slug is the vendor slug; checked + * for every lead-order pick). Served as an SVG with no CORS header, so it is only ever an ``. */ +export const connectorIconUrl = (slug: string): string => `https://logos.composio.dev/api/${slug}` + + export function connectorText(value: ToolCallMessagePart['result']): string | undefined { return typeof value === 'string' ? value : undefined } @@ -152,74 +122,6 @@ export function connectorCalls(name: string, args: ToolCallMessagePart['result'] }) } -export function connectionRows( - args: ToolCallMessagePart['result'], - result: ToolCallMessagePart['result'] -): ConnectorRow[] { - const input = recordOf(args) - const output = recordOf(result) - const rows = new Map() - - const add = (item: ToolCallMessagePart['result']) => { - const slug = connectorText(item) - - if (slug !== undefined) { - if (/^[a-z0-9_-]+$/i.test(slug)) { - rows.set(slug, rows.get(slug) ?? { connector: slug }) - } - - return - } - - const row = recordOf(item) - const connector = connectorText(row.connector) - - if (connector === undefined || !/^[a-z0-9_-]+$/i.test(connector)) { - return - } - - const merged: ConnectorRow = { ...rows.get(connector), connector } - - if (row.connected === true || row.connected === false) { - merged.connected = row.connected - } - - if (row.enabled === true || row.enabled === false) { - merged.enabled = row.enabled - } - - const connectionStatus = connectorText(row.connectionStatus) - - if (connectionStatus && isConnectionStatus(connectionStatus)) { - merged.connectionStatus = connectionStatus - } - - for (const key of ['name', 'description'] as const) { - const text = connectorText(row[key]) - - if (text !== undefined) { - merged[key] = text - } - } - - rows.set(connector, merged) - } - - if (Array.isArray(input.connectors)) { - input.connectors.forEach(add) - } else if (connectorText(input.connectors) !== undefined) { - add(input.connectors) - } - - for (const key of ['connectors', 'results', 'pending']) { - if (Array.isArray(output[key])) { - output[key].forEach(add) - } - } - - return [...rows.values()] -} - /** Authorization URLs may carry tokens; reject non-HTTPS or embedded credentials. */ export function connectorAuthorizationUrl(value: ToolCallMessagePart['result']): string | null { const text = connectorText(value) diff --git a/apps/desktop/src/lib/deeplink-routes.ts b/apps/desktop/src/lib/deeplink-routes.ts index 252db87efa..547728b565 100644 --- a/apps/desktop/src/lib/deeplink-routes.ts +++ b/apps/desktop/src/lib/deeplink-routes.ts @@ -12,6 +12,7 @@ export type DeepLinkAction = * catalog by the caller; the raw name is never treated as a git identifier. */ | { type: 'plugin-catalog-install'; name: string } | { type: 'composer-blueprint'; name: string; params: Record } + | { type: 'connection-done'; op: string; status: string } | { type: 'ignore' } function truthyParam(value: string | undefined, defaultValue = false): boolean { @@ -33,6 +34,15 @@ export function resolveDeepLinkAction(payload: DeepLinkPayload | null | undefine return { type: 'composer-blueprint', name: payload.name, params: payload.params || {} } } + // The browser leg of a connection came back (hermes://connections/done?op=…&status=…). The op id + // names the operation to show; the status is carried but never moves a row, because the link is + // whatever the user's browser was pointed at. + if (payload.kind === 'connections' && payload.name === 'done') { + const op = (payload.params?.op || '').trim() + + return op ? { type: 'connection-done', op, status: (payload.params?.status || '').trim() } : { type: 'ignore' } + } + // A `catalog` param claims the link outright: even when a `repo` rides along // (or the name is empty/bogus) the outcome is the catalog lookup's verdict, // never a git-path install of whatever else the link carried. diff --git a/apps/desktop/src/lib/first-build-start.test.ts b/apps/desktop/src/lib/first-build-start.test.ts deleted file mode 100644 index 951ecd58b2..0000000000 --- a/apps/desktop/src/lib/first-build-start.test.ts +++ /dev/null @@ -1,24 +0,0 @@ -import { describe, expect, it } from 'vitest' - -import type { ChatMessagePart } from '@/lib/chat-messages' - -import { canStartWithConnections } from './first-build-start' - -const wait = (overrides: Partial>): ChatMessagePart => - ({ - type: 'tool-call', - toolCallId: 'wait-1', - toolName: 'manage_connections', - args: { action: 'wait' }, - ...overrides - }) as ChatMessagePart - -describe('canStartWithConnections', () => { - it('offers the start while a connection wait is in flight', () => { - expect(canStartWithConnections(wait({}))).toBe(true) - }) - - it('withdraws the start once the wait is sealed without a result', () => { - expect(canStartWithConnections(wait({ completedAt: 5 }))).toBe(false) - }) -}) diff --git a/apps/desktop/src/lib/first-build-start.ts b/apps/desktop/src/lib/first-build-start.ts deleted file mode 100644 index 6a43b4cebd..0000000000 --- a/apps/desktop/src/lib/first-build-start.ts +++ /dev/null @@ -1,40 +0,0 @@ -import type { ChatMessagePart } from '@/lib/chat-messages' -import { connectorAuthorizationUrl, connectorTitle, recordOf } from '@/lib/connector-tools' -import type { FirstBuildConnectorRow } from '@/store/first-build-connectors' - -function naturalJoin(names: string[]): string { - return names.length < 2 ? names.join('') : `${names.slice(0, -1).join(', ')} and ${names.at(-1)}` -} - -export function buildConnectionStartMessage(rows: readonly FirstBuildConnectorRow[]): string { - const connected = rows.filter(row => row.phase === 'connected').map(row => connectorTitle(row.connector)) - const skipped = rows.filter(row => row.phase !== 'connected').map(row => connectorTitle(row.connector)) - - return ( - (connected.length ? `Start with ${naturalJoin(connected)} connected.` : 'Start without connections.') + - (skipped.length ? ` I skipped ${naturalJoin(skipped)}.` : '') - ) -} - -export function canStartWithConnections(part: ChatMessagePart): boolean { - if (part.type !== 'tool-call' || part.toolName !== 'manage_connections') { - return false - } - - const action = recordOf(part.args).action - const output = recordOf(part.result) - - if (action === 'wait') { - return (part.result === undefined && part.completedAt === undefined) || output.status === 'pending' - } - - return ( - action === 'connect' && - Array.isArray(output.results) && - output.results.some(item => { - const entry = recordOf(item) - - return entry.status === 'initiated' && connectorAuthorizationUrl(entry.connect_url) !== null - }) - ) -} diff --git a/apps/desktop/src/store/connection-request.test.ts b/apps/desktop/src/store/connection-request.test.ts index aa8e942706..d611eca9b1 100644 --- a/apps/desktop/src/store/connection-request.test.ts +++ b/apps/desktop/src/store/connection-request.test.ts @@ -20,10 +20,19 @@ import { $gateway } from './gateway' const WIRE = { deadline_at: 1_800_000_000, op_id: 'op-1', + seq: 1, tool_call_id: 'call-1', timeout_seconds: 120, targets: [ - { action: 'connect' as const, kind: 'connector' as const, name: 'gmail', state: 'pending' as const }, + { + action: 'connect' as const, + discovery_error: 'tool discovery failed', + instructions: 'Authorize Gmail.', + kind: 'connector' as const, + name: 'gmail', + required_env: [{ default: 'primary', name: 'ACCOUNT', prompt: 'Account', required: true, secret: false }], + state: 'pending' as const + }, { action: 'connect' as const, kind: 'connector' as const, name: 'notion', state: 'pending' as const } ] } @@ -42,6 +51,10 @@ function request(sessionId: string | null, opId = 'op-1'): ConnectionRequest { type Snapshot = Parameters[1] type Frame = Parameters[1] +/** The backend stamps every write with a rising `seq`; the fixture counts the same way so a frame + * built later is newer than one built earlier unless a test says otherwise. */ +let nextSeq = WIRE.seq + 1 + /** Every `connection.update` frame carries the operation snapshot; `states` overrides per-target state. */ function frame( states: Record, @@ -50,6 +63,7 @@ function frame( return { deadline_at: WIRE.deadline_at, op_id: 'op-1', + seq: nextSeq++, settled: false, settled_by: null, targets: WIRE.targets.map(target => ({ ...target, state: states[target.name] ?? target.state })), @@ -76,6 +90,11 @@ describe('connection-request store', () => { ['gmail', 'connector', 'pending'], ['notion', 'connector', 'pending'] ]) + expect(parsed?.targets[0]).toMatchObject({ + discoveryError: 'tool discovery failed', + instructions: 'Authorize Gmail.', + requiredEnv: [{ default: 'primary', name: 'ACCOUNT', prompt: 'Account', required: true, secret: false }] + }) expect(parsed?.settled).toBe(false) }) @@ -102,6 +121,7 @@ describe('connection-request store', () => { const overlaid = applyOperationStatus(req, { deadline_at: WIRE.deadline_at, op_id: 'op-1', + seq: nextSeq++, settled: false, settled_by: null, targets: [ diff --git a/apps/desktop/src/store/connection-request.ts b/apps/desktop/src/store/connection-request.ts index 02819fef25..d6509fe41c 100644 --- a/apps/desktop/src/store/connection-request.ts +++ b/apps/desktop/src/store/connection-request.ts @@ -4,15 +4,34 @@ import type { ConnectionRequestPayload, ConnectionSettleReason, ConnectionTargetAction, + ConnectionTargetEnvField, ConnectionTargetKind, ConnectionTargetState, ConnectionUpdatePayload } from '@hermes/shared' import { atom, computed } from 'nanostores' +import type { SetupField } from '@/components/ui/setup-field-list' + import { $gateway } from './gateway' -export type { ConnectionSettleReason, ConnectionTargetAction, ConnectionTargetKind, ConnectionTargetState } +/** The backend sends ``prompt`` as null when the catalog entry has none; the form takes an absent one. */ +const envFields = (fields: ConnectionTargetEnvField[] | null | undefined): SetupField[] => + (fields ?? []).map(({ default: defaultValue, name, prompt, required, secret }) => ({ + default: defaultValue, + name, + prompt: prompt ?? undefined, + required, + secret + })) + +export type { + ConnectionSettleReason, + ConnectionTargetAction, + ConnectionTargetEnvField, + ConnectionTargetKind, + ConnectionTargetState +} /** One target of the operation as the renderer knows it. State comes only from the backend * (`connection.request`, `connectors.operation.status`, `connection.update`); the card never sets it. */ @@ -23,7 +42,14 @@ export interface ConnectionTarget { state: ConnectionTargetState detail: string connectUrl: null | string + /** The vendor account of a managed target once a mint named one; empty before that and on MCP targets. */ + connectionId: string + /** Toolkit metadata on connector targets; empty on an MCP target. */ tools: string[] + /** Credentials an MCP install is still waiting for; empty on every other target. */ + requiredEnv: SetupField[] + instructions: string | null + discoveryError: string | null } /** The session's connection operation. `deadlineAt`, `opId`, `targets[].state`, `settled` and @@ -32,6 +58,8 @@ export interface ConnectionRequest { /** The model's tool call that opened the operation. The card lives on that row and no other. */ toolCallId: string opId: string + /** The sequence of the newest frame this cache holds; an older frame for the same op is dropped. */ + seq: number /** Unix seconds; backend-owned. */ deadlineAt: number targets: ConnectionTarget[] @@ -42,12 +70,11 @@ export interface ConnectionRequest { sessionId: string | null } -/** Answers the card may give for one target. Anything else the backend refuses (4002). */ +/** Answers the card may give for one target: the user said no, or the user consented and the backend + * does the work. The card never reports an outcome; only the backend moves a target. */ export type ConnectionTargetOutcome = | { name: string; status: 'skipped' } - | { name: string; status: 'connected'; tools?: string[] } - | { name: string; status: 'initiated' } - | { name: string; status: 'failed'; detail?: string } + | { env?: Record; name: string; status: 'approved' } export interface ConnectionOutcome { targets?: ConnectionTargetOutcome[] @@ -100,7 +127,11 @@ function parseTarget(entry: ConnectionOperationTarget): ConnectionTarget | null kind: entry.kind === 'connector' ? 'connector' : 'mcp', name, state: targetState(entry.state) ?? 'pending', - tools: entry.tools ?? [] + tools: entry.tools ?? [], + connectionId: entry.connection_id ?? '', + requiredEnv: envFields(entry.required_env), + instructions: entry.instructions ?? null, + discoveryError: entry.discovery_error ?? null } } @@ -124,6 +155,7 @@ export function normalizeConnectionRequest( deadlineAt: payload.deadline_at, opId: payload.op_id, receivedAt: Date.now() / 1000, + seq: payload.seq, sessionId, settled: false, settledBy: null, @@ -132,9 +164,14 @@ export function normalizeConnectionRequest( } } -/** Overlay the authoritative `connectors.operation.status` snapshot on the cached request. */ -export function applyOperationStatus(request: ConnectionRequest, status: ConnectionOperationStatus): ConnectionRequest { - if (status.op_id !== request.opId) { +/** Overlay the authoritative `connectors.operation.status` snapshot on the cached request. Frames for + * another operation, and frames the operation wrote before the one already applied, change nothing: + * the transport can reorder them and an older one would regress a row. */ +export function applyOperationStatus( + request: ConnectionRequest, + status: ConnectionOperationStatus +): ConnectionRequest { + if (status.op_id !== request.opId || status.seq <= request.seq) { return request } @@ -151,11 +188,14 @@ export function applyOperationStatus(request: ConnectionRequest, status: Connect // Same reference on a no-op so subscribers do not re-render for an identical frame. const unchanged = request.deadlineAt === status.deadline_at && + request.seq === status.seq && request.settled === status.settled && request.settledBy === settledBy && targets.every((target, index) => target === request.targets[index]) - return unchanged ? request : { ...request, deadlineAt: status.deadline_at, settled: status.settled, settledBy, targets } + return unchanged + ? request + : { ...request, deadlineAt: status.deadline_at, seq: status.seq, settled: status.settled, settledBy, targets } } function mergeLiveTarget(target: ConnectionTarget, live: ConnectionOperationTarget): ConnectionTarget { @@ -164,22 +204,45 @@ function mergeLiveTarget(target: ConnectionTarget, live: ConnectionOperationTarg connectUrl: live.connect_url ?? target.connectUrl, detail: live.detail ?? target.detail, state: live.state, - tools: live.tools ?? target.tools + tools: live.tools ?? target.tools, + connectionId: live.connection_id ?? target.connectionId, + requiredEnv: live.required_env ? envFields(live.required_env) : target.requiredEnv, + instructions: live.instructions === undefined ? target.instructions : live.instructions, + discoveryError: live.discovery_error === undefined ? target.discoveryError : live.discovery_error } const same = next.connectUrl === target.connectUrl && + next.connectionId === target.connectionId && next.detail === target.detail && + next.instructions === target.instructions && + next.discoveryError === target.discoveryError && next.state === target.state && next.tools.length === target.tools.length && - next.tools.every((tool, index) => tool === target.tools[index]) + next.tools.every((tool, index) => tool === target.tools[index]) && + sameEnvFields(next.requiredEnv, target.requiredEnv) return same ? target : next } +// Every frame carries a fresh array, so identity would churn the row and remount its open inputs. +const sameEnvFields = (next: SetupField[], previous: SetupField[]): boolean => + next.length === previous.length && + next.every( + (field, index) => + field.name === previous[index].name && + field.prompt === previous[index].prompt && + field.required === previous[index].required && + field.secret === previous[index].secret && + field.default === previous[index].default + ) + /** Apply one `connection.update` frame. Every frame carries the operation's full target snapshot, so * the store overlays it; frames for another operation or for a settled request are ignored. */ -export function applyConnectionUpdate(request: ConnectionRequest, update: ConnectionUpdatePayload): ConnectionRequest { +export function applyConnectionUpdate( + request: ConnectionRequest, + update: ConnectionUpdatePayload +): ConnectionRequest { if (update.op_id !== request.opId || request.settled) { return request } diff --git a/apps/desktop/src/store/first-build-connectors.ts b/apps/desktop/src/store/first-build-connectors.ts deleted file mode 100644 index e582ab0929..0000000000 --- a/apps/desktop/src/store/first-build-connectors.ts +++ /dev/null @@ -1,283 +0,0 @@ -import type { ToolCallMessagePart } from '@assistant-ui/react' -import { map } from 'nanostores' - -import { endFirstBuildConnect, isFirstBuildSession } from '@/app/contrib/handoff-receipt' -import { - connectionRows, - connectorAuthorizationUrl, - type ConnectorRow, - connectorText, - recordOf -} from '@/lib/connector-tools' -import { buildConnectionStartMessage, canStartWithConnections } from '@/lib/first-build-start' -import { readKey, writeKey } from '@/lib/storage' -export type FirstBuildConnectorPart = Pick - -// Onboarding's own poller; PR3 (NS-869) moves the guided flow onto the connection operation. -export type FirstBuildConnectorPhase = 'idle' | 'opening' | 'waiting' | 'connected' | 'timeout' | 'error' | 'skipped' - -export interface FirstBuildConnectorRow extends ConnectorRow { - phase: FirstBuildConnectorPhase - error?: string - connectUrl?: string -} - -export type FirstBuildConnectorRequest = ( - method: string, - params: { session_id: string; connectors?: string[]; reconnect?: boolean } -) => Promise - -export interface FirstBuildConnectorState { - toolCallId: string - rows: FirstBuildConnectorRow[] - started: boolean - /** The "[setup] links opened" note, held until the session is idle. The gateway rejects a submit while the - * model's turn runs, and the model usually calls wait in that same turn, so this note is only a fallback. */ - pendingNote?: string -} - -export const $firstBuildConnections = map>({}) - -interface OpenLinksDeps { - open?: (url: string) => Promise -} - -export async function openFirstBuildLinks(storedId: string, part: FirstBuildConnectorPart, deps: OpenLinksDeps) { - if ( - !isFirstBuildSession(storedId) || - part.toolName !== 'manage_connections' || - !['connect', 'reconnect'].includes(String(recordOf(part.args).action)) - ) { - return - } - - const output = recordOf(part.result) - - if (!Array.isArray(output.results)) { - return - } - - const entries = output.results.map(recordOf) - const previous = $firstBuildConnections.get()[storedId] - - const rows = connectionRows(part.args, part.result).map((seed): FirstBuildConnectorRow => { - const existing = previous?.rows.find(row => row.connector === seed.connector) - const entry = entries.find(row => row.connector === seed.connector) - const connectUrl = entry?.status === 'initiated' ? connectorAuthorizationUrl(entry.connect_url) : null - - return { - ...seed, - ...existing, - phase: - entry?.status === 'active' - ? 'connected' - : connectUrl && previous?.toolCallId !== part.toolCallId - ? 'waiting' - : (existing?.phase ?? 'idle'), - connectUrl: connectUrl ?? undefined - } - }) - - $firstBuildConnections.setKey(storedId, { - toolCallId: part.toolCallId, - rows, - started: previous?.started ?? readKey(`hermes.onboarding.started.v1.${storedId}`) === '1' - }) - - const links = entries.flatMap(entry => { - const url = entry.status === 'initiated' ? connectorAuthorizationUrl(entry.connect_url) : null - const connector = connectorText(entry.connector) - - return url && connector !== undefined ? [{ connector, url }] : [] - }) - - const key = `hermes.onboarding.links-opened.v1.${part.toolCallId}` - - if (!deps.open || !links.length || readKey(key) === '1') { - return - } - - // Claim before opening so concurrent renders and relaunches cannot open the batch twice. - writeKey(key, '1') - const open = deps.open - const outcomes = await Promise.allSettled(links.map(link => open(link.url))) - const opened = links.filter((_link, index) => outcomes[index].status === 'fulfilled') - - const state = $firstBuildConnections.get()[storedId] - - if (opened.length && state?.toolCallId === part.toolCallId) { - $firstBuildConnections.setKey(storedId, { - ...state, - pendingNote: `[setup] links opened for ${opened.map(link => link.connector).join(', ')}` - }) - } -} - -/** Delivers the held note once the session is idle, and only while the connect call that produced the links is - * still the newest connector part. A newer part means the model already called wait itself. */ -export function flushFirstBuildNote( - storedId: string, - newestToolCallId: string | undefined, - busy: boolean, - submit: (text: string) => boolean -): void { - const state = $firstBuildConnections.get()[storedId] - - if (!state?.pendingNote || busy) { - return - } - - if (!isFirstBuildSession(storedId) || newestToolCallId !== state.toolCallId || submit(state.pendingNote)) { - $firstBuildConnections.setKey(storedId, { ...state, pendingNote: undefined }) - } -} - -export function watchFirstBuildRows( - storedId: string, - runtimeId: string, - part: FirstBuildConnectorPart, - request: FirstBuildConnectorRequest -) { - const action = recordOf(part.args).action - - if ( - !isFirstBuildSession(storedId) || - part.toolName !== 'manage_connections' || - (action !== 'wait' && !(action === 'connect' && canStartWithConnections({ ...part, type: 'tool-call' }))) - ) { - return - } - - const output = recordOf(part.result) - const polling = action === 'connect' || part.result === undefined || output.status === 'pending' - - if (action === 'wait' && ['connected', 'timeout', 'interrupted'].includes(String(output.status))) { - endFirstBuildConnect(storedId) - } - - const connected = new Set( - (Array.isArray(output.connectors) ? output.connectors : []).flatMap(item => { - const entry = recordOf(item) - const slug = connectorText(item) ?? connectorText(entry.connector) - - return slug !== undefined && entry.connected !== false ? [slug] : [] - }) - ) - - const pending = new Set(Array.isArray(output.pending) ? output.pending : []) - const previous = $firstBuildConnections.get()[storedId] - - const rows = connectionRows(part.args, part.result).map((seed): FirstBuildConnectorRow => { - const existing = previous?.rows.find(row => row.connector === seed.connector) - let phase = existing?.phase ?? 'waiting' - - if (output.status !== 'interrupted') { - if (connected.has(seed.connector)) { - phase = 'connected' - } else if (output.status === 'timeout' && pending.has(seed.connector)) { - phase = 'timeout' - } else if (polling && phase !== 'connected') { - phase = 'waiting' - } - } - - return { ...seed, ...existing, phase } - }) - - $firstBuildConnections.setKey(storedId, { - toolCallId: part.toolCallId, - rows, - started: previous?.started ?? readKey(`hermes.onboarding.started.v1.${storedId}`) === '1' - }) - - if (!polling) { - return - } - - let cancelled = false - let failures = 0 - const deadline = Date.now() + 150000 - let timer: ReturnType | undefined - - const current = () => { - const state = $firstBuildConnections.get()[storedId] - - return ( - !cancelled && - failures < 3 && - Date.now() < deadline && - isFirstBuildSession(storedId) && - !state?.started && - state?.toolCallId === part.toolCallId - ) - } - - const poll = async () => { - if (!current()) { - return - } - - try { - const result = await request<{ available: boolean; connectors: ConnectorRow[] }>('connectors.list', { - session_id: runtimeId - }) - - if (!current()) { - return - } - - const state = $firstBuildConnections.get()[storedId] - - const rows = state.rows.map((row): FirstBuildConnectorRow => { - const live = result.connectors.find(item => item.connector === row.connector) - - if (!result.available || !live || live.enabled === false) { - return row.phase === 'connected' ? row : { ...row, enabled: false, phase: 'error', error: 'unavailable' } - } - - return { ...row, ...live, phase: live.connected ? 'connected' : 'waiting', error: undefined } - }) - - $firstBuildConnections.setKey(storedId, { ...state, rows }) - failures = 0 - } catch { - if (!current()) { - return - } - - const state = $firstBuildConnections.get()[storedId] - $firstBuildConnections.setKey(storedId, { - ...state, - rows: state.rows.map(row => (row.phase === 'connected' ? row : { ...row, phase: 'error', error: 'status' })) - }) - failures += 1 - } - - if (current()) { - timer = setTimeout(() => void poll(), 2000) - } - } - - void poll() - - return () => { - cancelled = true - clearTimeout(timer) - } -} - -/** A true result from submit means the composer delivered the text through send, steer or queue. */ -export function startFirstBuild(storedId: string, submit: (text: string) => boolean): void { - const state = $firstBuildConnections.get()[storedId] - const key = `hermes.onboarding.started.v1.${storedId}` - - if (!isFirstBuildSession(storedId) || !state || state.started || readKey(key) === '1') { - return - } - - if (submit(buildConnectionStartMessage(state.rows))) { - writeKey(key, '1') - endFirstBuildConnect(storedId) - $firstBuildConnections.setKey(storedId, { ...state, started: true }) - } -} diff --git a/apps/desktop/src/store/onboarding-capabilities.test.ts b/apps/desktop/src/store/onboarding-capabilities.test.ts index 46554649ac..1afdaf9617 100644 --- a/apps/desktop/src/store/onboarding-capabilities.test.ts +++ b/apps/desktop/src/store/onboarding-capabilities.test.ts @@ -1,13 +1,11 @@ import { afterEach, expect, it, vi } from 'vitest' -import { buildFirstTaskSeedMessages } from '@/components/onboarding-chat/setup-profile' -import { DEFAULT_ANSWERS } from '@/store/onboarding-answers' import { readOnboardingCapabilities } from '@/store/onboarding-capabilities' import { buildChatOnboardingSeedMessages } from '@/store/onboarding-script' const api = vi.fn() -it('carries fresh catalog evidence into the guide and the exact working-profile handoff seed', async () => { +it('carries fresh catalog evidence into the guide, read from the pinned backend', async () => { vi.stubGlobal('window', { hermesDesktop: { api } }) const entry = { @@ -23,10 +21,6 @@ it('carries fresh catalog evidence into the guide and the exact working-profile expect(guide[0].content).toContain('"name":"future-studio"') expect(guide[0].content).toContain('"readiness":"setup_required"') expect(guide[0].display_kind).toBe('hidden') - - const handoff = await buildFirstTaskSeedMessages('Make a scene in Future Studio', DEFAULT_ANSWERS, 'build', scope) - expect(handoff[0].content).toContain('"name":"future-studio"') - expect(handoff[0].content).toContain('not necessarily the desktop computer') expect(api).toHaveBeenLastCalledWith(expect.objectContaining(scope)) expect(api.mock.calls.every(([request]) => request.path === '/api/mcp/catalog?detect_apps=true')).toBe(true) }) diff --git a/apps/desktop/src/store/onboarding-script.ts b/apps/desktop/src/store/onboarding-script.ts index 1afe370c27..ab78807aa6 100644 --- a/apps/desktop/src/store/onboarding-script.ts +++ b/apps/desktop/src/store/onboarding-script.ts @@ -139,7 +139,7 @@ export function buildChatOnboardingPrompt(suggestedName?: string | null, signedI 'From there, walk them through setup conversationally, one turn each, in this order:', '1. This turn is exactly four things and then you stop: a few warm words about their name, then ::onboarding{step="name" value="THEIR_NAME"} on a line of its own (THEIR_NAME being the name they actually gave; it renders as nothing and just saves it), then one short sentence about their colour, then ::onboarding{step="look"} on a line of its own. That is one turn, not two, and it is not a conflict with RULE 3: the name line is not a question, the look card is, and it is the last thing you write.', '2. Then the apps they already use, so Hermes can connect to them later: one short sentence that makes clear what connecting means — you would read and act inside those apps for them (their inbox, their calendar, their repos), not message them there — then ::onboarding{step="connectors"} on a line of its own. Chat apps like Discord or Telegram are a different thing (how they reach you) and are not what this card is asking about; if they bring one up, say it lives in Messaging in the app’s settings and move on.', - 'CONNECTING, IF THEY ASK FOR IT HERE. The picks are preferences, not connections — but if at any point they ask you to connect an app, or say they want one wired up now, do it in this chat: call manage_connections action="status" once, then one action="connect" with EVERY app they named as a batch (connectors=["gmail","googlecalendar"], not one call per app). The app renders that as one card with a row per app and the call blocks until every app is connected, the user continues or skips, or the deadline passes; never paste the links, never describe a settings page. The result lists each app as connected, skipped or not_connected; continue from that. Never call connect a second time for an app that already has a card. If an app is not in the status catalog, say so plainly. There is no Connectors page in Settings; do not send them to one.', + 'CONNECTING, IF THEY ASK FOR IT HERE. The picks are preferences, not connections — but if at any point they ask you to connect an app, or say they want one wired up now, do it in this chat: call manage_connections action="status" once, then one action="connect" with EVERY app they named as a batch (connectors=["gmail","googlecalendar"], not one call per app). The app renders that as one card with a row per app and the call blocks until every app is connected, or the user presses Continue, or the deadline passes; never paste the links, never describe a settings page. The result lists each app as connected, skipped or not_connected; continue from that. Never call connect a second time for an app that already has a card. If an app is not in the status catalog, say so plainly. There is no Connectors page in Settings; do not send them to one.', // The only place sign-in is named before it is needed. It sits at the connectors step because the user has just // listed the accounts they use. ...(signedIn diff --git a/apps/shared/src/gateway-contract.generated.ts b/apps/shared/src/gateway-contract.generated.ts index 53e62306dd..be20e5dab8 100644 --- a/apps/shared/src/gateway-contract.generated.ts +++ b/apps/shared/src/gateway-contract.generated.ts @@ -466,6 +466,7 @@ export interface ConnectionOperationParams { /** ``methods_connectors._operation_view``: the operation's full snapshot. */ export interface ConnectionOperationStatus { op_id: string + seq: number deadline_at: number settled: boolean settled_at?: number | null @@ -481,8 +482,12 @@ export interface ConnectionOperationTarget { action: ConnectionTargetAction state: ConnectionTargetState detail?: string | null + instructions?: string | null + discovery_error?: string | null connect_url?: string | null + connection_id?: string | null attempt?: string | null + required_env?: ConnectionTargetEnvField[] | null tools?: string[] | null hint?: string | null } @@ -490,6 +495,17 @@ export type ConnectionTargetKind = 'connector' | 'mcp' export type ConnectionTargetAction = 'authorize' | 'connect' | 'enable' | 'install' | 'reconnect' /** ``tools/connectors/contract.py::TargetState``. */ export type ConnectionTargetState = 'pending' | 'initiated' | 'connected' | 'skipped' | 'failed' | 'expired' | 'unavailable' | 'not_connected' +/** One credential an MCP install still needs; the card renders a field per entry and sends the values back with the approval. */ +export interface ConnectionTargetEnvField { + name: string + required: boolean + secret: boolean + default: string + prompt?: string | null +} +export interface ConnectionWakeResult { + status: string +} export interface ConnectionRespondParams { profile?: string | null session_id: string @@ -501,15 +517,15 @@ export interface ConnectionAnswer { targets?: ConnectionAnswerTarget[] settled_by?: ConnectionSettleReason | null } -/** One row's answer from the card. ``status`` is what the card observed for that row (``tools/connectors/mcp.py::_OUTCOME_STATES`` maps it onto a target state); ``state`` is the older spelling of the same field and one of the two is present. */ +/** One row's answer from the card. ``env`` carries the credential values an install asked for through ``required_env``. */ export interface ConnectionAnswerTarget { name: string - status?: string | null - state?: string | null + status: ConnectionAnswerStatus detail?: string | null - tools?: string[] | null - [key: string]: unknown + env?: Record | null } +/** What the card says about one row: ``tools/connectors/mcp.py::apply_answer``. */ +export type ConnectionAnswerStatus = 'approved' | 'skipped' export interface ConnectionRespondResult { status: string settled: boolean @@ -779,6 +795,7 @@ export interface ConnectorsConnectParams { /** The operation the connect opened (or re-minted on): ``tools/connectors/managed.py`` ``_off_desktop_result`` / ``methods_connectors._reissue``. ``status``/``note`` ride along from the tool result when the call ran through ``manage_connections``. */ export interface ConnectorsConnectResult { op_id: string + seq: number deadline_at: number settled: boolean settled_at?: number | null @@ -2555,6 +2572,7 @@ export interface OpenRequestEntry { /** ``ConnectionOperation.request_payload``: opens the card; also the ``pending_connection`` resume snapshot so a client that missed the event restores the card with the server's deadline. */ export interface ConnectionRequestPayload { op_id: string + seq: number deadline_at: number timeout_seconds: number targets: ConnectionOperationTarget[] @@ -3816,6 +3834,7 @@ export interface TourStep { /** ``methods_connectors._connection_update``: one target transition (``target``/``from``/``to``/ ``actor``) or the settlement (none of those), with the full snapshot. */ export interface ConnectionUpdatePayload { op_id: string + seq: number deadline_at: number settled: boolean settled_at?: number | null @@ -3828,7 +3847,7 @@ export interface ConnectionUpdatePayload { detail?: string | null } /** ``tools/connectors/contract.py::Actor``. */ -export type ConnectionActor = 'user' | 'renderer_flow' | 'backend_watcher' | 'clock' +export type ConnectionActor = 'user' | 'backend_watcher' | 'clock' /** ``tui_gateway/entry.py`` (stdio) / ``tui_gateway/ws.py`` (WebSocket) first frame. */ export interface GatewayReadyPayload { skin: SkinPayload @@ -4262,6 +4281,8 @@ export interface RpcMethods { 'connectors.list': { params: ConnectorsListParams; result: ConnectorsListResult } /** The current snapshot of one open operation on an owned session. */ 'connectors.operation.status': { params: ConnectionOperationParams; result: ConnectionOperationStatus } + /** The browser leg came back (hermes://connections/done): read the accounts now, not at the next tick. */ + 'connectors.operation.wake': { params: ConnectionOperationParams; result: ConnectionWakeResult } /** List/add/remove/pause/resume cron jobs in the (optionally profile-scoped) cron store. */ 'cron.manage': { params: CronManageParams; result: CronManageResult } /** Block/unblock NEW spawns globally (active children keep running); returns the new state. */ @@ -4667,6 +4688,7 @@ export const RPC_METHODS = [ 'connectors.connect', 'connectors.list', 'connectors.operation.status', + 'connectors.operation.wake', 'cron.manage', 'delegation.pause', 'delegation.status', diff --git a/apps/shared/src/gateway-contract.openrpc.json b/apps/shared/src/gateway-contract.openrpc.json index fedd20e3c3..05ab54998d 100644 --- a/apps/shared/src/gateway-contract.openrpc.json +++ b/apps/shared/src/gateway-contract.openrpc.json @@ -618,6 +618,24 @@ } } }, + { + "name": "connectors.operation.wake", + "summary": "The browser leg came back (hermes://connections/done): read the accounts now, not at the next tick.", + "params": [ + { + "name": "params", + "schema": { + "$ref": "#/components/schemas/ConnectionOperationParams" + } + } + ], + "result": { + "name": "result", + "schema": { + "$ref": "#/components/schemas/ConnectionWakeResult" + } + } + }, { "name": "cron.manage", "summary": "List/add/remove/pause/resume cron jobs in the (optionally profile-scoped) cron store.", @@ -8308,7 +8326,6 @@ "description": "``tools/connectors/contract.py::Actor``.", "enum": [ "user", - "renderer_flow", "backend_watcher", "clock" ], @@ -8341,37 +8358,25 @@ "title": "ConnectionAnswer", "type": "object" }, + "ConnectionAnswerStatus": { + "description": "What the card says about one row: ``tools/connectors/mcp.py::apply_answer``.", + "enum": [ + "approved", + "skipped" + ], + "title": "ConnectionAnswerStatus", + "type": "string" + }, "ConnectionAnswerTarget": { - "additionalProperties": true, - "description": "One row's answer from the card. ``status`` is what the card observed for that row\n(``tools/connectors/mcp.py::_OUTCOME_STATES`` maps it onto a target state); ``state`` is the\nolder spelling of the same field and one of the two is present.", + "additionalProperties": false, + "description": "One row's answer from the card. ``env`` carries the credential values an install asked for\nthrough ``required_env``.", "properties": { "name": { "title": "Name", "type": "string" }, "status": { - "anyOf": [ - { - "type": "string" - }, - { - "type": "null" - } - ], - "default": null, - "title": "Status" - }, - "state": { - "anyOf": [ - { - "type": "string" - }, - { - "type": "null" - } - ], - "default": null, - "title": "State" + "$ref": "#/components/schemas/ConnectionAnswerStatus" }, "detail": { "anyOf": [ @@ -8385,24 +8390,25 @@ "default": null, "title": "Detail" }, - "tools": { + "env": { "anyOf": [ { - "items": { + "additionalProperties": { "type": "string" }, - "type": "array" + "type": "object" }, { "type": "null" } ], "default": null, - "title": "Tools" + "title": "Env" } }, "required": [ - "name" + "name", + "status" ], "title": "ConnectionAnswerTarget", "type": "object" @@ -8446,6 +8452,10 @@ "title": "Op Id", "type": "string" }, + "seq": { + "title": "Seq", + "type": "integer" + }, "deadline_at": { "title": "Deadline At", "type": "number" @@ -8487,6 +8497,7 @@ }, "required": [ "op_id", + "seq", "deadline_at", "settled", "targets" @@ -8523,6 +8534,30 @@ "default": null, "title": "Detail" }, + "instructions": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "default": null, + "title": "Instructions" + }, + "discovery_error": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "default": null, + "title": "Discovery Error" + }, "connect_url": { "anyOf": [ { @@ -8535,6 +8570,18 @@ "default": null, "title": "Connect Url" }, + "connection_id": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "default": null, + "title": "Connection Id" + }, "attempt": { "anyOf": [ { @@ -8547,6 +8594,21 @@ "default": null, "title": "Attempt" }, + "required_env": { + "anyOf": [ + { + "items": { + "$ref": "#/components/schemas/ConnectionTargetEnvField" + }, + "type": "array" + }, + { + "type": "null" + } + ], + "default": null, + "title": "Required Env" + }, "tools": { "anyOf": [ { @@ -8592,6 +8654,10 @@ "title": "Op Id", "type": "string" }, + "seq": { + "title": "Seq", + "type": "integer" + }, "deadline_at": { "title": "Deadline At", "type": "number" @@ -8622,6 +8688,7 @@ }, "required": [ "op_id", + "seq", "deadline_at", "timeout_seconds", "targets" @@ -8706,6 +8773,48 @@ "title": "ConnectionTargetAction", "type": "string" }, + "ConnectionTargetEnvField": { + "additionalProperties": false, + "description": "One credential an MCP install still needs; the card renders a field per entry and sends the\nvalues back with the approval.", + "properties": { + "name": { + "title": "Name", + "type": "string" + }, + "required": { + "title": "Required", + "type": "boolean" + }, + "secret": { + "title": "Secret", + "type": "boolean" + }, + "default": { + "type": "string", + "title": "Default" + }, + "prompt": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "default": null, + "title": "Prompt" + } + }, + "required": [ + "name", + "required", + "secret", + "default" + ], + "title": "ConnectionTargetEnvField", + "type": "object" + }, "ConnectionTargetKind": { "enum": [ "connector", @@ -8737,6 +8846,10 @@ "title": "Op Id", "type": "string" }, + "seq": { + "title": "Seq", + "type": "integer" + }, "deadline_at": { "title": "Deadline At", "type": "number" @@ -8835,6 +8948,7 @@ }, "required": [ "op_id", + "seq", "deadline_at", "settled", "targets" @@ -8842,6 +8956,20 @@ "title": "ConnectionUpdatePayload", "type": "object" }, + "ConnectionWakeResult": { + "additionalProperties": false, + "properties": { + "status": { + "title": "Status", + "type": "string" + } + }, + "required": [ + "status" + ], + "title": "ConnectionWakeResult", + "type": "object" + }, "ConnectorRow": { "additionalProperties": true, "description": "One ``manage_connections`` status entry after ``connector_ui_payload`` redaction; the\nconnector service owns the closed key set, so unknown metadata passes through.", @@ -8962,6 +9090,10 @@ "title": "Op Id", "type": "string" }, + "seq": { + "title": "Seq", + "type": "integer" + }, "deadline_at": { "title": "Deadline At", "type": "number" @@ -9027,6 +9159,7 @@ }, "required": [ "op_id", + "seq", "deadline_at", "settled", "targets" diff --git a/hermes_cli/cli_agent_setup_mixin.py b/hermes_cli/cli_agent_setup_mixin.py index c0159ecb7c..be196092ac 100644 --- a/hermes_cli/cli_agent_setup_mixin.py +++ b/hermes_cli/cli_agent_setup_mixin.py @@ -650,11 +650,13 @@ class CLIAgentSetupMixin: effective_model = model_override or self.model # -q never builds the prompt_toolkit app, so the clarify modal can't be # answered — answer headless instead of polling until clarify_timeout. + single_query_mode = getattr(self, "_single_query_mode", False) clarify_callback = ( # See #94943. _single_query_clarify_callback - if getattr(self, "_single_query_mode", False) + if single_query_mode else self._clarify_callback) + connection_callback = None if single_query_mode else self._connection_callback self.agent = AIAgent( model=effective_model, api_key=runtime.get("api_key"), base_url=runtime.get("base_url"), provider=runtime.get("provider"), @@ -676,7 +678,7 @@ class CLIAgentSetupMixin: provider_data_collection=self._provider_data_collection, openrouter_min_coding_score=self._openrouter_min_coding_score, session_id=self.session_id, platform="cli", session_db=self._session_db, - clarify_callback=clarify_callback, + clarify_callback=clarify_callback, connection_callback=connection_callback, reasoning_callback=self._current_reasoning_callback(), fallback_model=self._fallback_model, thinking_callback=self._on_thinking, checkpoints_enabled=self.checkpoints_enabled, diff --git a/hermes_cli/cli_modal_mixin.py b/hermes_cli/cli_modal_mixin.py index 68b993d14e..0d69b8d5f1 100644 --- a/hermes_cli/cli_modal_mixin.py +++ b/hermes_cli/cli_modal_mixin.py @@ -12,6 +12,7 @@ import queue import sys import threading import time as _time +import webbrowser from hermes_cli.callbacks import prompt_for_secret from typing import Optional @@ -81,6 +82,8 @@ class CLIModalMixin: """Modal overlays for the interactive CLI: clarify, approval, sudo/secret capture, command palette, slash-confirm, external editor.""" + _connection_state = None + def _open_external_editor(self, buffer=None) -> bool: """Open the active input buffer in an external editor.""" from cli import _DIM, _RST, _cprint @@ -92,7 +95,8 @@ class CLIModalMixin: _cprint(f"{_DIM}Wait for the current command to finish before opening the editor.{_RST}") return False if (self._sudo_state or self._secret_state or self._approval_state - or getattr(self, "_slash_confirm_state", None) or self._clarify_state): + or getattr(self, "_slash_confirm_state", None) or self._clarify_state + or self._connection_state): _cprint(f"{_DIM}Finish the active prompt before opening the editor.{_RST}") return False target_buffer = buffer or getattr(app, "current_buffer", None) @@ -423,7 +427,8 @@ class CLIModalMixin: if getattr(self, "_command_palette_state", None): return if (self._model_picker_state or self._clarify_state or self._approval_state - or self._slash_confirm_state or self._sudo_state or self._secret_state): + or self._slash_confirm_state or self._sudo_state or self._secret_state + or self._connection_state): return self._capture_modal_input_snapshot() self._command_palette_state = { @@ -623,6 +628,324 @@ class CLIModalMixin: _cprint(f"\n{_DIM}(clarify timed out after {timeout}s — agent will decide){_RST}") return _CLARIFY_TIMEOUT_REPLY + # --- Connection setup -------------------------------------------------- + def _connection_operation(self, payload): + from tools.connectors import live + + session_key = getattr(self, "session_id", "") or "" + op_id = str(payload.get("op_id") or "") + return live.get(session_key, op_id) or live.current(session_key) + + def _connection_install_hook(self) -> bool: + from tools.connectors.operation import ConnectionOperation + + if ConnectionOperation.on_change is not None: + return False + ConnectionOperation.on_change = self._connection_on_change + return True + + def _connection_restore_hook(self) -> None: + from tools.connectors.operation import ConnectionOperation + + if ConnectionOperation.on_change == self._connection_on_change: + ConnectionOperation.on_change = None + + def _connection_close(self) -> None: + self._connection_state = None + self._connection_restore_hook() + self._restore_modal_input_snapshot() + self._paint_now() + + @staticmethod + def _connection_fields(target) -> list[dict]: + return [dict(field) for field in target.get("required_env") or () if isinstance(field, dict)] + + @staticmethod + def _connection_opening_phase(target) -> str: + """The phase a target opens in. A pending install or enable waits for the user's Connect + even with no fields to fill; a pending authorize is the backend still minting the link.""" + target_state = target.get("state") + if target_state == "initiated" and target.get("connect_url"): + return "url" + if target_state == "failed": + return "failed" + if target_state == "pending" and target.get("action") != "authorize": + return "form" + return "waiting" + + def _connection_show_target(self, payload, index: int) -> None: + targets = payload.get("targets") or [] + if not targets: + self._connection_close() + return + index = max(0, min(index, len(targets) - 1)) + target = targets[index] + fields = self._connection_fields(target) + previous = self._connection_state or {} + drafts = previous.setdefault("drafts", {}) + target_draft = drafts.setdefault(target.get("name", ""), {}) + for field in fields: + if "type" not in field: + field["type"] = "secret" if field.get("secret") else "plain" + if field.get("type") != "secret" and field.get("name") not in target_draft: + target_draft[field.get("name")] = str(field.get("default") or "") + self._connection_state = { + **previous, + "payload": payload, + "target_index": index, + "target": target, + "fields": fields, + "field_index": 0, + "selected": 0, + "phase": self._connection_opening_phase(target), + "drafts": drafts, + } + self._connection_sync_input_buffer() + self._paint_now() + + def _connection_active_field_is_secret(self) -> bool: + state = self._connection_state + if not state or state.get("phase") not in {"form", "failed"}: + return False + fields = state.get("fields") or [] + index = state.get("field_index", 0) + return 0 <= index < len(fields) and fields[index].get("type") == "secret" + + def _connection_prefill_text(self) -> str: + state = self._connection_state + if not state or self._connection_active_field_is_secret(): + return "" + fields = state.get("fields") or [] + index = state.get("field_index", 0) + if not (0 <= index < len(fields)): + return "" + field = fields[index] + target_name = state.get("target", {}).get("name", "") + return str(state.get("drafts", {}).get(target_name, {}).get(field.get("name"), "")) + + def _connection_sync_input_buffer(self) -> None: + app = getattr(self, "_app", None) + if app is None: + return + + def _apply() -> None: + try: + buf = app.current_buffer + buf.text = self._connection_prefill_text() + buf.cursor_position = len(buf.text) + except Exception: + pass + + loop = getattr(app, "loop", None) + if loop is not None and threading.current_thread() is not threading.main_thread(): + try: + loop.call_soon_threadsafe(_apply) + return + except Exception: + pass + _apply() + + def _connection_callback(self, payload): + """Collect the first setup decision, then return while operation updates repaint the panel.""" + if not isinstance(payload, dict): + return None + self._capture_modal_input_snapshot() + installed = self._connection_install_hook() + self._connection_show_target(payload, 0) + state = self._connection_state + if state is None: + return None + state["owns_hook"] = installed + state["tool_thread_id"] = threading.current_thread().ident + if state["phase"] == "waiting": + return None + response_queue = queue.Queue() + state["response_queue"] = response_queue + self._ring_bell(prompt=True, context="connection setup") + return response_queue.get() + + def _connection_answer(self, *, approve: bool) -> None: + state = self._connection_state + if not state: + return + target = state["target"] + operation = self._connection_operation(state["payload"]) + if operation is None: + self._connection_close() + return + name = str(target.get("name") or "") + answer = {"name": name, "status": "approved" if approve else "skipped"} + if approve: + answer["env"] = dict(state.get("drafts", {}).get(name, {})) + from tools.connectors.mcp import apply_answer + + # The backend applies the answer on this thread, and its change hook sets the next phase + # (the URL step, the form again for a missing field) before apply_answer returns. Set the + # waiting phase first so it cannot overwrite that. + state["phase"] = "waiting" + apply_answer(operation, json.dumps({"targets": [answer]})) + response_queue = state.get("response_queue") + if response_queue is not None: + response_queue.put(answer["status"]) + state.pop("response_queue", None) + self._paint_now() + + def _connection_retry(self) -> None: + state = self._connection_state + if not state: + return + operation = self._connection_operation(state["payload"]) + if operation is None: + self._connection_close() + return + target = state["target"] + if target.get("state") in {"pending", "failed", "expired"}: + # Connect on a failed row is the same attempt with the values now in the draft. + self._connection_answer(approve=True) + return + from tools.connectors.mcp import retry + + state["phase"] = "waiting" + retry(operation, [str(target.get("name") or "")]) + self._paint_now() + + def _connection_continue(self) -> None: + state = self._connection_state + if not state: + return + operation = self._connection_operation(state["payload"]) + if operation is not None: + from tools.connectors.mcp import apply_answer + + apply_answer(operation, json.dumps({"settled_by": "continue"})) + self._connection_close() + + def _connection_open_url(self) -> None: + state = self._connection_state + if state and state.get("phase") == "url" and state["target"].get("connect_url"): + webbrowser.open(state["target"]["connect_url"]) + + def _connection_cancel(self) -> None: + self._connection_answer(approve=False) + + def _connection_interrupt(self) -> None: + state = self._connection_state + if state: + from tools.interrupt import set_interrupt + + operation = self._connection_operation(state["payload"]) + set_interrupt(True, thread_id=state.get("tool_thread_id")) + if operation is not None: + operation.wake.set() + self._connection_close() + + def _connection_on_change(self, operation, _change, snapshot) -> None: + state = self._connection_state + if not state or operation.op_id != state["payload"].get("op_id"): + return + if snapshot.get("settled_at") is not None: + self._connection_close() + return + active_name = state["target"].get("name") + target = next((item for item in snapshot.get("targets") or () if item.get("name") == active_name), None) + if target is None: + return + state["payload"] = snapshot + state["target"] = target + target_state = target.get("state") + if target_state == "initiated" and target.get("connect_url"): + state["phase"] = "url" + elif target_state == "failed": + state["phase"] = "failed" + elif target_state == "pending" and self._connection_fields(target): + # The backend refused the answer because a required field is still empty: reopen the + # form on the first one it named, over the draft the panel kept. + missing = {field.get("name") for field in self._connection_fields(target)} + names = [field.get("name") for field in state.get("fields") or []] + state["field_index"] = next((i for i, name in enumerate(names) if name in missing), 0) + state["phase"] = "form" + self._connection_sync_input_buffer() + elif target_state == "connected" and target.get("discovery_error"): + state["phase"] = "authorized" + elif target_state in {"connected", "skipped"}: + unresolved = [ + item for item in snapshot.get("targets") or () + if item.get("state") not in {"connected", "skipped"} + ] + if unresolved: + self._connection_show_target(snapshot, (snapshot.get("targets") or []).index(unresolved[0])) + return + state["phase"] = "connected" + else: + state["phase"] = "waiting" + self._paint_now() + + def _connection_set_field(self, value: str) -> None: + """Save the active input row in the private draft and advance to the next row/action.""" + state = self._connection_state + if not state or state.get("phase") not in {"form", "failed"}: + return + fields = state.get("fields") or [] + index = state.get("field_index", 0) + if not (0 <= index < len(fields)): + return + field = fields[index] + state["drafts"][state["target"].get("name", "")][field.get("name")] = value + state["field_index"] = min(index + 1, len(fields)) + self._connection_sync_input_buffer() + self._paint_now() + + def _connection_submit(self) -> None: + """Enter action for prompt_toolkit bindings: URL open, selected action, or field advance.""" + state = self._connection_state + if not state: + return + phase = state.get("phase") + if phase == "url": + self._connection_open_url() + elif phase == "authorized": + self._connection_continue() + elif phase in {"form", "failed"} and state.get("field_index", 0) >= len(state.get("fields") or []): + (self._connection_retry if state.get("selected", 0) == 0 else self._connection_cancel)() + + def _connection_field_lines(self, state, target) -> list[str]: + draft = state.get("drafts", {}).get(target.get("name", ""), {}) + lines = [] + for field in state.get("fields") or (): + marker = "*" if field.get("required") else "" + value = "Set" if field.get("type") == "secret" and draft.get(field.get("name")) else draft.get(field.get("name"), "") + lines.append(f"{field.get('prompt') or field.get('name')}{marker}: {value}") + return lines + + def _connection_render_lines(self) -> list[str]: + """Panel copy without exposing secret drafts; the prompt_toolkit renderer consumes these rows.""" + state = self._connection_state + if not state: + return [] + target = state["target"] + phase = state.get("phase") + lines = [f"Set up {target.get('name', '')}"] + if target.get("instructions"): + lines.append(str(target["instructions"])) + if phase == "form": + lines.extend(self._connection_field_lines(state, target)) + lines.append("Connect Cancel") + elif phase == "url": + lines.extend([str(target.get("connect_url") or ""), str(target.get("detail") or ""), "Press Enter to open in browser"]) + elif phase == "failed": + lines.append(str(target.get("detail") or "Connection failed")) + lines.extend(self._connection_field_lines(state, target)) + lines.append("Connect Cancel") + elif phase == "authorized": + # A connected row cannot be re-run inside this operation; the agent retries discovery + # with its next manage_connections call, which needs no new consent. + lines.extend(["Authorized. Tools unavailable.", "Continue"]) + elif phase == "connected": + lines.append("Connected") + else: + lines.append(str(target.get("detail") or "Waiting…")) + return [line for line in lines if line] + # --- Batch clarify (multi-question, issue #18450) ----------------------- def _clarify_batch_set_active(self, state, index) -> None: """Point the batch clarify panel at question ``index``: mirror it into the flat keys the @@ -966,6 +1289,8 @@ class CLIModalMixin: if self._approval_state: _put(self._approval_state, "deny") self._approval_state = None + if self._connection_state: + self._connection_interrupt() if self._clarify_state: _put(self._clarify_state, "The user cancelled. Use your best judgement to proceed.") self._clarify_state = None diff --git a/hermes_cli/cli_tui_mixin.py b/hermes_cli/cli_tui_mixin.py index f68d8b4eaf..0d9ef1d347 100644 --- a/hermes_cli/cli_tui_mixin.py +++ b/hermes_cli/cli_tui_mixin.py @@ -307,6 +307,8 @@ class CLITuiMixin: return _state_fragment("class:sudo-prompt", "🔐") if self._secret_state: return _state_fragment("class:sudo-prompt", "🔑") + if self._connection_state: + return _state_fragment("class:prompt-working", "⚙") if self._approval_state or getattr(self, "_slash_confirm_state", None): return _state_fragment("class:prompt-working", "⚠") if self._clarify_freetext: @@ -374,6 +376,7 @@ class CLITuiMixin: *, sudo_widget, secret_widget, + connection_widget=None, approval_widget, slash_confirm_widget=None, clarify_widget, @@ -394,6 +397,7 @@ class CLITuiMixin: Window(height=0), sudo_widget, secret_widget, + connection_widget, approval_widget, slash_confirm_widget, clarify_widget, @@ -711,7 +715,9 @@ class CLITuiMixin: return self._render_scroll_list_panel( state, "⚙ Command Palette", hint, labels, min_width=50, max_width=90, indent=' ') - def _render_sudo_style_panel(self, title: str, body_lines: list[str]): + def _render_sudo_style_panel( + self, title: str, body_lines: list[str], row_styles: list[str] | None = None + ): """Bordered ``sudo-*`` panel: blank, each body line, blank, body-final line, blank.""" from cli import _panel_box_width box_width = _panel_box_width(title, body_lines) @@ -720,7 +726,8 @@ class CLITuiMixin: for i, text in enumerate(body_lines): if i == len(body_lines) - 1 and i > 0: panel.blank() - panel.row('class:sudo-text', text) + style = row_styles[i] if row_styles and i < len(row_styles) else 'class:sudo-text' + panel.row(style, text) panel.blank() return panel.close() @@ -765,6 +772,34 @@ class CLITuiMixin: content_lines.insert(1, str(help_text)) return self._render_sudo_style_panel('🔑 Skill Setup Required', content_lines) + def _get_connection_display_fragments(self): + state = self._connection_state + lines = self._connection_render_lines() + if not state or not lines: + return [] + body_lines = lines[1:] + styles = ['class:sudo-text'] * len(body_lines) + phase = state.get("phase") + fields = state.get("fields") or [] + instructions_offset = 1 if state.get("target", {}).get("instructions") else 0 + if phase in {"form", "failed"}: + field_offset = instructions_offset + (1 if phase == "failed" else 0) + field_index = state.get("field_index", 0) + if 0 <= field_index < len(fields): + styles[field_offset + field_index] = 'class:clarify-selected' + elif body_lines: + action_index = len(body_lines) - 1 + styles[action_index] = 'class:clarify-selected' + choices = ["Connect", "Cancel"] + selected = min(1, max(0, state.get("selected", 0))) + body_lines[action_index] = " ".join( + f"▸ {choice}" if i == selected else choice for i, choice in enumerate(choices) + ) + elif phase == "authorized" and body_lines: + styles[-1] = 'class:clarify-selected' + body_lines[-1] = "▸ Continue" + return self._render_sudo_style_panel(lines[0], body_lines, styles) + # (state attr, deadline attr, hint) for the modal prompts with a countdown hint row. _TUI_MODAL_HINTS = ( ("_sudo_state", "_sudo_deadline", ' password hidden · Enter to skip'), @@ -774,6 +809,19 @@ class CLITuiMixin: ) def _tui_hint_text(self): + if self._connection_state: + phase = self._connection_state.get("phase") + hints = { + "form": " type the value, Enter for next field · ↑/↓ move · ESC cancel", + "failed": " type the value, Enter for next field · ↑/↓ move · ESC cancel", + "url": " Enter to open in browser · ESC cancel", + "authorized": " ↑/↓ select, Enter to confirm", + "waiting": " waiting for the backend… · Ctrl+C interrupt", + } + hint = hints.get(phase, " connection setup") + deadline = float(self._connection_state.get("payload", {}).get("deadline_at") or 0) + countdown = f" ({max(0, int(deadline - time.time()))}s)" if deadline else "" + return [('class:hint', hint), ('class:clarify-countdown', countdown)] for state_attr, deadline_attr, hint in self._TUI_MODAL_HINTS: if getattr(self, state_attr): if state_attr == "_sudo_state" and ((self._sudo_state.get("vault_save") or {}).get("step") == "identifier" @@ -874,7 +922,8 @@ class CLITuiMixin: event.app.invalidate() return # Don't START recording during interactive prompts. - if self._clarify_state or self._sudo_state or self._approval_state or self._slash_confirm_state: + if (self._clarify_state or self._sudo_state or self._approval_state + or self._slash_confirm_state or self._connection_state): return # Cut TTS so the user can start talking: stop_playback() just terminates a subprocess; # the stop event drains the streaming pipeline if one is live. @@ -939,7 +988,8 @@ class CLITuiMixin: (left by an earlier interrupt) can't swallow the press before the agent-interrupt branch, leaving the chat frozen (#14026). """ - if not (self._sudo_state or self._secret_state or self._approval_state or self._clarify_state): + if not (self._sudo_state or self._secret_state or self._approval_state + or self._clarify_state or self._connection_state): return False self._clear_active_overlays_for_interrupt() event.app.current_buffer.reset() @@ -1114,8 +1164,12 @@ class CLITuiMixin: return None def _tui_handle_escape_modal(self, event): - """ESC cancels active secret/sudo/slash-confirm prompts.""" - if self._secret_state: + """ESC cancels active secret/sudo/connection/slash-confirm prompts.""" + if self._connection_state: + self._connection_cancel() + event.app.current_buffer.reset() + event.app.invalidate() + elif self._secret_state: self._cancel_secret_capture() event.app.current_buffer.reset() event.app.invalidate() @@ -1197,6 +1251,48 @@ class CLITuiMixin: """Ctrl+G (or Alt+G in VSCode/Cursor) opens the draft in an external editor.""" self._open_external_editor(event.current_buffer) + def _tui_connection_move(self, event, delta: int) -> None: + state = self._connection_state + if not state: + return + phase = state.get("phase") + if phase in {"form", "failed"}: + fields = state.get("fields") or [] + current = state.get("field_index", 0) + if current < len(fields): + field = fields[current] + target_name = state.get("target", {}).get("name", "") + state["drafts"][target_name][field.get("name")] = event.app.current_buffer.text + state["field_index"] = min(len(fields), max(0, current + delta)) + elif delta < 0 and fields: + state["field_index"] = len(fields) - 1 + else: + state["selected"] = (state.get("selected", 0) + delta) % 2 + self._connection_sync_input_buffer() + elif phase == "authorized": + state["selected"] = (state.get("selected", 0) + delta) % 2 + self._paint_now() + event.app.invalidate() + + def _tui_connection_up(self, event): + self._tui_connection_move(event, -1) + + def _tui_connection_down(self, event): + self._tui_connection_move(event, 1) + + def _tui_connection_side(self, event): + state = self._connection_state + if not state: + return + fields = state.get("fields") or [] + if state.get("phase") == "authorized" or ( + state.get("phase") in {"form", "failed"} + and state.get("field_index", 0) >= len(fields) + ): + state["selected"] = 1 - min(1, max(0, state.get("selected", 0))) + self._paint_now() + event.app.invalidate() + def _tui_model_picker_down(self, event): state = self._model_picker_state if not state: @@ -1536,6 +1632,17 @@ class CLITuiMixin: """Enter while a modal overlay is up: submit it. True when handled.""" from cli import _cprint buf = event.app.current_buffer + if self._connection_state: + state = self._connection_state + fields = state.get("fields") or [] + if state.get("phase") in {"form", "failed"} and state.get("field_index", 0) < len(fields): + value = buf.text + buf.reset() + self._connection_set_field(value) + else: + self._connection_submit() + event.app.invalidate() + return True if self._sudo_state: self._sudo_state["response_queue"].put(buf.text) self._sudo_state = None @@ -1815,7 +1922,7 @@ class CLITuiMixin: def _tui_hint_height(self): if ( - self._sudo_state or self._secret_state or self._approval_state + self._sudo_state or self._secret_state or self._approval_state or self._connection_state or self._slash_confirm_state or self._clarify_state or self._command_running): return 1 # Keep a spacer while the agent runs on roomy terminals; reclaim the row on narrow screens. @@ -1869,6 +1976,7 @@ class CLITuiMixin: self._command_status = "" self._secret_state = None # skill-setup secret capture self._secret_deadline = 0 + self._connection_state = None self._attached_images: list[Path] = [] # clipboard image attachments self._image_counter = 0 @@ -1926,8 +2034,8 @@ class CLITuiMixin: # Buffer.auto_up/auto_down browse history when on the first/last line. _normal_input = Condition( lambda: not self._clarify_state and not self._approval_state and not self._slash_confirm_state - and not self._sudo_state and not self._secret_state and not self._model_picker_state - and not self._command_palette_state) + and not self._sudo_state and not self._secret_state and not self._connection_state + and not self._model_picker_state and not self._command_palette_state) kb.add('up', filter=_normal_input)(self._tui_history_up) kb.add('down', filter=_normal_input)(self._tui_history_down) kb.add('c-l')(self._tui_handle_ctrl_l) @@ -1937,7 +2045,8 @@ class CLITuiMixin: kb.add('c-q')(self._tui_handle_ctrl_q) kb.add('c-d')(self._tui_handle_ctrl_d) _modal_prompt_active = Condition( - lambda: bool(self._secret_state or self._sudo_state or self._slash_confirm_state)) + lambda: bool(self._secret_state or self._sudo_state or self._slash_confirm_state + or self._connection_state)) kb.add('escape', filter=_modal_prompt_active, eager=True)(self._tui_handle_escape_modal) kb.add('escape', 'escape', filter=~_modal_prompt_active)(self._tui_handle_double_escape) kb.add('c-z')(self._tui_handle_ctrl_z) @@ -1959,14 +2068,15 @@ class CLITuiMixin: # unbound there and arrives as ('escape', 'g') — register it as a fallback. _editor_filter = Condition( lambda: not self._clarify_state and not self._approval_state - and not self._sudo_state and not self._secret_state) + and not self._sudo_state and not self._secret_state and not self._connection_state) kb.add('c-g', filter=_editor_filter)( kb.add('escape', 'g', filter=_editor_filter)(self._tui_handle_open_in_editor)) # Ctrl+S prompt stash: park a draft, send something else, bring it back. Suppressed while # a modal prompt owns the composer so Ctrl+S can't stash a password. _stash_filter = Condition( lambda: not self._clarify_state and not self._approval_state and not self._sudo_state - and not self._secret_state and not self._slash_confirm_state and not self._model_picker_state + and not self._secret_state and not self._connection_state and not self._slash_confirm_state + and not self._model_picker_state ) _stash_panel_filter = Condition(lambda: self._prompt_stash.panel_open and bool(len(self._prompt_stash))) kb.add('c-s', filter=_stash_filter)(self._tui_handle_prompt_stash) @@ -1987,6 +2097,11 @@ class CLITuiMixin: and not self._clarify_freetext) kb.add('up', filter=_clarify_nav)(self._tui_clarify_up) kb.add('down', filter=_clarify_nav)(self._tui_clarify_down) + _connection_nav = Condition(lambda: bool(self._connection_state)) + kb.add('up', filter=_connection_nav)(self._tui_connection_up) + kb.add('down', filter=_connection_nav)(self._tui_connection_down) + kb.add('left', filter=_connection_nav)(self._tui_connection_side) + kb.add('right', filter=_connection_nav)(self._tui_connection_side) # Multi-select: Space toggles the checkbox under the cursor. kb.add('space', filter=Condition( lambda: bool(self._clarify_state) and not self._clarify_freetext @@ -2027,7 +2142,7 @@ class CLITuiMixin: kb.add('c-p', filter=Condition( lambda: not self._command_palette_state and not self._model_picker_state and not self._clarify_state and not self._approval_state and not self._slash_confirm_state and not self._sudo_state - and not self._secret_state + and not self._secret_state and not self._connection_state ))(self._tui_open_command_palette) kb.add('up', filter=_palette)(self._tui_command_palette_up) kb.add('down', filter=_palette)(self._tui_command_palette_down) @@ -2119,6 +2234,8 @@ class CLITuiMixin: clarify_widget = self._tui_overlay_widget(self._get_clarify_display_fragments, "_clarify_state") sudo_widget = self._tui_overlay_widget(self._get_sudo_display_fragments, "_sudo_state") secret_widget = self._tui_overlay_widget(self._get_secret_display_fragments, "_secret_state") + connection_widget = self._tui_overlay_widget( + self._get_connection_display_fragments, "_connection_state") approval_widget = self._tui_overlay_widget(self._get_approval_display_fragments, "_approval_state") slash_confirm_widget = self._tui_overlay_widget( self._get_slash_confirm_display_fragments, "_slash_confirm_state") @@ -2160,6 +2277,7 @@ class CLITuiMixin: layout = Layout(FooterSplit(self._build_tui_layout_children( sudo_widget=sudo_widget, secret_widget=secret_widget, + connection_widget=connection_widget, approval_widget=approval_widget, slash_confirm_widget=slash_confirm_widget, clarify_widget=clarify_widget, @@ -2239,7 +2357,9 @@ class CLITuiMixin: filter=Condition(lambda: (bool(cli_ref._sudo_state) and (cli_ref._sudo_state.get("vault_save") or {}).get("step") != "identifier" and not cli_ref._sudo_state.get("vault_code")) - or bool(cli_ref._secret_state)))) + or bool(cli_ref._secret_state) + or (bool(cli_ref._connection_state) + and cli_ref._connection_active_field_is_secret())))) class _PlaceholderProcessor(Processor): """Render grayed-out placeholder text inside the input when empty.""" diff --git a/hermes_cli/mcp_catalog.py b/hermes_cli/mcp_catalog.py index b941c6ddc9..5e7b687cc6 100644 --- a/hermes_cli/mcp_catalog.py +++ b/hermes_cli/mcp_catalog.py @@ -656,6 +656,30 @@ def _apply_tool_selection( _say(f" ✓ {len(chosen_names)}/{len(probed)} tools enabled.") +def card_install_config(entry: CatalogEntry) -> dict: + """The ``mcp_servers.`` block a connection card installs, built in memory. + + Same block :func:`install_entry` writes, minus everything a terminal owns: no prompts, no + probe, no checklist. The caller saves it only once the server accepted the connection. Tool + filter priority matches :func:`_apply_tool_selection`: a prior user selection survives a + reinstall, else the manifest's curated filter, else none. + """ + install_dir = _do_git_install(entry) if entry.install is not None else None + cfg = _build_server_config(entry, install_dir) + cfg["enabled"] = True + prior_include = _read_prior_tool_list(entry.name, "include") + prior_exclude = _read_prior_tool_list(entry.name, "exclude") + if prior_include is not None: + cfg["tools"] = {"include": prior_include} + elif prior_exclude is not None: + cfg["tools"] = {"exclude": prior_exclude} + elif entry.tools.default_excluded: + cfg["tools"] = {"exclude": list(entry.tools.default_excluded)} + elif entry.tools.default_enabled: + cfg["tools"] = {"include": list(entry.tools.default_enabled)} + return cfg + + def install_entry(entry: CatalogEntry, *, enable: bool = True) -> None: """Install a catalog entry end-to-end. diff --git a/hermes_cli/mcp_config.py b/hermes_cli/mcp_config.py index 3c1a1ae2b9..088a094b5c 100644 --- a/hermes_cli/mcp_config.py +++ b/hermes_cli/mcp_config.py @@ -446,6 +446,12 @@ def _probe_single_server( tools_found: List[Tuple[str, str]] = [] async def _probe(): + from tools import mcp_tool as _core + + claimed = [] + claim_token = _core._connect_server_claim.set(claimed.append) + if details is not None: + details["initialized"] = False try: server = await asyncio.wait_for(_connect_server(name, config), timeout=connect_timeout) except asyncio.TimeoutError: @@ -454,6 +460,10 @@ def _probe_single_server( f"Connecting to MCP server '{name}' timed out after {float(connect_timeout):.0f}s " "(bounded by connect_timeout; an OAuth login also by oauth.timeout)" ) from None + finally: + _core._connect_server_claim.reset(claim_token) + if details is not None and claimed: + details["initialized"] = claimed[0].initialize_result is not None try: for t in server._tools: desc = getattr(t, "description", "") or "" diff --git a/tests/agent/test_run_agent.py b/tests/agent/test_run_agent.py index b90e5805c4..2451585aa0 100644 --- a/tests/agent/test_run_agent.py +++ b/tests/agent/test_run_agent.py @@ -2547,10 +2547,16 @@ class TestAgentRuntimePostHookOwnershipSync: "tools.read_window_tool.read_window_below_tool", lambda **kwargs: '{"ok":true}', ) - # manage_connections / setup_mcp shim: no GUI callback on this fake agent, so the MCP - # leg settles `unavailable` without a card; pin the catalog so the run is hermetic. + # manage_connections / setup_mcp shim: no card on this fake agent, so the MCP leg runs the + # backend at once; pin the catalog and the backend so the run is hermetic. monkeypatch.setattr("tools.connectors.mcp._catalog_names", lambda: ["linear"]) monkeypatch.setattr("tools.connectors.mcp._configured_names", lambda: []) + + class _NoInstallBackend: + def required_env(self, name): + return [{"name": "LINEAR_API_KEY", "prompt": "API key", "required": True}] + + monkeypatch.setattr("tools.connectors.mcp._default_backend", _NoInstallBackend) monkeypatch.setattr(agent, "_get_session_db_for_recall", lambda: None) monkeypatch.setattr( agent, diff --git a/tests/hermes_cli/test_cli_clarify_batch.py b/tests/hermes_cli/test_cli_clarify_batch.py index 809dbe42bd..7658c0f911 100644 --- a/tests/hermes_cli/test_cli_clarify_batch.py +++ b/tests/hermes_cli/test_cli_clarify_batch.py @@ -237,6 +237,53 @@ class TestClarifyBatchPanel: thread.join(timeout=2) assert result["value"] == "a" + # The connection modal uses the same agent-thread queue handoff. Connect sends the + # approved target and env directly to the backend, while rendering masks secrets. + cli._connection_state = None + cli._capture_modal_input_snapshot = MagicMock() + cli._restore_modal_input_snapshot = MagicMock() + cli._ring_bell = MagicMock() + cli.session_id = "session" + operation = MagicMock(op_id="op") + payload = { + "op_id": "op", + "targets": [{ + "name": "asana", + "state": "pending", + "instructions": "Use an Asana app.", + "required_env": [ + {"name": "CLIENT_ID", "prompt": "Client ID", "required": True, "default": "default-id"}, + {"name": "CLIENT_SECRET", "prompt": "Client secret", "required": True, "secret": True}, + ], + }], + } + connection_result = {} + with patch.object(cli, "_connection_operation", return_value=operation), patch( + "tools.connectors.mcp.apply_answer" + ) as apply_answer: + connection_thread = threading.Thread( + target=lambda: connection_result.setdefault("value", cli._connection_callback(payload)), daemon=True + ) + connection_thread.start() + deadline = time.time() + 2 + while cli._connection_state is None and time.time() < deadline: + time.sleep(0.01) + state = cli._connection_state + state["drafts"]["asana"]["CLIENT_SECRET"] = "never-render-this" + assert "never-render-this" not in "\n".join(cli._connection_render_lines()) + assert "Client secret*: Set" in cli._connection_render_lines() + cli._connection_answer(approve=True) + connection_thread.join(timeout=2) + + sent = json.loads(apply_answer.call_args.args[1]) + assert sent == {"targets": [{ + "name": "asana", + "status": "approved", + "env": {"CLIENT_ID": "default-id", "CLIENT_SECRET": "never-render-this"}, + }]} + assert connection_result["value"] == "approved" + cli._connection_close() + class TestClarifyBatchNavigation: """Shift-Tab, answer restore on re-visit, and Other edit-prefill.""" diff --git a/tests/hermes_cli/test_mcp_catalog.py b/tests/hermes_cli/test_mcp_catalog.py index 2939172ecc..8426588175 100644 --- a/tests/hermes_cli/test_mcp_catalog.py +++ b/tests/hermes_cli/test_mcp_catalog.py @@ -222,7 +222,8 @@ class TestManifestParsing: "type": "api_key", "env": [ {"name": "DEMO_KEY", "prompt": "API key", "secret": True}, - {"name": "DEMO_URL", "prompt": "Base URL", "secret": False, "required": False}, + {"name": "DEMO_URL", "prompt": "Base URL", "secret": False, + "required": False, "default": "https://demo.example"}, ], } ) @@ -236,6 +237,7 @@ class TestManifestParsing: assert e.auth.env[0].secret is True assert e.auth.env[1].required is False assert e.auth.env[1].secret is False + assert e.auth.env[1].default == "https://demo.example" def test_http_api_key_builds_bearer_headers_template(self, catalog_dir): body = _basic_manifest( diff --git a/tests/hermes_cli/test_mcp_catalog_env_boundary.py b/tests/hermes_cli/test_mcp_catalog_env_boundary.py index 7e131f83ed..a6670c5cb5 100644 --- a/tests/hermes_cli/test_mcp_catalog_env_boundary.py +++ b/tests/hermes_cli/test_mcp_catalog_env_boundary.py @@ -142,23 +142,27 @@ def test_catalog_accepts_declared_credential( catalog_env: Path, monkeypatch: pytest.MonkeyPatch, ): - import hermes_cli.mcp_catalog as mcp_catalog + import hermes_cli.mcp_config as mcp_config + from agent.secret_scope import get_secret + from tools.connectors.mcp import _CatalogBackend - installs: list[str] = [] - monkeypatch.setattr( - mcp_catalog, - "install_entry", - lambda entry, enable=True: installs.append(entry.name), - ) + probes: list[str] = [] - response = client.post( - "/api/mcp/catalog/install", - headers=HEADERS, - json={"name": "demo", "env": {"DEMO_API_KEY": "valid-demo-value"}}, - ) + def probe(name, cfg, **_kwargs): + # The credential is in scope for the probe, and nothing is saved before it answers. + assert get_secret("DEMO_API_KEY") == "valid-demo-value" + assert not (catalog_env / ".env").exists() + assert "demo" not in mcp_config._get_mcp_servers() + probes.append(name) + return [("demo_tool", "")] - assert response.status_code == 200 - assert installs == ["demo"] + monkeypatch.setattr(mcp_config, "_probe_single_server", probe) + + assert _CatalogBackend().install( + "demo", {"DEMO_API_KEY": "valid-demo-value"} + ) == ["demo_tool"] + assert probes == ["demo"] + assert "demo" in mcp_config._get_mcp_servers() assert "DEMO_API_KEY=valid-demo-value" in ( catalog_env / ".env" ).read_text(encoding="utf-8") diff --git a/tests/hermes_cli/test_single_query_clarify.py b/tests/hermes_cli/test_single_query_clarify.py index 1f9caafab4..36241c5afb 100644 --- a/tests/hermes_cli/test_single_query_clarify.py +++ b/tests/hermes_cli/test_single_query_clarify.py @@ -71,6 +71,8 @@ def test_agent_construction_gates_clarify_callback_on_single_query_mode(): ) init_agent_src = inspect.getsource(mixin_mod.CLIAgentSetupMixin._init_agent) assert 'clarify_callback=' in init_agent_src + assert "connection_callback = None if single_query_mode" in init_agent_src + assert "connection_callback=connection_callback" in init_agent_src assert '"_single_query_mode"' in init_agent_src or "'_single_query_mode'" in init_agent_src, ( "the clarify_callback wiring no longer consults _single_query_mode — " "-q turns would hang on the interactive modal again (#94943)" diff --git a/tests/tools/test_connector_local_batches.py b/tests/tools/test_connector_local_batches.py index da393bff9c..8eb354745b 100644 --- a/tests/tools/test_connector_local_batches.py +++ b/tests/tools/test_connector_local_batches.py @@ -58,7 +58,7 @@ def test_single_local_unwrap_keeps_session_db_todo_store_and_setup_callback(tmp_ operation = live.get("current-session", payload["op_id"]) if operation is not None: apply_answer(operation, json.dumps( - {"targets": [{"name": t["name"], "status": "declined"} for t in payload["targets"]]})) + {"targets": [{"name": t["name"], "status": "skipped"} for t in payload["targets"]]})) operation.settle(SettleReason.all_resolved) threading.Timer(0.02, respond).start() diff --git a/tests/tools/test_connector_session_scope.py b/tests/tools/test_connector_session_scope.py index c9cf572a77..b8a264c271 100644 --- a/tests/tools/test_connector_session_scope.py +++ b/tests/tools/test_connector_session_scope.py @@ -41,7 +41,7 @@ def test_connector_scope_controls_schema_discovery_and_execution(monkeypatch, en remote.append("execute") return [{"data": "sent", "error": None} for _ in planned] - def list_connectors(self): + def list_connectors(self, **_): remote.append("status") return [] diff --git a/tests/tools/test_connectors_contract.py b/tests/tools/test_connectors_contract.py index 0546682672..8614fb875e 100644 --- a/tests/tools/test_connectors_contract.py +++ b/tests/tools/test_connectors_contract.py @@ -21,8 +21,8 @@ def test_every_state_is_reachable_from_pending_in_some_kind(): seen.add(nxt) frontier.append(nxt) reachable |= seen - # not_connected / unavailable are stamped by settle(), never transitioned to. - assert reachable | {c.TargetState.not_connected, c.TargetState.unavailable} == set(c.TargetState) + # not_connected is stamped by settle(), never transitioned to. + assert reachable | {c.TargetState.not_connected} == set(c.TargetState) def test_resolved_states_end_the_target_and_are_never_left(): @@ -31,25 +31,20 @@ def test_resolved_states_end_the_target_and_are_never_left(): assert not c.TRANSITIONS.get((kind, state)), (kind, state) -def test_only_the_backend_watcher_or_renderer_flow_may_report_connected(): +def test_only_the_backend_watcher_may_report_connected(): + # The card renders the operation; it never witnesses an outcome, whatever the kind. for (kind, _from), edges in c.TRANSITIONS.items(): actor = edges.get(c.TargetState.connected) if actor is not None: - assert actor in {c.Actor.backend_watcher, c.Actor.renderer_flow}, (kind, _from) + assert actor == c.Actor.backend_watcher, (kind, _from) assert edges.get(c.TargetState.skipped) in {None, c.Actor.user}, (kind, _from) -def test_managed_connected_is_never_a_renderer_claim(): - # The gateway is the only witness of a managed account flip; the card cannot assert it. - for (kind, _from), edges in c.TRANSITIONS.items(): - if kind == "connector": - assert edges.get(c.TargetState.connected) != c.Actor.renderer_flow - - def test_allowed_is_a_pure_lookup(): assert c.allowed("connector", c.TargetState.initiated, c.TargetState.connected) == c.Actor.backend_watcher assert c.allowed("connector", c.TargetState.initiated, c.TargetState.expired) == c.Actor.clock - assert c.allowed("mcp", c.TargetState.initiated, c.TargetState.connected) == c.Actor.renderer_flow + assert c.allowed("mcp", c.TargetState.initiated, c.TargetState.connected) == c.Actor.backend_watcher + assert c.allowed("mcp", c.TargetState.failed, c.TargetState.initiated) == c.Actor.user assert c.allowed("connector", c.TargetState.connected, c.TargetState.pending) is None diff --git a/tests/tools/test_connectors_gateway_client.py b/tests/tools/test_connectors_gateway_client.py index f09cb8b6c4..5df0142ea0 100644 --- a/tests/tools/test_connectors_gateway_client.py +++ b/tests/tools/test_connectors_gateway_client.py @@ -40,7 +40,7 @@ class FakeTransport: def request(self, method, url, *, headers=None, json=None, timeout=None): self.requests.append( - {"method": method, "url": url, "headers": dict(headers or {}), "json": json} + {"method": method, "url": url, "headers": dict(headers or {}), "json": json, "timeout": timeout} ) outcome = self.responses.pop(0) if isinstance(outcome, Exception): @@ -219,6 +219,7 @@ def test_connection_required_stays_inside_the_200_envelope(): "message": "connect gmail", "connector": "gmail", "connectUrl": "https://example.test/connect/1", + "connectionId": "ca_1", }, } ] @@ -228,6 +229,7 @@ def test_connection_required_stays_inside_the_200_envelope(): (result,) = make_client(transport).execute(planned(PLAN_CALLS[:1])) assert result["error"]["code"] == "CONNECTION_REQUIRED" assert result["error"]["connect_url"] == "https://example.test/connect/1" + assert result["error"]["connection_id"] == "ca_1" # --------------------------------------------------------------------------- diff --git a/tests/tools/test_connectors_gateway_wire.py b/tests/tools/test_connectors_gateway_wire.py index 9dfea3b27b..9382310226 100644 --- a/tests/tools/test_connectors_gateway_wire.py +++ b/tests/tools/test_connectors_gateway_wire.py @@ -15,16 +15,19 @@ def test_connections_result_carries_status_reason_under_either_spelling(): assert row.status_reason == "vendor: bad scope" -def test_list_item_accepts_the_seven_states_and_absence(): - for value in ("active", "initiated", "failed", "expired", "revoked", "inactive", "initializing"): - item = wire.ConnectorListItem.model_validate({"connector": "gmail", "connected": False, "connectionStatus": value}) +ITEM = {"connector": "gmail", "connected": False} + + +def test_list_item_accepts_the_six_contract_states_and_absence(): + for value in ("pending", "active", "failed", "expired", "revoked", "inactive"): + item = wire.ConnectorListItem.model_validate({**ITEM, "connectionStatus": value}) assert item.connection_status == value - assert wire.ConnectorListItem.model_validate({"connector": "gmail", "connected": True}).connection_status is None + assert wire.ConnectorListItem.model_validate({**ITEM, "connected": True}).connection_status is None def test_list_item_rejects_an_unknown_status_loudly(): with pytest.raises(ValidationError): - wire.ConnectorListItem.model_validate({"connector": "gmail", "connected": False, "connectionStatus": "weird"}) + wire.ConnectorListItem.model_validate({**ITEM, "connectionStatus": "weird"}) def _connection_required_entry(): diff --git a/tests/tools/test_connectors_managed.py b/tests/tools/test_connectors_managed.py index df3dc8deb0..db8f536e65 100644 --- a/tests/tools/test_connectors_managed.py +++ b/tests/tools/test_connectors_managed.py @@ -16,7 +16,6 @@ from unittest.mock import patch import pytest -import tools.connectors.tool # registers the tool from tools.connectors import contract as c from tools.connectors import live from tools.connectors.tool import MANAGE_CONNECTIONS_SCHEMA, manage_connections @@ -30,44 +29,68 @@ def _clean_live(): class GatewayFake: - """Scripted gateway. ``flips`` maps connector -> the list call number on which it reports connected.""" + """Scripted gateway. - def __init__(self, connected=(), flips=None, mint_status="initiated", status_reason=None): + ``flips`` maps connector -> the account read on which that account first answers ``active``; + ``rows`` maps connector -> the status each successive read answers (the last value repeats, an + entry may be ``None`` for a 404 or an exception to raise). ``list_connectors`` is the reconnect + repair check only; the watcher reads accounts.""" + + def __init__(self, connected=(), flips=None, rows=None, mint_status="initiated", status_reason=None, + mint_connection_id=True): self.connected = set(connected) self.flips = dict(flips or {}) + self.rows = dict(rows or {}) self.mint_status = mint_status self.status_reason = status_reason + self.mint_connection_id = mint_connection_id self.lists = 0 self.mints = [] - self.statuses = {} # slug -> connectionStatus per list call (last value repeats) + self.reads = [] # (connection_id, timeout) in call order + self.slug_of = {} # connection id -> connector slug - def list_connectors(self): + def list_connectors(self, *, timeout=None): self.lists += 1 - for slug, on in self.flips.items(): - if self.lists >= on: - self.connected.add(slug) - rows = [] - for s in ("gmail", "notion"): - row = {"connector": s, "enabled": True, "connected": s in self.connected} - script = self.statuses.get(s) - if script: - row["connectionStatus"] = script[min(self.lists, len(script)) - 1] - row["connected"] = row["connectionStatus"] == "active" - rows.append(row) - return rows + return [{"connector": s, "enabled": True, "connected": s in self.connected} for s in ("gmail", "notion")] - def connections(self, connectors, *, reinitiate=False): - self.mints.append((tuple(connectors), reinitiate)) + def connections(self, connectors, *, reinitiate=False, return_to=None, op=None): + self.mints.append({"connectors": tuple(connectors), "reinitiate": reinitiate, "return_to": return_to, "op": op}) results = [] for slug in connectors: row = {"connector": slug, "status": self.mint_status, "reinitiated": reinitiate} if self.mint_status == "initiated": row["connect_url"] = f"https://connect.example/{slug}/{len(self.mints)}" + if self.mint_connection_id and self.mint_status in ("initiated", "active"): + connection_id = f"ca_{slug}_{len(self.mints)}" + self.slug_of[connection_id] = slug + row["connection_id"] = connection_id if self.status_reason: row["status_reason"] = self.status_reason results.append(row) return {"results": results, "summary": {"total": len(connectors)}} + def account_status(self, connection_id, *, timeout=None): + self.reads.append((connection_id, timeout)) + slug = self.slug_of.get(connection_id, "") + nth = sum(1 for cid, _ in self.reads if cid == connection_id) + status = self._status_of(slug, nth) + if isinstance(status, Exception): + raise status + if status is None: + return None + return {"connectionId": connection_id, "connector": slug, "status": status, + "statusReason": self.status_reason or "", "label": f"{slug}_a", "active": status == "active", + "createdAt": "2026-09-14T10:00:00.000Z", "updatedAt": "2026-09-14T10:00:00.000Z"} + + def _status_of(self, slug, nth): + script = self.rows.get(slug) + if script: + return script[min(nth, len(script)) - 1] + flip = self.flips.get(slug) + if flip is not None and nth >= flip: + return "active" + return "active" if slug in self.connected and not self.flips else "pending" + def _desktop_callback(answer=None): """A callback that emits the card and returns immediately (fire-and-forget, PR2 shape).""" @@ -82,8 +105,10 @@ def _desktop_callback(answer=None): def _run(args, gw, *, callback=None, tick=0.0, platform="desktop"): - with patch("tools.connectors.run.WATCH_INTERVAL_SECONDS", tick), \ - patch("tools.connectors.managed.session_platform", return_value=platform): + # Two seams read the surface: managed decides whether a card exists, the client decides whether a + # return target rides the mint. + with patch("tools.connectors.managed.WATCH_TICK_SECONDS", tick), \ + patch("tools.connectors.gateway.client.session_platform", return_value=platform): return json.loads(manage_connections( args, client_factory=lambda: gw, connection_callback=callback, session_id="s1", )) @@ -117,7 +142,7 @@ def test_desktop_connect_mints_once_emits_the_card_and_returns_outcomes_without_ cb = _desktop_callback() out = _run({"action": "connect", "connectors": ["gmail", "notion"]}, gw, callback=cb) - assert gw.mints == [(("gmail", "notion"), False)] # one mint for every target, up front + assert [m["connectors"] for m in gw.mints] == [("gmail", "notion")] # one mint for every target, up front (payload,) = cb.seen assert payload["op_id"] == out["op_id"] assert [t["name"] for t in payload["targets"]] == ["gmail", "notion"] @@ -147,13 +172,19 @@ def test_watcher_transitions_on_flip_and_settles_by_deadline_when_nothing_flips( out = _run({"action": "connect", "connectors": ["gmail"]}, gw, callback=_desktop_callback(), tick=0.01) assert out["settled_by"] == "deadline" assert out["targets"][0]["state"] == "not_connected" - assert gw.lists >= 2 # it did poll + assert len(gw.reads) >= 2 # it did poll -def test_watcher_polls_once_per_tick_for_the_whole_operation(): - gw = GatewayFake(flips={"gmail": 3, "notion": 3}) +def test_the_watcher_reads_one_account_per_target_per_tick_and_never_the_list(): + """The per-account route replaced the list walk: the watch loop asks for the accounts the mint + named and nothing else, once each per tick, and stops reading a target once it resolves.""" + gw = GatewayFake(flips={"gmail": 3, "notion": 2}) _run({"action": "connect", "connectors": ["gmail", "notion"]}, gw, callback=_desktop_callback()) - assert gw.lists == 3 # shared scan, not one per target + reads = [connection_id for connection_id, _ in gw.reads] + assert gw.lists == 0 + assert reads.count("ca_gmail_1") == 3 + assert reads.count("ca_notion_1") == 2 # connected on read 2; never read again + assert set(reads) == {"ca_gmail_1", "ca_notion_1"} def test_respond_from_the_card_skips_a_target_and_wakes_the_loop(): @@ -201,49 +232,13 @@ def test_reconnect_force_always_reinitiates_even_when_active(): gw = GatewayFake(connected={"gmail"}, flips={"gmail": 1}) with patch("tools.connectors.operation.OPERATION_DEADLINE_SECONDS", 0.05): _run({"action": "reconnect", "connectors": ["gmail"], "force": True}, gw, callback=_desktop_callback(), tick=0.01) - assert gw.mints == [(("gmail",), True)] - - -def test_force_does_not_settle_connected_from_the_old_account(): - """An account switch: the vendor keeps the old account active while the new link waits. `connected` - on the list is the old account until the row has read as anything else once.""" - gw = GatewayFake(connected={"gmail"}) - gw.statuses = {"gmail": ["active", "active", "initializing", "active"]} - out = _run({"action": "reconnect", "connectors": ["gmail"], "force": True}, gw, callback=_desktop_callback(), tick=0.01) - assert out["settled_by"] == "all_resolved" - assert out["targets"][0]["state"] == "connected" - assert gw.lists == 4 # reads 1-2 were the old account; 3 was the new attempt; 4 saw it connected - - -def test_force_reads_a_failed_new_attempt_as_failed_not_as_still_waiting(): - gw = GatewayFake(connected={"gmail"}) - gw.statuses = {"gmail": ["active", "failed"]} - seen = [] - - def cb(payload): - op_id["v"] = payload["op_id"] - - op_id = {} - original = gw.list_connectors - - def spy(): - rows = original() - op = live.get("s1", op_id["v"]) if op_id else None - if op is not None: - seen.append(op.target("gmail").state.value) - return rows - - gw.list_connectors = spy - with patch("tools.connectors.operation.OPERATION_DEADLINE_SECONDS", 0.2): - _run({"action": "reconnect", "connectors": ["gmail"], "force": True}, gw, callback=cb, tick=0.01) - # Read 1 saw the old account (still initiated); the failed row on read 2 was applied, not swallowed. - assert "failed" in seen + assert [(m["connectors"], m["reinitiate"]) for m in gw.mints] == [(("gmail",), True)] def test_reconnect_on_a_disconnected_target_reinitiates(): gw = GatewayFake(flips={"gmail": 2}) _run({"action": "reconnect", "connectors": ["gmail"]}, gw, callback=_desktop_callback()) - assert gw.mints == [(("gmail",), True)] + assert [(m["connectors"], m["reinitiate"]) for m in gw.mints] == [(("gmail",), True)] # --------------------------------------------------------------------------- @@ -257,17 +252,17 @@ def test_off_desktop_connect_returns_links_and_does_not_block(): assert out["status"] == "initiated" assert out["targets"][0]["connect_url"].startswith("https://connect.example/gmail/") assert "op_id" in out - assert gw.lists == 0 # no watcher without a card + assert gw.lists == 0 and gw.reads == [] # no watcher without a card assert live.current("s1") is None -def test_platform_not_callback_presence_decides_the_url(): - # The TUI-in-a-terminal has a gateway callback attached but no card; the URL must be in the result. - gw = GatewayFake() +def test_callback_presence_decides_the_card_path_on_tui(): + gw = GatewayFake(flips={"gmail": 1}) cb = _desktop_callback() out = _run({"action": "connect", "connectors": ["gmail"]}, gw, callback=cb, platform="tui") - assert out["targets"][0]["connect_url"] - assert cb.seen == [] # no card emitted off-desktop + assert len(cb.seen) == 1 + assert out["targets"][0]["state"] == "connected" + assert "connect_url" not in out["targets"][0] # --------------------------------------------------------------------------- @@ -303,22 +298,20 @@ def test_connected_read_on_a_failed_target_is_ignored_not_an_error(): def test_continue_during_a_connected_read_keeps_the_settled_result(): - """Settling while a list read is in flight must not let that read's `connected` raise into tool_error.""" + """Settling while an account read is in flight must not let that read's `active` raise into + tool_error: the Continue landed first, so its frozen result stands and the read is dropped.""" gw = GatewayFake() settled = threading.Event() - original = gw.list_connectors + original = gw.account_status - def slow_list(): - rows = original() - if gw.lists == 2: - live_op = live.get("s1", op_id["v"]) - live_op.settle(c.SettleReason.continue_) - settled.set() - gw.connected.add("gmail") - rows = original() - return rows + def settle_mid_read(connection_id, **kwargs): + row = original(connection_id, **kwargs) + live_op = live.get("s1", op_id["v"]) + live_op.settle(c.SettleReason.continue_) + settled.set() + return dict(row, status="active", active=True) - gw.list_connectors = slow_list + gw.account_status = settle_mid_read op_id = {} def cb(payload): diff --git a/tests/tools/test_connectors_mcp.py b/tests/tools/test_connectors_mcp.py index 13e3ba3715..e907a0199a 100644 --- a/tests/tools/test_connectors_mcp.py +++ b/tests/tools/test_connectors_mcp.py @@ -1,10 +1,10 @@ """MCP targets of manage_connections (the fold that retired setup_mcp). Contracts: -- mixed managed + MCP call off-desktop: managed proceeds, MCP settles ``unavailable`` with the - terminal hint; neither leaks into the other's result -- callback-less registry dispatch is deterministic and never blocks -- the GUI callback round-trip: renderer answer folds into the operation, settles once +- the backend owns the work: authorize mints its own URL, install writes credentials and installs, + enable flips the flag; the card only says approved / skipped / continue +- a card claim of any other state moves nothing +- off the desktop there is no card: the work runs at once and the result carries the link - catalog validation: install is catalog-only, enable/authorize need a configured server - the replay shim keeps an old ``setup_mcp`` call dispatching - deadline ownership: fixed operation deadline + sequential-deadline exemption @@ -17,10 +17,8 @@ from unittest.mock import patch import pytest -import tools.connectors.tool # registers the tool from tools.connectors.contract import SettleReason, TargetState from tools.connectors import live -from tools.connectors import operation as op from tools.connectors.mcp import apply_answer from tools.connectors.tool import MANAGE_CONNECTIONS_SCHEMA, manage_connections from tools.registry import registry @@ -29,6 +27,69 @@ CATALOG = ["figma", "linear", "notion"] CONFIGURED = {"paper": {"command": "paper-mcp"}, "linear": {"url": "https://mcp.linear.app/mcp"}} +class FakeAttempt: + """An OAuth flow in flight, as the watcher reads it.""" + + def __init__(self, auth_url): + self.auth_url = auth_url + self.status = "pending" + self.error = "" + self.tools = [] + self.discovery_error = "" + + def poll(self): + return {"status": self.status, "error": self.error, "tools": list(self.tools), + "discovery_error": self.discovery_error} + + def approve(self, tools): + self.tools, self.status = list(tools), "approved" + + def fail(self, error): + self.error, self.status = error, "error" + + +class FakeBackend: + """The one fake: the catalog, the installer and the OAuth flow runner behind ``mcp.py``.""" + + def __init__(self, *, missing_env=(), tools=("read", "write"), registered_tools=(), + install_error="", registration_error="", oauth_error=""): + self.calls = [] + self.attempts = {} + self.missing_env = list(missing_env) + self.tools = list(tools) + self.registered_tools = list(registered_tools) + self.install_error = install_error + self.registration_error = registration_error + self.oauth_error = oauth_error + + def required_env(self, name): + self.calls.append(("required_env", name)) + return [{"name": key, "prompt": f"{key}?", "required": True, + "secret": True, "default": ""} for key in self.missing_env] + + def start_oauth(self, name): + self.calls.append(("start_oauth", name)) + if self.oauth_error: + raise RuntimeError(self.oauth_error) + attempt = FakeAttempt(f"https://auth.example/{name}/{len(self.attempts) + 1}") + self.attempts[name] = attempt + return attempt + + def install(self, name, env): + self.calls.append(("install", name, dict(env))) + if self.install_error: + raise RuntimeError(self.install_error) + return list(self.tools) + + def enable(self, name): + self.calls.append(("enable", name)) + + +@pytest.fixture +def backend(): + return FakeBackend() + + @pytest.fixture(autouse=True) def _clean_live(): live.reset_for_tests() @@ -37,18 +98,45 @@ def _clean_live(): @pytest.fixture(autouse=True) -def _catalog(): +def _catalog(backend): + # The default backend is patched too: a call that cannot be handed one (registry dispatch, the + # inline executor) must never reach the real catalog or installer from a test. + registered = [] + + def register(runner, target, name): + runner.backend.calls.append(("register", name)) + if runner.backend.registration_error: + from tools.connectors.mcp import _detail + + return [], _detail(runner.backend.registration_error, runner, target) + names = list(runner.backend.registered_tools) + for tool_name in names: + registry.register( + name=tool_name, + toolset=f"mcp-{name}", + schema={"name": tool_name, "description": f"Registered {tool_name}", "parameters": {}}, + handler=lambda *_args, **_kwargs: "{}", + ) + registered.append(tool_name) + return names, "" + with patch("tools.connectors.mcp._catalog_names", return_value=CATALOG), \ patch("tools.connectors.mcp._configured_names", return_value=sorted(CONFIGURED)), \ - patch("tools.connectors.mcp.session_platform", return_value="desktop"): + patch("tools.connectors.mcp._default_backend", return_value=backend), \ + patch("tools.connectors.mcp._register_connected", side_effect=register): yield + with registry._lock: + for tool_name in registered: + registry._tools.pop(tool_name, None) + if registered: + registry._generation += 1 class FakeClient: def __init__(self): self.calls = [] - def list_connectors(self): + def list_connectors(self, **_): self.calls.append("list") return [{"connector": "gmail", "enabled": True, "connected": False}] @@ -57,39 +145,121 @@ class FakeClient: return {"results": [{"connector": c, "status": "initiated", "connect_url": f"https://x/{c}"} for c in connectors]} +def _mcp_target(name): + return {"name": name, "mcp": True} + + def _linear(**kw): return {"name": "linear", "mcp": True, **kw} # --------------------------------------------------------------------------- -# off-desktop: no approval surface +# the card round-trip: the backend does the work, the card answers approved / skipped # --------------------------------------------------------------------------- -def test_mcp_targets_without_a_callback_settle_unavailable_with_the_terminal_hint(): - out = json.loads(manage_connections({"action": "install", "connectors": [_linear()]})) - assert out["status"] == "unavailable" +def _answering(answer, *, session_id="s1", delay=0.01): + """A card that emits (callback returns None) and answers the live operation a moment later, + the way ``connection.respond`` does from the renderer.""" + seen = [] + + def callback(payload): + seen.append(payload) + + def respond(): + operation = live.get(session_id, payload["op_id"]) + if operation is not None: + apply_answer(operation, answer) + + if answer is not None: + threading.Timer(delay, respond).start() + return None + + callback.seen = seen + return callback -def test_mcp_targets_off_the_desktop_settle_unavailable_even_with_a_callback(): - """The Ink TUI has the gateway callback attached but no card; the surface decides, never the callback.""" - callback = _answering(None) - with patch("tools.connectors.mcp.session_platform", return_value="tui"), \ - patch("tools.connectors.operation.OPERATION_DEADLINE_SECONDS", 5): - out = _mcp({"action": "install", "connectors": [_linear()]}, callback) - assert callback.seen == [] - assert out["status"] == "unavailable" - assert out["settled_by"] == SettleReason.unavailable.value - (target,) = out["targets"] - assert target["state"] == TargetState.unavailable.value - assert target["hint"] == "hermes mcp install linear / hermes mcp login linear" +def _mcp(args, callback, **kw): + with patch("tools.connectors.run.WATCH_INTERVAL_SECONDS", 0.01): + return json.loads(manage_connections(args, connection_callback=callback, session_id="s1", **kw)) + + +def test_install_waits_for_the_credentials_it_declares_and_installs_with_them(): + backend = FakeBackend( + missing_env=["FIGMA_TOKEN"], tools=["probe_only"], + registered_tools=["mcp__figma__get_file", "mcp__figma__list_files"], + ) + answer = json.dumps({"targets": [{"name": "figma", "status": "approved", "env": {"FIGMA_TOKEN": "tok-1"}}]}) + callback = _answering(answer) + out = _mcp({"action": "install", "connectors": [_mcp_target("figma")]}, callback, mcp_backend=backend) + + (offered,) = callback.seen[0]["targets"] + assert offered["state"] == TargetState.pending.value + assert offered["required_env"] == [{"name": "FIGMA_TOKEN", "prompt": "FIGMA_TOKEN?", + "required": True, "secret": True, "default": ""}] + assert ("install", "figma", {"FIGMA_TOKEN": "tok-1"}) in backend.calls + (settled,) = out["targets"] + assert settled["state"] == TargetState.connected.value + assert settled["tools"] == ["mcp__figma__get_file", "mcp__figma__list_files"] + assert all(name in settled["tools_listing"] for name in settled["tools"]) + assert "tool_describe" in settled["tools_listing"] and "tool_call" in settled["tools_listing"] + + +def test_a_card_claim_other_than_approved_or_skipped_moves_nothing(backend): + answer = json.dumps({"targets": [{"name": "paper", "status": "connected", "tools": ["x"]}], + "settled_by": "continue"}) + out = _mcp({"action": "enable", "connectors": [_mcp_target("paper")]}, _answering(answer), mcp_backend=backend) + + assert backend.calls == [] + (settled,) = out["targets"] + assert settled["state"] == TargetState.not_connected.value + assert out["settled_by"] == SettleReason.continue_.value + + +def test_no_answer_settles_by_deadline_and_marks_targets_not_connected(backend): + with patch("tools.connectors.operation.OPERATION_DEADLINE_SECONDS", 0.05): + out = _mcp({"action": "install", "connectors": [_linear()]}, _answering(None), mcp_backend=backend) + assert out["settled_by"] == SettleReason.deadline.value + assert out["targets"][0]["state"] == TargetState.not_connected.value assert "error" not in out -def test_registry_dispatch_never_blocks_and_never_reaches_a_card(): +def test_mcp_secrets_never_reach_the_model(): + backend = FakeBackend(missing_env=["LINEAR_API_KEY"], registration_error="sk-secret was rejected") + answer = json.dumps({"targets": [{"name": "linear", "status": "approved", + "env": {"LINEAR_API_KEY": "sk-secret"}}]}) + out = _mcp({"action": "install", "connectors": [_linear()]}, _answering(answer), mcp_backend=backend) + payload = json.dumps(out) + assert "sk-secret" not in payload + assert "[REDACTED]" in payload + assert out["targets"][0]["state"] == TargetState.connected.value + assert out["targets"][0]["tools"] == [] + assert out["targets"][0]["discovery_error"] == "[REDACTED] was rejected" + + +# --------------------------------------------------------------------------- +# off the desktop: no card, so the work runs at once +# --------------------------------------------------------------------------- + + +def _off_desktop(args, **kw): + return json.loads(manage_connections(args, session_id="s1", **kw)) + + +def test_off_desktop_authorize_returns_the_link_at_once_and_opens_no_operation(backend): + out = _off_desktop({"action": "authorize", "connectors": [_mcp_target("paper")]}, mcp_backend=backend) + + (target,) = out["targets"] + assert target["state"] == TargetState.initiated.value + assert target["connect_url"] == "https://auth.example/paper/1" + assert out["status"] == "initiated" + assert live.current("s1") is None + + +def test_registry_dispatch_never_blocks_and_never_reaches_a_card(backend): # registry.dispatch forwards no callback; the call must return, not block. - out = json.loads(registry.dispatch("manage_connections", {"action": "install", "connectors": [_linear()]})) - assert out["status"] == "unavailable" + out = json.loads(registry.dispatch("manage_connections", {"action": "enable", "connectors": [_linear()]})) + assert out["targets"][0]["state"] == TargetState.connected.value def test_a_managed_action_never_accepts_mcp_targets_and_vice_versa(): @@ -131,72 +301,6 @@ def test_install_is_catalog_only_and_lists_the_catalog_on_a_miss(): def test_enable_and_authorize_need_a_configured_server(): out = json.loads(manage_connections({"action": "enable", "connectors": [{"name": "figma", "mcp": True}]})) assert "figma" in out["error"] and "paper" in out["error"] - out = json.loads(manage_connections({"action": "authorize", "connectors": [{"name": "paper", "mcp": True}]})) - assert out["status"] == "unavailable" # known server, no card here - - -# --------------------------------------------------------------------------- -# the GUI round-trip: the card answers through connection.respond, the op settles -# --------------------------------------------------------------------------- - - -def _answering(answer, *, session_id="s1", delay=0.02): - """A card that emits (callback returns None) and answers the live operation a moment later, - the way ``connection.respond`` does from the renderer.""" - seen = [] - - def callback(payload): - seen.append(payload) - - def respond(): - operation = live.get(session_id, payload["op_id"]) - if operation is not None: - apply_answer(operation, answer) - if not operation.settled and answer: - operation.settle(SettleReason.all_resolved if operation.all_resolved else SettleReason.continue_) - - if answer is not None: - threading.Timer(delay, respond).start() - return None - - callback.seen = seen - return callback - - -def _mcp(args, callback, **kw): - with patch("tools.connectors.run.WATCH_INTERVAL_SECONDS", 0.01): - return json.loads(manage_connections(args, connection_callback=callback, session_id="s1", **kw)) - - -def test_callback_answer_folds_into_the_operation_and_settles_once(): - callback = _answering(json.dumps({"settled_by": "all_resolved", "targets": [ - {"name": "linear", "status": "installed", "tools": ["a", "b"]}, - {"name": "figma", "status": "declined"}, - ]})) - out = _mcp({"action": "install", "connectors": [_linear(), {"name": "figma", "mcp": True}]}, callback) - (payload,) = callback.seen - assert "reason" not in payload - assert [t["name"] for t in payload["targets"]] == ["linear", "figma"] - assert payload["timeout_seconds"] == op.OPERATION_DEADLINE_SECONDS - assert out["status"] == "settled" and out["settled_by"] == SettleReason.all_resolved.value - by_name = {t["name"]: t for t in out["targets"]} - assert by_name["linear"]["state"] == TargetState.connected.value and by_name["linear"]["tools"] == ["a", "b"] - assert by_name["figma"]["state"] == TargetState.skipped.value - - -def test_no_answer_settles_by_deadline_and_marks_targets_not_connected(): - with patch("tools.connectors.operation.OPERATION_DEADLINE_SECONDS", 0.05): - out = _mcp({"action": "install", "connectors": [_linear()]}, _answering(None)) - assert out["settled_by"] == SettleReason.deadline.value - assert out["targets"][0]["state"] == TargetState.not_connected.value - assert "error" not in out - - -def test_mcp_secrets_never_reach_the_model(): - # A renderer that echoes a credential field: only the allowed keys survive. - answer = json.dumps({"targets": [{"name": "linear", "status": "installed", "api_key": "sk-secret", "env": {"K": "v"}}]}) - out = _mcp({"action": "install", "connectors": [_linear()]}, _answering(answer)) - assert "sk-secret" not in json.dumps(out) # --------------------------------------------------------------------------- @@ -208,21 +312,21 @@ def _agent(callback): return SimpleNamespace(session_id="s1", connection_callback=callback) -def test_inline_executor_hands_the_agent_callback_to_the_tool(): +def test_inline_executor_hands_the_agent_callback_to_the_tool(backend): from agent.inline_tool_executors import INLINE_TOOL_EXECUTORS, InlineToolContext - callback = _answering(json.dumps({"targets": [{"name": "linear", "status": "installed"}]})) + callback = _answering(json.dumps({"targets": [{"name": "paper", "status": "approved"}]})) with patch("tools.connectors.run.WATCH_INTERVAL_SECONDS", 0.01): out = json.loads(INLINE_TOOL_EXECUTORS["manage_connections"]( - _agent(callback), {"action": "install", "connectors": [_linear()]}, InlineToolContext("task"))) + _agent(callback), {"action": "enable", "connectors": [_mcp_target("paper")]}, InlineToolContext("task"))) assert len(callback.seen) == 1 assert out["targets"][0]["state"] == TargetState.connected.value -def test_setup_mcp_replay_shim_translates_to_an_mcp_target(): +def test_setup_mcp_replay_shim_translates_to_an_mcp_target(backend): from agent.inline_tool_executors import INLINE_TOOL_EXECUTORS, InlineToolContext - callback = _answering(json.dumps({"targets": [{"name": "linear", "status": "declined"}]})) + callback = _answering(json.dumps({"targets": [{"name": "linear", "status": "skipped"}]})) with patch("tools.connectors.run.WATCH_INTERVAL_SECONDS", 0.01): out = json.loads(INLINE_TOOL_EXECUTORS["setup_mcp"]( _agent(callback), {"server": "linear", "action": "install", "reason": "old convo"}, InlineToolContext("task", tool_call_id="call-9"))) @@ -252,12 +356,20 @@ def test_the_bounded_wait_owns_the_deadline_not_the_sequential_guard(): assert "manage_connections" in te._SEQUENTIAL_DEADLINE_EXEMPT_TOOLS -def test_settle_reason_comes_from_target_state_not_the_renderer(): - # The renderer answered one of two targets and claimed all_resolved; the operation is not resolved. - answer = json.dumps({"settled_by": "all_resolved", "targets": [{"name": "linear", "status": "declined"}]}) - out = _mcp({"action": "install", "connectors": [_linear(), {"name": "figma", "mcp": True}]}, _answering(answer)) - assert out["settled_by"] == SettleReason.continue_.value - by_name = {t["name"]: t for t in out["targets"]} - assert by_name["linear"]["state"] == TargetState.skipped.value - assert by_name["figma"]["state"] == TargetState.not_connected.value - assert "detail" not in by_name["figma"] +# --------------------------------------------------------------------------- +# the surface, the actor of a repeated failure, the settle race, the worker guards +# --------------------------------------------------------------------------- + + +def test_a_desktop_session_with_no_callback_gets_the_link_at_once_and_opens_no_operation(backend): + """A call that arrives without the callback (registry dispatch, say from execute_code) has + nothing to render a card, so an operation would block the tool for its whole deadline with + nobody to answer it. The link goes to the model instead, the way it does off the desktop.""" + with patch("tools.connectors.operation.OPERATION_DEADLINE_SECONDS", 0.2), \ + patch("tools.connectors.run.WATCH_INTERVAL_SECONDS", 0.01): + out = json.loads(manage_connections({"action": "authorize", "connectors": [_mcp_target("paper")]}, + connection_callback=None, session_id="s1", mcp_backend=backend)) + + assert out["status"] == "initiated" + assert out["targets"][0]["connect_url"] == "https://auth.example/paper/1" + assert live.current("s1") is None diff --git a/tests/tools/test_connectors_operation.py b/tests/tools/test_connectors_operation.py index 499efa52a6..cca4eaa738 100644 --- a/tests/tools/test_connectors_operation.py +++ b/tests/tools/test_connectors_operation.py @@ -81,12 +81,14 @@ def test_target_keeps_the_link_and_the_mint_detail_across_transitions(): def test_request_payload_carries_the_live_target_snapshot(): - operation = op.ConnectionOperation([op.Target("gmail", "connector", "reconnect")], tool_call_id="call-1") + operation = op.ConnectionOperation( + [op.Target("gmail", "connector", "reconnect", instructions="Finish setup")], + tool_call_id="call-1") operation.transition("gmail", c.TargetState.initiated, c.Actor.backend_watcher, connect_url="https://l/gmail") payload = operation.request_payload() (target,) = payload["targets"] assert target == {"name": "gmail", "kind": "connector", "action": "reconnect", "state": "initiated", - "connect_url": "https://l/gmail"} + "instructions": "Finish setup", "connect_url": "https://l/gmail"} # The model's own id keys the card to its tool row; a later op for the same apps gets a new one. assert payload["tool_call_id"] == "call-1" assert "reason" not in payload diff --git a/tests/tools/test_connectors_tool.py b/tests/tools/test_connectors_tool.py index 39bf8624f7..ce02f76448 100644 --- a/tests/tools/test_connectors_tool.py +++ b/tests/tools/test_connectors_tool.py @@ -17,7 +17,7 @@ class FakeClient: def __init__(self): self.calls = [] - def list_connectors(self): + def list_connectors(self, **_): self.calls.append(("list",)) return [ {"connector": "gmail", "enabled": True, "connected": False}, diff --git a/tests/tui_gateway/test_connector_operation_e2e.py b/tests/tui_gateway/test_connector_operation_e2e.py index 978fdecf60..49d1e21764 100644 --- a/tests/tui_gateway/test_connector_operation_e2e.py +++ b/tests/tui_gateway/test_connector_operation_e2e.py @@ -79,7 +79,7 @@ class FakeConnectorClient: self._connected = set() self._lock = threading.Lock() - def list_connectors(self): + def list_connectors(self, **_): with self._lock: connected = set(self._connected) return [ @@ -87,19 +87,35 @@ class FakeConnectorClient: for name in ("gmail", "notion") ] - def connections(self, connectors, *, reinitiate=False): + def connections(self, connectors, *, reinitiate=False, **_): return { "results": [ { "connector": name, "status": "initiated", "connect_url": f"https://connect.example/{name}", + "connection_id": f"ca_{name}", "reinitiated": reinitiate, } for name in connectors ] } + def account_status(self, connection_id, *, timeout=None): + """The one route the watcher reads: the account the mint named.""" + name = connection_id.split("ca_", 1)[-1] + with self._lock: + active = name in self._connected + return { + "connectionId": connection_id, + "connector": name, + "status": "active" if active else "pending", + "label": f"{name}_a", + "active": active, + "createdAt": "2026-09-14T10:00:00.000Z", + "updatedAt": "2026-09-14T10:00:00.000Z", + } + def set_connected(self, name): with self._lock: self._connected.add(name) @@ -149,7 +165,7 @@ def test_desktop_connect_settles_through_callback_response(owned_session, monkey """The actual tool thread waits for the gateway card outcome, not a fake callback.""" owner, _ = owned_session client = FakeConnectorClient() - monkeypatch.setattr("tools.connectors.run.WATCH_INTERVAL_SECONDS", 0.05) + monkeypatch.setattr("tools.connectors.managed.WATCH_TICK_SECONDS", 0.05) result = {} finished = threading.Event() @@ -238,7 +254,9 @@ def test_cli_connect_returns_urls_without_emitting_a_card(owned_session): {"action": "connect", "connectors": ["gmail", "notion"]}, client_factory=lambda: client, session_id=SID, - connection_callback=server._agent_cbs(SID)["connection_callback"], + # No card exists where no callback is attached (registry dispatch, messaging); + # the classic CLI attaches one now and draws its own panel. + connection_callback=None, ) ) finally: @@ -250,19 +268,19 @@ def test_cli_connect_returns_urls_without_emitting_a_card(owned_session): assert live.current(SID) is None -def test_connection_respond_rejects_managed_connected_claims_and_strangers(owned_session): +def test_connection_respond_ignores_outcome_claims_and_rejects_strangers(owned_session): owner, stranger = owned_session operation = ConnectionOperation([Target("gmail", "connector", "connect")], session_key=SID) live.open(operation) operation.transition("gmail", TargetState.initiated, Actor.backend_watcher) - rejected = _rpc( + refused = _rpc( owner, "connection.respond", op_id=operation.op_id, result={"targets": [{"name": "gmail", "status": "connected"}]}, ) - assert rejected["error"]["code"] == 4002 + assert refused["error"]["code"] == 4002, refused assert operation.target("gmail").state == TargetState.initiated foreign = _rpc( diff --git a/tests/tui_gateway/test_connector_operation_rpc.py b/tests/tui_gateway/test_connector_operation_rpc.py index 8360eba99f..fc7723e7de 100644 --- a/tests/tui_gateway/test_connector_operation_rpc.py +++ b/tests/tui_gateway/test_connector_operation_rpc.py @@ -75,6 +75,19 @@ def _rpc(client, method, **params): client.transport) +def _connect_rpc(client, **params): + """``connectors.connect`` runs on the long-handler pool and writes its reply to the transport.""" + before = len(client.frames) + _rpc(client, "connectors.connect", **params) + deadline = time.time() + 2 + while time.time() < deadline: + replies = [f for f in list(client.frames)[before:] if f.get("id") == 7] + if replies: + return replies[-1] + time.sleep(0.01) + raise AssertionError("no reply") + + def _open_op(): operation = ConnectionOperation([Target("gmail", "connector", "connect"), Target("notion", "connector", "connect")], session_key=SID) @@ -124,14 +137,18 @@ def test_respond_drives_the_live_operation_and_emits_update(owned): assert updates[-1]["payload"]["op_id"] == operation.op_id -def test_respond_cannot_claim_connected_for_a_managed_target(owned): +def test_respond_cannot_claim_an_outcome_for_any_target(owned): + """The card renders the operation; only skip, approve and Continue are its to say. The contract + refuses any other claim (4002) before it reaches the operation.""" owner, _, _ = owned operation = _open_op() operation.transition("gmail", TargetState.initiated, Actor.backend_watcher) reply = _rpc(owner, "connection.respond", op_id=operation.op_id, - result={"targets": [{"name": "gmail", "status": "connected"}]}) - assert reply["error"]["code"] == 4002 + result={"targets": [{"name": "gmail", "status": "connected"}, + {"name": "notion", "status": "failed"}]}) + assert reply["error"]["code"] == 4002, reply assert operation.target("gmail").state == TargetState.initiated + assert operation.target("notion").state == TargetState.pending def test_respond_continue_settles_and_emits_the_settlement_update(owned): @@ -164,7 +181,7 @@ def test_panel_connect_reissues_only_a_dead_link(owned, monkeypatch): mints = [] class Client: - def connections(self, names, *, reinitiate=False): + def connections(self, names, *, reinitiate=False, **_): mints.append((tuple(names), reinitiate)) return {"results": [{"connector": n, "status": "initiated", "connect_url": f"https://l/{n}/2"} for n in names]} @@ -173,23 +190,11 @@ def test_panel_connect_reissues_only_a_dead_link(owned, monkeypatch): monkeypatch.setattr("tools.connectors.connectors_available", lambda: True) monkeypatch.setattr("model_tools._select_tool_names", lambda *a, **k: {"manage_connections"}) - def long_rpc(**params): - # connectors.connect runs on the pool and writes its reply to the transport. - before = len(owner.frames) - _rpc(owner, "connectors.connect", **params) - deadline = time.time() + 2 - while time.time() < deadline: - replies = [f for f in list(owner.frames)[before:] if f.get("id") == 7] - if replies: - return replies[-1] - time.sleep(0.01) - raise AssertionError("no reply") - - refused = long_rpc(connectors=["gmail"]) + refused = _connect_rpc(owner, connectors=["gmail"]) assert refused["error"]["code"] == 4002 and mints == [] assert operation.target("gmail").connect_url == "https://l/gmail/1" - reply = long_rpc(connectors=["notion"]) + reply = _connect_rpc(owner, connectors=["notion"]) assert "result" in reply, reply assert mints == [(("notion",), True)] assert operation.target("notion").state == TargetState.initiated @@ -206,7 +211,7 @@ def test_a_failed_reissue_leaves_the_row_failed_with_no_link(owned, monkeypatch, operation.transition("notion", dead, actor, detail="vendor: nope") class Client: - def connections(self, names, *, reinitiate=False): + def connections(self, names, *, reinitiate=False, **_): return {"results": [{"connector": n, "status": "failed", "status_reason": "vendor: still no"} for n in names]} monkeypatch.setattr("tools.connectors.gateway.client.ConnectorClient", Client) @@ -222,4 +227,3 @@ def test_a_failed_reissue_leaves_the_row_failed_with_no_link(owned, monkeypatch, assert target.state == TargetState.failed assert target.connect_url is None assert target.detail == "vendor: still no" - diff --git a/tests/tui_gateway/test_mcp_oauth_cancel.py b/tests/tui_gateway/test_mcp_oauth_cancel.py index 0fd7918f0e..7e974f015d 100644 --- a/tests/tui_gateway/test_mcp_oauth_cancel.py +++ b/tests/tui_gateway/test_mcp_oauth_cancel.py @@ -16,8 +16,7 @@ def test_cancel_is_scoped_idempotent_and_releases_worker( monkeypatch.setattr(sessions, "_sessions", {}) finished = threading.Event() - def worker(session_id, *_args): - flow = sessions._sessions[session_id]["flow"] + def worker(*_args, flow, on_done=None): try: asyncio.run( flow.publish_authorization_url( @@ -30,9 +29,11 @@ def test_cancel_is_scoped_idempotent_and_releases_worker( flow.mark_error(str(exc)) finally: flow.mark_worker_done() + if on_done is not None: + on_done() finished.set() - monkeypatch.setattr(sessions, "_worker", worker) + monkeypatch.setattr(sessions, "run_worker", worker) home = str(tmp_path / "origin") monkeypatch.setenv("HERMES_HOME", home) result = sessions.start_flow( @@ -118,11 +119,18 @@ def test_session_operations_require_resolved_owner(tmp_path, monkeypatch, operat def test_cancel_does_not_revoke_an_approved_flow(tmp_path, monkeypatch): + from tools.connectors.mcp_oauth import OAuthAttempt + home = str(tmp_path) flow = DashboardOAuthFlow( "approved", "reports", None, home, "http://127.0.0.1:49152/callback" ) + flow.discovery_error = "tools/list unavailable" flow.mark_approved() + assert OAuthAttempt("https://idp.example", flow).poll() == { + "status": "approved", "error": "", "tools": [], + "discovery_error": "tools/list unavailable", + } flow.mark_worker_done() monkeypatch.setattr( sessions, diff --git a/tests/tui_gateway/test_mcp_oauth_client_callback.py b/tests/tui_gateway/test_mcp_oauth_client_callback.py index 5735a88c7d..ae45e1ead3 100644 --- a/tests/tui_gateway/test_mcp_oauth_client_callback.py +++ b/tests/tui_gateway/test_mcp_oauth_client_callback.py @@ -17,12 +17,11 @@ import threading import pytest from hermes_constants import get_hermes_home +from tools.connectors import mcp_oauth +from tools.connectors.mcp_oauth import _validate_client_redirect_uri from tools.mcp_dashboard_oauth import DashboardOAuthFlow from tui_gateway import mcp_oauth_sessions -from tui_gateway.mcp_oauth_sessions import ( - _validate_client_redirect_uri, - deliver_callback_flow, -) +from tui_gateway.mcp_oauth_sessions import deliver_callback_flow # --------------------------------------------------------------------------- @@ -73,9 +72,7 @@ def _fake_worker_publishes_url(monkeypatch, state="teststate123"): carrying *state* and then waits for the callback like the real worker's SDK does.""" - def worker(session_id, hermes_home, server_name, cfg, reconnect_live): - rec = mcp_oauth_sessions._sessions.get(session_id) - flow = rec["flow"] + def worker(hermes_home, server_name, cfg, reconnect_live, *, flow, on_done=None, **_card_options): import asyncio asyncio.run( @@ -91,19 +88,22 @@ def _fake_worker_publishes_url(monkeypatch, state="teststate123"): flow.mark_error(str(exc)) finally: flow.mark_worker_done() + if on_done is not None: + on_done() - monkeypatch.setattr(mcp_oauth_sessions, "_worker", worker) + monkeypatch.setattr(mcp_oauth_sessions, "run_worker", worker) + return worker def test_start_flow_client_redirect_skips_gateway_listener(monkeypatch): - _fake_worker_publishes_url(monkeypatch) + worker = _fake_worker_publishes_url(monkeypatch) bound = [] - real_listener = mcp_oauth_sessions._start_loopback_listener + real_receiver = mcp_oauth._start_loopback_receiver monkeypatch.setattr( - mcp_oauth_sessions, - "_start_loopback_listener", - lambda flow: bound.append(flow) or real_listener(flow), + mcp_oauth, + "_start_loopback_receiver", + lambda flow: bound.append(flow) or real_receiver(flow), ) result = mcp_oauth_sessions.start_flow( @@ -127,6 +127,43 @@ def test_start_flow_client_redirect_skips_gateway_listener(monkeypatch): ) rec["flow"]._worker_done.wait(5) + # The connection-card path does not require a dashboard web server: it binds the same backend + # receiver, registers the flow for callback relay, and carries the SSH paste hint in detail. + import hermes_cli.mcp_config as mcp_config + + monkeypatch.setattr( + mcp_config, + "_get_mcp_servers", + lambda: {"cardy": {"url": "https://mcp.example.com/mcp", "auth": "oauth"}}, + ) + monkeypatch.setattr(mcp_oauth, "run_worker", worker) + monkeypatch.setenv("SSH_CLIENT", "192.0.2.1 12345 22") + attempt = mcp_oauth.start("cardy") + assert attempt.flow.redirect_uri.startswith("http://127.0.0.1:") + assert "copy the full URL" in attempt.detail + assert attempt.flow.flow_id in mcp_oauth_sessions._sessions + deliver_callback_flow( + attempt.flow.flow_id, "cardy", code="authcode", state="teststate123" + ) + attempt.flow._worker_done.wait(5) + + # A pre-registered client owns its pinned listener inside the SDK; the receiver picker must + # publish that URI without attempting a second bind. + pinned = DashboardOAuthFlow( + "pinned", "asana", None, str(get_hermes_home()), "" + ) + pinned_bound = [] + monkeypatch.setattr( + mcp_oauth, "_start_loopback_receiver", lambda flow: pinned_bound.append(flow) + ) + assert mcp_oauth.choose_callback_receiver( + pinned, + {"oauth": {"client_id": "asana-client", "redirect_host": "localhost", + "redirect_port": 27890}}, + ) is None + assert pinned.redirect_uri == "http://localhost:27890/callback" + assert pinned_bound == [] + def test_start_flow_rejects_bad_client_redirect(monkeypatch): _fake_worker_publishes_url(monkeypatch) @@ -211,7 +248,7 @@ def test_loopback_listener_forwards_iss(): import urllib.request flow = _make_session(session_id="sess-relay-loop", server="loopy", state="loopstate") - httpd = mcp_oauth_sessions._start_loopback_listener(flow) + httpd = mcp_oauth._start_loopback_receiver(flow) try: port = httpd.server_address[1] urllib.request.urlopen( diff --git a/tools/connectors/contract.py b/tools/connectors/contract.py index 65912f9f63..696f38f2de 100644 --- a/tools/connectors/contract.py +++ b/tools/connectors/contract.py @@ -16,14 +16,12 @@ class TargetState(str, Enum): skipped = "skipped" failed = "failed" expired = "expired" - unavailable = "unavailable" # Stamped by settle() on every unresolved target; never a transition target. not_connected = "not_connected" class Actor(str, Enum): user = "user" - renderer_flow = "renderer_flow" backend_watcher = "backend_watcher" clock = "clock" @@ -33,17 +31,17 @@ class SettleReason(str, Enum): continue_ = "continue" deadline = "deadline" interrupt = "interrupt" - unavailable = "unavailable" KINDS: Tuple[str, ...] = ("connector", "mcp") -RESOLVED_STATES = frozenset({TargetState.connected, TargetState.skipped, TargetState.unavailable}) +RESOLVED_STATES = frozenset({TargetState.connected, TargetState.skipped}) _S, _A = TargetState, Actor -# (kind, from) -> {to: the only actor allowed to cause it}. A managed `connected` is witnessed by the -# gateway alone; an MCP `connected` (auth completed) is reported by the renderer that ran the flow. +# (kind, from) -> {to: the only actor allowed to cause it}. Every `connected` is witnessed by the +# backend: the gateway's account list for a managed target, the install / enable / OAuth worker for +# an MCP one. The card can only skip a target, or ask for a failed one to be run again. TRANSITIONS: Dict[Tuple[str, TargetState], Dict[TargetState, Actor]] = { ("connector", _S.pending): {_S.initiated: _A.backend_watcher, _S.failed: _A.backend_watcher, _S.skipped: _A.user}, ("connector", _S.initiated): { @@ -51,8 +49,8 @@ TRANSITIONS: Dict[Tuple[str, TargetState], Dict[TargetState, Actor]] = { }, ("connector", _S.failed): {_S.initiated: _A.user, _S.skipped: _A.user}, ("connector", _S.expired): {_S.initiated: _A.user, _S.skipped: _A.user}, - ("mcp", _S.pending): {_S.initiated: _A.renderer_flow, _S.failed: _A.renderer_flow, _S.skipped: _A.user}, - ("mcp", _S.initiated): {_S.connected: _A.renderer_flow, _S.failed: _A.renderer_flow, _S.skipped: _A.user}, + ("mcp", _S.pending): {_S.initiated: _A.backend_watcher, _S.failed: _A.backend_watcher, _S.skipped: _A.user}, + ("mcp", _S.initiated): {_S.connected: _A.backend_watcher, _S.failed: _A.backend_watcher, _S.skipped: _A.user}, ("mcp", _S.failed): {_S.initiated: _A.user, _S.skipped: _A.user}, } diff --git a/tools/connectors/gateway/bridge.py b/tools/connectors/gateway/bridge.py index 2755bededb..61d283e534 100644 --- a/tools/connectors/gateway/bridge.py +++ b/tools/connectors/gateway/bridge.py @@ -122,9 +122,13 @@ def run_remote( ) for plan in planned ] + from tools.connectors.gateway.client import return_to_args + try: client = (client_factory or _default_client_factory)() - remote_results = client.execute(wire_planned) + # A CONNECTION_REQUIRED link minted by this call is the user's next click, so the call names + # the surface that browser should come back to. + remote_results = client.execute(wire_planned, **return_to_args()) entries = splice_remote_results(planned, remote_results) except ToolGatewayError as exc: logger.debug( @@ -163,7 +167,7 @@ def run_remote( # Retry confirmed misses once without disturbing successful sibling slots. try: - fallback_results = client.execute(fallback_planned) + fallback_results = client.execute(fallback_planned, **return_to_args()) fallback_entries = splice_remote_results(fallback_planned, fallback_results) except ToolGatewayError as exc: logger.debug( diff --git a/tools/connectors/gateway/client.py b/tools/connectors/gateway/client.py index 2fdc7005e3..85de9b732c 100644 --- a/tools/connectors/gateway/client.py +++ b/tools/connectors/gateway/client.py @@ -6,6 +6,7 @@ A dispatch-local idempotency key permits one execute retry; connections are neve from __future__ import annotations import logging +import os import uuid from typing import Any, Callable, Optional, Protocol, Sequence @@ -13,17 +14,19 @@ import requests from pydantic import ValidationError from tools.connectors.gateway import wire +from tools.connectors.gateway.config import session_platform from tools.connectors.gateway.errors import ( GatewayAuthError, GatewayUnavailable, ToolGatewayError, parse_gateway_error, ) + from tools.connectors.gateway.merge import PlannedCall logger = logging.getLogger(__name__) -__all__ = ["ConnectorClient", "Transport"] +__all__ = ["ConnectorClient", "Transport", "return_to_args"] DEFAULT_TIMEOUT_SECONDS = 30.0 EXECUTE_TIMEOUT_SECONDS = 60.0 @@ -100,41 +103,73 @@ class ConnectorClient: return wire.ConnectorSchemasResponse.model_validate(payload).model_dump() def connections( - self, connectors: Sequence[str], *, reinitiate: bool = False + self, connectors: Sequence[str], *, reinitiate: bool = False, + return_to: Optional[str] = None, op: Optional[str] = None, ) -> dict[str, Any]: """Never retry: the gateway cannot deduplicate authorization starts.""" body = wire.ConnectorConnectionsRequest( - connectors=list(connectors), reinitiate=reinitiate - ).model_dump(by_alias=True) + connectors=list(connectors), reinitiate=reinitiate, return_to=return_to, op=op, + ).model_dump(by_alias=True, exclude_none=True) payload = self._post(wire.CONNECTOR_CONNECTIONS_PATH, body, retries=0) return wire.ConnectorConnectionsResponse.model_validate(payload).model_dump() - def list_connectors(self) -> list[dict[str, Any]]: + def list_connectors(self, *, timeout: float = DEFAULT_TIMEOUT_SECONDS) -> list[dict[str, Any]]: + """Every page of the session's toolkit list, each typed whole. ``timeout`` applies per page: the + watcher bounds it by the operation's remaining deadline so a stalled page cannot hold the + operation open.""" items: list[dict[str, Any]] = [] cursor: Optional[str] = None for _ in range(20): path = f"{wire.CONNECTORS_PATH}?limit=50" if cursor: path += f"&cursor={cursor}" - payload = self._request("GET", path, None) - if not isinstance(payload, dict) or "error" in payload: - raise ToolGatewayError("invalid connector list page", code="INVALID_RESPONSE") - page = payload.get("items") - if not isinstance(page, list) or any(not isinstance(entry, dict) for entry in page): - raise ToolGatewayError("invalid connector list items", code="INVALID_RESPONSE") - try: - items.extend(wire.ConnectorListItem.model_validate(entry).model_dump(by_alias=True) for entry in page) - except ValidationError as exc: - raise ToolGatewayError(f"invalid connector list item: {exc.errors()[0].get('msg')}", - code="INVALID_RESPONSE") from exc - cursor = payload.get("nextCursor") + page = self._parse(wire.ConnectorListResponse, self._request("GET", path, None, timeout=timeout), + "connector list page") + items.extend(item.model_dump(by_alias=True) for item in page.items) + cursor = page.next_cursor if not cursor: return items - if not isinstance(cursor, str): - raise ToolGatewayError("invalid connector list cursor", code="INVALID_RESPONSE") raise ToolGatewayError("connector list pagination incomplete", code="INVALID_RESPONSE") - def execute(self, planned: Sequence[PlannedCall]) -> list[dict[str, Any]]: + def list_accounts( + self, *, connector: Optional[str] = None, status: Optional[Sequence[str]] = None + ) -> list[dict[str, Any]]: + query = [] + if connector: + query.append(f"connector={connector}") + if status: + query.append(f"status={','.join(status)}") + path = wire.CONNECTOR_ACCOUNTS_PATH + (f"?{'&'.join(query)}" if query else "") + page = self._parse(wire.ConnectorAccountsResponse, self._request("GET", path, None), "connector accounts") + return [row.model_dump(by_alias=True) for row in page.accounts] + + def account_status( + self, connection_id: str, *, timeout: float = DEFAULT_TIMEOUT_SECONDS + ) -> Optional[dict[str, Any]]: + """One account's row; ``None`` when the gateway no longer knows it. A 429 raises ``RateLimited``. + ``timeout`` is the watcher's remaining deadline, so a stalled read cannot outlive its operation.""" + try: + payload = self._request("GET", f"{wire.CONNECTOR_ACCOUNTS_PATH}/{connection_id}", None, timeout=timeout) + except GatewayUnavailable as exc: + if exc.code == "connection_not_found": + return None + raise + return self._parse(wire.ConnectorAccount, payload, "connector account").model_dump(by_alias=True) + + @staticmethod + def _parse(model: Any, payload: Any, what: str) -> Any: + if not isinstance(payload, dict) or "error" in payload: + raise ToolGatewayError(f"invalid {what}", code="INVALID_RESPONSE") + try: + return model.model_validate(payload) + except ValidationError as exc: + first = exc.errors()[0] + raise ToolGatewayError(f"invalid {what}: {'.'.join(str(p) for p in first.get('loc', ()))}: {first.get('msg')}", + code="INVALID_RESPONSE") from exc + + def execute( + self, planned: Sequence[PlannedCall], *, return_to: Optional[str] = None, op: Optional[str] = None, + ) -> list[dict[str, Any]]: """Return gateway results in request order; merge owns length mismatches.""" body = wire.ConnectorExecuteRequest( tools=[ @@ -142,8 +177,9 @@ class ConnectorClient: connector=plan.connector, tool=plan.tool, arguments=plan.arguments ) for plan in planned - ] - ).model_dump(by_alias=True) + ], + return_to=return_to, op=op, + ).model_dump(by_alias=True, exclude_none=True) # Keep the key dispatch-local so its retry reuses it without a shared store. idempotency_key = str(uuid.uuid4()) payload = self._post( @@ -214,7 +250,7 @@ class ConnectorClient: if 200 <= status < 300: return response.json() - error = parse_gateway_error(status, _safe_json(response)) + error = parse_gateway_error(status, _safe_json(response), getattr(response, "headers", None)) if error.retryable and attempt < retries: last_error = error logger.debug( @@ -230,6 +266,19 @@ class ConnectorClient: raise last_error +def return_to_args(*, op: Optional[str] = None) -> dict[str, Any]: + """The ``returnTo`` / ``op`` arguments a connect or execute call carries so the vendor's done page + can send the browser back to the app that asked for the connection. + + Only the desktop registers a URL scheme for that return, so every other surface sends neither + field and keeps the vendor's own done page. The dev build registers ``hermes-dev://`` instead, and + announces itself to its backend with ``HERMES_DESKTOP_DEV_SERVER``.""" + if session_platform() != "desktop": + return {} + target = "hermes-desktop-dev" if os.environ.get("HERMES_DESKTOP_DEV_SERVER") else "hermes-desktop" + return {"return_to": target, "op": op} if op else {"return_to": target} + + def _result_dict(result: wire.ConnectorExecuteResult) -> dict[str, Any]: error = None if result.error is not None: @@ -238,6 +287,8 @@ def _result_dict(result: wire.ConnectorExecuteResult) -> dict[str, Any]: error["connector"] = result.error.connector if result.error.connect_url: error["connect_url"] = result.error.connect_url + if result.error.connection_id: + error["connection_id"] = result.error.connection_id if result.error.hint: error["hint"] = result.error.hint return {"data": result.data, "error": error} diff --git a/tools/connectors/gateway/errors.py b/tools/connectors/gateway/errors.py index 5d20ca6554..1109732aed 100644 --- a/tools/connectors/gateway/errors.py +++ b/tools/connectors/gateway/errors.py @@ -48,19 +48,38 @@ class IdempotencyConflict(ToolGatewayError): """Never retry a reused idempotency key with a different body.""" -def parse_gateway_error(status: int, body: Any) -> ToolGatewayError: - """Parse every gateway error envelope without raising on a malformed body.""" +class RateLimited(ToolGatewayError): + """429 with the gateway's retry hint in seconds; the caller decides whether to wait.""" + + def __init__(self, message: str, *, retry_after: float, **kwargs: Any) -> None: + super().__init__(message, **kwargs) + self.retry_after = retry_after + + +def parse_gateway_error(status: int, body: Any, headers: Optional[Mapping[str, Any]] = None) -> ToolGatewayError: + """Parse every gateway error envelope without raising on a malformed body. + + Three envelope shapes exist: ``{error: {code, message}}`` on most routes, ``{error: ""}`` + on the account routes, and a flat ``{code, message, requestId, retryAfterMs}`` on a 429.""" code = f"HTTP_{status}" message = "" request_id = None + retry_after_ms: Optional[float] = None if isinstance(body, Mapping): envelope = body.get("error") if isinstance(envelope, Mapping): code = str(envelope.get("code") or code) message = str(envelope.get("message") or "") + elif isinstance(envelope, str) and envelope: + code = envelope + elif body.get("code"): + code = str(body["code"]) + message = str(body.get("message") or "") raw_request_id = body.get("requestId") if raw_request_id is not None: request_id = str(raw_request_id) + if isinstance(body.get("retryAfterMs"), (int, float)): + retry_after_ms = float(body["retryAfterMs"]) elif body: message = str(body)[:500] if not message: @@ -77,6 +96,10 @@ def parse_gateway_error(status: int, body: Any) -> ToolGatewayError: return GatewayUnavailable(message, **kwargs) if status == 409: return IdempotencyConflict(message, **kwargs) + if status == 429: + header = (headers or {}).get("Retry-After") if headers else None + retry_after = float(header) if header not in (None, "") else (retry_after_ms or 1000.0) / 1000.0 + return RateLimited(message, retry_after=retry_after, **kwargs) return ToolGatewayError(message, retryable=status >= 500, **kwargs) diff --git a/tools/connectors/gateway/wire.py b/tools/connectors/gateway/wire.py index 3a1a5b4add..0a201dfcde 100644 --- a/tools/connectors/gateway/wire.py +++ b/tools/connectors/gateway/wire.py @@ -14,6 +14,14 @@ CONNECTOR_SEARCH_PATH = f"{CONNECTORS_PATH}/search" CONNECTOR_SCHEMAS_PATH = f"{CONNECTORS_PATH}/schemas" CONNECTOR_EXECUTE_PATH = f"{CONNECTORS_PATH}/execute" CONNECTOR_CONNECTIONS_PATH = f"{CONNECTORS_PATH}/connections" +CONNECTOR_ACCOUNTS_PATH = f"{CONNECTORS_PATH}/accounts" + +# The gateway's six-state account status (contract `CONNECTOR_CONNECTION_STATUSES`; the vendor's +# INITIALIZING and INITIATED both arrive as `pending`). Present only once the session binds an +# account; a value outside this set is a contract break and fails validation. +ConnectionStatus = Literal["pending", "active", "failed", "expired", "revoked", "inactive"] +# Where the vendor's done page sends the browser after consent; the dev desktop registers hermes-dev://. +ConnectorReturnTarget = Literal["hermes-desktop", "hermes-desktop-dev", "portal"] # Hermes dispatch caps batches lower, so client-side chunking is deliberately absent. WIRE_BATCH_MAX = 25 @@ -89,6 +97,8 @@ class ConnectorToolError(_Wire): message: str connector: Optional[str] = None connect_url: Optional[str] = Field(default=None, alias="connectUrl") + # The account the link was minted for; absent when the link mint failed. Absent means nothing to watch. + connection_id: Optional[str] = Field(default=None, alias="connectionId") hint: Optional[str] = None @@ -96,10 +106,16 @@ class ConnectorExecuteCall(_Wire): connector: str tool: str arguments: dict[str, Any] = Field(default_factory=dict) + # On the wire for the multi-account switch; never sent by hermes today, because the vendor answers + # 400 to any value while multi-account is off (contract probe F2). + account: Optional[str] = None class ConnectorExecuteRequest(_Wire): tools: list[ConnectorExecuteCall] + # Ride any CONNECTION_REQUIRED link the call mints back to the surface that asked. + return_to: Optional[ConnectorReturnTarget] = Field(default=None, alias="returnTo") + op: Optional[str] = Field(default=None, min_length=1, max_length=128) class ConnectorExecuteResult(_Wire): @@ -121,23 +137,26 @@ class ConnectorExecuteResponse(_Wire): class ConnectorConnectionsRequest(_Wire): connectors: list[str] reinitiate: bool = False + alias: Optional[str] = None + return_to: Optional[ConnectorReturnTarget] = Field(default=None, alias="returnTo") + # The caller's operation id, echoed on the hermes://connections/done link. + op: Optional[str] = Field(default=None, min_length=1, max_length=128) class ConnectorConnectionResult(_Wire): connector: str status: Literal["active", "initiated", "failed"] connect_url: Optional[str] = Field(default=None, alias="connectUrl") + # The vendor account the mint created or observed. Optional by vendor semantics (a no-auth toolkit + # answers active with none); a target without one has nothing to watch. + connection_id: Optional[str] = Field(default=None, alias="connectionId") + alias: Optional[str] = None instruction: Optional[str] = None # Vendor error_message on ``failed``; the list route never carries it. status_reason: Optional[str] = Field(default=None, alias="statusReason") reinitiated: bool = False -# The gateway's seven-state account status. Present only once an account exists for the -# toolkit; a value outside this set is a contract break and fails validation. -ConnectionStatus = Literal["active", "initiated", "failed", "expired", "revoked", "inactive", "initializing"] - - class ConnectorListItem(_Wire): connector: str enabled: bool = True @@ -147,6 +166,28 @@ class ConnectorListItem(_Wire): disabled_tools: list[str] = Field(default_factory=list, alias="disabledTools") +class ConnectorListResponse(_Wire): + items: list[ConnectorListItem] + next_cursor: Optional[str] = Field(alias="nextCursor") + + +class ConnectorAccount(_Wire): + connection_id: str = Field(alias="connectionId") + connector: str + status: ConnectionStatus + status_reason: Optional[str] = Field(default=None, alias="statusReason") + label: str = Field(min_length=1) + alias: Optional[str] = None + # The newest active account for this connector: the one the vendor executes with. + active: bool + created_at: str = Field(alias="createdAt") + updated_at: str = Field(alias="updatedAt") + + +class ConnectorAccountsResponse(_Wire): + accounts: list[ConnectorAccount] + + class ConnectorConnectionsSummary(_Wire): total: int = 0 active: int = 0 diff --git a/tools/connectors/live.py b/tools/connectors/live.py index b18da4cdd4..2b95a603da 100644 --- a/tools/connectors/live.py +++ b/tools/connectors/live.py @@ -1,11 +1,16 @@ -"""Live operations, one open per session, found by ``op_id``. The RPC layer reads and drives -operations through here; the tool thread that minted one closes it on settle.""" +"""Live operations, one open per (profile, session), found by ``op_id``. The RPC layer reads and +drives operations through here; the tool thread that minted one closes it on settle. + +The profile is part of the key: two multiplexed profiles can carry the same timestamp-based +session key, and each side (the tool thread under the turn's profile override, the RPC under the +session's profile scope) resolves it through ``hermes_home_key``.""" from __future__ import annotations import threading -from typing import Dict, Optional +from typing import Dict, Optional, Tuple +from hermes_constants import get_process_hermes_home, hermes_home_key from tools.connectors.operation import ConnectionOperation @@ -15,34 +20,43 @@ class OperationAlreadyOpen(RuntimeError): self.existing = existing -_open: Dict[str, ConnectionOperation] = {} +_open: Dict[Tuple[str, str], ConnectionOperation] = {} _lock = threading.Lock() +def _key(session_key: str, profile_home: Optional[str]) -> Tuple[str, str]: + """A session record names its profile home only for a non-default profile; the tool thread sees + the same home through its turn override, and the default profile through the process home.""" + return hermes_home_key(profile_home or get_process_hermes_home()), session_key + + def open(operation: ConnectionOperation) -> None: # noqa: A001 - the verb is the API + operation.profile_key = hermes_home_key() + key = (operation.profile_key, operation.session_key) with _lock: - existing = _open.get(operation.session_key) + existing = _open.get(key) if existing is not None and not existing.settled: raise OperationAlreadyOpen(existing) - _open[operation.session_key] = operation + _open[key] = operation -def current(session_key: str) -> Optional[ConnectionOperation]: +def current(session_key: str, *, profile_home: Optional[str] = None) -> Optional[ConnectionOperation]: with _lock: - operation = _open.get(session_key) + operation = _open.get(_key(session_key, profile_home)) return operation if operation is not None and not operation.settled else None -def get(session_key: str, op_id: str) -> Optional[ConnectionOperation]: +def get(session_key: str, op_id: str, *, profile_home: Optional[str] = None) -> Optional[ConnectionOperation]: with _lock: - operation = _open.get(session_key) + operation = _open.get(_key(session_key, profile_home)) return operation if operation is not None and operation.op_id == op_id else None def close(operation: ConnectionOperation) -> None: + key = (operation.profile_key, operation.session_key) with _lock: - if _open.get(operation.session_key) is operation: - del _open[operation.session_key] + if _open.get(key) is operation: + del _open[key] def reset_for_tests() -> None: diff --git a/tools/connectors/managed.py b/tools/connectors/managed.py index 733e55460d..6a55953bfb 100644 --- a/tools/connectors/managed.py +++ b/tools/connectors/managed.py @@ -4,26 +4,44 @@ reads status first and reinitiates only what is not connected (``force`` always reinitiates). On a desktop session the call blocks until the operation settles and the result carries no URL; the card owns the links. Off the desktop the result carries the URLs and returns at once, until -PR3 delivers them as their own message. The watcher hook reads the gateway list once per tick -(the exact-status route replaces this call when the gateway ships it).""" +PR3 delivers them as their own message. The watcher hook reads one route per pending target: +that target's own account row, at 1 Hz.""" from __future__ import annotations import json import logging -from typing import Any, Callable, Dict, List, Optional +import time +from typing import Any, Callable, Dict, List, Optional, Tuple -from tools.connectors.contract import Actor, TargetState -from tools.connectors.gateway.config import operation_session_key, session_platform -from tools.connectors.operation import ConnectionOperation, Target +from tools.connectors.contract import Actor, TargetState, allowed +from tools.connectors.gateway.config import operation_session_key +from tools.connectors.gateway.errors import RateLimited +from tools.connectors.operation import ConnectionOperation, IllegalTransition, Target from tools.connectors.run import Kind, run_operation from tools.registry import tool_error logger = logging.getLogger(__name__) -# Statuses on the gateway list that end an attempt. The list's statusReason is generic copy, so the -# detail recorded at mint time is kept; only a missing detail is filled from the list. -_TERMINAL_LIST_STATUS = {"failed": TargetState.failed, "expired": TargetState.expired, "revoked": TargetState.failed} +# The account route carries its own 180/min budget, so one read per pending target per second stays +# inside it and still flips the card within a second of the user finishing at the vendor. +WATCH_TICK_SECONDS = 1.0 + +# A read never outlives the operation, never asks for less than one second, and never holds the +# loop for more than ten: Continue must be able to return the tool while a gateway hangs. +_MIN_READ_SECONDS = 1.0 +_MAX_READ_SECONDS = 10.0 + +# The six-state account vocabulary -> the state that read ends the attempt in, and who caused it. +# `pending` is not here: it is the attempt still running, and moves nothing. +_ACCOUNT_OUTCOME: Dict[str, Tuple[TargetState, Actor]] = { + "active": (TargetState.connected, Actor.backend_watcher), + "failed": (TargetState.failed, Actor.backend_watcher), + "revoked": (TargetState.failed, Actor.backend_watcher), + "inactive": (TargetState.failed, Actor.backend_watcher), + # The link's TTL ran out; the gateway reports it, the clock caused it. + "expired": (TargetState.expired, Actor.clock), +} NOTE = ( "Settled once. connected → use the app now; skipped → the user chose Not now, do not connect it " @@ -38,15 +56,20 @@ def _default_client(): def _status_by_slug(client: Any) -> Dict[str, Dict[str, Any]]: + """The toolkit list, by slug. Only the reconnect repair check reads it: it answers "is this app + already connected" before any account exists for the watcher to read.""" return {str(i.get("connector", "")).lower(): i for i in client.list_connectors() if isinstance(i, dict)} def mint(client: Any, operation: ConnectionOperation, names: List[str], *, reinitiate: bool, actor: Actor) -> None: """Mint links for ``names`` and apply the gateway's per-app answer to the operation. ``actor`` is - the watcher on the first mint and the user on Try again.""" + the watcher on the first mint and the user on Try again. The operation id rides along so the + vendor's done page can name it on the way back to the desktop.""" + from tools.connectors.gateway.client import return_to_args + if not names: return - response = client.connections(names, reinitiate=reinitiate) + response = client.connections(names, reinitiate=reinitiate, **return_to_args(op=operation.op_id)) for entry in response.get("results", []): name = str(entry.get("connector") or "").lower() target = operation.target(name) @@ -54,13 +77,18 @@ def mint(client: Any, operation: ConnectionOperation, names: List[str], *, reini continue status = str(entry.get("status") or "") detail = str(entry.get("status_reason") or entry.get("statusReason") or "") + connection_id = entry.get("connection_id") or entry.get("connectionId") if status == "active": operation.transition(name, TargetState.initiated, actor) - operation.transition(name, TargetState.connected, Actor.backend_watcher) + operation.transition(name, TargetState.connected, Actor.backend_watcher, connection_id=connection_id) elif status == "initiated": + if not connection_id: + logger.warning("connector %s: the mint named no account, so the watcher cannot read it; " + "only the card or the deadline can end the row", name) operation.transition( name, TargetState.initiated, actor, - connect_url=entry.get("connect_url"), attempt=entry.get("attempt"), detail=detail, + connect_url=entry.get("connect_url"), connection_id=connection_id, attempt=entry.get("attempt"), + detail=detail, ) elif target.state == TargetState.failed: # Failed again: no state change to emit, but the old link is dead and the vendor's text is new. @@ -75,35 +103,72 @@ def mint(client: Any, operation: ConnectionOperation, names: List[str], *, reini operation.transition(name, TargetState.failed, Actor.backend_watcher, detail=detail) -def _observe(client: Any, operation: ConnectionOperation) -> None: +def _status_for(client: Any, target: Target, *, timeout: float) -> Optional[Dict[str, Any]]: + """The one route the watcher reads: that target's own account row. ``None`` means "nothing to + apply this tick" — no account to read, a rate-limit still in force, an account the gateway does + not know yet (404 until the deadline), or a read that failed. A 429 is raised to the tick: its + budget is the principal's, so it is not this one target's to wait out.""" + if not target.connection_id or time.time() < target.next_read_at: + return None try: - status = _status_by_slug(client) + return client.account_status(target.connection_id, timeout=timeout) + except RateLimited: + raise except Exception as exc: - logger.debug("connector watch poll failed: %s", exc) + logger.debug("connector account read failed for %s: %s", target.name, exc) + return None + + +def _apply_read(operation: ConnectionOperation, target: Target, status: str, reason: str) -> None: + """Apply one account read to one target. The RPC thread can move the row while the read is in + flight — a Skip resolves it, a Continue freezes the whole result — and the read then has no + live row to move: it is dropped, not raised into the tool result (that would end the watch + with the operation still open and no card to answer it). Any other refusal is a real + contract violation.""" + outcome = _ACCOUNT_OUTCOME.get(status) + if outcome is None: return + to, actor = outcome + try: + if allowed(target.kind, target.state, to) is None: + # No edge from pending: the read is itself the witness that the attempt started. + operation.transition(target.name, TargetState.initiated, Actor.backend_watcher) + operation.transition(target.name, to, actor, detail=reason or target.detail) + except IllegalTransition: + if not operation.settled and _live(target): + raise + logger.debug("connector %s: %s read dropped, the row is %s", target.name, status, target.state.value) + + +def _live(target: Target) -> bool: + """Only a live attempt (pending, initiated) can be advanced by a gateway read; a failed or + expired link waits for the user, and a resolved row is done.""" + return target.state in (TargetState.pending, TargetState.initiated) + + +def _park(operation: ConnectionOperation, until: float) -> None: + """A 429 is per principal, not per account: every live target waits out the same Retry-After.""" for target in operation.targets: - # Only a live attempt (pending, initiated) can be advanced by a gateway read; a failed or expired - # link waits for the user, and a settled op is frozen. - if operation.settled or target.state not in (TargetState.pending, TargetState.initiated): + if _live(target): + target.next_read_at = until + + +def _observe(client: Any, operation: ConnectionOperation) -> None: + """One account read per live target per tick, sequential: this is the only thread reading them. + A 429 ends the tick: the next read would spend the same refused budget.""" + for target in operation.targets: + # A settled op is frozen; a row that is not live waits for the user or is done. + if operation.settled or not _live(target): continue - row = status.get(target.name) + timeout = min(_MAX_READ_SECONDS, max(_MIN_READ_SECONDS, operation.remaining_seconds())) + try: + row = _status_for(client, target, timeout=timeout) + except RateLimited as exc: + _park(operation, time.time() + exc.retry_after) + return if row is None: continue - row_status = str(row.get("connectionStatus") or "").lower() - if target.awaiting_new_attempt: - if row.get("connected") or row_status == "active": - continue - target.awaiting_new_attempt = False - if row.get("connected"): - if target.state == TargetState.pending: - operation.transition(target.name, TargetState.initiated, Actor.backend_watcher) - operation.transition(target.name, TargetState.connected, Actor.backend_watcher) - continue - terminal = _TERMINAL_LIST_STATUS.get(row_status) - if terminal is not None and target.state == TargetState.initiated: - # `expired` is the link TTL running out; the gateway reports it, the clock caused it. - actor = Actor.clock if terminal == TargetState.expired else Actor.backend_watcher - operation.transition(target.name, terminal, actor, detail=target.detail or str(row.get("statusReason") or "")) + _apply_read(operation, target, str(row.get("status") or "").lower(), str(row.get("statusReason") or "")) def _prepare(client: Any, action: str, force: bool) -> Callable[[ConnectionOperation], None]: @@ -113,10 +178,8 @@ def _prepare(client: Any, action: str, force: bool) -> Callable[[ConnectionOpera mint(client, operation, names, reinitiate=False, actor=Actor.backend_watcher) return if force: + # The re-mint names a new account; the watcher reads that one, never the old row. mint(client, operation, names, reinitiate=True, actor=Actor.backend_watcher) - for target in operation.targets: - if target.state == TargetState.initiated: - target.awaiting_new_attempt = True return status = _status_by_slug(client) repair = [] @@ -173,12 +236,12 @@ def run_managed_action( ) force = bool(args.get("force", False)) session_key = operation_session_key(session_id) - if session_platform() != "desktop" or connection_callback is None: + if connection_callback is None: return _off_desktop_result(client, action, connectors, force, session_key) return run_operation( [Target(n, "connector", action) for n in connectors], Kind(prepare=_prepare(client, action, force), observe=lambda op: _observe(client, op), note=NOTE), - session_key=session_key, tool_call_id=tool_call_id, + session_key=session_key, tool_call_id=tool_call_id, tick_seconds=WATCH_TICK_SECONDS, connection_callback=connection_callback, with_urls_in_result=False, ) except Exception as exc: diff --git a/tools/connectors/mcp.py b/tools/connectors/mcp.py index 3d1948ae26..d91a7b753b 100644 --- a/tools/connectors/mcp.py +++ b/tools/connectors/mcp.py @@ -1,35 +1,50 @@ -"""MCP targets of ``manage_connections``: the renderer runs install / enable / OAuth and answers -through ``connection.respond``; nothing else observes an MCP flow today (PR3 moves OAuth -observation server-side). Calls without an approval callback settle unavailable.""" +"""MCP targets of ``manage_connections``: the backend installs, enables and authorizes; the card is +a projection of the operation and may only say approved, skipped or continue. + +An MCP target runs the same ``run.py`` lifecycle a managed connector runs. ``prepare`` starts an +OAuth flow, or records the credentials an install still needs; the card's approval starts the +install or the enable; ``observe`` reads the outcome on every tick. A session that attaches no +connection callback runs every action at once and receives the authorization URL in the result. +""" from __future__ import annotations +import contextlib +import contextvars import json import logging +import threading +import time +from dataclasses import dataclass, field from typing import Any, Callable, Dict, List, Optional from tools.connectors.contract import Actor, SettleReason, TargetState -from tools.connectors.gateway.config import operation_session_key, session_platform -from tools.connectors.operation import ConnectionOperation, Target +from tools.connectors.gateway.config import operation_session_key +from tools.connectors.operation import ConnectionOperation, IllegalTransition, Target from tools.connectors.run import Kind, run_operation from tools.registry import tool_error logger = logging.getLogger(__name__) -# The renderer's answer vocabulary → target state. A failed approval stays open; a decline resolves. -_OUTCOME_STATES = { - "installed": TargetState.connected, "enabled": TargetState.connected, "authorized": TargetState.connected, - "connected": TargetState.connected, - "declined": TargetState.skipped, "skipped": TargetState.skipped, - "error": TargetState.failed, "failed": TargetState.failed, -} - -UNAVAILABLE_HINT = "hermes mcp install {name} / hermes mcp login {name}" +# One wait for every authorization URL of a call, not one per target: the flows are started +# together, and a provider that is slow to publish its URL must not delay the others. +PREPARE_WAIT_SECONDS = 30.0 NOTE = ( - "Settled once; do not re-ask for any target the user skipped or that timed out — continue " - "without it or ask in chat. Tools of a newly installed or authorized server become available " - "on your next turn." + "Settled once; do not re-ask for any target the user skipped or that timed out. Connected " + "targets' tools are available now through tool_describe/tool_call and are named under " + "tools_listing. A target with discovery_error is authorized but its tools are unavailable; " + "retry discovery with manage_connections using that target's authorize or install action " + "without asking for consent again." +) + +OFF_DESKTOP_NOTE = ( + "No connection callback is attached in this session. Show any connect_url to the user so they " + "open it in a browser, then ask them to say when they are done. Connected targets' tools are " + "available now through tool_describe/tool_call and are named under tools_listing. A target with " + "discovery_error is authorized but its tools are unavailable; retry discovery with " + "manage_connections using that target's authorize or install action without asking for consent " + "again. Do not re-ask for skipped or timed-out targets." ) @@ -68,44 +83,695 @@ def validate_mcp_names(action: str, names: List[str]) -> Optional[str]: ) +# --------------------------------------------------------------------------- +# the backend: the catalog, the installer, the OAuth flow +# --------------------------------------------------------------------------- + + +def _catalog_entry(name: str): + from hermes_cli.mcp_catalog import get_entry + + entry = get_entry(name) + if entry is None: + raise ValueError(f"no catalog entry '{name}'") + return entry + + +class _CatalogBackend: + """The real work behind an MCP target. One object so a caller can pass another one in.""" + + def required_env(self, name: str) -> List[Dict[str, Any]]: + """The credentials the catalog entry declares that have no value yet.""" + from hermes_cli.config import get_env_value + + return [{"name": spec.name, "prompt": spec.prompt, "required": spec.required, + "secret": spec.secret, "default": "" if spec.secret else spec.default} + for spec in (_catalog_entry(name).auth.env or []) if not get_env_value(spec.name)] + + def start_oauth(self, name: str) -> Any: + from tools.connectors import mcp_oauth + + # The TUI/Desktop card path supplies its advertised client redirect in a later part. + return mcp_oauth.start(name, client_redirect_uri=None) + + def installs_with_oauth(self, name: str) -> bool: + """A catalog entry whose own OAuth the card must run. Provider-mediated OAuth is not one: + its token comes from ``hermes auth ``, so the plain probe covers it.""" + auth = _catalog_entry(name).auth + return auth.type == "oauth" and not auth.provider + + def start_install_oauth(self, name: str, env: Dict[str, str]) -> Any: + """Install an OAuth entry through the card's flow. The configuration is built in memory and + lands, together with the setup values, only when ``initialize`` accepts the token.""" + from hermes_cli.mcp_catalog import card_install_config + from tools.connectors import mcp_oauth + + entry = _catalog_entry(name) + _check_declared(name, entry, env) + return mcp_oauth.start(name, cfg=card_install_config(entry), env=env, + on_commit=lambda: _save_env(env)) + + def install(self, name: str, env: Dict[str, str]) -> List[str]: + """Probe the entry's in-memory configuration with ephemeral credentials; save both only + after the server answered. A failure writes nothing, so a failed reinstall keeps the + previous configuration.""" + from agent.secret_scope import current_secret_scope, reset_secret_scope, set_secret_scope + from hermes_cli.mcp_catalog import card_install_config + from hermes_cli.mcp_config import _probe_single_server, _save_mcp_server + + entry = _catalog_entry(name) + _check_declared(name, entry, env) + cfg = card_install_config(entry) + token = set_secret_scope({**dict(current_secret_scope() or {}), **env}) + try: + tools = [str(tool[0]) for tool in (_probe_single_server(name, cfg) or [])] + finally: + reset_secret_scope(token) + if not _save_mcp_server(name, cfg): + raise RuntimeError(f"'{name}' was rejected: suspicious command/args configuration") + _save_env(env) + return tools + + def enable(self, name: str) -> None: + """Flip ``enabled`` under the scope and lock the dashboard's toggle route uses + (``PUT /api/mcp/servers/{name}/enabled``): the two read-modify-write paths run in one + process, so an unserialised write here drops whichever landed first.""" + from hermes_cli.config import load_config, save_config + from hermes_cli.web_routers._common import config_write_scope + + with config_write_scope(None): + config = load_config() + servers = config.get("mcp_servers") + if not isinstance(servers, dict) or not isinstance(servers.get(name), dict): + raise ValueError(f"'{name}' is not a configured MCP server") + servers[name]["enabled"] = True + save_config(config) + + +def _check_declared(name: str, entry: Any, env: Dict[str, str]) -> None: + """Configuring one MCP is not a general env-writing primitive: refuse the whole map before the + first write if any key is undeclared or unwritable.""" + from hermes_cli.config import validate_env_var_name_for_write + + declared = {spec.name for spec in (entry.auth.env or [])} + for key in env: + if key not in declared: + raise ValueError(f"'{name}' does not declare the environment variable {key}") + validate_env_var_name_for_write(key) + + +def _save_env(env: Dict[str, str]) -> None: + from hermes_cli.config import save_env_value + + for key, value in env.items(): + if value: + save_env_value(key, value) + + +def _default_backend() -> Any: + return _CatalogBackend() + + +# --------------------------------------------------------------------------- +# the runner: per-operation work, reachable from the RPC thread by op_id +# --------------------------------------------------------------------------- + + +@dataclass +class _Work: + """One target's work in flight: an OAuth attempt the watcher polls, or a worker's outcome.""" + + attempt: Any = None + done: threading.Event = field(default_factory=threading.Event) + tools: List[str] = field(default_factory=list) + error: str = "" + + +class _Runner: + """The backend plus the work for one operation's targets.""" + + def __init__(self, action: str, backend: Any): + self.action = action + self.backend = backend + self.op_id: Optional[str] = None + self.work: Dict[str, _Work] = {} + # The credentials the card approved, per target. Try again carries none (a failed row has + # no fields), so the install that runs again is the one the user approved. Kept here and + # not on the target: the values are secrets, and the runner is the one object whose life + # is exactly the operation's. + self.approved_env: Dict[str, Dict[str, str]] = {} + + def run(self, table: Dict[str, Callable], operation: ConnectionOperation, target: Target, + env: Optional[Dict[str, str]] = None) -> None: + table[self.action](self, operation, target, env or {}) + + def spawn(self, operation: ConnectionOperation, target: Target, call: Callable[[], Any]) -> None: + """Run one blocking backend call on a worker thread; ``observe`` reports its outcome.""" + if operation.settled: # Continue landed between the state read and here + logger.debug("mcp %s %s: not started, the operation settled first", self.action, target.name) + return + work = _Work() + self.work[target.name] = work + + def body() -> None: + tools: List[str] = [] + error = "" + try: + tools = [str(name) for name in (call() or [])] + except Exception as exc: + error = _detail(exc, self, target) + if operation.settled: + # The result froze while the work ran; there is no row left to report into. + logger.debug("mcp %s %s: outcome dropped, the operation settled first", + self.action, target.name) + return + work.tools, work.error = tools, error + work.done.set() + operation.wake.set() + + # The worker runs in a copy of the calling thread's context: a named-profile turn binds its + # home through a contextvar, and the install must write the credentials into that home. + threading.Thread(target=contextvars.copy_context().run, args=(body,), daemon=True, + name=f"mcp-{self.action}-{target.name}").start() + + def prepare(self, operation: ConnectionOperation) -> None: + _RUNNERS[operation.op_id] = self + self.op_id = operation.op_id + self.operation = operation + if self.action == "authorize" and len(operation.targets) > 1: + self._prepare_together(operation) + return + for target in operation.targets: + self.run(_PREPARE, operation, target) + + def _prepare_together(self, operation: ConnectionOperation) -> None: + """Start every OAuth flow at once and wait for the URLs once. Each flow blocks until its + provider publishes an authorization URL, so a sequential prepare would keep the card empty + for one wait per target. + + The wait bounds how long prepare blocks, not how long a provider may take: a row still + pending afterwards is left to its own thread, which is the only writer of that row and + ends with the URL or the flow's own failure. Failing it here as well would make two + writers of one row, and a URL that arrives a moment later would have no row to land on. + + Each thread runs in its own copy of the calling thread's context: a named-profile turn + binds its home through a contextvar, and the flow resolves ``mcp_servers`` and stores the + token by that home.""" + threads = [threading.Thread(target=contextvars.copy_context().run, + args=(self.run, _PREPARE, operation, target), daemon=True, + name=f"mcp-prepare-{target.name}") for target in operation.targets] + for thread in threads: + thread.start() + deadline = time.time() + PREPARE_WAIT_SECONDS + for thread in threads: + thread.join(max(0.0, deadline - time.time())) + + def observe(self, operation: ConnectionOperation) -> None: + for target in operation.targets: + # Only a live target can be advanced by a read; a failed one waits for Try again. + if operation.settled or target.state not in (TargetState.pending, TargetState.initiated): + continue + _OBSERVE[self.action](self, operation, target) + + def close(self) -> None: + """The operation is over. An attempt still waiting on the browser is stopped when the user + ended the turn, and otherwise left to finish: the card closed, not the authorization. What + it commits is picked up before the session's next turn (``adopt_late_connections``).""" + if self.op_id is not None: + _RUNNERS.pop(self.op_id, None) + operation = getattr(self, "operation", None) + for name, work in list(self.work.items()): + if work.attempt is None or operation is None: + continue + if operation.settled_by == SettleReason.interrupt: + from tools.connectors.mcp_oauth import cancel_attempt + + cancel_attempt(work.attempt.flow) + else: + _LATE_ATTEMPTS.setdefault(operation.session_key, {})[name] = work.attempt + self.work.clear() + + +# session key -> {server: attempt} for OAuth attempts that outlived their card. +_LATE_ATTEMPTS: Dict[str, Dict[str, Any]] = {} + + +def adopt_late_connections(agent: Any) -> List[str]: + """Register the servers whose authorization committed after their card had closed, and add + them to the agent's toolset selection. Runs between turns, so the result that said "not + connected" is followed by a turn in which the tools are there.""" + session_key = operation_session_key(getattr(agent, "session_id", None)) + attempts = _LATE_ATTEMPTS.get(session_key) + if not attempts: + return [] + adopted: List[str] = [] + for name, attempt in list(attempts.items()): + snapshot = attempt.poll() + if snapshot["status"] == "pending": + continue + attempts.pop(name, None) + if snapshot["status"] != "approved" or snapshot.get("discovery_error"): + continue + try: + from tools.mcp_tool_config import _load_mcp_config + from tools.mcp_tool_discovery import register_mcp_servers + + config = _load_mcp_config().get(name) + if isinstance(config, dict): + register_mcp_servers({name: config}) + adopted.append(name) + except Exception: + logger.debug("late MCP connection %s was not adopted", name, exc_info=True) + if not attempts: + _LATE_ATTEMPTS.pop(session_key, None) + enabled = getattr(agent, "enabled_toolsets", None) + if adopted and enabled is not None and "no_mcp" not in enabled: + agent.enabled_toolsets = [*enabled, *(n for n in adopted if n not in enabled)] + return adopted + + +# op_id -> the runner driving it, so the card's answer and Try again (RPC thread) find the work. +_RUNNERS: Dict[str, _Runner] = {} + + +def open_runner(action: str, backend: Any = None) -> _Runner: + """The runner for one MCP operation. ``prepare`` binds it to the operation, so the card's + answer and its Try again — both of which arrive on another thread — find the same work.""" + return _Runner(action, backend or _default_backend()) + + +# Errors that carry no message reach the user as their class name; say what happened instead. +_BARE_ERRORS = { + "CancelledError": "tool discovery was interrupted; run the same action again to list the tools", + "TimeoutError": "the server did not answer in time", +} + + +def _detail(exc: Any, runner: _Runner, target: Target) -> str: + """The user-facing text of a failure. Every value the card submitted for this target is + replaced by exact match before the pattern redactor runs: an opaque credential has no + recognizable shape, so only the runner knows what to remove.""" + from agent.redact import redact_sensitive_text + + text = str(exc) or (_BARE_ERRORS.get(exc.__class__.__name__, exc.__class__.__name__) + if isinstance(exc, BaseException) else "error") + text = _BARE_ERRORS.get(text, text) # a worker hands over the class name of a message-less error + for value in runner.approved_env.get(target.name, {}).values(): + if value: + text = text.replace(value, "[REDACTED]") + return redact_sensitive_text(text, force=True) or "error" + + +def _catalog_instructions(name: str) -> str: + """The manifest's ``post_install`` text for a catalog name; a custom configured server has none.""" + from hermes_cli.mcp_catalog import get_entry + + entry = get_entry(name) + return str(entry.post_install or "") if entry is not None else "" + + +def _move(operation: ConnectionOperation, target: Target, to: TargetState, actor: Actor, **fields: Any) -> bool: + """Move one target from the prepare, worker-outcome or observe path. + + Continue on the RPC thread can settle the operation between any read of the target's state and + this call. A settled operation has a frozen result, so the lost move is dropped rather than + raised into the tool result; anything else is a real contract violation.""" + try: + operation.transition(target.name, to, actor, **fields) + return True + except IllegalTransition: + if not operation.settled: + raise + logger.debug("mcp target %s: %s dropped, the operation settled first", target.name, to.value) + return False + + +def _fail(operation: ConnectionOperation, target: Target, detail: str) -> None: + """Report a failure, whatever the row was doing: a repeated failure has no state change to + emit, only newer text.""" + if operation.settled: + logger.debug("mcp target %s: failure dropped, the operation settled first", target.name) + return + if target.state == TargetState.failed: + operation.refresh(target.name, connect_url=None, detail=detail, actor=Actor.backend_watcher) + return + target.connect_url = None # whatever link the row was offering is dead + _move(operation, target, TargetState.failed, Actor.backend_watcher, detail=detail) + + +def _register_connected(runner: _Runner, target: Target, name: str) -> tuple[List[str], str]: + """Register one committed server in the current profile scope and report its callable names.""" + try: + from tools.mcp_tool_config import _load_mcp_config + from tools.mcp_tool_discovery import register_mcp_servers + + config = _load_mcp_config().get(name) + if not isinstance(config, dict): + raise RuntimeError(f"no committed MCP configuration for '{name}'") + register_mcp_servers({name: config}) + return _registered_tool_names(name), "" + except Exception as exc: + return [], _detail(exc, runner, target) + + +def _registered_tool_names(name: str, wait_seconds: float = 30.0) -> List[str]: + """The server's callable names, read from the registry once its registration has finished. + + Registration is a no-op for a server the process already holds, and that includes one another + task is still connecting: saving the configuration wakes the config watcher, which starts its + own connect, so a large server (hundreds of tools) was reported with no tools three seconds + before they were registered. A server that failed discovery earlier is parked with no tools + and is woken once. A server that finished registering with no tools is a valid empty list.""" + from tools import mcp_tool as _core + from tools.mcp_tool_loop import reconnect_mcp_server + from tools.mcp_tool_scope import _resolve_server_key + from tools.registry import registry + + key = _resolve_server_key(name) + deadline = time.time() + wait_seconds + woken = False + while True: + names = registry.get_tool_names_for_toolset(f"mcp-{name}") + if names or time.time() >= deadline: + return names + if key not in _core._server_connecting: + server = _core._servers.get(key) + finished = server is not None and getattr(server, "session", None) is not None \ + and hasattr(server, "_registered_tool_names") + if finished: + return names + if woken or server is None or not reconnect_mcp_server(name): + return names + woken = True + time.sleep(0.25) + + +def _connect(operation: ConnectionOperation, target: Target, tools: List[str], discovery_error: str = "") -> None: + extra: Dict[str, Any] = {"tools": tools} + if discovery_error: + extra["discovery_error"] = discovery_error + _move(operation, target, TargetState.connected, Actor.backend_watcher, **extra) + + +def _actor(target: Target) -> Actor: + """Try again is the user's move; a first attempt is the backend's.""" + return Actor.user if target.state == TargetState.failed else Actor.backend_watcher + + +def _start_oauth(runner: _Runner, operation: ConnectionOperation, target: Target, env: Dict[str, str]) -> None: + actor = _actor(target) + target.instructions = _catalog_instructions(target.name) + try: + attempt = runner.backend.start_oauth(target.name) + except Exception as exc: + _fail(operation, target, _detail(exc, runner, target)) + return + runner.work[target.name] = _Work(attempt=attempt) + _move(operation, target, TargetState.initiated, actor, connect_url=attempt.auth_url, + detail=getattr(attempt, "detail", "")) + + +def _declare_env(runner: _Runner, operation: ConnectionOperation, target: Target, env: Dict[str, str]) -> None: + """The install row waits pending; the card draws a field per credential it still needs.""" + target.instructions = _catalog_instructions(target.name) + try: + required = runner.backend.required_env(target.name) + except Exception as exc: + _fail(operation, target, _detail(exc, runner, target)) + return + target.required_env = required + + +def _missing_required(runner: _Runner, target: Target, env: Dict[str, str]) -> List[Dict[str, Any]]: + """The declared credentials that still have no value. The install runs on a worker thread, + where ``install_entry``'s prompt for a missing credential would block on stdin forever.""" + declared = runner.backend.required_env(target.name) + return [spec for spec in declared + if spec.get("required", True) and not env.get(str(spec.get("name") or ""))] + + +def _start_install(runner: _Runner, operation: ConnectionOperation, target: Target, env: Dict[str, str]) -> None: + approved = {**runner.approved_env.get(target.name, {}), **env} + try: + missing = _missing_required(runner, target, approved) + except Exception as exc: + _fail(operation, target, _detail(exc, runner, target)) + return + if missing: + # The row stays pending and the card draws a field per credential it still needs; the + # refresh is what tells the renderer to ask again. + target.required_env = missing + operation.refresh(target.name, connect_url=target.connect_url, actor=Actor.backend_watcher, + detail=f"waiting for {', '.join(str(spec['name']) for spec in missing)}") + return + runner.approved_env[target.name] = approved + actor = _actor(target) + target.required_env = [] # the credentials are written by the install; the row stops asking + if not _move(operation, target, TargetState.initiated, actor, detail=""): + return + if _installs_with_oauth(runner, target): + _start_install_oauth(runner, operation, target, approved) + return + runner.spawn(operation, target, lambda: runner.backend.install(target.name, approved)) + + +def _installs_with_oauth(runner: _Runner, target: Target) -> bool: + try: + return bool(runner.backend.installs_with_oauth(target.name)) + except Exception: + return False # the install itself reports a bad entry + + +def _start_install_oauth(runner: _Runner, operation: ConnectionOperation, target: Target, + env: Dict[str, str]) -> None: + """The row is already ``initiated``; publish the authorization link onto it. The same + ``initiated`` + ``connect_url`` pair is what every card reads as its URL step.""" + try: + attempt = runner.backend.start_install_oauth(target.name, env) + except Exception as exc: + _fail_install(runner, operation, target, exc) + return + runner.work[target.name] = _Work(attempt=attempt) + if operation.settled: + return + operation.refresh(target.name, connect_url=attempt.auth_url, actor=Actor.backend_watcher, + detail=getattr(attempt, "detail", "")) + + +def _fail_install(runner: _Runner, operation: ConnectionOperation, target: Target, error: Any) -> None: + """A failed install asks for its fields again, so the card can reopen the form over the draft + it kept. Nothing was saved, so every declared field is still missing.""" + with contextlib.suppress(Exception): + target.required_env = runner.backend.required_env(target.name) + _fail(operation, target, _detail(error, runner, target)) + + +def _do_enable(runner: _Runner, operation: ConnectionOperation, target: Target, env: Dict[str, str]) -> None: + actor = _actor(target) + if not _move(operation, target, TargetState.initiated, actor, detail=""): + return + try: + runner.backend.enable(target.name) + except Exception as exc: + _fail(operation, target, _detail(exc, runner, target)) + return + tools, discovery_error = _register_connected(runner, target, target.name) + _connect(operation, target, tools, discovery_error) + + +def _install_now(runner: _Runner, operation: ConnectionOperation, target: Target, env: Dict[str, str]) -> None: + """Off the desktop nobody can fill a credential in, so a missing one is the answer.""" + try: + missing = [spec["name"] for spec in runner.backend.required_env(target.name) if spec.get("required", True)] + except Exception as exc: + _fail(operation, target, _detail(exc, runner, target)) + return + if missing: + from hermes_constants import display_hermes_home + + _fail(operation, target, f"set {', '.join(missing)} in the environment or " + f"{display_hermes_home()}/.env, then install again") + return + actor = _actor(target) + if _installs_with_oauth(runner, target): + # No card here, so the result carries the link; the flow's own worker commits the install. + try: + attempt = runner.backend.start_install_oauth(target.name, {}) + except Exception as exc: + _fail(operation, target, _detail(exc, runner, target)) + return + runner.work[target.name] = _Work(attempt=attempt) + _move(operation, target, TargetState.initiated, actor, connect_url=attempt.auth_url, + detail=getattr(attempt, "detail", "")) + return + _move(operation, target, TargetState.initiated, actor) + try: + runner.backend.install(target.name, {}) + except Exception as exc: + _fail(operation, target, _detail(exc, runner, target)) + return + tools, discovery_error = _register_connected(runner, target, target.name) + _connect(operation, target, tools, discovery_error) + + +def _observe_oauth(runner: _Runner, operation: ConnectionOperation, target: Target) -> None: + work = runner.work.get(target.name) + if work is None or work.attempt is None: + return + snapshot = work.attempt.poll() + status = snapshot.get("status") + if status not in ("approved", "error"): + return + runner.work.pop(target.name, None) + if status == "approved": + discovery_error = snapshot.get("discovery_error") or "" + if discovery_error: + _connect(operation, target, [], _detail(discovery_error, runner, target)) + else: + tools, registration_error = _register_connected(runner, target, target.name) + _connect(operation, target, tools, registration_error) + return + error = snapshot.get("error") or "the authorization flow failed" + if runner.action == "install": + _fail_install(runner, operation, target, error) + return + _fail(operation, target, _detail(error, runner, target)) + + +def _observe_install(runner: _Runner, operation: ConnectionOperation, target: Target) -> None: + """An install is an OAuth attempt for an OAuth entry and a worker for every other one.""" + work = runner.work.get(target.name) + observe = _observe_oauth if work is not None and work.attempt is not None else _observe_worker + observe(runner, operation, target) + + +def _observe_worker(runner: _Runner, operation: ConnectionOperation, target: Target) -> None: + work = runner.work.get(target.name) + if work is None or not work.done.is_set(): + return + runner.work.pop(target.name, None) + if work.error: + if runner.action == "install": + _fail_install(runner, operation, target, work.error) + return + _fail(operation, target, _detail(work.error, runner, target)) + return + tools, discovery_error = _register_connected(runner, target, target.name) + _connect(operation, target, tools, discovery_error) + + +def _nothing(runner: _Runner, operation: ConnectionOperation, target: Target, env: Dict[str, str]) -> None: + """Authorize needs no approval: the row's verb opens the link the flow already minted.""" + + +_PREPARE = {"authorize": _start_oauth, "install": _declare_env, "enable": _nothing} +_APPROVE = {"authorize": _nothing, "install": _start_install, "enable": _do_enable} +_RETRY = {"authorize": _start_oauth, "install": _start_install, "enable": _do_enable} +_OBSERVE = {"authorize": _observe_oauth, "install": _observe_install, "enable": _observe_worker} +_OFF_DESKTOP = {"authorize": _start_oauth, "install": _install_now, "enable": _do_enable} + + +# --------------------------------------------------------------------------- +# the card's answer and its Try again +# --------------------------------------------------------------------------- + + +def _answer_env(entry: Dict[str, Any]) -> Dict[str, str]: + raw = entry.get("env") + return {str(k): str(v) for k, v in raw.items()} if isinstance(raw, dict) else {} + + def apply_answer(operation: ConnectionOperation, raw: str) -> None: - """Fold the card's ``connection.respond`` payload into the operation. Settlement is derived - from target states afterwards, never from the card's own ``settled_by`` claim.""" + """Fold the card's ``connection.respond`` payload into the operation: a skip, an approval that + starts the backend's work, and Continue. The card never reports an outcome, so any other claim + moves nothing.""" try: answer = json.loads(raw) except (TypeError, ValueError): answer = {} if not isinstance(answer, dict): answer = {} + runner = _RUNNERS.get(operation.op_id) for entry in answer.get("targets") or (): if not isinstance(entry, dict): continue - name = str(entry.get("name") or "").strip().lower() - state = _OUTCOME_STATES.get(str(entry.get("state") or entry.get("status") or "").lower()) - target = operation.target(name) - if target is None or state is None: + target = operation.target(str(entry.get("name") or "").strip().lower()) + if target is None: continue - actor = Actor.user if state == TargetState.skipped else Actor.renderer_flow - extra = {k: v for k, v in entry.items() if k in ("tools",)} - if target.state == TargetState.pending and state != TargetState.skipped: - operation.transition(name, TargetState.initiated, Actor.renderer_flow) - operation.transition(name, state, actor, detail=str(entry.get("detail") or ""), **extra) + status = str(entry.get("status") or "").lower() + if status == "skipped": + # A row the backend resolved before this move landed has nothing to move; the rest of + # the answer still applies. A settled operation is frozen. The check and the move are + # not one step, so the refusal itself is the witness, not a read taken before it. + kept = _cancel_attempt(runner, target) + try: + operation.transition(target.name, TargetState.skipped, Actor.user, + **({"detail": kept} if kept else {})) + except IllegalTransition: + if not target.resolved and not operation.settled: + raise + elif status == "approved" and runner is not None: + if target.state == TargetState.pending: + runner.run(_APPROVE, operation, target, _answer_env(entry)) + elif target.state in (TargetState.failed, TargetState.expired): + # Connect on the form a failed row reopened: the same attempt, with the new values. + runner.run(_RETRY, operation, target, _answer_env(entry)) if answer.get("settled_by") == SettleReason.continue_.value and not operation.all_resolved: operation.settle(SettleReason.continue_) -def _unavailable(operation: ConnectionOperation) -> str: +AUTHORIZATION_KEPT = ("the authorization had already completed when this was canceled, so it was " + "kept; run the same action again to list the tools") + + +def _cancel_attempt(runner: Optional[_Runner], target: Target) -> str: + """Stop the target's OAuth attempt so a late reply cannot be adopted. Returns the note for a + cancel that lost the race: the attempt had committed, and a completed authorization stays.""" + work = runner.work.pop(target.name, None) if runner is not None else None + flow = getattr(getattr(work, "attempt", None), "flow", None) + if flow is None: + return "" + from tools.connectors.mcp_oauth import cancel_attempt + + return AUTHORIZATION_KEPT if cancel_attempt(flow) else "" + + +def retry(operation: ConnectionOperation, names: List[str]) -> Optional[str]: + """Re-run the named MCP targets on the open operation (the card's Try again): a fresh OAuth + flow, a fresh install, a fresh enable. Returns an error message when the operation is not one + this module is running.""" + runner = _RUNNERS.get(operation.op_id) + if runner is None: + return "this operation has no MCP work to re-run" + if operation.settled: + return "this operation has settled; its result is frozen" + for name in names: + target = operation.target(name) + if target is not None: + runner.run(_RETRY, operation, target) + return None + + +# --------------------------------------------------------------------------- +# the tool entry point +# --------------------------------------------------------------------------- + + +class _DetachedOperation(ConnectionOperation): + """The operation behind an off-desktop call. It is never registered in ``live`` and no card + renders it, so it publishes no ``connection.update``: a frame would reach a session whose + renderer knows nothing about the operation.""" + + on_change = None + + +def _off_desktop_result(runner: _Runner, names: List[str], action: str, session_key: str) -> str: + operation = _DetachedOperation([Target(n, "mcp", action) for n in names], session_key=session_key) for target in operation.targets: - target.state = TargetState.unavailable - target.detail = "no approval surface in this session" - target.extra = {"hint": UNAVAILABLE_HINT.format(name=target.name)} - operation.settle(SettleReason.unavailable) - payload = operation.result() - payload["status"] = "unavailable" - payload["note"] = ( - "This session has no approval card, so local MCP servers cannot be set up here. Tell " - "the user to run the terminal commands in each target's 'hint', then continue." - ) + runner.run(_OFF_DESKTOP, operation, target) + payload = operation.result(with_urls=True) + payload["status"] = "initiated" if any(t.state == TargetState.initiated for t in operation.targets) else "settled" + payload["note"] = OFF_DESKTOP_NOTE return json.dumps(payload, ensure_ascii=False) @@ -116,21 +782,23 @@ def run_mcp_operation( connection_callback: Optional[Callable[[Dict[str, Any]], Optional[str]]], session_id: Optional[str], tool_call_id: Optional[str] = None, + backend: Any = None, ) -> str: error = validate_mcp_names(action, names) if error: return tool_error(error) - targets = [Target(n, "mcp", action) for n in names] + runner = open_runner(action, backend) session_key = operation_session_key(session_id) - # The surface decides, not the callback: every tui_gateway session has the callback attached, - # the Ink TUI included, and only the desktop renders the card. - if session_platform() != "desktop" or connection_callback is None: - return _unavailable(ConnectionOperation(targets, session_key=session_key)) - def prepare(operation: ConnectionOperation) -> None: - pass - - return run_operation( - targets, Kind(prepare=prepare, observe=lambda op: None, note=NOTE), - session_key=session_key, tool_call_id=tool_call_id, connection_callback=connection_callback, - with_urls_in_result=False, - ) + # Every interactive surface that renders the card attaches this callback. Registry dispatch and + # messaging sessions attach none, so they receive the link instead of opening an unanswerable op. + if connection_callback is None: + return _off_desktop_result(runner, names, action, session_key) + try: + return run_operation( + [Target(n, "mcp", action) for n in names], + Kind(prepare=runner.prepare, observe=runner.observe, note=NOTE), + session_key=session_key, tool_call_id=tool_call_id, + connection_callback=connection_callback, with_urls_in_result=False, + ) + finally: + runner.close() diff --git a/tools/connectors/mcp_oauth.py b/tools/connectors/mcp_oauth.py new file mode 100644 index 0000000000..79ea23f130 --- /dev/null +++ b/tools/connectors/mcp_oauth.py @@ -0,0 +1,342 @@ +"""MCP OAuth worker and callback receivers used by connection cards and gateway RPC flows.""" + +from __future__ import annotations + +import http.server +import logging +import os +import secrets +import threading +import time +from contextlib import suppress +from dataclasses import dataclass +from pathlib import Path +from typing import Any, Callable, Dict, Optional +from urllib.parse import urlparse + +logger = logging.getLogger(__name__) + +URL_TIMEOUT_SECONDS = 30.0 + + +def probe_with_rollback( + server_name: str, cfg: dict, hermes_home: str, flow, reconnect_live: bool, *, + on_commit: Optional[Callable[[], None]] = None) -> None: + """Roll back failures through initialize; commit authorization before tool discovery. + + ``on_commit`` runs right after the configuration is saved: a card install persists its setup + values there, so they land with the authorization and never before it.""" + from hermes_cli.mcp_config import _oauth_tokens_present, _probe_single_server + from tools.mcp_dashboard_oauth import exception_message + from tools.mcp_oauth import HermesTokenStorage, login_connect_timeout + from tools.mcp_oauth_manager import get_manager + manager = get_manager() + storage = HermesTokenStorage(server_name) + # An attempt that replaced a still-running one starts from that one's half-written files, so + # it carries the older attempt's snapshot: the state from before either of them. + backup = getattr(flow, "inherited_backup", None) or storage.snapshot() + if flow is not None: + flow.backup = backup + previous_entry = None + details: Dict[str, Any] = {} + tools: list = [] + discovery_error = "" + + def undo() -> None: + # ``manager.remove`` cleared the pre-attempt tokens, so anything on disk now is this + # attempt's grant, and it is not being kept: put the snapshot back. A newer attempt for + # the same server owns the token files; an older one must not write over it. + if flow is not None and _ACTIVE.get((hermes_home, server_name)) not in (None, flow): + return + storage.restore(backup) + manager.restore_entry(server_name, previous_entry, hermes_home=hermes_home) + + try: + previous_entry = manager.remove(server_name, hermes_home=hermes_home) + tools = _probe_single_server( + server_name, cfg, connect_timeout=login_connect_timeout(cfg), details=details) + if not _oauth_tokens_present(server_name): + details["initialized"] = False + raise RuntimeError( + "The server responded, but no OAuth token was obtained — " + "this provider may require a manually-registered OAuth client.") + except Exception as exc: + if not details.get("initialized"): + undo() + raise + tools, discovery_error = [], exception_message(exc) + try: + _commit(server_name, cfg, on_commit, flow) + except AttemptCanceled: + undo() + raise + if flow is not None: + flow.tools = [{"name": t, "description": d} for t, d in tools] + flow.discovery_error = discovery_error + flow.mark_approved() + if discovery_error: + return + if reconnect_live: + from tools.mcp_tool_loop import reconnect_mcp_server + reconnect_mcp_server(server_name) + + +class AttemptCanceled(RuntimeError): + """The user canceled this attempt before it committed; nothing of it is kept.""" + + +# One lock orders a cancel against the commit, so an attempt is either canceled with nothing kept or +# committed with everything kept. A worker parked inside the token request cannot be interrupted; +# it is stopped here, at the one point where its result would be adopted. +_COMMIT_GUARD = threading.Lock() +# (hermes home, server) -> the newest card attempt. A retry or a new operation replaces an attempt +# whose worker is still waiting on the browser; the older one is canceled so it cannot commit later. +_ACTIVE: Dict[tuple, Any] = {} + + +def cancel_attempt(flow) -> bool: + """Cancel a card attempt. True when it had already committed: the authorization stays.""" + with _COMMIT_GUARD: + if getattr(flow, "committed", False): + return True + flow.cancelled = True + # Wakes a worker that is still waiting for the browser; a cancelled flow is never re-minted. + flow.mark_error("canceled", cancelled=True) + return False + + +def _commit(server_name: str, cfg: dict, on_commit: Optional[Callable[[], None]], flow=None) -> None: + from hermes_cli.mcp_config import _save_mcp_server + + with _COMMIT_GUARD: + if flow is not None and getattr(flow, "cancelled", False): + raise AttemptCanceled("canceled") + if not _save_mcp_server(server_name, cfg): + raise RuntimeError(f"'{server_name}' was rejected: suspicious command/args configuration") + if on_commit is not None: + on_commit() + if flow is not None: + flow.committed = True + + +def _reuse_saved_authorization( + server_name: str, cfg: dict, flow, on_commit: Optional[Callable[[], None]]) -> bool: + """Connect with the tokens already on disk, with no browser step and no consent. + + Retrying discovery for a server that is authorized must not ask the user to sign in again, and + must not delete the working grant first. Any failure here falls through to the interactive + flow, which replaces the grant.""" + from hermes_cli.mcp_config import _oauth_tokens_present, _probe_single_server + from tools.mcp_oauth import suppress_interactive_oauth + + if not _oauth_tokens_present(server_name): + return False + try: + with suppress_interactive_oauth(): + tools = _probe_single_server(server_name, cfg, connect_timeout=30) + _commit(server_name, cfg, on_commit, flow) + except Exception as exc: + logger.debug("saved authorization for %s was not usable: %s", server_name, exc) + return False + if flow is not None: + flow.tools = [{"name": t, "description": d} for t, d in tools] + flow.discovery_error = "" + flow.mark_approved() + return True + + +def run_worker( + hermes_home: str, server_name: str, cfg: dict, reconnect_live: bool, *, + flow, on_done: Optional[Callable[[], None]] = None, + env: Optional[Dict[str, str]] = None, on_commit: Optional[Callable[[], None]] = None, + reuse_saved: bool = False) -> None: + """Drive the interactive MCP OAuth probe under the shared callback bridge. + + ``env`` holds a card install's setup values. They join the secret scope for this attempt only, + so the configuration can reference them before anything is saved. ``reuse_saved`` is the + card's rule: a server whose saved tokens still work connects with no consent step. The RPC + session surface keeps it off, because its caller waits for an authorization URL.""" + from hermes_constants import reset_hermes_home_override, set_hermes_home_override + try: + from agent.secret_scope import ( + build_profile_secret_scope, reset_secret_scope, set_secret_scope) + from tools.mcp_dashboard_oauth import dashboard_oauth_flow + from tools.mcp_oauth import force_interactive_oauth + home_token = set_hermes_home_override(hermes_home) + secret_token = set_secret_scope({**build_profile_secret_scope(Path(hermes_home)), **(env or {})}) + try: + if not (reuse_saved and flow is not None + and _reuse_saved_authorization(server_name, cfg, flow, on_commit)): + with force_interactive_oauth(), dashboard_oauth_flow(flow): + probe_with_rollback( + server_name, cfg, hermes_home, flow, reconnect_live, on_commit=on_commit) + finally: + reset_secret_scope(secret_token) + reset_hermes_home_override(home_token) + except Exception as exc: + from tools.mcp_dashboard_oauth import exception_message + msg = exception_message(exc) + with suppress(Exception): + from tools.mcp_oauth import humanize_oauth_registration_error + msg = humanize_oauth_registration_error( + server_name, exc, server_url=cfg.get("url") if isinstance(cfg, dict) else None + ) or msg + if flow is not None: + flow.mark_error(msg) + finally: + if flow is not None: + flow.mark_worker_done() + with _COMMIT_GUARD: + if _ACTIVE.get((hermes_home, server_name)) is flow: + _ACTIVE.pop((hermes_home, server_name), None) + if on_done is not None: + on_done() + + +def _validate_client_redirect_uri(uri: str) -> str: + """Accept only plain-http loopback URLs (RFC 8252).""" + parsed = urlparse(str(uri or "").strip()) + host = (parsed.hostname or "").lower() + if (parsed.scheme != "http" or host not in ("127.0.0.1", "localhost", "::1") or not parsed.port + or parsed.username is not None or parsed.password is not None): + raise ValueError( + "client_redirect_uri must be a loopback http URL like http://127.0.0.1:/callback") + return f"http://{'[' + host + ']' if ':' in host else host}:{parsed.port}{parsed.path or '/callback'}" + + +def _start_loopback_receiver(flow) -> "http.server.HTTPServer": + """Bind the single backend-hosted one-shot receiver and feed its callback into ``flow``.""" + from tools.mcp_oauth import _parse_redirect_query + + class _Handler(http.server.BaseHTTPRequestHandler): + def do_GET(self): # noqa: N802 + parsed = urlparse(self.path) + if parsed.path.rstrip("/") not in ("/callback", ""): + self.send_response(404) + self.end_headers() + return + body = b"

Authorization received

You can close this tab and return to Hermes.

" + status = 200 + try: + flow.deliver_callback(**_parse_redirect_query(parsed.query)) + except Exception: + body = b"

OAuth callback rejected

The callback was invalid or already used.

" + status = 400 + self.send_response(status) + self.send_header("Content-Type", "text/html; charset=utf-8") + self.end_headers() + with suppress(Exception): + self.wfile.write(body) + + def log_message(self, format, *args): + return + + httpd = http.server.HTTPServer(("127.0.0.1", 0), _Handler) + threading.Thread( + target=httpd.serve_forever, kwargs={"poll_interval": 0.5}, daemon=True, + name=f"mcp-oauth-cb-{flow.server_name}").start() + return httpd + + +def _pinned_loopback(cfg: dict) -> bool: + oauth = cfg.get("oauth") or {} + return bool(oauth.get("client_id") and oauth.get("redirect_port")) + + +def _pinned_redirect_uri(cfg: dict) -> str: + oauth = cfg.get("oauth") or {} + host = oauth.get("redirect_host") or "127.0.0.1" + return f"http://{host}:{oauth['redirect_port']}/callback" + + +def choose_callback_receiver(flow, cfg: dict, client_redirect_uri: Optional[str] = None): + """Set the redirect consumed by the SDK and return the optional backend HTTP receiver.""" + if _pinned_loopback(cfg): + flow.redirect_uri = _pinned_redirect_uri(cfg) + return None + if client_redirect_uri is not None: + flow.redirect_uri = _validate_client_redirect_uri(client_redirect_uri) + return None + httpd = _start_loopback_receiver(flow) + flow.redirect_uri = f"http://127.0.0.1:{httpd.server_address[1]}/callback" + return httpd + + +def _ssh_detail(redirect_uri: str) -> str: + if not (os.environ.get("SSH_CLIENT") or os.environ.get("SSH_TTY")) or not redirect_uri: + return "" + from tools.mcp_oauth import _SSH_HINT_LOOPBACK + parsed = urlparse(redirect_uri) + return _SSH_HINT_LOOPBACK.format(host=parsed.hostname or "127.0.0.1", port=parsed.port or 0).strip() + + +@dataclass +class OAuthAttempt: + """One flow in flight: its URL, callback bridge, and outcome.""" + + auth_url: str + flow: Any + detail: str = "" + + def poll(self) -> Dict[str, Any]: + snapshot = self.flow.snapshot() + raw = snapshot.get("status") + status = raw if raw in ("approved", "error") else "pending" + tools = [str(t.get("name") or "") for t in (getattr(self.flow, "tools", None) or [])] + return {"status": status, "error": snapshot.get("error") or "", + "tools": [t for t in tools if t] if status == "approved" else [], + "discovery_error": (getattr(self.flow, "discovery_error", "") or "") + if status == "approved" else ""} + + +def start( + server_name: str, *, url_timeout: float = URL_TIMEOUT_SECONDS, + client_redirect_uri: Optional[str] = None, cfg: Optional[dict] = None, + env: Optional[Dict[str, str]] = None, on_commit: Optional[Callable[[], None]] = None, +) -> OAuthAttempt: + """Start a card OAuth flow and wait until its authorization URL is published. + + ``cfg`` is an install's in-memory configuration; without it the saved one is authorized.""" + from hermes_cli.mcp_config import _get_mcp_servers + from hermes_constants import get_hermes_home + from tools.mcp_dashboard_oauth import DashboardOAuthFlow + from tui_gateway import mcp_oauth_sessions + + cfg = dict(cfg if cfg is not None else _get_mcp_servers().get(server_name) or {}) + if not cfg: + raise RuntimeError(f"'{server_name}' is not a configured MCP server") + if not cfg.get("url"): + raise RuntimeError(f"'{server_name}' is a stdio server: it takes env keys, not OAuth") + cfg["auth"] = "oauth" + hermes_home = str(get_hermes_home().expanduser().resolve(strict=False)) + flow = DashboardOAuthFlow( + flow_id=secrets.token_urlsafe(24), server_name=server_name, profile=None, + hermes_home=hermes_home, redirect_uri="", reconnect_live=False) + with _COMMIT_GUARD: + older = _ACTIVE.get((hermes_home, server_name)) + _ACTIVE[(hermes_home, server_name)] = flow + if older is not None and not older.worker_done: + flow.inherited_backup = getattr(older, "backup", None) + cancel_attempt(older) + httpd = choose_callback_receiver(flow, cfg, client_redirect_uri) + mcp_oauth_sessions.register_flow(flow, httpd=httpd) + threading.Thread( + target=run_worker, args=(hermes_home, server_name, cfg, False), + kwargs={"flow": flow, "on_done": lambda: mcp_oauth_sessions.finish_flow(flow.flow_id), + "reuse_saved": True, + **({"env": env, "on_commit": on_commit} if env or on_commit else {})}, + daemon=True, name=f"mcp-oauth-{server_name}").start() + deadline = time.time() + url_timeout + while time.time() < deadline: + snapshot = flow.snapshot() + if snapshot.get("status") == "approved": + return OAuthAttempt(auth_url="", flow=flow) # the saved authorization still works + if snapshot.get("authorization_url"): + return OAuthAttempt( + auth_url=snapshot["authorization_url"], flow=flow, + detail=_ssh_detail(flow.redirect_uri) if client_redirect_uri is None else "") + if snapshot.get("status") == "error": + raise RuntimeError(snapshot.get("error") or "the OAuth flow failed before authorization") + time.sleep(0.05) + flow.mark_error("Timed out waiting for MCP authorization URL") + raise TimeoutError(f"timed out waiting for the authorization URL for '{server_name}'") diff --git a/tools/connectors/operation.py b/tools/connectors/operation.py index 91414d9244..b2a20e741f 100644 --- a/tools/connectors/operation.py +++ b/tools/connectors/operation.py @@ -25,13 +25,19 @@ class Target: action: str state: TargetState = TargetState.pending detail: str = "" + instructions: str = "" connect_url: Optional[str] = None + # The vendor account a managed mint created or observed. Not the desktop transport's connection id. + connection_id: Optional[str] = None # Opaque per-attempt handle when the gateway mints one (absent today; the status route adds it). attempt: Optional[str] = None - # `reconnect force` on a connected account: the list reports the OLD account `active` until the user - # signs in again, so `connected` is not believed until the row has read as anything else once. - awaiting_new_attempt: bool = False - # Renderer-reported fields passed through to the model (``tools`` after MCP OAuth). + # Earliest the watcher may read this target's account again; set from a 429's Retry-After so a + # rate-limited route is not hammered once per second. + next_read_at: float = 0.0 + # The credentials an MCP install still needs ({name, prompt, required}); the card draws a + # field per entry and holds its verb until every required one has text. + required_env: List[Dict[str, Any]] = field(default_factory=list) + # Fields a transition passes through to the model (``tools`` on a connected MCP target). extra: Dict[str, Any] = field(default_factory=dict) @property @@ -42,10 +48,16 @@ class Target: out: Dict[str, Any] = {"name": self.name, "kind": self.kind, "action": self.action, "state": self.state.value} if self.detail: out["detail"] = self.detail + if self.instructions: + out["instructions"] = self.instructions if with_url and self.connect_url: out["connect_url"] = self.connect_url + if self.connection_id: + out["connection_id"] = self.connection_id if self.attempt: out["attempt"] = self.attempt + if self.required_env: + out["required_env"] = self.required_env out.update(self.extra) return out @@ -53,10 +65,14 @@ class Target: @dataclass class ConnectionOperation: # The gateway installs its ``connection.update`` emitter here once; pure data otherwise. - on_change: ClassVar[Optional[Callable[["ConnectionOperation", Optional[Dict[str, Any]]], None]]] = None + on_change: ClassVar[ + Optional[Callable[["ConnectionOperation", Optional[Dict[str, Any]], Dict[str, Any]], None]] + ] = None targets: List[Target] session_key: str = "" + # Stamped by ``live.open``: the profile home the operation was opened under. + profile_key: str = "" # The model's id for the call that opened the operation; the card binds to that tool row only. tool_call_id: Optional[str] = None op_id: str = field(default_factory=lambda: uuid.uuid4().hex[:12]) @@ -64,6 +80,9 @@ class ConnectionOperation: deadline_at: float = 0.0 settled_at: Optional[float] = None settled_by: Optional[SettleReason] = None + # Monotonic write counter. Every frame carries the seq of the snapshot it was built from, so a + # renderer that keeps the highest seq per op can drop a frame that arrives after a newer one. + seq: int = 0 # Set on every transition and on settle; the waiting loop sleeps on it. wake: threading.Event = field(default_factory=threading.Event, repr=False) _settled_snapshot: Optional[Dict[str, Any]] = field(default=None, repr=False) @@ -78,7 +97,8 @@ class ConnectionOperation: def transition( self, name: str, to: TargetState, actor: Actor, *, detail: Optional[str] = None, - connect_url: Optional[str] = None, attempt: Optional[str] = None, **extra: Any, + connect_url: Optional[str] = None, connection_id: Optional[str] = None, attempt: Optional[str] = None, + **extra: Any, ) -> Optional[Dict[str, Any]]: """Move one target; the contract decides whether ``actor`` may. Returns the change, or None when the target is already in ``to``. Allowed after settlement: the frozen result stays.""" @@ -97,31 +117,49 @@ class ConnectionOperation: change["detail"] = target.detail if connect_url is not None: target.connect_url = connect_url + if connection_id is not None: + target.connection_id = connection_id if attempt is not None: target.attempt = attempt if extra: target.extra = dict(extra) + snapshot = self._bump_locked() self.wake.set() - self._changed(change) + self._changed(change, snapshot) return change - def refresh(self, name: str, *, connect_url: Optional[str], detail: str) -> None: - """Replace a target's link and detail without a state change (a repeated failure).""" + def refresh(self, name: str, *, connect_url: Optional[str], detail: str, actor: Actor = Actor.user) -> None: + """Replace a target's link and detail without a state change (a repeated failure). + + ``actor`` says who produced the new text: a second failure of a backend attempt is the + backend's report, not the user's move, and the frame must not claim otherwise.""" target = self.target(name) if target is None: raise IllegalTransition(f"unknown target {name!r}") with self._lock: target.connect_url = connect_url target.detail = detail - change = {"target": name, "from": target.state.value, "to": target.state.value, "actor": Actor.user.value, + change = {"target": name, "from": target.state.value, "to": target.state.value, "actor": actor.value, "detail": detail} + snapshot = self._bump_locked() self.wake.set() - self._changed(change) + self._changed(change, snapshot) - def _changed(self, change: Optional[Dict[str, Any]]) -> None: + def _bump_locked(self) -> Dict[str, Any]: + """Advance the write counter and take the snapshot that frame carries. Both happen under + ``_lock`` so a second writer cannot backdate this frame with its own state. + + After settlement the frozen snapshot is the frame, seq included: a later write changes + nothing the renderer can see, so it must not leave the operation naming a seq no frame + carried (the resume snapshot would wait for a frame that never comes).""" + if self._settled_snapshot is None: + self.seq += 1 + return self._result_locked() + + def _changed(self, change: Optional[Dict[str, Any]], snapshot: Dict[str, Any]) -> None: hook = type(self).on_change if hook is not None: - hook(self, change) + hook(self, change, snapshot) @property def all_resolved(self) -> bool: @@ -144,9 +182,9 @@ class ConnectionOperation: for target in self.targets: if not target.resolved: target.state = TargetState.not_connected - self._settled_snapshot = self._snapshot_locked() + self._settled_snapshot = self._bump_locked() self.wake.set() - self._changed(None) + self._changed(None, self._settled_snapshot) return True def settle_if_all_resolved(self) -> bool: @@ -155,30 +193,58 @@ class ConnectionOperation: def _snapshot_locked(self, *, with_urls: bool = True) -> Dict[str, Any]: return { "op_id": self.op_id, + "seq": self.seq, "deadline_at": self.deadline_at, "settled_at": self.settled_at, "settled_by": self.settled_by.value if self.settled_by else None, "targets": [t.snapshot(with_url=with_urls) for t in self.targets], } + def _result_locked(self, *, with_urls: bool = True) -> Dict[str, Any]: + if self._settled_snapshot is not None: + targets = [dict(t) for t in self._settled_snapshot["targets"]] + if not with_urls: + for t in targets: + t.pop("connect_url", None) + return dict(self._settled_snapshot, targets=targets) + return self._snapshot_locked(with_urls=with_urls) + def result(self, *, with_urls: bool = True) -> Dict[str, Any]: - """The settled result (frozen at settle time), or the live snapshot before settlement.""" + """The settled model result, or the live snapshot before settlement. + + Deferred MCP listings are model-only result data. Change frames and request payloads use + the internal snapshots, so renderers never receive the catalog block. + """ with self._lock: - if self._settled_snapshot is not None: - targets = [dict(t) for t in self._settled_snapshot["targets"]] - if not with_urls: - for t in targets: - t.pop("connect_url", None) - return dict(self._settled_snapshot, targets=targets) - return self._snapshot_locked(with_urls=with_urls) + result = self._result_locked(with_urls=with_urls) + if result.get("settled_at") is None: + return result + from tools.registry import registry + from tools.tool_search_catalog import build_catalog_listing_with_form + + for target in result["targets"]: + # Setup instructions are the card's text for the user. A catalog entry's notes can be + # older than this flow ("restart your session to load the tools"), and the model would + # repeat them after a result that says the tools are callable now. + target.pop("instructions", None) + names = target.get("tools") if target.get("kind") == "mcp" else None + if not names: + continue + definitions = registry.get_definitions(set(names), quiet=True) + listing, _form = build_catalog_listing_with_form(definitions, max_tokens=1500) + if listing: + target["tools_listing"] = listing + return result def request_payload(self) -> Dict[str, Any]: """The ``connection.request`` payload and the resume snapshot: identity, live target snapshots (links included, the panel owns them), server-owned deadline.""" with self._lock: targets = [t.snapshot() for t in self.targets] + seq = self.seq payload: Dict[str, Any] = { "op_id": self.op_id, + "seq": seq, "deadline_at": self.deadline_at, "timeout_seconds": OPERATION_DEADLINE_SECONDS, "targets": targets, diff --git a/tools/connectors/tool.py b/tools/connectors/tool.py index 82ca38dd37..13fef6e20e 100644 --- a/tools/connectors/tool.py +++ b/tools/connectors/tool.py @@ -17,6 +17,7 @@ def manage_connections( args: Dict[str, Any], *, client_factory: Optional[Callable[[], Any]] = None, + mcp_backend: Optional[Any] = None, session_id: Optional[str] = None, tool_call_id: Optional[str] = None, connection_callback: Optional[Callable[[Dict[str, Any]], Optional[str]]] = None, @@ -32,7 +33,7 @@ def manage_connections( if action in MCP_ACTIONS: return run_mcp_operation( - mcp_targets, action, + mcp_targets, action, backend=mcp_backend, connection_callback=connection_callback, session_id=session_id, tool_call_id=tool_call_id, ) @@ -63,10 +64,10 @@ MANAGE_CONNECTIONS_SCHEMA = { "'enable' re-enables a disabled configured server, 'authorize' runs its OAuth. " "They show the user an approval card and block until it settles. Never hand-edit " "mcp_servers config — always use this tool. Never re-ask after a skip or timeout: continue " - "without the app or ask in chat. A newly installed or authorized server's tools arrive on " - "your next turn. Off the desktop app the MCP targets come back 'unavailable' with the " - "terminal commands to give the user. This tool can NOT disconnect, delete, or revoke an " - "account — that is deliberately user-only. When asked, say so and direct the user to the " + "without the app or ask in chat. A connected server's tools are named in the result and are " + "callable at once through tool_describe/tool_call. Where no card exists an MCP target runs at once and the result says what " + "happened, with a link for the user to open when one is needed. This tool can NOT " + "disconnect, delete, or revoke an account — that is deliberately user-only. When asked, say so and direct the user to the " "Nous Portal (their org's Connectors page) or the desktop app." ), "parameters": { diff --git a/tools/mcp_dashboard_oauth.py b/tools/mcp_dashboard_oauth.py index d56f27fe5d..37bd86cb3b 100644 --- a/tools/mcp_dashboard_oauth.py +++ b/tools/mcp_dashboard_oauth.py @@ -51,6 +51,7 @@ class DashboardOAuthFlow: authorization_url: str | None = None error: str | None = None tools: list[dict] = field(default_factory=list) + discovery_error: str = "" # The user abandoned this flow: terminal for good, never re-minted (see publish_authorization_url). cancelled: bool = field(default=False, init=False) expected_state: str | None = field(default=None, init=False) diff --git a/tui_gateway/contracts/connectors_operation.py b/tui_gateway/contracts/connectors_operation.py index 4ff5f0176f..4a8009ff1f 100644 --- a/tui_gateway/contracts/connectors_operation.py +++ b/tui_gateway/contracts/connectors_operation.py @@ -46,7 +46,6 @@ class ConnectionActor(WireEnum): """``tools/connectors/contract.py::Actor``.""" user = "user" - renderer_flow = "renderer_flow" backend_watcher = "backend_watcher" clock = "clock" @@ -61,6 +60,17 @@ class ConnectionSettleReason(WireEnum): unavailable = "unavailable" +class ConnectionTargetEnvField(Payload): + """One credential an MCP install still needs; the card renders a field per entry and sends the + values back with the approval.""" + + name: str + required: bool + secret: bool + default: str + prompt: str | None = None + + class ConnectionOperationTarget(Payload): """``Target.snapshot``: the link minted up front rides here, never in the model result. ``extra`` keys a leg records (``tools``, ``hint``) are typed here as they appear.""" @@ -70,8 +80,14 @@ class ConnectionOperationTarget(Payload): action: ConnectionTargetAction state: ConnectionTargetState detail: str | None = None + instructions: str | None = None + discovery_error: str | None = None connect_url: str | None = None + # The vendor account a managed mint created or observed; never the desktop transport's id. + connection_id: str | None = None attempt: str | None = None + # Present only on an MCP install that is waiting for credentials. + required_env: list[ConnectionTargetEnvField] | None = None tools: list[str] | None = None hint: str | None = None @@ -81,6 +97,9 @@ class ConnectionRequestPayload(Payload): snapshot so a client that missed the event restores the card with the server's deadline.""" op_id: str + # Monotonic write counter for the operation; a frame whose seq is not higher than the one the + # renderer holds is older and moves no row. + seq: int deadline_at: float timeout_seconds: float targets: list[ConnectionOperationTarget] @@ -96,6 +115,7 @@ class ConnectionOperationStatus(Result): """``methods_connectors._operation_view``: the operation's full snapshot.""" op_id: str + seq: int deadline_at: float settled: bool settled_at: float | None = None @@ -126,18 +146,29 @@ method("connectors.operation.status", params=ConnectionOperationParams, result=C doc="The current snapshot of one open operation on an owned session.") -class ConnectionAnswerTarget(Params): - """One row's answer from the card. ``status`` is what the card observed for that row - (``tools/connectors/mcp.py::_OUTCOME_STATES`` maps it onto a target state); ``state`` is the - older spelling of the same field and one of the two is present.""" +class ConnectionWakeResult(Result): + status: str - model_config = Params.model_config | {"extra": "allow"} + +method("connectors.operation.wake", params=ConnectionOperationParams, result=ConnectionWakeResult, + doc="The browser leg came back (hermes://connections/done): read the accounts now, not at the next tick.") + + +class ConnectionAnswerStatus(WireEnum): + """What the card says about one row: ``tools/connectors/mcp.py::apply_answer``.""" + + approved = "approved" + skipped = "skipped" + + +class ConnectionAnswerTarget(Params): + """One row's answer from the card. ``env`` carries the credential values an install asked for + through ``required_env``.""" name: str - status: str | None = None - state: str | None = None + status: ConnectionAnswerStatus detail: str | None = None - tools: list[str] | None = None + env: dict[str, str] | None = None class ConnectionAnswer(Params): diff --git a/tui_gateway/mcp_oauth_sessions.py b/tui_gateway/mcp_oauth_sessions.py index 5adfe6e1dd..5a979b800b 100644 --- a/tui_gateway/mcp_oauth_sessions.py +++ b/tui_gateway/mcp_oauth_sessions.py @@ -1,20 +1,22 @@ -"""Session-backed MCP OAuth flows for the gateway (mcp.servers.oauth.*): ``start`` spawns a -worker and returns ``{session_id, auth_url, flow}``; ``poll`` reports ``{status}`` until tokens -land. Reuses ``hermes mcp login``'s probe under ``force_interactive_oauth`` plus -``DashboardOAuthFlow``; the only new piece is a loopback listener feeding ``deliver_callback``. -Remote backends host the listener (``client_redirect_uri``) and relay via -``deliver_callback_flow``.""" +"""Session-backed MCP OAuth flows for gateway RPC and connection-card callback relays. + +The worker and callback receiver selection live in ``tools/connectors/mcp_oauth.py``. This module +owns flow registration, profile checks, polling, cancellation, and relayed callback delivery. +""" from __future__ import annotations -import http.server import secrets import threading import time from contextlib import suppress -from pathlib import Path from typing import Any, Dict, Optional -from urllib.parse import urlparse + +from tools.connectors.mcp_oauth import ( + _validate_client_redirect_uri, + choose_callback_receiver, + run_worker, +) # session_id -> record wrapping the shared DashboardOAuthFlow bridge plus bookkeeping. _sessions: Dict[str, Dict[str, Any]] = {} @@ -34,118 +36,27 @@ def _shutdown_listener(rec: Dict[str, Any]) -> None: rec["httpd"] = None -def _validate_client_redirect_uri(uri: str) -> str: - """Accept only plain-http loopback URLs (RFC 8252) so the gateway can't pin an - attacker-controlled redirect into a DCR registration.""" - parsed = urlparse(str(uri or "").strip()) - host = (parsed.hostname or "").lower() - if (parsed.scheme != "http" or host not in ("127.0.0.1", "localhost", "::1") or not parsed.port - or parsed.username is not None or parsed.password is not None): - raise ValueError( - "client_redirect_uri must be a loopback http URL like http://127.0.0.1:/callback") - return f"http://{'[' + host + ']' if ':' in host else host}:{parsed.port}{parsed.path or '/callback'}" +def register_flow(flow, *, httpd=None) -> Dict[str, Any]: + """Register a callback-relay flow so both RPC and card starts share ownership checks.""" + rec = { + "session_id": flow.flow_id, + "server_name": flow.server_name, + "hermes_home": flow.hermes_home, + "flow": flow, + "httpd": httpd, + "created_at": time.time(), + } + with _sessions_lock: + _sessions[flow.flow_id] = rec + return rec -def _start_loopback_listener(flow) -> "http.server.HTTPServer": - """Bind a loopback callback listener feeding ``flow.deliver_callback``; returns the - HTTPServer already serving on a daemon thread (caller pins ``flow.redirect_uri`` from it).""" - from tools.mcp_oauth import _parse_redirect_query - - class _Handler(http.server.BaseHTTPRequestHandler): - def do_GET(self): # noqa: N802 — stdlib naming - parsed = urlparse(self.path) - if parsed.path.rstrip("/") not in ("/callback", ""): - self.send_response(404) - self.end_headers() - return - body = b"

Authorization received

You can close this tab and return to Hermes.

" - status = 200 - try: - flow.deliver_callback(**_parse_redirect_query(parsed.query)) - except Exception: - body = b"

OAuth callback rejected

The callback was invalid or already used.

" - status = 400 - self.send_response(status) - self.send_header("Content-Type", "text/html; charset=utf-8") - self.end_headers() - with suppress(Exception): - self.wfile.write(body) - - def log_message(self, *_a): # silence stdlib request logging - return - - httpd = http.server.HTTPServer(("127.0.0.1", 0), _Handler) - threading.Thread( - target=httpd.serve_forever, kwargs={"poll_interval": 0.5}, daemon=True, - name=f"mcp-oauth-cb-{flow.server_name}").start() - return httpd - - -def _probe_with_rollback( - server_name: str, cfg: dict, hermes_home: str, flow, reconnect_live: bool) -> None: - """Run the OAuth probe; on ANY failure restore the prior token file + manager entry.""" - from hermes_cli.mcp_config import _oauth_tokens_present, _probe_single_server, _save_mcp_server - from tools.mcp_oauth import HermesTokenStorage, login_connect_timeout - from tools.mcp_oauth_manager import get_manager - manager = get_manager() - storage = HermesTokenStorage(server_name) - backup = storage.snapshot() - previous_entry = None - try: - previous_entry = manager.remove(server_name, hermes_home=hermes_home) - tools = _probe_single_server(server_name, cfg, connect_timeout=login_connect_timeout(cfg)) - if not _oauth_tokens_present(server_name): - raise RuntimeError( - "The server responded, but no OAuth token was obtained — " - "this provider may require a manually-registered OAuth client.") - _save_mcp_server(server_name, cfg) - if flow is not None: - flow.tools = [{"name": t, "description": d} for t, d in tools] - flow.mark_approved() - if reconnect_live: - from tools.mcp_tool_loop import reconnect_mcp_server - reconnect_mcp_server(server_name) - except Exception: - storage.restore(backup, only_if_absent=True) - manager.restore_entry(server_name, previous_entry, hermes_home=hermes_home) - raise - - -def _worker( - session_id: str, hermes_home: str, server_name: str, cfg: dict, reconnect_live: bool) -> None: - """Drive the interactive MCP OAuth probe under the shared dashboard bridge (same wrapping - as ``web_server._run_dashboard_mcp_oauth``), keyed to our session record.""" - from hermes_constants import reset_hermes_home_override, set_hermes_home_override - rec = _sessions.get(session_id) - flow = rec["flow"] if rec else None - try: - from agent.secret_scope import ( - build_profile_secret_scope, reset_secret_scope, set_secret_scope) - from tools.mcp_dashboard_oauth import dashboard_oauth_flow - from tools.mcp_oauth import force_interactive_oauth - home_token = set_hermes_home_override(hermes_home) - secret_token = set_secret_scope(build_profile_secret_scope(Path(hermes_home))) - try: - with force_interactive_oauth(), dashboard_oauth_flow(flow): - _probe_with_rollback(server_name, cfg, hermes_home, flow, reconnect_live) - finally: - reset_secret_scope(secret_token) - reset_hermes_home_override(home_token) - except Exception as exc: - from tools.mcp_dashboard_oauth import exception_message - msg = exception_message(exc) - with suppress(Exception): - from tools.mcp_oauth import humanize_oauth_registration_error - msg = humanize_oauth_registration_error( - server_name, exc, server_url=cfg.get("url") if isinstance(cfg, dict) else None - ) or msg - if flow is not None: - flow.mark_error(msg) - finally: - if flow is not None: - flow.mark_worker_done() - if rec is not None: - _shutdown_listener(rec) +def finish_flow(session_id: str) -> None: + """Release a finished flow's backend listener without removing relay-visible outcome state.""" + with _sessions_lock: + rec = _sessions.get(session_id) + if rec is not None: + _shutdown_listener(rec) def start_flow( @@ -171,19 +82,12 @@ def start_flow( session_id = secrets.token_urlsafe(24) flow = DashboardOAuthFlow( flow_id=session_id, server_name=server_name, profile=None, hermes_home=hermes_home, - redirect_uri="", # set below once the loopback port is known - reconnect_live=reconnect_live) - # Client-hosted listener: a 127.0.0.1 port here would be unreachable from the browser. - httpd = None if client_redirect_uri else _start_loopback_listener(flow) - flow.redirect_uri = ( - client_redirect_uri or f"http://127.0.0.1:{httpd.server_address[1]}/callback") - rec = { - "session_id": session_id, "server_name": server_name, "hermes_home": hermes_home, - "flow": flow, "httpd": httpd, "created_at": time.time()} - with _sessions_lock: - _sessions[session_id] = rec + redirect_uri="", reconnect_live=reconnect_live) + httpd = choose_callback_receiver(flow, cfg, client_redirect_uri) + rec = register_flow(flow, httpd=httpd) threading.Thread( - target=_worker, args=(session_id, hermes_home, server_name, dict(cfg), reconnect_live), + target=run_worker, args=(hermes_home, server_name, dict(cfg), reconnect_live), + kwargs={"flow": flow, "on_done": lambda: _shutdown_listener(rec)}, daemon=True, name=f"mcp-oauth-{server_name}").start() try: auth_url = None diff --git a/tui_gateway/methods_connectors.py b/tui_gateway/methods_connectors.py index 399cf54756..2cfe9e0aed 100644 --- a/tui_gateway/methods_connectors.py +++ b/tui_gateway/methods_connectors.py @@ -102,7 +102,8 @@ def _dispatch_connector_rpc(rid, sid, owner, profile_home, args): return _connector_rpc_error(rid, 4031, "CONNECTORS_UNAVAILABLE", "Connectors are not available in this session.") if not _connector_owner_matches(sid, owner, profile_home): return _connector_rpc_error(rid, 4001, "NOT_OWNER", "session ownership changed") - if args["action"] != "status" and (operation := live.current(owner["session_key"])) is not None: + if args["action"] != "status" and ( + operation := live.current(owner["session_key"], profile_home=owner.get("profile_home"))) is not None: # The card's Try again / Connect while the model's operation is open: reissue on that op. return _reissue(rid, operation, args) raw = model_tools.handle_function_call( @@ -124,32 +125,61 @@ def _dispatch_connector_rpc(rid, sid, owner, profile_home, args): def _reissue(rid, operation, args): - """Re-mint links for the named targets on the open operation (user actor).""" - from tools.connectors.contract import Actor, TargetState - from tools.connectors.gateway.client import ConnectorClient - from tools.connectors.managed import mint + """The card's Try again on the open operation: a managed target is re-minted at the gateway, an + MCP target re-runs its own install / enable / OAuth. Dead rows only — a waiting target already + holds the link the card reopens, and never calls here.""" + from tools.connectors.contract import TargetState, allowed from tui_gateway.connector_payload import connector_ui_payload - # Only a dead link is re-minted. A waiting target already holds its link (minted up front); - # the card re-opens that one and never calls here for it. targets = [operation.target(n) for n in args["connectors"]] if any(t is None for t in targets): return _connector_rpc_error(rid, 4004, "UNKNOWN_TARGET", "no such target on the open operation") + if len({t.kind for t in targets}) != 1: + return _connector_rpc_error(rid, 4000, "INVALID_PARAMS", "one target kind per request") stale = [t.name for t in targets if t.state in (TargetState.failed, TargetState.expired)] if len(stale) != len(targets): return _connector_rpc_error(rid, 4002, "LINK_STILL_VALID", - "only a failed or expired target can be re-minted; reopen the stored link") - mint(ConnectorClient(), operation, stale, reinitiate=True, actor=Actor.user) + "only a failed or expired target can be re-run; reopen the stored link") + if operation.settled: + # Continue can land between the state read above and the re-run: the result is frozen and + # a fresh mint or flow would have no row to report into. + return _connector_rpc_error(rid, 4002, "REISSUE_REFUSED", "the operation has settled") + # The contract says which dead state a kind can leave: an MCP target has no move out of expired. + frozen = [t.name for t in targets if allowed(t.kind, t.state, TargetState.initiated) is None] + if frozen: + return _connector_rpc_error(rid, 4002, "REISSUE_REFUSED", + f"this target cannot be run again: {', '.join(frozen)}") + error = _REISSUE_BY_KIND[targets[0].kind](operation, stale) + if error: + return _connector_rpc_error(rid, 4002, "REISSUE_REFUSED", error) return _ok(rid, connector_ui_payload(_operation_view(operation))) +def _remint_managed(operation, names): + from tools.connectors.contract import Actor + from tools.connectors.gateway.client import ConnectorClient + from tools.connectors.managed import mint + + mint(ConnectorClient(), operation, names, reinitiate=True, actor=Actor.user) + return None + + +def _rerun_mcp(operation, names): + from tools.connectors.mcp import retry + + return retry(operation, names) + + +_REISSUE_BY_KIND = {"connector": _remint_managed, "mcp": _rerun_mcp} + + def _live_operation(rid, params, owner): from tools.connectors import live op_id = params.get("op_id") if not isinstance(op_id, str) or not op_id: return None, _connector_rpc_error(rid, 4000, "INVALID_PARAMS", "op_id required") - operation = live.get(owner["session_key"], op_id) + operation = live.get(owner["session_key"], op_id, profile_home=owner.get("profile_home")) if operation is None: return None, _connector_rpc_error(rid, 4004, "UNKNOWN_OPERATION", "no open operation with that op_id in this session") return operation, None @@ -178,14 +208,32 @@ def _(rid, params): return _ok(rid, connector_ui_payload(_operation_view(operation))) +@method("connectors.operation.wake") +def _(rid, params): + """The desktop came back from the vendor's done page: read the accounts now instead of at the + next tick. The link is not trusted for anything else; this only shortens the wait.""" + owner, error = _owned_session(rid, params) + if error: + return error + operation, error = _live_operation(rid, params, owner) + if error: + return error + operation.wake.set() + return _ok(rid, {"status": "ok"}) + + @method("connection.respond") def _(rid, params): - """The card's answer for the operation named by ``op_id``: per-target user / renderer-flow - transitions and an optional Continue. The contract decides what the card may claim.""" + """The card's answer for the operation named by ``op_id``: per target an approval that starts + the backend's work or a skip, plus an optional Continue. The card never witnesses an outcome, + so any other claim moves nothing.""" + from pydantic import ValidationError + from tools.connectors import live from tools.connectors.contract import SettleReason from tools.connectors.mcp import apply_answer from tools.connectors.operation import IllegalTransition + from tui_gateway.contracts.connectors_operation import ConnectionAnswer owner, error = _owned_session(rid, params) if error: @@ -193,31 +241,48 @@ def _(rid, params): operation, error = _live_operation(rid, params, owner) if error: return error + # The wire check refuses unknown keys only; the closed answer vocabulary (approved / skipped) is + # this handler's refusal, so a card claiming ``connected`` is answered, not logged. try: - apply_answer(operation, json.dumps(params["result"])) - except IllegalTransition as exc: - return _connector_rpc_error(rid, 4002, "ILLEGAL_TRANSITION", str(exc)) - if not operation.settled and operation.all_resolved: - operation.settle(SettleReason.all_resolved) + answer = ConnectionAnswer.model_validate(params["result"]) + except ValidationError as exc: + return _connector_rpc_error(rid, 4002, "INVALID_ANSWER", exc.errors()[0].get("msg", "invalid answer")) + # An approval runs the backend's work on this thread (an enable writes config.yaml, an install + # stores credentials), so the answer is applied under the session's profile the way + # ``_connector_rpc`` binds it; the RPC thread carries no profile of its own. + scope = {"profile_home": owner.get("profile_home") or str(_hermes_home)} + with _session_profile_runtime_scope(scope): + try: + apply_answer(operation, answer.model_dump_json(exclude_none=True)) + except IllegalTransition as exc: + return _connector_rpc_error(rid, 4002, "ILLEGAL_TRANSITION", str(exc)) + if not operation.settled and operation.all_resolved: + operation.settle(SettleReason.all_resolved) if operation.settled: live.close(operation) return _ok(rid, {"status": "ok", "settled": operation.settled}) +def _snapshot_view(snapshot): + return {**snapshot, "settled": snapshot.get("settled_at") is not None} + + def _operation_view(operation): - return {**operation.result(), "settled": operation.settled} + return _snapshot_view(operation.result()) -def _connection_update(operation, change=None): +def _connection_update(operation, change, snapshot): """Emit ``connection.update`` for one transition, a link refresh, or settlement. Every frame - carries the full target snapshot so the renderer never reconstructs state from deltas.""" + carries the full target snapshot so the renderer never reconstructs state from deltas. The + snapshot was taken under the operation's lock with the ``seq`` it carries, so a frame can never + show a state newer than its own seq names.""" from tui_gateway import server with server._sessions_lock: sid = next((s for s, c in server._sessions.items() if c.get("session_key") == operation.session_key), None) if sid is None: return - payload = _operation_view(operation) + payload = _snapshot_view(snapshot) if change: payload.update(change) server._emit("connection.update", sid, payload) diff --git a/tui_gateway/server.py b/tui_gateway/server.py index ee4607ca3f..5d92ef124f 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -737,7 +737,8 @@ def _pending_connection_request_payload(sid: str) -> dict | None: from tools.connectors import live session = _sessions.get(sid) - operation = live.current(str(session.get("session_key") or "")) if session else None + operation = (live.current(str(session.get("session_key") or ""), profile_home=session.get("profile_home")) + if session else None) return operation.request_payload() if operation is not None else None diff --git a/ui-tui/src/__tests__/createGatewayEventHandler.test.ts b/ui-tui/src/__tests__/createGatewayEventHandler.test.ts index 861c249630..f10fbfd85f 100644 --- a/ui-tui/src/__tests__/createGatewayEventHandler.test.ts +++ b/ui-tui/src/__tests__/createGatewayEventHandler.test.ts @@ -1,5 +1,10 @@ +import type { ConnectionOperationTarget } from '@hermes/shared/gateway-events' import { beforeEach, describe, expect, it, vi } from 'vitest' +import { + $connectionOperation, + resetConnectionOperationsForTests +} from '../app/connectionOperationStore.js' import { createGatewayEventHandler } from '../app/createGatewayEventHandler.js' import { createServerRequestHandler } from '../app/createServerRequestHandler.js' import { getOverlayState, patchOverlayState, resetOverlayState } from '../app/overlayStore.js' @@ -81,6 +86,7 @@ describe('createGatewayEventHandler', () => { resetUiState() resetTurnState() resetServerRequestsForTests() + resetConnectionOperationsForTests() turnController.fullReset() patchUiState({ showReasoning: true }) }) @@ -104,6 +110,34 @@ describe('createGatewayEventHandler', () => { expect(getUiState().status).toBe('ready') expect(getOverlayState().approval).toBeNull() expect(getTurnState().tools).toEqual([]) + + const target: ConnectionOperationTarget = { action: 'install', kind: 'mcp', name: 'asana', state: 'pending' } + onEvent({ + session_id: 'focused', + payload: { + deadline_at: 10, + op_id: 'op-1', + seq: 2, + targets: [target], + timeout_seconds: 30 + }, + type: 'connection.request' + }) + expect($connectionOperation.get()).toMatchObject({ opId: 'op-1', seq: 2, targets: [target] }) + expect(getOverlayState().connection).toEqual({ opId: 'op-1' }) + + onEvent({ + session_id: 'focused', + payload: { + deadline_at: 11, + op_id: 'op-1', + seq: 1, + settled: false, + targets: [{ ...target, state: 'failed' }] + }, + type: 'connection.update' + }) + expect($connectionOperation.get()).toMatchObject({ seq: 2, targets: [target] }) }) it('keeps the durable session id when a session.info payload omits it', () => { diff --git a/ui-tui/src/app/connectionOperationStore.ts b/ui-tui/src/app/connectionOperationStore.ts new file mode 100644 index 0000000000..defa08e53d --- /dev/null +++ b/ui-tui/src/app/connectionOperationStore.ts @@ -0,0 +1,77 @@ +import type { + ConnectionOperationTarget, + ConnectionRequestPayload, + ConnectionUpdatePayload +} from '@hermes/shared/gateway-events' +import { atom } from 'nanostores' + +import { patchOverlayState } from './overlayStore.js' + +export interface ConnectionOperationSnapshot { + deadlineAt: number + opId: string + seq: number + targets: ConnectionOperationTarget[] + toolCallId: null | string +} + +export interface ConnectionOverlayState { + opId: string +} + +export const $connectionOperation = atom(null) + +const settledOperationIds = new Set() + +export function applyConnectionRequest(payload: ConnectionRequestPayload): void { + const current = $connectionOperation.get() + + if (settledOperationIds.has(payload.op_id)) { + return + } + + if (current?.opId === payload.op_id && payload.seq <= current.seq) { + return + } + + $connectionOperation.set({ + deadlineAt: payload.deadline_at, + opId: payload.op_id, + seq: payload.seq, + targets: payload.targets, + toolCallId: payload.tool_call_id ?? null + }) + patchOverlayState({ connection: { opId: payload.op_id } }) +} + +export function applyConnectionUpdate(payload: ConnectionUpdatePayload): void { + const current = $connectionOperation.get() + + if (!current || current.opId !== payload.op_id || payload.seq <= current.seq) { + return + } + + if (payload.settled) { + settledOperationIds.add(payload.op_id) + clearConnectionOperation() + + return + } + + $connectionOperation.set({ + ...current, + deadlineAt: payload.deadline_at, + seq: payload.seq, + targets: payload.targets + }) +} + +export function clearConnectionOperation(): void { + $connectionOperation.set(null) + patchOverlayState({ connection: null }) +} + +export function resetConnectionOperationsForTests(): void { + settledOperationIds.clear() + clearConnectionOperation() +} diff --git a/ui-tui/src/app/createGatewayEventHandler.ts b/ui-tui/src/app/createGatewayEventHandler.ts index 223dc5315c..5e610f53be 100644 --- a/ui-tui/src/app/createGatewayEventHandler.ts +++ b/ui-tui/src/app/createGatewayEventHandler.ts @@ -27,6 +27,7 @@ import { bootSeededPin, invalidateBootBackground, writeBootTheme } from '../lib/ import { defaultThemeForCurrentBackground, fromSkin, skinIsLight, type Theme, themeToneHex } from '../theme.js' import type { Msg, SessionInfo, SubagentProgress } from '../types.js' +import { applyConnectionRequest, applyConnectionUpdate } from './connectionOperationStore.js' import { applyDelegationStatus, getDelegationState } from './delegationStore.js' import type { GatewayEventHandlerContext, NoticeLevel } from './interfaces.js' import { getOverlayState, patchOverlayState } from './overlayStore.js' @@ -793,6 +794,20 @@ export function createGatewayEventHandler(ctx: GatewayEventHandlerContext): (ev: } switch (ev.type) { + case 'connection.request': + if (ev.payload) { + applyConnectionRequest(ev.payload) + } + + return + + case 'connection.update': + if (ev.payload) { + applyConnectionUpdate(ev.payload) + } + + return + case 'gateway.ready': handleReady(ev.payload?.skin) diff --git a/ui-tui/src/app/interfaces.ts b/ui-tui/src/app/interfaces.ts index bd14925065..f3bf26f779 100644 --- a/ui-tui/src/app/interfaces.ts +++ b/ui-tui/src/app/interfaces.ts @@ -281,6 +281,10 @@ export interface SubscriptionOverlayState { stepUpRetry?: null | SubscriptionStepUpRetry } +export interface ConnectionOverlayState { + opId: string +} + export interface OverlayState { agents: boolean agentsInitialHistoryIndex: number @@ -288,6 +292,7 @@ export interface OverlayState { billing: BillingOverlayState | null clarify: ClarifyReq | null confirm: ConfirmReq | null + connection: ConnectionOverlayState | null /** Ambient widget apps — glanceable dock, non-blocking (never in $isBlocked). */ ambient: ActiveWidget[] /** Modal widget app — owns input, blocks the composer. */ diff --git a/ui-tui/src/app/overlayStore.ts b/ui-tui/src/app/overlayStore.ts index 437d16d139..e36ac0a223 100644 --- a/ui-tui/src/app/overlayStore.ts +++ b/ui-tui/src/app/overlayStore.ts @@ -10,6 +10,7 @@ const buildOverlayState = (): OverlayState => ({ billing: null, clarify: null, confirm: null, + connection: null, ambient: [], widget: null, journey: false, @@ -35,6 +36,7 @@ export const $isBlocked = computed( billing, clarify, confirm, + connection, journey, modelPicker, pager, @@ -54,6 +56,7 @@ export const $isBlocked = computed( billing || clarify || confirm || + connection || journey || modelPicker || pager || diff --git a/ui-tui/src/app/useSessionLifecycle.ts b/ui-tui/src/app/useSessionLifecycle.ts index 2845f567ee..1dd16d09d7 100644 --- a/ui-tui/src/app/useSessionLifecycle.ts +++ b/ui-tui/src/app/useSessionLifecycle.ts @@ -19,6 +19,7 @@ import type { import { asRpcResult } from '../lib/rpc.js' import type { Msg, PanelSection, SessionInfo } from '../types.js' +import { applyConnectionRequest, clearConnectionOperation } from './connectionOperationStore.js' import type { ComposerActions, GatewayRpc, StateSetter } from './interfaces.js' import { patchOverlayState } from './overlayStore.js' import { scheduleResumeScrollToBottom } from './sessionResumeView.js' @@ -387,6 +388,13 @@ export function useSessionLifecycle(opts: UseSessionLifecycleOptions) { usage: usageFrom(info) }) hydrateLiveSessionInflight(r.inflight) + + if (r.pending_connection) { + applyConnectionRequest(r.pending_connection) + } else { + clearConnectionOperation() + } + cancelResumeScrollRef.current?.() cancelResumeScrollRef.current = scheduleResumeScrollToBottom(scrollRef) diff --git a/ui-tui/src/components/appOverlays.tsx b/ui-tui/src/components/appOverlays.tsx index ff305cff71..21c66fcd1b 100644 --- a/ui-tui/src/components/appOverlays.tsx +++ b/ui-tui/src/components/appOverlays.tsx @@ -10,6 +10,7 @@ import { $uiSessionId, $uiTheme } from '../app/uiStore.js' import { ActiveSessionSwitcher } from './activeSessionSwitcher.js' import { FloatBox } from './appChrome.js' import { BillingOverlay } from './billingOverlay.js' +import { ConnectionSetupOverlay } from './connectionSetupOverlay.js' import { MaskedPrompt } from './maskedPrompt.js' import { ModelPicker } from './modelPicker.js' import { OverlayHint } from './overlayControls.js' @@ -116,6 +117,14 @@ export function PromptZone({ ) } + if (overlay.connection) { + return ( + + + + ) + } + if (overlay.confirm) { const req = overlay.confirm diff --git a/ui-tui/src/components/connectionSetupOverlay.tsx b/ui-tui/src/components/connectionSetupOverlay.tsx new file mode 100644 index 0000000000..94a2c04040 --- /dev/null +++ b/ui-tui/src/components/connectionSetupOverlay.tsx @@ -0,0 +1,338 @@ +import { Box, Text, useInput } from '@hermes/ink' +import type { ConnectionOperationTarget, ConnectionRespondParams, ConnectionTargetEnvField } from '@hermes/shared/gateway-events' +import { useStore } from '@nanostores/react' +import { useEffect, useMemo, useState } from 'react' + +import { $connectionOperation, clearConnectionOperation } from '../app/connectionOperationStore.js' +import { useGateway } from '../app/gatewayContext.js' +import { $uiSessionId } from '../app/uiStore.js' +import { openExternalUrl } from '../lib/openExternalUrl.js' +import type { Theme } from '../theme.js' + +import { TextInput } from './textInput.js' + +interface ConnectionSetupOverlayProps { + cols: number + t: Theme +} + +interface FormKeyHandlers { + cancel: () => void + connect: () => void + fieldCount: number + selectorFocused: boolean + setAction: (update: (value: 0 | 1) => 0 | 1) => void + setFocus: (update: (value: number) => number) => void + submitting: boolean +} + +interface InputKey { + downArrow: boolean + escape: boolean + leftArrow: boolean + return: boolean + rightArrow: boolean + shift: boolean + tab: boolean + upArrow: boolean +} + +const isUnresolved = (target: ConnectionOperationTarget): boolean => + !['connected', 'skipped', 'expired', 'unavailable'].includes(target.state) || Boolean(target.discovery_error) + +const initialDraft = (fields: ConnectionTargetEnvField[]): Record => + Object.fromEntries(fields.map(field => [field.name, field.secret ? '' : field.default])) + +const fieldLabel = (field: ConnectionTargetEnvField): string => field.prompt || field.name + +const isAuthorizedWithoutTools = (target: ConnectionOperationTarget | null): boolean => + target?.state === 'connected' && Boolean(target.discovery_error) + +const isAwaitingBrowser = (target: ConnectionOperationTarget | null): boolean => + target?.state === 'initiated' && Boolean(target.connect_url) + +/** Key routing while the field list and the Connect/Cancel selector are on screen. */ +function handleFormKey(key: InputKey, h: FormKeyHandlers): void { + const rows = h.fieldCount + 1 + const back = (value: number) => (value - 1 + rows) % rows + const forward = (value: number) => (value + 1) % rows + + if (h.submitting) { + return + } + + if (key.shift && key.tab) { + h.setFocus(back) + } else if (key.tab || (key.downArrow && !h.selectorFocused)) { + h.setFocus(forward) + } else if (key.upArrow && !h.selectorFocused) { + h.setFocus(back) + } else if (h.selectorFocused && (key.leftArrow || key.rightArrow || key.upArrow || key.downArrow)) { + h.setAction(value => (value === 0 ? 1 : 0)) + } else if (h.selectorFocused && key.return) { + h.connect() + } +} + +function AuthorizedWithoutTools({ error, t }: { error: string; t: Theme }) { + return ( + + Authorized. Tools unavailable. + {error} + ▸ Continue + Esc close + + ) +} + +function AwaitingBrowser({ t, target }: { t: Theme; target: ConnectionOperationTarget }) { + return ( + + Set up {target.name} + {target.connect_url} + {target.detail ? {target.detail} : null} + Press Enter to open in browser + + ) +} + +interface FieldRowProps { + cols: number + draftValue: string + field: ConnectionTargetEnvField + focused: boolean + onChange: (value: string) => void + onSubmit: () => void + showSet: boolean + submitting: boolean + t: Theme +} + +function FieldRow({ cols, draftValue, field, focused, onChange, onSubmit, showSet, submitting, t }: FieldRowProps) { + return ( + + + {focused ? '▸ ' : ' '}{fieldLabel(field)}{field.required ? ' *' : ''} + + + {showSet ? ( + Set + ) : ( + + )} + + + ) +} + +interface SetupFormProps { + action: 0 | 1 + cols: number + draft: Record + fields: ConnectionTargetEnvField[] + focus: number + missingRequired: ConnectionTargetEnvField | undefined + onChange: (name: string, value: string) => void + onFieldSubmit: (index: number) => void + selectorFocused: boolean + submittedSecrets: Set + submitting: boolean + t: Theme + target: ConnectionOperationTarget +} + +function SetupForm(p: SetupFormProps) { + const { t, target } = p + + return ( + + Set up {target.name} + {target.instructions ? {target.instructions} : null} + {p.fields.map((field, index) => ( + p.onChange(field.name, value)} + onSubmit={() => p.onFieldSubmit(index)} + showSet={p.submittedSecrets.has(field.name) && p.submitting} + submitting={p.submitting} + t={t} + /> + ))} + {target.state === 'failed' && target.detail ? {target.detail} : null} + + {p.action === 0 ? '▸ ' : ' '}Connect {p.action === 1 ? '▸ ' : ' '}Cancel + + {p.missingRequired ? {fieldLabel(p.missingRequired)} is required. : null} + {p.submitting ? Pending… : null} + ↑/↓ or Tab move · ←/→ select · Enter confirm · Esc cancel + + ) +} + +// The component owns one draft across backend snapshots; splitting it would remount and erase failed submissions. +export function ConnectionSetupOverlay({ cols, t }: ConnectionSetupOverlayProps) { + const operation = useStore($connectionOperation) + const sid = useStore($uiSessionId) + const { gw } = useGateway() + const target = operation?.targets.find(isUnresolved) ?? null + const fields = useMemo(() => target?.required_env ?? [], [target?.required_env]) + const targetKey = `${operation?.opId ?? ''}:${target?.name ?? ''}` + const [draft, setDraft] = useState>(() => initialDraft(fields)) + const [focus, setFocus] = useState(0) + const [action, setAction] = useState<0 | 1>(0) + const [submitting, setSubmitting] = useState(false) + const [submittedSecrets, setSubmittedSecrets] = useState>(() => new Set()) + + // A new target starts clean. Every backend snapshot carries a freshly parsed required_env, so the + // same target's fields only fill in what the draft lacks: a failed Connect keeps what was typed. + useEffect(() => { + setDraft({}) + setFocus(0) + setAction(0) + setSubmitting(false) + setSubmittedSecrets(new Set()) + }, [targetKey]) + + useEffect(() => { + setDraft(current => ({ ...initialDraft(fields), ...current })) + }, [fields]) + + useEffect(() => { + if (target?.state === 'failed') { + setSubmitting(false) + } + + if (target?.state === 'connected') { + setDraft(current => + Object.fromEntries(fields.map(field => [field.name, field.secret ? '' : current[field.name] ?? ''])) + ) + } + }, [fields, target?.state]) + + useEffect(() => { + if (operation && operation.targets.every(item => !isUnresolved(item))) { + clearConnectionOperation() + } + }, [operation]) + + const missingRequired = fields.find(field => field.required && !draft[field.name]?.trim()) + const selectorFocused = focus === fields.length + + const respond = (result: ConnectionRespondParams['result']) => { + if (!operation || !sid || submitting) { + return + } + + setSubmitting(true) + void gw + .request('connection.respond', { op_id: operation.opId, result, session_id: sid }) + .catch(() => setSubmitting(false)) + } + + const settleOnKey = (key: InputKey): boolean => { + if (isAuthorizedWithoutTools(target)) { + if (key.escape || key.return) { + respond({ settled_by: 'continue' }) + } + + return true + } + + if (isAwaitingBrowser(target)) { + if (key.return && target?.connect_url) { + openExternalUrl(target.connect_url) + } + + return !key.escape + } + + return false + } + + const cancel = () => { + if (!target) { + clearConnectionOperation() + + return + } + + respond({ targets: [{ name: target.name, status: 'skipped' }] }) + } + + const connect = () => { + if (!target || missingRequired) { + const index = missingRequired ? fields.indexOf(missingRequired) : 0 + setFocus(index < 0 ? 0 : index) + + return + } + + setSubmittedSecrets(new Set(fields.filter(field => field.secret).map(field => field.name))) + respond({ targets: [{ env: draft, name: target.name, status: 'approved' }] }) + } + + // A single input owner guarantees Esc and navigation cause exactly one action. + useInput((_ch, key) => { + if (settleOnKey(key)) { + return + } + + if (key.escape) { + cancel() + + return + } + + handleFormKey(key, { + cancel, + connect: () => (action === 0 ? connect() : cancel()), + fieldCount: fields.length, + selectorFocused, + setAction, + setFocus, + submitting + }) + }) + + if (!operation || !target) { + return null + } + + if (isAuthorizedWithoutTools(target)) { + return + } + + if (isAwaitingBrowser(target)) { + return + } + + return ( + setDraft(current => ({ ...current, [name]: value }))} + onFieldSubmit={index => setFocus(index === fields.length - 1 ? fields.length : index + 1)} + selectorFocused={selectorFocused} + submittedSecrets={submittedSecrets} + submitting={submitting} + t={t} + target={target} + /> + ) +} diff --git a/website/docs/reference/tools-reference.md b/website/docs/reference/tools-reference.md index b65bc8b5f3..d79d5f3663 100644 --- a/website/docs/reference/tools-reference.md +++ b/website/docs/reference/tools-reference.md @@ -64,7 +64,7 @@ One tool for both kinds of external app. A target is a managed connector (`"gmai | Tool | Description | Requires environment | |------|-------------|----------------------| -| `manage_connections` | Managed actions: `status`, `connect`, `reconnect` (repairs only what is not connected; `force: true` restarts a working one). MCP actions, for `mcp: true` targets only: `install` a catalog entry, `enable` a disabled configured server, `authorize` (OAuth). On the desktop every action shows a card and blocks until each target is connected, skipped, or the deadline passes; the result lists targets as `connected`, `skipped` or `not_connected` and carries no link. On surfaces with no card (CLI, TUI, messaging) managed targets return a `connect_url` per app for the user to open, and MCP targets return `unavailable` with the `hermes mcp install ` / `hermes mcp login ` commands. Cannot disconnect or revoke an account. | — | +| `manage_connections` | Managed actions: `status`, `connect`, `reconnect` (repairs only what is not connected; `force: true` restarts a working one). MCP actions, for `mcp: true` targets only: `install` a catalog entry, `enable` a disabled configured server, `authorize` (OAuth). In the desktop app, the terminal UI and the classic CLI every action shows a card and blocks until each target is connected, skipped, or the 300-second deadline passes; the result lists targets as `connected`, `skipped` or `not_connected` and carries no link. An MCP `install` collects the entry's setup values in the card, runs the entry's OAuth from the card when it has one (the user opens the link; nothing opens by itself), and saves configuration, tokens and values together when the server accepts the token. A connected MCP target carries `tools` (the registered names) and `tools_listing`, and those tools are callable through `tool_describe`/`tool_call` in the same turn. A target with `discovery_error` is authorized but its tools could not be listed; call `install` or `authorize` for it again to retry discovery without new consent. On surfaces with no card (messaging, scripted dispatch) managed targets return a `connect_url` per app for the user to open, and an MCP target runs at once: `authorize` and an OAuth `install` return the authorization URL, other installs and `enable` report the outcome, and a missing credential comes back as `failed` naming the variable to set. Cannot disconnect or revoke an account. | — | The deadline for one call is five minutes, fixed by the backend when the call starts; reopening the chat or restarting the desktop never extends it. The tool is present only when the diff --git a/website/docs/user-guide/features/mcp.md b/website/docs/user-guide/features/mcp.md index 82eae758eb..5ac6816141 100644 --- a/website/docs/user-guide/features/mcp.md +++ b/website/docs/user-guide/features/mcp.md @@ -57,10 +57,33 @@ Hermes ships a curated catalog of MCP servers that Nous staff has reviewed and merged. They're disabled by default — install only what you actually want. -In the desktop app you can also ask: "add the Linear MCP". The agent calls -`manage_connections` with an `mcp: true` target, an approval card appears in -the chat, and Install writes the same config the CLI would. On the CLI and in -messaging apps the agent relays the commands below instead. +You can also ask in chat: "add the Linear MCP". The agent calls +`manage_connections` with an `mcp: true` target and a setup card appears. The +card works the same way in the desktop app (a dialog), the terminal UI +(`hermes --tui`, a callout above the composer) and the classic CLI (a panel): + +1. **Fields.** If the entry declares setup values, the card shows all of them + at once. A plain value is prefilled with its default. A secret is masked. + Nothing is saved while you type. +2. **Connect or Cancel.** Cancel skips that one server; other servers in the + same request continue. +3. **Authorization.** For an OAuth entry the card shows the authorization link. + Hermes never opens the browser by itself: click **Open in browser** on the + desktop, or press Enter in the terminal. Over SSH the card tells you how to + reach the callback port or paste the redirected URL. +4. **Save.** Hermes saves the server configuration, the tokens and your setup + values together, once the server has accepted the new token and the first + connection has returned. If the server rejects the token, or you cancel + before that point, nothing from the attempt is kept, your earlier + configuration and tokens stay as they were, and a failed form reopens with + what you typed. A server that is already authorized connects with its saved + tokens; Hermes asks you to authorize again only when they no longer work. +5. **Tools.** Hermes then lists the server's tools and registers them. The + agent can call them in the same turn. If authorization worked and the tool + list failed, the card says "Authorized. Tools unavailable." and the agent can + run discovery again later without asking you to authorize again. + +In messaging apps there is no card; the agent relays the commands below. ```bash hermes mcp # interactive picker (default)