diff --git a/src/main/runtime/orca-runtime-get-status.ts b/src/main/runtime/orca-runtime-get-status.ts index 8da950285e6..27414703158 100644 --- a/src/main/runtime/orca-runtime-get-status.ts +++ b/src/main/runtime/orca-runtime-get-status.ts @@ -50,6 +50,7 @@ import { wakeFolderRepoGitUpgradeWatch } from '../ipc/folder-repo-git-upgrade-wa import { runWorktreeChangeInvalidators } from '../ipc/worktree-change-invalidators' import { MACHINE_NAME_PUBLISH_WAIT_MS } from './runtime-machine-name' import { getManagedServerActions } from './managed-server-actions-registry' +import type { WorktreeSortOrderUpdate } from '../../shared/worktree/sort-order-update' type RuntimeStatusHost = { getAvailableAuthoritativeWindow(): unknown @@ -292,21 +293,29 @@ export abstract class OrcaRuntimeWithGetStatus extends OrcaRuntimeWithGetRuntime notifyWorktreesChanged( repoId: string, - options?: { invalidateGitScans?: boolean; sortOrderOnly?: boolean } + options?: { + invalidateGitScans?: boolean + sortOrderOnly?: boolean + sortOrders?: WorktreeSortOrderUpdate[] + } ): void { // Headless hosts invalidate here unless only metadata changed. if (options?.invalidateGitScans !== false) { runWorktreeChangeInvalidators(repoId) } if (options) { - this.notifier?.worktreesChanged(repoId, undefined, options) + const { sortOrders: _sortOrders, ...notifierOptions } = options + this.notifier?.worktreesChanged(repoId, undefined, notifierOptions) } else { this.notifier?.worktreesChanged(repoId) } this.emitClientEvent({ type: 'worktreesChanged', repoId, - ...(options?.sortOrderOnly === true ? { sortOrderOnly: true } : {}) + ...(options?.sortOrderOnly === true ? { sortOrderOnly: true } : {}), + ...(options?.sortOrderOnly === true && options.sortOrders + ? { sortOrders: options.sortOrders } + : {}) }) } diff --git a/src/main/runtime/orca-runtime-tests/lineage-and-scan-cache-part-03.spec.ts b/src/main/runtime/orca-runtime-tests/lineage-and-scan-cache-part-03.spec.ts index 64706d755e8..782bbffaa3c 100644 --- a/src/main/runtime/orca-runtime-tests/lineage-and-scan-cache-part-03.spec.ts +++ b/src/main/runtime/orca-runtime-tests/lineage-and-scan-cache-part-03.spec.ts @@ -255,7 +255,15 @@ describe('OrcaRuntimeService', () => { expect(setWorktreeMeta).toHaveBeenCalledTimes(2) expect(events).toEqual([ - { type: 'worktreesChanged', repoId: TEST_REPO_ID, sortOrderOnly: true } + { + type: 'worktreesChanged', + repoId: TEST_REPO_ID, + sortOrderOnly: true, + sortOrders: [ + { worktreeId: secondId, sortOrder: metaById[secondId].sortOrder }, + { worktreeId: firstId, sortOrder: metaById[firstId].sortOrder } + ] + } ]) }) diff --git a/src/main/runtime/runtime-created-worktree-terminal-target.test.ts b/src/main/runtime/runtime-created-worktree-terminal-target.test.ts index 89b4274a296..f3a0131792c 100644 --- a/src/main/runtime/runtime-created-worktree-terminal-target.test.ts +++ b/src/main/runtime/runtime-created-worktree-terminal-target.test.ts @@ -394,7 +394,17 @@ describe('workspace activation and metadata reentry', () => { ) expect(notify.mock.calls).toEqual([ ['repo'], - ['repo', { invalidateGitScans: false, sortOrderOnly: true }] + [ + 'repo', + { + invalidateGitScans: false, + sortOrderOnly: true, + sortOrders: [ + { worktreeId: runtime.selectedWorktree.id, sortOrder: expect.any(Number) }, + { worktreeId: otherId, sortOrder: expect.any(Number) } + ] + } + ] ]) }) diff --git a/src/main/runtime/runtime-managed-worktree-metadata.ts b/src/main/runtime/runtime-managed-worktree-metadata.ts index 8f78ce92345..4fe2f2769ce 100644 --- a/src/main/runtime/runtime-managed-worktree-metadata.ts +++ b/src/main/runtime/runtime-managed-worktree-metadata.ts @@ -2,7 +2,10 @@ import type { GitPushTarget, Worktree } from '../../shared/worktree/types' import type { WorktreeMeta } from '../../shared/worktree/meta-types' import { worktreeWorkspaceKey } from '../../shared/workspace-scope' import { splitWorktreeId } from '../../shared/worktree/id' -import { planWorktreeSortOrderUpdates } from '../../shared/worktree/sort-order-update' +import { + planWorktreeSortOrderUpdates, + type WorktreeSortOrderUpdate +} from '../../shared/worktree/sort-order-update' import { stripOrcaProvenanceMetaUpdates } from '../worktree-removal-safety' import { isRemovedFolderWorkspaceInstance } from '../folder-workspace-meta-presence' import type { RuntimeStore } from './runtime-store-contract' @@ -139,7 +142,7 @@ export function persistRuntimeManagedWorktreeSortOrder(args: { orderedIds: string[] store: RuntimeStore invalidateResolved: () => void - notifyChanged: (repoId: string) => void + notifyChanged: (repoId: string, sortOrders: WorktreeSortOrderUpdate[]) => void }): { updated: number } { // A moving project keeps its old order; the rest of the batch still lands. const updates = planWorktreeSortOrderUpdates( @@ -154,14 +157,15 @@ export function persistRuntimeManagedWorktreeSortOrder(args: { return { updated: 0 } } args.invalidateResolved() - const repoIds = new Set( - updates.flatMap(({ worktreeId }) => { - const parsed = splitWorktreeId(worktreeId) - return parsed ? [parsed.repoId] : [] - }) - ) - for (const repoId of repoIds) { - args.notifyChanged(repoId) + const updatesByRepo = new Map() + for (const update of updates) { + const repoId = splitWorktreeId(update.worktreeId)?.repoId + if (repoId) { + updatesByRepo.set(repoId, [...(updatesByRepo.get(repoId) ?? []), update]) + } + } + for (const [repoId, sortOrders] of updatesByRepo) { + args.notifyChanged(repoId, sortOrders) } return { updated: updates.length } } diff --git a/src/main/runtime/runtime-sort-order-scan-cache.test.ts b/src/main/runtime/runtime-sort-order-scan-cache.test.ts index 8d8d4ed1ebc..34ca93efbf0 100644 --- a/src/main/runtime/runtime-sort-order-scan-cache.test.ts +++ b/src/main/runtime/runtime-sort-order-scan-cache.test.ts @@ -12,6 +12,7 @@ import { store } from './orca-runtime-test-fixtures.spec' import { registerRuntimeWindowLifecycle } from '../window/runtime-window-lifecycle' +import type { RuntimeClientEvent } from '../../shared/runtime-client-events' vi.mock('electron', () => ({ ipcMain: { on: vi.fn(), removeListener: vi.fn() } })) @@ -46,18 +47,29 @@ async function createScannedRuntime(): Promise ): Promise { - const events: { type: string; repoId?: string }[] = [] + const events: RuntimeClientEvent[] = [] const unsubscribe = runtime.onClientEvent((event) => events.push(event)) expect(runtime.persistManagedWorktreeSortOrder([secondId, firstId])).toEqual({ updated: 2 }) unsubscribe() const listed = await runtime.listDetectedManagedWorktrees(`id:${TEST_REPO_ID}`) - expect(events).toEqual([{ type: 'worktreesChanged', repoId: TEST_REPO_ID, sortOrderOnly: true }]) expect(listWorktrees).toHaveBeenCalledTimes(1) const sortOrderOf = (id: string) => listed.worktrees.find((worktree) => worktree.id === id)?.sortOrder ?? Number.NaN expect(sortOrderOf(secondId)).toBeGreaterThan(sortOrderOf(firstId)) + // The event carries the saved values so a client can patch rows without refetching them. + expect(events).toEqual([ + { + type: 'worktreesChanged', + repoId: TEST_REPO_ID, + sortOrderOnly: true, + sortOrders: [ + { worktreeId: secondId, sortOrder: sortOrderOf(secondId) }, + { worktreeId: firstId, sortOrder: sortOrderOf(firstId) } + ] + } + ]) runtime.publishWorktreeRemovalChange(TEST_REPO_ID) await runtime.listDetectedManagedWorktrees(`id:${TEST_REPO_ID}`) diff --git a/src/main/runtime/workspaces/workspace-metadata.ts b/src/main/runtime/workspaces/workspace-metadata.ts index 1adc4f4df86..188f371c319 100644 --- a/src/main/runtime/workspaces/workspace-metadata.ts +++ b/src/main/runtime/workspaces/workspace-metadata.ts @@ -172,8 +172,12 @@ export abstract class OrcaRuntimeWithCreateManagedRemoteWorktree extends OrcaRun orderedIds, store: this.store, invalidateResolved: () => this.invalidateResolvedWorktreeCache(), - notifyChanged: (repoId) => - this.notifyWorktreesChanged(repoId, { invalidateGitScans: false, sortOrderOnly: true }) + notifyChanged: (repoId, sortOrders) => + this.notifyWorktreesChanged(repoId, { + invalidateGitScans: false, + sortOrderOnly: true, + sortOrders + }) }) } diff --git a/src/renderer/src/hooks/ipc-events/runtime-client-ipc-bridge-ownership.test.ts b/src/renderer/src/hooks/ipc-events/runtime-client-ipc-bridge-ownership.test.ts index 2dbc2737ea3..e48d86d244e 100644 --- a/src/renderer/src/hooks/ipc-events/runtime-client-ipc-bridge-ownership.test.ts +++ b/src/renderer/src/hooks/ipc-events/runtime-client-ipc-bridge-ownership.test.ts @@ -117,7 +117,7 @@ function createHarness() { replay = false, event?: | RuntimeClientEvent - | { type: 'worktreesChanged'; repoId: string; sortOrderOnly: unknown } + | { type: 'worktreesChanged'; repoId: string; sortOrderOnly: unknown; sortOrders?: unknown } ) => { const response = { id: 'r', @@ -213,6 +213,53 @@ it.each([true, false, undefined, 'true', 1, null])( } ) +it.each([ + { sortOrderOnly: true, valid: true, known: true, kept: true }, + { sortOrderOnly: undefined, valid: true, known: true, kept: false }, + { sortOrderOnly: true, valid: false, known: true, kept: false }, + // An unknown repo refetches so a slow discovery cannot replay older values last. + { sortOrderOnly: true, valid: true, known: false, kept: false } +])('forwards published order values only on a valid order-only event (%o)', async (input) => { + const h = createHarness() + const sortOrders = input.valid + ? [{ worktreeId: 'remote-repo::/a', sortOrder: 5 }] + : [{ worktreeId: 'remote-repo::/a' }] + if (input.known) { + useAppStore.setState({ + repos: [ + { + id: 'remote-repo', + path: '/remote/repo', + displayName: 'Remote repo', + badgeColor: 'blue', + addedAt: 1, + executionHostId: 'runtime:host-a' + } + ] + }) + } + try { + h.start() + h.emit(0, false, { + type: 'worktreesChanged', + repoId: 'remote-repo', + sortOrderOnly: input.sortOrderOnly, + sortOrders + }) + for (let index = 0; index < 10; index += 1) { + await Promise.resolve() + } + expect(h.enqueue).toHaveBeenCalledWith({ + repoId: 'remote-repo', + executionHostId: 'runtime:host-a', + ...(input.sortOrderOnly === true ? { sortOrderOnly: true } : {}), + ...(input.kept ? { sortOrders } : {}) + }) + } finally { + await h.finish() + } +}) + it('invalidates lineage before unknown-repository discovery can block a structural event', async () => { const h = createHarness() const repos = Promise.withResolvers() diff --git a/src/renderer/src/hooks/ipc-events/runtime-client-ipc-bridge.ts b/src/renderer/src/hooks/ipc-events/runtime-client-ipc-bridge.ts index 6eab4308a65..4fd0f8fb5a4 100644 --- a/src/renderer/src/hooks/ipc-events/runtime-client-ipc-bridge.ts +++ b/src/renderer/src/hooks/ipc-events/runtime-client-ipc-bridge.ts @@ -32,6 +32,7 @@ import { invalidateRuntimeClientEventReplay } from './runtime-environment-subscription-selection' import type { WorktreeEventRuntime } from './worktree-event-runtime' +import { readPublishedSortOrders } from '../../store/slices/worktrees/listing/worktree-sort-order-patch' export function registerRuntimeClientIpcBridge( unsubs: (() => void)[], @@ -42,15 +43,15 @@ export function registerRuntimeClientIpcBridge( unsubs.push(() => { stopped = true }) + const isRuntimeEventRepoKnown = ({ repoId, executionHostId }: OwnedWorktreeEvent): boolean => + (useAppStore.getState().repos ?? []).some( + (repo) => repo.id === repoId && getRepoExecutionHostId(repo) === executionHostId + ) const ensureRuntimeEventRepoKnown = async ( environmentId: string, - { repoId, executionHostId }: OwnedWorktreeEvent + event: OwnedWorktreeEvent ): Promise => { - if ( - (useAppStore.getState().repos ?? []).some( - (repo) => repo.id === repoId && getRepoExecutionHostId(repo) === executionHostId - ) - ) { + if (isRuntimeEventRepoKnown(event)) { return } await useAppStore.getState().fetchRuntimeEnvironmentRepos(environmentId) @@ -141,6 +142,13 @@ export function registerRuntimeClientIpcBridge( ...(event.sortOrderOnly === true ? { sortOrderOnly: true } : {}) } }) + const sortOrders = + event.sortOrderOnly === true ? readPublishedSortOrders(event.sortOrders) : null + if (sortOrders && isRuntimeEventRepoKnown(owned)) { + // Why sync: published values must enqueue in receipt order; an unknown repo refetches instead. + worktreeChangeRefreshQueue.enqueue({ ...owned, sortOrders }) + return + } void ensureRuntimeEventRepoKnown(environmentId, owned).then(() => worktreeChangeRefreshQueue.enqueue(owned) ) diff --git a/src/renderer/src/hooks/ipc-events/worktree-event-lineage-refresh.test.ts b/src/renderer/src/hooks/ipc-events/worktree-event-lineage-refresh.test.ts index a87c1eda007..3bb28b6fd40 100644 --- a/src/renderer/src/hooks/ipc-events/worktree-event-lineage-refresh.test.ts +++ b/src/renderer/src/hooks/ipc-events/worktree-event-lineage-refresh.test.ts @@ -27,9 +27,17 @@ vi.mock('../../store', () => ({ useAppStore: { get getState() { return store.getState + }, + get setState() { + return store.setState } } })) +const sshStateGeneration = vi.hoisted(() => ({ value: 0 })) +vi.mock('@/store/slices/runtime-environment-ssh', async (importOriginal) => ({ + ...(await importOriginal()), + getEnvironmentSshStateGeneration: () => sshStateGeneration.value +})) vi.mock('@/lib/worktree-activation', () => ({ activateAndRevealWorktree: vi.fn() })) vi.mock('sonner', () => ({ toast: { warning: vi.fn() } })) vi.mock('@/components/worktree-base-fallback-notice', () => ({ @@ -103,6 +111,48 @@ describe('remote worktree event lineage refresh', () => { expect(responseBytes * 25).toBeGreaterThan(900_000) }) + it('downloads nothing for a 25-repository save whose host publishes the values', async () => { + const rows = Array.from({ length: 105 }, (_, index) => + makeWorktree({ + id: `repo-${index % 25}::/home/orca/workspaces/project/feature-${index}`, + repoId: `repo-${index % 25}`, + hostId: 'runtime:env-1', + runtimeOwnerEnvironmentId: 'env-1', + sortOrder: index + }) + ) + const worktreesByRepo: Record[]> = {} + for (const row of rows) { + ;(worktreesByRepo[row.repoId] ??= []).push(row) + } + store.setState({ worktreesByRepo, sortEpoch: 3 }) + const { worktreeChangeRefreshQueue: queue } = createWorktreeEventRuntime(unsubs, () => false) + const now = 1_800_000_000_000 + const events = Object.entries(worktreesByRepo).map(([repoId, repoRows]) => ({ + type: 'worktreesChanged' as const, + repoId, + sortOrderOnly: true, + sortOrders: repoRows.map((row, index) => ({ + worktreeId: row.id, + sortOrder: now - index * 1000 + })) + })) + for (const event of events) { + queue.enqueue({ ...event, executionHostId: 'runtime:env-1' }) + } + await flushPromises() + expect(store.getState().fetchWorktrees).not.toHaveBeenCalled() + expect(store.getState().fetchWorktreeLineage).not.toHaveBeenCalled() + expect(store.getState().sortEpoch).toBe(3) + expect(store.getState().worktreesByRepo['repo-0'][0].sortOrder).toBe(now) + // The save now costs only the pushed events (about 16 KB with real-length ids). + const eventBytes = events.reduce( + (sum, event) => sum + Buffer.byteLength(JSON.stringify(event)), + 0 + ) + expect(eventBytes).toBeLessThan(15_000) + }) + it.each([undefined, false])( 'keeps the full refresh for an older or unmarked event (%s)', async (sortOrderOnly) => { @@ -238,7 +288,7 @@ function createActualActionFixture() { } describe('order-only refresh with actual store actions', () => { - it('hydrates a cold client, then keeps new sortOrder and sortEpoch without a lineage download', async () => { + it('hydrates a cold client, then takes an older host order save without a lineage download or re-sort', async () => { const h = createActualActionFixture() h.queue.enqueue({ repoId: 'repo', executionHostId: 'runtime:env-1', sortOrderOnly: true }) await vi.waitFor(() => expect(isRemoteWorktreeLineageFresh(store.getState, 'env-1')).toBe(true)) @@ -249,10 +299,114 @@ describe('order-only refresh with actual store actions', () => { h.queue.enqueue({ repoId: 'repo', executionHostId: 'runtime:env-1', sortOrderOnly: true }) await vi.waitFor(() => expect(store.getState().worktreesByRepo.repo[0].sortOrder).toBe(20)) await flushPromises() - expect(store.getState().sortEpoch).toBe(beforeEpoch + 1) + // A sortOrder-only refetch must not schedule the next Smart re-sort and save (#27268). + expect(store.getState().sortEpoch).toBe(beforeEpoch) expect(h.lineageReadCount()).toBe(1) }) + it('patches a published order save in place without any host read or re-sort', async () => { + const h = createActualActionFixture() + await h.warm() + const readsBefore = runtimeEnvironmentCall.mock.calls.length + const beforeEpoch = store.getState().sortEpoch + h.queue.enqueue({ + repoId: 'repo', + executionHostId: 'runtime:env-1', + sortOrderOnly: true, + sortOrders: [{ worktreeId: h.worktree.id, sortOrder: 99 }] + }) + await flushPromises() + expect(store.getState().worktreesByRepo.repo[0].sortOrder).toBe(99) + expect(store.getState().sortEpoch).toBe(beforeEpoch) + expect(runtimeEnvironmentCall.mock.calls.length).toBe(readsBefore) + }) + + it('refetches when a published row was never loaded here', async () => { + const h = createActualActionFixture() + await h.warm() + store.setState({ worktreesByRepo: {} }) + h.setOrder(42) + h.queue.enqueue({ + repoId: 'repo', + executionHostId: 'runtime:env-1', + sortOrderOnly: true, + sortOrders: [{ worktreeId: h.worktree.id, sortOrder: 99 }] + }) + await vi.waitFor(() => expect(store.getState().worktreesByRepo.repo?.[0]?.sortOrder).toBe(42)) + }) + + it('still retries lineage on a published save when the host snapshot is not fresh', async () => { + const h = createActualActionFixture() + h.queue.enqueue({ + repoId: 'repo', + executionHostId: 'runtime:env-1', + sortOrderOnly: true, + sortOrders: [{ worktreeId: h.worktree.id, sortOrder: 99 }] + }) + await vi.waitFor(() => expect(h.lineageReadCount()).toBe(1)) + expect( + runtimeEnvironmentCall.mock.calls.filter( + ([request]) => request.method !== 'worktree.lineageList' + ) + ).toEqual([]) + expect(store.getState().worktreesByRepo.repo[0].sortOrder).toBe(99) + }) + + it.each(['reconnect', 'event-stream gap'] as const)( + 'refetches instead of applying values published before a %s', + async (gap) => { + const h = createActualActionFixture() + await h.warm() + const listing = Promise.withResolvers() + h.holdListing(listing.promise) + h.queue.enqueue({ repoId: 'repo', executionHostId: 'runtime:env-1' }) + h.queue.enqueue({ + repoId: 'repo', + executionHostId: 'runtime:env-1', + sortOrderOnly: true, + sortOrders: [{ worktreeId: h.worktree.id, sortOrder: 99 }] + }) + if (gap === 'reconnect') { + advanceRuntimeEnvironmentConnectionGeneration('env-1') + } else { + sshStateGeneration.value += 1 + } + h.setOrder(200) + listing.resolve() + await vi.waitFor(() => expect(store.getState().worktreesByRepo.repo[0].sortOrder).toBe(200)) + await flushPromises() + expect(store.getState().worktreesByRepo.repo[0].sortOrder).toBe(200) + } + ) + + it('leaves a same-id row owned by another host untouched', async () => { + const h = createActualActionFixture() + const local = makeWorktree({ id: h.worktree.id, repoId: 'repo', hostId: 'local', sortOrder: 5 }) + store.setState({ worktreesByRepo: { repo: [h.worktree, local] } }) + h.queue.enqueue({ + repoId: 'repo', + executionHostId: 'runtime:env-1', + sortOrderOnly: true, + sortOrders: [{ worktreeId: h.worktree.id, sortOrder: 99 }] + }) + await flushPromises() + expect(store.getState().worktreesByRepo.repo.map((row) => row.sortOrder)).toEqual([99, 5]) + }) + + it('still bumps sortEpoch when a refetch changes more than sortOrder', async () => { + const h = createActualActionFixture() + const beforeEpoch = store.getState().sortEpoch + runtimeEnvironmentCall.mockImplementation(async () => ({ + id: 'rpc', + ok: true, + result: makeDetectedResult('repo', [{ ...h.worktree, sortOrder: 30, isUnread: true }]), + _meta: { runtimeId: 'remote-runtime' } + })) + await store.getState().fetchWorktrees('repo', { executionHostId: 'runtime:env-1' }) + expect(store.getState().worktreesByRepo.repo[0].isUnread).toBe(true) + expect(store.getState().sortEpoch).toBe(beforeEpoch + 1) + }) + it('retries failed ordinary and best-effort reads on the next order-only event', async () => { vi.spyOn(console, 'error').mockImplementation(() => {}) const h = createActualActionFixture() diff --git a/src/renderer/src/hooks/ipc-events/worktree-event-runtime.ts b/src/renderer/src/hooks/ipc-events/worktree-event-runtime.ts index e094964b9d2..44a923fcb28 100644 --- a/src/renderer/src/hooks/ipc-events/worktree-event-runtime.ts +++ b/src/renderer/src/hooks/ipc-events/worktree-event-runtime.ts @@ -6,6 +6,12 @@ import { } from '../../../../shared/execution-host' import type { RuntimeClientEvent } from '../../../../shared/runtime-client-events' import type { AppState } from '../../store/types' +import type { WorktreeSortOrderUpdate } from '../../../../shared/worktree/sort-order-update' +import { + hasHostRowsForSortOrders, + patchWorktreeSortOrders +} from '../../store/slices/worktrees/listing/worktree-sort-order-patch' +import { buildRuntimeClientEventEnvironmentKey } from './runtime-environment-subscription-selection' import { useAppStore } from '../../store' import { invalidateRemoteWorktreeLineage, @@ -17,6 +23,11 @@ import { } from '../worktree-change-refresh-queue' const WORKTREE_RENAME_PURGE_GRACE_MS = 20_000 + +// Why the subscription key: it changes on reconnect, re-pair and event-stream replay invalidation. +function runtimeSortOrderFence(environmentId: string): string { + return buildRuntimeClientEventEnvironmentKey([environmentId]) +} const recentlyRenamedWorktreeIdExpiry = new Map() function getAuthoritativeDetectedWorktreeIds(state: AppState, repoId: string): Set | null { @@ -54,8 +65,35 @@ export function createWorktreeEventRuntime( forceLocalOwner?: boolean executionHostId?: ExecutionHostId sortOrderOnly?: boolean + sortOrders?: WorktreeSortOrderUpdate[] + sortOrderFence?: string } ): Promise => { + const sortOrderOwner = parseExecutionHostId(options?.executionHostId) + if ( + options?.sortOrderOnly === true && + options.sortOrders && + options.executionHostId && + sortOrderOwner?.kind === 'runtime' && + options.sortOrderFence === runtimeSortOrderFence(sortOrderOwner.environmentId) && + !renamed && + !options.forceLocalOwner && + hasHostRowsForSortOrders( + useAppStore.getState(), + repoId, + options.executionHostId, + options.sortOrders + ) + ) { + // Why: the host published the saved values, so refetching the repo would only re-download them (#27268). + const { executionHostId, sortOrders } = options + useAppStore.setState((s) => patchWorktreeSortOrders(s, repoId, executionHostId, sortOrders)) + // Keep the existing retry for a host whose lineage snapshot is not fresh. + if (!isRemoteWorktreeLineageFresh(useAppStore.getState, sortOrderOwner.environmentId)) { + await useAppStore.getState().fetchWorktreeLineage({ executionHostId }) + } + return + } const localRefreshStartedWithRuntime = options?.forceLocalOwner === true && isRuntimeEnvironmentActive() // Why: capture active-ness before migration moves the pointer; re-key maps before the diff so a rename isn't a deletion. @@ -171,7 +209,11 @@ export function createWorktreeEventRuntime( if (owner?.kind === 'runtime' && (event.sortOrderOnly !== true || event.renamed)) { invalidateRemoteWorktreeLineage(useAppStore.getState, owner.environmentId) } - refreshQueue.enqueue(event) + refreshQueue.enqueue( + event.sortOrders && owner?.kind === 'runtime' + ? { ...event, sortOrderFence: runtimeSortOrderFence(owner.environmentId) } + : event + ) } } unsubs.push(worktreeChangeRefreshQueue.dispose) diff --git a/src/renderer/src/hooks/worktree-change-refresh-queue.test.ts b/src/renderer/src/hooks/worktree-change-refresh-queue.test.ts index 6ccb6e0f228..8169ea26efd 100644 --- a/src/renderer/src/hooks/worktree-change-refresh-queue.test.ts +++ b/src/renderer/src/hooks/worktree-change-refresh-queue.test.ts @@ -246,4 +246,44 @@ describe('createWorktreeChangeRefreshQueue', () => { }) } ) + + it('merges queued published order saves and drops the values once any refresh needs a refetch', async () => { + const firstRefresh = Promise.withResolvers() + const handler = vi.fn().mockReturnValueOnce(firstRefresh.promise).mockResolvedValue(undefined) + const queue = createWorktreeChangeRefreshQueue(handler) + const owner = { repoId: 'repo', executionHostId: 'runtime:env-1' as const, sortOrderOnly: true } + queue.enqueue({ repoId: 'repo', executionHostId: 'runtime:env-1' }) + queue.enqueue({ + ...owner, + sortOrders: [ + { worktreeId: 'a', sortOrder: 1 }, + { worktreeId: 'b', sortOrder: 2 } + ] + }) + queue.enqueue({ ...owner, sortOrders: [{ worktreeId: 'b', sortOrder: 3 }] }) + firstRefresh.resolve() + await flushPromises() + expect(handler).toHaveBeenLastCalledWith('repo', undefined, { + forceLocalOwner: undefined, + executionHostId: 'runtime:env-1', + sortOrderOnly: true, + sortOrders: [ + { worktreeId: 'a', sortOrder: 1 }, + { worktreeId: 'b', sortOrder: 3 } + ] + }) + + const held = Promise.withResolvers() + handler.mockReturnValueOnce(held.promise) + queue.enqueue({ repoId: 'repo', executionHostId: 'runtime:env-1' }) + queue.enqueue({ ...owner, sortOrders: [{ worktreeId: 'a', sortOrder: 4 }] }) + queue.enqueue(owner) + held.resolve() + await flushPromises() + expect(handler).toHaveBeenLastCalledWith('repo', undefined, { + forceLocalOwner: undefined, + executionHostId: 'runtime:env-1', + sortOrderOnly: true + }) + }) }) diff --git a/src/renderer/src/hooks/worktree-change-refresh-queue.ts b/src/renderer/src/hooks/worktree-change-refresh-queue.ts index 95783035e6c..6632aa8b225 100644 --- a/src/renderer/src/hooks/worktree-change-refresh-queue.ts +++ b/src/renderer/src/hooks/worktree-change-refresh-queue.ts @@ -1,4 +1,5 @@ import type { ExecutionHostId } from '../../../shared/execution-host' +import type { WorktreeSortOrderUpdate } from '../../../shared/worktree/sort-order-update' type WorktreeRename = { oldWorktreeId: string @@ -9,6 +10,9 @@ type WorktreeChangeOwner = { forceLocalOwner?: boolean executionHostId?: ExecutionHostId sortOrderOnly?: boolean + sortOrders?: WorktreeSortOrderUpdate[] + // Connection the values were published on; a patch from another connection refetches instead. + sortOrderFence?: string } type WorktreeChangeEvent = { @@ -19,6 +23,8 @@ type WorktreeChangeEvent = { forceLocalOwner?: boolean executionHostId?: ExecutionHostId sortOrderOnly?: boolean + sortOrders?: WorktreeSortOrderUpdate[] + sortOrderFence?: string } type WorktreeChangeRefreshHandler = ( @@ -39,6 +45,17 @@ export type WorktreeChangeRefreshQueue = { enqueue: (event: WorktreeChangeEvent) => void } +function mergeSortOrders( + earlier: readonly WorktreeSortOrderUpdate[], + later: readonly WorktreeSortOrderUpdate[] +): WorktreeSortOrderUpdate[] { + const byId = new Map(earlier.map((entry) => [entry.worktreeId, entry])) + for (const entry of later) { + byId.set(entry.worktreeId, entry) + } + return [...byId.values()] +} + export function createWorktreeChangeRefreshQueue( handler: WorktreeChangeRefreshHandler ): WorktreeChangeRefreshQueue { @@ -54,7 +71,10 @@ export function createWorktreeChangeRefreshQueue( await handler(repoId, next?.renamed, { forceLocalOwner: next?.forceLocalOwner, ...(next?.executionHostId ? { executionHostId: next.executionHostId } : {}), - ...(next?.sortOrderOnly === true ? { sortOrderOnly: true } : {}) + ...(next?.sortOrderOnly === true ? { sortOrderOnly: true } : {}), + ...(next?.sortOrderOnly === true && next.sortOrders + ? { sortOrders: next.sortOrders, sortOrderFence: next.sortOrderFence } + : {}) }) } catch (error) { console.error('Failed to refresh changed worktrees:', error) @@ -107,11 +127,24 @@ export function createWorktreeChangeRefreshQueue( state.queue.push({ forceLocalOwner: event.forceLocalOwner, executionHostId: event.executionHostId, - ...(event.sortOrderOnly === true ? { sortOrderOnly: true } : {}) + ...(event.sortOrderOnly === true ? { sortOrderOnly: true } : {}), + ...(event.sortOrderOnly === true && event.sortOrders + ? { sortOrders: event.sortOrders, sortOrderFence: event.sortOrderFence } + : {}) }) } else if (event.sortOrderOnly !== true) { // An order save cannot erase an adjacent structural refresh. delete lastQueued.sortOrderOnly + delete lastQueued.sortOrders + delete lastQueued.sortOrderFence + } else if (lastQueued.sortOrders) { + if (event.sortOrders && event.sortOrderFence === lastQueued.sortOrderFence) { + lastQueued.sortOrders = mergeSortOrders(lastQueued.sortOrders, event.sortOrders) + } else { + // Values missing or from another connection: the refetch reads current ones. + delete lastQueued.sortOrders + delete lastQueued.sortOrderFence + } } } diff --git a/src/renderer/src/store/adopt-from-endpoint.ts b/src/renderer/src/store/adopt-from-endpoint.ts index 2bdb56874c4..71471ffa61e 100644 --- a/src/renderer/src/store/adopt-from-endpoint.ts +++ b/src/renderer/src/store/adopt-from-endpoint.ts @@ -17,6 +17,7 @@ import { adoptEndpointRepo } from '../../../shared/endpoint-catalog-adoption' import type { RuntimeClientTarget } from '../../../shared/runtime-client-target' +import type { WorktreeSortOrderUpdate } from '../../../shared/worktree/sort-order-update' import { getRuntimeTargetHostId } from '../runtime/runtime-client-target' export type FetchedRepo = { kind: 'repo'; row: Repo } @@ -32,6 +33,7 @@ type WorktreeEventRow = { repoId: string renamed?: { oldWorktreeId: string; newWorktreeId: string } sortOrderOnly?: boolean + sortOrders?: WorktreeSortOrderUpdate[] } /** A worktree change or activation an endpoint pushed; the endpoint is its only owner fact. */ export type EndpointWorktreeEvent = { kind: 'worktreeEvent'; row: WorktreeEventRow } diff --git a/src/renderer/src/store/slices/worktrees/listing/fetched-worktree-merge.ts b/src/renderer/src/store/slices/worktrees/listing/fetched-worktree-merge.ts index 55efb22ff04..0d3055c970f 100644 --- a/src/renderer/src/store/slices/worktrees/listing/fetched-worktree-merge.ts +++ b/src/renderer/src/store/slices/worktrees/listing/fetched-worktree-merge.ts @@ -4,7 +4,11 @@ import type { AppState } from '../../../types' import type { WorktreeSlice } from '../../worktree-helpers' import type { Worktree } from '../../../../../../shared/worktree/types' import { routeListingBranchSwitchesThroughGitIdentity } from '../../worktree-listing-branch-switch' -import { areDetectedWorktreeResultsEqual, areWorktreesEqual } from './worktree-catalog-visibility' +import { + areDetectedWorktreeResultsEqual, + areWorktreesEqual, + worktreesDifferOnlyInSortOrder +} from './worktree-catalog-visibility' import { mergeDetectedWorktreesForHost } from './detected-worktree-host-merge' import { getRemovedWorktreeIdsAfterAuthoritativeScan, @@ -250,7 +254,10 @@ export function mergeFetchedWorktrees( ...s.worktreesByRepo, [args.repoId]: mergedWorktrees }, - sortEpoch: s.sortEpoch + 1 + // 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) + ? {} + : { sortEpoch: s.sortEpoch + 1 }) } : {}), ...(detectedChanged diff --git a/src/renderer/src/store/slices/worktrees/listing/worktree-catalog-visibility.ts b/src/renderer/src/store/slices/worktrees/listing/worktree-catalog-visibility.ts index 2fcf8d51d28..a3fb2ef3ac2 100644 --- a/src/renderer/src/store/slices/worktrees/listing/worktree-catalog-visibility.ts +++ b/src/renderer/src/store/slices/worktrees/listing/worktree-catalog-visibility.ts @@ -1,3 +1,4 @@ +import { structuralValuesEqualIgnoringUndefined } from '../../../../../../shared/structural-value-equality' import { catalogRowsEqual } from '../../worktree-catalog-reconciliation' import type { DetectedWorktreeListResult, Worktree } from '../../../../../../shared/worktree/types' @@ -5,6 +6,27 @@ export function areWorktreesEqual(current: Worktree[] | undefined, next: Worktre return catalogRowsEqual(current, next) } +/** True when the rows match in place except for `sortOrder`, which an order save rewrites. */ +export function worktreesDifferOnlyInSortOrder( + current: readonly Worktree[] | undefined, + next: readonly Worktree[] +): boolean { + if (!current || current.length !== next.length) { + return false + } + return current.every((row, index) => { + const incoming = next[index] + return ( + row === incoming || + (row.id === incoming.id && + structuralValuesEqualIgnoringUndefined( + { ...row, sortOrder: 0 }, + { ...incoming, sortOrder: 0 } + )) + ) + }) +} + export function areDetectedWorktreeResultsEqual( current: DetectedWorktreeListResult | undefined, next: DetectedWorktreeListResult diff --git a/src/renderer/src/store/slices/worktrees/listing/worktree-sort-order-patch.test.ts b/src/renderer/src/store/slices/worktrees/listing/worktree-sort-order-patch.test.ts new file mode 100644 index 00000000000..621f638ac79 --- /dev/null +++ b/src/renderer/src/store/slices/worktrees/listing/worktree-sort-order-patch.test.ts @@ -0,0 +1,21 @@ +import { describe, expect, it } from 'vitest' +import { readPublishedSortOrders } from './worktree-sort-order-patch' + +describe('readPublishedSortOrders', () => { + it('reads valid entries and treats an absent field as an older host', () => { + expect(readPublishedSortOrders([{ worktreeId: 'repo::/a', sortOrder: 5 }])).toEqual([ + { worktreeId: 'repo::/a', sortOrder: 5 } + ]) + expect(readPublishedSortOrders(undefined)).toBeNull() + }) + + it('falls back to a refetch when any entry is malformed', () => { + expect( + readPublishedSortOrders([ + { worktreeId: 'repo::/a', sortOrder: 5 }, + { worktreeId: 'repo::/b', sortOrder: Number.NaN } + ]) + ).toBeNull() + expect(readPublishedSortOrders([{ worktreeId: 7, sortOrder: 5 }])).toBeNull() + }) +}) diff --git a/src/renderer/src/store/slices/worktrees/listing/worktree-sort-order-patch.ts b/src/renderer/src/store/slices/worktrees/listing/worktree-sort-order-patch.ts new file mode 100644 index 00000000000..75f2ea01b55 --- /dev/null +++ b/src/renderer/src/store/slices/worktrees/listing/worktree-sort-order-patch.ts @@ -0,0 +1,76 @@ +import type { ExecutionHostId } from '../../../../../../shared/execution-host' +import type { WorktreeSortOrderUpdate } from '../../../../../../shared/worktree/sort-order-update' +import type { AppState } from '../../../types' +import { worktreeHostMatchOptions, worktreeMatchesHost } from './worktree-host-ownership' + +/** + * 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). + */ +export function patchWorktreeSortOrders( + state: AppState, + repoId: string, + hostId: ExecutionHostId, + sortOrders: readonly WorktreeSortOrderUpdate[] +): Pick | AppState { + const current = state.worktreesByRepo[repoId] + if (!current || sortOrders.length === 0) { + return state + } + const sortOrderById = new Map(sortOrders.map((entry) => [entry.worktreeId, entry.sortOrder])) + const matchOptions = worktreeHostMatchOptions(state, repoId, hostId) + let changed = false + const next = current.map((worktree) => { + const sortOrder = sortOrderById.get(worktree.id) + if ( + sortOrder === undefined || + !Number.isFinite(sortOrder) || + worktree.sortOrder === sortOrder || + !worktreeMatchesHost(worktree, hostId, matchOptions) + ) { + return worktree + } + changed = true + return { ...worktree, sortOrder } + }) + return changed ? { worktreesByRepo: { ...state.worktreesByRepo, [repoId]: next } } : state +} + +/** False when a published row is missing here, so the listing needs a refetch rather than a patch. */ +export function hasHostRowsForSortOrders( + state: AppState, + repoId: string, + hostId: ExecutionHostId, + sortOrders: readonly WorktreeSortOrderUpdate[] +): boolean { + const matchOptions = worktreeHostMatchOptions(state, repoId, hostId) + const ids = new Set( + (state.worktreesByRepo[repoId] ?? []) + .filter((worktree) => worktreeMatchesHost(worktree, hostId, matchOptions)) + .map((worktree) => worktree.id) + ) + return sortOrders.every((entry) => ids.has(entry.worktreeId)) +} + +/** Valid published order entries, or null when an older or malformed host sent none. */ +export function readPublishedSortOrders(value: unknown): WorktreeSortOrderUpdate[] | null { + if (!Array.isArray(value)) { + return null + } + const entries: WorktreeSortOrderUpdate[] = [] + for (const entry of value) { + if ( + typeof entry !== 'object' || + entry === null || + !('worktreeId' in entry) || + !('sortOrder' in entry) || + typeof entry.worktreeId !== 'string' || + typeof entry.sortOrder !== 'number' || + !Number.isFinite(entry.sortOrder) + ) { + return null + } + entries.push({ worktreeId: entry.worktreeId, sortOrder: entry.sortOrder }) + } + return entries +} diff --git a/src/shared/runtime-client-events.ts b/src/shared/runtime-client-events.ts index 90c610ce8e0..af26fb90a1a 100644 --- a/src/shared/runtime-client-events.ts +++ b/src/shared/runtime-client-events.ts @@ -8,12 +8,19 @@ import type { SshConnectionState } from './ssh-types' import type { TerminalSideEffectBatch } from './terminal-side-effect-facts' import type { RuntimeNativeChatLaunchDraftResolution } from './runtime-types' import type { RuntimeNavigationTarget } from './runtime-navigation' +import type { WorktreeSortOrderUpdate } from './worktree/sort-order-update' export type RuntimeClientEvent = | { type: 'reposChanged' } /** The Workspace options UI state changed; a mirroring phone re-reads it. */ | { type: 'workspaceViewChanged' } - | { type: 'worktreesChanged'; repoId: string; sortOrderOnly?: boolean } + | { + type: 'worktreesChanged' + repoId: string + sortOrderOnly?: boolean + // Why: lets a client patch an order save in place instead of refetching the repo (#27268). + sortOrders?: WorktreeSortOrderUpdate[] + } | ({ type: 'nativeChatLaunchDraftResolved' } & RuntimeNativeChatLaunchDraftResolution) | { type: 'terminalSideEffects'; batch: TerminalSideEffectBatch } // Why: SSH connections live on the runtime host; paired clients have no IPC