From 92a306dd8829a7cb8d19051721ada90f6057ba7f Mon Sep 17 00:00:00 2001 From: tlgnkl <67519432+tlgnkl@users.noreply.github.com> Date: Sun, 27 Sep 2026 12:59:36 +0300 Subject: [PATCH 1/4] fix(session): let a populated ssh partition own its workspace over leftover local tabs adoptStrandedHostPartitionSession kept the base (`local`) copy of any workspace it held terminal tabs for. `local` is never pruned for an SSH workspace, so on long-lived profiles it is the stale copy: boot restored tabs closed weeks ago as fresh shells and dropped live agent tabs whose unified rows live in `ssh:` (#23390), and the main-side publish fallback uploaded the same stale rows to the host snapshot on every sync (#22503). When the host partition holds a populated tab row for the workspace, it now owns it. A base tab the host neither lists nor tombstoned survives if it still names a PTY, or if it was created after the host's newest tab or close for that workspace (a tab parked on an unresolved host has no PTY and no persisted marker). Carried tabs get a unified entry so they keep a surface. An empty host row or a contested id keeps the old behaviour. Fixes #23390 Fixes #22503 Co-Authored-By: Claude Opus 5.5 (1M context) --- .../ssh-host-partition-session-export.test.ts | 18 +- ...e-session-ssh-partition-round-trip.test.ts | 29 ++- .../workspace-session-base-tab-carryover.ts | 102 ++++++++ ...ession-stranded-partition-adoption.test.ts | 244 ++++++++++++++++++ ...ace-session-stranded-partition-adoption.ts | 48 +++- 5 files changed, 419 insertions(+), 22 deletions(-) create mode 100644 src/shared/workspace-session-base-tab-carryover.ts create mode 100644 src/shared/workspace-session-stranded-partition-adoption.test.ts diff --git a/src/main/ipc/ssh-host-partition-session-export.test.ts b/src/main/ipc/ssh-host-partition-session-export.test.ts index 1ec5167f866b..d28817389f20 100644 --- a/src/main/ipc/ssh-host-partition-session-export.test.ts +++ b/src/main/ipc/ssh-host-partition-session-export.test.ts @@ -162,7 +162,7 @@ beforeEach(() => { /** The observed shape from #12721: the runtime owns the tab list in `ssh:` while the * local blob still carries the worktree key with an empty list. */ -function createStrandedStore(): InstanceType { +function createStrandedStore(localTabs: TerminalTab[] = []): InstanceType { const dir = realpathSync(mkdtempSync(join(tmpdir(), 'orca-ssh-partition-export-'))) directories.push(dir) const store = createSqliteTestStore(Store, { dataFile: join(dir, 'orca-data.json') }) @@ -172,7 +172,7 @@ function createStrandedStore(): InstanceType { store.addRepo(remoteRepo(OTHER_REPO_ID, '/elsewhere/checkout', OTHER_TARGET_ID)) store.setWorkspaceSession({ ...getDefaultWorkspaceSession(), - tabsByWorktree: { [WORKTREE_ID]: [] } + tabsByWorktree: { [WORKTREE_ID]: localTabs } }) store.setWorkspaceSession( { @@ -235,4 +235,18 @@ describe('remoteWorkspace:setForConnectedTargets session fallback', () => { expect(hostSnapshot.session.tabsByWorktreePath[WORKTREE_PATH]).not.toEqual([]) }) + + it('publishes the owning partition rather than leftover local tabs (#22503)', async () => { + // Leftover `local` rows used to win whenever they were non-empty, so every sync re-uploaded + // tabs the user had closed and the next reconnect respawned them as empty shells. + const store = createStrandedStore([ + { ...runtimeAuthoredTab(), id: 'tab-closed-long-ago', ptyId: null, title: 'Terminal 7' } + ]) + + await publishToConnectedTarget(store) + + expect(hostSnapshot.session.tabsByWorktreePath[WORKTREE_PATH]?.map((tab) => tab.id)).toEqual([ + 'tab-runtime' + ]) + }) }) 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 19050b9ff670..06d75c3239eb 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 @@ -121,26 +121,41 @@ 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('leaves a workspace the local partition holds tabs for untouched when the host has none', async () => { + // The base keeps its own copy while the owning partition has nothing to say about the tabs. + const partitions = strandedPartitions([], [tab('tab-local')]) + const read = await fetchWorkspaceSessionWithRuntimeHostOwners(partitionedApi(partitions), repos) + + expect(read.session.tabsByWorktree[WORKTREE_ID]?.map((entry) => entry.id)).toEqual([ + 'tab-local' + ]) + }) + + it('lets a populated host row replace leftover local tabs (#22503)', async () => { + // `local` is never pruned for an SSH workspace, so its PTY-less rows are tabs closed since the + // split. Letting them win restored each one as a fresh shell on every launch. const read = await fetchWorkspaceSessionWithRuntimeHostOwners( - partitionedApi(strandedPartitions([tab('tab-runtime')], [tab('tab-local')])), + partitionedApi(strandedPartitions([tab('tab-runtime')], [tab('tab-local', { ptyId: null })])), repos ) expect(read.session.tabsByWorktree[WORKTREE_ID]?.map((entry) => entry.id)).toEqual([ - 'tab-local' + 'tab-runtime' ]) + expect(read.session.activeTabIdByWorktree?.[WORKTREE_ID]).toBe('tab-runtime') + expect(read.contestedPrimaryHostBySessionKey[WORKTREE_ID]).toBe(SSH_HOST_ID) }) - it("leaves that workspace's other rows alone as well", async () => { + it('keeps a local tab that still names a PTY the host never listed', async () => { const read = await fetchWorkspaceSessionWithRuntimeHostOwners( partitionedApi(strandedPartitions([tab('tab-runtime')], [tab('tab-local')])), repos ) - expect(read.session.activeTabIdByWorktree?.[WORKTREE_ID]).toBeUndefined() + expect(read.session.tabsByWorktree[WORKTREE_ID]?.map((entry) => entry.id)).toEqual([ + 'tab-runtime', + 'tab-local' + ]) }) it('routes the reunited workspace back to the partition that owns it', async () => { diff --git a/src/shared/workspace-session-base-tab-carryover.ts b/src/shared/workspace-session-base-tab-carryover.ts new file mode 100644 index 000000000000..1e3964610618 --- /dev/null +++ b/src/shared/workspace-session-base-tab-carryover.ts @@ -0,0 +1,102 @@ +import type { Tab } from './tab-types' +import type { TerminalTab } from './terminal-tab-types' +import type { WorkspaceSessionState } from './workspace-session-state-types' +import { normalizeWorkspaceSessionKeyToWorkspaceId } from './workspace-scope' + +/** + * Base tabs kept when the host's populated row replaces the base's (#22503, #23390). + * + * Kept: not listed or tombstoned by the host, and either still names a PTY or was created after + * the host's newest evidence for this workspace. Why the time bound: a tab parked on an unresolved + * host has no PTY yet and leaves no persisted marker; legacy residue is older than the host's rows. + */ +export function baseTabsTheHostNeverListed( + baseTabs: readonly TerminalTab[] | undefined, + hostTabs: readonly TerminalTab[], + base: WorkspaceSessionState, + host: WorkspaceSessionState, + workspaceKey: string +): TerminalTab[] { + if (!baseTabs || baseTabs.length === 0) { + return [] + } + const listed = new Set(hostTabs.map((tab) => tab.id)) + const closed = (tabId: string): boolean => + Object.hasOwn(host.closedTerminalTabTombstonesByTabId ?? {}, tabId) || + Object.hasOwn(base.closedTerminalTabTombstonesByTabId ?? {}, tabId) + const hostSeenUntil = newestHostEvidenceAt(hostTabs, host, workspaceKey) + return baseTabs.filter( + (tab) => + !listed.has(tab.id) && !closed(tab.id) && (!!tab.ptyId || tab.createdAt > hostSeenUntil) + ) +} + +function newestHostEvidenceAt( + hostTabs: readonly TerminalTab[], + host: WorkspaceSessionState, + workspaceKey: string +): number { + const workspaceId = normalizeWorkspaceSessionKeyToWorkspaceId(workspaceKey) + let newest = 0 + for (const tab of hostTabs) { + newest = Math.max(newest, tab.createdAt) + } + for (const tombstone of Object.values(host.closedTerminalTabTombstonesByTabId ?? {})) { + if (normalizeWorkspaceSessionKeyToWorkspaceId(tombstone.worktreeId) === workspaceId) { + newest = Math.max(newest, tombstone.closedAt) + } + } + return newest +} + +/** + * Give each carried terminal a unified entry: a unified-format session renders only terminals listed + * there, and a bare row hydrates with no surface and loses its PTY lease (#23390). Group repair + * during hydration re-homes an entry whose group does not exist. + */ +export function attachCarriedTabsToUnifiedRows( + next: WorkspaceSessionState, + base: WorkspaceSessionState, + host: WorkspaceSessionState, + carriedByKey: ReadonlyMap +): void { + if (carriedByKey.size === 0 || !next.unifiedTabs) { + return + } + const unifiedTabs = { ...next.unifiedTabs } + for (const [key, carried] of carriedByKey) { + const row = unifiedTabs[key] ?? [] + const present = new Set(row.map((tab) => tab.entityId)) + const groupId = + host.activeGroupIdByWorktree?.[key] ?? host.tabGroups?.[key]?.[0]?.id ?? `carried:${key}` + const additions: Tab[] = [] + for (const terminal of carried) { + if (present.has(terminal.id)) { + continue + } + const own = base.unifiedTabs?.[key]?.find( + (tab) => tab.contentType === 'terminal' && tab.entityId === terminal.id + ) + additions.push(own ? { ...own, groupId } : unifiedEntryFor(terminal, key, groupId)) + } + if (additions.length > 0) { + unifiedTabs[key] = [...row, ...additions] + } + } + next.unifiedTabs = unifiedTabs +} + +function unifiedEntryFor(terminal: TerminalTab, worktreeId: string, groupId: string): Tab { + return { + id: terminal.id, + entityId: terminal.id, + groupId, + worktreeId, + contentType: 'terminal', + label: terminal.title, + customLabel: terminal.customTitle, + color: terminal.color, + sortOrder: terminal.sortOrder, + createdAt: terminal.createdAt + } +} diff --git a/src/shared/workspace-session-stranded-partition-adoption.test.ts b/src/shared/workspace-session-stranded-partition-adoption.test.ts new file mode 100644 index 000000000000..e499232ca382 --- /dev/null +++ b/src/shared/workspace-session-stranded-partition-adoption.test.ts @@ -0,0 +1,244 @@ +import { describe, expect, it } from 'vitest' +import { getDefaultWorkspaceSession } from './constants' +import type { Tab, TabGroup } from './tab-types' +import type { TerminalTab } from './terminal-tab-types' +import type { WorkspaceSessionState } from './workspace-session-state-types' +import { adoptStrandedHostPartitionSession } from './workspace-session-stranded-partition-adoption' + +const WORKTREE_ID = 'repo-remote::/remote/checkout/feature' + +function tab(id: string, ptyId: string | null = `pty-${id}`): TerminalTab { + return { + id, + ptyId, + worktreeId: WORKTREE_ID, + title: id, + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + } +} + +function unified(id: string, groupId: string): Tab { + return { + id, + entityId: id, + groupId, + worktreeId: WORKTREE_ID, + contentType: 'terminal', + label: id, + customLabel: null, + color: null, + sortOrder: 0, + createdAt: 1 + } +} + +function group(id: string, tabOrder: string[]): TabGroup { + return { + id, + worktreeId: WORKTREE_ID, + activeTabId: tabOrder[0] ?? null, + tabOrder, + recentTabIds: [] + } +} + +function session(overrides: Partial): WorkspaceSessionState { + return { ...getDefaultWorkspaceSession(), ...overrides } +} + +function tombstones( + ...tabIds: string[] +): WorkspaceSessionState['closedTerminalTabTombstonesByTabId'] { + return Object.fromEntries(tabIds.map((id) => [id, { closedAt: 1, worktreeId: WORKTREE_ID }])) +} + +type OwnershipCase = { + name: string + base: TerminalTab[] + /** null: the host names the workspace but holds no tab row for it. */ + host: TerminalTab[] | null + hostTombstones: string[] + expected: string[] + adopted: boolean +} + +const OWNERSHIP_CASES: OwnershipCase[] = [ + { + name: 'local empty, host populated: adopts the host tabs', + base: [], + host: [tab('h1')], + hostTombstones: [], + expected: ['h1'], + adopted: true + }, + { + name: 'both populated with disjoint ids: host replaces the PTY-less local leftovers', + base: [tab('l1', null), tab('l2', null)], + host: [tab('h1'), tab('h2')], + hostTombstones: [], + expected: ['h1', 'h2'], + adopted: true + }, + { + name: 'both hold the same tab: the host copy wins', + base: [{ ...tab('shared'), title: 'Terminal 3' }], + host: [{ ...tab('shared'), title: 'claude' }], + hostTombstones: [], + expected: ['shared'], + adopted: true + }, + { + name: 'a local tab the host tombstoned never comes back, even with a PTY', + base: [tab('closed')], + host: [tab('h1')], + hostTombstones: ['closed'], + expected: ['h1'], + adopted: true + }, + { + name: 'a local tab that still names a PTY the host never listed is kept', + base: [tab('live-local')], + host: [tab('h1')], + hostTombstones: [], + expected: ['h1', 'live-local'], + adopted: true + }, + { + name: 'a PTY-less local tab newer than every host row is kept (parked on an unresolved host)', + base: [{ ...tab('parked', null), createdAt: 50 }], + host: [{ ...tab('h1'), createdAt: 10 }], + hostTombstones: [], + expected: ['h1', 'parked'], + adopted: true + }, + { + name: 'PTY-less local tabs older than the host rows are residue and dropped (#22503)', + base: [ + { ...tab('old1', null), createdAt: 5 }, + { ...tab('old2', null), createdAt: 9 } + ], + host: [{ ...tab('h1'), createdAt: 10 }], + hostTombstones: [], + expected: ['h1'], + adopted: true + }, + { + name: 'a tombstone beats a newer PTY-less local tab', + base: [{ ...tab('parked', null), createdAt: 50 }], + host: [{ ...tab('h1'), createdAt: 10 }], + hostTombstones: ['parked'], + expected: ['h1'], + adopted: true + }, + { + name: 'only local populated: the base keeps its own copy', + base: [tab('l1')], + host: null, + hostTombstones: [], + expected: ['l1'], + adopted: false + }, + { + name: 'host row explicitly empty: the base keeps its tabs', + base: [tab('l1')], + host: [], + hostTombstones: [], + expected: ['l1'], + adopted: false + } +] + +describe('adoptStrandedHostPartitionSession terminal tab ownership', () => { + it.each(OWNERSHIP_CASES)('$name', ({ base, host, hostTombstones, expected, adopted }) => { + const hostSession = session({ + tabsByWorktree: host === null ? {} : { [WORKTREE_ID]: host }, + // Names the workspace even when it holds no tab row, as an editor-only host row would. + activeTabTypeByWorktree: { [WORKTREE_ID]: 'terminal' }, + closedTerminalTabTombstonesByTabId: tombstones(...hostTombstones) + }) + + const result = adoptStrandedHostPartitionSession( + session({ tabsByWorktree: { [WORKTREE_ID]: base } }), + hostSession + ) + + expect(result.session.tabsByWorktree[WORKTREE_ID]?.map((entry) => entry.id)).toEqual(expected) + expect(result.adoptedWorkspaceIds.has(WORKTREE_ID)).toBe(adopted) + }) + + it('bounds a PTY-less local tab by the newest host close, not only the newest host tab', () => { + const result = adoptStrandedHostPartitionSession( + session({ tabsByWorktree: { [WORKTREE_ID]: [{ ...tab('l1', null), createdAt: 20 }] } }), + session({ + tabsByWorktree: { [WORKTREE_ID]: [{ ...tab('h1'), createdAt: 10 }] }, + closedTerminalTabTombstonesByTabId: { + other: { closedAt: 30, worktreeId: WORKTREE_ID } + } + }) + ) + + expect(result.session.tabsByWorktree[WORKTREE_ID]?.map((entry) => entry.id)).toEqual(['h1']) + }) + + it('keeps the host title for a tab both partitions hold', () => { + const result = adoptStrandedHostPartitionSession( + session({ tabsByWorktree: { [WORKTREE_ID]: [{ ...tab('shared'), title: 'Terminal 3' }] } }), + session({ tabsByWorktree: { [WORKTREE_ID]: [{ ...tab('shared'), title: 'claude' }] } }) + ) + + expect(result.session.tabsByWorktree[WORKTREE_ID]?.[0]?.title).toBe('claude') + }) + + it('takes the host unified rows over a local copy that has none (#23390)', () => { + const result = adoptStrandedHostPartitionSession( + session({ + tabsByWorktree: { [WORKTREE_ID]: [tab('agent')] }, + unifiedTabs: {}, + tabGroups: {} + }), + session({ + tabsByWorktree: { [WORKTREE_ID]: [tab('agent')] }, + unifiedTabs: { [WORKTREE_ID]: [unified('agent', 'g1')] }, + tabGroups: { [WORKTREE_ID]: [group('g1', ['agent'])] } + }) + ) + + expect(result.session.unifiedTabs?.[WORKTREE_ID]?.map((entry) => entry.entityId)).toEqual([ + 'agent' + ]) + expect(result.session.tabGroups?.[WORKTREE_ID]?.[0]?.tabOrder).toEqual(['agent']) + }) + + it('gives a carried local terminal a unified entry in the host group', () => { + const result = adoptStrandedHostPartitionSession( + session({ tabsByWorktree: { [WORKTREE_ID]: [tab('live-local')] }, unifiedTabs: {} }), + session({ + tabsByWorktree: { [WORKTREE_ID]: [tab('h1')] }, + unifiedTabs: { [WORKTREE_ID]: [unified('h1', 'g1')] }, + tabGroups: { [WORKTREE_ID]: [group('g1', ['h1'])] }, + activeGroupIdByWorktree: { [WORKTREE_ID]: 'g1' } + }) + ) + + expect( + result.session.unifiedTabs?.[WORKTREE_ID]?.map((entry) => [entry.entityId, entry.groupId]) + ).toEqual([ + ['h1', 'g1'], + ['live-local', 'g1'] + ]) + }) + + it('still lets a contested id keep its own populated base row', () => { + const result = adoptStrandedHostPartitionSession( + session({ tabsByWorktree: { [WORKTREE_ID]: [tab('l1', null)] } }), + session({ tabsByWorktree: { [WORKTREE_ID]: [tab('h1')] } }), + { contestedSessionKeys: new Set([WORKTREE_ID]) } + ) + + expect(result.session.tabsByWorktree[WORKTREE_ID]?.map((entry) => entry.id)).toEqual(['l1']) + expect(result.adoptedWorkspaceIds.has(WORKTREE_ID)).toBe(false) + }) +}) diff --git a/src/shared/workspace-session-stranded-partition-adoption.ts b/src/shared/workspace-session-stranded-partition-adoption.ts index 8be7e1f81eb3..c3745bbfc4db 100644 --- a/src/shared/workspace-session-stranded-partition-adoption.ts +++ b/src/shared/workspace-session-stranded-partition-adoption.ts @@ -10,6 +10,11 @@ import { buildWorktreeIdByTabId, worktreeIdForPaneKey } from './workspace-session-host-records' +import type { TerminalTab } from './terminal-tab-types' +import { + attachCarriedTabsToUnifiedRows, + baseTabsTheHostNeverListed +} from './workspace-session-base-tab-carryover' /** * Fold rows a host partition holds alone back into the session the readers assemble. @@ -43,11 +48,12 @@ import { * routes those back to that partition directly, so a row does not depend on a repo catalog naming * the host before it can be returned to where it was read from. * - * The one thing the base keeps unconditionally is a workspace it holds **terminal tabs** for. That - * is the live copy the user is looking at, and merging a stale partition into it would re-add tabs - * they had closed on every launch. Leaving it alone keeps this a one-shot repair, at the cost of - * not recovering rows stranded beside a populated workspace — which are stranded on main today too, - * so it is never a new loss. An EMPTY tab row is not such a copy: an empty list is not evidence + * The base keeps a workspace it holds **terminal tabs** for only while the host partition holds + * none. When both hold tabs the base row is the leftover — `local` is never pruned for an SSH + * workspace, so letting it win revived closed tabs and hid live ones (#22503, #23390) — and the + * host row replaces it; `baseTabsTheHostNeverListed` picks the base tabs that survive. An EMPTY + * host row is not ownership (it may be #12721's poisoned list), so the base keeps its tabs. + * An EMPTY base tab row is not a live copy either: an empty list is not evidence * 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 @@ -80,15 +86,25 @@ function browserPagesWorkspaceId(entry: unknown): string | null { return recordWorkspaceId(first) } -/** Workspaces the base holds terminal tabs for: its live copies, which adoption never touches. */ -function workspacesTheBaseOwns(base: WorkspaceSessionState): Set { - const owned = new Set() - for (const [key, tabs] of Object.entries(base.tabsByWorktree ?? {})) { +function workspacesWithTerminalTabs(session: WorkspaceSessionState): Set { + const populated = new Set() + for (const [key, tabs] of Object.entries(session.tabsByWorktree ?? {})) { if (Array.isArray(tabs) && tabs.length > 0) { - owned.add(normalizeWorkspaceSessionKeyToWorkspaceId(key)) + populated.add(normalizeWorkspaceSessionKeyToWorkspaceId(key)) } } - return owned + return populated +} + +/** Workspaces the base holds terminal tabs for and the host holds none: the base's own copies. */ +function workspacesTheBaseOwns( + base: WorkspaceSessionState, + host: WorkspaceSessionState +): Set { + const hostPopulated = workspacesWithTerminalTabs(host) + return new Set( + [...workspacesWithTerminalTabs(base)].filter((workspaceId) => !hostPopulated.has(workspaceId)) + ) } /** @@ -108,7 +124,7 @@ function adoptableWorkspaceIds( base: WorkspaceSessionState, host: WorkspaceSessionState ): Set { - const owned = workspacesTheBaseOwns(base) + const owned = workspacesTheBaseOwns(base, host) return collectWorkspaceIds(host, (workspaceId) => owned.has(workspaceId)) } @@ -295,6 +311,7 @@ export function adoptStrandedHostPartitionSession( contested.has(normalizeWorkspaceSessionKeyToWorkspaceId(key)) const next: WorkspaceSessionState = { ...base, tabsByWorktree: { ...base.tabsByWorktree } } + const carriedByKey = new Map() for (const [key, tabs] of Object.entries(host.tabsByWorktree ?? {})) { if (!adopts(key) || !Array.isArray(tabs)) { continue @@ -304,7 +321,11 @@ export function adoptStrandedHostPartitionSession( // "the base has tabs here" is what let #12721's empty local list win over the host's real one // whenever the id happened to be contested. if (!isContested(key) || hostHasNothingFor(next.tabsByWorktree[key])) { - next.tabsByWorktree[key] = tabs + const carried = baseTabsTheHostNeverListed(next.tabsByWorktree[key], tabs, base, host, key) + next.tabsByWorktree[key] = carried.length > 0 ? [...tabs, ...carried] : tabs + if (carried.length > 0) { + carriedByKey.set(key, carried) + } } } // Why the split's own indexes: they are what decided which partition each tab-, pane- and @@ -385,6 +406,7 @@ export function adoptStrandedHostPartitionSession( break } } + attachCarriedTabsToUnifiedRows(next, base, host, carriedByKey) // 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( From 6e16be701444ffe93e0c4b2c57a6540a8cce15d2 Mon Sep 17 00:00:00 2001 From: tlgnkl <67519432+tlgnkl@users.noreply.github.com> Date: Sun, 27 Sep 2026 13:17:55 +0300 Subject: [PATCH 2/4] fix(session): keep a host-won row's records and unified surface consistent When the host tab row replaces the base's, its tab- and pane-keyed records (terminal layouts, reattach ids, PTY incarnations) now replace the base's for the tabs the host listed, so a live tab is not paired with a dead relay's PTY. The unified row drops the entries of discarded base tabs even when the host supplied no unified row, every surviving terminal gets an entry stamped with its own worktree id, and new entries are appended to the chosen group's tab order. The pane-key parse is shared through tabIdForPaneKey. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../workspace-session-base-tab-carryover.ts | 83 +++++++++++----- src/shared/workspace-session-host-records.ts | 11 ++- ...ession-stranded-partition-adoption.test.ts | 94 ++++++++++++++++++- ...ace-session-stranded-partition-adoption.ts | 39 +++++--- 4 files changed, 190 insertions(+), 37 deletions(-) diff --git a/src/shared/workspace-session-base-tab-carryover.ts b/src/shared/workspace-session-base-tab-carryover.ts index 1e3964610618..971d19a3ba6a 100644 --- a/src/shared/workspace-session-base-tab-carryover.ts +++ b/src/shared/workspace-session-base-tab-carryover.ts @@ -10,7 +10,7 @@ import { normalizeWorkspaceSessionKeyToWorkspaceId } from './workspace-scope' * the host's newest evidence for this workspace. Why the time bound: a tab parked on an unresolved * host has no PTY yet and leaves no persisted marker; legacy residue is older than the host's rows. */ -export function baseTabsTheHostNeverListed( +function baseTabsTheHostNeverListed( baseTabs: readonly TerminalTab[] | undefined, hostTabs: readonly TerminalTab[], base: WorkspaceSessionState, @@ -49,49 +49,90 @@ function newestHostEvidenceAt( return newest } +export type HostWonTabRow = { + tabs: TerminalTab[] + carried: readonly TerminalTab[] + /** Base tab ids the host row replaced and did not carry. */ + discardedTabIds: ReadonlySet +} + +/** The row a populated host tab row leaves behind once it replaces the base's. */ +export function hostWonTabRow( + baseTabs: readonly TerminalTab[] | undefined, + hostTabs: TerminalTab[], + base: WorkspaceSessionState, + host: WorkspaceSessionState, + workspaceKey: string +): HostWonTabRow { + const carried = baseTabsTheHostNeverListed(baseTabs, hostTabs, base, host, workspaceKey) + const tabs = carried.length > 0 ? [...hostTabs, ...carried] : hostTabs + const kept = new Set(tabs.map((terminal) => terminal.id)) + const discardedTabIds = new Set( + (baseTabs ?? []).map((terminal) => terminal.id).filter((id) => !kept.has(id)) + ) + return { tabs, carried, discardedTabIds } +} + /** - * Give each carried terminal a unified entry: a unified-format session renders only terminals listed - * there, and a bare row hydrates with no surface and loses its PTY lease (#23390). Group repair - * during hydration re-homes an entry whose group does not exist. + * Line the unified surface up with a row the host won: a unified-format session renders only the + * terminals listed there, so a tab without an entry loses its PTY lease (#23390), and a discarded + * base tab's entry would keep showing a closed tab when the host supplied no unified row of its own. */ -export function attachCarriedTabsToUnifiedRows( +export function reconcileHostWonUnifiedRows( next: WorkspaceSessionState, base: WorkspaceSessionState, - host: WorkspaceSessionState, - carriedByKey: ReadonlyMap + rowsByKey: ReadonlyMap ): void { - if (carriedByKey.size === 0 || !next.unifiedTabs) { + if (rowsByKey.size === 0 || !next.unifiedTabs) { return } const unifiedTabs = { ...next.unifiedTabs } - for (const [key, carried] of carriedByKey) { - const row = unifiedTabs[key] ?? [] - const present = new Set(row.map((tab) => tab.entityId)) - const groupId = - host.activeGroupIdByWorktree?.[key] ?? host.tabGroups?.[key]?.[0]?.id ?? `carried:${key}` + const tabGroups = next.tabGroups ? { ...next.tabGroups } : undefined + for (const [key, { tabs, carried, discardedTabIds }] of rowsByKey) { + const row = (unifiedTabs[key] ?? []).filter( + (entry) => entry.contentType !== 'terminal' || !discardedTabIds.has(entry.entityId) + ) + const present = new Set(row.map((entry) => entry.entityId)) + const groups = tabGroups?.[key] + const groupId = next.activeGroupIdByWorktree?.[key] ?? groups?.[0]?.id ?? `carried:${key}` + const carriedIds = new Set(carried.map((terminal) => terminal.id)) const additions: Tab[] = [] - for (const terminal of carried) { + for (const terminal of tabs) { if (present.has(terminal.id)) { continue } - const own = base.unifiedTabs?.[key]?.find( - (tab) => tab.contentType === 'terminal' && tab.entityId === terminal.id - ) - additions.push(own ? { ...own, groupId } : unifiedEntryFor(terminal, key, groupId)) + const own = carriedIds.has(terminal.id) + ? base.unifiedTabs?.[key]?.find( + (entry) => entry.contentType === 'terminal' && entry.entityId === terminal.id + ) + : undefined + additions.push(own ? { ...own, groupId } : unifiedEntryFor(terminal, groupId)) } - if (additions.length > 0) { + if (additions.length > 0 || row.length !== (unifiedTabs[key]?.length ?? 0)) { unifiedTabs[key] = [...row, ...additions] } + if (tabGroups && groups) { + const addedIds = additions.map((entry) => entry.id) + tabGroups[key] = groups.map((group) => { + const tabOrder = group.tabOrder.filter((tabId) => !discardedTabIds.has(tabId)) + return group.id === groupId + ? { ...group, tabOrder: [...tabOrder, ...addedIds] } + : { ...group, tabOrder } + }) + } } next.unifiedTabs = unifiedTabs + if (tabGroups) { + next.tabGroups = tabGroups + } } -function unifiedEntryFor(terminal: TerminalTab, worktreeId: string, groupId: string): Tab { +function unifiedEntryFor(terminal: TerminalTab, groupId: string): Tab { return { id: terminal.id, entityId: terminal.id, groupId, - worktreeId, + worktreeId: terminal.worktreeId, contentType: 'terminal', label: terminal.title, customLabel: terminal.customTitle, diff --git a/src/shared/workspace-session-host-records.ts b/src/shared/workspace-session-host-records.ts index 68867cffe8ff..125c37119230 100644 --- a/src/shared/workspace-session-host-records.ts +++ b/src/shared/workspace-session-host-records.ts @@ -54,14 +54,19 @@ export function extendWorktreeIdByTabId( return byTab } -/** The workspace a pane key belongs to. A pane key is `:`; both the split and the +/** The tab a pane key belongs to. A pane key is `:`; both the split and the * stranded-partition adoption resolve it here so neither can parse it its own way. */ +export function tabIdForPaneKey(paneKey: string): string | undefined { + const separator = paneKey.lastIndexOf(':') + return separator > 0 ? paneKey.slice(0, separator) : undefined +} + export function worktreeIdForPaneKey( worktreeIdByTabId: Map, paneKey: string ): string | undefined { - const separator = paneKey.lastIndexOf(':') - return separator > 0 ? worktreeIdByTabId.get(paneKey.slice(0, separator)) : undefined + const tabId = tabIdForPaneKey(paneKey) + return tabId === undefined ? undefined : worktreeIdByTabId.get(tabId) } export function buildWorktreeIdByFileId(state: WorkspaceSessionState): Map { diff --git a/src/shared/workspace-session-stranded-partition-adoption.test.ts b/src/shared/workspace-session-stranded-partition-adoption.test.ts index e499232ca382..f3777cb3e405 100644 --- a/src/shared/workspace-session-stranded-partition-adoption.test.ts +++ b/src/shared/workspace-session-stranded-partition-adoption.test.ts @@ -1,9 +1,10 @@ import { describe, expect, it } from 'vitest' import { getDefaultWorkspaceSession } from './constants' import type { Tab, TabGroup } from './tab-types' -import type { TerminalTab } from './terminal-tab-types' +import type { TerminalLayoutSnapshot, TerminalTab } from './terminal-tab-types' import type { WorkspaceSessionState } from './workspace-session-state-types' import { adoptStrandedHostPartitionSession } from './workspace-session-stranded-partition-adoption' +import { worktreeWorkspaceKey } from './workspace-scope' const WORKTREE_ID = 'repo-remote::/remote/checkout/feature' @@ -242,3 +243,94 @@ describe('adoptStrandedHostPartitionSession terminal tab ownership', () => { expect(result.adoptedWorkspaceIds.has(WORKTREE_ID)).toBe(false) }) }) + +describe('adoptStrandedHostPartitionSession host-won tab records', () => { + function layout(ptyId: string): TerminalLayoutSnapshot { + return { + root: { type: 'leaf', leafId: 'leaf' }, + activeLeafId: 'leaf', + expandedLeafId: null, + ptyIdsByLeafId: { leaf: ptyId } + } + } + + it('takes the host tab-keyed and pane-keyed records for a tab the host copy wins', () => { + const result = adoptStrandedHostPartitionSession( + session({ + tabsByWorktree: { [WORKTREE_ID]: [tab('shared'), tab('live-local')] }, + terminalLayoutsByTabId: { + shared: layout('stale-pty'), + 'live-local': layout('local-pty') + }, + remoteSessionIdsByTabId: { shared: 'stale-session', 'live-local': 'local-session' }, + terminalPtyIncarnationsByPaneKey: { 'shared:leaf': 'stale-incarnation' } + }), + session({ + tabsByWorktree: { [WORKTREE_ID]: [tab('shared')] }, + terminalLayoutsByTabId: { shared: layout('live-pty') }, + remoteSessionIdsByTabId: { shared: 'live-session' }, + terminalPtyIncarnationsByPaneKey: { 'shared:leaf': 'live-incarnation' } + }) + ) + + expect(result.session.terminalLayoutsByTabId.shared?.ptyIdsByLeafId).toEqual({ + leaf: 'live-pty' + }) + expect(result.session.terminalLayoutsByTabId['live-local']?.ptyIdsByLeafId).toEqual({ + leaf: 'local-pty' + }) + expect(result.session.remoteSessionIdsByTabId).toEqual({ + shared: 'live-session', + 'live-local': 'local-session' + }) + expect(result.session.terminalPtyIncarnationsByPaneKey).toEqual({ + 'shared:leaf': 'live-incarnation' + }) + }) + + it('adds a carried terminal to the host group tab order', () => { + const result = adoptStrandedHostPartitionSession( + session({ tabsByWorktree: { [WORKTREE_ID]: [tab('live-local')] }, unifiedTabs: {} }), + session({ + tabsByWorktree: { [WORKTREE_ID]: [tab('h1')] }, + unifiedTabs: { [WORKTREE_ID]: [unified('h1', 'g1')] }, + tabGroups: { [WORKTREE_ID]: [group('g1', ['h1'])] }, + activeGroupIdByWorktree: { [WORKTREE_ID]: 'g1' } + }) + ) + + expect(result.session.tabGroups?.[WORKTREE_ID]?.[0]?.tabOrder).toEqual(['h1', 'live-local']) + }) + + it("stamps a synthesized unified entry with the terminal's own worktree id", () => { + const key = worktreeWorkspaceKey(WORKTREE_ID) + const result = adoptStrandedHostPartitionSession( + session({ tabsByWorktree: { [key]: [tab('live-local')] }, unifiedTabs: {} }), + session({ + tabsByWorktree: { [key]: [tab('h1')] }, + unifiedTabs: { [key]: [{ ...unified('h1', 'g1') }] }, + tabGroups: { [key]: [group('g1', ['h1'])] } + }) + ) + + const carried = result.session.unifiedTabs?.[key]?.find((e) => e.entityId === 'live-local') + expect(carried?.worktreeId).toBe(WORKTREE_ID) + }) + + it('drops the unified entries of discarded local tabs when the host holds no unified row', () => { + const result = adoptStrandedHostPartitionSession( + session({ + tabsByWorktree: { [WORKTREE_ID]: [tab('old', null)] }, + unifiedTabs: { [WORKTREE_ID]: [unified('old', 'g-local')] }, + tabGroups: { [WORKTREE_ID]: [group('g-local', ['old'])] }, + activeGroupIdByWorktree: { [WORKTREE_ID]: 'g-local' } + }), + session({ tabsByWorktree: { [WORKTREE_ID]: [{ ...tab('h1'), createdAt: 10 }] } }) + ) + + expect( + result.session.unifiedTabs?.[WORKTREE_ID]?.map((entry) => [entry.entityId, entry.groupId]) + ).toEqual([['h1', 'g-local']]) + expect(result.session.tabGroups?.[WORKTREE_ID]?.[0]?.tabOrder).toEqual(['h1']) + }) +}) diff --git a/src/shared/workspace-session-stranded-partition-adoption.ts b/src/shared/workspace-session-stranded-partition-adoption.ts index c3745bbfc4db..fc3522212bed 100644 --- a/src/shared/workspace-session-stranded-partition-adoption.ts +++ b/src/shared/workspace-session-stranded-partition-adoption.ts @@ -8,12 +8,13 @@ import { isWorktreeHostIdentity as isHostQualifiedSessionKey } from './worktree/ import { buildWorktreeIdByFileId, buildWorktreeIdByTabId, + tabIdForPaneKey, worktreeIdForPaneKey } from './workspace-session-host-records' -import type { TerminalTab } from './terminal-tab-types' import { - attachCarriedTabsToUnifiedRows, - baseTabsTheHostNeverListed + hostWonTabRow, + reconcileHostWonUnifiedRows, + type HostWonTabRow } from './workspace-session-base-tab-carryover' /** @@ -311,7 +312,8 @@ export function adoptStrandedHostPartitionSession( contested.has(normalizeWorkspaceSessionKeyToWorkspaceId(key)) const next: WorkspaceSessionState = { ...base, tabsByWorktree: { ...base.tabsByWorktree } } - const carriedByKey = new Map() + const hostWonRows = new Map() + const hostWonTabIds = new Set() for (const [key, tabs] of Object.entries(host.tabsByWorktree ?? {})) { if (!adopts(key) || !Array.isArray(tabs)) { continue @@ -321,10 +323,11 @@ export function adoptStrandedHostPartitionSession( // "the base has tabs here" is what let #12721's empty local list win over the host's real one // whenever the id happened to be contested. if (!isContested(key) || hostHasNothingFor(next.tabsByWorktree[key])) { - const carried = baseTabsTheHostNeverListed(next.tabsByWorktree[key], tabs, base, host, key) - next.tabsByWorktree[key] = carried.length > 0 ? [...tabs, ...carried] : tabs - if (carried.length > 0) { - carriedByKey.set(key, carried) + const row = hostWonTabRow(next.tabsByWorktree[key], tabs, base, host, key) + next.tabsByWorktree[key] = row.tabs + hostWonRows.set(key, row) + for (const terminal of tabs) { + hostWonTabIds.add(terminal.id) } } } @@ -377,12 +380,24 @@ export function adoptStrandedHostPartitionSession( } break } + // Why replace for a host-won tab: its host row is the live copy, and a base layout or + // reattach id beside it would pair the live tab with a dead relay's PTY. case 'tabKeyed': - adoptRecord(next, host, field, (key) => adoptsResolved(worktreeIdByTabId.get(key))) + adoptRecord( + next, + host, + field, + (key) => adoptsResolved(worktreeIdByTabId.get(key)), + (key) => hostWonTabIds.has(key) + ) break case 'paneKeyed': - adoptRecord(next, host, field, (key) => - adoptsResolved(worktreeIdForPaneKey(worktreeIdByTabId, key)) + adoptRecord( + next, + host, + field, + (key) => adoptsResolved(worktreeIdForPaneKey(worktreeIdByTabId, key)), + (key) => hostWonTabIds.has(tabIdForPaneKey(key) ?? '') ) break case 'sleepingAgentKeyed': @@ -406,7 +421,7 @@ export function adoptStrandedHostPartitionSession( break } } - attachCarriedTabsToUnifiedRows(next, base, host, carriedByKey) + reconcileHostWonUnifiedRows(next, base, hostWonRows) // 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( From 41b423544dc5edb6c43e5538963befa255ac88f3 Mon Sep 17 00:00:00 2001 From: tlgnkl <67519432+tlgnkl@users.noreply.github.com> Date: Sun, 27 Sep 2026 21:33:44 +0300 Subject: [PATCH 3/4] fix(session): carry unsaved base drafts and newer browsers into a host-won row When a populated host row replaces the base's, the base-only entries of `openFilesByWorktree` and `browserTabsByWorktree` were dropped wholesale, including an unsaved `dirtyDraftContent` that nothing else can restore. The host copy still wins every entry it lists; a base-only editor file survives when it holds an unsaved draft, and a base-only browser workspace survives under the terminal tabs' time bound. Carried entries get a unified surface like carried terminals do. The unified reconcile now files additions under a group that exists, and creates that group when the workspace has none, instead of pointing entries at a `carried:` group missing from `tabGroups`. Co-Authored-By: Claude Opus 5.5 (1M context) --- ...e-session-ssh-partition-round-trip.test.ts | 34 +- ...rkspace-session-base-row-carryover.test.ts | 308 ++++++++++++++++++ .../workspace-session-base-row-carryover.ts | 154 +++++++++ .../workspace-session-base-tab-carryover.ts | 142 ++++++-- ...ace-session-stranded-partition-adoption.ts | 8 +- 5 files changed, 609 insertions(+), 37 deletions(-) create mode 100644 src/shared/workspace-session-base-row-carryover.test.ts create mode 100644 src/shared/workspace-session-base-row-carryover.ts 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 06d75c3239eb..c77320c96f9e 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 @@ -328,10 +328,7 @@ describe('ssh host partition rows the host has nothing for', () => { ) }) - it('still adopts a populated host row over the base leftovers', 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. - const partitions = emptyHostRowsOverBaseDraft(false) + function withHostOpenFile(partitions: ReturnType) { partitions[SSH_HOST_ID] = session({ ...partitions[SSH_HOST_ID], openFilesByWorktree: { @@ -345,6 +342,17 @@ describe('ssh host partition rows the host has nothing for', () => { ] } }) + return partitions + } + + it('still adopts a populated host row over the base leftovers', 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. + const partitions = withHostOpenFile(emptyHostRowsOverBaseDraft(false)) + const [baseFile] = partitions.local.openFilesByWorktree?.[WORKTREE_ID] ?? [] + partitions.local.openFilesByWorktree = { + [WORKTREE_ID]: [{ ...baseFile, dirtyDraftContent: undefined }] + } const read = await fetchWorkspaceSessionWithRuntimeHostOwners(partitionedApi(partitions), repos) @@ -353,6 +361,24 @@ describe('ssh host partition rows the host has nothing for', () => { ).toEqual(['src/host.ts']) }) + it('keeps an unsaved draft the populated host row does not list', async () => { + // Replacing the row must not be how a draft dies: nothing downstream can recover it. + const read = await fetchWorkspaceSessionWithRuntimeHostOwners( + partitionedApi(withHostOpenFile(emptyHostRowsOverBaseDraft(false))), + repos + ) + + expect( + read.session.openFilesByWorktree?.[WORKTREE_ID]?.map((file) => [ + file.relativePath, + file.dirtyDraftContent + ]) + ).toEqual([ + ['src/host.ts', undefined], + ['src/main.ts', 'unsaved work'] + ]) + }) + it('adopts the layout of a tab the host slice names only in unifiedTabs', async () => { // `buildWorktreeIdByTabId` — the index the split routes by — resolves unified-only tabs as well // as `tabsByWorktree` ones, so their tab-keyed rows are written to this partition. A read that diff --git a/src/shared/workspace-session-base-row-carryover.test.ts b/src/shared/workspace-session-base-row-carryover.test.ts new file mode 100644 index 000000000000..69ddd87af18e --- /dev/null +++ b/src/shared/workspace-session-base-row-carryover.test.ts @@ -0,0 +1,308 @@ +import { describe, expect, it } from 'vitest' +import type { BrowserWorkspace } from './browser-workspace-types' +import { getDefaultWorkspaceSession } from './constants' +import type { Tab, TabGroup } from './tab-types' +import type { TerminalTab } from './terminal-tab-types' +import type { PersistedOpenFile, WorkspaceSessionState } from './workspace-session-state-types' +import { adoptStrandedHostPartitionSession } from './workspace-session-stranded-partition-adoption' + +const WORKTREE_ID = 'repo-remote::/remote/checkout/feature' + +function tab(id: string, createdAt = 1): TerminalTab { + return { + id, + ptyId: `pty-${id}`, + worktreeId: WORKTREE_ID, + title: id, + customTitle: null, + color: null, + sortOrder: 0, + createdAt + } +} + +function file(name: string, overrides: Partial = {}): PersistedOpenFile { + return { + filePath: `/remote/checkout/feature/${name}`, + relativePath: name, + worktreeId: WORKTREE_ID, + language: 'typescript', + ...overrides + } +} + +function browser(id: string, createdAt: number): BrowserWorkspace { + return { + id, + worktreeId: WORKTREE_ID, + url: `https://example.test/${id}`, + title: id, + loading: false, + faviconUrl: null, + canGoBack: false, + canGoForward: false, + loadError: null, + createdAt + } +} + +function unified(id: string, groupId: string, contentType: Tab['contentType'] = 'terminal'): Tab { + return { + id, + entityId: id, + groupId, + worktreeId: WORKTREE_ID, + contentType, + label: id, + customLabel: null, + color: null, + sortOrder: 0, + createdAt: 1 + } +} + +function group(id: string, tabOrder: string[]): TabGroup { + return { id, worktreeId: WORKTREE_ID, activeTabId: tabOrder[0] ?? null, tabOrder } +} + +function session(overrides: Partial): WorkspaceSessionState { + return { ...getDefaultWorkspaceSession(), ...overrides } +} + +/** Both partitions hold terminal tabs, so the host row wins the workspace (#22503). */ +function hostWon( + base: Partial, + host: Partial, + options?: Parameters[2] +): WorkspaceSessionState { + return adoptStrandedHostPartitionSession( + session({ tabsByWorktree: { [WORKTREE_ID]: [tab('l1')] }, ...base }), + session({ tabsByWorktree: { [WORKTREE_ID]: [tab('l1')] }, ...host }), + options + ).session +} + +describe('adoptStrandedHostPartitionSession editor files in a host-won row', () => { + it('keeps a base-only unsaved draft and drops a base-only clean file', () => { + const result = hostWon( + { + openFilesByWorktree: { + [WORKTREE_ID]: [file('draft.ts', { dirtyDraftContent: 'unsaved' }), file('clean.ts')] + } + }, + { openFilesByWorktree: { [WORKTREE_ID]: [file('host.ts')] } } + ) + + expect( + result.openFilesByWorktree?.[WORKTREE_ID]?.map((entry) => [ + entry.relativePath, + entry.dirtyDraftContent + ]) + ).toEqual([ + ['host.ts', undefined], + ['draft.ts', 'unsaved'] + ]) + }) + + it('lets the host copy win a file both rows list', () => { + const result = hostWon( + { openFilesByWorktree: { [WORKTREE_ID]: [file('same.ts', { dirtyDraftContent: 'stale' })] } }, + { openFilesByWorktree: { [WORKTREE_ID]: [file('same.ts')] } } + ) + + expect(result.openFilesByWorktree?.[WORKTREE_ID]).toEqual([file('same.ts')]) + }) + + it('treats the same path under another runtime as a different file', () => { + const result = hostWon( + { + openFilesByWorktree: { + [WORKTREE_ID]: [ + file('same.ts', { runtimeEnvironmentId: 'env-1', dirtyDraftContent: 'unsaved' }) + ] + } + }, + { openFilesByWorktree: { [WORKTREE_ID]: [file('same.ts')] } } + ) + + expect( + result.openFilesByWorktree?.[WORKTREE_ID]?.map((entry) => entry.runtimeEnvironmentId) + ).toEqual([undefined, 'env-1']) + }) + + it('does not carry a read-only file, whose draft hydration discards anyway', () => { + const result = hostWon( + { + openFilesByWorktree: { + [WORKTREE_ID]: [file('log.txt', { readOnly: true, dirtyDraftContent: 'x' })] + } + }, + { openFilesByWorktree: { [WORKTREE_ID]: [file('host.ts')] } } + ) + + expect(result.openFilesByWorktree?.[WORKTREE_ID]?.map((entry) => entry.relativePath)).toEqual([ + 'host.ts' + ]) + }) + + it('gives a carried draft an editor entry at the end of the host group', () => { + const draft = file('draft.ts', { dirtyDraftContent: 'unsaved' }) + const result = hostWon( + { openFilesByWorktree: { [WORKTREE_ID]: [draft] } }, + { + openFilesByWorktree: { [WORKTREE_ID]: [file('host.ts')] }, + unifiedTabs: { + [WORKTREE_ID]: [ + unified('l1', 'g1'), + { ...unified(file('host.ts').filePath, 'g1', 'editor'), sortOrder: 4 } + ] + }, + tabGroups: { [WORKTREE_ID]: [group('g1', ['l1', file('host.ts').filePath])] } + } + ) + + const entry = result.unifiedTabs?.[WORKTREE_ID]?.find((tab) => tab.entityId === draft.filePath) + expect(entry).toMatchObject({ + id: draft.filePath, + contentType: 'editor', + groupId: 'g1', + worktreeId: WORKTREE_ID, + label: 'draft.ts', + sortOrder: 5 + }) + expect(result.tabGroups?.[WORKTREE_ID]?.[0]?.tabOrder).toEqual([ + 'l1', + file('host.ts').filePath, + draft.filePath + ]) + }) + + it('carries a draft even when the base held no terminal tabs for the workspace', () => { + const result = adoptStrandedHostPartitionSession( + session({ + openFilesByWorktree: { [WORKTREE_ID]: [file('draft.ts', { dirtyDraftContent: 'x' })] } + }), + session({ openFilesByWorktree: { [WORKTREE_ID]: [file('host.ts')] } }) + ).session + + expect(result.openFilesByWorktree?.[WORKTREE_ID]?.map((entry) => entry.relativePath)).toEqual([ + 'host.ts', + 'draft.ts' + ]) + }) + + it('leaves a contested id to its own base row', () => { + const draft = file('draft.ts', { dirtyDraftContent: 'unsaved' }) + const result = hostWon( + { openFilesByWorktree: { [WORKTREE_ID]: [draft] } }, + { openFilesByWorktree: { [WORKTREE_ID]: [file('host.ts')] } }, + { contestedSessionKeys: new Set([WORKTREE_ID]) } + ) + + expect(result.openFilesByWorktree?.[WORKTREE_ID]).toEqual([draft]) + }) +}) + +describe('adoptStrandedHostPartitionSession browser workspaces in a host-won row', () => { + it('keeps a base-only browser created after the host evidence and drops older residue', () => { + const result = hostWon( + { browserTabsByWorktree: { [WORKTREE_ID]: [browser('old', 5), browser('new', 30)] } }, + { + tabsByWorktree: { [WORKTREE_ID]: [tab('l1', 10)] }, + browserTabsByWorktree: { [WORKTREE_ID]: [browser('host', 20)] } + } + ) + + expect(result.browserTabsByWorktree?.[WORKTREE_ID]?.map((entry) => entry.id)).toEqual([ + 'host', + 'new' + ]) + }) + + it('lets the host copy win a browser both rows list', () => { + const result = hostWon( + { browserTabsByWorktree: { [WORKTREE_ID]: [{ ...browser('b1', 30), title: 'stale' }] } }, + { browserTabsByWorktree: { [WORKTREE_ID]: [browser('b1', 20)] } } + ) + + expect(result.browserTabsByWorktree?.[WORKTREE_ID]?.map((entry) => entry.title)).toEqual(['b1']) + }) + + it("carries a kept browser's own unified entry and builds none it did not have", () => { + const result = hostWon( + { + browserTabsByWorktree: { [WORKTREE_ID]: [browser('own', 30), browser('bare', 31)] }, + unifiedTabs: { [WORKTREE_ID]: [unified('own', 'g-local', 'browser')] } + }, + { + browserTabsByWorktree: { [WORKTREE_ID]: [browser('host', 20)] }, + unifiedTabs: { [WORKTREE_ID]: [unified('l1', 'g1'), unified('host', 'g1', 'browser')] }, + tabGroups: { [WORKTREE_ID]: [group('g1', ['l1', 'host'])] } + } + ) + + expect( + result.unifiedTabs?.[WORKTREE_ID]?.map((entry) => [entry.entityId, entry.groupId]) + ).toEqual([ + ['l1', 'g1'], + ['host', 'g1'], + ['own', 'g1'] + ]) + }) +}) + +describe('adoptStrandedHostPartitionSession unified group for carried entries', () => { + it('creates the group a carried entry is filed under when the workspace has none', () => { + const result = adoptStrandedHostPartitionSession( + session({ tabsByWorktree: { [WORKTREE_ID]: [tab('live-local')] }, unifiedTabs: {} }), + session({ + tabsByWorktree: { [WORKTREE_ID]: [tab('h1')] }, + unifiedTabs: { [WORKTREE_ID]: [] }, + tabGroups: {} + }) + ).session + + const groupId = `carried:${WORKTREE_ID}` + expect(result.unifiedTabs?.[WORKTREE_ID]?.map((entry) => entry.groupId)).toEqual([ + groupId, + groupId + ]) + expect(result.tabGroups?.[WORKTREE_ID]).toEqual([ + { + id: groupId, + worktreeId: WORKTREE_ID, + activeTabId: 'h1', + tabOrder: ['h1', 'live-local'], + recentTabIds: ['h1'] + } + ]) + expect(result.activeGroupIdByWorktree?.[WORKTREE_ID]).toBe(groupId) + }) + + it('files carried entries under an existing group when the active pointer is stale', () => { + const result = adoptStrandedHostPartitionSession( + session({ tabsByWorktree: { [WORKTREE_ID]: [tab('live-local')] }, unifiedTabs: {} }), + session({ + tabsByWorktree: { [WORKTREE_ID]: [tab('h1')] }, + unifiedTabs: { [WORKTREE_ID]: [unified('h1', 'g1')] }, + tabGroups: { [WORKTREE_ID]: [group('g1', ['h1'])] }, + activeGroupIdByWorktree: { [WORKTREE_ID]: 'gone' } + }) + ).session + + expect( + result.unifiedTabs?.[WORKTREE_ID]?.find((entry) => entry.entityId === 'live-local')?.groupId + ).toBe('g1') + expect(result.tabGroups?.[WORKTREE_ID]?.[0]?.tabOrder).toEqual(['h1', 'live-local']) + }) + + it('does not invent tab groups for a session that hydrates in the legacy format', () => { + const result = adoptStrandedHostPartitionSession( + session({ tabsByWorktree: { [WORKTREE_ID]: [tab('live-local')] }, unifiedTabs: {} }), + session({ tabsByWorktree: { [WORKTREE_ID]: [tab('h1')] } }) + ).session + + expect(result.tabGroups).toBeUndefined() + expect(result.activeGroupIdByWorktree).toBeUndefined() + }) +}) diff --git a/src/shared/workspace-session-base-row-carryover.ts b/src/shared/workspace-session-base-row-carryover.ts new file mode 100644 index 000000000000..267b09ffbfe7 --- /dev/null +++ b/src/shared/workspace-session-base-row-carryover.ts @@ -0,0 +1,154 @@ +import type { BrowserWorkspace } from './browser-workspace-types' +import type { Tab } from './tab-types' +import type { PersistedOpenFile, WorkspaceSessionState } from './workspace-session-state-types' +import { newestHostEvidenceAt, type CarriedSurface } from './workspace-session-base-tab-carryover' + +/** + * Base editor files and browser workspaces kept when a populated host row replaces the base's. + * + * The rule `baseTabsTheHostNeverListed` applies to terminal tabs, carried over to the other rows a + * host-owned workspace replaces wholesale: the host copy wins every entry it lists, and a base-only + * entry survives only when dropping it would lose something nothing else restores. + * - An editor file survives when it holds an unsaved draft. `RemoteWorkspaceSession` carries + * terminal fields only, so a dropped `dirtyDraftContent` is gone for good. A clean file is still + * on disk, and a base-only one is most often a file the owning copy has closed since. + * - A browser workspace survives under the terminal tabs' time bound: created after the host's + * newest evidence for the workspace. Legacy residue is older than the host's rows; a browser + * opened while the host was unresolved is not. + * + * Not merged, by design: the `active*` pointers and `tabGroupLayouts` each name one choice and + * follow the row that won; `clientHostedBrowserPagesByWorktree` is authored by the runtime alone, so + * the host copy is the only one; `browserPagesByWorkspace` and `markdownFrontmatterVisible` are + * keyed per entity and only ever gap-filled, so a carried entry's own rows are already kept. + */ +/** Folds the surviving base entries into the rows adoption replaced, and returns the unified + * surfaces they need. */ +export function carryBaseEntriesIntoHostRows( + next: WorkspaceSessionState, + base: WorkspaceSessionState, + host: WorkspaceSessionState +): Map { + const surfacesByKey = new Map() + const addSurface = (key: string, surface: CarriedSurface): void => { + surfacesByKey.set(key, [...(surfacesByKey.get(key) ?? []), surface]) + } + + const replacedFileRows = replacedRowKeys(next, base, host, 'openFilesByWorktree') + if (replacedFileRows.length > 0) { + const openFilesByWorktree = { ...next.openFilesByWorktree } + for (const key of replacedFileRows) { + const hostFiles = openFilesByWorktree[key] ?? [] + const carried = unsavedBaseFiles(base.openFilesByWorktree?.[key], hostFiles) + if (carried.length === 0) { + continue + } + openFilesByWorktree[key] = [...hostFiles, ...carried] + for (const file of carried) { + addSurface(key, { + contentType: 'editor', + entityId: file.filePath, + build: (placement) => editorEntryFor(file, placement) + }) + } + } + next.openFilesByWorktree = openFilesByWorktree + } + + const replacedBrowserRows = replacedRowKeys(next, base, host, 'browserTabsByWorktree') + if (replacedBrowserRows.length > 0) { + const browserTabsByWorktree = { ...next.browserTabsByWorktree } + for (const key of replacedBrowserRows) { + const hostBrowsers = browserTabsByWorktree[key] ?? [] + const carried = newerBaseBrowsers(base.browserTabsByWorktree?.[key], hostBrowsers, host, key) + if (carried.length === 0) { + continue + } + browserTabsByWorktree[key] = [...hostBrowsers, ...carried] + // No `build`: browser hydration creates the entry for a workspace the unified row lacks. + for (const browser of carried) { + addSurface(key, { contentType: 'browser', entityId: browser.id }) + } + } + next.browserTabsByWorktree = browserTabsByWorktree + } + return surfacesByKey +} + +/** + * Keys whose base row adoption replaced with the host's. `adoptRecord` writes the host's row object + * itself, so a row `next` shares with the host where the base held a different one was replaced — + * a gap-fill has no base row, and a declined or contested key keeps the base's object. + */ +function replacedRowKeys( + next: WorkspaceSessionState, + base: WorkspaceSessionState, + host: WorkspaceSessionState, + field: 'openFilesByWorktree' | 'browserTabsByWorktree' +): string[] { + const baseRows = base[field] ?? {} + return Object.entries(host[field] ?? {}) + .filter( + ([key, hostRow]) => + Object.hasOwn(baseRows, key) && baseRows[key] !== hostRow && next[field]?.[key] === hostRow + ) + .map(([key]) => key) +} + +/** Hydration keys a restored file by path within its runtime, so the same pair is one file. */ +function openFileIdentity(file: PersistedOpenFile): string { + return `${file.runtimeEnvironmentId ?? ''}\u0000${file.filePath}` +} + +function unsavedBaseFiles( + baseFiles: readonly PersistedOpenFile[] | undefined, + hostFiles: readonly PersistedOpenFile[] +): PersistedOpenFile[] { + const listed = new Set(hostFiles.map(openFileIdentity)) + // Why read-only files are excluded: hydration discards their drafts, so there is nothing to keep. + return (baseFiles ?? []).filter( + (file) => + file.dirtyDraftContent !== undefined && + file.readOnly !== true && + !listed.has(openFileIdentity(file)) + ) +} + +function newerBaseBrowsers( + baseBrowsers: readonly BrowserWorkspace[] | undefined, + hostBrowsers: readonly BrowserWorkspace[], + host: WorkspaceSessionState, + key: string +): BrowserWorkspace[] { + if (!baseBrowsers || baseBrowsers.length === 0) { + return [] + } + const listed = new Set(hostBrowsers.map((browser) => browser.id)) + let hostSeenUntil = newestHostEvidenceAt(host.tabsByWorktree?.[key] ?? [], host, key) + for (const browser of hostBrowsers) { + hostSeenUntil = Math.max(hostSeenUntil, browser.createdAt) + } + return baseBrowsers.filter( + (browser) => !listed.has(browser.id) && browser.createdAt > hostSeenUntil + ) +} + +/** Built the way legacy hydration builds an editor entry, which is what a carried file lacked. */ +function editorEntryFor( + file: PersistedOpenFile, + { groupId, sortOrder }: { groupId: string; sortOrder: number } +): Tab { + return { + id: file.filePath, + entityId: file.filePath, + groupId, + worktreeId: file.worktreeId, + contentType: 'editor', + label: file.relativePath, + customLabel: null, + color: null, + sortOrder, + createdAt: Date.now(), + isPreview: file.isPreview, + isPinned: false + } +} diff --git a/src/shared/workspace-session-base-tab-carryover.ts b/src/shared/workspace-session-base-tab-carryover.ts index 971d19a3ba6a..40c50c9243d4 100644 --- a/src/shared/workspace-session-base-tab-carryover.ts +++ b/src/shared/workspace-session-base-tab-carryover.ts @@ -31,7 +31,7 @@ function baseTabsTheHostNeverListed( ) } -function newestHostEvidenceAt( +export function newestHostEvidenceAt( hostTabs: readonly TerminalTab[], host: WorkspaceSessionState, workspaceKey: string @@ -74,57 +74,137 @@ export function hostWonTabRow( } /** - * Line the unified surface up with a row the host won: a unified-format session renders only the - * terminals listed there, so a tab without an entry loses its PTY lease (#23390), and a discarded - * base tab's entry would keep showing a closed tab when the host supplied no unified row of its own. + * A base editor file or browser workspace carried into a row the host won. `build` is the entry to + * add when the base had none of its own; without one the hydration of that kind supplies it. + */ +export type CarriedSurface = { + contentType: 'editor' | 'browser' + entityId: string + build?: (placement: { groupId: string; sortOrder: number }) => Tab +} + +/** + * Line the unified surface up with rows the host won: a unified-format session renders only what is + * listed there, so a terminal without an entry loses its PTY lease (#23390) and a carried editor + * draft would restore with no tab to show it; a discarded base tab's entry would keep showing a + * closed tab when the host supplied no unified row of its own. */ export function reconcileHostWonUnifiedRows( next: WorkspaceSessionState, base: WorkspaceSessionState, - rowsByKey: ReadonlyMap + rowsByKey: ReadonlyMap, + carriedSurfacesByKey: ReadonlyMap = new Map() ): void { - if (rowsByKey.size === 0 || !next.unifiedTabs) { + if ((rowsByKey.size === 0 && carriedSurfacesByKey.size === 0) || !next.unifiedTabs) { return } const unifiedTabs = { ...next.unifiedTabs } const tabGroups = next.tabGroups ? { ...next.tabGroups } : undefined - for (const [key, { tabs, carried, discardedTabIds }] of rowsByKey) { + let activeGroupIdByWorktree = next.activeGroupIdByWorktree + for (const key of new Set([...rowsByKey.keys(), ...carriedSurfacesByKey.keys()])) { + const hostWon = rowsByKey.get(key) + const discardedTabIds = hostWon?.discardedTabIds ?? new Set() const row = (unifiedTabs[key] ?? []).filter( (entry) => entry.contentType !== 'terminal' || !discardedTabIds.has(entry.entityId) ) - const present = new Set(row.map((entry) => entry.entityId)) - const groups = tabGroups?.[key] - const groupId = next.activeGroupIdByWorktree?.[key] ?? groups?.[0]?.id ?? `carried:${key}` - const carriedIds = new Set(carried.map((terminal) => terminal.id)) - const additions: Tab[] = [] - for (const terminal of tabs) { - if (present.has(terminal.id)) { - continue - } - const own = carriedIds.has(terminal.id) - ? base.unifiedTabs?.[key]?.find( - (entry) => entry.contentType === 'terminal' && entry.entityId === terminal.id - ) - : undefined - additions.push(own ? { ...own, groupId } : unifiedEntryFor(terminal, groupId)) - } + const groups = tabGroups?.[key] ?? [] + const activeGroupId = next.activeGroupIdByWorktree?.[key] + // Why only a group that exists: a stale active-group pointer would file the additions under a + // group no tab strip renders. + const groupId = + groups.find((group) => group.id === activeGroupId)?.id ?? groups[0]?.id ?? `carried:${key}` + const additions = [ + ...terminalAdditions(hostWon, row, base.unifiedTabs?.[key], groupId), + ...surfaceAdditions(carriedSurfacesByKey.get(key), row, base.unifiedTabs?.[key], groupId) + ] if (additions.length > 0 || row.length !== (unifiedTabs[key]?.length ?? 0)) { unifiedTabs[key] = [...row, ...additions] } - if (tabGroups && groups) { - const addedIds = additions.map((entry) => entry.id) - tabGroups[key] = groups.map((group) => { - const tabOrder = group.tabOrder.filter((tabId) => !discardedTabIds.has(tabId)) - return group.id === groupId - ? { ...group, tabOrder: [...tabOrder, ...addedIds] } - : { ...group, tabOrder } - }) + if (!tabGroups) { + // A session without tab groups hydrates in the legacy format, which ignores `unifiedTabs`; + // inventing groups here would switch every workspace in it to the unified format. + continue } + const addedIds = additions.map((entry) => entry.id) + if (groups.length === 0) { + if (addedIds.length > 0) { + tabGroups[key] = [ + { + id: groupId, + worktreeId: key, + activeTabId: addedIds[0], + tabOrder: addedIds, + recentTabIds: [addedIds[0]] + } + ] + activeGroupIdByWorktree = { ...activeGroupIdByWorktree, [key]: groupId } + } + continue + } + tabGroups[key] = groups.map((group) => { + const tabOrder = group.tabOrder.filter((tabId) => !discardedTabIds.has(tabId)) + return group.id === groupId + ? { ...group, tabOrder: [...tabOrder, ...addedIds] } + : { ...group, tabOrder } + }) } next.unifiedTabs = unifiedTabs if (tabGroups) { next.tabGroups = tabGroups } + if (activeGroupIdByWorktree !== next.activeGroupIdByWorktree) { + next.activeGroupIdByWorktree = activeGroupIdByWorktree + } +} + +function terminalAdditions( + hostWon: HostWonTabRow | undefined, + row: readonly Tab[], + baseRow: readonly Tab[] | undefined, + groupId: string +): Tab[] { + if (!hostWon) { + return [] + } + const present = new Set( + row.filter((entry) => entry.contentType === 'terminal').map((entry) => entry.entityId) + ) + const carriedIds = new Set(hostWon.carried.map((terminal) => terminal.id)) + const additions: Tab[] = [] + for (const terminal of hostWon.tabs) { + if (present.has(terminal.id)) { + continue + } + const own = carriedIds.has(terminal.id) + ? baseRow?.find((entry) => entry.contentType === 'terminal' && entry.entityId === terminal.id) + : undefined + additions.push(own ? { ...own, groupId } : unifiedEntryFor(terminal, groupId)) + } + return additions +} + +function surfaceAdditions( + surfaces: readonly CarriedSurface[] | undefined, + row: readonly Tab[], + baseRow: readonly Tab[] | undefined, + groupId: string +): Tab[] { + const additions: Tab[] = [] + // Why after every existing entry: unified hydration orders a row by `sortOrder`. + let sortOrder = Math.max(-1, ...row.map((entry) => entry.sortOrder)) + for (const surface of surfaces ?? []) { + const names = (entry: Tab): boolean => + entry.contentType === surface.contentType && entry.entityId === surface.entityId + if (row.some(names)) { + continue + } + const own = baseRow?.find(names) + const entry = own ? { ...own, groupId } : surface.build?.({ groupId, sortOrder: ++sortOrder }) + if (entry) { + additions.push(entry) + } + } + return additions } function unifiedEntryFor(terminal: TerminalTab, groupId: string): Tab { diff --git a/src/shared/workspace-session-stranded-partition-adoption.ts b/src/shared/workspace-session-stranded-partition-adoption.ts index fc3522212bed..b4b13b84dfb3 100644 --- a/src/shared/workspace-session-stranded-partition-adoption.ts +++ b/src/shared/workspace-session-stranded-partition-adoption.ts @@ -16,6 +16,7 @@ import { reconcileHostWonUnifiedRows, type HostWonTabRow } from './workspace-session-base-tab-carryover' +import { carryBaseEntriesIntoHostRows } from './workspace-session-base-row-carryover' /** * Fold rows a host partition holds alone back into the session the readers assemble. @@ -52,7 +53,8 @@ import { * The base keeps a workspace it holds **terminal tabs** for only while the host partition holds * none. When both hold tabs the base row is the leftover — `local` is never pruned for an SSH * workspace, so letting it win revived closed tabs and hid live ones (#22503, #23390) — and the - * host row replaces it; `baseTabsTheHostNeverListed` picks the base tabs that survive. An EMPTY + * host row replaces it; `baseTabsTheHostNeverListed` picks the base tabs that survive, and + * `carryBaseEntriesIntoHostRows` the unsaved editor drafts and newer browsers. An EMPTY * host row is not ownership (it may be #12721's poisoned list), so the base keeps its tabs. * An EMPTY base tab row is not a live copy either: an empty list is not evidence * that anything was closed (`mergeDirectSshRemoteWorkspaceSession` argues this at length, and @@ -421,7 +423,9 @@ export function adoptStrandedHostPartitionSession( break } } - reconcileHostWonUnifiedRows(next, base, hostWonRows) + // Why after the walk: a replaced row's base-only entries are picked against the row that won. + const carriedSurfaces = carryBaseEntriesIntoHostRows(next, base, host) + reconcileHostWonUnifiedRows(next, base, hostWonRows, carriedSurfaces) // 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( From 4e8c030dc5a4c75cd00ff4cd9ede607e10950a3b Mon Sep 17 00:00:00 2001 From: tlgnkl <67519432+tlgnkl@users.noreply.github.com> Date: Sun, 27 Sep 2026 21:49:59 +0300 Subject: [PATCH 4/4] fix(session): key a carried draft's tab by the id hydration gives the file A carried editor entry was keyed by the bare file path, but editor hydration keys a file by owner: a runtime-owned file always gets its owned id, and when a same-path file for another runtime hydrates first, a bare-path tab is migrated to that file. So a draft whose path the host also held under another runtime came back in `openFiles` with no tab. The carried entry now takes the id hydration will match to the file, the base's own entry is re-keyed to it, and dedupe uses the same id. `buildOwnedEditorFileId` and `runtimeOwnerKey` move to /shared so the session read path and the editor slice share one definition. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../slices/editor/file-ids/editor-file-ids.ts | 14 +- ...-partition-carried-draft-hydration.test.ts | 190 ++++++++++++++++++ src/shared/editor-file-id.ts | 19 ++ .../workspace-session-base-row-carryover.ts | 49 ++++- .../workspace-session-base-tab-carryover.ts | 25 ++- 5 files changed, 272 insertions(+), 25 deletions(-) create mode 100644 src/renderer/src/store/slices/stranded-partition-carried-draft-hydration.test.ts create mode 100644 src/shared/editor-file-id.ts diff --git a/src/renderer/src/store/slices/editor/file-ids/editor-file-ids.ts b/src/renderer/src/store/slices/editor/file-ids/editor-file-ids.ts index 03b90872ef63..0933017428b8 100644 --- a/src/renderer/src/store/slices/editor/file-ids/editor-file-ids.ts +++ b/src/renderer/src/store/slices/editor/file-ids/editor-file-ids.ts @@ -4,10 +4,9 @@ import type { DiffSource, EditorOpenTargetOptions, OpenFile } from '../types/ope import { areLocalWindowsWslPathAliases } from '../../../../../../shared/cross-platform-path' import { getConnectionIdForFileFromState } from '@/lib/connection-owner-resolution' import { isLocalWindowsDesktopClient } from '@/lib/desktop-window-chrome' +import { buildOwnedEditorFileId, runtimeOwnerKey } from '../../../../../../shared/editor-file-id' -export function runtimeOwnerKey(runtimeEnvironmentId: string | null | undefined): string | null { - return runtimeEnvironmentId?.trim() || null -} +export { buildOwnedEditorFileId, runtimeOwnerKey } export function isSameEditorOwner( file: Pick, @@ -37,15 +36,6 @@ export function canReuseLocalWslAlias( ) } -export function buildOwnedEditorFileId( - filePath: string, - worktreeId: string, - runtimeEnvironmentId: string | null | undefined -): string { - const runtimeKey = runtimeOwnerKey(runtimeEnvironmentId) ?? 'local' - return `editor:${encodeURIComponent(worktreeId)}:${encodeURIComponent(runtimeKey)}:${encodeURIComponent(filePath)}` -} - export function buildDiffEditorFileId( worktreeId: string, diffSource: DiffSource, diff --git a/src/renderer/src/store/slices/stranded-partition-carried-draft-hydration.test.ts b/src/renderer/src/store/slices/stranded-partition-carried-draft-hydration.test.ts new file mode 100644 index 000000000000..d86ac381eb59 --- /dev/null +++ b/src/renderer/src/store/slices/stranded-partition-carried-draft-hydration.test.ts @@ -0,0 +1,190 @@ +/** + * A draft that stranded-partition adoption carries into a host-won row has to come back as a tab. + * The shape of the carried unified entry is not enough: hydration assigns editor ids by owner and + * migrates tab ids between files that share a path, so this runs the real tab and editor hydration. + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type * as AgentStatusModule from '@/lib/agent-status' +import type { Tab, TabGroup } from '../../../../shared/tab-types' +import type { + PersistedOpenFile, + WorkspaceSessionState +} from '../../../../shared/workspace-session-state-types' +import { getDefaultWorkspaceSession } from '../../../../shared/constants' +import { buildOwnedEditorFileId } from '../../../../shared/editor-file-id' +import { adoptStrandedHostPartitionSession } from '../../../../shared/workspace-session-stranded-partition-adoption' +import { createStoreSessionMockApi } from './store-session-test-harness' +import { createTestStore, makeWorktree } from './store-test-helpers' + +vi.mock('sonner', () => ({ toast: { info: vi.fn(), success: vi.fn(), error: vi.fn() } })) +vi.mock('@/lib/agent-status', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, detectAgentStatusFromTitle: vi.fn().mockReturnValue(null) } +}) + +createStoreSessionMockApi() + +const WORKTREE_ID = 'repo-1::/workspace' +const SAME_PATH = '/workspace/src/same.ts' + +function file(filePath: string, overrides: Partial = {}): PersistedOpenFile { + return { + filePath, + relativePath: filePath.slice('/workspace/'.length), + worktreeId: WORKTREE_ID, + language: 'typescript', + ...overrides + } +} + +function editorTab(id: string, sortOrder = 0): Tab { + return { + id, + entityId: id, + groupId: 'g1', + worktreeId: WORKTREE_ID, + contentType: 'editor', + label: id, + customLabel: null, + color: null, + sortOrder, + createdAt: 1 + } +} + +function group(tabOrder: string[]): TabGroup { + return { id: 'g1', worktreeId: WORKTREE_ID, activeTabId: tabOrder[0] ?? null, tabOrder } +} + +function session(overrides: Partial): WorkspaceSessionState { + return { ...getDefaultWorkspaceSession(), ...overrides } +} + +/** Adopt, then restore the result the way startup does. */ +function restore(base: WorkspaceSessionState, host: WorkspaceSessionState) { + const adopted = adoptStrandedHostPartitionSession(base, host).session + const store = createTestStore() + store.setState({ + repos: [ + { id: 'repo-1', path: '/workspace', displayName: 'Repo', badgeColor: '#000', addedAt: 0 } + ], + worktreesByRepo: { + 'repo-1': [makeWorktree({ id: WORKTREE_ID, repoId: 'repo-1', path: '/workspace' })] + }, + activeWorktreeId: WORKTREE_ID + }) + store.getState().hydrateTabsSession(adopted) + store.getState().hydrateEditorSession(adopted) + return store.getState() +} + +/** Every restored file paired with the editor tab that shows it, or null when none does. */ +function visibleFiles(state: ReturnType) { + const tabs = state.unifiedTabsByWorktree[WORKTREE_ID] ?? [] + return state.openFiles.map((open) => ({ + runtime: open.runtimeEnvironmentId ?? null, + draft: state.editorDrafts[open.id] ?? null, + tab: tabs.find((tab) => tab.contentType === 'editor' && tab.entityId === open.id)?.id ?? null + })) +} + +describe('a carried draft through real hydration', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('shows a runtime-owned draft whose path the host holds for the local runtime', () => { + const state = restore( + session({ + openFilesByWorktree: { + [WORKTREE_ID]: [ + file(SAME_PATH, { runtimeEnvironmentId: 'env-1', dirtyDraftContent: 'x' }) + ] + } + }), + session({ + openFilesByWorktree: { [WORKTREE_ID]: [file(SAME_PATH)] }, + unifiedTabs: { [WORKTREE_ID]: [editorTab(SAME_PATH)] }, + tabGroups: { [WORKTREE_ID]: [group([SAME_PATH])] } + }) + ) + + expect(visibleFiles(state)).toEqual([ + { runtime: null, draft: null, tab: SAME_PATH }, + { + runtime: 'env-1', + draft: 'x', + tab: buildOwnedEditorFileId(SAME_PATH, WORKTREE_ID, 'env-1') + } + ]) + }) + + it('shows a local draft whose path the host holds for a runtime under the bare path', () => { + // The host's own tab is persisted under the bare path, which hydration hands to the runtime + // file; the carried local file has to be reached through its local owned id instead. + const state = restore( + session({ + openFilesByWorktree: { [WORKTREE_ID]: [file(SAME_PATH, { dirtyDraftContent: 'x' })] } + }), + session({ + openFilesByWorktree: { + [WORKTREE_ID]: [file(SAME_PATH, { runtimeEnvironmentId: 'env-1' })] + }, + unifiedTabs: { [WORKTREE_ID]: [editorTab(SAME_PATH)] }, + tabGroups: { [WORKTREE_ID]: [group([SAME_PATH])] } + }) + ) + + expect(visibleFiles(state)).toEqual([ + { + runtime: 'env-1', + draft: null, + tab: buildOwnedEditorFileId(SAME_PATH, WORKTREE_ID, 'env-1') + }, + { runtime: null, draft: 'x', tab: SAME_PATH } + ]) + }) + + it('keeps the bare path as the tab id of an ordinary local draft', () => { + const draftPath = '/workspace/src/draft.ts' + const state = restore( + session({ + openFilesByWorktree: { [WORKTREE_ID]: [file(draftPath, { dirtyDraftContent: 'x' })] } + }), + session({ + openFilesByWorktree: { [WORKTREE_ID]: [file('/workspace/src/host.ts')] }, + unifiedTabs: { [WORKTREE_ID]: [editorTab('/workspace/src/host.ts')] }, + tabGroups: { [WORKTREE_ID]: [group(['/workspace/src/host.ts'])] } + }) + ) + + expect(visibleFiles(state)).toEqual([ + { runtime: null, draft: null, tab: '/workspace/src/host.ts' }, + { runtime: null, draft: 'x', tab: draftPath } + ]) + }) + + it("re-keys the base's own entry persisted under the bare path of a runtime-owned draft", () => { + const state = restore( + session({ + openFilesByWorktree: { + [WORKTREE_ID]: [ + file(SAME_PATH, { runtimeEnvironmentId: 'env-1', dirtyDraftContent: 'x' }) + ] + }, + unifiedTabs: { [WORKTREE_ID]: [{ ...editorTab(SAME_PATH), customLabel: 'mine' }] } + }), + session({ + openFilesByWorktree: { [WORKTREE_ID]: [file(SAME_PATH)] }, + unifiedTabs: { [WORKTREE_ID]: [editorTab(SAME_PATH)] }, + tabGroups: { [WORKTREE_ID]: [group([SAME_PATH])] } + }) + ) + + const ownedId = buildOwnedEditorFileId(SAME_PATH, WORKTREE_ID, 'env-1') + expect(visibleFiles(state).map((entry) => entry.tab)).toEqual([SAME_PATH, ownedId]) + expect( + state.unifiedTabsByWorktree[WORKTREE_ID]?.find((tab) => tab.id === ownedId)?.customLabel + ).toBe('mine') + }) +}) diff --git a/src/shared/editor-file-id.ts b/src/shared/editor-file-id.ts new file mode 100644 index 000000000000..ae444ff90b80 --- /dev/null +++ b/src/shared/editor-file-id.ts @@ -0,0 +1,19 @@ +/** + * Editor file identity shared by the renderer's editor slice and the session read path. + * + * Why in /shared: stranded-partition adoption builds unified entries for editor files it carries + * across partitions, and those must carry the id hydration will give the file, or the file restores + * with no tab to show it. + */ +export function runtimeOwnerKey(runtimeEnvironmentId: string | null | undefined): string | null { + return runtimeEnvironmentId?.trim() || null +} + +export function buildOwnedEditorFileId( + filePath: string, + worktreeId: string, + runtimeEnvironmentId: string | null | undefined +): string { + const runtimeKey = runtimeOwnerKey(runtimeEnvironmentId) ?? 'local' + return `editor:${encodeURIComponent(worktreeId)}:${encodeURIComponent(runtimeKey)}:${encodeURIComponent(filePath)}` +} diff --git a/src/shared/workspace-session-base-row-carryover.ts b/src/shared/workspace-session-base-row-carryover.ts index 267b09ffbfe7..b14e91f1bd38 100644 --- a/src/shared/workspace-session-base-row-carryover.ts +++ b/src/shared/workspace-session-base-row-carryover.ts @@ -1,4 +1,6 @@ import type { BrowserWorkspace } from './browser-workspace-types' +import { FLOATING_TERMINAL_WORKTREE_ID } from './constants' +import { buildOwnedEditorFileId, runtimeOwnerKey } from './editor-file-id' import type { Tab } from './tab-types' import type { PersistedOpenFile, WorkspaceSessionState } from './workspace-session-state-types' import { newestHostEvidenceAt, type CarriedSurface } from './workspace-session-base-tab-carryover' @@ -42,14 +44,21 @@ export function carryBaseEntriesIntoHostRows( if (carried.length === 0) { continue } - openFilesByWorktree[key] = [...hostFiles, ...carried] - for (const file of carried) { + const row = [...hostFiles, ...carried] + openFilesByWorktree[key] = row + carried.forEach((file, index) => { + const tabId = hydratedEditorTabId(file, key, row.slice(0, hostFiles.length + index)) addSurface(key, { contentType: 'editor', - entityId: file.filePath, - build: (placement) => editorEntryFor(file, placement) + entityId: tabId, + // Why every form: the base persisted the entry under whichever id the file had then. + baseEntityIds: [ + buildOwnedEditorFileId(file.filePath, key, file.runtimeEnvironmentId), + file.filePath + ], + build: (placement) => editorEntryFor(file, tabId, placement) }) - } + }) } next.openFilesByWorktree = openFilesByWorktree } @@ -96,7 +105,30 @@ function replacedRowKeys( /** Hydration keys a restored file by path within its runtime, so the same pair is one file. */ function openFileIdentity(file: PersistedOpenFile): string { - return `${file.runtimeEnvironmentId ?? ''}\u0000${file.filePath}` + return `${runtimeOwnerKey(file.runtimeEnvironmentId) ?? ''}\u0000${file.filePath}` +} + +/** + * The unified tab id that `hydrate-editor-session.ts` will match to a carried file, given the files + * hydration walks before it in the same row. Without it a draft restores with no tab to show it. + * - A runtime-owned file, or any file of the floating workspace, hydrates under its owned id. + * - A local file hydrates under its path. But when an earlier file holds the same path for a + * runtime, hydration reads the path as taken: it moves a tab named by the bare path to that + * earlier file, and moves one named by the local owned id to this one. + */ +function hydratedEditorTabId( + file: PersistedOpenFile, + key: string, + earlier: readonly PersistedOpenFile[] +): string { + const owned = buildOwnedEditorFileId(file.filePath, key, file.runtimeEnvironmentId) + if (key === FLOATING_TERMINAL_WORKTREE_ID || runtimeOwnerKey(file.runtimeEnvironmentId)) { + return owned + } + const pathTaken = earlier.some( + (other) => other.filePath === file.filePath && runtimeOwnerKey(other.runtimeEnvironmentId) + ) + return pathTaken ? owned : file.filePath } function unsavedBaseFiles( @@ -135,11 +167,12 @@ function newerBaseBrowsers( /** Built the way legacy hydration builds an editor entry, which is what a carried file lacked. */ function editorEntryFor( file: PersistedOpenFile, + tabId: string, { groupId, sortOrder }: { groupId: string; sortOrder: number } ): Tab { return { - id: file.filePath, - entityId: file.filePath, + id: tabId, + entityId: tabId, groupId, worktreeId: file.worktreeId, contentType: 'editor', diff --git a/src/shared/workspace-session-base-tab-carryover.ts b/src/shared/workspace-session-base-tab-carryover.ts index 40c50c9243d4..22120a9cad6e 100644 --- a/src/shared/workspace-session-base-tab-carryover.ts +++ b/src/shared/workspace-session-base-tab-carryover.ts @@ -79,7 +79,10 @@ export function hostWonTabRow( */ export type CarriedSurface = { contentType: 'editor' | 'browser' + /** The id hydration will look the entry up by; an editor entry's tab id is the same value. */ entityId: string + /** Other ids the base's own entry may name the same entity by, most specific first. */ + baseEntityIds?: readonly string[] build?: (placement: { groupId: string; sortOrder: number }) => Tab } @@ -193,13 +196,25 @@ function surfaceAdditions( // Why after every existing entry: unified hydration orders a row by `sortOrder`. let sortOrder = Math.max(-1, ...row.map((entry) => entry.sortOrder)) for (const surface of surfaces ?? []) { - const names = (entry: Tab): boolean => - entry.contentType === surface.contentType && entry.entityId === surface.entityId - if (row.some(names)) { + const names = + (entityId: string) => + (entry: Tab): boolean => + entry.contentType === surface.contentType && entry.entityId === entityId + if (row.some(names(surface.entityId))) { continue } - const own = baseRow?.find(names) - const entry = own ? { ...own, groupId } : surface.build?.({ groupId, sortOrder: ++sortOrder }) + const own = [surface.entityId, ...(surface.baseEntityIds ?? [])] + .map((entityId) => baseRow?.find(names(entityId))) + .find((entry) => entry !== undefined) + // Why an editor entry is re-keyed: the base's own id may be a form hydration now gives a + // different file, and an editor tab's id is its entity id. + const rekeyed = + own && surface.contentType === 'editor' + ? { ...own, id: surface.entityId, entityId: surface.entityId } + : own + const entry = rekeyed + ? { ...rekeyed, groupId } + : surface.build?.({ groupId, sortOrder: ++sortOrder }) if (entry) { additions.push(entry) }