From 899d2aad968570393e07b7bb4db7947de42a724a Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 26 Oct 2026 06:51:39 -0700 Subject: [PATCH] fix(ssh): say why SSH failed when Connect never reaches the host (#33928) When the SSH session itself could not open (key rejected, port refused, host not answering, host key not trusted), Connect on a host Orca had not set up yet said "Orca couldn't start its server on this host" and offered a plain SSH terminal that could not work either. Errors from opening the SSH session are now marked where they are dialed, and a setup that fails on one ends the connect with SSH's own reason. A held move keeps its status. The connect-failure toast also drops Electron's IPC prefix. --- config/scripts/pr-e2e-source-routing.mjs | 10 ++ .../connection/ssh-connection-manager.test.ts | 9 ++ .../ssh/connection/ssh-connection-manager.ts | 12 +++ ...host-server-on-connect-ssh-failure.test.ts | 60 ++++++++++++ src/main/ssh/ssh-host-server-on-connect.ts | 64 +------------ src/main/ssh/ssh-host-server-setup-outcome.ts | 92 +++++++++++++++++++ src/main/ssh/ssh-session-start-failure.ts | 20 ++++ .../src/ssh/ssh-host-server-status-copy.ts | 3 +- .../e2e/ssh-orcad-ssh-failure-reason.spec.ts | 79 ++++++++++++++++ 9 files changed, 285 insertions(+), 64 deletions(-) create mode 100644 src/main/ssh/ssh-host-server-on-connect-ssh-failure.test.ts create mode 100644 src/main/ssh/ssh-host-server-setup-outcome.ts create mode 100644 src/main/ssh/ssh-session-start-failure.ts create mode 100644 tests/e2e/ssh-orcad-ssh-failure-reason.spec.ts diff --git a/config/scripts/pr-e2e-source-routing.mjs b/config/scripts/pr-e2e-source-routing.mjs index a1e3edb4d11..9d97d3c61eb 100644 --- a/config/scripts/pr-e2e-source-routing.mjs +++ b/config/scripts/pr-e2e-source-routing.mjs @@ -144,6 +144,16 @@ export const PR_E2E_SOURCE_ROUTES = [ file )) }, + { + id: 'ssh.orcad-ssh-failure-reason', + specs: ['tests/e2e/ssh-orcad-ssh-failure-reason.spec.ts'], + matches: (file) => + MANAGED_SSH_HARNESS.test(file) || + (isProductSource(file) && + /^src\/(?:main\/(?:ipc\/ssh\/(?:ssh-connect-flow|ssh-host-server-)|ssh\/(?:ssh-session-start-failure|ssh-host-server-|connection\/ssh-connection-manager))|renderer\/src\/ssh\/(?:ssh-host-server-status-copy|ssh-connect-failure-toast))/.test( + file + )) + }, { id: 'ssh.orcad-update-anyway', specs: ['tests/e2e/ssh-orcad-update-anyway.spec.ts'], diff --git a/src/main/ssh/connection/ssh-connection-manager.test.ts b/src/main/ssh/connection/ssh-connection-manager.test.ts index 05285e496b2..b079db8fc05 100644 --- a/src/main/ssh/connection/ssh-connection-manager.test.ts +++ b/src/main/ssh/connection/ssh-connection-manager.test.ts @@ -55,6 +55,7 @@ import { resetSshCredentialDeclinesForTests } from '../ssh-credential-decline' import { isAuthError } from '../ssh-reconnect-error-classification' +import { isSshSessionStartFailure } from '../ssh-session-start-failure' import { recordSshUserDisconnected, resetSshUserDisconnectsForTests } from '../ssh-user-disconnect' const target = { @@ -91,6 +92,14 @@ describe('SshConnectionManager', () => { expect(manager.getConnection(target.id)).toBeUndefined() }) + it('marks a dial that never reached the host, so setup reports SSH’s reason', async () => { + const refused = new Error('connect ECONNREFUSED 10.0.0.1:22') + mockState.connectResults.push(Promise.reject(refused)) + const manager = new SshConnectionManager({ onStateChange: vi.fn() }) + await expect(manager.connect(target)).rejects.toBe(refused) + expect(isSshSessionStartFailure(new Error('Deploy failed', { cause: refused }))).toBe(true) + }) + it('removes the registration when bulk teardown fails', async () => { const manager = new SshConnectionManager({ onStateChange: vi.fn() }) await manager.connect(target) diff --git a/src/main/ssh/connection/ssh-connection-manager.ts b/src/main/ssh/connection/ssh-connection-manager.ts index d2168d801ad..6cbc72b2612 100644 --- a/src/main/ssh/connection/ssh-connection-manager.ts +++ b/src/main/ssh/connection/ssh-connection-manager.ts @@ -8,6 +8,7 @@ import { } from '../ssh-credential-decline' import { createSshUserDisconnectedError, isSshUserDisconnected } from '../ssh-user-disconnect' import { SSH_CONNECT_IN_PROGRESS_SUFFIX } from '../../../shared/ssh-sign-in-required-copy' +import { markSshSessionStartFailure } from '../ssh-session-start-failure' /** A remembered sign-in only applies to the endpoint it was given for; an edited host must ask. */ function signInEndpoint(target: SshTarget): string { @@ -39,6 +40,17 @@ export class SshConnectionManager { /** `probe`: a user's connection test, which a Disconnect does not refuse. */ async connect(target: SshTarget, options?: { probe?: boolean }): Promise { + try { + return await this.openSession(target, options) + } catch (error) { + throw markSshSessionStartFailure(error) + } + } + + private async openSession( + target: SshTarget, + options?: { probe?: boolean } + ): Promise { // Refuse user-disconnected hosts before pool reuse, including transports opened by connection tests (#30086). if (!options?.probe && isSshUserDisconnected(target.id)) { throw createSshUserDisconnectedError(target.label) diff --git a/src/main/ssh/ssh-host-server-on-connect-ssh-failure.test.ts b/src/main/ssh/ssh-host-server-on-connect-ssh-failure.test.ts new file mode 100644 index 00000000000..db615c9c3e9 --- /dev/null +++ b/src/main/ssh/ssh-host-server-on-connect-ssh-failure.test.ts @@ -0,0 +1,60 @@ +/** A setup whose SSH dial never reached the host fails with SSH's reason, never as a server failure. */ +import { describe, expect, it, vi } from 'vitest' +import type { SshTarget } from '../../shared/ssh-types' +import { resolveHostServerOnConnect } from './ssh-host-server-on-connect' +import { hostServerDepsStub } from './ssh-host-server-on-connect-test-deps' +import { markSshSessionStartFailure } from './ssh-session-start-failure' + +const target: SshTarget = { id: 'ssh-1', label: 'Box', host: 'box', port: 22, username: 'me' } + +function quietly(run: () => Promise): Promise { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => undefined) + return run().finally(() => warn.mockRestore()) +} + +describe('a connect whose SSH session never opens', () => { + it('fails a fresh host with the SSH error and releases its deploy claim', async () => { + const rejected = markSshSessionStartFailure( + new Error('All configured authentication methods failed') + ) + const deps = hostServerDepsStub({ + isEmptyHost: () => true, + deploy: async () => { + throw new Error('Deploy failed', { cause: rejected }) + } + }) + await expect(quietly(() => resolveHostServerOnConnect(target, deps))).rejects.toThrow( + 'Deploy failed' + ) + expect(deps.abandonDeploy).toHaveBeenCalledWith(target) + expect(deps.recordUnavailable).not.toHaveBeenCalled() + }) + + it('fails a host with saved projects the same way when its move cannot dial', async () => { + const refused = markSshSessionStartFailure(new Error('connect ECONNREFUSED 10.0.0.1:22')) + const deps = hostServerDepsStub({ + convert: async () => { + throw refused + } + }) + await expect(quietly(() => resolveHostServerOnConnect(target, deps))).rejects.toBe(refused) + expect(deps.abandonConversion).toHaveBeenCalledWith(target) + }) + + it('keeps a held move’s status, which offers the way to finish it', async () => { + const timedOut = markSshSessionStartFailure( + new Error('Timed out while waiting for SSH authentication') + ) + const deps = hostServerDepsStub({ + hasUnfinishedConversion: () => true, + convert: async () => { + throw timedOut + } + }) + await expect(quietly(() => resolveHostServerOnConnect(target, deps))).resolves.toMatchObject({ + route: 'unserved', + reason: 'failed', + held: true + }) + }) +}) diff --git a/src/main/ssh/ssh-host-server-on-connect.ts b/src/main/ssh/ssh-host-server-on-connect.ts index 7941afcf503..79ec0fe4dde 100644 --- a/src/main/ssh/ssh-host-server-on-connect.ts +++ b/src/main/ssh/ssh-host-server-on-connect.ts @@ -17,9 +17,8 @@ import { type ManagedServerUpdateDeps } from './managed-server-update-check' import { managedResultAfterUpdate, unservedManagedResult } from './managed-server-serving-gate' -import { orcadMigrationConflictGuidance } from './orcad-migration-conflict-guidance' import { keptForBridgedTerminals, relayTerminalReason } from './ssh-host-server-bridged-unserved' -import { boundedDetail, setupFailure, type SetupFailure } from './ssh-host-server-setup-failure' +import { deferred, setupFailed } from './ssh-host-server-setup-outcome' import { classifyOrcadHostUnavailable, ORCAD_TUNNEL_UNAVAILABLE_REASON @@ -29,7 +28,6 @@ import { reportHostServerConversion, reportHostServerDeploy, reportHostServerEvent, - reportHostServerSetupError, startHostServerDecisionTrace, type HostServerDecisionTrace, type HostServerReport @@ -225,23 +223,6 @@ async function convertHost( } } -function setupFailed( - target: SshTarget, - deps: HostServerOnConnectDeps, - trace: HostServerDecisionTrace, - error: unknown, - empty: boolean -): Promise { - console.warn('[ssh] Managed Orca server setup failed:', error) - reportHostServerSetupError(deps.report, target, trace, error) - const unavailable = classifyOrcadHostUnavailable(error) - const conflict = empty ? null : orcadMigrationConflictGuidance(error) - if (conflict) { - return unfinished(target, deps, unavailable, empty, 'refused', { detail: conflict }) - } - return unfinished(target, deps, unavailable, empty, 'failed', setupFailure(error)) -} - async function afterDeploy( target: SshTarget, deps: HostServerOnConnectDeps, @@ -290,46 +271,3 @@ async function afterConversion( } } } - -function deferred( - target: SshTarget, - deps: HostServerOnConnectDeps, - result: { code: string; reason: string }, - empty: boolean -): Promise { - console.warn(`[ssh] Managed Orca server setup deferred (${result.code}):`, result.reason) - const unavailable = classifyOrcadHostUnavailable({ code: result.code }) - return unfinished(target, deps, unavailable, empty, 'deferred', { detail: result.reason }) -} - -/** Release failed setup claims/fences only with absence proof; record definitive host incompatibility separately. */ -async function unfinished( - target: SshTarget, - deps: HostServerOnConnectDeps, - unavailable: string | null, - empty: boolean, - reason: 'deferred' | 'failed' | 'refused', - failure: SetupFailure -): Promise { - try { - await (empty ? deps.abandonDeploy(target) : deps.abandonConversion(target)) - } catch (error) { - // Retain fences when staging/registration leaves destination state unproven. - console.warn('[ssh] The managed Orca server setup keeps its fence:', error) - } - // Re-read the journal: abort may have released the fence or discovered commit. - const held = !empty && !unavailable && deps.hasUnfinishedConversion(target) - if (!unavailable) { - // Publish host-specific setup detail, including bounded orcad.log tail when available. - return { - route: 'unserved', - reason, - detail: boundedDetail(failure.detail), - ...(failure.contactLost ? { contactLost: true as const } : {}), - ...(failure.nodeDownloadBlocked ? { nodeDownloadBlocked: failure.nodeDownloadBlocked } : {}), - ...(held ? { held: true as const } : {}) - } - } - deps.recordUnavailable(target, unavailable) - return { route: 'unserved', reason: 'orcad_unavailable', detail: unavailable } -} diff --git a/src/main/ssh/ssh-host-server-setup-outcome.ts b/src/main/ssh/ssh-host-server-setup-outcome.ts new file mode 100644 index 00000000000..d2443aa54d0 --- /dev/null +++ b/src/main/ssh/ssh-host-server-setup-outcome.ts @@ -0,0 +1,92 @@ +/** How a managed-server setup that did not finish ends: with SSH's own reason, or as an unserved status. */ +import type { SshTarget } from '../../shared/ssh-types' +import type { + HostServerOnConnectDeps, + HostServerOnConnectResult +} from './ssh-host-server-on-connect' +import { orcadMigrationConflictGuidance } from './orcad-migration-conflict-guidance' +import { boundedDetail, setupFailure, type SetupFailure } from './ssh-host-server-setup-failure' +import { isSshSessionStartFailure } from './ssh-session-start-failure' +import { classifyOrcadHostUnavailable } from '../orcad/orcad-host-unavailable' +import { + reportHostServerSetupError, + type HostServerDecisionTrace +} from './ssh-host-server-connect-events' + +export function setupFailed( + target: SshTarget, + deps: HostServerOnConnectDeps, + trace: HostServerDecisionTrace, + error: unknown, + empty: boolean +): Promise { + console.warn('[ssh] Managed Orca server setup failed:', error) + reportHostServerSetupError(deps.report, target, trace, error) + if (isSshSessionStartFailure(error)) { + return sshSessionStartFailed(target, deps, empty, error) + } + const unavailable = classifyOrcadHostUnavailable(error) + const conflict = empty ? null : orcadMigrationConflictGuidance(error) + if (conflict) { + return unfinished(target, deps, unavailable, empty, 'refused', { detail: conflict }) + } + return unfinished(target, deps, unavailable, empty, 'failed', setupFailure(error)) +} + +/** SSH never connected, so nothing was learned about the server: fail the connect with SSH's reason. */ +async function sshSessionStartFailed( + target: SshTarget, + deps: HostServerOnConnectDeps, + empty: boolean, + error: unknown +): Promise { + const result = await unfinished(target, deps, null, empty, 'failed', setupFailure(error)) + // A held move keeps its own status, which offers the way to finish or undo it. + if (result.route === 'unserved' && result.held) { + return result + } + throw error +} + +export function deferred( + target: SshTarget, + deps: HostServerOnConnectDeps, + result: { code: string; reason: string }, + empty: boolean +): Promise { + console.warn(`[ssh] Managed Orca server setup deferred (${result.code}):`, result.reason) + const unavailable = classifyOrcadHostUnavailable({ code: result.code }) + return unfinished(target, deps, unavailable, empty, 'deferred', { detail: result.reason }) +} + +/** Release failed setup claims/fences only with absence proof; record definitive host incompatibility separately. */ +async function unfinished( + target: SshTarget, + deps: HostServerOnConnectDeps, + unavailable: string | null, + empty: boolean, + reason: 'deferred' | 'failed' | 'refused', + failure: SetupFailure +): Promise { + try { + await (empty ? deps.abandonDeploy(target) : deps.abandonConversion(target)) + } catch (error) { + // Retain fences when staging/registration leaves destination state unproven. + console.warn('[ssh] The managed Orca server setup keeps its fence:', error) + } + // Re-read the journal: abort may have released the fence or discovered commit. + const held = !empty && !unavailable && deps.hasUnfinishedConversion(target) + if (!unavailable) { + // Publish host-specific setup detail, including bounded orcad.log tail when available. + return { + route: 'unserved', + reason, + detail: boundedDetail(failure.detail), + ...(failure.contactLost ? { contactLost: true as const } : {}), + ...(failure.nodeDownloadBlocked ? { nodeDownloadBlocked: failure.nodeDownloadBlocked } : {}), + ...(held ? { held: true as const } : {}) + } + } + deps.recordUnavailable(target, unavailable) + return { route: 'unserved', reason: 'orcad_unavailable', detail: unavailable } +} diff --git a/src/main/ssh/ssh-session-start-failure.ts b/src/main/ssh/ssh-session-start-failure.ts new file mode 100644 index 00000000000..efd7ae5269e --- /dev/null +++ b/src/main/ssh/ssh-session-start-failure.ts @@ -0,0 +1,20 @@ +// Errors from opening the SSH session itself, so a setup that never reached the host is not called a server failure. +const sessionStartFailures = new WeakSet() + +export function markSshSessionStartFailure(error: T): T { + if (typeof error === 'object' && error !== null) { + sessionStartFailures.add(error) + } + return error +} + +/** Follows wrapping causes, as setup steps rethrow the dial's error inside their own. */ +export function isSshSessionStartFailure(error: unknown, depth = 0): boolean { + if (typeof error !== 'object' || error === null) { + return false + } + if (sessionStartFailures.has(error)) { + return true + } + return depth < 4 && error instanceof Error && isSshSessionStartFailure(error.cause, depth + 1) +} diff --git a/src/renderer/src/ssh/ssh-host-server-status-copy.ts b/src/renderer/src/ssh/ssh-host-server-status-copy.ts index be6e549dd1e..7d9f88c86ba 100644 --- a/src/renderer/src/ssh/ssh-host-server-status-copy.ts +++ b/src/renderer/src/ssh/ssh-host-server-status-copy.ts @@ -6,6 +6,7 @@ import type { } from '../../../shared/ssh-types' import { translate } from '@/i18n/i18n' import { useAppStore } from '@/store' +import { extractIpcErrorMessage } from '@/lib/ipc-error' import { launchSessionEndedLine } from './ssh-host-launch-session-copy' import { bridgedTerminalsLine } from './ssh-host-bridged-terminals-copy' import { managedUpdateLine, withDetail } from './ssh-host-server-update-copy' @@ -92,7 +93,7 @@ export function sshTargetConnectFailureText( ): string { return sshConnectFailureText( useAppStore.getState().sshConnectionStates.get(targetId), - error instanceof Error ? error.message : fallback + extractIpcErrorMessage(error, fallback) ) } diff --git a/tests/e2e/ssh-orcad-ssh-failure-reason.spec.ts b/tests/e2e/ssh-orcad-ssh-failure-reason.spec.ts new file mode 100644 index 00000000000..c05c89dbe44 --- /dev/null +++ b/tests/e2e/ssh-orcad-ssh-failure-reason.spec.ts @@ -0,0 +1,79 @@ +// A Connect whose SSH session never opens names SSH's reason, not a server failure with a plain-SSH fallback. +// ORCA_E2E_ORCAD_CONVERT_HOST=docker; ORCA_E2E_ORCAD_CONVERT_TEMPLATE must be built for the Docker host. +import type { ElectronApplication } from '@stablyai/playwright-test' +import { expect, test } from './helpers/orca-app' +import { waitForSessionReady } from './helpers/store' +import { createRestartSession } from './helpers/orca-restart' +import { openSshHostSettings } from './helpers/ssh-config-host-picker' +import { + ORCAD_CONVERT_HOST_ENV, + startOrcadConvertHost, + type OrcadConvertHost +} from './helpers/orcad-convert-host' + +const HOST = process.env[ORCAD_CONVERT_HOST_ENV] +const TEMPLATE_SOURCE = process.env.ORCA_E2E_ORCAD_CONVERT_TEMPLATE + +test.skip( + HOST !== 'docker' || !TEMPLATE_SOURCE, + `Set ${ORCAD_CONVERT_HOST_ENV}=docker and ORCA_E2E_ORCAD_CONVERT_TEMPLATE` +) + +let host: OrcadConvertHost | null = null +test.afterEach(() => { + host?.cleanup() + host = null +}) + +const CASES = [ + { name: 'a refused SSH port', reason: /ECONNREFUSED/, refused: true }, + { name: 'a key the host rejects', reason: /authentication methods failed/i, refused: false } +] as const + +for (const fault of CASES) { + // oxlint-disable-next-line no-empty-pattern -- Playwright's first fixture arg must be an object pattern; this test owns its app launch. + test(`Connect to ${fault.name} says why SSH failed`, async ({}, testInfo) => { + // Starts a Docker host and an app; a rejected key also waits on its password prompt. + test.setTimeout(6 * 60_000) + host = startOrcadConvertHost('docker', testInfo) + if (!fault.refused) { + host.exec!(': > /root/.ssh/authorized_keys') + } + const input = { ...host.input, ...(fault.refused ? { port: 1 } : {}) } + // No agent, so the only offered key is the one the host rejects. + const session = createRestartSession(testInfo, { + ORCA_ORCAD_TEMPLATE_PATH: TEMPLATE_SOURCE!, + SSH_AUTH_SOCK: '' + }) + let app: ElectronApplication | null = null + try { + const launched = await session.launch() + app = launched.app + const page = launched.page + await waitForSessionReady(page) + await page.evaluate((target) => window.api.ssh.addTarget({ target }), input) + const section = await openSshHostSettings(page) + const card = section.locator(`[data-ssh-target-card][data-ssh-target-label="${input.label}"]`) + await card.hover() + await card.getByRole('button', { name: 'Connect', exact: true }).click() + if (!fault.refused) { + // The rejected key falls back to a password prompt; the user dismisses it. + const prompt = page.getByRole('dialog', { name: 'SSH Password' }) + await expect(prompt).toBeVisible({ timeout: 60_000 }) + await prompt.getByRole('button', { name: 'Cancel' }).click() + } + await expect(card).toContainText(fault.reason, { timeout: 2 * 60_000 }) + await page.screenshot({ path: testInfo.outputPath('card.png') }) + await expect(card).not.toContainText('couldn’t start its server') + await expect(card.getByRole('button', { name: 'Open plain SSH terminal' })).toHaveCount(0) + const toasts = page.locator('[data-sonner-toast]') + await expect(toasts.filter({ hasText: 'Error invoking remote method' })).toHaveCount(0) + await expect(toasts.filter({ hasText: 'couldn’t start its server' })).toHaveCount(0) + } finally { + if (app) { + await session.close(app) + } + await session.dispose() + } + }) +}