mirror of
https://github.com/stablyai/orca.git
synced 2026-10-19 08:03:13 +00:00
fix(space): harden stale worktree pruning
This commit is contained in:
@@ -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<typeof RunnerModule>()),
|
||||
gitExecFileAsync: gitExecFileAsyncMock
|
||||
}))
|
||||
|
||||
vi.mock('./status', async (importOriginal) => ({
|
||||
...(await importOriginal<typeof StatusModule>()),
|
||||
runWithGitReadCacheInvalidation: <T>(run: () => Promise<T>): Promise<T> => 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)
|
||||
})
|
||||
})
|
||||
@@ -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)
|
||||
|
||||
@@ -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}`,
|
||||
|
||||
+29
-17
@@ -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()
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
@@ -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
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -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<string, unknown>) {
|
||||
private async pruneWorktrees(params: Record<string, unknown>, 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 {}
|
||||
}
|
||||
|
||||
|
||||
@@ -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<WorkspaceSpaceSortKey>('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 ? (
|
||||
|
||||
@@ -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<void>>()
|
||||
.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()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -167,9 +167,23 @@ export const createWorkspaceSpaceSlice: StateCreator<AppState, [], [], Workspace
|
||||
return
|
||||
}
|
||||
get().recordFeatureInteraction?.('workspace-cleanup')
|
||||
for (const repoId of repoIds) {
|
||||
await window.api.worktrees.pruneStaleRegistrations({ repoId })
|
||||
let firstError: unknown
|
||||
// Why: one project id can appear once per host, while IPC already fans that id out to all hosts.
|
||||
for (const repoId of new Set(repoIds)) {
|
||||
try {
|
||||
await window.api.worktrees.pruneStaleRegistrations({ repoId })
|
||||
} catch (error) {
|
||||
firstError ??= error
|
||||
}
|
||||
}
|
||||
try {
|
||||
// Why: successful repos must disappear from the notice even when another host failed.
|
||||
await get().refreshWorkspaceSpace()
|
||||
} catch (error) {
|
||||
firstError ??= error
|
||||
}
|
||||
if (firstError) {
|
||||
throw firstError
|
||||
}
|
||||
await get().refreshWorkspaceSpace()
|
||||
}
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user