From d0f1cd1b77828b430f792d12bbf19dc04ced79f9 Mon Sep 17 00:00:00 2001 From: roboomp Date: Tue, 21 Jul 2026 22:07:42 +0000 Subject: [PATCH] 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 --- packages/coding-agent/CHANGELOG.md | 4 + packages/coding-agent/src/utils/git.ts | 101 +++++++++--------- .../test/git-missing-binary.test.ts | 49 +++++++++ .../coding-agent/test/git-reftable.test.ts | 22 +++- 4 files changed, 126 insertions(+), 50 deletions(-) create mode 100644 packages/coding-agent/test/git-missing-binary.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 75904e901..2446cb3f5 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed `omp` crashing with `[Unhandled Rejection] Error: ENOENT ... uv_spawn 'git'` when `git` is not installed/on `PATH` (e.g. Windows relying on WSL git). The read-only git helpers in `utils/git.ts` spawned `git` directly and only inspected the exit code, so `Bun.spawn`/`Bun.spawnSync` throwing `ENOENT` at launch escaped as an unhandled rejection; missing-binary launches now degrade to a non-zero result (async) or `null` (sync ref readers) instead of crashing, while mutating/checked commands keep surfacing the clean "git is not installed." error ([#6169](https://github.com/can1357/oh-my-pi/issues/6169)). + ## [17.0.6] - 2026-07-20 - Fixed failed plan-mode exits leaving the session on the restored execution model while plan mode remained active and silently changing ambient `xd://` tool presentation; rollback now restores the plan model, thinking level, and exact top-level-versus-mounted tool partition so exit can be retried safely ([#6013](https://github.com/can1357/oh-my-pi/pull/6013)). diff --git a/packages/coding-agent/src/utils/git.ts b/packages/coding-agent/src/utils/git.ts index 16b74f5e8..22fc38183 100644 --- a/packages/coding-agent/src/utils/git.ts +++ b/packages/coding-agent/src/utils/git.ts @@ -217,6 +217,12 @@ export const GIT_NETWORK_TIMEOUT_MS = 30 * 60 * 1000; export const GIT_COMMAND_OUTPUT_LIMIT_BYTES = 8 * 1024 * 1024; const GIT_COMMAND_TIMEOUT_EXIT_CODE = 124; +// Exit code returned when the `git` binary cannot be launched at all (spawn +// ENOENT). Mirrors the POSIX "command not found" code so read-only callers that +// degrade on any non-zero exit treat a missing git the same as a failed +// invocation instead of letting the raw spawn ENOENT escape as an unhandled +// rejection. +const GIT_SPAWN_ENOENT_EXIT_CODE = 127; const GIT_OUTPUT_TRUNCATED_MARKER = "\n[git subprocess output truncated after 8 MiB]\n"; const GIT_COMMAND_TERMINATE_GRACE_MS = 5_000; @@ -388,6 +394,29 @@ function ensureAvailable(): void { } } +/** + * Launch a `git` plumbing command synchronously and decode stdout. Returns the + * exit code plus trimmed stdout; a missing `git` binary (spawn ENOENT) is + * reported as {@link GIT_SPAWN_ENOENT_EXIT_CODE} so sync read-only callers + * degrade to `null` instead of throwing an uncaught error during rendering. + */ +function gitSpawnSyncText(cwd: string, args: readonly string[]): { exitCode: number; stdout: string } { + const commandArgs = withShortLivedGitConfig(withNoOptionalLocks(args)); + try { + const result = Bun.spawnSync(["git", ...commandArgs], { + cwd, + env: buildGitEnv(), + stdout: "pipe", + stderr: "pipe", + windowsHide: true, + }); + return { exitCode: result.exitCode ?? 0, stdout: new TextDecoder().decode(result.stdout).trim() }; + } catch (err) { + if (isEnoent(err)) return { exitCode: GIT_SPAWN_ENOENT_EXIT_CODE, stdout: "" }; + throw err; + } +} + function formatCommandFailure( args: readonly string[], result: Pick, @@ -401,15 +430,21 @@ function formatCommandFailure( async function git(cwd: string, args: readonly string[], options: CommandOptions = {}): Promise { const commandArgs = withShortLivedGitConfig(options.readOnly ? withNoOptionalLocks(args) : [...args]); - const child = Bun.spawn(["git", ...commandArgs], { - cwd, - env: buildGitEnv(options.env), - signal: options.signal, - stdin: normalizeStdin(options.stdin), - stdout: "pipe", - stderr: "pipe", - windowsHide: true, - }); + let child: Subprocess; + try { + child = Bun.spawn(["git", ...commandArgs], { + cwd, + env: buildGitEnv(options.env), + signal: options.signal, + stdin: normalizeStdin(options.stdin), + stdout: "pipe", + stderr: "pipe", + windowsHide: true, + }); + } catch (err) { + if (isEnoent(err)) return { exitCode: GIT_SPAWN_ENOENT_EXIT_CODE, stdout: "", stderr: "git is not installed." }; + throw err; + } return await collectSubprocessResult("git", commandArgs, child, options); } @@ -890,28 +925,12 @@ async function resolveHeadStateReftable(repository: GitRepository, signal?: Abor } function resolveHeadStateReftableSync(repository: GitRepository): GitHeadState | null { - ensureAvailable(); - const symArgs = withShortLivedGitConfig(withNoOptionalLocks(["symbolic-ref", "HEAD"])); - const symResult = Bun.spawnSync(["git", ...symArgs], { - cwd: repository.repoRoot, - env: buildGitEnv(), - stdout: "pipe", - stderr: "pipe", - windowsHide: true, - }); - - const revArgs = withShortLivedGitConfig(withNoOptionalLocks(["rev-parse", "--verify", "HEAD"])); - const revResult = Bun.spawnSync(["git", ...revArgs], { - cwd: repository.repoRoot, - env: buildGitEnv(), - stdout: "pipe", - stderr: "pipe", - windowsHide: true, - }); - const commit = revResult.exitCode === 0 ? new TextDecoder().decode(revResult.stdout).trim() || null : null; + const symResult = gitSpawnSyncText(repository.repoRoot, ["symbolic-ref", "HEAD"]); + const revResult = gitSpawnSyncText(repository.repoRoot, ["rev-parse", "--verify", "HEAD"]); + const commit = revResult.exitCode === 0 ? revResult.stdout || null : null; if (symResult.exitCode === 0) { - const ref = new TextDecoder().decode(symResult.stdout).trim(); + const ref = symResult.stdout; const branchName = ref.startsWith(LOCAL_BRANCH_PREFIX) ? ref.slice(LOCAL_BRANCH_PREFIX.length) : null; return { ...repository, @@ -933,29 +952,13 @@ function resolveHeadStateReftableSync(repository: GitRepository): GitHeadState | function readRefSync(repository: GitRepository, targetRef: string): string | null { if (isReftableRepoSync(repository)) { - ensureAvailable(); - const symArgs = withShortLivedGitConfig(withNoOptionalLocks(["symbolic-ref", targetRef])); - const symResult = Bun.spawnSync(["git", ...symArgs], { - cwd: repository.repoRoot, - env: buildGitEnv(), - stdout: "pipe", - stderr: "pipe", - windowsHide: true, - }); + const symResult = gitSpawnSyncText(repository.repoRoot, ["symbolic-ref", targetRef]); if (symResult.exitCode === 0) { - const stdoutText = new TextDecoder().decode(symResult.stdout).trim(); - return `${HEAD_REF_PREFIX} ${stdoutText}`; + return `${HEAD_REF_PREFIX} ${symResult.stdout}`; } - const revArgs = withShortLivedGitConfig(withNoOptionalLocks(["rev-parse", "--verify", targetRef])); - const revResult = Bun.spawnSync(["git", ...revArgs], { - cwd: repository.repoRoot, - env: buildGitEnv(), - stdout: "pipe", - stderr: "pipe", - windowsHide: true, - }); + const revResult = gitSpawnSyncText(repository.repoRoot, ["rev-parse", "--verify", targetRef]); if (revResult.exitCode === 0) { - return new TextDecoder().decode(revResult.stdout).trim() || null; + return revResult.stdout || null; } return null; } diff --git a/packages/coding-agent/test/git-missing-binary.test.ts b/packages/coding-agent/test/git-missing-binary.test.ts new file mode 100644 index 000000000..57e2d4c73 --- /dev/null +++ b/packages/coding-agent/test/git-missing-binary.test.ts @@ -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"); + }); +}); diff --git a/packages/coding-agent/test/git-reftable.test.ts b/packages/coding-agent/test/git-reftable.test.ts index 8796c7643..3dea84e27 100644 --- a/packages/coding-agent/test/git-reftable.test.ts +++ b/packages/coding-agent/test/git-reftable.test.ts @@ -1,4 +1,4 @@ -import { afterAll, beforeAll, describe, expect, test } from "bun:test"; +import { afterAll, beforeAll, describe, expect, test, vi } from "bun:test"; import * as fs from "node:fs/promises"; import * as os from "node:os"; import * as path from "node:path"; @@ -103,6 +103,26 @@ describe.skipIf(!supportsReftable)("git reftable support", () => { expect(nonexistentExists).toBe(false); }); + // #6169: the sync reftable ref readers spawn `git symbolic-ref`/`rev-parse`. + // When git is absent from PATH the spawn throws ENOENT synchronously; the + // readers must degrade (HEAD resolves as detached with no commit) instead of + // letting the raw ENOENT escape during a TUI render. + test("head.resolveSync degrades when the git binary is missing", () => { + const err = new Error('Executable not found in $PATH: "git"'); + (err as NodeJS.ErrnoException).code = "ENOENT"; + vi.spyOn(Bun, "spawnSync").mockImplementation(() => { + throw err; + }); + try { + const headStateSync = git.head.resolveSync(sharedRepoDir); + expect(headStateSync).not.toBeNull(); + expect(headStateSync?.kind).toBe("detached"); + expect(headStateSync?.commit).toBeNull(); + } finally { + vi.restoreAllMocks(); + } + }); + test("handles git config trailing comments correctly", async () => { const repository = await git.repo.resolve(configRepoDir); expect(repository).not.toBeNull();