From b0a2a96a53629410ffbe1fce68a280c577cb1b83 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 31 May 2026 07:23:26 -0700 Subject: [PATCH] perf: bound filesystem auth cache rebuild (#4186) --- src/main/ipc/filesystem-auth.test.ts | 26 ++++++++++++++++++-- src/main/ipc/filesystem-auth.ts | 36 +++++++++++++++++++++------- 2 files changed, 52 insertions(+), 10 deletions(-) diff --git a/src/main/ipc/filesystem-auth.test.ts b/src/main/ipc/filesystem-auth.test.ts index d59f23fa61d..5d8e7094eb9 100644 --- a/src/main/ipc/filesystem-auth.test.ts +++ b/src/main/ipc/filesystem-auth.test.ts @@ -32,9 +32,9 @@ const repo: Repo = { kind: 'git' } -function makeStore(): Store { +function makeStore(repos: Repo[] = [repo]): Store { return { - getRepos: () => [repo], + getRepos: () => repos, getSettings: () => ({}) } as unknown as Store } @@ -67,6 +67,28 @@ describe('filesystem auth worktree roots', () => { ) expect(listRepoWorktrees).toHaveBeenCalledTimes(1) }) + + it('bounds concurrent repo probes while rebuilding authorized roots', async () => { + const repos = Array.from({ length: 20 }, (_, index) => ({ + ...repo, + id: `repo-${index}`, + path: `/repos/app-${index}` + })) + let active = 0 + let maxActive = 0 + vi.mocked(listRepoWorktrees).mockImplementation(async () => { + active += 1 + maxActive = Math.max(maxActive, active) + await new Promise((resolve) => setTimeout(resolve, 1)) + active -= 1 + return [] + }) + + await rebuildAuthorizedRootsCache(makeStore(repos)) + + expect(listRepoWorktrees).toHaveBeenCalledTimes(repos.length) + expect(maxActive).toBeLessThanOrEqual(8) + }) }) describe('filesystem-auth path containment', () => { diff --git a/src/main/ipc/filesystem-auth.ts b/src/main/ipc/filesystem-auth.ts index 3e4a84598c4..814215a231b 100644 --- a/src/main/ipc/filesystem-auth.ts +++ b/src/main/ipc/filesystem-auth.ts @@ -16,6 +16,7 @@ const registeredWorktreeRootsByRepo = new Map>() const registeredWorktreeRootRepoIds = new Set() let registeredWorktreeRootsDirty = true let registeredWorktreeRootsRefresh: Promise | null = null +const AUTHORIZED_ROOTS_REBUILD_CONCURRENCY = 8 export function authorizeExternalPath(targetPath: string): void { const resolvedTarget = resolve(targetPath) @@ -91,11 +92,8 @@ export function isPathAllowed(targetPath: string, store: Store): boolean { } export async function rebuildAuthorizedRootsCache(store: Store): Promise { - // Why: repos are processed in parallel so the cache rebuild completes in - // wall-clock time proportional to the slowest single repo, not the sum of - // all repos. The previous sequential loop was the main bottleneck on - // Windows where each `git worktree list` + realpath chain takes 500 ms+ - // due to slower process creation and antivirus I/O scanning. + // Why: repos are processed with bounded parallelism so the cache rebuild + // keeps the Windows speedup without spawning one git process per repo. // // Why no realpath() here: this rebuild runs on repo/worktree invalidation, // so canonicalizing every repo root would repeatedly touch TCC-protected @@ -104,8 +102,10 @@ export async function rebuildAuthorizedRootsCache(store: Store): Promise { // destructive or read/write operation, so the security boundary remains // enforced where it matters. const repos = getLocalRepos(store) - const perProjectResults = await Promise.all( - repos.map(async (repo) => { + const perProjectResults = await mapWithConcurrency( + repos, + AUTHORIZED_ROOTS_REBUILD_CONCURRENCY, + async (repo) => { const roots: string[] = [] try { roots.push(resolve(repo.path)) @@ -121,7 +121,7 @@ export async function rebuildAuthorizedRootsCache(store: Store): Promise { console.warn(`[filesystem-auth] skipping repo ${repo.path} during cache rebuild:`, error) } return { repoId: repo.id, roots } - }) + } ) registeredWorktreeRoots.clear() @@ -139,6 +139,26 @@ export async function rebuildAuthorizedRootsCache(store: Store): Promise { registeredWorktreeRootsDirty = false } +async function mapWithConcurrency( + items: readonly T[], + maxConcurrent: number, + mapper: (item: T) => Promise +): Promise { + const results: R[] = [] + let nextIndex = 0 + const workerCount = Math.min(maxConcurrent, items.length) + await Promise.all( + Array.from({ length: workerCount }, async () => { + while (nextIndex < items.length) { + const index = nextIndex + nextIndex += 1 + results[index] = await mapper(items[index]) + } + }) + ) + return results +} + export function registerWorktreeRootsForRepo( store: Store, repoId: string,