From 7b83b2dcdc80293205cdae3ccbf95b5bd9ef4fe2 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 6 May 2026 23:30:03 -0700 Subject: [PATCH] fix: avoid repeated macOS privacy prompts (#1524) * fix: avoid repeated macos privacy prompts * fix: reduce background worktree permission probes * chore: pin oxlint for ci * fix: preserve optional rpc params with zod 4.4 * fix: preserve optional inline rpc params with zod 4.4 --- src/main/claude-usage/store.ts | 40 +---- src/main/codex-usage/store.ts | 39 +---- src/main/git/status.test.ts | 25 ++- src/main/git/status.ts | 17 +- src/main/ipc/filesystem-auth.ts | 151 ++++++++++++++---- src/main/ipc/filesystem.test.ts | 21 ++- src/main/ipc/filesystem.ts | 4 +- src/main/ipc/repos-create.test.ts | 20 +-- src/main/ipc/repos-remote.test.ts | 2 +- src/main/ipc/repos-sparse-presets.test.ts | 2 +- src/main/ipc/repos.ts | 10 +- src/main/ipc/worktrees-windows.test.ts | 10 +- src/main/ipc/worktrees.ts | 43 +++-- .../runtime/rpc/methods/browser-schemas.ts | 2 + src/main/terminal-history.ts | 8 +- src/main/usage-worktree-metadata.test.ts | 54 +++++++ src/main/usage-worktree-metadata.ts | 66 ++++++++ .../window/attach-main-window-services.ts | 17 +- .../window/history-gc-worktree-ids.test.ts | 18 +++ src/main/window/history-gc-worktree-ids.ts | 7 + src/relay/git-handler-status-ops.ts | 22 ++- src/relay/git-handler-utils.ts | 17 +- src/relay/git-handler.test.ts | 4 + .../right-sidebar/useGitStatusPolling.ts | 41 ++--- .../src/store/slices/worktree-helpers.ts | 4 + .../src/store/slices/worktrees.test.ts | 33 ++++ src/renderer/src/store/slices/worktrees.ts | 61 +++++-- src/shared/types.ts | 2 + 28 files changed, 532 insertions(+), 208 deletions(-) create mode 100644 src/main/usage-worktree-metadata.test.ts create mode 100644 src/main/usage-worktree-metadata.ts create mode 100644 src/main/window/history-gc-worktree-ids.test.ts create mode 100644 src/main/window/history-gc-worktree-ids.ts diff --git a/src/main/claude-usage/store.ts b/src/main/claude-usage/store.ts index da7dfdd2b..dcd143cf6 100644 --- a/src/main/claude-usage/store.ts +++ b/src/main/claude-usage/store.ts @@ -2,7 +2,6 @@ import { app } from 'electron' import { existsSync, mkdirSync, readFileSync, renameSync, writeFileSync } from 'fs' import { dirname, join } from 'path' -import type { Repo } from '../../shared/types' import type { ClaudeUsageBreakdownKind, ClaudeUsageBreakdownRow, @@ -13,16 +12,10 @@ import type { ClaudeUsageSessionRow, ClaudeUsageSummary } from '../../shared/claude-usage-types' -import { listRepoWorktrees } from '../repo-worktrees' import type { Store } from '../persistence' -import { mergeWorktree } from '../ipc/worktree-logic' +import { loadKnownUsageWorktreesByRepo } from '../usage-worktree-metadata' import type { ClaudeUsagePersistedState } from './types' -import { - createWorktreeRefs, - getDefaultWorktreeLabel, - getSessionProjectLabel, - scanClaudeUsageFiles -} from './scanner' +import { createWorktreeRefs, getSessionProjectLabel, scanClaudeUsageFiles } from './scanner' const SCHEMA_VERSION = 1 const STALE_MS = 5 * 60_000 @@ -131,12 +124,6 @@ function getLocalDay(timestamp: string): string | null { return `${year}-${month}-${day}` } -type ClaudeUsageWorktree = { - worktreeId: string - path: string - displayName: string -} - export class ClaudeUsageStore { private state: ClaudeUsagePersistedState private readonly store: Store @@ -226,7 +213,7 @@ export class ClaudeUsageStore { this.scanPromise = (async () => { try { const repos = this.store.getRepos() - const worktreesByRepo = await this.loadWorktreesByRepo(repos) + const worktreesByRepo = loadKnownUsageWorktreesByRepo(this.store, repos) const result = await scanClaudeUsageFiles(createWorktreeRefs(repos, worktreesByRepo)) this.state.processedFiles = result.processedFiles this.state.sessions = result.sessions @@ -502,25 +489,4 @@ export class ClaudeUsageStore { return true }) } - - private async loadWorktreesByRepo(repos: Repo[]): Promise> { - const worktreesByRepo = new Map() - - for (const repo of repos) { - const gitWorktrees = await listRepoWorktrees(repo) - const mapped = gitWorktrees.map((worktree) => { - const worktreeId = `${repo.id}::${worktree.path}` - const meta = this.store.getWorktreeMeta(worktreeId) - const merged = mergeWorktree(repo.id, worktree, meta, repo.displayName) - return { - worktreeId, - path: worktree.path, - displayName: merged.displayName || getDefaultWorktreeLabel(worktree.path) - } - }) - worktreesByRepo.set(repo.id, mapped) - } - - return worktreesByRepo - } } diff --git a/src/main/codex-usage/store.ts b/src/main/codex-usage/store.ts index a2f98de2b..ff14948f0 100644 --- a/src/main/codex-usage/store.ts +++ b/src/main/codex-usage/store.ts @@ -2,7 +2,6 @@ import { app } from 'electron' import { dirname, join } from 'path' import { existsSync, mkdirSync, readFileSync, renameSync, writeFileSync } from 'fs' -import type { Repo } from '../../shared/types' import type { CodexUsageBreakdownKind, CodexUsageBreakdownRow, @@ -13,11 +12,10 @@ import type { CodexUsageSessionRow, CodexUsageSummary } from '../../shared/codex-usage-types' -import { listRepoWorktrees } from '../repo-worktrees' import type { Store } from '../persistence' -import { mergeWorktree } from '../ipc/worktree-logic' +import { loadKnownUsageWorktreesByRepo, type UsageWorktreeRef } from '../usage-worktree-metadata' import type { CodexUsagePersistedState } from './types' -import { createWorktreeRefs, getDefaultWorktreeLabel, scanCodexUsageFiles } from './scanner' +import { createWorktreeRefs, scanCodexUsageFiles } from './scanner' const SCHEMA_VERSION = 2 const STALE_MS = 5 * 60_000 @@ -141,12 +139,6 @@ function getLocalDay(timestamp: string): string | null { return `${year}-${month}-${day}` } -type CodexUsageWorktree = { - worktreeId: string - path: string - displayName: string -} - type ScopedCodexUsageModelRow = { modelKey: string modelLabel: string @@ -159,7 +151,7 @@ type ScopedCodexUsageModelRow = { totalTokens: number } -function getWorktreeFingerprint(worktreesByRepo: Map): string { +function getWorktreeFingerprint(worktreesByRepo: Map): string { const rows = [...worktreesByRepo.entries()] .flatMap(([repoId, worktrees]) => worktrees.map((worktree) => @@ -259,7 +251,7 @@ export class CodexUsageStore { this.scanPromise = (async () => { try { const repos = this.store.getRepos() - const worktreesByRepo = await this.loadWorktreesByRepo(repos) + const worktreesByRepo = loadKnownUsageWorktreesByRepo(this.store, repos) const worktreeFingerprint = getWorktreeFingerprint(worktreesByRepo) const result = await scanCodexUsageFiles( createWorktreeRefs(repos, worktreesByRepo), @@ -591,28 +583,7 @@ export class CodexUsageStore { private async getCurrentWorktreeFingerprint(): Promise { const repos = this.store.getRepos() - const worktreesByRepo = await this.loadWorktreesByRepo(repos) + const worktreesByRepo = loadKnownUsageWorktreesByRepo(this.store, repos) return getWorktreeFingerprint(worktreesByRepo) } - - private async loadWorktreesByRepo(repos: Repo[]): Promise> { - const worktreesByRepo = new Map() - - for (const repo of repos) { - const gitWorktrees = await listRepoWorktrees(repo) - const mapped = gitWorktrees.map((worktree) => { - const worktreeId = `${repo.id}::${worktree.path}` - const meta = this.store.getWorktreeMeta(worktreeId) - const merged = mergeWorktree(repo.id, worktree, meta, repo.displayName) - return { - worktreeId, - path: worktree.path, - displayName: merged.displayName || getDefaultWorktreeLabel(worktree.path) - } - }) - worktreesByRepo.set(repo.id, mapped) - } - - return worktreesByRepo - } } diff --git a/src/main/git/status.test.ts b/src/main/git/status.test.ts index 4efa161df..fb182ea47 100644 --- a/src/main/git/status.test.ts +++ b/src/main/git/status.test.ts @@ -294,13 +294,36 @@ describe('getStatus', () => { // wrapped in double quotes) and the parser would store that literal // string as entry.path, breaking sidebar display + downstream blob reads. expect(gitExecFileAsyncMock).toHaveBeenCalledWith( - ['-c', 'core.quotePath=false', 'status', '--porcelain=v2', '--untracked-files=all'], + [ + '-c', + 'core.quotePath=false', + 'status', + '--porcelain=v2', + '--branch', + '--untracked-files=all' + ], { cwd: '/repo' } ) expect(result.entries).toEqual([ { path: 'docs/日本語/sample.md', status: 'modified', area: 'unstaged' } ]) }) + + it('parses branch identity from porcelain v2 branch headers', async () => { + readFileMock.mockResolvedValue('gitdir: /repo/.git/worktrees/feature\n') + existsSyncMock.mockReturnValue(false) + gitExecFileAsyncMock.mockResolvedValueOnce({ + stdout: + '# branch.oid abcdef1234567890\n# branch.head feature/prompts\n1 .M N... 100644 100644 100644 ce013625030ba8dba906f756967f9e9ca394464a ce013625030ba8dba906f756967f9e9ca394464a src/app.ts\n' + }) + + const result = await getStatus('/repo') + + expect(result).toMatchObject({ + head: 'abcdef1234567890', + branch: 'refs/heads/feature/prompts' + }) + }) }) describe('detectConflictOperation', () => { diff --git a/src/main/git/status.ts b/src/main/git/status.ts index fbdbf298e..4eeda8fab 100644 --- a/src/main/git/status.ts +++ b/src/main/git/status.ts @@ -23,6 +23,8 @@ const MAX_GIT_SHOW_BYTES = 10 * 1024 * 1024 */ export async function getStatus(worktreePath: string): Promise { const entries: GitStatusEntry[] = [] + let head: string | undefined + let branch: string | undefined // Why: detectConflictOperation (4 existsSync + readFile) and git status are // independent. Running them concurrently saves one round-trip of I/O latency. @@ -32,7 +34,7 @@ export async function getStatus(worktreePath: string): Promise // in double quotes. Without it, the parsed entry.path is unreadable in the // sidebar and downstream `git show :"docs/\346..."` lookups silently miss. const statusPromise = gitExecFileAsync( - ['-c', 'core.quotePath=false', 'status', '--porcelain=v2', '--untracked-files=all'], + ['-c', 'core.quotePath=false', 'status', '--porcelain=v2', '--branch', '--untracked-files=all'], { cwd: worktreePath } ) const conflictOperation = await conflictPromise @@ -47,6 +49,17 @@ export async function getStatus(worktreePath: string): Promise continue } + if (line.startsWith('# branch.oid ')) { + head = line.slice('# branch.oid '.length).trim() + continue + } + + if (line.startsWith('# branch.head ')) { + const branchHead = line.slice('# branch.head '.length).trim() + branch = branchHead && branchHead !== '(detached)' ? `refs/heads/${branchHead}` : '' + continue + } + if (line.startsWith('1 ') || line.startsWith('2 ')) { // Changed entries: "1 XY sub mH mI mW hH path" or "2 XY sub mH mI mW hH X\tscore\tpath\torigPath" const parts = line.split(' ') @@ -95,7 +108,7 @@ export async function getStatus(worktreePath: string): Promise // Not a git repo or git not available } - return { entries, conflictOperation } + return { entries, conflictOperation, head, branch } } function parseStatusChar(char: string): GitFileStatus { diff --git a/src/main/ipc/filesystem-auth.ts b/src/main/ipc/filesystem-auth.ts index af9aed31e..88856eefd 100644 --- a/src/main/ipc/filesystem-auth.ts +++ b/src/main/ipc/filesystem-auth.ts @@ -1,5 +1,5 @@ -import { realpath } from 'fs/promises' import { resolve, relative, dirname, basename, isAbsolute } from 'path' +import { realpath } from 'fs/promises' import type { Store } from '../persistence' import { listRepoWorktrees } from '../repo-worktrees' @@ -8,6 +8,8 @@ export const PATH_ACCESS_DENIED_MESSAGE = const authorizedExternalPaths = new Set() const registeredWorktreeRoots = new Set() +const registeredWorktreeRootsByRepo = new Map>() +const registeredWorktreeRootRepoIds = new Set() let registeredWorktreeRootsDirty = true let registeredWorktreeRootsRefresh: Promise | null = null @@ -17,6 +19,18 @@ export function authorizeExternalPath(targetPath: string): void { export function invalidateAuthorizedRootsCache(): void { registeredWorktreeRootsDirty = true + // Why: dirty roots cannot be trusted for auth short-circuits. Fresh + // worktrees:list results will seed safe per-repo roots before a full rebuild. + registeredWorktreeRoots.clear() + registeredWorktreeRootsByRepo.clear() + registeredWorktreeRootRepoIds.clear() +} + +function getLocalRepos(store: Store) { + // Why: SSH repo paths are meaningful on the remote host. Treating them as + // local roots can both authorize unrelated local folders and probe paths + // that Orca should only touch through the SSH provider. + return store.getRepos().filter((repo) => !repo.connectionId) } /** @@ -40,7 +54,7 @@ export function isDescendantOrEqual(resolvedTarget: string, resolvedBase: string } export function getAllowedRoots(store: Store): string[] { - const roots = store.getRepos().map((repo) => resolve(repo.path)) + const roots = getLocalRepos(store).map((repo) => resolve(repo.path)) const workspaceDir = store.getSettings().workspaceDir if (workspaceDir) { roots.push(resolve(workspaceDir)) @@ -67,17 +81,22 @@ export async function rebuildAuthorizedRootsCache(store: Store): Promise { // 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. - const repos = store.getRepos() + // + // Why no realpath() here: this rebuild runs on repo/worktree invalidation, + // so canonicalizing every repo root would repeatedly touch TCC-protected + // folders on macOS even when the user is idle. The actual + // file handlers still canonicalize the specific target path before any + // destructive or read/write operation, so the security boundary remains + // enforced where it matters. + const repos = getLocalRepos(store) const perRepoResults = await Promise.all( repos.map(async (repo) => { const roots: string[] = [] try { - roots.push(await normalizeExistingPath(repo.path)) + roots.push(resolve(repo.path)) const worktrees = await listRepoWorktrees(repo) - const worktreeRoots = await Promise.all( - worktrees.map((wt) => normalizeExistingPath(wt.path)) - ) + const worktreeRoots = worktrees.map((wt) => resolve(wt.path)) roots.push(...worktreeRoots) } catch (error) { // Why: a single inaccessible repo (EACCES, EIO, etc.) must not break @@ -86,19 +105,50 @@ export async function rebuildAuthorizedRootsCache(store: Store): Promise { // the rest proceed. console.warn(`[filesystem-auth] skipping repo ${repo.path} during cache rebuild:`, error) } - return roots + return { repoId: repo.id, roots } }) ) registeredWorktreeRoots.clear() - for (const roots of perRepoResults) { + registeredWorktreeRootsByRepo.clear() + registeredWorktreeRootRepoIds.clear() + for (const { repoId, roots } of perRepoResults) { + const normalizedRoots = new Set() for (const root of roots) { + normalizedRoots.add(root) registeredWorktreeRoots.add(root) } + registeredWorktreeRootsByRepo.set(repoId, normalizedRoots) + registeredWorktreeRootRepoIds.add(repoId) } registeredWorktreeRootsDirty = false } +export function registerWorktreeRootsForRepo( + store: Store, + repoId: string, + worktreeRoots: string[] +): void { + const localRepoIds = new Set(getLocalRepos(store).map((repo) => repo.id)) + for (const registeredRepoId of registeredWorktreeRootsByRepo.keys()) { + if (!localRepoIds.has(registeredRepoId)) { + registeredWorktreeRootsByRepo.delete(registeredRepoId) + registeredWorktreeRootRepoIds.delete(registeredRepoId) + } + } + + if (!localRepoIds.has(repoId)) { + refreshRegisteredWorktreeRoots() + registeredWorktreeRootsDirty = !allLocalRepoRootsRegistered(localRepoIds) + return + } + + registeredWorktreeRootsByRepo.set(repoId, new Set(worktreeRoots.map((root) => resolve(root)))) + registeredWorktreeRootRepoIds.add(repoId) + refreshRegisteredWorktreeRoots() + registeredWorktreeRootsDirty = !allLocalRepoRootsRegistered(localRepoIds) +} + export async function ensureAuthorizedRootsCache(store: Store): Promise { if (!registeredWorktreeRootsDirty) { return @@ -181,26 +231,16 @@ async function isPathAllowedIncludingRegisteredWorktrees( return true } + if (isRegisteredWorktreePath(targetPath)) { + return true + } + await ensureAuthorizedRootsCache(store) // Why: external linked worktrees are already trusted for git operations. // Cache their normalized roots once and reuse that index so quick-open and // file explorer do not spawn `git worktree list` on every filesystem read. - for (const root of registeredWorktreeRoots) { - if (isDescendantOrEqual(targetPath, root)) { - return true - } - } - - return false -} - -async function normalizeExistingPath(targetPath: string): Promise { - try { - return await realpath(targetPath) - } catch { - return resolve(targetPath) - } + return isRegisteredWorktreePath(targetPath) } /** @@ -225,21 +265,66 @@ export async function resolveRegisteredWorktreePath( const resolvedTarget = resolve(worktreePath) - // Resolve through symlinks when the path exists on disk, so that we - // compare canonical paths on both sides (git worktree list also resolves - // symlinks). - const normalizedTarget = await normalizeExistingPath(resolvedTarget) + if (registeredWorktreeRoots.has(resolvedTarget)) { + return resolvedTarget + } - await ensureAuthorizedRootsCache(store) - for (const root of registeredWorktreeRoots) { - if (normalizedTarget === root) { - return normalizedTarget - } + if (registeredWorktreeRootsDirty) { + await ensureAuthorizedRootsCache(store) + } + + if (registeredWorktreeRoots.has(resolvedTarget)) { + return resolvedTarget + } + + // Resolve through symlinks only after the cheap registered-root check. + // On macOS, realpath() can itself trigger TCC prompts for protected roots. + const normalizedTarget = await normalizeExistingPath(resolvedTarget) + if (registeredWorktreeRoots.has(normalizedTarget)) { + return normalizedTarget } throw new Error('Access denied: unknown repository or worktree path') } +function refreshRegisteredWorktreeRoots(): void { + registeredWorktreeRoots.clear() + for (const roots of registeredWorktreeRootsByRepo.values()) { + for (const root of roots) { + registeredWorktreeRoots.add(root) + } + } +} + +function allLocalRepoRootsRegistered(localRepoIds: Set): boolean { + for (const repoId of localRepoIds) { + if (!registeredWorktreeRootRepoIds.has(repoId)) { + return false + } + } + return true +} + +function isRegisteredWorktreePath(targetPath: string): boolean { + for (const root of registeredWorktreeRoots) { + if (isDescendantOrEqual(targetPath, root)) { + return true + } + } + return false +} + +async function normalizeExistingPath(resolvedPath: string): Promise { + try { + return resolve(await realpath(resolvedPath)) + } catch (error) { + if (isENOENT(error)) { + return resolvedPath + } + throw error + } +} + export function validateGitRelativeFilePath(worktreePath: string, filePath: string): string { if (!filePath || filePath.includes('\0') || resolve(filePath) === filePath) { throw new Error('Access denied: invalid git file path') diff --git a/src/main/ipc/filesystem.test.ts b/src/main/ipc/filesystem.test.ts index 8898a5220..de89fcb2d 100644 --- a/src/main/ipc/filesystem.test.ts +++ b/src/main/ipc/filesystem.test.ts @@ -96,7 +96,7 @@ vi.mock('../providers/ssh-git-dispatch', () => ({ })) import { registerFilesystemHandlers } from './filesystem' -import { invalidateAuthorizedRootsCache } from './filesystem-auth' +import { invalidateAuthorizedRootsCache, registerWorktreeRootsForRepo } from './filesystem-auth' // Why: paths are resolved via path.resolve() in production code, so test // data must use resolved paths to avoid Unix-vs-Windows mismatches. @@ -219,6 +219,12 @@ describe('registerFilesystemHandlers', () => { expect(readFileMock).not.toHaveBeenCalled() }) + it('does not enumerate worktrees when filesystem handlers register', () => { + registerFilesystemHandlers(store as never) + + expect(listWorktreesMock).not.toHaveBeenCalled() + }) + it('rejects writes to directories', async () => { lstatMock.mockResolvedValue({ isDirectory: () => true }) @@ -344,6 +350,19 @@ describe('registerFilesystemHandlers', () => { expect(stageFileMock).toHaveBeenCalledWith(WORKTREE_FEATURE_PATH, path.join('src', 'file.ts')) }) + it('uses worktree roots seeded by worktrees:list without rebuilding the cache', async () => { + registerWorktreeRootsForRepo(store as never, 'repo-1', [REPO_PATH, WORKTREE_FEATURE_PATH]) + getStatusMock.mockResolvedValue({ entries: [] }) + + registerFilesystemHandlers(store as never) + + await handlers.get('git:status')!(null, { worktreePath: WORKTREE_FEATURE_PATH }) + + expect(listWorktreesMock).not.toHaveBeenCalled() + expect(realpathMock).not.toHaveBeenCalledWith(WORKTREE_FEATURE_PATH) + expect(getStatusMock).toHaveBeenCalledWith(WORKTREE_FEATURE_PATH) + }) + it('rejects git file paths that escape the selected worktree', async () => { registerFilesystemHandlers(store as never) diff --git a/src/main/ipc/filesystem.ts b/src/main/ipc/filesystem.ts index c50fdb5a2..0309b1d64 100644 --- a/src/main/ipc/filesystem.ts +++ b/src/main/ipc/filesystem.ts @@ -46,8 +46,7 @@ import { resolveRegisteredWorktreePath, validateGitRelativeFilePath, isENOENT, - authorizeExternalPath, - rebuildAuthorizedRootsCache + authorizeExternalPath } from './filesystem-auth' import { listQuickOpenFiles } from './filesystem-list-files' import { registerFilesystemMutationHandlers } from './filesystem-mutations' @@ -106,7 +105,6 @@ async function isBinaryFilePrefix(filePath: string): Promise { } export function registerFilesystemHandlers(store: Store): void { - void rebuildAuthorizedRootsCache(store) const activeTextSearches = new Map() // ─── Filesystem ───────────────────────────────────────── diff --git a/src/main/ipc/repos-create.test.ts b/src/main/ipc/repos-create.test.ts index 460f4b7d6..ef5b78197 100644 --- a/src/main/ipc/repos-create.test.ts +++ b/src/main/ipc/repos-create.test.ts @@ -20,7 +20,7 @@ const { readdirMock, rmMock, gitExecFileAsyncMock, - rebuildAuthorizedRootsCacheMock + invalidateAuthorizedRootsCacheMock } = vi.hoisted(() => ({ handleMock: vi.fn(), removeHandlerMock: vi.fn(), @@ -36,7 +36,7 @@ const { readdirMock: vi.fn(), rmMock: vi.fn(), gitExecFileAsyncMock: vi.fn(), - rebuildAuthorizedRootsCacheMock: vi.fn().mockResolvedValue(undefined) + invalidateAuthorizedRootsCacheMock: vi.fn() })) vi.mock('electron', () => ({ @@ -68,7 +68,7 @@ vi.mock('../git/repo', () => ({ })) vi.mock('./filesystem-auth', () => ({ - rebuildAuthorizedRootsCache: rebuildAuthorizedRootsCacheMock + invalidateAuthorizedRootsCache: invalidateAuthorizedRootsCacheMock })) vi.mock('../providers/ssh-git-dispatch', () => ({ @@ -111,7 +111,7 @@ describe('repos:create', () => { mockStore.getRepos.mockReset().mockReturnValue([]) mockStore.addRepo.mockReset() mockWindow.webContents.send.mockReset() - rebuildAuthorizedRootsCacheMock.mockReset().mockResolvedValue(undefined) + invalidateAuthorizedRootsCacheMock.mockReset() // Default baseline: target does NOT exist yet, mkdir succeeds, git OK. accessMock.mockReset().mockRejectedValue(new Error('ENOENT')) @@ -340,17 +340,17 @@ describe('repos:create', () => { // ── authorized-roots cache refresh ──────────────────────────────── - it('rebuilds the authorized-roots cache after a successful folder create', async () => { - // Roots cache must be refreshed so the renderer can read the new path - // without waiting for the next full reconciliation. + it('invalidates the authorized-roots cache after a successful folder create', async () => { + // Direct repo roots come from store state; invalidation clears stale linked + // roots without scanning every existing repo during creation. await callCreate({ parentPath: '/tmp', name: 'rooted', kind: 'folder' }) - expect(rebuildAuthorizedRootsCacheMock).toHaveBeenCalledTimes(1) + expect(invalidateAuthorizedRootsCacheMock).toHaveBeenCalledTimes(1) }) it('does NOT rebuild the authorized-roots cache on a validation failure', async () => { const result = await callCreate({ parentPath: '/tmp', name: ' ', kind: 'git' }) expect(result).toEqual({ error: 'Name cannot be empty' }) - expect(rebuildAuthorizedRootsCacheMock).not.toHaveBeenCalled() + expect(invalidateAuthorizedRootsCacheMock).not.toHaveBeenCalled() }) it('does NOT rebuild the authorized-roots cache when dedup short-circuits', async () => { @@ -359,7 +359,7 @@ describe('repos:create', () => { await callCreate({ parentPath: '/tmp', name: 'dupe2', kind: 'git' }) - expect(rebuildAuthorizedRootsCacheMock).not.toHaveBeenCalled() + expect(invalidateAuthorizedRootsCacheMock).not.toHaveBeenCalled() }) // ── dedup-by-path ───────────────────────────────────────────────── diff --git a/src/main/ipc/repos-remote.test.ts b/src/main/ipc/repos-remote.test.ts index 6f6f288f5..871a98948 100644 --- a/src/main/ipc/repos-remote.test.ts +++ b/src/main/ipc/repos-remote.test.ts @@ -54,7 +54,7 @@ vi.mock('../git/repo', async () => { }) vi.mock('./filesystem-auth', () => ({ - rebuildAuthorizedRootsCache: vi.fn().mockResolvedValue(undefined) + invalidateAuthorizedRootsCache: vi.fn() })) vi.mock('../providers/ssh-git-dispatch', () => ({ diff --git a/src/main/ipc/repos-sparse-presets.test.ts b/src/main/ipc/repos-sparse-presets.test.ts index 71ab88b6a..cab508738 100644 --- a/src/main/ipc/repos-sparse-presets.test.ts +++ b/src/main/ipc/repos-sparse-presets.test.ts @@ -44,7 +44,7 @@ vi.mock('../git/repo', () => ({ })) vi.mock('./filesystem-auth', () => ({ - rebuildAuthorizedRootsCache: vi.fn().mockResolvedValue(undefined) + invalidateAuthorizedRootsCache: vi.fn() })) vi.mock('../providers/ssh-git-dispatch', () => ({ diff --git a/src/main/ipc/repos.ts b/src/main/ipc/repos.ts index 15ac89ad2..b6239163c 100644 --- a/src/main/ipc/repos.ts +++ b/src/main/ipc/repos.ts @@ -8,7 +8,7 @@ import type { Store } from '../persistence' import type { Repo, BaseRefDefaultResult, SparsePreset } from '../../shared/types' import { isFolderRepo } from '../../shared/repo-kind' import { REPO_COLORS } from '../../shared/constants' -import { rebuildAuthorizedRootsCache } from './filesystem-auth' +import { invalidateAuthorizedRootsCache } from './filesystem-auth' import type { ChildProcess } from 'child_process' import { access, mkdir, readdir, rm } from 'fs/promises' import { gitExecFileAsync, gitSpawn } from '../git/runner' @@ -107,7 +107,7 @@ export function registerRepoHandlers(mainWindow: BrowserWindow, store: Store): v } store.addRepo(repo) - await rebuildAuthorizedRootsCache(store) + invalidateAuthorizedRootsCache() notifyReposChanged(mainWindow) emitRepoAdded('folder_picker', false) return { repo } @@ -404,7 +404,7 @@ export function registerRepoHandlers(mainWindow: BrowserWindow, store: Store): v } store.addRepo(repo) - await rebuildAuthorizedRootsCache(store) + invalidateAuthorizedRootsCache() notifyReposChanged(mainWindow) emitRepoAdded('folder_picker', false) return { repo } @@ -413,7 +413,7 @@ export function registerRepoHandlers(mainWindow: BrowserWindow, store: Store): v ipcMain.handle('repos:remove', async (_event, args: { repoId: string }) => { store.removeRepo(args.repoId) - await rebuildAuthorizedRootsCache(store) + invalidateAuthorizedRootsCache() notifyReposChanged(mainWindow) }) @@ -665,7 +665,7 @@ export function registerRepoHandlers(mainWindow: BrowserWindow, store: Store): v } store.addRepo(repo) - await rebuildAuthorizedRootsCache(store) + invalidateAuthorizedRootsCache() notifyReposChanged(mainWindow) emitRepoAdded('clone_url', false) return repo diff --git a/src/main/ipc/worktrees-windows.test.ts b/src/main/ipc/worktrees-windows.test.ts index 2ce4d4acc..cde115111 100644 --- a/src/main/ipc/worktrees-windows.test.ts +++ b/src/main/ipc/worktrees-windows.test.ts @@ -237,13 +237,9 @@ describe('registerWorktreeHandlers – Windows path handling', () => { isBare: false, isMainWorktree: false } - // Three calls: (1) worktrees:create finds the new worktree, - // (2) rebuildAuthorizedRootsCache enumerates worktrees for the repo, - // (3) worktrees:list enumerates worktrees again. - listWorktreesMock - .mockResolvedValueOnce([worktreeEntry]) - .mockResolvedValueOnce([worktreeEntry]) - .mockResolvedValueOnce([worktreeEntry]) + // Two calls: (1) worktrees:create finds the new worktree, + // (2) worktrees:list enumerates worktrees again. + listWorktreesMock.mockResolvedValueOnce([worktreeEntry]).mockResolvedValueOnce([worktreeEntry]) store.setWorktreeMeta.mockReturnValue({ lastActivityAt: 123, displayName: 'Improve Dashboard' diff --git a/src/main/ipc/worktrees.ts b/src/main/ipc/worktrees.ts index 1d82f5e50..6850538ab 100644 --- a/src/main/ipc/worktrees.ts +++ b/src/main/ipc/worktrees.ts @@ -5,7 +5,13 @@ import { rm } from 'fs/promises' import type { Store } from '../persistence' import { isFolderRepo } from '../../shared/repo-kind' import { deleteWorktreeHistoryDir } from '../terminal-history' -import type { CreateWorktreeArgs, CreateWorktreeResult, WorktreeMeta } from '../../shared/types' +import type { + CreateWorktreeArgs, + CreateWorktreeResult, + GitWorktreeInfo, + Repo, + WorktreeMeta +} from '../../shared/types' import { removeWorktree } from '../git/worktree' import { gitExecFileAsync } from '../git/runner' import { getDefaultRemote } from '../git/repo' @@ -33,7 +39,7 @@ import { createRemoteWorktree, notifyWorktreesChanged } from './worktree-remote' -import { rebuildAuthorizedRootsCache, ensureAuthorizedRootsCache } from './filesystem-auth' +import { invalidateAuthorizedRootsCache, registerWorktreeRootsForRepo } from './filesystem-auth' import type { OrcaRuntimeService } from '../runtime/orca-runtime' import { killAllProcessesForWorktree } from '../runtime/worktree-teardown' import { getLocalPtyProvider } from './pty' @@ -70,6 +76,23 @@ function warnOnce(keySet: Set, key: string, message: string, error?: unk } } +function rememberLocalWorktreeRoots( + store: Store, + repo: Repo, + gitWorktrees: GitWorktreeInfo[] +): void { + if (repo.connectionId) { + return + } + // Why: worktrees:list already paid the `git worktree list` cost. Reusing + // that result keeps later git/file IPC validation from doing a second + // background scan that can trigger macOS folder-permission prompts. + registerWorktreeRootsForRepo(store, repo.id, [ + repo.path, + ...gitWorktrees.map((worktree) => worktree.path) + ]) +} + export function registerWorktreeHandlers( mainWindow: BrowserWindow, store: Store, @@ -90,10 +113,6 @@ export function registerWorktreeHandlers( ipcMain.removeHandler('hooks:writeIssueCommand') ipcMain.handle('worktrees:listAll', async () => { - // Why: use ensureAuthorizedRootsCache (not rebuild) to avoid redundantly - // listing git worktrees when the cache is already fresh — the handler - // itself calls listWorktrees for every repo below. - await ensureAuthorizedRootsCache(store) const repos = store.getRepos() // Why: repos are listed in parallel so total time = slowest repo, not @@ -119,6 +138,7 @@ export function registerWorktreeHandlers( } else { gitWorktrees = await listRepoWorktrees(repo) } + rememberLocalWorktreeRoots(store, repo, gitWorktrees) loggedWorktreeListFailures.delete(`${repo.id}:${repo.path}`) return gitWorktrees.map((gw) => { const worktreeId = `${repo.id}::${gw.path}` @@ -132,6 +152,7 @@ export function registerWorktreeHandlers( `[worktrees] failed to list worktrees for repo "${repo.displayName}" (${repo.id}) at ${repo.path}`, err ) + rememberLocalWorktreeRoots(store, repo, []) return [] } }) @@ -141,10 +162,6 @@ export function registerWorktreeHandlers( }) ipcMain.handle('worktrees:list', async (_event, args: { repoId: string }) => { - // Why: use ensureAuthorizedRootsCache (not rebuild) to avoid redundantly - // listing git worktrees when the cache is already fresh — the handler - // itself calls listWorktrees below. - await ensureAuthorizedRootsCache(store) const repo = store.getRepo(args.repoId) if (!repo) { return [] @@ -175,6 +192,7 @@ export function registerWorktreeHandlers( } else { gitWorktrees = await listRepoWorktrees(repo) } + rememberLocalWorktreeRoots(store, repo, gitWorktrees) loggedWorktreeListFailures.delete(`${repo.id}:${repo.path}`) return gitWorktrees.map((gw) => { const worktreeId = `${repo.id}::${gw.path}` @@ -188,6 +206,7 @@ export function registerWorktreeHandlers( `[worktrees] failed to list worktrees for repo "${repo.displayName}" (${repo.id}) at ${repo.path}`, err ) + rememberLocalWorktreeRoots(store, repo, []) return [] } }) @@ -412,7 +431,7 @@ export function registerWorktreeHandlers( await gitExecFileAsync(['worktree', 'prune'], { cwd: repo.path }).catch(() => {}) store.removeWorktreeMeta(args.worktreeId) deleteWorktreeHistoryDir(args.worktreeId) - await rebuildAuthorizedRootsCache(store) + invalidateAuthorizedRootsCache() notifyWorktreesChanged(mainWindow, repoId) return } @@ -420,7 +439,7 @@ export function registerWorktreeHandlers( } store.removeWorktreeMeta(args.worktreeId) deleteWorktreeHistoryDir(args.worktreeId) - await rebuildAuthorizedRootsCache(store) + invalidateAuthorizedRootsCache() notifyWorktreesChanged(mainWindow, repoId) } diff --git a/src/main/runtime/rpc/methods/browser-schemas.ts b/src/main/runtime/rpc/methods/browser-schemas.ts index 42ebcbc32..96e864bb5 100644 --- a/src/main/runtime/rpc/methods/browser-schemas.ts +++ b/src/main/runtime/rpc/methods/browser-schemas.ts @@ -60,6 +60,7 @@ export const Screenshot = BrowserTarget.extend({ export const FullScreenshot = BrowserTarget.extend({ format: z .unknown() + .optional() .transform((v) => (v === 'jpeg' ? 'jpeg' : 'png')) .pipe(z.enum(['png', 'jpeg'])) }) @@ -166,6 +167,7 @@ export const Check = BrowserTarget.extend({ element: requiredString('Missing required --element'), checked: z .unknown() + .optional() .transform((v) => v !== false) .pipe(z.boolean()) }) diff --git a/src/main/terminal-history.ts b/src/main/terminal-history.ts index ba4ea4e25..63c734452 100644 --- a/src/main/terminal-history.ts +++ b/src/main/terminal-history.ts @@ -309,9 +309,8 @@ function gcScanRoot( return result } -/** Run background GC to prune history directories for worktrees that no - * longer exist. Must be called after live worktree enumeration is complete - * to avoid deleting history for worktrees that haven't been discovered yet. */ +/** Run background GC to prune history directories for worktrees that are no + * longer in Orca's known live-worktree set. */ export function runHistoryGc(liveWorktreeIds: Set): void { try { const main = gcScanRoot(getHistoryRoot(), liveWorktreeIds) @@ -348,8 +347,7 @@ export function runHistoryGc(liveWorktreeIds: Set): void { } /** Schedule GC after a delay so it runs after workspace hydration completes. - * `getLiveWorktreeIds` should enumerate all currently known worktree IDs - * (e.g. by listing repos and their git worktrees). */ + * `getLiveWorktreeIds` should use already-known IDs, not probe repo paths. */ export function scheduleHistoryGc(getLiveWorktreeIds: () => Promise>): void { // Why 10s: avoids competing with startup-critical I/O while still running // early enough to clean up before the user notices disk usage (§7.6). diff --git a/src/main/usage-worktree-metadata.test.ts b/src/main/usage-worktree-metadata.test.ts new file mode 100644 index 000000000..0a6221122 --- /dev/null +++ b/src/main/usage-worktree-metadata.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, it, vi } from 'vitest' +import { loadKnownUsageWorktreesByRepo } from './usage-worktree-metadata' + +describe('loadKnownUsageWorktreesByRepo', () => { + it('builds usage worktree refs from repo roots and persisted metadata', () => { + const store = { + getAllWorktreeMeta: vi.fn(() => ({ + 'repo-1::/workspace/repo-a-feature': { + displayName: 'Feature A' + }, + 'repo-2::/remote/repo-b-feature': { + displayName: 'Remote feature' + }, + malformed: { + displayName: 'Ignored' + } + })) + } + const repos = [ + { + id: 'repo-1', + path: '/workspace/repo-a', + displayName: 'Repo A' + }, + { + id: 'repo-2', + path: '/remote/repo-b', + displayName: 'Remote Repo', + connectionId: 'ssh-1' + } + ] + + expect(loadKnownUsageWorktreesByRepo(store as never, repos as never)).toEqual( + new Map([ + [ + 'repo-1', + [ + { + worktreeId: 'repo-1::/workspace/repo-a', + path: '/workspace/repo-a', + displayName: 'Repo A' + }, + { + worktreeId: 'repo-1::/workspace/repo-a-feature', + path: '/workspace/repo-a-feature', + displayName: 'Feature A' + } + ] + ] + ]) + ) + expect(store.getAllWorktreeMeta).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/main/usage-worktree-metadata.ts b/src/main/usage-worktree-metadata.ts new file mode 100644 index 000000000..f08bc98d6 --- /dev/null +++ b/src/main/usage-worktree-metadata.ts @@ -0,0 +1,66 @@ +import { basename } from 'path' +import type { Repo } from '../shared/types' +import type { Store } from './persistence' + +export type UsageWorktreeRef = { + worktreeId: string + path: string + displayName: string +} + +function getDefaultUsageWorktreeLabel(pathValue: string): string { + return basename(pathValue) +} + +function parseKnownWorktreeId(worktreeId: string): { repoId: string; worktreePath: string } | null { + const sepIdx = worktreeId.indexOf('::') + if (sepIdx === -1) { + return null + } + return { + repoId: worktreeId.slice(0, sepIdx), + worktreePath: worktreeId.slice(sepIdx + 2) + } +} + +export function loadKnownUsageWorktreesByRepo( + store: Pick, + repos: Repo[] +): Map { + const localRepos = repos.filter((repo) => !repo.connectionId) + const repoIds = new Set(localRepos.map((repo) => repo.id)) + const worktreesByRepo = new Map() + const seenPathsByRepo = new Map>() + + for (const repo of localRepos) { + worktreesByRepo.set(repo.id, [ + { + worktreeId: `${repo.id}::${repo.path}`, + path: repo.path, + displayName: repo.displayName || getDefaultUsageWorktreeLabel(repo.path) + } + ]) + seenPathsByRepo.set(repo.id, new Set([repo.path])) + } + + // Why: usage scans are background/opt-in analytics. Do not spawn + // `git worktree list` here; it can re-touch macOS protected folders. + for (const [worktreeId, meta] of Object.entries(store.getAllWorktreeMeta())) { + const parsed = parseKnownWorktreeId(worktreeId) + if (!parsed || !repoIds.has(parsed.repoId)) { + continue + } + const seenPaths = seenPathsByRepo.get(parsed.repoId) + if (seenPaths?.has(parsed.worktreePath)) { + continue + } + seenPaths?.add(parsed.worktreePath) + worktreesByRepo.get(parsed.repoId)?.push({ + worktreeId, + path: parsed.worktreePath, + displayName: meta.displayName || getDefaultUsageWorktreeLabel(parsed.worktreePath) + }) + } + + return worktreesByRepo +} diff --git a/src/main/window/attach-main-window-services.ts b/src/main/window/attach-main-window-services.ts index 0d9b54a79..08d34ebb1 100644 --- a/src/main/window/attach-main-window-services.ts +++ b/src/main/window/attach-main-window-services.ts @@ -23,8 +23,8 @@ import { dismissNudge } from '../updater' import { scheduleHistoryGc } from '../terminal-history' -import { listRepoWorktrees } from '../repo-worktrees' import type { ClaudeRuntimeAuthPreparation } from '../claude-accounts/runtime-auth-service' +import { getKnownWorktreeIdsForHistoryGc } from './history-gc-worktree-ids' export function attachMainWindowServices( mainWindow: BrowserWindow, @@ -50,19 +50,10 @@ export function attachMainWindowServices( // and ensures the handlers are re-installed on macOS app re-activation when // the main window is recreated. registerDaemonManagementHandlers() - // Why: GC runs on a 10s delay so live worktree enumeration completes first. - // Uses git worktree list (not store.getWorktreeMeta) because untouched - // worktrees have no metadata entries — see design doc §7.6. + // Why: do not enumerate repo paths from background GC. `git worktree list` + // can re-touch protected folders on macOS and trigger folder-access prompts. scheduleHistoryGc(async () => { - const repos = store.getRepos() - const ids = new Set() - for (const repo of repos) { - const worktrees = await listRepoWorktrees(repo) - for (const wt of worktrees) { - ids.add(`${repo.id}::${wt.path}`) - } - } - return ids + return getKnownWorktreeIdsForHistoryGc(store) }) registerSshHandlers(store, () => mainWindow, runtime) registerFileDropRelay(mainWindow) diff --git a/src/main/window/history-gc-worktree-ids.test.ts b/src/main/window/history-gc-worktree-ids.test.ts new file mode 100644 index 000000000..1aca7d585 --- /dev/null +++ b/src/main/window/history-gc-worktree-ids.test.ts @@ -0,0 +1,18 @@ +import { describe, expect, it, vi } from 'vitest' +import { getKnownWorktreeIdsForHistoryGc } from './history-gc-worktree-ids' + +describe('getKnownWorktreeIdsForHistoryGc', () => { + it('uses persisted metadata keys without probing repo paths', () => { + const store = { + getAllWorktreeMeta: vi.fn(() => ({ + 'repo-1::/worktree-a': {}, + 'repo-2::/worktree-b': {} + })) + } + + expect(getKnownWorktreeIdsForHistoryGc(store as never)).toEqual( + new Set(['repo-1::/worktree-a', 'repo-2::/worktree-b']) + ) + expect(store.getAllWorktreeMeta).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/main/window/history-gc-worktree-ids.ts b/src/main/window/history-gc-worktree-ids.ts new file mode 100644 index 000000000..beec2f9cf --- /dev/null +++ b/src/main/window/history-gc-worktree-ids.ts @@ -0,0 +1,7 @@ +import type { Store } from '../persistence' + +export function getKnownWorktreeIdsForHistoryGc( + store: Pick +): Set { + return new Set(Object.keys(store.getAllWorktreeMeta())) +} diff --git a/src/relay/git-handler-status-ops.ts b/src/relay/git-handler-status-ops.ts index 98aa116f8..509fd780d 100644 --- a/src/relay/git-handler-status-ops.ts +++ b/src/relay/git-handler-status-ops.ts @@ -49,11 +49,18 @@ export async function getStatusOp( git: GitExec, validatePath: (p: string) => void, params: Record -): Promise<{ entries: Record[]; conflictOperation: string }> { +): Promise<{ + entries: Record[] + conflictOperation: string + head?: string + branch?: string +}> { const worktreePath = params.worktreePath as string validatePath(worktreePath) const conflictOperation = await detectConflictOperation(worktreePath) const entries: Record[] = [] + let head: string | undefined + let branch: string | undefined try { // Why: -c core.quotePath=false keeps non-ASCII filenames as raw UTF-8 in @@ -61,11 +68,20 @@ export async function getStatusOp( // entry.path renders as gibberish in the source-control sidebar and // downstream blob lookups miss. const { stdout } = await git( - ['-c', 'core.quotePath=false', 'status', '--porcelain=v2', '--untracked-files=all'], + [ + '-c', + 'core.quotePath=false', + 'status', + '--porcelain=v2', + '--branch', + '--untracked-files=all' + ], worktreePath ) const parsed = parseStatusOutput(stdout) entries.push(...parsed.entries) + head = parsed.head + branch = parsed.branch for (const uLine of parsed.unmergedLines) { const entry = parseUnmergedEntry(worktreePath, uLine) @@ -77,5 +93,5 @@ export async function getStatusOp( // not a git repo or git not available } - return { entries, conflictOperation } + return { entries, conflictOperation, head, branch } } diff --git a/src/relay/git-handler-utils.ts b/src/relay/git-handler-utils.ts index 292ee494e..d51119736 100644 --- a/src/relay/git-handler-utils.ts +++ b/src/relay/git-handler-utils.ts @@ -72,15 +72,30 @@ export function parseConflictKind(xy: string): string | null { export function parseStatusOutput(stdout: string): { entries: Record[] unmergedLines: string[] + head?: string + branch?: string } { const entries: Record[] = [] const unmergedLines: string[] = [] + let head: string | undefined + let branch: string | undefined for (const line of stdout.split(/\r?\n/)) { if (!line) { continue } + if (line.startsWith('# branch.oid ')) { + head = line.slice('# branch.oid '.length).trim() + continue + } + + if (line.startsWith('# branch.head ')) { + const branchHead = line.slice('# branch.head '.length).trim() + branch = branchHead && branchHead !== '(detached)' ? `refs/heads/${branchHead}` : '' + continue + } + if (line.startsWith('1 ') || line.startsWith('2 ')) { const parts = line.split(' ') const xy = parts[1] @@ -130,7 +145,7 @@ export function parseStatusOutput(stdout: string): { } } - return { entries, unmergedLines } + return { entries, unmergedLines, head, branch } } /** diff --git a/src/relay/git-handler.test.ts b/src/relay/git-handler.test.ts index 514b92c3d..e950639a1 100644 --- a/src/relay/git-handler.test.ts +++ b/src/relay/git-handler.test.ts @@ -65,9 +65,13 @@ describe('GitHandler', () => { const result = (await dispatcher.callRequest('git.status', { worktreePath: tmpDir })) as { entries: Record[] conflictOperation: string + head?: string + branch?: string } expect(result.entries).toEqual([]) expect(result.conflictOperation).toBe('unknown') + expect(result.branch).toMatch(/^refs\/heads\//) + expect(typeof result.head).toBe('string') }) it('detects untracked files', async () => { diff --git a/src/renderer/src/components/right-sidebar/useGitStatusPolling.ts b/src/renderer/src/components/right-sidebar/useGitStatusPolling.ts index 97a0f88e8..eacd404a2 100644 --- a/src/renderer/src/components/right-sidebar/useGitStatusPolling.ts +++ b/src/renderer/src/components/right-sidebar/useGitStatusPolling.ts @@ -11,7 +11,7 @@ export function useGitStatusPolling(): void { const activeWorktree = useActiveWorktree() const allWorktrees = useAllWorktrees() const activeWorktreeId = useAppStore((s) => s.activeWorktreeId) - const fetchWorktrees = useAppStore((s) => s.fetchWorktrees) + const updateWorktreeGitIdentity = useAppStore((s) => s.updateWorktreeGitIdentity) const setGitStatus = useAppStore((s) => s.setGitStatus) const fetchUpstreamStatus = useAppStore((s) => s.fetchUpstreamStatus) const setConflictOperation = useAppStore((s) => s.setConflictOperation) @@ -56,11 +56,25 @@ export function useGitStatusPolling(): void { connectionId })) as GitStatusResult setGitStatus(activeWorktreeId, status) + // Why: branch switches can happen inside a terminal. `git status + // --branch` gives us the new identity without a separate worktree-list + // poll that would repeatedly touch repo/worktree roots. + updateWorktreeGitIdentity(activeWorktreeId, { + head: status.head, + branch: status.branch + }) await fetchUpstreamStatus(activeWorktreeId, worktreePath, connectionId) } catch { // ignore } - }, [activeRepoSupportsGit, activeWorktreeId, fetchUpstreamStatus, worktreePath, setGitStatus]) + }, [ + activeRepoSupportsGit, + activeWorktreeId, + fetchUpstreamStatus, + worktreePath, + setGitStatus, + updateWorktreeGitIdentity + ]) useEffect(() => { void fetchStatus() @@ -83,29 +97,6 @@ export function useGitStatusPolling(): void { } }, [fetchStatus]) - useEffect(() => { - if (!activeRepoId || !activeRepoSupportsGit) { - return - } - - // Why: checkout/switch operations happen inside the terminal, outside the - // renderer's normal worktree-change events. Poll the active repo's worktree - // list so a branch change updates the sidebar's PR key instead of leaving - // the previous merged PR attached to this worktree indefinitely. - void fetchWorktrees(activeRepoId) - const intervalId = setInterval(() => { - if (document.hasFocus()) { - void fetchWorktrees(activeRepoId) - } - }, POLL_INTERVAL_MS) - const onFocus = (): void => void fetchWorktrees(activeRepoId) - window.addEventListener('focus', onFocus) - return () => { - clearInterval(intervalId) - window.removeEventListener('focus', onFocus) - } - }, [activeRepoId, activeRepoSupportsGit, fetchWorktrees]) - // Why: poll conflict operation for non-active worktrees that have a stale // non-unknown operation. This is a lightweight fs-only check (no git status) // so it won't cause performance issues even with many worktrees. diff --git a/src/renderer/src/store/slices/worktree-helpers.ts b/src/renderer/src/store/slices/worktree-helpers.ts index 1f4d45026..dee101781 100644 --- a/src/renderer/src/store/slices/worktree-helpers.ts +++ b/src/renderer/src/store/slices/worktree-helpers.ts @@ -108,6 +108,10 @@ export type WorktreeSlice = { * one-shot at hydration time. See design §4.4. */ purgeWorktreeTerminalState: (worktreeIds: string[]) => void + updateWorktreeGitIdentity: ( + worktreeId: string, + identity: { head?: string; branch?: string } + ) => void } export function findWorktreeById( diff --git a/src/renderer/src/store/slices/worktrees.test.ts b/src/renderer/src/store/slices/worktrees.test.ts index 57b101527..3a84f4b47 100644 --- a/src/renderer/src/store/slices/worktrees.test.ts +++ b/src/renderer/src/store/slices/worktrees.test.ts @@ -167,6 +167,37 @@ describe('fetchWorktrees', () => { }) }) +describe('updateWorktreeGitIdentity', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('updates branch identity from git status without fetching worktrees', () => { + const store = createTestStore() + const existing = makeWorktree({ + id: 'repo1::/path/wt1', + repoId: 'repo1', + path: '/path/wt1', + head: 'old-head', + branch: 'refs/heads/main' + }) + + store.setState({ worktreesByRepo: { repo1: [existing] }, sortEpoch: 3 } as Partial) + + store.getState().updateWorktreeGitIdentity('repo1::/path/wt1', { + head: 'new-head', + branch: 'refs/heads/feature' + }) + + expect(store.getState().worktreesByRepo.repo1[0]).toMatchObject({ + head: 'new-head', + branch: 'refs/heads/feature' + }) + expect(store.getState().sortEpoch).toBe(4) + expect(mockApi.worktrees.list).not.toHaveBeenCalled() + }) +}) + describe('removeWorktree state cleanup', () => { beforeEach(() => { vi.clearAllMocks() @@ -652,6 +683,7 @@ describe('fetchAllWorktrees hydration-time purge (design §4.4)', () => { await store.getState().fetchAllWorktrees() expect(store.getState().hasHydratedWorktreePurge).toBe(true) + expect(mockApi.worktrees.list).toHaveBeenCalledTimes(2) expect(store.getState().tabsByWorktree).toEqual({ 'repoA::/a/wt1': [{ id: 'tab-A', worktreeId: 'repoA::/a/wt1' }], 'repoB::/b/wt1': [{ id: 'tab-B', worktreeId: 'repoB::/b/wt1' }] @@ -667,6 +699,7 @@ describe('fetchAllWorktrees hydration-time purge (design §4.4)', () => { await store.getState().fetchAllWorktrees() + expect(mockApi.worktrees.list).toHaveBeenCalledTimes(4) expect(store.getState().tabsByWorktree['repoA::/a/new-zombie']).toBeDefined() }) }) diff --git a/src/renderer/src/store/slices/worktrees.ts b/src/renderer/src/store/slices/worktrees.ts index 1e8dc8092..f9f6e98c3 100644 --- a/src/renderer/src/store/slices/worktrees.ts +++ b/src/renderer/src/store/slices/worktrees.ts @@ -114,24 +114,24 @@ export const createWorktreeSlice: StateCreator // (design §2c), which means the user would still need a second restart // post-upgrade to reclaim memory. // - // Safety gate: fetchWorktrees swallows IPC errors (catch at :93-95) - // and short-circuits on empty-replace when cached data exists - // (empty-guard at :82-84). Neither signal bubbles up to the caller, so - // we can't distinguish "threw" from "returned []" from "returned ≥1" - // by inspecting post-state alone. If we declared the union of - // worktreesByRepo authoritative without confirming every repo - // succeeded, a single transient git error at launch would wipe every - // tabsByWorktree entry for the affected repo — the exact data-loss - // class the empty-guard exists to prevent. Probe the IPC directly to - // get the precise per-repo result, and defer the purge until every - // repo returns success AND at least one has >0 worktrees. In steady - // state this fires on the first fully-successful launch; in the - // degraded state it simply waits. + // Safety gate: fetchWorktrees swallows IPC errors and short-circuits on + // empty-replace when cached data exists. Neither signal bubbles up to the + // caller, so we probe the IPC directly to get the per-repo success signal, + // then apply that same payload to state instead of listing each repo again. const results = await Promise.all( repos.map(async (r) => { try { const list = await window.api.worktrees.list({ repoId: r.id }) - await get().fetchWorktrees(r.id) + const current = get().worktreesByRepo[r.id] + if ( + !areWorktreesEqual(current, list) && + !(list.length === 0 && current && current.length > 0) + ) { + set((s) => ({ + worktreesByRepo: { ...s.worktreesByRepo, [r.id]: list }, + sortEpoch: s.sortEpoch + 1 + })) + } return { repoId: r.id, ok: list.length > 0 } } catch (err) { console.error(`Failed to fetch worktrees for repo ${r.id}:`, err) @@ -162,6 +162,39 @@ export const createWorktreeSlice: StateCreator set({ hasHydratedWorktreePurge: true }) }, + updateWorktreeGitIdentity: (worktreeId, identity) => { + set((s) => { + const repoId = getRepoIdFromWorktreeId(worktreeId) + const current = s.worktreesByRepo[repoId] + if (!current) { + return {} + } + + let changed = false + const next = current.map((worktree) => { + if (worktree.id !== worktreeId) { + return worktree + } + const nextHead = identity.head ?? worktree.head + const nextBranch = identity.branch ?? worktree.branch + if (nextHead === worktree.head && nextBranch === worktree.branch) { + return worktree + } + changed = true + return { ...worktree, head: nextHead, branch: nextBranch } + }) + + if (!changed) { + return {} + } + + return { + worktreesByRepo: { ...s.worktreesByRepo, [repoId]: next }, + sortEpoch: s.sortEpoch + 1 + } + }) + }, + createWorktree: async ( repoId, name, diff --git a/src/shared/types.ts b/src/shared/types.ts index 5184fe82f..aca54b0c1 100644 --- a/src/shared/types.ts +++ b/src/shared/types.ts @@ -1586,6 +1586,8 @@ export type GitStatusEntry = GitUncommittedEntry export type GitStatusResult = { entries: GitStatusEntry[] conflictOperation: GitConflictOperation + head?: string + branch?: string } // Why: when hasUpstream is false, ahead/behind are placeholder zeros, not a