fix(worktrees): preserve unverifiable disk witness (#20713)

* fix(worktrees): preserve unverifiable disk witness

* fix(worktrees): follow gitdir/commondir markers in disk witness

The disk witness validates created worktrees by reading the repo's common directory from disk. Previously it only checked for a direct .git directory and returned a status object that conflated different failure modes.

Now it properly follows .gitdir and commondir pointer files to locate the true common directory, fixing detection on repos with linked git directories (worktrees, submodules) and WSL scenarios. Error handling is simplified: definitive absence returns undefined, other read failures throw with proper cause chains, eliminating the ambiguous "unverifiable" state that would mask real errors.

* fix: validate gitdir marker targets are directories

When a .git marker points to a missing or non-directory path, that's
unverifiable—not the same as an absent .git file (bare repo). Validate
accessibility before reading commondir to catch these errors clearly.

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, '')

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()
+  })
+})

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()
+    }
+  })
 })

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<string | undefined> {
+  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<string | undefined> {
+  // 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<void> {
+  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<string> {
+  try {
+    const pointer = await readFile(join(gitDir, 'commondir'), 'utf8')
+    return resolveGitMetadataPath(gitDir, pointer) ?? gitDir
+  } catch (error) {
+    if (!isDefinitiveAbsence(error)) {
+      throw error
+    }
+    return gitDir
   }
 }
 

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 () => {

src/main/ipc/created-worktree-reconciliation.ts

@@ -53,7 +53,7 @@ export async function resolveCreatedWorktree(
   options?: GitWorktreeExecOptions
 ): Promise<CreatedWorktreeResolution> {
   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)
 }