diff --git a/src/main/persistence/tracking-repos/worktree-identity-migration-field-coverage.test.ts b/src/main/persistence/tracking-repos/worktree-identity-migration-field-coverage.test.ts new file mode 100644 index 00000000000..24abf824003 --- /dev/null +++ b/src/main/persistence/tracking-repos/worktree-identity-migration-field-coverage.test.ts @@ -0,0 +1,233 @@ +/** + * Every persisted session field that can name a worktree must lose the old identity when the + * worktree is re-keyed. + * + * Driven by `WORKSPACE_SESSION_WORKTREE_REFERENCE_KIND` rather than a list of its own: that table + * is already a compile-error-to-skip census of how each field names an owner, and the migration + * was the one path with no census at all. Three fields had fallen out of it — + * `clientHostedBrowserPagesByWorktree` (key AND row `workspaceId`), + * `closedTerminalTabTombstonesByTabId` and `clientHostedBrowserCloseIntentsByEnvironment` — each + * one a row that keeps matching on an id nothing answers to any more. + * + * The oracle is `collectWorkspaceSessionWorktreeOwners`, the shipping collector, so a fixture + * cannot be "the shape the assertion expects": it only counts as a reference if the collector + * already reads it as one. + */ +import { describe, expect, it } from 'vitest' +import { getDefaultWorkspaceSession } from '../../../shared/constants' +import type { PersistedState } from '../../../shared/persisted-state-types' +import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' +import { worktreeWorkspaceKey } from '../../../shared/workspace-scope' +import { + collectWorkspaceSessionWorktreeOwners, + WORKSPACE_SESSION_WORKTREE_REFERENCE_KIND +} from '../restoring-sessions/session-worktree-ownership' +import { migrateWorktreeIdentity } from './worktree-identity-migration' + +const REPO = 'repo' +const OLD = `${REPO}::/old/path` +const NEW = `${REPO}::/new/path` +const CANDIDATES = new Set([OLD, NEW]) + +type SessionField = keyof WorkspaceSessionState + +/** One fixture per field, each holding exactly that field's reference to OLD. */ +const REFERENCE_FIXTURES: Partial>> = { + activeWorkspaceKey: { activeWorkspaceKey: worktreeWorkspaceKey(OLD) }, + activeWorktreeId: { activeWorktreeId: OLD }, + tabsByWorktree: { + tabsByWorktree: { + [OLD]: [ + { + id: 'tab-1', + ptyId: null, + worktreeId: OLD, + title: 't', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + } + ] + } + }, + activeWorktreeIdsOnShutdown: { activeWorktreeIdsOnShutdown: [OLD] }, + openFilesByWorktree: { + openFilesByWorktree: { + [OLD]: [ + { + filePath: '/old/path/a.ts', + relativePath: 'a.ts', + worktreeId: OLD, + language: 'ts', + dirtyDraftContent: 'unsaved' + } + ] + } + }, + activeFileIdByWorktree: { activeFileIdByWorktree: { [OLD]: '/old/path/a.ts' } }, + browserTabsByWorktree: { + browserTabsByWorktree: { + [OLD]: [{ id: 'bw', worktreeId: OLD, title: 'b', createdAt: 1, activePageId: 'p' }] as never + } + }, + browserPagesByWorkspace: { + browserPagesByWorkspace: { + bw: [ + { + id: 'p', + workspaceId: 'bw', + worktreeId: OLD, + url: 'https://e.com', + title: 'E', + loading: false, + faviconUrl: null, + canGoBack: false, + canGoForward: false, + loadError: null, + createdAt: 1 + } + ] as never + } + }, + activeBrowserTabIdByWorktree: { activeBrowserTabIdByWorktree: { [OLD]: 'bw' } }, + clientHostedBrowserPagesByWorktree: { + clientHostedBrowserPagesByWorktree: { + [OLD]: [ + { + v: 1, + browserPageId: 'chp', + workspaceId: OLD, + browserProfileId: 'profile', + url: 'https://e.com', + title: 'E', + pairedDeviceId: 'device', + savedAt: 1 + } + ] as never + } + }, + clientHostedBrowserCloseIntentsByEnvironment: { + clientHostedBrowserCloseIntentsByEnvironment: { + 'env-1': [{ browserPageId: 'chp', worktreeId: OLD, closedAt: 3 }] + } + }, + activeTabTypeByWorktree: { activeTabTypeByWorktree: { [OLD]: 'terminal' } }, + activeTabIdByWorktree: { activeTabIdByWorktree: { [OLD]: 'tab-1' } }, + unifiedTabs: { + unifiedTabs: { + [OLD]: [ + { + id: 'tab-1', + entityId: 'tab-1', + groupId: 'g', + worktreeId: OLD, + contentType: 'terminal', + label: 't', + customLabel: null, + color: null, + sortOrder: 0, + createdAt: 1 + } + ] as never + } + }, + tabGroups: { + tabGroups: { + [OLD]: [{ id: 'g', worktreeId: OLD, kind: 'terminal', sortOrder: 0, createdAt: 1 }] as never + } + }, + tabGroupLayouts: { tabGroupLayouts: { [OLD]: { type: 'leaf', groupId: 'g' } as never } }, + activeGroupIdByWorktree: { activeGroupIdByWorktree: { [OLD]: 'g' } }, + lastVisitedAtByWorktreeId: { + lastVisitedAtByWorktreeId: { [OLD]: 10, [`ssh:target|${OLD}`]: 20 } + }, + defaultTerminalTabsAppliedByWorktreeId: { + defaultTerminalTabsAppliedByWorktreeId: { [OLD]: true } + }, + sleepingAgentSessionsByPaneKey: { + sleepingAgentSessionsByPaneKey: { + 'tab-1:leaf': { + paneKey: 'tab-1:leaf', + worktreeId: OLD, + agent: 'claude', + providerSession: {}, + prompt: 'p', + state: 'idle', + capturedAt: 1, + updatedAt: 1 + } as never + } + }, + terminalSurfaceTombstonesByPaneKey: { + terminalSurfaceTombstonesByPaneKey: { + 'tab-1:leaf': { + worktreeId: OLD, + parentTabId: 'tab-1', + leafId: 'leaf', + ptyId: 'pty', + incarnationId: 'inc', + retiredAt: 1 + } + } + }, + closedTerminalTabTombstonesByTabId: { + closedTerminalTabTombstonesByTabId: { 'tab-1': { closedAt: 5, worktreeId: OLD } } + } +} + +function persistedState(session: WorkspaceSessionState): PersistedState { + return { + worktreeMeta: {}, + worktreeLineageById: {}, + workspaceLineageByChildKey: {}, + workspaceSession: session, + workspaceSessionsByHostId: {} + } as unknown as PersistedState +} + +const referencingFields = (Object.keys(WORKSPACE_SESSION_WORKTREE_REFERENCE_KIND) as SessionField[]) + .filter((field) => WORKSPACE_SESSION_WORKTREE_REFERENCE_KIND[field] !== 'none') + .sort() + +describe('migrateWorktreeIdentity worktree-reference coverage', () => { + it('has a fixture for every field the ownership census says can name a worktree', () => { + const missing = referencingFields.filter((field) => !REFERENCE_FIXTURES[field]) + expect(missing).toEqual([]) + }) + + for (const field of referencingFields) { + it(`re-points ${field} off the old identity`, () => { + const session: WorkspaceSessionState = { + ...getDefaultWorkspaceSession(), + ...REFERENCE_FIXTURES[field] + } + // The fixture is only a reference if the shipping collector reads it as one. + expect([...collectWorkspaceSessionWorktreeOwners(session, CANDIDATES)]).toEqual([OLD]) + migrateWorktreeIdentity(persistedState(session), OLD, NEW) + expect([...collectWorkspaceSessionWorktreeOwners(session, CANDIDATES)]).toEqual([NEW]) + }) + } + + // The collector reads this map by key only, so the row's own copy of the id needs its own check: + // rehydration republishes a page only while `workspaceId` still equals the key it is filed under. + it('re-points the workspaceId inside each client-hosted browser page row', () => { + const session: WorkspaceSessionState = { + ...getDefaultWorkspaceSession(), + ...REFERENCE_FIXTURES.clientHostedBrowserPagesByWorktree + } + migrateWorktreeIdentity(persistedState(session), OLD, NEW) + expect(session.clientHostedBrowserPagesByWorktree?.[NEW]?.[0]?.workspaceId).toBe(NEW) + }) + + it('migrates host partitions, not just the local blob', () => { + const hostSession: WorkspaceSessionState = { + ...getDefaultWorkspaceSession(), + ...REFERENCE_FIXTURES.closedTerminalTabTombstonesByTabId + } + const state = persistedState(getDefaultWorkspaceSession()) + state.workspaceSessionsByHostId = { 'ssh:target': hostSession } + expect(migrateWorktreeIdentity(state, OLD, NEW)).toBe(true) + expect(hostSession.closedTerminalTabTombstonesByTabId?.['tab-1']?.worktreeId).toBe(NEW) + }) +}) diff --git a/src/main/persistence/tracking-repos/worktree-identity-migration.ts b/src/main/persistence/tracking-repos/worktree-identity-migration.ts index 2d2fa484ad6..f57a48f5791 100644 --- a/src/main/persistence/tracking-repos/worktree-identity-migration.ts +++ b/src/main/persistence/tracking-repos/worktree-identity-migration.ts @@ -10,6 +10,78 @@ import { } from '../../../shared/worktree/host-qualified-identity' import { splitWorktreeIdForFilesystem } from '../../../shared/worktree/id' +/** Session maps whose value names the worktree it belongs to, keyed by something else. */ +const WORKTREE_ROW_RECORD_SESSION_FIELDS = [ + 'sleepingAgentSessionsByPaneKey', + 'terminalSurfaceTombstonesByPaneKey', + 'closedTerminalTabTombstonesByTabId' +] as const satisfies readonly (keyof WorkspaceSessionState)[] + +/** Same, but each value is an array of such rows. */ +const WORKTREE_ROW_ARRAY_SESSION_FIELDS = [ + 'clientHostedBrowserCloseIntentsByEnvironment' +] as const satisfies readonly (keyof WorkspaceSessionState)[] + +type WorktreeNamingRow = { worktreeId: string } + +function repointRow( + row: WorktreeNamingRow, + oldWorktreeId: string, + newWorktreeId: string +): WorktreeNamingRow | null { + return row?.worktreeId === oldWorktreeId ? { ...row, worktreeId: newWorktreeId } : null +} + +function repointRowRecord( + session: WorkspaceSessionState, + field: (typeof WORKTREE_ROW_RECORD_SESSION_FIELDS)[number], + oldWorktreeId: string, + newWorktreeId: string +): boolean { + const record = session[field] as Record | undefined + if (!record) { + return false + } + const next: Record = { ...record } + let changed = false + for (const [key, row] of Object.entries(next)) { + const repointed = repointRow(row, oldWorktreeId, newWorktreeId) + if (repointed) { + next[key] = repointed + changed = true + } + } + if (changed) { + ;(session as Record)[field] = next + } + return changed +} + +function repointRowArrays( + session: WorkspaceSessionState, + field: (typeof WORKTREE_ROW_ARRAY_SESSION_FIELDS)[number], + oldWorktreeId: string, + newWorktreeId: string +): boolean { + const record = session[field] as Record | undefined + if (!record) { + return false + } + const next: Record = { ...record } + let changed = false + for (const [key, rows] of Object.entries(next)) { + if (!Array.isArray(rows) || !rows.some((row) => row?.worktreeId === oldWorktreeId)) { + continue + } + next[key] = rows.map((row) => repointRow(row, oldWorktreeId, newWorktreeId) ?? row) + changed = true + } + if (changed) { + ;(session as Record)[field] = next + } + return changed +} + /** * Re-keys every worktreeId-keyed record in `state` from `oldWorktreeId` to `newWorktreeId`. Mutates `state` in place; * returns whether anything changed so the caller can gate its save. No-op when the ids match. @@ -60,6 +132,14 @@ export function migrateWorktreeIdentity( return false } let sessionChanged = false + /** Known and deliberately unresolved: when the target key ALREADY exists, the source wins and + * the target's row is lost. `lastVisitedAtByWorktreeId` below is the one map that settles it + * (`Math.max`), and its comment names the case — a partial migration leaves both identities + * behind. There is no safe blanket rule here: "keep the target" is right when the target holds + * a real closed-last-terminal tombstone (`tabsByWorktree[target] === []` is user intent, see + * runtime/workspace-session-worktree-id.ts), and "keep the source" is right when the target row + * is a stub, and nothing records which is newer. Reachable only by a repeated or partial + * migration: on a normal rename this store holds rows under the old id alone. */ const moveSessionKey = ( record: Record | undefined, mapValue: (value: T) => T = (value) => value @@ -114,6 +194,14 @@ export function migrateWorktreeIdentity( sessionChanged = true } } + // Why the row too: rehydration only republishes a row whose `workspaceId` still equals the key + // it is filed under, so re-keying the map alone would strand every page under the new id. + sessionChanged = + moveSessionKey(session.clientHostedBrowserPagesByWorktree, (rows) => + rows.map((row) => + row.workspaceId === oldWorktreeId ? { ...row, workspaceId: newWorktreeId } : row + ) + ) || sessionChanged sessionChanged = moveSessionKey(session.activeBrowserTabIdByWorktree) || sessionChanged sessionChanged = moveSessionKey(session.activeTabTypeByWorktree) || sessionChanged sessionChanged = moveSessionKey(session.activeTabIdByWorktree) || sessionChanged @@ -162,35 +250,17 @@ export function migrateWorktreeIdentity( session.activeWorkspaceKey = newWorkspaceKey sessionChanged = true } - if (session.sleepingAgentSessionsByPaneKey) { - let sleepingChanged = false - const nextSleeping = { ...session.sleepingAgentSessionsByPaneKey } - for (const [paneKey, record] of Object.entries(nextSleeping)) { - if (record.worktreeId !== oldWorktreeId) { - continue - } - nextSleeping[paneKey] = { ...record, worktreeId: newWorktreeId } - sleepingChanged = true - } - if (sleepingChanged) { - session.sleepingAgentSessionsByPaneKey = nextSleeping - sessionChanged = true - } + // Why every row-valued map and not just the two that used to be here: a record keyed by pane or + // tab id still names its worktree in the value, and a stale one silently stops matching. A + // `closedTerminalTabTombstonesByTabId` row left on the old id never suppresses the tab it was + // minted for and never gets acknowledged, so the remote merge re-adds a tab the user closed. + for (const field of WORKTREE_ROW_RECORD_SESSION_FIELDS) { + sessionChanged = + repointRowRecord(session, field, oldWorktreeId, newWorktreeId) || sessionChanged } - if (session.terminalSurfaceTombstonesByPaneKey) { - let tombstonesChanged = false - const nextTombstones = { ...session.terminalSurfaceTombstonesByPaneKey } - for (const [paneKey, tombstone] of Object.entries(nextTombstones)) { - if (tombstone.worktreeId !== oldWorktreeId) { - continue - } - nextTombstones[paneKey] = { ...tombstone, worktreeId: newWorktreeId } - tombstonesChanged = true - } - if (tombstonesChanged) { - session.terminalSurfaceTombstonesByPaneKey = nextTombstones - sessionChanged = true - } + for (const field of WORKTREE_ROW_ARRAY_SESSION_FIELDS) { + sessionChanged = + repointRowArrays(session, field, oldWorktreeId, newWorktreeId) || sessionChanged } return sessionChanged } diff --git a/src/renderer/src/store/slices/worktrees/session/worktree-identity-rename-row-worktree-ids.test.ts b/src/renderer/src/store/slices/worktrees/session/worktree-identity-rename-row-worktree-ids.test.ts new file mode 100644 index 00000000000..7fad6903c79 --- /dev/null +++ b/src/renderer/src/store/slices/worktrees/session/worktree-identity-rename-row-worktree-ids.test.ts @@ -0,0 +1,91 @@ +/** + * Rename has to re-point the maps that name their worktree in the VALUE, not the key. + * + * `WORKTREE_ID_KEYED_MAP_KEYS` covers the `*ByWorktree` maps, and the rename path deliberately + * skips tab- and file-keyed ones because those ids survive a rename. Two of the skipped maps carry + * the worktree id inside each row, and a stale one there is not residue — it is a suppression that + * silently stops matching: + * + * - `closedTerminalTabTombstonesByTabId`: the remote merge only suppresses a host tab when the + * tombstone's worktree equals the tab's, so a tombstone left on the old id re-admits a terminal + * tab the user closed, and never gets acknowledged because no snapshot covers the old id. + * - `clientHostedBrowserCloseIntentsByEnvironment`: the replay targets `intent.worktreeId`, and an + * unresolvable selector answers `selector_not_found` — a code the replay reads as "definitively + * gone" and uses to DROP the intent, leaving the page the user closed open forever. + * + * Main-process counterpart: worktree-identity-migration-field-coverage.test.ts. + */ +import { describe, expect, it } from 'vitest' +import type { AppState } from '../../../types' +import { buildWorktreeRenameState } from './worktree-identity-rename-state' + +const OLD = 'repo1::/ws/old' +const NEW = 'repo1::/ws/new' +const OTHER = 'repo1::/ws/other' + +function appState(overrides: Partial): AppState { + return { + lastVisitedAtByWorktreeId: {}, + everActivatedWorktreeIds: new Set(), + closedTerminalTabTombstonesByTabId: {}, + clientHostedBrowserCloseIntentsByEnvironment: {}, + ...overrides + } as unknown as AppState +} + +describe('buildWorktreeRenameState value-owned worktree rows', () => { + it('re-points a closed-terminal-tab tombstone onto the new worktree id', () => { + const next = buildWorktreeRenameState( + appState({ + closedTerminalTabTombstonesByTabId: { + 'tab-1': { closedAt: 5, worktreeId: OLD, ackRevision: 3 }, + 'tab-2': { closedAt: 6, worktreeId: OTHER } + } + }), + OLD, + NEW + ) + expect(next.closedTerminalTabTombstonesByTabId).toEqual({ + 'tab-1': { closedAt: 5, worktreeId: NEW, ackRevision: 3 }, + 'tab-2': { closedAt: 6, worktreeId: OTHER } + }) + }) + + it('re-points a client-hosted browser close intent onto the new worktree id', () => { + const next = buildWorktreeRenameState( + appState({ + clientHostedBrowserCloseIntentsByEnvironment: { + 'env-1': [ + { browserPageId: 'page-1', worktreeId: OLD, closedAt: 3 }, + { browserPageId: 'page-2', worktreeId: OTHER, closedAt: 4 } + ], + 'env-2': [{ browserPageId: 'page-3', worktreeId: OTHER, closedAt: 5 }] + } + }), + OLD, + NEW + ) + expect(next.clientHostedBrowserCloseIntentsByEnvironment).toEqual({ + 'env-1': [ + { browserPageId: 'page-1', worktreeId: NEW, closedAt: 3 }, + { browserPageId: 'page-2', worktreeId: OTHER, closedAt: 4 } + ], + 'env-2': [{ browserPageId: 'page-3', worktreeId: OTHER, closedAt: 5 }] + }) + }) + + it('emits neither map when no row names the renamed worktree', () => { + const next = buildWorktreeRenameState( + appState({ + closedTerminalTabTombstonesByTabId: { 'tab-2': { closedAt: 6, worktreeId: OTHER } }, + clientHostedBrowserCloseIntentsByEnvironment: { + 'env-1': [{ browserPageId: 'page-2', worktreeId: OTHER, closedAt: 4 }] + } + }), + OLD, + NEW + ) + expect(Object.hasOwn(next, 'closedTerminalTabTombstonesByTabId')).toBe(false) + expect(Object.hasOwn(next, 'clientHostedBrowserCloseIntentsByEnvironment')).toBe(false) + }) +}) diff --git a/src/renderer/src/store/slices/worktrees/session/worktree-identity-rename-state.ts b/src/renderer/src/store/slices/worktrees/session/worktree-identity-rename-state.ts index dee9ebb7f84..1d863890685 100644 --- a/src/renderer/src/store/slices/worktrees/session/worktree-identity-rename-state.ts +++ b/src/renderer/src/store/slices/worktrees/session/worktree-identity-rename-state.ts @@ -192,6 +192,43 @@ export function buildWorktreeRenameState( const pendingReconnectWorktreeIds = s.pendingReconnectWorktreeIds?.includes(oldWorktreeId) ? s.pendingReconnectWorktreeIds.map((id) => (id === oldWorktreeId ? newWorktreeId : id)) : s.pendingReconnectWorktreeIds + // Why these two and not just the pane records below: both are keyed by something other than the + // worktree, so the rename path skipped them, but each row names the worktree in its VALUE. A + // close tombstone on the old id never matches the merge's worktree scope, so a terminal tab the + // user closed is re-added by the next host snapshot; a close intent on the old id replays against + // a selector that no longer resolves, which reads as `definitively gone` and drops the intent + // while the page is still open. Both are resurrections the maps exist to prevent. + const repointRows = ( + rows: readonly T[] + ): { rows: T[]; changed: boolean } => { + let changed = false + const next = rows.map((row) => { + if (row.worktreeId !== oldWorktreeId) { + return row + } + changed = true + return { ...row, worktreeId: newWorktreeId } + }) + return { rows: next, changed } + } + const currentClosedTombstones = s.closedTerminalTabTombstonesByTabId ?? {} + const closedTombstoneEntries = repointRows( + Object.entries(currentClosedTombstones).map(([tabId, tombstone]) => ({ ...tombstone, tabId })) + ) + const closedTerminalTabTombstonesByTabId = closedTombstoneEntries.changed + ? Object.fromEntries( + closedTombstoneEntries.rows.map(({ tabId, ...tombstone }) => [tabId, tombstone]) + ) + : s.closedTerminalTabTombstonesByTabId + const currentCloseIntents = s.clientHostedBrowserCloseIntentsByEnvironment ?? {} + let closeIntentsChanged = false + const clientHostedBrowserCloseIntentsByEnvironment = Object.fromEntries( + Object.entries(currentCloseIntents).map(([environmentId, intents]) => { + const repointed = repointRows(intents) + closeIntentsChanged = closeIntentsChanged || repointed.changed + return [environmentId, repointed.changed ? repointed.rows : intents] + }) + ) const currentSleepingAgentSessionsByPaneKey = s.sleepingAgentSessionsByPaneKey ?? {} const sleepingAgentSessionsByPaneKey = Object.values(currentSleepingAgentSessionsByPaneKey).some( (record) => record.worktreeId === oldWorktreeId @@ -220,6 +257,10 @@ export function buildWorktreeRenameState( ...(sleepingAgentSessionsByPaneKey !== s.sleepingAgentSessionsByPaneKey ? { sleepingAgentSessionsByPaneKey } : {}), + ...(closedTerminalTabTombstonesByTabId !== s.closedTerminalTabTombstonesByTabId + ? { closedTerminalTabTombstonesByTabId } + : {}), + ...(closeIntentsChanged ? { clientHostedBrowserCloseIntentsByEnvironment } : {}), ...(s.activeWorktreeId === oldWorktreeId ? { activeWorktreeId: newWorktreeId } : {}), // The active workspace key derives from the worktree id, so keep it in sync when the active worktree is renamed. ...(s.activeWorkspaceKey === worktreeWorkspaceKey(oldWorktreeId)