diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index 5534d9599fa..9bebf8de638 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -333,6 +333,7 @@ jobs: . != "tests/e2e/ssh-restart-tab-accumulation.spec.ts" and . != "tests/e2e/ssh-skill-installation.spec.ts" and . != "tests/e2e/ssh-stale-resume-execution-host-scope.spec.ts" and + . != "tests/e2e/ssh-startup-local-shadow.spec.ts" and . != "tests/e2e/ssh-terminal-window-wake-stale-grid-repro.spec.ts" and . != "tests/e2e/terminal-inline-images-ssh.spec.ts" and . != "tests/e2e/ssh-docker-watcher-isolation.spec.ts" and diff --git a/config/scripts/ci-e2e-job-selection.mjs b/config/scripts/ci-e2e-job-selection.mjs index 1e36d079979..53747f9d496 100644 --- a/config/scripts/ci-e2e-job-selection.mjs +++ b/config/scripts/ci-e2e-job-selection.mjs @@ -26,6 +26,7 @@ export const DOCKER_SSH_E2E_SPECS = [ 'tests/e2e/ssh-restart-tab-accumulation.spec.ts', 'tests/e2e/ssh-skill-installation.spec.ts', 'tests/e2e/ssh-stale-resume-execution-host-scope.spec.ts', + 'tests/e2e/ssh-startup-local-shadow.spec.ts', 'tests/e2e/ssh-terminal-window-wake-stale-grid-repro.spec.ts', 'tests/e2e/terminal-inline-images-ssh.spec.ts', 'tests/e2e/ssh-docker-watcher-isolation.spec.ts', diff --git a/config/scripts/run-ssh-docker-e2e.mjs b/config/scripts/run-ssh-docker-e2e.mjs index 58cac941f40..e418f9e7118 100644 --- a/config/scripts/run-ssh-docker-e2e.mjs +++ b/config/scripts/run-ssh-docker-e2e.mjs @@ -78,6 +78,7 @@ const result = spawnSync( 'tests/e2e/ssh-restart-tab-accumulation.spec.ts', 'tests/e2e/ssh-skill-installation.spec.ts', 'tests/e2e/ssh-stale-resume-execution-host-scope.spec.ts', + 'tests/e2e/ssh-startup-local-shadow.spec.ts', 'tests/e2e/ssh-terminal-window-wake-stale-grid-repro.spec.ts', 'tests/e2e/terminal-inline-images-ssh.spec.ts', '--config', diff --git a/src/main/ipc/remote-workspace-target-session-export.test.ts b/src/main/ipc/remote-workspace-target-session-export.test.ts new file mode 100644 index 00000000000..aa60c2d2929 --- /dev/null +++ b/src/main/ipc/remote-workspace-target-session-export.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, it } from 'vitest' +import type { Store } from '../persistence' +import type { Repo } from '../../shared/repo-types' +import { getDefaultWorkspaceSession } from '../../shared/constants' +import type { TerminalTab } from '../../shared/terminal-tab-types' +import type { WorkspaceSessionState } from '../../shared/workspace-session-state-types' +import { createRepoRowExecutionHostLookup } from '../../shared/worktree-execution-host-resolution' +import { + createWorktreeOwnerResolver, + persistedSessionForTarget +} from './remote-workspace-target-session-export' + +const TARGET_ID = 'target-1' +const WORKTREE_ID = 'repo-1::/remote/repo' + +function tab(id: string): TerminalTab { + return { + id, + ptyId: null, + worktreeId: WORKTREE_ID, + title: id, + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + } +} + +function session(tabs: TerminalTab[]): WorkspaceSessionState { + return { ...getDefaultWorkspaceSession(), tabsByWorktree: { [WORKTREE_ID]: tabs } } +} + +describe('persistedSessionForTarget', () => { + it("publishes the ssh partition's tabs, not a stray local copy of an SSH-owned workspace", () => { + const partitions: Record = { + local: session([tab('tab-stale')]), + [`ssh:${TARGET_ID}`]: session([tab('tab-live')]) + } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the publish fallback reads only getWorkspaceSession. + const store = { + getWorkspaceSession: (hostId?: string) => partitions[hostId ?? 'local'] + } as unknown as Store + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: ownership resolution reads only id, connectionId and executionHostId. + const repos = [{ id: 'repo-1', connectionId: TARGET_ID, executionHostId: null }] as Repo[] + + const published = persistedSessionForTarget( + store, + TARGET_ID, + createWorktreeOwnerResolver(createRepoRowExecutionHostLookup(repos)) + ) + + expect(published.tabsByWorktree[WORKTREE_ID]?.map((entry) => entry.id)).toEqual(['tab-live']) + }) +}) diff --git a/src/main/ipc/remote-workspace-target-session-export.ts b/src/main/ipc/remote-workspace-target-session-export.ts index c88f8cb620b..9adef9312e9 100644 --- a/src/main/ipc/remote-workspace-target-session-export.ts +++ b/src/main/ipc/remote-workspace-target-session-export.ts @@ -113,9 +113,14 @@ function catalogAttributionForPartition( resolveWorktreeOwner: WorktreeOwnerResolver, host: WorkspaceSessionState, hostId: ExecutionHostId -): { contestedSessionKeys: Set; foreignSessionKeys: Set } { +): { + contestedSessionKeys: Set + foreignSessionKeys: Set + ownedSessionKeys: Set +} { const contestedSessionKeys = new Set() const foreignSessionKeys = new Set() + const ownedSessionKeys = new Set() for (const workspaceId of workspaceIdsNamedByPartition(host)) { // A folder key names no repo, and `getRepoIdFromWorktreeId` hands back the whole key rather // than nothing, so the catalog would be asked about `folder:` and answer `unknown`. @@ -130,7 +135,9 @@ function catalogAttributionForPartition( } } else if (resolution.hostId !== hostId) { foreignSessionKeys.add(workspaceId) + } else { + ownedSessionKeys.add(workspaceId) } } - return { contestedSessionKeys, foreignSessionKeys } + return { contestedSessionKeys, foreignSessionKeys, ownedSessionKeys } } diff --git a/src/renderer/src/lib/workspace-session-host-hydration.ts b/src/renderer/src/lib/workspace-session-host-hydration.ts index ddf0f803386..9852dd56385 100644 --- a/src/renderer/src/lib/workspace-session-host-hydration.ts +++ b/src/renderer/src/lib/workspace-session-host-hydration.ts @@ -222,6 +222,7 @@ export async function fetchWorkspaceSessionWithRuntimeHostOwners( for (const [hostId, slice] of sshPartitions) { const adoption = adoptStrandedHostPartitionSession(session, slice, { contestedSessionKeys: attribution.contestedSessionKeys, + ownedSessionKeys: attribution.ownedSessionKeysByHostId.get(hostId), foreignSessionKeys: unownedSessionKeys( session, attribution.contestedSessionKeys, @@ -289,12 +290,11 @@ function sshPartitionCatalogAttribution( repos: readonly Pick[], sshPartitions: readonly (readonly [ExecutionHostId, WorkspaceSessionState | null])[], mergedContested: ReadonlySet -): { - contestedSessionKeys: Set - foreignSessionKeysByHostId: Map> -} { +) { const contestedSessionKeys = new Set(mergedContested) const foreignSessionKeysByHostId = new Map>() + // Ids the catalog resolves to the ssh partition that names them. + const ownedSessionKeysByHostId = new Map>() const repoLookup = createRepoRowExecutionHostLookup(repos) const ownedByHostId = new Map>() for (const [hostId, slice] of sshPartitions) { @@ -303,6 +303,7 @@ function sshPartitionCatalogAttribution( } const foreign = new Set() const owned = new Set() + const catalogOwned = new Set() for (const workspaceId of workspaceIdsNamedByPartition(slice)) { // A folder key carries no repo id at all, and `getRepoIdFromWorktreeId` hands back the whole // key rather than nothing, so the catalog would be asked about `folder:` and answer @@ -317,7 +318,9 @@ function sshPartitionCatalogAttribution( foreign.add(workspaceId) continue } - if (resolution?.kind === 'unresolved' && resolution.reason === 'ambiguous') { + if (resolution?.kind === 'resolved') { + catalogOwned.add(workspaceId) + } else if (resolution?.reason === 'ambiguous') { contestedSessionKeys.add(workspaceId) } owned.add(workspaceId) @@ -326,6 +329,7 @@ function sshPartitionCatalogAttribution( foreignSessionKeysByHostId.set(hostId, foreign) } ownedByHostId.set(hostId, owned) + ownedSessionKeysByHostId.set(hostId, catalogOwned) } // Why co-presence is asked only of the ids left after the catalog has spoken: one partition // holding residue the catalog attributes elsewhere is a single owner plus a leftover, not a @@ -333,7 +337,7 @@ function sshPartitionCatalogAttribution( for (const workspaceId of sessionKeysHeldByMultiplePartitionSets([...ownedByHostId.values()])) { contestedSessionKeys.add(workspaceId) } - return { contestedSessionKeys, foreignSessionKeysByHostId } + return { contestedSessionKeys, foreignSessionKeysByHostId, ownedSessionKeysByHostId } } /** Ids that appear in more than one of these per-partition sets. */ diff --git a/src/renderer/src/lib/workspace-session-ssh-partition-ownership.test.ts b/src/renderer/src/lib/workspace-session-ssh-partition-ownership.test.ts index 371b4f2b77b..26a3545f229 100644 --- a/src/renderer/src/lib/workspace-session-ssh-partition-ownership.test.ts +++ b/src/renderer/src/lib/workspace-session-ssh-partition-ownership.test.ts @@ -344,3 +344,29 @@ describe('a bare workspace id two partitions both hold', () => { expect(read.contestedPrimaryHostBySessionKey?.[WORKTREE_ID]).toBe(OTHER_SSH_HOST_ID) }) }) + +describe('a stray local copy beside a leftover in another ssh partition', () => { + const REPO_ID = 'repo-remote' + const WORKTREE_ID = `${REPO_ID}::/remote/checkout/feature` + + // Both sort orders, since partitions are read in sorted host-id order. + it.each([ + [TARGET_ID, OTHER_SSH_HOST_ID], + [OTHER_TARGET_ID, SSH_HOST_ID] + ])('keeps the rows of the partition the catalog names (owner %s)', async (owner, leftover) => { + const read = await fetchWorkspaceSessionWithRuntimeHostOwners( + partitionedApi({ + local: session({ tabsByWorktree: { [WORKTREE_ID]: [tab('tab-local', WORKTREE_ID)] } }), + [`ssh:${owner}`]: session({ + tabsByWorktree: { [WORKTREE_ID]: [tab('tab-live', WORKTREE_ID)] } + }), + [leftover]: session({ + tabsByWorktree: { [WORKTREE_ID]: [tab('tab-leftover', WORKTREE_ID)] } + }) + }), + [{ id: REPO_ID, connectionId: owner, executionHostId: null }] + ) + + expect(read.session.tabsByWorktree[WORKTREE_ID]?.map((entry) => entry.id)).toEqual(['tab-live']) + }) +}) diff --git a/src/renderer/src/lib/workspace-session-ssh-partition-round-trip.test.ts b/src/renderer/src/lib/workspace-session-ssh-partition-round-trip.test.ts index 13cc1baf507..f803b6f17d2 100644 --- a/src/renderer/src/lib/workspace-session-ssh-partition-round-trip.test.ts +++ b/src/renderer/src/lib/workspace-session-ssh-partition-round-trip.test.ts @@ -23,6 +23,7 @@ import type { WorkspaceSessionState } from '../../../shared/workspace-session-st import { shouldAutoCreateInitialTerminal } from '@/components/terminal/initial-terminal' import { mergeDirectSshRemoteWorkspaceSession } from '../hooks/remote-workspace-session-merge' import { fetchWorkspaceSessionWithRuntimeHostOwners } from './workspace-session-host-hydration' +import { worktreeWorkspaceKey } from '../../../shared/workspace-scope' const TARGET_ID = 'target-1' const SSH_HOST_ID: ExecutionHostId = `ssh:${TARGET_ID}` @@ -121,25 +122,159 @@ describe('ssh host partition hydration', () => { ).toBe('session-1') }) - it('leaves a workspace the local partition already holds tabs for untouched', async () => { - // The other direction of the same rule, and the reason adoption is only gap-filling: merging - // into a populated row would re-add tabs the user had closed on every launch. + it('ignores a stray local copy of a workspace the catalog places on the ssh target', async () => { + // `local` rows for an SSH workspace are residue (#23390, #25616). Keeping them restored tabs + // the user had closed and dropped the live ones the SSH partition holds. const read = await fetchWorkspaceSessionWithRuntimeHostOwners( partitionedApi(strandedPartitions([tab('tab-runtime')], [tab('tab-local')])), repos ) + expect(read.session.tabsByWorktree[WORKTREE_ID]?.map((entry) => entry.id)).toEqual([ + 'tab-runtime' + ]) + expect(read.session.activeTabIdByWorktree?.[WORKTREE_ID]).toBe('tab-runtime') + expect(read.contestedPrimaryHostBySessionKey[WORKTREE_ID]).toBe(SSH_HOST_ID) + }) + + it('keeps the ssh partition agent-resume records beside a stray local copy', async () => { + const partitions = strandedPartitions([tab('tab-runtime')], [tab('tab-local')]) + partitions[SSH_HOST_ID] = session({ + ...partitions[SSH_HOST_ID], + sleepingAgentSessionsByPaneKey: { + 'tab-runtime:leaf-1': { + paneKey: 'tab-runtime:leaf-1', + worktreeId: WORKTREE_ID, + tabId: 'tab-runtime', + agent: 'claude', + providerSession: { key: 'session_id', id: 'session-1' }, + prompt: 'resume me', + state: 'done', + capturedAt: 5, + updatedAt: 5 + } satisfies SleepingAgentSessionRecord + } + }) + + const read = await fetchWorkspaceSessionWithRuntimeHostOwners(partitionedApi(partitions), repos) + + expect(Object.keys(read.session.sleepingAgentSessionsByPaneKey ?? {})).toEqual([ + 'tab-runtime:leaf-1' + ]) + }) + + it('keeps local tabs when the ssh partition names the workspace but holds no tabs', async () => { + const read = await fetchWorkspaceSessionWithRuntimeHostOwners( + partitionedApi(strandedPartitions([], [tab('tab-local')])), + repos + ) + expect(read.session.tabsByWorktree[WORKTREE_ID]?.map((entry) => entry.id)).toEqual([ 'tab-local' ]) }) - it("leaves that workspace's other rows alone as well", async () => { + it('ignores a stray local copy when the ssh partition keys its tabs by workspace key', async () => { + const partitions = strandedPartitions([tab('tab-runtime')], [tab('tab-local')]) + partitions[SSH_HOST_ID] = session({ + tabsByWorktree: { [worktreeWorkspaceKey(WORKTREE_ID)]: [tab('tab-runtime')] } + }) + + const read = await fetchWorkspaceSessionWithRuntimeHostOwners(partitionedApi(partitions), repos) + + expect( + Object.values(read.session.tabsByWorktree) + .flat() + .map((entry) => entry.id) + ).toEqual(['tab-runtime']) + }) + + it("replaces a stray local copy's records for a tab id the ssh partition shares", async () => { + // Relay reattach copied tab ids into `local`, so the stale and live rows can share a key. + const partitions = strandedPartitions([tab('tab-shared')], [tab('tab-shared')]) + const layout = (activeLeafId: string) => ({ root: null, activeLeafId, expandedLeafId: null }) + partitions.local = session({ + ...partitions.local, + terminalLayoutsByTabId: { 'tab-shared': layout('leaf-stale') } + }) + partitions[SSH_HOST_ID] = session({ + ...partitions[SSH_HOST_ID], + terminalLayoutsByTabId: { 'tab-shared': layout('leaf-live') } + }) + + const read = await fetchWorkspaceSessionWithRuntimeHostOwners(partitionedApi(partitions), repos) + + expect(read.session.terminalLayoutsByTabId?.['tab-shared']?.activeLeafId).toBe('leaf-live') + }) + + it("keeps a stray local copy's unsaved draft beside the ssh partition's open files", async () => { + const openFile = (relativePath: string, dirtyDraftContent?: string) => ({ + filePath: `${WORKTREE_PATH}/${relativePath}`, + relativePath, + worktreeId: WORKTREE_ID, + language: 'typescript', + ...(dirtyDraftContent === undefined ? {} : { dirtyDraftContent }) + }) + const partitions = strandedPartitions([tab('tab-runtime')], [tab('tab-local')]) + partitions.local = session({ + ...partitions.local, + openFilesByWorktree: { [WORKTREE_ID]: [openFile('src/main.ts', 'unsaved work')] } + }) + partitions[SSH_HOST_ID] = session({ + ...partitions[SSH_HOST_ID], + openFilesByWorktree: { [WORKTREE_ID]: [openFile('src/live.ts')] } + }) + + const read = await fetchWorkspaceSessionWithRuntimeHostOwners(partitionedApi(partitions), repos) + + expect(read.session.tabsByWorktree[WORKTREE_ID]?.map((entry) => entry.id)).toEqual([ + 'tab-runtime' + ]) + expect( + read.session.openFilesByWorktree?.[WORKTREE_ID]?.map((file) => [ + file.relativePath, + file.dirtyDraftContent + ]) + ).toEqual([ + ['src/live.ts', undefined], + ['src/main.ts', 'unsaved work'] + ]) + }) + + it("keeps a stray local copy's unsaved draft over the ssh partition's clean entry", async () => { + const file = { + filePath: `${WORKTREE_PATH}/src/main.ts`, + relativePath: 'src/main.ts', + worktreeId: WORKTREE_ID, + language: 'typescript' + } + const partitions = strandedPartitions([tab('tab-runtime')], [tab('tab-local')]) + partitions.local = session({ + ...partitions.local, + openFilesByWorktree: { [WORKTREE_ID]: [{ ...file, dirtyDraftContent: 'unsaved work' }] } + }) + partitions[SSH_HOST_ID] = session({ + ...partitions[SSH_HOST_ID], + openFilesByWorktree: { [WORKTREE_ID]: [file] } + }) + + const read = await fetchWorkspaceSessionWithRuntimeHostOwners(partitionedApi(partitions), repos) + + expect( + read.session.openFilesByWorktree?.[WORKTREE_ID]?.map((entry) => entry.dirtyDraftContent) + ).toEqual(['unsaved work']) + }) + + it('leaves a populated local copy alone when the catalog cannot name its owner', async () => { + // Without a repo row the local copy may be the live one, so the gap-filling rule still applies. const read = await fetchWorkspaceSessionWithRuntimeHostOwners( partitionedApi(strandedPartitions([tab('tab-runtime')], [tab('tab-local')])), - repos + [] ) + expect(read.session.tabsByWorktree[WORKTREE_ID]?.map((entry) => entry.id)).toEqual([ + 'tab-local' + ]) expect(read.session.activeTabIdByWorktree?.[WORKTREE_ID]).toBeUndefined() }) @@ -313,9 +448,10 @@ describe('ssh host partition rows the host has nothing for', () => { ) }) - it('still adopts a populated host row over the base leftovers', async () => { + it('still adopts a populated host row over the base leftovers, keeping only their drafts', async () => { // The other side of the same rule: the guard must be about the host having nothing, not about - // the base having something, or adoption stops repairing the split it exists for. + // the base having something, or adoption stops repairing the split it exists for. An unsaved + // draft the host row lacks is the one leftover kept, since nothing else can recover it. const partitions = emptyHostRowsOverBaseDraft(false) partitions[SSH_HOST_ID] = session({ ...partitions[SSH_HOST_ID], @@ -335,7 +471,7 @@ describe('ssh host partition rows the host has nothing for', () => { expect( read.session.openFilesByWorktree?.[WORKTREE_ID]?.map((file) => file.relativePath) - ).toEqual(['src/host.ts']) + ).toEqual(['src/host.ts', 'src/main.ts']) }) it('adopts the layout of a tab the host slice names only in unifiedTabs', async () => { diff --git a/src/shared/workspace-session-stranded-partition-adoption.ts b/src/shared/workspace-session-stranded-partition-adoption.ts index 8be7e1f81eb..c36d35b4bef 100644 --- a/src/shared/workspace-session-stranded-partition-adoption.ts +++ b/src/shared/workspace-session-stranded-partition-adoption.ts @@ -51,7 +51,10 @@ import { * that anything was closed (`mergeDirectSshRemoteWorkspaceSession` argues this at length, and * docs/reference/ssh-execution-boundary.md makes it general — "we could not see it" is * `unverifiable`, never proof of absence). Treating it as the truth is what published an empty tab - * list and let `replace-session` delete the host's copy (#12721). + * list and let `replace-session` delete the host's copy (#12721). Nor is a copy of a workspace the + * repo catalog places on this host while the host holds its tabs: the base's rows there are residue + * that dropped the host's agent-resume records on the first save (#23390, #25616), so they are + * dropped before adoption — except unsaved drafts, which nothing else holds. */ type KeyedRecord = Record @@ -126,39 +129,17 @@ function collectWorkspaceIds( collected.add(workspaceId) } } + // Tab-, pane- and file-keyed rows resolve to nothing here: the worktree-keyed rows they hang + // off already name their workspaces. + const none = new Map() for (const field of SESSION_FIELDS) { - const ownership: WorkspaceSessionFieldOwnership = WORKSPACE_SESSION_FIELD_OWNERSHIP[field] - const value = host[field] - switch (ownership) { - case 'global': - case 'hostPrivate': - case 'tabKeyed': - case 'paneKeyed': - case 'fileKeyed': - // Keyed by something the workspaces below already account for. - break - case 'worktreeKeyed': - for (const key of Object.keys(asRecord(value) ?? {})) { - consider(key) - } - break - case 'worktreeArray': - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: worktreeArray fields hold worktree ids; the ownership table is what says so, not the value's static type. - for (const id of Array.isArray(value) ? (value as string[]) : []) { - consider(id) - } - break - case 'sleepingAgentKeyed': - case 'surfaceTombstoneKeyed': - for (const entry of Object.values(asRecord(value) ?? {})) { - consider(recordWorkspaceId(entry)) - } - break - case 'browserWorkspaceKeyed': - for (const entry of Object.values(asRecord(value) ?? {})) { - consider(browserPagesWorkspaceId(entry)) - } - break + const workspaceOf = entryWorkspaceResolver(field, none, none) + const value: unknown = host[field] + const entries = Array.isArray(value) + ? value.map((id: unknown) => [id, id] as const) + : Object.entries(asRecord(value) ?? {}) + for (const [key, entry] of entries) { + consider(typeof key === 'string' ? workspaceOf?.(key, entry) : null) } } return collected @@ -250,6 +231,83 @@ export function partitionRowsTheWriteWontReturn( return parked as WorkspaceSessionState | null } +/** Which workspace an entry of a scoped field belongs to; null for global and host-private fields. */ +function entryWorkspaceResolver( + field: keyof WorkspaceSessionState, + worktreeIdByTabId: Map, + worktreeIdByFileId: Map +): ((key: string, entry: unknown) => string | null | undefined) | null { + const ownership: WorkspaceSessionFieldOwnership = WORKSPACE_SESSION_FIELD_OWNERSHIP[field] + switch (ownership) { + case 'global': + case 'hostPrivate': + return null + case 'worktreeKeyed': + case 'worktreeArray': + return (key) => key + case 'tabKeyed': + return (key) => worktreeIdByTabId.get(key) + case 'paneKeyed': + return (key) => worktreeIdForPaneKey(worktreeIdByTabId, key) + case 'sleepingAgentKeyed': + case 'surfaceTombstoneKeyed': + // Keyed opaquely, but each record names its own workspace — the only routing left once the + // tab or pane it describes is gone, and the same one `splitWorkspaceSessionByHost` uses. + return (_key, entry) => recordWorkspaceId(entry) + case 'browserWorkspaceKeyed': + return (_key, entry) => browserPagesWorkspaceId(entry) + case 'fileKeyed': + // Routed by the open file's workspace, through the same index the split routed it by. + return (key) => worktreeIdByFileId.get(key) + } +} + +/** The base without any rows for these workspaces, routed by the same indexes the split uses. */ +function withoutWorkspaces( + base: WorkspaceSessionState, + workspaceIds: ReadonlySet +): WorkspaceSessionState { + const worktreeIdByTabId = buildWorktreeIdByTabId(base) + const worktreeIdByFileId = buildWorktreeIdByFileId(base) + const next: KeyedRecord = { ...base } + for (const field of SESSION_FIELDS) { + const workspaceOf = entryWorkspaceResolver(field, worktreeIdByTabId, worktreeIdByFileId) + const keeps = (key: string, entry: unknown): boolean => + !workspaceIds.has(normalizeWorkspaceSessionKeyToWorkspaceId(workspaceOf?.(key, entry) ?? '')) + const value: unknown = base[field] + if (!workspaceOf || !value) { + continue + } + next[field] = Array.isArray(value) + ? value.filter((id: string) => keeps(id, id)) + : Object.fromEntries(Object.entries(value).filter(([key, entry]) => keeps(key, entry))) + } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: every field is the base's own value or a filtered copy of it. + return next as WorkspaceSessionState +} + +/** Unsaved drafts the base held that the adopted rows lack: no other channel holds one. */ +function keepUnsavedDrafts(next: WorkspaceSessionState, base: WorkspaceSessionState): void { + for (const [key, baseFiles] of Object.entries(base.openFilesByWorktree ?? {})) { + const files = next.openFilesByWorktree?.[key] ?? [] + const drafts = new Map( + baseFiles.filter((file) => file.dirtyDraftContent !== undefined).map((f) => [f.filePath, f]) + ) + if (files === baseFiles || drafts.size === 0) { + continue + } + // A host entry with its own draft wins; a clean one yields to the base's draft for that path. + const held = new Set(files.map((file) => file.filePath)) + const merged = [ + ...files.map( + (file) => (file.dirtyDraftContent === undefined && drafts.get(file.filePath)) || file + ), + ...[...drafts.values()].filter((file) => !held.has(file.filePath)) + ] + next.openFilesByWorktree = { ...next.openFilesByWorktree, [key]: merged } + } +} + export type StrandedPartitionAdoptionOptions = { /** Session keys the read found claimed by more than one partition. */ contestedSessionKeys?: ReadonlySet @@ -260,6 +318,8 @@ export type StrandedPartitionAdoptionOptions = { * rows stay where they are, which is the leak direction the boundary doc asks for. */ foreignSessionKeys?: ReadonlySet + /** Keys the repo catalog attributes to this host. */ + ownedSessionKeys?: ReadonlySet } export type StrandedPartitionAdoption = { @@ -271,13 +331,26 @@ export type StrandedPartitionAdoption = { const NOTHING_ADOPTED: ReadonlySet = new Set() export function adoptStrandedHostPartitionSession( - base: WorkspaceSessionState, + originalBase: WorkspaceSessionState, host: WorkspaceSessionState | null | undefined, options: StrandedPartitionAdoptionOptions = {} ): StrandedPartitionAdoption { if (!host) { - return { session: base, adoptedWorkspaceIds: NOTHING_ADOPTED } + return { session: originalBase, adoptedWorkspaceIds: NOTHING_ADOPTED } } + const contested = new Set() + for (const key of options.contestedSessionKeys ?? []) { + contested.add(normalizeWorkspaceSessionKeyToWorkspaceId(key)) + } + // Owned, uncontested and holding tabs: the host's copy is the live one. No tabs is not a close. + const superseded = new Set() + for (const [key, tabs] of Object.entries(host.tabsByWorktree ?? {})) { + const id = normalizeWorkspaceSessionKeyToWorkspaceId(key) + if (!hostHasNothingFor(tabs) && !contested.has(id) && options.ownedSessionKeys?.has(id)) { + superseded.add(id) + } + } + const base = superseded.size > 0 ? withoutWorkspaces(originalBase, superseded) : originalBase const adoptable = adoptableWorkspaceIds(base, host) for (const key of options.foreignSessionKeys ?? []) { adoptable.delete(normalizeWorkspaceSessionKeyToWorkspaceId(key)) @@ -285,10 +358,6 @@ export function adoptStrandedHostPartitionSession( if (adoptable.size === 0) { return { session: base, adoptedWorkspaceIds: NOTHING_ADOPTED } } - const contested = new Set() - for (const key of options.contestedSessionKeys ?? []) { - contested.add(normalizeWorkspaceSessionKeyToWorkspaceId(key)) - } const adopts = (key: string): boolean => adoptable.has(normalizeWorkspaceSessionKeyToWorkspaceId(key)) const isContested = (key: string): boolean => @@ -314,9 +383,6 @@ export function adoptStrandedHostPartitionSession( // the keyed fields do not depend on the ownership table's declaration order. const worktreeIdByTabId = buildWorktreeIdByTabId(host) const worktreeIdByFileId = buildWorktreeIdByFileId(host) - const adoptsResolved = (worktreeId: string | undefined): boolean => - worktreeId !== undefined && adopts(worktreeId) - for (const field of SESSION_FIELDS) { const ownership: WorkspaceSessionFieldOwnership = WORKSPACE_SESSION_FIELD_OWNERSHIP[field] switch (ownership) { @@ -357,34 +423,18 @@ export function adoptStrandedHostPartitionSession( break } case 'tabKeyed': - adoptRecord(next, host, field, (key) => adoptsResolved(worktreeIdByTabId.get(key))) - break case 'paneKeyed': - adoptRecord(next, host, field, (key) => - adoptsResolved(worktreeIdForPaneKey(worktreeIdByTabId, key)) - ) - break case 'sleepingAgentKeyed': case 'surfaceTombstoneKeyed': - // Keyed opaquely, but each record names its own workspace — the only routing left once the - // tab or pane it describes is gone, and the same one `splitWorkspaceSessionByHost` uses. - adoptRecord(next, host, field, (_key, entry) => { - const workspaceId = recordWorkspaceId(entry) - return workspaceId !== null && adopts(workspaceId) - }) - break case 'browserWorkspaceKeyed': - adoptRecord(next, host, field, (_key, entry) => { - const workspaceId = browserPagesWorkspaceId(entry) - return workspaceId !== null && adopts(workspaceId) - }) - break - case 'fileKeyed': - // Routed by the open file's workspace, through the same index the split routed it by. - adoptRecord(next, host, field, (key) => adoptsResolved(worktreeIdByFileId.get(key))) + case 'fileKeyed': { + const workspaceOf = entryWorkspaceResolver(field, worktreeIdByTabId, worktreeIdByFileId) + adoptRecord(next, host, field, (key, entry) => adopts(workspaceOf?.(key, entry) ?? '')) break + } } } + keepUnsavedDrafts(next, originalBase) // Why contested ids are withheld: the write path would route the whole bare id here, carrying the // co-claimant's rows into this host's partition — the loss the gap-fill above exists to prevent. const adoptedWorkspaceIds = new Set( diff --git a/tests/e2e/ssh-startup-local-shadow.spec.ts b/tests/e2e/ssh-startup-local-shadow.spec.ts new file mode 100644 index 00000000000..f561c4ddbc5 --- /dev/null +++ b/tests/e2e/ssh-startup-local-shadow.spec.ts @@ -0,0 +1,146 @@ +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, + readRemoteTerminalTabs +} from './helpers/docker-ssh-relay-terminal-tabs' +import { + cleanupDockerSshRelayTarget, + startDockerSshRelayTarget, + type DockerSshRelayTarget +} from './helpers/docker-ssh-relay-target' +import { connectDockerSshRelayTarget } from './helpers/docker-ssh-relay-connection' +import { createRestartSession, readRestartRendererState } from './helpers/orca-restart' +import { + mutateStoppedProfileState, + readPersistedProfileState +} from './helpers/persisted-profile-state' + +const RUN_DOCKER_SSH = process.env.ORCA_E2E_SSH_DOCKER === '1' +const STALE_TAB_ID = 'stale-closed-tab' + +test.use({ seedTestRepo: false }) + +type JsonRecord = Record + +function isRecord(value: unknown): value is JsonRecord { + return typeof value === 'object' && value !== null && !Array.isArray(value) +} + +function record(parent: JsonRecord, key: string): JsonRecord { + const value = parent[key] + if (isRecord(value)) { + return value + } + const created: JsonRecord = {} + parent[key] = created + return created +} + +function sshPartition(state: JsonRecord, targetId: string): JsonRecord { + return record(record(state, 'workspaceSessionsByHostId'), `ssh:${targetId}`) +} + +async function readRestoredTabIds(page: Page, worktreeId: string): Promise { + return readRestartRendererState(async () => + (await readRemoteTerminalTabs(page, worktreeId)).map((tab) => tab.id).sort() + ).catch(() => null) +} + +/** + * A `local` row for an SSH workspace is residue: builds before #19572 wrote it, and relay reattach + * wrote it later (#25616). Startup used to keep that copy whenever it held a tab and skip the SSH + * partition's own rows, so live tabs vanished, closed ones came back, and the workspace's + * agent-resume records were dropped and then erased by the first save (#23390). + */ +test.describe('SSH workspace startup with a stray local copy', () => { + 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('the SSH partition wins over a stray local copy at startup', 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 + const page = first.page + await waitForSessionReady(page) + const { worktreeId, targetId } = await connectDockerSshRelayTarget(page, target) + await expect.poll(() => waitForActiveWorktree(page), { timeout: 30_000 }).toBe(worktreeId) + await waitForActiveTerminalManager(page, 60_000) + await waitForActivePanePtyId(page, 60_000) + await createRemoteTerminalTab(page, worktreeId) + const liveTabIds = (await readRemoteTerminalTabs(page, worktreeId)).map((tab) => tab.id) + expect(liveTabIds).toHaveLength(2) + // Past the ~1s debounced save, so the SSH partition holds both tabs. + await page.waitForTimeout(3_000) + await restart.close(app) + app = null + + const [olderTabId, newerTabId] = liveTabIds + const resumePaneKey = mutateStoppedProfileState(restart.userDataDir, (state) => { + const ssh = sshPartition(state, targetId) + const layout = record(record(ssh, 'terminalLayoutsByTabId'), newerTabId) + const [leafId] = Object.keys(record(layout, 'ptyIdsByLeafId')) + if (!leafId) { + throw new Error(`Expected a bound leaf for ${newerTabId}: ${JSON.stringify(layout)}`) + } + const paneKey = `${newerTabId}:${leafId}` + const sshTabs = record(ssh, 'tabsByWorktree')[worktreeId] + if (!Array.isArray(sshTabs) || sshTabs.length !== 2) { + throw new Error(`Expected both tabs in ssh:${targetId}, got ${JSON.stringify(sshTabs)}`) + } + record(ssh, 'sleepingAgentSessionsByPaneKey')[paneKey] = { + paneKey, + tabId: newerTabId, + worktreeId, + agent: 'codex', + providerSession: { key: 'session_id', id: 'codex-resume-probe' }, + prompt: 'continue', + state: 'done', + capturedAt: 10, + updatedAt: 10, + origin: 'worktree-sleep' + } + // The stray copy: the older tab under its creation title, plus a tab closed long ago. + const olderRow = sshTabs.find((tab) => isRecord(tab) && tab.id === olderTabId) + record(record(state, 'workspaceSession'), 'tabsByWorktree')[worktreeId] = [ + { ...olderRow, title: 'Terminal 18', customTitle: null }, + { ...olderRow, id: STALE_TAB_ID, ptyId: null, title: 'Terminal 3', customTitle: null } + ] + return paneKey + }) + + const second = await restart.launch() + app = second.app + await waitForSessionReady(second.page) + await expect + .poll(() => readRestoredTabIds(second.page, worktreeId), { timeout: 60_000 }) + .toEqual([...liveTabIds].sort()) + // Past the debounced save, which is what used to erase the SSH partition's resume record. + await second.page.waitForTimeout(3_000) + await restart.close(app) + app = null + + const persisted = readPersistedProfileState(restart.userDataDir) + expect( + Object.keys(record(sshPartition(persisted, targetId), 'sleepingAgentSessionsByPaneKey')), + 'agent-resume record in the SSH partition' + ).toContain(resumePaneKey) + const localTabs = record(record(persisted, 'workspaceSession'), 'tabsByWorktree') + expect(localTabs[worktreeId] ?? [], 'stray local copy after a save').toEqual([]) + } finally { + if (app) { + await restart.close(app) + } + await restart.dispose() + cleanupDockerSshRelayTarget(target) + } + }) +})