fix(agent): stop blocking CLI startup on an unbounded git status

src/core/git_commit_guard.ts

@@ -11,11 +11,57 @@ export interface GitRunner {
   run(args: string[]): GitRunResult;
 }
 
+/**
+ * Options prefixed to EVERY git invocation this runner makes.
+ *
+ * `--no-optional-locks` is not cosmetic. Without it a plain `git status`
+ * rewrites the user's index as a side effect of being read, so merely starting
+ * the agent mutates the repository and can lose a race with the user's own git
+ * process in the same worktree. Inspection must be inspection.
+ * `core.literalPathspecs` keeps a path that happens to begin with `:` from
+ * being reinterpreted as pathspec magic.
+ */
+export const GIT_GLOBAL_ARGS: readonly string[] = [
+  "--no-optional-locks",
+  "-c",
+  "core.literalPathspecs=true",
+];
+
+/**
+ * The pathspec appended to the two repository-state probes.
+ *
+ * `git status` reports the WHOLE repository, not the directory it runs in.
+ * Unbounded, the probe is O(entire repository) even when the agent's workspace
+ * is one small directory inside it, and `--untracked-files=all` turns that into
+ * an enumeration of every untracked path individually. The guard only ever
+ * stages paths inside the workspace, so bounding the probe to the workspace
+ * subtree removes work that could never have produced a candidate.
+ */
+export const WORKSPACE_PATHSPEC: readonly string[] = ["--", "."];
+
+/** `git status` probe: dirty paths in the workspace subtree, NUL-delimited. */
+export const STATUS_PROBE: readonly string[] = [
+  "status",
+  "--porcelain=v1",
+  "-z",
+  "--untracked-files=all",
+  ...WORKSPACE_PATHSPEC,
+];
+
+/** `git diff --cached` probe: staged paths in the workspace subtree. */
+export const STAGED_PROBE: readonly string[] = [
+  "diff",
+  "--cached",
+  "--name-only",
+  "-z",
+  ...WORKSPACE_PATHSPEC,
+];
+
 export class SpawnGitRunner implements GitRunner {
   constructor(private readonly cwd: string) {}
 
   run(args: string[]): GitRunResult {
-    const result = spawnSync("git", ["-c", "core.literalPathspecs=true", ...args], {
+    const result = spawnSync("git", [...GIT_GLOBAL_ARGS, ...args], {
       cwd: this.cwd,
       shell: false,
       encoding: "utf8",
@@ -98,14 +144,27 @@ export interface GitCommitResult {
   exitCode: number;
 }
 
+/**
+ * Staging guard for the `git_commit` tool.
+ *
+ * Constructing the guard takes the "already dirty before the agent touched
+ * anything" baseline, so construction must happen BEFORE the run's first
+ * workspace mutation — it cannot be deferred all the way to the first
+ * `git_commit` call. At that point the baseline would equal the current state,
+ * the set difference in `planGitCommit` would be empty for every commit, and
+ * the tool would become an unconditional no-op. The owner (`ToolExecutor`)
+ * therefore constructs the guard lazily on the first MUTATING tool call rather
+ * than at CLI start: still before anything can have changed, but off the
+ * startup path and never paid at all by a run that only reads.
+ */
 export class GitCommitGuard {
   private readonly initialDirty: string[];
   private readonly initialStaged: string[];
   private readonly initError: string | null;
 
   constructor(private readonly runner: GitRunner) {
-    const dirty = runner.run(["status", "--porcelain=v1", "-z", "--untracked-files=all"]);
-    const staged = runner.run(["diff", "--cached", "--name-only", "-z"]);
+    const dirty = runner.run([...STATUS_PROBE]);
+    const staged = runner.run([...STAGED_PROBE]);
     this.initialDirty = dirty.ok ? parsePorcelainPaths(dirty.stdout) : [];
     this.initialStaged = staged.ok ? parseNulPaths(staged.stdout) : [];
     this.initError = dirty.ok && staged.ok ? null : "not a usable git repository";
@@ -114,8 +173,8 @@ export class GitCommitGuard {
   commit(message: string): GitCommitResult {
     if (this.initError) return { output: "[git_commit refused: " + this.initError + "]", exitCode: 1 };
 
-    const dirty = this.runner.run(["status", "--porcelain=v1", "-z", "--untracked-files=all"]);
-    const staged = this.runner.run(["diff", "--cached", "--name-only", "-z"]);
+    const dirty = this.runner.run([...STATUS_PROBE]);
+    const staged = this.runner.run([...STAGED_PROBE]);
     if (!dirty.ok || !staged.ok) {
       return { output: "[git_commit refused: unable to inspect repository state]", exitCode: 1 };
     }
@@ -131,7 +190,7 @@ export class GitCommitGuard {
     const added = this.runner.run(["add", "-A", "--", ...plan.candidates]);
     if (!added.ok) return { output: "[git_commit add failed: " + added.stderr.trim() + "]", exitCode: added.exitCode };
 
-    const after = this.runner.run(["diff", "--cached", "--name-only", "-z"]);
+    const after = this.runner.run([...STAGED_PROBE]);
     const stagedAfter = after.ok ? parseNulPaths(after.stdout) : [];
     if (!after.ok || !samePaths(stagedAfter, plan.candidates)) {
       this.runner.run(["reset", "-q", "HEAD", "--", ...plan.candidates]);

src/core/tool_executor.ts

@@ -25,6 +25,17 @@ const DEFAULT_TEST_CMD = "";
 const SNAPSHOT_MAX_BYTES = 1024 * 1024;
 const SEARCH_MAX_HITS = 40;
 const SEARCH_SKIP_DIRS = new Set([".git", "node_modules", "dist"]);
+/**
+ * Tools that can change the workspace. The git commit guard's baseline must be
+ * taken before the first of these runs, and there is no reason to take it
+ * before that: a session that only reads never touches git at all.
+ */
+const MUTATING_TOOLS: ReadonlySet<string> = new Set([
+  "write_file",
+  "run_shell",
+  "run_tests",
+  "git_commit",
+]);
 
 /** Per-call execution controls for the shell-backed tools. */
 export interface RunOptions {
@@ -51,7 +62,12 @@ export interface FileSnapshot {
 
 export class ToolExecutor {
   private readonly root: string;
-  private readonly committer: GitCommitGuard;
+  /**
+   * Built on first use by armCommitGuard(), never in the constructor. See the
+   * comment there — constructing it runs synchronous git probes, and doing that
+   * eagerly froze the CLI before its first turn.
+   */
+  private committer: GitCommitGuard | null = null;
 
   constructor(
     cwd: string,
@@ -60,7 +76,29 @@ export class ToolExecutor {
     // Canonicalize the root once (resolve any symlinks in the workspace path).
     const r = resolve(cwd);
     this.root = existsSync(r) ? realpathSync(r) : r;
-    this.committer = new GitCommitGuard(new SpawnGitRunner(this.root));
+  }
+
+  /**
+   * Build the git commit guard if this run has not built one yet.
+   *
+   * Constructing the guard is what takes its "dirty before the agent started"
+   * baseline, and that costs two synchronous `git` calls. Doing it in the
+   * ToolExecutor constructor meant every `aether agent` / `aether chat` paid
+   * for it at startup, on the main thread, before the first turn rendered —
+   * whether or not the run ever committed anything.
+   *
+   * The baseline still has to be taken before the agent's first mutation, so
+   * it cannot be deferred to the first `git_commit`: at that point the baseline
+   * would equal the current state and every commit would find nothing new.
+   * The correct moment is the first MUTATING tool call — the workspace is
+   * provably untouched by this run, the CLI is already interactive, and a
+   * session that only reads never pays the cost at all.
+   */
+  private armCommitGuard(): GitCommitGuard {
+    if (!this.committer) {
+      this.committer = new GitCommitGuard(new SpawnGitRunner(this.root));
+    }
+    return this.committer;
   }
 
   /**
@@ -219,6 +257,9 @@ export class ToolExecutor {
       return { output: `[tool ${name} rejected: ${validation.error}]`, exitCode: 1 };
     }
     const args = validation.args;
+    // Take the git baseline before the first tool that could change the
+    // workspace — never at construction, and never as late as git_commit.
+    if (MUTATING_TOOLS.has(name)) this.armCommitGuard();
     try {
       switch (name as ToolName) {
         case "read_file":
@@ -259,6 +300,9 @@ export class ToolExecutor {
       return { output: `[tool ${name} rejected: ${validation.error}]`, exitCode: 1 };
     }
     const args = validation.args;
+    // run_shell / run_tests never reach execute() — arm here too, before the
+    // command that may edit the workspace actually starts.
+    if (MUTATING_TOOLS.has(name)) this.armCommitGuard();
     if (name === "web_search") {
       const limit = Number(args["limit"]);
       const text = await webSearch(
@@ -367,7 +411,7 @@ export class ToolExecutor {
   }
 
   private gitCommit(message: string): ToolResult {
-    return this.committer.commit(message);
+    return this.armCommitGuard().commit(message);
   }
 }
 

test/bridge.test.ts

@@ -1,8 +1,8 @@
 import { test } from "node:test";
 import assert from "node:assert/strict";
-import { mkdtempSync, readFileSync, rmSync, symlinkSync, mkdirSync, existsSync, writeFileSync } from "node:fs";
-import { tmpdir } from "node:os";
+import { readFileSync, rmSync, symlinkSync, mkdirSync, existsSync, writeFileSync } from "node:fs";
 import { dirname, join } from "node:path";
+import { TEMP_ROOT, tmpWorkspace } from "./tmp_workspace.js";
 import { fileURLToPath } from "node:url";
 
 import {
@@ -146,7 +146,7 @@ test("LineBuffer yields multiple complete lines in one chunk", () => {
 
 // --- tool executor (one path-guard, [exit N] shape) ------------------------
 test("ToolExecutor writes then reads a file in the workspace", () => {
-  const dir = mkdtempSync(join(tmpdir(), "aether-tool-"));
+  const dir = tmpWorkspace("aether-tool-");
   try {
     const ex = new ToolExecutor(dir);
     const w = ex.execute("write_file", { path: "a.txt", content: "hello" });
@@ -161,7 +161,7 @@ test("ToolExecutor writes then reads a file in the workspace", () => {
 });
 
 test("ToolExecutor refuses a path escaping the workspace", () => {
-  const dir = mkdtempSync(join(tmpdir(), "aether-tool-"));
+  const dir = tmpWorkspace("aether-tool-");
   try {
     const ex = new ToolExecutor(dir);
     const r = ex.execute("read_file", { path: "../../etc/passwd" });
@@ -173,7 +173,7 @@ test("ToolExecutor refuses a path escaping the workspace", () => {
 });
 
 test("repo_search finds literal matches recursively without external grep", () => {
-  const dir = mkdtempSync(join(tmpdir(), "aether-search-"));
+  const dir = tmpWorkspace("aether-search-");
   try {
     mkdirSync(join(dir, "sub"));
     mkdirSync(join(dir, "node_modules"));
@@ -197,7 +197,7 @@ test("repo_search finds literal matches recursively without external grep", () =
 });
 
 test("repo_search caps matches at 40 hits", () => {
-  const dir = mkdtempSync(join(tmpdir(), "aether-search-cap-"));
+  const dir = tmpWorkspace("aether-search-cap-");
   try {
     writeFileSync(join(dir, "many.txt"), Array.from({ length: 50 }, (_, i) => `needle ${i}`).join("\n"), "utf8");
     const r = new ToolExecutor(dir).execute("repo_search", { query: "needle" });
@@ -222,7 +222,7 @@ test("capHeadTail keeps the END (pytest summary) when output is over the cap", (
 });
 
 test("unknown tool returns a clear error, never throws", () => {
-  const ex = new ToolExecutor(tmpdir());
+  const ex = new ToolExecutor(TEMP_ROOT);
   const r = ex.execute("frobnicate", {});
   assert.equal(r.exitCode, 1);
   assert.match(r.output, /unknown tool/);
@@ -254,7 +254,7 @@ class FakeBrain implements Brain {
 }
 
 test("hostLoop executes a tool_call and feeds the result back", async () => {
-  const dir = mkdtempSync(join(tmpdir(), "aether-host-"));
+  const dir = tmpWorkspace("aether-host-");
   try {
     const brain = new FakeBrain();
     const exec = new ToolExecutor(dir);
@@ -279,7 +279,7 @@ test("hostLoop executes a tool_call and feeds the result back", async () => {
 });
 
 test("hostLoop gate denies a tool_call: not executed, brain gets a refusal result", async () => {
-  const dir = mkdtempSync(join(tmpdir(), "aether-gate-"));
+  const dir = tmpWorkspace("aether-gate-");
   try {
     const brain = new FakeBrain(); // emits write_file id=c1 then done on result
     const exec = new ToolExecutor(dir);
@@ -327,7 +327,7 @@ class TwoCallBrain implements Brain {
 }
 
 test("two sequential tool calls: each result pairs to its own id, in order", async () => {
-  const dir = mkdtempSync(join(tmpdir(), "aether-corr-"));
+  const dir = tmpWorkspace("aether-corr-");
   try {
     const exec = new ToolExecutor(dir);
     exec.execute("write_file", { path: "a.txt", content: "AAA" });
@@ -347,8 +347,8 @@ test("two sequential tool calls: each result pairs to its own id, in order", asy
 
 // --- probe 2: path-guard canonicalization (the security boundary) ----------
 test("path-guard rejects traversal, absolute, and symlink escapes", () => {
-  const dir = mkdtempSync(join(tmpdir(), "aether-guard-"));
-  const outside = mkdtempSync(join(tmpdir(), "aether-outside-"));
+  const dir = tmpWorkspace("aether-guard-");
+  const outside = tmpWorkspace("aether-outside-");
   try {
     const ex = new ToolExecutor(dir);
     // .. traversal
@@ -378,7 +378,7 @@ test("run_shell surfaces a non-zero exit code and captures stderr", async (t) =>
   // `node -e` (not a platform-specific shell one-liner) so this passes
   // identically on Windows and POSIX without a win32/posix branch, and runs
   // in an isolated tmpdir so it can't leave stray output under process.cwd().
-  const ex = new ToolExecutor(mkdtempSync(join(tmpdir(), "aether-sh-")));
+  const ex = new ToolExecutor(tmpWorkspace("aether-sh-"));
   const r = await ex.executeAsync("run_shell", {
     command: `node -e "process.stderr.write('boom'); process.exit(3)"`,
   });

test/git_commit_guard.test.ts

@@ -1,7 +1,10 @@
 import { test } from "node:test";
 import assert from "node:assert/strict";
 import {
+  GIT_GLOBAL_ARGS,
   GitCommitGuard,
+  STAGED_PROBE,
+  STATUS_PROBE,
   parseNulPaths,
   parsePorcelainPaths,
   planGitCommit,
@@ -27,6 +30,22 @@ class FakeRunner implements GitRunner {
   }
 }
 
+test("repository probes are bounded to the workspace and take no optional locks", () => {
+  // `git status` reports the whole repository regardless of where it runs, and
+  // `--untracked-files=all` enumerates every untracked path individually. A
+  // workspace that is a small directory inside a large repository therefore
+  // paid O(entire repository) per probe — measured at over 120s on a machine
+  // whose temp directory sits inside a version-controlled home directory. The
+  // pathspec bounds it to the subtree the guard can actually stage from.
+  assert.ok(STATUS_PROBE.includes("--"), "status probe must carry a pathspec");
+  assert.deepEqual(STATUS_PROBE.slice(-2), ["--", "."]);
+  assert.deepEqual(STAGED_PROBE.slice(-2), ["--", "."]);
+  // Reading a repository must not write to it: without this, merely starting
+  // the agent rewrites the user's index.
+  assert.ok(GIT_GLOBAL_ARGS.includes("--no-optional-locks"));
+  assert.ok(GIT_GLOBAL_ARGS.includes("core.literalPathspecs=true"));
+});
+
 test("porcelain parsers handle rename records, spaces, and deduplication", () => {
   assert.deepEqual(
     parsePorcelainPaths(" M dirty file.txt\0R  new.ts\0old.ts\0?? added.ts\0"),

test/process_tree.test.ts

@@ -11,9 +11,9 @@
 
 import { test } from "node:test";
 import assert from "node:assert/strict";
-import { mkdtempSync, writeFileSync } from "node:fs";
-import { tmpdir } from "node:os";
+import { writeFileSync } from "node:fs";
 import { join } from "node:path";
+import { tmpWorkspace } from "./tmp_workspace.js";
 import { ToolExecutor } from "../src/core/tool_executor.js";
 
 /** True while a pid exists. Signal 0 tests for existence without delivering. */
@@ -59,7 +59,7 @@ function pidsFrom(output: string): { child: number | null; grandchild: number |
 }
 
 test("a timed-out command kills its whole tree, not just the shell", async () => {
-  const dir = mkdtempSync(join(tmpdir(), "aether-tree-"));
+  const dir = tmpWorkspace("aether-tree-");
   const script = treeScript(dir);
   const exec = new ToolExecutor(dir);
 
@@ -78,7 +78,7 @@ test("a timed-out command kills its whole tree, not just the shell", async () =>
 });
 
 test("an aborted command kills its whole tree and reports aborted, not timed out", async () => {
-  const dir = mkdtempSync(join(tmpdir(), "aether-abort-"));
+  const dir = tmpWorkspace("aether-abort-");
   const script = treeScript(dir);
   const exec = new ToolExecutor(dir);
   const controller = new AbortController();
@@ -104,7 +104,7 @@ test("an aborted command kills its whole tree and reports aborted, not timed out
 });
 
 test("a normal command still returns its real exit code and output", async () => {
-  const dir = mkdtempSync(join(tmpdir(), "aether-ok-"));
+  const dir = tmpWorkspace("aether-ok-");
   const exec = new ToolExecutor(dir);
 
   const ok = await exec.executeAsync("run_shell", { command: `"${process.execPath}" -e "console.log('hi')"` });
@@ -119,7 +119,7 @@ test("the event loop keeps running while a command is in flight", async () => {
   // spawnSync blocked the loop outright, freezing heartbeats, renderers and any
   // AbortController for the duration. Proving a timer fires during the call is
   // what distinguishes async execution from a merely faster synchronous one.
-  const dir = mkdtempSync(join(tmpdir(), "aether-loop-"));
+  const dir = tmpWorkspace("aether-loop-");
   const exec = new ToolExecutor(dir);
   let ticks = 0;
   const timer = setInterval(() => {