diff --git a/config/flaky-test-pattern-baseline.txt b/config/flaky-test-pattern-baseline.txt index f321478654d..71739d272ec 100644 --- a/config/flaky-test-pattern-baseline.txt +++ b/config/flaky-test-pattern-baseline.txt @@ -1146,7 +1146,6 @@ 6 test-no-wall-clock:src/renderer/src/components/workspace-cleanup/workspace-cleanup-confirm-remove.test.tsx 2 test-no-wall-clock:src/renderer/src/components/worktree-creation/WorktreeCreationPanel.test.tsx 2 test-no-wall-clock:src/renderer/src/hooks/ipc-events/agent-status-terminal-title-tab-write.test.ts -1 test-no-wall-clock:src/renderer/src/hooks/ipc-events/worktree-event-lineage-refresh.test.ts 3 test-no-wall-clock:src/renderer/src/hooks/useAutomationDispatchEvents.test.ts 1 test-no-wall-clock:src/renderer/src/hooks/useIpcEvents-agent-status-hook-titles.test.ts 1 test-no-wall-clock:src/renderer/src/hooks/useIpcEvents-browser-tab-create.test.ts diff --git a/config/workspace-record-read-baseline.txt b/config/workspace-record-read-baseline.txt index 4aa54c90b5e..d049fa3f0c5 100644 --- a/config/workspace-record-read-baseline.txt +++ b/config/workspace-record-read-baseline.txt @@ -68,7 +68,6 @@ 3 src/renderer/src/store/slices/worktrees/listing/fetch-detected-worktrees.ts # store writer 13 src/renderer/src/store/slices/worktrees/listing/fetched-worktree-merge.ts # store writer 6 src/renderer/src/store/slices/worktrees/listing/known-ssh-worktree-fetch.ts # store writer -3 src/renderer/src/store/slices/worktrees/listing/worktree-sort-order-patch.ts # store writer 4 src/renderer/src/store/slices/worktrees/metadata/hosted-review-link-mutation.ts # store writer 8 src/renderer/src/store/slices/worktrees/metadata/update-worktrees-meta.ts # store writer 2 src/renderer/src/store/slices/worktrees/metadata/worktree-git-identity-update.ts # store writer diff --git a/src/renderer/src/components/sidebar/HostRemoveDialog.test.tsx b/src/renderer/src/components/sidebar/HostRemoveDialog.test.tsx new file mode 100644 index 00000000000..6637e9f4e6b --- /dev/null +++ b/src/renderer/src/components/sidebar/HostRemoveDialog.test.tsx @@ -0,0 +1,156 @@ +// @vitest-environment happy-dom + +import '@testing-library/jest-dom/vitest' +import { act, cleanup, fireEvent, render, screen } from '@testing-library/react' +import { afterEach, beforeEach, expect, it, vi } from 'vitest' +import { useAppStore } from '@/store' +import { getWorktreeMapFromState } from '@/store/selectors' +import { makeWorktree, TEST_REPO } from '@/store/slices/worktrees-slice-test-fixtures' +import { HostRemoveDialog } from './HostRemoveDialog' + +const initialState = useAppStore.getState() +const onOpenChange = vi.fn() + +function dialogs(openHost: string | null) { + return ['one', 'two'].map((id) => ( + + )) +} + +function replaceCatalogs(count: number) { + useAppStore.setState({ + worktreesByRepo: { + [TEST_REPO.id]: Array.from({ length: count }, (_, index) => + makeWorktree({ id: `workspace-${index}`, repoId: TEST_REPO.id }) + ) + }, + detectedWorktreesByRepo: {} + }) +} + +beforeEach(() => { + useAppStore.setState({ + repos: [{ ...TEST_REPO, connectionId: 'one' }], + openFiles: [], + editorDrafts: {} + }) + replaceCatalogs(1) +}) + +afterEach(() => { + cleanup() + useAppStore.setState(initialState, true) + vi.restoreAllMocks() + vi.unstubAllGlobals() +}) + +it('releases both obsolete catalogs and cached indexes while host dialogs stay mounted', async () => { + const view = render(dialogs(null)) + + function openAndCloseHost(id: string) { + view.rerender(dialogs(id)) + expect(screen.getByRole('dialog')).toHaveTextContent(`Remove ${id}?`) + const state = useAppStore.getState() + const retired = { + worktrees: new WeakRef(state.worktreesByRepo), + detected: new WeakRef(state.detectedWorktreesByRepo), + index: new WeakRef(getWorktreeMapFromState(state)) + } + view.rerender(dialogs(null)) + expect(screen.queryByRole('dialog')).not.toBeInTheDocument() + act(() => replaceCatalogs(2)) + return retired + } + + const retiredCatalogs = ['one', 'two'].map(openAndCloseHost) + if (typeof globalThis.gc !== 'function') { + throw new Error('Run with the repository Vitest --expose-gc config') + } + for (let round = 0; round < 3; round++) { + await new Promise((resolve) => setImmediate(resolve)) + globalThis.gc() + } + for (const retired of retiredCatalogs) { + expect(retired.worktrees.deref()).toBeUndefined() + expect(retired.detected.deref()).toBeUndefined() + expect(retired.index.deref()).toBeUndefined() + } + + view.rerender(dialogs('one')) + expect(screen.getByRole('dialog')).toHaveTextContent('Its 3 workspaces stay in Orca') +}) + +it('keeps workspace details through the closing animation', () => { + const getStyle = window.getComputedStyle.bind(window) + vi.spyOn(window, 'getComputedStyle').mockImplementation((element, ...args) => { + const style = getStyle(element, ...args) + if (element.getAttribute('data-slot') === 'dialog-content') { + return new Proxy(style, { + get: (target, property) => + property === 'animationName' + ? element.getAttribute('data-state') === 'closed' + ? 'exit' + : 'enter' + : // oxlint-disable-next-line anti-slop/no-reflect-get -- Forward CSS properties without changing their receiver. + Reflect.get(target, property) + }) + } + return style + }) + const view = render(dialogs('one')) + const dialog = screen.getByRole('dialog') + const removeButton = screen.getByRole('button', { name: 'Remove host' }) + expect(removeButton).toBeEnabled() + expect(dialog).toHaveTextContent('Its 2 workspaces stay in Orca') + + view.rerender(dialogs(null)) + expect(dialog).toBeInTheDocument() + expect(dialog).toHaveAttribute('data-state', 'closed') + expect(removeButton).toBeDisabled() + expect(dialog).toHaveTextContent('Its 2 workspaces stay in Orca') + + const event = new Event('animationend', { bubbles: true }) + Object.defineProperty(event, 'animationName', { value: 'exit' }) + fireEvent(dialog, event) + expect(dialog).not.toBeInTheDocument() +}) + +it.each(['success', 'failure'] as const)( + 'keeps removal pending across dismiss/reopen and releases it after %s', + async (outcome) => { + const pending = Promise.withResolvers() + const remove = vi.fn().mockReturnValue(pending.promise) + vi.stubGlobal('api', { ssh: { removeTarget: remove } }) + useAppStore.setState({ + clearRemovedSshTargetState: vi.fn(), + updateSettings: vi.fn().mockResolvedValue(undefined) + }) + const view = render(dialogs('one')) + fireEvent.click(screen.getByRole('button', { name: 'Remove host' })) + expect(remove).toHaveBeenCalledTimes(1) + view.rerender(dialogs(null)) + expect(screen.queryByRole('dialog')).not.toBeInTheDocument() + view.rerender(dialogs('one')) + const button = screen.getByRole('button', { name: 'Remove host' }) + expect(button).toBeDisabled() + fireEvent.click(button) + expect(remove).toHaveBeenCalledTimes(1) + + await act(async () => { + if (outcome === 'success') { + pending.resolve() + } else { + pending.reject(new Error('Removal failed')) + } + await pending.promise.catch(() => {}) + }) + expect(button).toBeEnabled() + } +) diff --git a/src/renderer/src/components/sidebar/HostRemoveDialog.tsx b/src/renderer/src/components/sidebar/HostRemoveDialog.tsx index ef7b6be373e..beeca9fe995 100644 --- a/src/renderer/src/components/sidebar/HostRemoveDialog.tsx +++ b/src/renderer/src/components/sidebar/HostRemoveDialog.tsx @@ -39,25 +39,41 @@ type HostRemoveDialogProps = { target: NonNullable | { kind: 'wsl'; environmentId: string } } -export function HostRemoveDialog({ +export function HostRemoveDialog(props: HostRemoveDialogProps): React.JSX.Element { + const [busy, setBusy] = useState(false) + return ( + + + + + + ) +} + +// Radix unmounts the body after its exit animation, releasing catalog subscriptions and snapshots. +function HostRemoveDialogBody({ open, onOpenChange, hostId, label, - target -}: HostRemoveDialogProps): React.JSX.Element { - const [busy, setBusy] = useState(false) + target, + busy, + setBusy +}: HostRemoveDialogProps & { + busy: boolean + setBusy: (busy: boolean) => void +}): React.JSX.Element { // Keep workspaces by default; destructive remote removal requires explicit Advanced opt-in. const [advancedOpen, setAdvancedOpen] = useState(false) const [deleteWorkspaces, setDeleteWorkspaces] = useState(false) const mountedRef = useMountedRef() const repos = useAppStore((s) => s.repos) - const worktreesByRepo = useAppStore((s) => selectWorktreesByRepo(s)) - const detectedWorktreesByRepo = useAppStore((s) => selectDetectedWorktreesByRepo(s)) + const worktreesByRepo = useAppStore(selectWorktreesByRepo) + const detectedWorktreesByRepo = useAppStore(selectDetectedWorktreesByRepo) const sshResolution = useMemo(() => { - if (target.kind !== 'ssh') { + if (target.kind !== 'ssh' || !worktreesByRepo) { return null } return resolveSshHostRemoval({ @@ -68,7 +84,7 @@ export function HostRemoveDialog({ }, [target, repos, worktreesByRepo]) const unsavedTargets = useMemo(() => { - if (!sshResolution || !deleteWorkspaces) { + if (!sshResolution || !deleteWorkspaces || !worktreesByRepo || !detectedWorktreesByRepo) { return [] } const hostId = toSshExecutionHostId(sshResolution.targetId) @@ -109,7 +125,7 @@ export function HostRemoveDialog({ } const runSshRemoval = async (): Promise => { - if (target.kind === 'runtime') { + if (busy || target.kind === 'runtime') { return } setBusy(true) @@ -121,9 +137,6 @@ export function HostRemoveDialog({ ) // Keep the host if workspace cleanup fails so stranded rows remain reachable for retry. if (failedIds.length > 0) { - if (mountedRef.current) { - setBusy(false) - } toast.error( translate( 'auto.components.sidebar.HostRemoveDialog.workspacesFailed', @@ -158,9 +171,7 @@ export function HostRemoveDialog({ ) ) } finally { - if (mountedRef.current) { - setBusy(false) - } + setBusy(false) } } @@ -199,101 +210,90 @@ export function HostRemoveDialog({ ) return ( - - - - - {translate( - 'auto.components.sidebar.HostRemoveDialog.3c4d5e6f7a', - 'Remove {{value0}}?', - { - value0: label - } - )} - - {description} - + <> + + + {translate('auto.components.sidebar.HostRemoveDialog.3c4d5e6f7a', 'Remove {{value0}}?', { + value0: label + })} + + {description} + - {target.kind === 'ssh' && hasWorkspaces ? ( -
- -
-
-
- - - {deleteOptionLabel} - - {translate( - 'auto.components.sidebar.HostRemoveDialog.alsoForgetLocalHint', - 'Clears them from Orca only. Remote files, worktrees, and branches are left untouched.' - )} - + {target.kind === 'ssh' && hasWorkspaces ? ( +
+ +
+
+
+ + + {deleteOptionLabel} + + {translate( + 'auto.components.sidebar.HostRemoveDialog.alsoForgetLocalHint', + 'Clears them from Orca only. Remote files, worktrees, and branches are left untouched.' + )} -
+
- ) : null} +
+ ) : null} - + - + + + {target.kind === 'runtime' ? ( - {target.kind === 'runtime' ? ( - - ) : ( - - )} - - -
+ ) : ( + + )} + + ) } diff --git a/src/renderer/src/components/sidebar/WorktreeContextMenu.closed-menu-work.test.tsx b/src/renderer/src/components/sidebar/WorktreeContextMenu.closed-menu-work.test.tsx new file mode 100644 index 00000000000..e16afd14836 --- /dev/null +++ b/src/renderer/src/components/sidebar/WorktreeContextMenu.closed-menu-work.test.tsx @@ -0,0 +1,165 @@ +/** + * @vitest-environment happy-dom + */ +import { act } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { Worktree } from '../../../../shared/worktree/types' +import { useAppStore } from '@/store' +import { makeLineage, makeWorktree } from '../../store/slices/worktrees-slice-test-fixtures' +import WorktreeContextMenu from './WorktreeContextMenu' +import type * as WorkspaceDeleteLineageModule from './workspace-delete-lineage' + +globalThis.IS_REACT_ACT_ENVIRONMENT = true + +const lineageTraversals = vi.hoisted(() => ({ count: 0 })) + +vi.mock('./workspace-delete-lineage', async (importOriginal) => { + const actual = await importOriginal() + return { + getWorkspaceDeleteLineage: (...args: Parameters) => { + lineageTraversals.count += 1 + return actual.getWorkspaceDeleteLineage(...args) + } + } +}) + +vi.mock('@/store', async () => { + const { create } = await import('zustand') + const noop = (): void => {} + return { + useAppStore: create>(() => ({ + unifiedTabsByWorktree: {}, + agentStatusByPaneKey: {}, + structuredChatSleptAtByPaneKey: {}, + updateWorktreeMeta: noop, + setWorkspaceUnreadByUser: noop, + setWorktreesPinnedAndReveal: noop, + workspaceStatuses: [], + hostWorkspaceStatuses: {}, + removedRuntimeEnvironmentIds: [], + openModal: noop, + projectGroups: [], + createProjectGroup: noop, + moveProjectToGroup: noop, + deleteStateByWorktreeId: {}, + worktreeLineageById: {}, + workspaceLineageByChildKey: {}, + updateWorktreeLineage: noop, + tabsByWorktree: {}, + ptyIdsByTabId: {}, + browserTabsByWorktree: {}, + worktreesByRepo: {}, + repos: [] + })) + } +}) + +vi.mock('@/components/ui/dropdown-menu', () => { + const Passthrough = (props: { children?: React.ReactNode }) =>
{props.children}
+ return { + DropdownMenu: (props: { children?: React.ReactNode; open?: boolean }) => + props.open ?
{props.children}
: null, + DropdownMenuContent: Passthrough, + DropdownMenuItem: Passthrough, + DropdownMenuSeparator: () =>
, + DropdownMenuShortcut: Passthrough, + DropdownMenuLabel: Passthrough, + DropdownMenuSub: Passthrough, + DropdownMenuSubContent: Passthrough, + DropdownMenuSubTrigger: Passthrough, + DropdownMenuTrigger: Passthrough, + DropdownMenuRadioGroup: Passthrough, + DropdownMenuRadioItem: Passthrough + } +}) + +vi.mock('@/components/ui/tooltip', () => ({ + Tooltip: ({ children }: { children: React.ReactNode }) => <>{children}, + TooltipTrigger: ({ children }: { children: React.ReactNode }) => <>{children}, + TooltipContent: ({ children }: { children: React.ReactNode }) => <>{children} +})) +vi.mock('lucide-react', async () => + (await import('../tab-bar/lucide-icon-stub-fixture')).stubEveryIcon() +) +vi.mock('@/i18n/i18n', () => ({ + i18n: { language: 'en' }, + translate: (_key: string, fallback: string) => fallback +})) +vi.mock('@/hooks/useShortcutLabel', () => ({ + useOptionalShortcutLabel: () => null, + formatShortcutLabel: () => '', + formatOptionalShortcutLabel: () => null +})) +vi.mock('./WorktreeDeveloperMenu', () => ({ WorktreeDeveloperMenu: () => null })) +vi.mock('./WorktreeOpenInMenu', () => ({ WorktreeOpenInSubMenu: () => null })) +vi.mock('./WorkspaceSleepMenuItems', () => ({ WorkspaceSleepMenuItems: () => null })) + +const mounted: { container: HTMLDivElement; root: Root }[] = [] + +afterEach(() => { + for (const { root, container } of mounted) { + act(() => root.unmount()) + container.remove() + } + mounted.length = 0 +}) + +function row(id: string, sortOrder = 0): Worktree { + return makeWorktree({ + id: `repo::/w/${id}`, + repoId: 'repo', + instanceId: `${id}-instance`, + hostId: 'local', + path: `/w/${id}`, + sortOrder + }) +} + +describe('closed WorktreeContextMenu', () => { + it('skips lineage traversal for rank churn, then opens with the current descendants', () => { + const rows = Array.from({ length: 200 }, (_, index) => row(`w${index}`, index)) + useAppStore.setState({ worktreesByRepo: { repo: rows } }) + for (const worktree of rows.slice(0, 40)) { + const container = document.createElement('div') + document.body.appendChild(container) + const root = createRoot(container) + act(() => root.render(card)) + mounted.push({ container, root }) + } + lineageTraversals.count = 0 + + for (let burst = 0; burst < 9; burst += 1) { + act(() => + useAppStore.setState({ + worktreesByRepo: { repo: rows.map((item) => ({ ...item, sortOrder: burst })) } + }) + ) + } + expect(lineageTraversals.count).toBe(0) + + // A child linked while every menu was closed must appear on the next open. + const parent = rows[0] + const child = rows[1] + act(() => + useAppStore.setState({ + worktreeLineageById: { + [child.id]: makeLineage({ + worktreeId: child.id, + worktreeInstanceId: child.instanceId, + parentWorktreeId: parent.id, + parentWorktreeInstanceId: parent.instanceId + }) + } + }) + ) + const scope = mounted[0].container.querySelector('[data-worktree-context-menu-scope]') + act(() => { + scope?.dispatchEvent( + new MouseEvent('contextmenu', { bubbles: true, cancelable: true, clientX: 1, clientY: 1 }) + ) + }) + expect(lineageTraversals.count).toBeGreaterThan(0) + expect(mounted[0].container.textContent).toContain('Delete with Descendants…') + }) +}) diff --git a/src/renderer/src/components/sidebar/WorktreeContextMenu.copy-worktree-name.test.tsx b/src/renderer/src/components/sidebar/WorktreeContextMenu.copy-worktree-name.test.tsx index 3c8a896fb7c..337eb107669 100644 --- a/src/renderer/src/components/sidebar/WorktreeContextMenu.copy-worktree-name.test.tsx +++ b/src/renderer/src/components/sidebar/WorktreeContextMenu.copy-worktree-name.test.tsx @@ -39,13 +39,17 @@ vi.mock('@/store', () => ({ }) })) -vi.mock('@/store/selectors', () => ({ - useAllWorktrees: () => [], - useRepoById: (repoId?: string) => - repoId ? { id: repoId, name: repoId, displayName: repoId, projectGroupId: null } : undefined, - useRepoMap: () => new Map(), - useWorktreeMap: () => new Map() -})) +vi.mock('@/store/selectors', () => { + const emptyWorktrees: never[] = [] + const emptyMap = new Map() + return { + getAllWorktreesFromState: () => emptyWorktrees, + useRepoById: (repoId?: string) => + repoId ? { id: repoId, name: repoId, displayName: repoId, projectGroupId: null } : undefined, + getRepoMapFromState: () => emptyMap, + getWorktreeMapFromState: () => emptyMap + } +}) vi.mock('@/i18n/i18n', () => ({ translate: (_key: string, fallback: string) => fallback, diff --git a/src/renderer/src/components/sidebar/WorktreeContextMenu.delete-shortcut.test.tsx b/src/renderer/src/components/sidebar/WorktreeContextMenu.delete-shortcut.test.tsx index 80b11f02c9b..79d999ae2f6 100644 --- a/src/renderer/src/components/sidebar/WorktreeContextMenu.delete-shortcut.test.tsx +++ b/src/renderer/src/components/sidebar/WorktreeContextMenu.delete-shortcut.test.tsx @@ -112,13 +112,17 @@ vi.mock('@/store', () => ({ ) })) -vi.mock('@/store/selectors', () => ({ - useAllWorktrees: () => [], - useRepoById: (repoId?: string) => - repoId ? { id: repoId, name: repoId, displayName: repoId } : undefined, - useRepoMap: () => new Map(), - useWorktreeMap: () => new Map() -})) +vi.mock('@/store/selectors', () => { + const emptyWorktrees: never[] = [] + const emptyMap = new Map() + return { + getAllWorktreesFromState: () => emptyWorktrees, + useRepoById: (repoId?: string) => + repoId ? { id: repoId, name: repoId, displayName: repoId } : undefined, + getRepoMapFromState: () => emptyMap, + getWorktreeMapFromState: () => emptyMap + } +}) vi.mock('./ProjectGroupNameDialog', () => ({ ProjectGroupNameDialog: () => null diff --git a/src/renderer/src/components/sidebar/WorktreeContextMenu.host-statuses.test.tsx b/src/renderer/src/components/sidebar/WorktreeContextMenu.host-statuses.test.tsx index f6c927e0a06..aa5b786fd7c 100644 --- a/src/renderer/src/components/sidebar/WorktreeContextMenu.host-statuses.test.tsx +++ b/src/renderer/src/components/sidebar/WorktreeContextMenu.host-statuses.test.tsx @@ -45,13 +45,17 @@ vi.mock('@/store', () => ({ }) })) -vi.mock('@/store/selectors', () => ({ - useAllWorktrees: () => [], - useRepoById: (repoId?: string) => - repoId ? { id: repoId, name: repoId, displayName: repoId, projectGroupId: null } : undefined, - useRepoMap: () => new Map(), - useWorktreeMap: () => new Map() -})) +vi.mock('@/store/selectors', () => { + const emptyWorktrees: never[] = [] + const emptyMap = new Map() + return { + getAllWorktreesFromState: () => emptyWorktrees, + useRepoById: (repoId?: string) => + repoId ? { id: repoId, name: repoId, displayName: repoId, projectGroupId: null } : undefined, + getRepoMapFromState: () => emptyMap, + getWorktreeMapFromState: () => emptyMap + } +}) vi.mock('@/i18n/i18n', () => ({ translate: (_key: string, fallback: string) => fallback, diff --git a/src/renderer/src/components/sidebar/WorktreeContextMenu.react185-bystander.test.tsx b/src/renderer/src/components/sidebar/WorktreeContextMenu.react185-bystander.test.tsx index 7dff691eea3..35d9f307ed9 100644 --- a/src/renderer/src/components/sidebar/WorktreeContextMenu.react185-bystander.test.tsx +++ b/src/renderer/src/components/sidebar/WorktreeContextMenu.react185-bystander.test.tsx @@ -43,13 +43,17 @@ vi.mock('@/store', () => ({ }) })) -vi.mock('@/store/selectors', () => ({ - useAllWorktrees: () => [], - useRepoById: (repoId?: string) => - repoId ? { id: repoId, name: repoId, displayName: repoId, projectGroupId: null } : undefined, - useRepoMap: () => new Map(), - useWorktreeMap: () => new Map() -})) +vi.mock('@/store/selectors', () => { + const emptyWorktrees: never[] = [] + const emptyMap = new Map() + return { + getAllWorktreesFromState: () => emptyWorktrees, + useRepoById: (repoId?: string) => + repoId ? { id: repoId, name: repoId, displayName: repoId, projectGroupId: null } : undefined, + getRepoMapFromState: () => emptyMap, + getWorktreeMapFromState: () => emptyMap + } +}) vi.mock('@/i18n/i18n', () => ({ translate: (_key: string, fallback: string) => fallback, diff --git a/src/renderer/src/components/sidebar/WorktreeDeveloperMenuReveal.test.tsx b/src/renderer/src/components/sidebar/WorktreeDeveloperMenuReveal.test.tsx index 52f2a0a8c79..7779697b451 100644 --- a/src/renderer/src/components/sidebar/WorktreeDeveloperMenuReveal.test.tsx +++ b/src/renderer/src/components/sidebar/WorktreeDeveloperMenuReveal.test.tsx @@ -32,12 +32,16 @@ vi.mock('@/store', () => { } }) -vi.mock('@/store/selectors', () => ({ - useAllWorktrees: () => [], - useRepoById: () => null, - useRepoMap: () => new Map(), - useWorktreeMap: () => new Map() -})) +vi.mock('@/store/selectors', () => { + const emptyWorktrees: never[] = [] + const emptyMap = new Map() + return { + getAllWorktreesFromState: () => emptyWorktrees, + useRepoById: () => null, + getRepoMapFromState: () => emptyMap, + getWorktreeMapFromState: () => emptyMap + } +}) vi.mock('@/i18n/i18n', () => ({ translate: (_key: string, fallback: string) => fallback })) diff --git a/src/renderer/src/components/sidebar/use-menu-scoped-worktree-catalog.ts b/src/renderer/src/components/sidebar/use-menu-scoped-worktree-catalog.ts new file mode 100644 index 00000000000..c201e52683e --- /dev/null +++ b/src/renderer/src/components/sidebar/use-menu-scoped-worktree-catalog.ts @@ -0,0 +1,20 @@ +import { useAppStore } from '@/store' +import { + getAllWorktreesFromState, + getRepoMapFromState, + getWorktreeMapFromState +} from '@/store/selectors' +import { EMPTY_REPO_MAP, EMPTY_WORKTREE_MAP, EMPTY_WORKTREES } from './worktree-context-menu-policy' + +// Closed menus select stable empties so rank or catalog churn cannot wake every mounted row menu. +export function useMenuScopedWorktreeCatalog(menuOpen: boolean) { + // Check first so closed menus never rebuild the shared catalog indexes. + const repoMap = useAppStore((s) => (menuOpen ? getRepoMapFromState(s) : EMPTY_REPO_MAP)) + const worktreeMap = useAppStore((s) => + menuOpen ? getWorktreeMapFromState(s) : EMPTY_WORKTREE_MAP + ) + const allWorktrees = useAppStore((s) => + menuOpen ? getAllWorktreesFromState(s) : EMPTY_WORKTREES + ) + return { repoMap, worktreeMap, allWorktrees } +} diff --git a/src/renderer/src/components/sidebar/use-worktree-context-menu-model.tsx b/src/renderer/src/components/sidebar/use-worktree-context-menu-model.tsx index fd5b08a0d78..3b8e998d22e 100644 --- a/src/renderer/src/components/sidebar/use-worktree-context-menu-model.tsx +++ b/src/renderer/src/components/sidebar/use-worktree-context-menu-model.tsx @@ -1,12 +1,13 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { useAppStore } from '@/store' -import { useAllWorktrees, useRepoById, useRepoMap, useWorktreeMap } from '@/store/selectors' +import { useRepoById } from '@/store/selectors' import type { Worktree } from '../../../../shared/worktree/types' import { getCyclicProjectedWorktreeLineageIds, getLineageRenderInfo } from '../../../../shared/sidebar-list/worktree-lineage-projection' import { useContextWorkspaceStatuses } from './use-workspace-board-statuses' +import { useMenuScopedWorktreeCatalog } from './use-menu-scoped-worktree-catalog' import { getEligibleWorktreeParents } from './worktree-parent-candidates' import { hasSleepableWorkspaceActivity, @@ -93,9 +94,7 @@ export function useWorktreeContextMenuModel({ const parentPickerUnmountTimerRef = useRef(null) const lifecycleStartedRef = useRef(false) const isDeleting = deleteState?.isDeleting ?? false - const repoMap = useRepoMap() - const worktreeMap = useWorktreeMap() - const allWorktrees = useAllWorktrees() + const { repoMap, worktreeMap, allWorktrees } = useMenuScopedWorktreeCatalog(menuOpen) // Closed menus select stable empty maps; opening reads live maps without subscribing idle wrappers to teardown churn. const worktreeLineageById = useAppStore((s) => selectMenuScopedMap(menuOpen, s.worktreeLineageById, EMPTY_WORKTREE_LINEAGE_BY_ID) @@ -164,7 +163,7 @@ export function useWorktreeContextMenuModel({ [activeContextWorktrees, sleepActivity] ) const lineageMenuActions = useWorkspaceLineageMenuActions({ - enabled: !isMultiContext, + enabled: menuOpen && !isMultiContext, parent: worktree, worktrees: allWorktrees, lineageById: worktreeLineageById, @@ -355,7 +354,6 @@ export function useWorktreeContextMenuModel({ return { ...commands, activeContextWorktrees, - allWorktrees, batchDeleteWorktrees, browserTabsByWorktree, children, diff --git a/src/renderer/src/components/sidebar/worktree-context-menu-policy.ts b/src/renderer/src/components/sidebar/worktree-context-menu-policy.ts index 1e017ecc1b9..ca830ca1d18 100644 --- a/src/renderer/src/components/sidebar/worktree-context-menu-policy.ts +++ b/src/renderer/src/components/sidebar/worktree-context-menu-policy.ts @@ -29,6 +29,9 @@ export const EMPTY_DELETE_STATE_BY_WORKTREE_ID: AppState['deleteStateByWorktreeI export const EMPTY_WORKTREE_LINEAGE_BY_ID: AppState['worktreeLineageById'] = {} export const EMPTY_WORKSPACE_LINEAGE_BY_CHILD_KEY: AppState['workspaceLineageByChildKey'] = {} export const EMPTY_CYCLIC_LINEAGE_IDS: ReadonlySet = new Set() +export const EMPTY_WORKTREES: readonly Worktree[] = [] +export const EMPTY_WORKTREE_MAP: ReadonlyMap = new Map() +export const EMPTY_REPO_MAP: ReadonlyMap = new Map() // Return the shared empty reference while closed; open menus read current maps. export function selectMenuScopedMap(menuOpen: boolean, live: T, empty: T): T { diff --git a/src/renderer/src/components/sidebar/worktree-parent-candidates.ts b/src/renderer/src/components/sidebar/worktree-parent-candidates.ts index 4ed14fba93c..645ae1c48f7 100644 --- a/src/renderer/src/components/sidebar/worktree-parent-candidates.ts +++ b/src/renderer/src/components/sidebar/worktree-parent-candidates.ts @@ -9,14 +9,14 @@ type ParentCandidateArgs = { child: Worktree worktrees: readonly Worktree[] lineageById: Record - worktreeMap: Map - repoMap: Map> + worktreeMap: ReadonlyMap + repoMap: ReadonlyMap> cyclicLineageIds?: ReadonlySet } function getWorktreeOwnerHostId( worktree: Worktree, - repoMap: Map> + repoMap: ReadonlyMap> ): string | null { const repo = repoMap.get(worktree.repoId) return repo ? getWorktreeExecutionHostId(worktree, repo) : (worktree.hostId ?? null) diff --git a/src/renderer/src/components/sidebar/worktree-parent-eligibility.ts b/src/renderer/src/components/sidebar/worktree-parent-eligibility.ts index ec761af7732..f0eb866d0c0 100644 --- a/src/renderer/src/components/sidebar/worktree-parent-eligibility.ts +++ b/src/renderer/src/components/sidebar/worktree-parent-eligibility.ts @@ -9,7 +9,7 @@ type ParentEligibilityArgs = { child: Worktree candidateParent: Worktree lineageById: Record - worktreeMap: Map + worktreeMap: ReadonlyMap cyclicLineageIds?: ReadonlySet } diff --git a/src/renderer/src/hooks/ipc-events/worktree-event-lineage-refresh.test.ts b/src/renderer/src/hooks/ipc-events/worktree-event-lineage-refresh.test.ts index 9a78c872bdf..b328f44c7e5 100644 --- a/src/renderer/src/hooks/ipc-events/worktree-event-lineage-refresh.test.ts +++ b/src/renderer/src/hooks/ipc-events/worktree-event-lineage-refresh.test.ts @@ -13,6 +13,7 @@ import { } from '../../store/slices/worktrees-slice-test-fixtures' import { replaceRuntimeEnvironmentRevisions } from '../../runtime/runtime-environment-revision' import { advanceRuntimeEnvironmentConnectionGeneration } from '../../store/slices/runtime-status-connection-generation' +import { WORKTREE_SORT_ORDER_BATCH_WINDOW_MS } from './worktree-sort-order-publication-batch' import { createWorktreeEventRuntime } from './worktree-event-runtime' import { hostWorktreeTarget } from '../../../../shared/worktree/host-qualified-identity' import { @@ -73,6 +74,7 @@ afterEach(() => { unsubscribe() } vi.restoreAllMocks() + vi.useRealTimers() }) describe('remote worktree event lineage refresh', () => { @@ -138,10 +140,21 @@ describe('remote worktree event lineage refresh', () => { sortOrder: now - index * 1000 })) })) + let catalogCommits = 0 + unsubs.push( + store.subscribe((next, previous) => { + if (next.worktreesByRepo !== previous.worktreesByRepo) { + catalogCommits += 1 + } + }) + ) for (const event of events) { queue.enqueue({ ...event, executionHostId: 'runtime:env-1' }) } + await vi.waitFor(() => expect(catalogCommits).toBe(1)) await flushPromises() + // One commit for the whole burst, so global consumers render once rather than per repo. + expect(catalogCommits).toBe(1) expect(store.getState().fetchWorktrees).not.toHaveBeenCalled() expect(store.getState().fetchWorktreeLineage).not.toHaveBeenCalled() expect(store.getState().sortEpoch).toBe(3) @@ -369,12 +382,81 @@ describe('order-only refresh with actual store actions', () => { sortOrderOnly: true, sortOrders: [{ worktreeId: h.worktree.id, sortOrder: 99 }] }) + await vi.waitFor(() => expect(store.getState().worktreesByRepo.repo[0].sortOrder).toBe(99)) await flushPromises() - expect(store.getState().worktreesByRepo.repo[0].sortOrder).toBe(99) expect(store.getState().sortEpoch).toBe(beforeEpoch) expect(runtimeEnvironmentCall.mock.calls.length).toBe(readsBefore) }) + it.each([true, false])( + 'fences only the submitted repository listing (same repository: %s)', + async (sameRepo) => { + vi.useFakeTimers() + const h = createActualActionFixture() + await h.warm() + const fetch = vi.spyOn(store.getState(), 'fetchWorktrees') + const publishedOrders: number[] = [] + const unsubscribe = store.subscribe((state) => { + publishedOrders.push(state.worktreesByRepo.repo[0].sortOrder) + }) + unsubs.push(unsubscribe) + h.queue.enqueue({ + repoId: 'repo', + executionHostId: 'runtime:env-1', + sortOrderOnly: true, + sortOrders: [{ worktreeId: h.worktree.id, sortOrder: 99 }] + }) + h.setOrder(42) + if (sameRepo) { + await store.getState().fetchWorktrees('repo', { executionHostId: 'runtime:env-1' }) + expect(store.getState().worktreesByRepo.repo[0].sortOrder).toBe(42) + } else { + store.setState((state) => ({ + worktreesByRepo: { ...state.worktreesByRepo, unrelated: [] } + })) + } + fetch.mockClear() + await vi.advanceTimersByTimeAsync(WORKTREE_SORT_ORDER_BATCH_WINDOW_MS) + expect(store.getState().worktreesByRepo.repo[0].sortOrder).toBe(sameRepo ? 42 : 99) + expect(fetch).toHaveBeenCalledTimes(sameRepo ? 1 : 0) + if (sameRepo) { + expect(publishedOrders).not.toContain(99) + } + } + ) + + it('refetches instead of patching when the connection changes inside the batch window', async () => { + const h = createActualActionFixture() + await h.warm() + h.queue.enqueue({ + repoId: 'repo', + executionHostId: 'runtime:env-1', + sortOrderOnly: true, + sortOrders: [{ worktreeId: h.worktree.id, sortOrder: 99 }] + }) + advanceRuntimeEnvironmentConnectionGeneration('env-1') + h.setOrder(42) + // The refetch starts only after the flush rejected the stale values, so nothing patches later. + await vi.waitFor(() => expect(store.getState().worktreesByRepo.repo[0].sortOrder).toBe(42)) + }) + + it('runs a structural refresh queued behind a pending order patch after the patch', async () => { + const h = createActualActionFixture() + await h.warm() + h.queue.enqueue({ + repoId: 'repo', + executionHostId: 'runtime:env-1', + sortOrderOnly: true, + sortOrders: [{ worktreeId: h.worktree.id, sortOrder: 99 }] + }) + h.setOrder(7) + h.queue.enqueue({ repoId: 'repo', executionHostId: 'runtime:env-1' }) + await flushPromises() + expect(store.getState().worktreesByRepo.repo[0].sortOrder).not.toBe(7) + // The newer listing lands last, so the older published value cannot overwrite it. + await vi.waitFor(() => expect(store.getState().worktreesByRepo.repo[0].sortOrder).toBe(7)) + }) + it('refetches when a published row was never loaded here', async () => { const h = createActualActionFixture() await h.warm() @@ -443,8 +525,9 @@ describe('order-only refresh with actual store actions', () => { sortOrderOnly: true, sortOrders: [{ worktreeId: h.worktree.id, sortOrder: 99 }] }) - await flushPromises() - expect(store.getState().worktreesByRepo.repo.map((row) => row.sortOrder)).toEqual([99, 5]) + await vi.waitFor(() => + expect(store.getState().worktreesByRepo.repo.map((row) => row.sortOrder)).toEqual([99, 5]) + ) }) it('still bumps sortEpoch when a refetch changes more than sortOrder', async () => { diff --git a/src/renderer/src/hooks/ipc-events/worktree-event-runtime.ts b/src/renderer/src/hooks/ipc-events/worktree-event-runtime.ts index 302f052d199..e2f90cff55c 100644 --- a/src/renderer/src/hooks/ipc-events/worktree-event-runtime.ts +++ b/src/renderer/src/hooks/ipc-events/worktree-event-runtime.ts @@ -10,8 +10,9 @@ import { hostWorktreeTarget } from '../../../../shared/worktree/host-qualified-i import type { WorktreeSortOrderUpdate } from '../../../../shared/worktree/sort-order-update' import { hasHostRowsForSortOrders, - patchWorktreeSortOrders + patchWorktreeSortOrderBatch } from '../../store/slices/worktrees/listing/worktree-sort-order-patch' +import { createWorktreeSortOrderPublicationBatch } from './worktree-sort-order-publication-batch' import { buildRuntimeClientEventEnvironmentKey } from './runtime-environment-subscription-selection' import { useAppStore } from '../../store' import { @@ -68,6 +69,9 @@ export function createWorktreeEventRuntime( unsubs: (() => void)[], isRuntimeEnvironmentActive: () => boolean ): WorktreeEventRuntime { + const sortOrderBatch = createWorktreeSortOrderPublicationBatch((patches) => + useAppStore.setState((s) => patchWorktreeSortOrderBatch(s, patches)) + ) const handleWorktreesChanged = async ( repoId: string, renamed?: { oldWorktreeId: string; newWorktreeId: string }, @@ -96,13 +100,28 @@ export function createWorktreeEventRuntime( ) ) { // Why: the host published the saved values, so refetching the repo would only re-download them (#27268). - const { executionHostId, sortOrders } = options - useAppStore.setState((s) => patchWorktreeSortOrders(s, repoId, executionHostId, sortOrders)) - // Keep the existing retry for a host whose lineage snapshot is not fresh. - if (!isRemoteWorktreeLineageFresh(useAppStore.getState, sortOrderOwner.environmentId)) { - await useAppStore.getState().fetchWorktreeLineage({ executionHostId }) + const { executionHostId, sortOrders, sortOrderFence } = options + const submittedListing = selectWorktreesByRepo(useAppStore.getState())[repoId] + // Awaiting keeps this repo's later events (renames, structural refreshes) behind the patch. + const outcome = await sortOrderBatch.submit({ + repoId, + hostId: executionHostId, + sortOrders, + isCurrent: () => + sortOrderFence === runtimeSortOrderFence(sortOrderOwner.environmentId) && + selectWorktreesByRepo(useAppStore.getState())[repoId] === submittedListing && + hasHostRowsForSortOrders(useAppStore.getState(), repoId, executionHostId, sortOrders) + }) + if (outcome === 'disposed') { + return + } + if (outcome === 'applied') { + // Keep the existing retry for a host whose lineage snapshot is not fresh. + if (!isRemoteWorktreeLineageFresh(useAppStore.getState, sortOrderOwner.environmentId)) { + await useAppStore.getState().fetchWorktreeLineage({ executionHostId }) + } + return } - return } const localRefreshStartedWithRuntime = options?.forceLocalOwner === true && isRuntimeEnvironmentActive() @@ -211,6 +230,7 @@ export function createWorktreeEventRuntime( dispose() { disposed = true refreshQueue.dispose() + sortOrderBatch.dispose() }, enqueue(event) { if (disposed) { diff --git a/src/renderer/src/hooks/ipc-events/worktree-sort-order-publication-batch.ts b/src/renderer/src/hooks/ipc-events/worktree-sort-order-publication-batch.ts new file mode 100644 index 00000000000..f176fd22c75 --- /dev/null +++ b/src/renderer/src/hooks/ipc-events/worktree-sort-order-publication-batch.ts @@ -0,0 +1,75 @@ +import type { WorktreeSortOrderPatch } from '../../store/slices/worktrees/listing/worktree-sort-order-patch' + +// Hosts publish one order event per repo; one window turns a burst into one store commit. +export const WORKTREE_SORT_ORDER_BATCH_WINDOW_MS = 32 + +export type WorktreeSortOrderBatchOutcome = 'applied' | 'refetch' | 'disposed' + +type PendingPatch = WorktreeSortOrderPatch & { + isCurrent: () => boolean + resolve: (outcome: WorktreeSortOrderBatchOutcome) => void +} + +export type WorktreeSortOrderPublicationBatch = { + /** Resolves after the flush; 'refetch' means the values went stale and the caller must reread the repo. */ + submit: ( + patch: WorktreeSortOrderPatch & { isCurrent: () => boolean } + ) => Promise + dispose: () => void +} + +export function createWorktreeSortOrderPublicationBatch( + apply: (patches: readonly WorktreeSortOrderPatch[]) => void, + windowMs = WORKTREE_SORT_ORDER_BATCH_WINDOW_MS +): WorktreeSortOrderPublicationBatch { + let pending: PendingPatch[] = [] + let timer: ReturnType | null = null + let disposed = false + + const flush = (): void => { + timer = null + const batch = pending + pending = [] + // Re-check at flush: a reconnect or listing change during the window invalidates the published values. + const current = batch.filter((entry) => entry.isCurrent()) + try { + if (current.length > 0) { + apply(current) + } + } catch (error) { + console.error('Failed to apply worktree order updates:', error) + for (const entry of batch) { + entry.resolve('refetch') + } + return + } + for (const entry of batch) { + entry.resolve(current.includes(entry) ? 'applied' : 'refetch') + } + } + + return { + submit(patch) { + if (disposed) { + return Promise.resolve('disposed') + } + return new Promise((resolve) => { + pending.push({ ...patch, resolve }) + // Fixed from the first entry, so a steady stream cannot postpone the flush. + timer ??= setTimeout(flush, windowMs) + }) + }, + dispose() { + disposed = true + if (timer !== null) { + clearTimeout(timer) + timer = null + } + const batch = pending + pending = [] + for (const entry of batch) { + entry.resolve('disposed') + } + } + } +} diff --git a/src/renderer/src/store/slices/worktrees/listing/worktree-sort-order-patch.ts b/src/renderer/src/store/slices/worktrees/listing/worktree-sort-order-patch.ts index d886e9abdd4..38370f33912 100644 --- a/src/renderer/src/store/slices/worktrees/listing/worktree-sort-order-patch.ts +++ b/src/renderer/src/store/slices/worktrees/listing/worktree-sort-order-patch.ts @@ -2,6 +2,13 @@ import type { ExecutionHostId } from '../../../../../../shared/execution-host' import type { WorktreeSortOrderUpdate } from '../../../../../../shared/worktree/sort-order-update' import type { AppState } from '../../../types' import { worktreeHostMatchOptions, worktreeMatchesHost } from './worktree-host-ownership' +import { selectWorktreesByRepo } from '../../../workspace-record-selectors' + +export type WorktreeSortOrderPatch = { + repoId: string + hostId: ExecutionHostId + sortOrders: readonly WorktreeSortOrderUpdate[] +} /** Apply host order saves via sortOrderEpoch; sortEpoch would schedule another live Smart save (#27268). */ export function patchWorktreeSortOrders( @@ -10,32 +17,47 @@ export function patchWorktreeSortOrders( hostId: ExecutionHostId, sortOrders: readonly WorktreeSortOrderUpdate[] ): Pick | AppState { - const current = state.worktreesByRepo[repoId] - if (!current || sortOrders.length === 0) { - return state - } - const sortOrderById = new Map(sortOrders.map((entry) => [entry.worktreeId, entry.sortOrder])) - const matchOptions = worktreeHostMatchOptions(state, repoId, hostId) - let changed = false - const next = current.map((worktree) => { - const sortOrder = sortOrderById.get(worktree.id) - if ( - sortOrder === undefined || - !Number.isFinite(sortOrder) || - worktree.sortOrder === sortOrder || - !worktreeMatchesHost(worktree, hostId, matchOptions) - ) { - return worktree + return patchWorktreeSortOrderBatch(state, [{ repoId, hostId, sortOrders }]) +} + +/** One publication for many repo buckets; later patches win for a row patched twice. */ +export function patchWorktreeSortOrderBatch( + state: AppState, + patches: readonly WorktreeSortOrderPatch[] +): Pick | AppState { + const original = selectWorktreesByRepo(state) + let worktreesByRepo = original + for (const { repoId, hostId, sortOrders } of patches) { + const current = worktreesByRepo[repoId] + if (!current || sortOrders.length === 0) { + continue } - changed = true - return { ...worktree, sortOrder } - }) - return changed - ? { - worktreesByRepo: { ...state.worktreesByRepo, [repoId]: next }, - sortOrderEpoch: state.sortOrderEpoch + 1 + const sortOrderById = new Map(sortOrders.map((entry) => [entry.worktreeId, entry.sortOrder])) + const matchOptions = worktreeHostMatchOptions(state, repoId, hostId) + let changed = false + const next = current.map((worktree) => { + const sortOrder = sortOrderById.get(worktree.id) + if ( + sortOrder === undefined || + !Number.isFinite(sortOrder) || + worktree.sortOrder === sortOrder || + !worktreeMatchesHost(worktree, hostId, matchOptions) + ) { + return worktree } - : state + changed = true + return { ...worktree, sortOrder } + }) + if (changed) { + if (worktreesByRepo === original) { + worktreesByRepo = { ...original } + } + worktreesByRepo[repoId] = next + } + } + return worktreesByRepo === original + ? state + : { worktreesByRepo, sortOrderEpoch: state.sortOrderEpoch + 1 } } /** False when a published row is missing here, so the listing needs a refetch rather than a patch. */ @@ -47,7 +69,7 @@ export function hasHostRowsForSortOrders( ): boolean { const matchOptions = worktreeHostMatchOptions(state, repoId, hostId) const ids = new Set( - (state.worktreesByRepo[repoId] ?? []) + (selectWorktreesByRepo(state)[repoId] ?? []) .filter((worktree) => worktreeMatchesHost(worktree, hostId, matchOptions)) .map((worktree) => worktree.id) )