fix(workspaces): reconcile cleanup dialog state

This commit is contained in:
Brennan Benson
2026-08-10 17:38:10 -07:00
parent e16ec251ab
commit 64b2bfb674
18 changed files with 278 additions and 37 deletions
@@ -112,7 +112,11 @@ export default function WorkspaceCleanupDialog(): React.JSX.Element {
const candidates = useMemo(() => scan?.candidates ?? [], [scan?.candidates])
const gitEvidenceNeeded = open && needsWorkspaceCleanupGitEvidence(browse.filters, browse.sort)
const gitEvidence = useWorkspaceCleanupGitEvidence({ enabled: gitEvidenceNeeded, candidates })
const gitEvidence = useWorkspaceCleanupGitEvidence({
enabled: gitEvidenceNeeded,
candidates,
scannedAt: scan?.scannedAt ?? null
})
const evidencedCandidates = useMemo(
() => applyWorkspaceCleanupGitEvidence(candidates, gitEvidence.evidenceByWorktreeId),
[candidates, gitEvidence.evidenceByWorktreeId]
@@ -253,17 +257,19 @@ export default function WorkspaceCleanupDialog(): React.JSX.Element {
setSelectedIds((current) => toggleSetMember(current, worktreeId))
}, [])
const selectableWorktreeIds = facetRows.selectableWorktreeIds
const selectableWorktreeIds = useMemo(
() =>
removal.removalInFlight
? []
: facetRows.selectableWorktreeIds.filter((id) => !deletingWorktreeIds.has(id)),
[deletingWorktreeIds, facetRows.selectableWorktreeIds, removal.removalInFlight]
)
const toggleSelectAll = useCallback(
(selectAll: boolean) => {
selectionTouchedRef.current = true
setSelectedIds(
selectAll
? new Set(selectableWorktreeIds.filter((id) => !deletingWorktreeIds.has(id)))
: new Set()
)
setSelectedIds(selectAll ? new Set(selectableWorktreeIds) : new Set())
},
[deletingWorktreeIds, selectableWorktreeIds]
[selectableWorktreeIds]
)
// Why: stable per-row handlers so React.memo keeps unchanged CandidateRow
@@ -334,6 +340,7 @@ export default function WorkspaceCleanupDialog(): React.JSX.Element {
<WorkspaceCleanupBrowseToolbar
browse={browse}
facetRows={facetRows}
selectableCount={selectableWorktreeIds.length}
selectedCount={selectedCount}
spaceScanning={workspaceSpaceScanning}
gitPendingCount={gitEvidence.pendingWorktreeIds.size}
@@ -13,6 +13,7 @@ import {
buildWorkspaceCleanupFacetList,
buildWorkspaceCleanupSizeIndex,
buildWorkspaceCleanupWorktreeIndex,
countWorkspaceCleanupMeasuredRows,
type WorkspaceCleanupFacets
} from './workspace-cleanup-facets'
import {
@@ -143,6 +144,6 @@ export function useWorkspaceCleanupFacetRows({
options,
reviewInfoByWorktreeId,
sizeByWorktreeId,
measuredSizeCount: sizeByWorktreeId.size
measuredSizeCount: countWorkspaceCleanupMeasuredRows(facets)
}
}
@@ -0,0 +1,111 @@
// @vitest-environment happy-dom
import { act, renderHook, waitFor } from '@testing-library/react'
import { beforeEach, describe, expect, it, vi } from 'vitest'
import type { WorkspaceCleanupScanResult } from '../../../../shared/workspace-cleanup'
import { makeFacetCandidate } from './workspace-cleanup-facet.test.fixture'
import { useWorkspaceCleanupGitEvidence } from './use-workspace-cleanup-git-evidence'
const holders = vi.hoisted(() => ({ scan: vi.fn() }))
vi.mock('@/store', () => ({
useAppStore: (selector: (state: { scanWorkspaceCleanup: typeof holders.scan }) => unknown) =>
selector({ scanWorkspaceCleanup: holders.scan })
}))
type PendingScan = {
worktreeId: string
resolve: (result: WorkspaceCleanupScanResult) => void
}
function deferredCandidate(worktreeId: string) {
return makeFacetCandidate({
worktreeId,
git: { clean: null, upstreamAhead: null, upstreamBehind: null, checkedAt: null }
})
}
describe('useWorkspaceCleanupGitEvidence', () => {
let pending: PendingScan[]
beforeEach(() => {
pending = []
holders.scan.mockReset()
holders.scan.mockImplementation(({ worktreeId }: { worktreeId: string }) => {
return new Promise<WorkspaceCleanupScanResult>((resolve) => {
pending.push({ worktreeId, resolve })
})
})
})
it('counts queued rows and isolates a restarted pass from stale settlements', async () => {
const candidates = ['a', 'b', 'c', 'd', 'e', 'f'].map(deferredCandidate)
const view = renderHook(
({ enabled }: { enabled: boolean }) =>
useWorkspaceCleanupGitEvidence({ enabled, candidates, scannedAt: 1 }),
{ initialProps: { enabled: true } }
)
await waitFor(() => expect(view.result.current.totalCount).toBe(6))
expect(view.result.current.pendingWorktreeIds.size).toBe(6)
expect(holders.scan).toHaveBeenCalledTimes(4)
view.rerender({ enabled: false })
await waitFor(() => expect(view.result.current.pendingWorktreeIds.size).toBe(0))
view.rerender({ enabled: true })
await waitFor(() => expect(holders.scan).toHaveBeenCalledTimes(8))
expect(view.result.current.pendingWorktreeIds.size).toBe(6)
await act(async () => {
pending[0]?.resolve({
scannedAt: 2,
candidates: [makeFacetCandidate({ worktreeId: pending[0].worktreeId })],
errors: []
})
})
expect(view.result.current.pendingWorktreeIds.size).toBe(6)
expect(view.result.current.evidenceByWorktreeId.size).toBe(0)
await act(async () => {
pending[4]?.resolve({
scannedAt: 3,
candidates: [makeFacetCandidate({ worktreeId: pending[4].worktreeId })],
errors: []
})
})
await waitFor(() => expect(view.result.current.evidenceByWorktreeId.size).toBe(1))
expect(view.result.current.pendingWorktreeIds.size).toBe(5)
})
it('restarts evidence collection when the settled scan snapshot changes', async () => {
const candidates = [deferredCandidate('a')]
const view = renderHook(
({ scannedAt }: { scannedAt: number }) =>
useWorkspaceCleanupGitEvidence({ enabled: true, candidates, scannedAt }),
{ initialProps: { scannedAt: 1 } }
)
await waitFor(() => expect(holders.scan).toHaveBeenCalledTimes(1))
view.rerender({ scannedAt: 2 })
await waitFor(() => expect(holders.scan).toHaveBeenCalledTimes(2))
await act(async () => {
pending[0]?.resolve({
scannedAt: 1,
candidates: [makeFacetCandidate({ worktreeId: 'a' })],
errors: []
})
})
expect(view.result.current.evidenceByWorktreeId.size).toBe(0)
expect(view.result.current.pendingWorktreeIds).toEqual(new Set(['a']))
await act(async () => {
pending[1]?.resolve({
scannedAt: 2,
candidates: [makeFacetCandidate({ worktreeId: 'a' })],
errors: []
})
})
await waitFor(() => expect(view.result.current.evidenceByWorktreeId.size).toBe(1))
})
})
@@ -29,10 +29,12 @@ const EMPTY_EVIDENCE: WorkspaceCleanupGitEvidenceState = {
*/
export function useWorkspaceCleanupGitEvidence({
enabled,
candidates
candidates,
scannedAt
}: {
enabled: boolean
candidates: readonly WorkspaceCleanupCandidate[]
scannedAt: number | null
}): WorkspaceCleanupGitEvidenceState {
const scanWorkspaceCleanup = useAppStore((s) => s.scanWorkspaceCleanup)
const mountedRef = useMountedRef()
@@ -40,8 +42,11 @@ export function useWorkspaceCleanupGitEvidence({
const evidenceRef = useRef(new Map<string, WorkspaceCleanupCandidate>())
const attemptedRef = useRef(new Set<string>())
const queueRef = useRef<string[]>([])
const queuedRef = useRef(new Set<string>())
const inFlightRef = useRef(new Set<string>())
const totalRef = useRef(0)
const enabledRef = useRef(enabled)
const scannedAtRef = useRef(scannedAt)
// Why: results that land after the user cleared the git filter belong to a
// stale pass and must not resurrect its pending banner.
const generationRef = useRef(0)
@@ -52,8 +57,8 @@ export function useWorkspaceCleanupGitEvidence({
}
setState({
evidenceByWorktreeId: new Map(evidenceRef.current),
pendingWorktreeIds: new Set(inFlightRef.current),
checkedCount: totalRef.current - queueRef.current.length - inFlightRef.current.size,
pendingWorktreeIds: new Set([...queuedRef.current, ...inFlightRef.current]),
checkedCount: totalRef.current - queuedRef.current.size - inFlightRef.current.size,
totalCount: totalRef.current
})
}, [mountedRef])
@@ -68,13 +73,18 @@ export function useWorkspaceCleanupGitEvidence({
if (worktreeId === undefined) {
break
}
queuedRef.current.delete(worktreeId)
inFlightRef.current.add(worktreeId)
attemptedRef.current.add(worktreeId)
void scanWorkspaceCleanup({ worktreeId })
.then((result) => {
const refreshed = result.candidates.find(
(candidate) => candidate.worktreeId === worktreeId
)
let refreshed: WorkspaceCleanupCandidate | undefined
for (const candidate of result.candidates) {
if (candidate.worktreeId === worktreeId) {
refreshed = candidate
break
}
}
if (refreshed && generation === generationRef.current) {
evidenceRef.current.set(worktreeId, refreshed)
}
@@ -84,10 +94,10 @@ export function useWorkspaceCleanupGitEvidence({
// attemptedRef keeps it from retrying in a loop.
})
.finally(() => {
inFlightRef.current.delete(worktreeId)
if (generation !== generationRef.current) {
return
}
inFlightRef.current.delete(worktreeId)
publish()
pump()
})
@@ -96,24 +106,37 @@ export function useWorkspaceCleanupGitEvidence({
}, [publish, scanWorkspaceCleanup])
useEffect(() => {
if (!enabled) {
const wasEnabled = enabledRef.current
const snapshotChanged = scannedAtRef.current !== scannedAt
enabledRef.current = enabled
scannedAtRef.current = scannedAt
if ((wasEnabled && !enabled) || (enabled && snapshotChanged)) {
generationRef.current += 1
queueRef.current = []
queuedRef.current.clear()
inFlightRef.current.clear()
attemptedRef.current.clear()
evidenceRef.current.clear()
totalRef.current = 0
publish()
}
if (!enabled) {
return
}
const targets = selectWorkspaceCleanupGitEvidenceTargets(candidates, {
resolvedWorktreeIds: attemptedRef.current
}).filter((id) => !queueRef.current.includes(id))
}).filter((id) => !queuedRef.current.has(id) && !inFlightRef.current.has(id))
if (targets.length === 0) {
return
}
queueRef.current.push(...targets)
for (const target of targets) {
queuedRef.current.add(target)
}
totalRef.current += targets.length
pump()
}, [candidates, enabled, pump, publish])
}, [candidates, enabled, pump, publish, scannedAt])
return state
}
@@ -13,6 +13,7 @@ import { WorkspaceCleanupSortHeader } from './workspace-cleanup-sort-header'
export function WorkspaceCleanupBrowseToolbar({
browse,
facetRows,
selectableCount,
selectedCount,
spaceScanning,
gitPendingCount,
@@ -22,6 +23,7 @@ export function WorkspaceCleanupBrowseToolbar({
}: {
browse: WorkspaceCleanupBrowseController
facetRows: WorkspaceCleanupFacetRows
selectableCount: number
selectedCount: number
spaceScanning: boolean
gitPendingCount: number
@@ -54,7 +56,7 @@ export function WorkspaceCleanupBrowseToolbar({
/>
<WorkspaceCleanupSortHeader
sort={browse.sort}
selectableCount={facetRows.selectableWorktreeIds.length}
selectableCount={selectableCount}
selectedCount={selectedCount}
onToggleSortField={browse.toggleSortField}
onToggleSelectAll={onToggleSelectAll}
@@ -140,6 +140,15 @@ describe('workspace cleanup facet controls', () => {
expect(onPatch).toHaveBeenCalledWith('safety', { dismissed: 'only' })
})
it('labels blocker matching without exposing model tokens', () => {
renderFacets(createDefaultWorkspaceCleanupFilterState(), createPatchMock())
expect(control('Blocker match: Has any')).not.toBeNull()
expect(control('Blocker match: Has none')).not.toBeNull()
expect(container?.textContent).not.toContain('any-of')
expect(container?.textContent).not.toContain('none-of')
})
it('toggles the unsized escape hatch for the opt-in space scan', () => {
const onPatch = createPatchMock()
renderFacets(createDefaultWorkspaceCleanupFilterState(), onPatch)
@@ -3,6 +3,7 @@ import { Checkbox } from '@/components/ui/checkbox'
import { Input } from '@/components/ui/input'
import { Label } from '@/components/ui/label'
import { cn } from '@/lib/utils'
import { parseWorkspaceCleanupFacetNumber } from './workspace-cleanup-facet-panel-model'
/** One facet group inside the filter panel, with its own match count. */
export function FacetSection({
@@ -48,11 +49,12 @@ export function FacetToggleList<T extends string>({
getLabel: (value: T) => string
onChange: (next: T[]) => void
}): React.JSX.Element {
const selectedSet = new Set(selected)
return (
<FacetField label={label}>
<div className="flex flex-wrap gap-1">
{values.map((value) => {
const active = selected.includes(value)
const active = selectedSet.has(value)
return (
<button
key={value}
@@ -144,7 +146,7 @@ export function FacetNumberField({
aria-label={label}
value={value === null ? '' : String(value)}
placeholder={placeholder}
onChange={(event) => onChange(parseFacetNumber(event.target.value))}
onChange={(event) => onChange(parseWorkspaceCleanupFacetNumber(event.target.value))}
className="h-7 w-24 text-xs"
/>
{suffix ? <span className="text-[11px] text-muted-foreground">{suffix}</span> : null}
@@ -219,12 +221,3 @@ function FacetField({
</div>
)
}
export function parseFacetNumber(raw: string): number | null {
const trimmed = raw.trim()
if (trimmed === '') {
return null
}
const parsed = Number(trimmed)
return Number.isFinite(parsed) && parsed >= 0 ? parsed : null
}
@@ -2,7 +2,8 @@ import { describe, expect, it } from 'vitest'
import {
buildWorkspaceCleanupFacetList,
buildWorkspaceCleanupSizeIndex,
buildWorkspaceCleanupWorktreeIndex
buildWorkspaceCleanupWorktreeIndex,
countWorkspaceCleanupMeasuredRows
} from './workspace-cleanup-facets'
import {
DAY,
@@ -69,6 +70,11 @@ describe('facet building', () => {
expect(index.has('b')).toBe(false)
})
it('counts measurements only for rows in the current fleet', () => {
const rows = [makeFacets({ sizeBytes: 10 }), makeNamedFacets('unsized')]
expect(countWorkspaceCleanupMeasuredRows(rows)).toBe(1)
})
it('indexes worktrees across repos by id', () => {
const index = buildWorkspaceCleanupWorktreeIndex({
'repo-1': [{ id: 'w1' }],
@@ -3,6 +3,7 @@ import type { HostedReviewProvider } from '../../../../shared/hosted-review'
import type { WorkspaceCleanupTier } from '../../../../shared/workspace-cleanup'
import type {
WorkspaceCleanupAgentState,
WorkspaceCleanupBlockerMode,
WorkspaceCleanupGitState,
WorkspaceCleanupIdleSignal,
WorkspaceCleanupPresence,
@@ -166,3 +167,9 @@ export function getWorkspaceCleanupPresenceLabel(value: WorkspaceCleanupPresence
return translate('components.workspace.cleanup.browse.presence.none', 'Has none')
}
}
export function getWorkspaceCleanupBlockerModeLabel(value: WorkspaceCleanupBlockerMode): string {
return value === 'any-of'
? translate('components.workspace.cleanup.browse.blockerModeAny', 'Has any')
: translate('components.workspace.cleanup.browse.blockerModeNone', 'Has none')
}
@@ -35,3 +35,12 @@ export function toMegabytes(bytes: number | null): number | null {
export function fromMegabytes(megabytes: number | null): number | null {
return megabytes === null ? null : Math.round(megabytes * WORKSPACE_CLEANUP_MEGABYTE)
}
export function parseWorkspaceCleanupFacetNumber(raw: string): number | null {
const trimmed = raw.trim()
if (trimmed === '') {
return null
}
const parsed = Number(trimmed)
return Number.isFinite(parsed) && parsed >= 0 ? parsed : null
}
@@ -37,7 +37,7 @@ export function sortWorkspaceCleanupFacets(
sort: WorkspaceCleanupSortState
): WorkspaceCleanupFacets[] {
const multiplier = sort.direction === 'asc' ? 1 : -1
return [...facets].sort(
return facets.toSorted(
(left, right) =>
comparePrimary(left, right, sort.field, multiplier) || compareTieBreaks(left, right)
)
@@ -174,6 +174,10 @@ export function buildWorkspaceCleanupSizeIndex(
return index
}
export function countWorkspaceCleanupMeasuredRows(rows: readonly WorkspaceCleanupFacets[]): number {
return rows.reduce((count, row) => count + (row.sizeBytes === null ? 0 : 1), 0)
}
export function buildWorkspaceCleanupWorktreeIndex(
worktreesByRepo: Readonly<Record<string, readonly WorkspaceCleanupWorktreeFacts[]>>
): Map<string, WorkspaceCleanupWorktreeFacts> {
@@ -111,4 +111,38 @@ describe('applyWorkspaceCleanupGitEvidence', () => {
const candidates = [deferredCandidate('a')]
expect(applyWorkspaceCleanupGitEvidence(candidates, new Map())).toBe(candidates)
})
it('merges only git facts into the current broad-scan row', () => {
const current = deferredCandidate('a')
current.displayName = 'current name'
current.blockers = ['dismissed']
current.fingerprint = 'current-fingerprint'
const refreshed = makeFacetCandidate({
worktreeId: 'a',
displayName: 'stale name',
blockers: ['dirty-files'],
fingerprint: 'focused-fingerprint',
git: { clean: false, upstreamAhead: 0, upstreamBehind: 0, checkedAt: 5 }
})
const [row] = applyWorkspaceCleanupGitEvidence([current], new Map([['a', refreshed]]))
expect(row.displayName).toBe('current name')
expect(row.fingerprint).toBe('current-fingerprint')
expect(row.blockers).toEqual(expect.arrayContaining(['dismissed', 'dirty-files']))
expect(row.git).toEqual(refreshed.git)
})
it('keeps newer broad-scan git evidence', () => {
const current = makeFacetCandidate({
worktreeId: 'a',
git: { clean: true, upstreamAhead: 0, upstreamBehind: 0, checkedAt: 10 }
})
const older = makeFacetCandidate({
worktreeId: 'a',
git: { clean: false, upstreamAhead: 2, upstreamBehind: 0, checkedAt: 5 }
})
expect(applyWorkspaceCleanupGitEvidence([current], new Map([['a', older]]))[0]).toBe(current)
})
})
@@ -2,6 +2,7 @@ import type {
WorkspaceCleanupBlocker,
WorkspaceCleanupCandidate
} from '../../../../shared/workspace-cleanup'
import { applyWorkspaceCleanupPolicy } from '../../../../shared/workspace-cleanup'
import type {
WorkspaceCleanupFilterState,
WorkspaceCleanupSortField,
@@ -77,5 +78,22 @@ export function applyWorkspaceCleanupGitEvidence(
if (evidenceByWorktreeId.size === 0) {
return candidates
}
return candidates.map((candidate) => evidenceByWorktreeId.get(candidate.worktreeId) ?? candidate)
return candidates.map((candidate) => {
const evidence = evidenceByWorktreeId.get(candidate.worktreeId)
if (
evidence === undefined ||
(candidate.git.checkedAt !== null &&
candidate.git.checkedAt >= (evidence.git.checkedAt ?? Number.NEGATIVE_INFINITY))
) {
return candidate
}
return applyWorkspaceCleanupPolicy({
...candidate,
blockers: [
...candidate.blockers.filter((blocker) => !GIT_DERIVED_BLOCKERS.has(blocker)),
...evidence.blockers.filter((blocker) => GIT_DERIVED_BLOCKERS.has(blocker))
],
git: evidence.git
})
})
}
@@ -21,6 +21,7 @@ import {
FacetToggleList
} from './workspace-cleanup-facet-controls'
import {
getWorkspaceCleanupBlockerModeLabel,
getWorkspaceCleanupGitStateLabel,
getWorkspaceCleanupPresenceLabel,
getWorkspaceCleanupReviewProviderLabel,
@@ -176,6 +177,7 @@ export function WorkspaceCleanupGitReviewFacets({
label={translate('components.workspace.cleanup.browse.blockerMode', 'Blocker match')}
value={filters.safety.blockerMode}
options={WORKSPACE_CLEANUP_BLOCKER_MODE_VALUES}
getLabel={getWorkspaceCleanupBlockerModeLabel}
onChange={(blockerMode) => onPatch('safety', { blockerMode })}
/>
<FacetToggleList
@@ -84,4 +84,12 @@ describe('WorkspaceCleanupSortHeader', () => {
expect(onToggleSelectAll).toHaveBeenCalledWith(true)
})
it('exposes a partial selection as mixed', () => {
render({ field: 'name', direction: 'asc' }, { selectableCount: 3, selectedCount: 1 })
expect(
container?.querySelector<HTMLButtonElement>('[role="checkbox"]')?.getAttribute('aria-checked')
).toBe('mixed')
})
})
@@ -1,5 +1,5 @@
import React from 'react'
import { ArrowDown, ArrowUp, Check, ChevronsUpDown } from 'lucide-react'
import { ArrowDown, ArrowUp, Check, ChevronsUpDown, Minus } from 'lucide-react'
import { Button } from '@/components/ui/button'
import {
DropdownMenu,
@@ -30,12 +30,13 @@ export function WorkspaceCleanupSortHeader({
onToggleSelectAll: (selectAll: boolean) => void
}): React.JSX.Element {
const allSelected = selectableCount > 0 && selectedCount >= selectableCount
const someSelected = selectableCount > 0 && selectedCount > 0 && !allSelected
return (
<div className="flex items-center gap-1 border-b border-border bg-muted/25 px-3 py-1.5">
<button
type="button"
role="checkbox"
aria-checked={allSelected}
aria-checked={someSelected ? 'mixed' : allSelected}
disabled={selectableCount === 0}
aria-label={translate(
'components.workspace.cleanup.browse.selectAll',
@@ -44,7 +45,11 @@ export function WorkspaceCleanupSortHeader({
onClick={() => onToggleSelectAll(!allSelected)}
className="flex size-4 shrink-0 items-center justify-center rounded border border-border bg-background text-primary hover:bg-accent disabled:opacity-40 focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-ring"
>
{allSelected ? <Check className="size-3" strokeWidth={3} /> : null}
{allSelected ? (
<Check className="size-3" strokeWidth={3} />
) : someSelected ? (
<Minus className="size-3" strokeWidth={3} />
) : null}
</button>
<span className="min-w-0 flex-1 text-xs text-muted-foreground">
{translate(
+2
View File
@@ -15032,6 +15032,8 @@
"protected": "Protected"
},
"blockerMode": "Blocker match",
"blockerModeAny": "Has any",
"blockerModeNone": "Has none",
"blockers": "Blockers",
"dismissed": "Ignored",
"selectableOnly": "Only workspaces I can delete now",