mirror of
https://github.com/stablyai/orca.git
synced 2026-10-24 00:04:01 +00:00
Neil
311325b9ea
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/<name>/dev/worktrees/<branch> '<branch>' 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 "<branch>" 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".
66 lines
2.3 KiB
TypeScript
66 lines
2.3 KiB
TypeScript
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<Error | null> {
|
|
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
|
|
}
|