diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index be2386e2447..5534d9599fa 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -328,6 +328,7 @@ jobs: . != "tests/e2e/ssh-lost-kill-tab-resurrection.spec.ts" and . != "tests/e2e/ssh-pi-compatible-agent-title.spec.ts" and . != "tests/e2e/ssh-port-forward-lifecycle.spec.ts" and + . != "tests/e2e/ssh-reattach-home-partition.spec.ts" and . != "tests/e2e/ssh-reconnect-tab-destruction.spec.ts" and . != "tests/e2e/ssh-restart-tab-accumulation.spec.ts" and . != "tests/e2e/ssh-skill-installation.spec.ts" and diff --git a/config/scripts/ci-e2e-job-selection.mjs b/config/scripts/ci-e2e-job-selection.mjs index e2e207ce58b..1e36d079979 100644 --- a/config/scripts/ci-e2e-job-selection.mjs +++ b/config/scripts/ci-e2e-job-selection.mjs @@ -21,6 +21,7 @@ export const DOCKER_SSH_E2E_SPECS = [ 'tests/e2e/ssh-lost-kill-tab-resurrection.spec.ts', 'tests/e2e/ssh-pi-compatible-agent-title.spec.ts', 'tests/e2e/ssh-port-forward-lifecycle.spec.ts', + 'tests/e2e/ssh-reattach-home-partition.spec.ts', 'tests/e2e/ssh-reconnect-tab-destruction.spec.ts', 'tests/e2e/ssh-restart-tab-accumulation.spec.ts', 'tests/e2e/ssh-skill-installation.spec.ts', diff --git a/config/scripts/run-ssh-docker-e2e.mjs b/config/scripts/run-ssh-docker-e2e.mjs index 72fbbb9d44f..58cac941f40 100644 --- a/config/scripts/run-ssh-docker-e2e.mjs +++ b/config/scripts/run-ssh-docker-e2e.mjs @@ -73,6 +73,7 @@ const result = spawnSync( 'tests/e2e/ssh-lost-kill-tab-resurrection.spec.ts', 'tests/e2e/ssh-pi-compatible-agent-title.spec.ts', 'tests/e2e/ssh-port-forward-lifecycle.spec.ts', + 'tests/e2e/ssh-reattach-home-partition.spec.ts', 'tests/e2e/ssh-reconnect-tab-destruction.spec.ts', 'tests/e2e/ssh-restart-tab-accumulation.spec.ts', 'tests/e2e/ssh-skill-installation.spec.ts', diff --git a/src/main/persistence/loading-store/pty-binding-refusals.ts b/src/main/persistence/loading-store/pty-binding-refusals.ts index 6576ac3d007..9de369b7682 100644 --- a/src/main/persistence/loading-store/pty-binding-refusals.ts +++ b/src/main/persistence/loading-store/pty-binding-refusals.ts @@ -25,7 +25,7 @@ export function ptyBindingIsRefused( bindingWorktreeId: string, paneKey: string, /** Every host partition: a close is recorded where the tab lived, which need not be where this - * binding lands (a relay reattach binds into `local`). */ + * binding lands (older relay reattaches left SSH panes in `local`). */ partitions: readonly TerminalSessionPartition[] ): boolean { if (args.expectedSourceBinding) { diff --git a/src/main/persistence/terminal-topology/terminal-owner-invariants.ts b/src/main/persistence/terminal-topology/terminal-owner-invariants.ts index a5311486a06..fad25738ea3 100644 --- a/src/main/persistence/terminal-topology/terminal-owner-invariants.ts +++ b/src/main/persistence/terminal-topology/terminal-owner-invariants.ts @@ -24,7 +24,7 @@ export type TerminalLeafOwner = { export type TerminalOwnerConflictReason = | 'pty_bound_to_other_leaf' | 'leaf_in_other_tab' - // Kept apart: the relay reattach writes SSH panes into `local`, so this may be one moved surface. + // Kept apart: older relay reattaches left SSH panes in `local`, so this may be one moved surface. | 'leaf_in_other_tab_on_other_host' /** `runtime:` partitions belong to a remote Orca server and are written only by its tab sync. */ @@ -83,7 +83,7 @@ export function isSameTerminal( /** * The saved leaf a binding into `hostId` would duplicate, if any. The same tab:leaf in two - * partitions is one surface: the relay reattach still binds an SSH pane into `local`. + * partitions is one surface: older relay reattaches left SSH panes in `local`. */ export function findTerminalBindingConflict( binding: { tabId: string; leafId: string; ptyId: string; incarnationId?: string }, diff --git a/src/main/runtime/orca-runtime-reconcile-headless-mobile-session-browser-tabs.ts b/src/main/runtime/orca-runtime-reconcile-headless-mobile-session-browser-tabs.ts index 6ce5f8a3650..146f87e095a 100644 --- a/src/main/runtime/orca-runtime-reconcile-headless-mobile-session-browser-tabs.ts +++ b/src/main/runtime/orca-runtime-reconcile-headless-mobile-session-browser-tabs.ts @@ -127,8 +127,8 @@ export class OrcaRuntimeWithReconcileHeadlessMobileSessionBrowserTabs extends Or * binds one leaf in two tabs and orphans the PTY under the new one) and refuses the correct ones. * Same resolution `restoreReattachedPtyRuntime` already does for its own reattach fence. * - * Both workspace partitions are read because SSH spawns bind panes into `ssh:` while - * reattach binds into `local`; consulting one would report "nowhere" for a pane the other holds. + * Both workspace partitions are read because older builds' relay reattach left SSH panes in + * `local`; consulting one would report "nowhere" for a pane the other holds. */ protected findCurrentTerminalTabIdForLeaf(targetId: string, leafId: string): string | undefined { for (const leaf of this.leaves.values()) { diff --git a/src/main/ssh/ssh-relay-session-reconnect-incarnation.test.ts b/src/main/ssh/ssh-relay-session-reconnect-incarnation.test.ts index ce223ae0b9c..7231b20f6e4 100644 --- a/src/main/ssh/ssh-relay-session-reconnect-incarnation.test.ts +++ b/src/main/ssh/ssh-relay-session-reconnect-incarnation.test.ts @@ -452,6 +452,8 @@ describe('SshRelaySession reconnect incarnation ordering', () => { mayReviveRetiredSurface: false, origin: 'relay_reattach' }) + // Into the pane's home partition: a `local` copy shadowed it at the next startup (STA-9544). + expect(mockStore.persistPtyBinding).toHaveBeenCalledWith(expect.any(Function), 'ssh:target-1') expect(vi.mocked(mockStore.persistPtyBinding).mock.invocationCallOrder[0]).toBeLessThan( vi.mocked(mockStore.markSshRemotePtyLeasesAttachedAsync).mock.invocationCallOrder[0]! ) diff --git a/src/main/ssh/ssh-relay-session.ts b/src/main/ssh/ssh-relay-session.ts index d11587af4cf..e7f7c60a15c 100644 --- a/src/main/ssh/ssh-relay-session.ts +++ b/src/main/ssh/ssh-relay-session.ts @@ -2995,33 +2995,33 @@ export class SshRelaySession { if (lease?.worktreeId && lease.tabId && lease.leafId) { const { worktreeId, leafId, tabId: leaseTabId } = lease let tabId = lease.tabId + // The pane's home is this target's partition, where its spawn bound it. Binding into `local` + // left a copy there that startup then preferred over the home copy (STA-9544). + const hostId = toSshExecutionHostId(this.targetId) const bound = await this.store.persistPtyBinding(() => { if (!shouldContinue()) { return null } - const session = this.store.getWorkspaceSession?.() + const session = this.store.getWorkspaceSession?.(hostId) + // Rows an older build left in `local`; the renderer moves them home on its next save. + const legacySession = this.store.getWorkspaceSession?.() // The lease froze its tabId at write time; `detachTerminalPaneToTab` moves a live pane, so // trusting it would fence this reattach to the tab the pane LEFT and refuse a pane that // merely moved. Leaf is the identity, the tab is only where it currently sits. - // SSH spawns bind panes into `ssh:` while this reattach binds into `local`, so a - // fence that consulted only one partition would read "no pane" for a pane the other holds. - const hostSession = this.store.getWorkspaceSession?.(toSshExecutionHostId(this.targetId)) tabId = findTerminalTabIdForLeaf(session, leafId) ?? - findTerminalTabIdForLeaf(hostSession, leafId) ?? + findTerminalTabIdForLeaf(legacySession, leafId) ?? leaseTabId // Absence of the pane only means "the user closed it" once the persisted membership // speaks for this worktree. Before that it means the renderer has not published its // layout yet, and refusing there drops a tab the user still has — the regression that // reverted this fix twice. Losing a tab is worse than keeping a duplicate, so an // unauthoritative session still gets the creating write. - // Authority is read from `local` because that is the partition this write lands in — it - // is local's absence we would be interpreting. But a pane the other partition still holds - // is not gone, so it keeps its creating write: refusing there would strand a live pane + // A pane only `local` still holds is not gone either: refusing it would strand a live pane // behind a binding reattach can no longer reach. const mayCreate = !hasHostAuthoritativeTerminalMembership(session, worktreeId) || - findTerminalTabIdForLeaf(hostSession, leafId) !== undefined + findTerminalTabIdForLeaf(legacySession, leafId) !== undefined return { worktreeId: worktreeId, tabId, @@ -3032,7 +3032,7 @@ export class SshRelaySession { mayReviveRetiredSurface: false, origin: 'relay_reattach' as const } - }) + }, hostId) if (!shouldContinue()) { return 'missing-surface' } diff --git a/tests/e2e/ssh-reattach-home-partition.spec.ts b/tests/e2e/ssh-reattach-home-partition.spec.ts new file mode 100644 index 00000000000..9c5451ccfdf --- /dev/null +++ b/tests/e2e/ssh-reattach-home-partition.spec.ts @@ -0,0 +1,169 @@ +import type { ElectronApplication, Page } from '@stablyai/playwright-test' +import { test, expect } from './helpers/orca-app' +import { waitForActiveWorktree, waitForSessionReady } from './helpers/store' +import { waitForActivePanePtyId, waitForActiveTerminalManager } from './helpers/terminal' +import { createRemoteTerminalTab } from './helpers/docker-ssh-relay-terminal-tabs' +import { + cleanupDockerSshRelayTarget, + DOCKER_SSH_RELAY_REMOTE_REPO_PATH, + startDockerSshRelayTarget, + type DockerSshRelayTarget +} from './helpers/docker-ssh-relay-target' +import { connectDockerSshRelayTarget } from './helpers/docker-ssh-relay-connection' +import { dropDockerSshRelayTransport } from './helpers/docker-ssh-relay-faults' +import { createRestartSession, readRestartRendererState } from './helpers/orca-restart' +import { readPersistedProfileState } from './helpers/persisted-profile-state' +import { toSshExecutionHostId } from '../../src/shared/execution-host' + +const RUN_DOCKER_SSH = process.env.ORCA_E2E_SSH_DOCKER === '1' + +test.use({ seedTestRepo: false }) + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null +} + +async function readWorkspace(page: Page, worktreeId: string): Promise { + return readRestartRendererState(() => + page.evaluate((id) => { + const state = window.__store?.getState() + if (!state) { + return null + } + return JSON.stringify({ + tabs: (state.tabsByWorktree[id] ?? []).length, + files: state.openFiles.filter((file) => file.worktreeId === id).map((f) => f.relativePath) + }) + }, worktreeId) + ).catch(() => null) +} + +function readPersistedSshOpenFiles( + userDataDir: string, + targetId: string, + worktreeId: string +): unknown[] { + const sessions = readPersistedProfileState(userDataDir).workspaceSessionsByHostId + const session = isRecord(sessions) ? sessions[toSshExecutionHostId(targetId)] : undefined + const files = + isRecord(session) && isRecord(session.openFilesByWorktree) + ? session.openFilesByWorktree[worktreeId] + : undefined + return Array.isArray(files) + ? files.map((file) => (isRecord(file) ? file.relativePath : undefined)) + : [] +} + +type PrivateInvokeHandlers = { + _invokeHandlers?: Map unknown> +} + +/** Main's SSH state, read without a renderer; it reports `connected` only once the relay's + * reattach has finished (the relay override holds `reconnecting` until then). */ +function readMainSshState(app: ElectronApplication, targetId: string): Promise { + return app.evaluate(({ ipcMain }, id) => { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: test-only read of Electron's private invoke-handler map; the handler is called only if present. + const { _invokeHandlers: handlers } = ipcMain as unknown as PrivateInvokeHandlers + return handlers?.get('ssh:getState')?.({}, { targetId: id }) + }, targetId) +} + +/** + * A relay reattach used to bind the SSH pane into the `local` partition. With a window open the + * renderer's next save erased that copy within a second, but a reattach with no window (macOS + * keeps Orca running after its window closes) left it on disk. Startup then kept the `local` copy + * and skipped the SSH partition's rows for that workspace, so its open editor tabs were missing + * and its agent-resume records were dropped (STA-9544). + */ +test.describe('SSH relay reattach home partition', () => { + test.skip(!RUN_DOCKER_SSH, 'Set ORCA_E2E_SSH_DOCKER=1 to run Docker-backed SSH tests.') + test.skip(process.platform === 'win32', 'Docker SSH restore uses POSIX SSH tooling.') + + test('a reattach with no window open keeps the SSH workspace on the next launch', async (// oxlint-disable-next-line no-empty-pattern -- This restart test owns every Electron launch. + {}, testInfo) => { + test.setTimeout(600_000) + const restart = createRestartSession(testInfo) + let target: DockerSshRelayTarget | null = null + let app: ElectronApplication | null = null + try { + target = startDockerSshRelayTarget(testInfo) + const first = await restart.launch() + app = first.app + let page = first.page + await waitForSessionReady(page) + const remote = await connectDockerSshRelayTarget(page, target) + const { targetId, worktreeId } = remote + await expect.poll(() => waitForActiveWorktree(page), { timeout: 30_000 }).toBe(worktreeId) + await waitForActiveTerminalManager(page, 60_000) + await waitForActivePanePtyId(page, 60_000) + await createRemoteTerminalTab(page, worktreeId) + await page.evaluate( + ({ worktreeId, filePath }) => { + window.__store!.getState().openFile({ + filePath, + relativePath: 'README.md', + worktreeId, + language: 'markdown', + mode: 'edit' + }) + }, + { worktreeId, filePath: `${DOCKER_SSH_RELAY_REMOTE_REPO_PATH}/README.md` } + ) + const expected = JSON.stringify({ tabs: 2, files: ['README.md'] }) + await expect.poll(() => readWorkspace(page, worktreeId)).toBe(expected) + await expect + .poll(() => readPersistedSshOpenFiles(restart.userDataDir, targetId, worktreeId)) + .toEqual(['README.md']) + const before = await readMainSshState(first.app, targetId) + expect(before).toMatchObject({ + status: 'connected', + connectionGeneration: expect.any(Number) + }) + + await app.evaluate(({ app: electronApp, BrowserWindow }) => { + // Linux/Windows quit when the last window closes; keep running as macOS does. + electronApp.removeAllListeners('window-all-closed') + electronApp.on('window-all-closed', () => {}) + for (const window of BrowserWindow.getAllWindows()) { + window.close() + } + }) + expect(dropDockerSshRelayTransport(target)).toBeGreaterThan(0) + // Main reconnects on its own and reattaches both panes; no window is left to save over it. + await expect + .poll( + async () => { + const after = await readMainSshState(first.app, targetId) + return ( + isRecord(after) && + isRecord(before) && + after.status === 'connected' && + (after.providerEpoch !== before.providerEpoch || + after.connectionGeneration !== before.connectionGeneration) + ) + }, + { timeout: 120_000 } + ) + .toBe(true) + await restart.close(app) + app = null + const local = readPersistedProfileState(restart.userDataDir).workspaceSession + const localTabs = + isRecord(local) && isRecord(local.tabsByWorktree) ? local.tabsByWorktree : {} + expect.soft(localTabs[worktreeId] ?? [], 'SSH tabs in `local`').toEqual([]) + + const second = await restart.launch() + app = second.app + page = second.page + await expect + .poll(() => readWorkspace(page, worktreeId), { timeout: 60_000, intervals: [500] }) + .toBe(expected) + } finally { + if (app) { + await restart.close(app) + } + await restart.dispose() + cleanupDockerSshRelayTarget(target) + } + }) +})