From 311325b9eaa17feb0f1fbca60403df97ef0a5e7a Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Tue, 25 Aug 2026 05:56:35 -0700 Subject: [PATCH] fix(worktrees): explain the object-store failure instead of leaking git's argv MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit What the user hits today: creating a workspace on a branch whose repository is missing objects fails with a raw dump of git's stderr glued to the command Orca ran — "Error invoking remote method 'worktrees:create': Error: Command failed: git worktree add /Users//dev/worktrees/ '' fatal: unable to read tree (0413351...)". It leaks the user's home directory and full argv into a toast (and into bug reports), and it says nothing about what to do next. The create-time preflight does not catch it either: it peels the branch with `^{commit}`, which reads only the commit object, so a repo whose commit object survives while its root tree does not passes the preflight cleanly and then dies inside the checkout. After this change the same failure reads: "Orca could not create this workspace because the repository object database is missing objects. Git reported: unable to read tree (0413351...). The commit for "" is present but its root tree object is missing, so Git could not check any files out. Run git fsck in the repository to confirm what is missing, then re-fetch from the remote or re-clone to restore it." No path, no argv. The sidebar row keeps the short title and the creation panel shows the full diagnosis, which the row would truncate. Not a preflight. Peeling `^{tree}` on every create costs a whole extra git process (~15 ms on a 2.9 GiB repo) and still proves nothing about subtrees or blobs, so it would slow every success for a partial guarantee. Git's stderr from the failed command stays the authority; the `^{commit}` / `^{tree}` / promisor probes run only after it fails, and only to add detail. Evidence discipline, because this is exactly where the claim can outrun the observation: the "commit is present but its root tree is missing" sentence is emitted only when the commit peel actually succeeded, since a failed `^{tree}` peel is equally what an unreadable *commit* produces. Probes distinguish "git answered no" (exit 1 with empty stderr) from "we never got an answer" (any other status, or an exit 1 git explained on stderr — mode-000 objects, dead SSH transport, killed process), and the latter degrades to the neutral wording rather than asserting absence. Partial-clone detection has the same three-way verdict. Covered on every create path: - `worktree add` dies reading the tree directly. - Sparse creates pass --no-checkout, so add exits 0 and the failure lands in the follow-up checkout; probes there target the repo, which outlives the rollback. - SSH/remote creates diagnose client-side against the repo on the execution host, so no wire change and any relay version benefits. - Diagnosis probes are bounded (10s): on a partial clone the `^{tree}` peel can trigger a promisor fetch, and an unreachable remote would otherwise turn a bounded failure into a create that never settles. The remote path passes the bound explicitly rather than inheriting the mux's 30s ambient default. - Redacted messages keep the `git_failed` telemetry bucket, and unreadable-object creates still classify as `permission_denied`; the classifier reads the raw git text off the error's cause chain, which never leaves the main process. - The renderer runs the same classifier, so a pre-fix host or older relay that still rethrows git verbatim gets redacted client-side. Tests were written first and verified failing against the raw-leak behavior. The regex set is pinned by a real-git reproduction that builds a repo, deletes its root tree, and asserts on the actual binary's stderr rather than remembered wording — including the sparse path, where git 2.44 words the same missing root tree as "unable to parse commit". --- src/main/git/git-exec-error-text.ts | 16 + .../worktree-add-missing-object-real.test.ts | 285 ++++++++++++++++++ ...add-object-store-diagnosis-timeout.test.ts | 66 ++++ .../worktree-add-object-store-error.test.ts | 144 +++++++++ .../git/worktree-add-object-store-error.ts | 65 ++++ .../worktree-object-store-diagnosis.test.ts | 137 +++++++++ .../git/worktree-object-store-diagnosis.ts | 83 +++++ src/main/git/worktree.ts | 46 ++- .../workspace-create-error-classifier.test.ts | 40 +++ .../ipc/workspace-create-error-classifier.ts | 24 +- ...ee-remote-object-store-probe-bound.test.ts | 32 ++ src/main/ipc/worktree-remote.ts | 46 ++- src/main/runtime/orca-runtime.ts | 5 + .../WorktreeCreationPanel.test.tsx | 15 + .../WorktreeCreationPanel.tsx | 6 +- .../src/lib/pending-worktree-creation.ts | 3 + .../lib/workspace-create-error-format.test.ts | 123 ++++++++ .../src/lib/workspace-create-error-format.ts | 59 ++++ ...eation-flow-object-store-diagnosis.test.ts | 209 +++++++++++++ .../src/lib/worktree-creation-flow.ts | 12 +- .../src/store/slices/worktree-helpers.ts | 1 + src/shared/git-object-store-failure.test.ts | 153 ++++++++++ src/shared/git-object-store-failure.ts | 168 +++++++++++ 23 files changed, 1722 insertions(+), 16 deletions(-) create mode 100644 src/main/git/git-exec-error-text.ts create mode 100644 src/main/git/worktree-add-missing-object-real.test.ts create mode 100644 src/main/git/worktree-add-object-store-diagnosis-timeout.test.ts create mode 100644 src/main/git/worktree-add-object-store-error.test.ts create mode 100644 src/main/git/worktree-add-object-store-error.ts create mode 100644 src/main/git/worktree-object-store-diagnosis.test.ts create mode 100644 src/main/git/worktree-object-store-diagnosis.ts create mode 100644 src/main/ipc/worktree-remote-object-store-probe-bound.test.ts create mode 100644 src/renderer/src/lib/worktree-creation-flow-object-store-diagnosis.test.ts create mode 100644 src/shared/git-object-store-failure.test.ts create mode 100644 src/shared/git-object-store-failure.ts diff --git a/src/main/git/git-exec-error-text.ts b/src/main/git/git-exec-error-text.ts new file mode 100644 index 00000000000..95c78d8624a --- /dev/null +++ b/src/main/git/git-exec-error-text.ts @@ -0,0 +1,16 @@ +/** + * Everything a rejected git exec wrote, as one string. + * + * Why both fields: `execFile` folds git's stderr into `message`, but relay and + * buffer-capped runners can surface the fatal line only on a separate `stderr`. + */ +export function readGitExecErrorText(error: unknown): string { + if (typeof error === 'string') { + return error + } + if (typeof error !== 'object' || error === null) { + return '' + } + const record = error as { message?: unknown; stderr?: unknown } + return [record.message, record.stderr].filter((part) => typeof part === 'string').join('\n') +} diff --git a/src/main/git/worktree-add-missing-object-real.test.ts b/src/main/git/worktree-add-missing-object-real.test.ts new file mode 100644 index 00000000000..8d2e71aa37e --- /dev/null +++ b/src/main/git/worktree-add-missing-object-real.test.ts @@ -0,0 +1,285 @@ +import { execFileSync } from 'node:child_process' +import { chmodSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, describe, expect, it } from 'vitest' +import { GIT_OBJECT_STORE_FAILURE_ANCHOR } from '../../shared/git-object-store-failure' +import { classifyWorkspaceCreateError } from '../ipc/workspace-create-error-classifier' +import { addSparseWorktree, addWorktree } from './worktree' + +// Real-binary reproduction of the field report: the branch's commit object is +// present (so every commit-only preflight passes) but its root tree is gone. +// Root and Windows ignore mode 000, so the unreadable-object shape cannot be staged there. +const chmodBlocksReads = ((): boolean => { + const probeDir = mkdtempSync(join(tmpdir(), 'orca-chmod-probe-')) + try { + const probe = join(probeDir, 'probe') + writeFileSync(probe, 'x') + chmodSync(probe, 0o000) + try { + readFileSync(probe) + return false + } catch { + return true + } + } finally { + rmSync(probeDir, { recursive: true, force: true }) + } +})() + +describe('addWorktree real Git contract for a missing root tree', () => { + const tempPaths: string[] = [] + + afterEach(() => { + for (const path of tempPaths.splice(0)) { + rmSync(path, { recursive: true, force: true }) + } + }) + + const buildRepoWithMissingRootTree = (): { repoPath: string } => { + const repoPath = mkdtempSync(join(tmpdir(), 'orca-missing-tree-')) + tempPaths.push(repoPath) + const git = (...args: string[]): string => + execFileSync('git', args, { cwd: repoPath, encoding: 'utf8' }).trim() + + git('init', '--quiet') + git('config', 'user.name', 'Orca Test') + git('config', 'user.email', 'orca@example.test') + git('config', 'commit.gpgSign', 'false') + git('config', 'core.hooksPath', '.git/no-hooks') + writeFileSync(join(repoPath, 'fixture.txt'), 'base\n') + // Sparse cone mode needs a real directory to select. + mkdirSync(join(repoPath, 'pkg')) + writeFileSync(join(repoPath, 'pkg', 'inner.txt'), 'inner\n') + git('add', '.') + git('commit', '-m', 'base') + git('branch', '-M', 'main') + + const tree = git('rev-parse', 'HEAD^{tree}') + // Detach the branch from HEAD's reflog-reachable checkout before removing the tree. + git('branch', 'akulafb/test', 'HEAD') + rmSync(join(repoPath, '.git', 'objects', tree.slice(0, 2), tree.slice(2)), { force: true }) + + // The exact preflight Orca runs today still succeeds on this repo. + expect(git('rev-parse', '--verify', '--quiet', 'refs/heads/akulafb/test^{commit}')).toMatch( + /^[0-9a-f]{40}$/ + ) + return { repoPath } + } + + it('reports a repairable object-store failure instead of leaking the path and argv', async () => { + const { repoPath } = buildRepoWithMissingRootTree() + const worktreePath = join(repoPath, '..', `wt-${Date.now()}`) + tempPaths.push(worktreePath) + + const error = await addWorktree( + repoPath, + worktreePath, + 'akulafb/test', + undefined, + false, + false, + { checkoutExistingBranch: true } + ).then( + () => null, + (thrown: unknown) => thrown as Error + ) + + expect(error).toBeInstanceOf(Error) + expect(error?.message).toContain(GIT_OBJECT_STORE_FAILURE_ANCHOR) + expect(error?.message).toContain('akulafb/test') + expect(error?.message).toContain('git fsck') + expect(error?.message).not.toContain(worktreePath) + expect(error?.message).not.toContain('git worktree add') + expect(error?.message).not.toContain('Command failed') + }) + + it('reports the same failure for the new-branch form, which reads the same tree', async () => { + const { repoPath } = buildRepoWithMissingRootTree() + const worktreePath = join(repoPath, '..', `wt-b-${Date.now()}`) + tempPaths.push(worktreePath) + + const error = await addWorktree( + repoPath, + worktreePath, + 'orca/from-broken-base', + 'akulafb/test' + ).then( + () => null, + (thrown: unknown) => thrown as Error + ) + + expect(error?.message).toContain(GIT_OBJECT_STORE_FAILURE_ANCHOR) + expect(error?.message).not.toContain(worktreePath) + }) + + // The sparse path never fails in `worktree add`: it passes --no-checkout, which exits 0 + // without reading the tree, and dies in the follow-up `git checkout` instead. + it('diagnoses the sparse create, which fails in the follow-up checkout rather than in add', async () => { + const { repoPath } = buildRepoWithMissingRootTree() + const worktreePath = join(repoPath, '..', `wt-sparse-${Date.now()}`) + tempPaths.push(worktreePath) + + const error = await addSparseWorktree( + repoPath, + worktreePath, + 'akulafb/test', + ['pkg'], + undefined, + false, + { checkoutExistingBranch: true } + ).then( + () => null, + (thrown: unknown) => thrown as Error + ) + + expect(error).toBeInstanceOf(Error) + expect(error?.message).toContain(GIT_OBJECT_STORE_FAILURE_ANCHOR) + expect(error?.message).toContain('akulafb/test') + expect(error?.message).toContain('git fsck') + // The commit really is readable here, so the specific sentence is earned. + expect(error?.message).toContain('root tree object is missing') + expect(error?.message).not.toContain('Command failed') + expect(error?.message).not.toContain('git checkout') + }) + + // The inverse corruption: the ROOT TREE survives and the COMMIT is the unreadable + // object. Git answers `fatal: invalid reference`, and both `^{commit}` and `^{tree}` + // peels exit 1 — so a tree-only probe would wrongly report "commit present, tree gone". + const buildRepoWithCorruptCommit = (): { repoPath: string; treeOid: string } => { + const repoPath = mkdtempSync(join(tmpdir(), 'orca-corrupt-commit-')) + tempPaths.push(repoPath) + const git = (...args: string[]): string => + execFileSync('git', args, { cwd: repoPath, encoding: 'utf8' }).trim() + + git('init', '--quiet') + git('config', 'user.name', 'Orca Test') + git('config', 'user.email', 'orca@example.test') + git('config', 'commit.gpgSign', 'false') + git('config', 'core.hooksPath', '.git/no-hooks') + writeFileSync(join(repoPath, 'fixture.txt'), 'base\n') + git('add', 'fixture.txt') + git('commit', '-m', 'base') + git('branch', '-M', 'main') + git('branch', 'akulafb/test', 'HEAD') + + const commit = git('rev-parse', 'refs/heads/akulafb/test') + const treeOid = git('rev-parse', 'refs/heads/akulafb/test^{tree}') + const commitFile = join(repoPath, '.git', 'objects', commit.slice(0, 2), commit.slice(2)) + chmodSync(commitFile, 0o644) + writeFileSync(commitFile, '') + + // The tree really is still on disk; only the commit is unreadable. + expect(git('cat-file', '-t', treeOid)).toBe('tree') + return { repoPath, treeOid } + } + + it('does not claim the commit survived when git could not read the commit', async () => { + const { repoPath } = buildRepoWithCorruptCommit() + const worktreePath = join(repoPath, '..', `wt-c-${Date.now()}`) + tempPaths.push(worktreePath) + + const error = await addWorktree( + repoPath, + worktreePath, + 'akulafb/test', + undefined, + false, + false, + { checkoutExistingBranch: true } + ).then( + () => null, + (thrown: unknown) => thrown as Error + ) + + expect(error?.message).toContain(GIT_OBJECT_STORE_FAILURE_ANCHOR) + // Both halves of the specific sentence would be false here. + expect(error?.message).not.toContain('root tree object is missing') + expect(error?.message).not.toContain('is present but') + expect(error?.message).toContain('Git could not read every object') + }) + + // The everyday shape of a repo written by `sudo git`, a root-owned bind mount, or a + // bad-umask share: the tree object is right there on disk and Git may not open it. + const buildRepoWithUnreadableRootTree = (): { repoPath: string; treeOid: string } => { + const repoPath = mkdtempSync(join(tmpdir(), 'orca-unreadable-tree-')) + tempPaths.push(repoPath) + const git = (...args: string[]): string => + execFileSync('git', args, { cwd: repoPath, encoding: 'utf8' }).trim() + + git('init', '--quiet') + git('config', 'user.name', 'Orca Test') + git('config', 'user.email', 'orca@example.test') + git('config', 'commit.gpgSign', 'false') + git('config', 'core.hooksPath', '.git/no-hooks') + writeFileSync(join(repoPath, 'fixture.txt'), 'base\n') + git('add', 'fixture.txt') + git('commit', '-m', 'base') + git('branch', '-M', 'main') + git('branch', 'akulafb/test', 'HEAD') + + const treeOid = git('rev-parse', 'refs/heads/akulafb/test^{tree}') + const treeFile = join(repoPath, '.git', 'objects', treeOid.slice(0, 2), treeOid.slice(2)) + chmodSync(treeFile, 0o000) + return { repoPath, treeOid } + } + + it.skipIf(!chmodBlocksReads)( + 'does not report an unreadable root tree as a missing one', + async () => { + const { repoPath, treeOid } = buildRepoWithUnreadableRootTree() + const worktreePath = join(repoPath, '..', `wt-eacces-${Date.now()}`) + tempPaths.push(worktreePath) + + const error = await addWorktree( + repoPath, + worktreePath, + 'akulafb/test', + undefined, + false, + false, + { checkoutExistingBranch: true } + ).then( + () => null, + (thrown: unknown) => thrown as Error + ) + + // The object it supposedly lost is still on disk, byte for byte. + chmodSync( + join(repoPath, '.git', 'objects', treeOid.slice(0, 2), treeOid.slice(2)), + 0o444 + ) + expect( + execFileSync('git', ['cat-file', '-t', treeOid], { cwd: repoPath, encoding: 'utf8' }).trim() + ).toBe('tree') + + expect(error?.message).toContain(GIT_OBJECT_STORE_FAILURE_ANCHOR) + expect(error?.message).not.toContain('root tree object is missing') + expect(error?.message).toContain('Git could not read every object') + } + ) + + it.skipIf(!chmodBlocksReads)( + 'keeps an unreadable-object create in the permission_denied telemetry bucket', + async () => { + const { repoPath } = buildRepoWithUnreadableRootTree() + const worktreePath = join(repoPath, '..', `wt-eacces-class-${Date.now()}`) + tempPaths.push(worktreePath) + + const error = await addWorktree( + repoPath, + worktreePath, + 'akulafb/test', + undefined, + false, + false, + { checkoutExistingBranch: true } + ).then( + () => null, + (thrown: unknown) => thrown as Error + ) + + expect(classifyWorkspaceCreateError(error)).toBe('permission_denied') + } + ) +}) diff --git a/src/main/git/worktree-add-object-store-diagnosis-timeout.test.ts b/src/main/git/worktree-add-object-store-diagnosis-timeout.test.ts new file mode 100644 index 00000000000..076684b017c --- /dev/null +++ b/src/main/git/worktree-add-object-store-diagnosis-timeout.test.ts @@ -0,0 +1,66 @@ +// The failure-path object-store probes must stay bounded: on a partial clone they can +// trigger a promisor fetch against an unreachable remote. +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { + gitExecFileAsyncMock, + gitExecFileSyncMock, + translateWslOutputPathsMock, + moveWorktreeDirectoryToTrashMock +} = vi.hoisted(() => ({ + gitExecFileAsyncMock: vi.fn(), + gitExecFileSyncMock: vi.fn(), + translateWslOutputPathsMock: vi.fn((output: string) => output), + moveWorktreeDirectoryToTrashMock: vi.fn() +})) + +vi.mock('./runner', () => ({ + gitExecFileAsync: gitExecFileAsyncMock, + gitExecFileSync: gitExecFileSyncMock, + translateWslOutputPaths: translateWslOutputPathsMock +})) + +vi.mock('../worktree-trash', () => ({ + moveWorktreeDirectoryToTrash: moveWorktreeDirectoryToTrashMock.mockResolvedValue(undefined), + restoreWorktreeDirectoryFromTrash: vi.fn().mockResolvedValue(true), + scheduleWorktreeTrashDeletion: vi.fn() +})) + +import { addWorktree, WORKTREE_OBJECT_STORE_DIAGNOSIS_TIMEOUT_MS } from './worktree' +import { registerWorktreeSuiteHooks } from './worktree-test-harness' + +registerWorktreeSuiteHooks() + +describe('worktree add object-store diagnosis probes', () => { + beforeEach(() => { + gitExecFileAsyncMock.mockReset() + gitExecFileSyncMock.mockReset() + }) + + it('bounds every diagnosis probe so a promisor fetch cannot hang the create', async () => { + gitExecFileAsyncMock.mockImplementation(async (args: string[]) => { + if (args[0] === 'worktree') { + throw new Error( + "Command failed: git worktree add /repo-feature 'feature/test'\n" + + 'fatal: unable to read tree (041335168f0214913840aaaaaaaaaaaaaaaaaaaa)' + ) + } + throw Object.assign(new Error('Command failed'), { code: 1 }) + }) + + await expect( + addWorktree('/repo', '/repo-feature', 'feature/test', 'feature/test', false, false, { + checkoutExistingBranch: true + }) + ).rejects.toThrow('repository object database is missing objects') + + const probeCalls = gitExecFileAsyncMock.mock.calls.filter(([args]) => args[0] !== 'worktree') + expect(probeCalls.length).toBe(3) + for (const [, options] of probeCalls) { + expect(typeof options.timeout).toBe('number') + expect(options.timeout).toBe(WORKTREE_OBJECT_STORE_DIAGNOSIS_TIMEOUT_MS) + } + // Bounded well under the add timeout it diagnoses, so the failure stays a failure. + expect(WORKTREE_OBJECT_STORE_DIAGNOSIS_TIMEOUT_MS).toBeLessThan(30_000) + }) +}) diff --git a/src/main/git/worktree-add-object-store-error.test.ts b/src/main/git/worktree-add-object-store-error.test.ts new file mode 100644 index 00000000000..415812c39cd --- /dev/null +++ b/src/main/git/worktree-add-object-store-error.test.ts @@ -0,0 +1,144 @@ +import { describe, expect, it, vi } from 'vitest' +import { describeWorktreeAddObjectStoreFailure } from './worktree-add-object-store-error' + +// Node's execFile glues git's stderr onto the argv line; this is what both the local +// runner and the SSH relay hand back for the reported failure. +const rawLocalError = new Error( + "Command failed: git worktree add /Users/akulafb/dev/worktrees/test 'akulafb/test'\n" + + "Preparing worktree (checking out 'akulafb/test')\n" + + 'fatal: unable to read tree (041335168f0214913840aaaaaaaaaaaaaaaaaaaa)' +) + +describe('describeWorktreeAddObjectStoreFailure', () => { + it('leaves unrelated failures for the caller to rethrow untouched', async () => { + const runGit = vi.fn() + const unrelated = new Error( + "Command failed: git worktree add /Users/akulafb/wt\nfatal: 'wt' already exists" + ) + + await expect( + describeWorktreeAddObjectStoreFailure(unrelated, { + runGit, + branch: 'akulafb/test', + checkoutRef: 'refs/heads/akulafb/test' + }) + ).resolves.toBeNull() + expect(runGit).not.toHaveBeenCalled() + }) + + it('does not re-diagnose an error a caller already described', async () => { + // The sparse path funnels addWorktree failures through the same catch. + const runGit = vi.fn() + const alreadyDescribed = await describeWorktreeAddObjectStoreFailure(rawLocalError, { + runGit: async () => { + throw Object.assign(new Error('Command failed'), { code: 1 }) + }, + branch: 'akulafb/test', + checkoutRef: 'refs/heads/akulafb/test' + }) + + await expect( + describeWorktreeAddObjectStoreFailure(alreadyDescribed, { + runGit, + branch: 'akulafb/test', + checkoutRef: 'refs/heads/akulafb/test' + }) + ).resolves.toBeNull() + expect(runGit).not.toHaveBeenCalled() + }) + + it('reads git fatals that arrive only on a separate stderr field', async () => { + const stderrOnly = Object.assign(new Error('Command failed with exit code 128'), { + stderr: 'fatal: unable to read tree 041335168f0214913840aaaaaaaaaaaaaaaaaaaa\n' + }) + + const described = await describeWorktreeAddObjectStoreFailure(stderrOnly, { + runGit: async () => { + throw Object.assign(new Error('Command failed'), { code: 1 }) + }, + branch: 'akulafb/test', + checkoutRef: 'refs/heads/akulafb/test' + }) + + expect(described?.message).toContain('repository object database is missing objects') + }) + + it('probes the checked-out ref and redacts the path and argv', async () => { + // The reported shape: the commit peels, the tree does not. + const runGit = vi.fn(async (args: string[]) => + args[3] === 'refs/heads/akulafb/test^{commit}' + ? { stdout: 'c8a40a3a1ebd165bbdc29303e8c0e7330442abaa\n' } + : Promise.reject(Object.assign(new Error('Command failed'), { code: 1 })) + ) + + const described = await describeWorktreeAddObjectStoreFailure(rawLocalError, { + runGit, + branch: 'akulafb/test', + checkoutRef: 'refs/heads/akulafb/test' + }) + + expect(runGit).toHaveBeenCalledWith([ + 'rev-parse', + '--verify', + '--quiet', + 'refs/heads/akulafb/test^{tree}' + ]) + expect(described?.message).toContain('root tree object is missing') + expect(described?.message).not.toContain('/Users/akulafb') + expect(described?.message).not.toContain('git worktree add') + // The raw argv stays reachable for main-process logs and telemetry. + expect(described?.cause).toBe(rawLocalError) + }) + + it('falls back to the generic sentence when the commit is unreadable too', async () => { + // Both peels answer a wordless "no": the commit is gone too, so neither half is observed. + const runGit = vi.fn(async () => { + throw Object.assign(new Error('Command failed'), { code: 1 }) + }) + + const described = await describeWorktreeAddObjectStoreFailure(rawLocalError, { + runGit, + branch: 'akulafb/test', + checkoutRef: 'refs/heads/akulafb/test' + }) + + expect(described?.message).not.toContain('root tree object is missing') + expect(described?.message).toContain('Git could not read every object') + }) + + it('still describes the failure when the SSH relay cannot run the probes', async () => { + // Relay errors carry no git exit status, so nothing may be claimed about the tree. + const runGit = vi.fn(async () => { + throw new Error('SSH connection is not available. Please reconnect and try again.') + }) + + const described = await describeWorktreeAddObjectStoreFailure(rawLocalError, { + runGit, + branch: 'akulafb/test', + checkoutRef: 'refs/heads/akulafb/test' + }) + + expect(described?.message).toContain('repository object database is missing objects') + expect(described?.message).not.toContain('root tree object is missing') + expect(described?.message).not.toContain('partial clone') + expect(described?.message).not.toContain('/Users/akulafb') + }) + + it('adds promisor guidance when the remote host reports a partial clone', async () => { + const runGit = vi.fn(async (args: string[]) => { + if (args[0] === 'rev-parse') { + throw Object.assign(new Error('Command failed'), { code: 1 }) + } + return { stdout: 'remote.origin.promisor true\n' } + }) + + const described = await describeWorktreeAddObjectStoreFailure(rawLocalError, { + runGit, + branch: 'akulafb/test', + checkoutRef: 'refs/heads/akulafb/test' + }) + + expect(described?.message).toContain('partial clone') + expect(described?.message).toContain('git fetch --refetch') + }) +}) diff --git a/src/main/git/worktree-add-object-store-error.ts b/src/main/git/worktree-add-object-store-error.ts new file mode 100644 index 00000000000..76382bd1a7c --- /dev/null +++ b/src/main/git/worktree-add-object-store-error.ts @@ -0,0 +1,65 @@ +import { + classifyGitObjectStoreFailure, + formatGitObjectStoreFailureMessage, + isGitObjectStoreFailureMessage +} from '../../shared/git-object-store-failure' +import { readGitExecErrorText } from './git-exec-error-text' +import { diagnoseWorktreeObjectStore } from './worktree-object-store-diagnosis' + +type GitRunner = (args: string[]) => Promise<{ stdout: string }> + +export type WorktreeAddObjectStoreContext = { + /** Runs git in the repo that owns the object store; SSH passes the relay executor. */ + runGit: GitRunner + /** Branch the user asked for, shown back to them. */ + branch: string + /** Rev whose tree the checkout needed: the existing branch, or the resolved base. */ + checkoutRef: string +} + +/** + * Replace a raw `git worktree add` object-store failure with a redacted, diagnosed one. + * + * Returns `null` when the error is something else, so callers rethrow unchanged. + * Shared by the local/WSL and SSH create paths, which duplicate the same preflight + * and must not diverge on how the failure reads. + */ +export async function describeWorktreeAddObjectStoreFailure( + error: unknown, + context: WorktreeAddObjectStoreContext +): Promise { + const text = readGitExecErrorText(error) + // Why: a diagnosis quotes git's wording back, so re-classifying one would re-probe and + // nest it. The sparse path can hand us an error already described by `addWorktree`. + if (isGitObjectStoreFailureMessage(text)) { + return null + } + const failure = classifyGitObjectStoreFailure(text) + if (!failure) { + return null + } + + // Diagnosis must never mask the failure we already proved, so probe errors are swallowed. + const diagnosis = await diagnoseWorktreeObjectStore(context.runGit, context.checkoutRef).catch( + () => + ({ + commit: 'unverifiable', + rootTree: 'unverifiable', + partialClone: 'unverifiable' + }) as const + ) + + const described = new Error( + formatGitObjectStoreFailureMessage({ + failure, + branch: context.branch, + commit: diagnosis.commit, + rootTree: diagnosis.rootTree, + partialClone: diagnosis.partialClone + }) + ) + // Why keep the original: main-process logs and telemetry still want the raw argv; only + // the message crossing IPC to the renderer is redacted. + described.cause = error + return described +} diff --git a/src/main/git/worktree-object-store-diagnosis.test.ts b/src/main/git/worktree-object-store-diagnosis.test.ts new file mode 100644 index 00000000000..7a9c65ac2e9 --- /dev/null +++ b/src/main/git/worktree-object-store-diagnosis.test.ts @@ -0,0 +1,137 @@ +import { describe, expect, it, vi } from 'vitest' +import { diagnoseWorktreeObjectStore } from './worktree-object-store-diagnosis' + +const execError = (code: number | undefined): Error => + Object.assign(new Error('Command failed'), code === undefined ? {} : { code }) + +// Real runners put git's stderr on `message` (execFile) or a `stderr` field (relay). +const execErrorWithStderr = (code: number, stderr: string): Error => + Object.assign(new Error(stderr.trim()), { code, stderr }) + +describe('diagnoseWorktreeObjectStore', () => { + it('reports the root tree missing only when git itself answered "no such object"', async () => { + const runGit = vi.fn(async () => { + // A genuinely absent object: `--quiet` exits 1 saying nothing at all. + throw execError(1) + }) + + await expect(diagnoseWorktreeObjectStore(runGit, 'refs/heads/akulafb/test')).resolves.toEqual({ + commit: 'missing', + rootTree: 'missing', + partialClone: 'no' + }) + expect(runGit).toHaveBeenCalledWith([ + 'rev-parse', + '--verify', + '--quiet', + 'refs/heads/akulafb/test^{tree}' + ]) + }) + + it('never turns "we could not run the probe" into "the object is absent"', async () => { + // Dead SSH transport / killed process: no git exit status at all. + const runGit = vi.fn(async () => { + throw execError(undefined) + }) + + await expect(diagnoseWorktreeObjectStore(runGit, 'refs/heads/x')).resolves.toEqual({ + commit: 'unverifiable', + rootTree: 'unverifiable', + partialClone: 'unverifiable' + }) + }) + + it('treats a non-1 git exit status as unverifiable rather than missing', async () => { + const runGit = vi.fn(async () => { + throw execError(128) + }) + + await expect(diagnoseWorktreeObjectStore(runGit, 'refs/heads/x')).resolves.toEqual({ + commit: 'unverifiable', + rootTree: 'unverifiable', + partialClone: 'unverifiable' + }) + }) + + it('does not call an object "missing" when git said it could not READ it', async () => { + // Real git 2.44, tree object on disk at mode 000: the peel exits 1 AND prints why. + const runGit = vi.fn(async (args: string[]) => { + if (args[3] === 'refs/heads/feat^{commit}') { + return { stdout: '9116c0f36c8838ea180a19d9d66e3d1fbc7bb5d9\n' } + } + throw execErrorWithStderr( + 1, + 'error: unable to open loose object ba856f78f34fcefae5d72ef4aec60e70a52ea4a0: Permission denied\n' + ) + }) + + const diagnosis = await diagnoseWorktreeObjectStore(runGit, 'refs/heads/feat') + + expect(diagnosis.commit).toBe('present') + expect(diagnosis.rootTree).toBe('unverifiable') + }) + + it('does not call an object "missing" when the peel tripped over a corrupt file', async () => { + // Real git 2.44 with the commit object truncated to zero bytes. + const runGit = vi.fn(async () => { + throw execErrorWithStderr( + 1, + 'error: object file .git/objects/cc/e159f9d17b185c6da8513db0a7a4d4f8d6585a is empty\n' + ) + }) + + await expect(diagnoseWorktreeObjectStore(runGit, 'refs/heads/feat')).resolves.toEqual({ + commit: 'unverifiable', + rootTree: 'unverifiable', + partialClone: 'unverifiable' + }) + }) + + it('observes a promisor remote as a partial clone', async () => { + const runGit = vi.fn(async (args: string[]) => { + if (args[0] === 'rev-parse') { + return { stdout: 'ba856f78f34fcefae5d72ef4aec60e70a52ea4a0\n' } + } + return { stdout: 'remote.origin.promisor true\n' } + }) + + await expect(diagnoseWorktreeObjectStore(runGit, 'refs/heads/x')).resolves.toEqual({ + commit: 'present', + rootTree: 'present', + partialClone: 'yes' + }) + expect(runGit).toHaveBeenCalledWith(['config', '--get-regexp', '^remote\\..*\\.promisor$']) + }) + + it('probes the commit too, so an unreadable commit is not read as a missing tree', async () => { + // Both peels answer a wordless "no", so the commit is absent rather than unreadable. + const runGit = vi.fn(async (_args: string[]) => { + throw execError(1) + }) + + const diagnosis = await diagnoseWorktreeObjectStore(runGit, 'refs/heads/akulafb/test') + + expect(diagnosis.commit).toBe('missing') + expect(runGit).toHaveBeenCalledWith([ + 'rev-parse', + '--verify', + '--quiet', + 'refs/heads/akulafb/test^{commit}' + ]) + }) + + it('observes the commit present when only the tree peel fails', async () => { + const runGit = vi.fn(async (args: string[]) => { + if (args[3] === 'refs/heads/x^{commit}') { + return { stdout: 'c8a40a3a1ebd165bbdc29303e8c0e7330442abaa\n' } + } + throw execError(1) + }) + + await expect(diagnoseWorktreeObjectStore(runGit, 'refs/heads/x')).resolves.toEqual({ + commit: 'present', + rootTree: 'missing', + partialClone: 'no' + }) + }) +}) diff --git a/src/main/git/worktree-object-store-diagnosis.ts b/src/main/git/worktree-object-store-diagnosis.ts new file mode 100644 index 00000000000..72d237ef7e0 --- /dev/null +++ b/src/main/git/worktree-object-store-diagnosis.ts @@ -0,0 +1,83 @@ +import type { GitObjectPresence, PartialCloneVerdict } from '../../shared/git-object-store-failure' +import { readGitExecErrorText } from './git-exec-error-text' + +/** + * Failure-path-only diagnosis for a `worktree add` that died reading objects. + * + * Why not a preflight: peeling `^{tree}` costs a whole extra git process on every + * create (measured at ~15 ms on a 2.9 GiB repo, the same as the `^{commit}` peel + * it would sit next to) and still proves nothing about subtrees or blobs, so it + * would slow every success for a partial guarantee. Git's own stderr from the + * failed command is the authority; these probes only add detail after it fails. + * + * Git floor: `rev-parse --verify --quiet`, `^{commit}`, `^{tree}` and + * `config --get-regexp` all long predate the 2.25 baseline, so no capability probe is needed. + * + * Executor-injected so the SSH path routes the same argv through the relay. + */ + +export type WorktreeObjectStoreDiagnosis = { + /** Needed to read `rootTree`: an unreadable commit fails the tree peel for its own reason. */ + commit: GitObjectPresence + rootTree: GitObjectPresence + partialClone: PartialCloneVerdict +} + +type GitRunner = (args: string[]) => Promise<{ stdout: string }> + +// Git's own diagnostics. Verified on 2.44.0: a genuinely absent object (or ref, or config +// key) exits 1 with EMPTY stderr, while an object that is present-but-unopenable exits 1 +// *after* printing `error: unable to open loose object : Permission denied` (mode 000) +// or `error: object file is empty` (truncated). Status 1 alone cannot tell them apart. +const GIT_DIAGNOSTIC_LINE = /^(?:error|fatal):/m + +// Why silence too: `--quiet`/`--get-regexp` answer "no" with a *wordless* status 1. Any other +// status, an error carrying no status at all (dead SSH transport, killed process), or a status 1 +// git explained on stderr means the probe never got an answer — and "we could not read it" must +// never be reported as "it is absent". +function gitAnsweredNo(error: unknown): boolean { + if ((error as { code?: unknown } | null)?.code !== 1) { + return false + } + return !GIT_DIAGNOSTIC_LINE.test(readGitExecErrorText(error)) +} + +// Why peel both: `^{tree}` answers a silent "no" when the TREE is gone *and* when the +// COMMIT is gone (verified on git 2.44 with the commit object deleted: both peels exit 1 with +// empty stderr), so the tree peel alone cannot tell those apart. Only the pair does; callers +// must not read `rootTree` on its own. +async function probePeel( + runGit: GitRunner, + rev: string, + peel: 'commit' | 'tree' +): Promise { + try { + const { stdout } = await runGit(['rev-parse', '--verify', '--quiet', `${rev}^{${peel}}`]) + return stdout.trim().length > 0 ? 'present' : 'missing' + } catch (error) { + return gitAnsweredNo(error) ? 'missing' : 'unverifiable' + } +} + +async function probePartialClone(runGit: GitRunner): Promise { + try { + // Why promisor and not extensions.partialClone: current Git records the filter on the + // remote (`remote..promisor`) and leaves the extension unset on fresh clones. + const { stdout } = await runGit(['config', '--get-regexp', '^remote\\..*\\.promisor$']) + return stdout.trim().length > 0 ? 'yes' : 'no' + } catch (error) { + return gitAnsweredNo(error) ? 'no' : 'unverifiable' + } +} + +export async function diagnoseWorktreeObjectStore( + runGit: GitRunner, + rev: string +): Promise { + const [commit, rootTree, partialClone] = await Promise.all([ + probePeel(runGit, rev, 'commit'), + probePeel(runGit, rev, 'tree'), + probePartialClone(runGit) + ]) + return { commit, rootTree, partialClone } +} diff --git a/src/main/git/worktree.ts b/src/main/git/worktree.ts index 55c5d33cf79..b8d0487fac9 100644 --- a/src/main/git/worktree.ts +++ b/src/main/git/worktree.ts @@ -35,6 +35,7 @@ import { withLocalGitCapabilityCacheForExecution } from './git-capability-state' import { gitExecFileAsync, translateWslOutputPaths } from './runner' import { resolveGitDir, runWithGitReadCacheInvalidation } from './status' import { hasWorktreeBaseCommitRef, probeWorktreeBaseRefPresence } from './worktree-base-ref-probe' +import { describeWorktreeAddObjectStoreFailure } from './worktree-add-object-store-error' export type AddWorktreeResult = { localBaseRefRefresh?: LocalBaseRefRefreshResult @@ -101,6 +102,9 @@ export const WORKTREE_REMOVAL_PREFLIGHT_TIMEOUT_MS = 30_000 export const WORKTREE_REMOVAL_REGISTRATION_TIMEOUT_MS = 30_000 // Why: one wedged shared scan otherwise hangs every later list, including create's post-add re-list. export const WORKTREE_LIST_TIMEOUT_MS = 30_000 +// Why: on a partial clone the failure-path `^{tree}` peel can trigger a promisor fetch, so an +// unreachable remote would turn a bounded `worktree add` failure into a create that never settles. +export const WORKTREE_OBJECT_STORE_DIAGNOSIS_TIMEOUT_MS = 10_000 /** * `ORCA_WORKTREE_ADD_TIMEOUT_MS` clamped into [{@link WORKTREE_ADD_TIMEOUT_MS}, @@ -1023,11 +1027,27 @@ async function performAddWorktree( args.push(effectiveBase) } } - await gitExecFileAsync(args, { - ...gitExecOptions(repoPath, options), - // Why: resolve per call — hoisting this to a module const would freeze the override at import. - timeout: resolveWorktreeAddTimeoutMs() - }) + try { + await gitExecFileAsync(args, { + ...gitExecOptions(repoPath, options), + // Why: resolve per call — hoisting this to a module const would freeze the override at import. + timeout: resolveWorktreeAddTimeoutMs() + }) + } catch (error) { + // Why here: `worktree add` reads the root tree, which the commit-only preflight never touched. + const described = await describeWorktreeAddObjectStoreFailure(error, { + runGit: (gitArgs) => + gitExecFileAsync(gitArgs, { + ...gitExecOptions(repoPath, options), + timeout: WORKTREE_OBJECT_STORE_DIAGNOSIS_TIMEOUT_MS + }), + branch, + checkoutRef: options.checkoutExistingBranch + ? `refs/heads/${branch}` + : (effectiveBase ?? 'HEAD') + }) + throw described ?? error + } if (options.checkoutExistingBranch) { return localBaseRefRefresh ? { localBaseRefRefresh } : {} @@ -1110,8 +1130,22 @@ export async function addSparseWorktree( ) return addResult } catch (error) { + // Why here too: sparse passes --no-checkout, so `worktree add` never reads the tree and + // exits 0; the object-store failure surfaces in the sparse-checkout/checkout calls above. + // Probes run against repoPath, whose object store outlives the rollback below. + const described = await describeWorktreeAddObjectStoreFailure(error, { + runGit: (gitArgs) => + gitExecFileAsync(gitArgs, { + ...gitExecOptions(repoPath, options), + timeout: WORKTREE_OBJECT_STORE_DIAGNOSIS_TIMEOUT_MS + }), + branch, + // `worktree add -b` already created the branch, so its ref resolves either way. + checkoutRef: `refs/heads/${branch}` + }) + const failure = described ?? error const wrapped: SparseWorktreeCreateError = - error instanceof Error ? (error as SparseWorktreeCreateError) : new Error(String(error)) + failure instanceof Error ? (failure as SparseWorktreeCreateError) : new Error(String(failure)) if (created) { if (!options.checkoutExistingBranch) { try { diff --git a/src/main/ipc/workspace-create-error-classifier.test.ts b/src/main/ipc/workspace-create-error-classifier.test.ts index 9013cf3065d..443f584d699 100644 --- a/src/main/ipc/workspace-create-error-classifier.test.ts +++ b/src/main/ipc/workspace-create-error-classifier.test.ts @@ -5,6 +5,7 @@ // a renamed throw site needs regression coverage. import { describe, expect, it } from 'vitest' +import { formatGitObjectStoreFailureMessage } from '../../shared/git-object-store-failure' import { classifyWorkspaceCreateError } from './workspace-create-error-classifier' describe('classifyWorkspaceCreateError', () => { @@ -56,6 +57,45 @@ describe('classifyWorkspaceCreateError', () => { expect(classifyWorkspaceCreateError(err)).toBe('git_failed') }) + it('keeps a redacted object-store failure in git_failed', () => { + // The redacted message drops 'fatal:' and the argv, so it needs its own anchor. + const err = new Error( + formatGitObjectStoreFailureMessage({ + failure: { kind: 'unreadable-tree', oid: '041335168f0214913840aaaaaaaaaaaaaaaaaaaa' }, + branch: 'akulafb/test', + commit: 'present', + rootTree: 'missing', + partialClone: 'no' + }) + ) + expect(err.message).not.toContain('fatal:') + expect(classifyWorkspaceCreateError(err)).toBe('git_failed') + }) + + it('keeps a redacted permission failure in permission_denied, not git_failed', () => { + // Real git 2.44 for a tree object at mode 000: the keyword only exists in the raw text. + const raw = new Error( + "Command failed: git worktree add /Users/akulafb/dev/worktrees/test 'akulafb/test'\n" + + 'error: unable to open loose object ba856f78f34fcefae5d72ef4aec60e70a52ea4a0: Permission denied\n' + + 'fatal: unable to read tree c34d8d3a1ebd165bbdc29303e8c0e7330442abaa' + ) + expect(classifyWorkspaceCreateError(raw)).toBe('permission_denied') + + const described = new Error( + formatGitObjectStoreFailureMessage({ + failure: { kind: 'unreadable-tree', oid: 'c34d8d3a1ebd165bbdc29303e8c0e7330442abaa' }, + branch: 'akulafb/test', + commit: 'present', + rootTree: 'unverifiable', + partialClone: 'no' + }) + ) + described.cause = raw + + expect(described.message.toLowerCase()).not.toContain('permission denied') + expect(classifyWorkspaceCreateError(described)).toBe('permission_denied') + }) + it('falls through to unknown for unrecognised errors', () => { const err = new Error('something completely unexpected') expect(classifyWorkspaceCreateError(err)).toBe('unknown') diff --git a/src/main/ipc/workspace-create-error-classifier.ts b/src/main/ipc/workspace-create-error-classifier.ts index 5a24493fd62..45ac3c31f14 100644 --- a/src/main/ipc/workspace-create-error-classifier.ts +++ b/src/main/ipc/workspace-create-error-classifier.ts @@ -12,12 +12,30 @@ // widening this match set later requires explicit review, not silent regex // expansion. Anything we cannot confidently bucket falls through to `unknown`. +import { GIT_OBJECT_STORE_FAILURE_ANCHOR } from '../../shared/git-object-store-failure' import type { WorkspaceCreateErrorClass } from '../../shared/telemetry-events' +// Why the cause chain and not just `message`: object-store failures reach here redacted +// (worktree-add-object-store-error.ts rebuilds the message and keeps the raw git text on +// `cause`), and that raw text is the only place `Permission denied`/`EACCES` survives. Reading +// it back is a widening of the INPUT, not of the match set below, and the matched strings still +// never leave this process — only the enum does. +const MAX_CAUSE_DEPTH = 4 + +function readErrorChainText(error: unknown): string { + const parts: string[] = [] + let current: unknown = error + for (let depth = 0; depth < MAX_CAUSE_DEPTH && current instanceof Error; depth += 1) { + parts.push(current.message) + current = current.cause + } + return parts.join('\n') +} + export function classifyWorkspaceCreateError(error: unknown): WorkspaceCreateErrorClass { // Why: throw sites mix capitalization ('Worktree created...' vs lowercased // git messages); normalize once so all anchors below can be lowercase literals. - const text = (error instanceof Error ? error.message : '').toLowerCase() + const text = readErrorChainText(error).toLowerCase() if (text.includes('could not resolve a default base ref')) { return 'base_ref_missing' @@ -45,7 +63,9 @@ export function classifyWorkspaceCreateError(error: unknown): WorkspaceCreateErr if ( text.includes('fatal:') || text.includes('git worktree') || - text.includes('created but not found in listing') + text.includes('created but not found in listing') || + // Redaction strips 'fatal:' and the argv from object-store failures; keep them bucketed. + text.includes(GIT_OBJECT_STORE_FAILURE_ANCHOR) ) { return 'git_failed' } diff --git a/src/main/ipc/worktree-remote-object-store-probe-bound.test.ts b/src/main/ipc/worktree-remote-object-store-probe-bound.test.ts new file mode 100644 index 00000000000..d5e351519ab --- /dev/null +++ b/src/main/ipc/worktree-remote-object-store-probe-bound.test.ts @@ -0,0 +1,32 @@ +// The object-store diagnosis probes run on the create FAILURE path, where a partial clone +// can turn `rev-parse ^{tree}` into a promisor fetch against an unreachable remote. The local +// path bounds every probe at WORKTREE_OBJECT_STORE_DIAGNOSIS_TIMEOUT_MS +// (worktree-add-object-store-diagnosis-timeout.test.ts); the SSH path must not silently +// inherit the multiplexer's 30s ambient default and hold a failing create three times longer. +import { readFileSync } from 'node:fs' +import { join } from 'node:path' +import { describe, expect, it } from 'vitest' + +import { WORKTREE_OBJECT_STORE_DIAGNOSIS_TIMEOUT_MS } from '../git/worktree' + +const SOURCE = readFileSync(join(__dirname, 'worktree-remote.ts'), 'utf8') + +// Call-site audit (same shape as global-fetch-call-site-audit.test.ts): the diagnosis +// runner is built inline at each create path, so the bound is only visible in the source. +const DIAGNOSIS_RUNNER = /provider\.exec\(gitArgs,\s*repo\.path([^)]*)\)/g + +describe('SSH worktree-create object-store diagnosis probes', () => { + it('bounds every probe instead of inheriting the 30s relay request default', () => { + const runners = [...SOURCE.matchAll(DIAGNOSIS_RUNNER)].map(([, args]) => args) + + // Both create paths diagnose: plain `worktree add`, and the sparse follow-up checkout. + expect(runners).toHaveLength(2) + for (const args of runners) { + expect(args).toContain('timeoutMs') + } + }) + + it('keeps the SSH bound well under the ambient relay request timeout it replaces', () => { + expect(WORKTREE_OBJECT_STORE_DIAGNOSIS_TIMEOUT_MS).toBeLessThan(30_000) + }) +}) diff --git a/src/main/ipc/worktree-remote.ts b/src/main/ipc/worktree-remote.ts index 1d63cf97deb..b58e3312f08 100644 --- a/src/main/ipc/worktree-remote.ts +++ b/src/main/ipc/worktree-remote.ts @@ -26,7 +26,13 @@ import type { WorktreeHeadIdentity } from '../../shared/worktree/types' import { getPRForBranch } from '../github/client' -import { listWorktrees, addWorktree, addSparseWorktree } from '../git/worktree' +import { + listWorktrees, + addWorktree, + addSparseWorktree, + WORKTREE_OBJECT_STORE_DIAGNOSIS_TIMEOUT_MS +} from '../git/worktree' +import { describeWorktreeAddObjectStoreFailure } from '../git/worktree-add-object-store-error' import type { AddWorktreeOptions, AddWorktreeResult } from '../git/worktree' import { getBranchConflictKind, @@ -594,6 +600,11 @@ function normalizeLocalBranchName(branchName: string | undefined): string { return branchName?.replace(/^refs\/heads\//, '') ?? '' } +// Preflight contract: `^{commit}` peels the ref and reads ONLY the commit object, so a +// pass proves the branch tip is resolvable — NOT that its tree and blobs are readable. +// `git worktree add` reads the root tree next and can still die there; that failure is +// diagnosed and redacted in git/worktree-add-object-store-error.ts. Do not widen this +// probe into a tree walk: it runs on every create and would slow every success. async function canCheckoutExistingLocalBranch( repoPath: string, branchName: string, @@ -728,6 +739,11 @@ async function hasCommitRefSsh( } } +// Preflight contract: `^{commit}` peels the ref and reads ONLY the commit object, so a +// pass proves the branch tip is resolvable — NOT that its tree and blobs are readable. +// `git worktree add` reads the root tree next and can still die there; that failure is +// diagnosed and redacted in git/worktree-add-object-store-error.ts. Do not widen this +// probe into a tree walk: it runs on every create and would slow every success. async function canCheckoutExistingLocalBranchSsh( provider: SshGitProvider, repoPath: string, @@ -1783,7 +1799,18 @@ export async function createRemoteWorktree( `Older relay reported an authorization error; please reconnect to deploy the latest relay. (${err.message})` ) } - throw err + // Client-side so the diagnosis works against any relay version, without a wire change. + const described = await describeWorktreeAddObjectStoreFailure(err, { + runGit: (gitArgs) => + // Why bound: unbounded here is the mux's 30s ambient default, 3x the local path's + // bound, so a promisor fetch at an unreachable remote holds the failing create for it. + provider.exec(gitArgs, repo.path, { + timeoutMs: WORKTREE_OBJECT_STORE_DIAGNOSIS_TIMEOUT_MS + }), + branch: branchName, + checkoutRef: checkoutExistingBranch ? `refs/heads/${branchName}` : baseBranch + }) + throw described ?? err } if (sparseDirectories.length > 0) { try { @@ -1816,7 +1843,20 @@ export async function createRemoteWorktree( if (!rollbackSucceeded && shouldRetireGeneratedName) { await retireGeneratedWorktreeName(store, repo, settings, effectiveSanitizedName) } - throw err + // Parity with local addSparseWorktree: --no-checkout means `worktree add` never read + // the tree, so the object-store failure lands here instead. Probes target repo.path, + // which outlives the rollback above. + const described = await describeWorktreeAddObjectStoreFailure(err, { + runGit: (gitArgs) => + // Why bound: unbounded here is the mux's 30s ambient default, 3x the local path's + // bound, so a promisor fetch at an unreachable remote holds the failing create for it. + provider.exec(gitArgs, repo.path, { + timeoutMs: WORKTREE_OBJECT_STORE_DIAGNOSIS_TIMEOUT_MS + }), + branch: branchName, + checkoutRef: `refs/heads/${branchName}` + }) + throw described ?? err } } diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 98f138a3567..674c58bbc67 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -2608,6 +2608,11 @@ function addListenerToMap(map: Map>, key: string, listener: T) } } +// Preflight contract: `^{commit}` peels the ref and reads ONLY the commit object, so a +// pass proves the branch tip is resolvable — NOT that its tree and blobs are readable. +// `git worktree add` reads the root tree next and can still die there; that failure is +// diagnosed and redacted in git/worktree-add-object-store-error.ts. Do not widen this +// probe into a tree walk: it runs on every create and would slow every success. async function canCheckoutExistingLocalBranch( repoPath: string, branchName: string, diff --git a/src/renderer/src/components/worktree-creation/WorktreeCreationPanel.test.tsx b/src/renderer/src/components/worktree-creation/WorktreeCreationPanel.test.tsx index bb8e5687239..074f65c7afb 100644 --- a/src/renderer/src/components/worktree-creation/WorktreeCreationPanel.test.tsx +++ b/src/renderer/src/components/worktree-creation/WorktreeCreationPanel.test.tsx @@ -166,6 +166,21 @@ describe('WorktreeCreationPanel', () => { expect(container.textContent).toContain('Retry') }) + it('renders the full object-store diagnosis, not just its short label', async () => { + mocks.state.pendingWorktreeCreations['create-1'] = { + ...mocks.state.pendingWorktreeCreations['create-1'], + status: 'error', + error: 'Repository objects are missing', + errorDetail: + 'Orca could not create this workspace because the repository object database is missing objects. ' + + 'Run git fsck in the repository to confirm what is missing.' + } + + const container = await renderPanel(false) + + expect(container.textContent).toContain('git fsck') + }) + it('omits the recipe output panel for a non-VM creation failure', async () => { mocks.state.pendingWorktreeCreations['create-1'] = { ...mocks.state.pendingWorktreeCreations['create-1'], diff --git a/src/renderer/src/components/worktree-creation/WorktreeCreationPanel.tsx b/src/renderer/src/components/worktree-creation/WorktreeCreationPanel.tsx index c56d78a44e0..e47c3e5cb90 100644 --- a/src/renderer/src/components/worktree-creation/WorktreeCreationPanel.tsx +++ b/src/renderer/src/components/worktree-creation/WorktreeCreationPanel.tsx @@ -106,7 +106,8 @@ export default function WorktreeCreationPanel({ log={entry.provisioningLog ?? ''} error={ isError - ? (entry.error ?? + ? (entry.errorDetail ?? + entry.error ?? translate( 'auto.components.worktree.creation.WorktreeCreationPanel.767951265d', 'Something went wrong while creating the worktree.' @@ -126,7 +127,8 @@ export default function WorktreeCreationPanel({ )} - {entry.error ?? + {entry.errorDetail ?? + entry.error ?? translate( 'auto.components.worktree.creation.WorktreeCreationPanel.767951265d', 'Something went wrong while creating the worktree.' diff --git a/src/renderer/src/lib/pending-worktree-creation.ts b/src/renderer/src/lib/pending-worktree-creation.ts index 42ec2f2c392..26a43b79a23 100644 --- a/src/renderer/src/lib/pending-worktree-creation.ts +++ b/src/renderer/src/lib/pending-worktree-creation.ts @@ -127,7 +127,10 @@ export type PendingWorktreeCreation = { * background creates set this immediately so the faux tab strip stays stable * from create start through terminal handoff. */ loaderVisible: boolean + /** Short label for the truncating sidebar row. */ error?: string + /** Full diagnosis for the creation panel; the sidebar row would truncate it away. */ + errorDetail?: string provisioningLog?: string request: WorktreeCreationRequest } diff --git a/src/renderer/src/lib/workspace-create-error-format.test.ts b/src/renderer/src/lib/workspace-create-error-format.test.ts index 4f8fc7a591a..649e628d312 100644 --- a/src/renderer/src/lib/workspace-create-error-format.test.ts +++ b/src/renderer/src/lib/workspace-create-error-format.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it } from 'vitest' +import { formatGitObjectStoreFailureMessage } from '../../../shared/git-object-store-failure' import { formatWorkspaceCreateError, getWorkspaceCreateErrorToastMessage @@ -29,6 +30,128 @@ describe('formatWorkspaceCreateError', () => { expect(formatted.help).toBeDefined() }) + it('redacts a raw leaked worktree-add failure from an older host', () => { + // Pre-fix hosts (and older relays) rethrow git's message verbatim through Electron. + const formatted = formatWorkspaceCreateError( + new Error( + "Error invoking remote method 'worktrees:create': Error: Command failed: " + + "git worktree add /Users/akulafb/dev/worktrees/test 'akulafb/test'\n" + + 'fatal: unable to read tree (041335168f0214913840aaaaaaaaaaaaaaaaaaaa)' + ) + ) + + expect(formatted.title).toBe('Repository objects are missing') + expect(formatted.message).not.toContain('/Users/akulafb') + expect(formatted.message).not.toContain('git worktree add') + expect(formatted.message).not.toContain('Error invoking remote method') + expect(formatted.help).toContain('git fsck') + expect(getWorkspaceCreateErrorToastMessage(formatted)).toBe('Repository objects are missing') + }) + + it("redacts the sparse path's checkout failure, which names no path in git's own wording", () => { + // Verbatim git 2.44.0 stderr for a sparse create whose root tree is gone; the leak is + // the argv line execFile prepends, which carries the branch the user typed. + const formatted = formatWorkspaceCreateError( + new Error( + "Error invoking remote method 'worktrees:create': Error: Command failed: " + + 'git checkout akulafb/test\n' + + 'fatal: unable to parse commit 435b1d6c622920a72b8984ec55742106c5434436' + ) + ) + + expect(formatted.title).toBe('Repository objects are missing') + expect(formatted.message).toContain( + 'unable to parse commit 435b1d6c622920a72b8984ec55742106c5434436' + ) + // Nothing was probed on this side, so no claim about which object is gone. + expect(formatted.message).not.toContain('root tree object is missing') + expect(formatted.message).not.toContain('Command failed') + expect(formatted.message).not.toContain('git checkout') + expect(formatted.help).toContain('git fsck') + }) + + it('keeps the rollback cleanup note the host appended after the repair guidance', () => { + const diagnosed = formatGitObjectStoreFailureMessage({ + failure: { kind: 'unparsable-commit', oid: '435b1d6c622920a72b8984ec55742106c5434436' }, + branch: 'akulafb/test', + commit: 'present', + rootTree: 'missing', + partialClone: 'no' + }) + + const formatted = formatWorkspaceCreateError( + new Error( + `${diagnosed} (cleanup also failed — the partially created worktree at "/Users/akulafb/dev/worktrees/test" may need manual removal)` + ) + ) + + expect(formatted.message).not.toContain('may need manual removal') + expect(formatted.help).toContain('may need manual removal') + }) + + it('keeps the diagnosed message the host already composed', () => { + const hostMessage = formatGitObjectStoreFailureMessage({ + failure: { kind: 'unreadable-tree', oid: '041335168f0214913840aaaaaaaaaaaaaaaaaaaa' }, + branch: 'akulafb/test', + commit: 'present', + rootTree: 'missing', + partialClone: 'no' + }) + + const formatted = formatWorkspaceCreateError(new Error(hostMessage)) + + expect(formatted.title).toBe('Repository objects are missing') + expect(formatted.message).toContain('root tree object is missing') + expect(formatted.message).not.toContain('Error invoking remote method') + // Repair guidance moves into help so the toast stays short. + expect(formatted.message).not.toContain('git fsck') + expect(formatted.help).toContain('git fsck') + expect(getWorkspaceCreateErrorToastMessage(formatted)).toBe('Repository objects are missing') + }) + + it('strips the IPC envelope from a diagnosed message a modern host composed', () => { + // Electron's ipcRenderer.invoke prefixes every rejection it forwards; the host text is intact behind it. + const hostMessage = formatGitObjectStoreFailureMessage({ + failure: { kind: 'unreadable-tree', oid: '041335168f0214913840aaaaaaaaaaaaaaaaaaaa' }, + branch: 'akulafb/test', + commit: 'present', + rootTree: 'missing', + partialClone: 'no' + }) + + const formatted = formatWorkspaceCreateError( + new Error(`Error invoking remote method 'worktrees:create': Error: ${hostMessage}`) + ) + + expect(formatted.message).not.toContain('Error invoking remote method') + expect(formatted.message.startsWith('Orca could not create this workspace')).toBe(true) + expect(formatted.message).toContain('root tree object is missing') + expect(formatted.help).toContain('git fsck') + }) + + it('strips the IPC envelope even when a newer host worded the diagnosis its own way', () => { + const formatted = formatWorkspaceCreateError( + new Error( + "Error invoking remote method 'worktrees:create': Error: Future host wording: the repository object database is missing objects." + ) + ) + + expect(formatted.message).not.toContain('Error invoking remote method') + expect(formatted.message).toContain('Future host wording') + }) + + it('still surfaces repair guidance when a newer host worded its own tail differently', () => { + const formatted = formatWorkspaceCreateError( + new Error( + 'Orca could not create this workspace because the repository object database is missing objects. Some future wording.' + ) + ) + + expect(formatted.title).toBe('Repository objects are missing') + expect(formatted.message).toContain('Some future wording.') + expect(formatted.help).toContain('git fsck') + }) + it('passes unknown errors through unchanged', () => { const formatted = formatWorkspaceCreateError(new Error('fatal: not a git repository')) diff --git a/src/renderer/src/lib/workspace-create-error-format.ts b/src/renderer/src/lib/workspace-create-error-format.ts index d33f0d066fe..d80cb598132 100644 --- a/src/renderer/src/lib/workspace-create-error-format.ts +++ b/src/renderer/src/lib/workspace-create-error-format.ts @@ -1,4 +1,12 @@ import { translate } from '@/i18n/i18n' +import { + classifyGitObjectStoreFailure, + formatGitObjectStoreFailureMessage, + isGitObjectStoreFailureMessage, + splitGitObjectStoreRepairGuidance, + unwrapGitObjectStoreFailureMessage +} from '../../../shared/git-object-store-failure' + export type WorkspaceCreateErrorDisplay = { title: string message: string @@ -21,12 +29,63 @@ export function formatWorkspaceCreateError(error: unknown): WorkspaceCreateError } } + const objectStore = formatGitObjectStoreError(message) + if (objectStore) { + return objectStore + } + return { title: message, message } } +const OBJECT_STORE_TITLE = 'Repository objects are missing' + +/** + * Why classify here and not only on the host: the raw text also reaches this client from + * older hosts and relays that rethrow git's stderr verbatim, and that text carries the + * user's absolute worktree path and the whole argv. Redacting client-side fixes the leak + * for every host version without a wire change. + */ +function formatGitObjectStoreError(message: string): WorkspaceCreateErrorDisplay | null { + // An already-diagnosed host message passes through; raw git stderr gets composed here instead. + const failure = isGitObjectStoreFailureMessage(message) + ? null + : classifyGitObjectStoreFailure(message) + const composed = failure + ? // Nothing was probed on this side, so claim neither a missing tree nor a partial clone. + formatGitObjectStoreFailureMessage({ + failure, + branch: extractQuotedBranch(message) ?? 'this branch', + commit: 'unverifiable', + rootTree: 'unverifiable', + partialClone: 'unverifiable' + }) + : isGitObjectStoreFailureMessage(message) + ? unwrapGitObjectStoreFailureMessage(message) + : null + if (composed === null) { + return null + } + const { summary, repair } = splitGitObjectStoreRepairGuidance(composed) + return { title: OBJECT_STORE_TITLE, message: summary, help: repair } +} + +// Git echoes the checked-out branch as `(checking out 'name')`; the surrounding argv is discarded. +function extractQuotedBranch(message: string): string | null { + return message.match(/checking out '([^']+)'/)?.[1] ?? null +} + export function getWorkspaceCreateErrorToastMessage(error: WorkspaceCreateErrorDisplay): string { return error.help ? error.title : error.message } + +/** + * Full text for surfaces that render one string (the creation panel). The background create + * path never rethrows, so the composer footer that shows title+message+help never runs and + * this is the only place the user can read what to repair. + */ +export function getWorkspaceCreateErrorDetail(error: WorkspaceCreateErrorDisplay): string { + return error.help ? `${error.message} ${error.help}` : error.message +} diff --git a/src/renderer/src/lib/worktree-creation-flow-object-store-diagnosis.test.ts b/src/renderer/src/lib/worktree-creation-flow-object-store-diagnosis.test.ts new file mode 100644 index 00000000000..53b54146b1e --- /dev/null +++ b/src/renderer/src/lib/worktree-creation-flow-object-store-diagnosis.test.ts @@ -0,0 +1,209 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { + PendingWorktreeCreation, + WorktreeCreationRequest +} from '@/lib/pending-worktree-creation' + +// Split from worktree-creation-flow.test.ts: the background create path's handling of a +// diagnosed object-store failure, where the pending entry is the only surface the user reads. + +type TestActiveView = 'terminal' | 'tasks' + +const store = { + settings: { + activeRuntimeEnvironmentId: null as string | null, + experimentalNativeChat: undefined as boolean | undefined, + openAgentTabsInChatByDefault: undefined as boolean | undefined + }, + activeView: 'terminal' as TestActiveView, + activePendingCreationId: 'creation-1' as string | null, + repos: [] as { id: string; connectionId: string | null }[], + pendingWorktreeCreations: {} as Record, + beginPendingWorktreeCreation: vi.fn((entry: PendingWorktreeCreation) => { + store.pendingWorktreeCreations[entry.creationId] = entry + store.activePendingCreationId = entry.creationId + }), + updatePendingWorktreeCreation: vi.fn( + (creationId: string, patch: Partial) => { + const entry = store.pendingWorktreeCreations[creationId] + if (entry) { + store.pendingWorktreeCreations[creationId] = { ...entry, ...patch } + } + } + ), + removePendingWorktreeCreation: vi.fn((creationId: string) => { + delete store.pendingWorktreeCreations[creationId] + }), + updateWorktreeMeta: vi.fn(), + setActivePendingWorktreeCreation: vi.fn(), + setActiveView: vi.fn(), + setSidebarOpen: vi.fn(), + createWorktree: vi.fn(() => new Promise(() => {})), + setupProjectExistingFolder: vi.fn(), + refreshRuntimeEnvironmentStatus: vi.fn(), + seedNativeChatLaunchDraft: vi.fn(), + setTabViewMode: vi.fn(), + tabsByWorktree: {} as Record, + unifiedTabsByWorktree: {} +} + +vi.mock('@/store', () => ({ + useAppStore: { + getState: () => store + } +})) + +vi.mock('@/lib/browser-uuid', () => ({ + createBrowserUuid: () => 'creation-1' +})) + +vi.mock('@/lib/worktree-activation', () => ({ + activateAndRevealWorktree: vi.fn(() => false) +})) + +vi.mock('@/lib/worktree-initial-terminal-seeding', () => ({ + ensureWorktreeHasInitialTerminal: vi.fn() +})) + +vi.mock('@/lib/workspace-activation-terminal-focus', () => ({ + queueWorkspaceActivationTerminalFocus: vi.fn() +})) + +vi.mock('@/lib/new-workspace', () => ({ + ensureAgentStartupInTerminal: vi.fn() +})) + +vi.mock('sonner', () => ({ + toast: { + error: vi.fn() + } +})) + +vi.mock('@/lib/ephemeral-vm-workspace-target', () => ({ + prepareEphemeralVmWorkspaceTarget: vi.fn() +})) + +import { toast } from 'sonner' +import { ensureWorktreeHasInitialTerminal } from '@/lib/worktree-initial-terminal-seeding' +import { + continueBackgroundWorktreeCreation, + retryBackgroundWorktreeCreation +} from './worktree-creation-flow' + +function makeRequest(overrides: Partial = {}): WorktreeCreationRequest { + return { + repoId: 'repo-1', + name: 'feature', + setupDecision: 'inherit', + agent: null, + pendingFirstAgentMessageRename: false, + note: '', + startupPlan: null, + quickPrompt: '', + quickTelemetry: null, + ...overrides + } +} + +function makePendingCreation(request: WorktreeCreationRequest): PendingWorktreeCreation { + return { + creationId: 'creation-1', + phase: 'preparing', + status: 'creating', + startedAt: 1, + indeterminate: false, + loaderVisible: true, + request + } +} + +async function flushAsyncWorktreeCreation(): Promise { + await Promise.resolve() + await Promise.resolve() +} + +beforeEach(() => { + vi.clearAllMocks() + store.settings.activeRuntimeEnvironmentId = null + store.activeView = 'terminal' + store.activePendingCreationId = 'creation-1' + store.repos = [] + store.pendingWorktreeCreations = { 'creation-1': makePendingCreation(makeRequest()) } + store.createWorktree.mockImplementation(() => new Promise(() => {})) + store.tabsByWorktree = {} + store.unifiedTabsByWorktree = {} + vi.mocked(ensureWorktreeHasInitialTerminal).mockReturnValue('tab-1') +}) + +describe('staged background worktree creation on an object-store failure', () => { + it('keeps the actionable diagnosis on the pending entry while the toast stays short', async () => { + store.activeView = 'tasks' + store.createWorktree.mockRejectedValueOnce( + new Error( + 'Orca could not create this workspace because the repository object database is missing objects. ' + + 'Git reported: unable to read tree (041335168f0214913840aaaaaaaaaaaaaaaaaaaa). ' + + 'Run git fsck in the repository to confirm what is missing, then re-fetch from the remote or re-clone to restore it.' + ) + ) + + const started = continueBackgroundWorktreeCreation('creation-1', makeRequest(), { + revealCreationSurface: false + }) + + expect(started).toBe(true) + await flushAsyncWorktreeCreation() + // The panel is the only surface the user gets, so it must carry the oid and the repair command. + expect(store.updatePendingWorktreeCreation).toHaveBeenCalledWith( + 'creation-1', + expect.objectContaining({ + status: 'error', + error: 'Repository objects are missing', + errorDetail: expect.stringContaining('git fsck') + }) + ) + const detail = store.updatePendingWorktreeCreation.mock.calls.at(-1)?.[1]?.errorDetail ?? '' + expect(detail).toContain('041335168f0214913840aaaaaaaaaaaaaaaaaaaa') + expect(toast.error).toHaveBeenCalledWith('Repository objects are missing') + }) + + it('surfaces the sparse path failure, which git words as an unparsable commit', async () => { + store.activeView = 'tasks' + // Verbatim git 2.44.0 stderr for a sparse create whose root tree is gone: the failure + // lands in the follow-up `git checkout`, not in `worktree add --no-checkout`. + store.createWorktree.mockRejectedValueOnce( + new Error( + 'Command failed: git checkout akulafb/test\n' + + 'fatal: unable to parse commit 435b1d6c622920a72b8984ec55742106c5434436' + ) + ) + + continueBackgroundWorktreeCreation('creation-1', makeRequest(), { + revealCreationSurface: false + }) + await flushAsyncWorktreeCreation() + + const detail = store.updatePendingWorktreeCreation.mock.calls.at(-1)?.[1]?.errorDetail ?? '' + expect(detail).toContain('unable to parse commit 435b1d6c622920a72b8984ec55742106c5434436') + expect(detail).not.toContain('Command failed') + expect(toast.error).toHaveBeenCalledWith('Repository objects are missing') + }) + + it('clears the previous diagnosis when a retry starts', async () => { + store.createWorktree.mockRejectedValueOnce( + new Error( + 'Orca could not create this workspace because the repository object database is missing objects. ' + + 'Run git fsck in the repository to confirm what is missing.' + ) + ) + continueBackgroundWorktreeCreation('creation-1', makeRequest(), { + revealCreationSurface: false + }) + await flushAsyncWorktreeCreation() + expect(store.pendingWorktreeCreations['creation-1']?.errorDetail).toContain('git fsck') + + retryBackgroundWorktreeCreation('creation-1') + + // Why: the panel prefers errorDetail, so a stale one would outlive the failure it described. + expect(store.pendingWorktreeCreations['creation-1']?.errorDetail).toBeUndefined() + }) +}) diff --git a/src/renderer/src/lib/worktree-creation-flow.ts b/src/renderer/src/lib/worktree-creation-flow.ts index d99a9792282..b17443878da 100644 --- a/src/renderer/src/lib/worktree-creation-flow.ts +++ b/src/renderer/src/lib/worktree-creation-flow.ts @@ -13,6 +13,7 @@ import { import { getProvisionedRootCreateOptions } from '@/lib/provisioned-root-create-options' import { formatWorkspaceCreateError, + getWorkspaceCreateErrorDetail, getWorkspaceCreateErrorToastMessage } from '@/lib/workspace-create-error-format' import type { CreateWorktreeResult } from '../../../shared/worktree/create-types' @@ -157,18 +158,21 @@ async function executeWorktreeCreation( if (preparedRequest.ephemeralVmRuntimeId) { await cleanupEphemeralVmRuntimeForFailedCreate(preparedRequest) } - const message = getWorkspaceCreateErrorToastMessage(formatWorkspaceCreateError(error)) + const formatted = formatWorkspaceCreateError(error) // Why: an error must stay on the same creation surface that owns the faux // tab strip, rather than falling back to stale previous-workspace tabs. useAppStore.getState().updatePendingWorktreeCreation(creationId, { status: 'error', - error: message, + error: getWorkspaceCreateErrorToastMessage(formatted), + // Why: the panel renders one string and this path never rethrows, so the short + // title alone would drop the only repair instructions the user ever sees. + errorDetail: getWorkspaceCreateErrorDetail(formatted), ...(preparedRequest.ephemeralVmRecipe ? { request } : {}) }) // Why: only toast when the panel isn't already showing this error (the user // navigated away), so a visible failure isn't announced twice. if (!isPendingCreationSurfaceVisible(creationId)) { - toast.error(message) + toast.error(getWorkspaceCreateErrorToastMessage(formatted)) } return } @@ -323,6 +327,7 @@ export function continueBackgroundWorktreeCreation( status: 'creating', startedAt: Date.now(), error: undefined, + errorDetail: undefined, provisioningLog: undefined, request }) @@ -352,6 +357,7 @@ export function retryBackgroundWorktreeCreation(creationId: string): void { ? 'provisioning-vm' : 'fetching', error: undefined, + errorDetail: undefined, provisioningLog: undefined }) store.setActivePendingWorktreeCreation(creationId) diff --git a/src/renderer/src/store/slices/worktree-helpers.ts b/src/renderer/src/store/slices/worktree-helpers.ts index fb368bec070..a72558590de 100644 --- a/src/renderer/src/store/slices/worktree-helpers.ts +++ b/src/renderer/src/store/slices/worktree-helpers.ts @@ -223,6 +223,7 @@ export type WorktreeSlice = { status?: 'creating' | 'error' startedAt?: number error?: string + errorDetail?: string loaderVisible?: boolean request?: PendingWorktreeCreation['request'] provisioningLog?: string diff --git a/src/shared/git-object-store-failure.test.ts b/src/shared/git-object-store-failure.test.ts new file mode 100644 index 00000000000..ab54e8ffb6a --- /dev/null +++ b/src/shared/git-object-store-failure.test.ts @@ -0,0 +1,153 @@ +import { describe, expect, it } from 'vitest' +import { + GIT_OBJECT_STORE_FAILURE_ANCHOR, + GIT_OBJECT_STORE_REPAIR_GUIDANCE, + classifyGitObjectStoreFailure, + formatGitObjectStoreFailureMessage +} from './git-object-store-failure' + +// Verbatim field report (1.4.187 / macOS), path redacted the way the reporter redacted it. +const FIELD_STDERR = + "Command failed: git worktree add /Users/akulafb/dev/worktrees/test 'akulafb/test'\n" + + "Preparing worktree (checking out 'akulafb/test')\n" + + 'fatal: unable to read tree (041335168f0214913840aaaaaaaaaaaaaaaaaaaa)' + +describe('classifyGitObjectStoreFailure', () => { + it('classifies the reported parenthesised unreadable-tree fatal and keeps the oid', () => { + expect(classifyGitObjectStoreFailure(FIELD_STDERR)).toEqual({ + kind: 'unreadable-tree', + oid: '041335168f0214913840aaaaaaaaaaaaaaaaaaaa' + }) + }) + + it('classifies the unparenthesised form emitted by other Git versions', () => { + expect( + classifyGitObjectStoreFailure( + 'fatal: unable to read tree 6d4882f90c4c5154793d3e4ec49968a42f87005f' + ) + ).toEqual({ kind: 'unreadable-tree', oid: '6d4882f90c4c5154793d3e4ec49968a42f87005f' }) + }) + + it('classifies missing blobs, corrupt loose objects and empty object files', () => { + expect( + classifyGitObjectStoreFailure( + "error: missing blob object 'deadbeefdeadbeefdeadbeefdeadbeefdeadbeef'" + ) + ).toEqual({ kind: 'missing-object', oid: 'deadbeefdeadbeefdeadbeefdeadbeefdeadbeef' }) + expect( + classifyGitObjectStoreFailure('error: object file .git/objects/ab/cd is empty')?.kind + ).toBe('corrupt-object') + expect( + classifyGitObjectStoreFailure( + 'error: loose object abcd1234 (stored in .git/objects/ab/cd) is corrupt' + )?.kind + ).toBe('corrupt-object') + }) + + // Verbatim `git checkout` stderr on git 2.44.0 when the branch's root tree is deleted: + // the sparse-create path dies here, not in `worktree add --no-checkout`, which exits 0. + it('classifies the checkout-side wording the sparse create path dies on', () => { + expect( + classifyGitObjectStoreFailure( + 'Command failed: git checkout akulafb/test\nfatal: unable to parse commit 435b1d6c622920a72b8984ec55742106c5434436' + ) + ).toEqual({ + kind: 'unparsable-commit', + oid: '435b1d6c622920a72b8984ec55742106c5434436' + }) + }) + + it('does not claim an object-store failure for unrelated git errors', () => { + expect(classifyGitObjectStoreFailure('fatal: not a git repository')).toBeNull() + expect( + classifyGitObjectStoreFailure("fatal: 'wt' already exists and is not an empty directory") + ).toBeNull() + expect(classifyGitObjectStoreFailure('')).toBeNull() + }) +}) + +describe('formatGitObjectStoreFailureMessage', () => { + const failure = classifyGitObjectStoreFailure(FIELD_STDERR)! + + it('never leaks the local path or the argv that failed', () => { + const message = formatGitObjectStoreFailureMessage({ + failure, + branch: 'akulafb/test', + commit: 'present', + rootTree: 'missing', + partialClone: 'no' + }) + + expect(message).not.toContain('/Users/akulafb') + expect(message).not.toContain('Command failed') + expect(message).not.toContain('git worktree add') + expect(message).toContain(GIT_OBJECT_STORE_FAILURE_ANCHOR) + expect(message).toContain('akulafb/test') + expect(message).toContain('git fsck') + }) + + it('only claims the root tree is missing when the probe actually observed it', () => { + const unverifiable = formatGitObjectStoreFailureMessage({ + failure, + branch: 'akulafb/test', + commit: 'unverifiable', + rootTree: 'unverifiable', + partialClone: 'unverifiable' + }) + + expect(unverifiable).not.toContain('root tree object is missing') + expect(unverifiable).not.toContain('partial clone') + expect(unverifiable).toContain(GIT_OBJECT_STORE_FAILURE_ANCHOR) + }) + + it('recommends a refetch when a promisor remote was actually observed', () => { + const partial = formatGitObjectStoreFailureMessage({ + failure, + branch: 'akulafb/test', + commit: 'present', + rootTree: 'missing', + partialClone: 'yes' + }) + + expect(partial).toContain('partial clone') + expect(partial).toContain('git fetch') + }) + + it('does not claim the commit survived when the commit probe did not observe it', () => { + // Real git 2.44 on a repo with an emptied commit object: `^{commit}` and `^{tree}` + // both exit 1, so a bare tree peel proves nothing about the tree. + const message = formatGitObjectStoreFailureMessage({ + failure, + branch: 'akulafb/test', + commit: 'missing', + rootTree: 'missing', + partialClone: 'no' + }) + + expect(message).not.toContain('root tree object is missing') + expect(message).not.toContain('is present but') + expect(message).toContain('Git could not read every object') + }) + + it('claims the commit-present/tree-missing shape only when both probes observed it', () => { + const message = formatGitObjectStoreFailureMessage({ + failure, + branch: 'akulafb/test', + commit: 'present', + rootTree: 'missing', + partialClone: 'no' + }) + + expect(message).toContain('root tree object is missing') + }) + + it('does not blanket-exonerate Orca, which runs `git worktree prune` and leaves auto-gc on', () => { + // src/main/git/worktree.ts, local-worktree-removal-recovery.ts, orca-runtime.ts and + // relay/git-handler-worktree-remove.ts all run `worktree prune`; auto-maintenance is + // suppressed for exactly one fetch, so Orca cannot promise nothing of its doing ran. + expect(GIT_OBJECT_STORE_REPAIR_GUIDANCE).not.toMatch(/never runs/i) + expect(GIT_OBJECT_STORE_REPAIR_GUIDANCE).not.toMatch(/nothing in orca/i) + expect(GIT_OBJECT_STORE_REPAIR_GUIDANCE).not.toMatch(/prune/i) + expect(GIT_OBJECT_STORE_REPAIR_GUIDANCE).toContain('git fsck') + }) +}) diff --git a/src/shared/git-object-store-failure.ts b/src/shared/git-object-store-failure.ts new file mode 100644 index 00000000000..580141bd84d --- /dev/null +++ b/src/shared/git-object-store-failure.ts @@ -0,0 +1,168 @@ +/** + * Classify "Git cannot read an object it needs" failures and turn them into a + * message a user can act on. + * + * Why this exists: `git worktree add` resolves the branch tip from the ref + * database but reads the *tree* from the object database, so a repo whose commit + * object survives while its tree does not passes every commit-only preflight and + * then dies inside the checkout. Orca cannot prevent that (the damage is + * environment-side: filtered clone, interrupted fetch, gc/repack race, disk + * corruption), so the failure path has to name it and say how to repair it. + * + * Lives in shared/ because the classifier must run on the client: the raw text + * can also arrive from an older relay host that never wrapped it. + */ + +export type GitObjectStoreFailureKind = + | 'unreadable-tree' + | 'unparsable-commit' + | 'missing-object' + | 'corrupt-object' + +export type GitObjectStoreFailure = { + kind: GitObjectStoreFailureKind + oid: string | null +} + +/** Stable phrase both the host message and the client formatter key off. */ +export const GIT_OBJECT_STORE_FAILURE_ANCHOR = 'repository object database is missing objects' + +const OID = '[0-9a-f]{7,64}' + +// Why ordered: an unreadable tree is the specific shape `worktree add` dies on, and +// its wording ("unable to read tree") would otherwise be swallowed by the generic +// missing-object patterns below. +const FAILURE_PATTERNS: readonly { kind: GitObjectStoreFailureKind; pattern: RegExp }[] = [ + // git's tree-walk.c die() prints the oid it was asked to dereference; the parenthesised + // form is what newer Git emits, the bare form is what 2.44 and friends emit. + { kind: 'unreadable-tree', pattern: new RegExp(`unable to read tree\\s*\\(?(${OID})\\)?`, 'i') }, + { kind: 'unreadable-tree', pattern: /unable to read tree/i }, + // The sparse create path dies in the follow-up `git checkout`, not in `worktree add + // --no-checkout`, and checkout words the same missing root tree as a commit it cannot + // parse (verbatim on git 2.44.0). The commit object itself is usually intact. + { + kind: 'unparsable-commit', + pattern: new RegExp(`unable to parse commit(?:\\s+(${OID}))?`, 'i') + }, + { kind: 'corrupt-object', pattern: new RegExp(`object file .*? is empty`, 'i') }, + { kind: 'corrupt-object', pattern: new RegExp(`loose object (${OID}).*? is corrupt`, 'i') }, + { kind: 'corrupt-object', pattern: /unable to unpack .*? header/i }, + { + kind: 'missing-object', + pattern: new RegExp(`missing (?:blob|tree|commit|tag) object '?(${OID})'?`, 'i') + }, + { kind: 'missing-object', pattern: new RegExp(`unable to read (${OID})`, 'i') }, + { kind: 'missing-object', pattern: new RegExp(`could not read object (${OID})`, 'i') }, + { kind: 'missing-object', pattern: new RegExp(`object not found:? (${OID})`, 'i') }, + { kind: 'missing-object', pattern: /did not send all necessary objects/i }, + { kind: 'missing-object', pattern: /promisor-remote: unable to fetch/i } +] + +export function classifyGitObjectStoreFailure(text: string): GitObjectStoreFailure | null { + if (!text) { + return null + } + for (const { kind, pattern } of FAILURE_PATTERNS) { + const match = text.match(pattern) + if (match) { + return { kind, oid: match[1] ?? null } + } + } + return null +} + +export type GitObjectPresence = 'present' | 'missing' | 'unverifiable' +export type PartialCloneVerdict = 'yes' | 'no' | 'unverifiable' + +export type GitObjectStoreFailureReport = { + failure: GitObjectStoreFailure + /** Branch or ref the checkout was for; the user's own name, not a filesystem path. */ + branch: string + commit: GitObjectPresence + rootTree: GitObjectPresence + partialClone: PartialCloneVerdict +} + +// Why rebuilt rather than echoed: git's stderr reaches us glued to `Command failed: git +// worktree add `, and re-emitting any of it is how the user's +// home directory and full argv ended up in bug reports. Only kind + oid survive. +function describeGitReport(failure: GitObjectStoreFailure): string { + if (failure.kind === 'corrupt-object') { + return failure.oid + ? `a corrupt object file for ${failure.oid}` + : 'a corrupt or truncated object file' + } + if (failure.kind === 'unreadable-tree') { + return failure.oid ? `unable to read tree (${failure.oid})` : 'unable to read tree' + } + if (failure.kind === 'unparsable-commit') { + return failure.oid ? `unable to parse commit ${failure.oid}` : 'unable to parse a commit' + } + return failure.oid ? `missing object ${failure.oid}` : 'a missing object' +} + +export function formatGitObjectStoreFailureMessage(report: GitObjectStoreFailureReport): string { + const sentences = [ + `Orca could not create this workspace because the ${GIT_OBJECT_STORE_FAILURE_ANCHOR}.`, + `Git reported: ${describeGitReport(report.failure)}.` + ] + + // Both halves need their own evidence: a failed `^{tree}` peel is equally what an + // unreadable COMMIT produces, so without a commit sighting neither claim is observed. + if (report.commit === 'present' && report.rootTree === 'missing') { + sentences.push( + `The commit for "${report.branch}" is present but its root tree object is missing, so Git could not check any files out.` + ) + } else { + sentences.push(`Git could not read every object the checkout of "${report.branch}" needs.`) + } + + if (report.partialClone === 'yes') { + sentences.push( + 'This repository is a partial clone, so those objects were meant to be downloaded lazily from its promisor remote; run git fetch (git fetch --refetch on Git 2.36 or newer) to refill them.' + ) + } + + sentences.push(GIT_OBJECT_STORE_REPAIR_GUIDANCE) + + return sentences.join(' ') +} + +/** + * Trailing repair sentence, kept in the host-composed message so logs stay self-contained + * and split back out by the client so the toast can show the short title instead. + */ +// No blanket "Orca did not do this": Orca runs `git worktree prune` on several removal paths +// and leaves Git's auto-maintenance (gc.auto) enabled on ordinary fetches, so it cannot promise +// nothing of its doing touched the object store. Keep this purely actionable. +export const GIT_OBJECT_STORE_REPAIR_GUIDANCE = + 'Run git fsck in the repository to confirm what is missing, then re-fetch from the remote or re-clone to restore it.' + +export function splitGitObjectStoreRepairGuidance(message: string): { + summary: string + repair: string +} { + const index = message.indexOf(GIT_OBJECT_STORE_REPAIR_GUIDANCE) + return index === -1 + ? { summary: message, repair: GIT_OBJECT_STORE_REPAIR_GUIDANCE } + : // Why slice to the end, not the constant: callers append after it (the sparse + // rollback's "cleanup also failed" note), and dropping that loses the only place + // the user is told a half-created worktree is still on disk. + { summary: message.slice(0, index).trimEnd(), repair: message.slice(index) } +} + +export function isGitObjectStoreFailureMessage(message: string): boolean { + return message.toLowerCase().includes(GIT_OBJECT_STORE_FAILURE_ANCHOR) +} + +const DIAGNOSIS_OPENING = 'Orca could not create this workspace because the' + +// Electron's ipcRenderer.invoke rebuilds the rejection as a string instead of preserving the +// Error, so a host-composed diagnosis reaches the renderer behind transport text. +const TRANSPORT_ENVELOPE = /^Error invoking remote method '[^']*':\s*(?:\w*Error:\s*)?/ + +/** Drop the transport wrapper so a host-composed diagnosis is rendered as the host wrote it. */ +export function unwrapGitObjectStoreFailureMessage(message: string): string { + const opening = message.indexOf(DIAGNOSIS_OPENING) + return opening > 0 ? message.slice(opening) : message.replace(TRANSPORT_ENVELOPE, '') +}