fix(git): the fetch timeout reaches the whole tree, a tick covers every session on its folder, and the rest of the review (review)

`execFile` signals the `git` parent only, so a fetch that times out can leave its http helper
still running, and on the interval that repeats every run with nobody watching. `runFetch` now
spawns `detached`, so the fetch and the helper it spawns share one process group, and the kill
is SIGTERM to the negative pid then SIGKILL `FETCH_KILL_ESCALATION_MS` later; the escalation is
not cancelled when the promise settles, because the caller is answered at the timeout while the
group is still dying. The output cap is kept, and the clone's slot queue is not needed here.

The tick is two passes: every enabled local session is collected by folder, a folder is due when
ANY session open on it is due, the folder is probed once for the group, and the run is published
to every session on it. A single pass decided by the order the sessions were read, so [long,
short] published only the short one and the 3600 s session's row never updated again while its
folder was fetched every 30 s.

`gitNonInteractiveEnv` writes BatchMode ssh as a default, not an override: `core.sshCommand` is
read inside the same slot and the same bound as the fetch it serves, and `fetchEnvFor` keeps the
repository's answer when it has one, because that is usually the setup that works on a machine
with several ssh setups. The `accept-new` host key it can write is stated next to the BatchMode
note: a prompt is stopped, a write is not.

The write claim is scoped to the Git panel and to the timer, with cloning and worktree management
named as the writes that happen when the user asked for them, and the zh keys sit with the group
of the UI surface their English string belongs to.

Tests: the process-group test proves the tree by group membership, because the helper is a
grandchild (`git fetch` runs `git remote-http`, which spawns `git-remote-http`) and because the
group is what the negative pid reaches, and it remembers each process once found instead of
re-scanning a group it had already emptied. The non-zero-exit test changed premises: `git fetch
--quiet` in a repository with no remote exits 0 on git 2.47.3, so resolving empty stdout there was
correct, and the case that has to reject is a remote pointing at a path that is not a repository.
The join test asserts after the run and does not await the request before the held run is
released, which would wait on the run the test is holding.

src/git-fetch.ts

@@ -4,15 +4,17 @@
  * (`src/git-fetch-scheduler.ts`).
  *
  * The status module (src/git-workspace-status.ts) is deliberately offline: "behind" is as of the last
- * fetch, so a number that is stale can only be made current by fetching. This is the one place Codeman
- * runs a git command that writes, and on a timer it is also the only one that can run without being asked.
- * Nothing here commits, checks out, merges or touches the working tree: `git fetch` updates remote-tracking
+ * fetch, so a number that is stale can only be made current by fetching. This is the only git command the
+ * Git panel runs that writes, and the only git command Codeman runs on a timer. Cloning a repository and
+ * managing worktrees write too, but those are things the user asked for at the moment they asked; this is
+ * the one that can happen later, by itself. Nothing here commits, checks out, merges or touches the working
+ * tree: `git fetch` updates remote-tracking
  * refs and nothing else, and `FETCH_CONFIG_ARGS` keeps it from starting a gc in a repository an agent is
  * already using. It is not harmless in the way a read is: a fetch holds git's ref lock, so an agent's own
  * `git pull` in the same clone can collide with it (`cannot lock ref`), which is why one folder is fetched
  * by at most one run at a time (`fetchWorkspaceOnce`).
  *
