diff --git a/agent/inline_tool_executors.py b/agent/inline_tool_executors.py index 3e15f8a0b7..b0b8a035aa 100644 --- a/agent/inline_tool_executors.py +++ b/agent/inline_tool_executors.py @@ -156,7 +156,7 @@ def _manage_connections(agent, args: dict, ctx: InlineToolContext) -> Any: from tools.connectors.gateway import config as gateway_config return manage_connections( - args, session_id=getattr(agent, "session_id", None), + args, session_id=getattr(agent, "session_id", None), tool_call_id=ctx.tool_call_id, connection_callback=getattr(agent, "connection_callback", None), connectors_available=gateway_config.connectors_available, ) @@ -168,7 +168,6 @@ def _setup_mcp_shim(agent, args: dict, ctx: InlineToolContext) -> Any: return _manage_connections(agent, { "action": args.get("action", "install"), "connectors": [{"name": args.get("server", ""), "mcp": True}], - "reason": args.get("reason", ""), }, ctx) diff --git a/agent/tool_executor.py b/agent/tool_executor.py index 91ba64c541..fa73ec6b62 100644 --- a/agent/tool_executor.py +++ b/agent/tool_executor.py @@ -786,7 +786,7 @@ 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). -# ``manage_connections`` waits on connections.wait_timeout_seconds; the generic deadline +# ``manage_connections`` waits on the connection operation's own deadline; the generic deadline # would return tool_timeout while its approval card is still open. _SEQUENTIAL_DEADLINE_EXEMPT_TOOLS = frozenset({"delegate_task", "manage_connections"}) 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 6030517a19..56aab46b87 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,12 @@ +import type { ConnectionRequestPayload, ConnectionUpdatePayload, GatewayEvent } from '@hermes/shared' + 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 { settlePendingClarifyToolCall } from '@/lib/chat-messages' import { $clarifyRequests, clearClarifyRequest } from '@/store/clarify' -import { $connectionRequests, clearConnectionRequest } from '@/store/connection-request' +import { normalizeConnectionRequest, setConnectionRequest, updateConnectionRequest } from '@/store/connection-request' +import { dispatchNativeNotification } from '@/store/native-notifications' import { $approvalRequests, $secretRequests, @@ -20,6 +25,15 @@ import { forgetServerRequest } from '@/store/server-requests' import type { GatewayEventContext } from './types' +type ConnectionRequestEvent = GatewayEvent<'connection.request'> & { payload: ConnectionRequestPayload } +type ConnectionUpdateEvent = GatewayEvent<'connection.update'> & { payload: ConnectionUpdatePayload } + +const isConnectionRequestEvent = (event: GatewayEvent): event is ConnectionRequestEvent => + event.type === 'connection.request' && event.payload !== undefined + +const isConnectionUpdateEvent = (event: GatewayEvent): event is ConnectionUpdateEvent => + event.type === 'connection.update' && event.payload !== undefined + /** The blocking-input family arrives as server→client REQUESTS (see * `server-requests.ts`); the one EVENT in the family is `request.cancel`, the * backend withdrawing an open request (timeout / interrupt / session close): @@ -29,6 +43,39 @@ import type { GatewayEventContext } from './types' export function handleInputRequestEvent(ctx: GatewayEventContext): boolean { const { deps, event, payload, sessionId, occurredAt } = ctx + if (isConnectionRequestEvent(event)) { + // Park per-session and upsert a stable tool row so the card renders even if tool.start was missed. + const request = normalizeConnectionRequest(event.payload, sessionId ?? null) + + if (request) { + setConnectionRequest(request) + + if (sessionId) { + deps.upsertToolCall(sessionId, connectionRequestToolPayload(request), 'running') + deps.updateSessionState(sessionId, state => ({ ...state, needsInput: true })) + } + + dispatchNativeNotification({ + body: request.targets.map(target => target.name).join(', '), + kind: 'input', + sessionId, + title: translateNow('notifications.native.inputTitle') + }) + } + + return true + } + + if (isConnectionUpdateEvent(event)) { + updateConnectionRequest(sessionId ?? null, event.payload) + + if (event.payload.settled && sessionId) { + deps.updateSessionState(sessionId, state => ({ ...state, needsInput: false })) + } + + return true + } + if (event.type !== 'request.cancel') { return false } @@ -81,12 +128,6 @@ export function handleInputRequestEvent(ctx: GatewayEventContext): boolean { clearVaultSaveLoginRequest(sessionId, id) } else if ($vaultUnlockRequests.get()[key]?.requestId === id) { clearVaultUnlockRequest(sessionId, id) - } 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 aaf9fac978..2497dfd408 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,8 +1,6 @@ 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' @@ -24,26 +22,22 @@ function deliver(method: string, params: Record, activeSessionI } describe('connection request routing', () => { - afterEach(() => { - $connectionRequests.set({}) - resetServerRequestsForTests() - }) + it('does not route connection operations through the server-request rail', () => { + const { handled, respond } = deliver( + 'connection', + { + deadline_at: 1_800_000_000, + op_id: 'op-1', + session_id: 'session-a', + targets: [{ action: 'install', kind: 'mcp', name: 'linear' }], + timeout_seconds: 60, + tool_call_id: 'call-1' + }, + 'session-a' + ) - 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') + expect(handled).toBe(false) + expect(respond).not.toHaveBeenCalled() }) }) 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 37df0e2811..e13b58edb8 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,15 +1,11 @@ -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 { dispatchNativeNotification } from '@/store/native-notifications' import { @@ -44,11 +40,6 @@ 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) : '' }) @@ -265,27 +256,6 @@ const vaultUnlockPrompt: Handler = ctx => { notifyInput(ctx, translateNow('prompts.vaultUnlockTitle', displayName)) } -const connection: Handler = ctx => { - const { deps, request, sessionId } = ctx - const entry = normalizeConnectionRequest(paramsOf(request, 'connection'), request.id, sessionId || null) - - if (!entry) { - request.respond({ settled_by: 'all_resolved', targets: [] }) - - return - } - - rememberServerRequest(request) - setConnectionRequest(entry) - - if (sessionId) { - deps.upsertToolCall(sessionId, connectionRequestToolPayload(entry), 'running') - } - - markNeedsInput(ctx) - notifyInput(ctx, entry.reason || entry.targets.map(target => target.name).join(', ')) -} - // ── Desktop-surface bridges (answered immediately, no card) ───────────────── const terminalRead: Handler = ({ request }) => { @@ -401,7 +371,6 @@ const tour: Handler = ({ isActiveSession, request, sessionId }) => { export const SERVER_REQUEST_HANDLERS: Record = { approval, clarify, - 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 a8036b3871..138c6c77bb 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 @@ -28,6 +28,7 @@ import { setSessionYolo } from '@/lib/yolo-session' import { $clarifyRequests } from '@/store/clarify' import { migrateSessionDraft } from '@/store/composer' import { clearQueuedPrompts, migrateQueuedPrompts } from '@/store/composer-queue' +import { $connectionRequests } from '@/store/connection-request' import { openGatewayForAgent, openGatewayForProfile, @@ -140,6 +141,7 @@ import { singleFlightSessionResume } from '../use-prompt-actions/single-flight-r import { sessionCreateOverrideParams, type SessionCreateOverrides, type SessionSeedMessage } from './create-overrides' import { pendingClarifyToolPayload, restorePendingClarifyFromSnapshot } from './restore-pending-clarify' +import { projectPendingConnection, restorePendingConnectionFromSnapshot } from './restore-pending-connection' import { createPersistedDisplayTranscriptProvenance, hasPersistedDisplayTranscriptProvenance, @@ -338,6 +340,15 @@ interface FreshSessionDraftOptions { workspaceTarget?: NewChatWorkspaceTarget } +/** Session-state patch for a restored blocking prompt row; the first non-null projection is used. */ +function livePromptStreamId( + ...projections: ({ streamId: string } | null)[] +): { awaitingResponse: false; sawAssistantPayload: true; streamId: string } | Record { + const live = projections.find(Boolean) + + return live ? { awaitingResponse: false, sawAssistantPayload: true, streamId: live.streamId } : {} +} + function restorePendingApproval(response: SessionResumeResult, sessionId: string): boolean { const pending = response.pending_approval @@ -1201,6 +1212,7 @@ export function useSessionActions({ const activateStartedAt = Date.now() / 1000 const activateBaselineState = sessionStateByRuntimeIdRef.current.get(cachedRuntimeId) ?? cachedViewState const clarifyRequestIdAtActivateStart = $clarifyRequests.get()[cachedRuntimeId]?.requestId + const connectionOpIdAtActivateStart = $connectionRequests.get()[cachedRuntimeId]?.opId try { activated = await requestForSession('session.activate', { @@ -1251,6 +1263,13 @@ export function useSessionActions({ const pendingClarify = pendingClarifyState.request + const pendingConnection = restorePendingConnectionFromSnapshot( + activated, + cachedRuntimeId, + activateStartedAt, + connectionOpIdAtActivateStart + ).request + const clarifyAuthoritativelyAbsent = pendingClarifyState.authoritativeAbsent && !$clarifyRequests.get()[cachedRuntimeId] @@ -1316,6 +1335,7 @@ export function useSessionActions({ needsInput: pendingApproval || Boolean(pendingClarify) || + Boolean(pendingConnection) || (clarifyAuthoritativelyAbsent ? false : state.needsInput), // Adopting someone else's turn: we'll stream its reply // without ever having received its prompt, so the settle @@ -1426,8 +1446,16 @@ export function useSessionActions({ ) : null + const pendingConnectionProjection = projectPendingConnection( + pendingClarifyProjection?.messages ?? clearedClarifyProjection?.messages ?? activatedMessages, + pendingConnection + ) + const visibleActivatedMessages = - pendingClarifyProjection?.messages ?? clearedClarifyProjection?.messages ?? activatedMessages + pendingConnectionProjection?.messages ?? + pendingClarifyProjection?.messages ?? + clearedClarifyProjection?.messages ?? + activatedMessages releaseTranscriptView() @@ -1450,13 +1478,7 @@ export function useSessionActions({ acceptedPersistedDisplayTranscript || hasValidProvenance ? (expectedProvenance ?? undefined) : undefined, - ...(pendingClarifyProjection - ? { - awaitingResponse: false, - sawAssistantPayload: true, - streamId: pendingClarifyProjection.streamId - } - : {}), + ...(livePromptStreamId(pendingConnectionProjection, pendingClarifyProjection)), ...(clearedClarifyProjection ? { streamId: state.busy ? (clearedClarifyProjection.streamId ?? state.streamId) : null @@ -1806,6 +1828,8 @@ export function useSessionActions({ const pendingApproval = restorePendingApproval(resumed, resumed.session_id) const pendingClarifyState = restorePendingClarifyFromSnapshot(resumed, resumed.session_id, resumeStartedAt) const pendingClarify = pendingClarifyState.request + const pendingConnection = restorePendingConnectionFromSnapshot(resumed, resumed.session_id, resumeStartedAt).request + const clarifyAuthoritativelyAbsent = pendingClarifyState.authoritativeAbsent && !$clarifyRequests.get()[resumed.session_id] @@ -1833,8 +1857,16 @@ export function useSessionActions({ ) : null + const pendingConnectionProjection = projectPendingConnection( + pendingClarifyProjection?.messages ?? clearedClarifyProjection?.messages ?? messagesForView, + pendingConnection + ) + const visibleMessagesForView = - pendingClarifyProjection?.messages ?? clearedClarifyProjection?.messages ?? messagesForView + pendingConnectionProjection?.messages ?? + pendingClarifyProjection?.messages ?? + clearedClarifyProjection?.messages ?? + messagesForView // The eagerly painted REST page is persisted-display authority: stamp // its provenance so the next warm switch to this session paints it @@ -1861,7 +1893,9 @@ export function useSessionActions({ turnLive: state.turnLive || resumedRunning, needsInput: pendingApproval || - Boolean(pendingClarify) || (clarifyAuthoritativelyAbsent ? false : state.needsInput), + Boolean(pendingClarify) || + Boolean(pendingConnection) || + (clarifyAuthoritativelyAbsent ? false : state.needsInput), adoptedRunningTurn: state.adoptedRunningTurn || resumedRunning, ...(inFlightRecovery.applied ? { @@ -1874,13 +1908,7 @@ export function useSessionActions({ : { turnStartedAt: resumedRunning && resumedTurnStartedAt !== null ? resumedTurnStartedAt : null }), - ...(pendingClarifyProjection - ? { - awaitingResponse: false, - sawAssistantPayload: true, - streamId: pendingClarifyProjection.streamId - } - : {}), + ...(livePromptStreamId(pendingConnectionProjection, pendingClarifyProjection)), ...(clearedClarifyProjection ? { streamId: resumedRunning ? (clearedClarifyProjection.streamId ?? state.streamId) : null diff --git a/apps/desktop/src/app/session/hooks/use-session-actions/restore-pending-connection.ts b/apps/desktop/src/app/session/hooks/use-session-actions/restore-pending-connection.ts index c994aee7ea..7e88599326 100644 --- a/apps/desktop/src/app/session/hooks/use-session-actions/restore-pending-connection.ts +++ b/apps/desktop/src/app/session/hooks/use-session-actions/restore-pending-connection.ts @@ -1,16 +1,58 @@ import { type ChatMessage, type GatewayEventPayload, restorePendingBlockingToolCall } from '@/lib/chat-messages' -import type { ConnectionRequest } from '@/store/connection-request' +import { + $connectionRequests, + clearConnectionRequest, + type ConnectionRequest, + normalizeConnectionRequest, + setConnectionRequest +} from '@/store/connection-request' +import type { SessionResumeResult } from '@/types/hermes' + +export interface PendingConnectionResumeState { + authoritativeAbsent: boolean + cleared: ConnectionRequest | null + request: ConnectionRequest | null +} + +/** Restore a pending connection card from a resume snapshot. A missing snapshot clears only + * requests that existed before the RPC started. */ +export function restorePendingConnectionFromSnapshot( + response: Pick, + sessionId: string, + resumeStartedAt: number, + opIdAtStart?: string +): PendingConnectionResumeState { + const request = normalizeConnectionRequest(response.pending_connection, sessionId) + + if (!request) { + const current = $connectionRequests.get()[sessionId] + + const existedAtStart = Boolean(current && opIdAtStart && current.opId === opIdAtStart) + const definitelyOlder = Boolean(current?.receivedAt !== undefined && current.receivedAt < resumeStartedAt) + + if (current && (existedAtStart || definitelyOlder)) { + clearConnectionRequest(current.opId, sessionId) + + return { authoritativeAbsent: true, cleared: current, request: null } + } + + return { authoritativeAbsent: true, cleared: null, request: null } + } + + setConnectionRequest(request) + + return { authoritativeAbsent: false, cleared: null, 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 + action: request.targets[0]?.action ?? (request.targets[0]?.kind === 'connector' ? 'connect' : 'install'), + connectors: request.targets.map(target => ({ mcp: target.kind === 'mcp', name: target.name })) }, name: 'manage_connections', - tool_id: request.requestId + tool_id: request.toolCallId } } diff --git a/apps/desktop/src/app/session/hooks/use-session-actions/utils.ts b/apps/desktop/src/app/session/hooks/use-session-actions/utils.ts index 5e9322c3e4..0aaf5430f0 100644 --- a/apps/desktop/src/app/session/hooks/use-session-actions/utils.ts +++ b/apps/desktop/src/app/session/hooks/use-session-actions/utils.ts @@ -751,7 +751,7 @@ type LiveSessionProjection = Pick + + + ) +} + +function renderConnector(request = REQUEST) { + setSessionOwnerHint(SESSION_ID, OWNER) + setConnectionRequest(request) + + return render( + + + + + + ) +} + +afterEach(() => { + cleanup() + $connectionRequests.set({}) + $gateway.set(null) + _resetSessionOwnerHintsForTests({ storage: true }) + vi.useRealTimers() + vi.clearAllMocks() +}) + +describe('ConnectorTool operation card', () => { + it('does not call connectors.list or create a timer on mount', () => { + vi.useFakeTimers() + const request = vi.fn() + // SAFETY: the store calls only `request`; the rest of the client is never touched in these tests. + $gateway.set({ request } as never) + + renderOffer() + + expect(request).not.toHaveBeenCalledWith('connectors.list', expect.anything()) + expect(vi.getTimerCount()).toBe(0) + }) + + it('opens the stored link from Connect on a waiting row, without an RPC', async () => { + const request = vi.fn() + // SAFETY: the store calls only `request`; the rest of the client is never touched in these tests. + $gateway.set({ request } as never) + const openExternal = vi.fn() + // SAFETY: the card reads only `openExternal` from the preload bridge. + window.hermesDesktop = { openExternal } as never + + renderConnector({ ...REQUEST, targets: [{ ...REQUEST.targets[0], state: 'initiated' }] }) + + const connect = await waitFor(() => screen.getByRole('button', { name: 'Connect' })) + expect(connect.hasAttribute('disabled')).toBe(false) + fireEvent.click(connect) + + expect(openExternal).toHaveBeenCalledWith('https://connect.example/gmail') + expect(request).not.toHaveBeenCalledWith('connectors.connect', expect.anything()) + }) + + it('sends Not now as a per-target skipped response', async () => { + const request = vi.fn().mockResolvedValue({ status: 'ok' }) + // SAFETY: the store calls only `request`; the rest of the client is never touched in these tests. + $gateway.set({ request } as never) + + renderConnector() + + await waitFor(() => { + expect(screen.getByRole('button', { name: 'Not now' })).toBeTruthy() + }) + fireEvent.click(screen.getByRole('button', { name: 'Not now' })) + + await waitFor(() => { + expect(request).toHaveBeenCalledWith('connection.respond', { + op_id: 'operation-1', + result: { targets: [{ name: 'gmail', status: 'skipped' }] }, + session_id: SESSION_ID + }) + }) + }) + + it('never binds to a tool row from a different call, even for the same apps', () => { + // A second connect for gmail opens a new operation on a new tool_call_id. The old row must stay + // dead: it is matched by id only, never by connector names. + expect(connectionRequestOwnsPart(props(), { ...REQUEST, opId: 'operation-2', toolCallId: 'connector-call-2' })).toBe(false) + expect(connectionRequestOwnsPart(props(), REQUEST)).toBe(true) + }) + + it('renders settled operations with no live controls', () => { + renderOffer({ ...REQUEST, settled: true, settledBy: 'all_resolved' }) + + // ScaffoldRow paints every settled tool row as a disabled disclosure button; the contract is + // that nothing is actionable: no enabled button, no Connect / Not now / Continue. + const buttons = [...window.document.querySelectorAll('[data-connector-offer] button')] + + expect(buttons.every(button => button.hasAttribute('disabled'))).toBe(true) + expect(screen.queryByRole('button', { name: 'Not now' })).toBeNull() + expect(screen.queryByRole('button', { name: 'Continue' })).toBeNull() + }) +}) diff --git a/apps/desktop/src/components/assistant-ui/connector-tool.tsx b/apps/desktop/src/components/assistant-ui/connector-tool.tsx index 24dafa891c..907ad6f51e 100644 --- a/apps/desktop/src/components/assistant-ui/connector-tool.tsx +++ b/apps/desktop/src/components/assistant-ui/connector-tool.tsx @@ -1,130 +1,103 @@ import type { ToolCallMessagePartProps } from '@assistant-ui/react' +import type { ConnectionTargetState } from '@hermes/shared' import { useStore } from '@nanostores/react' -import { useEffect, useMemo, useRef, useState } from 'react' +import { useEffect, useMemo, useState } from 'react' -import { requestComposerSubmit } from '@/app/chat/composer/focus' import { useSessionView } from '@/app/chat/session-view' -import { isFirstBuildSession } from '@/app/contrib/handoff-receipt' import { resolveSessionOwner } from '@/app/session/hooks/use-session-actions/utils' -import { FirstBuildConnectorOffer } from '@/components/assistant-ui/first-build-connectors' import { ToolFallback } from '@/components/assistant-ui/tool/fallback' import { Button } from '@/components/ui/button' -import { ConnectorCard, type ConnectorCardCopy } from '@/components/ui/connector-card' -import { Loader } from '@/components/ui/loader' -import { SearchField } from '@/components/ui/search-field' +import { + ConnectorCard, + type ConnectorCardCopy, + type ConnectorCardOutcome, + type ConnectorCardState, + ConnectorSummary, + outcomeMeta +} from '@/components/ui/connector-card' import { useI18n } from '@/i18n' -import { connectionRows, connectorCalls, connectorTitle, connectorToolName, recordOf } from '@/lib/connector-tools' -import { cn } from '@/lib/utils' -import { createConnectorFlow } from '@/store/connector-flow' +import { connectorCalls, connectorText, connectorTitle, connectorToolName, recordOf } from '@/lib/connector-tools' +import { + type ConnectionRequest, + type ConnectionTarget, + continueConnectionRequest, + sessionConnectionRequest, + skipConnectionTarget +} from '@/store/connection-request' import { requestGatewayForAgent } from '@/store/gateway' import { $activeGatewayProfile } from '@/store/profile' import { assertSessionOwnerResolved } from '@/store/session-owner-resolution' import { isSessionOwnerRoute } from '@/store/session-request-router' +interface ConnectorOwner { + connectionId: null | string + profile: string +} + +/** Names requested by a manage_connections part, including an event-projected row. */ +function requestedConnectorNames(args: ToolCallMessagePartProps['args']): string[] { + const connectors = recordOf(args).connectors + const entries = Array.isArray(connectors) ? connectors : [connectors] + + return entries.flatMap(entry => { + const row = recordOf(entry) + const name = connectorText(entry) ?? connectorText(row.name) ?? connectorText(row.connector) + const trimmed = name?.trim() + + return trimmed ? [trimmed] : [] + }) +} + +function matchingTargetNames(left: readonly string[], right: readonly string[]): boolean { + if (left.length !== right.length) { + return false + } + + const leftSorted = [...left].sort() + const rightSorted = [...right].sort() + + return leftSorted.every((name, index) => name === rightSorted[index]) +} + +/** The card lives on the tool row whose id opened the operation and on no other. */ +export function connectionRequestOwnsPart(props: ToolCallMessagePartProps, request: ConnectionRequest | null): boolean { + return Boolean(request && props.toolCallId === request.toolCallId) +} + export function ConnectorTool(props: ToolCallMessagePartProps) { const view = useSessionView() const runtimeId = useStore(view.$runtimeId) const storedId = useStore(view.$storedId) - const messages = useStore(view.$messages) - const firstBuild = isFirstBuildSession(storedId) - - // One live card per offer. Every manage_connections call renders through - // here, but only one of them is the card the user acts on; the rest render - // as settled tool rows. Consecutive calls naming the same apps are one - // exchange: connect, the wait the agent stays in while the user signs in, - // and the status it runs once the connection is active. The card is the - // first call of the last exchange. Using the newest call would turn the - // card into a row during authorization and create a new card below it. A - // catalog listing (status with nothing named) after a targeted ask never - // starts an exchange; it reads state and offers nothing. - const offers = messages - .flatMap(message => message.parts) - .filter( - part => - part.type === 'tool-call' && - (part.toolName === 'manage_connections' || connectorCalls(part.toolName, part.args).length > 0) - ) - - const keyOf = (part: (typeof offers)[number]) => - part.type === 'tool-call' - ? connectionRows(part.args, part.result) - .map(row => row.connector) - .sort() - .join('|') - : '' - - const targeted = (part: (typeof offers)[number]) => { - if (part.type !== 'tool-call') { - return false - } - - const asked = recordOf(part.args).connectors - - return Array.isArray(asked) && asked.length > 0 - } - - let liveId: string | undefined - let liveKey: string | null = null - let sawTargeted = false - - for (const part of offers) { - if (part.type !== 'tool-call') { - continue - } - - const key = keyOf(part) - - if (sawTargeted && !targeted(part)) { - continue - } - - sawTargeted ||= targeted(part) - - if (key !== liveKey) { - liveKey = key - liveId = part.toolCallId - } - } - - const historical = liveId !== props.toolCallId - // A status call with no target list describes the whole catalog. It answers - // the model's question, so it renders as a tool row; as cards it would put a - // Connect button on every app the gateway knows. - const input = recordOf(props.args) + const $request = useMemo(() => sessionConnectionRequest(runtimeId), [runtimeId]) + const request = useStore($request) + const targetNames = requestedConnectorNames(props.args) const untargetedStatus = props.toolName === 'manage_connections' && - (input.action ?? 'status') === 'status' && - !(Array.isArray(input.connectors) && input.connectors.length > 0) + (recordOf(props.args).action ?? 'status') === 'status' && + targetNames.length === 0 - // Neither kind of part is the live offer, so neither resolves a session - // owner nor polls the gateway. - const inert = historical || untargetedStatus - - const [owner, setOwner] = useState<{ - storedId: string - runtimeId: string - connectionId: null | string - profile: string - } | null>(null) - - const [ownerFailure, setOwnerFailure] = useState(null) + const live = !untargetedStatus && connectionRequestOwnsPart(props, request) + // Owner routes and hints are keyed by the stored id, not the runtime id the events carry. + const ownerSessionId = storedId + const [owner, setOwner] = useState(null) useEffect(() => { - if (!storedId || !runtimeId || inert) { + if (!ownerSessionId || !live) { + setOwner(null) + return } let cancelled = false const ambientProfile = $activeGatewayProfile.get() - void resolveSessionOwner(storedId) + + void resolveSessionOwner(ownerSessionId) .then(scope => { - assertSessionOwnerResolved(scope, { method: 'connectors.list', sessionId: storedId }) + assertSessionOwnerResolved(scope, { method: 'connectors.connect', sessionId: ownerSessionId }) if (!cancelled) { setOwner({ - storedId, - runtimeId, connectionId: isSessionOwnerRoute(scope) ? scope.connectionId : null, profile: isSessionOwnerRoute(scope) ? scope.profile : scope || ambientProfile }) @@ -133,122 +106,131 @@ export function ConnectorTool(props: ToolCallMessagePartProps) { .catch(() => { if (!cancelled) { setOwner(null) - setOwnerFailure(`${storedId}:${runtimeId}`) } }) return () => { cancelled = true } - }, [storedId, runtimeId, inert]) - const rows = connectionRows(props.args, props.result) - const signature = rows.map(row => row.connector).join('|') - const target = view.kind === 'tile' ? `tile:${storedId}` : 'main' + }, [live, ownerSessionId]) - // The same shape as the TUI: the agent stays inside manage_connections - // action="wait", which blocks the turn and polls the gateway, instead of - // deciding what "not connected" means and building around the app. Each - // card action sends one hidden line so the agent takes the right next call. - // Read through a ref so the flow, memoised on identity, always submits to - // the current composer target rather than the one it was built with. The - // composer handles busy: a hidden request mid-turn steers or queues there. - const nudgeRef = useRef((_text: string) => {}) - - nudgeRef.current = (text: string) => { - requestComposerSubmit(`[connectors] ${text}`, { displayKind: 'hidden', target }) - } - - const flow = useMemo(() => { - if (firstBuild || inert || !runtimeId || !owner || owner.storedId !== storedId || owner.runtimeId !== runtimeId) { - return null - } - - const seeds = signature ? signature.split('|').map(connector => ({ connector })) : [] - - return createConnectorFlow(runtimeId, seeds, { - request: (method, params) => requestGatewayForAgent(owner.connectionId, owner.profile, method, params, 45000), - open: async url => { - if (!window.hermesDesktop?.openExternal) { - throw new Error('System browser unavailable') - } - - await window.hermesDesktop.openExternal(url) - }, - onWaiting: slug => - nudgeRef.current( - `The user clicked Connect for ${connectorTitle(slug)} and the sign-in is open in their browser. Call manage_connections action="wait" connectors=["${slug}"] now and hold there until it reports connected. Do NOT call connect again — a second link cancels the one they are signing in with. Say nothing until wait returns.` - ) - }) - }, [runtimeId, owner, storedId, signature, inert, firstBuild]) - - const { t } = useI18n() - // Ordinary sessions require a click to begin authorization. - useEffect(() => { - if (!flow) { - return - } - - return () => flow.dispose() - }, [flow]) - useEffect(() => { - if (flow) { - void flow.refresh() - } - }, [flow, props.result]) - - if (inert) { + if (!live || !request) { return } - if (firstBuild && storedId && owner?.storedId === storedId && owner.runtimeId === runtimeId) { - return ( - - ) - } - - if (!flow) { - return ( -

