diff --git a/src/main/ipc/workspace-cleanup.ts b/src/main/ipc/workspace-cleanup.ts index 49f553d36f2..829a1918470 100644 --- a/src/main/ipc/workspace-cleanup.ts +++ b/src/main/ipc/workspace-cleanup.ts @@ -57,10 +57,7 @@ export function registerWorkspaceCleanupHandlers( ) ipcMain.handle('workspaceCleanup:dismiss', (_event, args: WorkspaceCleanupDismissArgs) => { - // Why spread the whole record: updateUI replaces `workspaceCleanup` wholesale, - // so writing dismissals alone would erase the persisted browse state. - const currentState = store.getUI().workspaceCleanup - const next = { ...currentState?.dismissals } + const next = { ...store.getUI().workspaceCleanup?.dismissals } for (const dismissal of args.dismissals ?? []) { if ( dismissal && @@ -71,13 +68,11 @@ export function registerWorkspaceCleanupHandlers( next[dismissal.worktreeId] = dismissal } } - store.updateUI({ workspaceCleanup: { ...currentState, dismissals: next } }) + store.updateUI({ workspaceCleanup: { dismissals: next } }) }) ipcMain.handle('workspaceCleanup:clearDismissals', () => { - store.updateUI({ - workspaceCleanup: { ...store.getUI().workspaceCleanup, dismissals: {} } - }) + store.updateUI({ workspaceCleanup: { dismissals: {} } }) }) ipcMain.handle( diff --git a/src/main/persistence.test.ts b/src/main/persistence.test.ts index b59ca6f5986..de1bf8aaff3 100644 --- a/src/main/persistence.test.ts +++ b/src/main/persistence.test.ts @@ -41,6 +41,7 @@ import { SshConnectionStore } from './ssh/ssh-connection-store' import { setSourceControlActionDefault } from '../shared/source-control-ai-actions' import { LEGACY_DEFAULT_SSH_RELAY_GRACE_PERIOD_SECONDS } from '../shared/ssh-types' import { closeTerminalTabInWorkspaceSession } from '../shared/workspace-session-terminal-tab-close' +import { createDefaultWorkspaceCleanupBrowseState } from '../shared/workspace-cleanup-browse-state' // Shared mutable state so the electron mock can reference a per-test directory const testState = { dir: '' } @@ -6495,6 +6496,29 @@ describe('Store', () => { }) }) + it('updateUI preserves browse state when a legacy peer publishes dismissals only', async () => { + const store = await createStore() + const browse = createDefaultWorkspaceCleanupBrowseState() + browse.filters.query = 'stale' + + store.updateUI({ workspaceCleanup: { dismissals: {}, browse } }) + store.updateUI({ + workspaceCleanup: { + dismissals: { + 'wt-1': { + worktreeId: 'wt-1', + dismissedAt: 1700000000000, + fingerprint: 'fp-1', + classifierVersion: 2 + } + } + } + }) + + expect(store.getUI().workspaceCleanup?.browse).toEqual(browse) + expect(store.getUI().workspaceCleanup?.dismissals).toHaveProperty('wt-1') + }) + it('updateUI merges contextual tour seen ids instead of replacing stale snapshots', async () => { const store = await createStore() diff --git a/src/main/persistence.ts b/src/main/persistence.ts index 8a74aea7b8a..d6791ebc9be 100644 --- a/src/main/persistence.ts +++ b/src/main/persistence.ts @@ -176,6 +176,7 @@ import { } from '../shared/cross-platform-path' import { normalizeTerminalQuickCommands } from '../shared/terminal-quick-commands' import { normalizeTaskProviderSettings } from '../shared/task-providers' +import { mergeWorkspaceCleanupUIState } from '../shared/workspace-cleanup-ui-state' import { normalizeAutoRenameBranchFromWorkDefaultOn } from '../shared/auto-rename-branch-from-work-settings' import { addMobilePairingCustomAddress, @@ -6041,6 +6042,10 @@ export class Store { const nextUI = { ...currentUI, ...durableUpdates, + workspaceCleanup: mergeWorkspaceCleanupUIState( + currentUI.workspaceCleanup, + durableUpdates.workspaceCleanup + ), groupBy: durableUpdates.groupBy ? normalizeGroupBy(durableUpdates.groupBy) : normalizeGroupBy(this.state.ui?.groupBy), diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 9d250a74956..ed8161ebd53 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -15054,7 +15054,6 @@ "retainedDoneAgents": "Finished agent transcripts", "contextPresence": "Open tabs, terminals, comments", "completelyEmpty": "Nothing left to lose", - "presetModified": "edited", "noWorkspaces": "No workspaces found.", "noMatches": "No workspaces match these filters.", "noMatchesDescription": "Every workspace is in one list — widen a facet or clear the filters.", diff --git a/src/renderer/src/store/slices/workspace-cleanup-browse.ts b/src/renderer/src/store/slices/workspace-cleanup-browse.ts index 91134db96fa..9e917284912 100644 --- a/src/renderer/src/store/slices/workspace-cleanup-browse.ts +++ b/src/renderer/src/store/slices/workspace-cleanup-browse.ts @@ -31,8 +31,8 @@ export const createWorkspaceCleanupBrowseSlice: StateCreator< } persistTimer = setTimeout(() => { persistTimer = null - // Why dismissals ride along: main replaces `workspaceCleanup` wholesale on - // ui.set, so a browse-only payload would erase every dismissal. + // Why dismissals ride along: the legacy wire schema requires the field, + // while `browse` stays optional for older clients. window.api.ui .set({ workspaceCleanup: { diff --git a/src/renderer/src/web/web-preload-api.ts b/src/renderer/src/web/web-preload-api.ts index c90c7cd95ec..31cbdca057c 100644 --- a/src/renderer/src/web/web-preload-api.ts +++ b/src/renderer/src/web/web-preload-api.ts @@ -146,6 +146,7 @@ import { parseRuntimeNativeChatTurnLifecycle } from '@/components/native-chat/native-chat-runtime-contract' import { createWebFileMutationMethods } from './web-file-mutation-methods' +import { mergeWorkspaceCleanupUIState } from '../../../shared/workspace-cleanup-ui-state' const SETTINGS_STORAGE_KEY = 'orca.web.settings.v1' const UI_STORAGE_KEY = 'orca.web.ui.v1' @@ -3916,6 +3917,10 @@ function mergeWebUIState( return { ...base, ...safeUpdates, + workspaceCleanup: mergeWorkspaceCleanupUIState( + base.workspaceCleanup, + safeUpdates.workspaceCleanup + ), worktreeCardProperties: normalizeWorktreeCardProperties( safeUpdates.worktreeCardProperties ?? base.worktreeCardProperties ), diff --git a/src/shared/workspace-cleanup-browse-state-persistence.test.ts b/src/shared/workspace-cleanup-browse-state-persistence.test.ts index 251c5c25db0..d0fad16e1c1 100644 --- a/src/shared/workspace-cleanup-browse-state-persistence.test.ts +++ b/src/shared/workspace-cleanup-browse-state-persistence.test.ts @@ -55,6 +55,22 @@ describe('workspace cleanup browse state persistence', () => { expect(normalized).not.toHaveProperty('unknownTopLevelField') }) + it('drops invalid persisted host ids without losing valid SSH and runtime hosts', () => { + const normalized = normalizeWorkspaceCleanupBrowseState({ + filters: { + location: { + hostIds: ['local', 'ssh:build%20box', 'not-a-host', 'runtime:linux', 'ssh:'] + } + } + }) + + expect(normalized.filters.location.hostIds).toEqual([ + 'local', + 'ssh:build%20box', + 'runtime:linux' + ]) + }) + it.each([null, undefined, 'corrupt', 42, [], { filters: 'nonsense' }])( 'never throws on a corrupt blob (%p)', (value) => { diff --git a/src/shared/workspace-cleanup-filter-state-codec.ts b/src/shared/workspace-cleanup-filter-state-codec.ts index 5465adc75d2..5af505a281e 100644 --- a/src/shared/workspace-cleanup-filter-state-codec.ts +++ b/src/shared/workspace-cleanup-filter-state-codec.ts @@ -1,4 +1,4 @@ -import type { ExecutionHostId } from './execution-host' +import { normalizeExecutionHostId, type ExecutionHostId } from './execution-host' import type { HostedReviewProvider } from './hosted-review' import { WORKSPACE_CLEANUP_AGENT_STATE_VALUES, @@ -99,7 +99,7 @@ export function normalizeWorkspaceCleanupFilterState(value: unknown): WorkspaceC completelyEmpty: asBoolean(context.completelyEmpty, base.context.completelyEmpty) }, location: { - hostIds: asStringList(location.hostIds) as ExecutionHostId[], + hostIds: asExecutionHostIdList(location.hostIds), repoIds: asStringList(location.repoIds), pathPrefix: asString(location.pathPrefix, base.location.pathPrefix) }, @@ -199,6 +199,23 @@ export function asStringList(value: unknown): string[] { return [...seen] } +function asExecutionHostIdList(value: unknown): ExecutionHostId[] { + if (!Array.isArray(value)) { + return [] + } + const result = new Set() + for (const entry of value) { + if (typeof entry !== 'string') { + continue + } + const hostId = normalizeExecutionHostId(entry) + if (hostId) { + result.add(hostId) + } + } + return [...result] +} + function asTriState( value: unknown, fallback: WorkspaceCleanupFilterState['status']['archived'] diff --git a/src/shared/workspace-cleanup-ui-state.test.ts b/src/shared/workspace-cleanup-ui-state.test.ts new file mode 100644 index 00000000000..12aef389652 --- /dev/null +++ b/src/shared/workspace-cleanup-ui-state.test.ts @@ -0,0 +1,50 @@ +import { describe, expect, it } from 'vitest' +import { createDefaultWorkspaceCleanupBrowseState } from './workspace-cleanup-browse-state' +import { mergeWorkspaceCleanupUIState } from './workspace-cleanup-ui-state' + +describe('workspace cleanup UI state', () => { + it('keeps browse state when a legacy peer publishes dismissals only', () => { + const browse = createDefaultWorkspaceCleanupBrowseState() + browse.filters.query = 'stale' + + expect( + mergeWorkspaceCleanupUIState( + { dismissals: {}, browse }, + { + dismissals: { + 'wt-1': { + worktreeId: 'wt-1', + dismissedAt: 1700000000000, + fingerprint: 'fp-1', + classifierVersion: 2 + } + } + } + ) + ).toEqual({ + dismissals: { + 'wt-1': { + worktreeId: 'wt-1', + dismissedAt: 1700000000000, + fingerprint: 'fp-1', + classifierVersion: 2 + } + }, + browse + }) + }) + + it('accepts browse state published by a compatible peer', () => { + const current = createDefaultWorkspaceCleanupBrowseState() + current.filters.query = 'local' + const incoming = createDefaultWorkspaceCleanupBrowseState() + incoming.filters.query = 'host' + + expect( + mergeWorkspaceCleanupUIState( + { dismissals: {}, browse: current }, + { dismissals: {}, browse: incoming } + )?.browse + ).toBe(incoming) + }) +}) diff --git a/src/shared/workspace-cleanup-ui-state.ts b/src/shared/workspace-cleanup-ui-state.ts new file mode 100644 index 00000000000..a674810c590 --- /dev/null +++ b/src/shared/workspace-cleanup-ui-state.ts @@ -0,0 +1,15 @@ +import type { WorkspaceCleanupUIState } from './workspace-cleanup' + +/** Keep browse state when an older client or host publishes dismissals only. */ +export function mergeWorkspaceCleanupUIState( + current: WorkspaceCleanupUIState | undefined, + incoming: WorkspaceCleanupUIState | undefined +): WorkspaceCleanupUIState | undefined { + if (!incoming) { + return current + } + return { + ...current, + ...incoming + } +}