- * Same split as git-clone.ts: the pure half (`buildFetchArgs`, `describeFetchFailure`,
+ * Same split as git-clone.ts: the pure half (`buildFetchArgs`, `describeFetchFailure`, `fetchEnvFor`,
  * `gitFetchSettingsFrom`, `gitFetchStatusFrom`) holds every decision and tests without spawning, and the IO
  * half runs one bounded `git fetch --quiet` per repository.
  *
@@ -27,19 +29,31 @@
  *   - One run per folder at a time: `fetchWorkspaceOnce` joins a run already going for the same folder
  *     instead of starting a second one, because two fetches in one clone collide on the ref lock.
  *   - Bounded: the git timeout from Settings → Bottom bar per repository, `MAX_REPOS_LIMIT` repositories at
- *     most, a hard `timeout` on each spawn, and `FETCH_CONCURRENCY` git processes at a time for the whole
- *     run — one `Limiter`, shared by the scheduler's tick, not one bound per folder.
+ *     most, a hard wall-clock bound on each spawn, and `FETCH_CONCURRENCY` git processes at a time for the
+ *     whole run — one `Limiter`, shared by the scheduler's tick, not one bound per folder.
  *   - Non-interactive: `GIT_TERMINAL_PROMPT=0` and BatchMode ssh, so a private repository that needs a
  *     login fails fast instead of hanging the request on an invisible username prompt. Codeman collects
  *     no credentials; a repository the user's own git is already signed in to works because that is the
- *     user's existing setup, so unlike a clone no credential helper is disabled here.
+ *     user's existing setup, so unlike a clone no credential helper is disabled here. That BatchMode ssh is
+ *     a default, never an override: `fetchEnvFor` keeps whatever the repository's `core.sshCommand` or the
+ *     user's own `GIT_SSH_COMMAND` already says, because on a machine with several ssh setups that choice
+ *     is usually the one that works. ⚠️ The default still leaves a record: `StrictHostKeyChecking=accept-new`
+ *     writes an unknown host key into `~/.ssh/known_hosts` without asking — BatchMode stops a prompt, not a
+ *     write.
+ *   - The timeout kills the PROCESS GROUP, not just the `git` parent. A fetch over http spawns
+ *     `git-remote-http` children, and `execFile`'s `timeout` signals the parent only, so the answer can go
+ *     out while a helper is still running — and on a timer that repeats every tick with nobody watching.
+ *     `spawn` with `detached: true` puts the tree in its own group so the escalation (SIGTERM, then SIGKILL
+ *     `FETCH_KILL_ESCALATION_MS` later) reaches the children too, which is why the negative pid is used
+ *     rather than `child.kill()`. Same shape as `runGit` in git-clone.ts, without that module's slot queue:
+ *     fetch is bounded by `FETCH_CONCURRENCY`, not by a clone queue.
  *   - Failure text reaching a client is control-stripped and credential-redacted (`sanitizeGitOutput`),
  *     and the browser writes it with `textContent` only.
  *
  * @module git-fetch
  */
 
-import { execFile } from 'node:child_process';
+import { execFile, spawn } from 'node:child_process';
 import { basename, relative, resolve } from 'node:path';
 import { promisify } from 'node:util';
 import { classifyGitFailure, gitNonInteractiveEnv, sanitizeGitOutput, type GitFailureCode } from './git-clone.js';
@@ -63,6 +77,15 @@ const execFileAsync = promisify(execFile);
 export const FETCH_CONCURRENCY = 4;
 /** A fetch on a busy remote can print a lot; a bound on what we will hold. */
 const MAX_FETCH_OUTPUT_BYTES = 2 * 1024 * 1024;
+/** git's stderr is kept as a bounded tail (as in git-clone.ts): enough for the reason, not the whole stream. */
+const MAX_FETCH_STDERR_BYTES = 8_192;
+/** A `git config --get` answer is one line; this is a sanity cap, not an expected size. */
+const MAX_SSH_COMMAND_BYTES = 4_096;
+/**
+ * How long the process group gets to answer SIGTERM before the same escalation goes to SIGKILL. Kept as a
+ * const so a test can say how long it is willing to wait for the group to actually die.
+ */
+export const FETCH_KILL_ESCALATION_MS = 3_000;
 
 /**
  * Config overrides set ahead of the argv, so an unattended fetch cannot do more than move refs: no
@@ -151,15 +174,148 @@ export function describeFetchFailure(err: unknown): FetchFailure {
   return failure;
 }
 
-/** Fetch is allowed to write refs, so this runner does not carry `--no-optional-locks`: fetch takes a required lock anyway. */
+/**
+ * The environment a fetch gets, as a pure decision so it can be pinned without spawning git.
+ *
+ * `gitNonInteractiveEnv` supplies the BatchMode default, and the default is only a default: git reads
+ * `GIT_SSH_COMMAND` from the environment ahead of the repository's `core.sshCommand`, so setting it would
+ * override a repository that already chose its own ssh — an alternative host, an identity file, a jump box —
+ * and break a setup that works today. That is the only case where this module makes an ssh choice on the
+ * user's behalf, so it is the whole decision here: the caller supplies the value it read, and this decides
+ * between the user's environment and the repository's own answer.
+ */
+export function fetchEnvFor(base: NodeJS.ProcessEnv, repoSshCommand: string | null): NodeJS.ProcessEnv {
+  const env = gitNonInteractiveEnv(base);
+  // `gitNonInteractiveEnv` only writes the default when the environment had nothing, so this is exactly the
+  // case where it was a choice rather than a pass-through — and the repository's own answer wins over it.
+  if (!base.GIT_SSH_COMMAND && repoSshCommand) delete env.GIT_SSH_COMMAND;
+  return env;
+}
+
+/**
+ * The repository's own `core.sshCommand`, or null. One bounded, read-only `git config --get` per fetch:
+ * git would honour that config itself if we did not write `GIT_SSH_COMMAND` over it, so honouring it means
+ * not writing one. `--get` reads local and global config, which is the value git would have used, and no
+ * hook or filter runs on the way to it.
+ *
+ * The read happens inside the same slot and the same wall-clock bound as the fetch it serves, so it cannot
+ * add a fifth git process or push a fetch past the user's timeout — it spends part of it.
+ */
+export async function readRepoSshCommand(cwd: string, timeoutMs: number): Promise<string | null> {
+  try {
+    const { stdout } = await execFileAsync('git', ['config', '--get', 'core.sshCommand'], {
+      cwd,
+      timeout: timeoutMs,
+      maxBuffer: MAX_SSH_COMMAND_BYTES,
+      env: gitNonInteractiveEnv(),
+    });
+    return stdout.trim() || null;
+  } catch {
+    // A repository with no such setting, or a read that could not finish: the default is what git would have
+    // used anyway, and a fetch must not fail because the check for a config value failed.
+    return null;
+  }
+}
+
+/**
+ * Fetch is allowed to write refs, so this runner does not carry `--no-optional-locks`: fetch takes a
+ * required lock anyway.
+ *
+ * `spawn`, not `execFile`, because the bound has to reach the whole tree (see the fileoverview): the
+ * caller is answered at the timeout, and the SIGTERM → SIGKILL escalation is deliberately NOT cancelled
+ * when the answer goes out, because the promise is a promise about the answer while the group still has to
+ * die. That is the difference from `runGit` in git-clone.ts, which disarms the escalation at `finish` and so
+ * can leave a `git-remote-http` child running after a timed-out call.
+ *
+ * The thrown error keeps the shape `describeFetchFailure` reads: `stderr` for the reason, `message` for a
+ * spawn failure (`spawn git ENOENT` → GIT_MISSING), `killed` and `code` for the timeout. A non-zero exit
+ * rejects the way `execFile` would — `code` is then the exit code — because resolving it would report a real
+ * failure as a clean run with an empty reason.
+ */
 export const runFetch: GitRunner = async (cwd, args, opts) => {
-  const { stdout } = await execFileAsync('git', [...FETCH_CONFIG_ARGS, ...args], {
-    cwd,
-    timeout: opts?.timeoutMs ?? DEFAULT_GIT_TIMEOUT_MS,
-    maxBuffer: MAX_FETCH_OUTPUT_BYTES,
-    env: gitNonInteractiveEnv(),
+  const timeoutMs = opts?.timeoutMs ?? DEFAULT_GIT_TIMEOUT_MS;
+  const env = fetchEnvFor(process.env, await readRepoSshCommand(cwd, timeoutMs));
+
+  return new Promise<string>((resolve, reject) => {
+    let child: ReturnType<typeof spawn>;
+    try {
+      child = spawn('git', [...FETCH_CONFIG_ARGS, ...args], {
+        cwd,
+        env,
+        stdio: ['ignore', 'pipe', 'pipe'],
+        detached: true,
+      });
+    } catch (err) {
+      reject(err);
+      return;
+    }
+
+    let stdout = '';
+    let stderr = '';
+    let stdoutBytes = 0;
+    let timedOut = false;
+    let settled = false;
+    let killTimer: NodeJS.Timeout | undefined;
+
+    const killTree = (signal: NodeJS.Signals) => {
+      try {
+        if (child.pid) process.kill(-child.pid, signal);
+      } catch {
+        try {
+          child.kill(signal);
+        } catch {
+          /* already gone */
+        }
+      }
+    };
+
+    const fail = (spawnError?: string, exitCode?: number | null) => {
+      if (settled) return;
+      settled = true;
+      // Only the timeout timer is disarmed here. On a timeout the escalation is what the caller is waiting
+      // for, and cancelling it is the bug this runner exists to fix.
+      clearTimeout(timeoutTimer);
+      reject(
+        Object.assign(new Error(spawnError ?? `Command failed: git ${args.join(' ')}`), {
+          stderr,
+          killed: timedOut,
+          code: timedOut ? 'ETIMEDOUT' : (exitCode ?? undefined),
+        })
+      );
+    };
+
+    const timeoutTimer = setTimeout(() => {
+      timedOut = true;
+      killTree('SIGTERM');
+      killTimer = setTimeout(() => killTree('SIGKILL'), FETCH_KILL_ESCALATION_MS);
+      fail();
+    }, timeoutMs);
+
+    child.stdout?.on('data', (chunk: Buffer) => {
+      stdoutBytes += chunk.length;
+      if (stdoutBytes <= MAX_FETCH_OUTPUT_BYTES) stdout += chunk.toString('utf-8');
+    });
+    child.stderr?.on('data', (chunk: Buffer) => {
+      stderr += chunk.toString('utf-8');
+      if (stderr.length > MAX_FETCH_STDERR_BYTES * 2) stderr = stderr.slice(-MAX_FETCH_STDERR_BYTES);
+    });
+
+    const finish = (code: number | null) => {
+      if (settled) return;
+      // execFile's contract, which `describeFetchFailure` depends on: a non-zero exit rejects, so a real
+      // git failure keeps its reason in `stderr` instead of becoming a green row with an empty one. `null`
+      // means a signal, reachable only when something outside this runner killed the group — our own timeout
+      // has already answered by then, and `settled` above is what keeps that answer the answer.
+      if (code !== 0) return fail(undefined, code);
+      settled = true;
+      clearTimeout(timeoutTimer);
+      if (killTimer) clearTimeout(killTimer);
+      resolve(stdout);
+    };
+
+    child.on('error', (err) => fail(String(err)));
+    child.on('close', (code) => finish(code));
   });
-  return stdout;
 };
 
 export interface GitFetchEntry {
@@ -191,9 +347,12 @@ export interface GitFetchReport {
  * whether the user clicked or the interval fired, because a number the user did not ask for should read
  * differently from one they did.
  */
+/** What a run was, and why it happened. */
+export type GitFetchTrigger = 'click' | 'interval';
+
 export interface GitFetchStatus {
   ranAt: number;
-  trigger: 'click' | 'interval';
+  trigger: GitFetchTrigger;
   /** As in the report: `ok`, or the state that stopped the run. */
   state: GitFetchReport['state'];
   /** Repositories this run covered. */
@@ -204,7 +363,17 @@ export interface GitFetchStatus {
   firstFailure?: { name: string; code: FetchFailureCode };
 }
 
-export function gitFetchStatusFrom(report: GitFetchReport, trigger: GitFetchStatus['trigger']): GitFetchStatus {
+/**
+ * One run, with the trigger that caused it. A joiner gets the run already going, so what it reports is that
+ * run's trigger rather than the one that made it ask: a click that joins an interval run did not fetch
+ * anything itself, and the row the panel shows has to describe the fetch that actually happened.
+ */
+export interface GitFetchRun {
+  report: GitFetchReport;
+  trigger: GitFetchTrigger;
+}
+
+export function gitFetchStatusFrom(report: GitFetchReport, trigger: GitFetchTrigger): GitFetchStatus {
   const failed = report.repos.filter((r) => !r.ok);
   const status: GitFetchStatus = {
     ranAt: report.fetchedAt,
@@ -283,25 +452,34 @@ export async function fetchGitWorkspaceOverview(cwd: string, opts: GitFetchOptio
  * Runs already going, keyed by the resolved folder. A request for a folder another run is already working on
  * JOINS that run instead of starting a second `git fetch` in the same clone: two fetches there collide on
  * git's ref lock (`cannot lock ref 'refs/remotes/origin/main'`), which is what a click during a tick would
- * otherwise do. A joiner gets the run already going's answer, including its limits — the point is not to
- * fetch twice, not to fetch with the freshest options.
+ * otherwise do. A joiner gets the run already going's answer, including its limits and its trigger — the point
+ * is not to fetch twice, not to fetch with the freshest options, and a click that fetched nothing cannot
+ * report itself as the fetch.
  *
  * Keyed by `resolve`, not `realpath`, because `realpath` is itself one of the unbounded fs calls rule #516
  * bounds for user-chosen paths; the scheduler probes the folder bounded and calls this. Two paths that
  * resolve the same are the same folder here; a workspace reached through a symlink resolves to the link, so
  * a link and its target can be fetched by two runs — the accepted trade for keeping the probe bounded.
  */
-const lane = new Map<string, Promise<GitFetchReport>>();
+const lane = new Map<string, Promise<GitFetchRun>>();
 
-export async function fetchWorkspaceOnce(folder: string, opts: GitFetchOptions = {}): Promise<GitFetchReport> {
+export async function fetchWorkspaceOnce(
+  folder: string,
+  opts: GitFetchOptions = {},
+  trigger: GitFetchTrigger
+): Promise<GitFetchRun> {
   const key = resolve(folder);
   const going = lane.get(key);
+  // The joiner is handed the run already going as it is, trigger included: it did not cause that fetch, so it
+  // must not report itself as having.
   if (going) return going;
-  const run = fetchGitWorkspaceOverview(key, opts).finally(() => {
-    // Only the run that is still the lane's own: a later run for the same folder must not be dropped by an
-    // earlier one finishing late.
-    if (lane.get(key) === run) lane.delete(key);
-  });
+  const run: Promise<GitFetchRun> = fetchGitWorkspaceOverview(key, opts)
+    .then((report) => ({ report, trigger }))
+    .finally(() => {
+      // Only the run that is still the lane's own: a later run for the same folder must not be dropped by an
+      // earlier one finishing late.
+      if (lane.get(key) === run) lane.delete(key);
+    });
   lane.set(key, run);
   return run;
 }