Prune detected worktree scan generation markers (#7679)
* Prune detected worktree scan generations * Invalidate pending scans in test reset hook Co-authored-by: Orca <help@stably.ai> --------- Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Neil
GitHub
parent
a92fd26c96
commit
19b63c4cb4
|
|
@ -246,7 +246,11 @@ import {
|
|||
notifyWorktreesChanged
|
||||
} from './worktree-remote'
|
||||
import { invalidateAuthorizedRootsCache, resolveRegisteredWorktreePath } from './filesystem-auth'
|
||||
import { __resetDetectedWorktreeScanCacheForTests, registerWorktreeHandlers } from './worktrees'
|
||||
import {
|
||||
__getDetectedWorktreeScanCacheStatsForTests,
|
||||
__resetDetectedWorktreeScanCacheForTests,
|
||||
registerWorktreeHandlers
|
||||
} from './worktrees'
|
||||
|
||||
type HandlerMap = Record<string, (_event: unknown, args: unknown) => unknown>
|
||||
|
||||
|
|
@ -2633,6 +2637,199 @@ describe('registerWorktreeHandlers', () => {
|
|||
expect(listWorktreesMock).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('does not retain invalidated detected scans after they settle', async () => {
|
||||
let resolveScan: (worktrees: GitWorktreeInfo[]) => void = () => {}
|
||||
listWorktreesMock.mockImplementation(
|
||||
() =>
|
||||
new Promise((resolve) => {
|
||||
resolveScan = resolve as (worktrees: GitWorktreeInfo[]) => void
|
||||
})
|
||||
)
|
||||
|
||||
const pendingList = handlers['worktrees:listDetected'](null, { repoId: 'repo-1' })
|
||||
await Promise.resolve()
|
||||
|
||||
expect(__getDetectedWorktreeScanCacheStatsForTests()).toMatchObject({
|
||||
cacheSize: 0,
|
||||
inFlightSize: 1
|
||||
})
|
||||
|
||||
notifyWorktreesChanged(mainWindow as never, 'repo-1')
|
||||
|
||||
expect(__getDetectedWorktreeScanCacheStatsForTests()).toMatchObject({
|
||||
cacheSize: 0,
|
||||
inFlightSize: 0
|
||||
})
|
||||
|
||||
resolveScan([
|
||||
{
|
||||
path: '/workspace/repo',
|
||||
head: 'main-head',
|
||||
branch: 'refs/heads/main',
|
||||
isBare: false,
|
||||
isMainWorktree: true
|
||||
}
|
||||
])
|
||||
await pendingList
|
||||
|
||||
expect(__getDetectedWorktreeScanCacheStatsForTests()).toMatchObject({
|
||||
cacheSize: 0,
|
||||
inFlightSize: 0
|
||||
})
|
||||
})
|
||||
|
||||
it('does not accumulate scan bookkeeping across prolonged repository churn', async () => {
|
||||
store.getRepo.mockImplementation((repoId: string) => ({
|
||||
id: repoId,
|
||||
path: `/workspace/${repoId}`,
|
||||
displayName: repoId,
|
||||
badgeColor: '#000',
|
||||
addedAt: 0,
|
||||
worktreeBaseRef: null
|
||||
}))
|
||||
listWorktreesMock.mockImplementation(async (repoPath: string) => [
|
||||
{
|
||||
path: repoPath,
|
||||
head: 'main-head',
|
||||
branch: 'refs/heads/main',
|
||||
isBare: false,
|
||||
isMainWorktree: true
|
||||
}
|
||||
])
|
||||
|
||||
for (let index = 0; index < 128; index += 1) {
|
||||
const repoId = `repo-${index}`
|
||||
await handlers['worktrees:listDetected'](null, { repoId })
|
||||
notifyWorktreesChanged(mainWindow as never, repoId)
|
||||
}
|
||||
|
||||
expect(__getDetectedWorktreeScanCacheStatsForTests()).toEqual({
|
||||
cacheSize: 0,
|
||||
inFlightSize: 0
|
||||
})
|
||||
expect(listWorktreesMock).toHaveBeenCalledTimes(128)
|
||||
})
|
||||
|
||||
it('keeps a replacement scan current after an older scan settles first', async () => {
|
||||
const resolvers: ((worktrees: GitWorktreeInfo[]) => void)[] = []
|
||||
listWorktreesMock.mockImplementation(
|
||||
() =>
|
||||
new Promise((resolve) => {
|
||||
resolvers.push(resolve as (worktrees: GitWorktreeInfo[]) => void)
|
||||
})
|
||||
)
|
||||
const result = [
|
||||
{
|
||||
path: '/workspace/repo',
|
||||
head: 'main-head',
|
||||
branch: 'refs/heads/main',
|
||||
isBare: false,
|
||||
isMainWorktree: true
|
||||
}
|
||||
]
|
||||
|
||||
const staleList = handlers['worktrees:listDetected'](null, { repoId: 'repo-1' })
|
||||
await Promise.resolve()
|
||||
notifyWorktreesChanged(mainWindow as never, 'repo-1')
|
||||
const replacementList = handlers['worktrees:listDetected'](null, { repoId: 'repo-1' })
|
||||
await Promise.resolve()
|
||||
|
||||
resolvers[0](result)
|
||||
await staleList
|
||||
expect(__getDetectedWorktreeScanCacheStatsForTests()).toEqual({
|
||||
cacheSize: 0,
|
||||
inFlightSize: 1
|
||||
})
|
||||
|
||||
resolvers[1](result)
|
||||
await replacementList
|
||||
expect(__getDetectedWorktreeScanCacheStatsForTests()).toEqual({
|
||||
cacheSize: 1,
|
||||
inFlightSize: 0
|
||||
})
|
||||
})
|
||||
|
||||
it('does not let an older scan overwrite a replacement that settles first', async () => {
|
||||
const resolvers: ((worktrees: GitWorktreeInfo[]) => void)[] = []
|
||||
listWorktreesMock.mockImplementation(
|
||||
() =>
|
||||
new Promise((resolve) => {
|
||||
resolvers.push(resolve as (worktrees: GitWorktreeInfo[]) => void)
|
||||
})
|
||||
)
|
||||
store.getAllWorktreeLineage.mockReturnValue({
|
||||
'repo-1::/workspace/fresh-worktree': {
|
||||
worktreeId: 'repo-1::/workspace/fresh-worktree',
|
||||
worktreeInstanceId: 'child-instance',
|
||||
parentWorktreeId: 'repo-1::/workspace/repo',
|
||||
parentWorktreeInstanceId: 'parent-instance',
|
||||
origin: 'manual',
|
||||
capture: {
|
||||
source: 'manual-action',
|
||||
confidence: 'explicit'
|
||||
},
|
||||
createdAt: 0
|
||||
}
|
||||
})
|
||||
const mainWorktree: GitWorktreeInfo = {
|
||||
path: '/workspace/repo',
|
||||
head: 'main-head',
|
||||
branch: 'refs/heads/main',
|
||||
isBare: false,
|
||||
isMainWorktree: true
|
||||
}
|
||||
|
||||
const staleList = handlers['worktrees:listDetected'](null, { repoId: 'repo-1' })
|
||||
await Promise.resolve()
|
||||
notifyWorktreesChanged(mainWindow as never, 'repo-1')
|
||||
const replacementList = handlers['worktrees:listDetected'](null, { repoId: 'repo-1' })
|
||||
await Promise.resolve()
|
||||
|
||||
resolvers[1]([
|
||||
mainWorktree,
|
||||
{
|
||||
path: '/workspace/fresh-worktree',
|
||||
head: 'fresh-head',
|
||||
branch: 'refs/heads/fresh-worktree',
|
||||
isBare: false,
|
||||
isMainWorktree: false
|
||||
}
|
||||
])
|
||||
await replacementList
|
||||
|
||||
resolvers[0]([
|
||||
mainWorktree,
|
||||
{
|
||||
path: '/workspace/stale-worktree',
|
||||
head: 'stale-head',
|
||||
branch: 'refs/heads/stale-worktree',
|
||||
isBare: false,
|
||||
isMainWorktree: false
|
||||
}
|
||||
])
|
||||
await staleList
|
||||
|
||||
const cached = (await handlers['worktrees:listDetected'](null, {
|
||||
repoId: 'repo-1'
|
||||
})) as { worktrees: Worktree[] }
|
||||
expect(cached.worktrees.map((worktree) => worktree.path)).toEqual([
|
||||
'/workspace/repo',
|
||||
'/workspace/fresh-worktree'
|
||||
])
|
||||
expect(store.removeWorktreeLineage).not.toHaveBeenCalled()
|
||||
await expect(
|
||||
resolveRegisteredWorktreePath('/workspace/fresh-worktree', store as never)
|
||||
).resolves.toBe(resolve('/workspace/fresh-worktree'))
|
||||
await expect(
|
||||
resolveRegisteredWorktreePath('/workspace/stale-worktree', store as never)
|
||||
).rejects.toThrow('Access denied: unknown repository or worktree path')
|
||||
expect(listWorktreesMock).toHaveBeenCalledTimes(2)
|
||||
expect(__getDetectedWorktreeScanCacheStatsForTests()).toEqual({
|
||||
cacheSize: 1,
|
||||
inFlightSize: 0
|
||||
})
|
||||
})
|
||||
|
||||
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[]) => {
|
||||
|
|
|
|||
|
|
@ -469,37 +469,59 @@ type DetectedWorktreeScanCacheEntry = {
|
|||
worktrees: GitWorktreeInfo[]
|
||||
}
|
||||
|
||||
type DetectedWorktreeScan = {
|
||||
invalidated: boolean
|
||||
promise: Promise<GitWorktreeInfo[]>
|
||||
}
|
||||
|
||||
type DetectedWorktreeScanResult = {
|
||||
gitWorktrees: GitWorktreeInfo[]
|
||||
fresh: boolean
|
||||
}
|
||||
|
||||
const detectedWorktreeScanCache = new Map<string, DetectedWorktreeScanCacheEntry>()
|
||||
const detectedWorktreeScanInFlight = new Map<string, Promise<GitWorktreeInfo[]>>()
|
||||
const detectedWorktreeScanGenerations = new Map<string, number>()
|
||||
const detectedWorktreeScanInFlight = new Map<string, DetectedWorktreeScan>()
|
||||
|
||||
function invalidateDetectedWorktreeScanCache(repoId: string): void {
|
||||
const keyPrefix = `${repoId}\0`
|
||||
for (const key of new Set([
|
||||
...detectedWorktreeScanCache.keys(),
|
||||
...detectedWorktreeScanInFlight.keys(),
|
||||
...detectedWorktreeScanGenerations.keys()
|
||||
...detectedWorktreeScanInFlight.keys()
|
||||
])) {
|
||||
if (!key.startsWith(keyPrefix)) {
|
||||
continue
|
||||
}
|
||||
detectedWorktreeScanCache.delete(key)
|
||||
detectedWorktreeScanInFlight.delete(key)
|
||||
detectedWorktreeScanGenerations.set(key, (detectedWorktreeScanGenerations.get(key) ?? 0) + 1)
|
||||
const inFlight = detectedWorktreeScanInFlight.get(key)
|
||||
if (inFlight) {
|
||||
// Why: the detached scan keeps this token, so later scans can settle
|
||||
// without making an older result fresh again.
|
||||
inFlight.invalidated = true
|
||||
detectedWorktreeScanInFlight.delete(key)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
registerWorktreeChangeInvalidator(invalidateDetectedWorktreeScanCache)
|
||||
|
||||
export function __resetDetectedWorktreeScanCacheForTests(): void {
|
||||
// Why: scans still pending across a test reset must not repopulate the
|
||||
// cache afterward and leak state into the next test.
|
||||
for (const scan of detectedWorktreeScanInFlight.values()) {
|
||||
scan.invalidated = true
|
||||
}
|
||||
detectedWorktreeScanCache.clear()
|
||||
detectedWorktreeScanInFlight.clear()
|
||||
detectedWorktreeScanGenerations.clear()
|
||||
}
|
||||
|
||||
export function __getDetectedWorktreeScanCacheStatsForTests(): {
|
||||
cacheSize: number
|
||||
inFlightSize: number
|
||||
} {
|
||||
return {
|
||||
cacheSize: detectedWorktreeScanCache.size,
|
||||
inFlightSize: detectedWorktreeScanInFlight.size
|
||||
}
|
||||
}
|
||||
|
||||
async function listDetectedGitWorktrees(
|
||||
|
|
@ -522,24 +544,25 @@ async function listDetectedGitWorktrees(
|
|||
|
||||
const inFlight = detectedWorktreeScanInFlight.get(cacheKey)
|
||||
if (inFlight) {
|
||||
return { gitWorktrees: await inFlight, fresh: false }
|
||||
return { gitWorktrees: await inFlight.promise, fresh: false }
|
||||
}
|
||||
|
||||
const scan = listRepoWorktrees(repo, localWorktreeGitOptions)
|
||||
const generation = detectedWorktreeScanGenerations.get(cacheKey) ?? 0
|
||||
const scan: DetectedWorktreeScan = {
|
||||
invalidated: false,
|
||||
promise: listRepoWorktrees(repo, localWorktreeGitOptions)
|
||||
}
|
||||
detectedWorktreeScanInFlight.set(cacheKey, scan)
|
||||
try {
|
||||
const gitWorktrees = await scan
|
||||
const gitWorktrees = await scan.promise
|
||||
// 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(cacheKey) ?? 0) === generation
|
||||
if (isCurrentGeneration) {
|
||||
if (!scan.invalidated) {
|
||||
detectedWorktreeScanCache.set(cacheKey, {
|
||||
worktrees: gitWorktrees,
|
||||
expiresAt: Date.now() + DETECTED_WORKTREE_SCAN_CACHE_TTL_MS
|
||||
})
|
||||
}
|
||||
return { gitWorktrees, fresh: isCurrentGeneration }
|
||||
return { gitWorktrees, fresh: !scan.invalidated }
|
||||
} finally {
|
||||
if (detectedWorktreeScanInFlight.get(cacheKey) === scan) {
|
||||
detectedWorktreeScanInFlight.delete(cacheKey)
|
||||
|
|
|
|||
Loading…
Reference in New Issue