From d12a4b1c5c258326037fcc73f72580c09403fc08 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Thu, 29 Oct 2026 11:47:48 -0700 Subject: [PATCH] perf(sidebar): skip closed-menu catalog work and batch remote order publications (#36002) * perf(sidebar): keep closed workspace menus and host dialogs off catalog churn Closed row context menus subscribed to the full worktree list, worktree map and repo map, and ran the delete-lineage traversal whenever any worktree row changed. At 1,618 workspaces every rank-only update re-rendered each mounted menu and rebuilt the lineage projection per menu. - Menus now select stable empties for the catalog while closed and gate the lineage/sleep/delete subtree calculation on menuOpen; opening re-renders with the live host-qualified rows. - Per-host HostRemoveDialog no longer resolves SSH removal scope while closed, retaining the last open snapshot through the exit animation. Measured in happy-dom with 40 mounted closed menus over 1,618 rows: nine rank bursts went from ~52 ms and 360 lineage traversals to ~2.6 ms and 0. * perf(worktrees): batch remote order publications across repos into one commit Hosts publish one sortOrderOnly worktreesChanged event per changed repo, and each fast-path patch was its own store commit, so a multi-project reorder produced back-to-back global renders (nine ~95 ms tasks in the 10-28 trace). Published order patches now join a fixed 32 ms window and apply in a single patchWorktreeSortOrderBatch transaction (one worktreesByRepo copy, one sortOrderEpoch bump). Each repo handler awaits its flush, so renames and structural refreshes queued for the same repo still run after the patch. At flush the connection fence and host-row presence are re-checked; stale entries fall back to the existing refetch path. Same-id host twins keep the existing host matching. * perf(sidebar): skip catalog index builds for closed menus Check menuOpen before calling the catalog getters so a closed menu never rebuilds the shared repo/worktree indexes on catalog changes. * Prevent stale host dialog subscriptions and sort-order patches - Unmount dialog content after exit while preserving pending removal state - Fence batched sort updates to the submitted repository listing * Disable host removal while its dialog closes --- config/flaky-test-pattern-baseline.txt | 1 - config/workspace-record-read-baseline.txt | 1 - .../sidebar/HostRemoveDialog.test.tsx | 156 ++++++++++++++ .../components/sidebar/HostRemoveDialog.tsx | 200 +++++++++--------- ...ktreeContextMenu.closed-menu-work.test.tsx | 165 +++++++++++++++ ...reeContextMenu.copy-worktree-name.test.tsx | 18 +- ...rktreeContextMenu.delete-shortcut.test.tsx | 18 +- ...WorktreeContextMenu.host-statuses.test.tsx | 18 +- ...reeContextMenu.react185-bystander.test.tsx | 18 +- .../WorktreeDeveloperMenuReveal.test.tsx | 16 +- .../use-menu-scoped-worktree-catalog.ts | 20 ++ .../use-worktree-context-menu-model.tsx | 10 +- .../sidebar/worktree-context-menu-policy.ts | 3 + .../sidebar/worktree-parent-candidates.ts | 6 +- .../sidebar/worktree-parent-eligibility.ts | 2 +- .../worktree-event-lineage-refresh.test.ts | 89 +++++++- .../ipc-events/worktree-event-runtime.ts | 34 ++- .../worktree-sort-order-publication-batch.ts | 75 +++++++ .../listing/worktree-sort-order-patch.ts | 72 ++++--- 19 files changed, 741 insertions(+), 181 deletions(-) create mode 100644 src/renderer/src/components/sidebar/HostRemoveDialog.test.tsx create mode 100644 src/renderer/src/components/sidebar/WorktreeContextMenu.closed-menu-work.test.tsx create mode 100644 src/renderer/src/components/sidebar/use-menu-scoped-worktree-catalog.ts create mode 100644 src/renderer/src/hooks/ipc-events/worktree-sort-order-publication-batch.ts 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) )