(test): drain a capped git instead of killing it, so its repository can be removed on Windows

.ai/contexts/changes-view.md

@@ -14,6 +14,7 @@ integration: `.ai/contexts/viewer-panel.md` ("Changes mode").
 |---|---|
 | `git-changes.js` | Pure parser — no electron, no DOM, no fs. `require()`-d from `main.js` and from tests, same pattern as `remote-hosts.js` / `derive-project-path.js`. |
 | `git-changes-runner.js` | Runs the git commands, local or remote, behind one interface. |
+| `run-to-exit.js` | The local spawn both runners share: settles on `close`, drains past the stdout cap instead of killing — see "A capped read waits for git to exit". |
 | `git-changes-target.js` | cwd resolution for the panel's IPCs, extracted out of `main.js` for testability (same rationale as `delete-session-target.js`). |
 | `git-changes-file.js` | The content pair and the write target behind the editable diff: the `<rev>:<path>` guard, the repository-containment check, the read and the write. |
 | `git-changes-watch.js` | The registry behind `git-changes-watch`: arms `fs.watch`, debounces, re-arms after a rename, and reports the repo-relative path. |
@@ -36,7 +37,7 @@ integration: `.ai/contexts/viewer-panel.md` ("Changes mode").
 
 `createGitChangesRunner({kind, cwd, alias, exec, timeoutMs, fsOps})` → `{status(), diff(path, {staged, untracked}), isWorkTree()}`. `status()` runs three commands in parallel (`git status --porcelain=v2 --branch -uall -z`, `git diff --numstat -z`, `git diff --cached --numstat -z`), merges them, and reports `untrackedCollapsed` (see "Untracked files"). `diff()` runs `git diff [--cached] -- <path>` — or, with `untracked: true`, `git diff --no-index -- /dev/null <path>` (see "Untracked files") — capped at 512 KB (`MAX_DIFF_BYTES`) measured in UTF-8 bytes and cut on a line boundary, with a `truncated` flag.
 
-- **Local** (`kind: 'local'`): `child_process.execFile('git', args, {cwd, timeout, maxBuffer})` — cwd is `execFile`'s own option, never a `-C` argument. No shell is invoked, so argument content cannot be interpreted as a command regardless of what it contains; timeout 10s.
+- **Local** (`kind: 'local'`): `runToExit('git', args, {cwd, timeoutMs, maxBuffer})` (`run-to-exit.js`, a `child_process.spawn` with no shell) — cwd is the spawn's own option, never a `-C` argument. No shell is invoked, so argument content cannot be interpreted as a command regardless of what it contains; timeout 10s. See "A capped read waits for git to exit".
 - **Remote** (`kind: 'remote'`): the same ssh transport `remote-attach.js` already uses for the tmux probe/restore calls (`buildRemoteCommandArgs`, `defaultRunRemoteCommand`) — `ssh -o BatchMode=yes -o ConnectTimeout=5 -n <alias> "git -C '<cwd>' '--literal-pathspecs' 'diff' '--' '<path>' ..."`. Timeout 20s. This command string DOES run through a shell on the far end.
 - **`invoke(args, remoteOpts)`** is the single choke point both `status()` and `diff()` go through: it prepends `--literal-pathspecs` (`buildGitArgs`, see "Quoting rule") to every argv/command, and threads `remoteOpts.maxStdoutBytes` to the remote transport only (the local path's `execFile` `maxBuffer` already bounds it).
 
@@ -90,9 +91,9 @@ git's default untracked mode; if the retry succeeds the result comes back
 `? dir/` rows, and a note saying the untracked listing is coarse.
 
 The retry is gated on the failure signature (`isStdoutCapFailure`: the remote
-transport's own `stdout exceeded <n> bytes`, or `execFile`'s
-`stdout maxBuffer length exceeded` — both non-localized, one ours and one
-Node's). Any other failure returns its own error untouched: retrying on every
+transport's own `stdout exceeded <n> bytes`, or the local runner's
+`stdout maxBuffer length exceeded`, worded as Node's `execFile` words it — both
+non-localized). Any other failure returns its own error untouched: retrying on every
 non-zero exit would tell a user whose repository is unreadable
 (`could not read directory: Permission denied`) that they have too many
 untracked files, and discard the real message on the way. If the retry itself
@@ -351,7 +352,7 @@ of them produces `reason: 'not-a-repo'`.
 **A cwd that is gone is ruled out before the corroboration is trusted.** The walk
 below answers "no `.git` anywhere" for a path that does not exist, so a deleted
 worktree outside a repository would otherwise be reported as "not a git
-repository". `execFile` happens to fail to spawn for such a cwd — code `-1`, not 128 — but the local and
+repository". The local spawn happens to fail for such a cwd — code `-1`, not 128 — but the local and
 remote transports differ here (`git -C <gone>` exits 128), so the check is an
 outcome of its own rather than something left to a code that happens not to
 match. It also decides the wording: `spawn git ENOENT` reads as "git is not
@@ -723,3 +724,30 @@ hook), the scratch-repo test wrote `tracked.txt` into the outer repository's
 index and rewrote its local `user.email`; the test helper now drops `GIT_*` /
 `HUSKY*` for the scratch repo and disables its hooks, and the runner no longer
 trusts them either.
