From c19604012a76d533119fbc1261fb4e9b6de36765 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 14 Oct 2026 16:50:01 -0700 Subject: [PATCH] fix(worktrees): check missing roots at Git listing boundary --- .../worktree-list-reader-cancellation.test.ts | 19 +++++++++++++++++-- src/main/git/worktree-list-reader.ts | 16 ++++++++++++++++ src/main/git/worktree-listing.ts | 4 ++++ .../detected-scan-failure-authority.test.ts | 10 +++++++++- src/main/repo-worktrees.test.ts | 15 +++++---------- src/main/repo-worktrees.ts | 8 +------- .../repo-worktree-resolution-scan.test.ts | 15 --------------- .../runtime/repo-worktree-resolution-scan.ts | 8 -------- 8 files changed, 52 insertions(+), 43 deletions(-) diff --git a/src/main/git/worktree-list-reader-cancellation.test.ts b/src/main/git/worktree-list-reader-cancellation.test.ts index d5a46730e5d..06346e0afd8 100644 --- a/src/main/git/worktree-list-reader-cancellation.test.ts +++ b/src/main/git/worktree-list-reader-cancellation.test.ts @@ -3,9 +3,14 @@ import type * as FsPromises from 'node:fs/promises' import type { GitWorktreeInfo } from '../../shared/worktree/types' import type { gitExecFileAsync } from './runner' -const { statProbe, gitExec } = vi.hoisted(() => ({ +const { statProbe, gitExec, missingPathProbe } = vi.hoisted(() => ({ statProbe: vi.fn<(worktreePath: string) => Promise>(), - gitExec: vi.fn() + gitExec: vi.fn(), + missingPathProbe: vi.fn<() => Promise>() +})) + +vi.mock('../local-worktree-filesystem', () => ({ + isLocalWorktreePathMissing: missingPathProbe })) vi.mock('node:fs/promises', async (importOriginal) => ({ @@ -22,6 +27,7 @@ vi.mock('../../shared/git-worktree-admin', () => ({ import { clearGitCapabilityStateForTests } from './git-capability-state' import { readWorktreeList } from './worktree-list-reader' +import { listWorktreesStrictAllowingTrueEmpty } from './worktree-listing' function porcelainRow(worktreePath: string, markers: string[] = []): string { return [ @@ -43,6 +49,7 @@ beforeEach(() => { clearGitCapabilityStateForTests() statProbe.mockReset() gitExec.mockReset() + missingPathProbe.mockReset().mockResolvedValue(false) porcelain = porcelainRow('/repo') + Array.from({ length: 32 }, (_, index) => porcelainRow(`/repo/task-${index}`)).join('') @@ -54,6 +61,14 @@ beforeEach(() => { }) }) +it('skips Git for a missing repo path and scans after the path returns', async () => { + missingPathProbe.mockResolvedValueOnce(true).mockResolvedValueOnce(false) + await expect(listWorktreesStrictAllowingTrueEmpty('/repo')).resolves.toEqual([]) + expect(gitExec).not.toHaveBeenCalled() + await readWorktreeList('/repo') + expect(gitExec).toHaveBeenCalled() +}) + describe('old-Git native worktree listing cancellation', () => { it('rejects before pending existence probes finish and prevents further probes', async () => { const controller = new AbortController() diff --git a/src/main/git/worktree-list-reader.ts b/src/main/git/worktree-list-reader.ts index d6bf5c2b2a3..04394fb4782 100644 --- a/src/main/git/worktree-list-reader.ts +++ b/src/main/git/worktree-list-reader.ts @@ -22,9 +22,18 @@ import { translateWorktreePath } from './worktree-path-comparison' import { gitExecFileAsync } from './runner' +import { isLocalWorktreePathMissing } from '../local-worktree-filesystem' const PRUNABLE_EXISTENCE_PROBE_CONCURRENCY = 8 +export class MissingWorktreePathError extends Error { + readonly code = 'ENOENT' + + constructor(repoPath: string) { + super(`Repository path missing: ${repoPath}`) + } +} + type RepoLocation = { topLevel: string; commonDir: string; gitDir: string } function parseRepoLocation(repoPath: string, output: string): RepoLocation | undefined { @@ -209,6 +218,13 @@ export async function readWorktreeList( repoPath: string, options: GitWorktreeExecOptions = {} ): Promise { + throwIfSignalAborted(options.signal) + // A host check may be unverifiable; only positive absence skips Git. + const missingPath = await isLocalWorktreePathMissing(repoPath, options).catch(() => false) + throwIfSignalAborted(options.signal) + if (missingPath) { + throw new MissingWorktreePathError(repoPath) + } const execOptions = { cwd: repoPath, ...options, diff --git a/src/main/git/worktree-listing.ts b/src/main/git/worktree-listing.ts index 5dd1a4e29c7..a0d5c86eceb 100644 --- a/src/main/git/worktree-listing.ts +++ b/src/main/git/worktree-listing.ts @@ -9,6 +9,7 @@ import { toWslExecutionSpace } from '../../shared/wsl-paths' import type { GitWorktreeInfo } from '../../shared/worktree/types' import { readCheckedOutBranchRef, + MissingWorktreePathError, readRepoCommonDirFromGit, readRepoLocation, readTranslatedWorktreeGraph, @@ -70,6 +71,9 @@ export async function listWorktreesUnshared( * path is gone, or it is not a Git repo. Every other failure means the scan could not read Git. */ async function isTrueEmptyWorktreeListing(repoPath: string, err: unknown): Promise { + if (err instanceof MissingWorktreePathError) { + return true + } if (getErrorCode(err) === 'ENOENT') { try { await stat(repoPath) diff --git a/src/main/ipc/worktrees/listing/detected-scan-failure-authority.test.ts b/src/main/ipc/worktrees/listing/detected-scan-failure-authority.test.ts index 5c600a0f8ff..f05773bc9f9 100644 --- a/src/main/ipc/worktrees/listing/detected-scan-failure-authority.test.ts +++ b/src/main/ipc/worktrees/listing/detected-scan-failure-authority.test.ts @@ -2,7 +2,14 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import type { Repo } from '../../../../shared/repo-types' import type { DetectedWorktreeListResult } from '../../../../shared/worktree/types' -const gitExecFileAsyncMock = vi.hoisted(() => vi.fn()) +const { gitExecFileAsyncMock, missingPathProbe } = vi.hoisted(() => ({ + gitExecFileAsyncMock: vi.fn(), + missingPathProbe: vi.fn() +})) + +vi.mock('../../../local-worktree-filesystem', () => ({ + isLocalWorktreePathMissing: missingPathProbe +})) vi.mock('electron', () => ({ ipcMain: { handle: vi.fn(), removeHandler: vi.fn() }, @@ -66,6 +73,7 @@ async function listDetected(): Promise { describe('detected worktree listing authority', () => { beforeEach(() => { gitExecFileAsyncMock.mockReset() + missingPathProbe.mockReset().mockResolvedValue(false) removeWorktreeLineage.mockReset() __resetDetectedWorktreeScanCacheForTests() _resetWorktreeScanCacheForTests() diff --git a/src/main/repo-worktrees.test.ts b/src/main/repo-worktrees.test.ts index 97580b5cccb..31bdd90b0a7 100644 --- a/src/main/repo-worktrees.test.ts +++ b/src/main/repo-worktrees.test.ts @@ -1,14 +1,10 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' -const { listWorktreeGraphMock, listWorktreesMock, listWorktreesStrictMock, missingPathMock } = - vi.hoisted(() => ({ - listWorktreeGraphMock: vi.fn(), - listWorktreesMock: vi.fn(), - listWorktreesStrictMock: vi.fn(), - missingPathMock: vi.fn() - })) - -vi.mock('./local-worktree-filesystem', () => ({ isLocalWorktreePathMissing: missingPathMock })) +const { listWorktreeGraphMock, listWorktreesMock, listWorktreesStrictMock } = vi.hoisted(() => ({ + listWorktreeGraphMock: vi.fn(), + listWorktreesMock: vi.fn(), + listWorktreesStrictMock: vi.fn() +})) vi.mock('./git/worktree', () => ({ listWorktreeGraph: listWorktreeGraphMock, @@ -32,7 +28,6 @@ describe('repo-worktrees', () => { listWorktreeGraphMock.mockReset() listWorktreesMock.mockReset() listWorktreesStrictMock.mockReset() - missingPathMock.mockReset().mockResolvedValue(false) }) it('creates a stable synthetic worktree for folder repos', () => { diff --git a/src/main/repo-worktrees.ts b/src/main/repo-worktrees.ts index f70436bb239..e92a8e50612 100644 --- a/src/main/repo-worktrees.ts +++ b/src/main/repo-worktrees.ts @@ -12,7 +12,6 @@ import { getRepoExecutionHostId, LOCAL_EXECUTION_HOST_ID } from '../shared/execu import { resolveGitRouteForHost } from './providers/execution-host-provider-dispatch' import { areWorktreePathsEqual } from './ipc/worktree-logic' import { WorktreeCatalogUnavailableError } from '../shared/worktree/worktree-catalog-availability' -import { isLocalWorktreePathMissing } from './local-worktree-filesystem' type LocalRepoWorktreeListOptions = { wslDistro?: string @@ -62,12 +61,7 @@ export async function listRepoWorktreesForDetectedScan( repo: Repo, options?: LocalRepoWorktreeListOptions ): Promise { - return listRoutedRepoWorktrees(repo, options, async (repoPath, localOptions) => { - if (await isLocalWorktreePathMissing(repoPath, localOptions)) { - return [] - } - return listWorktreesSharedStrictAllowingTrueEmpty(repoPath, localOptions) - }) + return listRoutedRepoWorktrees(repo, options, listWorktreesSharedStrictAllowingTrueEmpty) } async function listRoutedRepoWorktrees( diff --git a/src/main/runtime/repo-worktree-resolution-scan.test.ts b/src/main/runtime/repo-worktree-resolution-scan.test.ts index e1a76d65169..9d85d07fd58 100644 --- a/src/main/runtime/repo-worktree-resolution-scan.test.ts +++ b/src/main/runtime/repo-worktree-resolution-scan.test.ts @@ -2,26 +2,11 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import { listWorktreesStrict } from '../git/worktree' import { scanLocalRepoWorktreesForResolution } from './repo-worktree-resolution-scan' -const { missingPathMock } = vi.hoisted(() => ({ missingPathMock: vi.fn() })) - vi.mock('../git/worktree', () => ({ listWorktreesStrict: vi.fn() })) -vi.mock('../local-worktree-filesystem', () => ({ - isLocalWorktreePathMissing: missingPathMock -})) describe('scanLocalRepoWorktreesForResolution', () => { beforeEach(() => { vi.mocked(listWorktreesStrict).mockReset() - missingPathMock.mockReset().mockResolvedValue(false) - }) - - it('skips Git for a missing repo folder', async () => { - missingPathMock.mockResolvedValue(true) - expect(await scanLocalRepoWorktreesForResolution('/repo', {})).toEqual({ - ok: false, - worktrees: [] - }) - expect(listWorktreesStrict).not.toHaveBeenCalled() }) it('degrades a local Git execution failure instead of reporting an empty success', async () => { diff --git a/src/main/runtime/repo-worktree-resolution-scan.ts b/src/main/runtime/repo-worktree-resolution-scan.ts index 2210b25a545..052118cf2bf 100644 --- a/src/main/runtime/repo-worktree-resolution-scan.ts +++ b/src/main/runtime/repo-worktree-resolution-scan.ts @@ -1,6 +1,5 @@ import type { GitWorktreeInfo } from '../../shared/worktree/types' import { listWorktreesStrict } from '../git/worktree' -import { isLocalWorktreePathMissing } from '../local-worktree-filesystem' import type { LocalProjectWorktreeGitOptions } from '../project-runtime-git-options' export type RuntimeWorktreeScanResult = @@ -11,13 +10,6 @@ export async function scanLocalRepoWorktreesForResolution( repoPath: string, options: LocalProjectWorktreeGitOptions ): Promise { - try { - if (await isLocalWorktreePathMissing(repoPath, options)) { - return { ok: false, worktrees: [] } - } - } catch { - // A failed host check is not proof of absence; preserve the Git scan's verdict. - } try { const worktrees = options.wslDistro ? await listWorktreesStrict(repoPath, options)