From c3cad041dc572cfd2c2472f609e2c007fae7b8a1 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Mon, 3 Aug 2026 18:54:13 -0700 Subject: [PATCH 1/6] feat(sidebar): inline SSH reconnect control on workspace cards Replaces the blocking SshDisconnectedDialog with an inline pill in the workspace card title row, and unifies the SSH connect vocabulary across the sidebar card, terminal overlay, host-header menu, and status-bar row. - new src/renderer/src/ssh/ modules: typechecker-total status predicates (recoverability), a shared in-flight connect registry, the promoted UI connect timeout, and the shared connect verb table - WorktreeCardSshHostControl: one 16px pill shape for every state, icon-only in compact/new card modes, passive glyph for connected/null/removed hosts - migrates the four duplicated status predicates to the shared module - deletes SshDisconnectedDialog (and its window-capture Enter handler) --- .../components/NewWorkspaceComposerCard.tsx | 28 +- .../settings/ssh-target-action-state.ts | 9 +- .../sidebar/AutoRenameFailedDialog.tsx | 4 +- .../sidebar/HostSectionHeaderMenu.tsx | 11 +- .../sidebar/SshDisconnectedDialog.tsx | 205 ------------ .../WorktreeCard.affiliate-list-mode.test.tsx | 4 - .../WorktreeCard.compact-hover.test.tsx | 4 - ....compact-ports-hover-independence.test.tsx | 4 - ...orktreeCard.hosted-review-refresh.test.tsx | 4 - .../sidebar/WorktreeCard.lineage.test.tsx | 4 - .../WorktreeCard.merged-pr-display.test.tsx | 4 - .../WorktreeCard.pinned-repo-icon.test.tsx | 4 - .../sidebar/WorktreeCard.pr-display.test.tsx | 4 - .../WorktreeCard.quick-actions.test.tsx | 4 - ...WorktreeCard.ssh-reconnect-prompt.test.tsx | 71 ++-- .../src/components/sidebar/WorktreeCard.tsx | 66 ++-- .../WorktreeCardSshHostControl.test.tsx | 305 ++++++++++++++++++ .../sidebar/WorktreeCardSshHostControl.tsx | 273 ++++++++++++++++ ....lineage-agent-expansion-coupling.test.tsx | 4 - .../WorktreeList.lineage-child-card.test.ts | 20 -- ...ktreeList.lineage-child-real-card.test.tsx | 4 - ...treeList.status-lane-lineage-drop.test.tsx | 4 - .../status-bar/SshTargetStatusRow.tsx | 10 +- .../TerminalSshReconnectOverlay.test.tsx | 46 ++- .../TerminalSshReconnectOverlay.tsx | 54 ++-- .../terminal-pane/pty-connection.ts | 2 +- src/renderer/src/hooks/useIpcEvents.ts | 3 +- src/renderer/src/i18n/locales/en.json | 21 ++ .../src/lib/new-workspace-ssh-gate.ts | 3 +- .../src/ssh/ssh-connect-in-flight.test.ts | 73 +++++ src/renderer/src/ssh/ssh-connect-in-flight.ts | 51 +++ .../src/ssh/ssh-connect-ui-timeout.test.ts | 43 +++ .../src/ssh/ssh-connect-ui-timeout.ts | 31 ++ src/renderer/src/ssh/ssh-connect-verb.ts | 21 ++ .../ssh/ssh-connection-recoverability.test.ts | 72 +++++ .../src/ssh/ssh-connection-recoverability.ts | 36 +++ 36 files changed, 1080 insertions(+), 426 deletions(-) delete mode 100644 src/renderer/src/components/sidebar/SshDisconnectedDialog.tsx create mode 100644 src/renderer/src/components/sidebar/WorktreeCardSshHostControl.test.tsx create mode 100644 src/renderer/src/components/sidebar/WorktreeCardSshHostControl.tsx create mode 100644 src/renderer/src/ssh/ssh-connect-in-flight.test.ts create mode 100644 src/renderer/src/ssh/ssh-connect-in-flight.ts create mode 100644 src/renderer/src/ssh/ssh-connect-ui-timeout.test.ts create mode 100644 src/renderer/src/ssh/ssh-connect-ui-timeout.ts create mode 100644 src/renderer/src/ssh/ssh-connect-verb.ts create mode 100644 src/renderer/src/ssh/ssh-connection-recoverability.test.ts create mode 100644 src/renderer/src/ssh/ssh-connection-recoverability.ts diff --git a/src/renderer/src/components/NewWorkspaceComposerCard.tsx b/src/renderer/src/components/NewWorkspaceComposerCard.tsx index 72ad520f2de4..b7a07bf36b6d 100644 --- a/src/renderer/src/components/NewWorkspaceComposerCard.tsx +++ b/src/renderer/src/components/NewWorkspaceComposerCard.tsx @@ -66,6 +66,7 @@ import type { TaskSourceContext } from '../../../shared/task-source-context' import type { RuntimeStatus } from '../../../shared/runtime-types' import { unwrapRuntimeRpcResult } from '@/runtime/runtime-rpc-client' import { translate } from '@/i18n/i18n' +import { withUiConnectTimeout } from '@/ssh/ssh-connect-ui-timeout' type RepoOption = React.ComponentProps['repos'][number] type EphemeralVmRecipeOption = NonNullable[number] @@ -210,33 +211,6 @@ function getSshStatusLabel(status: SshConnectionStatus): string { return SSH_STATUS_LABELS[status] ?? status } -// Why: bound how long the run-target picker waits on a host connect so a stalled backend -// connect can't leave the row's disabled/spinner state stuck forever. The backend keeps going. -const RUN_TARGET_CONNECT_UI_TIMEOUT_MS = 20_000 - -async function withUiConnectTimeout(promise: Promise): Promise { - let timer: ReturnType | undefined - const timeout = new Promise((_, reject) => { - timer = setTimeout(() => { - reject( - new Error( - translate( - 'auto.components.NewWorkspaceComposerCard.connectTimedOut', - 'Connection timed out. It may still be connecting in the background.' - ) - ) - ) - }, RUN_TARGET_CONNECT_UI_TIMEOUT_MS) - }) - try { - return await Promise.race([promise, timeout]) - } finally { - if (timer) { - clearTimeout(timer) - } - } -} - function SetupCommandPreview({ setupConfig }: { setupConfig: SetupConfig }): React.JSX.Element { // Why: just the script in a quiet monochrome card — the source label (orca.yaml / local) and // the run-setup toggle live in the section header above, so the card carries no chrome of its diff --git a/src/renderer/src/components/settings/ssh-target-action-state.ts b/src/renderer/src/components/settings/ssh-target-action-state.ts index 38db0cdbf281..c0f076cc2820 100644 --- a/src/renderer/src/components/settings/ssh-target-action-state.ts +++ b/src/renderer/src/components/settings/ssh-target-action-state.ts @@ -1,15 +1,10 @@ +import { isConnectingSshStatus } from '@/ssh/ssh-connection-recoverability' import type { SshConnectionStatus } from '../../../../shared/ssh-types' export type SshTargetBusyAction = 'terminate' | 'reset' | 'remove' -const SSH_TARGET_CONNECTING_STATUSES: ReadonlySet = new Set([ - 'connecting', - 'deploying-relay', - 'reconnecting' -]) - export function isSshTargetConnecting(status: SshConnectionStatus): boolean { - return SSH_TARGET_CONNECTING_STATUSES.has(status) + return isConnectingSshStatus(status) } export function shouldClearPendingSshReset({ diff --git a/src/renderer/src/components/sidebar/AutoRenameFailedDialog.tsx b/src/renderer/src/components/sidebar/AutoRenameFailedDialog.tsx index caf4a5ab5eb5..6ea256e5aa3e 100644 --- a/src/renderer/src/components/sidebar/AutoRenameFailedDialog.tsx +++ b/src/renderer/src/components/sidebar/AutoRenameFailedDialog.tsx @@ -26,7 +26,7 @@ type AutoRenameFailedDialogProps = { * CLI output when main still holds it (in-memory, lost on restart), falling * back to the persisted excerpt — either can run many lines, so it gets a * dedicated scrollable surface rather than a tooltip — see the sibling - * SshDisconnectedDialog pattern. + * AddRemoteHostDialog pattern. */ export function AutoRenameFailedDialog({ open, @@ -150,7 +150,7 @@ export function AutoRenameFailedDialog({ {/* Why: Close backs the user out, so it stays quiet (outline, not a - solid CTA) — matching the sibling SshDisconnectedDialog. */} + solid CTA) — matching the other sidebar dialogs. */} diff --git a/src/renderer/src/components/sidebar/HostSectionHeaderMenu.tsx b/src/renderer/src/components/sidebar/HostSectionHeaderMenu.tsx index 5c944fa3a1e7..3aead74fd819 100644 --- a/src/renderer/src/components/sidebar/HostSectionHeaderMenu.tsx +++ b/src/renderer/src/components/sidebar/HostSectionHeaderMenu.tsx @@ -24,6 +24,7 @@ import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip import { useMountedRef } from '@/hooks/useMountedRef' import { useAppStore } from '@/store' import { translate } from '@/i18n/i18n' +import { sshConnectVerb } from '@/ssh/ssh-connect-verb' import { parseExecutionHostId } from '../../../../shared/execution-host' import { describeRuntimeCompatBlock } from '../../../../shared/protocol-compat' import { @@ -74,18 +75,18 @@ export function HostSectionHeaderMenu({ row }: { row: HostHeaderRow }): React.JS const [renameOpen, setRenameOpen] = useState(false) const [removeOpen, setRemoveOpen] = useState(false) const mountedRef = useMountedRef() - const sshConnected = useAppStore((s) => { + const sshStatus = useAppStore((s) => { const parsed = parseExecutionHostId(row.hostId) if (parsed?.kind !== 'ssh') { - return false + return null } - return s.sshConnectionStates.get(parsed.targetId)?.status === 'connected' + return s.sshConnectionStates.get(parsed.targetId)?.status ?? null }) const model = buildHostHeaderMenuModel({ kind: row.kind, health: row.health, - sshConnected, + sshConnected: sshStatus === 'connected', compatibility: row.compatibility }) const removalTarget = resolveHostRemoval(row.hostId) @@ -242,7 +243,7 @@ export function HostSectionHeaderMenu({ row }: { row: HostHeaderRow }): React.JS {model.actions.includes('ssh-reconnect') && ( void runSshAction('connect')}> - {translate('auto.components.sidebar.HostSectionHeaderMenu.63f36455cc', 'Reconnect')} + {sshConnectVerb(sshStatus)} )} {model.actions.includes('ssh-disconnect') && ( diff --git a/src/renderer/src/components/sidebar/SshDisconnectedDialog.tsx b/src/renderer/src/components/sidebar/SshDisconnectedDialog.tsx deleted file mode 100644 index fd4a460e29af..000000000000 --- a/src/renderer/src/components/sidebar/SshDisconnectedDialog.tsx +++ /dev/null @@ -1,205 +0,0 @@ -import { useCallback, useEffect, useState } from 'react' -import { toast } from 'sonner' -import { Loader2, Server, ServerOff } from 'lucide-react' -import { - Dialog, - DialogContent, - DialogDescription, - DialogFooter, - DialogHeader, - DialogTitle -} from '@/components/ui/dialog' -import { Button } from '@/components/ui/button' -import { useMountedRef } from '@/hooks/useMountedRef' -import { statusColor } from '@/components/settings/SshTargetCard' -import type { SshConnectionStatus } from '../../../../shared/ssh-types' -import { translate } from '@/i18n/i18n' - -type SshDisconnectedDialogProps = { - open: boolean - onOpenChange: (open: boolean) => void - targetId: string - targetLabel: string - status: SshConnectionStatus -} - -const STATUS_MESSAGES: Partial> = { - get disconnected() { - return translate( - 'auto.components.sidebar.SshDisconnectedDialog.disconnected', - 'This SSH host is not connected.' - ) - }, - get reconnecting() { - return translate( - 'auto.components.sidebar.SshDisconnectedDialog.reconnecting', - 'Reconnecting to the remote host...' - ) - }, - get 'reconnection-failed'() { - return translate( - 'auto.components.sidebar.SshDisconnectedDialog.reconnectionFailed', - 'Reconnection to the remote host failed.' - ) - }, - get error() { - return translate( - 'auto.components.sidebar.SshDisconnectedDialog.376bed88e5', - 'The connection to the remote host encountered an error.' - ) - }, - get 'auth-failed'() { - return translate( - 'auto.components.sidebar.SshDisconnectedDialog.authFailed', - 'Authentication to the remote host failed.' - ) - } -} - -function isReconnectable(status: SshConnectionStatus): boolean { - return ['disconnected', 'reconnection-failed', 'error', 'auth-failed'].includes(status) -} - -export function SshDisconnectedDialog({ - open, - onOpenChange, - targetId, - targetLabel, - status -}: SshDisconnectedDialogProps): React.JSX.Element { - const [connecting, setConnecting] = useState(false) - const mountedRef = useMountedRef() - - const handleReconnect = useCallback(async () => { - setConnecting(true) - try { - await window.api.ssh.connect({ targetId }) - if (mountedRef.current) { - onOpenChange(false) - } - } catch (err) { - toast.error( - err instanceof Error - ? err.message - : translate( - 'auto.components.sidebar.SshDisconnectedDialog.656368f3a2', - 'Reconnection failed' - ) - ) - } finally { - if (mountedRef.current) { - setConnecting(false) - } - } - }, [mountedRef, targetId, onOpenChange]) - - const isConnecting = - connecting || - status === 'connecting' || - status === 'deploying-relay' || - status === 'reconnecting' - const reconnectingMessage = - STATUS_MESSAGES.reconnecting ?? - translate( - 'auto.components.sidebar.SshDisconnectedDialog.reconnecting', - 'Reconnecting to the remote host...' - ) - const disconnectedMessage = - STATUS_MESSAGES.disconnected ?? - translate( - 'auto.components.sidebar.SshDisconnectedDialog.disconnected', - 'This SSH host is not connected.' - ) - const message = isConnecting - ? reconnectingMessage - : (STATUS_MESSAGES[status] ?? disconnectedMessage) - const showReconnect = isReconnectable(status) - - useEffect(() => { - // Window-level Enter handler. The dialog typically appears while focus - // is inside an embedded terminal (xterm) or editor (monaco) that - // aggressively reclaims focus, so dialog-scoped key handlers never - // fire. Listening on window (capture phase) catches Enter regardless - // of where focus actually lives while the dialog is open. - if (!open || !showReconnect || isConnecting) { - return undefined - } - const onKeyDown = (event: KeyboardEvent): void => { - if (event.key !== 'Enter' || event.defaultPrevented) { - return - } - if (event.isComposing) { - return - } - event.preventDefault() - event.stopPropagation() - void handleReconnect() - } - window.addEventListener('keydown', onKeyDown, true) - return () => window.removeEventListener('keydown', onKeyDown, true) - }, [open, showReconnect, isConnecting, handleReconnect]) - - return ( - - - - - {isConnecting ? ( - - ) : ( - - )} - {isConnecting - ? translate( - 'auto.components.sidebar.SshDisconnectedDialog.cb5938ae79', - 'Reconnecting...' - ) - : translate( - 'auto.components.sidebar.SshDisconnectedDialog.11552bf786', - 'SSH Disconnected' - )} - - {message} - - -
- -
- {targetLabel} -
- {isConnecting ? ( - - ) : ( - - )} -
- - - - {showReconnect && ( - - )} - - - - ) -} diff --git a/src/renderer/src/components/sidebar/WorktreeCard.affiliate-list-mode.test.tsx b/src/renderer/src/components/sidebar/WorktreeCard.affiliate-list-mode.test.tsx index 1f15663a5eee..f9b3976a826a 100644 --- a/src/renderer/src/components/sidebar/WorktreeCard.affiliate-list-mode.test.tsx +++ b/src/renderer/src/components/sidebar/WorktreeCard.affiliate-list-mode.test.tsx @@ -80,10 +80,6 @@ vi.mock('./WorktreeCardAgents', () => ({ default: () =>
})) -vi.mock('./SshDisconnectedDialog', () => ({ - SshDisconnectedDialog: () => null -})) - vi.mock('./WorktreeContextMenu', () => ({ default: ({ children }: { children: ReactNode }) => (
{children}
diff --git a/src/renderer/src/components/sidebar/WorktreeCard.compact-hover.test.tsx b/src/renderer/src/components/sidebar/WorktreeCard.compact-hover.test.tsx index 9aa2f31b577f..707ce16ec61b 100644 --- a/src/renderer/src/components/sidebar/WorktreeCard.compact-hover.test.tsx +++ b/src/renderer/src/components/sidebar/WorktreeCard.compact-hover.test.tsx @@ -111,10 +111,6 @@ vi.mock('./WorktreeCardAgents', () => ({ ) })) -vi.mock('./SshDisconnectedDialog', () => ({ - SshDisconnectedDialog: () => null -})) - vi.mock('./WorktreeContextMenu', () => ({ default: ({ children }: { children: ReactNode }) => <>{children}, CLOSE_ALL_CONTEXT_MENUS_EVENT: 'orca:test-close-context-menus', diff --git a/src/renderer/src/components/sidebar/WorktreeCard.compact-ports-hover-independence.test.tsx b/src/renderer/src/components/sidebar/WorktreeCard.compact-ports-hover-independence.test.tsx index 3887c52c79d0..5aee22c8b163 100644 --- a/src/renderer/src/components/sidebar/WorktreeCard.compact-ports-hover-independence.test.tsx +++ b/src/renderer/src/components/sidebar/WorktreeCard.compact-ports-hover-independence.test.tsx @@ -124,10 +124,6 @@ vi.mock('./WorktreeCardAgents', () => ({ default: () =>
})) -vi.mock('./SshDisconnectedDialog', () => ({ - SshDisconnectedDialog: () => null -})) - vi.mock('./WorktreeContextMenu', () => ({ default: ({ children }: { children: ReactNode }) => <>{children}, CLOSE_ALL_CONTEXT_MENUS_EVENT: 'orca:test-close-context-menus', diff --git a/src/renderer/src/components/sidebar/WorktreeCard.hosted-review-refresh.test.tsx b/src/renderer/src/components/sidebar/WorktreeCard.hosted-review-refresh.test.tsx index 3cc0f9dd1a0c..98c2a1b3712d 100644 --- a/src/renderer/src/components/sidebar/WorktreeCard.hosted-review-refresh.test.tsx +++ b/src/renderer/src/components/sidebar/WorktreeCard.hosted-review-refresh.test.tsx @@ -63,10 +63,6 @@ vi.mock('./WorktreeCardAgents', () => ({ default: () => null })) -vi.mock('./SshDisconnectedDialog', () => ({ - SshDisconnectedDialog: () => null -})) - vi.mock('./WorktreeContextMenu', () => ({ default: ({ children }: { children: ReactNode }) => <>{children}, CLOSE_ALL_CONTEXT_MENUS_EVENT: 'orca:test-close-context-menus', diff --git a/src/renderer/src/components/sidebar/WorktreeCard.lineage.test.tsx b/src/renderer/src/components/sidebar/WorktreeCard.lineage.test.tsx index 71fa30bed265..cbbf5a35b862 100644 --- a/src/renderer/src/components/sidebar/WorktreeCard.lineage.test.tsx +++ b/src/renderer/src/components/sidebar/WorktreeCard.lineage.test.tsx @@ -53,10 +53,6 @@ vi.mock('./WorktreeCardAgents', () => ({ default: () => null })) -vi.mock('./SshDisconnectedDialog', () => ({ - SshDisconnectedDialog: () => null -})) - vi.mock('./WorktreeContextMenu', () => ({ default: ({ children }: { children: ReactNode }) => <>{children}, CLOSE_ALL_CONTEXT_MENUS_EVENT: 'orca:test-close-context-menus', diff --git a/src/renderer/src/components/sidebar/WorktreeCard.merged-pr-display.test.tsx b/src/renderer/src/components/sidebar/WorktreeCard.merged-pr-display.test.tsx index 9df826e66d08..02927202e294 100644 --- a/src/renderer/src/components/sidebar/WorktreeCard.merged-pr-display.test.tsx +++ b/src/renderer/src/components/sidebar/WorktreeCard.merged-pr-display.test.tsx @@ -68,10 +68,6 @@ vi.mock('./WorktreeCardAgents', () => ({ default: () => null })) -vi.mock('./SshDisconnectedDialog', () => ({ - SshDisconnectedDialog: () => null -})) - vi.mock('./WorktreeContextMenu', () => ({ default: ({ children }: { children: ReactNode }) => <>{children}, CLOSE_ALL_CONTEXT_MENUS_EVENT: 'orca:test-close-context-menus', diff --git a/src/renderer/src/components/sidebar/WorktreeCard.pinned-repo-icon.test.tsx b/src/renderer/src/components/sidebar/WorktreeCard.pinned-repo-icon.test.tsx index 88d5d65d8187..9da7873b3884 100644 --- a/src/renderer/src/components/sidebar/WorktreeCard.pinned-repo-icon.test.tsx +++ b/src/renderer/src/components/sidebar/WorktreeCard.pinned-repo-icon.test.tsx @@ -59,10 +59,6 @@ vi.mock('./WorktreeCardAgents', () => ({ default: () => null })) -vi.mock('./SshDisconnectedDialog', () => ({ - SshDisconnectedDialog: () => null -})) - vi.mock('./WorktreeContextMenu', () => ({ default: ({ children }: { children: ReactNode }) => <>{children}, CLOSE_ALL_CONTEXT_MENUS_EVENT: 'orca:test-close-context-menus', diff --git a/src/renderer/src/components/sidebar/WorktreeCard.pr-display.test.tsx b/src/renderer/src/components/sidebar/WorktreeCard.pr-display.test.tsx index 7df0cd25c94f..fef27ea71ccf 100644 --- a/src/renderer/src/components/sidebar/WorktreeCard.pr-display.test.tsx +++ b/src/renderer/src/components/sidebar/WorktreeCard.pr-display.test.tsx @@ -72,10 +72,6 @@ vi.mock('./WorktreeCardAgents', () => ({ default: () => null })) -vi.mock('./SshDisconnectedDialog', () => ({ - SshDisconnectedDialog: () => null -})) - vi.mock('./WorktreeContextMenu', () => ({ default: ({ children }: { children: ReactNode }) => <>{children}, CLOSE_ALL_CONTEXT_MENUS_EVENT: 'orca:test-close-context-menus', diff --git a/src/renderer/src/components/sidebar/WorktreeCard.quick-actions.test.tsx b/src/renderer/src/components/sidebar/WorktreeCard.quick-actions.test.tsx index d21085ab3cbf..7456cce2faa0 100644 --- a/src/renderer/src/components/sidebar/WorktreeCard.quick-actions.test.tsx +++ b/src/renderer/src/components/sidebar/WorktreeCard.quick-actions.test.tsx @@ -72,10 +72,6 @@ vi.mock('./WorktreeCardAgents', () => ({ default: () => null })) -vi.mock('./SshDisconnectedDialog', () => ({ - SshDisconnectedDialog: () => null -})) - vi.mock('./WorktreeContextMenu', () => ({ default: ({ children }: { children: ReactNode }) => <>{children}, CLOSE_ALL_CONTEXT_MENUS_EVENT: 'orca:test-close-context-menus', diff --git a/src/renderer/src/components/sidebar/WorktreeCard.ssh-reconnect-prompt.test.tsx b/src/renderer/src/components/sidebar/WorktreeCard.ssh-reconnect-prompt.test.tsx index 1b2b6c9f664d..6f5c10d40c4b 100644 --- a/src/renderer/src/components/sidebar/WorktreeCard.ssh-reconnect-prompt.test.tsx +++ b/src/renderer/src/components/sidebar/WorktreeCard.ssh-reconnect-prompt.test.tsx @@ -70,24 +70,6 @@ vi.mock('./use-worktree-activity-status', () => ({ useWorktreeActivityStatus: () => 'idle' })) -vi.mock('./SshDisconnectedDialog', () => ({ - SshDisconnectedDialog: ({ - open, - status, - targetLabel - }: { - open: boolean - status: string - targetLabel: string - }) => ( -
- ) -})) - vi.mock('./WorktreeContextMenu', () => ({ default: ({ children }: { children: ReactNode }) => <>{children}, CLOSE_ALL_CONTEXT_MENUS_EVENT: 'orca:test-close-context-menus', @@ -144,7 +126,9 @@ describe('WorktreeCard SSH reconnect prompt', () => { worktreeCardProperties = ['status'] }) - it('does not auto-open the blocking reconnect dialog for a restored active disconnected SSH worktree', () => { + // Supersedes the old "does not auto-open the blocking reconnect dialog" assertion: + // the dialog is gone, so no card state can open one. + it('offers an inline reconnect control and never a blocking dialog for a disconnected SSH worktree', () => { sshConnectionStates.set('ssh-target-1', { status: 'disconnected' }) sshTargetLabels.set('ssh-target-1', 'Remote target') @@ -152,11 +136,50 @@ describe('WorktreeCard SSH reconnect prompt', () => { ) - // The dialog is blocking, so being the active/restored card must not steal - // focus app-wide; it only opens on deliberate click (see handleClick). - expect(markup).toContain('data-ssh-disconnected-dialog="closed"') - // The disconnected state is still discoverable via the non-blocking card chip. - expect(markup).toContain('SSH disconnected') + expect(markup).toContain('Connect to SSH host Remote target') + expect(markup).toContain('Connect') + expect(markup).not.toContain('ssh-disconnected-dialog') + }) + + it('names the failure state in the control verb rather than a generic Connect', () => { + sshConnectionStates.set('ssh-target-1', { status: 'auth-failed' }) + sshTargetLabels.set('ssh-target-1', 'Remote target') + + const markup = renderToStaticMarkup( + + ) + + expect(markup).toContain('Reconnect SSH host Remote target') + expect(markup).toContain('authentication failed') + }) + + it('renders the passive host glyph, not a control, when the SSH host is connected', () => { + sshConnectionStates.set('ssh-target-1', { status: 'connected' }) + sshTargetLabels.set('ssh-target-1', 'Remote target') + + const markup = renderToStaticMarkup( + + ) + + expect(markup).toContain('Project on SSH host Remote target') + expect(markup).not.toContain('Connect to SSH host') + }) + + // Why: the control keys off repo.connectionId, which is orthogonal to workspace kind — + // a folder workspace on a disconnected SSH host must get the same affordance. + it('offers the control for a folder workspace on a disconnected SSH host', () => { + sshConnectionStates.set('ssh-target-1', { status: 'error' }) + sshTargetLabels.set('ssh-target-1', 'Remote target') + + const markup = renderToStaticMarkup( + + ) + + expect(markup).toContain('Retry SSH connection to Remote target') }) it('marks a runtime-host worktree disconnected when its environment has no status', () => { diff --git a/src/renderer/src/components/sidebar/WorktreeCard.tsx b/src/renderer/src/components/sidebar/WorktreeCard.tsx index 9cc437fc5136..f2a22e58c178 100644 --- a/src/renderer/src/components/sidebar/WorktreeCard.tsx +++ b/src/renderer/src/components/sidebar/WorktreeCard.tsx @@ -21,13 +21,13 @@ import { } from 'lucide-react' import CacheTimer, { usePromptCacheCountdownStartedAt } from './CacheTimer' import WorktreeContextMenu from './WorktreeContextMenu' -import { SshDisconnectedDialog } from './SshDisconnectedDialog' import { AutoRenameFailedDialog } from './AutoRenameFailedDialog' import { LinearAgentSkillSetupPrompt } from './LinearAgentSkillSetupPrompt' import WorktreeCardAgents from './WorktreeCardAgents' import { useWorktreeAgentRows } from './useWorktreeAgentRows' import { WorktreeCardStatusSlot } from './WorktreeCardStatusSlot' import { cn } from '@/lib/utils' +import { WorktreeCardSshHostControl } from './WorktreeCardSshHostControl' import { activateWorktreeFromSidebar } from '@/lib/sidebar-worktree-activation' import { isFolderRepo } from '../../../../shared/repo-kind' import type { HostedReviewInfo } from '../../../../shared/hosted-review' @@ -94,7 +94,8 @@ import { DEFAULT_AGENT_ACTIVITY_DISPLAY_MODE } from '../../../../shared/constant import { getExplicitRuntimeEnvironmentIdForWorktree } from '@/lib/worktree-runtime-owner' import { selectRuntimeAwareSshStatus, - selectRuntimeAwareSshTargetLabel + selectRuntimeAwareSshTargetLabel, + selectRuntimeAwareSshTargetRemoved } from '@/store/slices/runtime-environment-ssh' import { hydrateRuntimeEnvironmentSshState } from '@/runtime/runtime-environment-ssh-state' @@ -362,9 +363,11 @@ const WorktreeCard = React.memo(function WorktreeCard({ } }, [sshOwnerEnvironmentId]) const isSshDisconnected = sshStatus != null && sshStatus !== 'connected' - // Why: terminal views have their own reconnect overlay; reserve the blocking dialog for non-terminal views (default to terminal when ambiguous). - const activeViewIsTerminal = useAppStore( - (s) => (s.activeTabTypeByWorktree?.[worktree.id] ?? 'terminal') === 'terminal' + // Why: only reported on positive evidence, so a removed host never offers a Connect that can only fail. + const sshTargetRemoved = useAppStore((s) => + repo?.connectionId + ? selectRuntimeAwareSshTargetRemoved(s, sshOwnerEnvironmentId, repo.connectionId) + : false ) const parsedRepoHost = parseExecutionHostId(repo?.executionHostId) @@ -390,8 +393,6 @@ const WorktreeCard = React.memo(function WorktreeCard({ } return !s.runtimeStatusByEnvironmentId.get(runtimeOwnerEnvironmentId)?.status }) - // Why: the reconnect dialog blocks, so it never auto-shows for the active card (would steal app-wide focus); opens only on deliberate focus (handleClick). - const [showDisconnectedDialog, setShowDisconnectedDialog] = useState(false) const [titleRenaming, setTitleRenaming] = useState(false) const [showRenameErrorDialog, setShowRenameErrorDialog] = useState(false) // Why: read the target label from its owning host's store instead of exposing HUB-private SSH metadata as client-local state. @@ -877,10 +878,6 @@ const WorktreeCard = React.memo(function WorktreeCard({ worktree.id, worktree.hostId ?? (repo ? getRepoExecutionHostId(repo) : undefined) ) - // Why: a deliberate card click warrants the blocking reconnect prompt; skip it when a terminal already shows the overlay. - if (isSshDisconnected && !activeViewIsTerminal) { - setShowDisconnectedDialog(true) - } onActivate?.() }, [ @@ -893,7 +890,6 @@ const WorktreeCard = React.memo(function WorktreeCard({ isDeleting, activationRowKey, isSshDisconnected, - activeViewIsTerminal, onActivate, onImmediateActivate, onSelectionGesture @@ -1428,28 +1424,15 @@ const WorktreeCard = React.memo(function WorktreeCard({ )} {repo?.connectionId && ( - - - - {isSshDisconnected ? ( - - ) : ( - - )} - - - - {isSshDisconnected - ? translate( - 'auto.components.sidebar.WorktreeCard.021538e1d1', - 'SSH disconnected' - ) - : translate( - 'auto.components.sidebar.WorktreeCard.ca74db7550', - 'Project on SSH host' - )} - - + )} {!repo?.connectionId && parsedRepoHost?.kind === 'runtime' && ( @@ -1457,7 +1440,10 @@ const WorktreeCard = React.memo(function WorktreeCard({ {isRuntimeDisconnected ? ( - + // Passive by design: runtime ("Orca server") hosts have no + // renderer-reachable connect API, unlike the SSH glyph above which is + // now a control. Don't "fix" the inconsistency by wiring one up. + ) : ( )} @@ -1940,16 +1926,6 @@ const WorktreeCard = React.memo(function WorktreeCard({ )} - {repo?.connectionId && ( - - )} - {typeof worktree.firstAgentMessageRenameError === 'string' && worktree.firstAgentMessageRenameError.length > 0 && ( ({ error: vi.fn() })) + +const environmentSshMocks = vi.hoisted(() => ({ + connectRuntimeEnvironmentSshTarget: vi.fn(), + resyncRuntimeEnvironmentSshTargets: vi.fn() +})) + +vi.mock('sonner', () => ({ toast: { error: toastMocks.error } })) + +vi.mock('@/runtime/runtime-environment-ssh-state', () => environmentSshMocks) + +vi.mock('@/i18n/i18n', () => ({ + translate: (_key: string, fallback: string, values?: Record) => + fallback.replace('{{value0}}', values?.value0 ?? '') +})) + +vi.mock('@/components/ui/tooltip', () => ({ + Tooltip: ({ children }: { children: React.ReactNode }) => <>{children}, + TooltipContent: ({ children }: { children: React.ReactNode }) => ( + {children} + ), + TooltipTrigger: ({ children }: { children: React.ReactNode }) => <>{children} +})) + +function installSshApi( + connect: ReturnType, + overrides: Record> = {} +): void { + Object.defineProperty(window, 'api', { + configurable: true, + value: { + ssh: { + connect, + listTargets: vi.fn().mockResolvedValue([]), + listRemovedTargetLabels: vi.fn().mockResolvedValue({}), + ...overrides + } + } + }) +} + +function renderControl( + props: Partial> = {} +) { + return render( + {}} + {...props} + /> + ) +} + +describe('WorktreeCardSshHostControl', () => { + beforeEach(() => { + useAppStore.setState(useAppStore.getInitialState(), true) + resetSshConnectInFlightForTests() + toastMocks.error.mockReset() + environmentSshMocks.connectRuntimeEnvironmentSshTarget.mockReset() + environmentSshMocks.resyncRuntimeEnvironmentSshTargets.mockReset() + installSshApi(vi.fn().mockResolvedValue(undefined)) + }) + + afterEach(() => { + cleanup() + }) + + it('offers a Connect control naming the host for a disconnected target', () => { + renderControl() + + expect(screen.getByRole('button', { name: 'Connect to SSH host devbox' })).toBeEnabled() + }) + + // Why: the verb must match the terminal overlay, host-header menu, and status-bar row. + it.each([ + ['auth-failed', 'Reconnect'], + ['error', 'Retry'], + ['reconnection-failed', 'Retry'], + ['disconnected', 'Connect'] + ] as const)('labels the %s state %s', (status, verb) => { + renderControl({ status }) + + expect(screen.getByRole('button')).toHaveTextContent(verb) + }) + + it('distinguishes an auth failure from a generic connection failure in the tooltip', () => { + const { container } = renderControl({ status: 'auth-failed' }) + expect(container.querySelector('[data-tooltip]')).toHaveTextContent( + 'devbox · authentication failed' + ) + + cleanup() + const retry = renderControl({ status: 'error' }) + expect(retry.container.querySelector('[data-tooltip]')).toHaveTextContent( + 'devbox · connection failed' + ) + }) + + it.each(['error', 'reconnection-failed', 'auth-failed'] as const)( + 'tints the %s state with the destructive token, not the quiet one', + (status) => { + renderControl({ status }) + + expect(screen.getByRole('button')).toHaveClass('text-destructive') + } + ) + + it.each(['connecting', 'deploying-relay', 'reconnecting'] as const)( + 'shows a disabled busy control while the host is %s', + (status) => { + renderControl({ status }) + + const button = screen.getByRole('button', { name: 'Connecting to SSH host devbox' }) + expect(button).toBeDisabled() + expect(button).toHaveAttribute('aria-busy', 'true') + } + ) + + it('renders the passive host glyph, not a control, when connected', () => { + renderControl({ status: 'connected' }) + + expect(screen.queryByRole('button')).not.toBeInTheDocument() + expect(screen.getByText('Project on SSH host devbox')).toBeInTheDocument() + }) + + // Why: runtime-owned targets deliberately have no renderer-visible status; the card has + // always shown the plain host glyph there rather than a false disconnected state. + it('renders the passive host glyph for a null status', () => { + renderControl({ status: null }) + + expect(screen.queryByRole('button')).not.toBeInTheDocument() + expect(screen.getByText('Project on SSH host devbox')).toBeInTheDocument() + }) + + // Why: this is the exact bug targetRemoved exists to prevent — a Connect that can only fail. + it('never offers Connect for a removed host, even in a failed state', () => { + renderControl({ status: 'error', targetRemoved: true }) + + expect(screen.queryByRole('button')).not.toBeInTheDocument() + expect(screen.getByText('SSH host devbox was removed')).toBeInTheDocument() + }) + + it('keeps the label available to assistive tech but not on screen in icon-only mode', () => { + renderControl({ iconOnly: true }) + + const button = screen.getByRole('button', { name: 'Connect to SSH host devbox' }) + expect(button.querySelector('.sr-only')).toHaveTextContent('Connect') + expect(button).toHaveClass('w-4') + }) + + it('connects and mirrors the returned state so deferred PTY reattach can resume', async () => { + const connectedState: SshConnectionState = { + targetId: 'ssh-target-1', + status: 'connected', + error: null, + reconnectAttempt: 0, + remotePlatform: 'linux' + } + const connect = vi.fn().mockResolvedValue(connectedState) + installSshApi(connect) + const user = userEvent.setup() + renderControl() + + await user.click(screen.getByRole('button')) + + expect(connect).toHaveBeenCalledWith({ targetId: 'ssh-target-1' }) + await waitFor(() => + expect(useAppStore.getState().sshConnectionStates.get('ssh-target-1')).toEqual(connectedState) + ) + }) + + // Why: reconnecting a host and navigating to its workspace are separate intents; the + // control sits inside the card's own click target. + it('does not activate the surrounding card when clicked', async () => { + const onCardClick = vi.fn() + const user = userEvent.setup() + render( +
+ {}} + /> +
+ ) + + await user.click(screen.getByRole('button')) + + expect(onCardClick).not.toHaveBeenCalled() + }) + + it('reports connect failures and resyncs target metadata so a ghost host converges', async () => { + const connect = vi.fn().mockRejectedValue(new Error('SSH target "ssh-target-1" not found')) + const listTargets = vi + .fn() + .mockResolvedValue([ + { id: 'ssh-live', label: 'devbox', host: 'devbox', port: 22, username: 'me' } + ]) + const listRemovedTargetLabels = vi + .fn() + .mockResolvedValue({ 'ssh-target-1': 'devbox (removed)' }) + installSshApi(connect, { listTargets, listRemovedTargetLabels }) + const user = userEvent.setup() + renderControl() + + await user.click(screen.getByRole('button')) + + await waitFor(() => + expect(toastMocks.error).toHaveBeenCalledWith('SSH target "ssh-target-1" not found') + ) + await waitFor(() => { + expect(useAppStore.getState().sshTargetLabels.get('ssh-live')).toBe('devbox') + expect(useAppStore.getState().removedSshTargetLabels.get('ssh-target-1')).toBe( + 'devbox (removed)' + ) + }) + }) + + it('routes connect to the owning Orca server for a remote-owned target', async () => { + const connect = vi.fn().mockResolvedValue(undefined) + installSshApi(connect) + environmentSshMocks.connectRuntimeEnvironmentSshTarget.mockResolvedValue(null) + const user = userEvent.setup() + renderControl({ sshOwnerEnvironmentId: 'env-1' }) + + await user.click(screen.getByRole('button')) + + await waitFor(() => + expect(environmentSshMocks.connectRuntimeEnvironmentSshTarget).toHaveBeenCalledWith( + 'env-1', + 'ssh-target-1' + ) + ) + // The local ssh API must never see a remote host's target. + expect(connect).not.toHaveBeenCalled() + }) + + // Why: N cards can share one host, and a passphrase-gated target would prompt N times. + it('suppresses a sibling card dialing a host that is already connecting', async () => { + const connect = vi.fn().mockReturnValue(new Promise(() => {})) + installSshApi(connect) + const user = userEvent.setup() + render( + <> + {}} + /> + {}} + /> + + ) + + const [first, second] = screen.getAllByRole('button') + await user.click(first) + + await waitFor(() => expect(second).toBeDisabled()) + expect(connect).toHaveBeenCalledTimes(1) + }) + + it('keeps the pill 16px tall in every state so the title row never shifts', () => { + const heights = new Set() + for (const status of ['disconnected', 'error', 'connecting'] as const) { + cleanup() + renderControl({ status }) + const classes = screen.getByRole('button').className + heights.add(classes.split(' ').find((token) => token.startsWith('h-')) ?? 'none') + } + + expect([...heights]).toEqual(['h-4']) + }) +}) diff --git a/src/renderer/src/components/sidebar/WorktreeCardSshHostControl.tsx b/src/renderer/src/components/sidebar/WorktreeCardSshHostControl.tsx new file mode 100644 index 000000000000..75020336e928 --- /dev/null +++ b/src/renderer/src/components/sidebar/WorktreeCardSshHostControl.tsx @@ -0,0 +1,273 @@ +import { useCallback } from 'react' +import { Loader2, Server, ServerOff } from 'lucide-react' +import { toast } from 'sonner' +import { Button } from '@/components/ui/button' +import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' +import { cn } from '@/lib/utils' +import { translate } from '@/i18n/i18n' +import { useAppStore } from '@/store' +import { + connectRuntimeEnvironmentSshTarget, + resyncRuntimeEnvironmentSshTargets +} from '@/runtime/runtime-environment-ssh-state' +import { canConnectSshStatus, isConnectingSshStatus } from '@/ssh/ssh-connection-recoverability' +import { sshConnectingLabel, sshConnectVerb } from '@/ssh/ssh-connect-verb' +import { withUiConnectTimeout } from '@/ssh/ssh-connect-ui-timeout' +import { + beginSshConnect, + endSshConnect, + isSshConnectInFlight, + useSshConnectInFlight +} from '@/ssh/ssh-connect-in-flight' +import type { SshConnectionStatus } from '../../../../shared/ssh-types' + +type WorktreeCardSshHostControlProps = { + targetId: string + /** Card passes `sshTargetLabel || repo.displayName` — the selector can return a bare target id. */ + targetLabel: string + /** Null for runtime-owned targets: renders the passive connected glyph, as before. */ + status: SshConnectionStatus | null + targetRemoved: boolean + /** Non-null when the SSH target belongs to a remote Orca server; routes connect to that runtime. */ + sshOwnerEnvironmentId: string | null + /** True when the row cannot afford a visible label: icon-only with an sr-only label. */ + iconOnly: boolean + onPointerDown: React.PointerEventHandler +} + +// One shape for every state, from the sibling rename-failed control in the same title row +// (WorktreeCard.tsx). States differ only by color token, so the pill never changes height. +const PILL_BASE = + 'h-4 shrink-0 gap-0.5 rounded !px-0.5 text-[10px] font-medium leading-none has-[>svg]:!px-0.5' +const PILL_QUIET = + 'text-muted-foreground border border-worktree-sidebar-border bg-worktree-sidebar shadow-none hover:bg-worktree-sidebar-accent hover:text-foreground focus-visible:ring-1 focus-visible:ring-worktree-sidebar-ring' +const PILL_FAILED = + 'text-destructive border border-destructive/40 bg-destructive/10 hover:bg-destructive/15 hover:text-destructive focus-visible:ring-1 focus-visible:ring-worktree-sidebar-ring' + +function PassiveGlyph({ + icon, + tooltip, + accessibleName, + targetLabel +}: { + icon: React.ReactNode + tooltip: string + accessibleName: string + targetLabel: string +}): React.JSX.Element { + return ( + + + + {icon} + {accessibleName} + + + + {tooltip} + + + ) +} + +export function WorktreeCardSshHostControl({ + targetId, + targetLabel, + status, + targetRemoved, + sshOwnerEnvironmentId, + iconOnly, + onPointerDown +}: WorktreeCardSshHostControlProps): React.JSX.Element | null { + const setSshConnectionState = useAppStore((store) => store.setSshConnectionState) + // Why: shared registry, not local state — the terminal overlay and every other card on + // this host dial the same connection, and the store status lags a click by one IPC hop. + const inFlight = useSshConnectInFlight(targetId) + + const handleConnect = useCallback(async () => { + if (isSshConnectInFlight(targetId) || isConnectingSshStatus(status)) { + return + } + beginSshConnect(targetId) + try { + if (sshOwnerEnvironmentId) { + // Bucket state is written inside the helper, mirroring the local path. + await connectRuntimeEnvironmentSshTarget(sshOwnerEnvironmentId, targetId) + } else { + const connectState = await withUiConnectTimeout(window.api.ssh.connect({ targetId })) + if (connectState) { + // Why: ssh.connect can resolve before the global state-change IPC lands; + // the waiting deferred PTY reattach path keys off this renderer store. + setSshConnectionState(targetId, connectState) + } + } + } catch (err) { + toast.error( + err instanceof Error + ? err.message + : translate( + 'auto.components.sidebar.WorktreeCardSshHostControl.connectFailed', + 'SSH connection failed' + ) + ) + // Why: a failed connect usually means the renderer's target metadata is stale + // (target removed, or re-added under a new id). Resync so the control converges to + // the removed state instead of offering the same failing Connect forever (STA-1468). + // Apply the target list first — a removed-labels failure must not discard it. + if (sshOwnerEnvironmentId) { + void resyncRuntimeEnvironmentSshTargets(sshOwnerEnvironmentId).catch(() => {}) + } else { + void (async () => { + const targets = await window.api.ssh.listTargets() + useAppStore.getState().setSshTargetsMetadata(targets) + const removedLabels = await window.api.ssh.listRemovedTargetLabels() + useAppStore.getState().setRemovedSshTargetLabels(removedLabels) + })().catch(() => {}) + } + } finally { + // Registry state outlives this component; sidebar rows unmount under virtualization. + endSshConnect(targetId) + } + }, [setSshConnectionState, sshOwnerEnvironmentId, status, targetId]) + + // Why: a removed host can never connect, so it is checked before any status — offering + // Connect there is the exact bug targetRemoved exists to prevent. It also drops the + // destructive tint: a removed host is a settled fact, not an error to act on. + if (targetRemoved) { + return ( + } + tooltip={translate( + 'auto.components.sidebar.WorktreeCardSshHostControl.removedTooltip', + 'SSH host removed — reconnect unavailable' + )} + accessibleName={translate( + 'auto.components.sidebar.WorktreeCardSshHostControl.removedName', + 'SSH host {{value0}} was removed', + { value0: targetLabel } + )} + /> + ) + } + + // A null status is a runtime-owned target: no renderer-reachable connect, and the card + // has always shown the plain host glyph here rather than a disconnected state. + if (status === null || status === 'connected') { + return ( + } + tooltip={translate( + 'auto.components.sidebar.WorktreeCardSshHostControl.connectedTooltip', + 'Project on SSH host' + )} + accessibleName={translate( + 'auto.components.sidebar.WorktreeCardSshHostControl.connectedName', + 'Project on SSH host {{value0}}', + { value0: targetLabel } + )} + /> + ) + } + + const connecting = inFlight || isConnectingSshStatus(status) + const canConnect = canConnectSshStatus(status) + if (!connecting && !canConnect) { + // Defensive: every remaining member is either connecting or recoverable, but never + // render a dead button if the union grows. + return null + } + + const failed = status === 'error' || status === 'reconnection-failed' || status === 'auth-failed' + const label = connecting ? sshConnectingLabel() : sshConnectVerb(status) + const accessibleName = connecting + ? translate( + 'auto.components.sidebar.WorktreeCardSshHostControl.connectingName', + 'Connecting to SSH host {{value0}}', + { value0: targetLabel } + ) + : status === 'auth-failed' + ? translate( + 'auto.components.sidebar.WorktreeCardSshHostControl.authFailedName', + 'Reconnect SSH host {{value0}} — authentication failed', + { value0: targetLabel } + ) + : failed + ? translate( + 'auto.components.sidebar.WorktreeCardSshHostControl.retryName', + 'Retry SSH connection to {{value0}}', + { value0: targetLabel } + ) + : translate( + 'auto.components.sidebar.WorktreeCardSshHostControl.connectName', + 'Connect to SSH host {{value0}}', + { value0: targetLabel } + ) + const tooltip = connecting + ? accessibleName + : status === 'auth-failed' + ? translate( + 'auto.components.sidebar.WorktreeCardSshHostControl.authFailedTooltip', + '{{value0}} · authentication failed', + { value0: targetLabel } + ) + : failed + ? translate( + 'auto.components.sidebar.WorktreeCardSshHostControl.failedTooltip', + '{{value0}} · connection failed', + { value0: targetLabel } + ) + : accessibleName + + return ( + <> + + + + + + {tooltip} + + + {/* Why: announce transitions only, not every render, or a busy sidebar spams AT. */} + + {accessibleName} + + + ) +} diff --git a/src/renderer/src/components/sidebar/WorktreeList.lineage-agent-expansion-coupling.test.tsx b/src/renderer/src/components/sidebar/WorktreeList.lineage-agent-expansion-coupling.test.tsx index 82630b66d8bf..5a644f0d265c 100644 --- a/src/renderer/src/components/sidebar/WorktreeList.lineage-agent-expansion-coupling.test.tsx +++ b/src/renderer/src/components/sidebar/WorktreeList.lineage-agent-expansion-coupling.test.tsx @@ -144,10 +144,6 @@ vi.mock('./CacheTimer', () => ({ // NOTE: intentionally NOT mocking ./WorktreeCardAgents — we render the real one. -vi.mock('./SshDisconnectedDialog', () => ({ - SshDisconnectedDialog: () => null -})) - vi.mock('./WorktreeContextMenu', () => ({ default: ({ children }: { children: ReactNode }) => <>{children}, CLOSE_ALL_CONTEXT_MENUS_EVENT: 'orca:test-close-context-menus', diff --git a/src/renderer/src/components/sidebar/WorktreeList.lineage-child-card.test.ts b/src/renderer/src/components/sidebar/WorktreeList.lineage-child-card.test.ts index 53d16e0d7b44..9d97e5020652 100644 --- a/src/renderer/src/components/sidebar/WorktreeList.lineage-child-card.test.ts +++ b/src/renderer/src/components/sidebar/WorktreeList.lineage-child-card.test.ts @@ -212,26 +212,6 @@ vi.mock('./WorktreeContextMenu', () => ({ WORKTREE_CONTEXT_MENU_SCOPE_ATTR: 'data-orca-context-menu-scope' })) -vi.mock('./SshDisconnectedDialog', () => ({ - SshDisconnectedDialog: ({ - open, - status, - targetId, - targetLabel - }: { - open: boolean - status: string - targetId: string - targetLabel: string - }) => - React.createElement('aside', { - 'data-lineage-ssh-dialog': open ? 'open' : 'closed', - 'data-ssh-status': status, - 'data-ssh-target-id': targetId, - 'data-ssh-target-label': targetLabel - }) -})) - vi.mock('@/components/ui/tooltip', () => ({ Tooltip: ({ children }: { children: React.ReactNode }) => React.createElement(React.Fragment, null, children), diff --git a/src/renderer/src/components/sidebar/WorktreeList.lineage-child-real-card.test.tsx b/src/renderer/src/components/sidebar/WorktreeList.lineage-child-real-card.test.tsx index 7a5dfbb13f94..52f3a5914739 100644 --- a/src/renderer/src/components/sidebar/WorktreeList.lineage-child-real-card.test.tsx +++ b/src/renderer/src/components/sidebar/WorktreeList.lineage-child-real-card.test.tsx @@ -142,10 +142,6 @@ vi.mock('./WorktreeCardAgents', () => ({ SUPPRESS_WORKTREE_LIST_SCROLL_ADJUSTMENT_EVENT: 'orca:test-suppress-scroll-adjustment' })) -vi.mock('./SshDisconnectedDialog', () => ({ - SshDisconnectedDialog: () => null -})) - vi.mock('./WorktreeContextMenu', () => ({ default: ({ children }: { children: ReactNode }) => <>{children}, CLOSE_ALL_CONTEXT_MENUS_EVENT: 'orca:test-close-context-menus', diff --git a/src/renderer/src/components/sidebar/WorktreeList.status-lane-lineage-drop.test.tsx b/src/renderer/src/components/sidebar/WorktreeList.status-lane-lineage-drop.test.tsx index 0311879c5b79..00c9e47a0c80 100644 --- a/src/renderer/src/components/sidebar/WorktreeList.status-lane-lineage-drop.test.tsx +++ b/src/renderer/src/components/sidebar/WorktreeList.status-lane-lineage-drop.test.tsx @@ -143,10 +143,6 @@ vi.mock('./WorktreeCardAgents', () => ({ SUPPRESS_WORKTREE_LIST_SCROLL_ADJUSTMENT_EVENT: 'orca:test-suppress-scroll-adjustment' })) -vi.mock('./SshDisconnectedDialog', () => ({ - SshDisconnectedDialog: () => null -})) - vi.mock('./WorktreeContextMenu', () => ({ default: ({ children }: { children: ReactNode }) => <>{children}, CLOSE_ALL_CONTEXT_MENUS_EVENT: 'orca:test-close-context-menus', diff --git a/src/renderer/src/components/status-bar/SshTargetStatusRow.tsx b/src/renderer/src/components/status-bar/SshTargetStatusRow.tsx index a19b17b40e9f..712d4db9e153 100644 --- a/src/renderer/src/components/status-bar/SshTargetStatusRow.tsx +++ b/src/renderer/src/components/status-bar/SshTargetStatusRow.tsx @@ -5,13 +5,11 @@ import { translate } from '@/i18n/i18n' import { useMountedRef } from '@/hooks/useMountedRef' import { useAppStore } from '../../store' import { STATUS_LABELS, statusColor } from '../settings/SshTargetCard' +import { canConnectSshStatus } from '@/ssh/ssh-connection-recoverability' +import { sshConnectVerb } from '@/ssh/ssh-connect-verb' import type { SshConnectionStatus } from '../../../../shared/ssh-types' import type { RemoteWorkspaceSyncStatus } from '../../store/slices/ssh' -function isReconnectable(status: SshConnectionStatus): boolean { - return ['disconnected', 'reconnection-failed', 'error', 'auth-failed'].includes(status) -} - function syncStatusLabel(status: RemoteWorkspaceSyncStatus | undefined): string | null { switch (status?.phase) { case 'pulling': @@ -132,13 +130,13 @@ export function SshTargetStatusRow({
{busy ? ( - ) : isReconnectable(status) ? ( + ) : canConnectSshStatus(status) ? ( ) : status === 'connected' ? ( )} diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index fdace14f0d68..b1eb09008658 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -8246,7 +8246,7 @@ export function connectPanePty( const alreadyConnected = useAppStore.getState().sshConnectionStates.get(connectionId)?.status === 'connected' if (!alreadyConnected) { - // Wait for the user-driven connect (SshDisconnectedDialog → passphrase → ssh.connect) to complete. + // Wait for the user-driven connect (sidebar card control or terminal reconnect overlay → passphrase → ssh.connect) to complete. // Why: resolve on terminal-failure statuses too ('auth-failed'/'error'/'reconnection-failed') so it can't hang forever if the user cancels or the connect fails. const outcome = await new Promise((resolve) => { // Why: 'disconnected' counts as terminal only after a non-disconnected status was seen (a real connect attempt that returned to 'disconnected'). diff --git a/src/renderer/src/hooks/useIpcEvents.ts b/src/renderer/src/hooks/useIpcEvents.ts index 35c48344e634..bed3e4e86c89 100644 --- a/src/renderer/src/hooks/useIpcEvents.ts +++ b/src/renderer/src/hooks/useIpcEvents.ts @@ -24,6 +24,7 @@ import type { SplitTerminalPaneDetail, CloseTerminalPaneDetail } from '@/constan import { getVisibleWorktreeIds } from '@/components/sidebar/visible-worktrees' import { activateTabNumberShortcut } from '@/lib/tab-number-shortcuts' import { nextEditorFontZoomLevel, computeEditorFontSize } from '@/lib/editor-font-zoom' +import { canConnectSshStatus } from '@/ssh/ssh-connection-recoverability' import type { TerminalLayoutSnapshot, TerminalPaneLayoutNode, @@ -2806,7 +2807,7 @@ export function useIpcEvents(): void { const previous = store.sshConnectionStates?.get(targetId) store.setSshConnectionState(targetId, state) - if (['disconnected', 'auth-failed', 'reconnection-failed', 'error'].includes(state.status)) { + if (canConnectSshStatus(state.status)) { reconnectAuthorityByTarget.delete(targetId) reconnectCoordinator.invalidate(targetId) // Why: remote agent list is tied to a live relay; clear on disconnect so reconnect re-detects against the new relay. diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index b3ce98a4edd4..b8b1233eea6b 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -5155,6 +5155,19 @@ "3b7ea51793": "Clear search", "7f1c2e94a5": "Search text is too long — the board is unfiltered", "9a4d0f6b21": "Too long" + }, + "WorktreeCardSshHostControl": { + "connectFailed": "SSH connection failed", + "removedTooltip": "SSH host removed — reconnect unavailable", + "removedName": "SSH host {{value0}} was removed", + "connectedTooltip": "Project on SSH host", + "connectedName": "Project on SSH host {{value0}}", + "connectingName": "Connecting to SSH host {{value0}}", + "authFailedName": "Reconnect SSH host {{value0}} — authentication failed", + "retryName": "Retry SSH connection to {{value0}}", + "connectName": "Connect to SSH host {{value0}}", + "authFailedTooltip": "{{value0}} · authentication failed", + "failedTooltip": "{{value0}} · connection failed" } }, "shared": { @@ -14599,6 +14612,14 @@ "emptyTrace": "No log is available for this GitLab job.", "timedOut": "Timed out loading the GitLab job log." } + }, + "ssh": { + "sshConnectVerb": { + "reconnect": "Reconnect", + "retry": "Retry", + "connect": "Connect", + "connecting": "Connecting…" + } } }, "components": { diff --git a/src/renderer/src/lib/new-workspace-ssh-gate.ts b/src/renderer/src/lib/new-workspace-ssh-gate.ts index f597e18e2457..ac1ae7afe55b 100644 --- a/src/renderer/src/lib/new-workspace-ssh-gate.ts +++ b/src/renderer/src/lib/new-workspace-ssh-gate.ts @@ -1,3 +1,4 @@ +import { isConnectingSshStatus } from '@/ssh/ssh-connection-recoverability' import { isRuntimeOwnedSshTargetId } from '../../../shared/execution-host' import type { SshConnectionStatus } from '../../../shared/ssh-types' @@ -9,7 +10,7 @@ export type SelectedRepoSshGate = { } export function isSshConnectInProgress(status: SshConnectionStatus | null): boolean { - return status === 'connecting' || status === 'deploying-relay' || status === 'reconnecting' + return isConnectingSshStatus(status) } export function getSelectedRepoSshGate(input: { diff --git a/src/renderer/src/ssh/ssh-connect-in-flight.test.ts b/src/renderer/src/ssh/ssh-connect-in-flight.test.ts new file mode 100644 index 000000000000..7e215dff9936 --- /dev/null +++ b/src/renderer/src/ssh/ssh-connect-in-flight.test.ts @@ -0,0 +1,73 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { + beginSshConnect, + endSshConnect, + isSshConnectInFlight, + resetSshConnectInFlightForTests, + subscribeSshConnectInFlight +} from './ssh-connect-in-flight' + +describe('ssh connect in-flight registry', () => { + beforeEach(() => { + resetSshConnectInFlightForTests() + }) + + it('tracks connects per target, so one host dialing does not disable another', () => { + beginSshConnect('ssh-a') + + expect(isSshConnectInFlight('ssh-a')).toBe(true) + expect(isSshConnectInFlight('ssh-b')).toBe(false) + }) + + it('clears the target when the connect settles', () => { + beginSshConnect('ssh-a') + endSshConnect('ssh-a') + + expect(isSshConnectInFlight('ssh-a')).toBe(false) + }) + + it('notifies subscribers on both edges', () => { + const listener = vi.fn() + subscribeSshConnectInFlight(listener) + + beginSshConnect('ssh-a') + endSshConnect('ssh-a') + + expect(listener).toHaveBeenCalledTimes(2) + }) + + // Why: every surface renders from one registry entry, so a duplicate begin must not + // emit again — and the paired end must not clear the flag while a connect is still live. + it('ignores a duplicate begin without re-notifying', () => { + const listener = vi.fn() + subscribeSshConnectInFlight(listener) + + beginSshConnect('ssh-a') + beginSshConnect('ssh-a') + + expect(listener).toHaveBeenCalledTimes(1) + expect(isSshConnectInFlight('ssh-a')).toBe(true) + }) + + // Why: handleConnect ends in a finally block that can run for a target it never began + // (early return paths), and a spurious notify would re-render every subscribed card. + it('ignores an end for a target that was never in flight', () => { + const listener = vi.fn() + subscribeSshConnectInFlight(listener) + + endSshConnect('ssh-a') + + expect(listener).not.toHaveBeenCalled() + expect(isSshConnectInFlight('ssh-a')).toBe(false) + }) + + it('stops notifying after unsubscribe, so unmounted sidebar rows do not leak', () => { + const listener = vi.fn() + const unsubscribe = subscribeSshConnectInFlight(listener) + + unsubscribe() + beginSshConnect('ssh-a') + + expect(listener).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/ssh/ssh-connect-in-flight.ts b/src/renderer/src/ssh/ssh-connect-in-flight.ts new file mode 100644 index 000000000000..9f95a2d2951f --- /dev/null +++ b/src/renderer/src/ssh/ssh-connect-in-flight.ts @@ -0,0 +1,51 @@ +import { useCallback, useSyncExternalStore } from 'react' + +// Why: the store status lags a user click by one IPC hop (main broadcasts 'connecting' +// after ssh.connect starts), and every workspace card on a host shares one connection. +// Component-local state would let two surfaces — or N cards on the same host — each fire +// a connect, which on a passphrase-gated target means N credential prompts. +const inFlightTargetIds = new Set() +const listeners = new Set<() => void>() + +function emit(): void { + for (const listener of listeners) { + listener() + } +} + +export function subscribeSshConnectInFlight(listener: () => void): () => void { + listeners.add(listener) + return () => { + listeners.delete(listener) + } +} + +export function beginSshConnect(targetId: string): void { + if (inFlightTargetIds.has(targetId)) { + return + } + inFlightTargetIds.add(targetId) + emit() +} + +export function endSshConnect(targetId: string): void { + if (!inFlightTargetIds.delete(targetId)) { + return + } + emit() +} + +export function isSshConnectInFlight(targetId: string): boolean { + return inFlightTargetIds.has(targetId) +} + +export function useSshConnectInFlight(targetId: string): boolean { + const getSnapshot = useCallback(() => inFlightTargetIds.has(targetId), [targetId]) + return useSyncExternalStore(subscribeSshConnectInFlight, getSnapshot, getSnapshot) +} + +/** Test-only: the registry is module state, so specs must reset it between cases. */ +export function resetSshConnectInFlightForTests(): void { + inFlightTargetIds.clear() + emit() +} diff --git a/src/renderer/src/ssh/ssh-connect-ui-timeout.test.ts b/src/renderer/src/ssh/ssh-connect-ui-timeout.test.ts new file mode 100644 index 000000000000..74688e70342a --- /dev/null +++ b/src/renderer/src/ssh/ssh-connect-ui-timeout.test.ts @@ -0,0 +1,43 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { SSH_CONNECT_UI_TIMEOUT_MS, withUiConnectTimeout } from './ssh-connect-ui-timeout' + +vi.mock('@/i18n/i18n', () => ({ + translate: (_key: string, fallback: string) => fallback +})) + +describe('withUiConnectTimeout', () => { + beforeEach(() => { + vi.useFakeTimers() + }) + + afterEach(() => { + vi.useRealTimers() + }) + + it('passes a resolved connect state straight through', async () => { + await expect(withUiConnectTimeout(Promise.resolve('connected'))).resolves.toBe('connected') + }) + + it('propagates the connect rejection rather than the timeout message', async () => { + const failing = withUiConnectTimeout(Promise.reject(new Error('Passphrase rejected'))) + + await expect(failing).rejects.toThrow('Passphrase rejected') + }) + + // Why: ssh.connect has no timeout of its own, so a stalled backend would otherwise leave + // the control disabled with a spinner forever. + it('rejects a stalled connect once the UI budget elapses', async () => { + const stalled = withUiConnectTimeout(new Promise(() => {})) + const assertion = expect(stalled).rejects.toThrow(/Connection timed out/) + + await vi.advanceTimersByTimeAsync(SSH_CONNECT_UI_TIMEOUT_MS) + + await assertion + }) + + it('clears the timer when the connect settles first, leaving no pending work', async () => { + await withUiConnectTimeout(Promise.resolve('connected')) + + expect(vi.getTimerCount()).toBe(0) + }) +}) diff --git a/src/renderer/src/ssh/ssh-connect-ui-timeout.ts b/src/renderer/src/ssh/ssh-connect-ui-timeout.ts new file mode 100644 index 000000000000..0f9ae58c4f67 --- /dev/null +++ b/src/renderer/src/ssh/ssh-connect-ui-timeout.ts @@ -0,0 +1,31 @@ +import { translate } from '@/i18n/i18n' + +// Why: ssh.connect has no built-in timeout, so bound how long a UI control waits on it — +// a stalled backend connect must not leave a disabled spinner stuck forever. The backend +// keeps going regardless. +export const SSH_CONNECT_UI_TIMEOUT_MS = 20_000 + +export async function withUiConnectTimeout(promise: Promise): Promise { + let timer: ReturnType | undefined + const timeout = new Promise((_, reject) => { + timer = setTimeout(() => { + reject( + new Error( + // Key kept from the original NewWorkspaceComposerCard home so existing + // translations survive the move. + translate( + 'auto.components.NewWorkspaceComposerCard.connectTimedOut', + 'Connection timed out. It may still be connecting in the background.' + ) + ) + ) + }, SSH_CONNECT_UI_TIMEOUT_MS) + }) + try { + return await Promise.race([promise, timeout]) + } finally { + if (timer) { + clearTimeout(timer) + } + } +} diff --git a/src/renderer/src/ssh/ssh-connect-verb.ts b/src/renderer/src/ssh/ssh-connect-verb.ts new file mode 100644 index 000000000000..ec0b37ecd864 --- /dev/null +++ b/src/renderer/src/ssh/ssh-connect-verb.ts @@ -0,0 +1,21 @@ +import { translate } from '@/i18n/i18n' +import type { SshConnectionStatus } from '../../../shared/ssh-types' + +// Why: the sidebar card, the terminal overlay, the host-header menu, and the status-bar row +// can all be on screen at once. One vocabulary here stops them describing the same click +// three different ways. +export function sshConnectVerb(status: SshConnectionStatus | null | undefined): string { + switch (status) { + case 'auth-failed': + return translate('auto.ssh.sshConnectVerb.reconnect', 'Reconnect') + case 'error': + case 'reconnection-failed': + return translate('auto.ssh.sshConnectVerb.retry', 'Retry') + default: + return translate('auto.ssh.sshConnectVerb.connect', 'Connect') + } +} + +export function sshConnectingLabel(): string { + return translate('auto.ssh.sshConnectVerb.connecting', 'Connecting…') +} diff --git a/src/renderer/src/ssh/ssh-connection-recoverability.test.ts b/src/renderer/src/ssh/ssh-connection-recoverability.test.ts new file mode 100644 index 000000000000..05aa06eb26d1 --- /dev/null +++ b/src/renderer/src/ssh/ssh-connection-recoverability.test.ts @@ -0,0 +1,72 @@ +import { describe, expect, it } from 'vitest' +import type { SshConnectionStatus } from '../../../shared/ssh-types' +import { canConnectSshStatus, isConnectingSshStatus } from './ssh-connection-recoverability' + +// Union growth is caught by the typechecker (the modules use total Records), not here. +// These cases pin the classification itself, which four call sites now depend on. +const ALL_STATUSES: SshConnectionStatus[] = [ + 'disconnected', + 'connecting', + 'auth-failed', + 'deploying-relay', + 'connected', + 'reconnecting', + 'reconnection-failed', + 'error' +] + +describe('isConnectingSshStatus', () => { + it.each(['connecting', 'deploying-relay', 'reconnecting'] as const)( + 'treats %s as an attempt already under way', + (status) => { + expect(isConnectingSshStatus(status)).toBe(true) + } + ) + + it.each(['disconnected', 'auth-failed', 'connected', 'reconnection-failed', 'error'] as const)( + 'does not treat %s as connecting', + (status) => { + expect(isConnectingSshStatus(status)).toBe(false) + } + ) +}) + +describe('canConnectSshStatus', () => { + it.each(['disconnected', 'auth-failed', 'reconnection-failed', 'error'] as const)( + 'offers a user-driven connect for %s', + (status) => { + expect(canConnectSshStatus(status)).toBe(true) + } + ) + + it.each(['connecting', 'deploying-relay', 'reconnecting', 'connected'] as const)( + 'withholds connect for %s', + (status) => { + expect(canConnectSshStatus(status)).toBe(false) + } + ) +}) + +// Why: runtime-owned targets deliberately yield a null status. Both predicates must read +// that as "nothing to offer" so the card falls through to the passive host glyph. +describe('absent status', () => { + it.each([null, undefined])('classifies %s as neither connecting nor connectable', (status) => { + expect(isConnectingSshStatus(status)).toBe(false) + expect(canConnectSshStatus(status)).toBe(false) + }) +}) + +describe('the two predicates together', () => { + it('never claims a status is both connecting and connectable', () => { + for (const status of ALL_STATUSES) { + expect(isConnectingSshStatus(status) && canConnectSshStatus(status)).toBe(false) + } + }) + + it('leaves only connected outside both sets, so no state renders a dead control', () => { + const unclassified = ALL_STATUSES.filter( + (status) => !isConnectingSshStatus(status) && !canConnectSshStatus(status) + ) + expect(unclassified).toEqual(['connected']) + }) +}) diff --git a/src/renderer/src/ssh/ssh-connection-recoverability.ts b/src/renderer/src/ssh/ssh-connection-recoverability.ts new file mode 100644 index 000000000000..83e87b75216f --- /dev/null +++ b/src/renderer/src/ssh/ssh-connection-recoverability.ts @@ -0,0 +1,36 @@ +import type { SshConnectionStatus } from '../../../shared/ssh-types' + +// Why: a total Record makes a new SshConnectionStatus member a typecheck failure. An +// array + .includes() would silently classify it as "not recoverable", leaving its cards +// with a dead glyph and no way to reconnect. +const CONNECTING_BY_STATUS: Record = { + disconnected: false, + connecting: true, + 'auth-failed': false, + 'deploying-relay': true, + connected: false, + reconnecting: true, + 'reconnection-failed': false, + error: false +} + +const CAN_CONNECT_BY_STATUS: Record = { + disconnected: true, + connecting: false, + 'auth-failed': true, + 'deploying-relay': false, + connected: false, + reconnecting: false, + 'reconnection-failed': true, + error: true +} + +/** Relay deployment and reconnect are host-driven transients: no user action helps yet. */ +export function isConnectingSshStatus(status: SshConnectionStatus | null | undefined): boolean { + return status ? CONNECTING_BY_STATUS[status] : false +} + +/** Failure states a user-initiated connect can recover from. */ +export function canConnectSshStatus(status: SshConnectionStatus | null | undefined): boolean { + return status ? CAN_CONNECT_BY_STATUS[status] : false +} From 3e5c6e9ffef5cfc93f8926a4f1da6886f339b3a4 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Mon, 3 Aug 2026 19:08:00 -0700 Subject: [PATCH 2/6] fix(ssh): address review round 1 on the inline reconnect control - reconnect surfaces get a 180s UI connect fence instead of the composer's 20s: main allows 120s for an interactive passphrase before the 30s connect even starts, so the short cap toasted "timed out" and ran the stale-metadata resync against a host that was about to connect fine - carries the existing es/ja/ko/zh translations onto the shared connect verbs (they were en-only, regressing four locales) and drops the dead SshDisconnectedDialog key namespace - migrates the three remaining copies of the connecting predicate (SshStatusSegment, SshTargetRow, external-automation-source-availability) - SshTargetStatusRow and SshTargetRow now join the shared in-flight registry, so a connect started on one surface disables the others immediately - drops the per-card aria-live region that duplicated the button's own label --- ...external-automation-source-availability.ts | 3 +- .../src/components/sidebar/SshTargetRow.tsx | 35 +++---- .../sidebar/WorktreeCardSshHostControl.tsx | 97 +++++++++---------- .../status-bar/SshStatusSegment.tsx | 7 +- .../status-bar/SshTargetStatusRow.tsx | 16 ++- .../TerminalSshReconnectOverlay.tsx | 7 +- src/renderer/src/i18n/locales/en.json | 13 --- src/renderer/src/i18n/locales/es.json | 21 ++-- src/renderer/src/i18n/locales/ja.json | 21 ++-- src/renderer/src/i18n/locales/ko.json | 21 ++-- src/renderer/src/i18n/locales/zh.json | 21 ++-- .../src/ssh/ssh-connect-ui-timeout.test.ts | 23 ++++- .../src/ssh/ssh-connect-ui-timeout.ts | 13 ++- 13 files changed, 151 insertions(+), 147 deletions(-) diff --git a/src/renderer/src/components/automations/external-automation-source-availability.ts b/src/renderer/src/components/automations/external-automation-source-availability.ts index 92f0749f8924..acb51056e73f 100644 --- a/src/renderer/src/components/automations/external-automation-source-availability.ts +++ b/src/renderer/src/components/automations/external-automation-source-availability.ts @@ -3,6 +3,7 @@ import type { ExternalAutomationProvider } from '../../../../shared/automations-types' import type { SshConnectionStatus } from '../../../../shared/ssh-types' +import { isConnectingSshStatus } from '@/ssh/ssh-connection-recoverability' export type ExternalAutomationSourceAvailability = { statusLabel: string @@ -75,7 +76,7 @@ export function getExternalAutomationSourceAvailability({ } export function isSshConnectionBusy(status: SshConnectionStatus | undefined): boolean { - return status === 'connecting' || status === 'deploying-relay' || status === 'reconnecting' + return isConnectingSshStatus(status) } export function getExternalAutomationActionDisabledMessage(args: { diff --git a/src/renderer/src/components/sidebar/SshTargetRow.tsx b/src/renderer/src/components/sidebar/SshTargetRow.tsx index 3ce6db39e76f..98323550c97f 100644 --- a/src/renderer/src/components/sidebar/SshTargetRow.tsx +++ b/src/renderer/src/components/sidebar/SshTargetRow.tsx @@ -4,10 +4,17 @@ * Why extracted: keeps AddRepoSteps.tsx under the 400-line oxlint limit * while isolating the inline-connect interaction logic. */ -import React, { useCallback, useRef, useState } from 'react' +import React from 'react' import { Loader2 } from 'lucide-react' import type { SshTarget, SshConnectionState } from '../../../../shared/ssh-types' import { translate } from '@/i18n/i18n' +import { isConnectingSshStatus } from '@/ssh/ssh-connection-recoverability' +import { + beginSshConnect, + endSshConnect, + isSshConnectInFlight, + useSshConnectInFlight +} from '@/ssh/ssh-connect-in-flight' type Props = { target: SshTarget & { state?: SshConnectionState } @@ -22,15 +29,12 @@ export function SshTargetRow({ onSelect, onConnect }: Props): React.JSX.Element { - const [connecting, setConnecting] = useState(false) - const mountedRef = useRef(true) + // Why: the shared registry replaces local state — every SSH surface dials one connection + // per target, and it survives this row unmounting mid-connect. + const connecting = useSshConnectInFlight(target.id) const status = target.state?.status ?? 'disconnected' const isConnected = status === 'connected' - const isBusy = - connecting || - status === 'connecting' || - status === 'deploying-relay' || - status === 'reconnecting' + const isBusy = connecting || isConnectingSshStatus(status) const dotColor = isConnected ? 'bg-green-500' : isBusy @@ -47,26 +51,17 @@ export function SshTargetRow({ // Why: prevent the row's onClick from also firing and treating the click // as a selection when the target is disconnected. e.stopPropagation() - if (isBusy) { + if (isBusy || isSshConnectInFlight(target.id)) { return } - setConnecting(true) + beginSshConnect(target.id) void onConnect(target.id).finally(() => { - if (mountedRef.current) { - setConnecting(false) - } + endSshConnect(target.id) }) } - const handleRowRootRef = useCallback((node: HTMLDivElement | null): void => { - // Why: SSH connects can resolve after this row is removed; the row ref - // gives the async completion the same guard without a mount-only Effect. - mountedRef.current = node !== null - }, []) - return (
- - - - - - {tooltip} - - - {/* Why: announce transitions only, not every render, or a busy sidebar spams AT. */} - - {accessibleName} - - + + + + + + {tooltip} + + ) } diff --git a/src/renderer/src/components/status-bar/SshStatusSegment.tsx b/src/renderer/src/components/status-bar/SshStatusSegment.tsx index 42049cb5a255..70e500ad09a3 100644 --- a/src/renderer/src/components/status-bar/SshStatusSegment.tsx +++ b/src/renderer/src/components/status-bar/SshStatusSegment.tsx @@ -21,10 +21,7 @@ import { RuntimeHostStatusRow, type RuntimeHostConnectionState } from './Runtime import { SshTargetStatusRow } from './SshTargetStatusRow' import type { RemoteRuntimeSharedConnectionDiagnostics } from '../../../../shared/remote-runtime-shared-control-types' import { connectRuntimeEnvironmentAndRecordStatus } from './runtime-environment-explicit-connect' - -function isConnecting(status: SshConnectionStatus): boolean { - return ['connecting', 'deploying-relay', 'reconnecting'].includes(status) -} +import { isConnectingSshStatus } from '@/ssh/ssh-connection-recoverability' type HostStatus = 'connected' | 'disconnected' | 'connecting' @@ -70,7 +67,7 @@ function sshStatusForOverall(status: SshConnectionStatus): HostStatus { if (status === 'connected') { return 'connected' } - return isConnecting(status) ? 'connecting' : 'disconnected' + return isConnectingSshStatus(status) ? 'connecting' : 'disconnected' } function runtimeHostConnectionState({ diff --git a/src/renderer/src/components/status-bar/SshTargetStatusRow.tsx b/src/renderer/src/components/status-bar/SshTargetStatusRow.tsx index 712d4db9e153..c8fa9100b901 100644 --- a/src/renderer/src/components/status-bar/SshTargetStatusRow.tsx +++ b/src/renderer/src/components/status-bar/SshTargetStatusRow.tsx @@ -7,6 +7,12 @@ import { useAppStore } from '../../store' import { STATUS_LABELS, statusColor } from '../settings/SshTargetCard' import { canConnectSshStatus } from '@/ssh/ssh-connection-recoverability' import { sshConnectVerb } from '@/ssh/ssh-connect-verb' +import { + beginSshConnect, + endSshConnect, + isSshConnectInFlight, + useSshConnectInFlight +} from '@/ssh/ssh-connect-in-flight' import type { SshConnectionStatus } from '../../../../shared/ssh-types' import type { RemoteWorkspaceSyncStatus } from '../../store/slices/ssh' @@ -60,9 +66,16 @@ export function SshTargetStatusRow({ const [busy, setBusy] = useState(false) const mountedRef = useMountedRef() const recordFeatureInteraction = useAppStore((s) => s.recordFeatureInteraction) + // Why: shared with the sidebar card control and terminal overlay — a connect started here + // must disable those too, in the window before main broadcasts 'connecting'. + const connectInFlight = useSshConnectInFlight(targetId) const visibleSyncStatusLabel = syncStatusLabel(syncStatus) const handleConnect = useCallback(async () => { + if (isSshConnectInFlight(targetId)) { + return + } + beginSshConnect(targetId) setBusy(true) try { await window.api.ssh.connect({ targetId }) @@ -74,6 +87,7 @@ export function SshTargetStatusRow({ : translate('auto.components.status.bar.SshStatusSegment.2c29e2de68', 'Connection failed') ) } finally { + endSshConnect(targetId) if (mountedRef.current) { setBusy(false) } @@ -128,7 +142,7 @@ export function SshTargetStatusRow({ ) : null}
- {busy ? ( + {busy || connectInFlight ? ( ) : canConnectSshStatus(status) ? ( diff --git a/src/renderer/src/ssh/ssh-connect-verb.test.ts b/src/renderer/src/ssh/ssh-connect-verb.test.ts new file mode 100644 index 000000000000..b3fcdcdcdc33 --- /dev/null +++ b/src/renderer/src/ssh/ssh-connect-verb.test.ts @@ -0,0 +1,35 @@ +import { describe, expect, it, vi } from 'vitest' +import { sshConnectingLabel, sshConnectVerb } from './ssh-connect-verb' + +vi.mock('@/i18n/i18n', () => ({ + translate: (_key: string, fallback: string) => fallback +})) + +// Why: the sidebar card control, terminal overlay, host-header menu, and status-bar row all +// read this table. It is the only thing keeping them from naming one click three ways. +describe('sshConnectVerb', () => { + it('names an authentication failure a reconnect', () => { + expect(sshConnectVerb('auth-failed')).toBe('Reconnect') + }) + + it.each(['error', 'reconnection-failed'] as const)('names %s a retry', (status) => { + expect(sshConnectVerb(status)).toBe('Retry') + }) + + it.each(['disconnected', 'connecting', 'deploying-relay', 'reconnecting', 'connected'] as const)( + 'falls back to Connect for %s', + (status) => { + expect(sshConnectVerb(status)).toBe('Connect') + } + ) + + it.each([null, undefined])('falls back to Connect for %s', (status) => { + expect(sshConnectVerb(status)).toBe('Connect') + }) +}) + +describe('sshConnectingLabel', () => { + it('uses the single-character ellipsis, matching the rest of the catalog', () => { + expect(sshConnectingLabel()).toBe('Connecting…') + }) +}) From 9d5c47137fe8746410aee3df3a016ea51708d427 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Mon, 3 Aug 2026 19:49:56 -0700 Subject: [PATCH 4/6] temp checkin of files --- .../claude-opus-ssh-reconnect-judgment.md | 180 ++++++++++ .../ssh-reconnect-sidebar-control-design.md | 329 ++++++++++++++++++ 2 files changed, 509 insertions(+) create mode 100644 artifacts/claude-opus-ssh-reconnect-judgment.md create mode 100644 artifacts/ssh-reconnect-sidebar-control-design.md diff --git a/artifacts/claude-opus-ssh-reconnect-judgment.md b/artifacts/claude-opus-ssh-reconnect-judgment.md new file mode 100644 index 000000000000..0890d7621626 --- /dev/null +++ b/artifacts/claude-opus-ssh-reconnect-judgment.md @@ -0,0 +1,180 @@ +# Judgment — Sidebar SSH reconnect affordance (Grok vs Codex) + +**Judge:** Claude Opus 5 (independent UI design judge) +**Date:** 2026-08-03 +**Artifacts reviewed:** both standalone HTMLs (read in full) and both PNG renders (viewed). +**Grounding:** `docs/STYLEGUIDE.md`, `src/renderer/src/assets/main.css`, `src/renderer/src/components/sidebar/WorktreeCard.tsx`, `src/renderer/src/components/sidebar/SshDisconnectedDialog.tsx`, `src/renderer/src/components/terminal-pane/TerminalSshReconnectOverlay.tsx`, `src/renderer/src/components/sidebar/WorktreeCardMetadataControls.tsx`, `src/renderer/src/components/ui/button.tsx`, `src/shared/ssh-types.ts`. +**No production code was modified.** + +> Note on the renders: both PNGs are single-viewport captures that cut off at ~1000px, so the state-matrix / mapping / a11y sections below the fold are visible only in the HTML. Judgment below is based on the HTML for content and on the PNGs for actual rendered visual weight — which is where the most decisive difference showed up. + +--- + +## 1. Verified ground truth (what the code actually does today) + +| Fact | Evidence | +| --- | --- | +| The sidebar cue is a passive, non-focusable `` wrapping `ServerOff className="size-3 text-red-400"` with tooltip "SSH disconnected". | `WorktreeCard.tsx:1430–1452` | +| A *second*, near-identical block exists for runtime ("Orca server") hosts, also `ServerOff text-red-400`, also passive. | `WorktreeCard.tsx:1455–1490` | +| Card click opens the blocking dialog **only** when the active view is not a terminal. | `WorktreeCard.tsx:880–883`, comment at `:365` and `:393` | +| Whole card gets `opacity-60` when SSH **or** runtime is disconnected. | `WorktreeCard.tsx:1892` | +| `sshStatus` is `null` for runtime-owned SSH targets (deliberate — suppresses false "disconnected"). | `WorktreeCard.tsx:352–358` | +| `sshOwnerEnvironmentId` is already computed in the card, so runtime-routed connect is reachable from the sidebar today. | `WorktreeCard.tsx:349–351` | +| `SshConnectionStatus` has **8** members: `disconnected, connecting, auth-failed, deploying-relay, connected, reconnecting, reconnection-failed, error`. | `src/shared/ssh-types.ts:102–110` | +| Reconnectable set is `['disconnected','reconnection-failed','error','auth-failed']`, duplicated verbatim in the dialog and the overlay. | `SshDisconnectedDialog.tsx:59–61` vs `TerminalSshReconnectOverlay.tsx:35–37` | +| The overlay's connect is **not** a bare `ssh.connect`: it branches on `sshOwnerEnvironmentId`, mirrors `setSshConnectionState` (because `ssh.connect` can resolve before the state IPC lands), and on failure resyncs target metadata + removed labels (STA-1468). | `TerminalSshReconnectOverlay.tsx:91–137` | +| A removed target must never be offered Connect; `targetRemoved` is a real prop and `removedSshTargetLabels` lives in the renderer store, so the sidebar can derive it. | `TerminalSshReconnectOverlay.tsx:19–21, 88–89`; `store/slices/ssh.ts:39`; `store/slices/runtime-environment-ssh.ts:300, 321` | +| There is an existing test contract that the dialog must not auto-open for a restored active disconnected worktree. | `WorktreeCard.ssh-reconnect-prompt.test.tsx:147` | +| `WorktreeCard.tsx` is the **only** non-test consumer of `SshDisconnectedDialog`. | grep across `src` | +| `Button` has both `xs` (h-6, 12px svg) and `icon-xs` (size-6) — both proposals' primitive claims are valid. | `ui/button.tsx:23, 27` | +| **Precedent A — icon-only ghost action in the card:** `MetadataActionIcon` = `Button variant="ghost" size="icon-xs" className="size-6"` + `stopPropagation` + Tooltip. | `WorktreeCardMetadataControls.tsx:38–60` | +| **Precedent B — compact *labeled* destructive action in the title row:** the "rename failed" control is a `Button variant="ghost"` at `h-4`, `text-[10px]`, `text-destructive border-destructive/40 bg-destructive/10`, icon `size-2.5`, **plus** a tooltip even though it has a visible label. | `WorktreeCard.tsx:1523–1551` | +| Sidebar surface tokens in this pane are the `worktree-sidebar` family (`--worktree-sidebar: #2a2a2a` in dark), not the generic `--sidebar` (#171717). | `main.css:287–288`; `WorktreeList.tsx:4086, 4227, 4272` | + +Precedent B matters a lot: it means a short *labeled* pill in the title row is house style, not an invention — and that a tooltip alongside a visible label is already accepted here, so the STYLEGUIDE line "don't tooltip a control that has a visible label" is not a live objection against Codex. + +--- + +## 2. Scorecard + +Scale 1–5. Weighting is mine, stated so it can be argued with. + +| Criterion | Weight | Grok | Codex | +| --- | --- | --- | --- | +| Clarity of the affordance | 1.0 | 3 | **5** | +| Discoverability (the actual brief) | 1.5 | 2 | **5** | +| Consistency with Orca style / components / tokens | 1.25 | **5** | 3 | +| Accessibility | 1.25 | 3 | **4** | +| State handling: connected / connecting / error / retry / removed | 1.5 | **5** | 2 | +| Minimal UI disruption | 1.25 | **5** | 2 | +| Implementation fit (does the wiring survive contact with the code) | 1.25 | 3 | **5** | +| Correctness of stated assumptions about the codebase | 1.0 | **5** | 3 | +| **Weighted total (max 50)** | | **38.75** | **35.5** | + +--- + +## 3. Grok — assessment + +### What it gets right + +1. **Complete state model.** All 8 `SshConnectionStatus` members are enumerated and mapped, including `deploying-relay` and `error`, which Codex silently drops. Its `isConnectingStatus` / `canConnectStatus` predicates are copied correctly from the real source. +2. **It is the only proposal that handles `targetRemoved`.** The ghost state ("SSH host removed — reconnect unavailable", no Connect, overlay offers Remove workspace) is a faithful port of `TerminalSshReconnectOverlay.tsx:88–89` and of the STA-1468 lesson: never render a Connect that can only fail. This is the single largest correctness gap between the two. +3. **Token fidelity.** It mirrors the `worktree-sidebar` family (`#2a2a2a` fill, `#353535` accent, `worktree-sidebar-ring`), keeps `red-400` / `yellow-500` exactly as the existing dialog and card use them, invents no tokens, and keeps the existing `opacity-60`. The render visibly *is* the Orca sidebar. +4. **Accurate, checkable citations.** `~1430–1452`, `~880–883`, the `WorktreeCardMetadataControls` pattern, the focus-steal rationale on `showDisconnectedDialog` — every one verified true. It also correctly identifies that the reconnectable-status predicate is duplicated between dialog and overlay and proposes extracting it. +5. **Respects the runtime-host boundary.** It explicitly says: do not mint a chip when `sshStatus` is `null` (runtime-owned targets), and treat the runtime-host `ServerOff` block at `:1455+` as a follow-up gated on a runtime reconnect API existing. Codex never mentions that second block at all, even though it renders the identical red icon. +6. **Additive, not subtractive.** Chip is a new parallel path; the tested dialog behavior and the overlay stay as-is. Lowest-risk landing. + +### Where it is wrong or weak + +1. **It under-delivers the brief.** The whole premise is "the red icon looks like a status badge, not a control" — and the fix is… the same 12px red icon, in the same place, at the same weight. Compare the two PNGs: Grok's after-state is visually indistinguishable from its own before-state except for a tooltip. Hover-to-discover does not solve discoverability for a user who never suspects the glyph is clickable. Grok's own non-goal ("no permanent Reconnect text button — too heavy for a dense sidebar") is asserted, not argued, and Precedent B (`rename failed`) refutes it: the card already carries a labeled 10px destructive pill in that exact row. +2. **Real a11y defect: `disabled` for the connected and removed states.** Grok renders the chip as a `