+
+## A capped read waits for git to exit
+
+Both local runners (`defaultRunGit` in `git-changes-file.js`, `defaultLocalExec`
+in `git-changes-runner.js`) go through `runToExit` (`run-to-exit.js`), which
+settles on the child's `close` and never kills a child for overrunning its
+stdout cap. Past the cap it keeps reading and drops the bytes, so git runs to
+its own end; the result carries `overflow: true` and the same
+`stdout maxBuffer length exceeded` message `execFile` would give.
+
+The reason is Windows. There, a git that was killed leaves its working
+directory busy for a moment after `execFile` has reported its exit: the
+repository cannot be removed (`EBUSY` on `rmdir`) although neither the `git.exe`
+on `PATH` nor the `mingw64\bin\git.exe` it starts as its own child is still
+reported alive. A git that runs to its own end does not. On `windows-2022` the
+over-the-cap subtest of `test/git-changes-file-real-git.test.js` hit `EBUSY` in
+its cleanup in 24 of 8400 runs while the cap killed git, and in none of 8400
+once git was drained instead.
+
+A timeout still kills: a git that hangs cannot be waited for. The pipes are
+closed on this side first, as `execFile` does, so a grandchild that still holds
+them cannot delay `close`. On Windows a timed-out git therefore leaves its
+working directory busy for a moment after the call has returned, as above.
+
+`fs.rmSync`'s `maxRetries` does not cover this failure on Node 20 and 22: their
+recursive removal retries only after emptying a directory (`ENOTEMPTY`,
+`EPERM`); an `EBUSY` on the first `rmdir` of a directory is thrown at once.

git-changes-file.js

@@ -5,7 +5,7 @@
 const realFs = require('fs');
 const path = require('path');
 const crypto = require('crypto');
-const { execFile } = require('child_process');
+const { runToExit } = require('./run-to-exit');
 const { localGitEnv } = require('./git-changes-runner');
 const { parseStatusPorcelainV2 } = require('./git-changes');
 const { resolveOnDisk, isInsideDir } = require('./resolve-path-on-disk');
@@ -51,23 +51,10 @@ function requireLocalTarget(target) {
   return target;
 }
 
