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,