From fe4bc46bd6998f119176e1247bf2bc3d4c1bce52 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 15 Jul 2026 16:12:15 -0700 Subject: [PATCH] fix(space): harden stale worktree pruning --- src/main/git/prune-worktrees.test.ts | 34 ++++++++++++ src/main/git/worktree.ts | 11 +++- src/main/ipc/worktrees.test.ts | 55 ++++++++++++++++++- src/main/ipc/worktrees.ts | 46 ++++++++++------ src/relay/git-handler-prune-worktrees.test.ts | 47 ++++++++++++++++ src/relay/git-handler.ts | 8 ++- .../status-bar/WorkspaceSpaceManagerPanel.tsx | 8 +-- .../src/store/slices/workspace-space.test.ts | 32 ++++++++++- .../src/store/slices/workspace-space.ts | 20 ++++++- 9 files changed, 231 insertions(+), 30 deletions(-) create mode 100644 src/main/git/prune-worktrees.test.ts create mode 100644 src/relay/git-handler-prune-worktrees.test.ts diff --git a/src/main/git/prune-worktrees.test.ts b/src/main/git/prune-worktrees.test.ts new file mode 100644 index 00000000000..33b7d2efc89 --- /dev/null +++ b/src/main/git/prune-worktrees.test.ts @@ -0,0 +1,34 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type * as RunnerModule from './runner' +import type * as StatusModule from './status' + +const gitExecFileAsyncMock = vi.hoisted(() => vi.fn().mockResolvedValue({ stdout: '', stderr: '' })) + +vi.mock('./runner', async (importOriginal) => ({ + ...(await importOriginal()), + gitExecFileAsync: gitExecFileAsyncMock +})) + +vi.mock('./status', async (importOriginal) => ({ + ...(await importOriginal()), + runWithGitReadCacheInvalidation: (run: () => Promise): Promise => run() +})) + +import { pruneWorktrees, WORKTREE_PRUNE_TIMEOUT_MS } from './worktree' + +describe('pruneWorktrees', () => { + beforeEach(() => { + gitExecFileAsyncMock.mockClear() + }) + + it('bounds native and WSL prune commands by default', async () => { + await pruneWorktrees('/repo', { wslDistro: 'Ubuntu' }) + + expect(gitExecFileAsyncMock).toHaveBeenCalledWith(['worktree', 'prune'], { + cwd: '/repo', + timeout: WORKTREE_PRUNE_TIMEOUT_MS, + wslDistro: 'Ubuntu' + }) + expect(WORKTREE_PRUNE_TIMEOUT_MS).toBeGreaterThan(0) + }) +}) diff --git a/src/main/git/worktree.ts b/src/main/git/worktree.ts index cda466af58b..05980cf5303 100644 --- a/src/main/git/worktree.ts +++ b/src/main/git/worktree.ts @@ -89,6 +89,7 @@ const PRUNABLE_EXISTENCE_PROBE_CONCURRENCY = 8 // checkout (mirrors the SYNC runner's cloud-placeholder floor, #7225). export const WORKTREE_ADD_TIMEOUT_MS = 180_000 export const WORKTREE_REMOVAL_PREFLIGHT_TIMEOUT_MS = 30_000 +export const WORKTREE_PRUNE_TIMEOUT_MS = 30_000 function gitExecOptions( cwd: string, @@ -1187,7 +1188,15 @@ export async function pruneWorktrees( // preserves locked registrations, so agent-locked worktrees survive. try { await runWithGitReadCacheInvalidation(() => - gitExecFileAsync(['worktree', 'prune'], gitExecOptions(repoPath, options)) + gitExecFileAsync( + ['worktree', 'prune'], + gitExecOptions(repoPath, { + ...options, + // Why: a wedged filesystem must not leave the Space action disabled indefinitely. + timeout: + options.timeout && options.timeout > 0 ? options.timeout : WORKTREE_PRUNE_TIMEOUT_MS + }) + ) ) } finally { bumpWorktreeScanGeneration(repoPath) diff --git a/src/main/ipc/worktrees.test.ts b/src/main/ipc/worktrees.test.ts index a3b1c0381a1..4daa4c745c4 100644 --- a/src/main/ipc/worktrees.test.ts +++ b/src/main/ipc/worktrees.test.ts @@ -256,7 +256,11 @@ import { __resetSshWorktreeCreateFetchCacheForTests, notifyWorktreesChanged } from './worktree-remote' -import { invalidateAuthorizedRootsCache, resolveRegisteredWorktreePath } from './filesystem-auth' +import { + invalidateAuthorizedRootsCache, + registerWorktreeRootsForRepo, + resolveRegisteredWorktreePath +} from './filesystem-auth' import { __getDetectedWorktreeScanCacheStatsForTests, __resetDetectedWorktreeScanCacheForTests, @@ -6112,6 +6116,55 @@ describe('registerWorktreeHandlers', () => { ).rejects.toThrow(/older Orca relay/) }) + it('continues shared-host pruning and invalidates stale roots after a host fails', async () => { + const remoteRepo = { + id: 'repo-1', + path: '/remote/repo', + displayName: 'repo', + badgeColor: '#000', + addedAt: 0, + connectionId: 'ssh-1' + } + const localRepo = { + id: 'repo-1', + path: '/workspace/repo', + displayName: 'repo', + badgeColor: '#000', + addedAt: 0 + } + store.getRepos.mockReturnValue([remoteRepo, localRepo]) + listWorktreesMock.mockResolvedValue([ + { + path: localRepo.path, + head: 'abc123', + branch: 'refs/heads/main', + isBare: false, + isMainWorktree: true + } + ]) + registerWorktreeRootsForRepo(store as never, localRepo.id, [ + localRepo.path, + '/workspace/stale-wt' + ]) + await expect( + resolveRegisteredWorktreePath('/workspace/stale-wt', store as never) + ).resolves.toBe('/workspace/stale-wt') + getSshGitProviderMock.mockReturnValue({ + pruneWorktrees: vi.fn().mockRejectedValue(new Error('remote unavailable')) + }) + pruneWorktreesMock.mockResolvedValue(undefined) + + await expect( + handlers['worktrees:pruneStaleRegistrations'](null, { repoId: 'repo-1' }) + ).rejects.toThrow('remote unavailable') + + expect(pruneWorktreesMock).toHaveBeenCalledWith(localRepo.path, expect.anything()) + expect(mainWindow.webContents.send).toHaveBeenCalledTimes(1) + await expect( + resolveRegisteredWorktreePath('/workspace/stale-wt', store as never) + ).rejects.toThrow('unknown repository or worktree path') + }) + it('limits concurrent repo scans in worktrees:listAll while preserving order', async () => { const repos = Array.from({ length: 10 }, (_, index) => ({ id: `repo-${index}`, diff --git a/src/main/ipc/worktrees.ts b/src/main/ipc/worktrees.ts index 4fa591bc72c..c286694467b 100644 --- a/src/main/ipc/worktrees.ts +++ b/src/main/ipc/worktrees.ts @@ -2034,30 +2034,42 @@ export function registerWorktreeHandlers( if (repos.length === 0) { throw new Error(`Repo not found: ${args.repoId}`) } - for (const repo of repos) { - if (isFolderRepo(repo)) { - continue - } - if (repo.connectionId) { - const provider = requireSshGitProvider(repo.connectionId) + let firstError: unknown + let didAttemptPrune = false + try { + for (const repo of repos) { + if (isFolderRepo(repo)) { + continue + } + didAttemptPrune = true try { - await provider.pruneWorktrees(repo.path) + if (repo.connectionId) { + const provider = requireSshGitProvider(repo.connectionId) + await provider.pruneWorktrees(repo.path) + } else { + await pruneWorktrees(repo.path, getLocalProjectWorktreeGitOptions(store, repo)) + } } catch (error) { // Why: an already-deployed relay predating git.pruneWorktrees // rejects with "Method not found"; surface what to do instead. - if (/method not found/i.test(error instanceof Error ? error.message : '')) { - throw new Error( - 'The connected SSH host is running an older Orca relay without worktree prune support. Reconnect to update it, or run `git worktree prune` there.' - ) - } - throw error + firstError ??= /method not found/i.test(error instanceof Error ? error.message : '') + ? new Error( + 'The connected SSH host is running an older Orca relay without worktree prune support. Reconnect to update it, or run `git worktree prune` there.' + ) + : error } - } else { - await pruneWorktrees(repo.path, getLocalProjectWorktreeGitOptions(store, repo)) } - notifyWorktreesChanged(mainWindow, repo.id) + } finally { + // Why: Git may prune one host (or part of one registry) before another host fails. + invalidateAuthorizedRootsCache() + } + if (didAttemptPrune) { + // Why: one event invalidates all host views, including partial mutations after a Git error. + notifyWorktreesChanged(mainWindow, args.repoId) + } + if (firstError) { + throw firstError } - invalidateAuthorizedRootsCache() } ) diff --git a/src/relay/git-handler-prune-worktrees.test.ts b/src/relay/git-handler-prune-worktrees.test.ts new file mode 100644 index 00000000000..ffafda9aa62 --- /dev/null +++ b/src/relay/git-handler-prune-worktrees.test.ts @@ -0,0 +1,47 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { RelayContext } from './context' +import { GitHandler } from './git-handler' +import { + createMockDispatcher, + type MockDispatcher, + type RelayDispatcher +} from './git-handler-test-setup' + +type GitSpyTarget = { + git( + args: string[], + cwd: string, + options?: { signal?: AbortSignal } + ): Promise<{ stdout: string; stderr: string }> +} + +describe('relay worktree prune', () => { + let dispatcher: MockDispatcher + let handler: GitHandler + + beforeEach(() => { + dispatcher = createMockDispatcher() + handler = new GitHandler(dispatcher as unknown as RelayDispatcher, new RelayContext()) + }) + + afterEach(() => { + handler.dispose() + }) + + it('forwards request cancellation to the Git subprocess', async () => { + const controller = new AbortController() + const gitSpy = vi + .spyOn(handler as unknown as GitSpyTarget, 'git') + .mockResolvedValue({ stdout: '', stderr: '' }) + + await dispatcher.callRequest( + 'git.pruneWorktrees', + { repoPath: '/repo' }, + { isStale: () => false, signal: controller.signal } + ) + + expect(gitSpy).toHaveBeenCalledWith(['worktree', 'prune'], '/repo', { + signal: controller.signal + }) + }) +}) diff --git a/src/relay/git-handler.ts b/src/relay/git-handler.ts index fe1496597d0..79d0df6719e 100644 --- a/src/relay/git-handler.ts +++ b/src/relay/git-handler.ts @@ -233,7 +233,7 @@ export class GitHandler { this.dispatcher.onRequest('git.listWorktrees', (p, context) => this.listWorktrees(p, context)) this.dispatcher.onRequest('git.addWorktree', (p) => this.addWorktree(p)) this.dispatcher.onRequest('git.removeWorktree', (p) => this.removeWorktree(p)) - this.dispatcher.onRequest('git.pruneWorktrees', (p) => this.pruneWorktrees(p)) + this.dispatcher.onRequest('git.pruneWorktrees', (p, context) => this.pruneWorktrees(p, context)) this.dispatcher.onRequest('git.worktreeIsClean', (p) => this.worktreeIsClean(p)) this.dispatcher.onRequest('git.refreshLocalBaseRefForWorktreeCreate', (p) => this.refreshLocalBaseRefForWorktreeCreate(p) @@ -1410,11 +1410,13 @@ export class GitHandler { : remove() } - private async pruneWorktrees(params: Record) { + private async pruneWorktrees(params: Record, context?: RequestContext) { const repoPath = params.repoPath as string // Why: prune only drops registrations whose directory is gone; git itself // preserves locked registrations, so agent-locked worktrees survive. - await this.runWithGitReadCacheClear(() => this.git(['worktree', 'prune'], repoPath)) + await this.runWithGitReadCacheClear(() => + this.git(['worktree', 'prune'], repoPath, { signal: context?.signal }) + ) return {} } diff --git a/src/renderer/src/components/status-bar/WorkspaceSpaceManagerPanel.tsx b/src/renderer/src/components/status-bar/WorkspaceSpaceManagerPanel.tsx index 250d7822042..90cad0df061 100644 --- a/src/renderer/src/components/status-bar/WorkspaceSpaceManagerPanel.tsx +++ b/src/renderer/src/components/status-bar/WorkspaceSpaceManagerPanel.tsx @@ -1258,7 +1258,7 @@ export function WorkspaceSpaceManagerPanel(): React.JSX.Element { ) const [isPruningStale, setIsPruningStale] = useState(false) const pruneStale = useCallback((): void => { - if (!analysis || isPruningStale) { + if (!analysis || isPruningStale || isScanning) { return } const repoIds = analysis.repos @@ -1289,7 +1289,7 @@ export function WorkspaceSpaceManagerPanel(): React.JSX.Element { .finally(() => { setIsPruningStale(false) }) - }, [analysis, isPruningStale, pruneStaleWorktreeRegistrations]) + }, [analysis, isPruningStale, isScanning, pruneStaleWorktreeRegistrations]) const [query, setQuery] = useState('') const [onlyDeletable, setOnlyDeletable] = useState(false) const [sortKey, setSortKey] = useState('size') @@ -1748,7 +1748,7 @@ export function WorkspaceSpaceManagerPanel(): React.JSX.Element { variant="outline" size="sm" onClick={isScanning ? cancelScan : refresh} - disabled={progress?.state === 'cancelling'} + disabled={isPruningStale || progress?.state === 'cancelling'} className="w-28 gap-1.5" > {isScanning ? ( @@ -1829,7 +1829,7 @@ export function WorkspaceSpaceManagerPanel(): React.JSX.Element { variant="outline" size="sm" onClick={pruneStale} - disabled={isPruningStale} + disabled={isPruningStale || isScanning} className="gap-1.5" > {isPruningStale ? ( diff --git a/src/renderer/src/store/slices/workspace-space.test.ts b/src/renderer/src/store/slices/workspace-space.test.ts index 00f6696a5e5..2504da535c6 100644 --- a/src/renderer/src/store/slices/workspace-space.test.ts +++ b/src/renderer/src/store/slices/workspace-space.test.ts @@ -1,5 +1,5 @@ import { create } from 'zustand' -import { describe, expect, it } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' import type { WorkspaceSpaceAnalysis } from '../../../../shared/workspace-space-types' import type { AppState } from '../types' import { createWorkspaceSpaceSlice } from './workspace-space' @@ -89,6 +89,10 @@ function makeAnalysis(): WorkspaceSpaceAnalysis { } describe('workspace space slice', () => { + afterEach(() => { + vi.unstubAllGlobals() + }) + it('removes deleted worktrees from cached analysis totals', () => { const store = createWorkspaceSpaceTestStore() store.setState({ workspaceSpaceAnalysis: makeAnalysis() }) @@ -106,4 +110,30 @@ describe('workspace space slice', () => { reclaimableBytes: 0 }) }) + + it('dedupes shared repo ids, continues after failure, and refreshes once', async () => { + const failure = new Error('remote unavailable') + const pruneStaleRegistrations = vi + .fn<(args: { repoId: string }) => Promise>() + .mockRejectedValueOnce(failure) + .mockResolvedValueOnce(undefined) + const analyze = vi.fn().mockResolvedValue({ ok: true, analysis: makeAnalysis() }) + vi.stubGlobal('window', { + api: { + worktrees: { pruneStaleRegistrations }, + workspaceSpace: { analyze } + } + }) + const store = createWorkspaceSpaceTestStore() + + await expect( + store.getState().pruneStaleWorktreeRegistrations(['repo-1', 'repo-1', 'repo-2']) + ).rejects.toBe(failure) + + expect(pruneStaleRegistrations.mock.calls).toEqual([ + [{ repoId: 'repo-1' }], + [{ repoId: 'repo-2' }] + ]) + expect(analyze).toHaveBeenCalledOnce() + }) }) diff --git a/src/renderer/src/store/slices/workspace-space.ts b/src/renderer/src/store/slices/workspace-space.ts index 3327a0b2930..517b3aabffc 100644 --- a/src/renderer/src/store/slices/workspace-space.ts +++ b/src/renderer/src/store/slices/workspace-space.ts @@ -167,9 +167,23 @@ export const createWorkspaceSpaceSlice: StateCreator