From 8c4f57e8efeca186d7c8bc5f68cbce3157b84634 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 23 Jun 2026 11:07:49 -0700 Subject: [PATCH] Reduce repeated git probes during worktree refresh (#6153) Co-authored-by: Orca --- src/main/github/client.test.ts | 102 +++++- src/main/github/client.ts | 73 ++++ src/main/ipc/worktree-change-invalidators.ts | 25 ++ src/main/ipc/worktree-remote.ts | 4 + src/main/ipc/worktrees.test.ts | 317 +++++++++++++++++- src/main/ipc/worktrees.ts | 92 ++++- .../attach-main-window-services.test.ts | 14 +- .../window/attach-main-window-services.ts | 9 +- 8 files changed, 622 insertions(+), 14 deletions(-) create mode 100644 src/main/ipc/worktree-change-invalidators.ts diff --git a/src/main/github/client.test.ts b/src/main/github/client.test.ts index 8493317a4..9e6e5eb92 100644 --- a/src/main/github/client.test.ts +++ b/src/main/github/client.test.ts @@ -110,7 +110,8 @@ import { updatePRTitle, _getMergeQueueCacheSizeForTests, _resetOwnerRepoCache, - _resetMergeQueueCacheForTests + _resetMergeQueueCacheForTests, + __resetTrackedUpstreamBranchCacheForTests } from './client' describe('checkOrcaStarred', () => { @@ -179,6 +180,7 @@ describe('getPRForBranch', () => { acquireMock.mockResolvedValue(undefined) _resetOwnerRepoCache() _resetMergeQueueCacheForTests() + __resetTrackedUpstreamBranchCacheForTests() }) it('queries GitHub by head branch when the remote is on github.com', async () => { @@ -1160,6 +1162,104 @@ describe('getPRForBranch', () => { }) }) + it('does not repeat missing tracked-upstream probes during PR refresh polling', async () => { + resolvePRRepositoryCandidatesMock.mockResolvedValue({ + candidates: [{ owner: 'acme', repo: 'widgets' }], + headRepo: { owner: 'acme', repo: 'widgets' } + }) + ghExecFileAsyncMock.mockResolvedValue({ stdout: JSON.stringify([]) }) + gitExecFileAsyncMock.mockRejectedValue( + new Error("fatal: no upstream configured for branch 'no-pr-branch'") + ) + + await getPRForBranch('/repo-root', 'no-pr-branch') + await getPRForBranch('/repo-root', 'no-pr-branch') + await getPRForBranch('/repo-root', 'no-pr-branch') + + const trackedUpstreamCalls = gitExecFileAsyncMock.mock.calls.filter(([args]) => + (args as string[]).includes('no-pr-branch@{upstream}') + ) + expect(trackedUpstreamCalls).toHaveLength(1) + }) + + it('coalesces concurrent missing tracked-upstream probes', async () => { + resolvePRRepositoryCandidatesMock.mockResolvedValue({ + candidates: [{ owner: 'acme', repo: 'widgets' }], + headRepo: { owner: 'acme', repo: 'widgets' } + }) + ghExecFileAsyncMock.mockResolvedValue({ stdout: JSON.stringify([]) }) + gitExecFileAsyncMock.mockImplementation(async () => { + await Promise.resolve() + throw new Error("fatal: no upstream configured for branch 'no-pr-branch'") + }) + + await Promise.all([ + getPRForBranch('/repo-root', 'no-pr-branch'), + getPRForBranch('/repo-root', 'no-pr-branch'), + getPRForBranch('/repo-root', 'no-pr-branch') + ]) + + const trackedUpstreamCalls = gitExecFileAsyncMock.mock.calls.filter(([args]) => + (args as string[]).includes('no-pr-branch@{upstream}') + ) + expect(trackedUpstreamCalls).toHaveLength(1) + }) + + it('keeps missing tracked-upstream probes separate for host and WSL runtimes', async () => { + resolvePRRepositoryCandidatesMock.mockResolvedValue({ + candidates: [{ owner: 'acme', repo: 'widgets' }], + headRepo: { owner: 'acme', repo: 'widgets' } + }) + ghExecFileAsyncMock.mockResolvedValue({ stdout: JSON.stringify([]) }) + gitExecFileAsyncMock.mockRejectedValue( + new Error("fatal: no upstream configured for branch 'no-pr-branch'") + ) + + await getPRForBranch('/repo-root', 'no-pr-branch') + await getPRForBranch('/repo-root', 'no-pr-branch', null, null, null, { + localGitExecOptions: { wslDistro: 'Ubuntu' } + }) + await getPRForBranch('/repo-root', 'no-pr-branch') + await getPRForBranch('/repo-root', 'no-pr-branch', null, null, null, { + localGitExecOptions: { wslDistro: 'Ubuntu' } + }) + + const trackedUpstreamCalls = gitExecFileAsyncMock.mock.calls.filter(([args]) => + (args as string[]).includes('no-pr-branch@{upstream}') + ) + expect(trackedUpstreamCalls).toHaveLength(2) + expect(trackedUpstreamCalls[0][1]).toEqual({ cwd: '/repo-root' }) + expect(trackedUpstreamCalls[1][1]).toEqual({ + cwd: '/repo-root', + wslDistro: 'Ubuntu' + }) + }) + + it('rechecks missing tracked-upstream probes after the null-cache TTL expires', async () => { + vi.useFakeTimers() + try { + resolvePRRepositoryCandidatesMock.mockResolvedValue({ + candidates: [{ owner: 'acme', repo: 'widgets' }], + headRepo: { owner: 'acme', repo: 'widgets' } + }) + ghExecFileAsyncMock.mockResolvedValue({ stdout: JSON.stringify([]) }) + gitExecFileAsyncMock + .mockRejectedValueOnce(new Error("fatal: no upstream configured for branch 'feature'")) + .mockResolvedValueOnce({ stdout: 'origin/contributor/original\n', stderr: '' }) + + await getPRForBranch('/repo-root', 'feature') + await vi.advanceTimersByTimeAsync(30_001) + await getPRForBranch('/repo-root', 'feature') + + const trackedUpstreamCalls = gitExecFileAsyncMock.mock.calls.filter(([args]) => + (args as string[]).includes('feature@{upstream}') + ) + expect(trackedUpstreamCalls).toHaveLength(2) + } finally { + vi.useRealTimers() + } + }) + it('uses the tracked upstream remote owner for fork branch lookup', async () => { resolvePRRepositoryCandidatesMock.mockResolvedValueOnce({ candidates: [ diff --git a/src/main/github/client.ts b/src/main/github/client.ts index 6bf0976ba..158c28ef4 100644 --- a/src/main/github/client.ts +++ b/src/main/github/client.ts @@ -2273,6 +2273,20 @@ type TrackedUpstreamBranch = { branchName: string } +const TRACKED_UPSTREAM_NULL_CACHE_TTL_MS = 30_000 + +type TrackedUpstreamNullCacheEntry = { + expiresAt: number +} + +const trackedUpstreamNullCache = new Map() +const trackedUpstreamInFlight = new Map>() + +export function __resetTrackedUpstreamBranchCacheForTests(): void { + trackedUpstreamNullCache.clear() + trackedUpstreamInFlight.clear() +} + function parseTrackedUpstreamBranch( upstreamRef: string, branchName: string @@ -2289,6 +2303,65 @@ async function getTrackedUpstreamBranch( branchName: string, connectionId?: string | null, localGitOptions: { wslDistro?: string } = {} +): Promise { + // Why: branches without configured upstreams are stable misses during PR + // polling; cache only nulls so positive PR discovery stays fresh. + const cacheKey = getTrackedUpstreamBranchCacheKey( + repoPath, + branchName, + connectionId, + localGitOptions + ) + const now = Date.now() + const cachedNull = trackedUpstreamNullCache.get(cacheKey) + if (cachedNull && cachedNull.expiresAt > now) { + return null + } + if (cachedNull) { + trackedUpstreamNullCache.delete(cacheKey) + } + + const inFlight = trackedUpstreamInFlight.get(cacheKey) + if (inFlight) { + return inFlight + } + + const probe = probeTrackedUpstreamBranch(repoPath, branchName, connectionId, localGitOptions) + trackedUpstreamInFlight.set(cacheKey, probe) + try { + const result = await probe + if (result) { + trackedUpstreamNullCache.delete(cacheKey) + } else { + trackedUpstreamNullCache.set(cacheKey, { + expiresAt: now + TRACKED_UPSTREAM_NULL_CACHE_TTL_MS + }) + } + return result + } finally { + if (trackedUpstreamInFlight.get(cacheKey) === probe) { + trackedUpstreamInFlight.delete(cacheKey) + } + } +} + +function getTrackedUpstreamBranchCacheKey( + repoPath: string, + branchName: string, + connectionId?: string | null, + localGitOptions: { wslDistro?: string } = {} +): string { + const runtimeKey = connectionId + ? `ssh:${connectionId}` + : `local:${localGitOptions.wslDistro ?? 'host'}` + return [runtimeKey, repoPath, branchName].join('\0') +} + +async function probeTrackedUpstreamBranch( + repoPath: string, + branchName: string, + connectionId?: string | null, + localGitOptions: { wslDistro?: string } = {} ): Promise { const args = ['rev-parse', '--abbrev-ref', '--symbolic-full-name', `${branchName}@{upstream}`] try { diff --git a/src/main/ipc/worktree-change-invalidators.ts b/src/main/ipc/worktree-change-invalidators.ts new file mode 100644 index 000000000..a43657743 --- /dev/null +++ b/src/main/ipc/worktree-change-invalidators.ts @@ -0,0 +1,25 @@ +const worktreeChangeInvalidators = new Set<(repoId: string) => void>() + +export function registerWorktreeChangeInvalidator( + invalidator: (repoId: string) => void +): () => void { + worktreeChangeInvalidators.add(invalidator) + return () => { + worktreeChangeInvalidators.delete(invalidator) + } +} + +export function runWorktreeChangeInvalidators(repoId: string): void { + const invalidators = Array.from(worktreeChangeInvalidators) + for (const invalidator of invalidators) { + try { + invalidator(repoId) + } catch (error) { + console.warn('[worktrees] worktree change invalidator failed:', error) + } + } +} + +export function __resetWorktreeChangeInvalidatorsForTests(): void { + worktreeChangeInvalidators.clear() +} diff --git a/src/main/ipc/worktree-remote.ts b/src/main/ipc/worktree-remote.ts index 50fb87e58..74dfb7db9 100644 --- a/src/main/ipc/worktree-remote.ts +++ b/src/main/ipc/worktree-remote.ts @@ -57,6 +57,7 @@ import type { SshGitProvider } from '../providers/ssh-git-provider' import { TUI_AGENT_CONFIG, isTuiAgent } from '../../shared/tui-agent-config' import { isWindowsAbsolutePathLike } from '../../shared/cross-platform-path' import { getSshGitUsername } from '../git/git-username' +import { runWorktreeChangeInvalidators } from './worktree-change-invalidators' type CreateWorktreeArgsWithSystemProvenance = CreateWorktreeArgs & { automationProvenance?: AutomationWorkspaceProvenance @@ -1328,6 +1329,9 @@ async function getRemoteLocalBaseRefUpdateSuggestionForWorktreeCreate( } export function notifyWorktreesChanged(mainWindow: BrowserWindow, repoId: string): void { + // Why: invalidate detected-worktree caches before renderer observers react, + // so follow-up listDetected reads post-change state. + runWorktreeChangeInvalidators(repoId) if (!mainWindow.isDestroyed()) { mainWindow.webContents.send('worktrees:changed', { repoId }) } diff --git a/src/main/ipc/worktrees.test.ts b/src/main/ipc/worktrees.test.ts index 3b7130d04..0f043c7c6 100644 --- a/src/main/ipc/worktrees.test.ts +++ b/src/main/ipc/worktrees.test.ts @@ -3,7 +3,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import { lstat, mkdir, mkdtemp, rm, writeFile } from 'fs/promises' import { tmpdir } from 'os' import { join, resolve } from 'path' -import type { CreateWorktreeResult } from '../../shared/types' +import type { CreateWorktreeResult, GitWorktreeInfo, Worktree } from '../../shared/types' const ORIGINAL_PLATFORM = process.platform @@ -226,9 +226,12 @@ vi.mock('./pty', () => ({ getLocalPtyProvider: getLocalPtyProviderMock })) -import { __resetSshWorktreeCreateFetchCacheForTests } from './worktree-remote' +import { + __resetSshWorktreeCreateFetchCacheForTests, + notifyWorktreesChanged +} from './worktree-remote' import { invalidateAuthorizedRootsCache, resolveRegisteredWorktreePath } from './filesystem-auth' -import { registerWorktreeHandlers } from './worktrees' +import { __resetDetectedWorktreeScanCacheForTests, registerWorktreeHandlers } from './worktrees' type HandlerMap = Record unknown> @@ -272,6 +275,7 @@ describe('registerWorktreeHandlers', () => { beforeEach(() => { setPlatform(ORIGINAL_PLATFORM) __resetSshWorktreeCreateFetchCacheForTests() + __resetDetectedWorktreeScanCacheForTests() invalidateAuthorizedRootsCache() for (const m of [ handleMock, @@ -2033,6 +2037,313 @@ describe('registerWorktreeHandlers', () => { }) }) + it('reuses a recent authoritative detected worktree scan', async () => { + listWorktreesMock.mockResolvedValue([ + { + path: '/workspace/repo', + head: 'main-head', + branch: 'refs/heads/main', + isBare: false, + isMainWorktree: true + } + ]) + + const first = await handlers['worktrees:listDetected'](null, { repoId: 'repo-1' }) + const second = await handlers['worktrees:listDetected'](null, { repoId: 'repo-1' }) + + expect(first).toEqual(second) + expect(listWorktreesMock).toHaveBeenCalledTimes(1) + }) + + it('coalesces concurrent authoritative detected worktree scans', async () => { + listWorktreesMock.mockImplementation(async () => { + await Promise.resolve() + return [ + { + path: '/workspace/repo', + head: 'main-head', + branch: 'refs/heads/main', + isBare: false, + isMainWorktree: true + } + ] + }) + + await Promise.all([ + handlers['worktrees:listDetected'](null, { repoId: 'repo-1' }), + handlers['worktrees:listDetected'](null, { repoId: 'repo-1' }), + handlers['worktrees:listDetected'](null, { repoId: 'repo-1' }) + ]) + + expect(listWorktreesMock).toHaveBeenCalledTimes(1) + }) + + it('rechecks detected worktree metadata while reusing a cached raw scan', async () => { + listWorktreesMock.mockResolvedValue([ + { + path: '/workspace/repo', + head: 'main-head', + branch: 'refs/heads/main', + isBare: false, + isMainWorktree: true + } + ]) + + let currentMeta = makeWorktreeMeta({ isPinned: false }) + store.getWorktreeMeta.mockImplementation(() => currentMeta) + store.setWorktreeMeta.mockImplementation(() => currentMeta) + const first = (await handlers['worktrees:listDetected'](null, { + repoId: 'repo-1' + })) as { worktrees: Worktree[] } + currentMeta = makeWorktreeMeta({ isPinned: true }) + const second = (await handlers['worktrees:listDetected'](null, { + repoId: 'repo-1' + })) as { worktrees: Worktree[] } + + expect(first.worktrees[0].isPinned).toBe(false) + expect(second.worktrees[0].isPinned).toBe(true) + expect(listWorktreesMock).toHaveBeenCalledTimes(1) + }) + + it('rescans detected worktrees after the scan cache TTL expires', async () => { + vi.useFakeTimers() + try { + listWorktreesMock + .mockResolvedValueOnce([ + { + path: '/workspace/repo', + head: 'main-head', + branch: 'refs/heads/main', + isBare: false, + isMainWorktree: true + } + ]) + .mockResolvedValueOnce([ + { + path: '/workspace/repo', + head: 'main-head', + branch: 'refs/heads/main', + isBare: false, + isMainWorktree: true + }, + { + path: '/workspace/new-worktree', + head: 'feature-head', + branch: 'refs/heads/feature', + isBare: false, + isMainWorktree: false + } + ]) + + await handlers['worktrees:listDetected'](null, { repoId: 'repo-1' }) + await vi.advanceTimersByTimeAsync(5_001) + const second = (await handlers['worktrees:listDetected'](null, { + repoId: 'repo-1' + })) as { worktrees: Worktree[] } + + expect(second.worktrees.map((worktree) => worktree.path)).toEqual([ + '/workspace/repo', + '/workspace/new-worktree' + ]) + expect(listWorktreesMock).toHaveBeenCalledTimes(2) + } finally { + vi.useRealTimers() + } + }) + + it('starts the detected scan cache TTL after a slow scan completes', async () => { + vi.useFakeTimers() + try { + listWorktreesMock + .mockImplementationOnce( + () => + new Promise((resolve) => { + setTimeout( + () => + resolve([ + { + path: '/workspace/repo', + head: 'main-head', + branch: 'refs/heads/main', + isBare: false, + isMainWorktree: true + } + ]), + 6_000 + ) + }) + ) + .mockResolvedValueOnce([ + { + path: '/workspace/repo', + head: 'main-head', + branch: 'refs/heads/main', + isBare: false, + isMainWorktree: true + }, + { + path: '/workspace/new-worktree', + head: 'feature-head', + branch: 'refs/heads/feature', + isBare: false, + isMainWorktree: false + } + ]) + + const first = handlers['worktrees:listDetected'](null, { repoId: 'repo-1' }) + await vi.advanceTimersByTimeAsync(6_000) + await first + const second = (await handlers['worktrees:listDetected'](null, { + repoId: 'repo-1' + })) as { worktrees: Worktree[] } + + expect(second.worktrees.map((worktree) => worktree.path)).toEqual(['/workspace/repo']) + expect(listWorktreesMock).toHaveBeenCalledTimes(1) + } finally { + vi.useRealTimers() + } + }) + + it('invalidates the detected scan cache before worktree change notifications', async () => { + listWorktreesMock + .mockResolvedValueOnce([ + { + path: '/workspace/repo', + head: 'main-head', + branch: 'refs/heads/main', + isBare: false, + isMainWorktree: true + } + ]) + .mockResolvedValueOnce([ + { + path: '/workspace/repo', + head: 'main-head', + branch: 'refs/heads/main', + isBare: false, + isMainWorktree: true + }, + { + path: '/workspace/new-worktree', + head: 'feature-head', + branch: 'refs/heads/feature', + isBare: false, + isMainWorktree: false + } + ]) + + await handlers['worktrees:listDetected'](null, { repoId: 'repo-1' }) + notifyWorktreesChanged(mainWindow as never, 'repo-1') + const second = (await handlers['worktrees:listDetected'](null, { + repoId: 'repo-1' + })) as { worktrees: Worktree[] } + + expect(second.worktrees).toHaveLength(2) + expect(listWorktreesMock).toHaveBeenCalledTimes(2) + }) + + it('rescans detected worktrees after the local create flow notifies worktree changes', async () => { + listWorktreesMock + .mockResolvedValueOnce([ + { + path: '/workspace/repo', + head: 'main-head', + branch: 'refs/heads/main', + isBare: false, + isMainWorktree: true + } + ]) + .mockResolvedValueOnce([ + { + path: '/workspace/repo', + head: 'main-head', + branch: 'refs/heads/main', + isBare: false, + isMainWorktree: true + }, + { + path: '/workspace/improve-dashboard', + head: 'feature-head', + branch: 'refs/heads/improve-dashboard', + isBare: false, + isMainWorktree: false + } + ]) + .mockResolvedValueOnce([ + { + path: '/workspace/repo', + head: 'main-head', + branch: 'refs/heads/main', + isBare: false, + isMainWorktree: true + }, + { + path: '/workspace/improve-dashboard', + head: 'feature-head', + branch: 'refs/heads/improve-dashboard', + isBare: false, + isMainWorktree: false + } + ]) + + await handlers['worktrees:listDetected'](null, { repoId: 'repo-1' }) + await handlers['worktrees:create'](null, { + repoId: 'repo-1', + name: 'improve-dashboard' + }) + const detected = (await handlers['worktrees:listDetected'](null, { + repoId: 'repo-1' + })) as { worktrees: Worktree[] } + + expect(detected.worktrees.map((worktree) => worktree.path)).toEqual([ + '/workspace/repo', + '/workspace/improve-dashboard' + ]) + expect(listWorktreesMock).toHaveBeenCalledTimes(3) + }) + + it('does not run fresh-scan side effects from a detected scan invalidated while in flight', async () => { + let resolveScan: (worktrees: GitWorktreeInfo[]) => void = () => {} + listWorktreesMock.mockImplementation( + () => + new Promise((resolve) => { + resolveScan = resolve as (worktrees: GitWorktreeInfo[]) => void + }) + ) + store.getAllWorktreeLineage.mockReturnValue({ + 'repo-1::/workspace/new-worktree': { + worktreeId: 'repo-1::/workspace/new-worktree', + worktreeInstanceId: 'child-instance', + parentWorktreeId: 'repo-1::/workspace/repo', + parentWorktreeInstanceId: 'parent-instance', + origin: 'manual', + capture: { + source: 'manual-action', + confidence: 'explicit' + }, + createdAt: 0 + } + }) + + const pendingList = handlers['worktrees:listDetected'](null, { repoId: 'repo-1' }) + await Promise.resolve() + notifyWorktreesChanged(mainWindow as never, 'repo-1') + resolveScan([ + { + path: '/workspace/repo', + head: 'main-head', + branch: 'refs/heads/main', + isBare: false, + isMainWorktree: true + } + ]) + + await pendingList + + expect(store.removeWorktreeLineage).not.toHaveBeenCalled() + expect(listWorktreesMock).toHaveBeenCalledTimes(1) + }) + it('fetches the same-repo PR head via the SSH tracking-ref RPC, not git.exec', async () => { const fetchRemoteTrackingRef = vi.fn(async () => {}) const exec = vi.fn(async (args: string[]) => { diff --git a/src/main/ipc/worktrees.ts b/src/main/ipc/worktrees.ts index 564d369d9..f550d1107 100644 --- a/src/main/ipc/worktrees.ts +++ b/src/main/ipc/worktrees.ts @@ -77,6 +77,7 @@ import { cleanupUnusedWorktreePushTargetRemoteSsh, notifyWorktreesChanged } from './worktree-remote' +import { registerWorktreeChangeInvalidator } from './worktree-change-invalidators' import { invalidateAuthorizedRootsCache, isENOENT, @@ -387,6 +388,83 @@ function getPreservedBranchCleanupTarget( const loggedUnavailableSshGitProviders = new Set() const loggedWorktreeListFailures = new Set() const loggedMalformedWorktreeMetaKeys = new Set() +// Why: absorb renderer polling bursts while keeping external worktree-change +// lag bounded to one short refresh window. +const DETECTED_WORKTREE_SCAN_CACHE_TTL_MS = 5_000 + +type DetectedWorktreeScanCacheEntry = { + expiresAt: number + worktrees: GitWorktreeInfo[] +} + +type DetectedWorktreeScanResult = { + gitWorktrees: GitWorktreeInfo[] + fresh: boolean +} + +const detectedWorktreeScanCache = new Map() +const detectedWorktreeScanInFlight = new Map>() +const detectedWorktreeScanGenerations = new Map() + +function invalidateDetectedWorktreeScanCache(repoId: string): void { + detectedWorktreeScanCache.delete(repoId) + detectedWorktreeScanInFlight.delete(repoId) + detectedWorktreeScanGenerations.set( + repoId, + (detectedWorktreeScanGenerations.get(repoId) ?? 0) + 1 + ) +} + +registerWorktreeChangeInvalidator(invalidateDetectedWorktreeScanCache) + +export function __resetDetectedWorktreeScanCacheForTests(): void { + detectedWorktreeScanCache.clear() + detectedWorktreeScanInFlight.clear() + detectedWorktreeScanGenerations.clear() +} + +async function listDetectedGitWorktrees( + store: Store, + repo: Repo +): Promise { + if (repo.connectionId || isFolderRepo(repo)) { + return { + gitWorktrees: await listRepoWorktrees(repo, getLocalProjectWorktreeGitOptions(store, repo)), + fresh: true + } + } + + const cached = detectedWorktreeScanCache.get(repo.id) + if (cached && cached.expiresAt > Date.now()) { + return { gitWorktrees: cached.worktrees, fresh: false } + } + + const inFlight = detectedWorktreeScanInFlight.get(repo.id) + if (inFlight) { + return { gitWorktrees: await inFlight, fresh: false } + } + + const scan = listRepoWorktrees(repo, getLocalProjectWorktreeGitOptions(store, repo)) + const generation = detectedWorktreeScanGenerations.get(repo.id) ?? 0 + detectedWorktreeScanInFlight.set(repo.id, scan) + try { + const gitWorktrees = await scan + // Why: a create/remove notification can invalidate while the git scan is + // still running. Do not let that stale scan repopulate the cache afterward. + const isCurrentGeneration = (detectedWorktreeScanGenerations.get(repo.id) ?? 0) === generation + if (isCurrentGeneration) { + detectedWorktreeScanCache.set(repo.id, { + worktrees: gitWorktrees, + expiresAt: Date.now() + DETECTED_WORKTREE_SCAN_CACHE_TTL_MS + }) + } + return { gitWorktrees, fresh: isCurrentGeneration } + } finally { + if (detectedWorktreeScanInFlight.get(repo.id) === scan) { + detectedWorktreeScanInFlight.delete(repo.id) + } + } +} function warnOnce(keySet: Set, key: string, message: string, error?: unknown): void { if (keySet.has(key)) { @@ -946,6 +1024,7 @@ export function registerWorktreeHandlers( try { let gitWorktrees: GitWorktreeInfo[] + let freshScan = true if (isFolderRepo(repo)) { return { repoId: repo.id, @@ -966,13 +1045,14 @@ export function registerWorktreeHandlers( } gitWorktrees = await provider.listWorktrees(repo.path) } else { - gitWorktrees = await listRepoWorktrees( - repo, - getLocalProjectWorktreeGitOptions(store, repo) - ) + const scan = await listDetectedGitWorktrees(store, repo) + gitWorktrees = scan.gitWorktrees + freshScan = scan.fresh + } + if (freshScan) { + rememberLocalWorktreeRoots(store, repo, gitWorktrees) + pruneLineageForMissingRepoWorktrees(store, repo, gitWorktrees) } - rememberLocalWorktreeRoots(store, repo, gitWorktrees) - pruneLineageForMissingRepoWorktrees(store, repo, gitWorktrees) loggedWorktreeListFailures.delete(`${repo.id}:${repo.path}`) return { repoId: repo.id, diff --git a/src/main/window/attach-main-window-services.test.ts b/src/main/window/attach-main-window-services.test.ts index 1091ef714..f69ca694c 100644 --- a/src/main/window/attach-main-window-services.test.ts +++ b/src/main/window/attach-main-window-services.test.ts @@ -17,7 +17,8 @@ const { registerPtyHandlersMock, hydrateLocalPtyRegistryAtBootMock, setupAutoUpdaterMock, - browserManagerUnregisterAllMock + browserManagerUnregisterAllMock, + runWorktreeChangeInvalidatorsMock } = vi.hoisted(() => ({ onMock: vi.fn(), removeAllListenersMock: vi.fn(), @@ -33,7 +34,8 @@ const { registerPtyHandlersMock: vi.fn(), hydrateLocalPtyRegistryAtBootMock: vi.fn(), setupAutoUpdaterMock: vi.fn(), - browserManagerUnregisterAllMock: vi.fn() + browserManagerUnregisterAllMock: vi.fn(), + runWorktreeChangeInvalidatorsMock: vi.fn() })) vi.mock('electron', () => ({ @@ -64,6 +66,10 @@ vi.mock('../ipc/worktrees', () => ({ registerWorktreeHandlers: registerWorktreeHandlersMock })) +vi.mock('../ipc/worktree-change-invalidators', () => ({ + runWorktreeChangeInvalidators: runWorktreeChangeInvalidatorsMock +})) + vi.mock('../ipc/pty', () => ({ getLocalPtyProvider: vi.fn(), registerPtyHandlers: registerPtyHandlersMock @@ -592,5 +598,9 @@ describe('attachMainWindowServices', () => { } ] ]) + expect(runWorktreeChangeInvalidatorsMock).toHaveBeenCalledWith('repo-1') + expect(runWorktreeChangeInvalidatorsMock.mock.invocationCallOrder[0]).toBeLessThan( + sendMock.mock.invocationCallOrder[0] + ) }) }) diff --git a/src/main/window/attach-main-window-services.ts b/src/main/window/attach-main-window-services.ts index ce720ca61..eace514bc 100644 --- a/src/main/window/attach-main-window-services.ts +++ b/src/main/window/attach-main-window-services.ts @@ -36,6 +36,7 @@ import { isNativeFileDropPayload, type NativeFileDropPayload } from '../../share import { requestMobileMarkdownFromRenderer } from './mobile-markdown-request-relay' import type { CodexAccountSelectionTarget } from '../codex-accounts/runtime-selection' import type { ClaudeAccountSelectionTarget } from '../claude-accounts/runtime-selection' +import { runWorktreeChangeInvalidators } from '../ipc/worktree-change-invalidators' let appReloadHandlerTokenCounter = 0 let activeAppReloadHandlerToken: number | null = null @@ -216,8 +217,12 @@ function registerRuntimeWindowLifecycle( } } runtime.setNotifier({ - worktreesChanged: (repoId, renamed) => - send('worktrees:changed', renamed ? { repoId, renamed } : { repoId }), + worktreesChanged: (repoId, renamed) => { + // Why: clear detected-worktree scan caches before renderer listeners + // handle this event, preventing stale TTL reads after mutations. + runWorktreeChangeInvalidators(repoId) + send('worktrees:changed', renamed ? { repoId, renamed } : { repoId }) + }, worktreeBaseStatus: (event) => send('worktree:baseStatus', event), worktreeRemoteBranchConflict: (event) => send('worktree:remoteBranchConflict', event), reposChanged: () => send('repos:changed'),