- {ownerFailure === `${storedId}:${runtimeId}` ? t.connectors.ownerMissing : t.connectors.checking} -

- ) - } - - return ( - - nudgeRef.current( - `The user chose Not now for ${connectorTitle(slug)}. Do not connect it, do not route around it with another client, credential or CLI for the same app. Continue the task without it, or ask what they want to do.` - ) - } - /> - ) + return owner ? : null } +type ConnectorCopy = ReturnType['t']['connectors'] +type ConnectorAction = 'none' | 'open' | 'reissue' + +interface ConnectorCardPhase { + action: ConnectorAction + cardState: ConnectorCardState + dismissed: boolean + outcome: (target: ConnectionTarget, copy: ConnectorCopy) => ConnectorCardOutcome | undefined + phase: (copy: ConnectorCopy) => string | undefined + requiresUrl: boolean + unresolved: boolean +} + +const noOutcome = (): undefined => undefined +const noPhase = (): undefined => undefined + +const errorOutcome = (target: ConnectionTarget, copy: ConnectorCopy): ConnectorCardOutcome => ({ + detail: target.detail || copy.failed, + status: 'error' +}) + +const connectedOutcome = (target: ConnectionTarget): ConnectorCardOutcome => ({ + status: 'connected', + tools: target.tools +}) + +const CONNECTOR_CARD_PHASES = { + connected: { + action: 'none', + cardState: 'connected', + dismissed: false, + outcome: connectedOutcome, + phase: noPhase, + requiresUrl: false, + unresolved: false + }, + expired: { + action: 'reissue', + cardState: 'needs_auth', + dismissed: false, + outcome: errorOutcome, + phase: noPhase, + requiresUrl: false, + unresolved: true + }, + failed: { + action: 'reissue', + cardState: 'not_configured', + dismissed: false, + outcome: errorOutcome, + phase: noPhase, + requiresUrl: false, + unresolved: true + }, + initiated: { + action: 'open', + cardState: 'not_configured', + dismissed: false, + outcome: noOutcome, + phase: copy => copy.waiting, + requiresUrl: true, + unresolved: true + }, + not_connected: { + action: 'none', + cardState: 'not_configured', + dismissed: false, + outcome: (target, copy) => ({ detail: target.detail || copy.notConnected, status: 'error' }), + phase: noPhase, + requiresUrl: false, + unresolved: true + }, + pending: { + action: 'open', + cardState: 'not_configured', + dismissed: false, + outcome: noOutcome, + phase: noPhase, + requiresUrl: true, + unresolved: true + }, + skipped: { + action: 'none', + cardState: 'not_configured', + dismissed: true, + outcome: noOutcome, + phase: noPhase, + requiresUrl: false, + unresolved: false + }, + unavailable: { + action: 'none', + cardState: 'disabled', + dismissed: false, + outcome: errorOutcome, + phase: noPhase, + requiresUrl: false, + unresolved: true + } +} satisfies Record + interface ConnectorOfferProps { - flow: ReturnType - /** Called when the user declines the app with Not now. */ - onSkipped: (slug: string) => void + owner: ConnectorOwner + request: ConnectionRequest } -export function ConnectorOffer({ flow, onSkipped }: ConnectorOfferProps) { - const state = useStore(flow.state) - const { t } = useI18n() - const copy = t.connectors - const [query, setQuery] = useState('') - const active = state.rows.some(row => row.phase === 'opening' || row.phase === 'waiting') - - const cardCopy: ConnectorCardCopy = { +function connectorCardCopy(copy: ConnectorCopy): ConnectorCardCopy { + return { connectAction: copy.connect, + connectTitle: copy.connectTitle, decline: copy.skip, envRequired: '', grantAction: copy.grant, @@ -264,116 +246,116 @@ export function ConnectorOffer({ flow, onSkipped }: ConnectorOfferProps) { trustVerified: () => '', trustVerifiedTip: () => '' } +} - if (state.loading) { - return +export function ConnectorOffer({ owner, request }: ConnectorOfferProps) { + const { t } = useI18n() + const copy = t.connectors + const cardCopy = connectorCardCopy(copy) + const [reissuing, setReissuing] = useState>(new Set()) + const unresolved = request.targets.some(target => CONNECTOR_CARD_PHASES[target.state].unresolved) + + const reissue = async (name: string): Promise => { + setReissuing(current => new Set(current).add(name)) + + try { + await requestGatewayForAgent( + owner.connectionId, + owner.profile, + 'connectors.connect', + { + connectors: [name], + reconnect: true, + session_id: request.sessionId + }, + 45000 + ) + } finally { + setReissuing(current => { + const next = new Set(current) + next.delete(name) + + return next + }) + } } - const rows = state.rows.filter(row => connectorTitle(row.connector).toLowerCase().includes(query.toLowerCase())) - // A targeted ask ("connect Gmail") is one or two cards, each already a - // complete question. A heading, a disclaimer and a refresh control over them - // read as a settings panel inside the chat. Only a catalog listing, which - // the model gets by asking for status with nothing named, shows that chrome. - const catalog = state.rows.length > 4 + // A settled operation is a static per-target summary: no controls, no polling, nothing live. + if (request.settled) { + return ( +
+ {request.targets.map(target => { + const phase = CONNECTOR_CARD_PHASES[target.state] + const outcome = phase.outcome(target, copy) ?? { status: 'declined' as const } + + return ( + + ) + })} +
+ ) + } return (
- {catalog ? ( -
-
- {copy.title} - -
-

{copy.disclaimer}

+ {request.targets.map(target => { + const phase = CONNECTOR_CARD_PHASES[target.state] + const title = connectorTitle(target.name) + const waitingForReissue = reissuing.has(target.name) + + return ( + { + if (phase.action === 'open' && target.connectUrl && window.hermesDesktop?.openExternal) { + void window.hermesDesktop.openExternal(target.connectUrl) + } + + if (phase.action === 'reissue') { + void reissue(target.name) + } + }} + onDismiss={() => void skipConnectionTarget(request, target.name)} + otherBusy={reissuing.size > 0 && !waitingForReissue} + outcome={phase.outcome(target, copy)} + phase={phase.phase(copy)} + state={phase.cardState} + variant="avatar" + /> + ) + })} + {unresolved ? ( +
+
) : null} - {state.error ? ( -

- {copy.statusError} - -

- ) : null} - {!state.available && !state.error ? ( -

{copy.unavailable}

- ) : null} - {catalog ? : null} -
- {rows.map(row => ( -
- void flow.connect(row.connector)} - onDismiss={() => { - const wasPending = ['opening', 'waiting'].includes(row.phase) - flow.skip(row.connector) - - // A cancel mid-authorization is not a skip: the agent may - // still be in wait, which reports the timeout to it. - if (!wasPending) { - onSkipped(row.connector) - } - }} - otherBusy={active && !['opening', 'waiting'].includes(row.phase)} - outcome={ - row.phase === 'connected' - ? { status: 'connected' } - : row.phase === 'error' - ? { - status: 'error', - detail: - row.error === 'connect' - ? copy.connectError - : row.error === 'unavailable' - ? copy.unavailable - : copy.statusError - } - : undefined - } - phase={row.phase === 'opening' ? copy.opening : row.phase === 'waiting' ? copy.waiting : undefined} - state={ - row.enabled === false - ? 'disabled' - : ['expired', 'revoked'].includes(row.connectionStatus ?? '') - ? 'needs_auth' - : 'not_configured' - } - variant="avatar" - /> - {row.phase === 'timeout' ? ( -
- {copy.timeout} - -
- ) : null} -
- ))} - {!rows.length && state.available ?

{copy.empty}

: null} -
) } /** Keep execution output in the standard disclosure, with one row per app call. */ export function ConnectorExecution(props: ToolCallMessagePartProps) { + const view = useSessionView() + const sessionId = useStore(view.$runtimeId) + const $request = useMemo(() => sessionConnectionRequest(sessionId), [sessionId]) + const request = useStore($request) const calls = connectorCalls(props.toolName, props.args) const input = recordOf(props.args) const batch = Array.isArray(input.calls) ? input.calls : [input] @@ -390,15 +372,21 @@ export function ConnectorExecution(props: ToolCallMessagePartProps) { .filter((_call, index) => { const item = recordOf(props.toolName === 'tool_call' ? results[index] : props.result) - return ['CONNECTION_REQUIRED', 'CONNECTION_EXPIRED', 'AUTH_REQUIRED'].includes( - String(recordOf(item.error).code ?? '') - ) + return recordOf(item.error).connect_card_available === true }) .map(call => { // SAFETY: connectorCalls includes only names accepted by connectorToolName. return connectorToolName(call.name)!.connector }) + const openRepair = + request && + !request.settled && + matchingTargetNames( + repair, + request.targets.map(target => target.name) + ) + return ( <> {calls.map((call, index) => { @@ -421,7 +409,7 @@ export function ConnectorExecution(props: ToolCallMessagePartProps) { /> ) })} - {repair.length ? ( + {openRepair ? ( ) : 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 93fd2c3c4e..067a299de3 100644 --- a/apps/desktop/src/components/assistant-ui/mcp-setup-tool.tsx +++ b/apps/desktop/src/components/assistant-ui/mcp-setup-tool.tsx @@ -35,28 +35,23 @@ type SetupAction = 'authorize' | 'enable' | 'install' interface SetupArgs { server: string action: SetupAction - reason: string } const CATALOG_INSTALL_POLL_MS = 1500 -// Thrown by the in-flight flow when the user cancels — the declined respond -// has already been sent, so the catch path must swallow this, not report it. +// The declined response is already sent before this sentinel reaches the catch path. 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 [target] = mcpTargets('manage_connections', row) return { action: target?.action ?? 'install', - reason: typeof row.reason === 'string' ? row.reason : '', server: target?.name ?? '' } } -/** The first target's state from the settled operation. */ interface SettledResult { status?: 'connected' | 'not_connected' | 'skipped' | 'unavailable' detail?: string @@ -84,9 +79,6 @@ function readSetupResult(result: unknown): SettledResult { const SHELL_CLASS = `${WIDGET_SHELL_CLASS} text-[length:var(--conversation-text-font-size)] text-(--ui-text-primary)` -/** The card's strings, from this tool's own copy. The verb changes with the - * action (Install / Enable / Authorize); the rest is the shared consent - * vocabulary every connector card speaks. */ function cardCopy( copy: ReturnType['t']['assistant']['mcpSetup'], action: SetupAction @@ -114,7 +106,6 @@ function cardCopy( } export const McpSetupTool = (props: ToolCallMessagePartProps) => { - // Settled → static outcome line (the flow already ran or was declined). if (props.result !== undefined) { return } @@ -125,7 +116,6 @@ export const McpSetupTool = (props: ToolCallMessagePartProps) => { const McpSetupLive = (props: ToolCallMessagePartProps) => { const messageRunning = useAuiState(selectMessageRunning) - // Stopped mid-prompt with no result — don't leave a dead interactive panel. if (!messageRunning) { return } @@ -163,8 +153,6 @@ function McpSetupSettled({ args, result }: ToolCallMessagePartProps) { 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 - // to: the name, then the verdict as meta. A failure keeps its reason. return ( sessionConnectionRequest(sessionId), [sessionId]) const request = useStore($request) @@ -194,19 +181,15 @@ function McpSetupPending({ args }: ToolCallMessagePartProps) { 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) const [envDraft, setEnvDraft] = useState>({}) const [entry, setEntry] = useState(undefined) const [envOpen, setEnvOpen] = useState(false) - // Set when the user cancels mid-flight (a stuck OAuth tab, a hung install). - // The in-flight flow checks it at every poll boundary and aborts via the - // CANCELLED sentinel; the declined respond has already been sent by then. const cancelRef = useRef(false) - // tool.start arrives before the server request; disable the buttons until the request exists. - const ready = Boolean(request?.requestId) + // `tool.start` arrives before `connection.request`. + const ready = Boolean(request) const respond = useCallback( async (outcome: ConnectionTargetOutcome) => { @@ -220,16 +203,14 @@ function McpSetupPending({ args }: ToolCallMessagePartProps) { return } - const success = outcome.state === 'installed' || outcome.state === 'enabled' || outcome.state === 'authorized' + const success = outcome.status === 'connected' if (success) { - // No reload.mcp: the between-turns refresh registers the new server's tools. invalidateMcpSuggestionIndex() } -try { - // One target: this answer settles the operation. - await respondToConnectionRequest(request, { settled_by: 'all_resolved', targets: [outcome] }) + try { + await respondToConnectionRequest(request, { targets: [outcome] }) } catch (error) { notifyError(error, copy.sendFailed) } @@ -238,11 +219,10 @@ try { ) const decline = useCallback(() => { - // While a flow is in flight this is a CANCEL: answer declined right away - // and let the abandoned work notice via cancelRef at its next poll. + // Respond first; cancelRef stops abandoned work at its next poll. cancelRef.current = true triggerHaptic('cancel') - void respond({ name: server, state: 'declined' }) + void respond({ name: server, status: 'skipped' }) }, [respond, server]) const approve = useCallback(async () => { @@ -250,8 +230,7 @@ try { const oauthScope = capabilityScoped() setWorking(true) - // Poll-boundary abort for the background-install loop; the OAuth flows - // carry their own cancel via completeMcpDesktopOAuth's `cancelled`. + // OAuth owns its cancellation; polling needs an explicit boundary check. const throwIfCancelled = (value: T): T => { if (cancelRef.current) { throw CANCELLED @@ -264,7 +243,7 @@ try { if (action === 'enable') { await setMcpServerEnabled(server, true) triggerHaptic('submit') - await respond({ name: server, state: 'enabled' }) + await respond({ name: server, status: 'connected' }) return } @@ -277,12 +256,11 @@ try { }) triggerHaptic('submit') - await respond({ name: server, state: 'authorized', tools: (flow.tools ?? []).map(tool => tool.name) }) + await respond({ name: server, status: 'connected', tools: (flow.tools ?? []).map(tool => tool.name) }) return } - // Install from the catalog only. Required credentials are prompted inline first. let resolved = entry if (resolved === undefined) { @@ -292,7 +270,7 @@ try { } if (!resolved) { - await respond({ detail: copy.notInCatalog(server), name: server, state: 'error' }) + await respond({ detail: copy.notInCatalog(server), name: server, status: 'failed' }) return } @@ -300,7 +278,6 @@ try { const required = resolved.required_env.filter(env => env.required) if (required.some(env => !envDraft[env.name]?.trim())) { - // Reveal the credential fields; the user approves again once filled. setEnvOpen(true) return @@ -308,8 +285,7 @@ try { const res = await installMcpCatalogEntry(server, envDraft) - // Git-backed entries clone in the background — poll to completion so a - // non-zero exit surfaces as a real failure instead of a false success. + // Poll background installs so non-zero exits cannot report false success. if (res.background && res.action) { for (;;) { const status = throwIfCancelled(await getActionStatus(res.action, 1)) @@ -327,10 +303,9 @@ try { } triggerHaptic('submit') - await respond({ name: server, state: 'installed' }) + await respond({ name: server, status: 'connected' }) } catch (error) { - // User cancel: the declined respond is already on the wire — the - // abandoned flow just stops, nothing to report. + // The declined response is already sent; do not report cancellation as failure. if (error === CANCELLED || error instanceof McpOAuthCancelled) { return } @@ -339,7 +314,7 @@ try { await respond({ detail: error instanceof Error ? error.message : String(error), name: server, - state: 'error' + status: 'failed' }) } finally { setWorking(false) @@ -351,12 +326,7 @@ try { 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 - // stays live while a flow is in flight — that's the cancel path. Stands - // down whenever a focusable control has focus (clarify's rule): a keystroke - // meant for the composer, a popover, or the card's own credential fields - // must never silently approve an install or throw away typed input. + // Do not capture shortcuts while a focusable control owns typed input. useEffect(() => { if (!ready) { return @@ -401,14 +371,11 @@ try { ) } - // The same consent card the connector offer renders: one shape for every - // "connect this?" in the transcript. `phase` is what flips the card into - // its working state (spinner on the action, decline becomes cancel). return ( { + cleanup() + $connectionRequests.set({}) + $gateway.set(null) + _resetSessionOwnerHintsForTests({ storage: true }) + vi.clearAllMocks() +}) + +describe('manage_connections routing outside guided onboarding', () => { + it('renders the operation card for a plain chat session', async () => { + const Fallback = MESSAGE_PARTS_COMPONENTS.tools.Fallback + setSessionOwnerHint(STORED_ID, OWNER) + setConnectionRequest(REQUEST) + // SAFETY: the card reads only `request` off the client in this test. + $gateway.set({ request: vi.fn().mockResolvedValue({ status: 'ok' }) } as never) + + render( + + + + + + ) + + await waitFor(() => { + expect(screen.getAllByRole('button', { name: 'Not now' })).toHaveLength(2) + }) + expect(screen.queryByText(/running manage connections/i)).toBeNull() + }) +}) 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 7217161fbd..d0351d5721 100644 --- a/apps/desktop/src/components/assistant-ui/thread/message-parts.tsx +++ b/apps/desktop/src/components/assistant-ui/thread/message-parts.tsx @@ -23,7 +23,6 @@ import { SCAFFOLD_LABEL_CLASS, SCAFFOLD_META_CLASS, ScaffoldRow } from '@/compon import { useI18n } from '@/i18n' 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' import { isTodoToolName } from '@/lib/todos' import { useEnterAnimation } from '@/lib/use-enter-animation' @@ -120,16 +119,15 @@ 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') { + if (props.toolName === 'manage_connections') { return } - if (isOnboardingEnabled() && connectorCalls(props.toolName, props.args).length > 0) { + if (connectorCalls(props.toolName, props.args).length > 0) { return } diff --git a/apps/desktop/src/components/assistant-ui/tool/approval.tsx b/apps/desktop/src/components/assistant-ui/tool/approval.tsx index d7ff03878f..e982bfe982 100644 --- a/apps/desktop/src/components/assistant-ui/tool/approval.tsx +++ b/apps/desktop/src/components/assistant-ui/tool/approval.tsx @@ -41,7 +41,7 @@ import type { ToolPart } from './fallback-model' // Binding is POSITIONAL, not command-matched: the desktop `tool.start` payload // carries no structured args (only tool_id/name/context — see // tui_gateway/server.py::_on_tool_start), so we cannot join the approval to the -// row by command string. `approval.request` can fire from the command guards +// row by command string. an approval server request can fire from the command guards // and protected-instruction file writes. The agent thread blocks on exactly one // approval at a time, so the single pending row of those tools IS the row that // raised it. The command/description text comes from `$approvalRequest` (the diff --git a/apps/desktop/src/components/onboarding-chat/setup-profile.ts b/apps/desktop/src/components/onboarding-chat/setup-profile.ts index 20e441435f..c3ab17e2d9 100644 --- a/apps/desktop/src/components/onboarding-chat/setup-profile.ts +++ b/apps/desktop/src/components/onboarding-chat/setup-profile.ts @@ -188,10 +188,9 @@ function connectFirstRunbook(picks: string[]): string[] { return [ `CONNECT FIRST. During setup the user picked these apps, given here as exact gateway slugs: ${named}. Your first action in this session, before any plan and before any other tool call, is ONE manage_connections call with action="connect" and connectors set to every one of those slugs. Do not call action="status" first; the slugs are exact and the catalog check is already done.`, 'If every result comes back already active, there is nothing to wait for: begin the task at once.', - "The app opens every sign-in from that result in the user's browser and shows one row per app, so never paste the links. In the same turn say one short line: which apps are being connected and, in a clause each, what this task gets from each one. Then end the turn.", - 'Then call manage_connections action="wait" with the same slugs and timeout_seconds=120, and say nothing until it returns. If the wait comes back as pending because the links were minted moments ago, end your turn: the app sends a hidden note that begins with "[setup] links opened" once your turn ends and the sign-ins are open, and that note is your cue to call the same wait again. A note that arrives after you have already waited needs no reply.', + 'That one call shows the user a card with one row per app and blocks until every app is connected, skipped, or the deadline passes; never paste links, never call "connect" again while the card is up. Its result lists each app as connected, skipped or not_connected.', 'The user can start early. A message from them that begins with "Start with" or "Start without" names the apps that are connected and the ones they skipped; treat it as the go signal and begin with the connected apps only.', - 'When the wait returns with every app connected, begin the task at once. When it returns with apps still pending, stop and ask in one line: which apps did not connect, and whether they want you to continue without them or try connecting again (a fresh action="connect" mints new links). Wait for their answer. If they choose to continue without an app, build the version of the task that needs no account for that part and say in one line what the connection would have added.', + 'When the result shows every app connected, begin the task at once. When some are skipped or not_connected, stop and ask in one line: which apps did not connect, and whether they want you to continue without them or try again (a fresh action="connect" mints new links). Wait for their answer. If they choose to continue without an app, build the version of the task that needs no account for that part and say in one line what the connection would have added.', 'Account data comes from the connected apps first. Tools already signed in on this machine, like a logged-in gh, are fair to use when the task benefits; say so in one line when you do.', "Discover a connected app's tools with tool_search and use real results for the task; never fabricate account data. Reading is separate from sending, deleting or scheduling: ask before those. No recurring job unless that is what they asked for.", 'Make the result something they can open: a single HTML page when the idea allows it, and at least one real reading or action through a connected app.' diff --git a/apps/desktop/src/components/ui/connector-card.test.tsx b/apps/desktop/src/components/ui/connector-card.test.tsx index fe877c73fb..0f32536b3f 100644 --- a/apps/desktop/src/components/ui/connector-card.test.tsx +++ b/apps/desktop/src/components/ui/connector-card.test.tsx @@ -94,11 +94,19 @@ describe('while it is working', () => { it('holds its own action but never the way out', () => { // While a connect is in flight, decline is the escape from a stuck // sign-in tab or a hung install. - renderCard({ phase: 'Installing…' }) + renderCard({ busy: true, phase: 'Installing…' }) + const [action] = screen.getAllByRole('button') + expect(action.hasAttribute('disabled')).toBe(true) expect(screen.getByRole('button', { name: 'Not now' }).hasAttribute('disabled')).toBe(false) }) + it('keeps Connect clickable under a phase label alone, so a waiting row can reopen its link', () => { + renderCard({ phase: 'Finish connecting in your browser…' }) + + expect(screen.getByRole('button', { name: 'Connect' }).hasAttribute('disabled')).toBe(false) + }) + it('holds its action while a sibling is mid-flight, so two sign-in tabs never race for focus', () => { renderCard({ otherBusy: true }) diff --git a/apps/desktop/src/components/ui/connector-card.tsx b/apps/desktop/src/components/ui/connector-card.tsx index a2bfc4ef60..c29bdea257 100644 --- a/apps/desktop/src/components/ui/connector-card.tsx +++ b/apps/desktop/src/components/ui/connector-card.tsx @@ -9,62 +9,38 @@ import { MarkdownLinkText } from '@/lib/external-link' import { CheckCircle2 } from '@/lib/icons' import { cn } from '@/lib/utils' -/** - * The consent card for connecting a thing to Hermes, as pure presentation. - * - * Deliberately knows nothing about MCP. It renders a subject, a state, and an - * outcome, and it calls back — what a connector IS, how one connects, and - * where the strings come from all belong to the caller. That boundary is the - * point: the transcript's inline setup card and any other surface that has to - * ask "connect this?" should look identical without sharing a data layer. - * - * Consent vocabulary follows the tool approval bar: primary-tinted action, - * quiet ghost decline, the same `mt-2` stand-off. - */ - -/** Where the subject stands before anything is attempted. */ +/** Presentation leaf: callers own connector semantics and localized copy. */ export type ConnectorCardState = 'connected' | 'disabled' | 'needs_auth' | 'not_configured' -/** How much the source vouches for the subject. */ export type ConnectorCardTrust = 'catalog' | 'community' | 'verified' -/** One credential the subject declares it needs. */ export interface ConnectorCardField { name: string prompt?: string required?: boolean } -/** What the card renders. Structural, so a caller's richer type satisfies it - * without this file importing that type. */ +/** Structural to accept callers' richer subject types without importing them. */ export interface ConnectorCardSubject extends ConnectorLogoSubject { description?: string - /** Named by the publisher and checked against the serving domain. */ publisher?: string requiredEnv?: ConnectorCardField[] - /** Ordered work only the user can do, elsewhere, before this can connect. */ setup?: string[] title: string trust?: ConnectorCardTrust } -/** How an attempt ended. */ export interface ConnectorCardOutcome { detail?: string - /** The failure was a refusal, not a fault: wrong key, expired grant, or a - * grant that never covered the tools. Asking again is the fix, so the card - * offers access rather than a retry. */ + /** An access refusal; offer authorization instead of retry. */ needsAuth?: boolean status: 'connected' | 'declined' | 'error' tools?: unknown[] } -/** Every string the card shows. Passed in rather than read from i18n so the - * card stays a leaf that anything can render, tests included. */ +/** Passed in so this presentation leaf has no i18n dependency. */ export interface ConnectorCardCopy { connectAction: string - /** The live offer's heading as a question — "Connect Gmail?" — the same - * shape the MCP setup card asks in. Absent, the card leads with the name. */ connectTitle?: (title: string) => string decline: string envRequired: string @@ -82,16 +58,12 @@ export interface ConnectorCardCopy { trustVerifiedTip: (publisher: string) => string } -/** What connecting actually means — the endpoint that will be contacted, - * or the catalog it came from. VS Code's trust dialog links the config it - * is about to trust; same idea. Shown under the description in tertiary. */ export interface ConnectorCardSource { text: string } const SHELL_CLASS = `${WIDGET_SHELL_CLASS} text-[length:var(--conversation-text-font-size)] text-(--ui-text-primary)` -// Same platform sniff the approval bar uses for its accelerator hint. const isMac = typeof navigator !== 'undefined' && /Mac|iP(hone|ad|od)/.test(navigator.platform) const hostOf = (url: null | string | undefined): string => { @@ -106,8 +78,6 @@ const hostOf = (url: null | string | undefined): string => { } } -/** What trails a settled subject's name: how it ended, and the useful number - * or reason behind that. */ export function outcomeMeta(outcome: ConnectorCardOutcome, copy: ConnectorCardCopy): string { if (outcome.status === 'connected') { const toolCount = Array.isArray(outcome.tools) ? outcome.tools.length : 0 @@ -122,14 +92,6 @@ export function outcomeMeta(outcome: ConnectorCardOutcome, copy: ConnectorCardCo return copy.stateDeclined } -/** - * A subject that no longer needs anything: connected, skipped, failed. - * - * Not a card. An answered offer is transcript scaffolding — the same kind of - * line as a settled tool run — so it renders through `ScaffoldRow` and reads - * like everything else already done. The name keeps full contrast because - * that's the content; the verdict trails it as meta. - */ export function ConnectorSummary({ connector, meta, @@ -137,12 +99,9 @@ export function ConnectorSummary({ }: { connector: ConnectorLogoSubject meta?: string - /** `ok` is the settled tool row's emerald; `error` its destructive. Absent - * is the neutral grey a skip or a no-answer reads in. */ tone?: 'error' | 'ok' }) { - // The scaffold mark goes on the row, never on a container holding several: - // opacity opens a stacking context and would pin every sibling to one level. + // Apply opacity to rows, not a shared container: it would create a stacking context for every sibling. return (
@@ -166,29 +125,13 @@ export function ConnectorSummary({ ) } -/** - * How much the source vouches for this subject. - * - * Only the exceptions get a badge. Something we shipped in a reviewed catalog - * is the ordinary case — badging it "reviewed" spends a word on every card to - * say "normal", and a label whose meaning nobody can guess teaches the user to - * ignore the one that matters. - * - * So: nothing for vetted sources. A publisher that proved it owns the serving - * domain says "verified · notion.com" — checkable identity, not an - * endorsement, which is why it names the domain instead of claiming trust. - * Everything else gets the amber "unreviewed" with the host in its tooltip: - * the card doesn't spend a line on an endpoint nobody reads, but for a - * publisher nobody has vouched for, the host is the whole question. - */ +// Catalog entries are ordinary; reserve badges for verified and community exceptions. function TrustBadge({ connector, copy }: { connector: ConnectorCardSubject; copy: ConnectorCardCopy }) { if (!connector.trust || connector.trust === 'catalog') { return null } if (connector.trust === 'verified') { - // No publisher domain means a curated directory of vendor remotes, which - // is the ordinary case again — nothing to say. if (!connector.publisher) { return null } @@ -211,49 +154,31 @@ function TrustBadge({ connector, copy }: { connector: ConnectorCardSubject; copy } export interface ConnectorCardProps { - /** Show ⌘⏎ / Esc beside the actions. Only the card that also LISTENS for - * those keys should claim them; a hint on a card that ignores the key is - * a lie the user finds out about by pressing it. */ + /** Only callers that handle these keys may show the accelerator hint. */ accelerators?: boolean - /** Settled cards fold to a scaffold line (the MCP setup default). The - * connector offer keeps the card standing with a green Connected in the - * action slot: a row of identical cards where one collapses reads as a - * row where one broke. */ collapseWhenSettled?: boolean connector: ConnectorCardSubject copy: ConnectorCardCopy - /** Waved off by the user. Collapses to the same settled line as success. */ dismissed?: boolean envDraft?: Record - /** Whether the credential fields are revealed. The caller owns this because - * a refused credential should reveal them without a second click. */ + /** The caller reveals fields after a refused credential. */ envOpen?: boolean onConnect: () => void onDismiss: () => void onEnvChange?: (key: string, value: string) => void - /** A sibling card is mid-flight. Two sign-in tabs racing for focus is - * hostile, so the action waits — but the decline never does. */ + /** Prevent concurrent sign-in tabs; decline remains enabled. */ otherBusy?: boolean actionDisabled?: boolean outcome?: ConnectorCardOutcome - /** Present only while working; replaces the resting state label. */ + /** The action itself is running (spinner, button held). Independent of `phase`: a row can show + * "Finish connecting in your browser" while Connect stays clickable to reopen the link. */ + busy?: boolean phase?: string source?: ConnectorCardSource state: ConnectorCardState - /** `avatar` leads with the mark at identity scale in a left gutter, the way - * the MCP and Messaging headers introduce a service. `compact` (default) - * trails a small mark on the right like the setup card's tool row. */ variant?: 'avatar' | 'compact' } -/** - * One subject's consent card. - * - * Owns everything about that subject and nothing about its siblings: its own - * trust badge, credential fields, failure reason, and its own action. A - * connected card collapses to a single confirmed line, because its offer is - * spent and the space belongs to the ones still asking. - */ export function ConnectorCard({ accelerators = false, collapseWhenSettled = true, @@ -267,6 +192,7 @@ export function ConnectorCard({ onEnvChange, otherBusy = false, actionDisabled = false, + busy = false, outcome, phase, source, @@ -277,8 +203,6 @@ export function ConnectorCard({ const connected = outcome?.status === 'connected' const failed = outcome?.status === 'error' - // Answered: the offer is spent, so the card collapses to a scaffold line and - // gives the space back to whatever is still asking. if ((connected || dismissed) && collapseWhenSettled) { return ( {copy.connectTitle ? copy.connectTitle(connector.title) : connector.title} - {/* While the card is working its phase replaces the resting state — - "Signing in…" is the one the user needs, because the browser tab - that just took focus is otherwise unexplained. */} {working ? ( {phase} ) : ( @@ -337,8 +247,6 @@ export function ConnectorCard({ {failed && outcome.detail ?

{outcome.detail}

: null} - {/* The part we cannot do. Numbered because order matters, linked - because the whole cost of these steps is finding the page. */} {steps.length > 0 && (
    {steps.map((step, index) => ( @@ -375,13 +283,6 @@ export function ConnectorCard({ )}
- {/* Same strip as the tool approval bar (tool/approval.tsx), down to its - stand-off: a bordered primary-tinted action plus a quiet ghost - decline. One consent vocabulary across the transcript. In the avatar - layout the strip sits on the text column, so the gutter stays the - mark's alone. Settled (and kept standing), the action slot holds the - verdict in the same box — green Connected, grey Skipped — so the row - of cards keeps its rhythm and nothing jumps. */}
{settled ? (
- {/* Never disabled: while a connect is in flight this is the way out - of a stuck sign-in tab or a hung install. */} + {/* Never disable: this exits a stuck sign-in tab or hung install. */}