fix(isolation-core): bound stalled git commands

packages/isolation-core/src/git/baseline.test.ts

@@ -2,10 +2,16 @@ import { expect, test } from "bun:test"
 import { cp, mkdir, readFile, rm, writeFile } from "node:fs/promises"
 import { join } from "node:path"
 import { repo, git } from "../backends/git-fixture"
-import { captureBaseline, IsolationBaselineTooLargeError } from "./baseline"
+import {
+  captureBaseline,
+  captureRepoBaseline,
+  ISOLATION_BASELINE_MAX_CONTENT_BYTES,
+  IsolationBaselineTooLargeError,
+  type BaselineReadRetryDetails,
+} from "./baseline"
 import { captureDeltaPatch } from "./delta"
 import { parseDiffGitLinePaths } from "./synthetic-tree"
-import { GitCommandError, runGit } from "./command"
+import { GitCommandError, GitCommandTimeoutError, runGit } from "./command"
 
 async function setup() {
   const f = await repo()
@@ -101,6 +107,48 @@ test("nested repository delta is separate and node_modules is excluded", async (
   expect(result.nestedPatches.map(n => n.relativePath)).toEqual(["libs/inner"])
   expect(paths(result.nestedPatches[0]!.patch)).toEqual(["new"])
 })
+test("a timed-out baseline read retries once with a fresh process and completes", async () => {
+  // given
+  const { repoRoot } = await setup()
+  await writeFile(join(repoRoot, "untracked"), "new\n")
+  const indexBefore = await readFile(join(repoRoot, ".git/index"))
+  let untrackedReadAttempts = 0
+  let successfulArgs: readonly string[] = []
+  let successfulOptionalLocks: string | undefined
+  const retries: BaselineReadRetryDetails[] = []
+  const executeGit: typeof runGit = async (args, options) => {
+    if (args.includes("ls-files")) {
+      untrackedReadAttempts++
+      // The deadline and tree teardown of a real stalled process are covered in command.test.ts; a real
+      // stand-in here leaves a Windows grandchild holding the fixture directory past teardown.
+      if (untrackedReadAttempts === 1) throw new GitCommandTimeoutError(args, options.cwd, 50)
+      successfulArgs = args
+      successfulOptionalLocks = options.env?.["GIT_OPTIONAL_LOCKS"]
+    }
+    return runGit(args, options)
+  }
+
+  // when
+  const baseline = await captureRepoBaseline(
+    repoRoot,
+    ISOLATION_BASELINE_MAX_CONTENT_BYTES,
+    { runGit: executeGit, onReadRetry: details => retries.push(details) },
+  )
+
+  // then
+  expect(baseline.untrackedFiles).toEqual(["untracked"])
+  expect(untrackedReadAttempts).toBe(2)
+  expect(successfulArgs.slice(0, 6)).toEqual([
+    "-c", "core.fsmonitor=false", "-c", "core.untrackedCache=false", "ls-files", "--others",
+  ])
+  expect(successfulOptionalLocks).toBe("0")
+  expect(retries).toEqual([{
+    args: ["ls-files", "--others", "--exclude-standard", "-z"],
+    cwd: repoRoot,
+    timeoutMs: 50,
+  }])
+  expect(await readFile(join(repoRoot, ".git/index"))).toEqual(indexBefore)
+})
 test("untracked content over injected 1 KiB cap is refused before rendering", async () => {
   const { repoRoot } = await setup()
   await writeFile(join(repoRoot, "large"), Buffer.alloc(1025))

packages/isolation-core/src/git/baseline.ts

@@ -1,8 +1,11 @@
 import { lstat, readdir } from "node:fs/promises"
 import { join, relative, sep } from "node:path"
-import { exists, runGit } from "./command"
+import { exists, GitCommandTimeoutError, runGit, type GitOptions } from "./command"
 
 export const ISOLATION_BASELINE_MAX_CONTENT_BYTES = 1024 * 1024 * 1024
+const BASELINE_GIT_TIMEOUT_MS = 10_000
+const BASELINE_READ_CONFIG = ["-c", "core.fsmonitor=false", "-c", "core.untrackedCache=false"] as const
+const BASELINE_READ_ENV = { GIT_OPTIONAL_LOCKS: "0" } as const
 export class IsolationBaselineTooLargeError extends Error {
   constructor(readonly repoRoot: string, readonly contentBytes: number | undefined, readonly budgetBytes = ISOLATION_BASELINE_MAX_CONTENT_BYTES) {
     super(`Working tree at ${repoRoot} exceeds the ${budgetBytes}-byte isolation snapshot budget. Commit or gitignore bulk content before isolation.`)
@@ -21,9 +24,44 @@ export interface WorktreeBaseline {
   root: RepoBaseline
   nested: { relativePath: string; baseline: RepoBaseline }[]
 }
+export interface BaselineReadRetryDetails {
+  readonly args: readonly string[]
+  readonly cwd: string
+  readonly timeoutMs: number
+}
+export interface BaselineCaptureOptions {
+  readonly onReadRetry?: (details: BaselineReadRetryDetails) => void
+  readonly runGit?: typeof runGit
+}
+
+const logBaselineReadRetry = (details: BaselineReadRetryDetails): void => {
+  console.warn("[isolation-core] retrying timed-out read-only Git command", details)
+}
+async function runBaselineRead(
+  args: string[],
+  gitOptions: GitOptions,
+  captureOptions: BaselineCaptureOptions = {},
+): ReturnType<typeof runGit> {
+  const executeGit = captureOptions.runGit ?? runGit
+  const hardenedArgs = [...BASELINE_READ_CONFIG, ...args]
+  const hardenedOptions = {
+    ...gitOptions,
+    env: { ...gitOptions.env, ...BASELINE_READ_ENV },
+    timeoutMs: gitOptions.timeoutMs ?? BASELINE_GIT_TIMEOUT_MS,
+  }
+  const attempt = () => executeGit(hardenedArgs, hardenedOptions)
+  try {
+    return await attempt()
+  } catch (error) {
+    if (!(error instanceof GitCommandTimeoutError)) throw error
+    const onReadRetry = captureOptions.onReadRetry ?? logBaselineReadRetry
+    onReadRetry({ args, cwd: gitOptions.cwd, timeoutMs: error.timeoutMs })
+    return attempt()
+  }
+}
 
 export async function discoverNestedRepos(repoRoot: string): Promise<string[]> {
-  const status = (await runGit(["submodule", "status"], { cwd: repoRoot })).stdout.toString()
+  const status = (await runBaselineRead(["submodule", "status"], { cwd: repoRoot })).stdout.toString()
   const submodules = new Set(status.split("\n").filter(Boolean).map(line => line.slice(42).replace(/ \([^\n]*\)$/, "")))
   const result: string[] = []
   const walk = async (dir: string): Promise<void> => {
@@ -40,17 +78,25 @@ export async function discoverNestedRepos(repoRoot: string): Promise<string[]> {
   return result.sort()
 }
 
-export async function captureRepoBaseline(repoRoot: string, budgetBytes = ISOLATION_BASELINE_MAX_CONTENT_BYTES): Promise<RepoBaseline> {
+export async function captureRepoBaseline(
+  repoRoot: string,
+  budgetBytes = ISOLATION_BASELINE_MAX_CONTENT_BYTES,
+  options: BaselineCaptureOptions = {},
+): Promise<RepoBaseline> {
   let remaining = budgetBytes
   const capture = async (args: string[], allowedExitCodes?: number[]): Promise<string> => {
-    const { stdout } = await runGit(args, {
+    const { stdout } = await runBaselineRead(args, {
       cwd: repoRoot, allowedExitCodes, maxOutputBytes: remaining,
       outputLimitError: () => new IsolationBaselineTooLargeError(repoRoot, undefined, budgetBytes),
-    })
+    }, options)
     remaining -= stdout.byteLength
     return stdout.toString()
   }
-  const head = await runGit(["rev-parse", "--verify", "--quiet", "HEAD"], { cwd: repoRoot, allowedExitCodes: [0, 1] })
+  const head = await runBaselineRead(
+    ["rev-parse", "--verify", "--quiet", "HEAD"],
+    { cwd: repoRoot, allowedExitCodes: [0, 1] },
+    options,
+  )
   const headCommit = head.stdout.toString().trim()
   const diffArgs = ["diff", "--binary", "--no-ext-diff", "--no-textconv", "--ignore-submodules=all"]
   const staged = await capture([...diffArgs, "--cached"])

packages/isolation-core/src/git/command.test.ts

@@ -3,7 +3,7 @@ import type { ChildProcess } from "node:child_process"
 import { rm } from "node:fs/promises"
 import { join } from "node:path"
 import { fixture } from "../test-fixture"
-import { GitCommandError, runGit } from "./command"
+import { GitCommandError, GitCommandTimeoutError, runGit } from "./command"
 import { IsolationUnavailableError } from "../backend"
 
 // Signal git by the pid runGit spawned instead of guessing it from shell
@@ -76,6 +76,22 @@ test("input written to a child that dies while alias-shell survivors hold its pi
   expect(Date.now() - started).toBeLessThan(5_000)
 })
 
+test("a git process that never exits is terminated at its command deadline", async () => {
+  // given
+  const f = await fixture()
+  let failure: unknown
+  const started = Date.now()
+
+  // when
+  failure = await runGit(["-c", "alias.wait=!sleep 7", "wait"], { cwd: f.repoRoot, timeoutMs: 50 })
+    .then(() => undefined, (error: unknown) => error)
+
+  // then
+  expect(failure).toBeInstanceOf(GitCommandTimeoutError)
+  expect(Date.now() - started).toBeLessThan(5_000)
+  if (process.platform === "win32") expect(await fixtureRootIsRemovable(f.root)).toBe(true)
+})
+
 test("a budget breach on a still-streaming child preserves the typed limit error", async () => {
   const f = await fixture()
   class BudgetError extends Error {}

packages/isolation-core/src/git/command.ts

@@ -3,6 +3,8 @@ import type { Readable } from "node:stream"
 import { lstat } from "node:fs/promises"
 import { IsolationUnavailableError } from "../backend"
 
+const DEFAULT_GIT_TIMEOUT_MS = 120_000
+
 export async function exists(path: string): Promise<boolean> {
   try { await lstat(path); return true } catch (error) {
     if (error instanceof Error && "code" in error && error.code === "ENOENT") return false
@@ -16,10 +18,18 @@ export class GitCommandError extends Error {
     this.name = "GitCommandError"
   }
 }
+export class GitCommandTimeoutError extends GitCommandError {
+  constructor(args: readonly string[], cwd: string, readonly timeoutMs: number) {
+    super(args, cwd, 124, `git timed out after ${timeoutMs}ms`)
+    this.name = "GitCommandTimeoutError"
+  }
+}
 export interface GitOptions {
   cwd: string
   env?: Record<string, string | undefined>
   signal?: AbortSignal
+  /** Maximum command lifetime before the full Git process tree is terminated. */
+  timeoutMs?: number
   input?: string | Buffer
   allowedExitCodes?: readonly number[]
   maxOutputBytes?: number
@@ -38,16 +48,39 @@ export interface GitOptions {
 // leaves alias shells behind), and they are exactly what must die. POSIX group
 // kill still reaches them; the win32 taskkill shot only lands while the
 // leader lives, so the drain grace in runGit covers the rest.
-function killTree(child: ChildProcess): void {
+async function killTree(child: ChildProcess): Promise<void> {
   if (child.pid === undefined) return
+  const killDirect = () => {
+    try { child.kill("SIGKILL") } catch (error) {
+      if (error instanceof Error) return
+      throw error
+    }
+  }
   if (process.platform !== "win32") {
     // POSIX: the child leads its own process group; a group that already died
     // leaves nothing worth killing, so the ESRCH fall-through is a plain kill.
-    try { process.kill(-child.pid, "SIGKILL"); return } catch { try { child.kill("SIGKILL") } catch { /* already exited */ } }
+    try { process.kill(-child.pid, "SIGKILL"); return } catch (error) {
+      if (!(error instanceof Error)) throw error
+      killDirect()
+    }
     return
   }
-  const killer = spawn("taskkill", ["/pid", String(child.pid), "/T", "/F"], { stdio: "ignore", windowsHide: true })
-  killer.once("error", () => { try { child.kill("SIGKILL") } catch { /* already exited */ } })
+  await new Promise<void>((resolve) => {
+    let settled = false
+    const finish = (needsFallback: boolean) => {
+      if (settled) return
+      settled = true
+      if (needsFallback) killDirect()
+      resolve()
+    }
+    try {
+      const killer = spawn("taskkill", ["/pid", String(child.pid), "/T", "/F"], { stdio: "ignore", windowsHide: true })
+      killer.once("error", () => finish(true))
+      killer.once("close", (code) => finish(code !== 0))
+    } catch {
+      finish(true)
+    }
+  })
 }
 
 /** Drain both pipes concurrently; reject before retaining output beyond the budget. */
@@ -63,14 +96,15 @@ export async function runGit(args: string[], options: GitOptions): Promise<{ cod
     })
     options.onSpawn?.(child)
     if (options.input !== undefined) {
-      child.stdin!.end(typeof options.input === "string" ? options.input : new Uint8Array(options.input))
+      if (child.stdin === null) throw new TypeError("spawned Git process is missing its piped stdin")
+      child.stdin.end(typeof options.input === "string" ? options.input : new Uint8Array(options.input))
     }
   } catch (error) {
     if (error instanceof Error && "code" in error && error.code === "ENOENT") throw new IsolationUnavailableError("git not on PATH")
     throw error
   }
   // A child that dies mid-write must surface as a failure, not an EPIPE crash.
-  child.stdin?.on("error", () => killTree(child))
+  child.stdin?.on("error", () => { void killTree(child) })
   // Drain both pipes concurrently; reject before retaining output beyond the budget.
   let retained = 0
   const collect = (stream: Readable): Promise<Buffer> => new Promise((resolve, reject) => {
@@ -79,7 +113,7 @@ export async function runGit(args: string[], options: GitOptions): Promise<{ cod
       retained += chunk.byteLength
       if (retained > (options.maxOutputBytes ?? Infinity)) {
         reject(options.outputLimitError?.() ?? new Error("Git output exceeds budget"))
-        killTree(child)
+        void killTree(child)
         // A grandchild may hold the pipe open past the kill; stop waiting on "end".
         child.stdout?.destroy()
         child.stderr?.destroy()
@@ -97,6 +131,9 @@ export async function runGit(args: string[], options: GitOptions): Promise<{ cod
   // failing; a normal drain closes them in milliseconds, so this only fires
   // when survivors hold the handles.
   const PIPE_DRAIN_GRACE_MS = 1_000
+  const timeoutMs = options.timeoutMs ?? DEFAULT_GIT_TIMEOUT_MS
+  let timeoutError: GitCommandTimeoutError | undefined
+  let timeout: ReturnType<typeof setTimeout> | undefined
   const exited = new Promise<number>((resolve, reject) => {
     // Node reports a missing executable through the async "error" event, so the
     // spawn try/catch above cannot see it; classify it here.
@@ -115,7 +152,7 @@ export async function runGit(args: string[], options: GitOptions): Promise<{ cod
     let drainGrace: ReturnType<typeof setTimeout> | undefined
     child.once("exit", (code, signal) => {
       if (signal === null && (options.allowedExitCodes ?? [0]).includes(code ?? 0)) return
-      killTree(child)
+      void killTree(child)
       drainGrace = setTimeout(() => {
         child.stdin?.destroy()
         child.stdout?.destroy()
@@ -124,26 +161,50 @@ export async function runGit(args: string[], options: GitOptions): Promise<{ cod
     })
     child.once("close", (code, signal) => {
       clearTimeout(drainGrace)
+      if (timeoutError !== undefined) return reject(timeoutError)
       // A signal death leaves exitCode null; "null ?? 0" would report success
       // for a killed git and its partial output.
       if (signal !== null) return reject(new GitCommandError(args, options.cwd, 128, `git terminated by signal ${signal}`))
       resolve(code ?? 0)
     })
   })
+  const deadline = new Promise<never>((_resolve, reject) => {
+    timeout = setTimeout(() => {
+      const error = new GitCommandTimeoutError(args, options.cwd, timeoutMs)
+      timeoutError = error
+      void killTree(child).finally(() => {
+        child.stdin?.destroy()
+        child.stdout?.destroy()
+        child.stderr?.destroy()
+        reject(error)
+      })
+    }, timeoutMs)
+    timeout.unref()
+  })
+  const stdoutStream = child.stdout
+  const stderrStream = child.stderr
+  if (stdoutStream === null || stderrStream === null) throw new TypeError("spawned Git process is missing piped output")
   try {
-    const [stdout, stderr, code] = await Promise.all([collect(child.stdout!), collect(child.stderr!), exited])
+    const [stdout, stderr, code] = await Promise.all([
+      collect(stdoutStream),
+      collect(stderrStream),
+      Promise.race([exited, deadline]),
+    ])
     options.signal?.throwIfAborted()
     if (!(options.allowedExitCodes ?? [0]).includes(code)) throw new GitCommandError(args, options.cwd, code, stderr.toString())
     return { code, stdout, stderr: stderr.toString() }
   } catch (error) {
-    killTree(child)
+    if (!(error instanceof GitCommandTimeoutError)) await killTree(child)
     child.stdout?.destroy()
     child.stderr?.destroy()
     // The teardown kill itself makes `exited` reject with a signal death; that
     // rejection must not displace the caller's error (the typed budget error,
     // for one) on its way out.
-    await exited.catch(() => {})
+    if (error instanceof GitCommandTimeoutError) void exited.catch(() => {})
+    else await exited.catch(() => {})
     throw error
+  } finally {
+    clearTimeout(timeout)
   }
 }
 

packages/isolation-core/src/index.ts

@@ -15,5 +15,5 @@ export * from "./git/detach-git-dir"
 export * from "./git/baseline"
 export * from "./git/delta"
 export * from "./git/synthetic-tree"
-export { runGit, GitCommandError } from "./git/command"
+export { runGit, GitCommandError, GitCommandTimeoutError } from "./git/command"
 export * from "./merge"