From e0ef0eb9c32a3341650c5a07690936a1a07a0ceb Mon Sep 17 00:00:00 2001 From: Siddharth Balyan <52913345+alt-glitch@users.noreply.github.com> Date: Tue, 15 Sep 2026 00:41:13 +0530 Subject: [PATCH] manage_connections covers local MCP servers; setup_mcp leaves the schema (NS-867, PR1) (#109517) * feat(connections): manage_connections covers local MCP servers; setup_mcp leaves the schema One model tool now connects the user to apps of both kinds. A target `{"name": "linear", "mcp": true}` is a locally configured MCP server; `install` / `enable` / `authorize` are its verbs. Bare strings and `{"name": ...}` stay managed connectors and that leg is unchanged. MCP targets run through one backend-owned connection operation (tools/connections_tool_operation.py): created with a server-side deadline from the new config key `connections.wait_timeout_seconds` (default 120, floor 5, no ceiling), per-target state, and exactly-once settlement (all resolved / Continue / deadline / interrupt). Unresolved targets freeze as `not_connected` with the settle reason. Why the fold works now: the approval card is reached through `agent.connection_callback` via the agent-level inline executor table, which is the only path that carries a GUI callback. Registry dispatch (every non-GUI surface) settles MCP targets as `unavailable` with the `hermes mcp install / login` hint; managed targets in the same call are unaffected. `setup_mcp` is removed from every advertised toolset and from the deferral list; an inline-table shim keeps calls from conversations opened before this change dispatching (prompt-cache protection). `_LEGACY_TOOL_ALIASES` is not the mechanism: inline tools bypass it. Gateway: `mcp.setup.request/respond` are replaced by `connection.request/respond/expire` (no wire compat; desktop ships with this). The bridge waits exactly the operation's deadline. The `session.resume` snapshot gains `pending_connection` so a reopened window restores the card with the original deadline. `manage_connections` joins `_SEQUENTIAL_DEADLINE_EXEMPT_TOOLS`: the operation owns its wait; the 420s guard must not report `tool_timeout` while the card is live. The portal `check_fn` on the tool is dropped in favour of a handler-level gate on the managed leg, so signed-out sessions can still approve local MCPs. * wip(desktop): connection.request store, resume restore, card routing for MCP targets Renderer half of the setup_mcp fold, first slice: connection-request store (mirrors clarify), connection.request/expire handling, pending_connection resume restore, mcpTargets() + isCardTool(name, args) so MCP-target manage_connections calls classify as cards. Not yet: the card component rewrite (mcp-setup-tool.tsx), mcp-directory.ts removal, vitest, docs. Does not typecheck until the card rewrite lands. * fix(config): hermes update turns on the connections toolset for saved toolset lists `hermes tools` writes an explicit `platform_toolsets.` list, and the resolver reads absence from that list as "unchecked". The `connections` toolset (#106842) shipped after most users last saved, so `manage_connections` is stripped from the schema on every install that ever opened the picker. The Nous entitlement gate never runs; the agent reports the tool as missing. Migration 44 -> 45 (renumbered when folded into #109517; main was already at 44) appends `connections` to each explicit per-platform list that lacks it and records the offer in `known_builtin_toolsets` where that record exists, so a later uncheck reads as a decline. It skips: platforms whose record already holds `connections` (the user saw the checkbox and left it off), bare composite lists ([hermes-cli]) that already inherit it, platforms where the toolset is not allowed, and any config whose `agent.disabled_toolsets` names `connections` (Blank Slate, `hermes tools --disable`), because the resolver subtracts that list last and the enable would never take effect. The explicit-list test is the resolver's own: any configurable or plugin key. `hermes update` runs migrations post-pull for the active profile and every sibling, so one update is enough. Fresh installs and composite users were never affected. * refactor: anti-slop pass on the desktop slice; shorten added comments Parse connection.request at the boundary with a typed wire interface instead of unknown + typeof; mcpTargets reuses connectorText; comments cut to one or two lines. slop-ratchet: no net-new findings in 13 touched files. * feat(desktop): the MCP approval card answers manage_connections; MCP Directory removed The existing card (mcp-setup-tool.tsx) now reads the connection-request store, renders for manage_connections calls with mcp:true targets, answers through connection.respond with a per-target outcome, and no longer calls reload.mcp after Install; the new server's tools arrive on the between-turns refresh. A settled operation renders the first target's frozen state. session.resume restores a pending card with its original deadline on both the activate and cold-resume paths. lib/mcp-directory.ts is deleted along with its two fallback branches (suggestion provider, card install). The catalog was already primary in both; a catalog miss now yields no suggestion / a notInCatalog error. The GitHub never-suggest test is rewritten on catalog-shaped data. vitest: connection-request store (6), suggestion provider, clarify restore. slop-ratchet: no net-new findings in 19 touched files. * chore: drop __pycache__ files swept in by an over-broad git add * fix(desktop): correlate the connection.request row with the model's tool call by reason The synthetic row from connection.request and the tool.start row carried different ids and no shared match value (op_id is not in the model's args), so the card mounted twice. reason is the arg both sides carry. * docs: manage_connections covers local MCP servers; connections.wait_timeout_seconds * fix(connections): settle reason derives from target state, never from the renderer A card that answers one of two targets and claims all_resolved must settle as continue with the other target not_connected; found live with a two-target call. * fix(desktop): a pending connection card re-arms on resume and activate The store entry was restored but the transcript row was not, so navigating away and back (or reloading) lost the card while the backend kept waiting. restorePendingClarifyToolCall's core is generalized to any blocking tool name and both resume paths project the connection row through it. Verified live: card restored after navigate-away and after a full renderer reload, deadline_at unchanged, approve settles connected. * style: literal wording in added comments, docstrings and docs * fix: shared gateway-event contract and config-schema category for the connection events connection.request/expire replace mcp.setup.* in apps/shared gateway-events (json list, BACKEND_EVENT_NAMES, GatewayEventMap) so the renderer's event union includes them and the tui_gateway contract test passes. The new `connections` config section folds into the agent tab like the other single-field sections. * style: import order (perfectionist) in the desktop and shared files this PR touches * chore: retrigger CI (zero-job dispatch failure, auto-heal) --- agent/agent_init.py | 4 +- agent/agent_runtime_helpers.py | 3 +- agent/inline_tool_executors.py | 27 +- agent/tool_executor.py | 4 +- .../composer/hooks/use-composer-submit.ts | 9 +- .../gateway-event/input-requests.ts | 10 +- .../gateway-event/server-requests.test.ts | 34 ++- .../gateway-event/server-requests.ts | 34 +-- .../hooks/use-session-actions/index.ts | 4 +- .../restore-pending-connection.ts | 23 ++ .../assistant-ui/mcp-setup-tool.tsx | 190 ++++++-------- .../assistant-ui/thread/message-parts.tsx | 11 +- .../assistant-ui/tool/fallback-model/index.ts | 4 +- .../components/assistant-ui/tool/fallback.tsx | 8 +- apps/desktop/src/lib/chat-messages/index.ts | 1 + .../src/lib/chat-messages/tool-parts.ts | 24 +- apps/desktop/src/lib/chat-messages/types.ts | 6 +- apps/desktop/src/lib/connector-tools.ts | 29 +++ apps/desktop/src/lib/gateway-events.ts | 1 - apps/desktop/src/lib/mcp-directory.ts | 189 -------------- apps/desktop/src/lib/render-weight.ts | 2 +- apps/desktop/src/lib/tool-render-class.ts | 17 +- .../src/store/connection-request.test.ts | 119 +++++++++ apps/desktop/src/store/connection-request.ts | 148 +++++++++++ apps/desktop/src/store/mcp-setup.ts | 112 --------- .../store/suggestion-providers/mcp.test.ts | 14 +- .../src/store/suggestion-providers/mcp.ts | 21 +- apps/shared/src/gateway-contract.generated.ts | 41 ++- apps/shared/src/gateway-contract.openrpc.json | 230 +++++++++++++---- cli-config.yaml.example | 7 + evals/core_tool_deferral/tasks.py | 15 +- evals/core_tool_deferral/worker.py | 10 +- hermes_cli/config_defaults.py | 6 +- hermes_cli/config_migrations.py | 49 ++++ hermes_cli/web_server_config.py | 1 + run_agent.py | 2 +- tests/agent/test_run_agent.py | 5 + ...est_sequential_deadline_delegate_exempt.py | 6 + .../test_config_migration_45_connections.py | 160 ++++++++++++ tests/tools/test_connections_tool.py | 33 ++- tests/tools/test_connections_tool_mcp.py | 238 ++++++++++++++++++ .../tools/test_connections_tool_operation.py | 79 ++++++ tests/tools/test_connector_local_batches.py | 27 +- tests/tools/test_desktop_tools_diet.py | 2 +- tests/tools/test_setup_mcp_tool.py | 83 ------ .../test_connection_server_request.py | 58 +++++ .../tui_gateway/test_gui_surface_toolsets.py | 1 - tools/connections_tool.py | 109 +++++--- tools/connections_tool_mcp.py | 222 ++++++++++++++++ tools/connections_tool_operation.py | 169 +++++++++++++ tools/setup_mcp_tool.py | 108 -------- tools/tool_search.py | 2 +- toolsets.py | 2 +- tui_gateway/AGENTS.md | 2 +- tui_gateway/agent_callbacks.py | 8 +- tui_gateway/contracts/server_requests.py | 73 +++++- tui_gateway/server.py | 15 +- .../programmatic-integration.md | 2 +- website/docs/reference/tools-reference.md | 15 ++ website/docs/reference/toolsets-reference.md | 1 + website/docs/user-guide/configuration.md | 12 + website/docs/user-guide/features/mcp.md | 5 + .../docs/user-guide/features/tool-search.md | 10 +- 63 files changed, 2003 insertions(+), 853 deletions(-) create mode 100644 apps/desktop/src/app/session/hooks/use-session-actions/restore-pending-connection.ts delete mode 100644 apps/desktop/src/lib/mcp-directory.ts create mode 100644 apps/desktop/src/store/connection-request.test.ts create mode 100644 apps/desktop/src/store/connection-request.ts delete mode 100644 apps/desktop/src/store/mcp-setup.ts create mode 100644 tests/hermes_cli/test_config_migration_45_connections.py create mode 100644 tests/tools/test_connections_tool_mcp.py create mode 100644 tests/tools/test_connections_tool_operation.py delete mode 100644 tests/tools/test_setup_mcp_tool.py create mode 100644 tests/tui_gateway/test_connection_server_request.py create mode 100644 tools/connections_tool_mcp.py create mode 100644 tools/connections_tool_operation.py delete mode 100644 tools/setup_mcp_tool.py 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