-function defaultRunGit(args, { cwd, timeoutMs, maxBuffer }) {
-  return new Promise((resolve) => {
-    execFile('git', args, { cwd, env: localGitEnv(), timeout: timeoutMs, maxBuffer, encoding: 'buffer', windowsHide: true },
-      (err, stdout, stderr) => {
-        const out = Buffer.isBuffer(stdout) ? stdout : Buffer.from(stdout || '');
-        if (err) {
-          resolve({
-            code: typeof err.code === 'number' ? err.code : -1,
-            stdout: out,
-            stderr: String(stderr || err.message || ''),
-            tooLarge: err.code === 'ERR_CHILD_PROCESS_STDIO_MAXBUFFER',
-          });
-          return;
-        }
-        resolve({ code: 0, stdout: out, stderr: '', tooLarge: false });
-      });
-  });
+async function defaultRunGit(args, { cwd, timeoutMs, maxBuffer }) {
+  const result = await runToExit('git', args, { cwd, env: localGitEnv(), timeoutMs, maxBuffer });
+  if (result.code === 0) return { code: 0, stdout: result.stdout, stderr: '', tooLarge: false };
+  return { code: result.code, stdout: result.stdout, stderr: result.stderr || result.message, tooLarge: result.overflow };
 }
 
 // see .ai/contexts/changes-view.md ("Containment, and which path the write runs on")

git-changes-runner.js

@@ -2,7 +2,7 @@
 
 'use strict';
 
-const { execFile } = require('child_process');
+const { runToExit } = require('./run-to-exit');
 const fs = require('fs');
 const path = require('path');
 const { defaultRunRemoteCommand } = require('./remote-attach');
@@ -184,17 +184,11 @@ function localGitEnv() {
   return env;
 }
 
-function defaultLocalExec(args, { cwd, timeoutMs }) {
-  return new Promise((resolve) => {
-    execFile('git', args, { cwd, env: localGitEnv(), timeout: timeoutMs, maxBuffer: LOCAL_MAX_BUFFER, windowsHide: true },
-      (err, stdout, stderr) => {
-        if (err) {
-          resolve({ code: typeof err.code === 'number' ? err.code : -1, stdout: stdout || '', stderr: stderr || err.message || String(err) });
-          return;
-        }
-        resolve({ code: 0, stdout: stdout || '', stderr: stderr || '' });
-      });
-  });
+async function defaultLocalExec(args, { cwd, timeoutMs }) {
+  const result = await runToExit('git', args, { cwd, env: localGitEnv(), timeoutMs, maxBuffer: LOCAL_MAX_BUFFER });
+  const stdout = result.stdout.toString('utf8');
+  if (result.code === 0) return { code: 0, stdout, stderr: result.stderr };
+  return { code: result.code, stdout, stderr: result.stderr || result.message };
 }
 
 // Bounds what git wrote — see .ai/contexts/changes-view.md ("Bounded error messages")

run-to-exit.js

@@ -0,0 +1,76 @@
+'use strict';
+
+const { spawn } = require('child_process');
+
+// see .ai/contexts/changes-view.md ("A capped read waits for git to exit")
+function runToExit(file, args, { cwd, env, timeoutMs, maxBuffer }, spawnFn = spawn) {
+  return new Promise((resolve) => {
+    const command = [file, ...args].join(' ');
+    const stdoutChunks = [];
+    const stderrChunks = [];
+    let stdoutBytes = 0;
+    let stderrBytes = 0;
+    let overflow = false;
+    let timedOut = false;
+    let spawnError = null;
+    let settled = false;
+    let timer = null;
+
+    const finish = (exitCode) => {
+      if (settled) return;
+      settled = true;
+      if (timer) clearTimeout(timer);
+      const stdout = Buffer.concat(stdoutChunks);
+      const stderr = Buffer.concat(stderrChunks).toString('utf8');
+      let message = '';
+      if (spawnError) message = spawnError.message;
+      else if (overflow) message = 'stdout maxBuffer length exceeded';
+      else if (exitCode !== 0) message = `Command failed: ${command}`;
+      const code = spawnError || overflow || timedOut || typeof exitCode !== 'number' ? -1 : exitCode;
+      resolve({ code, stdout, stderr, overflow, timedOut, message });
+    };
+
+    let child;
+    try {
+      child = spawnFn(file, args, { cwd, env, windowsHide: true });
+    } catch (err) {
+      spawnError = err;
+      finish(null);
+      return;
+    }
+
+    child.stdout.on('data', (chunk) => {
+      if (overflow) return;
+      if (stdoutBytes + chunk.length > maxBuffer) {
+        stdoutChunks.push(chunk.subarray(0, maxBuffer - stdoutBytes));
+        stdoutBytes = maxBuffer;
+        overflow = true;
+        return;
+      }
+      stdoutChunks.push(chunk);
+      stdoutBytes += chunk.length;
+    });
+    child.stderr.on('data', (chunk) => {
+      if (stderrBytes >= maxBuffer) return;
+      const kept = chunk.subarray(0, maxBuffer - stderrBytes);
+      stderrChunks.push(kept);
+      stderrBytes += kept.length;
+    });
+    if (timeoutMs > 0) {
+      timer = setTimeout(() => {
+        timedOut = true;
+        child.stdout.destroy();
+        child.stderr.destroy();
+        child.kill();
+      }, timeoutMs);
+    }
+    child.on('error', (err) => {
+      if (child.pid !== undefined) return;
+      spawnError = err;
+      finish(null);
+    });
+    child.on('close', (exitCode) => finish(exitCode));
+  });
+}
+
+module.exports = { runToExit };

test/git-changes-file-real-git.test.js

@@ -24,10 +24,7 @@ function mkTmp() {
   return fs.realpathSync.native(fs.mkdtempSync(path.join(os.tmpdir(), 'switchboard-gcf-real-')));
 }
 
-// The maxBuffer cap SIGTERMs an overrunning `git cat-file`, and execFile's
-// callback runs before that child has been reaped (measured: exitCode null,
-// killed true). On Windows a live process holds a handle on its working
-// directory, so removing the scratch repo can race it — hence the retries.
+// see .ai/contexts/changes-view.md ("A capped read waits for git to exit")
 function cleanup(dir) {
   fs.rmSync(dir, { recursive: true, force: true, maxRetries: 10, retryDelay: 100 });
 }

test/git-changes-runner-real-git.test.js

@@ -42,9 +42,7 @@ function mkTmp() {
   return fs.realpathSync.native(dir);
 }
 
-// Same race as test/git-changes-file-real-git.test.js: a git child killed by a
-// cap is still terminating when the assertion returns, and on Windows it holds
-// its working directory until it dies.
+// see .ai/contexts/changes-view.md ("A capped read waits for git to exit")
 function cleanup(dir) {
   fs.rmSync(dir, { recursive: true, force: true, maxRetries: 10, retryDelay: 100 });
 }

test/run-to-exit.test.js

@@ -0,0 +1,83 @@
+'use strict';
+
+const test = require('node:test');
+const assert = require('node:assert/strict');
+const fs = require('fs');
+const os = require('os');
+const path = require('path');
+const { spawn } = require('child_process');
+
+const { runToExit } = require('../run-to-exit');
+
+function mkTmp() {
+  return fs.mkdtempSync(path.join(os.tmpdir(), 'switchboard-rte-'));
+}
+
+function cleanup(dir) {
+  fs.rmSync(dir, { recursive: true, force: true, maxRetries: 10, retryDelay: 100 });
+}
+
+function node(script, opts = {}) {
+  let child = null;
+  const spawnFn = (file, args, options) => {
+    child = spawn(file, args, options);
+    return child;
+  };
+  const promise = runToExit(process.execPath, ['-e', script], { cwd: opts.cwd || process.cwd(), env: process.env, timeoutMs: opts.timeoutMs || 10_000, maxBuffer: opts.maxBuffer || 1024 * 1024 }, spawnFn);
+  return { promise, child: () => child };
+}
+
+test('runToExit: a child that overruns the stdout cap is left to finish, and the call settles only once it has exited', async () => {
+  const tmp = mkTmp();
+  try {
+    const marker = path.join(tmp, 'finished');
+    const script = `process.stdout.write('x'.repeat(8192), () => setTimeout(() => { require('fs').writeFileSync(${JSON.stringify(marker)}, 'done'); }, 300));`;
+    const run = node(script, { maxBuffer: 1024 });
+    const result = await run.promise;
+    assert.equal(result.overflow, true);
+    assert.equal(result.code, -1);
+    assert.equal(result.message, 'stdout maxBuffer length exceeded');
+    assert.equal(result.stdout.length, 1024, 'what fits under the cap is kept, the rest is drained and dropped');
+    assert.ok(fs.existsSync(marker), 'the child ran to its own end instead of being killed by the cap');
+    assert.notEqual(run.child().exitCode, null, 'the child has exited by the time the call settles');
+  } finally { cleanup(tmp); }
+});
+
+test('runToExit: a timeout kills the child and settles only after it has exited', async () => {
+  const run = node('setInterval(() => {}, 1000);', { timeoutMs: 200 });
+  const result = await run.promise;
+  assert.equal(result.timedOut, true);
+  assert.equal(result.code, -1);
+  const child = run.child();
+  assert.ok(child.exitCode !== null || child.signalCode !== null, 'the killed child has exited by the time the call settles');
+});
+
+test('runToExit: a timeout settles even when a grandchild still holds the output pipes', async () => {
+  const script = "require('child_process').spawn(process.execPath, ['-e', 'setTimeout(() => {}, 3000)'], { stdio: 'inherit' }); setInterval(() => {}, 1000);";
+  const started = Date.now();
+  const result = await node(script, { timeoutMs: 300 }).promise;
+  assert.equal(result.timedOut, true);
+  assert.ok(Date.now() - started < 2500, `settled after ${Date.now() - started} ms, not when the grandchild let go of the pipes`);
+});
+
+test('runToExit: a spawn that never ran settles with code -1 and the spawn error', async () => {
+  const result = await runToExit(process.execPath, ['-e', ''], { cwd: path.join(os.tmpdir(), 'switchboard-rte-missing-dir-does-not-exist'), env: process.env, timeoutMs: 10_000, maxBuffer: 1024 });
+  assert.equal(result.code, -1);
+  assert.match(result.message, /ENOENT/);
+});
+
+test('runToExit: a clean exit returns stdout and stderr with code 0', async () => {
+  const result = await node("process.stdout.write('out'); process.stderr.write('warn');").promise;
+  assert.equal(result.code, 0);
+  assert.equal(result.stdout.toString('utf8'), 'out');
+  assert.equal(result.stderr, 'warn');
+  assert.equal(result.overflow, false);
+  assert.equal(result.message, '');
+});
+
+test('runToExit: a non-zero exit keeps its code and stderr', async () => {
+  const result = await node("process.stderr.write('fatal: nope'); process.exitCode = 3;").promise;
+  assert.equal(result.code, 3);
+  assert.equal(result.stderr, 'fatal: nope');
+  assert.match(result.message, /^Command failed: /);
+});