fix(host-service): classify simple-git's missing-directory construct error as a typed NOT_FOUND

Merged
SA
3mo ago
+72-0

Problem

git.getStatus reports ~23 unhandled 500s/day (174 lifetime, 159 in 7d, still arriving on 1.21.0) for Cannot use simple-git on a directory that does not exist.

User-visible harm: none, and I want to be straight about that. Nothing on this path branches on the error code or cause — the v2 query client sets retry: false, and the host-service error formatter does not forward cause to the wire, so the renderer renders identically before and after this change. The harm is to us, and it is twofold:

  1. sentryMiddleware documents the invariant "anything still INTERNAL_SERVER_ERROR at this boundary is a bug." For the app's highest-traffic git procedure that is currently false, and real git.getStatus bugs land under ~23 noise events/day.
  2. resolveWorktreePath (two lines earlier in the same procedure) and the classifier's existing CWD_GONE branch both already report this exact physical condition as NOT_FOUND / WORKTREE_MISSING. Only the construct-time spelling of it escapes untyped.

I considered archiving instead. It's dominated: the issue has already been archived-and-regressed once (substatus regressed), and archiving buries the group that future genuine 500s on this procedure would land in. A pre-construct existsSync re-check is not an option either — it's the same TOCTOU window, just narrower. Classification is the only correct instrument for a race. No in-flight PR restructures this path (nearest neighbours #6260 and #6446 are prior extensions of this same classifier).

Root cause

resolveWorktreePath checks existsSync and hands the path onward. createUserSimpleGit(worktreePath) reaches simple-git's gitInstanceFactory, which re-checks folderExists(config.baseDir) and throws GitConstructError before any git process is spawned. Between those two steps the directory can be removed — the user deleted the workspace, an external tool pruned it, a cleanup ran concurrently. In the reported events the worker's status run had already succeeded and returned a baseRefFetchTarget, so the deletion lands during the coordinator's client construction.

Because that error is simple-git's own text and not one of the git-CLI conditions the classifier knows, it falls through rethrowEnvironmentalGitError as a 500.

Fix

One branch in classify-git-error.ts, mapping simple-git's construct message to the same NOT_FOUND / WORKTREE_MISSING the sibling guard already emits. Message preserved verbatim, as every other branch in this file does.

On the matcher — the error name is not usable here, and I verified this rather than assuming it. simple-git's GitError base class never assigns this.name, so a GitConstructError arrives named "Error". Production confirms it: the Sentry issue title reads Error: Cannot use simple-git..., not GitConstructError: .... A error.name === "GitConstructError" check would have compiled, read as the more structural choice, and been dead code that never fired. serializeWorkerError independently rules out anything richer: crossing the worker boundary it keeps only name/message/stack/code, dropping the config.baseDir own-property that GitConstructError otherwise carries. The message is the only signal that survives every path, and it is safe to key on — it is a hard-coded template literal in simple-git, never localised and never interpolated with user content, unlike git's own output.

Keying on the sentence is also the narrower choice, which is what the type-vs-condition distinction actually asks for. GitConstructError is the type simple-git would also use for construction mistakes that are ours; those must keep reporting as 500s, and they do, because they carry a different sentence.

Still reporting as 500s on this path — everything except the four environmental conditions now enumerated in the classifier. Concretely: any GitConstructError whose message is not the missing-directory sentence (i.e. our own misconfiguration of the client); index.lock contention; dubious-ownership rejections; bad revisions and unknown refs; cannot chdir to work tree; worker-pool timeouts and crashes; and every other git failure. The added negative tests pin the first of those.

cause: { kind: "WORKTREE_MISSING" } is reused rather than invented. To be explicit: nothing currently reads it on this path — the host-service errorFormatter only forwards teardownFailure, projectNotSetup and deleteInProgress, so cause never reaches the renderer here. It is included solely so this throw site matches the two sibling throw sites for the identical condition (resolve-worktree.ts:27 and the CWD_GONE branch above it); emitting a bare error would make this the only one of the three that drops the tag.

Adjacent throw sites, left alone

gitStatusSnapshotTask, gitFetchBaseRefTask, gitCommitFilesTask and the other worker tasks in workers/tasks/git.ts each call createUserSimpleGit(worktreePath) and can raise the same construct error. Their callers route through this same classifier where they classify at all; the ones that don't are outside this root cause and are not touched here.

Verification

bunx biome check on both changed files:

Checked 2 files in 14ms. No fixes applied.

cd packages/host-service && bun run typecheck:

$ tsc --noEmit --emitDeclarationOnly false

Negative tests were written first and run before the change — the two over-match guards and the misconfiguration guard passed, the positive case failed, confirming the branch was absent rather than accidentally already covered:

57 | 	test("simple-git construct error → NOT_FOUND / WORKTREE_MISSING", () => {
58 | 		const message = "Cannot use simple-git on a directory that does not exist";
59 | 		const thrown = capture(new Error(message));
60 | 		expect(thrown?.code).toBe("NOT_FOUND");
                            ^
error: expect(received).toBe(expected)

Expected: "NOT_FOUND"
Received: undefined

(fail) rethrowEnvironmentalGitError > simple-git construct error → NOT_FOUND / WORKTREE_MISSING [0.11ms]

 8 pass
 1 fail
 24 expect() calls
Ran 9 tests across 1 file. [89.00ms]

After the change, bun test packages/host-service/src/trpc/router/git:

bun test v1.3.14 (0d9b296a)

 90 pass
 0 fail
 196 expect() calls
Ran 90 tests across 8 files. [24.58s]

One of the added tests asserts the name observation directly against the real dependency, so a future simple-git release that starts setting this.name fails loudly here instead of silently changing what this branch matches.

Repo-wide bun run lint was not run — it hangs on cross-worktree bun locks.

Refs HOST-SERVICE-W

https://claude.ai/code/session_1d4c6d97-bdd8-4c49-b4b6-9182b085df16

Summary by cubic

Classifies simple-git’s “Cannot use simple-git on a directory that does not exist” construct error as NOT_FOUND/WORKTREE_MISSING in the host-service git classifier. Previously, git.getStatus returned INTERNAL_SERVER_ERROR for this race when the worktree was deleted between checks; user-visible behavior is unchanged, but Sentry noise drops and classification now matches sibling guards.

  • Adds a matcher in packages/host-service/src/trpc/router/git/utils/classify-git-error.ts keyed to the exact simple-git sentence; throws NOT_FOUND with cause: { kind: "WORKTREE_MISSING" }. Other construct errors and all non-environmental failures remain 500s.
  • Tests verify:
    • Correct mapping and verbatim message preservation.
    • No over-match on other “does not exist” messages (refs/paths/remotes).
    • Misconfiguration construct errors still surface as 500s.
    • GitConstructError arrives named “Error”, so name-based matching would be dead code.
  • No rollout or migrations. Addresses HOST-SERVICE-W by reclassifying this expected environmental race without changing renderer output. Adjacent throw sites are unchanged and continue to route through the same classifier where applicable.

Summary by CodeRabbit

  • Bug Fixes
    • Improved handling of missing Git working directories.
    • Users now receive a clear “not found” error when the configured base directory does not exist.
    • Unrelated Git configuration errors continue to be reported normally.