From 0a565d5d4d570d22b371862b84079f104302feec Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 7 Sep 2026 16:34:22 -0700 Subject: [PATCH 1/4] fix(ephemeral-vm): re-attach runtime-owned SSH relays after an app restart MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A runtime-owned SSH target (environment-recipe workspace) was only ever connected at provision and resume. After an app restart the record is still persisted as `running`, so `ephemeralVm:resumeWorkspace` returned it unchanged, nothing dialed the target, and every `pty:spawn` on the workspace failed with `No PTY provider for connection "runtime-ssh-…"`. Every generic SSH connect path deliberately skips runtime-owned ids (renderer startup restore, the pane connect gate, and `listTargets`), so the user had no Reconnect control either. The runtime layer now owns the re-attach it was assumed to own: - startup pass: once SSH handlers are registered, main re-attaches the relay of every runtime persisted as running (`suspend_failed` included — the VM is still up) whose relay this process does not hold; failures are logged, not fatal, and never rewrite the record. - activation: the resume handler re-attaches a running SSH runtime instead of returning early. A failed re-attach throws to the caller and leaves the record `running` — loss of the relay is not evidence the VM is gone. - spawn: a `pty:spawn` (renderer, runtime controller, or stable-pane adoption) that would miss the provider for a running runtime-owned target first re-attaches the relay, then resolves the provider. A single in-flight connect per target is shared between all three paths. - readiness: `connectRuntimeOwnedSshTarget` now waits for the PTY provider as well as the git and filesystem providers before reporting ready. - renderer: a provider miss on a runtime-owned target is no longer swallowed; it means the re-attach itself failed, and the message names the retry (open the workspace again or start a new terminal). Fixes #19173 --- .../ephemeral-vm-runtime-ssh-reattach.test.ts | 220 ++++++++++++++++++ src/main/ephemeral-vm-runtime-ssh-reattach.ts | 97 ++++++++ src/main/ephemeral-vm-runtime-ssh.test.ts | 162 +++++++++++++ src/main/ephemeral-vm-runtime-ssh.ts | 84 ++++++- ...hemeral-vm-runtime-handler-cleanup.test.ts | 101 +++++++- src/main/ipc/ephemeral-vm-runtime-handlers.ts | 13 +- src/main/ipc/ephemeral-vm.test.ts | 12 +- .../ipc/pty-startup-barrier-ordering.test.ts | 19 ++ src/main/ipc/pty/ipc/spawn-preflight.ts | 7 + src/main/ipc/pty/pane/adopt-stable.ts | 9 + .../missing-ssh-pty-provider-recovery.test.ts | 37 +++ .../missing-ssh-pty-provider-recovery.ts | 25 ++ src/main/ipc/pty/runtime/spawn-preflight.ts | 7 + .../window/attach-main-window-services.ts | 11 +- .../terminal-pane/ipc-pty-connect.ts | 15 +- .../pty-transport-spawn-errors.test.ts | 10 +- src/shared/ephemeral-vm-runtimes.ts | 20 +- 17 files changed, 819 insertions(+), 30 deletions(-) create mode 100644 src/main/ephemeral-vm-runtime-ssh-reattach.test.ts create mode 100644 src/main/ephemeral-vm-runtime-ssh-reattach.ts create mode 100644 src/main/ephemeral-vm-runtime-ssh.test.ts create mode 100644 src/main/ipc/pty/provider/missing-ssh-pty-provider-recovery.test.ts create mode 100644 src/main/ipc/pty/provider/missing-ssh-pty-provider-recovery.ts diff --git a/src/main/ephemeral-vm-runtime-ssh-reattach.test.ts b/src/main/ephemeral-vm-runtime-ssh-reattach.test.ts new file mode 100644 index 00000000000..c67d072cd88 --- /dev/null +++ b/src/main/ephemeral-vm-runtime-ssh-reattach.test.ts @@ -0,0 +1,220 @@ +import { mkdtempSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { upsertEphemeralVmRuntime } from '../shared/ephemeral-vm-runtime-store' +import type { + EphemeralVmRuntimeRecord, + EphemeralVmRuntimeStatus +} from '../shared/ephemeral-vm-runtimes' + +const { getRelayStateMock, reattachMock } = vi.hoisted(() => ({ + getRelayStateMock: vi.fn(), + reattachMock: vi.fn() +})) + +vi.mock('./ephemeral-vm-runtime-ssh', () => ({ + getRuntimeOwnedSshRelayState: getRelayStateMock, + reattachRuntimeOwnedSshTarget: reattachMock +})) + +import { + ensureRuntimeOwnedSshTargetAttached, + installRuntimeOwnedSshPtyProviderRecovery, + reattachRuntimeOwnedSshTargetsAtStartup +} from './ephemeral-vm-runtime-ssh-reattach' +import { recoverMissingSshPtyProvider } from './ipc/pty/provider/missing-ssh-pty-provider-recovery' +import { registerSshPtyProvider, unregisterSshPtyProvider } from './ipc/pty/provider/registry' + +const tempDirs: string[] = [] + +function makeUserData(): string { + const dir = mkdtempSync(join(tmpdir(), 'orca-vm-ssh-reattach-')) + tempDirs.push(dir) + return dir +} + +function sshRuntime( + id: string, + status: EphemeralVmRuntimeStatus, + overrides: Partial = {} +): EphemeralVmRuntimeRecord & { sshTargetId: string } { + return { + id, + recipeId: 'sandbox', + repoId: 'repo-1', + workspaceId: `ws-${id}`, + status, + cleanupStatus: 'not_started', + connectionMode: 'ssh', + sshTargetId: `runtime-ssh-${id}`, + createdAt: 1, + updatedAt: 1, + recipeResult: { + schemaVersion: 1, + connection: { + type: 'ssh', + projectRoot: '/sandbox/project', + target: { label: 'VM', host: '127.0.0.1', port: 2222, username: 'root' } + } + }, + ...overrides + } as EphemeralVmRuntimeRecord & { sshTargetId: string } +} + +beforeEach(() => { + getRelayStateMock.mockReset().mockReturnValue('detached') + reattachMock.mockReset().mockResolvedValue(undefined) +}) + +afterEach(() => { + for (const dir of tempDirs.splice(0)) { + rmSync(dir, { recursive: true, force: true }) + } + vi.restoreAllMocks() +}) + +describe('ensureRuntimeOwnedSshTargetAttached', () => { + it('does not dial a target whose relay is already attached', async () => { + getRelayStateMock.mockReturnValue('attached') + await ensureRuntimeOwnedSshTargetAttached(sshRuntime('a', 'running')) + expect(reattachMock).not.toHaveBeenCalled() + }) + + it('shares one in-flight connect between concurrent callers for the same target', async () => { + let release!: () => void + reattachMock.mockReturnValue(new Promise((resolve) => (release = resolve))) + const runtime = sshRuntime('b', 'running') + const first = ensureRuntimeOwnedSshTargetAttached(runtime) + const second = ensureRuntimeOwnedSshTargetAttached(runtime) + expect(reattachMock).toHaveBeenCalledTimes(1) + release() + await Promise.all([first, second]) + // Why: the entry must be cleared once settled, else a later failure could never retry. + await ensureRuntimeOwnedSshTargetAttached(runtime) + expect(reattachMock).toHaveBeenCalledTimes(2) + }) + + it('surfaces the connect failure to the caller and allows a retry', async () => { + reattachMock.mockRejectedValueOnce(new Error('ECONNREFUSED')) + const runtime = sshRuntime('c', 'running') + await expect(ensureRuntimeOwnedSshTargetAttached(runtime)).rejects.toThrow('ECONNREFUSED') + await expect(ensureRuntimeOwnedSshTargetAttached(runtime)).resolves.toBeUndefined() + expect(reattachMock).toHaveBeenCalledTimes(2) + }) +}) + +describe('reattachRuntimeOwnedSshTargetsAtStartup', () => { + it('re-attaches every running SSH runtime and skips the rest', async () => { + const userDataPath = makeUserData() + upsertEphemeralVmRuntime(userDataPath, sshRuntime('running', 'running')) + upsertEphemeralVmRuntime(userDataPath, sshRuntime('suspend-failed', 'suspend_failed')) + upsertEphemeralVmRuntime(userDataPath, sshRuntime('suspended', 'suspended')) + upsertEphemeralVmRuntime(userDataPath, sshRuntime('cleaned', 'cleaned')) + upsertEphemeralVmRuntime( + userDataPath, + sshRuntime('orca-server', 'running', { + connectionMode: 'orca-server', + sshTargetId: undefined, + runtimeEnvironmentId: 'env-1', + recipeResult: { schemaVersion: 1, pairingCode: 'code', projectRoot: '/w' } + }) + ) + + await reattachRuntimeOwnedSshTargetsAtStartup(() => userDataPath) + + expect(reattachMock.mock.calls.map(([runtime]) => runtime.id).sort()).toEqual([ + 'running', + 'suspend-failed' + ]) + }) + + it('keeps going when one runtime fails to re-attach', async () => { + const userDataPath = makeUserData() + upsertEphemeralVmRuntime(userDataPath, sshRuntime('fails', 'running')) + upsertEphemeralVmRuntime(userDataPath, sshRuntime('works', 'running')) + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + reattachMock.mockImplementation(async (runtime: EphemeralVmRuntimeRecord) => { + if (runtime.id === 'fails') { + throw new Error('host unreachable') + } + }) + + await expect( + reattachRuntimeOwnedSshTargetsAtStartup(() => userDataPath) + ).resolves.toBeUndefined() + + expect(reattachMock).toHaveBeenCalledTimes(2) + expect(warn).toHaveBeenCalledWith(expect.stringContaining('fails')) + expect(warn).toHaveBeenCalledWith(expect.stringContaining('host unreachable')) + }) +}) + +describe('installRuntimeOwnedSshPtyProviderRecovery', () => { + it('re-attaches a running runtime-owned target on a provider miss', async () => { + const userDataPath = makeUserData() + const runtime = sshRuntime('miss', 'running') + upsertEphemeralVmRuntime(userDataPath, runtime) + installRuntimeOwnedSshPtyProviderRecovery(() => userDataPath) + + await expect(recoverMissingSshPtyProvider(runtime.sshTargetId)).resolves.toBeUndefined() + + expect(reattachMock).toHaveBeenCalledWith(expect.objectContaining({ id: 'miss' })) + }) + + it('does nothing when the provider is already registered', () => { + const userDataPath = makeUserData() + const runtime = sshRuntime('registered', 'running') + upsertEphemeralVmRuntime(userDataPath, runtime) + installRuntimeOwnedSshPtyProviderRecovery(() => userDataPath) + registerSshPtyProvider(runtime.sshTargetId, {} as never) + try { + expect(recoverMissingSshPtyProvider(runtime.sshTargetId)).toBeUndefined() + expect(reattachMock).not.toHaveBeenCalled() + } finally { + unregisterSshPtyProvider(runtime.sshTargetId) + } + }) + + it.each([ + ['a user SSH target', 'ssh-1700000000-abc123'], + ['a local spawn', null], + ['an unknown runtime-owned id', 'runtime-ssh-not-persisted'] + ])('leaves the ordinary provider miss in place for %s', (_label, connectionId) => { + const userDataPath = makeUserData() + installRuntimeOwnedSshPtyProviderRecovery(() => userDataPath) + expect(recoverMissingSshPtyProvider(connectionId)).toBeUndefined() + expect(reattachMock).not.toHaveBeenCalled() + }) + + it('does not dial a runtime that is not expected to be up', () => { + const userDataPath = makeUserData() + const runtime = sshRuntime('asleep', 'suspended') + upsertEphemeralVmRuntime(userDataPath, runtime) + installRuntimeOwnedSshPtyProviderRecovery(() => userDataPath) + expect(recoverMissingSshPtyProvider(runtime.sshTargetId)).toBeUndefined() + expect(reattachMock).not.toHaveBeenCalled() + }) + + it('leaves a relay that is reconnecting on its own alone', () => { + const userDataPath = makeUserData() + const runtime = sshRuntime('self-healing', 'running') + upsertEphemeralVmRuntime(userDataPath, runtime) + getRelayStateMock.mockReturnValue('reconnecting') + installRuntimeOwnedSshPtyProviderRecovery(() => userDataPath) + expect(recoverMissingSshPtyProvider(runtime.sshTargetId)).toBeUndefined() + expect(reattachMock).not.toHaveBeenCalled() + }) + + it('names the retry when the re-attach fails', async () => { + const userDataPath = makeUserData() + const runtime = sshRuntime('refused', 'running') + upsertEphemeralVmRuntime(userDataPath, runtime) + reattachMock.mockRejectedValue(new Error('connect ECONNREFUSED 127.0.0.1:2222')) + installRuntimeOwnedSshPtyProviderRecovery(() => userDataPath) + + await expect(recoverMissingSshPtyProvider(runtime.sshTargetId)).rejects.toThrow( + /ECONNREFUSED 127\.0\.0\.1:2222.*Open the workspace again or start a new terminal/s + ) + }) +}) diff --git a/src/main/ephemeral-vm-runtime-ssh-reattach.ts b/src/main/ephemeral-vm-runtime-ssh-reattach.ts new file mode 100644 index 00000000000..b90237ac494 --- /dev/null +++ b/src/main/ephemeral-vm-runtime-ssh-reattach.ts @@ -0,0 +1,97 @@ +import { listEphemeralVmRuntimes } from '../shared/ephemeral-vm-runtime-store' +import { + runtimeExpectsLiveSshRelay, + type EphemeralVmRuntimeRecord +} from '../shared/ephemeral-vm-runtimes' +import { isRuntimeOwnedSshTargetId } from '../shared/execution-host' +import { setMissingSshPtyProviderRecovery } from './ipc/pty/provider/missing-ssh-pty-provider-recovery' +import { + getRuntimeOwnedSshRelayState, + reattachRuntimeOwnedSshTarget +} from './ephemeral-vm-runtime-ssh' + +const reattachInFlight = new Map>() + +/** + * Re-attach the relay of every runtime persisted as running whose relay this process + * does not hold. Runtime-owned targets are excluded from every generic SSH connect path + * (startup restore, pane connect, the host list), so this is the only thing that + * dials them after an app restart. Failures are logged, not thrown: the VM may be gone, + * and the resume/spawn paths retry on demand. + */ +export async function reattachRuntimeOwnedSshTargetsAtStartup( + getUserDataPath: () => string +): Promise { + let runtimes: (EphemeralVmRuntimeRecord & { sshTargetId: string })[] + try { + runtimes = listEphemeralVmRuntimes(getUserDataPath()).filter(runtimeExpectsLiveSshRelay) + } catch (error) { + console.warn(`[ephemeral-vm] Skipping SSH relay re-attach at startup: ${describeError(error)}`) + return + } + await Promise.all( + runtimes.map((runtime) => + ensureRuntimeOwnedSshTargetAttached(runtime).catch((error: unknown) => { + console.warn( + `[ephemeral-vm] Could not re-attach SSH relay for runtime ${runtime.id} at startup: ${describeError(error)}` + ) + }) + ) + ) +} + +function describeError(error: unknown): string { + return error instanceof Error ? error.message : String(error) +} + +/** Serialized per target so a startup pass, a resume, and a spawn share one connect. */ +export function ensureRuntimeOwnedSshTargetAttached( + runtime: EphemeralVmRuntimeRecord & { sshTargetId: string } +): Promise { + if (getRuntimeOwnedSshRelayState(runtime.sshTargetId) === 'attached') { + return Promise.resolve() + } + const existing = reattachInFlight.get(runtime.sshTargetId) + if (existing) { + return existing + } + const attempt = reattachRuntimeOwnedSshTarget(runtime).finally(() => { + if (reattachInFlight.get(runtime.sshTargetId) === attempt) { + reattachInFlight.delete(runtime.sshTargetId) + } + }) + reattachInFlight.set(runtime.sshTargetId, attempt) + return attempt +} + +/** + * Resolve a PTY-provider miss for a runtime-owned target by re-attaching its relay + * before the spawn resolves the provider. Non-runtime ids and runtimes that are not + * expected to be up return undefined so the ordinary "No PTY provider" path stands. + */ +export function installRuntimeOwnedSshPtyProviderRecovery(getUserDataPath: () => string): void { + setMissingSshPtyProviderRecovery((connectionId) => { + if (!isRuntimeOwnedSshTargetId(connectionId)) { + return undefined + } + // Why leave a self-reconnecting relay alone: it re-registers its provider itself, and + // dialing over it would tear the recovering session down. + if (getRuntimeOwnedSshRelayState(connectionId) === 'reconnecting') { + return undefined + } + const runtime = listEphemeralVmRuntimes(getUserDataPath()).find( + (entry) => entry.sshTargetId === connectionId + ) + if (!runtime || !runtimeExpectsLiveSshRelay(runtime)) { + return undefined + } + // Why rewrap: this message reaches the terminal as-is, and the bare connect error does + // not say which retry the user has (runtime-owned targets have no host-list Reconnect). + return ensureRuntimeOwnedSshTargetAttached(runtime).catch((error: unknown) => { + throw new Error( + `Could not re-attach the SSH relay for this workspace: ${describeError(error)} ` + + 'Open the workspace again or start a new terminal to retry.' + ) + }) + }) +} diff --git a/src/main/ephemeral-vm-runtime-ssh.test.ts b/src/main/ephemeral-vm-runtime-ssh.test.ts new file mode 100644 index 00000000000..966a1e9927c --- /dev/null +++ b/src/main/ephemeral-vm-runtime-ssh.test.ts @@ -0,0 +1,162 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { EphemeralVmRuntimeRecord } from '../shared/ephemeral-vm-runtimes' + +const mocks = vi.hoisted(() => ({ + connectRegisteredSshTarget: vi.fn(), + upsertRuntimeOwnedTarget: vi.fn(), + removeRegisteredSshTarget: vi.fn(), + disconnectRegisteredSshTarget: vi.fn(), + getRegisteredSshState: vi.fn(), + getSshGitProvider: vi.fn(), + getSshFilesystemProvider: vi.fn(), + getSshPtyProvider: vi.fn() +})) + +vi.mock('./ipc/ssh', () => ({ + connectRegisteredSshTarget: mocks.connectRegisteredSshTarget, + getSshConnectionStore: () => ({ upsertRuntimeOwnedTarget: mocks.upsertRuntimeOwnedTarget }) +})) +vi.mock('./ipc/ssh-session-teardown', () => ({ + removeRegisteredSshTarget: mocks.removeRegisteredSshTarget, + disconnectRegisteredSshTarget: mocks.disconnectRegisteredSshTarget +})) +vi.mock('./ssh/ssh-target-registry', () => ({ + getRegisteredSshState: mocks.getRegisteredSshState +})) +vi.mock('./providers/ssh-git-dispatch', () => ({ getSshGitProvider: mocks.getSshGitProvider })) +vi.mock('./providers/ssh-filesystem-dispatch', () => ({ + getSshFilesystemProvider: mocks.getSshFilesystemProvider +})) +vi.mock('./ipc/pty/provider/registry', () => ({ getSshPtyProvider: mocks.getSshPtyProvider })) + +import { + connectRuntimeOwnedSshTarget, + getRuntimeOwnedSshRelayState, + reattachRuntimeOwnedSshTarget +} from './ephemeral-vm-runtime-ssh' + +const TARGET_ID = 'runtime-ssh-orca-1' +const connection = { + type: 'ssh' as const, + projectRoot: '/sandbox/project', + target: { label: 'VM', host: '127.0.0.1', port: 2222, username: 'root' } +} +const runtime = { + id: 'orca-1', + recipeId: 'sandbox', + status: 'running', + cleanupStatus: 'not_started', + connectionMode: 'ssh', + sshTargetId: TARGET_ID, + createdAt: 1, + updatedAt: 1, + recipeResult: { schemaVersion: 1, connection } +} as EphemeralVmRuntimeRecord & { sshTargetId: string } + +beforeEach(() => { + vi.useFakeTimers() + for (const mock of Object.values(mocks)) { + mock.mockReset() + } + mocks.upsertRuntimeOwnedTarget.mockImplementation((runtimeId: string, target: object) => ({ + ...target, + id: `runtime-ssh-${runtimeId}` + })) + mocks.connectRegisteredSshTarget.mockResolvedValue({ targetId: TARGET_ID, status: 'connected' }) + mocks.removeRegisteredSshTarget.mockResolvedValue(undefined) + mocks.getSshGitProvider.mockReturnValue({}) + mocks.getSshFilesystemProvider.mockReturnValue({}) + mocks.getSshPtyProvider.mockReturnValue({}) +}) + +afterEach(() => { + vi.useRealTimers() +}) + +describe('connectRuntimeOwnedSshTarget', () => { + it('waits for the PTY provider as well as git and filesystem before reporting ready', async () => { + // Why: a terminal spawn right after connect used to race the relay's PTY provider registration. + let ptyReady = false + mocks.getSshPtyProvider.mockImplementation(() => (ptyReady ? {} : undefined)) + let settled = false + const pending = connectRuntimeOwnedSshTarget({ runtimeId: 'orca-1', connection }).then(() => { + settled = true + }) + await vi.advanceTimersByTimeAsync(1_000) + expect(settled).toBe(false) + ptyReady = true + await vi.advanceTimersByTimeAsync(200) + await pending + expect(settled).toBe(true) + }) + + it('removes the freshly persisted target when the providers never become ready', async () => { + mocks.getSshPtyProvider.mockReturnValue(undefined) + const pending = connectRuntimeOwnedSshTarget({ runtimeId: 'orca-1', connection }) + const rejection = expect(pending).rejects.toThrow('SSH relay providers were not ready') + await vi.advanceTimersByTimeAsync(11_000) + await rejection + expect(mocks.removeRegisteredSshTarget).toHaveBeenCalledWith(TARGET_ID) + }) +}) + +describe('getRuntimeOwnedSshRelayState', () => { + it.each([ + ['attached', 'connected', {}], + ['detached', 'connected', undefined], + ['reconnecting', 'reconnecting', undefined], + ['detached', 'disconnected', undefined], + ['detached', undefined, undefined] + ] as const)('reads %s from status=%s', (expected, status, ptyProvider) => { + mocks.getRegisteredSshState.mockReturnValue(status ? { status } : undefined) + mocks.getSshPtyProvider.mockReturnValue(ptyProvider) + expect(getRuntimeOwnedSshRelayState(TARGET_ID)).toBe(expected) + }) +}) + +describe('reattachRuntimeOwnedSshTarget', () => { + it('re-upserts the target row from the recipe result and dials it when detached', async () => { + mocks.getRegisteredSshState.mockReturnValue(undefined) + await reattachRuntimeOwnedSshTarget(runtime) + expect(mocks.upsertRuntimeOwnedTarget).toHaveBeenCalledWith('orca-1', connection.target) + expect(mocks.connectRegisteredSshTarget).toHaveBeenCalledWith(TARGET_ID) + }) + + it('keeps the target row when the dial fails, unlike the provisioning connect', async () => { + // Why: the VM is still recorded as running, so the workspace must keep pointing at + // its target for the next activation or spawn to retry. + mocks.getRegisteredSshState.mockReturnValue(undefined) + mocks.connectRegisteredSshTarget.mockResolvedValue({ + targetId: TARGET_ID, + status: 'error', + error: 'connect ECONNREFUSED' + }) + await expect(reattachRuntimeOwnedSshTarget(runtime)).rejects.toThrow('ECONNREFUSED') + expect(mocks.removeRegisteredSshTarget).not.toHaveBeenCalled() + }) + + it('does not dial an attached target', async () => { + mocks.getRegisteredSshState.mockReturnValue({ status: 'connected' }) + await reattachRuntimeOwnedSshTarget(runtime) + expect(mocks.connectRegisteredSshTarget).not.toHaveBeenCalled() + expect(mocks.upsertRuntimeOwnedTarget).not.toHaveBeenCalled() + }) + + it('refuses to dial over a relay that is reconnecting on its own', async () => { + mocks.getRegisteredSshState.mockReturnValue({ status: 'reconnecting' }) + await expect(reattachRuntimeOwnedSshTarget(runtime)).rejects.toThrow('still reconnecting') + expect(mocks.connectRegisteredSshTarget).not.toHaveBeenCalled() + }) + + it('waits for providers instead of redialing a connected transport', async () => { + mocks.getRegisteredSshState.mockReturnValue({ status: 'connected' }) + let ptyReady = false + mocks.getSshPtyProvider.mockImplementation(() => (ptyReady ? {} : undefined)) + const pending = reattachRuntimeOwnedSshTarget(runtime) + await vi.advanceTimersByTimeAsync(300) + ptyReady = true + await vi.advanceTimersByTimeAsync(200) + await pending + expect(mocks.connectRegisteredSshTarget).not.toHaveBeenCalled() + }) +}) diff --git a/src/main/ephemeral-vm-runtime-ssh.ts b/src/main/ephemeral-vm-runtime-ssh.ts index 770a43cbf8f..ab7e712871d 100644 --- a/src/main/ephemeral-vm-runtime-ssh.ts +++ b/src/main/ephemeral-vm-runtime-ssh.ts @@ -1,24 +1,38 @@ import { getSshFilesystemProvider } from './providers/ssh-filesystem-dispatch' import { getSshGitProvider } from './providers/ssh-git-dispatch' +import { getSshPtyProvider } from './ipc/pty/provider/registry' import { connectRegisteredSshTarget, getSshConnectionStore } from './ipc/ssh' import { disconnectRegisteredSshTarget, removeRegisteredSshTarget } from './ipc/ssh-session-teardown' +import { getRegisteredSshState } from './ssh/ssh-target-registry' import type { EphemeralVmRecipeConnection } from '../shared/ephemeral-vm-recipes' +import type { EphemeralVmRuntimeRecord } from '../shared/ephemeral-vm-runtimes' +import { getEphemeralVmRecipeResultConnection } from '../shared/ephemeral-vm-recipes' import type { SshTarget } from '../shared/ssh-types' const SSH_PROVIDER_READY_TIMEOUT_MS = 10_000 const SSH_PROVIDER_READY_INTERVAL_MS = 100 +type RuntimeOwnedSshConnection = Extract + export type RuntimeOwnedSshConnectionResult = { targetId: string target: SshTarget } +/** + * `attached`: connected with the PTY provider registered. `reconnecting`: the relay is + * recovering on its own and a fresh dial would tear that down. `detached`: nothing in + * this process serves the target — the state after an app restart, or the moment + * between a connect and the relay registering its providers. + */ +export type RuntimeOwnedSshRelayState = 'attached' | 'reconnecting' | 'detached' + export async function connectRuntimeOwnedSshTarget(args: { runtimeId: string - connection: Extract + connection: RuntimeOwnedSshConnection signal?: AbortSignal }): Promise { const store = getSshConnectionStore() @@ -27,11 +41,7 @@ export async function connectRuntimeOwnedSshTarget(args: { } const target = store.upsertRuntimeOwnedTarget(args.runtimeId, args.connection.target) try { - const state = await connectRegisteredSshTarget(target.id) - if (state.status !== 'connected') { - throw new Error(state.error || `SSH target did not connect: ${state.status}`) - } - await waitForRuntimeSshProviders(target.id, args.signal) + await connectAndAwaitRuntimeSshProviders(target.id, args.signal) } catch (error) { // The target is persisted at upsert, so a failed connect/provider-wait would // orphan it; remove it (idempotent) before rethrowing so cleanup is complete. @@ -41,6 +51,49 @@ export async function connectRuntimeOwnedSshTarget(args: { return { targetId: target.id, target } } +export function getRuntimeOwnedSshRelayState(targetId: string): RuntimeOwnedSshRelayState { + const status = getRegisteredSshState(targetId)?.status + if (status === 'connected' && getSshPtyProvider(targetId)) { + return 'attached' + } + return status === 'reconnecting' ? 'reconnecting' : 'detached' +} + +/** + * Re-establish the relay for a runtime that is still running. Unlike the provisioning + * connect, a failure keeps the target row: the workspace still points at it and the + * VM is up, so the next activation or terminal spawn retries instead of orphaning it. + */ +export async function reattachRuntimeOwnedSshTarget( + runtime: EphemeralVmRuntimeRecord & { sshTargetId: string } +): Promise { + const relayState = getRuntimeOwnedSshRelayState(runtime.sshTargetId) + if (relayState === 'attached') { + return + } + if (relayState === 'reconnecting') { + throw new Error(`SSH relay for runtime "${runtime.id}" is still reconnecting.`) + } + if (getRegisteredSshState(runtime.sshTargetId)?.status === 'connected') { + // Why not dial: the transport is up and the relay is about to register its providers; + // a second connect would tear that session down for nothing. + await waitForRuntimeSshProviders(runtime.sshTargetId) + return + } + const store = getSshConnectionStore() + if (!store) { + throw new Error('SSH handlers are not registered.') + } + const connection = getEphemeralVmRecipeResultConnection(runtime.recipeResult) + if (connection.type !== 'ssh') { + throw new Error(`Runtime "${runtime.id}" has no SSH connection to re-attach.`) + } + // Why re-upsert: the target row lives in the profile, the runtime record in its own + // file; recreating the row from the recipe result heals a profile that lost it. + const target = store.upsertRuntimeOwnedTarget(runtime.id, connection.target) + await connectAndAwaitRuntimeSshProviders(target.id) +} + export async function disconnectRuntimeOwnedSshTarget(targetId: string | undefined): Promise { if (!targetId) { return @@ -55,13 +108,30 @@ export async function removeRuntimeOwnedSshTarget(targetId: string | undefined): await removeRegisteredSshTarget(targetId) } +async function connectAndAwaitRuntimeSshProviders( + targetId: string, + signal?: AbortSignal +): Promise { + const state = await connectRegisteredSshTarget(targetId) + if (state.status !== 'connected') { + throw new Error(state.error || `SSH target did not connect: ${state.status}`) + } + await waitForRuntimeSshProviders(targetId, signal) +} + async function waitForRuntimeSshProviders(targetId: string, signal?: AbortSignal): Promise { const startedAt = Date.now() while (Date.now() - startedAt < SSH_PROVIDER_READY_TIMEOUT_MS) { if (signal?.aborted) { throw new Error(`SSH provider wait aborted for target "${targetId}".`) } - if (getSshGitProvider(targetId) && getSshFilesystemProvider(targetId)) { + // Why the PTY provider too: a terminal spawn right after connect otherwise races + // the relay's provider registration and fails with "No PTY provider". + if ( + getSshGitProvider(targetId) && + getSshFilesystemProvider(targetId) && + getSshPtyProvider(targetId) + ) { return } await new Promise((resolve) => setTimeout(resolve, SSH_PROVIDER_READY_INTERVAL_MS)) diff --git a/src/main/ipc/ephemeral-vm-runtime-handler-cleanup.test.ts b/src/main/ipc/ephemeral-vm-runtime-handler-cleanup.test.ts index adfabdd35bf..388565d169b 100644 --- a/src/main/ipc/ephemeral-vm-runtime-handler-cleanup.test.ts +++ b/src/main/ipc/ephemeral-vm-runtime-handler-cleanup.test.ts @@ -4,15 +4,23 @@ import { join } from 'node:path' import { afterEach, beforeEach, expect, it, vi } from 'vitest' import { upsertEphemeralVmRuntime } from '../../shared/ephemeral-vm-runtime-store' -const handlers = new Map unknown>() -const { getPathMock, handleMock, removeRuntimeOwnedSshTargetMock, removeHandlerMock } = vi.hoisted( - () => ({ - getPathMock: vi.fn(), - handleMock: vi.fn(), - removeRuntimeOwnedSshTargetMock: vi.fn(), - removeHandlerMock: vi.fn() - }) -) +const handlers = new Map< + string, + (_event: unknown, args: { runtimeId?: string; workspaceId?: string }) => unknown +>() +const { + getPathMock, + handleMock, + removeRuntimeOwnedSshTargetMock, + removeHandlerMock, + ensureRuntimeOwnedSshTargetAttachedMock +} = vi.hoisted(() => ({ + getPathMock: vi.fn(), + handleMock: vi.fn(), + removeRuntimeOwnedSshTargetMock: vi.fn(), + removeHandlerMock: vi.fn(), + ensureRuntimeOwnedSshTargetAttachedMock: vi.fn() +})) vi.mock('electron', () => ({ app: { getPath: getPathMock }, @@ -24,6 +32,9 @@ vi.mock('../ephemeral-vm-runtime-ssh', () => ({ disconnectRuntimeOwnedSshTarget: vi.fn(), removeRuntimeOwnedSshTarget: removeRuntimeOwnedSshTargetMock })) +vi.mock('../ephemeral-vm-runtime-ssh-reattach', () => ({ + ensureRuntimeOwnedSshTargetAttached: ensureRuntimeOwnedSshTargetAttachedMock +})) import { registerEphemeralVmRuntimeHandlers } from './ephemeral-vm-runtime-handlers' @@ -37,9 +48,13 @@ beforeEach(() => { handlers.clear() handleMock.mockReset() removeRuntimeOwnedSshTargetMock.mockReset().mockResolvedValue(undefined) + ensureRuntimeOwnedSshTargetAttachedMock.mockReset().mockResolvedValue(undefined) removeHandlerMock.mockReset() handleMock.mockImplementation( - (channel: string, handler: (_event: unknown, args: { runtimeId: string }) => unknown) => { + ( + channel: string, + handler: (_event: unknown, args: { runtimeId?: string; workspaceId?: string }) => unknown + ) => { handlers.set(channel, handler) } ) @@ -148,3 +163,69 @@ it('stops in-flight cleanup and retains the runtime for retry', async () => { }) await expect(cleanup).resolves.toMatchObject({ status: 'cleanup_failed' }) }) + +function runningSshRuntime(userDataPath: string, id: string, workspaceId: string): void { + upsertEphemeralVmRuntime(userDataPath, { + id, + recipeId: 'cloud-sandbox', + repoId: 'repo-1', + workspaceId, + status: 'running', + cleanupStatus: 'not_started', + connectionMode: 'ssh', + sshTargetId: `runtime-ssh-${id}`, + createdAt: 1, + updatedAt: 1, + recipeResult: { + schemaVersion: 1, + connection: { + type: 'ssh', + projectRoot: '/workspace/repo', + target: { label: 'VM', host: '127.0.0.1', port: 2222, username: 'root' } + } + } + }) +} + +it('re-attaches the SSH relay when a running SSH runtime is activated', async () => { + // Why: after an app restart the record is still 'running' but no relay exists in this + // process; the resume gate used to return the record untouched and leave it stranded. + const userDataPath = mkdtempSync(join(tmpdir(), 'orca-vm-runtime-handler-')) + tempDirs.push(userDataPath) + getPathMock.mockReturnValue(userDataPath) + runningSshRuntime(userDataPath, 'runtime-restarted', 'workspace-restarted') + registerEphemeralVmRuntimeHandlers({ getRepo: vi.fn() } as never) + + const resumed = await handlers.get('ephemeralVm:resumeWorkspace')?.(null, { + workspaceId: 'workspace-restarted' + }) + + expect(ensureRuntimeOwnedSshTargetAttachedMock).toHaveBeenCalledTimes(1) + expect(ensureRuntimeOwnedSshTargetAttachedMock).toHaveBeenCalledWith( + expect.objectContaining({ + id: 'runtime-restarted', + sshTargetId: 'runtime-ssh-runtime-restarted' + }) + ) + expect(resumed).toEqual(expect.objectContaining({ status: 'running' })) +}) + +it('reports a failed re-attach on activation and leaves the record running', async () => { + // Why: a relay that will not attach is not evidence the VM is gone; the next activation + // or terminal spawn retries, so the status must not flip to a resume failure. + const userDataPath = mkdtempSync(join(tmpdir(), 'orca-vm-runtime-handler-')) + tempDirs.push(userDataPath) + getPathMock.mockReturnValue(userDataPath) + runningSshRuntime(userDataPath, 'runtime-unreachable', 'workspace-unreachable') + ensureRuntimeOwnedSshTargetAttachedMock.mockRejectedValue(new Error('connect ECONNREFUSED')) + registerEphemeralVmRuntimeHandlers({ getRepo: vi.fn() } as never) + + await expect( + handlers.get('ephemeralVm:resumeWorkspace')?.(null, { workspaceId: 'workspace-unreachable' }) + ).rejects.toThrow('ECONNREFUSED') + + const runtimes = await handlers.get('ephemeralVm:listRuntimes')?.(null, {}) + expect(runtimes).toEqual([ + expect.objectContaining({ id: 'runtime-unreachable', status: 'running' }) + ]) +}) diff --git a/src/main/ipc/ephemeral-vm-runtime-handlers.ts b/src/main/ipc/ephemeral-vm-runtime-handlers.ts index 261c8050df8..2617580f7f9 100644 --- a/src/main/ipc/ephemeral-vm-runtime-handlers.ts +++ b/src/main/ipc/ephemeral-vm-runtime-handlers.ts @@ -4,7 +4,10 @@ import { listEphemeralVmRuntimes, updateEphemeralVmRuntimeStatus } from '../../shared/ephemeral-vm-runtime-store' -import type { EphemeralVmRuntimeRecord } from '../../shared/ephemeral-vm-runtimes' +import { + runtimeExpectsLiveSshRelay, + type EphemeralVmRuntimeRecord +} from '../../shared/ephemeral-vm-runtimes' import { getEphemeralVmRecipeResultConnection, getEphemeralVmRecipeResultPairingCode @@ -29,6 +32,7 @@ import { disconnectRuntimeOwnedSshTarget, removeRuntimeOwnedSshTarget } from '../ephemeral-vm-runtime-ssh' +import { ensureRuntimeOwnedSshTargetAttached } from '../ephemeral-vm-runtime-ssh-reattach' import { getRuntimeRecipeContext } from './ephemeral-vm-recipe-context' import { invalidateRuntimeEnvironmentTransport } from './runtime-environments' import { attachEphemeralVmRuntimeToWorkspace } from '../ephemeral-vm-runtime-attachment' @@ -203,6 +207,13 @@ export function registerEphemeralVmRuntimeHandlers(store: Store): void { return null } if (runtime.status !== 'suspended' && runtime.status !== 'resume_failed') { + // Why: after an app restart a record persisted as running has a live VM but no + // relay in this process (the provisioning connect does not survive restarts); + // activation is the moment to re-attach it. The record is left unchanged on + // failure — a relay that will not attach is not evidence the VM is gone. + if (runtimeExpectsLiveSshRelay(runtime)) { + await ensureRuntimeOwnedSshTargetAttached(runtime) + } return runtime } const recipeContext = getRuntimeRecipeContext(store, userDataPath, runtime.id) diff --git a/src/main/ipc/ephemeral-vm.test.ts b/src/main/ipc/ephemeral-vm.test.ts index 4f71dde0f9f..886f11c4fee 100644 --- a/src/main/ipc/ephemeral-vm.test.ts +++ b/src/main/ipc/ephemeral-vm.test.ts @@ -14,7 +14,8 @@ const { connectRuntimeOwnedSshTargetMock, disconnectRuntimeOwnedSshTargetMock, removeRuntimeOwnedSshTargetMock, - invalidateRuntimeEnvironmentTransportMock + invalidateRuntimeEnvironmentTransportMock, + ensureRuntimeOwnedSshTargetAttachedMock } = vi.hoisted(() => ({ handleMock: vi.fn(), removeHandlerMock: vi.fn(), @@ -22,7 +23,8 @@ const { connectRuntimeOwnedSshTargetMock: vi.fn(), disconnectRuntimeOwnedSshTargetMock: vi.fn(), removeRuntimeOwnedSshTargetMock: vi.fn(), - invalidateRuntimeEnvironmentTransportMock: vi.fn() + invalidateRuntimeEnvironmentTransportMock: vi.fn(), + ensureRuntimeOwnedSshTargetAttachedMock: vi.fn() })) vi.mock('electron', () => ({ @@ -44,6 +46,9 @@ vi.mock('../ephemeral-vm-runtime-ssh', () => ({ vi.mock('./runtime-environments', () => ({ invalidateRuntimeEnvironmentTransport: invalidateRuntimeEnvironmentTransportMock })) +vi.mock('../ephemeral-vm-runtime-ssh-reattach', () => ({ + ensureRuntimeOwnedSshTargetAttached: ensureRuntimeOwnedSshTargetAttachedMock +})) import { registerEphemeralVmHandlers } from './ephemeral-vm' @@ -115,6 +120,7 @@ describe('registerEphemeralVmHandlers', () => { disconnectRuntimeOwnedSshTargetMock.mockReset() removeRuntimeOwnedSshTargetMock.mockReset() invalidateRuntimeEnvironmentTransportMock.mockReset() + ensureRuntimeOwnedSshTargetAttachedMock.mockReset().mockResolvedValue(undefined) connectRuntimeOwnedSshTargetMock.mockResolvedValue({ targetId: 'runtime-ssh-orca-instance-1', target: { @@ -678,6 +684,8 @@ describe('registerEphemeralVmHandlers', () => { } as never) expect(runningResume).toEqual(expect.objectContaining({ status: 'running' })) expect(existsSync(join(repoPath, 'resume-mode.txt'))).toBe(false) + // Why: an orca-server runtime has no runtime-owned SSH relay to re-attach on activation. + expect(ensureRuntimeOwnedSshTargetAttachedMock).not.toHaveBeenCalled() const suspended = await handlers.get('ephemeralVm:suspendWorkspace')?.(null, { workspaceId: 'workspace-1' diff --git a/src/main/ipc/pty-startup-barrier-ordering.test.ts b/src/main/ipc/pty-startup-barrier-ordering.test.ts index dcf8a5d4c44..accdb8cab63 100644 --- a/src/main/ipc/pty-startup-barrier-ordering.test.ts +++ b/src/main/ipc/pty-startup-barrier-ordering.test.ts @@ -29,4 +29,23 @@ describe('PTY startup barrier ordering', () => { expect(barrierIndex).toBeLessThan(providerIndex) } }) + + it('gives a missing SSH PTY provider its recovery before every provider resolution', () => { + // Why: a runtime-owned SSH target has no relay after an app restart until its owner + // re-attaches it; each spawn path (renderer, runtime controller, stable-pane adoption) + // must await that recovery before `getProvider` throws on the miss. + for (const relPath of [ + 'src/main/ipc/pty/ipc/spawn-preflight.ts', + 'src/main/ipc/pty/runtime/spawn-preflight.ts', + 'src/main/ipc/pty/pane/adopt-stable.ts' + ]) { + const source = readRepoSource(relPath) + const recoveryIndex = source.indexOf('await providerRecovery') + const providerIndex = source.indexOf('getProvider(args.connectionId)') + expect(source).toContain('recoverMissingSshPtyProvider(args.connectionId)') + expect(recoveryIndex, relPath).toBeGreaterThanOrEqual(0) + expect(providerIndex, relPath).toBeGreaterThanOrEqual(0) + expect(recoveryIndex, relPath).toBeLessThan(providerIndex) + } + }) }) diff --git a/src/main/ipc/pty/ipc/spawn-preflight.ts b/src/main/ipc/pty/ipc/spawn-preflight.ts index f885d6e2f6f..3eb5422eea6 100644 --- a/src/main/ipc/pty/ipc/spawn-preflight.ts +++ b/src/main/ipc/pty/ipc/spawn-preflight.ts @@ -16,11 +16,18 @@ import { recoverFreshSpawnProviderRouting, routesFreshSpawnsToLocalProvider } from '../host-env/fresh-spawn-routing' +import { recoverMissingSshPtyProvider } from '../provider/missing-ssh-pty-provider-recovery' import { getAppPtyId, getProvider, getRelayPtyId } from '../provider/registry' import type { PtyIpcSpawnState } from './spawn-state' export async function preparePtyIpcSpawnPreflight(ctx: PtyIpcSpawnState): Promise { const args = ctx.args + // Why before the provider lookup: a runtime-owned SSH target has no relay after an app + // restart until its owner re-attaches it; the spawn would otherwise fail on the miss. + const providerRecovery = recoverMissingSshPtyProvider(args.connectionId) + if (providerRecovery) { + await providerRecovery + } // Establish daemon identity before the first await so hidden delivery is gated before byte zero. ctx.provider = getProvider(args.connectionId) ctx.isDaemonHostSpawn = diff --git a/src/main/ipc/pty/pane/adopt-stable.ts b/src/main/ipc/pty/pane/adopt-stable.ts index 7c1a7d5c818..a4b1b6f4fd6 100644 --- a/src/main/ipc/pty/pane/adopt-stable.ts +++ b/src/main/ipc/pty/pane/adopt-stable.ts @@ -1,6 +1,7 @@ import type { OrcaRuntimeService } from '../../../runtime/orca-runtime' import type { Store } from '../../../persistence' import { makePaneKey } from '../../../../shared/stable-pane-id' +import { recoverMissingSshPtyProvider } from '../provider/missing-ssh-pty-provider-recovery' import { getProvider } from '../provider/registry' import { makePaneSpawnReservationKey, paneSpawnReservationsByOwnerKey } from './spawn-reservation' import { @@ -15,6 +16,14 @@ export async function adoptStablePane( store: Store | undefined, args: AdoptStablePaneArgs ): Promise { + // Why first: adoption resolves the provider before the spawn preflights run, so a + // runtime-owned target's relay must be re-attached here too. Awaited ahead of the + // pending-adoption lookup so everything from that lookup to the map write stays + // synchronous and two spawns for one pane cannot both adopt. + const providerRecovery = recoverMissingSshPtyProvider(args.connectionId) + if (providerRecovery) { + await providerRecovery + } const paneKey = makePaneKey(args.tabId, args.leafId) const ownerKey = makePaneSpawnReservationKey(args.worktreeId, args.connectionId, paneKey) const pendingAdoption = ownerKey ? stablePaneAdoptionsByOwnerKey.get(ownerKey) : undefined diff --git a/src/main/ipc/pty/provider/missing-ssh-pty-provider-recovery.test.ts b/src/main/ipc/pty/provider/missing-ssh-pty-provider-recovery.test.ts new file mode 100644 index 00000000000..80dc89570f6 --- /dev/null +++ b/src/main/ipc/pty/provider/missing-ssh-pty-provider-recovery.test.ts @@ -0,0 +1,37 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { + recoverMissingSshPtyProvider, + setMissingSshPtyProviderRecovery +} from './missing-ssh-pty-provider-recovery' +import { registerSshPtyProvider, unregisterSshPtyProvider } from './registry' + +afterEach(() => { + setMissingSshPtyProviderRecovery(null) + unregisterSshPtyProvider('ssh-registered') +}) + +describe('recoverMissingSshPtyProvider', () => { + it('returns nothing for local spawns and when no recovery is installed', () => { + expect(recoverMissingSshPtyProvider(null)).toBeUndefined() + expect(recoverMissingSshPtyProvider(undefined)).toBeUndefined() + expect(recoverMissingSshPtyProvider('ssh-missing')).toBeUndefined() + }) + + it('consults the installed recovery only for connections with no registered provider', () => { + const recovery = vi.fn(() => Promise.resolve()) + setMissingSshPtyProviderRecovery(recovery) + registerSshPtyProvider('ssh-registered', {} as never) + + expect(recoverMissingSshPtyProvider('ssh-registered')).toBeUndefined() + expect(recovery).not.toHaveBeenCalled() + + const pending = recoverMissingSshPtyProvider('ssh-missing') + expect(pending).toBeInstanceOf(Promise) + expect(recovery).toHaveBeenCalledWith('ssh-missing') + }) + + it('lets the recovery decline a connection it does not own', () => { + setMissingSshPtyProviderRecovery(() => undefined) + expect(recoverMissingSshPtyProvider('ssh-missing')).toBeUndefined() + }) +}) diff --git a/src/main/ipc/pty/provider/missing-ssh-pty-provider-recovery.ts b/src/main/ipc/pty/provider/missing-ssh-pty-provider-recovery.ts new file mode 100644 index 00000000000..5d571dc233d --- /dev/null +++ b/src/main/ipc/pty/provider/missing-ssh-pty-provider-recovery.ts @@ -0,0 +1,25 @@ +import { sshProviders } from './registry' + +type MissingSshPtyProviderRecovery = (connectionId: string) => Promise | undefined + +let recovery: MissingSshPtyProviderRecovery | null = null + +/** + * Installed by the layer that owns a connection's relay health (today: the ephemeral-VM + * runtime for runtime-owned targets). Consulted only when a PTY operation arrives for an + * SSH connection with no registered provider, so the owner can re-attach the relay + * instead of the lookup failing on the miss. Which targets to dial is the owner's policy. + */ +export function setMissingSshPtyProviderRecovery(next: MissingSshPtyProviderRecovery | null): void { + recovery = next +} + +/** A promise only when a recovery is installed and the connection has no PTY provider. */ +export function recoverMissingSshPtyProvider( + connectionId: string | null | undefined +): Promise | undefined { + if (!connectionId || !recovery || sshProviders.has(connectionId)) { + return undefined + } + return recovery(connectionId) +} diff --git a/src/main/ipc/pty/runtime/spawn-preflight.ts b/src/main/ipc/pty/runtime/spawn-preflight.ts index 43fd2778119..1d9ef594423 100644 --- a/src/main/ipc/pty/runtime/spawn-preflight.ts +++ b/src/main/ipc/pty/runtime/spawn-preflight.ts @@ -3,6 +3,7 @@ import type { PtySpawnResult } from '../../../providers/types' import { LocalPtyProvider } from '../../../providers/local-pty-provider' import { isValidTerminalTabId } from '../../../../shared/terminal-tab-id' import { isTerminalLeafId } from '../../../../shared/stable-pane-id' +import { recoverMissingSshPtyProvider } from '../provider/missing-ssh-pty-provider-recovery' import { getAppPtyId, getProvider, getRelayPtyId } from '../provider/registry' import { buildPtyHostEnv } from '../host-env/assembly' import { @@ -54,6 +55,12 @@ export async function prepareRuntimePtySpawn( } } ctx.cwd = ctx.deps.resolvePtySpawnStartupCwd(args.worktreeId, args.cwd) + // Why: same relay re-attach as the renderer spawn path — `orca terminal create` on a + // runtime-owned workspace after an app restart must not fail on the provider miss. + const providerRecovery = recoverMissingSshPtyProvider(args.connectionId) + if (providerRecovery) { + await providerRecovery + } ctx.provider = getProvider(args.connectionId) const freshSpawnRecovery = ctx.preAdoptedStablePane ? undefined diff --git a/src/main/window/attach-main-window-services.ts b/src/main/window/attach-main-window-services.ts index 46621d7f76c..4ad569b6866 100644 --- a/src/main/window/attach-main-window-services.ts +++ b/src/main/window/attach-main-window-services.ts @@ -1,4 +1,4 @@ -import { ipcMain } from 'electron' +import { app, ipcMain } from 'electron' import type { BrowserWindow, IpcMainInvokeEvent } from 'electron' import type { Store } from '../persistence' import { @@ -20,6 +20,10 @@ import { } from '../ipc/pty' import { registerDaemonManagementHandlers } from '../ipc/pty-management' import { registerSshHandlers } from '../ipc/ssh' +import { + installRuntimeOwnedSshPtyProviderRecovery, + reattachRuntimeOwnedSshTargetsAtStartup +} from '../ephemeral-vm-runtime-ssh-reattach' import { registerRemoteWorkspaceHandlers } from '../ipc/remote-workspace' import { browserManager } from '../browser/browser-manager' import { hasSystemMediaAccess, requestSystemMediaAccess } from '../browser/browser-media-access' @@ -119,6 +123,11 @@ export function attachMainWindowServices( void hydrateLocalPtyRegistryAtBoot(store) } registerSshHandlers(store, () => mainWindow, runtime) + // Why after registerSshHandlers: both dial through the registered SSH connect. Runtime-owned + // targets are skipped by the renderer's startup restore, so main owns their re-attach. + const getUserDataPath = (): string => app.getPath('userData') + installRuntimeOwnedSshPtyProviderRecovery(getUserDataPath) + void reattachRuntimeOwnedSshTargetsAtStartup(getUserDataPath) registerRemoteWorkspaceHandlers(store, () => mainWindow) registerFileDropRelay(mainWindow) registerTccPromptNoticeHandlers(mainWindow) diff --git a/src/renderer/src/components/terminal-pane/ipc-pty-connect.ts b/src/renderer/src/components/terminal-pane/ipc-pty-connect.ts index 3023236de0e..096c25b4894 100644 --- a/src/renderer/src/components/terminal-pane/ipc-pty-connect.ts +++ b/src/renderer/src/components/terminal-pane/ipc-pty-connect.ts @@ -198,11 +198,16 @@ function handleConnectError( return undefined } if (connectionId && message.includes('No PTY provider for connection')) { - if (!isRuntimeOwnedSshTargetId(connectionId)) { - context - .getCallbacks() - .onError?.('SSH connection is not active. Use the reconnect dialog or Settings to connect.') - } + // Why runtime-owned targets get the raw message: main re-attaches their relay on spawn, + // so a provider miss here means that re-attach failed and its message names the retry. + // The Settings/reconnect-dialog wording applies only to hosts users can reconnect. + context + .getCallbacks() + .onError?.( + isRuntimeOwnedSshTargetId(connectionId) + ? message + : 'SSH connection is not active. Use the reconnect dialog or Settings to connect.' + ) } else { context.getCallbacks().onError?.(message) } diff --git a/src/renderer/src/components/terminal-pane/pty-transport-spawn-errors.test.ts b/src/renderer/src/components/terminal-pane/pty-transport-spawn-errors.test.ts index 2ff2c9062a2..7c468cd3166 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport-spawn-errors.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport-spawn-errors.test.ts @@ -119,8 +119,11 @@ describe('createIpcPtyTransport', () => { ) }) - it('suppresses the SSH-not-active toast for a runtime-owned (per-workspace-env) target', async () => { - // Why: a runtime-owned SSH target disappearing is expected teardown (no reconnect dialog exists), so no toast should fire. + it('surfaces the provider miss verbatim for a runtime-owned (per-workspace-env) target', async () => { + // Why: main re-attaches a runtime-owned relay on spawn, so a provider miss that still + // reaches the pane is a failed re-attach. Runtime-owned targets have no reconnect + // dialog or Settings entry, so the canned "use Settings" line would name a control that + // does not exist; the raw message carries the retry instead. const { createIpcPtyTransport } = await import('./pty-transport') const spawnMock = vi .fn() @@ -148,7 +151,8 @@ describe('createIpcPtyTransport', () => { callbacks: { onError } }) - expect(onError).not.toHaveBeenCalled() + expect(onError).toHaveBeenCalledWith('No PTY provider for connection runtime-ssh-orca-1') + expect(onError).not.toHaveBeenCalledWith(expect.stringContaining('Settings')) }) it('refuses to call a cross-connection SSH reattach expired, and still raises no error toast', async () => { diff --git a/src/shared/ephemeral-vm-runtimes.ts b/src/shared/ephemeral-vm-runtimes.ts index 97a1a48fbf2..b9c2fdba590 100644 --- a/src/shared/ephemeral-vm-runtimes.ts +++ b/src/shared/ephemeral-vm-runtimes.ts @@ -2,7 +2,8 @@ import { z } from 'zod' import { EphemeralVmRecipeConnectionResultSchema, EphemeralVmRecipeLegacyResultSchema, - EphemeralVmRecipeResultSchema + EphemeralVmRecipeResultSchema, + getEphemeralVmRecipeResultConnection } from './ephemeral-vm-recipes' export const EphemeralVmRuntimeStatusSchema = z.enum([ @@ -70,6 +71,23 @@ export const EphemeralVmRuntimeRecordSchema = z.object({ export type EphemeralVmRuntimeRecord = z.infer +/** + * A runtime whose VM is expected to be up but whose SSH relay lives only in the app + * process: the connect at provision/resume does not survive an app restart, and every + * generic SSH connect path (startup restore, pane connect, host list) skips runtime-owned + * targets, so the runtime layer must re-attach these itself. + */ +export function runtimeExpectsLiveSshRelay( + runtime: EphemeralVmRuntimeRecord +): runtime is EphemeralVmRuntimeRecord & { sshTargetId: string } { + return ( + runtime.connectionMode === 'ssh' && + typeof runtime.sshTargetId === 'string' && + (runtime.status === 'running' || runtime.status === 'suspend_failed') && + getEphemeralVmRecipeResultConnection(runtime.recipeResult).type === 'ssh' + ) +} + export const EphemeralVmRuntimeStoreSchema = z.object({ version: z.literal(1), runtimes: z.array(EphemeralVmRuntimeRecordSchema) From 023014e049295fa3bcd3053a06a1302b9adcb594 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 7 Sep 2026 23:18:39 -0700 Subject: [PATCH 2/4] fix(ephemeral-vm): harden runtime-owned SSH re-attach after verifier review Verifier findings on #19394, all addressed: - Wiring pin: the startup pass and miss-recovery install in attach-main-window-services are now pinned by an executing test that fails when either line is deleted (attach-main-window-services-runtime-ssh-reattach.test.ts). - Behavioural spawn-path pin: replaces the source-text indexOf ordering test with one that executes all three preflights (renderer, runtime controller, stable-pane adoption) against an empty registry and fails when the recovery is gated on sessionId / preAdoptedStablePane / ownsPaneSpawnReservation. - Startup pass honours lastRequiredPassphrase (deferred, not dialed), bounds fan-out to 4 concurrent dials, and every shared in-flight re-attach is capped at 15s with the dial aborted. - A reconnecting relay no longer fails activation (no red 'Failed to wake' toast); a connected transport with no providers is redialed instead of waiting out the provider timeout. - The miss hook now also serves requireSshGitProvider / requireSshFilesystemProvider (background, throttled), so the reporter's second string recovers too. - Runtime-owned provider-miss copy no longer names a Reconnect control that does not exist, is translated in the renderer, omits the internal id, and separates cause from retry hint. --- .../ephemeral-vm-runtime-ssh-reattach.test.ts | 154 +++++++++++++++-- src/main/ephemeral-vm-runtime-ssh-reattach.ts | 107 +++++++++--- src/main/ephemeral-vm-runtime-ssh.test.ts | 59 +++++-- src/main/ephemeral-vm-runtime-ssh.ts | 35 ++-- .../ipc/pty-startup-barrier-ordering.test.ts | 19 --- src/main/ipc/pty/ipc/spawn-preflight.ts | 4 +- ...-pty-provider-recovery-spawn-paths.test.ts | 158 ++++++++++++++++++ .../missing-ssh-pty-provider-recovery.test.ts | 12 +- .../missing-ssh-pty-provider-recovery.ts | 21 +-- src/main/ipc/pty/provider/registry.ts | 15 +- src/main/providers/ssh-filesystem-dispatch.ts | 3 + src/main/providers/ssh-git-dispatch.ts | 4 + .../ssh-provider-miss-recovery.test.ts | 88 ++++++++++ .../providers/ssh-provider-miss-recovery.ts | 53 ++++++ ...ndow-services-runtime-ssh-reattach.test.ts | 146 ++++++++++++++++ .../window/attach-main-window-services.ts | 4 +- .../terminal-pane/ipc-pty-connect.ts | 44 ++++- .../pty-transport-spawn-errors.test.ts | 92 ++++++---- src/renderer/src/i18n/locales/en.json | 6 + src/shared/ssh-pty-provider-missing.test.ts | 64 +++++++ src/shared/ssh-pty-provider-missing.ts | 72 ++++++++ 21 files changed, 1014 insertions(+), 146 deletions(-) create mode 100644 src/main/ipc/pty/provider/missing-ssh-pty-provider-recovery-spawn-paths.test.ts create mode 100644 src/main/providers/ssh-provider-miss-recovery.test.ts create mode 100644 src/main/providers/ssh-provider-miss-recovery.ts create mode 100644 src/main/window/attach-main-window-services-runtime-ssh-reattach.test.ts create mode 100644 src/shared/ssh-pty-provider-missing.test.ts create mode 100644 src/shared/ssh-pty-provider-missing.ts diff --git a/src/main/ephemeral-vm-runtime-ssh-reattach.test.ts b/src/main/ephemeral-vm-runtime-ssh-reattach.test.ts index c67d072cd88..3fc5de0e1c2 100644 --- a/src/main/ephemeral-vm-runtime-ssh-reattach.test.ts +++ b/src/main/ephemeral-vm-runtime-ssh-reattach.test.ts @@ -8,23 +8,31 @@ import type { EphemeralVmRuntimeStatus } from '../shared/ephemeral-vm-runtimes' -const { getRelayStateMock, reattachMock } = vi.hoisted(() => ({ +const { getRelayStateMock, reattachMock, needsCredentialPromptMock } = vi.hoisted(() => ({ getRelayStateMock: vi.fn(), - reattachMock: vi.fn() + reattachMock: vi.fn(), + needsCredentialPromptMock: vi.fn() })) vi.mock('./ephemeral-vm-runtime-ssh', () => ({ getRuntimeOwnedSshRelayState: getRelayStateMock, - reattachRuntimeOwnedSshTarget: reattachMock + reattachRuntimeOwnedSshTarget: reattachMock, + runtimeOwnedSshTargetNeedsCredentialPrompt: needsCredentialPromptMock })) import { + RUNTIME_SSH_REATTACH_TIMEOUT_MS, + RUNTIME_SSH_STARTUP_REATTACH_CONCURRENCY, ensureRuntimeOwnedSshTargetAttached, - installRuntimeOwnedSshPtyProviderRecovery, + installRuntimeOwnedSshProviderMissRecovery, reattachRuntimeOwnedSshTargetsAtStartup } from './ephemeral-vm-runtime-ssh-reattach' import { recoverMissingSshPtyProvider } from './ipc/pty/provider/missing-ssh-pty-provider-recovery' import { registerSshPtyProvider, unregisterSshPtyProvider } from './ipc/pty/provider/registry' +import { + recoverSshProviderMiss, + setSshProviderMissRecovery +} from './providers/ssh-provider-miss-recovery' const tempDirs: string[] = [] @@ -65,13 +73,16 @@ function sshRuntime( beforeEach(() => { getRelayStateMock.mockReset().mockReturnValue('detached') reattachMock.mockReset().mockResolvedValue(undefined) + needsCredentialPromptMock.mockReset().mockReturnValue(false) }) afterEach(() => { + setSshProviderMissRecovery(null) for (const dir of tempDirs.splice(0)) { rmSync(dir, { recursive: true, force: true }) } vi.restoreAllMocks() + vi.useRealTimers() }) describe('ensureRuntimeOwnedSshTargetAttached', () => { @@ -102,6 +113,35 @@ describe('ensureRuntimeOwnedSshTargetAttached', () => { await expect(ensureRuntimeOwnedSshTargetAttached(runtime)).resolves.toBeUndefined() expect(reattachMock).toHaveBeenCalledTimes(2) }) + + it('bounds the shared wait so a dial that never settles cannot hold every joiner', async () => { + // Why: a passphrase prompt with no listener, or a host that black-holes SYNs, would + // otherwise pin every spawn and activation that joined this promise indefinitely. + vi.useFakeTimers() + let observedSignal: AbortSignal | undefined + reattachMock.mockImplementation( + (_runtime: unknown, signal?: AbortSignal) => + new Promise(() => { + observedSignal = signal + }) + ) + const runtime = sshRuntime('hang', 'running') + const spawnJoiner = ensureRuntimeOwnedSshTargetAttached(runtime) + const activationJoiner = ensureRuntimeOwnedSshTargetAttached(runtime) + const rejections = Promise.all([ + expect(spawnJoiner).rejects.toThrow(/did not attach within 15s/), + expect(activationJoiner).rejects.toThrow(/did not attach within 15s/) + ]) + await vi.advanceTimersByTimeAsync(RUNTIME_SSH_REATTACH_TIMEOUT_MS - 1) + expect(observedSignal?.aborted).toBe(false) + await vi.advanceTimersByTimeAsync(1) + await rejections + expect(observedSignal?.aborted).toBe(true) + // Why: the entry is cleared on timeout so the next gesture re-checks state and redials. + reattachMock.mockResolvedValue(undefined) + await expect(ensureRuntimeOwnedSshTargetAttached(runtime)).resolves.toBeUndefined() + expect(reattachMock).toHaveBeenCalledTimes(2) + }) }) describe('reattachRuntimeOwnedSshTargetsAtStartup', () => { @@ -129,6 +169,25 @@ describe('reattachRuntimeOwnedSshTargetsAtStartup', () => { ]) }) + it('defers a target whose last connect needed a credential prompt', async () => { + // Why: the renderer's startup restore partitions on the persisted flag for the same + // reason — no one is listening for the prompt yet, and dialing would only burn the + // credential timeout. The first user gesture re-attaches it instead. + const userDataPath = makeUserData() + upsertEphemeralVmRuntime(userDataPath, sshRuntime('keyless', 'running')) + upsertEphemeralVmRuntime(userDataPath, sshRuntime('passphrase', 'running')) + needsCredentialPromptMock.mockImplementation( + (targetId: string) => targetId === 'runtime-ssh-passphrase' + ) + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + await reattachRuntimeOwnedSshTargetsAtStartup(() => userDataPath) + + expect(reattachMock.mock.calls.map(([runtime]) => runtime.id)).toEqual(['keyless']) + expect(warn).toHaveBeenCalledWith(expect.stringContaining('Deferring')) + expect(warn).toHaveBeenCalledWith(expect.stringContaining('passphrase')) + }) + it('keeps going when one runtime fails to re-attach', async () => { const userDataPath = makeUserData() upsertEphemeralVmRuntime(userDataPath, sshRuntime('fails', 'running')) @@ -148,25 +207,86 @@ describe('reattachRuntimeOwnedSshTargetsAtStartup', () => { expect(warn).toHaveBeenCalledWith(expect.stringContaining('fails')) expect(warn).toHaveBeenCalledWith(expect.stringContaining('host unreachable')) }) + + it('bounds how many relays it dials at once', async () => { + // Why: every record a crash left `running` is dialed here; an unbounded fan-out opens + // one SSH transport per record simultaneously. + const userDataPath = makeUserData() + const total = RUNTIME_SSH_STARTUP_REATTACH_CONCURRENCY * 3 + for (let i = 0; i < total; i += 1) { + upsertEphemeralVmRuntime(userDataPath, sshRuntime(`rt-${i}`, 'running')) + } + let inFlight = 0 + let peak = 0 + const releases: (() => void)[] = [] + reattachMock.mockImplementation( + () => + new Promise((resolve) => { + inFlight += 1 + peak = Math.max(peak, inFlight) + releases.push(() => { + inFlight -= 1 + resolve() + }) + }) + ) + + const pass = reattachRuntimeOwnedSshTargetsAtStartup(() => userDataPath) + await vi.waitFor(() => + expect(reattachMock).toHaveBeenCalledTimes(RUNTIME_SSH_STARTUP_REATTACH_CONCURRENCY) + ) + // Why drain this way: each release lets a worker start the next dial only after several + // microtask hops; wait for that dial to register before releasing again. + let released = 0 + while (released < total) { + await vi.waitFor(() => expect(releases.length).toBeGreaterThan(0)) + releases.shift()!() + released += 1 + } + await pass + + expect(reattachMock).toHaveBeenCalledTimes(total) + expect(peak).toBe(RUNTIME_SSH_STARTUP_REATTACH_CONCURRENCY) + }) }) -describe('installRuntimeOwnedSshPtyProviderRecovery', () => { - it('re-attaches a running runtime-owned target on a provider miss', async () => { +describe('installRuntimeOwnedSshProviderMissRecovery', () => { + it('re-attaches a running runtime-owned target on a PTY provider miss', async () => { const userDataPath = makeUserData() const runtime = sshRuntime('miss', 'running') upsertEphemeralVmRuntime(userDataPath, runtime) - installRuntimeOwnedSshPtyProviderRecovery(() => userDataPath) + installRuntimeOwnedSshProviderMissRecovery(() => userDataPath) await expect(recoverMissingSshPtyProvider(runtime.sshTargetId)).resolves.toBeUndefined() - expect(reattachMock).toHaveBeenCalledWith(expect.objectContaining({ id: 'miss' })) + expect(reattachMock).toHaveBeenCalledWith( + expect.objectContaining({ id: 'miss' }), + expect.any(AbortSignal) + ) + }) + + it('serves the git and filesystem miss sites through the same recovery', async () => { + // Why: the reporter's second string ("Remote connection dropped…") comes from those + // dispatchers; a PTY-only hook would leave the sidebar, file tree, and source control + // failing after a restart while terminals recovered. + const userDataPath = makeUserData() + const runtime = sshRuntime('git-miss', 'running') + upsertEphemeralVmRuntime(userDataPath, runtime) + installRuntimeOwnedSshProviderMissRecovery(() => userDataPath) + + await expect(recoverSshProviderMiss(runtime.sshTargetId)).resolves.toBeUndefined() + + expect(reattachMock).toHaveBeenCalledWith( + expect.objectContaining({ id: 'git-miss' }), + expect.any(AbortSignal) + ) }) it('does nothing when the provider is already registered', () => { const userDataPath = makeUserData() const runtime = sshRuntime('registered', 'running') upsertEphemeralVmRuntime(userDataPath, runtime) - installRuntimeOwnedSshPtyProviderRecovery(() => userDataPath) + installRuntimeOwnedSshProviderMissRecovery(() => userDataPath) registerSshPtyProvider(runtime.sshTargetId, {} as never) try { expect(recoverMissingSshPtyProvider(runtime.sshTargetId)).toBeUndefined() @@ -182,7 +302,7 @@ describe('installRuntimeOwnedSshPtyProviderRecovery', () => { ['an unknown runtime-owned id', 'runtime-ssh-not-persisted'] ])('leaves the ordinary provider miss in place for %s', (_label, connectionId) => { const userDataPath = makeUserData() - installRuntimeOwnedSshPtyProviderRecovery(() => userDataPath) + installRuntimeOwnedSshProviderMissRecovery(() => userDataPath) expect(recoverMissingSshPtyProvider(connectionId)).toBeUndefined() expect(reattachMock).not.toHaveBeenCalled() }) @@ -191,7 +311,7 @@ describe('installRuntimeOwnedSshPtyProviderRecovery', () => { const userDataPath = makeUserData() const runtime = sshRuntime('asleep', 'suspended') upsertEphemeralVmRuntime(userDataPath, runtime) - installRuntimeOwnedSshPtyProviderRecovery(() => userDataPath) + installRuntimeOwnedSshProviderMissRecovery(() => userDataPath) expect(recoverMissingSshPtyProvider(runtime.sshTargetId)).toBeUndefined() expect(reattachMock).not.toHaveBeenCalled() }) @@ -201,20 +321,24 @@ describe('installRuntimeOwnedSshPtyProviderRecovery', () => { const runtime = sshRuntime('self-healing', 'running') upsertEphemeralVmRuntime(userDataPath, runtime) getRelayStateMock.mockReturnValue('reconnecting') - installRuntimeOwnedSshPtyProviderRecovery(() => userDataPath) + installRuntimeOwnedSshProviderMissRecovery(() => userDataPath) expect(recoverMissingSshPtyProvider(runtime.sshTargetId)).toBeUndefined() expect(reattachMock).not.toHaveBeenCalled() }) - it('names the retry when the re-attach fails', async () => { + it('names the retry, as a separate sentence, when the re-attach fails', async () => { const userDataPath = makeUserData() const runtime = sshRuntime('refused', 'running') upsertEphemeralVmRuntime(userDataPath, runtime) reattachMock.mockRejectedValue(new Error('connect ECONNREFUSED 127.0.0.1:2222')) - installRuntimeOwnedSshPtyProviderRecovery(() => userDataPath) + installRuntimeOwnedSshProviderMissRecovery(() => userDataPath) + // Why the full string: `orca terminal create` shows it verbatim; the renderer re-renders + // it, so its shape is also the contract the renderer parser is pinned against. await expect(recoverMissingSshPtyProvider(runtime.sshTargetId)).rejects.toThrow( - /ECONNREFUSED 127\.0\.0\.1:2222.*Open the workspace again or start a new terminal/s + 'No PTY provider for connection "runtime-ssh-refused": the SSH relay for this workspace ' + + 'could not be re-attached: connect ECONNREFUSED 127.0.0.1:2222. ' + + 'Open the workspace again or start a new terminal to retry.' ) }) }) diff --git a/src/main/ephemeral-vm-runtime-ssh-reattach.ts b/src/main/ephemeral-vm-runtime-ssh-reattach.ts index b90237ac494..175982324b2 100644 --- a/src/main/ephemeral-vm-runtime-ssh-reattach.ts +++ b/src/main/ephemeral-vm-runtime-ssh-reattach.ts @@ -4,12 +4,25 @@ import { type EphemeralVmRuntimeRecord } from '../shared/ephemeral-vm-runtimes' import { isRuntimeOwnedSshTargetId } from '../shared/execution-host' -import { setMissingSshPtyProviderRecovery } from './ipc/pty/provider/missing-ssh-pty-provider-recovery' +import { forEachWithConcurrency } from '../shared/map-with-concurrency' +import { formatRuntimeOwnedSshRelayReattachFailed } from '../shared/ssh-pty-provider-missing' +import { setSshProviderMissRecovery } from './providers/ssh-provider-miss-recovery' import { getRuntimeOwnedSshRelayState, - reattachRuntimeOwnedSshTarget + reattachRuntimeOwnedSshTarget, + runtimeOwnedSshTargetNeedsCredentialPrompt } from './ephemeral-vm-runtime-ssh' +/** + * Why bounded like the renderer's startup restore (15 s per eager target): a target that + * neither connects nor fails would otherwise hold every spawn and activation that joined + * its in-flight promise. The underlying `ssh.connect` keeps running in main after the + * timeout, so a later caller can still find the relay attached. + */ +export const RUNTIME_SSH_REATTACH_TIMEOUT_MS = 15_000 +/** Why bounded: every record left `running` by a crash is dialed at startup. */ +export const RUNTIME_SSH_STARTUP_REATTACH_CONCURRENCY = 4 + const reattachInFlight = new Map>() /** @@ -18,6 +31,10 @@ const reattachInFlight = new Map>() * (startup restore, pane connect, the host list), so this is the only thing that * dials them after an app restart. Failures are logged, not thrown: the VM may be gone, * and the resume/spawn paths retry on demand. + * + * Targets whose last connect needed a credential are deferred, exactly as the renderer's + * startup restore defers them: nothing is listening for the prompt yet, and a dial here + * would only burn the credential timeout. They re-attach on the first user gesture. */ export async function reattachRuntimeOwnedSshTargetsAtStartup( getUserDataPath: () => string @@ -29,14 +46,21 @@ export async function reattachRuntimeOwnedSshTargetsAtStartup( console.warn(`[ephemeral-vm] Skipping SSH relay re-attach at startup: ${describeError(error)}`) return } - await Promise.all( - runtimes.map((runtime) => - ensureRuntimeOwnedSshTargetAttached(runtime).catch((error: unknown) => { - console.warn( - `[ephemeral-vm] Could not re-attach SSH relay for runtime ${runtime.id} at startup: ${describeError(error)}` - ) - }) - ) + const eager = runtimes.filter((runtime) => { + if (runtimeOwnedSshTargetNeedsCredentialPrompt(runtime.sshTargetId)) { + console.warn( + `[ephemeral-vm] Deferring SSH relay re-attach for runtime ${runtime.id}: it needs a credential prompt.` + ) + return false + } + return true + }) + await forEachWithConcurrency(eager, RUNTIME_SSH_STARTUP_REATTACH_CONCURRENCY, (runtime) => + ensureRuntimeOwnedSshTargetAttached(runtime).catch((error: unknown) => { + console.warn( + `[ephemeral-vm] Could not re-attach SSH relay for runtime ${runtime.id} at startup: ${describeError(error)}` + ) + }) ) } @@ -44,9 +68,14 @@ function describeError(error: unknown): string { return error instanceof Error ? error.message : String(error) } -/** Serialized per target so a startup pass, a resume, and a spawn share one connect. */ +/** + * Serialized per target so a startup pass, a resume, and a spawn share one connect. The + * shared promise is bounded; the timed-out dial keeps running in main and the entry is + * cleared so the next caller re-checks the relay state instead of joining a dead wait. + */ export function ensureRuntimeOwnedSshTargetAttached( - runtime: EphemeralVmRuntimeRecord & { sshTargetId: string } + runtime: EphemeralVmRuntimeRecord & { sshTargetId: string }, + timeoutMs: number = RUNTIME_SSH_REATTACH_TIMEOUT_MS ): Promise { if (getRuntimeOwnedSshRelayState(runtime.sshTargetId) === 'attached') { return Promise.resolve() @@ -55,7 +84,17 @@ export function ensureRuntimeOwnedSshTargetAttached( if (existing) { return existing } - const attempt = reattachRuntimeOwnedSshTarget(runtime).finally(() => { + const abort = new AbortController() + const attempt = raceWithTimeout( + reattachRuntimeOwnedSshTarget(runtime, abort.signal), + timeoutMs, + () => { + abort.abort() + return new Error( + `SSH relay for runtime "${runtime.id}" did not attach within ${Math.round(timeoutMs / 1000)}s.` + ) + } + ).finally(() => { if (reattachInFlight.get(runtime.sshTargetId) === attempt) { reattachInFlight.delete(runtime.sshTargetId) } @@ -64,18 +103,38 @@ export function ensureRuntimeOwnedSshTargetAttached( return attempt } +function raceWithTimeout( + work: Promise, + timeoutMs: number, + onTimeout: () => Error +): Promise { + if (!Number.isFinite(timeoutMs)) { + return work + } + let timer: ReturnType | undefined + const timeout = new Promise((_resolve, reject) => { + timer = setTimeout(() => reject(onTimeout()), timeoutMs) + }) + return Promise.race([work, timeout]).finally(() => { + clearTimeout(timer) + // Why: the work promise outlives a lost race; its rejection must not become unhandled. + work.catch(() => undefined) + }) +} + /** - * Resolve a PTY-provider miss for a runtime-owned target by re-attaching its relay - * before the spawn resolves the provider. Non-runtime ids and runtimes that are not - * expected to be up return undefined so the ordinary "No PTY provider" path stands. + * Resolve a provider miss (PTY, git, or filesystem) for a runtime-owned target by + * re-attaching its relay before the operation resolves the provider. Non-runtime ids and + * runtimes that are not expected to be up return undefined so the ordinary miss stands. */ -export function installRuntimeOwnedSshPtyProviderRecovery(getUserDataPath: () => string): void { - setMissingSshPtyProviderRecovery((connectionId) => { +export function installRuntimeOwnedSshProviderMissRecovery(getUserDataPath: () => string): void { + setSshProviderMissRecovery((connectionId) => { if (!isRuntimeOwnedSshTargetId(connectionId)) { return undefined } // Why leave a self-reconnecting relay alone: it re-registers its provider itself, and - // dialing over it would tear the recovering session down. + // dialing over it would tear the recovering session down. Waiting for it here would hold + // the spawn for a bounded-but-unknown time; the ordinary miss (with its retry hint) stands. if (getRuntimeOwnedSshRelayState(connectionId) === 'reconnecting') { return undefined } @@ -85,13 +144,11 @@ export function installRuntimeOwnedSshPtyProviderRecovery(getUserDataPath: () => if (!runtime || !runtimeExpectsLiveSshRelay(runtime)) { return undefined } - // Why rewrap: this message reaches the terminal as-is, and the bare connect error does - // not say which retry the user has (runtime-owned targets have no host-list Reconnect). + // Why rewrap in the provider-miss shape: `orca terminal create` shows it as-is and needs + // the retry named (runtime-owned targets have no host-list Reconnect); the renderer + // matches the prefix and re-renders the cause with translated copy and no internal id. return ensureRuntimeOwnedSshTargetAttached(runtime).catch((error: unknown) => { - throw new Error( - `Could not re-attach the SSH relay for this workspace: ${describeError(error)} ` + - 'Open the workspace again or start a new terminal to retry.' - ) + throw new Error(formatRuntimeOwnedSshRelayReattachFailed(connectionId, describeError(error))) }) }) } diff --git a/src/main/ephemeral-vm-runtime-ssh.test.ts b/src/main/ephemeral-vm-runtime-ssh.test.ts index 966a1e9927c..2ade6183ac6 100644 --- a/src/main/ephemeral-vm-runtime-ssh.test.ts +++ b/src/main/ephemeral-vm-runtime-ssh.test.ts @@ -4,6 +4,7 @@ import type { EphemeralVmRuntimeRecord } from '../shared/ephemeral-vm-runtimes' const mocks = vi.hoisted(() => ({ connectRegisteredSshTarget: vi.fn(), upsertRuntimeOwnedTarget: vi.fn(), + getTarget: vi.fn(), removeRegisteredSshTarget: vi.fn(), disconnectRegisteredSshTarget: vi.fn(), getRegisteredSshState: vi.fn(), @@ -14,7 +15,10 @@ const mocks = vi.hoisted(() => ({ vi.mock('./ipc/ssh', () => ({ connectRegisteredSshTarget: mocks.connectRegisteredSshTarget, - getSshConnectionStore: () => ({ upsertRuntimeOwnedTarget: mocks.upsertRuntimeOwnedTarget }) + getSshConnectionStore: () => ({ + upsertRuntimeOwnedTarget: mocks.upsertRuntimeOwnedTarget, + getTarget: mocks.getTarget + }) })) vi.mock('./ipc/ssh-session-teardown', () => ({ removeRegisteredSshTarget: mocks.removeRegisteredSshTarget, @@ -32,7 +36,8 @@ vi.mock('./ipc/pty/provider/registry', () => ({ getSshPtyProvider: mocks.getSshP import { connectRuntimeOwnedSshTarget, getRuntimeOwnedSshRelayState, - reattachRuntimeOwnedSshTarget + reattachRuntimeOwnedSshTarget, + runtimeOwnedSshTargetNeedsCredentialPrompt } from './ephemeral-vm-runtime-ssh' const TARGET_ID = 'runtime-ssh-orca-1' @@ -142,21 +147,55 @@ describe('reattachRuntimeOwnedSshTarget', () => { expect(mocks.upsertRuntimeOwnedTarget).not.toHaveBeenCalled() }) - it('refuses to dial over a relay that is reconnecting on its own', async () => { + it('resolves without dialing over a relay that is reconnecting on its own', async () => { + // Why resolve rather than throw: activation awaits this, and a relay that is healing + // itself is not a failed wake. Throwing put a red "Failed to wake" toast on a live VM. mocks.getRegisteredSshState.mockReturnValue({ status: 'reconnecting' }) - await expect(reattachRuntimeOwnedSshTarget(runtime)).rejects.toThrow('still reconnecting') + await expect(reattachRuntimeOwnedSshTarget(runtime)).resolves.toBeUndefined() expect(mocks.connectRegisteredSshTarget).not.toHaveBeenCalled() + expect(mocks.upsertRuntimeOwnedTarget).not.toHaveBeenCalled() }) - it('waits for providers instead of redialing a connected transport', async () => { + it('redials a connected transport whose relay never registered its providers', async () => { + // Why: transport `connected` with no PTY provider is the relay-lost grace window or a + // half-attached session. Waiting on providers there burned the timeout and never dialed; + // the registered connect treats a genuinely live relay as a refresh, so dialing is safe. mocks.getRegisteredSshState.mockReturnValue({ status: 'connected' }) - let ptyReady = false - mocks.getSshPtyProvider.mockImplementation(() => (ptyReady ? {} : undefined)) - const pending = reattachRuntimeOwnedSshTarget(runtime) + let dialed = false + mocks.connectRegisteredSshTarget.mockImplementation(async () => { + dialed = true + return { targetId: TARGET_ID, status: 'connected' } + }) + mocks.getSshPtyProvider.mockImplementation(() => (dialed ? {} : undefined)) + + await reattachRuntimeOwnedSshTarget(runtime) + + expect(mocks.connectRegisteredSshTarget).toHaveBeenCalledWith(TARGET_ID) + }) + + it('stops waiting for providers when the caller aborts', async () => { + mocks.getRegisteredSshState.mockReturnValue(undefined) + mocks.getSshPtyProvider.mockReturnValue(undefined) + const abort = new AbortController() + const pending = reattachRuntimeOwnedSshTarget(runtime, abort.signal) + const rejection = expect(pending).rejects.toThrow('aborted') await vi.advanceTimersByTimeAsync(300) - ptyReady = true + abort.abort() await vi.advanceTimersByTimeAsync(200) - await pending + await rejection + }) +}) + +describe('runtimeOwnedSshTargetNeedsCredentialPrompt', () => { + it.each([ + [true, { lastRequiredPassphrase: true }], + [false, { lastRequiredPassphrase: false }], + [false, {}], + [false, undefined] + ])('reads %s from the persisted row %j without dialing', (expected, row) => { + mocks.getTarget.mockReturnValue(row ? { id: TARGET_ID, ...row } : undefined) + expect(runtimeOwnedSshTargetNeedsCredentialPrompt(TARGET_ID)).toBe(expected) + expect(mocks.getTarget).toHaveBeenCalledWith(TARGET_ID) expect(mocks.connectRegisteredSshTarget).not.toHaveBeenCalled() }) }) diff --git a/src/main/ephemeral-vm-runtime-ssh.ts b/src/main/ephemeral-vm-runtime-ssh.ts index ab7e712871d..3427b262936 100644 --- a/src/main/ephemeral-vm-runtime-ssh.ts +++ b/src/main/ephemeral-vm-runtime-ssh.ts @@ -45,7 +45,7 @@ export async function connectRuntimeOwnedSshTarget(args: { } catch (error) { // The target is persisted at upsert, so a failed connect/provider-wait would // orphan it; remove it (idempotent) before rethrowing so cleanup is complete. - await removeRuntimeOwnedSshTarget(target.id).catch(() => undefined) + await removeRegisteredSshTarget(target.id).catch(() => undefined) throw error } return { targetId: target.id, target } @@ -59,25 +59,30 @@ export function getRuntimeOwnedSshRelayState(targetId: string): RuntimeOwnedSshR return status === 'reconnecting' ? 'reconnecting' : 'detached' } +/** + * Whether dialing this target would stop on a credential prompt. Read from the persisted + * row the same way the renderer's startup restore partitions eager vs deferred targets: + * without attempting a connection first. A prompt sent before the renderer has a + * listener would only burn the credential timeout. + */ +export function runtimeOwnedSshTargetNeedsCredentialPrompt(targetId: string): boolean { + return getSshConnectionStore()?.getTarget(targetId)?.lastRequiredPassphrase === true +} + /** * Re-establish the relay for a runtime that is still running. Unlike the provisioning * connect, a failure keeps the target row: the workspace still points at it and the * VM is up, so the next activation or terminal spawn retries instead of orphaning it. + * + * Resolves without dialing when the relay is `reconnecting`: it re-registers its + * providers itself and a second dial would tear the recovering session down. */ export async function reattachRuntimeOwnedSshTarget( - runtime: EphemeralVmRuntimeRecord & { sshTargetId: string } + runtime: EphemeralVmRuntimeRecord & { sshTargetId: string }, + signal?: AbortSignal ): Promise { const relayState = getRuntimeOwnedSshRelayState(runtime.sshTargetId) - if (relayState === 'attached') { - return - } - if (relayState === 'reconnecting') { - throw new Error(`SSH relay for runtime "${runtime.id}" is still reconnecting.`) - } - if (getRegisteredSshState(runtime.sshTargetId)?.status === 'connected') { - // Why not dial: the transport is up and the relay is about to register its providers; - // a second connect would tear that session down for nothing. - await waitForRuntimeSshProviders(runtime.sshTargetId) + if (relayState === 'attached' || relayState === 'reconnecting') { return } const store = getSshConnectionStore() @@ -91,7 +96,11 @@ export async function reattachRuntimeOwnedSshTarget( // Why re-upsert: the target row lives in the profile, the runtime record in its own // file; recreating the row from the recipe result heals a profile that lost it. const target = store.upsertRuntimeOwnedTarget(runtime.id, connection.target) - await connectAndAwaitRuntimeSshProviders(target.id) + // Why always dial, even over a `connected` transport with no providers: the registered + // connect already treats a live relay as a refresh, and a transport whose relay never + // came up (relay-lost grace, half-attached after a crash) is exactly what a redial heals. + // Waiting on it instead would burn the provider timeout and never recover. + await connectAndAwaitRuntimeSshProviders(target.id, signal) } export async function disconnectRuntimeOwnedSshTarget(targetId: string | undefined): Promise { diff --git a/src/main/ipc/pty-startup-barrier-ordering.test.ts b/src/main/ipc/pty-startup-barrier-ordering.test.ts index accdb8cab63..dcf8a5d4c44 100644 --- a/src/main/ipc/pty-startup-barrier-ordering.test.ts +++ b/src/main/ipc/pty-startup-barrier-ordering.test.ts @@ -29,23 +29,4 @@ describe('PTY startup barrier ordering', () => { expect(barrierIndex).toBeLessThan(providerIndex) } }) - - it('gives a missing SSH PTY provider its recovery before every provider resolution', () => { - // Why: a runtime-owned SSH target has no relay after an app restart until its owner - // re-attaches it; each spawn path (renderer, runtime controller, stable-pane adoption) - // must await that recovery before `getProvider` throws on the miss. - for (const relPath of [ - 'src/main/ipc/pty/ipc/spawn-preflight.ts', - 'src/main/ipc/pty/runtime/spawn-preflight.ts', - 'src/main/ipc/pty/pane/adopt-stable.ts' - ]) { - const source = readRepoSource(relPath) - const recoveryIndex = source.indexOf('await providerRecovery') - const providerIndex = source.indexOf('getProvider(args.connectionId)') - expect(source).toContain('recoverMissingSshPtyProvider(args.connectionId)') - expect(recoveryIndex, relPath).toBeGreaterThanOrEqual(0) - expect(providerIndex, relPath).toBeGreaterThanOrEqual(0) - expect(recoveryIndex, relPath).toBeLessThan(providerIndex) - } - }) }) diff --git a/src/main/ipc/pty/ipc/spawn-preflight.ts b/src/main/ipc/pty/ipc/spawn-preflight.ts index 3eb5422eea6..8e32734c39a 100644 --- a/src/main/ipc/pty/ipc/spawn-preflight.ts +++ b/src/main/ipc/pty/ipc/spawn-preflight.ts @@ -24,11 +24,13 @@ export async function preparePtyIpcSpawnPreflight(ctx: PtyIpcSpawnState): Promis const args = ctx.args // Why before the provider lookup: a runtime-owned SSH target has no relay after an app // restart until its owner re-attaches it; the spawn would otherwise fail on the miss. + // Only an SSH spawn can await here — a null connectionId returns undefined — so the + // daemon-identity invariant below still holds for the daemon-host path it governs. const providerRecovery = recoverMissingSshPtyProvider(args.connectionId) if (providerRecovery) { await providerRecovery } - // Establish daemon identity before the first await so hidden delivery is gated before byte zero. + // Establish daemon identity before the first await (of a local spawn) so hidden delivery is gated before byte zero. ctx.provider = getProvider(args.connectionId) ctx.isDaemonHostSpawn = !args.connectionId && diff --git a/src/main/ipc/pty/provider/missing-ssh-pty-provider-recovery-spawn-paths.test.ts b/src/main/ipc/pty/provider/missing-ssh-pty-provider-recovery-spawn-paths.test.ts new file mode 100644 index 00000000000..31f41b1d826 --- /dev/null +++ b/src/main/ipc/pty/provider/missing-ssh-pty-provider-recovery-spawn-paths.test.ts @@ -0,0 +1,158 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +vi.mock('electron', () => ({ + app: { getPath: () => '/tmp/orca-missing-provider-test', isPackaged: false } +})) +vi.mock('node-pty', () => ({ spawn: vi.fn(), default: { spawn: vi.fn() } })) + +import type { IPtyProvider } from '../../../providers/types' +import { setSshProviderMissRecovery } from '../../../providers/ssh-provider-miss-recovery' +import { sshProviders } from './registry' +import { preparePtyIpcSpawnPreflight } from '../ipc/spawn-preflight' +import { createPtyIpcSpawnState } from '../ipc/spawn-state' +import type { PtySpawnIpcArgs, PtySpawnIpcDeps } from '../ipc/spawn-types' +import { prepareRuntimePtySpawn } from '../runtime/spawn-preflight' +import { createRuntimePtySpawnState, type RuntimePtySpawnArgs } from '../runtime/spawn-state' +import type { PtyRuntimeControllerDeps } from '../runtime/controller-deps' +import { adoptStablePane } from '../pane/adopt-stable' +import { noCodexResumeLaunch } from '../host-env/codex-resume' + +const TARGET = 'runtime-ssh-orca-restarted' + +/** + * Behavioural pin for the spawn-time re-attach. Each spawn path is executed against a + * registry that has no provider for the target; the installed recovery registers one when + * it runs. If a path resolves the provider before (or without) awaiting the recovery, it + * throws the provider miss and the test fails. A source-text ordering check could not + * tell a gated recovery (`args.sessionId ? recover : undefined`) from an unconditional one. + */ +function installRegisteringRecovery(): ReturnType { + const provider = { spawn: vi.fn(async () => ({ id: `ssh:${TARGET}@@pty-1` })) } + const recovery = vi.fn((connectionId: string) => { + if (connectionId !== TARGET) { + return undefined + } + return Promise.resolve().then(() => { + sshProviders.set(TARGET, provider as unknown as IPtyProvider) + }) + }) + setSshProviderMissRecovery(recovery) + return recovery +} + +function rendererDeps(): PtySpawnIpcDeps { + return { + getLocalPtyStartupPromise: () => undefined, + adoptStablePane: vi.fn(async () => null), + assertFolderWorkspacePtyPathUsable: () => undefined, + resolvePtySpawnStartupCwd: (_worktreeId, cwd) => cwd, + localStartupCwdDirectoryExists: () => true, + prepareCodexResumeHome: () => null, + noCodexResumeLaunch, + resolveCodexResumeLaunch: async (command) => noCodexResumeLaunch(command), + reconcileSharedRuntimeResumeHome: async () => '', + stripSequencedStartupResumeArgv: (env) => env, + transitionSpawnHiddenRendererPtyDeliveryState: vi.fn() + } as unknown as PtySpawnIpcDeps +} + +function runtimeDeps(): PtyRuntimeControllerDeps { + return { + adoptStablePane: vi.fn(async () => null), + getLocalPtyStartupPromise: () => undefined, + getLocalPtyProviderStartupPromise: () => undefined, + prepareCodexResumeHome: () => null, + resolveCodexResumeLaunch: async (command) => noCodexResumeLaunch(command), + noCodexResumeLaunch, + reconcileSharedRuntimeResumeHome: async () => '', + stripSequencedStartupResumeArgv: (env) => env, + assertFolderWorkspacePtyPathUsable: () => undefined, + resolvePtySpawnStartupCwd: (_worktreeId, cwd) => cwd + } as unknown as PtyRuntimeControllerDeps +} + +beforeEach(() => { + sshProviders.delete(TARGET) +}) + +afterEach(() => { + setSshProviderMissRecovery(null) + sshProviders.delete(TARGET) +}) + +describe('renderer pty:spawn preflight', () => { + it.each([ + ['a fresh terminal (no sessionId)', {}], + ['a reattach (sessionId supplied)', { sessionId: `ssh:${TARGET}@@pty-1` }] + ])('re-attaches the relay before resolving the provider for %s', async (_label, extra) => { + const recovery = installRegisteringRecovery() + const args = { + cols: 80, + rows: 24, + connectionId: TARGET, + cwd: '/w', + ...extra + } as PtySpawnIpcArgs + const ctx = createPtyIpcSpawnState(rendererDeps(), args) + + await preparePtyIpcSpawnPreflight(ctx) + + expect(recovery).toHaveBeenCalledWith(TARGET) + expect(ctx.provider).toBe(sshProviders.get(TARGET)) + }) + + it('still fails on the miss when the recovery declines the connection', async () => { + setSshProviderMissRecovery(() => undefined) + const args = { cols: 80, rows: 24, connectionId: TARGET, cwd: '/w' } as PtySpawnIpcArgs + await expect( + preparePtyIpcSpawnPreflight(createPtyIpcSpawnState(rendererDeps(), args)) + ).rejects.toThrow(/^No PTY provider for connection/) + }) +}) + +describe('runtime controller spawn preflight', () => { + it.each([ + ['a fresh terminal', {}], + ['a caller-supplied session', { sessionId: `ssh:${TARGET}@@pty-1` }] + ])('re-attaches the relay before resolving the provider for %s', async (_label, extra) => { + const recovery = installRegisteringRecovery() + const args = { + cols: 80, + rows: 24, + connectionId: TARGET, + cwd: '/w', + ...extra + } as RuntimePtySpawnArgs + const ctx = createRuntimePtySpawnState(runtimeDeps(), args) + + await prepareRuntimePtySpawn(ctx) + + expect(recovery).toHaveBeenCalledWith(TARGET) + expect(ctx.provider).toBe(sshProviders.get(TARGET)) + }) +}) + +describe('stable-pane adoption', () => { + it.each([ + ['when it owns the pane spawn reservation', { ownsPaneSpawnReservation: true as const }], + ['when it does not own the reservation', {}] + ])('re-attaches the relay before resolving the provider %s', async (_label, extra) => { + const recovery = installRegisteringRecovery() + // No persisted owner → adoption returns null, but only after the recovery ran; without a + // provider the pre-recovery path would have thrown the miss on the way to the lookup. + const adopted = await adoptStablePane(undefined, undefined, { + cols: 80, + rows: 24, + cwd: '/w', + connectionId: TARGET, + worktreeId: 'repo::/w', + tabId: 'tab-1', + leafId: '11111111-1111-4111-8111-111111111111', + ...extra + }) + + expect(recovery).toHaveBeenCalledWith(TARGET) + expect(adopted).toBeNull() + expect(sshProviders.has(TARGET)).toBe(true) + }) +}) diff --git a/src/main/ipc/pty/provider/missing-ssh-pty-provider-recovery.test.ts b/src/main/ipc/pty/provider/missing-ssh-pty-provider-recovery.test.ts index 80dc89570f6..9ed13037d71 100644 --- a/src/main/ipc/pty/provider/missing-ssh-pty-provider-recovery.test.ts +++ b/src/main/ipc/pty/provider/missing-ssh-pty-provider-recovery.test.ts @@ -1,12 +1,10 @@ import { afterEach, describe, expect, it, vi } from 'vitest' -import { - recoverMissingSshPtyProvider, - setMissingSshPtyProviderRecovery -} from './missing-ssh-pty-provider-recovery' +import { setSshProviderMissRecovery } from '../../../providers/ssh-provider-miss-recovery' +import { recoverMissingSshPtyProvider } from './missing-ssh-pty-provider-recovery' import { registerSshPtyProvider, unregisterSshPtyProvider } from './registry' afterEach(() => { - setMissingSshPtyProviderRecovery(null) + setSshProviderMissRecovery(null) unregisterSshPtyProvider('ssh-registered') }) @@ -19,7 +17,7 @@ describe('recoverMissingSshPtyProvider', () => { it('consults the installed recovery only for connections with no registered provider', () => { const recovery = vi.fn(() => Promise.resolve()) - setMissingSshPtyProviderRecovery(recovery) + setSshProviderMissRecovery(recovery) registerSshPtyProvider('ssh-registered', {} as never) expect(recoverMissingSshPtyProvider('ssh-registered')).toBeUndefined() @@ -31,7 +29,7 @@ describe('recoverMissingSshPtyProvider', () => { }) it('lets the recovery decline a connection it does not own', () => { - setMissingSshPtyProviderRecovery(() => undefined) + setSshProviderMissRecovery(() => undefined) expect(recoverMissingSshPtyProvider('ssh-missing')).toBeUndefined() }) }) diff --git a/src/main/ipc/pty/provider/missing-ssh-pty-provider-recovery.ts b/src/main/ipc/pty/provider/missing-ssh-pty-provider-recovery.ts index 5d571dc233d..04a93ef0f5d 100644 --- a/src/main/ipc/pty/provider/missing-ssh-pty-provider-recovery.ts +++ b/src/main/ipc/pty/provider/missing-ssh-pty-provider-recovery.ts @@ -1,25 +1,16 @@ +import { recoverSshProviderMiss } from '../../../providers/ssh-provider-miss-recovery' import { sshProviders } from './registry' -type MissingSshPtyProviderRecovery = (connectionId: string) => Promise | undefined - -let recovery: MissingSshPtyProviderRecovery | null = null - /** - * Installed by the layer that owns a connection's relay health (today: the ephemeral-VM - * runtime for runtime-owned targets). Consulted only when a PTY operation arrives for an - * SSH connection with no registered provider, so the owner can re-attach the relay - * instead of the lookup failing on the miss. Which targets to dial is the owner's policy. + * A promise only when a recovery is installed, the connection has no PTY provider, and the + * owner claims it. Spawn paths await this before `getProvider` so a runtime-owned target + * whose relay is gone (app restart) is re-attached instead of failing on the miss. */ -export function setMissingSshPtyProviderRecovery(next: MissingSshPtyProviderRecovery | null): void { - recovery = next -} - -/** A promise only when a recovery is installed and the connection has no PTY provider. */ export function recoverMissingSshPtyProvider( connectionId: string | null | undefined ): Promise | undefined { - if (!connectionId || !recovery || sshProviders.has(connectionId)) { + if (!connectionId || sshProviders.has(connectionId)) { return undefined } - return recovery(connectionId) + return recoverSshProviderMiss(connectionId) } diff --git a/src/main/ipc/pty/provider/registry.ts b/src/main/ipc/pty/provider/registry.ts index 2a3d3b162fd..9adbc1c3cd1 100644 --- a/src/main/ipc/pty/provider/registry.ts +++ b/src/main/ipc/pty/provider/registry.ts @@ -1,6 +1,11 @@ import { LocalPtyProvider } from '../../../providers/local-pty-provider' import type { IPtyProvider } from '../../../providers/types' import { parseAppSshPtyId, toAppSshPtyId, toRelaySshPtyId } from '../../../providers/ssh-pty-id' +import { isRuntimeOwnedSshTargetId } from '../../../../shared/execution-host' +import { + formatRuntimeOwnedSshRelayNotAttached, + formatSshPtyProviderMissingError +} from '../../../../shared/ssh-pty-provider-missing' import { ptyOwnership } from './ownership-state' // ─── Provider Registry ────────────────────────────────────────────── @@ -30,9 +35,15 @@ export function getProvider(connectionId: string | null | undefined): IPtyProvid if (!provider) { // Why the suffix: this surfaces verbatim in `terminal create` on a reconnecting SSH host; the // bare id told the caller nothing about what to do. Keep the prefix — the renderer matches it. + // Runtime-owned targets are absent from the host list, so they must not be told to use Reconnect. throw new Error( - `No PTY provider for connection "${connectionId}": the SSH relay for this host is not attached ` + - '(reconnecting or disconnected). Wait for the host to reconnect, or use Reconnect on the SSH target.' + isRuntimeOwnedSshTargetId(connectionId) + ? formatRuntimeOwnedSshRelayNotAttached(connectionId) + : formatSshPtyProviderMissingError( + connectionId, + 'the SSH relay for this host is not attached (reconnecting or disconnected). ' + + 'Wait for the host to reconnect, or use Reconnect on the SSH target.' + ) ) } return provider diff --git a/src/main/providers/ssh-filesystem-dispatch.ts b/src/main/providers/ssh-filesystem-dispatch.ts index 04ef209bef1..eb871fab8d9 100644 --- a/src/main/providers/ssh-filesystem-dispatch.ts +++ b/src/main/providers/ssh-filesystem-dispatch.ts @@ -1,4 +1,5 @@ import type { IFilesystemProvider } from './types' +import { scheduleSshProviderMissRecovery } from './ssh-provider-miss-recovery' const sshProviders = new Map() @@ -44,6 +45,8 @@ export function getSshFilesystemProvider(connectionId: string): IFilesystemProvi export function requireSshFilesystemProvider(connectionId: string): IFilesystemProvider { const provider = getSshFilesystemProvider(connectionId) if (!provider) { + // Why: same as the git dispatcher — a runtime-owned relay re-attaches in the background. + scheduleSshProviderMissRecovery(connectionId) throw new Error(SSH_FILESYSTEM_PROVIDER_UNAVAILABLE_MESSAGE) } return provider diff --git a/src/main/providers/ssh-git-dispatch.ts b/src/main/providers/ssh-git-dispatch.ts index c0acc757ada..ce07f3b8530 100644 --- a/src/main/providers/ssh-git-dispatch.ts +++ b/src/main/providers/ssh-git-dispatch.ts @@ -1,4 +1,5 @@ import type { SshGitProvider } from './ssh-git-provider' +import { scheduleSshProviderMissRecovery } from './ssh-provider-miss-recovery' const sshProviders = new Map() const sshProviderGenerations = new Map() @@ -28,6 +29,9 @@ export function getSshGitProvider(connectionId: string): SshGitProvider | undefi export function requireSshGitProvider(connectionId: string): SshGitProvider { const provider = getSshGitProvider(connectionId) if (!provider) { + // Why: a runtime-owned relay has no host-list Reconnect; its owner re-attaches it in the + // background so the caller's next retry finds the provider. This call still fails. + scheduleSshProviderMissRecovery(connectionId) throw new Error(SSH_GIT_PROVIDER_UNAVAILABLE_MESSAGE) } return provider diff --git a/src/main/providers/ssh-provider-miss-recovery.test.ts b/src/main/providers/ssh-provider-miss-recovery.test.ts new file mode 100644 index 00000000000..d43d85e32be --- /dev/null +++ b/src/main/providers/ssh-provider-miss-recovery.test.ts @@ -0,0 +1,88 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { + SSH_FILESYSTEM_PROVIDER_UNAVAILABLE_MESSAGE, + requireSshFilesystemProvider +} from './ssh-filesystem-dispatch' +import { SSH_GIT_PROVIDER_UNAVAILABLE_MESSAGE, requireSshGitProvider } from './ssh-git-dispatch' +import { + recoverSshProviderMiss, + scheduleSshProviderMissRecovery, + setSshProviderMissRecovery +} from './ssh-provider-miss-recovery' + +const TARGET = 'runtime-ssh-orca-1' + +beforeEach(() => { + vi.useFakeTimers() +}) + +afterEach(() => { + setSshProviderMissRecovery(null) + vi.useRealTimers() + vi.restoreAllMocks() +}) + +describe('requireSshGitProvider / requireSshFilesystemProvider on a miss', () => { + // Why these two sites: they are where the reporter's second string ("Remote connection + // dropped…") comes from, reached by every git/file operation after an app restart. The + // call still fails — callers poll or retry — but the owner is asked to re-attach so the + // next attempt finds the provider, exactly as the PTY spawn path does synchronously. + it.each([ + ['git', () => requireSshGitProvider(TARGET), SSH_GIT_PROVIDER_UNAVAILABLE_MESSAGE], + [ + 'filesystem', + () => requireSshFilesystemProvider(TARGET), + SSH_FILESYSTEM_PROVIDER_UNAVAILABLE_MESSAGE + ] + ])('asks the installed recovery to re-attach on a %s provider miss', (_kind, call, message) => { + const recovery = vi.fn(() => Promise.resolve()) + setSshProviderMissRecovery(recovery) + + expect(call).toThrow(message) + + expect(recovery).toHaveBeenCalledWith(TARGET) + }) + + it('throws the unchanged message when no recovery is installed', () => { + expect(() => requireSshGitProvider(TARGET)).toThrow(SSH_GIT_PROVIDER_UNAVAILABLE_MESSAGE) + expect(() => requireSshFilesystemProvider(TARGET)).toThrow( + SSH_FILESYSTEM_PROVIDER_UNAVAILABLE_MESSAGE + ) + }) +}) + +describe('scheduleSshProviderMissRecovery', () => { + it('throttles repeated misses for one connection while a re-attach is recent', () => { + // Why: git status and file watches poll; without this a failing relay would be dialed + // once per poll. Distinct connections are throttled independently. + const recovery = vi.fn(() => Promise.resolve()) + setSshProviderMissRecovery(recovery) + + scheduleSshProviderMissRecovery(TARGET) + scheduleSshProviderMissRecovery(TARGET) + scheduleSshProviderMissRecovery('runtime-ssh-orca-2') + expect(recovery).toHaveBeenCalledTimes(2) + + vi.advanceTimersByTime(5_000) + scheduleSshProviderMissRecovery(TARGET) + expect(recovery).toHaveBeenCalledTimes(3) + }) + + it('logs, rather than surfaces, a failed background re-attach', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + setSshProviderMissRecovery(() => Promise.reject(new Error('connect ECONNREFUSED'))) + + scheduleSshProviderMissRecovery(TARGET) + await vi.runAllTimersAsync() + + expect(warn).toHaveBeenCalledWith(expect.stringContaining('ECONNREFUSED')) + }) + + it('lets a declined connection fall through without scheduling anything', () => { + const recovery = vi.fn(() => undefined) + setSshProviderMissRecovery(recovery) + scheduleSshProviderMissRecovery('ssh-user-target') + expect(recovery).toHaveBeenCalledWith('ssh-user-target') + expect(recoverSshProviderMiss('ssh-user-target')).toBeUndefined() + }) +}) diff --git a/src/main/providers/ssh-provider-miss-recovery.ts b/src/main/providers/ssh-provider-miss-recovery.ts new file mode 100644 index 00000000000..9453c6e71c2 --- /dev/null +++ b/src/main/providers/ssh-provider-miss-recovery.ts @@ -0,0 +1,53 @@ +/** + * Installed by the layer that owns a connection's relay health (today: the ephemeral-VM + * runtime for runtime-owned targets). Consulted when an SSH operation arrives for a + * connection with no registered provider, so the owner can re-attach the relay instead of + * the lookup failing on the miss. Which targets to dial is the owner's policy; a recovery + * returns undefined for connections it does not own. + * + * Kept dependency-free so the git and filesystem dispatchers can consult it without + * pulling the PTY registry into their module graph. + */ +type SshProviderMissRecovery = (connectionId: string) => Promise | undefined + +let recovery: SshProviderMissRecovery | null = null + +// Why throttled: git status and file watches poll; a miss per poll must not fan out into a +// re-attach per poll while one is already failing. +const BACKGROUND_RECOVERY_THROTTLE_MS = 5_000 +const backgroundRecoveryStartedAt = new Map() + +export function setSshProviderMissRecovery(next: SshProviderMissRecovery | null): void { + recovery = next + backgroundRecoveryStartedAt.clear() +} + +/** A promise only when a recovery is installed and claims the connection. */ +export function recoverSshProviderMiss(connectionId: string): Promise | undefined { + return recovery?.(connectionId) +} + +/** + * Fire-and-forget re-attach for synchronous miss sites (git/filesystem `require*`). The + * current call still fails; the caller's next poll or retry finds the provider registered. + */ +export function scheduleSshProviderMissRecovery(connectionId: string): void { + if (!recovery) { + return + } + const now = Date.now() + const startedAt = backgroundRecoveryStartedAt.get(connectionId) + if (startedAt !== undefined && now - startedAt < BACKGROUND_RECOVERY_THROTTLE_MS) { + return + } + backgroundRecoveryStartedAt.set(connectionId, now) + const pending = recovery(connectionId) + if (!pending) { + return + } + pending.catch((error: unknown) => { + console.warn( + `[ssh] Background provider re-attach failed for ${connectionId}: ${error instanceof Error ? error.message : String(error)}` + ) + }) +} diff --git a/src/main/window/attach-main-window-services-runtime-ssh-reattach.test.ts b/src/main/window/attach-main-window-services-runtime-ssh-reattach.test.ts new file mode 100644 index 00000000000..193d3f1988f --- /dev/null +++ b/src/main/window/attach-main-window-services-runtime-ssh-reattach.test.ts @@ -0,0 +1,146 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { Store } from '../persistence' + +const { + appGetPathMock, + registerSshHandlersMock, + installRuntimeOwnedSshProviderMissRecoveryMock, + reattachRuntimeOwnedSshTargetsAtStartupMock +} = vi.hoisted(() => ({ + appGetPathMock: vi.fn(), + registerSshHandlersMock: vi.fn(), + installRuntimeOwnedSshProviderMissRecoveryMock: vi.fn(), + reattachRuntimeOwnedSshTargetsAtStartupMock: vi.fn() +})) + +vi.mock('electron', () => ({ + app: { getPath: appGetPathMock }, + clipboard: {}, + systemPreferences: { + askForMediaAccess: vi.fn(async () => true), + getMediaAccessStatus: vi.fn(() => 'granted') + }, + ipcMain: { + on: vi.fn(), + removeAllListeners: vi.fn(), + removeListener: vi.fn(), + removeHandler: vi.fn(), + handle: vi.fn() + }, + powerMonitor: { on: vi.fn(), off: vi.fn() } +})) +vi.mock('../ipc/repos', () => ({ registerRepoHandlers: vi.fn() })) +vi.mock('../ipc/repos/repos-changed-notification', () => ({ + setRepoRemoteClientNotifier: vi.fn() +})) +vi.mock('../ipc/watched-worktree-catalog-notification', () => ({ + setWorktreeCatalogRemoteClientNotifier: vi.fn() +})) +vi.mock('../ipc/worktrees', () => ({ registerWorktreeHandlers: vi.fn() })) +vi.mock('../ipc/worktree-change-invalidators', () => ({ + runWorktreeChangeInvalidators: vi.fn() +})) +vi.mock('../ipc/pty', () => ({ getLocalPtyProvider: vi.fn(), registerPtyHandlers: vi.fn() })) +vi.mock('../memory/hydrate-local-pty-registry', () => ({ + hydrateLocalPtyRegistryAtBoot: vi.fn() +})) +vi.mock('../ipc/ssh', () => ({ registerSshHandlers: registerSshHandlersMock })) +vi.mock('../ephemeral-vm-runtime-ssh-reattach', () => ({ + installRuntimeOwnedSshProviderMissRecovery: installRuntimeOwnedSshProviderMissRecoveryMock, + reattachRuntimeOwnedSshTargetsAtStartup: reattachRuntimeOwnedSshTargetsAtStartupMock +})) +vi.mock('../ipc/worktree-base-directory-watcher', () => ({ + setWorktreeBaseDirectoryWatcherSyncContext: vi.fn(), + scheduleWorktreeBaseDirectoryWatcherSync: vi.fn() +})) +vi.mock('../browser/browser-manager', () => ({ browserManager: { unregisterAll: vi.fn() } })) +vi.mock('../updater', () => ({ + checkForUpdates: vi.fn(), + getUpdateStatus: vi.fn(), + quitAndInstall: vi.fn(), + dismissNudge: vi.fn(), + setupAutoUpdater: vi.fn() +})) +vi.mock('../macos-tcc-prompt-notice', () => ({ + acknowledgePendingTccPromptNotice: vi.fn(), + consumePendingTccPromptNotice: vi.fn(), + dismissTccPromptNotice: vi.fn(), + releasePendingTccPromptNotice: vi.fn() +})) + +import { attachMainWindowServices } from './attach-main-window-services' + +function createMainWindow(): unknown { + return { + id: 1, + isDestroyed: vi.fn(() => false), + on: vi.fn(), + once: vi.fn(), + webContents: { + id: 1, + getURL: vi.fn(() => 'file:///opt/orca/renderer/index.html'), + isDestroyed: vi.fn(() => false), + isLoadingMainFrame: vi.fn(() => true), + on: vi.fn(), + reload: vi.fn(), + session: { setPermissionRequestHandler: vi.fn(), setPermissionCheckHandler: vi.fn() } + } + } +} + +function createStore(): Store { + return { + getProfileStorageDirectory: vi.fn(() => '/profile-a'), + flushPendingAsync: vi.fn(() => Promise.resolve()) + } as unknown as Store +} + +function createRuntime(): unknown { + return { + attachWindow: vi.fn(), + setNotifier: vi.fn(), + markRendererReloading: vi.fn(), + markRendererReloadCancelled: vi.fn(), + markGraphReloadFailed: vi.fn(), + markGraphUnavailable: vi.fn() + } +} + +describe('attachMainWindowServices: runtime-owned SSH relay re-attach', () => { + beforeEach(() => { + vi.resetAllMocks() + appGetPathMock.mockReturnValue('/user-data') + reattachRuntimeOwnedSshTargetsAtStartupMock.mockResolvedValue(undefined) + }) + + it('installs the miss recovery and runs the startup pass after the SSH handlers can dial', () => { + // Why: runtime-owned targets are skipped by the renderer's startup restore, the pane + // connect gate, and the host list, so this wiring is the only thing that re-attaches + // them after an app restart (#19173). Both calls must happen, in this order, after + // `registerSshHandlers` has installed the connect they dial through. + const order: string[] = [] + registerSshHandlersMock.mockImplementation(() => { + order.push('registerSshHandlers') + }) + installRuntimeOwnedSshProviderMissRecoveryMock.mockImplementation(() => { + order.push('installRecovery') + }) + reattachRuntimeOwnedSshTargetsAtStartupMock.mockImplementation(async () => { + order.push('reattachAtStartup') + }) + + attachMainWindowServices(createMainWindow() as never, createStore(), createRuntime() as never) + + expect(order).toEqual(['registerSshHandlers', 'installRecovery', 'reattachAtStartup']) + }) + + it('hands both the live userData path resolver, not a snapshot', () => { + attachMainWindowServices(createMainWindow() as never, createStore(), createRuntime() as never) + + const [installGetUserDataPath] = installRuntimeOwnedSshProviderMissRecoveryMock.mock.calls[0] + const [reattachGetUserDataPath] = reattachRuntimeOwnedSshTargetsAtStartupMock.mock.calls[0] + expect(installGetUserDataPath()).toBe('/user-data') + expect(reattachGetUserDataPath()).toBe('/user-data') + expect(appGetPathMock).toHaveBeenCalledWith('userData') + }) +}) diff --git a/src/main/window/attach-main-window-services.ts b/src/main/window/attach-main-window-services.ts index 4ad569b6866..61fe998174c 100644 --- a/src/main/window/attach-main-window-services.ts +++ b/src/main/window/attach-main-window-services.ts @@ -21,7 +21,7 @@ import { import { registerDaemonManagementHandlers } from '../ipc/pty-management' import { registerSshHandlers } from '../ipc/ssh' import { - installRuntimeOwnedSshPtyProviderRecovery, + installRuntimeOwnedSshProviderMissRecovery, reattachRuntimeOwnedSshTargetsAtStartup } from '../ephemeral-vm-runtime-ssh-reattach' import { registerRemoteWorkspaceHandlers } from '../ipc/remote-workspace' @@ -126,7 +126,7 @@ export function attachMainWindowServices( // Why after registerSshHandlers: both dial through the registered SSH connect. Runtime-owned // targets are skipped by the renderer's startup restore, so main owns their re-attach. const getUserDataPath = (): string => app.getPath('userData') - installRuntimeOwnedSshPtyProviderRecovery(getUserDataPath) + installRuntimeOwnedSshProviderMissRecovery(getUserDataPath) void reattachRuntimeOwnedSshTargetsAtStartup(getUserDataPath) registerRemoteWorkspaceHandlers(store, () => mainWindow) registerFileDropRelay(mainWindow) diff --git a/src/renderer/src/components/terminal-pane/ipc-pty-connect.ts b/src/renderer/src/components/terminal-pane/ipc-pty-connect.ts index 096c25b4894..42b4fb78c0a 100644 --- a/src/renderer/src/components/terminal-pane/ipc-pty-connect.ts +++ b/src/renderer/src/components/terminal-pane/ipc-pty-connect.ts @@ -1,4 +1,9 @@ import { isRuntimeOwnedSshTargetId } from '../../../../shared/execution-host' +import { + SSH_PTY_PROVIDER_MISSING_PREFIX, + parseRuntimeOwnedSshRelayMiss +} from '../../../../shared/ssh-pty-provider-missing' +import { translate } from '@/i18n/i18n' import { extractIpcErrorMessage } from '@/lib/ipc-error' import { ensurePtyDispatcher } from './pty-dispatcher' import { @@ -18,6 +23,34 @@ import type { IpcPtyTransportOptions, PtyConnectResult, PtyTransport } from './p const SSH_PTY_CONNECTION_MISMATCH_MARKER = 'belongs to SSH connection' +function describeRuntimeOwnedSshRelayMiss(message: string): string { + const miss = parseRuntimeOwnedSshRelayMiss(message) + const retry = translate( + 'auto.components.terminalPane.ipcPtyConnect.runtimeOwnedSshRelayRetry', + 'Open the workspace again or start a new terminal to retry.' + ) + switch (miss.kind) { + case 'not-attached': + return translate( + 'auto.components.terminalPane.ipcPtyConnect.runtimeOwnedSshRelayMiss', + 'The SSH relay for this workspace is not attached. {{retry}}', + { retry } + ) + case 'reattach-failed': + return translate( + 'auto.components.terminalPane.ipcPtyConnect.runtimeOwnedSshRelayReattachFailed', + 'Could not re-attach the SSH relay for this workspace: {{cause}}. {{retry}}', + { cause: miss.cause, retry } + ) + case 'other': + return translate( + 'auto.components.terminalPane.ipcPtyConnect.runtimeOwnedSshRelayMissWithDetail', + 'The SSH relay for this workspace is not attached: {{detail}}. {{retry}}', + { detail: miss.detail, retry } + ) + } +} + type PtyConnectOptions = Parameters[0] type IpcPtyConnectContext = { @@ -197,15 +230,16 @@ function handleConnectError( // to the remount-and-reattach recovery instead of a fresh shell. return undefined } - if (connectionId && message.includes('No PTY provider for connection')) { - // Why runtime-owned targets get the raw message: main re-attaches their relay on spawn, - // so a provider miss here means that re-attach failed and its message names the retry. - // The Settings/reconnect-dialog wording applies only to hosts users can reconnect. + if (connectionId && message.includes(SSH_PTY_PROVIDER_MISSING_PREFIX)) { + // Why runtime-owned targets get their own copy: main re-attaches their relay on spawn, so a + // miss here is a failed re-attach. They have no reconnect dialog or Settings entry, so the + // canned line below would name a control that does not exist; the cause main reported is + // kept and the retry the user actually has is named (translated, without the internal id). context .getCallbacks() .onError?.( isRuntimeOwnedSshTargetId(connectionId) - ? message + ? describeRuntimeOwnedSshRelayMiss(message) : 'SSH connection is not active. Use the reconnect dialog or Settings to connect.' ) } else { diff --git a/src/renderer/src/components/terminal-pane/pty-transport-spawn-errors.test.ts b/src/renderer/src/components/terminal-pane/pty-transport-spawn-errors.test.ts index 7c468cd3166..96a231e61d6 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport-spawn-errors.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport-spawn-errors.test.ts @@ -1,4 +1,8 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { + formatRuntimeOwnedSshRelayNotAttached, + formatRuntimeOwnedSshRelayReattachFailed +} from '../../../../shared/ssh-pty-provider-missing' import { createTerminalSessionStateSaveFailureMessage } from '../../../../shared/terminal-session-state-save-failure' import { installIpcPtyWindow, restorePtySpecWindow } from './pty-transport-test-harness' @@ -119,41 +123,65 @@ describe('createIpcPtyTransport', () => { ) }) - it('surfaces the provider miss verbatim for a runtime-owned (per-workspace-env) target', async () => { - // Why: main re-attaches a runtime-owned relay on spawn, so a provider miss that still - // reaches the pane is a failed re-attach. Runtime-owned targets have no reconnect - // dialog or Settings entry, so the canned "use Settings" line would name a control that - // does not exist; the raw message carries the retry instead. - const { createIpcPtyTransport } = await import('./pty-transport') - const spawnMock = vi - .fn() - .mockRejectedValue(new Error('No PTY provider for connection runtime-ssh-orca-1')) - ;(globalThis as { window: typeof window }).window = { - ...originalWindow, - api: { - ...originalWindow?.api, - pty: { - ...originalWindow?.api?.pty, - spawn: spawnMock, - write: vi.fn(), - resize: vi.fn(), - kill: vi.fn(), - onData: vi.fn(() => () => {}), - onReplay: vi.fn(() => () => {}), - onExit: vi.fn(() => () => {}) + // Why these inputs come from the shared formatters: they are what main's registry and + // spawn-time re-attach actually throw (Electron wraps them in the invoke prefix); a + // hand-written literal would drift from that shape and keep passing. + it.each([ + [ + 'the relay was never re-attached', + formatRuntimeOwnedSshRelayNotAttached('runtime-ssh-orca-1'), + 'The SSH relay for this workspace is not attached. ' + + 'Open the workspace again or start a new terminal to retry.' + ], + [ + 'the spawn-time re-attach failed', + formatRuntimeOwnedSshRelayReattachFailed( + 'runtime-ssh-orca-1', + 'connect ECONNREFUSED 127.0.0.1:51816' + ), + 'Could not re-attach the SSH relay for this workspace: connect ECONNREFUSED 127.0.0.1:51816. ' + + 'Open the workspace again or start a new terminal to retry.' + ] + ])( + 'tells a runtime-owned (per-workspace-env) pane its real retry when %s', + async (_label, mainMessage, expected) => { + // Why: runtime-owned targets have no reconnect dialog, Settings entry, or host-list + // Reconnect — main excludes them from listTargets — so the canned "use Settings" line + // would name a control that does not exist. The pane gets the cause main reported plus + // the retry the user actually has, without the internal target id. + const { createIpcPtyTransport } = await import('./pty-transport') + const spawnMock = vi + .fn() + .mockRejectedValue(new Error(`Error invoking remote method 'pty:spawn': ${mainMessage}`)) + ;(globalThis as { window: typeof window }).window = { + ...originalWindow, + api: { + ...originalWindow?.api, + pty: { + ...originalWindow?.api?.pty, + spawn: spawnMock, + write: vi.fn(), + resize: vi.fn(), + kill: vi.fn(), + onData: vi.fn(() => () => {}), + onReplay: vi.fn(() => () => {}), + onExit: vi.fn(() => () => {}) + } } - } - } as unknown as typeof window + } as unknown as typeof window - const onError = vi.fn() - await createIpcPtyTransport({ connectionId: 'runtime-ssh-orca-1' }).connect({ - url: '', - callbacks: { onError } - }) + const onError = vi.fn() + await createIpcPtyTransport({ connectionId: 'runtime-ssh-orca-1' }).connect({ + url: '', + callbacks: { onError } + }) - expect(onError).toHaveBeenCalledWith('No PTY provider for connection runtime-ssh-orca-1') - expect(onError).not.toHaveBeenCalledWith(expect.stringContaining('Settings')) - }) + expect(onError).toHaveBeenCalledTimes(1) + expect(onError).toHaveBeenCalledWith(expected) + expect(onError).not.toHaveBeenCalledWith(expect.stringContaining('runtime-ssh-orca-1')) + expect(onError).not.toHaveBeenCalledWith(expect.stringMatching(/Settings|Reconnect on/)) + } + ) it('refuses to call a cross-connection SSH reattach expired, and still raises no error toast', async () => { // Retargeted from "…as expired instead of a red error toast" (#7661), which pinned the bug: diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 27595be19cf..d6a65798979 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -16714,6 +16714,12 @@ "terminalPane": { "useManualTerminalWorktreeParking": { "cannotPark": "These terminals cannot be parked safely." + }, + "ipcPtyConnect": { + "runtimeOwnedSshRelayRetry": "Open the workspace again or start a new terminal to retry.", + "runtimeOwnedSshRelayMiss": "The SSH relay for this workspace is not attached. {{retry}}", + "runtimeOwnedSshRelayReattachFailed": "Could not re-attach the SSH relay for this workspace: {{cause}}. {{retry}}", + "runtimeOwnedSshRelayMissWithDetail": "The SSH relay for this workspace is not attached: {{detail}}. {{retry}}" } }, "WorktreeBaseFallbackDialog": { diff --git a/src/shared/ssh-pty-provider-missing.test.ts b/src/shared/ssh-pty-provider-missing.test.ts new file mode 100644 index 00000000000..3d5bac5c340 --- /dev/null +++ b/src/shared/ssh-pty-provider-missing.test.ts @@ -0,0 +1,64 @@ +import { describe, expect, it } from 'vitest' +import { + formatRuntimeOwnedSshRelayNotAttached, + formatRuntimeOwnedSshRelayReattachFailed, + formatSshPtyProviderMissingError, + parseRuntimeOwnedSshRelayMiss +} from './ssh-pty-provider-missing' + +const ID = 'runtime-ssh-orca-3bc4d819' + +describe('runtime-owned SSH provider-miss messages', () => { + it('never tells a runtime-owned target to use a host-list Reconnect it does not have', () => { + // Why: `listTargets()` excludes runtime-owned rows, so "use Reconnect on the SSH target" + // names a control that does not exist for them — the reporter's symptom (d). + for (const message of [ + formatRuntimeOwnedSshRelayNotAttached(ID), + formatRuntimeOwnedSshRelayReattachFailed(ID, 'connect ECONNREFUSED 127.0.0.1:51816') + ]) { + expect(message).toMatch(/^No PTY provider for connection "runtime-ssh-orca-3bc4d819": /) + expect(message).not.toMatch(/Reconnect/) + expect(message).toMatch(/Open the workspace again or start a new terminal to retry\.$/) + } + }) + + it('separates the cause from the retry hint with a sentence break', () => { + // Why: the author's own proof transcript showed "…51816 Open the workspace…" run together. + expect( + formatRuntimeOwnedSshRelayReattachFailed(ID, 'connect ECONNREFUSED 127.0.0.1:51816') + ).toBe( + 'No PTY provider for connection "runtime-ssh-orca-3bc4d819": the SSH relay for this workspace ' + + 'could not be re-attached: connect ECONNREFUSED 127.0.0.1:51816. ' + + 'Open the workspace again or start a new terminal to retry.' + ) + expect(formatRuntimeOwnedSshRelayReattachFailed(ID, 'timed out.')).toContain( + 're-attached: timed out. Open the workspace' + ) + }) + + it.each([ + ['not-attached', formatRuntimeOwnedSshRelayNotAttached(ID), { kind: 'not-attached' }], + [ + 'reattach-failed', + formatRuntimeOwnedSshRelayReattachFailed(ID, 'connect ECONNREFUSED 127.0.0.1:51816'), + { kind: 'reattach-failed', cause: 'connect ECONNREFUSED 127.0.0.1:51816' } + ], + [ + 'reattach-failed wrapped by Electron invoke', + `Error invoking remote method 'pty:spawn': ${formatRuntimeOwnedSshRelayReattachFailed(ID, 'SSH relay for runtime "orca-1" did not attach within 15s.')}`, + { kind: 'reattach-failed', cause: 'SSH relay for runtime "orca-1" did not attach within 15s' } + ], + [ + 'an unrecognised detail', + formatSshPtyProviderMissingError(ID, 'something else entirely.'), + { kind: 'other', detail: 'something else entirely' } + ], + [ + 'a bare prefix with no detail', + `No PTY provider for connection "${ID}"`, + { kind: 'not-attached' } + ] + ])('parses %s back out for the renderer', (_label, message, expected) => { + expect(parseRuntimeOwnedSshRelayMiss(message)).toEqual(expected) + }) +}) diff --git a/src/shared/ssh-pty-provider-missing.ts b/src/shared/ssh-pty-provider-missing.ts new file mode 100644 index 00000000000..bd0caa6f300 --- /dev/null +++ b/src/shared/ssh-pty-provider-missing.ts @@ -0,0 +1,72 @@ +/** + * The wire shape of a PTY-provider miss. Main formats it, the renderer matches the prefix + * and, for runtime-owned targets, re-renders the detail with translated guidance instead of + * surfacing the internal target id. `orca terminal create` and paired clients see main's + * English text as-is, so it must stand on its own. + */ +export const SSH_PTY_PROVIDER_MISSING_PREFIX = 'No PTY provider for connection' + +/** Runtime-owned targets have no host-list Reconnect; these are the retries the user has. */ +export const RUNTIME_OWNED_SSH_RELAY_RETRY_HINT = + 'Open the workspace again or start a new terminal to retry.' +export const RUNTIME_OWNED_SSH_RELAY_NOT_ATTACHED = + 'the SSH relay for this workspace is not attached' +export const RUNTIME_OWNED_SSH_RELAY_REATTACH_FAILED_PREFIX = + 'the SSH relay for this workspace could not be re-attached: ' + +export function formatSshPtyProviderMissingError(connectionId: string, detail: string): string { + return `${SSH_PTY_PROVIDER_MISSING_PREFIX} "${connectionId}": ${detail}` +} + +/** The miss main raises when a runtime-owned target's relay is absent and nothing re-attached it. */ +export function formatRuntimeOwnedSshRelayNotAttached(connectionId: string): string { + return formatSshPtyProviderMissingError( + connectionId, + `${RUNTIME_OWNED_SSH_RELAY_NOT_ATTACHED}. ${RUNTIME_OWNED_SSH_RELAY_RETRY_HINT}` + ) +} + +/** The miss main raises when the spawn-time re-attach itself failed; `cause` is the dial error. */ +export function formatRuntimeOwnedSshRelayReattachFailed( + connectionId: string, + cause: string +): string { + const trimmedCause = cause.trim().replace(/\.\s*$/, '') + return formatSshPtyProviderMissingError( + connectionId, + `${RUNTIME_OWNED_SSH_RELAY_REATTACH_FAILED_PREFIX}${trimmedCause}. ${RUNTIME_OWNED_SSH_RELAY_RETRY_HINT}` + ) +} + +export type RuntimeOwnedSshRelayMiss = + | { kind: 'not-attached' } + | { kind: 'reattach-failed'; cause: string } + | { kind: 'other'; detail: string } + +/** + * Classifies a provider-miss message for a runtime-owned target so the renderer can render + * translated copy without the internal id. Anything it does not recognise is passed through + * as `other` with the detail main gave. + */ +export function parseRuntimeOwnedSshRelayMiss(message: string): RuntimeOwnedSshRelayMiss { + const start = message.indexOf(SSH_PTY_PROVIDER_MISSING_PREFIX) + const afterPrefix = + start === -1 ? message : message.slice(start + SSH_PTY_PROVIDER_MISSING_PREFIX.length) + const detailStart = afterPrefix.indexOf('": ') + // Why: a bare `No PTY provider for connection ""` (older main, no detail) is a plain miss. + const rawDetail = detailStart === -1 ? '' : afterPrefix.slice(detailStart + 3) + const detail = rawDetail + .replace(RUNTIME_OWNED_SSH_RELAY_RETRY_HINT, '') + .trim() + .replace(/\.\s*$/, '') + if (detail === RUNTIME_OWNED_SSH_RELAY_NOT_ATTACHED || detail === '') { + return { kind: 'not-attached' } + } + if (detail.startsWith(RUNTIME_OWNED_SSH_RELAY_REATTACH_FAILED_PREFIX)) { + return { + kind: 'reattach-failed', + cause: detail.slice(RUNTIME_OWNED_SSH_RELAY_REATTACH_FAILED_PREFIX.length) + } + } + return { kind: 'other', detail } +} From d680f9eb10576ae5edf254dd8e602f9521ec6402 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 8 Sep 2026 00:58:40 -0700 Subject: [PATCH 3/4] fix(ssh): require every relay provider before classifying a relay attached getRuntimeOwnedSshRelayState checked only the PTY provider, so a relay with PTY registered but git or filesystem missing reported 'attached'. The owner then skipped the re-attach and nothing else redialed, stranding the half- attached relay: every git or file operation kept failing on the absent provider, which is the miss the recovery hook was added to repair. The connect wait already required all three, so the codebase carried two definitions of 'the relay's providers are ready'. Extract the one predicate (areSshRelayProvidersRegistered) and consume it from both sites, rather than widening the check at each site independently. Requiring all three also matches what a successful connect already guarantees. --- src/main/ephemeral-vm-runtime-ssh.test.ts | 32 +++++++++++++++++++ src/main/ephemeral-vm-runtime-ssh.ts | 16 +++------- .../providers/ssh-relay-provider-readiness.ts | 21 ++++++++++++ 3 files changed, 57 insertions(+), 12 deletions(-) create mode 100644 src/main/providers/ssh-relay-provider-readiness.ts diff --git a/src/main/ephemeral-vm-runtime-ssh.test.ts b/src/main/ephemeral-vm-runtime-ssh.test.ts index 2ade6183ac6..7a04c401dee 100644 --- a/src/main/ephemeral-vm-runtime-ssh.test.ts +++ b/src/main/ephemeral-vm-runtime-ssh.test.ts @@ -119,6 +119,21 @@ describe('getRuntimeOwnedSshRelayState', () => { }) }) +describe('getRuntimeOwnedSshRelayState: half-attached relays', () => { + // Why every provider counts: the relay serves PTY, git, and filesystem from one session, + // so a partial set is half-attached. Reporting it attached is what strands it — the owner + // skips the re-attach and nothing else redials, so the absent provider never comes back. + it.each([ + ['git', () => mocks.getSshGitProvider.mockReturnValue(undefined)], + ['filesystem', () => mocks.getSshFilesystemProvider.mockReturnValue(undefined)], + ['PTY', () => mocks.getSshPtyProvider.mockReturnValue(undefined)] + ])('is detached, not attached, when only the %s provider is missing', (_label, absent) => { + mocks.getRegisteredSshState.mockReturnValue({ status: 'connected' }) + absent() + expect(getRuntimeOwnedSshRelayState(TARGET_ID)).toBe('detached') + }) +}) + describe('reattachRuntimeOwnedSshTarget', () => { it('re-upserts the target row from the recipe result and dials it when detached', async () => { mocks.getRegisteredSshState.mockReturnValue(undefined) @@ -173,6 +188,23 @@ describe('reattachRuntimeOwnedSshTarget', () => { expect(mocks.connectRegisteredSshTarget).toHaveBeenCalledWith(TARGET_ID) }) + it('repairs a half-attached relay whose PTY provider is present but git is missing', async () => { + // The reported shape: PTY present made the relay look attached, so the re-attach was + // skipped and every git operation kept failing on the missing provider. + mocks.getRegisteredSshState.mockReturnValue({ status: 'connected' }) + mocks.getSshPtyProvider.mockReturnValue({}) + let dialed = false + mocks.getSshGitProvider.mockImplementation(() => (dialed ? {} : undefined)) + mocks.connectRegisteredSshTarget.mockImplementation(async () => { + dialed = true + return { targetId: TARGET_ID, status: 'connected' } + }) + + await reattachRuntimeOwnedSshTarget(runtime) + + expect(mocks.connectRegisteredSshTarget).toHaveBeenCalledWith(TARGET_ID) + }) + it('stops waiting for providers when the caller aborts', async () => { mocks.getRegisteredSshState.mockReturnValue(undefined) mocks.getSshPtyProvider.mockReturnValue(undefined) diff --git a/src/main/ephemeral-vm-runtime-ssh.ts b/src/main/ephemeral-vm-runtime-ssh.ts index 3427b262936..7f80b383e0d 100644 --- a/src/main/ephemeral-vm-runtime-ssh.ts +++ b/src/main/ephemeral-vm-runtime-ssh.ts @@ -1,6 +1,4 @@ -import { getSshFilesystemProvider } from './providers/ssh-filesystem-dispatch' -import { getSshGitProvider } from './providers/ssh-git-dispatch' -import { getSshPtyProvider } from './ipc/pty/provider/registry' +import { areSshRelayProvidersRegistered } from './providers/ssh-relay-provider-readiness' import { connectRegisteredSshTarget, getSshConnectionStore } from './ipc/ssh' import { disconnectRegisteredSshTarget, @@ -23,7 +21,7 @@ export type RuntimeOwnedSshConnectionResult = { } /** - * `attached`: connected with the PTY provider registered. `reconnecting`: the relay is + * `attached`: connected with every relay provider registered. `reconnecting`: the relay is * recovering on its own and a fresh dial would tear that down. `detached`: nothing in * this process serves the target — the state after an app restart, or the moment * between a connect and the relay registering its providers. @@ -53,7 +51,7 @@ export async function connectRuntimeOwnedSshTarget(args: { export function getRuntimeOwnedSshRelayState(targetId: string): RuntimeOwnedSshRelayState { const status = getRegisteredSshState(targetId)?.status - if (status === 'connected' && getSshPtyProvider(targetId)) { + if (status === 'connected' && areSshRelayProvidersRegistered(targetId)) { return 'attached' } return status === 'reconnecting' ? 'reconnecting' : 'detached' @@ -134,13 +132,7 @@ async function waitForRuntimeSshProviders(targetId: string, signal?: AbortSignal if (signal?.aborted) { throw new Error(`SSH provider wait aborted for target "${targetId}".`) } - // Why the PTY provider too: a terminal spawn right after connect otherwise races - // the relay's provider registration and fails with "No PTY provider". - if ( - getSshGitProvider(targetId) && - getSshFilesystemProvider(targetId) && - getSshPtyProvider(targetId) - ) { + if (areSshRelayProvidersRegistered(targetId)) { return } await new Promise((resolve) => setTimeout(resolve, SSH_PROVIDER_READY_INTERVAL_MS)) diff --git a/src/main/providers/ssh-relay-provider-readiness.ts b/src/main/providers/ssh-relay-provider-readiness.ts new file mode 100644 index 00000000000..eb23259f5f7 --- /dev/null +++ b/src/main/providers/ssh-relay-provider-readiness.ts @@ -0,0 +1,21 @@ +import { getSshPtyProvider } from '../ipc/pty/provider/registry' +import { getSshFilesystemProvider } from './ssh-filesystem-dispatch' +import { getSshGitProvider } from './ssh-git-dispatch' + +/** + * The one definition of "this relay has registered its providers", shared by the connect + * wait and the attached check so the two cannot drift apart. + * + * A relay serves PTY, git, and filesystem from one session, so a partial set is a + * half-attached relay rather than a ready one. Classifying it as attached is what strands + * it: the owner skips the re-attach, nothing else redials, and every operation needing the + * absent provider keeps failing. Requiring all three also matches what a successful connect + * already guarantees, which is why the same predicate can serve both callers. + */ +export function areSshRelayProvidersRegistered(connectionId: string): boolean { + return Boolean( + getSshPtyProvider(connectionId) && + getSshGitProvider(connectionId) && + getSshFilesystemProvider(connectionId) + ) +} From f47828d3cb593cf2b841cb84e3410a325f3d63cd Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 8 Sep 2026 00:59:00 -0700 Subject: [PATCH 4/4] fix(ssh): bound the background provider-miss throttle map The git and filesystem dispatchers pass every SSH connection id in the app, and scheduleSshProviderMissRecovery recorded each one before asking the owner whether it claimed the connection. Ids the recovery declined were therefore retained forever, and the stale entry also swallowed the next miss for that id until the interval elapsed. Consult the recovery first and record only once it accepts, and prune entries past the throttle interval (an expired entry no longer throttles anything). --- .../ssh-provider-miss-recovery.test.ts | 31 ++++++++++++++++++- .../providers/ssh-provider-miss-recovery.ts | 16 +++++++++- 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/src/main/providers/ssh-provider-miss-recovery.test.ts b/src/main/providers/ssh-provider-miss-recovery.test.ts index d43d85e32be..3d54fa1b911 100644 --- a/src/main/providers/ssh-provider-miss-recovery.test.ts +++ b/src/main/providers/ssh-provider-miss-recovery.test.ts @@ -7,7 +7,8 @@ import { SSH_GIT_PROVIDER_UNAVAILABLE_MESSAGE, requireSshGitProvider } from './s import { recoverSshProviderMiss, scheduleSshProviderMissRecovery, - setSshProviderMissRecovery + setSshProviderMissRecovery, + sshProviderMissRecoveryThrottleEntryCount } from './ssh-provider-miss-recovery' const TARGET = 'runtime-ssh-orca-1' @@ -85,4 +86,32 @@ describe('scheduleSshProviderMissRecovery', () => { expect(recovery).toHaveBeenCalledWith('ssh-user-target') expect(recoverSshProviderMiss('ssh-user-target')).toBeUndefined() }) + + it('keeps re-consulting the owner for declined connections and retains none of them', () => { + // Why: these dispatchers are called with every SSH connection id in the app, most of + // which no owner claims. A declined id was never dialed, so there is nothing to back + // off from — throttling it would both retain it forever and swallow the next miss. + const recovery = vi.fn(() => undefined) + setSshProviderMissRecovery(recovery) + + scheduleSshProviderMissRecovery('ssh-user-a') + scheduleSshProviderMissRecovery('ssh-user-a') + scheduleSshProviderMissRecovery('ssh-user-b') + + expect(recovery).toHaveBeenCalledTimes(3) + expect(sshProviderMissRecoveryThrottleEntryCount()).toBe(0) + }) + + it('prunes claimed connections once their throttle interval has passed', () => { + setSshProviderMissRecovery(() => Promise.resolve()) + + scheduleSshProviderMissRecovery('runtime-ssh-orca-1') + scheduleSshProviderMissRecovery('runtime-ssh-orca-2') + expect(sshProviderMissRecoveryThrottleEntryCount()).toBe(2) + + vi.advanceTimersByTime(5_000) + scheduleSshProviderMissRecovery('runtime-ssh-orca-3') + + expect(sshProviderMissRecoveryThrottleEntryCount()).toBe(1) + }) }) diff --git a/src/main/providers/ssh-provider-miss-recovery.ts b/src/main/providers/ssh-provider-miss-recovery.ts index 9453c6e71c2..2cf7c9ae8a2 100644 --- a/src/main/providers/ssh-provider-miss-recovery.ts +++ b/src/main/providers/ssh-provider-miss-recovery.ts @@ -40,14 +40,28 @@ export function scheduleSshProviderMissRecovery(connectionId: string): void { if (startedAt !== undefined && now - startedAt < BACKGROUND_RECOVERY_THROTTLE_MS) { return } - backgroundRecoveryStartedAt.set(connectionId, now) + // Why prune before recording: an expired entry no longer throttles anything, and these + // dispatchers are called with every SSH connection id in the app. + for (const [id, at] of backgroundRecoveryStartedAt) { + if (now - at >= BACKGROUND_RECOVERY_THROTTLE_MS) { + backgroundRecoveryStartedAt.delete(id) + } + } const pending = recovery(connectionId) if (!pending) { + // Declined: the owner does not claim this connection, so there is nothing to throttle + // and recording it would retain an id this map will never act on. return } + backgroundRecoveryStartedAt.set(connectionId, now) pending.catch((error: unknown) => { console.warn( `[ssh] Background provider re-attach failed for ${connectionId}: ${error instanceof Error ? error.message : String(error)}` ) }) } + +/** Test-only: the throttle map is a memory bound, which is not observable from behaviour. */ +export function sshProviderMissRecoveryThrottleEntryCount(): number { + return backgroundRecoveryStartedAt.size +}