mirror of
https://github.com/stablyai/orca.git
synced 2026-09-23 16:02:24 +00:00
Neil
84584b61d0
* perf(git): cache sparse-checkout annotation on worktree listing `git worktree list` never reports sparse-checkout state, so every listing paid a per-worktree fs.stat + config read to detect it -- measured at ~9x the cost of the `git worktree list` call it decorates on a 1000-worktree repo. Cache the result per worktree path, invalidated by the existing worktree-change invalidator registry plus explicit remove/move hooks, with a 5-minute reconcile window bounding the one unwitnessed edge case (external `git sparse-checkout` toggle with extensions.worktreeConfig off), matching the precedent already accepted in readRepoWorktreeAdminFingerprint. * perf(git): normalize/scope sparse-checkout cache keys, add SWR Address independent-review follow-ups on the sparse-checkout annotation cache (#17859): - Extract canonicalWorktreePath() from areWorktreePathsEqual and key/invalidate the cache through it on both read and write, closing the disclosed path-spelling P2 outright instead of leaving it as a residual risk. - Scope cache entries and clears by repo path (derived from the invalidator registry's repoId via a store lookup, falling back to a full clear when the repo can't be resolved), so churn in one repo no longer evicts a sibling repo's warm cache. - Replace the hard 5-minute cutoff with stale-while-revalidate: past the window, callers get the cached value immediately while a deduplicated background probe corrects it and, on a flip, drives the existing worktrees-changed notification -- collapsing visible staleness from the full window to one refresh cycle at zero added listing latency. Also corrects a stale claim in the original PR description: newer Git does emit a `sparse` porcelain line (which annotateSparseCheckoutStatus already skips), but Orca's Git 2.25 compatibility baseline predates it, so the fallback detection this caches remains necessary. * fix(git): stop background sparse-checkout revalidation resurrecting invalidated entries Readiness-loop finding: a stale-while-revalidate probe in flight when a worktree is removed/moved (or a repo's cache is cleared) would still write its result back afterward, resurrecting an entry that was deliberately dropped. Guard the write with a presence check so an invalidated key stays absent until the next real read. * fix(git): identity-check the sparse-checkout SWR write-back guard The has()/presence guard from the previous commit only proved some entry existed at the key, not that it was the one this revalidation started from. A worktree removed and re-created at the same path while a background re-detect was in flight would repopulate the key with a fresh cold read, and the stale in-flight result would then overwrite it -- exactly the race greptile (P1) and pullfrog both flagged as still open. Compare the map's current entry by reference to the entry captured when the revalidation began; a mismatch means something else (invalidate, clear, or a fresh cold read) replaced it, and the stale result must not be written back. Added a regression test that fails against the old has() guard and passes with the identity check: invalidate and repopulate the key with a different value mid-flight, then let the stale revalidation settle and assert the fresh value survives.
89 lines
3.2 KiB
TypeScript
89 lines
3.2 KiB
TypeScript
// moveWorktree: relocating a checkout via `git worktree move`.
|
|
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
|
|
|
const {
|
|
gitExecFileAsyncMock,
|
|
gitExecFileSyncMock,
|
|
translateWslOutputPathsMock,
|
|
moveWorktreeDirectoryToTrashMock,
|
|
detectSparseCheckoutMock
|
|
} = vi.hoisted(() => ({
|
|
gitExecFileAsyncMock: vi.fn(),
|
|
gitExecFileSyncMock: vi.fn(),
|
|
translateWslOutputPathsMock: vi.fn((output: string) => output),
|
|
moveWorktreeDirectoryToTrashMock: vi.fn(),
|
|
detectSparseCheckoutMock: vi.fn()
|
|
}))
|
|
|
|
vi.mock('./runner', () => ({
|
|
gitExecFileAsync: gitExecFileAsyncMock,
|
|
gitExecFileSync: gitExecFileSyncMock,
|
|
translateWslOutputPaths: translateWslOutputPathsMock
|
|
}))
|
|
|
|
// Default: the checkout cannot be renamed aside, so removal deletes it in place.
|
|
vi.mock('../worktree-trash', () => ({
|
|
moveWorktreeDirectoryToTrash: moveWorktreeDirectoryToTrashMock.mockResolvedValue(undefined),
|
|
restoreWorktreeDirectoryFromTrash: vi.fn().mockResolvedValue(true),
|
|
scheduleWorktreeTrashDeletion: vi.fn()
|
|
}))
|
|
|
|
vi.mock('./worktree-sparse-state', () => ({
|
|
detectSparseCheckout: detectSparseCheckoutMock,
|
|
resolveGitCommonDir: vi.fn()
|
|
}))
|
|
|
|
import { moveWorktree } from './worktree'
|
|
import { registerWorktreeSuiteHooks } from './worktree-test-harness'
|
|
import {
|
|
__getSparseCheckoutStateCacheSizeForTests,
|
|
detectSparseCheckoutCached
|
|
} from './worktree-sparse-checkout-cache'
|
|
|
|
registerWorktreeSuiteHooks()
|
|
|
|
describe('moveWorktree', () => {
|
|
beforeEach(() => {
|
|
gitExecFileAsyncMock.mockReset()
|
|
})
|
|
|
|
it('runs `git worktree move` from the repo with old and new paths', async () => {
|
|
gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: '', stderr: '' })
|
|
await moveWorktree('/repo', '/ws/cunner', '/ws/worktree-creation-spinner')
|
|
expect(gitExecFileAsyncMock).toHaveBeenCalledWith(
|
|
['worktree', 'move', '/ws/cunner', '/ws/worktree-creation-spinner'],
|
|
{ cwd: '/repo' }
|
|
)
|
|
})
|
|
|
|
it('propagates git failures so the caller can fall back', async () => {
|
|
gitExecFileAsyncMock.mockRejectedValueOnce(new Error('fatal: destination exists'))
|
|
await expect(moveWorktree('/repo', '/ws/cunner', '/ws/taken')).rejects.toThrow(
|
|
'destination exists'
|
|
)
|
|
})
|
|
|
|
it('drops cached sparse-checkout state for both the old and new path', async () => {
|
|
detectSparseCheckoutMock.mockResolvedValue(true)
|
|
await detectSparseCheckoutCached('/repo', '/ws/cunner')
|
|
await detectSparseCheckoutCached('/repo', '/ws/worktree-creation-spinner')
|
|
expect(__getSparseCheckoutStateCacheSizeForTests()).toBe(2)
|
|
|
|
gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: '', stderr: '' })
|
|
await moveWorktree('/repo', '/ws/cunner', '/ws/worktree-creation-spinner')
|
|
|
|
expect(__getSparseCheckoutStateCacheSizeForTests()).toBe(0)
|
|
})
|
|
|
|
it('drops cached sparse-checkout state for both paths even when the move fails', async () => {
|
|
detectSparseCheckoutMock.mockResolvedValue(true)
|
|
await detectSparseCheckoutCached('/repo', '/ws/cunner')
|
|
await detectSparseCheckoutCached('/repo', '/ws/taken')
|
|
|
|
gitExecFileAsyncMock.mockRejectedValueOnce(new Error('fatal: destination exists'))
|
|
await expect(moveWorktree('/repo', '/ws/cunner', '/ws/taken')).rejects.toThrow()
|
|
|
|
expect(__getSparseCheckoutStateCacheSizeForTests()).toBe(0)
|
|
})
|
|
})
|