diff --git a/src/renderer/src/store/host-twin-isolation.test.ts b/src/renderer/src/store/host-twin-isolation.test.ts index f19e309088f..62ea7b80e58 100644 --- a/src/renderer/src/store/host-twin-isolation.test.ts +++ b/src/renderer/src/store/host-twin-isolation.test.ts @@ -208,12 +208,7 @@ const TWIN_KINDS: readonly HostTwinKind[] = [ ] /** ` | | ` → why the survivor is touched today (tracker Stage 1 worklist). */ -const KNOWN_TWIN_LEAKS: Readonly> = Object.fromEntries([ - [ - 'remove project | local+runtime | local', - 'a sleeping-agent record with no host evidence is dropped instead of kept for the survivor' - ] -]) +const KNOWN_TWIN_LEAKS: Readonly> = Object.fromEntries([]) function cellId(action: TwinAction, kind: HostTwinKind, target: 'local' | 'remote'): string { return `${action.id} | ${kind} | ${target}` diff --git a/src/renderer/src/store/repos/repo-removal-host-twin-attribution.ts b/src/renderer/src/store/repos/repo-removal-host-twin-attribution.ts index e835f0c47a4..e1bc36b637c 100644 --- a/src/renderer/src/store/repos/repo-removal-host-twin-attribution.ts +++ b/src/renderer/src/store/repos/repo-removal-host-twin-attribution.ts @@ -1,19 +1,15 @@ import type { AppState } from '../types' import { - LOCAL_EXECUTION_HOST_ID, toRuntimeExecutionHostId, type ExecutionHostId } from '../../../../shared/execution-host' import { belongsToRemovedHost, - isRuntimeHost, ptyHostEvidence, terminalTabBelongsToHost, terminalTabPtyEvidence, type HostEvidence } from './host-twin-terminal-evidence' - -export { belongsToRemovedHost, isRuntimeHost, type HostEvidence } import { toVisibleTabType } from '../../../../shared/tab-types' import { isExecutionHostAliasForWorktree } from '@/lib/worktree-execution-host-alias' import { getRepoIdFromWorktreeId } from '../../../../shared/worktree/id' @@ -22,72 +18,6 @@ import { hasOpenFileExecutionHostEvidence } from '@/lib/unified-tab-host-ownership' -/** Worktree ids of this host's project that another host also publishes, with those hosts. */ -export function getSurvivingTwinHostIds( - state: Pick, - projectId: string, - hostId: ExecutionHostId -): Map { - const rows = [ - ...(state.worktreesByRepo[projectId] ?? []), - ...(state.detectedWorktreesByRepo[projectId]?.worktrees ?? []) - ] - const hostOf = (row: { hostId?: ExecutionHostId }): ExecutionHostId => - row.hostId ?? LOCAL_EXECUTION_HOST_ID - const removedIds = new Set(rows.filter((row) => hostOf(row) === hostId).map((row) => row.id)) - const survivors = new Map() - for (const row of rows) { - const survivorHostId = hostOf(row) - if (!removedIds.has(row.id) || survivorHostId === hostId) { - continue - } - const hosts = survivors.get(row.id) ?? [] - if (!hosts.includes(survivorHostId)) { - survivors.set(row.id, [...hosts, survivorHostId]) - } - } - return survivors -} - -/** The twins whose surviving host still holds a row now, after the removal's host call. */ -export function retainSurvivingTwinHostIds( - state: Pick, - projectId: string, - twins: ReadonlyMap -): Map { - const rows = [ - ...(state.worktreesByRepo[projectId] ?? []), - ...(state.detectedWorktreesByRepo[projectId]?.worktrees ?? []) - ] - const retained = new Map() - for (const [worktreeId, hostIds] of twins) { - const live = hostIds.filter((hostId) => - rows.some( - (row) => row.id === worktreeId && (row.hostId ?? LOCAL_EXECUTION_HOST_ID) === hostId - ) - ) - if (live.length > 0) { - retained.set(worktreeId, live) - } - } - return retained -} - -/** - * Every id the removal covers, and those it may purge outright. State under an id another host - * still publishes is shared, so there only the removed host's attributed tabs and files go. - */ -export function splitRemovedWorktreeIds( - knownIds: readonly string[], - twinsBeforeCall: ReadonlyMap, - twinsNow: ReadonlyMap -): { worktreeIds: string[]; exclusiveWorktreeIds: string[] } { - const worktreeIds = [ - ...new Set([...knownIds, ...[...twinsBeforeCall.keys()].filter((id) => !twinsNow.has(id))]) - ] - return { worktreeIds, exclusiveWorktreeIds: worktreeIds.filter((id) => !twinsNow.has(id)) } -} - /** * State under a worktree id that a surviving host also publishes, attributed to the removed host. * Anything without host evidence stays: it may be the survivor's. @@ -105,14 +35,14 @@ export type RemovedHostTwinState = { } /** Runtime files always record their environment, so an unmarked one is local/SSH. */ -export function openFileHostEvidence( +function openFileHostEvidence( file: Parameters[0] ): HostEvidence { return hasOpenFileExecutionHostEvidence(file) ? getOpenFileExecutionHostId(file) : 'non-runtime' } /** A browser page's host: null is explicitly this client, absent is unknown. */ -export function pageHostEvidence(page: { +function pageHostEvidence(page: { browserRuntimeEnvironmentId?: string | null }): HostEvidence { if (page.browserRuntimeEnvironmentId === null) { @@ -240,39 +170,47 @@ export function collectRemovedHostTwinState( return removed } -/** - * The settled attribution, plus pre-call entries whose surface has since disappeared. A surface - * still present is judged on its latest evidence, which may now include a surviving host's pane. - */ -export function mergeRemovedHostTwinState( - beforeCall: RemovedHostTwinState, - settled: RemovedHostTwinState, +/** Keeps the three selection fields consistent once the removed host's copy is gone. */ +export function twinSelectionPatch( state: Pick< AppState, - 'tabsByWorktree' | 'unifiedTabsByWorktree' | 'openFiles' | 'browserTabsByWorktree' - > -): RemovedHostTwinState { - const terminalIds = new Set([ - ...Object.values(state.tabsByWorktree).flatMap((tabs) => tabs.map((tab) => tab.id)), - ...Object.values(state.unifiedTabsByWorktree).flatMap((tabs) => - tabs.flatMap((tab) => (tab.contentType === 'terminal' ? [tab.entityId] : [])) - ) - ]) - const unifiedIds = new Set( - Object.values(state.unifiedTabsByWorktree).flatMap((tabs) => tabs.map((tab) => tab.id)) - ) - const fileIds = new Set(state.openFiles.map((file) => file.id)) - const browserIds = new Set( - Object.values(state.browserTabsByWorktree).flatMap((tabs) => tabs.map((tab) => tab.id)) - ) - const keep = (a: Set, b: Set, present: Set): Set => - new Set([...b, ...[...a].filter((id) => !present.has(id))]) - return { - terminalTabIds: keep(beforeCall.terminalTabIds, settled.terminalTabIds, terminalIds), - fileIds: keep(beforeCall.fileIds, settled.fileIds, fileIds), - browserTabIds: keep(beforeCall.browserTabIds, settled.browserTabIds, browserIds), - otherTabIds: keep(beforeCall.otherTabIds, settled.otherTabIds, unifiedIds), - conversationIds: new Set([...beforeCall.conversationIds, ...settled.conversationIds]), - mixedTabPtyIds: settled.mixedTabPtyIds + | 'activeWorktreeId' + | 'activeWorkspaceExecutionHostId' + | 'worktreesByRepo' + | 'detectedWorktreesByRepo' + >, + twinHostIds: ReadonlyMap, + removedHostId: ExecutionHostId +): Partial { + // Why: with no surviving twin, the bulk purge owns the selection exactly as before. + if (twinHostIds.size === 0) { + return {} } + const selected = state.activeWorkspaceExecutionHostId + // Why canonicalized: a copy reached through a paired runtime is selected under that runtime's id. + const candidates = state.activeWorktreeId ? [state.activeWorktreeId] : [...twinHostIds.keys()] + const selectsRemovedHost = + selected === removedHostId || + (selected !== null && + candidates.some( + (worktreeId) => + twinHostIds.has(worktreeId) && + removedHostCanonicalizer( + state, + worktreeId, + removedHostId, + twinHostIds.get(worktreeId) ?? [] + )(selected) === removedHostId + )) + if (!selectsRemovedHost) { + return {} + } + const survivor = state.activeWorktreeId ? twinHostIds.get(state.activeWorktreeId)?.[0] : null + if (survivor) { + return { activeWorkspaceExecutionHostId: survivor } + } + // Why: a teardown close already deselected the workspace; nothing may still name the removed host. + return state.activeWorktreeId === null + ? { activeWorkspaceKey: null, activeWorkspaceExecutionHostId: null } + : {} } diff --git a/src/renderer/src/store/repos/repo-removal-host-twin-teardown.test.ts b/src/renderer/src/store/repos/repo-removal-host-twin-teardown.test.ts new file mode 100644 index 00000000000..aa0afb27b1b --- /dev/null +++ b/src/renderer/src/store/repos/repo-removal-host-twin-teardown.test.ts @@ -0,0 +1,48 @@ +import { describe, expect, it, vi } from 'vitest' +import { createTestStore } from '../slices/store-test-helpers' +import { closeRemovedHostTwinState, type RemovedHostTwins } from './repo-removal-host-twin-teardown' + +const cancel = vi.hoisted(() => + vi.fn((_worktreeIds: ReadonlySet, _ownedByHost?: unknown) => new Set()) +) +vi.mock('../slices/worktrees/teardown/purged-structured-launch-retirement', () => ({ + cancelStructuredLaunchesForWorktrees: cancel +})) + +const emptyState = () => ({ + terminalTabIds: new Set(), + fileIds: new Set(), + browserTabIds: new Set(), + otherTabIds: new Set(), + conversationIds: new Set(), + mixedTabPtyIds: new Set() +}) + +describe('closeRemovedHostTwinState', () => { + it('cancels launches only under each attributed host’s own twin ids', () => { + const store = createTestStore() + // A is the runtime copy's twin with an SSH survivor; B is attributed to that SSH host. + const twins: RemovedHostTwins = { + twinHostIds: new Map([ + ['repo::/a', ['ssh:target']], + ['repo::/b', ['local']] + ]), + byHost: [ + { + hostId: 'runtime:env', + twinHostIds: new Map([['repo::/a', ['ssh:target']]]), + removed: emptyState() + }, + { + hostId: 'ssh:target', + twinHostIds: new Map([['repo::/b', ['local']]]), + removed: emptyState() + } + ] + } + + closeRemovedHostTwinState(store.getState, store.setState, twins, { ...twins, byHost: [] }) + + expect(cancel.mock.calls.map(([ids]) => [...ids])).toEqual([['repo::/a'], ['repo::/b']]) + }) +}) diff --git a/src/renderer/src/store/repos/repo-removal-host-twin-teardown.ts b/src/renderer/src/store/repos/repo-removal-host-twin-teardown.ts index 21df8575f32..e53cb176cad 100644 --- a/src/renderer/src/store/repos/repo-removal-host-twin-teardown.ts +++ b/src/renderer/src/store/repos/repo-removal-host-twin-teardown.ts @@ -1,262 +1,152 @@ import type { AppState } from '../types' -import type { ExecutionHostId } from '../../../../shared/execution-host' -import { toSshExecutionHostId } from '../../../../shared/execution-host' +import { parseExecutionHostId, type ExecutionHostId } from '../../../../shared/execution-host' import { isRemoteRuntimePtyId } from '../../../../shared/remote-runtime-pty-id' import { buildTerminalTabRetirementPlans } from '../slices/terminal-tab-retirement' -import { omitSleepingAgentSessionsForWorktrees } from '../slices/worktrees/teardown/sleeping-agent-session-omission' -import type { RecentlyClosedTabKind } from '../slices/recently-closed-tabs' -import { omitRecordKeys } from '../slices/worktrees/teardown/record-key-omission' import { cancelStructuredLaunchesForWorktrees } from '../slices/worktrees/teardown/purged-structured-launch-retirement' +import { collectHostTwinPurgeScopes } from '../slices/worktrees/teardown/host-twin-worktree-purge' +import { omitRecordKeys } from '../slices/worktrees/teardown/record-key-omission' import { - belongsToRemovedHost, collectRemovedHostTwinState, - isRuntimeHost, - mergeRemovedHostTwinState, - openFileHostEvidence, removedHostCanonicalizer, - pageHostEvidence, - type HostEvidence, type RemovedHostTwinState } from './repo-removal-host-twin-attribution' -/** Which entries of one reopen stack are the removed host's; unmarked ones may be the survivor's. */ -function prunedEntries(stack: readonly T[] | undefined, isRemoved: (entry: T) => boolean) { - const pruned = (stack ?? []).map(isRemoved) - return pruned.includes(true) ? pruned : null +export type RemovedHostTwins = { + /** Twin ids with the hosts that still publish them. */ + twinHostIds: Map + /** Per attributed host (a paired runtime's copy names its SSH host), its surfaces there. */ + byHost: { + hostId: ExecutionHostId + /** This host's own twin ids with their survivors. */ + twinHostIds: ReadonlyMap + removed: RemovedHostTwinState + }[] } -/** The n-th kind entry of a type is that type's n-th snapshot, so pruned ones go from both. */ -function dropPrunedKinds( - kinds: readonly RecentlyClosedTabKind[], - kind: RecentlyClosedTabKind, - pruned: readonly boolean[] -): RecentlyClosedTabKind[] { - let index = 0 - return kinds.filter((entry) => entry !== kind || !pruned[index++]) -} - -function pruneRemovedHostReopenHistory( +/** The removed copy's ids that another host still publishes, matched as the purge matches them. */ +export function collectRemovedHostTwins( state: AppState, - twinHostIds: ReadonlyMap, - removedHostId: ExecutionHostId -): Partial | null { - const editors = { ...state.recentlyClosedEditorTabsByWorktree } - const browsers = { ...state.recentlyClosedBrowserTabsByWorktree } - const kinds = { ...state.recentlyClosedTabKindsByWorktree } - let changed = false - for (const [worktreeId, survivors] of twinHostIds) { - const canonical = removedHostCanonicalizer(state, worktreeId, removedHostId, survivors) - const editorPruned = prunedEntries(editors[worktreeId], (snapshot) => - belongsToRemovedHost(canonical(openFileHostEvidence(snapshot)), removedHostId, survivors) - ) - const browserPruned = prunedEntries( - browsers[worktreeId], - (snapshot) => - snapshot.pages.length > 0 && - snapshot.pages - .map(pageHostEvidence) - .every((evidence) => belongsToRemovedHost(canonical(evidence), removedHostId, survivors)) - ) - if (editorPruned) { - editors[worktreeId] = (editors[worktreeId] ?? []).filter((_, i) => !editorPruned[i]) - kinds[worktreeId] = dropPrunedKinds(kinds[worktreeId] ?? [], 'editor', editorPruned) - } - if (browserPruned) { - browsers[worktreeId] = (browsers[worktreeId] ?? []).filter((_, i) => !browserPruned[i]) - kinds[worktreeId] = dropPrunedKinds(kinds[worktreeId] ?? [], 'browser', browserPruned) - } - changed ||= Boolean(editorPruned || browserPruned) + worktreeIds: readonly string[], + hostId: ExecutionHostId +): RemovedHostTwins { + const scopes = [ + ...collectHostTwinPurgeScopes( + state, + worktreeIds.map((id) => ({ id, hostId })) + ).byHost + ] + return { + twinHostIds: new Map(scopes.flatMap(([, twins]) => [...twins])), + byHost: scopes.map(([attributedHostId, twins]) => ({ + hostId: attributedHostId, + twinHostIds: twins, + removed: collectRemovedHostTwinState(state, new Set(twins.keys()), attributedHostId, twins) + })) } - return changed - ? { - recentlyClosedEditorTabsByWorktree: editors, - recentlyClosedBrowserTabsByWorktree: browsers, - recentlyClosedTabKindsByWorktree: kinds - } - : null -} - -/** Sleeping-agent records can outlive their tab, so they are matched by the host they name too. */ -function omitRemovedHostSleepingSessions( - state: AppState, - twinHostIds: ReadonlyMap, - removedHostId: ExecutionHostId, - removed: RemovedHostTwinState -): ReturnType { - const doomedPaneKeys: string[] = [] - for (const [paneKey, record] of Object.entries(state.sleepingAgentSessionsByPaneKey ?? {})) { - const survivors = twinHostIds.get(record.worktreeId) - if (!survivors) { - continue - } - const evidence: HostEvidence = - record.connectionId === undefined - ? null - : record.connectionId - ? toSshExecutionHostId(record.connectionId) - : 'non-runtime' - if ( - (record.tabId !== undefined && removed.terminalTabIds.has(record.tabId)) || - belongsToRemovedHost( - removedHostCanonicalizer(state, record.worktreeId, removedHostId, survivors)(evidence), - removedHostId, - survivors - ) - ) { - doomedPaneKeys.push(paneKey) - } - } - return omitSleepingAgentSessionsForWorktrees(state, new Set(), doomedPaneKeys) } /** - * Closes only the removed host's twin state; the survivor's tabs, files and drafts stay. Merges - * the pre-call snapshot, which survives the announcement's refresh, with tabs opened meanwhile. + * Closes the surfaces attributed to the removed copy through their close actions, so chats, + * previews and shells retire as on any close. Anything unattributed (mixed tabs, reopen history, + * tabless sleeping records) stays: a stale entry, never the survivor's data lost. */ export function closeRemovedHostTwinState( get: () => AppState, set: (patch: (state: AppState) => Partial) => void, - twinHostIds: ReadonlyMap, - beforeCall: RemovedHostTwinState, - removedHostId: ExecutionHostId + twins: RemovedHostTwins, + preCall: RemovedHostTwins ): void { - const twinWorktreeIds = new Set(twinHostIds.keys()) - const removed = mergeRemovedHostTwinState( - beforeCall, - collectRemovedHostTwinState(get(), twinWorktreeIds, removedHostId, twinHostIds), - get() - ) - const sleepingPatch = omitRemovedHostSleepingSessions(get(), twinHostIds, removedHostId, removed) - if (Object.keys(sleepingPatch).length > 0) { - set(() => sleepingPatch) - } - const historyPatch = pruneRemovedHostReopenHistory(get(), twinHostIds, removedHostId) - if (historyPatch) { - set(() => historyPatch) - } - // Why: a runtime host stopped its own shells on removal, so no close is sent back to it. - const remoteCloseOwnedByHost = isRuntimeHost(removedHostId) - // Why planned together: the planner keeps a PTY another surviving tab still binds. - const plans = buildTerminalTabRetirementPlans(get(), [...removed.terminalTabIds]) - // Why batch-wide: the planner may give a PTY's teardown to a tab that only holds it as a hint. - const liveInBatch = new Set( - [...removed.terminalTabIds].flatMap((tabId) => get().ptyIdsByTabId[tabId] ?? []) - ) - const retiredPtyIds = new Set() - const killedPtyIds = new Set() - for (const [tabId, plan] of plans) { - for (const ptyId of [ - ...plan.localOrSshPtyIds, - ...plan.runtimeTerminals.map((terminal) => terminal.ptyId), - ...plan.cleanupOnlyPtyIds, - ...plan.unroutablePtyIds - ]) { - retiredPtyIds.add(ptyId) - } - if (!remoteCloseOwnedByHost) { + const state = get() + const presentTabIds = new Set([ + ...Object.values(state.tabsByWorktree).flatMap((tabs) => tabs.map((tab) => tab.id)), + ...Object.values(state.unifiedTabsByWorktree).flatMap((tabs) => tabs.map((tab) => tab.entityId)) + ]) + // Why pre-call hosts too: a concurrent removal of the other copy can leave no twin to rescan. + for (const hostId of new Set([...twins.byHost, ...preCall.byHost].map((entry) => entry.hostId))) { + const current = twins.byHost.find((entry) => entry.hostId === hostId) + const removed = current?.removed + // Why this host's ids only: another attributed host's survivor may be this host. + const hostTwinIds = current?.twinHostIds ?? new Map() + // Why: a host frame during the call can drop a tab without closeTab's agent and cache cleanup. + const terminalTabIds = new Set([ + ...(removed?.terminalTabIds ?? []), + ...preCall.byHost + .filter((entry) => entry.hostId === hostId) + .flatMap((entry) => [...entry.removed.terminalTabIds]) + .filter((tabId) => !presentTabIds.has(tabId)) + ]) + // Why: a runtime host stopped its own shells on removal, so no close is sent back to it. + const remoteCloseOwnedByHost = parseExecutionHostId(hostId)?.kind === 'runtime' + // Why planned together: the planner keeps a PTY another surviving tab still binds. + const plans = buildTerminalTabRetirementPlans(get(), [...terminalTabIds]) + // Why live-indexed only: a parked layout hint may be another host's session id. + const liveInBatch = new Set( + [...terminalTabIds].flatMap((tabId) => get().ptyIdsByTabId[tabId] ?? []) + ) + const killedPtyIds = new Set() + const retiredPtyIds = new Set() + for (const [tabId, plan] of plans) { + for (const ptyId of [ + ...plan.localOrSshPtyIds, + ...plan.runtimeTerminals.map((terminal) => terminal.ptyId), + ...plan.cleanupOnlyPtyIds, + ...plan.unroutablePtyIds + ]) { + retiredPtyIds.add(ptyId) + } // Why killed here: the shared id resolves to no single host, so the planner marks this // host's PTYs unroutable; attribution already proved they are the removed host's. - // Only live-indexed PTYs, as before: a parked layout hint may be another host's session id. - const candidates = [ - ...plan.localOrSshPtyIds, - ...plan.unroutablePtyIds, - ...plan.cleanupOnlyPtyIds - ] + const candidates = remoteCloseOwnedByHost + ? [] + : [...plan.localOrSshPtyIds, ...plan.unroutablePtyIds, ...plan.cleanupOnlyPtyIds] for (const ptyId of candidates) { if (liveInBatch.has(ptyId) && !isRemoteRuntimePtyId(ptyId) && !killedPtyIds.has(ptyId)) { killedPtyIds.add(ptyId) window.api.pty.kill(ptyId) } } + get().closeTab(tabId, { + reason: 'cleanup', + captureRecentlyClosed: false, + remoteCloseOwnedByHost, + localPtyTeardownOwnedExternally: !remoteCloseOwnedByHost, + precomputedRetirementPlan: plan + }) } - get().closeTab(tabId, { - reason: 'cleanup', - captureRecentlyClosed: false, - remoteCloseOwnedByHost, - localPtyTeardownOwnedExternally: !remoteCloseOwnedByHost, - precomputedRetirementPlan: plan - }) - } - if (!remoteCloseOwnedByHost) { // Why: the tab stays for its surviving pane, but this host's shell in it must not outlive the project. - for (const ptyId of removed.mixedTabPtyIds) { + for (const ptyId of remoteCloseOwnedByHost ? [] : (removed?.mixedTabPtyIds ?? [])) { if (!killedPtyIds.has(ptyId) && !isRemoteRuntimePtyId(ptyId)) { window.api.pty.kill(ptyId) } } - } - // Why: a parked or restored pane has no exit callback left to clear these; shared PTYs are not in the plans' lists. - set((state) => ({ - suppressedPtyExitIds: omitRecordKeys(state.suppressedPtyExitIds, retiredPtyIds), - pendingPaneAccountRestarts: omitRecordKeys(state.pendingPaneAccountRestarts, retiredPtyIds), - codexRestartNoticeByPtyId: omitRecordKeys(state.codexRestartNoticeByPtyId, retiredPtyIds) - })) - // Why: a pending chat's tab can already be gone, so its launch is cancelled through the registry. - cancelStructuredLaunchesForWorktrees( - new Set(twinHostIds.keys()), - (worktreeId, hostId) => - removedHostCanonicalizer( - get(), - worktreeId, - removedHostId, - twinHostIds.get(worktreeId) ?? [] - )(hostId) === removedHostId - ) - for (const fileId of removed.fileIds) { - // Why: a file the announcement's refresh already dropped would make closeFile reset the focused surface. - if (get().openFiles.some((file) => file.id === fileId)) { - get().closeFile(fileId, { captureRecentlyClosed: false }) + // Why: a parked or restored pane has no exit callback left to clear these; shared PTYs are not in the plans' lists. + set((state) => ({ + suppressedPtyExitIds: omitRecordKeys(state.suppressedPtyExitIds, retiredPtyIds), + pendingPaneAccountRestarts: omitRecordKeys(state.pendingPaneAccountRestarts, retiredPtyIds), + codexRestartNoticeByPtyId: omitRecordKeys(state.codexRestartNoticeByPtyId, retiredPtyIds) + })) + // Why: a pending chat's tab can already be gone, so its launch is cancelled through the registry. + cancelStructuredLaunchesForWorktrees( + new Set(hostTwinIds.keys()), + (worktreeId, launchHostId) => + removedHostCanonicalizer( + get(), + worktreeId, + hostId, + hostTwinIds.get(worktreeId) ?? [] + )(launchHostId) === hostId + ) + for (const fileId of removed?.fileIds ?? []) { + // Why: a file a listing refresh already dropped would make closeFile reset the focused surface. + if (get().openFiles.some((file) => file.id === fileId)) { + get().closeFile(fileId, { captureRecentlyClosed: false }) + } + } + for (const browserTabId of removed?.browserTabIds ?? []) { + get().closeBrowserTab(browserTabId, { reason: 'cleanup' }) + } + for (const tabId of removed?.otherTabIds ?? []) { + get().closeUnifiedTab(tabId, { preserveWorktreeSelection: true }) } } - for (const browserTabId of removed.browserTabIds) { - get().closeBrowserTab(browserTabId, { reason: 'cleanup' }) - } - for (const tabId of removed.otherTabIds) { - get().closeUnifiedTab(tabId, { preserveWorktreeSelection: true }) - } -} - -/** Keeps the three selection fields consistent once the removed host's copy is gone. */ -export function twinSelectionPatch( - state: Pick< - AppState, - | 'activeWorktreeId' - | 'activeWorkspaceExecutionHostId' - | 'worktreesByRepo' - | 'detectedWorktreesByRepo' - >, - twinHostIds: ReadonlyMap, - removedHostId: ExecutionHostId -): Partial { - // Why: with no surviving twin, the bulk purge owns the selection exactly as before. - if (twinHostIds.size === 0) { - return {} - } - const selected = state.activeWorkspaceExecutionHostId - // Why canonicalized: a copy reached through a paired runtime is selected under that runtime's id. - const candidates = state.activeWorktreeId ? [state.activeWorktreeId] : [...twinHostIds.keys()] - const selectsRemovedHost = - selected === removedHostId || - (selected !== null && - candidates.some( - (worktreeId) => - twinHostIds.has(worktreeId) && - removedHostCanonicalizer( - state, - worktreeId, - removedHostId, - twinHostIds.get(worktreeId) ?? [] - )(selected) === removedHostId - )) - if (!selectsRemovedHost) { - return {} - } - const survivor = state.activeWorktreeId ? twinHostIds.get(state.activeWorktreeId)?.[0] : null - if (survivor) { - return { activeWorkspaceExecutionHostId: survivor } - } - // Why: a teardown close already deselected the workspace; nothing may still name the removed host. - return state.activeWorktreeId === null - ? { activeWorkspaceKey: null, activeWorkspaceExecutionHostId: null } - : {} } diff --git a/src/renderer/src/store/repos/repo-removal.ts b/src/renderer/src/store/repos/repo-removal.ts index c41e966690f..38034b15515 100644 --- a/src/renderer/src/store/repos/repo-removal.ts +++ b/src/renderer/src/store/repos/repo-removal.ts @@ -17,13 +17,11 @@ import { stopRuntimeProjectTerminalsBeforeRemoval } from './runtime-project-term import type { RepoSlice } from './repo-state' import { ERROR_TOAST_DURATION } from './repo-state' import { mergeProjectCompatibilityForHostRepoChange } from './repo-catalog-identity' -import { closeRemovedHostTwinState, twinSelectionPatch } from './repo-removal-host-twin-teardown' import { - collectRemovedHostTwinState, - getSurvivingTwinHostIds, - retainSurvivingTwinHostIds, - splitRemovedWorktreeIds -} from './repo-removal-host-twin-attribution' + closeRemovedHostTwinState, + collectRemovedHostTwins +} from './repo-removal-host-twin-teardown' +import { twinSelectionPatch } from './repo-removal-host-twin-attribution' import { captureWorkspaceChatDraftKeys, deleteWorkspaceChatDrafts @@ -119,24 +117,21 @@ export function createRepoRemovalActions( if (!target) { throw new Error('The project host is unresolved. Refresh and retry.') } + const knownWorktreeIds = getKnownRepoWorktreeIds(get(), projectId, ownerHostId) // Why before the host call: its announcement can start a listing refresh that drops these tabs. - const survivingTwinHostIds = getSurvivingTwinHostIds(get(), projectId, ownerHostId) - const removedTwinState = collectRemovedHostTwinState( - get(), - new Set(survivingTwinHostIds.keys()), - ownerHostId, - survivingTwinHostIds + const preCallTwins = collectRemovedHostTwins(get(), knownWorktreeIds, ownerHostId) + const removedTwinEntityIds = new Set( + preCallTwins.byHost.flatMap(({ removed }) => [ + ...removed.terminalTabIds, + ...removed.conversationIds + ]) ) - const removedTwinEntityIds = new Set([ - ...removedTwinState.terminalTabIds, - ...removedTwinState.conversationIds - ]) const chatDraftKeys = captureWorkspaceChatDraftKeys( get(), - getKnownRepoWorktreeIds(get(), projectId, ownerHostId).map((workspaceId) => ({ + knownWorktreeIds.map((workspaceId) => ({ workspaceId, executionHostId: ownerHostId, - ...(survivingTwinHostIds.has(workspaceId) + ...(preCallTwins.twinHostIds.has(workspaceId) ? { onlyEntityIds: removedTwinEntityIds } : {}) })) @@ -150,6 +145,7 @@ export function createRepoRemovalActions( await stopRuntimeProjectTerminalsBeforeRemoval( target.environmentId, projectId, + // Why re-read: the registration await can let a listing refresh add worktrees. getKnownRepoWorktreeIds(get(), projectId, ownerHostId) ) } @@ -186,13 +182,15 @@ export function createRepoRemovalActions( const { clearRepoSlugCacheEntry } = await import('../../lib/repo-slug-index') clearRepoSlugCacheEntry(projectId) + const worktreeIds = [ + ...new Set([ + ...knownWorktreeIds, + ...getKnownRepoWorktreeIds(get(), projectId, ownerHostId) + ]) + ] // Why rechecked: a concurrent removal of the other copy leaves no survivor to share with. - const twinHostIds = retainSurvivingTwinHostIds(get(), projectId, survivingTwinHostIds) - const { worktreeIds, exclusiveWorktreeIds } = splitRemovedWorktreeIds( - getKnownRepoWorktreeIds(get(), projectId, ownerHostId), - survivingTwinHostIds, - twinHostIds - ) + const twins = collectRemovedHostTwins(get(), worktreeIds, ownerHostId) + const exclusiveWorktreeIds = worktreeIds.filter((id) => !twins.twinHostIds.has(id)) const localAgentContextProjectIds = ownerHostId === LOCAL_EXECUTION_HOST_ID ? [ @@ -214,7 +212,7 @@ export function createRepoRemovalActions( } // Why: use the canonical per-worktree purge to evict all worktree-scoped maps (hand-deletion leaked most); runs before the set() below so it still sees tabsByWorktree. - closeRemovedHostTwinState(get, set, twinHostIds, removedTwinState, ownerHostId) + closeRemovedHostTwinState(get, set, twins, preCallTwins) get().purgeWorktreeTerminalState(exclusiveWorktreeIds) deleteWorkspaceChatDrafts(chatDraftKeys) get().clearLocalDetectedAgentContextsForProjects(localAgentContextProjectIds) @@ -277,7 +275,7 @@ export function createRepoRemovalActions( hostId: ownerHostId }), activeRepoId: s.activeRepoId === projectId ? null : s.activeRepoId, - ...twinSelectionPatch(s, twinHostIds, ownerHostId), + ...twinSelectionPatch(s, twins.twinHostIds, ownerHostId), filterRepoIds: s.filterRepoIds.filter((id) => id !== projectId), worktreesByRepo: nextWorktrees, detectedWorktreesByRepo: nextDetectedWorktrees, diff --git a/src/renderer/src/store/slices/project-removal-host-twin-state.test.ts b/src/renderer/src/store/slices/project-removal-host-twin-state.test.ts index 55dc707fe9c..384126bad2d 100644 --- a/src/renderer/src/store/slices/project-removal-host-twin-state.test.ts +++ b/src/renderer/src/store/slices/project-removal-host-twin-state.test.ts @@ -225,11 +225,10 @@ describe('removing a remote project keeps its local twin', () => { expect(state.openFiles.map((file) => file.id)).toEqual(['/work/same/local.ts']) expect(state.editorDrafts).toEqual({ '/work/same/local.ts': 'unsaved local edit' }) expect(readNativeChatComposerDraft('tab-local:leaf-1').text).toBe('local chat draft') - // The survivor's reopen stack must not offer the removed host's files, then or before. + // Shared reopen history is left whole: a stale entry, never a lost one of the survivor's. expect( state.recentlyClosedEditorTabsByWorktree[WORKTREE_ID]?.map((snapshot) => snapshot.filePath) - ).toEqual(['/work/same/old-local.ts']) - expect(state.recentlyClosedTabKindsByWorktree[WORKTREE_ID]).toEqual(['browser', 'editor']) + ).toContain('/work/same/old-local.ts') }) it('closes the removed host’s browsers, chats and hydrated terminals, and tabs opened meanwhile', async () => { @@ -298,8 +297,7 @@ describe('removing a remote project keeps its local twin', () => { ]) expect( state.recentlyClosedBrowserTabsByWorktree[WORKTREE_ID]?.map((entry) => entry.workspace.id) - ).toEqual(['closed-local']) - expect(state.recentlyClosedTabKindsByWorktree[WORKTREE_ID]).toEqual(['browser']) + ).toContain('closed-local') }) it('attributes unstamped terminals by the host their PTY ids name, wherever those are kept', async () => { @@ -352,7 +350,7 @@ describe('removing a remote project keeps its local twin', () => { { ...closedSnapshot('/work/same/via-runtime.ts'), runtimeEnvironmentId: 'env-b' } ] }, - // Records whose tabs are gone: only the host they name says whose they are. + // Records whose tabs are gone. sleepingAgentSessionsByPaneKey: { 'gone-ssh:leaf': sleepingRecord('gone-ssh:leaf', 'target-1'), 'gone-local:leaf': sleepingRecord('gone-local:leaf', null) @@ -373,11 +371,11 @@ describe('removing a remote project keeps its local twin', () => { expect(ptyKill).toHaveBeenCalledWith(sshPtyId) // A parked layout hint closes its tab but is never killed: it may name another host's session. expect(ptyKill).not.toHaveBeenCalledWith(toAppSshPtyId('target-1', 'relay-2')) - expect(store.getState().recentlyClosedEditorTabsByWorktree[WORKTREE_ID] ?? []).toEqual([]) expect(store.getState().activeWorkspaceExecutionHostId).not.toBe(REMOTE_HOST) - expect(Object.keys(store.getState().sleepingAgentSessionsByPaneKey)).toEqual([ + // A tabless record names no tab the removal closes, so it stays (stale) rather than risk the survivor's. + expect(Object.keys(store.getState().sleepingAgentSessionsByPaneKey)).toContain( 'gone-local:leaf' - ]) + ) expect(store.getState().tabsByWorktree[WORKTREE_ID]?.map((tab) => tab.id)).toEqual([ 'tab-local' ]) @@ -428,7 +426,7 @@ describe('removing a remote project keeps its local twin', () => { ]) }) - it('removing the local copy drops its local-only browser history and stale editor tabs', async () => { + it('removing the local copy closes its stale editor tabs and stops the local pane in a mixed tab', async () => { const store = seedTwins() const closedBrowser = (id: string, browserRuntimeEnvironmentId: string | null) => ({ workspace: browserWorkspace(id), @@ -486,7 +484,7 @@ describe('removing a remote project keeps its local twin', () => { const state = store.getState() expect( state.recentlyClosedBrowserTabsByWorktree[WORKTREE_ID]?.map((entry) => entry.workspace.id) - ).toEqual(['closed-remote']) + ).toContain('closed-remote') expect(state.unifiedTabsByWorktree[WORKTREE_ID]?.map((tab) => tab.id)).toEqual([ 'tab-remote', 'editor:remote-file', @@ -498,7 +496,7 @@ describe('removing a remote project keeps its local twin', () => { expect(ptyKill).toHaveBeenCalledWith('pty-mixed-local') expect( state.recentlyClosedEditorTabsByWorktree[WORKTREE_ID]?.map((snapshot) => snapshot.filePath) - ).toEqual(['/work/same/old-remote.ts']) + ).toContain('/work/same/old-remote.ts') }) it('leaves no selection naming the removed host after its last editor closes', async () => { @@ -549,7 +547,7 @@ describe('removing a remote project keeps its local twin', () => { ) }) - it('judges a tab that gained a surviving pane during removal on its latest panes', async () => { + it('keeps a tab that gained a surviving pane during removal and stops its removed pane', async () => { const store = seedTwins() store.setState((s) => ({ unifiedTabsByWorktree: { @@ -577,6 +575,70 @@ describe('removing a remote project keeps its local twin', () => { expect(ptyKill).not.toHaveBeenCalledWith(remotePtyId) }) + it('still retires a removed-host tab a host frame dropped during the call', async () => { + const store = seedTwins() + const agentStatus = (paneKey: string) => ({ + state: 'working' as const, + prompt: '', + updatedAt: 1, + stateStartedAt: 1, + paneKey, + stateHistory: [], + agentType: 'claude' as const + }) + store.setState({ + agentStatusByPaneKey: { + 'tab-local:leaf': agentStatus('tab-local:leaf'), + 'tab-remote:leaf': agentStatus('tab-remote:leaf') + } + }) + // Like a snapshot reducer: the tab goes, its agent rows stay. + onRepoRemoved = () => + store.setState((s) => ({ + tabsByWorktree: { + [WORKTREE_ID]: (s.tabsByWorktree[WORKTREE_ID] ?? []).filter( + (tab) => tab.id !== 'tab-remote' + ) + }, + unifiedTabsByWorktree: { + [WORKTREE_ID]: (s.unifiedTabsByWorktree[WORKTREE_ID] ?? []).filter( + (tab) => tab.id !== 'tab-remote' + ) + } + })) + + await store.getState().removeProject(PROJECT_ID, { hostId: REMOTE_HOST }) + + expect(Object.keys(store.getState().agentStatusByPaneKey)).toEqual(['tab-local:leaf']) + }) + + it('retires a dropped removed-host tab when the other copy also went during the call', async () => { + const store = seedTwins() + // The other copy's removal and a host frame both land while this call is pending. + onRepoRemoved = () => + store.setState((s) => ({ + worktreesByRepo: { + [PROJECT_ID]: (s.worktreesByRepo[PROJECT_ID] ?? []).filter( + (row) => row.hostId !== LOCAL_HOST + ) + }, + tabsByWorktree: { + [WORKTREE_ID]: (s.tabsByWorktree[WORKTREE_ID] ?? []).filter( + (tab) => tab.id !== 'tab-remote' + ) + }, + unifiedTabsByWorktree: { + [WORKTREE_ID]: (s.unifiedTabsByWorktree[WORKTREE_ID] ?? []).filter( + (tab) => tab.id !== 'tab-remote' + ) + } + })) + + await store.getState().removeProject(PROJECT_ID, { hostId: REMOTE_HOST }) + + expect(store.getState().ptyIdsByTabId['tab-remote']).toBeUndefined() + }) + it('leaves the selection unchanged on a single-host removal', async () => { const store = createTestStore() const otherKey = worktreeWorkspaceKey('other-repo::/work/other') diff --git a/src/renderer/src/store/slices/worktrees/teardown/host-twin-worktree-purge.ts b/src/renderer/src/store/slices/worktrees/teardown/host-twin-worktree-purge.ts index a544ae64580..cdef4f304d1 100644 --- a/src/renderer/src/store/slices/worktrees/teardown/host-twin-worktree-purge.ts +++ b/src/renderer/src/store/slices/worktrees/teardown/host-twin-worktree-purge.ts @@ -16,9 +16,9 @@ import { getFolderWorkspaceHostId } from '../../../folder-workspaces/folder-work import { getProjectGroupHostId } from '../../project-group-owner-routing' import { collectRemovedHostTwinState, - removedHostCanonicalizer + removedHostCanonicalizer, + twinSelectionPatch } from '../../../repos/repo-removal-host-twin-attribution' -import { twinSelectionPatch } from '../../../repos/repo-removal-host-twin-teardown' import { removeWorktreeLayout } from '../../tabs/worktree-layout-lifecycle' import { buildActiveSurfacePatch } from '../../tabs/tabs-surface' import { worktreeHostMatchOptions, worktreeMatchesHost } from '../listing/worktree-host-ownership'