diff --git a/agent/agent_init.py b/agent/agent_init.py index e62cf5179b..50ff4ae4bf 100644 --- a/agent/agent_init.py +++ b/agent/agent_init.py @@ -2162,7 +2162,7 @@ _CALLBACK_PARAMS = ( "tool_progress_callback", "tool_start_callback", "tool_complete_callback", "thinking_callback", "reasoning_callback", "clarify_callback", "read_terminal_callback", "read_preview_callback", "drive_preview_callback", - "read_window_below_callback", "setup_mcp_callback", "tour_callback", + "read_window_below_callback", "connection_callback", "tour_callback", "step_callback", "stream_delta_callback", "interim_assistant_callback", "status_callback", "notice_callback", "notice_clear_callback", "event_callback", "reaction_callback", "tool_gen_callback", @@ -2185,7 +2185,7 @@ def init_agent( thinking_callback: callable = None, reasoning_callback: callable = None, clarify_callback: callable = None, read_terminal_callback: callable = None, read_preview_callback: callable = None, drive_preview_callback: callable = None, - read_window_below_callback: callable = None, setup_mcp_callback: callable = None, + read_window_below_callback: callable = None, connection_callback: callable = None, tour_callback: callable = None, step_callback: callable = None, stream_delta_callback: callable = None, interim_assistant_callback: callable = None, tool_gen_callback: callable = None, status_callback: callable = None, diff --git a/agent/agent_runtime_helpers.py b/agent/agent_runtime_helpers.py index 4c49896d5c..eda41e199e 100644 --- a/agent/agent_runtime_helpers.py +++ b/agent/agent_runtime_helpers.py @@ -79,7 +79,8 @@ def _ra(): AGENT_RUNTIME_POST_HOOK_TOOL_NAMES = frozenset({ "todo_list", "session_search", "memory", "clarify", "read_terminal", "desktop_preview", - "drive_preview", "annotate_preview", "read_window_below", "setup_mcp", "gui_tour", "delegate_task", + "drive_preview", "annotate_preview", "read_window_below", "manage_connections", "setup_mcp", "gui_tour", + "delegate_task", }) _TRAJECTORY_SYSTEM_PROMPT = ( diff --git a/agent/inline_tool_executors.py b/agent/inline_tool_executors.py index 98b9ef85ea..bcb6669bbc 100644 --- a/agent/inline_tool_executors.py +++ b/agent/inline_tool_executors.py @@ -150,6 +150,27 @@ def _desktop_preview(agent, args: dict, ctx: InlineToolContext) -> Any: return _handle_preview(args) +def _manage_connections(agent, args: dict, ctx: InlineToolContext) -> Any: + # The GUI callback lives on the agent; registry dispatch never forwards it. + from tools.connections_tool import _connectors_available, manage_connections + + return manage_connections( + args, session_id=getattr(agent, "session_id", None), + connection_callback=getattr(agent, "connection_callback", None), + connectors_available=_connectors_available, + ) + + +def _setup_mcp_shim(agent, args: dict, ctx: InlineToolContext) -> Any: + # Replay shim for conversations whose cached prompt still names setup_mcp. + # Not in _LEGACY_TOOL_ALIASES: inline tools bypass handle_function_call. + return _manage_connections(agent, { + "action": args.get("action", "install"), + "connectors": [{"name": args.get("server", ""), "mcp": True}], + "reason": args.get("reason", ""), + }, ctx) + + # Order is the historical if/elif order of ``execute_tool_calls_sequential``. INLINE_TOOL_EXECUTORS: Dict[str, InlineToolExecutor] = { "todo_list": _tool( @@ -192,10 +213,8 @@ INLINE_TOOL_EXECUTORS: Dict[str, InlineToolExecutor] = { ("action", "action", ""), ("surface", "surface"), ("selector", "selector"), ("title", "title"), ("text", "text"), ("side", "side"), ("steps", "steps"), ("step_index", "step_index"), ), - "setup_mcp": _callback_tool( - "tools.setup_mcp_tool", "setup_mcp_tool", "setup_mcp_callback", - ("server", "server", ""), ("action", "action", "install"), ("reason", "reason", ""), - ), + "manage_connections": _manage_connections, + "setup_mcp": _setup_mcp_shim, "delegate_task": lambda agent, args, ctx: agent._dispatch_delegate_task(args), } diff --git a/agent/tool_executor.py b/agent/tool_executor.py index 3fc5f6b8b5..91ba64c541 100644 --- a/agent/tool_executor.py +++ b/agent/tool_executor.py @@ -786,7 +786,9 @@ def _resolve_sequential_tool_timeout() -> float | None: # 420 s deadline every real batch "timed out" while its children ran on as orphans, and the orchestrator # spent the following hours polling transcripts (measured: 332 timeouts, ~$4k of orchestrator turns in # one run). -_SEQUENTIAL_DEADLINE_EXEMPT_TOOLS = frozenset({"delegate_task"}) +# ``manage_connections`` waits on connections.wait_timeout_seconds; the generic deadline +# would return tool_timeout while its approval card is still open. +_SEQUENTIAL_DEADLINE_EXEMPT_TOOLS = frozenset({"delegate_task", "manage_connections"}) def _abandoned_sequential_result(agent, ref: _ToolCallRef, message: str, result_cls, **outcome) -> _ManagedToolResult: diff --git a/apps/desktop/src/app/chat/composer/hooks/use-composer-submit.ts b/apps/desktop/src/app/chat/composer/hooks/use-composer-submit.ts index f1f18ea115..7339409ec2 100644 --- a/apps/desktop/src/app/chat/composer/hooks/use-composer-submit.ts +++ b/apps/desktop/src/app/chat/composer/hooks/use-composer-submit.ts @@ -7,7 +7,7 @@ import { hasClarifyRequest, skipClarifyRequest } from '@/store/clarify' import { clearSessionDraft, type ComposerAttachment } from '@/store/composer' import { resetBrowseState } from '@/store/composer-input-history' import { enqueueQueuedPrompt, type QueuedPromptEntry } from '@/store/composer-queue' -import { hasMcpSetupRequest, skipMcpSetupRequest } from '@/store/mcp-setup' +import { hasConnectionRequest, skipConnectionRequest } from '@/store/connection-request' import { hasBlockingPromptRequest } from '@/store/prompts' import { cloneAttachments, type QueueEditState } from '../composer-utils' @@ -234,10 +234,9 @@ export function useComposerSubmit({ void skipClarifyRequest(sessionId) } - // Same deal for a pending MCP setup card: the agent is blocked on - // mcp.setup.respond, so a typed message declines the card and rides on. - if (payloadPresent && !queueEdit && hasMcpSetupRequest(sessionId)) { - void skipMcpSetupRequest(sessionId) + // Same for a pending connection card: typing declines every target. + if (payloadPresent && !queueEdit && hasConnectionRequest(sessionId)) { + void skipConnectionRequest(sessionId) } // Approval / sudo / secret prompts also park the turn inside a tool batch, diff --git a/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event/input-requests.ts b/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event/input-requests.ts index fd4e6ea6cc..6030517a19 100644 --- a/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event/input-requests.ts +++ b/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event/input-requests.ts @@ -1,7 +1,7 @@ import { pendingClarifyToolPayload } from '@/app/session/hooks/use-session-actions/restore-pending-clarify' import { settlePendingClarifyToolCall } from '@/lib/chat-messages' import { $clarifyRequests, clearClarifyRequest } from '@/store/clarify' -import { $mcpSetupRequests, clearMcpSetupRequest } from '@/store/mcp-setup' +import { $connectionRequests, clearConnectionRequest } from '@/store/connection-request' import { $approvalRequests, $secretRequests, @@ -81,8 +81,12 @@ export function handleInputRequestEvent(ctx: GatewayEventContext): boolean { clearVaultSaveLoginRequest(sessionId, id) } else if ($vaultUnlockRequests.get()[key]?.requestId === id) { clearVaultUnlockRequest(sessionId, id) - } else if ($mcpSetupRequests.get()[key]?.requestId === id) { - clearMcpSetupRequest(id, sessionId) + } else if ($connectionRequests.get()[key]?.requestId === id) { + clearConnectionRequest(id, sessionId) + + if (sessionId) { + deps.updateSessionState(sessionId, state => ({ ...state, needsInput: false })) + } } return true diff --git a/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event/server-requests.test.ts b/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event/server-requests.test.ts index 28913fcacc..aaf9fac978 100644 --- a/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event/server-requests.test.ts +++ b/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event/server-requests.test.ts @@ -1,11 +1,19 @@ import { afterEach, describe, expect, it, vi } from 'vitest' +import { createClientSessionState } from '@/lib/chat-runtime' +import { $connectionRequests } from '@/store/connection-request' +import { resetServerRequestsForTests } from '@/store/server-requests' import { $toursEnabled } from '@/store/tours' import { handleServerRequest } from './server-requests' import type { ServerRequestContext } from './server-requests' -const deps = {} as ServerRequestContext['deps'] +const deps = { + activeSessionIdRef: { current: null }, + sessionInterrupted: () => false, + updateSessionState: (_sessionId, update) => update(createClientSessionState('stored-session')), + upsertToolCall: () => undefined +} as ServerRequestContext['deps'] function deliver(method: string, params: Record, activeSessionId: null | string) { const respond = vi.fn() @@ -15,6 +23,30 @@ function deliver(method: string, params: Record, activeSessionI return { fail, handled, respond } } +describe('connection request routing', () => { + afterEach(() => { + $connectionRequests.set({}) + resetServerRequestsForTests() + }) + + it('parks a connection request and replaces a replayed request with the same id', () => { + const params = { + deadline_at: 1_800_000_000, + op_id: 'op-1', + reason: 'Install Linear', + session_id: 'session-a', + targets: [{ action: 'install', kind: 'mcp', name: 'linear' }], + timeout_seconds: 60 + } + + expect(deliver('connection', params, 'session-a').handled).toBe(true) + expect($connectionRequests.get()['session-a']).toMatchObject({ opId: 'op-1', requestId: 'srq-1' }) + + expect(deliver('connection', { ...params, op_id: 'op-2' }, 'session-a').handled).toBe(true) + expect($connectionRequests.get()['session-a']?.opId).toBe('op-2') + }) +}) + describe('preview action request routing', () => { it('leaves a scoped action request unanswered in a window showing another session', () => { const { handled, respond, fail } = deliver('preview.act', { action: 'elements', session_id: 'session-a' }, 'session-b') diff --git a/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event/server-requests.ts b/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event/server-requests.ts index cbbef3b538..37df0e2811 100644 --- a/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event/server-requests.ts +++ b/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event/server-requests.ts @@ -1,13 +1,16 @@ +import type { ServerRequestMap } from '@hermes/shared' + import { readActivePreview } from '@/app/chat/right-rail/preview-reader' import { readActiveTerminal } from '@/app/right-sidebar/terminal/buffer' import { pendingClarifyToolPayload } from '@/app/session/hooks/use-session-actions/restore-pending-clarify' +import { connectionRequestToolPayload } from '@/app/session/hooks/use-session-actions/restore-pending-connection' import { translateNow } from '@/i18n' import { restorePendingClarifyToolCall } from '@/lib/chat-messages' import type { PreviewActAction } from '@/lib/preview-act/act-in-page' import type { TourAction, TourStep } from '@/lib/tour' import { normalizeChoices, normalizeQuestions, setClarifyRequest, warnDroppedChoices } from '@/store/clarify' +import { normalizeConnectionRequest, setConnectionRequest } from '@/store/connection-request' import type { ScopedServerRequest } from '@/store/gateway' -import { setMcpSetupRequest } from '@/store/mcp-setup' import { dispatchNativeNotification } from '@/store/native-notifications' import { receiveApprovalRequest, @@ -41,6 +44,11 @@ const loadPreviewEngine = () => { const str = (v: unknown): string => (typeof v === 'string' ? v : '') const num = (v: unknown): number | undefined => (typeof v === 'number' ? v : undefined) +/** The params of a request whose method the handler table already matched: the backend validated + * them against `ServerRequestMap[M]['params']` before sending, so the method name is the contract. */ +const paramsOf = (request: ScopedServerRequest, _method: M) => + request.params as unknown as ServerRequestMap[M]['params'] + /** Answer a string-valued request with a JSON-encoded result ('' = nothing / unavailable). */ const answerValue = (request: ScopedServerRequest, result: unknown) => request.respond({ value: result ? JSON.stringify(result) : '' }) @@ -257,33 +265,25 @@ const vaultUnlockPrompt: Handler = ctx => { notifyInput(ctx, translateNow('prompts.vaultUnlockTitle', displayName)) } -const mcpSetup: Handler = ctx => { - // setup_mcp tool (desktop GUI): the agent proposed an MCP server. Park the - // request per-session (like clarify) and upsert a stable pending tool row so - // the inline consent card has somewhere to render even when the tool.start - // event was missed (stream reconnect / hydration race). +const connection: Handler = ctx => { const { deps, request, sessionId } = ctx - const p = request.params - const server = str(p.server) - const rawAction = str(p.action) || 'install' - const action = rawAction === 'enable' || rawAction === 'authorize' ? rawAction : 'install' - const reason = str(p.reason) + const entry = normalizeConnectionRequest(paramsOf(request, 'connection'), request.id, sessionId || null) - if (!server) { - request.respond({ value: '' }) + if (!entry) { + request.respond({ settled_by: 'all_resolved', targets: [] }) return } rememberServerRequest(request) - setMcpSetupRequest({ action, reason, requestId: request.id, server, sessionId: sessionId || null }) + setConnectionRequest(entry) if (sessionId) { - deps.upsertToolCall(sessionId, { args: { action, reason, server }, name: 'setup_mcp', tool_id: request.id }, 'running') + deps.upsertToolCall(sessionId, connectionRequestToolPayload(entry), 'running') } markNeedsInput(ctx) - notifyInput(ctx, reason || server) + notifyInput(ctx, entry.reason || entry.targets.map(target => target.name).join(', ')) } // ── Desktop-surface bridges (answered immediately, no card) ───────────────── @@ -401,7 +401,7 @@ const tour: Handler = ({ isActiveSession, request, sessionId }) => { export const SERVER_REQUEST_HANDLERS: Record = { approval, clarify, - 'mcp.setup': mcpSetup, + connection, 'preview.act': previewAct, 'preview.read': previewRead, secret, diff --git a/apps/desktop/src/app/session/hooks/use-session-actions/index.ts b/apps/desktop/src/app/session/hooks/use-session-actions/index.ts index 7bbb4c14a3..a8036b3871 100644 --- a/apps/desktop/src/app/session/hooks/use-session-actions/index.ts +++ b/apps/desktop/src/app/session/hooks/use-session-actions/index.ts @@ -1806,7 +1806,6 @@ export function useSessionActions({ const pendingApproval = restorePendingApproval(resumed, resumed.session_id) const pendingClarifyState = restorePendingClarifyFromSnapshot(resumed, resumed.session_id, resumeStartedAt) const pendingClarify = pendingClarifyState.request - const clarifyAuthoritativelyAbsent = pendingClarifyState.authoritativeAbsent && !$clarifyRequests.get()[resumed.session_id] @@ -1861,7 +1860,8 @@ export function useSessionActions({ // Backend reported this turn running at resume time — live proof. turnLive: state.turnLive || resumedRunning, needsInput: - pendingApproval || Boolean(pendingClarify) || (clarifyAuthoritativelyAbsent ? false : state.needsInput), + pendingApproval || + Boolean(pendingClarify) || (clarifyAuthoritativelyAbsent ? false : state.needsInput), adoptedRunningTurn: state.adoptedRunningTurn || resumedRunning, ...(inFlightRecovery.applied ? { 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 new file mode 100644 index 0000000000..c994aee7ea --- /dev/null +++ b/apps/desktop/src/app/session/hooks/use-session-actions/restore-pending-connection.ts @@ -0,0 +1,23 @@ +import { type ChatMessage, type GatewayEventPayload, restorePendingBlockingToolCall } from '@/lib/chat-messages' +import type { ConnectionRequest } from '@/store/connection-request' + +/** Tool row for a pending operation whose `tool.start` event was missed. */ +export function connectionRequestToolPayload(request: ConnectionRequest): GatewayEventPayload & { name: string } { + return { + args: { + action: request.targets[0]?.action ?? 'install', + connectors: request.targets.map(target => ({ mcp: target.kind === 'mcp', name: target.name })), + reason: request.reason + }, + name: 'manage_connections', + tool_id: request.requestId + } +} + +/** Add the pending connection row to a projected transcript; null when there is none. */ +export function projectPendingConnection( + messages: ChatMessage[], + request: ConnectionRequest | null +): { messages: ChatMessage[]; streamId: string } | null { + return request ? restorePendingBlockingToolCall(messages, connectionRequestToolPayload(request)) : null +} 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 60cf49b8d7..93fd2c3c4e 100644 --- a/apps/desktop/src/components/assistant-ui/mcp-setup-tool.tsx +++ b/apps/desktop/src/components/assistant-ui/mcp-setup-tool.tsx @@ -9,27 +9,22 @@ import { useSessionView } from '@/app/chat/session-view' import { ToolFallback } from '@/components/assistant-ui/tool/fallback' import { WIDGET_SHELL_CLASS } from '@/components/chat/widget-shell' import { ConnectorCard, type ConnectorCardCopy, ConnectorSummary } from '@/components/ui/connector-card' -import { - addMcpServer, - getActionStatus, - getMcpCatalog, - installMcpCatalogEntry, - type McpCatalogEntry, - removeMcpServer, - setMcpServerEnabled -} from '@/hermes' +import { getActionStatus, getMcpCatalog, installMcpCatalogEntry, type McpCatalogEntry, setMcpServerEnabled } from '@/hermes' import { useI18n } from '@/i18n' +import { connectorText, 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 { directoryEntry } from '@/lib/mcp-directory' import { prettyName } from '@/lib/text' import { cn } from '@/lib/utils' +import { + type ConnectionTargetOutcome, + respondToConnectionRequest, + sessionConnectionRequest +} from '@/store/connection-request' import { $gateway } from '@/store/gateway' -import { clearMcpSetupRequest, type McpSetupOutcome, sessionMcpSetupRequest } from '@/store/mcp-setup' import { notifyError } from '@/store/notifications' -import { respondToServerRequest } from '@/store/server-requests' import { invalidateMcpSuggestionIndex } from '@/store/suggestion-providers/mcp' import { selectMessageRunning } from './tool/fallback-model' @@ -49,26 +44,42 @@ const CATALOG_INSTALL_POLL_MS = 1500 // has already been sent, so the catch path must swallow this, not report it. const CANCELLED = Symbol('mcp-setup-cancelled') +/** First MCP target of a `manage_connections` call; the card renders one server. */ function readSetupArgs(args: unknown): SetupArgs { const row = parseMaybeObject(args) - const rawAction = typeof row.action === 'string' ? row.action : 'install' + const [target] = mcpTargets('manage_connections', row) return { - action: rawAction === 'enable' || rawAction === 'authorize' ? rawAction : 'install', + action: target?.action ?? 'install', reason: typeof row.reason === 'string' ? row.reason : '', - server: typeof row.server === 'string' ? row.server : '' + server: target?.name ?? '' } } -/** The tool's settled JSON — the card's outcome plus the tool-only - * `unanswered` status (timeout, no user action). */ -type SettledResult = Omit, 'status'> & { - status?: McpSetupOutcome['status'] | 'unanswered' - note?: string +/** The first target's state from the settled operation. */ +interface SettledResult { + status?: 'connected' | 'not_connected' | 'skipped' | 'unavailable' + detail?: string + server?: string + tools?: string[] } function readSetupResult(result: unknown): SettledResult { - return parseMaybeObject(result) as SettledResult + const row = parseMaybeObject(result) + const [target] = Array.isArray(row.targets) ? row.targets.map(parseMaybeObject) : [] + + if (!target) { + return {} + } + + const STATES: readonly NonNullable[] = ['connected', 'not_connected', 'skipped', 'unavailable'] + + return { + detail: connectorText(target.detail), + server: connectorText(target.name), + status: STATES.find(state => state === target.state), + tools: Array.isArray(target.tools) ? target.tools.map(connectorText).filter((t): t is string => t !== undefined) : undefined + } } const SHELL_CLASS = `${WIDGET_SHELL_CLASS} text-[length:var(--conversation-text-font-size)] text-(--ui-text-primary)` @@ -129,24 +140,27 @@ function McpSetupSettled({ args, result }: ToolCallMessagePartProps) { const fromResult = useMemo(() => readSetupResult(result), [result]) const server = fromResult.server || fromArgs.server - const status = fromResult.status ?? 'error' + const status = fromResult.status ?? 'not_connected' const displayName = prettyName(server) - const line = - status === 'installed' - ? copy.installed(displayName) - : status === 'enabled' - ? copy.enabled(displayName) - : status === 'authorized' - ? copy.authorized(displayName) - : status === 'declined' - ? copy.declined - : status === 'unanswered' - ? copy.unanswered - : copy.failed(displayName) + const connectedLine = + fromArgs.action === 'enable' + ? copy.enabled(displayName) + : fromArgs.action === 'authorize' + ? copy.authorized(displayName) + : copy.installed(displayName) - const ok = status === 'installed' || status === 'enabled' || status === 'authorized' - const neutral = status === 'declined' || status === 'unanswered' + const line = + status === 'connected' + ? connectedLine + : status === 'skipped' + ? copy.declined + : status === 'not_connected' && fromResult.detail === 'deadline' + ? copy.unanswered + : copy.failed(displayName) + + const ok = status === 'connected' + const neutral = status === 'skipped' || (status === 'not_connected' && fromResult.detail === 'deadline') const toolCount = Array.isArray(fromResult.tools) ? fromResult.tools.length : 0 // Settled is scaffolding, the same line a spent connector offer collapses @@ -172,13 +186,14 @@ function McpSetupPending({ args }: ToolCallMessagePartProps) { // The tool row is in whichever session's transcript rendered it — read THAT // session's request (primary or tile), not the globally-active one. const sessionId = useStore(useSessionView().$runtimeId) - const $request = useMemo(() => sessionMcpSetupRequest(sessionId), [sessionId]) + const $request = useMemo(() => sessionConnectionRequest(sessionId), [sessionId]) const request = useStore($request) const gateway = useStore($gateway) const fromArgs = useMemo(() => readSetupArgs(args), [args]) - const server = fromArgs.server || request?.server || '' - const action: SetupAction = fromArgs.action ?? request?.action ?? 'install' + const [requestTarget] = request?.targets ?? [] + const server = fromArgs.server || requestTarget?.name || '' + const action: SetupAction = fromArgs.action ?? requestTarget?.action ?? 'install' const reason = fromArgs.reason || request?.reason || '' const [working, setWorking] = useState(false) @@ -190,16 +205,12 @@ function McpSetupPending({ args }: ToolCallMessagePartProps) { // CANCELLED sentinel; the declined respond has already been sent by then. const cancelRef = useRef(false) - // Race: tool.start fires a tick before mcp.setup.request — hold the buttons - // until the gateway request is wired (same spinner rule as clarify). + // tool.start arrives before the server request; disable the buttons until the request exists. const ready = Boolean(request?.requestId) const respond = useCallback( - async (outcome: McpSetupOutcome) => { - // Another path (cancel racing completion) may have already resolved this - // request; the store is the single source of truth, so bail if this - // session's entry is gone — same guard as the approval bar. - if (!request || sessionMcpSetupRequest(request.sessionId).get()?.requestId !== request.requestId) { + async (outcome: ConnectionTargetOutcome) => { + if (!request) { return } @@ -209,31 +220,21 @@ function McpSetupPending({ args }: ToolCallMessagePartProps) { return } - // Clear first: the answer is decided, and an in-flight RPC must not - // leave a live card that can be answered a second time. - clearMcpSetupRequest(request.requestId, request.sessionId) + const success = outcome.state === 'installed' || outcome.state === 'enabled' || outcome.state === 'authorized' - // A successful outcome changed mcp_servers — reload the live session - // BEFORE unblocking the tool, or the agent resumes being told the - // server is ready while its tool snapshot still lacks it (the same - // write-through mcp-tab's silentReload does; consent was the card - // click, so no confirm prompt). Reload failure isn't outcome failure: - // the config landed, tools arrive next session — report it and move on. - if (outcome.status === 'installed' || outcome.status === 'enabled' || outcome.status === 'authorized') { - try { - await gateway.request('reload.mcp', { confirm: true, session_id: request.sessionId ?? undefined }) - } catch (error) { - notifyError(error, copy.reloadFailed) - } - - // The just-set-up server must stop being suggested immediately. + if (success) { + // No reload.mcp: the between-turns refresh registers the new server's tools. invalidateMcpSuggestionIndex() } - respondToServerRequest(request.requestId, { value: JSON.stringify(outcome) }) - // tool.complete lands next → McpSetupSettled. +try { + // One target: this answer settles the operation. + await respondToConnectionRequest(request, { settled_by: 'all_resolved', targets: [outcome] }) + } catch (error) { + notifyError(error, copy.sendFailed) + } }, - [copy.gatewayDisconnected, copy.reloadFailed, copy.sendFailed, gateway, request] + [copy.gatewayDisconnected, copy.sendFailed, gateway, request] ) const decline = useCallback(() => { @@ -241,7 +242,7 @@ function McpSetupPending({ args }: ToolCallMessagePartProps) { // and let the abandoned work notice via cancelRef at its next poll. cancelRef.current = true triggerHaptic('cancel') - void respond({ server, status: 'declined' }) + void respond({ name: server, state: 'declined' }) }, [respond, server]) const approve = useCallback(async () => { @@ -263,7 +264,7 @@ function McpSetupPending({ args }: ToolCallMessagePartProps) { if (action === 'enable') { await setMcpServerEnabled(server, true) triggerHaptic('submit') - await respond({ server, status: 'enabled' }) + await respond({ name: server, state: 'enabled' }) return } @@ -276,16 +277,12 @@ function McpSetupPending({ args }: ToolCallMessagePartProps) { }) triggerHaptic('submit') - await respond({ server, status: 'authorized', tools: (flow.tools ?? []).map(tool => tool.name) }) + await respond({ name: server, state: 'authorized', tools: (flow.tools ?? []).map(tool => tool.name) }) return } - // Install: prefer the reviewed catalog entry when one exists; otherwise - // fall back to the desktop suggestion directory (official URL-only - // remotes), written through the same validated POST the dashboard's add - // form uses. Required catalog credentials get an inline prompt first - // (never pre-filled, never echoed back). + // Install from the catalog only. Required credentials are prompted inline first. let resolved = entry if (resolved === undefined) { @@ -295,38 +292,7 @@ function McpSetupPending({ args }: ToolCallMessagePartProps) { } if (!resolved) { - const known = directoryEntry(server) - - if (!known) { - await respond({ detail: copy.notInCatalog(server), server, status: 'error' }) - - return - } - - // URL-only remote: add to config, then run the OAuth/probe flow so - // "Install" lands the user on a working server, not a 401. If the - // flow dies after the config write (cancel, closed OAuth tab), roll - // the write back — decline means "no server", not an unauthorized - // entry squatting in mcp_servers (authoritative-write rule). - await addMcpServer({ name: known.name, url: known.url }, oauthScope) - - let flow - - try { - flow = await completeMcpDesktopOAuth({ - serverName: known.name, - profile: oauthScope, - cancelled: () => cancelRef.current - }) - } catch (error) { - await removeMcpServer(known.name, oauthScope).catch(() => { - // Rollback is best-effort; the primary error/cancel wins. - }) - throw error - } - - triggerHaptic('submit') - await respond({ server, status: 'installed', tools: (flow.tools ?? []).map(tool => tool.name) }) + await respond({ detail: copy.notInCatalog(server), name: server, state: 'error' }) return } @@ -361,7 +327,7 @@ function McpSetupPending({ args }: ToolCallMessagePartProps) { } triggerHaptic('submit') - await respond({ server, status: 'installed' }) + await respond({ name: server, state: 'installed' }) } catch (error) { // User cancel: the declined respond is already on the wire — the // abandoned flow just stops, nothing to report. @@ -372,8 +338,8 @@ function McpSetupPending({ args }: ToolCallMessagePartProps) { notifyError(error, copy.failed(server)) await respond({ detail: error instanceof Error ? error.message : String(error), - server, - status: 'error' + name: server, + state: 'error' }) } finally { setWorking(false) @@ -383,11 +349,7 @@ function McpSetupPending({ args }: ToolCallMessagePartProps) { const displayName = prettyName(server) const card = cardCopy(copy, action) - // What connecting actually means — the endpoint that will be contacted. - // Catalog entries carry their transport URL in the API response; the - // static directory remains a fallback rung for older backends. - const known = directoryEntry(server) - const sourceLine = action === 'install' ? (entry?.url ?? known?.url ?? copy.catalogSource) : null + const sourceLine = action === 'install' ? (entry?.url ?? copy.catalogSource) : null // ⌘/Ctrl+Enter → approve, Esc → decline/cancel. Same accelerators, same // guard shape as the approval bar (tool/approval.tsx). Unlike approve, Esc diff --git a/apps/desktop/src/components/assistant-ui/thread/message-parts.tsx b/apps/desktop/src/components/assistant-ui/thread/message-parts.tsx index 0cb367cb33..7217161fbd 100644 --- a/apps/desktop/src/components/assistant-ui/thread/message-parts.tsx +++ b/apps/desktop/src/components/assistant-ui/thread/message-parts.tsx @@ -21,7 +21,7 @@ import { ActivityTimerText } from '@/components/chat/activity-timer-text' import { GeneratedImage } from '@/components/chat/generated-image-result' import { SCAFFOLD_LABEL_CLASS, SCAFFOLD_META_CLASS, ScaffoldRow } from '@/components/chat/scaffold-row' import { useI18n } from '@/i18n' -import { connectorCalls } from '@/lib/connector-tools' +import { connectorCalls, mcpTargets } from '@/lib/connector-tools' import { generatedImageFromResult } from '@/lib/generated-images' import { isOnboardingEnabled } from '@/lib/onboarding-enabled' import { separateGluedReasoningBlocks } from '@/lib/reasoning-blocks' @@ -120,6 +120,11 @@ const ChainToolFallback: FC = props => { ) } + // MCP targets always render the card; managed connectors only under the onboarding gate. + if (mcpTargets(props.toolName, props.args).length > 0) { + return + } + if (isOnboardingEnabled() && props.toolName === 'manage_connections') { return } @@ -128,10 +133,6 @@ const ChainToolFallback: FC = props => { return } - if (props.toolName === 'setup_mcp') { - return - } - return } diff --git a/apps/desktop/src/components/assistant-ui/tool/fallback-model/index.ts b/apps/desktop/src/components/assistant-ui/tool/fallback-model/index.ts index a785a79cda..3b7bf7d5e1 100644 --- a/apps/desktop/src/components/assistant-ui/tool/fallback-model/index.ts +++ b/apps/desktop/src/components/assistant-ui/tool/fallback-model/index.ts @@ -4,7 +4,7 @@ import { type ToolTitleKey, translateNow } from '@/i18n' import { normalizeExternalUrl } from '@/lib/external-link' import { summarizeShellCommand } from '@/lib/summarize-command' import { capitalize, firstStringField, normalize } from '@/lib/text' -import { isCardTool, isFileEditTool, isSilentTool } from '@/lib/tool-render-class' +import { CONNECTION_CARD_KEY, isCardTool, isFileEditTool, isSilentTool } from '@/lib/tool-render-class' import { envelopeErrorText, toolResultRecord } from '@/lib/tool-result-metadata' import { extractToolErrorMessage, formatToolResultSummary } from '@/lib/tool-result-summary' @@ -41,7 +41,7 @@ export * from './types' // The transcript's render budget prices a turn by the same classification, so // it lives in `@/lib/tool-render-class` where both sides can reach it without // pulling this module's formatting/i18n weight into the cost path. -export { isCardTool, isFileEditTool, isSilentTool } +export { CONNECTION_CARD_KEY, isCardTool, isFileEditTool, isSilentTool } export interface DiffLineStats { added: number diff --git a/apps/desktop/src/components/assistant-ui/tool/fallback.tsx b/apps/desktop/src/components/assistant-ui/tool/fallback.tsx index ef1bca8c1a..a7bf9aa083 100644 --- a/apps/desktop/src/components/assistant-ui/tool/fallback.tsx +++ b/apps/desktop/src/components/assistant-ui/tool/fallback.tsx @@ -41,7 +41,7 @@ import { GlyphSpinner } from '@/components/ui/glyph-spinner' import { ToolIcon } from '@/components/ui/tool-icon' import { Tip } from '@/components/ui/tooltip' import { useI18n } from '@/i18n' -import { connectorCalls } from '@/lib/connector-tools' +import { connectorCalls, mcpTargets } from '@/lib/connector-tools' import { PrettyLink, LinkifiedText as SharedLinkifiedText, urlSlugTitleLabel } from '@/lib/external-link' import { AlertCircle, CheckCircle2 } from '@/lib/icons' import { isOnboardingEnabled } from '@/lib/onboarding-enabled' @@ -59,6 +59,7 @@ import { buildToolView, clampForDisplay, cleanVisibleText, + CONNECTION_CARD_KEY, countDiffLineStats, inlineDiffFromResult, isCardTool, @@ -1042,8 +1043,9 @@ export const ToolGroupSlot: FC part.type === 'tool-call' - ? isOnboardingEnabled() && connectorCalls(part.toolName, part.args).length - ? 'manage_connections' + ? (isOnboardingEnabled() && connectorCalls(part.toolName, part.args).length) || + mcpTargets(part.toolName, part.args).length + ? CONNECTION_CARD_KEY : part.toolName : '' ) diff --git a/apps/desktop/src/lib/chat-messages/index.ts b/apps/desktop/src/lib/chat-messages/index.ts index 0166e5769e..24dbe4c369 100644 --- a/apps/desktop/src/lib/chat-messages/index.ts +++ b/apps/desktop/src/lib/chat-messages/index.ts @@ -15,6 +15,7 @@ export { export type { UnspokenTurnSpeech } from './parts' export { branchGroupForUser, preserveLocalAssistantErrors } from './reconciliation' export { + restorePendingBlockingToolCall, restorePendingClarifyToolCall, sealOpenToolParts, settlePendingClarifyToolCall, diff --git a/apps/desktop/src/lib/chat-messages/tool-parts.ts b/apps/desktop/src/lib/chat-messages/tool-parts.ts index f817cb9bd7..8a8f655c23 100644 --- a/apps/desktop/src/lib/chat-messages/tool-parts.ts +++ b/apps/desktop/src/lib/chat-messages/tool-parts.ts @@ -88,9 +88,9 @@ function toolPayloadMatchValues(payload: GatewayEventPayload | undefined): strin // `clarify.request` (a fresh request id) must correlate with the `tool.start` // row (the model's tool_call_id) so the two ids don't produce a duplicate // clarify card — same correlation ClarifyToolPending uses for request↔args. - // `server` is setup_mcp's identifying arg, for the identical reason. + // `reason` is a connection request's identifying arg (op_id is not in the model's args). const query = - firstStringField(payloadArgs, ['search_term', 'query', 'question', 'server', 'command', 'code', 'path']) || + firstStringField(payloadArgs, ['search_term', 'query', 'question', 'reason', 'command', 'code', 'path']) || batchClarifyMatchValue(payloadArgs.questions) const context = typeof payload?.context === 'string' ? payload.context.trim() : '' @@ -381,7 +381,8 @@ interface PendingClarifyLocation { function findPendingClarifyLocation( messages: ChatMessage[], - payload: GatewayEventPayload + payload: GatewayEventPayload, + toolName = 'clarify' ): PendingClarifyLocation | null { const stableId = toolId(payload) const matchValues = toolPayloadMatchValues(payload) @@ -394,7 +395,7 @@ function findPendingClarifyLocation( for (let partIndex = message.parts.length - 1; partIndex >= 0; partIndex -= 1) { const part = message.parts[partIndex] - if (part.type !== 'tool-call' || part.toolName !== 'clarify' || part.result !== undefined) { + if (part.type !== 'tool-call' || part.toolName !== toolName || part.result !== undefined) { continue } @@ -539,8 +540,17 @@ export function restorePendingClarifyToolCall( payload: GatewayEventPayload, occurredAt = Date.now() / 1000 ): PendingClarifyProjection { - const clarifyPayload = { ...payload, name: 'clarify' } - const location = findPendingClarifyLocation(messages, clarifyPayload) + return restorePendingBlockingToolCall(messages, { ...payload, name: 'clarify' }, occurredAt) +} + +/** Restore a blocking tool row (clarify, connection card) from a resume snapshot: mark the + * existing pending part's message live, or append a row when the transcript has none. */ +export function restorePendingBlockingToolCall( + messages: ChatMessage[], + clarifyPayload: GatewayEventPayload & { name: string }, + occurredAt = Date.now() / 1000 +): PendingClarifyProjection { + const location = findPendingClarifyLocation(messages, clarifyPayload, clarifyPayload.name) if (location) { const message = messages[location.messageIndex] @@ -582,7 +592,7 @@ export function restorePendingClarifyToolCall( return { messages: next, streamId: tail.id } } - const streamId = nextLiveToolId('clarify-message') + const streamId = nextLiveToolId(`${clarifyPayload.name}-message`) return { messages: [ diff --git a/apps/desktop/src/lib/chat-messages/types.ts b/apps/desktop/src/lib/chat-messages/types.ts index 1cf7d325dd..169c4f7412 100644 --- a/apps/desktop/src/lib/chat-messages/types.ts +++ b/apps/desktop/src/lib/chat-messages/types.ts @@ -106,8 +106,10 @@ export type GatewayEventPayload = { // answers (qid → locked answer) rides along on reconnect replay only. questions?: unknown answers?: Record - // mcp.setup.request (setup_mcp tool — inline MCP consent card) - server?: string + // connection request (manage_connections MCP targets — inline approval card) + op_id?: string + deadline_at?: number + targets?: unknown action?: string reason?: string // approval.request (dangerous command / execute_code) — session-keyed diff --git a/apps/desktop/src/lib/connector-tools.ts b/apps/desktop/src/lib/connector-tools.ts index 7da1036497..97a20739e8 100644 --- a/apps/desktop/src/lib/connector-tools.ts +++ b/apps/desktop/src/lib/connector-tools.ts @@ -24,6 +24,35 @@ export function latestConnectorPart(messages: ChatMessage[]) { .at(-1) } +/** A `manage_connections` target of the local-MCP kind: `{name, mcp: true}`. */ +export interface McpTarget { + name: string + action: 'authorize' | 'enable' | 'install' +} + +const MCP_ACTIONS: readonly McpTarget['action'][] = ['install', 'enable', 'authorize'] + +/** MCP targets of a `manage_connections` call ([] for managed); read from args so live and + * settled rows classify alike. */ +export function mcpTargets(toolName: string, args: ToolCallMessagePart['result']): McpTarget[] { + if (toolName !== 'manage_connections') { + return [] + } + + const input = recordOf(args) + const action = MCP_ACTIONS.find(a => a === input.action) ?? 'install' + + if (!Array.isArray(input.connectors)) { + return [] + } + + return input.connectors.flatMap(entry => { + const name = isRecord(entry) && entry.mcp === true ? connectorText(entry.name)?.trim() : undefined + + return name ? [{ action, name: name.toLowerCase() }] : [] + }) +} + /** Connector names and statuses from the tool payload, for display only. No field here grants access. */ export interface ConnectorRow { connector: string diff --git a/apps/desktop/src/lib/gateway-events.ts b/apps/desktop/src/lib/gateway-events.ts index fb89644929..f06fef5d8c 100644 --- a/apps/desktop/src/lib/gateway-events.ts +++ b/apps/desktop/src/lib/gateway-events.ts @@ -27,7 +27,6 @@ export const UNSCOPED_STREAM_EVENT_TYPES = new Set([ 'browser.progress', 'clarify.request', 'error', - 'mcp.setup.request', 'message.complete', 'message.delta', 'message.interim', diff --git a/apps/desktop/src/lib/mcp-directory.ts b/apps/desktop/src/lib/mcp-directory.ts deleted file mode 100644 index 0426979abb..0000000000 --- a/apps/desktop/src/lib/mcp-directory.ts +++ /dev/null @@ -1,189 +0,0 @@ -/** - * COMPATIBILITY RUNG — superseded by the MCP catalog's `suggest` metadata. - * - * The Nous-approved install catalog (`optional-mcps//manifest.yaml`) - * is now the single source of truth for suggestible servers: each hosted - * remote entry declares its own `suggest.keywords` / `suggest.hosts`, served - * through `GET /api/mcp/catalog`. The suggestion provider and the inline - * setup card read the catalog first. - * - * This static list remains ONLY for older backends whose catalog responses - * carry no `suggest` field (the provider falls back to it when the catalog - * yields zero suggestible entries). Do not add new vendors here — add a - * manifest under `optional-mcps/` instead. Remove this file at the next - * backend contract bump. - * - * GitHub is intentionally absent (here AND in the catalog): its hosted MCP - * requires each MCP host to provide its own OAuth app (generic Dynamic - * Client Registration 404s at /register), and the bundled github/* skills - * via the gh CLI are the more capable integration. The composer's github - * suggestion provider offers the `github-auth` skill instead. - */ -export interface McpDirectoryEntry { - /** Server name as it will appear in mcp_servers config. */ - name: string - /** Lowercase whole-word/phrase triggers matched against the draft. */ - keywords: string[] - /** Hostname suffixes that trigger the suggestion when a pasted link points - * at the vendor ("yourco.atlassian.net" → atlassian). A pasted URL is the - * strongest intent signal there is — stronger than any keyword. */ - hosts?: string[] - /** Streamable-HTTP/SSE endpoint from the vendor's own docs. */ - url: string - /** Vendor documentation for the endpoint — shown on the card. */ - docs: string - description: string -} - -export const MCP_DIRECTORY: McpDirectoryEntry[] = [ - { - description: 'Jira issues and Confluence pages via Atlassian’s hosted MCP.', - docs: 'https://support.atlassian.com/rovo/docs/getting-started-with-the-atlassian-remote-mcp-server/', - hosts: ['atlassian.net', 'atlassian.com', 'jira.com'], - keywords: ['jira', 'confluence', 'atlassian', 'bitbucket'], - name: 'atlassian', - url: 'https://mcp.atlassian.com/v1/sse' - }, - { - description: 'Find, create, and update Linear issues and projects.', - docs: 'https://linear.app/docs/mcp', - hosts: ['linear.app'], - keywords: ['linear'], - name: 'linear', - url: 'https://mcp.linear.app/mcp' - }, - { - description: 'Design context and Code Connect from Figma files.', - docs: 'https://developers.figma.com/docs/figma-mcp-server/remote-server-installation/', - hosts: ['figma.com'], - keywords: ['figma', 'mockup', 'wireframe'], - name: 'figma', - url: 'https://mcp.figma.com/mcp' - }, - { - description: 'Issues, stack traces, and error context from Sentry.', - docs: 'https://docs.sentry.io/product/sentry-mcp/', - hosts: ['sentry.io'], - keywords: ['sentry', 'stack trace', 'crash report'], - name: 'sentry', - url: 'https://mcp.sentry.dev/mcp' - }, - { - description: 'Logs, monitors, dashboards, and incidents from Datadog.', - docs: 'https://docs.datadoghq.com/bits_ai/mcp_server/', - hosts: ['datadoghq.com', 'datadoghq.eu'], - keywords: ['datadog', 'apm'], - name: 'datadog', - url: 'https://mcp.datadoghq.com/api/unstable/mcp-server/mcp' - }, - // GitHub's hosted MCP is intentionally absent. Directory entries are wired - // through generic Dynamic Client Registration, but GitHub requires each MCP - // host to provide its own OAuth app (or use a PAT). Advertising it here makes - // both the composer pill and setup_mcp fail at /register with HTTP 404. - { - description: 'Pages and databases from your Notion workspace.', - docs: 'https://developers.notion.com/docs/mcp', - hosts: ['notion.so', 'notion.site'], - keywords: ['notion'], - name: 'notion', - url: 'https://mcp.notion.com/mcp' - }, - { - description: 'Payments, customers, and invoices via Stripe’s hosted MCP.', - docs: 'https://docs.stripe.com/mcp', - hosts: ['dashboard.stripe.com'], - keywords: ['stripe'], - name: 'stripe', - url: 'https://mcp.stripe.com' - }, - { - description: 'Deployments, logs, and projects via Vercel’s hosted MCP.', - docs: 'https://vercel.com/docs/mcp', - // No `vercel.app` on purpose (same rule as GitHub): pasted deploy-preview - // links are about the site being previewed, not about managing Vercel. - hosts: ['vercel.com'], - keywords: ['vercel'], - name: 'vercel', - url: 'https://mcp.vercel.com' - }, - { - description: 'Database, auth, and storage from your Supabase projects.', - docs: 'https://supabase.com/docs/guides/ai-tools/mcp', - hosts: ['supabase.com', 'supabase.co'], - keywords: ['supabase'], - name: 'supabase', - url: 'https://mcp.supabase.com/mcp' - }, - { - description: 'Sites, deploys, and env vars via Netlify’s hosted MCP.', - docs: 'https://docs.netlify.com/build/build-with-ai/agent-setup-guides/agent-setup-overview/', - // No `netlify.app` for the same deploy-preview reason as vercel.app. - hosts: ['netlify.com'], - keywords: ['netlify'], - name: 'netlify', - url: 'https://netlify-mcp.netlify.app/mcp' - }, - { - description: 'Models, datasets, Spaces, and papers from the Hugging Face Hub.', - docs: 'https://huggingface.co/docs/hub/agents-mcp', - hosts: ['huggingface.co', 'hf.co'], - keywords: ['hugging face', 'huggingface'], - // Underscored so prettyName renders "Hugging Face", not "Huggingface". - name: 'hugging_face', - url: 'https://huggingface.co/mcp' - }, - { - description: 'Tasks, projects, and goals from your Asana workspace.', - docs: 'https://developers.asana.com/docs/using-asanas-mcp-server', - hosts: ['asana.com'], - keywords: ['asana'], - name: 'asana', - url: 'https://mcp.asana.com/sse' - }, - { - description: 'Conversations, tickets, and customer data from Intercom.', - docs: 'https://developers.intercom.com/docs/guides/mcp', - hosts: ['intercom.com', 'intercom.io'], - keywords: ['intercom'], - name: 'intercom', - url: 'https://mcp.intercom.com/mcp' - }, - { - description: 'Bases, tables, and records from your Airtable workspace.', - docs: 'https://support.airtable.com/articles/9897799762-using-the-airtable-mcp-server', - hosts: ['airtable.com'], - keywords: ['airtable'], - name: 'airtable', - url: 'https://mcp.airtable.com/mcp' - }, - { - description: 'Sites, CMS collections, and pages via Webflow’s hosted MCP.', - docs: 'https://developers.webflow.com/mcp/reference/getting-started', - // No `webflow.io` — that's published staging sites, not Webflow intent. - hosts: ['webflow.com'], - keywords: ['webflow'], - name: 'webflow', - url: 'https://mcp.webflow.com/mcp' - }, - { - description: 'Payments, invoices, and subscriptions via PayPal’s hosted MCP.', - docs: 'https://developer.paypal.com/tools/mcp-server/', - hosts: ['developer.paypal.com'], - keywords: ['paypal'], - name: 'paypal', - url: 'https://mcp.paypal.com/sse' - }, - { - description: 'Catalog, orders, and payments via Square’s hosted MCP.', - docs: 'https://developer.squareup.com/docs/mcp', - hosts: ['squareup.com'], - // "square" the English word is everywhere ("square brackets", "square - // corners") — only the unambiguous brand form triggers. - keywords: ['squareup'], - name: 'square', - url: 'https://mcp.squareup.com/sse' - } -] - -export const directoryEntry = (name: string): McpDirectoryEntry | undefined => - MCP_DIRECTORY.find(entry => entry.name === name) diff --git a/apps/desktop/src/lib/render-weight.ts b/apps/desktop/src/lib/render-weight.ts index 9626c2e14a..32686d7b98 100644 --- a/apps/desktop/src/lib/render-weight.ts +++ b/apps/desktop/src/lib/render-weight.ts @@ -167,7 +167,7 @@ function partPaintWeight(part: unknown, measure: (parts: readonly unknown[]) => return 0 } - if (!isCardTool(toolName)) { + if (!isCardTool(toolName, part.args)) { return COLLAPSED_ROW_WEIGHT } diff --git a/apps/desktop/src/lib/tool-render-class.ts b/apps/desktop/src/lib/tool-render-class.ts index 6921ab894b..cf7b537471 100644 --- a/apps/desktop/src/lib/tool-render-class.ts +++ b/apps/desktop/src/lib/tool-render-class.ts @@ -8,6 +8,9 @@ * rather than inside either one. */ +import type { ToolCallMessagePart } from '@assistant-ui/react' + +import { mcpTargets } from '@/lib/connector-tools' import { isOnboardingEnabled } from '@/lib/onboarding-enabled' const FILE_EDIT_TOOL_NAMES = new Set(['edit_file', 'patch', 'write_file']) @@ -26,18 +29,22 @@ export function isFileEditTool(toolName: string): boolean { // - `clarify`, `image_generate` and `delegate_task` bypass ToolEntry to // render their own markup: a question the user has to answer, an image // they asked for, the several agents a fan-out is running. -// - `setup_mcp` and `manage_connections` are inline consent cards the user has to -// act on. Folding it into a "Using 2 tools" summary hides the buttons. +// - `manage_connections` is a consent card (MCP always, managed under the onboarding +// gate). Folding it into a "Using 2 tools" summary hides the buttons. // // Everything else is ephemeral activity — reads, searches, commands — which is // what a run summarizes and what the live ticker cycles through. -const CARD_TOOL_NAMES = new Set(['clarify', 'delegate_task', 'image_generate', 'setup_mcp']) +const CARD_TOOL_NAMES = new Set(['clarify', 'delegate_task', 'image_generate']) -export function isCardTool(toolName: string): boolean { +// Name the run splitter uses for a manage_connections part it has classified as a card. +export const CONNECTION_CARD_KEY = 'manage_connections:card' + +export function isCardTool(toolName: string, args?: ToolCallMessagePart['result']): boolean { return ( CARD_TOOL_NAMES.has(toolName) || + toolName === CONNECTION_CARD_KEY || isFileEditTool(toolName) || - (toolName === 'manage_connections' && isOnboardingEnabled()) + (toolName === 'manage_connections' && (isOnboardingEnabled() || mcpTargets(toolName, args).length > 0)) ) } diff --git a/apps/desktop/src/store/connection-request.test.ts b/apps/desktop/src/store/connection-request.test.ts new file mode 100644 index 0000000000..e0a2c48440 --- /dev/null +++ b/apps/desktop/src/store/connection-request.test.ts @@ -0,0 +1,119 @@ +import type { ConnectionRequestParams } from '@hermes/shared' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import { + $connectionRequests, + clearConnectionRequest, + type ConnectionRequest, + hasConnectionRequest, + normalizeConnectionRequest, + respondToConnectionRequest, + setConnectionRequest, + skipConnectionRequest +} from './connection-request' +import { rememberServerRequest, resetServerRequestsForTests } from './server-requests' + +const WIRE: ConnectionRequestParams = { + deadline_at: 1_800_000_000, + op_id: 'op-1', + reason: 'tickets', + session_id: 's1', + targets: [ + { action: 'install', kind: 'mcp', name: 'linear' }, + { action: 'install', kind: 'mcp', name: 'figma' } + ], + timeout_seconds: 120 +} + +function request(sessionId: string | null, requestId = 'req-1'): ConnectionRequest { + return normalizeConnectionRequest(WIRE, requestId, sessionId)! +} + +function remember(requestId: string) { + const respond = vi.fn() + rememberServerRequest({ fail: vi.fn(), id: requestId, method: 'connection', params: {}, respond }) + + return respond +} + +describe('connection-request store', () => { + beforeEach(() => { + $connectionRequests.set({}) + }) + + afterEach(() => { + $connectionRequests.set({}) + resetServerRequestsForTests() + }) + + it('normalizes the wire payload and keeps the server-owned deadline verbatim', () => { + const parsed = normalizeConnectionRequest(WIRE, 'req-1', 's1') + + expect(parsed?.deadlineAt).toBe(WIRE.deadline_at) + expect(parsed?.opId).toBe('op-1') + expect(parsed?.targets.map(t => t.name)).toEqual(['linear', 'figma']) + }) + + it('rejects a payload with no targets, no op id or no deadline', () => { + expect(normalizeConnectionRequest({ ...WIRE, targets: [] }, 'req-1', 's1')).toBeNull() + expect(normalizeConnectionRequest({ ...WIRE, op_id: '' }, 'req-1', 's1')).toBeNull() + expect(normalizeConnectionRequest({ ...WIRE, deadline_at: 0 }, 'req-1', 's1')).toBeNull() + expect(normalizeConnectionRequest(null, 'req-1', 's1')).toBeNull() + }) + + it('keeps requests from concurrent sessions independent', () => { + setConnectionRequest(request('a', 'req-a')) + setConnectionRequest(request('b', 'req-b')) + + expect(hasConnectionRequest('a')).toBe(true) + clearConnectionRequest('req-a', 'a') + expect(hasConnectionRequest('a')).toBe(false) + expect(hasConnectionRequest('b')).toBe(true) + }) + + it('a stale request id never clears a newer card', () => { + setConnectionRequest(request('a', 'req-new')) + clearConnectionRequest('req-old', 'a') + + expect($connectionRequests.get().a?.requestId).toBe('req-new') + }) + + it('respond clears the entry before the RPC and refuses a second answer', async () => { + const req = request('a') + const respond = remember(req.requestId) + setConnectionRequest(req) + + const first = await respondToConnectionRequest(req, { + settled_by: 'all_resolved', + targets: [{ name: 'linear', state: 'installed' }] + }) + + const second = await respondToConnectionRequest(req, { + settled_by: 'all_resolved', + targets: [{ name: 'linear', state: 'declined' }] + }) + + expect(first).toBe(true) + expect(second).toBe(false) + expect(respond).toHaveBeenCalledWith({ + settled_by: 'all_resolved', + targets: [{ name: 'linear', state: 'installed' }] + }) + }) + + it('skip declines every target of the pending operation', async () => { + const req = request('a') + const respond = remember(req.requestId) + setConnectionRequest(req) + + expect(await skipConnectionRequest('a')).toBe(true) + expect(await skipConnectionRequest('a')).toBe(false) + expect(respond).toHaveBeenCalledWith({ + settled_by: 'all_resolved', + targets: [ + { name: 'linear', state: 'declined' }, + { name: 'figma', state: 'declined' } + ] + }) + }) +}) diff --git a/apps/desktop/src/store/connection-request.ts b/apps/desktop/src/store/connection-request.ts new file mode 100644 index 0000000000..41855c91d5 --- /dev/null +++ b/apps/desktop/src/store/connection-request.ts @@ -0,0 +1,148 @@ +import type { + ConnectionAction, + ConnectionRequestParams, + ConnectionResult, + ConnectionRequestTarget as ConnectionTarget, + ConnectionTargetKind, + ConnectionTargetOutcomeState, + ConnectionTargetOutcome as GatewayConnectionTargetOutcome +} from '@hermes/shared' +import { atom, computed } from 'nanostores' + +import { respondToServerRequest } from './server-requests' + +/** Pending `connection` requests, keyed by runtime session id. The backend owns `opId`, + * targets and `deadlineAt`; the renderer never recomputes them. */ +export type { ConnectionAction, ConnectionTarget, ConnectionTargetKind } + +export interface ConnectionRequest { + requestId: string + opId: string + /** Unix seconds, server-owned. */ + deadlineAt: number + /** One sentence from the agent, shown on the card. */ + reason: string + targets: ConnectionTarget[] + /** Local receipt time (Unix seconds), used to reject stale resume cleanup. */ + receivedAt?: number + sessionId: string | null +} + +/** Generated target state for a connection operation result. */ +export type ConnectionTargetState = ConnectionTargetOutcomeState + +/** Generated target result for a connection operation. */ +export type ConnectionTargetOutcome = GatewayConnectionTargetOutcome + +/** Generated result sent through the server-request response rail. */ +export type ConnectionOutcome = ConnectionResult + +const keyFor = (sessionId: string | null | undefined): string => sessionId ?? '' + +export const $connectionRequests = atom>({}) + +/** One session's pending request (same shape as `sessionClarifyRequest`). */ +export const sessionConnectionRequest = (sessionId: string | null) => + computed($connectionRequests, requests => requests[keyFor(sessionId)] ?? null) + +/** Park the `connection` request's params (already validated against the generated contract by the + * backend). Null when it names no target: there is nothing for a card to show. */ +export function normalizeConnectionRequest( + params: ConnectionRequestParams | null | undefined, + requestId: string, + sessionId: string | null +): ConnectionRequest | null { + if (!params || !requestId || !params.op_id || params.deadline_at <= 0 || params.targets.length === 0) { + return null + } + + return { + deadlineAt: params.deadline_at, + opId: params.op_id, + reason: params.reason ?? '', + receivedAt: Date.now() / 1000, + requestId, + sessionId, + targets: params.targets.map(({ action, kind, name }) => ({ action, kind, name })) + } +} + +export function setConnectionRequest(request: ConnectionRequest): void { + $connectionRequests.set({ ...$connectionRequests.get(), [keyFor(request.sessionId)]: request }) +} + +export function clearConnectionRequest(requestId?: string, sessionId?: string | null): void { + const requests = $connectionRequests.get() + + if (sessionId !== undefined) { + const key = keyFor(sessionId) + const current = requests[key] + + if (!current || (requestId && current.requestId !== requestId)) { + return + } + + const next = { ...requests } + delete next[key] + $connectionRequests.set(next) + + return + } + + const next: Record = {} + let changed = false + + for (const [key, value] of Object.entries(requests)) { + if (requestId && value.requestId !== requestId) { + next[key] = value + } else { + changed = true + } + } + + if (changed) { + $connectionRequests.set(next) + } +} + +/** Non-reactive read for the composer's Enter handler. */ +export const hasConnectionRequest = (sessionId: string | null | undefined): boolean => + Boolean($connectionRequests.get()[keyFor(sessionId)]) + +/** Send the card's answer. Clears the entry first so the card cannot be answered twice; + * false when the request is already gone. */ +export async function respondToConnectionRequest(request: ConnectionRequest, outcome: ConnectionOutcome): Promise { + const current = $connectionRequests.get()[keyFor(request.sessionId)] + + if (!current || current.requestId !== request.requestId) { + return false + } + + clearConnectionRequest(request.requestId, request.sessionId) + + return respondToServerRequest(request.requestId, { + settled_by: outcome.settled_by, + targets: outcome.targets + }) +} + +/** Typing a message while the card is open declines every target, otherwise the typed + * message would wait behind the blocked tool until the deadline. */ +export async function skipConnectionRequest(sessionId: string | null | undefined): Promise { + const request = $connectionRequests.get()[keyFor(sessionId)] + + if (!request) { + return false + } + + try { + await respondToConnectionRequest(request, { + settled_by: 'all_resolved', + targets: request.targets.map(target => ({ name: target.name, state: 'declined' })) + }) + } catch { + // A failed skip must not block the message being sent; the tool settles on its deadline. + } + + return true +} diff --git a/apps/desktop/src/store/mcp-setup.ts b/apps/desktop/src/store/mcp-setup.ts deleted file mode 100644 index e8036e77f5..0000000000 --- a/apps/desktop/src/store/mcp-setup.ts +++ /dev/null @@ -1,112 +0,0 @@ -import { atom, computed } from 'nanostores' - -import { respondToServerRequest } from './server-requests' - -/** - * Pending `mcp.setup.request`s — the desktop half of the `setup_mcp` tool's - * blocking bridge (tools/setup_mcp_tool.py). Mirrors the clarify store: - * keyed by the runtime session id that raised the request so a background - * session can park its card while the user looks at another chat, and the - * inline McpSetupTool reads its own session's entry. - */ -export interface McpSetupRequest { - requestId: string - /** Catalog name (install) or mcp_servers config name (enable/authorize). */ - server: string - action: 'authorize' | 'enable' | 'install' - /** Agent-supplied one-liner: why this server helps right now. */ - reason: string - sessionId: string | null -} - -/** The card's answer, serialized back as the `mcp.setup` request's `{value}`. */ -export interface McpSetupOutcome { - status: 'authorized' | 'declined' | 'enabled' | 'error' | 'installed' - server: string - detail?: string - /** Tool names now available (OAuth flows report them). */ - tools?: string[] -} - -const keyFor = (sessionId: string | null | undefined): string => sessionId ?? '' - -export const $mcpSetupRequests = atom>({}) - -/** The setup request for one specific session — the transcript card reads - * this fixed-key view, same shape as `sessionClarifyRequest`. */ -export const sessionMcpSetupRequest = (sessionId: string | null) => - computed($mcpSetupRequests, requests => requests[keyFor(sessionId)] ?? null) - -export function setMcpSetupRequest(request: McpSetupRequest): void { - $mcpSetupRequests.set({ ...$mcpSetupRequests.get(), [keyFor(request.sessionId)]: request }) -} - -export function clearMcpSetupRequest(requestId?: string, sessionId?: string | null): void { - const requests = $mcpSetupRequests.get() - - if (sessionId !== undefined) { - const key = keyFor(sessionId) - const current = requests[key] - - if (!current || (requestId && current.requestId !== requestId)) { - return - } - - const next = { ...requests } - delete next[key] - $mcpSetupRequests.set(next) - - return - } - - const next: Record = {} - let changed = false - - for (const [key, value] of Object.entries(requests)) { - if (requestId && value.requestId !== requestId) { - next[key] = value - } else { - changed = true - } - } - - if (changed) { - $mcpSetupRequests.set(next) - } -} - -/** Whether `sessionId` has a setup card pending right now (imperative read — - * the composer checks this on Enter, not on every render). */ -export const hasMcpSetupRequest = (sessionId: string | null | undefined): boolean => - Boolean($mcpSetupRequests.get()[keyFor(sessionId)]) - -/** - * Answer `sessionId`'s pending setup card as declined and drop it locally, - * resolving to whether there was one to skip. - * - * The composer uses this when the user types a real message instead of acting - * on the card: setup_mcp blocks the agent inside its tool batch, so leaving - * the card unanswered would park the follow-up until the 10-minute timeout — - * the message looks sent and nothing happens. Typing IS the answer "not now": - * decline so the tool returns, then route the words normally. - * - * Mirrors skipClarifyRequest; mcp.setup.respond is allow_expired, so racing - * the timeout is harmless. - */ -export async function skipMcpSetupRequest(sessionId: string | null | undefined): Promise { - const request = $mcpSetupRequests.get()[keyFor(sessionId)] - - if (!request) { - return false - } - - // Clear first: the answer is already decided, and an in-flight RPC must not - // leave a live card the user can answer a second time. - clearMcpSetupRequest(request.requestId, request.sessionId) - - respondToServerRequest(request.requestId, { - value: JSON.stringify({ server: request.server, status: 'declined' }) - }) - - return true -} diff --git a/apps/desktop/src/store/suggestion-providers/mcp.test.ts b/apps/desktop/src/store/suggestion-providers/mcp.test.ts index f83a5b2c4b..4f5c8d641c 100644 --- a/apps/desktop/src/store/suggestion-providers/mcp.test.ts +++ b/apps/desktop/src/store/suggestion-providers/mcp.test.ts @@ -1,7 +1,5 @@ import { describe, expect, it } from 'vitest' -import { MCP_DIRECTORY } from '@/lib/mcp-directory' - import { matchSuggestions } from './mcp' const INDEX = [ @@ -95,9 +93,15 @@ describe('matchSuggestions', () => { ]) }) - it('does not offer GitHub through the generic OAuth registration path', () => { - const index = MCP_DIRECTORY.map(entry => ({ hosts: entry.hosts, keywords: entry.keywords, server: entry.name })) + it('does not offer GitHub: it is not in the catalog, so no index entry can match it', () => { + // GitHub is not in optional-mcps (its hosted MCP needs a per-host OAuth app), so a + // catalog-built index has no entry for it. + const catalogIndex = [ + { hosts: ['linear.app'], keywords: ['linear'], server: 'linear' }, + { hosts: ['figma.com'], keywords: ['figma'], server: 'figma' } + ] - expect(matchSuggestions('connect github', index)).toEqual([]) + expect(matchSuggestions('connect github please', catalogIndex)).toEqual([]) + expect(matchSuggestions('connect github please', [])).toEqual([]) }) }) diff --git a/apps/desktop/src/store/suggestion-providers/mcp.ts b/apps/desktop/src/store/suggestion-providers/mcp.ts index fbefc861db..f18c979bdc 100644 --- a/apps/desktop/src/store/suggestion-providers/mcp.ts +++ b/apps/desktop/src/store/suggestion-providers/mcp.ts @@ -2,7 +2,6 @@ import { capabilityScoped } from '@/api/client' import { addMcpServer, getMcpCatalog, listMcpServers, removeMcpServer } from '@/hermes' import { translateNow } from '@/i18n' import { completeMcpDesktopOAuth, McpOAuthCancelled } from '@/lib/mcp-dashboard-oauth' -import { MCP_DIRECTORY } from '@/lib/mcp-directory' import { prettyName } from '@/lib/text' import { type ComposerSuggestion, registerDraftProvider } from '@/store/composer-suggestions' import { $gateway } from '@/store/gateway' @@ -15,9 +14,7 @@ import { notifyError } from '@/store/notifications' * metadata (`GET /api/mcp/catalog` — the same reviewed manifests behind * `hermes mcp catalog`), by whole-word keyword and pasted-link host suffix, * excluding servers already configured. The catalog is the single source of - * truth for suggestible servers; the renderer-local `lib/mcp-directory.ts` - * remains only as a compatibility rung for older backends whose catalog - * entries carry no `suggest` field. A suggestion's invoke runs the whole + * truth for suggestible servers. A suggestion's invoke runs the whole * connect: validated config write → browser OAuth → live tool reload, with * rollback on cancel/failure so a decline never strands a half-configured * server. @@ -40,8 +37,7 @@ interface SuggestibleServer { url: string } -// Suggestible servers from the catalog (entries with `suggest` + an http -// url), or the static directory on backends that predate `suggest`. +// Suggestible servers from the catalog (entries with `suggest` + an http url). let suggestible: SuggestibleServer[] | null = null let suggestibleAt = 0 @@ -84,18 +80,7 @@ async function loadSuggestible(): Promise { url: entry.url! })) - // Compatibility rung: an older backend serves the catalog without any - // `suggest` metadata. Fall back to the static directory rather than - // silently losing the feature (remove once the backend contract bumps). - suggestible = - fromCatalog.length > 0 - ? fromCatalog - : MCP_DIRECTORY.map(entry => ({ - hosts: entry.hosts, - keywords: entry.keywords, - server: entry.name, - url: entry.url - })) + suggestible = fromCatalog suggestibleAt = Date.now() return suggestible diff --git a/apps/shared/src/gateway-contract.generated.ts b/apps/shared/src/gateway-contract.generated.ts index 84e78e4952..1e2e5ce217 100644 --- a/apps/shared/src/gateway-contract.generated.ts +++ b/apps/shared/src/gateway-contract.generated.ts @@ -3650,7 +3650,7 @@ export interface ApprovalResult { export interface EmptyRequestParams { session_id: string } -/** The answer to any one-string prompt (sudo, secret, vault prompts, desktop bridges, mcp.setup): ``''`` means skipped / declined. */ +/** The answer to any one-string prompt (sudo, secret, vault prompts, desktop bridges): ``''`` means skipped / declined. */ export interface ValueResult { value: string } @@ -3675,12 +3675,37 @@ export interface VaultCodeRequestParams { site?: string | null hint?: string | null } -export interface McpSetupRequestParams { +/** ``tools/connections_tool_operation.py::ConnectionOperation.request_payload`` — one operation, N targets, a server-owned deadline (epoch seconds) the restored card keeps. */ +export interface ConnectionRequestParams { session_id: string - server?: string | null - action?: string | null - reason?: string | null + op_id: string + deadline_at: number + timeout_seconds: number + reason?: string + targets: ConnectionRequestTarget[] } +/** One row of the card: ``tools/connections_tool_operation.py::ConnectionOperation.request_payload``. */ +export interface ConnectionRequestTarget { + name: string + kind: ConnectionTargetKind + action: ConnectionAction +} +export type ConnectionTargetKind = 'connector' | 'mcp' +export type ConnectionAction = 'authorize' | 'enable' | 'install' +/** Per-target outcomes. The backend derives the settle reason from target state; the renderer's ``settled_by`` is its own claim and is not trusted. */ +export interface ConnectionResult { + settled_by: ConnectionSettledBy + targets: ConnectionTargetOutcome[] +} +export type ConnectionSettledBy = 'all_resolved' | 'continue' +/** The card's answer for one target (``tools/connections_tool_mcp.py::_apply_answer``). */ +export interface ConnectionTargetOutcome { + name: string + state: ConnectionTargetOutcomeState + detail?: string | null + tools?: string[] | null +} +export type ConnectionTargetOutcomeState = 'installed' | 'enabled' | 'authorized' | 'declined' | 'error' export interface ReadRangeRequestParams { session_id: string start?: number | null @@ -4736,8 +4761,8 @@ export interface ServerRequestMap { approval: { params: ApprovalRequestParams; result: ApprovalResult } /** The clarify tool: ask the user one question or a batch. */ clarify: { params: ClarifyRequestParams; result: ClarifyResult } - /** Consent card for installing / enabling / authorising an MCP server. */ - 'mcp.setup': { params: McpSetupRequestParams; result: ValueResult } + /** The manage_connections approval card: install / enable / authorise local MCP servers. */ + connection: { params: ConnectionRequestParams; result: ConnectionResult } /** Click / type / scroll / annotate inside the in-app browser preview. */ 'preview.act': { params: PreviewActRequestParams; result: ValueResult } /** Read the in-app browser preview's text (JSON text answer). */ @@ -4763,7 +4788,7 @@ export type ServerRequestMethod = keyof ServerRequestMap export const SERVER_REQUEST_METHODS = [ 'approval', 'clarify', - 'mcp.setup', + 'connection', 'preview.act', 'preview.read', 'secret', diff --git a/apps/shared/src/gateway-contract.openrpc.json b/apps/shared/src/gateway-contract.openrpc.json index a6af1cf595..fcaff6c83a 100644 --- a/apps/shared/src/gateway-contract.openrpc.json +++ b/apps/shared/src/gateway-contract.openrpc.json @@ -8196,6 +8196,176 @@ "title": "ConfigShowResult", "type": "object" }, + "ConnectionAction": { + "enum": [ + "authorize", + "enable", + "install" + ], + "title": "ConnectionAction", + "type": "string" + }, + "ConnectionRequestParams": { + "additionalProperties": false, + "description": "``tools/connections_tool_operation.py::ConnectionOperation.request_payload`` \u2014 one operation,\nN targets, a server-owned deadline (epoch seconds) the restored card keeps.", + "properties": { + "session_id": { + "title": "Session Id", + "type": "string" + }, + "op_id": { + "title": "Op Id", + "type": "string" + }, + "deadline_at": { + "title": "Deadline At", + "type": "number" + }, + "timeout_seconds": { + "title": "Timeout Seconds", + "type": "number" + }, + "reason": { + "default": "", + "title": "Reason", + "type": "string" + }, + "targets": { + "items": { + "$ref": "#/components/schemas/ConnectionRequestTarget" + }, + "title": "Targets", + "type": "array" + } + }, + "required": [ + "session_id", + "op_id", + "deadline_at", + "timeout_seconds", + "targets" + ], + "title": "ConnectionRequestParams", + "type": "object" + }, + "ConnectionRequestTarget": { + "additionalProperties": false, + "description": "One row of the card: ``tools/connections_tool_operation.py::ConnectionOperation.request_payload``.", + "properties": { + "name": { + "title": "Name", + "type": "string" + }, + "kind": { + "$ref": "#/components/schemas/ConnectionTargetKind" + }, + "action": { + "$ref": "#/components/schemas/ConnectionAction" + } + }, + "required": [ + "name", + "kind", + "action" + ], + "title": "ConnectionRequestTarget", + "type": "object" + }, + "ConnectionResult": { + "additionalProperties": false, + "description": "Per-target outcomes. The backend derives the settle reason from target state; the renderer's\n``settled_by`` is its own claim and is not trusted.", + "properties": { + "settled_by": { + "$ref": "#/components/schemas/ConnectionSettledBy" + }, + "targets": { + "items": { + "$ref": "#/components/schemas/ConnectionTargetOutcome" + }, + "title": "Targets", + "type": "array" + } + }, + "required": [ + "settled_by", + "targets" + ], + "title": "ConnectionResult", + "type": "object" + }, + "ConnectionSettledBy": { + "enum": [ + "all_resolved", + "continue" + ], + "title": "ConnectionSettledBy", + "type": "string" + }, + "ConnectionTargetKind": { + "enum": [ + "connector", + "mcp" + ], + "title": "ConnectionTargetKind", + "type": "string" + }, + "ConnectionTargetOutcome": { + "additionalProperties": false, + "description": "The card's answer for one target (``tools/connections_tool_mcp.py::_apply_answer``).", + "properties": { + "name": { + "title": "Name", + "type": "string" + }, + "state": { + "$ref": "#/components/schemas/ConnectionTargetOutcomeState" + }, + "detail": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "default": null, + "title": "Detail" + }, + "tools": { + "anyOf": [ + { + "items": { + "type": "string" + }, + "type": "array" + }, + { + "type": "null" + } + ], + "default": null, + "title": "Tools" + } + }, + "required": [ + "name", + "state" + ], + "title": "ConnectionTargetOutcome", + "type": "object" + }, + "ConnectionTargetOutcomeState": { + "enum": [ + "installed", + "enabled", + "authorized", + "declined", + "error" + ], + "title": "ConnectionTargetOutcomeState", + "type": "string" + }, "ConnectorConnectEntry": { "additionalProperties": true, "description": "``tools/connections_tool.py`` per-connector authorization outcome.", @@ -14065,56 +14235,6 @@ "title": "McpServersTestResult", "type": "object" }, - "McpSetupRequestParams": { - "additionalProperties": false, - "properties": { - "session_id": { - "title": "Session Id", - "type": "string" - }, - "server": { - "anyOf": [ - { - "type": "string" - }, - { - "type": "null" - } - ], - "default": null, - "title": "Server" - }, - "action": { - "anyOf": [ - { - "type": "string" - }, - { - "type": "null" - } - ], - "default": null, - "title": "Action" - }, - "reason": { - "anyOf": [ - { - "type": "string" - }, - { - "type": "null" - } - ], - "default": null, - "title": "Reason" - } - }, - "required": [ - "session_id" - ], - "title": "McpSetupRequestParams", - "type": "object" - }, "MessageCompletePayload": { "additionalProperties": false, "description": "``prompt_turn._complete_turn_payload`` / ``session_auto_continue._emit_terminal_turn_error`` /\n``agent_callbacks._mirror_subagent_to_child`` (child watch mirror: ``text`` only) /\n``compute_host_bridge`` (``text`` + ``status``).", @@ -30925,7 +31045,7 @@ }, "ValueResult": { "additionalProperties": false, - "description": "The answer to any one-string prompt (sudo, secret, vault prompts, desktop bridges,\nmcp.setup): ``''`` means skipped / declined.", + "description": "The answer to any one-string prompt (sudo, secret, vault prompts, desktop bridges):\n``''`` means skipped / declined.", "properties": { "value": { "title": "Value", @@ -32856,20 +32976,20 @@ } }, { - "name": "mcp.setup", - "summary": "Consent card for installing / enabling / authorising an MCP server.", + "name": "connection", + "summary": "The manage_connections approval card: install / enable / authorise local MCP servers.", "params": [ { "name": "params", "schema": { - "$ref": "#/components/schemas/McpSetupRequestParams" + "$ref": "#/components/schemas/ConnectionRequestParams" } } ], "result": { "name": "result", "schema": { - "$ref": "#/components/schemas/ValueResult" + "$ref": "#/components/schemas/ConnectionResult" } } }, diff --git a/cli-config.yaml.example b/cli-config.yaml.example index 82e3bf7a10..c851bb1a94 100644 --- a/cli-config.yaml.example +++ b/cli-config.yaml.example @@ -1639,6 +1639,13 @@ code_execution: # kernel_idle_timeout: 1800 # Reap kernels idle longer than this (seconds) # max_session_kernels: 4 # Process-wide LRU cap on live kernels +# ============================================================================= +# Connections (manage_connections tool) +# ============================================================================= +# Deadline for one manage_connections call; fixed at creation, floor 5s, no ceiling. +connections: + wait_timeout_seconds: 120 + # ============================================================================= # Subagent Delegation # ============================================================================= diff --git a/evals/core_tool_deferral/tasks.py b/evals/core_tool_deferral/tasks.py index 990466a080..24b63e57c5 100644 --- a/evals/core_tool_deferral/tasks.py +++ b/evals/core_tool_deferral/tasks.py @@ -1,9 +1,9 @@ """Task battery for PR #97979 core-tool-deferral A/B. -Covers all 19 deferred tools: +Covers all 18 deferred tools: computer_use, session_search, clarify, image_generate, todo_list, process_manage, cronjob_manage, drive_preview, gui_tour, desktop_preview, - annotate_preview, show_tip, setup_mcp, desktop_project, close_terminal, + annotate_preview, show_tip, desktop_project, close_terminal, apply_layout, read_terminal, read_window_below, focus_pane plus an eager-surface control and a false-discovery distractor. @@ -300,16 +300,17 @@ def g_project(ctx): notes.append("desktop_project called but not with 'apollo'") else: notes.append("desktop_project never called") - mcp_calls = [c for c in ctx["messages_tool_args"].get("setup_mcp", []) - if "github" in json.dumps(c).lower()] - if _called(ctx, "setup_mcp"): + # MCP install is a manage_connections call with an mcp:true target. + mcp_calls = [c for c in ctx["messages_tool_args"].get("manage_connections", []) + if "github" in json.dumps(c).lower() and "mcp" in json.dumps(c).lower()] + if _called(ctx, "manage_connections"): score += 0.3 if mcp_calls: score += 0.2 else: - notes.append("setup_mcp called but not for github") + notes.append("manage_connections called but not for the github MCP") else: - notes.append("setup_mcp never called") + notes.append("manage_connections never called") return score, notes diff --git a/evals/core_tool_deferral/worker.py b/evals/core_tool_deferral/worker.py index 38d945fc72..b94d23d0e3 100644 --- a/evals/core_tool_deferral/worker.py +++ b/evals/core_tool_deferral/worker.py @@ -165,9 +165,11 @@ def read_window_below_cb(**kw): CALLBACK_LOG.append({"name": "read_window_below", "kw": kw}) return json.dumps({"title": "Invoices — draft", "text": WINDOW_BELOW}) -def setup_mcp_cb(name, action, reason): - CALLBACK_LOG.append({"name": "setup_mcp", "server": name, "action": action}) - return json.dumps({"success": True, "server": name, "status": "installed"}) +def connection_cb(payload): + # Answer every manage_connections MCP target as installed. + CALLBACK_LOG.append({"name": "manage_connections", "targets": payload.get("targets", [])}) + return json.dumps({"settled_by": "all_resolved", "targets": [ + {"name": t["name"], "status": "installed"} for t in payload.get("targets", [])]}) # --- import the tree's model_tools + patch registry stubs ------------------ import model_tools # noqa: E402 (triggers registrations + plugin discovery) @@ -226,7 +228,7 @@ agent = AIAgent( read_preview_callback=read_preview_cb, drive_preview_callback=drive_preview_cb, read_window_below_callback=read_window_below_cb, - setup_mcp_callback=setup_mcp_cb, + connection_callback=connection_cb, ) PREAMBLE = ("You are running inside the Hermes desktop app on the user's machine. " diff --git a/hermes_cli/config_defaults.py b/hermes_cli/config_defaults.py index a28d278996..c20dd2454a 100644 --- a/hermes_cli/config_defaults.py +++ b/hermes_cli/config_defaults.py @@ -1849,6 +1849,10 @@ DEFAULT_CONFIG = { # the portal sign-in every managed tool gates on. "connectors": {"enabled": True}, }, + # manage_connections operation deadline; fixed at creation, floor 5s, no ceiling. + "connections": { + "wait_timeout_seconds": 120, + }, "logging": { # File logging to ~/.hermes/logs/: agent.log captures INFO+, errors.log WARNING+. "level": "INFO", # minimum level for agent.log: DEBUG, INFO, WARNING "max_size_mb": 5, # max size per log file before rotation @@ -2415,7 +2419,7 @@ DEFAULT_CONFIG = { # Extra ports detection probes for an external llama-server (besides 8080). "detect_ports": [], }, - "_config_version": 44, # Config schema version - bump this when adding new required fields + "_config_version": 45, # Config schema version - bump this when adding new required fields } diff --git a/hermes_cli/config_migrations.py b/hermes_cli/config_migrations.py index 9b102c758b..7d14ea3fed 100644 --- a/hermes_cli/config_migrations.py +++ b/hermes_cli/config_migrations.py @@ -542,6 +542,53 @@ def _migrate_to_41(results: Dict[str, Any], quiet: bool) -> None: f"({', '.join(cleaned)}) — Bot Chat sessions now get the live roster instead.") +def _migrate_to_45(results: Dict[str, Any], quiet: bool) -> None: + # 44 → 45: append `connections` to every saved `platform_toolsets` list that predates it + # (an explicit list treats absence as unchecked). Skipped when `known_builtin_toolsets` + # already records `connections` (a decline) or `agent.disabled_toolsets` names it (the + # resolver subtracts that list last, so the append would have no effect). + from agent.skill_utils import parse_config_string_list + from hermes_cli.tools_config import _configurable_keys, _get_plugin_toolset_keys + from hermes_cli.toolset_scope import toolset_allowed_for_platform + + config = read_raw_config() + saved = config.get("platform_toolsets") + if not isinstance(saved, dict): + return + if "connections" in parse_config_string_list(_dict_at(config, "agent").get("disabled_toolsets")): + return + known = _dict_at(config, "known_builtin_toolsets") + # Same predicate the resolver uses to pick its explicit branch: any configurable or plugin key. + explicit_keys = _configurable_keys() | _get_plugin_toolset_keys() + enabled_for: List[str] = [] + for platform, toolsets in saved.items(): + if not isinstance(toolsets, list) or "connections" in toolsets: + continue + if not toolset_allowed_for_platform("connections", platform): + continue + # A composite like [hermes-cli] already inherits every core tool at read time. + if not any(str(ts) in explicit_keys for ts in toolsets): + continue + offered = known.get(platform) + if isinstance(offered, list) and "connections" in offered: + continue + saved[platform] = sorted({*map(str, toolsets), "connections"}) + if isinstance(offered, list): + known[platform] = sorted({*map(str, offered), "connections"}) + enabled_for.append(str(platform)) + if not enabled_for: + return + config["platform_toolsets"] = saved + if known: + config["known_builtin_toolsets"] = known + platforms = ", ".join(sorted(enabled_for)) + _commit( + config, results, quiet, + f"enabled the connections toolset for {platforms}", + f" ✓ Enabled the Connections toolset (Gmail, Linear, Notion, local MCP servers) for {platforms}. " + "Uncheck Connections in `hermes tools` to turn it off.") + + #: Registry of (target_version, step), strictly ascending; simple default-flip steps are #: declared inline via _rewrite_stale_default / _rewrite_key partials. Later steps observe #: earlier steps' writes via read_raw_config() (filesystem state). v12 is the support floor: @@ -660,6 +707,8 @@ MIGRATIONS: Tuple[Tuple[int, Callable[[Dict[str, Any], bool], None]], ...] = ( message=( " ✓ curator.archive_after_days 90→30 — skills unused for a month are archived to " "skills/.archive/ (recoverable with `hermes curator restore`). Set it back to 90 to keep the old window."))), + # 44 → 45: saved platform_toolsets lists predate the connections toolset (see _migrate_to_45). + (45, _migrate_to_45), ) diff --git a/hermes_cli/web_server_config.py b/hermes_cli/web_server_config.py index 8f2207f236..fcdd0f900c 100644 --- a/hermes_cli/web_server_config.py +++ b/hermes_cli/web_server_config.py @@ -196,6 +196,7 @@ _CATEGORY_MERGE: Dict[str, str] = { "runtime": "agent", "session": "general", "nous": "agent", + "connections": "agent", } diff --git a/run_agent.py b/run_agent.py index 5744825cfa..97328216a9 100644 --- a/run_agent.py +++ b/run_agent.py @@ -250,7 +250,7 @@ class AIAgent( reasoning_callback: callable = None, clarify_callback: callable = None, read_terminal_callback: callable = None, read_preview_callback: callable = None, drive_preview_callback: callable = None, read_window_below_callback: callable = None, - setup_mcp_callback: callable = None, tour_callback: callable = None, step_callback: callable = None, + connection_callback: callable = None, tour_callback: callable = None, step_callback: callable = None, stream_delta_callback: callable = None, interim_assistant_callback: callable = None, tool_gen_callback: callable = None, status_callback: callable = None, notice_callback: callable = None, notice_clear_callback: callable = None, diff --git a/tests/agent/test_run_agent.py b/tests/agent/test_run_agent.py index df512c9c20..f2252cd39c 100644 --- a/tests/agent/test_run_agent.py +++ b/tests/agent/test_run_agent.py @@ -2489,6 +2489,7 @@ class TestAgentRuntimePostHookOwnershipSync: ("drive_preview", {"action": "elements"}), ("annotate_preview", {"action": "clear"}), ("read_window_below", {}), + ("manage_connections", {"action": "install", "connectors": [{"name": "linear", "mcp": True}]}), ("setup_mcp", {"server": "linear", "action": "install"}), ("gui_tour", {"action": "stop"}), ("delegate_task", {"goal": "Check the child path"}), @@ -2546,6 +2547,10 @@ 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. + monkeypatch.setattr("tools.connections_tool_mcp._catalog_names", lambda: ["linear"]) + monkeypatch.setattr("tools.connections_tool_mcp._configured_names", lambda: []) monkeypatch.setattr(agent, "_get_session_db_for_recall", lambda: None) monkeypatch.setattr( agent, diff --git a/tests/agent/test_sequential_deadline_delegate_exempt.py b/tests/agent/test_sequential_deadline_delegate_exempt.py index 3b881c3904..a38ae1cad0 100644 --- a/tests/agent/test_sequential_deadline_delegate_exempt.py +++ b/tests/agent/test_sequential_deadline_delegate_exempt.py @@ -8,6 +8,12 @@ def test_delegate_task_is_exempt_from_the_sequential_deadline(): assert "delegate_task" in te._SEQUENTIAL_DEADLINE_EXEMPT_TOOLS +def test_manage_connections_owns_its_bounded_wait(): + # The connection operation's deadline is server-owned (connections.wait_timeout_seconds); + # the generic guard would report tool_timeout while the approval card is still live. + assert "manage_connections" in te._SEQUENTIAL_DEADLINE_EXEMPT_TOOLS + + def test_exemption_is_narrow(): assert "terminal" not in te._SEQUENTIAL_DEADLINE_EXEMPT_TOOLS assert "execute_code" not in te._SEQUENTIAL_DEADLINE_EXEMPT_TOOLS diff --git a/tests/hermes_cli/test_config_migration_45_connections.py b/tests/hermes_cli/test_config_migration_45_connections.py new file mode 100644 index 0000000000..273e11edda --- /dev/null +++ b/tests/hermes_cli/test_config_migration_45_connections.py @@ -0,0 +1,160 @@ +"""Migration 44→45: saved ``platform_toolsets`` lists gain the ``connections`` toolset. + +``hermes tools`` persists an explicit per-platform toolset list, and absence from +that list reads as "unchecked" — so a toolset that ships after the list was saved +stays off for picker users while composite (``[hermes-cli]``) users inherit it. +The 44→45 step turns ``connections`` on for stale lists, preserves an explicit +decline, and leaves composites/empty lists alone. +""" + +import os +from unittest.mock import patch + +import pytest +import yaml + + +class TestConnectionsToolsetMigration: + """Behaviour contract for ``_migrate_to_45`` driven through ``run_migrations``.""" + + @staticmethod + def _run_ladder(tmp_path, current_ver=44): + from hermes_cli.config_migrations import run_migrations + + results = {"env_added": [], "config_added": [], "warnings": []} + with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}): + run_migrations(current_ver, results, quiet=True) + return results + + @staticmethod + def _write_config(tmp_path, config): + (tmp_path / "config.yaml").write_text( + yaml.safe_dump(config), encoding="utf-8" + ) + + @staticmethod + def _read_config(tmp_path): + return yaml.safe_load((tmp_path / "config.yaml").read_text(encoding="utf-8")) + + def test_stale_list_gains_connections_for_every_platform(self, tmp_path): + """A list saved before the toolset shipped is offered it (and records the offer).""" + platforms = { + "cli": ["file", "terminal", "web"], + "telegram": ["file", "web"], + } + self._write_config( + tmp_path, + { + "_config_version": 44, + "platform_toolsets": platforms, + "known_builtin_toolsets": { + "cli": ["browser", "file", "memory", "skills", "terminal", "todo", "web"] + }, + }, + ) + + results = self._run_ladder(tmp_path) + raw = self._read_config(tmp_path) + + assert "connections" in raw["platform_toolsets"]["cli"] + assert "connections" in raw["platform_toolsets"]["telegram"] + # The offer is recorded so a later uncheck is a recorded decline, not another migration. + assert "connections" in raw["known_builtin_toolsets"]["cli"] + added = [entry for entry in results["config_added"] if "connections" in entry.lower()] + assert len(added) == 1, results["config_added"] + + @pytest.mark.parametrize( + "platform_toolsets, known", + [ + pytest.param( # (a) the user saw the checkbox and left it off + {"cli": ["file", "terminal", "web"]}, + {"cli": ["browser", "connections", "file", "web"]}, + id="declined", + ), + pytest.param( # (b) composite list already inherits every core tool + {"cli": ["hermes-cli"]}, + {}, + id="composite", + ), + pytest.param( # (c) empty picker selection — no configurable key to extend + {"cli": []}, + {}, + id="empty", + ), + ], + ) + def test_declines_composites_and_empty_lists_are_untouched( + self, tmp_path, platform_toolsets, known + ): + self._write_config( + tmp_path, + { + "_config_version": 44, + "platform_toolsets": platform_toolsets, + "known_builtin_toolsets": known, + }, + ) + + results = self._run_ladder(tmp_path) + raw = self._read_config(tmp_path) + + assert raw["platform_toolsets"] == platform_toolsets + assert results["config_added"] == [] + + @pytest.mark.parametrize("disabled", [["browser", "connections", "web"], '["connections"]'], ids=["list", "json-string"]) + def test_global_disable_is_not_overridden_or_claimed(self, tmp_path, disabled): + """Blank Slate and `hermes tools --disable` write agent.disabled_toolsets, which the resolver + subtracts last; appending to the platform list would print an enable that never takes effect.""" + platform_toolsets = {"cli": ["file", "skills", "terminal", "vision"]} + self._write_config( + tmp_path, + { + "_config_version": 44, + "platform_toolsets": platform_toolsets, + "agent": {"disabled_toolsets": disabled}, + }, + ) + + results = self._run_ladder(tmp_path) + raw = self._read_config(tmp_path) + + assert raw["platform_toolsets"] == platform_toolsets + assert raw["agent"]["disabled_toolsets"] == disabled + assert results["config_added"] == [] + + def test_rerun_adds_nothing_and_keeps_one_connections(self, tmp_path): + """Running the step twice is a no-op; the toolset appears exactly once.""" + self._write_config( + tmp_path, + { + "_config_version": 44, + "platform_toolsets": {"cli": ["file", "terminal", "web"]}, + "known_builtin_toolsets": {"cli": ["file", "terminal", "web"]}, + }, + ) + + self._run_ladder(tmp_path) + after_first = self._read_config(tmp_path)["platform_toolsets"]["cli"] + second = self._run_ladder(tmp_path) + after_second = self._read_config(tmp_path)["platform_toolsets"]["cli"] + + # Running the step again rewrites nothing and never appends a duplicate. + assert second["config_added"] == [] + assert after_second == after_first + assert after_second.count("connections") <= 1 + + def test_full_migration_stamps_the_current_version(self, tmp_path): + """A pre-45 config that takes this step ends at DEFAULT_CONFIG's version, never one short.""" + from hermes_cli.config import migrate_config + from hermes_cli.config_defaults import DEFAULT_CONFIG + + self._write_config( + tmp_path, + {"_config_version": 42, "platform_toolsets": {"cli": ["file", "terminal"]}}, + ) + with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}): + migrate_config(interactive=False, quiet=True) + raw = self._read_config(tmp_path) + + assert "connections" in raw["platform_toolsets"]["cli"] + assert raw["_config_version"] == DEFAULT_CONFIG["_config_version"] diff --git a/tests/tools/test_connections_tool.py b/tests/tools/test_connections_tool.py index f7da7cc8a4..68b246e707 100644 --- a/tests/tools/test_connections_tool.py +++ b/tests/tools/test_connections_tool.py @@ -132,15 +132,22 @@ def test_gateway_failure_is_a_model_actionable_error(): assert "connector gateway request failed" in out["error"] -def test_mcp_actions_are_not_this_tools_business(): - # Local MCP setup belongs to setup_mcp, which owns the desktop consent - # callback. Folding those actions in here promised a flow this tool has no - # way to reach, so they are rejected as unknown actions. +def test_mcp_actions_belong_to_mcp_targets_only(): + # The MCP verbs are in the enum for mcp:true targets only. The callback that an earlier + # fold could not reach through registry.dispatch now arrives via the inline executor. + enum = MANAGE_CONNECTIONS_SCHEMA["parameters"]["properties"]["action"]["enum"] + assert {"install", "enable", "authorize"} <= set(enum) + + out = json.loads(manage_connections({"action": "install", "connectors": ["linear"]})) + assert "mcp" in out["error"] and "install" in out["error"] + out = json.loads( - manage_connections({"action": "install", "server": "linear"}) + manage_connections({"action": "connect", "connectors": [{"name": "linear", "mcp": True}]}) ) + assert "managed-connector action" in out["error"] + + out = json.loads(manage_connections({"action": "uninstall", "connectors": ["gmail"]})) assert "action must be one of" in out["error"] - assert "install" not in MANAGE_CONNECTIONS_SCHEMA["parameters"]["properties"]["action"]["enum"] # --------------------------------------------------------------------------- @@ -491,9 +498,11 @@ def test_focus_mode_coding_posture_gets_the_tool(monkeypatch): assert "manage_connections" in _session_tool_names(selection, connectors=True) -def test_signed_out_session_sees_nothing(tmp_path, monkeypatch): - """check_fn is the only entitlement gate, on every surface.""" +def test_signed_out_session_keeps_the_tool_but_the_managed_leg_refuses(tmp_path, monkeypatch): + """The portal gate moved from check_fn into the managed leg: local MCP approvals need no + sign-in, so the schema stays; a managed action in a signed-out session is a plain error.""" from hermes_cli.tools_config import _get_platform_tools + from tools.registry import registry from tui_gateway.server import _load_enabled_toolsets monkeypatch.chdir(tmp_path) @@ -504,9 +513,11 @@ def test_signed_out_session_sees_nothing(tmp_path, monkeypatch): ["coding"], ] for selection in selections: - assert "manage_connections" not in _session_tool_names( - selection, connectors=False - ), selection + assert "manage_connections" in _session_tool_names(selection, connectors=False), selection + + with patch("tools.connections_tool._connectors_available", return_value=False): + out = json.loads(registry.dispatch("manage_connections", {"action": "status"})) + assert "not available in this session" in out["error"] def test_operator_can_still_turn_it_off(tmp_path, monkeypatch): diff --git a/tests/tools/test_connections_tool_mcp.py b/tests/tools/test_connections_tool_mcp.py new file mode 100644 index 0000000000..fd756f7966 --- /dev/null +++ b/tests/tools/test_connections_tool_mcp.py @@ -0,0 +1,238 @@ +"""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 +- catalog validation: install is catalog-only, enable/authorize need a configured server +- the replay shim keeps an old ``setup_mcp`` call dispatching +- deadline ownership: config key + sequential-deadline exemption +""" + +import json +from types import SimpleNamespace +from unittest.mock import patch + +import pytest + +import tools.connections_tool # registers the tool +from tools import connections_tool_operation as op +from tools.connections_tool import MANAGE_CONNECTIONS_SCHEMA, manage_connections +from tools.registry import registry + +CATALOG = ["figma", "linear", "notion"] +CONFIGURED = {"paper": {"command": "paper-mcp"}, "linear": {"url": "https://mcp.linear.app/mcp"}} + + +@pytest.fixture(autouse=True) +def _catalog(): + with patch("tools.connections_tool_mcp._catalog_names", return_value=CATALOG), \ + patch("tools.connections_tool_mcp._configured_names", return_value=sorted(CONFIGURED)): + yield + + +class FakeClient: + def __init__(self): + self.calls = [] + + def list_connectors(self): + self.calls.append("list") + return [{"connector": "gmail", "enabled": True, "connected": False}] + + def connections(self, connectors, *, reinitiate=False): + self.calls.append(("connections", tuple(connectors), reinitiate)) + return {"results": [{"connector": c, "status": "initiated", "connect_url": f"https://x/{c}"} for c in connectors]} + + +def _linear(**kw): + return {"name": "linear", "mcp": True, **kw} + + +# --------------------------------------------------------------------------- +# off-desktop: no approval surface +# --------------------------------------------------------------------------- + + +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" + assert out["settled_by"] == op.SETTLED_UNAVAILABLE + (target,) = out["targets"] + assert target["state"] == op.UNAVAILABLE + assert target["hint"] == "hermes mcp install linear / hermes mcp login linear" + assert "error" not in out + + +def test_registry_dispatch_never_blocks_and_never_reaches_a_card(): + # 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" + + +def test_a_managed_action_never_accepts_mcp_targets_and_vice_versa(): + client = FakeClient() + out = json.loads(manage_connections( + {"action": "connect", "connectors": ["gmail", _linear()]}, client_factory=lambda: client)) + assert "managed-connector action" in out["error"] + assert client.calls == [] # rejected before any gateway call + + out = json.loads(manage_connections({"action": "install", "connectors": ["gmail", _linear()]})) + assert "must carry" in out["error"] + + +def test_managed_leg_is_byte_for_byte_unchanged_by_the_fold(): + client = FakeClient() + out = json.loads(manage_connections( + {"action": "connect", "connectors": ["gmail", {"name": "gmail"}]}, client_factory=lambda: client)) + assert client.calls == [("connections", ("gmail",), False)] + assert out["results"][0]["connect_url"] == "https://x/gmail" + + +def test_unknown_target_fields_are_rejected(): + out = json.loads(manage_connections({"action": "install", "connectors": [_linear(url="https://evil")]})) + assert "unknown target field" in out["error"] and "url" in out["error"] + + +# --------------------------------------------------------------------------- +# catalog validation +# --------------------------------------------------------------------------- + + +def test_install_is_catalog_only_and_lists_the_catalog_on_a_miss(): + out = json.loads(manage_connections({"action": "install", "connectors": [{"name": "github", "mcp": True}]})) + assert "github" in out["error"] + assert "figma, linear, notion" in out["error"] + + +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 +# --------------------------------------------------------------------------- + + +def test_callback_answer_folds_into_the_operation_and_settles_once(): + seen = [] + + def callback(payload): + seen.append(payload) + return json.dumps({"settled_by": "all_resolved", "targets": [ + {"name": "linear", "status": "installed", "tools": ["a", "b"]}, + {"name": "figma", "status": "declined"}, + ]}) + + out = json.loads(manage_connections( + {"action": "install", "connectors": [_linear(), {"name": "figma", "mcp": True}], "reason": "tickets"}, + connection_callback=callback, wait_seconds=30)) + (payload,) = seen + assert payload["reason"] == "tickets" + assert [t["name"] for t in payload["targets"]] == ["linear", "figma"] + assert payload["deadline_at"] == pytest.approx(payload["deadline_at"]) # server-owned, present + assert payload["timeout_seconds"] == 30 + assert out["status"] == "settled" and out["settled_by"] == "all_resolved" + by_name = {t["name"]: t for t in out["targets"]} + assert by_name["linear"]["state"] == op.CONNECTED and by_name["linear"]["tools"] == ["a", "b"] + assert by_name["figma"]["state"] == op.SKIPPED + + +def test_no_answer_settles_by_deadline_and_marks_targets_not_connected(): + out = json.loads(manage_connections( + {"action": "install", "connectors": [_linear()]}, connection_callback=lambda payload: "", wait_seconds=5)) + assert out["settled_by"] == op.SETTLED_DEADLINE + assert out["targets"][0]["state"] == op.NOT_CONNECTED + 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 = json.loads(manage_connections( + {"action": "install", "connectors": [_linear()]}, connection_callback=lambda payload: answer, wait_seconds=5)) + assert "sk-secret" not in json.dumps(out) + + +# --------------------------------------------------------------------------- +# the inline executor + replay shim +# --------------------------------------------------------------------------- + + +def _agent(callback): + return SimpleNamespace(session_id="s1", connection_callback=callback) + + +def test_inline_executor_hands_the_agent_callback_to_the_tool(): + from agent.inline_tool_executors import INLINE_TOOL_EXECUTORS, InlineToolContext + + calls = [] + + def callback(payload): + calls.append(payload) + return json.dumps({"targets": [{"name": "linear", "status": "installed"}]}) + + out = json.loads(INLINE_TOOL_EXECUTORS["manage_connections"]( + _agent(callback), {"action": "install", "connectors": [_linear()]}, InlineToolContext("task"))) + assert len(calls) == 1 + assert out["targets"][0]["state"] == op.CONNECTED + + +def test_setup_mcp_replay_shim_translates_to_an_mcp_target(): + from agent.inline_tool_executors import INLINE_TOOL_EXECUTORS, InlineToolContext + + calls = [] + + def callback(payload): + calls.append(payload) + return json.dumps({"targets": [{"name": "linear", "status": "declined"}]}) + + out = json.loads(INLINE_TOOL_EXECUTORS["setup_mcp"]( + _agent(callback), {"server": "linear", "action": "install", "reason": "old convo"}, InlineToolContext("task"))) + assert calls[0]["targets"] == [{"name": "linear", "kind": "mcp", "action": "install"}] + assert calls[0]["reason"] == "old convo" + assert out["targets"][0]["state"] == op.SKIPPED + + +def test_setup_mcp_is_gone_from_every_advertised_toolset(): + from toolsets import TOOLSETS, resolve_toolset + + assert all("setup_mcp" not in resolve_toolset(name) for name in TOOLSETS) + assert "manage_connections" in resolve_toolset("connections") + assert "hand-edit" in MANAGE_CONNECTIONS_SCHEMA["description"] + assert "mcp_servers" in MANAGE_CONNECTIONS_SCHEMA["description"] + + +# --------------------------------------------------------------------------- +# deadline ownership +# --------------------------------------------------------------------------- + + +def test_the_bounded_wait_owns_the_deadline_not_the_sequential_guard(): + from agent import tool_executor as te + + assert "manage_connections" in te._SEQUENTIAL_DEADLINE_EXEMPT_TOOLS + + +def test_default_wait_comes_from_the_config_key(monkeypatch): + monkeypatch.setenv("HERMES_CONCURRENT_TOOL_TIMEOUT_S", "3") + seen = {} + with patch("tools.connections_tool_mcp.resolve_wait_timeout", return_value=77.0): + manage_connections({"action": "install", "connectors": [_linear()]}, + connection_callback=lambda p: seen.update(p) or "") + assert seen["timeout_seconds"] == 77.0 + + +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 = json.loads(manage_connections( + {"action": "install", "connectors": [_linear(), {"name": "figma", "mcp": True}]}, + connection_callback=lambda payload: answer, wait_seconds=5)) + assert out["settled_by"] == op.SETTLED_CONTINUE + by_name = {t["name"]: t for t in out["targets"]} + assert by_name["linear"]["state"] == op.SKIPPED + assert by_name["figma"]["state"] == op.NOT_CONNECTED and by_name["figma"]["detail"] == op.SETTLED_CONTINUE diff --git a/tests/tools/test_connections_tool_operation.py b/tests/tools/test_connections_tool_operation.py new file mode 100644 index 0000000000..aecb5a2391 --- /dev/null +++ b/tests/tools/test_connections_tool_operation.py @@ -0,0 +1,79 @@ +"""Connection operation: exactly-once settlement, server-owned deadline, config-bounded wait.""" + +import pytest + +from tools import connections_tool_operation as op + + +def _two_targets(): + return [op.Target("linear", "mcp", "install"), op.Target("figma", "mcp", "install")] + + +def test_settle_is_exactly_once_and_freezes_the_result(): + operation = op.ConnectionOperation(_two_targets(), wait_seconds=30) + operation.record_target("linear", op.CONNECTED) + assert operation.settle(op.SETTLED_CONTINUE) is True + frozen = operation.result() + # Later events update current state only, never the settled result. + assert operation.settle(op.SETTLED_DEADLINE) is False + operation.record_target("figma", op.CONNECTED) + assert operation.settled_by == op.SETTLED_CONTINUE + assert operation.result() == frozen + assert operation.target("figma").state == op.CONNECTED # live state did move + + +def test_unresolved_targets_are_marked_not_connected_at_settlement(): + operation = op.ConnectionOperation(_two_targets(), wait_seconds=30) + operation.record_target("linear", op.CONNECTED) + operation.settle(op.SETTLED_DEADLINE) + states = {t["name"]: t for t in operation.result()["targets"]} + assert states["linear"]["state"] == op.CONNECTED + assert states["figma"]["state"] == op.NOT_CONNECTED + assert states["figma"]["detail"] == op.SETTLED_DEADLINE + + +def test_all_resolved_means_connected_or_explicitly_skipped(): + operation = op.ConnectionOperation(_two_targets(), wait_seconds=30) + operation.record_target("linear", op.CONNECTED) + operation.record_target("figma", op.FAILED, "oauth denied") + # A recoverable failure keeps the operation open. + assert operation.settle_if_all_resolved() is False + operation.record_target("figma", op.SKIPPED) + assert operation.settle_if_all_resolved() is True + assert operation.settled_by == op.SETTLED_ALL_RESOLVED + + +def test_deadline_is_set_at_creation_and_never_recomputed(): + operation = op.ConnectionOperation(_two_targets(), wait_seconds=42) + first = operation.deadline_at + assert first == pytest.approx(operation.created_at + 42) + operation.record_target("linear", op.CONNECTED) + payload = operation.request_payload() + assert payload["deadline_at"] == first + assert payload["op_id"] == operation.op_id + assert [t["name"] for t in payload["targets"]] == ["linear", "figma"] + + +def test_record_target_rejects_unknown_names(): + operation = op.ConnectionOperation(_two_targets()) + assert operation.record_target("github", op.CONNECTED) is False + + +@pytest.mark.parametrize( + ("config", "expected"), + [ + ({}, op.WAIT_TIMEOUT_DEFAULT_SECONDS), + ({"connections": {"wait_timeout_seconds": 600}}, 600.0), + ({"connections": {"wait_timeout_seconds": 1}}, op.WAIT_TIMEOUT_FLOOR_SECONDS), + ({"connections": {"wait_timeout_seconds": "nope"}}, op.WAIT_TIMEOUT_DEFAULT_SECONDS), + ({"connections": {"wait_timeout_seconds": True}}, op.WAIT_TIMEOUT_DEFAULT_SECONDS), + ], +) +def test_wait_timeout_reads_only_its_own_key_with_a_floor_and_no_ceiling(config, expected): + assert op.resolve_wait_timeout(config) == expected + + +def test_legacy_timeouts_never_proxy_for_the_wait(monkeypatch): + # The batch guard env var and the clarify timeout are separate budgets. + monkeypatch.setenv("HERMES_CONCURRENT_TOOL_TIMEOUT_S", "7") + assert op.resolve_wait_timeout({"agent": {"clarify_timeout": 9}}) == op.WAIT_TIMEOUT_DEFAULT_SECONDS diff --git a/tests/tools/test_connector_local_batches.py b/tests/tools/test_connector_local_batches.py index 75d4668b48..a422f06c8b 100644 --- a/tests/tools/test_connector_local_batches.py +++ b/tests/tools/test_connector_local_batches.py @@ -42,31 +42,38 @@ def test_single_local_unwrap_keeps_session_db_todo_store_and_setup_callback(tmp_ db.create_session("past-session", source="cli") db.append_message("past-session", role="user", content="live-db-proof") callbacks = [] - def setup(server, action, reason): - callbacks.append((server, action, reason)) - return json.dumps({"status": "declined", "server": server}) + def connection(payload): + callbacks.append(payload) + return json.dumps({"targets": [{"name": t["name"], "status": "declined"} for t in payload["targets"]]}) agent = SimpleNamespace( - enabled_toolsets=["todo", "session_search", "desktop_ui"], disabled_toolsets=[], + enabled_toolsets=["todo", "session_search", "connections"], disabled_toolsets=[], session_id="current-session", _todo_store=TodoStore(), _memory_manager=None, - _get_session_db_for_recall=lambda: db, setup_mcp_callback=setup, + _get_session_db_for_recall=lambda: db, connection_callback=connection, ) calls = [ {"name": "session_search", "arguments": {"session_id": "past-session"}}, {"name": "todo_list", "arguments": {"todos": [{"id": "a", "content": "live-store-proof", "status": "pending"}]}}, - {"name": "setup_mcp", "arguments": {"server": "example", "action": "install", "reason": "live-callback-proof"}}, + {"name": "manage_connections", "arguments": { + "action": "install", "connectors": [{"name": "linear", "mcp": True}], "reason": "live-callback-proof"}}, ] results = [] try: for entry in calls: - name, args, error = _unwrap_tool_search_call( - agent, "tool_call", {"calls": [entry]}, flatten_probe=flatten_probe) + if entry["name"] == "manage_connections": + # Not deferrable, so it reaches invoke_tool directly and must find the agent callback. + name, args, error = entry["name"], entry["arguments"], None + else: + name, args, error = _unwrap_tool_search_call( + agent, "tool_call", {"calls": [entry]}, flatten_probe=flatten_probe) assert name == entry["name"] and error is None results.append(json.loads(invoke_tool( agent, name, args, "task", tool_call_id="call", pre_tool_block_checked=True))) assert "live-db-proof" in json.dumps(results[0]) assert agent._todo_store.read()[0]["content"] == "live-store-proof" - assert results[2] == {"status": "declined", "server": "example"} - assert callbacks == [("example", "install", "live-callback-proof")] + assert results[2]["targets"][0] == { + "name": "linear", "kind": "mcp", "action": "install", "state": "skipped"} + assert [(c["reason"], [t["name"] for t in c["targets"]]) for c in callbacks] == [ + ("live-callback-proof", ["linear"])] finally: db.close() diff --git a/tests/tools/test_desktop_tools_diet.py b/tests/tools/test_desktop_tools_diet.py index 6ab8d20a88..e8d90f2103 100644 --- a/tests/tools/test_desktop_tools_diet.py +++ b/tests/tools/test_desktop_tools_diet.py @@ -107,7 +107,7 @@ class TestDietBudget(unittest.TestCase): from model_tools import get_tool_definitions targets = { - "drive_preview", "tour", "annotate_preview", "setup_mcp", "tip", + "drive_preview", "tour", "annotate_preview", "tip", "desktop_preview", "desktop_project", "read_window_below", "apply_layout", "read_terminal", "focus_pane", } diff --git a/tests/tools/test_setup_mcp_tool.py b/tests/tools/test_setup_mcp_tool.py deleted file mode 100644 index 12842b6a71..0000000000 --- a/tests/tools/test_setup_mcp_tool.py +++ /dev/null @@ -1,83 +0,0 @@ -"""setup_mcp tool — the desktop inline MCP consent card's tool half. - -Behavior contracts: -- no callback (not the desktop app) → tool_error pointing at the CLI path -- empty/invalid args → tool_error -- callback answer passes through as JSON -- empty callback answer (timeout) → status "unanswered", never an error -""" - -import json - -import pytest - -from tools.setup_mcp_tool import SETUP_MCP_SCHEMA, setup_mcp_tool - - -def test_schema_forbids_hand_editing_mcp_servers_config(): - # Nothing else teaches the model this: the tool is desktop_ui-only, so - # without it a model could just write_file into mcp_servers config - # directly, bypassing the consent-card/OAuth flow this tool exists for. - assert "hand-edit" in SETUP_MCP_SCHEMA["description"] - assert "mcp_servers" in SETUP_MCP_SCHEMA["description"] - - -def test_requires_desktop_callback(): - result = json.loads(setup_mcp_tool(server="linear", callback=None)) - assert "error" in result - assert "hermes mcp install" in result["error"] - - -def test_requires_server_name(): - result = json.loads(setup_mcp_tool(server=" ", callback=lambda *a: "")) - assert "error" in result - - -def test_rejects_unknown_action(): - result = json.loads( - setup_mcp_tool(server="linear", action="uninstall", callback=lambda *a: "") - ) - assert "error" in result - assert "action" in result["error"] - - -def test_passes_through_renderer_outcome(): - outcome = {"status": "installed", "server": "linear"} - - def cb(server, action, reason): - assert server == "linear" - assert action == "install" - assert reason == "to read tickets" - return json.dumps(outcome) - - result = json.loads( - setup_mcp_tool(server="linear", action="install", reason="to read tickets", callback=cb) - ) - assert result == outcome - - -def test_timeout_returns_unanswered_not_error(): - result = json.loads(setup_mcp_tool(server="figma", callback=lambda *a: "")) - assert result["status"] == "unanswered" - assert result["server"] == "figma" - - -def test_callback_exception_is_tool_error(): - def cb(*a): - raise RuntimeError("gateway went away") - - result = json.loads(setup_mcp_tool(server="figma", callback=cb)) - assert "error" in result - - -def test_non_json_answer_wrapped_as_error_status(): - result = json.loads(setup_mcp_tool(server="figma", callback=lambda *a: "garbage")) - assert result["status"] == "error" - - -@pytest.mark.parametrize("action", ["install", "enable", "authorize"]) -def test_all_actions_accepted(action): - result = json.loads( - setup_mcp_tool(server="x", action=action, callback=lambda s, a, r: json.dumps({"status": "declined", "server": s})) - ) - assert result["status"] == "declined" diff --git a/tests/tui_gateway/test_connection_server_request.py b/tests/tui_gateway/test_connection_server_request.py new file mode 100644 index 0000000000..59dfb2edb3 --- /dev/null +++ b/tests/tui_gateway/test_connection_server_request.py @@ -0,0 +1,58 @@ +"""The ``manage_connections`` card is one ``connection`` server request per operation +(``tui_gateway/server.py::_connection_request``): the renderer's per-target outcomes reach the tool +as JSON text, and an unanswered request yields ``''`` so the operation settles on its own deadline +instead of raising. +""" + +import json + +import pytest + +import tui_gateway.server as server +from tui_gateway import server_requests +from tui_gateway.contracts import registry + + +@pytest.fixture +def sent(monkeypatch): + calls = [] + + def fake_send(method, sid, params, *, timeout, **_kw): + calls.append({"method": method, "sid": sid, "params": params, "timeout": timeout}) + return fake_send.result + + fake_send.result = None + fake_send.calls = calls + monkeypatch.setattr(server_requests, "send", fake_send) + return fake_send + + +def _payload(): + return {"op_id": "op1", "deadline_at": 1.0, "timeout_seconds": 42.0, "reason": "", + "targets": [{"name": "linear", "kind": "mcp", "action": "authorize"}]} + + +def test_answer_round_trips_as_json_and_waits_the_operation_deadline(sent): + sent.result = {"settled_by": "all_resolved", + "targets": [{"name": "linear", "state": "authorized", "tools": ["list_issues"]}]} + + raw = server._connection_request("s1", _payload()) + + assert sent.calls[0]["method"] == "connection" + assert sent.calls[0]["timeout"] == 42.0 + assert json.loads(raw) == sent.result + + +def test_unanswered_request_is_the_empty_answer(sent): + sent.result = None + assert server._connection_request("s1", _payload()) == "" + + +def test_request_payload_satisfies_the_declared_contract(): + """The tool's payload and the card's answer both validate against the contract the TS is + generated from, so a drift on either side fails here before it fails in a renderer.""" + contract = registry.SERVER_REQUESTS["connection"] + params, problem = registry.validate_params(contract, {"session_id": "s1", **_payload()}) + assert problem is None and params is not None + registry.check_result(contract, {"settled_by": "continue", + "targets": [{"name": "linear", "state": "declined"}]}) diff --git a/tests/tui_gateway/test_gui_surface_toolsets.py b/tests/tui_gateway/test_gui_surface_toolsets.py index 92463fe453..512821eee6 100644 --- a/tests/tui_gateway/test_gui_surface_toolsets.py +++ b/tests/tui_gateway/test_gui_surface_toolsets.py @@ -26,7 +26,6 @@ GUI_TOOLS = { "read_terminal", "read_window_below", "react_to_message", - "setup_mcp", "show_tip", "gui_tour", } diff --git a/tools/connections_tool.py b/tools/connections_tool.py index b8e573c375..8f0ed9f150 100644 --- a/tools/connections_tool.py +++ b/tools/connections_tool.py @@ -16,20 +16,15 @@ lifecycle: so guidance produced a burst of polls rather than a paced one. Waiting inside the call cannot be skipped and works the same on every platform. -Scope: gateway connectors ONLY. Local MCP servers stay with ``setup_mcp``, -which still exists and still works. An earlier draft folded ``install`` / -``enable`` / ``authorize`` in here, but the desktop consent card arrives -through a per-tool interception branch keyed on the name ``setup_mcp`` -(agent/tool_executor.py, agent/agent_runtime_helpers.py) and -``registry.dispatch`` never forwards a ``callback``. So the fold could only -ever return the "use the terminal" fallback while its schema advertised the -consent flow — a promise with no delivery path. +Scope: managed connectors AND local MCP servers. ``{"name": ..., "mcp": true}`` targets take +``install`` / ``enable`` / ``authorize`` through one connection operation +(``connections_tool_operation.py``, ``connections_tool_mcp.py``); the approval card is reached +only via the inline executor, so non-GUI surfaces get ``unavailable`` for MCP targets. De-authentication is deliberately NOT exposed to the model: disconnecting an account is a user decision, made in the portal dashboard. -Availability: gated by the portal sign-in the managed tools already use -(``check_fn``), so signed-out sessions see exactly today's behavior. +Availability: the managed leg is portal-gated in the handler; MCP targets need no sign-in. """ import json @@ -38,6 +33,13 @@ import threading import time from typing import Any, Callable, Dict, List, Optional +from tools.connections_tool_mcp import ( + ALL_ACTIONS, + MCP_ACTIONS, + normalize_targets, + run_mcp_operation, + validate_action, +) from tools.registry import registry, tool_error logger = logging.getLogger(__name__) @@ -299,27 +301,29 @@ def manage_connections( seen_instructions: Optional[set] = None, rendered_links: Optional[Dict[str, Dict[str, float]]] = None, session_id: Optional[str] = None, + connection_callback: Optional[Callable[[Dict[str, Any]], Optional[str]]] = None, + connectors_available: Optional[Callable[[], bool]] = None, + wait_seconds: Optional[float] = None, ) -> str: """Dispatch one ``manage_connections`` action. Returns a JSON string.""" action = str(args.get("action") or "status").strip().lower() + managed, mcp_targets, target_error = normalize_targets(args.get("connectors")) + if target_error: + return tool_error(target_error) + action_error = validate_action(action, managed, mcp_targets) + if action_error: + return tool_error(action_error) - if action not in _CONNECTOR_ACTIONS: - return tool_error( - f"action must be one of {', '.join(_CONNECTOR_ACTIONS)}. " - "Local MCP servers are set up with setup_mcp, not here. " - "Disconnecting an account is done by the user in the Nous Portal " - "dashboard, not through this tool." + if action in MCP_ACTIONS: + return run_mcp_operation( + mcp_targets, action, str(args.get("reason") or "").strip(), + connection_callback=connection_callback, session_id=session_id, wait_seconds=wait_seconds, ) - raw_connectors = args.get("connectors") - if isinstance(raw_connectors, str): - raw_connectors = [raw_connectors] - connectors: List[str] = [] - if isinstance(raw_connectors, list): - for c in raw_connectors: - c = str(c or "").strip().lower() - if c and c not in connectors: - connectors.append(c) + # Managed leg. Callers that pass a gate (registry handler, inline executor) are portal-gated. + if connectors_available is not None and not connectors_available(): + return tool_error("Connectors are not available in this session.") + connectors: List[str] = managed try: client = (client_factory or _default_client)() @@ -442,9 +446,11 @@ def manage_connections( MANAGE_CONNECTIONS_SCHEMA = { "name": "manage_connections", "description": ( - "Manage remote connector accounts (Gmail, Linear, Notion, ...) served " - "through the tool gateway. Actions: " - "'status' lists connectors and whether each is connected; 'connect' " + "Connect the user to apps: managed connector accounts (Gmail, Notion, ...) served " + "through the tool gateway, and local MCP servers from the catalog. Targets go in " + "'connectors': a bare slug or {\"name\": \"gmail\"} is a managed connector; " + "{\"name\": \"linear\", \"mcp\": true} is a local MCP server. " + "Managed actions: 'status' lists connectors and whether each is connected; 'connect' " "starts an authorization for the given connectors and returns a link " "for the USER to open in a browser (never open it yourself); " "'reconnect' restarts a broken authorization; " @@ -461,7 +467,14 @@ MANAGE_CONNECTIONS_SCHEMA = { "count). A 'timeout' or 'interrupted' result is NOT an " "error: the user has not finished connecting, so ask them whether to " "keep waiting, continue without those apps, or get fresh links. " - "Local MCP servers are configured separately. " + "MCP actions (targets must carry \"mcp\": true): 'install' adds a catalog entry, " + "'enable' re-enables a disabled configured server, 'authorize' runs its OAuth. " + "They show the user an approval card and block until it settles; the result lists " + "each target as connected / skipped / not_connected. Never hand-edit mcp_servers " + "config — always use this tool. Never re-ask after a skip or timeout: continue " + "without the server 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 Nous Portal (their org's Connectors page) or the desktop app." @@ -471,17 +484,34 @@ MANAGE_CONNECTIONS_SCHEMA = { "properties": { "action": { "type": "string", - "enum": list(_CONNECTOR_ACTIONS), - "description": "Defaults to status.", + "enum": list(ALL_ACTIONS), + "description": "Defaults to status. install/enable/authorize need mcp:true targets.", }, "connectors": { "type": "array", - "items": {"type": "string"}, + "items": { + "anyOf": [ + {"type": "string"}, + { + "type": "object", + "properties": { + "name": {"type": "string"}, + "mcp": {"type": "boolean", "description": "true = local MCP server."}, + }, + "required": ["name"], + "additionalProperties": False, + }, + ] + }, "description": ( - "Connector slugs. REQUIRED for connect, reconnect and wait " - "(e.g. [\"gmail\", \"linear\"]); optional filter for status." + "Targets. REQUIRED for every action but status " + "(e.g. [\"gmail\", {\"name\": \"linear\", \"mcp\": true}]); optional filter for status." ), }, + "reason": { + "type": "string", + "description": "MCP actions: one sentence on the approval card — why this helps right now.", + }, "timeout_seconds": { "type": "integer", "description": ( @@ -502,13 +532,10 @@ registry.register( name="manage_connections", toolset="connections", schema=MANAGE_CONNECTIONS_SCHEMA, - # Registry dispatch does not re-run check_fn: enforce the off switch for - # stale schemas without rebuilding a conversation's cached tool list. - handler=lambda args, **kw: ( - manage_connections(args, session_id=kw.get("session_id")) - if _connectors_available() - else tool_error("Connectors are not available in this session.") + # The portal gate is in the handler, not check_fn, so signed-out sessions keep the tool for + # MCP approvals. The registry path has no GUI callback. + handler=lambda args, **kw: manage_connections( + args, session_id=kw.get("session_id"), connectors_available=_connectors_available, ), - check_fn=_connectors_available, emoji="🔗", ) diff --git a/tools/connections_tool_mcp.py b/tools/connections_tool_mcp.py new file mode 100644 index 0000000000..91e21cbca6 --- /dev/null +++ b/tools/connections_tool_mcp.py @@ -0,0 +1,222 @@ +"""MCP targets of ``manage_connections``: target/action/catalog validation and the approval +leg. The card is reached via ``agent.connection_callback`` through the inline executor; +registry dispatch has no callback and settles targets ``unavailable``.""" + +from __future__ import annotations + +import json +import logging +from typing import Any, Callable, Dict, List, Optional, Tuple + +from tools.connections_tool_operation import ( + CONNECTED, + FAILED, + SETTLED_ALL_RESOLVED, + SETTLED_CONTINUE, + SETTLED_DEADLINE, + SETTLED_INTERRUPT, + SETTLED_UNAVAILABLE, + SKIPPED, + UNAVAILABLE, + ConnectionOperation, + Target, + resolve_wait_timeout, +) +from tools.registry import tool_error + +logger = logging.getLogger(__name__) + +CONNECTOR_ACTIONS = ("status", "connect", "reconnect", "wait") +MCP_ACTIONS = ("install", "enable", "authorize") +ALL_ACTIONS = CONNECTOR_ACTIONS + MCP_ACTIONS + +_TARGET_FIELDS = frozenset({"name", "mcp"}) + +# Renderer outcome → operation state. declined = Not now; error = recoverable, operation stays open. +_OUTCOME_STATES = { + "installed": CONNECTED, "enabled": CONNECTED, "authorized": CONNECTED, "connected": CONNECTED, + "declined": SKIPPED, "skipped": SKIPPED, + "error": FAILED, "failed": FAILED, +} + +UNAVAILABLE_HINT = "hermes mcp install {name} / hermes mcp login {name}" + + +def normalize_targets(raw: Any) -> Tuple[List[str], List[str], Optional[str]]: + """``connectors`` → ``(managed names, mcp names, error)``. Bare strings and ``{name}`` are + managed; ``{name, mcp: true}`` is a local MCP. Any other field is an error.""" + if raw is None: + return [], [], None + if isinstance(raw, (str, dict)): + raw = [raw] + if not isinstance(raw, list): + return [], [], "'connectors' must be a list of names or {name, mcp} objects." + managed: List[str] = [] + mcp: List[str] = [] + for item in raw: + if isinstance(item, dict): + unknown = sorted(set(item) - _TARGET_FIELDS) + if unknown: + return [], [], ( + f"unknown target field(s) {', '.join(unknown)}: a target is " + "{\"name\": \"\"} or {\"name\": \"\", \"mcp\": true}. Transport, " + "URLs and credentials come from the catalog manifest, never from the call." + ) + name = str(item.get("name") or "").strip().lower() + is_mcp = bool(item.get("mcp", False)) + else: + name, is_mcp = str(item or "").strip().lower(), False + if not name: + return [], [], "every target needs a non-empty 'name'." + bucket = mcp if is_mcp else managed + if name not in bucket: + bucket.append(name) + return managed, mcp, None + + +def validate_action(action: str, managed: List[str], mcp: List[str]) -> Optional[str]: + """MCP verbs need ``mcp:true`` targets; connector verbs need managed targets.""" + if action not in ALL_ACTIONS: + return ( + f"action must be one of {', '.join(ALL_ACTIONS)}. " + f"{', '.join(MCP_ACTIONS)} apply to local MCP servers " + "(targets {\"name\": ..., \"mcp\": true}); the rest apply to managed connectors. " + "Disconnecting an account is done by the user in the Nous Portal dashboard, not " + "through this tool." + ) + if action in MCP_ACTIONS: + if managed: + return ( + f"'{action}' is an MCP action: every target must carry \"mcp\": true " + f"(got managed connector(s) {', '.join(managed)}). Managed connectors use " + "connect / reconnect / wait / status." + ) + if not mcp: + return ( + f"'{action}' requires 'connectors': the MCP server name(s), e.g. " + "[{\"name\": \"linear\", \"mcp\": true}]." + ) + elif mcp: + return ( + f"'{action}' is a managed-connector action; MCP targets ({', '.join(mcp)}) use " + f"{', '.join(MCP_ACTIONS)}." + ) + return None + + +def _catalog_names() -> List[str]: + from hermes_cli.mcp_catalog import list_catalog + + return sorted(e.name for e in list_catalog()) + + +def _configured_names() -> List[str]: + from hermes_cli.mcp_catalog import installed_servers + + return sorted(installed_servers()) + + +def validate_mcp_names(action: str, names: List[str]) -> Optional[str]: + """install: catalog names only; enable/authorize: configured servers only.""" + try: + catalog = _catalog_names() + configured = _configured_names() + except Exception as exc: + return f"could not read the MCP catalog: {exc}" + allowed = set(catalog) if action == "install" else set(configured) + unknown = [n for n in names if n not in allowed] + if not unknown: + return None + if action == "install": + return ( + f"unknown MCP server(s) for install: {', '.join(unknown)}. Install works for " + f"catalog entries only: {', '.join(catalog) or '(empty catalog)'}." + + (f" Already configured (use enable/authorize): {', '.join(configured)}." if configured else "") + ) + return ( + f"unknown MCP server(s) for {action}: {', '.join(unknown)}. {action} works for servers " + f"already in mcp_servers: {', '.join(configured) or '(none configured)'}." + + (f" Catalog entries you can install: {', '.join(catalog)}." if catalog else "") + ) + + +def _unavailable_result(operation: ConnectionOperation) -> str: + for target in operation.targets: + operation.record_target( + target.name, UNAVAILABLE, "no approval surface in this session", + hint=UNAVAILABLE_HINT.format(name=target.name), + ) + operation.settle(SETTLED_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." + ) + return json.dumps(payload, ensure_ascii=False) + + +def _apply_answer(operation: ConnectionOperation, raw: str) -> str: + """Apply the renderer's per-target answer; returns the settle reason the target states imply.""" + try: + answer = json.loads(raw) + except (TypeError, ValueError): + answer = {} + if not isinstance(answer, dict): + answer = {} + 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()) + if not name or state is None: + continue + extra = {k: v for k, v in entry.items() if k in ("tools",)} + operation.record_target(name, state, str(entry.get("detail") or ""), **extra) + # Derived from target state; the renderer's own claim is ignored. + return SETTLED_ALL_RESOLVED if operation.all_resolved else SETTLED_CONTINUE + + +def run_mcp_operation( + names: List[str], + action: str, + reason: str, + *, + connection_callback: Optional[Callable[[Dict[str, Any]], Optional[str]]], + session_id: Optional[str], + wait_seconds: Optional[float] = None, +) -> str: + """One operation for the MCP targets of a call. Returns the tool's JSON string.""" + error = validate_mcp_names(action, names) + if error: + return tool_error(error) + operation = ConnectionOperation( + [Target(n, "mcp", action) for n in names], + session_key=str(session_id or ""), + wait_seconds=resolve_wait_timeout() if wait_seconds is None else wait_seconds, + ) + if connection_callback is None: + return _unavailable_result(operation) + + try: + raw = connection_callback(operation.request_payload(reason)) + except Exception as exc: + return tool_error(f"MCP approval flow failed: {exc}") + + settled_by = _apply_answer(operation, raw or "") + if raw: + operation.settle(settled_by) + else: + # Empty answer: deadline passed or the turn was interrupted. + from tools.interrupt import is_interrupted + + operation.settle(SETTLED_INTERRUPT if is_interrupted() else SETTLED_DEADLINE) + + payload = operation.result() + payload["status"] = "settled" + payload["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." + ) + return json.dumps(payload, ensure_ascii=False) diff --git a/tools/connections_tool_operation.py b/tools/connections_tool_operation.py new file mode 100644 index 0000000000..275db268fa --- /dev/null +++ b/tools/connections_tool_operation.py @@ -0,0 +1,169 @@ +"""One backend-owned connection operation per ``manage_connections`` call: targets, a +server-set deadline, exactly-once settlement. Pure data, no I/O.""" + +from __future__ import annotations + +import threading +import time +import uuid +from dataclasses import dataclass, field +from typing import Any, Dict, List, Optional + +# config.yaml ``connections.wait_timeout_seconds``; floor only, no ceiling. +WAIT_TIMEOUT_DEFAULT_SECONDS = 120.0 +WAIT_TIMEOUT_FLOOR_SECONDS = 5.0 + +# Target states. connected/skipped/unavailable are resolved; failed keeps the operation open. +PENDING = "pending" +CONNECTED = "connected" +SKIPPED = "skipped" +FAILED = "failed" +UNAVAILABLE = "unavailable" +# Stamped on unresolved targets at settlement; ``detail`` carries the settle reason. +NOT_CONNECTED = "not_connected" +RESOLVED_STATES = frozenset({CONNECTED, SKIPPED, UNAVAILABLE}) + +# How an operation settled. +SETTLED_ALL_RESOLVED = "all_resolved" +SETTLED_CONTINUE = "continue" +SETTLED_DEADLINE = "deadline" +SETTLED_INTERRUPT = "interrupt" +SETTLED_UNAVAILABLE = "unavailable" + + +def resolve_wait_timeout(config: Optional[Dict[str, Any]] = None) -> float: + """``connections.wait_timeout_seconds`` from config.yaml, floored, default 120. Reads only + that key; the executor and clarify budgets never proxy for it.""" + if config is None: + try: + from hermes_cli.config import load_config_readonly + + config = load_config_readonly() or {} + except Exception: + config = {} + section = config.get("connections") if isinstance(config, dict) else None + raw = section.get("wait_timeout_seconds") if isinstance(section, dict) else None + try: + value = WAIT_TIMEOUT_DEFAULT_SECONDS if raw is None or isinstance(raw, bool) else float(raw) + except (TypeError, ValueError): + value = WAIT_TIMEOUT_DEFAULT_SECONDS + if value != value: # NaN + value = WAIT_TIMEOUT_DEFAULT_SECONDS + return max(WAIT_TIMEOUT_FLOOR_SECONDS, value) + + +@dataclass +class Target: + name: str + kind: str # "mcp" | "connector" + action: str + state: str = PENDING + detail: str = "" + # Renderer-reported fields passed through to the model (e.g. ``tools`` after OAuth). + extra: Dict[str, Any] = field(default_factory=dict) + + @property + def resolved(self) -> bool: + return self.state in RESOLVED_STATES + + def snapshot(self) -> Dict[str, Any]: + out: Dict[str, Any] = {"name": self.name, "kind": self.kind, "action": self.action, "state": self.state} + if self.detail: + out["detail"] = self.detail + out.update(self.extra) + return out + + +@dataclass +class ConnectionOperation: + targets: List[Target] + session_key: str = "" + wait_seconds: float = WAIT_TIMEOUT_DEFAULT_SECONDS + op_id: str = field(default_factory=lambda: uuid.uuid4().hex[:12]) + created_at: float = field(default_factory=time.time) + deadline_at: float = 0.0 + settled_at: Optional[float] = None + settled_by: Optional[str] = None + _settled_snapshot: Optional[Dict[str, Any]] = field(default=None, repr=False) + _lock: threading.Lock = field(default_factory=threading.Lock, repr=False) + + def __post_init__(self) -> None: + if not self.deadline_at: + self.deadline_at = self.created_at + float(self.wait_seconds) + + # -- targets ------------------------------------------------------------- + + def target(self, name: str) -> Optional[Target]: + return next((t for t in self.targets if t.name == name), None) + + def record_target(self, name: str, state: str, detail: str = "", **extra: Any) -> bool: + """Update a target's live state; False for an unknown name. Allowed after settlement: + the frozen ``result()`` does not change.""" + target = self.target(name) + if target is None: + return False + with self._lock: + target.state = state + target.detail = detail or "" + target.extra = dict(extra) + return True + + @property + def all_resolved(self) -> bool: + return bool(self.targets) and all(t.resolved for t in self.targets) + + @property + def settled(self) -> bool: + return self.settled_at is not None + + def remaining_seconds(self, now: Optional[float] = None) -> float: + return max(0.0, self.deadline_at - (time.time() if now is None else now)) + + # -- settlement ---------------------------------------------------------- + + def settle(self, by: str, now: Optional[float] = None) -> bool: + """Compare-and-set: first caller freezes the result, later callers get False.""" + with self._lock: + if self.settled_at is not None: + return False + self.settled_at = time.time() if now is None else now + self.settled_by = by + # Unresolved targets become not_connected so the settled card shows no pending row. + for target in self.targets: + if not target.resolved: + reason = target.detail or by + target.state = NOT_CONNECTED + target.detail = reason + self._settled_snapshot = self._snapshot_locked() + return True + + def settle_if_all_resolved(self) -> bool: + return self.all_resolved and self.settle(SETTLED_ALL_RESOLVED) + + def _snapshot_locked(self) -> Dict[str, Any]: + return { + "op_id": self.op_id, + "deadline_at": self.deadline_at, + "settled_at": self.settled_at, + "settled_by": self.settled_by, + "targets": [t.snapshot() for t in self.targets], + } + + def result(self) -> Dict[str, Any]: + """The settled result (frozen at settle time), or the live snapshot before settlement.""" + with self._lock: + if self._settled_snapshot is not None: + return dict(self._settled_snapshot, targets=[dict(t) for t in self._settled_snapshot["targets"]]) + return self._snapshot_locked() + + def request_payload(self, reason: str = "") -> Dict[str, Any]: + """The ``connection.request`` payload: identity, targets, server-owned deadline.""" + return { + "op_id": self.op_id, + "deadline_at": self.deadline_at, + "timeout_seconds": float(self.wait_seconds), + "reason": reason or "", + "targets": [ + {"name": t.name, "kind": t.kind, "action": t.action} for t in self.targets + ], + } diff --git a/tools/setup_mcp_tool.py b/tools/setup_mcp_tool.py deleted file mode 100644 index d36ca7fc9d..0000000000 --- a/tools/setup_mcp_tool.py +++ /dev/null @@ -1,108 +0,0 @@ -#!/usr/bin/env python3 -"""Propose an MCP server to the user as an inline card in the desktop chat. - -The card (install / enable / authorize + decline) lives in the desktop renderer, so this -tool round-trips through the gateway's blocking-prompt bridge (the one ``clarify`` uses): -tui_gateway emits ``mcp.setup.request``, the renderer walks the user through the existing -REST flows (catalog install, enable, OAuth) and answers with ``mcp.setup.respond``. Lives in -the ``desktop_ui`` toolset, which the GUI gateway enables only for desktop-sourced sessions; -elsewhere the agent falls back to ``hermes mcp install `` in the terminal. -""" - -import json -from typing import Callable, Optional - -from tools.registry import registry, tool_error - -_ACTIONS = ("install", "enable", "authorize") - - -def setup_mcp_tool(server: str = "", action: str = "install", reason: str = "", callback: Optional[Callable] = None) -> str: - """Ask the desktop GUI to run an MCP setup flow; return its JSON outcome.""" - if callback is None: - # Still down — the server task is reconnecting, or it has exhausted its retry budget and parked - # (e.g. a dead stdio subprocess). Probing here would write into a dead/absent transport and re-arm - # the breaker forever (#16788). Instead, ask the (always-present) server task to rebuild the - # transport — which respawns a dead stdio subprocess — and return a clean "reconnecting" error so - # the model backs off without burning iterations. The breaker resets once the fresh session - # initializes (_run_stdio/_run_http call _reset_server_error). - return tool_error( - "setup_mcp is only available in the Hermes desktop app. Use the " - "terminal instead: `hermes mcp install ` for catalog entries, " - "`hermes mcp login ` for OAuth.") - - name = (server or "").strip() - if not name: - return tool_error("server is required — the catalog or config name of the MCP server.") - - action = (action or "install").strip().lower() - if action not in _ACTIONS: - return tool_error(f"action must be one of {', '.join(_ACTIONS)}.") - - try: - raw = callback(name, action, (reason or "").strip()) - except Exception as exc: - return tool_error(f"MCP setup flow failed: {exc}") - - if not raw: - # The renderer never answered (timeout / closed window). Distinct from an explicit - # decline, which arrives as {"status": "declined"}. - return json.dumps({ - "status": "unanswered", - "server": name, - "note": ("The user did not respond to the setup card. Do not retry " - "immediately; continue without the server or ask in chat."), - }, ensure_ascii=False) - - # Desktop answers with a JSON object; pass it through, else wrap the raw text. - try: - return json.dumps(json.loads(raw), ensure_ascii=False) - except (TypeError, ValueError): - return json.dumps({"status": "error", "detail": str(raw)}, ensure_ascii=False) - - -SETUP_MCP_SCHEMA = { - "name": "setup_mcp", - "description": ( - "Propose an MCP server as an inline consent card (install a catalog " - "entry, re-enable a disabled server, or run OAuth); blocks until the " - "user acts. Use when they ask to add an MCP or a task clearly needs " - "a missing one. Never hand-edit mcp_servers config for them — always " - "use this tool. Never re-ask after a decline — on declined/" - "unanswered, continue without it. Catalog names: `hermes mcp " - "catalog` in the terminal." - ), - "parameters": { - "type": "object", - "properties": { - "server": { - "type": "string", - "description": "Catalog name (install) or mcp_servers config name (enable/authorize).", - }, - "action": { - "type": "string", - "enum": ["install", "enable", "authorize"], - "description": "Defaults to install.", - }, - "reason": { - "type": "string", - "description": "One sentence on the card: why this helps right now.", - }, - }, - "required": ["server"], - }, -} - - -registry.register( - name="setup_mcp", - toolset="desktop_ui", - schema=SETUP_MCP_SCHEMA, - handler=lambda args, **kw: setup_mcp_tool( - server=args.get("server", ""), - action=args.get("action", "install"), - reason=args.get("reason", ""), - callback=kw.get("callback"), - ), - emoji="🔌", -) diff --git a/tools/tool_search.py b/tools/tool_search.py index 5dce3b60e8..ca372a01bf 100644 --- a/tools/tool_search.py +++ b/tools/tool_search.py @@ -135,7 +135,7 @@ _DEFAULT_DEFERRED_TOOLS = frozenset({ "todo_list", "process_manage", "cronjob_manage", # Desktop GUI surface (desktop_ui + project toolsets) "drive_preview", "gui_tour", "desktop_preview", "annotate_preview", - "show_tip", "setup_mcp", "desktop_project", "close_terminal", + "show_tip", "desktop_project", "close_terminal", "apply_layout", "read_terminal", "read_window_below", "focus_pane"}) diff --git a/toolsets.py b/toolsets.py index 24c58b9677..1b2da7f579 100644 --- a/toolsets.py +++ b/toolsets.py @@ -139,7 +139,7 @@ TOOLSETS = { "reactions (GUI sessions only)", ["read_terminal", "close_terminal", "desktop_preview", "drive_preview", "annotate_preview", "read_window_below", "focus_pane", "react_to_message", - "setup_mcp", "gui_tour", "show_tip"], + "gui_tour", "show_tip"], ), "clarify": _ts("Ask the user clarifying questions (multiple-choice or open-ended)", ["clarify"]), "code_execution": _ts("Run Python scripts that call tools programmatically (reduces LLM round trips)", ["execute_code"]), diff --git a/tui_gateway/AGENTS.md b/tui_gateway/AGENTS.md index 783f59df27..ac283b462b 100644 --- a/tui_gateway/AGENTS.md +++ b/tui_gateway/AGENTS.md @@ -20,7 +20,7 @@ Never move agent behaviour into the renderer. Newline-delimited JSON-RPC over stdio, peer-to-peer: client→server method calls, server→client **requests** (the agent asking the user something: `approval`, `clarify`, `sudo`, `secret`, `vault.*`, -`mcp.setup`, the desktop read/act bridges) and server→client `event` notifications. `tui_gateway/server.py` +`connection`, the desktop read/act bridges) and server→client `event` notifications. `tui_gateway/server.py` is the facade with the method/event catalog; methods live in `methods_*.py` siblings (`methods_config`, `methods_complete`, `methods_browser`, `methods_bot_relay`, ...), event publishing in `event_publisher.py` / `event_replay.py`, server→client requests in `server_requests.py` (`send()` blocks diff --git a/tui_gateway/agent_callbacks.py b/tui_gateway/agent_callbacks.py index 54b1944efb..eda50931e5 100644 --- a/tui_gateway/agent_callbacks.py +++ b/tui_gateway/agent_callbacks.py @@ -111,10 +111,10 @@ def _agent_cbs(sid: str) -> dict: "drive_preview_callback": lambda payload: _ask("preview.act", sid, dict(payload), timeout=45), # read_window_below (desktop GUI): main process enumerates native windows. "read_window_below_callback": lambda: _ask("window.read", sid, {}, timeout=30), - # setup_mcp (desktop GUI): consent card + install/enable/OAuth; long timeout on purpose - # (typing an API key, browser OAuth). - "setup_mcp_callback": lambda server, action, reason: _ask( - "mcp.setup", sid, {"server": server, "action": action, "reason": reason}, timeout=600), + # manage_connections approval card (desktop GUI): one ``connection`` server request per + # operation; the renderer answers with the per-target outcomes. Waits the operation's own + # deadline, which the payload carries. + "connection_callback": lambda payload: _connection_request(sid, dict(payload)), # tour (desktop GUI): renderer drives driver.js and answers the ``tour`` request. "tour_callback": lambda payload: _tour_request(sid, payload)} diff --git a/tui_gateway/contracts/server_requests.py b/tui_gateway/contracts/server_requests.py index 8488cf20ea..d9d6941e8a 100644 --- a/tui_gateway/contracts/server_requests.py +++ b/tui_gateway/contracts/server_requests.py @@ -20,8 +20,8 @@ class ServerRequestParams(Params): class ValueResult(Result): - """The answer to any one-string prompt (sudo, secret, vault prompts, desktop bridges, - mcp.setup): ``''`` means skipped / declined.""" + """The answer to any one-string prompt (sudo, secret, vault prompts, desktop bridges): + ``''`` means skipped / declined.""" value: str @@ -143,14 +143,71 @@ server_request("vault.code", params=VaultCodeRequestParams, result=ValueResult, doc="A one-time / 2FA code the user reads from their device.") -class McpSetupRequestParams(ServerRequestParams): - server: str | None = None - action: str | None = None - reason: str | None = None +# ── connection operation (manage_connections card) ──────────────────────────────────────────── -server_request("mcp.setup", params=McpSetupRequestParams, result=ValueResult, - doc="Consent card for installing / enabling / authorising an MCP server.") +class ConnectionTargetKind(WireEnum): + connector = "connector" + mcp = "mcp" + + +class ConnectionAction(WireEnum): + authorize = "authorize" + enable = "enable" + install = "install" + + +class ConnectionRequestTarget(Params): + """One row of the card: ``tools/connections_tool_operation.py::ConnectionOperation.request_payload``.""" + + name: str + kind: ConnectionTargetKind + action: ConnectionAction + + +class ConnectionRequestParams(ServerRequestParams): + """``tools/connections_tool_operation.py::ConnectionOperation.request_payload`` — one operation, + N targets, a server-owned deadline (epoch seconds) the restored card keeps.""" + + op_id: str + deadline_at: float + timeout_seconds: float + reason: str = "" + targets: list[ConnectionRequestTarget] + + +class ConnectionTargetOutcomeState(WireEnum): + installed = "installed" + enabled = "enabled" + authorized = "authorized" + declined = "declined" + error = "error" + + +class ConnectionTargetOutcome(Result): + """The card's answer for one target (``tools/connections_tool_mcp.py::_apply_answer``).""" + + name: str + state: ConnectionTargetOutcomeState + detail: str | None = None + tools: list[str] | None = None + + +class ConnectionSettledBy(WireEnum): + all_resolved = "all_resolved" + continue_ = "continue" + + +class ConnectionResult(Result): + """Per-target outcomes. The backend derives the settle reason from target state; the renderer's + ``settled_by`` is its own claim and is not trusted.""" + + settled_by: ConnectionSettledBy + targets: list[ConnectionTargetOutcome] + + +server_request("connection", params=ConnectionRequestParams, result=ConnectionResult, + doc="The manage_connections approval card: install / enable / authorise local MCP servers.") # ── desktop GUI bridges ─────────────────────────────────────────────────────────────────────── diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 4e013dc786..393fe1242a 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -1312,6 +1312,17 @@ def _ask(method: str, sid: str, params: dict, timeout: float | None = 300) -> st return value if isinstance(value, str) else json.dumps(value, ensure_ascii=False) +def _connection_request(sid: str, payload: dict) -> str: + """The ``manage_connections`` approval card: one ``connection`` server request per operation. + The renderer answers with ``{settled_by, targets}``; the tool receives it as JSON text (``''`` + when the renderer never answered: deadline, cancel, or a client without a card). The wait is the + operation's own deadline, carried in the payload.""" + from tui_gateway import server_requests + timeout = float(payload.get("timeout_seconds") or 120) + result = server_requests.send("connection", sid, params=payload, timeout=timeout) + return json.dumps(result, ensure_ascii=False) if result else "" + + def _clarify_timeout_seconds() -> float | None: """Clarify wait for the TUI/desktop bridge from the canonical config (gateway/CLI parity); 300s historical default if config can't be read; ``<= 0`` = unlimited → None (never auto-skip).""" @@ -1905,8 +1916,8 @@ def _tool_progress_enabled(sid: str) -> bool: def _tool_lifecycle_required_for_ui(name: str) -> bool: - """Interactive UI, not optional chrome: Desktop renders clarify / setup_mcp cards from the tool-call part.""" - return name in ("clarify", "setup_mcp") + """Interactive UI, not optional chrome: Desktop renders clarify / connection cards from the tool-call part.""" + return name in ("clarify", "manage_connections", "setup_mcp") def _restart_slash_worker(sid: str, session: dict): diff --git a/website/docs/developer-guide/programmatic-integration.md b/website/docs/developer-guide/programmatic-integration.md index e74662d7f3..434489b107 100644 --- a/website/docs/developer-guide/programmatic-integration.md +++ b/website/docs/developer-guide/programmatic-integration.md @@ -86,7 +86,7 @@ Approvals, clarify questions, sudo/secret prompts, vault unlock, MCP setup and t → {"jsonrpc":"2.0","id":"srq-7","result":{"choice":"once"}} ``` -Methods: `approval` → `{choice}`; `clarify` → `{answer}` (single) or `{answers}` / `{}` cancel (batch, with `clarify.lock` to lock one answer early); `sudo`, `secret`, `vault.code`, `vault.unlock` → `{value}`; `mcp.setup` → `{result}`; `terminal.read`, `window.read`, `preview.act`, `tour` → `{value}` (JSON text). Respond with a JSON-RPC error (`-32601`) for a method your host does not implement so the agent fails fast instead of waiting out the timeout. +Methods: `approval` → `{choice}`; `clarify` → `{answer}` (single) or `{answers}` / `{}` cancel (batch, with `clarify.lock` to lock one answer early); `sudo`, `secret`, `vault.code`, `vault.unlock` → `{value}`; `connection` → `{settled_by, targets}` (the `manage_connections` card: one outcome per target); `terminal.read`, `window.read`, `preview.act`, `tour` → `{value}` (JSON text). Respond with a JSON-RPC error (`-32601`) for a method your host does not implement so the agent fails fast instead of waiting out the timeout. When the gateway withdraws a question (timeout, interrupt, answered from another surface) it emits `request.cancel` `{ id, method, reason }`; clear only the matching prompt. `session.resume` / `session.activate` results and `session.events.since` carry `open_requests` — the still-open frames — so a reconnecting client re-renders (and can still answer) them. diff --git a/website/docs/reference/tools-reference.md b/website/docs/reference/tools-reference.md index 0308f71fcb..e9e5a2f10f 100644 --- a/website/docs/reference/tools-reference.md +++ b/website/docs/reference/tools-reference.md @@ -56,6 +56,21 @@ Per-surface behavior: If the prompt times out part-way, answers the user already locked are kept: the tool result carries them plus `"timed_out": true`, with the unanswered entries left blank, so the agent can distinguish a deliberate skip from an absent user. +## `connections` toolset + +One tool for both kinds of external app. A target is a managed connector (`"gmail"` or +`{"name": "gmail"}`, authorized through the Nous gateway) or a local MCP server +(`{"name": "linear", "mcp": true}`, an entry in `mcp_servers`). + +| Tool | Description | Requires environment | +|------|-------------|----------------------| +| `manage_connections` | Managed actions: `status`, `connect` (returns a link for the user), `reconnect`, `wait` (blocks until the connectors report connected). MCP actions, for `mcp: true` targets only: `install` a catalog entry, `enable` a disabled configured server, `authorize` (OAuth). MCP actions show an approval card on the desktop and block until the user acts or the deadline passes; every target settles as `connected`, `skipped` or `not_connected`. On surfaces with no card (CLI, TUI, messaging) MCP targets return `unavailable` with the `hermes mcp install ` / `hermes mcp login ` commands to give the user. Cannot disconnect or revoke an account. | — | + +The deadline for one call is `connections.wait_timeout_seconds` in `config.yaml` +(default 120, floor 5). The backend fixes it when the call starts; reopening the chat +or restarting the desktop never extends it. Managed actions additionally need the +portal sign-in the managed tools use; MCP approvals do not. + ## `code_execution` toolset | Tool | Description | Requires environment | diff --git a/website/docs/reference/toolsets-reference.md b/website/docs/reference/toolsets-reference.md index 34e1d78e6f..b79fbe8f52 100644 --- a/website/docs/reference/toolsets-reference.md +++ b/website/docs/reference/toolsets-reference.md @@ -55,6 +55,7 @@ Or in-session: | `browser` | `browser_back`, `browser_cdp`, `browser_click`, `browser_console`, `browser_dialog`, `browser_get_images`, `browser_navigate`, `browser_press`, `browser_scroll`, `browser_snapshot`, `browser_type`, `browser_vision`, `web_search` | Core browser automation. Includes `web_search` as a fallback for quick lookups. `browser_cdp` and `browser_dialog` are gated at runtime — registered only when a CDP endpoint is reachable at session start (via `/browser connect`, `browser.cdp_url` config, Browserbase, or Camofox). `browser_dialog` works together with the `pending_dialogs` and `frame_tree` fields that `browser_snapshot` adds when a CDP supervisor is attached. | | `clarify` | `clarify` | Ask the user a question when the agent needs clarification. | | `code_execution` | `execute_code` | Run Python scripts that call Hermes tools programmatically. | +| `connections` | `manage_connections` | Connect the user to apps: managed connector accounts through the Nous gateway, and local MCP servers from the catalog (install / enable / authorize behind an approval card on the desktop). | | `coding` | composite (`file` + `terminal` + `search` + `web` + `skills` + `browser` + `todo` + `memory` + `session_search` + `clarify` + `code_execution` + `delegation` + `vision`) | Coding-focused bundle for software work: file editing, terminal, search, web docs, skills, browser, delegate, and code execution. | | `cronjob` | `cronjob` | Schedule and manage recurring tasks. | | `debugging` | composite (`file` + `terminal` + `web`) | Debug bundle — file, process/terminal, web extract/search. | diff --git a/website/docs/user-guide/configuration.md b/website/docs/user-guide/configuration.md index b73afaadd5..82b4eab119 100644 --- a/website/docs/user-guide/configuration.md +++ b/website/docs/user-guide/configuration.md @@ -878,6 +878,18 @@ The MCP threshold is always capped at the (possibly context-scaled) generic per- Hermes also flags **provider-side elision**: when an MCP or web tool result embeds its own truncation markers (`...N more items`, `"has_more": true`, "saved to sandbox" notes), a one-line notice is appended to the result warning that the visible data is incomplete and should be paged/fetched before treating any enumeration as complete. +## Connections + +```yaml +connections: + wait_timeout_seconds: 120 # floor 5, no ceiling +``` + +How long one `manage_connections` call may stay open: the managed-connector +`wait`, or the approval card for a local MCP install / enable / authorize on the +desktop. The backend fixes the deadline when the call starts; reopening the chat +or restarting the desktop never extends it. + ## Global Toolset Disable To suppress specific toolsets across the CLI and every gateway platform in one diff --git a/website/docs/user-guide/features/mcp.md b/website/docs/user-guide/features/mcp.md index 6dd9abc6a7..45d7364f0c 100644 --- a/website/docs/user-guide/features/mcp.md +++ b/website/docs/user-guide/features/mcp.md @@ -57,6 +57,11 @@ 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. + ```bash hermes mcp # interactive picker (default) hermes mcp catalog # plain-text list, scriptable diff --git a/website/docs/user-guide/features/tool-search.md b/website/docs/user-guide/features/tool-search.md index 2c6ac08e38..b72adc0475 100644 --- a/website/docs/user-guide/features/tool-search.md +++ b/website/docs/user-guide/features/tool-search.md @@ -168,10 +168,12 @@ described in the rest of this page, with no errors shown to the model. A connector call that needs an account you haven't linked returns a `CONNECTION_REQUIRED` error carrying a connect link. The `manage_connections` -tool (available on the same condition as the connector bridge) lists -connectors and their connection state, starts an authorization, and can wait -for the user to finish it; disconnecting an account is done by the user in -the Portal. +tool lists connectors and their connection state, starts an authorization, +and can wait for the user to finish it; disconnecting an account is done by +the user in the Portal. The same tool also installs, enables and authorizes +local MCP servers from the catalog (targets with `mcp: true`), so it is +present whether or not you are signed in; only the managed-connector actions +need the sign-in. `tool_call` accepts a batch: `calls` is an array of `{name, arguments}` entries (a single call is an array of one). Each connector entry in a batch