perf: preserve unrelated authorization roots after desktop removal (#26294)

This commit is contained in:
Neil
2026-10-07 22:42:11 -07:00
committed by GitHub
parent 9f64453943
commit 8155e2e69f
3 changed files with 76 additions and 4 deletions
@@ -1,10 +1,13 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
import * as filesystemAuth from './filesystem-auth'
import * as authorizedRootsCache from './registered-worktree-roots-cache'
import * as localWorktreeFilesystem from '../local-worktree-filesystem'
import { lstat, mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import type { GitWorktreeInfo } from '../../shared/worktree/types'
import type { RedactableSpan } from '../observability/redactor'
import type { Store } from '../persistence'
import { _resetTracerForTests, setActiveSink } from '../observability/tracer'
import { agentHookServer } from '../agent-hooks/server'
import { makePaneKey } from '../../shared/stable-pane-id'
@@ -26,7 +29,13 @@ import {
getLocalPtyProviderMock,
getSshPtyProviderMock
} from './worktrees-test-module-mocks'
import { handlers, mainWindow, setupWorktreeHandlers, store } from './worktrees-test-harness'
import {
handlers,
harnessRepo,
mainWindow,
setupWorktreeHandlers,
store
} from './worktrees-test-harness'
import { makeWorktreeMeta, mockKnownFeatureWorktree } from './worktrees-test-fixtures'
import type { WorktreeRuntimeStub } from './worktrees-test-runtime-stub'
@@ -120,15 +129,47 @@ describe('registerWorktreeHandlers', () => {
beforeEach(() => {
runtimeStub = setupWorktreeHandlers()
vi.spyOn(filesystemAuth, 'invalidateAuthorizedRootsCacheForRepo').mockClear()
vi.spyOn(authorizedRootsCache, 'invalidateAuthorizedRootsCache').mockClear()
})
it('prunes the persisted cleanup and space snapshots on removal', async () => {
mockKnownFeatureWorktree()
getEffectiveHooksMock.mockReturnValue(null)
removeWorktreeMock.mockResolvedValue({})
const unrelatedRepo = { ...harnessRepo, id: 'repo-2', path: '/unrelated/repo' }
store.getRepos.mockReturnValue([harnessRepo, unrelatedRepo])
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: this existing IPC store fixture supplies the repository, project, folder, and settings methods the root registry reads.
const cacheStore = store as unknown as Store
authorizedRootsCache.registerWorktreeRootsForRepo(cacheStore, 'repo-1', [
'/workspace/feature-wt'
])
authorizedRootsCache.registerWorktreeRootsForRepo(cacheStore, 'repo-2', ['/unrelated/worktree'])
const removedRevision = authorizedRootsCache.getRegisteredWorktreeRootsRevision('repo-1')
const unrelatedRevision = authorizedRootsCache.getRegisteredWorktreeRootsRevision('repo-2')
await handlers['worktrees:remove'](null, { worktreeId: 'repo-1::/workspace/feature-wt' })
expect(authorizedRootsCache.getRegisteredWorktreeRootsRevision('repo-1')).toBeGreaterThan(
removedRevision
)
expect(authorizedRootsCache.getRegisteredWorktreeRootsRevision('repo-2')).toBe(
unrelatedRevision
)
expect(authorizedRootsCache.isRegisteredWorktreePath('/workspace/feature-wt', cacheStore)).toBe(
false
)
expect(authorizedRootsCache.isRegisteredWorktreePath('/unrelated/worktree', cacheStore)).toBe(
true
)
expect(filesystemAuth.invalidateAuthorizedRootsCacheForRepo).toHaveBeenCalledOnce()
expect(filesystemAuth.invalidateAuthorizedRootsCacheForRepo).toHaveBeenCalledWith(
store,
'repo-1'
)
expect(authorizedRootsCache.invalidateAuthorizedRootsCache).not.toHaveBeenCalled()
// A removed workspace must never resurrect from the cached scan snapshots.
expect(pruneCleanupScanSnapshotMock).toHaveBeenCalledWith(
'/profile-a',
@@ -142,6 +183,22 @@ describe('registerWorktreeHandlers', () => {
)
})
it('globally invalidates when the removed repo has no registered authorization owner', async () => {
mockKnownFeatureWorktree()
store.getRepos.mockReturnValue([])
getEffectiveHooksMock.mockReturnValue(null)
removeWorktreeMock.mockResolvedValue({})
await handlers['worktrees:remove'](null, { worktreeId: 'repo-1::/workspace/feature-wt' })
expect(filesystemAuth.invalidateAuthorizedRootsCacheForRepo).toHaveBeenCalledOnce()
expect(filesystemAuth.invalidateAuthorizedRootsCacheForRepo).toHaveBeenCalledWith(
store,
'repo-1'
)
expect(authorizedRootsCache.invalidateAuthorizedRootsCache).toHaveBeenCalledOnce()
})
it('purges only the selected host when a normal worktree id is owned locally and over SSH', async () => {
const worktreeId = 'repo-1::/workspace/feature-wt'
const localRepo = {
@@ -290,6 +347,13 @@ describe('registerWorktreeHandlers', () => {
worktreeId: 'repo-1::/workspace/feature-wt'
})
expect(filesystemAuth.invalidateAuthorizedRootsCacheForRepo).toHaveBeenCalledOnce()
expect(filesystemAuth.invalidateAuthorizedRootsCacheForRepo).toHaveBeenCalledWith(
store,
'repo-1'
)
expect(authorizedRootsCache.invalidateAuthorizedRootsCache).not.toHaveBeenCalled()
// Should have called git worktree prune to clean up stale tracking
expect(gitExecFileAsyncMock).toHaveBeenCalledWith(['worktree', 'prune'], {
cwd: '/workspace/repo'
@@ -1,4 +1,6 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
import * as filesystemAuth from './filesystem-auth'
import * as authorizedRootsCache from './registered-worktree-roots-cache'
import {
removeWorktreeLinkedPathsMock,
findExistingWorktreeSymlinkPathsMock,
@@ -104,6 +106,8 @@ describe('registerWorktreeHandlers', () => {
beforeEach(() => {
runtimeStub = setupWorktreeHandlers()
vi.spyOn(filesystemAuth, 'invalidateAuthorizedRootsCacheForRepo').mockClear()
vi.spyOn(authorizedRootsCache, 'invalidateAuthorizedRootsCache').mockClear()
})
it('fails dirty non-force deletes before PTY teardown', async () => {
@@ -139,6 +143,8 @@ describe('registerWorktreeHandlers', () => {
expect(removeWorktreeLinkedPathsMock).not.toHaveBeenCalled()
expect(killAllProcessesForWorktreeMock).not.toHaveBeenCalled()
expect(removeWorktreeMock).not.toHaveBeenCalled()
expect(filesystemAuth.invalidateAuthorizedRootsCacheForRepo).not.toHaveBeenCalled()
expect(authorizedRootsCache.invalidateAuthorizedRootsCache).not.toHaveBeenCalled()
})
it('propagates a timed-out removal preflight before watcher teardown', async () => {
@@ -200,6 +206,8 @@ describe('registerWorktreeHandlers', () => {
expect(removeWorktreeLinkedPathsMock).not.toHaveBeenCalled()
expect(killAllProcessesForWorktreeMock).not.toHaveBeenCalled()
expect(removeWorktreeMock).not.toHaveBeenCalled()
expect(filesystemAuth.invalidateAuthorizedRootsCacheForRepo).not.toHaveBeenCalled()
expect(authorizedRootsCache.invalidateAuthorizedRootsCache).not.toHaveBeenCalled()
})
it('rechecks a local Git lock after the archive hook before teardown', async () => {
@@ -22,7 +22,7 @@ import {
findExistingWorktreeSymlinkPaths,
removeWorktreeLinkedPaths
} from '../../worktree-symlinks'
import { invalidateAuthorizedRootsCache } from '../../registered-worktree-roots-cache'
import { invalidateAuthorizedRootsCacheForRepo } from '../../filesystem-auth'
import { runWorktreeChangeInvalidators } from '../../worktree-change-invalidators'
import {
formatWorktreeRemovalError,
@@ -276,7 +276,7 @@ async function finishLocalWorktreeRemoval({
hostId: removalHostId
})
)
invalidateAuthorizedRootsCache()
invalidateAuthorizedRootsCacheForRepo(store, repoId)
removalCompleted = true
return {}
} else {
@@ -315,7 +315,7 @@ async function finishLocalWorktreeRemoval({
)
})
await withWorktreeRemoveStageSpan('cache_invalidation', 'local', async () => {
invalidateAuthorizedRootsCache()
invalidateAuthorizedRootsCacheForRepo(store, repoId)
})
return removalResult ?? {}
}