diff --git a/src/main/git/worktree-created-description-real-git.test.ts b/src/main/git/worktree-created-description-real-git.test.ts index 4a5545dd4ba..e54da7e4783 100644 --- a/src/main/git/worktree-created-description-real-git.test.ts +++ b/src/main/git/worktree-created-description-real-git.test.ts @@ -112,16 +112,20 @@ describe('describeCreatedWorktree against the real Git binary', () => { // `mkfifo` stands in for a `.git` on a hung mount: the read never rejects on its own. it.skipIf(process.platform === 'win32')( - "still settles when the repo's .git blocks forever", + "settles with the unread witness named when the repo's .git blocks forever", async () => { const stalledRepo = join(scratchDir, 'stalled') await mkdir(stalledRepo, { recursive: true }) const stalledDotGit = join(stalledRepo, '.git') await execFileAsync('mkfifo', [stalledDotGit]) try { + // Rejecting, not resolving undefined: undefined becomes a bare "created worktree not found", + // which claims Git put the worktree somewhere else. A stalled mount proves no such thing. + const settledBy = Date.now() + 5_000 await expect( describeCreatedWorktree(stalledRepo, worktreePath, 'feature', { timeout: 250 }) - ).resolves.toBeUndefined() + ).rejects.toThrow(/^repo common dir unverifiable: could not read .*\.git: /) + expect(Date.now()).toBeLessThan(settledBy) } finally { // Release the pending read so the fifo does not pin a threadpool thread for the whole run. await writeFile(stalledDotGit, '') diff --git a/src/main/git/worktree-created-disk-witness.test.ts b/src/main/git/worktree-created-disk-witness.test.ts new file mode 100644 index 00000000000..e3f7d16b3fc --- /dev/null +++ b/src/main/git/worktree-created-disk-witness.test.ts @@ -0,0 +1,187 @@ +import { chmodSync, mkdirSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +vi.mock('./worktree-list-reader', () => ({ + readRepoLocation: vi.fn(), + readRepoCommonDirFromGit: vi.fn(), + readCheckedOutBranchRef: vi.fn(), + readWorktreeHeadOid: vi.fn(), + readTranslatedWorktreeGraph: vi.fn(), + readWorktreeList: vi.fn() +})) +vi.mock('./worktree-sparse-checkout-cache', () => ({ + detectSparseCheckoutCached: vi.fn(async () => false) +})) + +import { describeCreatedWorktree } from './worktree-listing' +import { + readCheckedOutBranchRef, + readRepoCommonDirFromGit, + readRepoLocation, + readWorktreeHeadOid +} from './worktree-list-reader' + +const readRepoLocationMock = vi.mocked(readRepoLocation) +const readRepoCommonDirFromGitMock = vi.mocked(readRepoCommonDirFromGit) +const readCheckedOutBranchRefMock = vi.mocked(readCheckedOutBranchRef) +const readWorktreeHeadOidMock = vi.mocked(readWorktreeHeadOid) + +/** Repo convention: root bypasses the mode bits, so `chmod 000` denies nothing there. */ +const CAN_DENY_READ = process.platform !== 'win32' && process.getuid?.() !== 0 + +let scratchDir = '' +let repoPath = '' +let worktreePath = '' + +/** realpath: the witness canonicalizes, and macOS `tmpdir()` is a symlink (`/var` -> `/private/var`). */ +beforeEach(() => { + scratchDir = realpathSync(mkdtempSync(join(tmpdir(), 'orca-created-witness-'))) + repoPath = join(scratchDir, 'repo') + worktreePath = join(scratchDir, 'workspaces', 'feature') + mkdirSync(repoPath, { recursive: true }) + readRepoLocationMock.mockResolvedValue({ + topLevel: worktreePath, + // Deliberately not the repo's store, so every case below reaches the disk witness. + commonDir: join(scratchDir, 'elsewhere', '.git') + }) + // Git's own reading disagrees; only the witness can break the tie. + readRepoCommonDirFromGitMock.mockResolvedValue(join(scratchDir, 'other-repo', '.git')) + readCheckedOutBranchRefMock.mockResolvedValue('refs/heads/feature') + readWorktreeHeadOidMock.mockResolvedValue('a'.repeat(40)) +}) + +afterEach(() => { + vi.clearAllMocks() + chmodSync(repoPath, 0o700) + rmSync(scratchDir, { recursive: true, force: true }) +}) + +describe('describeCreatedWorktree when Git and the repo disagree', () => { + it('reports nothing when the witness proves a different object store', async () => { + // A real `.git` file pointing somewhere else: the worktree genuinely is not this repo's. + const otherGitDir = join(scratchDir, 'other-repo', '.git') + mkdirSync(otherGitDir, { recursive: true }) + writeFileSync(join(repoPath, '.git'), `gitdir: ${otherGitDir}\n`) + await expect( + describeCreatedWorktree(repoPath, worktreePath, 'feature') + ).resolves.toBeUndefined() + }) + + it('throws when the .git marker points at a path that does not exist', async () => { + // Nothing is there to prove a store either way: a fabricated candidate would decide the create. + writeFileSync(join(repoPath, '.git'), `gitdir: ${join(scratchDir, 'gone', '.git')}\n`) + await expect(describeCreatedWorktree(repoPath, worktreePath, 'feature')).rejects.toMatchObject({ + message: expect.stringContaining('gitdir marker target unreadable') + }) + }) + + it('throws when the .git marker points at a file', async () => { + const notAGitDir = join(scratchDir, 'not-a-git-dir') + writeFileSync(notAGitDir, 'not a git dir\n') + writeFileSync(join(repoPath, '.git'), `gitdir: ${notAGitDir}\n`) + await expect(describeCreatedWorktree(repoPath, worktreePath, 'feature')).rejects.toMatchObject({ + message: expect.stringContaining('gitdir marker target is not a directory') + }) + }) + + it('reports nothing for a bare repo, whose missing .git is a real answer', async () => { + // No `.git` at all is definitive absence, not an unreadable witness. + await expect( + describeCreatedWorktree(repoPath, worktreePath, 'feature') + ).resolves.toBeUndefined() + }) + + it('reports nothing when .git is a path under a file, not a directory', async () => { + // ENOTDIR, the other spelling of absence: `repo` is a file, so `repo/.git` cannot exist. + const filePath = join(scratchDir, 'plain-file') + writeFileSync(filePath, 'not a repo\n') + await expect( + describeCreatedWorktree(filePath, worktreePath, 'feature') + ).resolves.toBeUndefined() + }) + + it('follows gitdir and commondir markers', async () => { + const commonDir = join(scratchDir, 'main', '.git') + const linkedGitDir = join(commonDir, 'worktrees', 'source') + mkdirSync(linkedGitDir, { recursive: true }) + writeFileSync(join(repoPath, '.git'), `gitdir: ${linkedGitDir}\n`) + writeFileSync(join(linkedGitDir, 'commondir'), '../..\n') + readRepoLocationMock.mockResolvedValue({ topLevel: worktreePath, commonDir }) + + await expect(describeCreatedWorktree(repoPath, worktreePath, 'feature')).resolves.toMatchObject( + { + branch: 'refs/heads/feature' + } + ) + }) + + it.skipIf(!CAN_DENY_READ)('throws when the .git marker exists but cannot be read', async () => { + const dotGit = join(repoPath, '.git') + writeFileSync(dotGit, 'gitdir: /somewhere\n') + chmodSync(dotGit, 0o000) + await expect(describeCreatedWorktree(repoPath, worktreePath, 'feature')).rejects.toMatchObject({ + message: expect.stringMatching(/^repo common dir unverifiable: could not read .*\.git: /), + cause: expect.objectContaining({ code: 'EACCES' }) + }) + }) + + // The other unverifiable branch -- the deadline firing on a `.git` that never answers -- needs a + // read that really blocks, so it lives in worktree-created-description-real-git.test.ts behind a + // fifo. A short timeout here would only race the filesystem. + + it('accepts the create when the witness agrees with the worktree', async () => { + const commonDir = join(repoPath, '.git') + mkdirSync(commonDir, { recursive: true }) + writeFileSync(join(commonDir, 'HEAD'), 'ref: refs/heads/main\n') + readRepoLocationMock.mockResolvedValue({ topLevel: worktreePath, commonDir }) + await expect(describeCreatedWorktree(repoPath, worktreePath, 'feature')).resolves.toEqual({ + path: worktreePath, + head: 'a'.repeat(40), + branch: 'refs/heads/feature', + isBare: false, + isMainWorktree: false + }) + }) +}) + +describe('describeCreatedWorktree before the witness is reached', () => { + it('never pays for the disk read when Git already agreed', async () => { + const commonDir = join(repoPath, '.git') + mkdirSync(commonDir, { recursive: true }) + readRepoLocationMock.mockResolvedValue({ topLevel: worktreePath, commonDir }) + readRepoCommonDirFromGitMock.mockResolvedValue(commonDir) + // chmod 000 would make the witness unverifiable; agreement means it is never opened. + if (CAN_DENY_READ) { + chmodSync(repoPath, 0o000) + } + await expect(describeCreatedWorktree(repoPath, worktreePath, 'feature')).resolves.toMatchObject( + { + branch: 'refs/heads/feature' + } + ) + }) + + it('reports nothing when Git could not confirm the worktree at all', async () => { + readRepoLocationMock.mockResolvedValue(undefined) + // An unconfirmed worktree is not an unverifiable common dir: resolving undefined under a repo + // whose witness cannot be read is how we know the witness was never consulted. + if (CAN_DENY_READ) { + chmodSync(repoPath, 0o000) + } + await expect( + describeCreatedWorktree(repoPath, worktreePath, 'feature') + ).resolves.toBeUndefined() + }) + + it('reports nothing when the worktree has the wrong branch checked out', async () => { + readCheckedOutBranchRefMock.mockResolvedValue('refs/heads/other') + if (CAN_DENY_READ) { + chmodSync(repoPath, 0o000) + } + await expect( + describeCreatedWorktree(repoPath, worktreePath, 'feature') + ).resolves.toBeUndefined() + }) +}) diff --git a/src/main/git/worktree-listing-created-sparse-distro.test.ts b/src/main/git/worktree-listing-created-sparse-distro.test.ts index 2b3922d5b9b..ab0326dcee2 100644 --- a/src/main/git/worktree-listing-created-sparse-distro.test.ts +++ b/src/main/git/worktree-listing-created-sparse-distro.test.ts @@ -102,4 +102,27 @@ describe('describeCreatedWorktree on a drvfs-spelled WSL worktree', () => { platformSpy.mockRestore() } }) + + it('uses the repo disk witness in the WSL execution namespace', async () => { + const platformSpy = vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') + readRepoCommonDirFromGitMock.mockResolvedValue('/other/.git') + statMock.mockImplementation(async (target: string) => { + const value = slashed(target) + if (value === `${slashed(REPO)}/.git`) { + return { isDirectory: () => true } + } + if (value === `${HOST_GIT_DIR}/info/sparse-checkout`) { + return { isFile: () => true, size: 12 } + } + throw missing() + }) + + try { + await expect( + describeCreatedWorktree(REPO, 'C:\\wt\\x', 'feature', { wslDistro: 'Ubuntu' }) + ).resolves.toMatchObject({ branch: 'refs/heads/feature' }) + } finally { + platformSpy.mockRestore() + } + }) }) diff --git a/src/main/git/worktree-listing.ts b/src/main/git/worktree-listing.ts index f027bac4bc1..e902a89a957 100644 --- a/src/main/git/worktree-listing.ts +++ b/src/main/git/worktree-listing.ts @@ -1,5 +1,8 @@ -import { realpath, stat } from 'node:fs/promises' +import { readFile, realpath, stat } from 'node:fs/promises' import { join, posix } from 'node:path' +import { isDefinitiveAbsence } from '../../shared/definitive-filesystem-absence' +import { resolveGitMetadataPath } from '../../shared/git-metadata-path' +import { parseGitdirMarkerPayload } from '../../shared/gitdir-marker-payload' import { isWorktreeCreatePreparation } from '../../shared/worktree/create-preparation' import { toWslExecutionSpace } from '../../shared/wsl-paths' import type { GitWorktreeInfo } from '../../shared/worktree/types' @@ -20,8 +23,6 @@ import { } from './worktree-operation-options' import { areWorktreePathsEqual, translateWorktreePath } from './worktree-path-comparison' import { detectSparseCheckoutCached } from './worktree-sparse-checkout-cache' -import { resolveGitCommonDir } from './worktree-sparse-state' -import { resolveGitDir } from './source-control/resolve-git-dir' const SPARSE_CHECKOUT_DETECTION_CONCURRENCY = 8 @@ -151,25 +152,75 @@ export async function annotateSparseCheckoutStatus( * * Deadlined because a `.git` on a hung mount (dead NFS/SSHFS, stalled WSL 9p) never rejects, and an * unbounded read here would leave the whole create IPC pending instead of failing like it used to. + * + * A missing `.git` is a real "no candidate"; every other read failure is unverifiable and rejects. */ async function readRepoCommonDirFromDisk( repoPath: string, timeoutMs: number ): Promise { + const dotGit = join(repoPath, '.git') try { - const dotGit = join(repoPath, '.git') - // A bare repo has no `.git`, and resolveGitDir would fabricate one; offer no candidate instead. - await withDeadline(stat(dotGit), timeoutMs) - const commonDir = await withDeadline( - resolveGitDir(repoPath).then(resolveGitCommonDir), - timeoutMs - ) - // Node answers in the caller's space, Git in the distro's. Without this the WSL candidate is a UNC - // path that can never equal Git's `/home/...`, leaving this witness inert on exactly the fallback - // path that needs it (realpath cannot bridge the two: a Linux path has no local inode). - return toWslExecutionSpace(commonDir) - } catch { - return undefined + const commonDir = await withDeadline(resolveRepoCommonDirFromDisk(repoPath, dotGit), timeoutMs) + return commonDir ? toWslExecutionSpace(commonDir) : undefined + } catch (error) { + // A bare repo has no `.git`; do not fabricate a candidate for it. + if (isDefinitiveAbsence(error)) { + return undefined + } + const reason = error instanceof Error ? error.message : String(error) + throw new Error(`repo common dir unverifiable: could not read ${dotGit}: ${reason}`, { + cause: error + }) + } +} + +async function resolveRepoCommonDirFromDisk( + repoPath: string, + dotGit: string +): Promise { + // The general metadata resolvers are intentionally best effort; a witness must preserve read failures. + const dotGitStats = await stat(dotGit) + let gitDir = dotGit + if (!dotGitStats.isDirectory()) { + const pointer = parseGitdirMarkerPayload(await readFile(dotGit, 'utf8')) + if (!pointer) { + return undefined + } + gitDir = resolveGitMetadataPath(repoPath, pointer) ?? dotGit + await assertGitDirIsDirectory(gitDir) + } + + return readCommonDirMarker(gitDir) +} + +/** + * A marker target that is missing or is not a directory is unverifiable, not an absent `.git`: + * without this, `commondir`'s own ENOENT/ENOTDIR would pass as absence and hand the caller the + * pointer target as a common dir it never proved exists. + */ +async function assertGitDirIsDirectory(gitDir: string): Promise { + let gitDirStats + try { + gitDirStats = await stat(gitDir) + } catch (error) { + // Rewrapped so the outer absence check cannot read this errno as a bare repo's missing `.git`. + throw new Error(`gitdir marker target unreadable: ${gitDir}`, { cause: error }) + } + if (!gitDirStats.isDirectory()) { + throw new Error(`gitdir marker target is not a directory: ${gitDir}`) + } +} + +async function readCommonDirMarker(gitDir: string): Promise { + try { + const pointer = await readFile(join(gitDir, 'commondir'), 'utf8') + return resolveGitMetadataPath(gitDir, pointer) ?? gitDir + } catch (error) { + if (!isDefinitiveAbsence(error)) { + throw error + } + return gitDir } } diff --git a/src/main/ipc/created-worktree-reconciliation.test.ts b/src/main/ipc/created-worktree-reconciliation.test.ts index 22912394d84..b62088ef105 100644 --- a/src/main/ipc/created-worktree-reconciliation.test.ts +++ b/src/main/ipc/created-worktree-reconciliation.test.ts @@ -139,14 +139,29 @@ describe('resolveCreatedWorktree', () => { ) }) + it('does not mistake a falsy rejection for a successful listing', async () => { + vi.mocked(listWorktreesSharedStrict).mockRejectedValue(undefined) + + await expect(resolveCreatedWorktree('/repo', '/workspaces/feature', 'feature')).rejects.toThrow( + 'undefined' + ) + }) + it('keeps the listing failure when the direct read itself throws', async () => { const failure = new Error('fatal: not a git repository') + const recoveryFailure = new Error('repo common dir unverifiable: deadline exceeded') + const warn = vi.spyOn(console, 'warn').mockImplementation(() => undefined) vi.mocked(listWorktreesSharedStrict).mockRejectedValue(failure) - vi.mocked(describeCreatedWorktree).mockRejectedValue(new Error('rev-parse exploded')) + vi.mocked(describeCreatedWorktree).mockRejectedValue(recoveryFailure) await expect(resolveCreatedWorktree('/repo', '/workspaces/feature', 'feature')).rejects.toBe( failure ) + expect(warn).toHaveBeenCalledWith('[worktrees:create] created-worktree recovery also failed', { + err: recoveryFailure, + worktreePath: '/workspaces/feature' + }) + warn.mockRestore() }) it('names the path and branch when the listing succeeded without the row', async () => { @@ -159,11 +174,16 @@ describe('resolveCreatedWorktree', () => { it("adds the direct read's failure when the listing merely omitted the row", async () => { vi.mocked(listWorktreesSharedStrict).mockResolvedValue([MAIN]) - vi.mocked(describeCreatedWorktree).mockRejectedValue(new Error('rev-parse exploded')) + const recoveryFailure = new Error('rev-parse exploded') + vi.mocked(describeCreatedWorktree).mockRejectedValue(recoveryFailure) - await expect(resolveCreatedWorktree('/repo', '/workspaces/feature', 'feature')).rejects.toThrow( - 'Worktree created but not found in listing: /workspaces/feature (branch feature): rev-parse exploded' - ) + await expect( + resolveCreatedWorktree('/repo', '/workspaces/feature', 'feature') + ).rejects.toMatchObject({ + message: + 'Worktree created but not found in listing: /workspaces/feature (branch feature): rev-parse exploded', + cause: recoveryFailure + }) }) it('charges the recovery what the listing left of the budget, not a fresh one', async () => { diff --git a/src/main/ipc/created-worktree-reconciliation.ts b/src/main/ipc/created-worktree-reconciliation.ts index 0f9b5cdbdb1..ac5b79954c2 100644 --- a/src/main/ipc/created-worktree-reconciliation.ts +++ b/src/main/ipc/created-worktree-reconciliation.ts @@ -53,7 +53,7 @@ export async function resolveCreatedWorktree( options?: GitWorktreeExecOptions ): Promise { const startedAt = Date.now() - let listingError: unknown + let listingError: Error | undefined try { const worktrees = options ? await listWorktreesSharedStrict(repoPath, options) @@ -63,11 +63,9 @@ export async function resolveCreatedWorktree( return { created, worktrees, listingComplete: true } } } catch (err) { - listingError = err + listingError = err instanceof Error ? err : new Error(String(err)) } - let described: GitWorktreeInfo | undefined - let describeError: unknown try { // One budget for verifying the create, not one per attempt: a hung Git already spent the // listing's deadline, and charging the recovery a fresh one doubles the wait before the error. @@ -75,26 +73,31 @@ export async function resolveCreatedWorktree( WORKTREE_LIST_TIMEOUT_MS - (Date.now() - startedAt), MIN_CREATED_WORKTREE_RECOVERY_MS ) - described = await describeCreatedWorktree(repoPath, worktreePath, branchName, { + const described = await describeCreatedWorktree(repoPath, worktreePath, branchName, { ...options, timeout: options?.timeout ?? remainingMs }) + if (described) { + return { created: described, worktrees: [], listingComplete: false } + } } catch (err) { - // Why keep, not rethrow: the recovery must not replace the listing's own, more informative failure. - describeError = err - } - if (described) { - return { created: described, worktrees: [], listingComplete: false } + if (listingError) { + // The listing's failure stays the thrown one, but the recovery's reason -- often + // `repo common dir unverifiable: ...` -- would otherwise vanish from the record entirely. + console.warn('[worktrees:create] created-worktree recovery also failed', { + err, + worktreePath + }) + throw listingError + } + // The listing simply omitted the row, so the direct read holds the only actionable failure. + const notFound = createdWorktreeNotFoundError(worktreePath, branchName) + throw new Error(`${notFound.message}: ${err instanceof Error ? err.message : String(err)}`, { + cause: err + }) } if (listingError) { throw listingError } - const notFound = createdWorktreeNotFoundError(worktreePath, branchName) - if (describeError) { - // The listing simply omitted the row, so the direct read holds the only actionable failure. - throw new Error( - `${notFound.message}: ${describeError instanceof Error ? describeError.message : String(describeError)}` - ) - } - throw notFound + throw createdWorktreeNotFoundError(worktreePath, branchName) }