fix(sidebar): cold-start Smart and Manual-fallback rows follow another client's order save (#29486)

A sortOrder-only change now bumps a separate sortOrderEpoch. The sidebar sort
recomputes on it only while the active mode ranks by persisted sortOrder (Smart
before a live signal, or Manual), neither of which saves the order, so live
Smart still cannot loop on its own saves.

Fixes #29336
This commit is contained in:
Neil
2026-10-15 23:29:43 -07:00
committed by GitHub
parent 6936409b43
commit 4494cd80e0
6 changed files with 72 additions and 5 deletions
@@ -10,6 +10,7 @@ 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'
import { makeTerminalTab } from '../../../../store/slices/worktrees-slice-test-fixtures'
const initialState = useAppStore.getInitialState()
const repo = makeRepo()
@@ -36,6 +37,19 @@ function replaceWorktrees(worktrees: Worktree[]): void {
}))
}
// Same shape as another client's order save landing here: sortOrder changes, sortEpoch does not.
function publishSortOrders(sortOrderById: Record<string, number>): void {
useAppStore.setState((s) => ({
worktreesByRepo: {
[repo.id]: (s.worktreesByRepo[repo.id] ?? []).map((worktree) => ({
...worktree,
sortOrder: sortOrderById[worktree.id] ?? worktree.sortOrder
}))
},
sortOrderEpoch: s.sortOrderEpoch + 1
}))
}
function renderSortOrder(options?: { strict?: boolean }) {
const renders = { count: 0 }
const hook = renderHook(
@@ -308,4 +322,42 @@ describe('useSidebarWorktreeSortOrder', () => {
expect(useAppStore.getState().settledSortEpoch).toBe(useAppStore.getState().sortEpoch)
expect(vi.getTimerCount()).toBe(0)
})
it.each(['smart', 'manual'] as const)(
'follows another client order save while %s ranks by persisted sortOrder',
(sortBy) => {
seed(
[makeWorktree('a', 'A', { sortOrder: 2 }), makeWorktree('b', 'B', { sortOrder: 1 })],
sortBy
)
const { result } = renderSortOrder()
expect(result.current).toEqual(['a', 'b'])
act(() => publishSortOrders({ a: 1, b: 2 }))
expect(result.current).toEqual(['b', 'a'])
expect(useAppStore.getState().sortEpoch).toBe(1)
}
)
it('keeps live Smart order steady when only persisted sortOrder changes', () => {
const persistSortOrder = vi.fn(async () => ({ updated: 0 }))
vi.stubGlobal('api', { worktrees: { persistSortOrder } })
try {
seed(
[makeWorktree('a', 'A', { sortOrder: 2 }), makeWorktree('b', 'B', { sortOrder: 1 })],
'smart'
)
useAppStore.setState({
tabsByWorktree: { a: [makeTerminalTab({ id: 'tab-a', worktreeId: 'a' })] },
ptyIdsByTabId: { 'tab-a': ['pty-a'] }
})
const { result } = renderSortOrder()
const before = result.current
const savesBefore = persistSortOrder.mock.calls.length
act(() => publishSortOrders({ a: 1, b: 2 }))
expect(result.current).toBe(before)
expect(persistSortOrder.mock.calls.length).toBe(savesBefore)
} finally {
vi.unstubAllGlobals()
}
})
})
@@ -58,9 +58,15 @@ export function useSidebarWorktreeSortOrder(args: {
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)
const sortOrderEpoch = useAppStore((s) => s.sortOrderEpoch)
// Why a latching ref: a live signal makes Smart authoritative for the session, even after that activity ends.
const sessionHasHadLiveSmartSignal = useRef(false)
// Why: cold Smart and Manual fallback rows rank by sortOrder and never persist it, so they can follow another client's save without looping.
const persistedRankTrigger =
sortBy === 'manual' || (sortBy === 'smart' && !sessionHasHadLiveSmartSignal.current)
? sortOrderEpoch
: 0
const recomputedSort = useMemo(() => {
const state = useAppStore.getState()
@@ -119,7 +125,7 @@ export function useSidebarWorktreeSortOrder(args: {
}
// settledSortEpoch is an intentional trigger not read in the memo; its change signals a recompute.
// oxlint-disable-next-line react-hooks/exhaustive-deps
}, [settledSortEpoch, repoMap, sortBy])
}, [settledSortEpoch, persistedRankTrigger, repoMap, sortBy])
// Why: stable ID order prevents rank-only refreshes from echoing an unchanged snapshot.
const sortedIds = useReusedArrayIdentity(recomputedSort.sortedIds)
@@ -136,6 +136,8 @@ export type WorktreeSlice = {
sortEpoch: number
/** sortEpoch after the settle window; the sidebar sort reads this (see store/settled-sort-epoch.ts). */
settledSortEpoch: number
/** Bumped by sortOrder-only changes, which skip sortEpoch so live Smart can't loop on its own saves. */
sortOrderEpoch: number
/**
* Worktree IDs that have been activated at least once during this app
* session. The first activation of a worktree is special: its
@@ -256,7 +256,7 @@ export function mergeFetchedWorktrees(
},
// Why: an order save re-reads as a sortOrder-only change; re-sorting on it would schedule the next save (#27268).
...(worktreesDifferOnlyInSortOrder(s.worktreesByRepo[args.repoId], mergedWorktrees)
? {}
? { sortOrderEpoch: s.sortOrderEpoch + 1 }
: { sortEpoch: s.sortEpoch + 1 })
}
: {}),
@@ -5,14 +5,14 @@ import { worktreeHostMatchOptions, worktreeMatchesHost } from './worktree-host-o
/**
* Applies a host's published order save to one repo's rows in place.
* Why no sortEpoch bump: an order save must not schedule the next Smart re-sort (#27268).
* Why sortOrderEpoch, not sortEpoch: an order save must not schedule the next live Smart re-sort (#27268).
*/
export function patchWorktreeSortOrders(
state: AppState,
repoId: string,
hostId: ExecutionHostId,
sortOrders: readonly WorktreeSortOrderUpdate[]
): Pick<AppState, 'worktreesByRepo'> | AppState {
): Pick<AppState, 'worktreesByRepo' | 'sortOrderEpoch'> | AppState {
const current = state.worktreesByRepo[repoId]
if (!current || sortOrders.length === 0) {
return state
@@ -33,7 +33,12 @@ export function patchWorktreeSortOrders(
changed = true
return { ...worktree, sortOrder }
})
return changed ? { worktreesByRepo: { ...state.worktreesByRepo, [repoId]: next } } : state
return changed
? {
worktreesByRepo: { ...state.worktreesByRepo, [repoId]: next },
sortOrderEpoch: state.sortOrderEpoch + 1
}
: state
}
/** False when a published row is missing here, so the listing needs a refetch rather than a patch. */
@@ -18,6 +18,7 @@ export const worktreeSliceInitialState: Pick<
| 'remoteBranchConflictByWorktreeId'
| 'sortEpoch'
| 'settledSortEpoch'
| 'sortOrderEpoch'
| 'everActivatedWorktreeIds'
| 'lastVisitedAtByWorktreeId'
| 'hasHydratedWorktreePurge'
@@ -39,6 +40,7 @@ export const worktreeSliceInitialState: Pick<
remoteBranchConflictByWorktreeId: {},
sortEpoch: 0,
settledSortEpoch: 0,
sortOrderEpoch: 0,
everActivatedWorktreeIds: new Set<string>(),
lastVisitedAtByWorktreeId: {},
hasHydratedWorktreePurge: false,