diff --git a/src/main/ipc/worktrees.test.ts b/src/main/ipc/worktrees.test.ts index b9ea11b85c8..49b4fa019c7 100644 --- a/src/main/ipc/worktrees.test.ts +++ b/src/main/ipc/worktrees.test.ts @@ -8747,6 +8747,9 @@ describe('registerWorktreeHandlers', () => { includeProviderInventory: true, includeLocalRegistry: false }) + expect(store.removeWorktreeMeta).toHaveBeenCalledWith(worktreeId, 'ssh:conn-1') + expect(advertisedUrlWatcherForgetWorktreeMock).not.toHaveBeenCalled() + expect(deleteWorktreeHistoryDirMock).not.toHaveBeenCalled() }) it('fences a mirrored runtime folder workspace sweep to its environment', async () => { diff --git a/src/main/ipc/worktrees.ts b/src/main/ipc/worktrees.ts index 9f7f949aeae..5d44457c8e1 100644 --- a/src/main/ipc/worktrees.ts +++ b/src/main/ipc/worktrees.ts @@ -318,8 +318,8 @@ function resolveWorktreeRemovalOwnerHostId( fallbackHostId?: ExecutionHostId ): ExecutionHostId | undefined { return ( - store.getWorktreeMeta(worktreeId)?.hostId ?? - (repo ? getRepoExecutionHostId(repo) : fallbackHostId) + fallbackHostId ?? + (repo ? getRepoExecutionHostId(repo) : store.getWorktreeMeta(worktreeId)?.hostId) ) } @@ -329,19 +329,23 @@ function removeWorktreeMetadataAndTransientState( hostId?: ExecutionHostId, snapshotPruneBatchId?: string ): void { + const persistedHostId = store.getWorktreeMeta(worktreeId)?.hostId + const preservesSameIdOwner = Boolean(hostId && persistedHostId && persistedHostId !== hostId) // Why: worktree IDs are path-derived and reusable; drop process-local caches before the same ID can map to a new workspace. if (hostId) { store.removeWorktreeMeta(worktreeId, hostId) } else { store.removeWorktreeMeta(worktreeId) } - advertisedUrlWatcher.forgetWorktree(worktreeId) - // Why: drop this worktree's localhost label routes so they don't accumulate in the proxy's route maps all session. - localhostWorktreeLabelProxy.unregisterWorktree(worktreeId) - // Why: schedule async history tree removal — never recursive-rmSync on the delete critical path. - deleteWorktreeHistoryDir(worktreeId) - // Why: release the removed worktree's PR-refresh aliases so coalesced queue entries don't retain it all session (memory creep). - pruneWorktreePRRefreshAliases(worktreeId) + if (!preservesSameIdOwner) { + advertisedUrlWatcher.forgetWorktree(worktreeId) + // Why: drop this worktree's localhost label routes so they don't accumulate in the proxy's route maps all session. + localhostWorktreeLabelProxy.unregisterWorktree(worktreeId) + // Why: schedule async history tree removal — never recursive-rmSync on the delete critical path. + deleteWorktreeHistoryDir(worktreeId) + // Why: release the removed worktree's PR-refresh aliases so coalesced queue entries don't retain it all session (memory creep). + pruneWorktreePRRefreshAliases(worktreeId) + } // Why: removed workspaces must never resurrect from the persisted cleanup/space scan snapshots. const snapshotDirectory = store.getProfileStorageDirectory() if (snapshotPruneBatchId) { diff --git a/src/main/persistence.test.ts b/src/main/persistence.test.ts index cdfd76df8a6..e414d3969d8 100644 --- a/src/main/persistence.test.ts +++ b/src/main/persistence.test.ts @@ -13092,14 +13092,15 @@ describe('Store host-partitioned workspace sessions', () => { expect( store.getWorkspaceSession('runtime:env-a').terminalTopologyRevisionByRepoId?.['repo-split'] - ).toBe(1) + ).toBeUndefined() expect( store.getWorkspaceSession('local').terminalTopologyRevisionByRepoId?.['repo-split'] ).toBeUndefined() expect(store.getWorkspaceSession('local').tabsByWorktree[otherWorktreeId]).toHaveLength(1) + expect(store.getWorktreeMeta(worktreeId)?.hostId).toBe('runtime:env-a') }) - it('trusts persisted ownership over a stale caller hostId', async () => { + it('preserves a same-id persisted owner when another qualified host is removed', async () => { const store = await createStore() const worktreeId = 'repo-split::/workspace/stale' const session = { @@ -13108,16 +13109,17 @@ describe('Store host-partitioned workspace sessions', () => { [worktreeId]: [makeTerminalTab({ id: 'stale-tab', worktreeId })] } } - store.setWorkspaceSession(session, 'runtime:env-a') + store.setWorkspaceSession(session, 'local') store.setWorkspaceSession(session, 'runtime:env-b') - store.setWorktreeMeta(worktreeId, { hostId: 'runtime:env-a' }) + store.setWorktreeMeta(worktreeId, { hostId: 'local' }) - // A caller's hostId comes from live routing and can go stale mid-removal; the same - // repoId::path can name a live worktree on env-b, whose tabs must survive. + // The confirmed removal target is env-b. Bare metadata belongs to the same-id + // local owner and must not redirect the post-delete purge back to local. store.removeWorktreeMeta(worktreeId, 'runtime:env-b') - expect(store.getWorkspaceSession('runtime:env-a').tabsByWorktree[worktreeId]).toBeUndefined() - expect(store.getWorkspaceSession('runtime:env-b').tabsByWorktree[worktreeId]).toHaveLength(1) + expect(store.getWorktreeMeta(worktreeId)?.hostId).toBe('local') + expect(store.getWorkspaceSession('local').tabsByWorktree[worktreeId]).toHaveLength(1) + expect(store.getWorkspaceSession('runtime:env-b').tabsByWorktree[worktreeId]).toBeUndefined() }) it('falls back to the caller hostId only when no ownership was recorded', async () => { diff --git a/src/main/persistence.ts b/src/main/persistence.ts index 99829912f2e..40b7387b398 100644 --- a/src/main/persistence.ts +++ b/src/main/persistence.ts @@ -5640,12 +5640,19 @@ export class Store { } removeWorktreeMeta(worktreeId: string, hostId?: ExecutionHostId | null): void { - // Persisted ownership beats stale live routing; hostId is only an ownerless fallback. - const owner = this.state.worktreeMeta[worktreeId]?.hostId ?? hostId + const persistedOwner = this.state.worktreeMeta[worktreeId]?.hostId + const owner = hostId ?? persistedOwner + const preservesDifferentPersistedOwner = Boolean( + hostId && persistedOwner && persistedOwner !== hostId + ) // Skip partitions main never wrote: materializing one fences every sibling worktree of the repo. const partitions = new Set( - workspaceSessionPartitionIdsForHost(owner).filter((partition) => - this.hasPersistedWorkspaceSession(partition) + workspaceSessionPartitionIdsForHost(owner).filter( + (partition) => + this.hasPersistedWorkspaceSession(partition) && + // The local partition can be a remote spill surface or a real local + // same-id owner. Preserve it when metadata proves the latter. + (!preservesDifferentPersistedOwner || partition === owner) ) ) // A repo-wide fence must not rebase a sibling's unpersisted tabs onto main's copy, and a spill @@ -5659,9 +5666,11 @@ export class Store { !this.partitionHasOtherRepoWorktreeTabs(worktreeId, partition)) ) ) - delete this.state.worktreeMeta[worktreeId] - delete this.state.worktreeLineageById[worktreeId] - delete this.state.workspaceLineageByChildKey[worktreeWorkspaceKey(worktreeId)] + if (!preservesDifferentPersistedOwner) { + delete this.state.worktreeMeta[worktreeId] + delete this.state.worktreeLineageById[worktreeId] + delete this.state.workspaceLineageByChildKey[worktreeWorkspaceKey(worktreeId)] + } for (const partition of partitions) { this.removeWorkspaceSessionOwnerInPartition(worktreeId, partition, { advanceTerminalTopologyRevision: fencedPartitions.has(partition) diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 0dc5fc76621..8726697a6d2 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -2363,6 +2363,35 @@ describe('OrcaRuntimeService', () => { expect(closeTab).toHaveBeenCalledTimes(2) }) + it('preserves bare-id runtime state when removing a different qualified owner', () => { + const removeWorktreeMeta = vi.fn() + const runtimeStore = { + ...store, + getWorktreeMeta: () => ({ ...store.getWorktreeMeta(TEST_WORKTREE_ID), hostId: 'local' }), + removeWorktreeMeta + } + const runtime = new OrcaRuntimeService(runtimeStore as never) + const internals = runtime as unknown as { + mobileSessionTabsByWorktree: Map + removeWorktreeMetadataAndHistory: ( + runtimeStore: typeof store, + worktreeId: string, + hostId: string + ) => void + } + const localSession = { tabs: [{ id: 'local-tab' }] } + internals.mobileSessionTabsByWorktree.set(TEST_WORKTREE_ID, localSession) + + internals.removeWorktreeMetadataAndHistory( + runtimeStore as typeof store, + TEST_WORKTREE_ID, + 'runtime:env-b' + ) + + expect(removeWorktreeMeta).toHaveBeenCalledWith(TEST_WORKTREE_ID, 'runtime:env-b') + expect(internals.mobileSessionTabsByWorktree.get(TEST_WORKTREE_ID)).toBe(localSession) + }) + it('claims the first window as authoritative and ignores later windows', () => { const runtime = createRuntime() diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 53f5a032e70..0dc5337375c 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -24629,21 +24629,28 @@ export class OrcaRuntimeService { } } - private removeWorktreeMetadataAndHistory(store: RuntimeStore, worktreeId: string): void { + private removeWorktreeMetadataAndHistory( + store: RuntimeStore, + worktreeId: string, + hostId?: ExecutionHostId + ): void { // Why: worktree IDs are path-derived and can be recreated, so removal must // purge history and process-local caches before the ID points at new state. - const hostId = store.getWorktreeMeta(worktreeId)?.hostId + const persistedHostId = store.getWorktreeMeta(worktreeId)?.hostId + const preservesSameIdOwner = Boolean(hostId && persistedHostId && persistedHostId !== hostId) if (hostId) { store.removeWorktreeMeta(worktreeId, hostId) } else { store.removeWorktreeMeta(worktreeId) } - this.mobileSessionTabsByWorktree.delete(worktreeId) - this.mobileSessionTabsAgentStatusHeartbeat.removeWorktree(worktreeId) - this.acceptedRendererMobileSnapshotByWorktree.delete(worktreeId) - advertisedUrlWatcher.forgetWorktree(worktreeId) - deleteWorktreeHistoryDir(worktreeId) - this.closeHeadlessBrowserPagesForWorktree(worktreeId) + if (!preservesSameIdOwner) { + this.mobileSessionTabsByWorktree.delete(worktreeId) + this.mobileSessionTabsAgentStatusHeartbeat.removeWorktree(worktreeId) + this.acceptedRendererMobileSnapshotByWorktree.delete(worktreeId) + advertisedUrlWatcher.forgetWorktree(worktreeId) + deleteWorktreeHistoryDir(worktreeId) + this.closeHeadlessBrowserPagesForWorktree(worktreeId) + } } // Why: headless offscreen browser pages are main-process BrowserWindows that @@ -24887,7 +24894,7 @@ export class OrcaRuntimeService { .catch(() => {}) } this.clearOptimisticReconcileToken(removalTarget.id) - this.removeWorktreeMetadataAndHistory(store, removalTarget.id) + this.removeWorktreeMetadataAndHistory(store, removalTarget.id, cleanupHostId) this.preservedBranchCleanupByScope.delete(cleanupScopeKey) this.invalidateResolvedWorktreeCache() this.invalidateWorktreeScanCacheForRepo(removalTarget.repoId) @@ -24930,7 +24937,7 @@ export class OrcaRuntimeService { console.warn(`[worktree-teardown] failed for ${removalTarget.id}:`, err) }) } - this.removeWorktreeMetadataAndHistory(store, removalTarget.id) + this.removeWorktreeMetadataAndHistory(store, removalTarget.id, cleanupHostId) this.preservedBranchCleanupByScope.delete(cleanupScopeKey) this.invalidateResolvedWorktreeCache() this.notifyWorktreesChanged(repo.id) @@ -25035,7 +25042,7 @@ export class OrcaRuntimeService { ) } this.clearOptimisticReconcileToken(removalTarget.id) - this.removeWorktreeMetadataAndHistory(store, removalTarget.id) + this.removeWorktreeMetadataAndHistory(store, removalTarget.id, cleanupHostId) this.preservedBranchCleanupByScope.delete(cleanupScopeKey) this.invalidateResolvedWorktreeCache() this.invalidateWorktreeScanCacheForRepo(removalTarget.repoId) @@ -25084,7 +25091,7 @@ export class OrcaRuntimeService { localWorktreeGitOptions ) this.clearOptimisticReconcileToken(removalTarget.id) - this.removeWorktreeMetadataAndHistory(store, removalTarget.id) + this.removeWorktreeMetadataAndHistory(store, removalTarget.id, cleanupHostId) this.preservedBranchCleanupByScope.delete(cleanupScopeKey) this.invalidateResolvedWorktreeCache() this.invalidateWorktreeScanCacheForRepo(removalTarget.repoId) @@ -25120,7 +25127,7 @@ export class OrcaRuntimeService { localWorktreeGitOptions )) this.clearOptimisticReconcileToken(removalTarget.id) - this.removeWorktreeMetadataAndHistory(store, removalTarget.id) + this.removeWorktreeMetadataAndHistory(store, removalTarget.id, cleanupHostId) this.preservedBranchCleanupByScope.delete(cleanupScopeKey) this.invalidateResolvedWorktreeCache() this.invalidateWorktreeScanCacheForRepo(removalTarget.repoId) @@ -25174,7 +25181,7 @@ export class OrcaRuntimeService { removedPushTarget ) this.clearOptimisticReconcileToken(removalTarget.id) - this.removeWorktreeMetadataAndHistory(store, removalTarget.id) + this.removeWorktreeMetadataAndHistory(store, removalTarget.id, cleanupHostId) this.invalidateResolvedWorktreeCache() this.invalidateWorktreeScanCacheForRepo(removalTarget.repoId) invalidateAuthorizedRootsCache() @@ -25220,7 +25227,7 @@ export class OrcaRuntimeService { removedPushTarget ) this.clearOptimisticReconcileToken(removalTarget.id) - this.removeWorktreeMetadataAndHistory(store, removalTarget.id) + this.removeWorktreeMetadataAndHistory(store, removalTarget.id, cleanupHostId) this.invalidateResolvedWorktreeCache() this.invalidateWorktreeScanCacheForRepo(removalTarget.repoId) invalidateAuthorizedRootsCache() @@ -25376,7 +25383,7 @@ export class OrcaRuntimeService { localWorktreeGitOptions ) this.clearOptimisticReconcileToken(removalTarget.id) - this.removeWorktreeMetadataAndHistory(store, removalTarget.id) + this.removeWorktreeMetadataAndHistory(store, removalTarget.id, cleanupHostId) this.preservedBranchCleanupByScope.delete(cleanupScopeKey) this.invalidateResolvedWorktreeCache() this.invalidateWorktreeScanCacheForRepo(removalTarget.repoId) @@ -25410,7 +25417,7 @@ export class OrcaRuntimeService { removedPushTarget ) this.clearOptimisticReconcileToken(removalTarget.id) - this.removeWorktreeMetadataAndHistory(store, removalTarget.id) + this.removeWorktreeMetadataAndHistory(store, removalTarget.id, cleanupHostId) this.invalidateResolvedWorktreeCache() this.invalidateWorktreeScanCacheForRepo(removalTarget.repoId) invalidateAuthorizedRootsCache() diff --git a/src/main/worktree-removal-repo-owner.test.ts b/src/main/worktree-removal-repo-owner.test.ts index 10afc2db6da..00aac41f5c6 100644 --- a/src/main/worktree-removal-repo-owner.test.ts +++ b/src/main/worktree-removal-repo-owner.test.ts @@ -1,8 +1,7 @@ /** - * STA-4343 (host side): a client that cannot send a host qualifier must not be - * able to trigger a wrong-host delete. A repo id registered on two hosts has no - * single owner, so an unqualified removal resolves to `ambiguous` and the - * caller refuses; a qualified one picks the named host's repo. + * STA-4343 (host side): a repo id registered on two hosts has no single owner, + * so unqualified legacy resolution is ambiguous and a qualified call picks the + * named host. The destructive RPC boundary rejects missing qualifiers earlier. */ import { describe, expect, it } from 'vitest' import { resolveWorktreeRemovalRepoOwner } from './worktree-removal-repo-owner' @@ -47,13 +46,6 @@ describe('resolveWorktreeRemovalRepoOwner', () => { expect(owner).toEqual({ kind: 'resolved', repo: sshRepo }) }) - it('keeps working unqualified while the id has one owner', () => { - expect(resolveWorktreeRemovalRepoOwner(makeStore([localRepo]), 'repo1')).toEqual({ - kind: 'resolved', - repo: localRepo - }) - }) - it('reports missing when the named host owns no such repo', () => { expect(resolveWorktreeRemovalRepoOwner(makeStore([localRepo]), 'repo1', 'ssh:ssh-1')).toEqual({ kind: 'missing' diff --git a/src/main/worktree-removal-repo-owner.ts b/src/main/worktree-removal-repo-owner.ts index 1aae804d0e4..f3755c897c4 100644 --- a/src/main/worktree-removal-repo-owner.ts +++ b/src/main/worktree-removal-repo-owner.ts @@ -14,10 +14,9 @@ export type WorktreeRemovalRepoOwner = /** * Which repo a destructive worktree removal belongs to. * - * A repo id can be registered once per execution host, so an unqualified call — - * an older client that cannot send `hostId` — is only safe while exactly one - * host owns it. Two owners resolve to `ambiguous` so the caller fails closed - * instead of deleting a same-id workspace on the wrong host (STA-4343). + * A repo id can be registered once per execution host. Destructive RPC callers + * reject a missing host before reaching this resolver; the unqualified branch + * remains only for direct legacy callers and still refuses multiple owners. */ export function resolveWorktreeRemovalRepoOwner( store: WorktreeRemovalRepoSource, diff --git a/src/renderer/src/components/workspace-cleanup/WorkspaceCleanupDialog.tsx b/src/renderer/src/components/workspace-cleanup/WorkspaceCleanupDialog.tsx index 607b39a829b..0fa30580ea6 100644 --- a/src/renderer/src/components/workspace-cleanup/WorkspaceCleanupDialog.tsx +++ b/src/renderer/src/components/workspace-cleanup/WorkspaceCleanupDialog.tsx @@ -11,14 +11,12 @@ import { canQueueWorkspaceCleanupCandidate, type WorkspaceCleanupCandidate } from '../../../../shared/workspace-cleanup' -import { - getWorkspaceCleanupCandidateIdentity, - getWorkspaceCleanupIdentityWorktreeId -} from '../../../../shared/workspace-cleanup-host-identity' +import { getWorkspaceCleanupCandidateIdentity } from '../../../../shared/workspace-cleanup-host-identity' import type { WorkspaceCleanupDeletionPhase } from './workspace-cleanup-candidate-row' import { WorkspaceCleanupBrowseToolbar } from './workspace-cleanup-browse-toolbar' import { WorkspaceCleanupConfirmRemove } from './workspace-cleanup-confirm-remove' import { WorkspaceCleanupDialogHeader } from './workspace-cleanup-dialog-header' +import { getWorkspaceCleanupDeletionPhaseByIdentity } from './workspace-cleanup-deletion-phases' import { WorkspaceCleanupInitialScanBanner, WorkspaceCleanupNotice, @@ -72,7 +70,7 @@ function WorkspaceCleanupDialogContent({ const refreshWorkspaceSpace = useAppStore((s) => s.refreshWorkspaceSpace) const workspaceSpaceScanning = useAppStore((s) => s.workspaceSpaceScanning) const workspaceSpaceProgress = useAppStore((s) => s.workspaceSpaceScanProgress) - const deletionPhaseByWorktreeId = useAppStore( + const genericDeletionPhaseByWorktreeId = useAppStore( useShallow((s) => { const phases: Record = {} for (const [worktreeId, state] of Object.entries(s.deleteStateByWorktreeId)) { @@ -83,11 +81,6 @@ function WorkspaceCleanupDialogContent({ return phases }) ) - const deletingWorktreeIds = useMemo( - () => new Set(Object.keys(deletionPhaseByWorktreeId)), - [deletionPhaseByWorktreeId] - ) - const browse = useWorkspaceCleanupBrowseState() // Why: facet counts/options are only rendered inside the filter popover; // tracking its open state lets their O(N) passes skip while it is closed. @@ -117,6 +110,19 @@ function WorkspaceCleanupDialogContent({ }, [open]) const { removalInFlightRef } = removal + const deletionPhaseByIdentity = useMemo( + () => + getWorkspaceCleanupDeletionPhaseByIdentity( + scan?.candidates ?? [], + removal.deletionPhaseByIdentity, + genericDeletionPhaseByWorktreeId + ), + [genericDeletionPhaseByWorktreeId, removal.deletionPhaseByIdentity, scan?.candidates] + ) + const deletingIdentities = useMemo( + () => new Set(Object.keys(deletionPhaseByIdentity)), + [deletionPhaseByIdentity] + ) const candidates = useMemo(() => scan?.candidates ?? [], [scan?.candidates]) const gitEvidenceNeeded = open && needsWorkspaceCleanupGitEvidence(browse.filters, browse.sort) @@ -185,9 +191,9 @@ function WorkspaceCleanupDialogContent({ (candidate): candidate is WorkspaceCleanupCandidate => candidate != null && canQueueWorkspaceCleanupCandidate(candidate) && - !deletingWorktreeIds.has(candidate.worktreeId) + !deletingIdentities.has(getWorkspaceCleanupCandidateIdentity(candidate)) ) - }, [deletingWorktreeIds, rows, selectedIds]) + }, [deletingIdentities, rows, selectedIds]) const selectedCount = selectedCandidates.length useEffect(() => { @@ -203,17 +209,16 @@ function WorkspaceCleanupDialogContent({ } defaultsAppliedForOpenRef.current = true setSelectedIds( - getDefaultSelectedWorkspaceCleanupIdentities(scan.candidates, deletingWorktreeIds) + getDefaultSelectedWorkspaceCleanupIdentities(scan.candidates, deletingIdentities) ) - }, [deletingWorktreeIds, loading, removalInFlightRef, scan, setSelectedIds]) + }, [deletingIdentities, loading, removalInFlightRef, scan, setSelectedIds]) const pruneSelectionToVisibleRows = useEffectEvent(() => { setSelectedIds((current) => { const next = new Set( [...current].filter( (identity) => - facetRows.facetMatchedIdentities.has(identity) && - !deletingWorktreeIds.has(getWorkspaceCleanupIdentityWorktreeId(identity)) + facetRows.facetMatchedIdentities.has(identity) && !deletingIdentities.has(identity) ) ) return next.size === current.size ? current : next @@ -232,8 +237,7 @@ function WorkspaceCleanupDialogContent({ }, [browse.filters, open, removal.confirming]) const pruneVanishedSelections = useEffectEvent(() => { - const isDeleting = (identity: string): boolean => - deletingWorktreeIds.has(getWorkspaceCleanupIdentityWorktreeId(identity)) + const isDeleting = (identity: string): boolean => deletingIdentities.has(identity) const kept = [...selectedIds].filter( (identity) => candidateIdentities.has(identity) && !isDeleting(identity) ) @@ -256,7 +260,7 @@ function WorkspaceCleanupDialogContent({ // Why: a settled refresh reconciles by host-qualified identity — selections survive it, // and only rows that truly no longer exist drop (with a visible reason). pruneVanishedSelections() - }, [candidateIdentities, deletingWorktreeIds, loading]) + }, [candidateIdentities, deletingIdentities, loading]) const ignoreCandidate = useCallback( (candidate: WorkspaceCleanupCandidate) => { @@ -301,10 +305,8 @@ function WorkspaceCleanupDialogContent({ () => removal.removalInFlight ? [] - : facetRows.selectableIdentities.filter( - (identity) => !deletingWorktreeIds.has(getWorkspaceCleanupIdentityWorktreeId(identity)) - ), - [deletingWorktreeIds, facetRows.selectableIdentities, removal.removalInFlight] + : facetRows.selectableIdentities.filter((identity) => !deletingIdentities.has(identity)), + [deletingIdentities, facetRows.selectableIdentities, removal.removalInFlight] ) const toggleSelectAll = useCallback( (selectAll: boolean) => { @@ -406,13 +408,12 @@ function WorkspaceCleanupDialogContent({ scannedCount={candidates.length} hasScanned={scan != null} loading={loading} - deletionPhaseByWorktreeId={deletionPhaseByWorktreeId} - deletingWorktreeIds={deletingWorktreeIds} + deletionPhaseByIdentity={deletionPhaseByIdentity} + deletingIdentities={deletingIdentities} expandedRowIds={expandedRowIds} selectedIds={selectedIds} gitPendingWorktreeIds={gitEvidence.pendingWorktreeIds} rowFailures={removal.rowFailures} - removalInFlight={removal.removalInFlight} scrollElement={rowsScrollElement} onClearFilters={browse.clearFilters} onToggleExpanded={toggleExpandedRow} diff --git a/src/renderer/src/components/workspace-cleanup/use-workspace-cleanup-removal.ts b/src/renderer/src/components/workspace-cleanup/use-workspace-cleanup-removal.ts index 3993cc6fa58..31373f519a0 100644 --- a/src/renderer/src/components/workspace-cleanup/use-workspace-cleanup-removal.ts +++ b/src/renderer/src/components/workspace-cleanup/use-workspace-cleanup-removal.ts @@ -18,6 +18,7 @@ export type WorkspaceCleanupRemovalController = { confirmCandidates: WorkspaceCleanupCandidate[] removalProgress: WorkspaceCleanupRemovalProgress | null removalInFlight: boolean + deletionPhaseByIdentity: Record /** Synchronous guard; state alone lags a second confirm click. */ removalInFlightRef: { current: boolean } /** Keyed by host-qualified identity so a failure marks only the confirmed host's row. */ @@ -54,6 +55,9 @@ export function useWorkspaceCleanupRemoval({ // Why: `removalProgress` only arrives once the batch reports, so rendering // needs its own in-flight flag; removalInFlightRef stays the synchronous guard. const [removalInFlight, setRemovalInFlight] = useState(false) + const [deletionPhaseByIdentity, setDeletionPhaseByIdentity] = useState< + Record + >({}) const [rowFailures, setRowFailures] = useState>({}) const removalInFlightRef = useRef(false) // Why: the dialog stays mounted across cleanup runs, so late settlements from @@ -118,6 +122,7 @@ export function useWorkspaceCleanupRemoval({ setRemovalInFlight(false) setConfirming(false) setConfirmCandidates([]) + setDeletionPhaseByIdentity({}) }, []) const confirmRemove = useCallback(() => { @@ -140,7 +145,11 @@ export function useWorkspaceCleanupRemoval({ // Why: a hung late settlement retains these callbacks for the renderer's // lifetime; capture only ids so it cannot pin the candidate objects. const removableWorktreeIds = removableCandidates.map((candidate) => candidate.worktreeId) + const removableIdentities = removableCandidates.map(getWorkspaceCleanupCandidateIdentity) setRowFailures({}) + setDeletionPhaseByIdentity( + Object.fromEntries(removableIdentities.map((identity) => [identity, 'queued' as const])) + ) markWorktreesQueuedForDeletion(removableWorktreeIds) const handleRemovalError = (): void => { for (const worktreeId of removableWorktreeIds) { @@ -175,10 +184,22 @@ export function useWorkspaceCleanupRemoval({ onProgress: (progress) => { if (mountedRef.current) { setRemovalProgress(progress) + setDeletionPhaseByIdentity((current) => + Object.fromEntries(Object.keys(current).map((identity) => [identity, 'deleting'])) + ) } }, onRowFailed: (failure) => { clearQueuedDeleteState(failure.worktreeId) + if (!mountedRef.current) { + return + } + const identity = getWorkspaceCleanupFailureIdentity(failure) + setDeletionPhaseByIdentity((current) => { + const next = { ...current } + delete next[identity] + return next + }) }, onResult: (result) => { const nextFailures: Record = {} @@ -236,6 +257,7 @@ export function useWorkspaceCleanupRemoval({ confirmCandidates, removalProgress, removalInFlight, + deletionPhaseByIdentity, removalInFlightRef, rowFailures, resetRowFailures, diff --git a/src/renderer/src/components/workspace-cleanup/workspace-cleanup-deletion-phases.test.ts b/src/renderer/src/components/workspace-cleanup/workspace-cleanup-deletion-phases.test.ts new file mode 100644 index 00000000000..4b5f373e5f2 --- /dev/null +++ b/src/renderer/src/components/workspace-cleanup/workspace-cleanup-deletion-phases.test.ts @@ -0,0 +1,45 @@ +import { describe, expect, it } from 'vitest' +import { getWorkspaceCleanupCandidateIdentity } from '../../../../shared/workspace-cleanup-host-identity' +import { makeCandidate } from './workspace-cleanup-presentation-fixtures' +import { getWorkspaceCleanupDeletionPhaseByIdentity } from './workspace-cleanup-deletion-phases' + +describe('workspace cleanup deletion phases', () => { + it('marks only the qualified row during a same-id cleanup removal', () => { + const local = makeCandidate({ executionHostId: 'local' }) + const remote = makeCandidate({ + worktreeId: local.worktreeId, + executionHostId: 'ssh:ssh-1', + connectionId: 'ssh-1' + }) + const localIdentity = getWorkspaceCleanupCandidateIdentity(local) + const remoteIdentity = getWorkspaceCleanupCandidateIdentity(remote) + + expect( + getWorkspaceCleanupDeletionPhaseByIdentity( + [local, remote], + { [remoteIdentity]: 'deleting' }, + { [local.worktreeId]: 'deleting' } + ) + ).toEqual({ [remoteIdentity]: 'deleting' }) + expect(localIdentity).not.toBe(remoteIdentity) + }) + + it('applies a non-cleanup deletion to every ambiguous same-id row', () => { + const local = makeCandidate({ executionHostId: 'local' }) + const remote = makeCandidate({ + worktreeId: local.worktreeId, + executionHostId: 'runtime:env-1' + }) + + expect( + getWorkspaceCleanupDeletionPhaseByIdentity( + [local, remote], + {}, + { [local.worktreeId]: 'queued' } + ) + ).toEqual({ + [getWorkspaceCleanupCandidateIdentity(local)]: 'queued', + [getWorkspaceCleanupCandidateIdentity(remote)]: 'queued' + }) + }) +}) diff --git a/src/renderer/src/components/workspace-cleanup/workspace-cleanup-deletion-phases.ts b/src/renderer/src/components/workspace-cleanup/workspace-cleanup-deletion-phases.ts new file mode 100644 index 00000000000..bdae4154028 --- /dev/null +++ b/src/renderer/src/components/workspace-cleanup/workspace-cleanup-deletion-phases.ts @@ -0,0 +1,27 @@ +import type { WorkspaceCleanupCandidate } from '../../../../shared/workspace-cleanup' +import { + getWorkspaceCleanupCandidateIdentity, + getWorkspaceCleanupIdentityWorktreeId +} from '../../../../shared/workspace-cleanup-host-identity' +import type { WorkspaceCleanupDeletionPhase } from './workspace-cleanup-candidate-row' + +export function getWorkspaceCleanupDeletionPhaseByIdentity( + candidates: readonly WorkspaceCleanupCandidate[], + cleanupPhases: Readonly>, + genericPhasesByWorktreeId: Readonly> +): Record { + const cleanupWorktreeIds = new Set( + Object.keys(cleanupPhases).map(getWorkspaceCleanupIdentityWorktreeId) + ) + const phases = { ...cleanupPhases } + for (const candidate of candidates) { + if (cleanupWorktreeIds.has(candidate.worktreeId)) { + continue + } + const phase = genericPhasesByWorktreeId[candidate.worktreeId] + if (phase) { + phases[getWorkspaceCleanupCandidateIdentity(candidate)] = phase + } + } + return phases +} diff --git a/src/renderer/src/components/workspace-cleanup/workspace-cleanup-row-list.tsx b/src/renderer/src/components/workspace-cleanup/workspace-cleanup-row-list.tsx index d348b21581d..019260dd384 100644 --- a/src/renderer/src/components/workspace-cleanup/workspace-cleanup-row-list.tsx +++ b/src/renderer/src/components/workspace-cleanup/workspace-cleanup-row-list.tsx @@ -16,15 +16,14 @@ export type WorkspaceCleanupRowListState = { scannedCount: number hasScanned: boolean loading: boolean - deletionPhaseByWorktreeId: Record - deletingWorktreeIds: ReadonlySet + deletionPhaseByIdentity: Record + deletingIdentities: ReadonlySet /** Host-qualified identities (see WorkspaceCleanupFacets.identity). */ expandedRowIds: ReadonlySet selectedIds: ReadonlySet gitPendingWorktreeIds: ReadonlySet /** Keyed by host-qualified identity so a failure marks only its own host's row. */ rowFailures: Record - removalInFlight: boolean scrollElement: HTMLDivElement | null onClearFilters: () => void onToggleExpanded: (identity: string) => void @@ -79,12 +78,12 @@ export function WorkspaceCleanupRowList(props: WorkspaceCleanupRowListState): Re sizeLabel={row.sizeBytes === null ? null : formatBytes(row.sizeBytes)} workspaceStatusLabel={row.workspaceStatusLabel} gitEvidencePending={props.gitPendingWorktreeIds.has(row.worktreeId)} - deletionPhase={props.deletionPhaseByWorktreeId[row.worktreeId]} + deletionPhase={props.deletionPhaseByIdentity[row.identity]} // Why: a background rescan must not lock rows; only an actual // removal batch or this row's own deletion disables it. - removing={props.removalInFlight || props.deletingWorktreeIds.has(row.worktreeId)} + removing={props.deletingIdentities.has(row.identity)} selected={ - props.selectedIds.has(row.identity) && !props.deletingWorktreeIds.has(row.worktreeId) + props.selectedIds.has(row.identity) && !props.deletingIdentities.has(row.identity) } failure={props.rowFailures[row.identity]} onToggleExpanded={props.onToggleExpanded} diff --git a/src/renderer/src/components/workspace-cleanup/workspace-cleanup-selection-model.ts b/src/renderer/src/components/workspace-cleanup/workspace-cleanup-selection-model.ts index c874943375c..d9dc94bb8df 100644 --- a/src/renderer/src/components/workspace-cleanup/workspace-cleanup-selection-model.ts +++ b/src/renderer/src/components/workspace-cleanup/workspace-cleanup-selection-model.ts @@ -18,12 +18,14 @@ export function formatVanishedSelectionNotice(count: number): string { /** Host-qualified identities, so a default selection cannot span two hosts' rows. */ export function getDefaultSelectedWorkspaceCleanupIdentities( candidates: readonly WorkspaceCleanupCandidate[], - deletingWorktreeIds: ReadonlySet = new Set() + deletingIdentities: ReadonlySet = new Set() ): Set { return new Set( candidates .filter( - (candidate) => candidate.selectedByDefault && !deletingWorktreeIds.has(candidate.worktreeId) + (candidate) => + candidate.selectedByDefault && + !deletingIdentities.has(getWorkspaceCleanupCandidateIdentity(candidate)) ) .map((candidate) => getWorkspaceCleanupCandidateIdentity(candidate)) )