From 4a41e246db6316e96115d90ebab89bdf34307d20 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Sun, 4 Oct 2026 15:55:52 -0700 Subject: [PATCH] Fix: settle sortEpoch in store to prevent React update depth exceeded (#25313) * fix(sidebar): stop Manual sort from mirroring sortEpoch into state In Manual mode the sort hook copied every sortEpoch bump into state from an effect, adding a nested React update per bump. A burst of store bumps at startup stacked those into 'Maximum update depth exceeded' (React #185). Manual now reads the live epoch directly; debounced modes are unchanged. * feat(sidebar): tell people when a drop switches sort to Manual Reordering by drag still switches the sidebar to Manual so the drop sticks, but it used to happen silently. Show a toast with a 'Back to ' action; it retires itself on any later sort change so it can't override a newer choice. Drops while already in Manual stay silent. * feat(store): settle sortEpoch in the store instead of in React state Add settledSortEpoch plus a 3 s settle timer owned by a store listener installed in the state creator. Every write path (slice actions, the web session sync patch) goes through it, so the settled value stays correct with no sidebar mounted. Manual, a sort-mode switch, and a bump that changes the non-archived row count settle in the same notify; other bumps restart the window. A reset that lands settled clears the timer, and the disposer runs on HMR teardown. * fix(sidebar): read the settled sort epoch instead of mirroring it into state The sort hook no longer copies sortEpoch into React state from an effect in any mode; it reads the store's settledSortEpoch directly. That pattern added a nested update per bump and stacked into "Maximum update depth exceeded" (React #185) under flushSync bursts. Tests cover Manual, add/remove, burst reset, no-bump row changes, mode switch, and 80 flushSync bumps in Recent while worktrees are added. * cleaning up * fix(store): settle bumps when rows update during pending window Detect structural changes on worktreesByRepo updates in addition to sort epoch changes. When a row arrives without its own bump during a pending settlement window, the changed composition must still trigger settlement. --- .../sidebar/WorkspaceKanbanDrawer.tsx | 2 - .../src/components/sidebar/WorktreeList.tsx | 2 +- .../sidebar/manual-sort-switch-toast.test.ts | 73 ++++ .../sidebar/manual-sort-switch-toast.ts | 59 ++++ .../use-workspace-kanban-worktree-actions.ts | 4 +- .../drag/use-status-mutations.ts | 14 +- .../listing/use-sort-order.test.tsx | 311 ++++++++++++++++++ .../worktree-list/listing/use-sort-order.ts | 52 +-- .../worktree-manual-order-store-write.test.ts | 25 ++ src/renderer/src/i18n/locales/en.json | 5 + src/renderer/src/store/index.ts | 3 + .../src/store/settled-sort-epoch.test.ts | 98 ++++++ src/renderer/src/store/settled-sort-epoch.ts | 64 ++++ .../src/store/slices/worktree-helpers.ts | 2 + .../session/worktree-slice-initial-state.ts | 2 + 15 files changed, 658 insertions(+), 58 deletions(-) create mode 100644 src/renderer/src/components/sidebar/manual-sort-switch-toast.test.ts create mode 100644 src/renderer/src/components/sidebar/manual-sort-switch-toast.ts create mode 100644 src/renderer/src/components/sidebar/worktree-list/listing/use-sort-order.test.tsx create mode 100644 src/renderer/src/store/settled-sort-epoch.test.ts create mode 100644 src/renderer/src/store/settled-sort-epoch.ts diff --git a/src/renderer/src/components/sidebar/WorkspaceKanbanDrawer.tsx b/src/renderer/src/components/sidebar/WorkspaceKanbanDrawer.tsx index a8ada9cc612..63e7880fa03 100644 --- a/src/renderer/src/components/sidebar/WorkspaceKanbanDrawer.tsx +++ b/src/renderer/src/components/sidebar/WorkspaceKanbanDrawer.tsx @@ -68,7 +68,6 @@ function WorkspaceKanbanDrawerContent({ const workspaceBoardColumnWidth = useAppStore((s) => s.workspaceBoardColumnWidth) const setWorkspaceBoardColumnWidth = useAppStore((s) => s.setWorkspaceBoardColumnWidth) const sortBy = useAppStore((s) => s.sortBy) - const setSortBy = useAppStore((s) => s.setSortBy) const sidebarOpen = useAppStore((s) => s.sidebarOpen) const sidebarWidth = useAppStore((s) => s.sidebarWidth) const boardRef = useRef(null) @@ -135,7 +134,6 @@ function WorkspaceKanbanDrawerContent({ laneFullWorktreeIds, laneViews, maybeSyncTaskStatuses: maybeSyncWorkspaceBoardTaskStatuses, - setSortBy, sortBy, updateWorktreeMeta, updateWorktreesMeta, diff --git a/src/renderer/src/components/sidebar/WorktreeList.tsx b/src/renderer/src/components/sidebar/WorktreeList.tsx index d50f06c0026..8fe26780866 100644 --- a/src/renderer/src/components/sidebar/WorktreeList.tsx +++ b/src/renderer/src/components/sidebar/WorktreeList.tsx @@ -108,7 +108,7 @@ const WorktreeList = React.memo(function WorktreeList({ const agentSendTargetWorktreeId = useAgentSendTargetWorktreeId() const { filterState, hasFilters, clearFilters, revealWorkspaceFilters } = useSidebarWorktreeFilters() - const sortedIds = useSidebarWorktreeSortOrder({ allWorktrees, repoMap, sortBy }) + const sortedIds = useSidebarWorktreeSortOrder({ repoMap, sortBy }) const manualOrderCatalog = useMemo( () => buildWorktreeManualOrderCatalog({ worktrees: allWorktrees, folderWorkspaces }), [allWorktrees, folderWorkspaces] diff --git a/src/renderer/src/components/sidebar/manual-sort-switch-toast.test.ts b/src/renderer/src/components/sidebar/manual-sort-switch-toast.test.ts new file mode 100644 index 00000000000..bafe2a6f7cb --- /dev/null +++ b/src/renderer/src/components/sidebar/manual-sort-switch-toast.test.ts @@ -0,0 +1,73 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { useAppStore } from '@/store' +import { switchSortToManualAfterDrop } from './manual-sort-switch-toast' + +const { toastInfo, toastDismiss } = vi.hoisted(() => ({ + toastInfo: vi.fn(), + toastDismiss: vi.fn() +})) +vi.mock('sonner', () => ({ toast: { info: toastInfo, dismiss: toastDismiss } })) + +const initialState = useAppStore.getInitialState() + +type ToastOptions = { + id?: string + onDismiss?: () => void + action?: { label: string; onClick: () => void } +} + +function lastToastOptions(): ToastOptions { + const options: unknown = toastInfo.mock.calls.at(-1)?.[1] + expect(options).toBeTypeOf('object') + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: asserted to be the options object passed to toast.info above. + return options as ToastOptions +} + +describe('switchSortToManualAfterDrop', () => { + beforeEach(() => { + useAppStore.setState(initialState, true) + toastInfo.mockClear() + toastDismiss.mockClear() + }) + + afterEach(() => { + if (toastInfo.mock.calls.length > 0) { + lastToastOptions().onDismiss?.() + } + useAppStore.setState(initialState, true) + }) + + it('switches to Manual and offers a way back to the previous sort', () => { + useAppStore.setState({ sortBy: 'smart' }) + switchSortToManualAfterDrop() + + expect(useAppStore.getState().sortBy).toBe('manual') + expect(toastInfo).toHaveBeenCalledTimes(1) + const options = lastToastOptions() + expect(options.action?.label).toBe('Back to Agent Activity') + + options.action?.onClick() + expect(useAppStore.getState().sortBy).toBe('smart') + }) + + it('does nothing when Manual is already active', () => { + useAppStore.setState({ sortBy: 'manual' }) + switchSortToManualAfterDrop() + + expect(toastInfo).not.toHaveBeenCalled() + }) + + it('retires the toast once the user picks another sort', () => { + useAppStore.setState({ sortBy: 'recent' }) + switchSortToManualAfterDrop() + const options = lastToastOptions() + + useAppStore.setState({ sortBy: 'name' }) + expect(toastDismiss).toHaveBeenCalledWith(options.id) + + // A round trip back to Manual must not let the stale action override it. + useAppStore.setState({ sortBy: 'manual' }) + options.action?.onClick() + expect(useAppStore.getState().sortBy).toBe('manual') + }) +}) diff --git a/src/renderer/src/components/sidebar/manual-sort-switch-toast.ts b/src/renderer/src/components/sidebar/manual-sort-switch-toast.ts new file mode 100644 index 00000000000..5018cf5b6b5 --- /dev/null +++ b/src/renderer/src/components/sidebar/manual-sort-switch-toast.ts @@ -0,0 +1,59 @@ +import { toast } from 'sonner' +import { translate } from '@/i18n/i18n' +import { useAppStore } from '@/store' +import { SORT_OPTIONS } from './sidebar-workspace-option-items' + +const MANUAL_SORT_SWITCH_TOAST_ID = 'sidebar-manual-sort-switch' + +// Why: a drop that reorders must switch to Manual so the placement sticks; say so, since it changes a shared setting. +export function switchSortToManualAfterDrop(): void { + const { sortBy: previousSortBy, setSortBy } = useAppStore.getState() + if (previousSortBy === 'manual') { + return + } + setSortBy('manual') + + // Why: any later sort change makes "Back to …" stale, so retire the toast instead of letting it clobber that choice. + let retired = false + const retire = (): void => { + retired = true + unsubscribe() + } + const unsubscribe = useAppStore.subscribe((state, prev) => { + if (state.sortBy !== prev.sortBy) { + retire() + toast.dismiss(MANUAL_SORT_SWITCH_TOAST_ID) + } + }) + const previousLabel = SORT_OPTIONS.find((option) => option.id === previousSortBy)?.label + toast.info( + translate('auto.components.sidebar.manualSortSwitch.title', 'Sort changed to Manual'), + { + id: MANUAL_SORT_SWITCH_TOAST_ID, + description: translate( + 'auto.components.sidebar.manualSortSwitch.description', + 'Manual keeps workspaces where you drop them.' + ), + duration: 8000, + onDismiss: retire, + onAutoClose: retire, + action: previousLabel + ? { + label: translate( + 'auto.components.sidebar.manualSortSwitch.restore', + 'Back to {{label}}', + { + label: previousLabel + } + ), + // The sortBy change retires the toast through the subscription above. + onClick: () => { + if (!retired) { + setSortBy(previousSortBy) + } + } + } + : undefined + } + ) +} diff --git a/src/renderer/src/components/sidebar/use-workspace-kanban-worktree-actions.ts b/src/renderer/src/components/sidebar/use-workspace-kanban-worktree-actions.ts index 96647d65763..b0efb012b8d 100644 --- a/src/renderer/src/components/sidebar/use-workspace-kanban-worktree-actions.ts +++ b/src/renderer/src/components/sidebar/use-workspace-kanban-worktree-actions.ts @@ -1,6 +1,7 @@ import { useCallback } from 'react' import { useAppStore } from '@/store' import { getWorkspaceStatus } from './workspace-status' +import { switchSortToManualAfterDrop } from './manual-sort-switch-toast' import { resolveFullLaneDropIndex } from './workspace-kanban-filtered-drop-index' import { buildManualOrderUpdatesForGroupDrop, @@ -19,7 +20,6 @@ export function useWorkspaceKanbanWorktreeActions(args: { laneFullWorktreeIds: ReadonlyMap laneViews: ReadonlyMap maybeSyncTaskStatuses: (worktreeIds: readonly string[], status: WorkspaceStatus) => void - setSortBy: ReturnType['setSortBy'] sortBy: ReturnType['sortBy'] updateWorktreeMeta: ReturnType['updateWorktreeMeta'] updateWorktreesMeta: ReturnType['updateWorktreesMeta'] @@ -145,7 +145,7 @@ export function useWorkspaceKanbanWorktreeActions(args: { return } if (writeManualOrder && order.changed) { - args.setSortBy('manual') + switchSortToManualAfterDrop() } recordInteraction() void args.updateWorktreesMeta(changed) diff --git a/src/renderer/src/components/sidebar/worktree-list/drag/use-status-mutations.ts b/src/renderer/src/components/sidebar/worktree-list/drag/use-status-mutations.ts index 6243f34cf02..249c2ef3b3a 100644 --- a/src/renderer/src/components/sidebar/worktree-list/drag/use-status-mutations.ts +++ b/src/renderer/src/components/sidebar/worktree-list/drag/use-status-mutations.ts @@ -16,6 +16,7 @@ import { } from '../../worktree-manual-order' import { buildWorkspaceKanbanSidebarDropUpdates } from '../../workspace-kanban-sidebar-drop' import type { SortBy } from '../../smart-sort' +import { switchSortToManualAfterDrop } from '../../manual-sort-switch-toast' import type { WorktreeStatusDropAtIndexArgs } from './drop-commit-context' import type { WorktreeManualOrderCatalog } from '../../worktree-manual-order-catalog' @@ -29,7 +30,6 @@ export function useWorktreeStatusMutations(args: { const { manualOrderCatalog, worktreeMap, workspaceStatuses, sortBy } = args const updateWorktreeMeta = useAppStore((s) => s.updateWorktreeMeta) const updateWorktreesMeta = useAppStore((s) => s.updateWorktreesMeta) - const setSortBy = useAppStore((s) => s.setSortBy) const setWorktreesPinnedAndReveal = useAppStore((s) => s.setWorktreesPinnedAndReveal) const moveWorktreeToStatus = useCallback( @@ -111,11 +111,11 @@ export function useWorktreeStatusMutations(args: { } // Why: the insertion line promises exact placement, so persist manual order on a cross-status drop. if (order.changed) { - setSortBy('manual') + switchSortToManualAfterDrop() } void updateWorktreesMeta([...updates.values()]) }, - [manualOrderCatalog, setSortBy, updateWorktreesMeta, worktreeMap, workspaceStatuses] + [manualOrderCatalog, updateWorktreesMeta, worktreeMap, workspaceStatuses] ) const pinWorktree = useCallback( @@ -146,7 +146,7 @@ export function useWorktreeStatusMutations(args: { allWorktreeIds: manualOrderCatalog.orderedIds }) if (result.changed) { - setSortBy('manual') + switchSortToManualAfterDrop() } void updateWorktreesMeta( [...result.updates].map(([worktreeId, updates]) => ({ @@ -156,7 +156,7 @@ export function useWorktreeStatusMutations(args: { })) ) }, - [manualOrderCatalog, setSortBy, updateWorktreesMeta, worktreeMap] + [manualOrderCatalog, updateWorktreesMeta, worktreeMap] ) const shouldShowWorkspaceBoardDropIndicator = useCallback( @@ -190,12 +190,12 @@ export function useWorktreeStatusMutations(args: { } // Why: switch to Manual when the drop changes order so the placement stays visible. if (result.shouldSwitchToManual) { - setSortBy('manual') + switchSortToManualAfterDrop() } useAppStore.getState().recordFeatureInteraction('workspace-board-actions') void updateWorktreesMeta(result.updates) }, - [manualOrderCatalog, setSortBy, sortBy, updateWorktreesMeta, worktreeMap, workspaceStatuses] + [manualOrderCatalog, sortBy, updateWorktreesMeta, worktreeMap, workspaceStatuses] ) return { diff --git a/src/renderer/src/components/sidebar/worktree-list/listing/use-sort-order.test.tsx b/src/renderer/src/components/sidebar/worktree-list/listing/use-sort-order.test.tsx new file mode 100644 index 00000000000..116cee843fd --- /dev/null +++ b/src/renderer/src/components/sidebar/worktree-list/listing/use-sort-order.test.tsx @@ -0,0 +1,311 @@ +// @vitest-environment happy-dom + +import { StrictMode, createElement } from 'react' +import { flushSync } from 'react-dom' +import { createRoot } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { act, cleanup, renderHook } from '@testing-library/react' +import { useAppStore } from '@/store' +import type { Worktree } from '../../../../../../shared/worktree/types' +import { makeRepo, makeWorktree } from '../../../worktree-jump-palette-test-fixtures' +import type { SortBy } from '../../smart-sort' +import { useSidebarWorktreeSortOrder } from './use-sort-order' + +const initialState = useAppStore.getInitialState() +const repo = makeRepo() +const repoMap = new Map([[repo.id, repo]]) + +// Why a settled bump: the store's add/remove baseline is the row count at the last sortEpoch change. +function seed(worktrees: Worktree[], sortBy: SortBy): void { + useAppStore.setState({ + sortBy, + sortEpoch: 1, + settledSortEpoch: 1, + worktreesByRepo: { [repo.id]: worktrees } + }) +} + +function bumpSortEpoch(): void { + useAppStore.setState((s) => ({ sortEpoch: s.sortEpoch + 1 })) +} + +function replaceWorktrees(worktrees: Worktree[]): void { + useAppStore.setState((s) => ({ + worktreesByRepo: { [repo.id]: worktrees }, + sortEpoch: s.sortEpoch + 1 + })) +} + +function renderSortOrder(options?: { strict?: boolean }) { + const renders = { count: 0 } + const hook = renderHook( + () => { + renders.count += 1 + const sortBy = useAppStore((s) => s.sortBy) + return useSidebarWorktreeSortOrder({ repoMap, sortBy }) + }, + { wrapper: options?.strict ? StrictMode : undefined } + ) + return { ...hook, renders } +} + +function renderSortOrderOutsideAct(sortBy: SortBy) { + const errors: unknown[] = [] + const latest: { ids: string[] } = { ids: [] } + const previousActEnvironment = globalThis.IS_REACT_ACT_ENVIRONMENT + globalThis.IS_REACT_ACT_ENVIRONMENT = false + const root = createRoot(document.createElement('div'), { + onUncaughtError: (error) => errors.push(error), + onCaughtError: (error) => errors.push(error) + }) + function Probe(): null { + latest.ids = useSidebarWorktreeSortOrder({ repoMap, sortBy }) + return null + } + flushSync(() => root.render(createElement(Probe))) + return { + errors, + latest, + unmount: () => { + root.unmount() + globalThis.IS_REACT_ACT_ENVIRONMENT = previousActEnvironment + } + } +} + +describe('useSidebarWorktreeSortOrder', () => { + beforeEach(() => { + useAppStore.setState(initialState, true) + }) + + afterEach(() => { + cleanup() + vi.useRealTimers() + useAppStore.setState(initialState, true) + }) + + it('renders once per sortEpoch bump in Manual (no hook-initiated re-render)', () => { + seed( + [makeWorktree('a', 'A', { manualOrder: 2 }), makeWorktree('b', 'B', { manualOrder: 1 })], + 'manual' + ) + const { renders } = renderSortOrder() + const before = renders.count + for (let i = 0; i < 10; i++) { + act(() => bumpSortEpoch()) + } + expect(renders.count - before).toBe(10) + }) + + it('applies a Manual reorder in the same commit as the bump', () => { + seed( + [makeWorktree('a', 'A', { manualOrder: 2 }), makeWorktree('b', 'B', { manualOrder: 1 })], + 'manual' + ) + const { result } = renderSortOrder({ strict: true }) + expect(result.current).toEqual(['a', 'b']) + act(() => + replaceWorktrees([ + makeWorktree('a', 'A', { manualOrder: 2 }), + makeWorktree('b', 'B', { manualOrder: 3 }) + ]) + ) + expect(result.current).toEqual(['b', 'a']) + }) + + it('survives a burst of synchronous store bumps in Manual', () => { + seed( + [makeWorktree('a', 'A', { manualOrder: 2 }), makeWorktree('b', 'B', { manualOrder: 1 })], + 'manual' + ) + const view = renderSortOrderOutsideAct('manual') + try { + for (let i = 0; i < 80; i++) { + flushSync(() => bumpSortEpoch()) + } + expect(view.errors).toEqual([]) + } finally { + view.unmount() + } + }) + + it('survives 80 synchronous bumps in Recent while worktrees are added, then settles in order', () => { + vi.useFakeTimers() + let worktrees = [makeWorktree('w0', 'W0', { lastActivityAt: 0 })] + seed(worktrees, 'recent') + const view = renderSortOrderOutsideAct('recent') + try { + for (let i = 1; i <= 80; i++) { + if (i % 4 === 1) { + // Why newest activity: a structural add must land at its sorted position (first) immediately. + worktrees = [...worktrees, makeWorktree(`w${i}`, `W${i}`, { lastActivityAt: i * 100 })] + flushSync(() => replaceWorktrees(worktrees)) + expect(view.latest.ids[0]).toBe(`w${i}`) + } else { + // Why reversed activity: these bumps reorder rows, so only the settle may apply them. + worktrees = worktrees.map((w, index) => ({ ...w, lastActivityAt: i * 100 - index })) + flushSync(() => replaceWorktrees(worktrees)) + flushSync(() => bumpSortEpoch()) + } + } + expect(view.latest.ids[0]).toBe('w77') + expect(view.errors).toEqual([]) + flushSync(() => vi.advanceTimersByTime(3_000)) + expect(view.errors).toEqual([]) + const expected = [...useAppStore.getState().worktreesByRepo[repo.id]] + .sort((a, b) => b.lastActivityAt - a.lastActivityAt) + .map((w) => w.id) + expect(view.latest.ids).toEqual(expected) + expect(view.latest.ids[0]).toBe('w0') + expect(vi.getTimerCount()).toBe(0) + } finally { + view.unmount() + } + }) + + it('debounces Recent re-sorts until the settle window passes', () => { + vi.useFakeTimers() + seed( + [ + makeWorktree('a', 'A', { lastActivityAt: 2_000 }), + makeWorktree('b', 'B', { lastActivityAt: 1_000 }) + ], + 'recent' + ) + const { result } = renderSortOrder() + expect(result.current).toEqual(['a', 'b']) + act(() => + replaceWorktrees([ + makeWorktree('a', 'A', { lastActivityAt: 2_000 }), + makeWorktree('b', 'B', { lastActivityAt: 3_000 }) + ]) + ) + expect(result.current).toEqual(['a', 'b']) + act(() => vi.advanceTimersByTime(3_000)) + expect(result.current).toEqual(['b', 'a']) + }) + + it('applies an added worktree immediately in debounced modes', () => { + vi.useFakeTimers() + seed([makeWorktree('a', 'A')], 'name') + const { result } = renderSortOrder() + act(() => replaceWorktrees([makeWorktree('a', 'A'), makeWorktree('0', '0 first')])) + expect(result.current).toEqual(['0', 'a']) + }) + + it('re-sorts on Manual -> Recent and stays put at settle', () => { + vi.useFakeTimers() + seed( + [ + makeWorktree('a', 'A', { manualOrder: 2, lastActivityAt: 1_000 }), + makeWorktree('b', 'B', { manualOrder: 1, lastActivityAt: 2_000 }) + ], + 'manual' + ) + const { result } = renderSortOrder() + for (let i = 0; i < 5; i++) { + act(() => bumpSortEpoch()) + } + expect(result.current).toEqual(['a', 'b']) + act(() => useAppStore.getState().setSortBy('recent')) + expect(result.current).toEqual(['b', 'a']) + const afterSwitch = result.current + act(() => vi.advanceTimersByTime(3_000)) + expect(result.current).toBe(afterSwitch) + }) + + it('restarts the settle window on every bump in a burst', () => { + vi.useFakeTimers() + seed( + [ + makeWorktree('a', 'A', { lastActivityAt: 2_000 }), + makeWorktree('b', 'B', { lastActivityAt: 1_000 }) + ], + 'recent' + ) + const { result } = renderSortOrder() + act(() => + replaceWorktrees([ + makeWorktree('a', 'A', { lastActivityAt: 2_000 }), + makeWorktree('b', 'B', { lastActivityAt: 3_000 }) + ]) + ) + act(() => vi.advanceTimersByTime(2_000)) + act(() => bumpSortEpoch()) + act(() => vi.advanceTimersByTime(2_000)) + expect(result.current).toEqual(['a', 'b']) + act(() => vi.advanceTimersByTime(1_000)) + expect(result.current).toEqual(['b', 'a']) + }) + + it('applies a removed worktree immediately in debounced modes', () => { + vi.useFakeTimers() + seed( + [ + makeWorktree('a', 'A', { lastActivityAt: 3_000 }), + makeWorktree('b', 'B', { lastActivityAt: 2_000 }), + makeWorktree('c', 'C', { lastActivityAt: 1_000 }) + ], + 'recent' + ) + const { result } = renderSortOrder() + act(() => + replaceWorktrees([ + makeWorktree('b', 'B', { lastActivityAt: 2_000 }), + makeWorktree('c', 'C', { lastActivityAt: 4_000 }) + ]) + ) + expect(result.current).toEqual(['c', 'b']) + }) + + it('does not re-sort when rows change without a sortEpoch bump', () => { + vi.useFakeTimers() + seed( + [ + makeWorktree('a', 'A', { lastActivityAt: 2_000 }), + makeWorktree('b', 'B', { lastActivityAt: 1_000 }) + ], + 'recent' + ) + const { result } = renderSortOrder() + const before = result.current + // Same shape as a stale-host purge that drops/changes rows without bumping sortEpoch. + act(() => + useAppStore.setState({ + worktreesByRepo: { + [repo.id]: [ + makeWorktree('a', 'A', { lastActivityAt: 2_000 }), + makeWorktree('b', 'B', { lastActivityAt: 3_000 }), + makeWorktree('c', 'C', { lastActivityAt: 4_000 }) + ] + } + }) + ) + act(() => vi.advanceTimersByTime(3_000)) + expect(result.current).toBe(before) + expect(vi.getTimerCount()).toBe(0) + }) + + it('switching sort mode cancels a pending settle and re-sorts right away', () => { + vi.useFakeTimers() + seed( + [ + makeWorktree('a', 'Zeta', { lastActivityAt: 2_000 }), + makeWorktree('b', 'Alpha', { lastActivityAt: 1_000 }) + ], + 'recent' + ) + const { result } = renderSortOrder() + act(() => + replaceWorktrees([ + makeWorktree('a', 'Zeta', { lastActivityAt: 2_000 }), + makeWorktree('b', 'Alpha', { lastActivityAt: 3_000 }) + ]) + ) + expect(vi.getTimerCount()).toBe(1) + act(() => useAppStore.getState().setSortBy('name')) + expect(result.current).toEqual(['b', 'a']) + expect(useAppStore.getState().settledSortEpoch).toBe(useAppStore.getState().sortEpoch) + expect(vi.getTimerCount()).toBe(0) + }) +}) diff --git a/src/renderer/src/components/sidebar/worktree-list/listing/use-sort-order.ts b/src/renderer/src/components/sidebar/worktree-list/listing/use-sort-order.ts index 815e66da55d..78d5fce41a3 100644 --- a/src/renderer/src/components/sidebar/worktree-list/listing/use-sort-order.ts +++ b/src/renderer/src/components/sidebar/worktree-list/listing/use-sort-order.ts @@ -1,11 +1,10 @@ -import { useEffect, useMemo, useRef, useState } from 'react' +import { useEffect, useMemo, useRef } from 'react' import { useAppStore } from '@/store' import { getAllWorktreesFromState } from '@/store/selectors' import { track } from '@/lib/telemetry' import { tabHasLivePty } from '@/lib/tab-has-live-pty' import { persistWorktreeSortOrderByHost } from '@/lib/worktree-sort-order-persistence' import type { Repo } from '../../../../../../shared/repo-types' -import type { Worktree } from '../../../../../../shared/worktree/types' import { buildWorktreeComparator, buildWorktreeSortLabels, @@ -20,9 +19,6 @@ import { } from '../../smart-attention' import { useReusedArrayIdentity } from './use-reused-array-identity' -// Debounce re-sort after a sortEpoch bump so background score changes don't jar row positions. -const SORT_SETTLE_MS = 3_000 - function trackSmartClassDistribution(attention: ReadonlyMap): void { let class1 = 0 let class2 = 0 @@ -52,52 +48,16 @@ function trackSmartClassDistribution(attention: ReadonlyMap s.sortEpoch) - const [debouncedSortEpoch, setDebouncedSortEpoch] = useState(sortEpoch) - const prevWorktreeCountRef = useRef(worktreeCount) - useEffect(() => { - if (debouncedSortEpoch === sortEpoch) { - return - } - - const structuralChange = worktreeCount !== prevWorktreeCountRef.current - prevWorktreeCountRef.current = worktreeCount - - // Why: manual drag/drop is direct manipulation; the settle-window delay would make a successful drop look broken. - if (structuralChange || sortBy === 'manual') { - setDebouncedSortEpoch(sortEpoch) - return - } - - const timer = setTimeout(() => setDebouncedSortEpoch(sortEpoch), SORT_SETTLE_MS) - return () => clearTimeout(timer) - }, [sortEpoch, debouncedSortEpoch, worktreeCount, sortBy]) - return debouncedSortEpoch -} - // ── Stable sort order ────────────────────────────────────────── // Why sortEpoch (not selection): selection side-effects (clearing isUnread, PR-cache refresh) must not reorder the sidebar under the user. // Why useMemo not useEffect: order must be computed synchronously before the worktrees memo reads it. export function useSidebarWorktreeSortOrder(args: { - allWorktrees: readonly Worktree[] repoMap: Map sortBy: SortBy }): string[] { - const { allWorktrees, repoMap, sortBy } = args - // Non-archived count — detects structural changes (add/remove) so the debounce below can apply immediately. - const worktreeCount = useMemo(() => { - let count = 0 - for (const worktree of allWorktrees) { - if (!worktree.isArchived) { - count++ - } - } - return count - }, [allWorktrees]) - const debouncedSortEpoch = useDebouncedSortEpoch(worktreeCount, sortBy) + const { repoMap, sortBy } = args + // Why settled (not live): the store coalesces bump bursts so rows don't jump (store/settled-sort-epoch.ts). + const settledSortEpoch = useAppStore((s) => s.settledSortEpoch) // Why a latching ref: a live signal makes Smart authoritative for the session, even after that activity ends. const sessionHasHadLiveSmartSignal = useRef(false) @@ -157,9 +117,9 @@ export function useSidebarWorktreeSortOrder(args: { attentionByWorktree: sortBy === 'smart' ? attentionByWorktree : null, detectedLiveSmartSignal } - // debouncedSortEpoch is an intentional trigger not read in the memo; its change (debounced) signals a recompute. + // settledSortEpoch is an intentional trigger not read in the memo; its change signals a recompute. // oxlint-disable-next-line react-hooks/exhaustive-deps - }, [debouncedSortEpoch, repoMap, sortBy]) + }, [settledSortEpoch, repoMap, sortBy]) // Why: stable ID order prevents rank-only refreshes from echoing an unchanged snapshot. const sortedIds = useReusedArrayIdentity(recomputedSort.sortedIds) diff --git a/src/renderer/src/components/sidebar/worktree-manual-order-store-write.test.ts b/src/renderer/src/components/sidebar/worktree-manual-order-store-write.test.ts index 6789c95df1b..97ff803fd0f 100644 --- a/src/renderer/src/components/sidebar/worktree-manual-order-store-write.test.ts +++ b/src/renderer/src/components/sidebar/worktree-manual-order-store-write.test.ts @@ -8,6 +8,9 @@ import { makeWorktree } from '../worktree-jump-palette-test-fixtures' import { buildWorktreeManualOrderCatalog } from './worktree-manual-order-catalog' import { useWorktreeStatusMutations } from './worktree-list/drag/use-status-mutations' +const toastInfo = vi.hoisted(() => vi.fn()) +vi.mock('sonner', () => ({ toast: { info: toastInfo, dismiss: vi.fn() } })) + /** * The sidebar drop only reorders if the payload `reorderWorktrees` builds is the * one `updateWorktreesMeta` consumes. The e2e that covered this replaced the store @@ -61,6 +64,7 @@ describe('sidebar manual-order drop', () => { beforeEach(() => { useAppStore.setState(initialState, true) updateMeta.mockClear() + toastInfo.mockClear() Object.assign(window, { api: { worktrees: { updateMeta } } }) }) @@ -85,6 +89,7 @@ describe('sidebar manual-order drop', () => { expect(manualOrderedIds()).toEqual([ids[1], ids[0], ids[2]]) expect(useAppStore.getState().sortBy).toBe('manual') + expect(toastInfo).toHaveBeenCalledTimes(1) expect(updateMeta).toHaveBeenCalledWith({ worktreeId: ids[0], executionHostId: 'local', @@ -111,5 +116,25 @@ describe('sidebar manual-order drop', () => { expect(useAppStore.getState().sortBy).toBe('smart') expect(useAppStore.getState().sortEpoch).toBe(epochBeforeDrop) expect(updateMeta).not.toHaveBeenCalled() + expect(toastInfo).not.toHaveBeenCalled() + }) + + it('reorders silently when Manual is already the sort', async () => { + const worktrees = seedManualOrderedRows(3) + useAppStore.setState({ sortBy: 'manual' }) + const ids = worktrees.map((worktree) => worktree.id) + const reorder = renderReorder(worktrees) + + await act(async () => { + reorder.current.reorderWorktrees({ + groups: [{ key: GROUP_KEY, worktreeIds: ids }], + sourceGroupKey: GROUP_KEY, + draggedIds: [ids[0]!], + dropIndex: 2 + }) + }) + + expect(manualOrderedIds()).toEqual([ids[1], ids[0], ids[2]]) + expect(toastInfo).not.toHaveBeenCalled() }) }) diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index cb7b6f4607f..35201a26a70 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -6507,6 +6507,11 @@ "copied": "Command copied", "retry": "Retry now", "restored": "Git is working again. Worktrees refreshed." + }, + "manualSortSwitch": { + "title": "Sort changed to Manual", + "description": "Manual keeps workspaces where you drop them.", + "restore": "Back to {{label}}" } }, "shared": { diff --git a/src/renderer/src/store/index.ts b/src/renderer/src/store/index.ts index e2527d265bc..307e5ee67ab 100644 --- a/src/renderer/src/store/index.ts +++ b/src/renderer/src/store/index.ts @@ -54,6 +54,7 @@ import { registerWorkspaceHttpLinkBrowserOpener } from '@/lib/http-link-routing' import { installStoreListenerCensus } from './store-listener-census' +import { installSettledSortEpoch } from './settled-sort-epoch' import { withReactCommitCascadeWriteProbe } from './react-commit-cascade-write-probe' import { withStoreIdentityChurnProbe } from './store-identity-churn-probe' import { @@ -73,6 +74,8 @@ const withDevelopmentStoreProbes = (createState: StateCreator) export const useAppStore = create()( withDevelopmentStoreProbes( withReactCommitCascadeWriteProbe((...a) => { + // Why first: settling inside the bump's own notify means hook subscribers never see it unsettled. + installSettledSortEpoch(a[2]) // Why: the inner api is only reachable here, before create() copies subscribe onto the hook. installStoreListenerCensus(a[2]) return { diff --git a/src/renderer/src/store/settled-sort-epoch.test.ts b/src/renderer/src/store/settled-sort-epoch.test.ts new file mode 100644 index 00000000000..a3a9cbe812e --- /dev/null +++ b/src/renderer/src/store/settled-sort-epoch.test.ts @@ -0,0 +1,98 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { useAppStore } from './index' +import { SORT_SETTLE_MS } from './settled-sort-epoch' +import { applyWebSessionTabsStorePatch } from '@/runtime/web-session-tabs-sync/store-patch' +import { makeWorktree } from '@/components/worktree-jump-palette-test-fixtures' + +const initialState = useAppStore.getInitialState() + +function bump(): void { + useAppStore.setState((s) => ({ sortEpoch: s.sortEpoch + 1 })) +} + +function settled(): boolean { + const { sortEpoch, settledSortEpoch } = useAppStore.getState() + return settledSortEpoch === sortEpoch +} + +describe('settledSortEpoch', () => { + beforeEach(() => { + vi.useFakeTimers() + useAppStore.setState(initialState, true) + }) + + afterEach(() => { + useAppStore.setState(initialState, true) + vi.useRealTimers() + }) + + it('settles a bump only after the settle window, with no component mounted', () => { + useAppStore.setState({ sortBy: 'smart' }) + bump() + expect(settled()).toBe(false) + vi.advanceTimersByTime(SORT_SETTLE_MS - 1) + expect(settled()).toBe(false) + vi.advanceTimersByTime(1) + expect(settled()).toBe(true) + expect(vi.getTimerCount()).toBe(0) + }) + + it('settles Manual bumps in the same write', () => { + useAppStore.setState({ sortBy: 'manual' }) + bump() + expect(settled()).toBe(true) + expect(vi.getTimerCount()).toBe(0) + }) + + it('settles a web session sync patch that bumps sortEpoch', () => { + useAppStore.setState({ sortBy: 'recent' }) + applyWebSessionTabsStorePatch( + (s) => ({ agentStatusEpoch: s.agentStatusEpoch + 1, sortEpoch: s.sortEpoch + 1 }), + { frames: [] } + ) + expect(settled()).toBe(false) + vi.advanceTimersByTime(SORT_SETTLE_MS) + expect(settled()).toBe(true) + }) + + it('treats an add that skipped its bump as structural on the next bump', () => { + useAppStore.setState({ sortBy: 'recent' }) + useAppStore.setState({ worktreesByRepo: { 'repo-1': [makeWorktree('a', 'A')] } }) + expect(vi.getTimerCount()).toBe(0) + bump() + expect(settled()).toBe(true) + }) + + it('settles a pending bump when a row arrives without its own bump', () => { + useAppStore.setState({ sortBy: 'recent' }) + bump() + expect(settled()).toBe(false) + useAppStore.setState({ worktreesByRepo: { 'repo-1': [makeWorktree('a', 'A')] } }) + expect(settled()).toBe(true) + expect(vi.getTimerCount()).toBe(0) + }) + + it('ignores archived rows when detecting adds and removes', () => { + useAppStore.setState((s) => ({ + sortBy: 'recent', + worktreesByRepo: { 'repo-1': [makeWorktree('a', 'A')] }, + sortEpoch: s.sortEpoch + 1 + })) + expect(settled()).toBe(true) + useAppStore.setState((s) => ({ + worktreesByRepo: { + 'repo-1': [makeWorktree('a', 'A'), makeWorktree('b', 'B', { isArchived: true })] + }, + sortEpoch: s.sortEpoch + 1 + })) + expect(settled()).toBe(false) + }) + + it('clears a pending settle when the store is reset', () => { + useAppStore.setState({ sortBy: 'recent' }) + bump() + expect(vi.getTimerCount()).toBe(1) + useAppStore.setState(initialState, true) + expect(vi.getTimerCount()).toBe(0) + }) +}) diff --git a/src/renderer/src/store/settled-sort-epoch.ts b/src/renderer/src/store/settled-sort-epoch.ts new file mode 100644 index 00000000000..0e505aeae21 --- /dev/null +++ b/src/renderer/src/store/settled-sort-epoch.ts @@ -0,0 +1,64 @@ +/** + * Maintains `settledSortEpoch`: the sortEpoch the sidebar sort actually reads. + * + * Why in the store: the sidebar used to mirror sortEpoch into React state from an + * effect, adding a nested update per bump; bursts of flushSync bumps stacked those + * into "Maximum update depth exceeded" (React #185). A store listener sees every + * write path — slice actions and runtime patches such as the web session sync — so + * the settled value stays correct without any component mounted. + */ +import type { StoreApi } from 'zustand' +import type { AppState } from './types' +import { getIndexedAllWorktrees } from './worktree-repo-index' + +// Why: time-decaying scores would make rows jump on every bump; coalesce a burst into one re-sort. +export const SORT_SETTLE_MS = 3_000 + +function countLiveWorktrees(worktreesByRepo: AppState['worktreesByRepo']): number { + let count = 0 + for (const worktree of getIndexedAllWorktrees(worktreesByRepo)) { + if (!worktree.isArchived) { + count++ + } + } + return count +} + +/** Call once from inside the store's state creator, passing its `api`. */ +export function installSettledSortEpoch( + api: Pick, 'setState' | 'subscribe'> +): void { + let timer: ReturnType | undefined + // Why a baseline from the last bump (not the previous write): a row change that skipped + // its bump (stale-host purge) must not re-sort on its own. + let liveWorktreeCountAtLastBump = 0 + + const settle = (): void => api.setState((s) => ({ settledSortEpoch: s.sortEpoch })) + + api.subscribe((state, previous) => { + const epochChanged = state.sortEpoch !== previous.sortEpoch + let structuralChange = false + // Why also on row writes: a bump-less add during a pending window must still settle now. + if (epochChanged || state.worktreesByRepo !== previous.worktreesByRepo) { + const count = countLiveWorktrees(state.worktreesByRepo) + structuralChange = count !== liveWorktreeCountAtLastBump + if (epochChanged) { + liveWorktreeCountAtLastBump = count + } + } + if (state.settledSortEpoch === state.sortEpoch) { + // Why: any write that lands settled — settle() itself or a store reset — retires the pending timer. + clearTimeout(timer) + return + } + // Why: adds/removes, Manual (direct manipulation), and a mode switch never wait out the window. + if (structuralChange || state.sortBy === 'manual' || state.sortBy !== previous.sortBy) { + settle() + return + } + if (epochChanged) { + clearTimeout(timer) + timer = setTimeout(settle, SORT_SETTLE_MS) + } + }) +} diff --git a/src/renderer/src/store/slices/worktree-helpers.ts b/src/renderer/src/store/slices/worktree-helpers.ts index 23edda5d982..3d24fe69cd6 100644 --- a/src/renderer/src/store/slices/worktree-helpers.ts +++ b/src/renderer/src/store/slices/worktree-helpers.ts @@ -132,6 +132,8 @@ export type WorktreeSlice = { * — NOT by selection-triggered side-effects like clearing `isUnread`. */ sortEpoch: number + /** sortEpoch after the settle window; the sidebar sort reads this (see store/settled-sort-epoch.ts). */ + settledSortEpoch: number /** * Worktree IDs that have been activated at least once during this app * session. The first activation of a worktree is special: its diff --git a/src/renderer/src/store/slices/worktrees/session/worktree-slice-initial-state.ts b/src/renderer/src/store/slices/worktrees/session/worktree-slice-initial-state.ts index 77e90b1a12e..c1683af84c3 100644 --- a/src/renderer/src/store/slices/worktrees/session/worktree-slice-initial-state.ts +++ b/src/renderer/src/store/slices/worktrees/session/worktree-slice-initial-state.ts @@ -17,6 +17,7 @@ export const worktreeSliceInitialState: Pick< | 'baseStatusByWorktreeId' | 'remoteBranchConflictByWorktreeId' | 'sortEpoch' + | 'settledSortEpoch' | 'everActivatedWorktreeIds' | 'lastVisitedAtByWorktreeId' | 'hasHydratedWorktreePurge' @@ -37,6 +38,7 @@ export const worktreeSliceInitialState: Pick< baseStatusByWorktreeId: {}, remoteBranchConflictByWorktreeId: {}, sortEpoch: 0, + settledSortEpoch: 0, everActivatedWorktreeIds: new Set(), lastVisitedAtByWorktreeId: {}, hasHydratedWorktreePurge: false,