mirror of
https://github.com/stablyai/orca.git
synced 2026-10-22 00:03:47 +00:00
fix: preserve colliding cleanup owner state
This commit is contained in:
@@ -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 () => {
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
+16
-7
@@ -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<ExecutionHostId>(
|
||||
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)
|
||||
|
||||
@@ -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<string, unknown>
|
||||
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()
|
||||
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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'
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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<string, WorkspaceCleanupDeletionPhase> = {}
|
||||
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}
|
||||
|
||||
@@ -18,6 +18,7 @@ export type WorkspaceCleanupRemovalController = {
|
||||
confirmCandidates: WorkspaceCleanupCandidate[]
|
||||
removalProgress: WorkspaceCleanupRemovalProgress | null
|
||||
removalInFlight: boolean
|
||||
deletionPhaseByIdentity: Record<string, 'queued' | 'deleting'>
|
||||
/** 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<string, 'queued' | 'deleting'>
|
||||
>({})
|
||||
const [rowFailures, setRowFailures] = useState<Record<string, string>>({})
|
||||
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<string, string> = {}
|
||||
@@ -236,6 +257,7 @@ export function useWorkspaceCleanupRemoval({
|
||||
confirmCandidates,
|
||||
removalProgress,
|
||||
removalInFlight,
|
||||
deletionPhaseByIdentity,
|
||||
removalInFlightRef,
|
||||
rowFailures,
|
||||
resetRowFailures,
|
||||
|
||||
+45
@@ -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'
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -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<Record<string, WorkspaceCleanupDeletionPhase>>,
|
||||
genericPhasesByWorktreeId: Readonly<Record<string, WorkspaceCleanupDeletionPhase>>
|
||||
): Record<string, WorkspaceCleanupDeletionPhase> {
|
||||
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
|
||||
}
|
||||
@@ -16,15 +16,14 @@ export type WorkspaceCleanupRowListState = {
|
||||
scannedCount: number
|
||||
hasScanned: boolean
|
||||
loading: boolean
|
||||
deletionPhaseByWorktreeId: Record<string, WorkspaceCleanupDeletionPhase>
|
||||
deletingWorktreeIds: ReadonlySet<string>
|
||||
deletionPhaseByIdentity: Record<string, WorkspaceCleanupDeletionPhase>
|
||||
deletingIdentities: ReadonlySet<string>
|
||||
/** Host-qualified identities (see WorkspaceCleanupFacets.identity). */
|
||||
expandedRowIds: ReadonlySet<string>
|
||||
selectedIds: ReadonlySet<string>
|
||||
gitPendingWorktreeIds: ReadonlySet<string>
|
||||
/** Keyed by host-qualified identity so a failure marks only its own host's row. */
|
||||
rowFailures: Record<string, string>
|
||||
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}
|
||||
|
||||
@@ -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<string> = new Set()
|
||||
deletingIdentities: ReadonlySet<string> = new Set()
|
||||
): Set<string> {
|
||||
return new Set(
|
||||
candidates
|
||||
.filter(
|
||||
(candidate) => candidate.selectedByDefault && !deletingWorktreeIds.has(candidate.worktreeId)
|
||||
(candidate) =>
|
||||
candidate.selectedByDefault &&
|
||||
!deletingIdentities.has(getWorkspaceCleanupCandidateIdentity(candidate))
|
||||
)
|
||||
.map((candidate) => getWorkspaceCleanupCandidateIdentity(candidate))
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user