fix(git): degrade gracefully when git binary is missing

The read-only git helpers spawned `git` directly and only inspected the
result exit code. When git is absent from PATH, Bun's spawn throws ENOENT
(uv_spawn 'git') at launch, which escaped as an unhandled rejection and
crashed the process (e.g. Windows without git, relying on WSL git).

Translate a missing-binary launch failure into a non-zero GitCommandResult
in the async git() wrapper and into a null degrade in the sync reftable ref
readers, matching the existing non-zero-exit handling. Mutating/checked
commands keep surfacing the clean "git is not installed." error.

Fixes #6169
This commit is contained in:
roboomp
2026-07-21 22:07:42 +00:00
parent 89d6a8f6d1
commit d0f1cd1b77
4 changed files with 126 additions and 50 deletions
@@ -0,0 +1,49 @@
import { afterEach, describe, expect, it, vi } from "bun:test";
import * as git from "@oh-my-pi/pi-coding-agent/utils/git";
// Regression coverage for #6169: when `git` is not on PATH, `Bun.spawn`/
// `Bun.spawnSync` throw synchronously with `code: "ENOENT"` (`uv_spawn 'git'`)
// rather than resolving to a non-zero exit. The read-only git helpers only
// inspect the result's exit code, so an unguarded spawn escaped as an unhandled
// rejection and crashed the process. The helpers must now degrade to their
// "no result" value instead.
function throwSpawnEnoent(): never {
const err = new Error('Executable not found in $PATH: "git"');
(err as NodeJS.ErrnoException).code = "ENOENT";
throw err;
}
afterEach(() => {
vi.restoreAllMocks();
});
describe("git helpers with git binary absent (#6169)", () => {
it("status.summary degrades to null instead of throwing ENOENT", async () => {
vi.spyOn(Bun, "spawn").mockImplementation(throwSpawnEnoent);
// A path that is not inside a git repo forces the subprocess fallback.
expect(await git.status.summary("/")).toBeNull();
});
it("diff.has surfaces a clean GitCommandError instead of a raw ENOENT rejection", async () => {
vi.spyOn(Bun, "spawn").mockImplementation(throwSpawnEnoent);
// `has` must tell exit 0 (no diff) from exit 1 (diff), so a missing binary
// (exit 127) cannot collapse to a boolean — but it surfaces as the tidy
// "git is not installed." error rather than the raw uv_spawn ENOENT crash.
await expect(git.diff.has("/")).rejects.toThrow("git is not installed.");
});
it("repo.root degrades to null instead of throwing ENOENT", async () => {
vi.spyOn(Bun, "spawn").mockImplementation(throwSpawnEnoent);
expect(await git.repo.root("/")).toBeNull();
});
it("re-raises non-ENOENT spawn failures", async () => {
vi.spyOn(Bun, "spawn").mockImplementation(() => {
const err = new Error("EACCES: permission denied");
(err as NodeJS.ErrnoException).code = "EACCES";
throw err;
});
await expect(git.status.summary("/")).rejects.toThrow("EACCES");
});
});