fix(workspace-cleanup): stop a sync broadcast reverting a filter you just set

`workspaceCleanupBrowse` re-hydrated on every hydration source. Two lines below
it, `activeView` is startup-guarded for the same reason and says so.

How it fired: the browse writer debounces 250ms. Any other `ui.set` inside that
window makes main broadcast to all windows (`main/ipc/ui.ts:54-61`) carrying
state that still holds the previous filters. The window hydrated that over the
edit, and the pending debounce then wrote `get().workspaceCleanupBrowse` -- by
then the reverted value -- so the revert was persisted, not just displayed.
Your own write echoing back is harmless; it is somebody else's write landing
first that does the damage. Intermittent, which is why it reads as "Orca forgot
my filter".

Guarded to `source === 'startup'`, matching activeView. The cost is that browse
state no longer syncs between windows or from mobile. That costs nothing today:
`mobile/src` has no workspace-cleanup consumer, and the popout runs a separate
store and JS context, so nothing else in the product edits browse state. Filters
become per-window, which is the call already made for activeView.

Rejected the alternative (last-write-wins via a local dirty generation): it
preserves a cross-window sync nobody uses, in exchange for more state to get
wrong on a path that just proved it can silently discard user input.

Three of the five tests fail against the unguarded source; the two that stay
green assert what must not change -- startup still restores, and dismissals
still hydrate on sync because they are main-owned.
This commit is contained in:
Brennan Benson
2026-08-18 14:50:42 -07:00
parent 1be6a035b9
commit 006374d98b
2 changed files with 88 additions and 1 deletions
@@ -0,0 +1,79 @@
import { describe, expect, it } from 'vitest'
import { createDefaultWorkspaceCleanupBrowseState } from '../../../../shared/workspace-cleanup-browse-state'
import { createUIStore, makePersistedUI } from './ui-slice-test-harness'
function persistedWithIdleDays(idleMinDays: number | null) {
const browse = createDefaultWorkspaceCleanupBrowseState()
browse.filters.activity.idleMinDays = idleMinDays
return makePersistedUI({ workspaceCleanup: { dismissals: {}, browse } })
}
describe('workspace cleanup browse hydration', () => {
it('restores persisted filters on startup', () => {
const store = createUIStore()
store.getState().hydratePersistedUI(persistedWithIdleDays(30), 'startup')
expect(store.getState().workspaceCleanupBrowse.filters.activity.idleMinDays).toBe(30)
})
it('does not let a sync broadcast revert a filter the user just set', () => {
// The browse writer debounces 250ms, so any other ui.set inside that window
// broadcasts state that still holds the previous filters. Re-hydrating it here
// reverted the edit, and the pending debounce then persisted the revert.
const store = createUIStore()
store.getState().hydratePersistedUI(persistedWithIdleDays(30), 'startup')
const edited = createDefaultWorkspaceCleanupBrowseState()
edited.filters.activity.idleMinDays = 45
store.setState({ workspaceCleanupBrowse: edited })
store.getState().hydratePersistedUI(persistedWithIdleDays(30), 'sync')
expect(store.getState().workspaceCleanupBrowse.filters.activity.idleMinDays).toBe(45)
})
it('does not let a sync broadcast revert a typed query', () => {
const store = createUIStore()
const edited = createDefaultWorkspaceCleanupBrowseState()
edited.filters.query = 'release'
store.setState({ workspaceCleanupBrowse: edited })
store.getState().hydratePersistedUI(persistedWithIdleDays(null), 'sync')
expect(store.getState().workspaceCleanupBrowse.filters.query).toBe('release')
})
it('defaults an absent source to sync, so an unlabelled broadcast cannot revert either', () => {
const store = createUIStore()
const edited = createDefaultWorkspaceCleanupBrowseState()
edited.filters.activity.idleMinDays = 45
store.setState({ workspaceCleanupBrowse: edited })
store.getState().hydratePersistedUI(persistedWithIdleDays(30))
expect(store.getState().workspaceCleanupBrowse.filters.activity.idleMinDays).toBe(45)
})
it('still hydrates dismissals on sync, which are main-owned and not edited here', () => {
const store = createUIStore()
const dismissal = {
worktreeId: 'repo-1::/repo/one',
dismissedAt: Date.now(),
fingerprint: 'fp-1',
classifierVersion: 2
}
store.getState().hydratePersistedUI(
makePersistedUI({
workspaceCleanup: {
dismissals: { [dismissal.worktreeId]: dismissal },
browse: createDefaultWorkspaceCleanupBrowseState()
}
}),
'sync'
)
expect(store.getState().workspaceCleanupDismissals[dismissal.worktreeId]).toBeDefined()
})
})
+9 -1
View File
@@ -2625,7 +2625,15 @@ export const createUISlice: StateCreator<AppState, [], [], UISlice> = (set, get)
// Why the normalizer rather than a cast: this blob is hand-editable and
// may come from an older or newer build; it degrades field by field
// instead of bricking the cleanup dialog.
workspaceCleanupBrowse: normalizeWorkspaceCleanupBrowseState(ui.workspaceCleanup?.browse),
// Why startup-only, like activeView below: the browse writer debounces 250ms, so
// any other ui.set inside that window broadcasts state still holding the previous
// filters. Re-hydrating it here reverted the edit, and the pending debounce then
// persisted the revert. Nothing else edits browse state (mobile has no cleanup
// consumer, the popout is a separate store), so filters are per-window.
workspaceCleanupBrowse:
source === 'startup'
? normalizeWorkspaceCleanupBrowseState(ui.workspaceCleanup?.browse)
: s.workspaceCleanupBrowse,
// Why: restore only on startup; on 'sync' broadcasts it would clobber the window's current per-window view.
activeView:
source === 'startup'