From e4f599df05e360ae78d79affcd442c361cc14c6e Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 10 Oct 2026 21:24:34 -0700 Subject: [PATCH] perf(worktrees): preserve Git scans when saving sidebar order (#27343) * perf(worktrees): preserve Git scans when saving sort order Credit @mmarabel for identifying the repeated scans and the host-only approach in #27271. Co-authored-by: mmarabel <166927047+mmarabel@users.noreply.github.com> * perf(worktrees): skip Git scan invalidation for order saves on desktop hosts too The desktop window notifier ran the worktree-change invalidators unconditionally, so a desktop (or paired-host) Orca still cleared Git scan caches on every sidebar order save. Carry the invalidateGitScans flag through the notifier; every other caller keeps the default. --------- Co-authored-by: mmarabel <166927047+mmarabel@users.noreply.github.com> --- ...-runtime-create-managed-remote-worktree.ts | 2 +- src/main/runtime/orca-runtime-get-status.ts | 18 ++-- src/main/runtime/runtime-notifier-contract.ts | 7 +- .../runtime-sort-order-scan-cache.test.ts | 87 +++++++++++++++++++ src/main/window/runtime-window-lifecycle.ts | 6 +- 5 files changed, 111 insertions(+), 9 deletions(-) create mode 100644 src/main/runtime/runtime-sort-order-scan-cache.test.ts diff --git a/src/main/runtime/orca-runtime-create-managed-remote-worktree.ts b/src/main/runtime/orca-runtime-create-managed-remote-worktree.ts index 179d796d0e4..e6993788827 100644 --- a/src/main/runtime/orca-runtime-create-managed-remote-worktree.ts +++ b/src/main/runtime/orca-runtime-create-managed-remote-worktree.ts @@ -179,7 +179,7 @@ export class OrcaRuntimeWithCreateManagedRemoteWorktree extends OrcaRuntimeWithC orderedIds, store: this.store, invalidateResolved: () => this.invalidateResolvedWorktreeCache(), - notifyChanged: (repoId) => this.notifyWorktreesChanged(repoId) + notifyChanged: (repoId) => this.notifyWorktreesChanged(repoId, { invalidateGitScans: false }) }) } diff --git a/src/main/runtime/orca-runtime-get-status.ts b/src/main/runtime/orca-runtime-get-status.ts index d962ae46ce9..7075736b568 100644 --- a/src/main/runtime/orca-runtime-get-status.ts +++ b/src/main/runtime/orca-runtime-get-status.ts @@ -267,11 +267,19 @@ export class OrcaRuntimeWithGetStatus extends OrcaRuntimeWithGetRuntimeId { return { tabId, worktreeId: pty.worktreeId } } - protected notifyWorktreesChanged(repoId: string): void { - // Why here: the listing re-runs a scan this generation overtook, and a headless host has no - // window notifier to bump it, so the runtime's own change event bumps before it is sent. - runWorktreeChangeInvalidators(repoId) - this.notifier?.worktreesChanged(repoId) + protected notifyWorktreesChanged( + repoId: string, + options?: { invalidateGitScans?: boolean } + ): void { + // Why: headless hosts have no window notifier, so invalidate here unless the change is metadata-only. + if (options?.invalidateGitScans !== false) { + runWorktreeChangeInvalidators(repoId) + } + if (options) { + this.notifier?.worktreesChanged(repoId, undefined, options) + } else { + this.notifier?.worktreesChanged(repoId) + } this.emitClientEvent({ type: 'worktreesChanged', repoId }) } diff --git a/src/main/runtime/runtime-notifier-contract.ts b/src/main/runtime/runtime-notifier-contract.ts index d13da3976f1..795b707a664 100644 --- a/src/main/runtime/runtime-notifier-contract.ts +++ b/src/main/runtime/runtime-notifier-contract.ts @@ -31,7 +31,12 @@ export type RuntimeNotifier = { selector?: { kind: 'self' } | { kind: 'ssh'; targetId: string } | { kind: 'orphan' } reason?: 'definition' | 'run' | 'usage' }): void - worktreesChanged(repoId: string, renamed?: { oldWorktreeId: string; newWorktreeId: string }): void + worktreesChanged( + repoId: string, + renamed?: { oldWorktreeId: string; newWorktreeId: string }, + // Why: false for metadata-only changes (sort order) that leave Git scan results valid. + options?: { invalidateGitScans?: boolean } + ): void worktreeBaseStatus?(event: WorktreeBaseStatusEvent): void worktreeRemoteBranchConflict?(event: WorktreeRemoteBranchConflictEvent): void reposChanged(): void diff --git a/src/main/runtime/runtime-sort-order-scan-cache.test.ts b/src/main/runtime/runtime-sort-order-scan-cache.test.ts new file mode 100644 index 00000000000..d6de711c683 --- /dev/null +++ b/src/main/runtime/runtime-sort-order-scan-cache.test.ts @@ -0,0 +1,87 @@ +import { describe, expect, it, vi } from 'vitest' +import { join } from 'node:path' +import type { BrowserWindow } from 'electron' +import './orca-runtime-test-lifecycle.spec' +import { OrcaRuntimeService, listWorktrees } from './orca-runtime-test-mocks.spec' +import type { WorktreeMeta } from './orca-runtime-test-mocks.spec' +import { + TEST_REPO_ID, + TEST_REPO_PATH, + makeWorktreeInfo, + makeWorktreeMeta, + store +} from './orca-runtime-test-fixtures.spec' +import { registerRuntimeWindowLifecycle } from '../window/runtime-window-lifecycle' + +vi.mock('electron', () => ({ ipcMain: { on: vi.fn(), removeListener: vi.fn() } })) + +const firstPath = join(TEST_REPO_PATH, 'first') +const secondPath = join(TEST_REPO_PATH, 'second') +const firstId = `${TEST_REPO_ID}::${firstPath}` +const secondId = `${TEST_REPO_ID}::${secondPath}` + +async function createScannedRuntime(): Promise> { + const metaById: Record = { + [firstId]: makeWorktreeMeta({ sortOrder: 200 }), + [secondId]: makeWorktreeMeta({ sortOrder: 100 }) + } + const runtime = new OrcaRuntimeService({ + ...store, + getAllWorktreeMeta: () => metaById, + getWorktreeMeta: (id: string) => metaById[id], + setWorktreeMeta: (id: string, meta: Partial) => { + metaById[id] = { ...metaById[id], ...meta } + return metaById[id] + } + }) + vi.mocked(listWorktrees).mockClear() + vi.mocked(listWorktrees).mockResolvedValue([ + makeWorktreeInfo(firstPath), + makeWorktreeInfo(secondPath) + ]) + await runtime.listDetectedManagedWorktrees(`id:${TEST_REPO_ID}`) + return runtime +} + +async function expectOrderSaveSkipsGitScan( + runtime: InstanceType +): Promise { + const events: { type: string; repoId?: string }[] = [] + 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 }]) + expect(listWorktrees).toHaveBeenCalledTimes(1) + const sortOrderOf = (id: string) => + listed.worktrees.find((worktree) => worktree.id === id)?.sortOrder ?? Number.NaN + expect(sortOrderOf(secondId)).toBeGreaterThan(sortOrderOf(firstId)) + + runtime.publishWorktreeRemovalChange(TEST_REPO_ID) + await runtime.listDetectedManagedWorktrees(`id:${TEST_REPO_ID}`) + expect(listWorktrees).toHaveBeenCalledTimes(2) +} + +describe('runtime sort-order saves', () => { + it('publishes the changed order without rescanning Git on a headless host', async () => { + await expectOrderSaveSkipsGitScan(await createScannedRuntime()) + }) + + it('publishes the changed order without rescanning Git through the desktop window notifier', async () => { + const runtime = await createScannedRuntime() + const send = vi.fn() + const mainWindow = { + id: 1, + isDestroyed: () => false, + on: vi.fn(), + webContents: { isDestroyed: () => false, send, on: vi.fn() } + } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: registration only reads id, isDestroyed, on and webContents. + registerRuntimeWindowLifecycle(mainWindow as unknown as BrowserWindow, runtime) + + await expectOrderSaveSkipsGitScan(runtime) + expect(send).toHaveBeenCalledWith('worktrees:changed', { repoId: TEST_REPO_ID }) + }) +}) diff --git a/src/main/window/runtime-window-lifecycle.ts b/src/main/window/runtime-window-lifecycle.ts index 95abe7c3515..66e131a2f31 100644 --- a/src/main/window/runtime-window-lifecycle.ts +++ b/src/main/window/runtime-window-lifecycle.ts @@ -37,9 +37,11 @@ export function registerRuntimeWindowLifecycle( }) const send = rendererNotifications.send runtime.setNotifier({ - worktreesChanged: (repoId, renamed) => { + worktreesChanged: (repoId, renamed, options) => { // Why: clear scan caches before the renderer handles this event, so it can't read stale TTL entries after a mutation. - runWorktreeChangeInvalidators(repoId) + if (options?.invalidateGitScans !== false) { + runWorktreeChangeInvalidators(repoId) + } send('worktrees:changed', renamed ? { repoId, renamed } : { repoId }) }, worktreeBaseStatus: (event) => send('worktree:baseStatus', event),