mirror of
https://github.com/stablyai/orca.git
synced 2026-10-07 08:02:21 +00:00
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 <previous sort>' 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.
This commit is contained in:
Jinjing
@@ -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<HTMLDivElement>(null)
|
||||
@@ -135,7 +134,6 @@ function WorkspaceKanbanDrawerContent({
|
||||
laneFullWorktreeIds,
|
||||
laneViews,
|
||||
maybeSyncTaskStatuses: maybeSyncWorkspaceBoardTaskStatuses,
|
||||
setSortBy,
|
||||
sortBy,
|
||||
updateWorktreeMeta,
|
||||
updateWorktreesMeta,
|
||||
|
||||
@@ -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]
|
||||
|
||||
@@ -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')
|
||||
})
|
||||
})
|
||||
@@ -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
|
||||
}
|
||||
)
|
||||
}
|
||||
@@ -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<string, readonly string[]>
|
||||
laneViews: ReadonlyMap<string, LaneView>
|
||||
maybeSyncTaskStatuses: (worktreeIds: readonly string[], status: WorkspaceStatus) => void
|
||||
setSortBy: ReturnType<typeof useAppStore.getState>['setSortBy']
|
||||
sortBy: ReturnType<typeof useAppStore.getState>['sortBy']
|
||||
updateWorktreeMeta: ReturnType<typeof useAppStore.getState>['updateWorktreeMeta']
|
||||
updateWorktreesMeta: ReturnType<typeof useAppStore.getState>['updateWorktreesMeta']
|
||||
@@ -145,7 +145,7 @@ export function useWorkspaceKanbanWorktreeActions(args: {
|
||||
return
|
||||
}
|
||||
if (writeManualOrder && order.changed) {
|
||||
args.setSortBy('manual')
|
||||
switchSortToManualAfterDrop()
|
||||
}
|
||||
recordInteraction()
|
||||
void args.updateWorktreesMeta(changed)
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
})
|
||||
@@ -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<string, WorktreeAttention>): void {
|
||||
let class1 = 0
|
||||
let class2 = 0
|
||||
@@ -52,52 +48,16 @@ function trackSmartClassDistribution(attention: ReadonlyMap<string, WorktreeAtte
|
||||
})
|
||||
}
|
||||
|
||||
// Why debounce: scores are time-decaying, so recomputing on every sortEpoch bump makes worktrees jump; settle to coalesce.
|
||||
// Structural changes (add/remove) bypass the debounce so a new worktree appears at its sorted position immediately.
|
||||
function useDebouncedSortEpoch(worktreeCount: number, sortBy: SortBy): number {
|
||||
const sortEpoch = useAppStore((s) => 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<string, Repo>
|
||||
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)
|
||||
|
||||
|
||||
@@ -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()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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": {
|
||||
|
||||
@@ -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<AppState, [], []>)
|
||||
export const useAppStore = create<AppState>()(
|
||||
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 {
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
})
|
||||
@@ -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<StoreApi<AppState>, 'setState' | 'subscribe'>
|
||||
): void {
|
||||
let timer: ReturnType<typeof setTimeout> | 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)
|
||||
}
|
||||
})
|
||||
}
|
||||
@@ -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
|
||||
|
||||
@@ -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<string>(),
|
||||
lastVisitedAtByWorktreeId: {},
|
||||
hasHydratedWorktreePurge: false,
|
||||
|
||||
Reference in New Issue
Block a user