chore(cli-registry): clean up dead code and stale claims left after #380

.changeset/cli-catalog-followups.md

@@ -0,0 +1,24 @@
+---
+"aicodeman": patch
+---
+
+Cleans up the loose ends the maintainer flagged as "worth knowing rather than fixing" when
+merging the CLI-catalogue-driven `install.sh`/Docker-agent-image PR (#380):
+
+- `install.sh` no longer carries `_cli_index`/`check_cli`/`get_cli_path`, three generic
+  lookup helpers left behind once the catalogue-driven menu and hints stopped calling them.
+- The generator no longer emits `CLI_KIND`/`CLI_NPM`, two bash arrays nothing in `install.sh`
+  read (the `.mjs`/`docker-hosts.ts` producers already read the JSON catalogue's `kind`/
+  `npmPackage` fields directly).
+- `detect_all_clis` now skips a disabled entry entirely rather than probing it and filtering
+  the result downstream — no stock entry ships disabled today, so this is a latent
+  inefficiency closed before it is a latent bug, not a behaviour change.
+- The install hint for a `launcherProfile` entry (DeepSeek today) now explains, in one line,
+  why it is a docs link rather than a runnable command — its docs page documents
+  `npm install -g @deepseek-ai/dsh`, which installs the launcher only and cannot drive a pane
+  on its own, the exact trap the menu already avoids by withholding the command. Driven by a
+  new generated `CLI_LAUNCHER_ONLY` array (from `discovery.launcherProfile`), not an id check.
+- The non-interactive default's comment no longer claims it is always Claude Code: on a
+  wget-only host, Claude's curl one-liner is filtered out of the offered list first, so the
+  default becomes whichever npm-based entry sorts earliest instead. Behaviour is unchanged
+  (and was already printed, so never silent) — only the comment was wrong.

install.sh

@@ -93,8 +93,7 @@ export PUPPETEER_SKIP_DOWNLOAD="${PUPPETEER_SKIP_DOWNLOAD:-1}"
 CLI_IDS=('claude' 'shell' 'opencode' 'codex' 'gemini' 'antigravity' 'pi' 'grok' 'deepseek' 'omp')
 CLI_LABELS=('Claude' 'Shell' 'OpenCode' 'Codex' 'Gemini' 'Antigravity' 'Pi' 'Grok' 'DeepSeek' 'OMP')
 CLI_ENABLED=(1 1 1 1 1 1 1 1 1 1)
-CLI_KIND=('agent' 'shell' 'agent' 'agent' 'agent' 'agent' 'agent' 'agent' 'agent' 'agent')
-CLI_NPM=('@anthropic-ai/claude-code' '' 'opencode-ai' '@openai/codex' '@google/gemini-cli' '' '@earendil-works/pi-coding-agent' '' '@deepseek-ai/dsh' '')
+CLI_LAUNCHER_ONLY=(0 0 0 0 0 0 0 0 1 0)
 CLI_DOCS=('https://docs.claude.com/claude-code' '' 'https://opencode.ai/docs' 'https://developers.openai.com/codex/cli' 'https://github.com/google-gemini/gemini-cli' 'https://antigravity.google/cli' 'https://pi.dev' 'https://github.com/xai-org/grok-build' 'https://github.com/deepseek-ai/deepseek-harness' 'https://omp.sh')
 CLI_CMD_LINUX=('curl -fsSL https://claude.ai/install.sh | bash' '' 'curl -fsSL https://opencode.ai/install | bash' 'npm install -g @openai/codex' 'npm install -g @google/gemini-cli' 'curl -fsSL https://antigravity.google/cli/install.sh | bash' 'npm install -g --ignore-scripts @earendil-works/pi-coding-agent' 'curl -fsSL https://x.ai/cli/install.sh | bash' '' 'curl -fsSL https://omp.sh/install | sh')
 CLI_CMD_DARWIN=('curl -fsSL https://claude.ai/install.sh | bash' '' 'curl -fsSL https://opencode.ai/install | bash' 'npm install -g @openai/codex' 'npm install -g @google/gemini-cli' 'curl -fsSL https://antigravity.google/cli/install.sh | bash' 'npm install -g --ignore-scripts @earendil-works/pi-coding-agent' 'curl -fsSL https://x.ai/cli/install.sh | bash' '' 'brew install can1357/tap/omp')
@@ -410,22 +409,6 @@ check_build_tools() {
 # test/install-sh-detection-parity.test.ts: the process PATH first (each declared
 # binary name in turn), then each known install path, dir-major.
 
-# Index of "$1" in CLI_IDS -> CLI_IDX, returning 1 with CLI_IDX=-1 when unknown.
-# A global rather than an echo because this runs inside loops, and a subshell per
-# lookup is a fork per CLI per call site.
-CLI_IDX=-1
-_cli_index() {
-    local want="$1" i
-    CLI_IDX=-1
-    for ((i = 0; i < ${#CLI_IDS[@]}; i++)); do
-        if [[ "${CLI_IDS[$i]}" == "$want" ]]; then
-            CLI_IDX=$i
-            return 0
-        fi
-    done
-    return 1
-}
-
 # `dsh` is the hardest name of the lot: Debian ships an unrelated `dsh`
 # (dancer's shell). The server-side resolver settles it by demanding the
 # harness's own help banner; detection here only feeds the "you have no AI CLI"
@@ -465,7 +448,8 @@ _cli_candidate_ok() {
 
 # Resolve every CLI in ONE pass, memoized.
 #
-# CLI_FOUND_PATH is parallel to CLI_IDS ('' when not found). CLI_FOUND_COUNT
+# CLI_FOUND_PATH is parallel to CLI_IDS ('' when not found, and also '' for a
+# DISABLED entry — it is never probed at all, see below). CLI_FOUND_COUNT
 # counts only ENABLED entries that have a binary to look for, which is what the
 # "no AI CLI found" gate asks about — `shell` has no binary and must never make
 # that gate think an agent is installed.
@@ -485,6 +469,16 @@ detect_all_clis() {
     for ((i = 0; i < ${#CLI_IDS[@]}; i++)); do
         found=""
 
+        # A disabled entry is never even probed: every consumer already filters
+        # on CLI_ENABLED before showing anything, so the command-v/stat calls
+        # below would be pure waste — and, unlike filtering downstream, skipping
+        # the probe here is what makes CLI_ENABLED mean "look for it" rather
+        # than just "offer it once found".
+        if [[ "${CLI_ENABLED[$i]}" != "1" ]]; then
+            CLI_FOUND_PATH[$i]=""
+            continue
+        fi
+
         # 1. The process PATH, each declared binary name in turn.
         bin_end=$((${CLI_BIN_OFF[$i]} + ${CLI_BIN_LEN[$i]}))
         for ((j = ${CLI_BIN_OFF[$i]}; j < bin_end; j++)); do
@@ -520,20 +514,6 @@ detect_all_clis() {
     return 0
 }
 
-# Is this CLI installed? Unknown id is "no", never an error.
-check_cli() {
-    detect_all_clis
-    _cli_index "$1" || return 1
-    [[ -n "${CLI_FOUND_PATH[$CLI_IDX]}" ]]
-}
-
-# Where it was found, or nothing.
-get_cli_path() {
-    detect_all_clis
-    _cli_index "$1" || return 1
-    printf '%s\n' "${CLI_FOUND_PATH[$CLI_IDX]}"
-}
-
 # ----------------------------------------------------------------------------
 # Catalogue helpers
 # ----------------------------------------------------------------------------
@@ -589,7 +569,12 @@ cli_catalog_names() {
 # the registry but an empty one here: installing the launcher alone leaves
 # nothing that can drive a pane, so the generator withholds the command for
 # any launcherProfile entry (see installCommandFor in generate-cli-catalog.mts)
-# and this hint falls through to the docs URL instead.
+# and this hint falls through to the docs URL instead — CLI_LAUNCHER_ONLY adds
+# one line explaining WHY it is a docs link and not a command, so a user who
+# follows that link straight to `npm install -g @deepseek-ai/dsh` (which the
+# docs page itself documents) does not land back in the same "installed but
+# cannot drive a pane" trap the menu exists to avoid. Data-driven, not an id
+# check: any future launcherProfile entry gets the same caveat for free.
 cli_catalog_print_install_hints() {
     detect_all_clis
     local i
@@ -601,6 +586,9 @@ cli_catalog_print_install_hints() {
             echo -e "    ${CYAN}${CLI_INSTALL_CMD_TRUSTED[$i]}${NC}   # ${CLI_LABELS[$i]}"
         elif [[ -n "${CLI_DOCS[$i]}" ]]; then
             echo -e "    ${CLI_LABELS[$i]}: see ${CYAN}${CLI_DOCS[$i]}${NC}"
+            if [[ "${CLI_LAUNCHER_ONLY[$i]}" == "1" ]]; then
+                echo -e "      (its package installs a launcher only — it needs a profile that can drive a pane, see the docs above)"
+            fi
         fi
     done
 }
@@ -679,9 +667,12 @@ offer_ai_cli_install() {
 
         local cli_choice=""
         if [[ "$NONINTERACTIVE" == "1" ]] || ! has_tty; then
-            # Explicit automation opt-in: default to the first offered entry,
-            # which is registry order, which is Claude Code (order 0) — the
-            # same default this prompt has always taken non-interactively.
+            # Explicit automation opt-in: default to the first OFFERED entry.
+            # That is registry order, which is Claude Code (order 0), UNLESS
+            # this is a wget-only host and Claude's curl one-liner was just
+            # filtered out of offer_idx above — there, the first survivor is
+            # whichever npm-based entry sorts earliest (Codex today), not
+            # Claude. Printed either way so the choice is never silent.
             cli_choice="1"
             info "CODEMAN_NONINTERACTIVE=1: defaulting to ${CLI_LABELS[${offer_idx[0]}]}"
         else

scripts/generate-cli-catalog.mts

@@ -135,8 +135,7 @@ export function renderInstallShBlock(entries: CliEntry[] = STOCK_CLIS): string {
   const ids: string[] = [];
   const labels: string[] = [];
   const enabled: string[] = [];
-  const kinds: string[] = [];
-  const npm: string[] = [];
+  const launcherOnly: string[] = [];
   const docs: string[] = [];
   const cmdLinux: string[] = [];
   const cmdDarwin: string[] = [];
@@ -151,8 +150,11 @@ export function renderInstallShBlock(entries: CliEntry[] = STOCK_CLIS): string {
     ids.push(shQuote(entry.id as string));
     labels.push(shQuote(entry.label));
     enabled.push(entry.enabled ? '1' : '0');
-    kinds.push(shQuote(entry.kind));
-    npm.push(shQuote(entry.discovery.install.npmPackage ?? ''));
+    // Parallel to CLI_IDS: 1 when this entry's install command installs a launcher rather
+    // than something that can drive a pane on its own (see installCommandFor below). Purely
+    // derived from discovery.launcherProfile — install.sh's hint printer reads this to add a
+    // caveat instead of hardcoding which id it means.
+    launcherOnly.push(entry.discovery.launcherProfile ? '1' : '0');
     docs.push(shQuote(entry.discovery.install.docsUrl ?? ''));
     cmdLinux.push(shQuote(installCommandFor(entry, 'linux')));
     cmdDarwin.push(shQuote(installCommandFor(entry, 'darwin')));
@@ -195,8 +197,7 @@ export function renderInstallShBlock(entries: CliEntry[] = STOCK_CLIS): string {
     arr('CLI_IDS', ids),
     arr('CLI_LABELS', labels),
     arr('CLI_ENABLED', enabled),
-    arr('CLI_KIND', kinds),
-    arr('CLI_NPM', npm),
+    arr('CLI_LAUNCHER_ONLY', launcherOnly),
     arr('CLI_DOCS', docs),
     arr('CLI_CMD_LINUX', cmdLinux),
     arr('CLI_CMD_DARWIN', cmdDarwin),

test/install-sh-invariants.test.ts

@@ -48,13 +48,12 @@ describe('install.sh generated-catalogue block', () => {
     );
   });
 
-  it('declares every array the detection code indexes', () => {
+  it('declares every array install.sh actually reads', () => {
     for (const name of [
       'CLI_IDS',
       'CLI_LABELS',
       'CLI_ENABLED',
-      'CLI_KIND',
-      'CLI_NPM',
+      'CLI_LAUNCHER_ONLY',
       'CLI_DOCS',
       'CLI_CMD_LINUX',
       'CLI_CMD_DARWIN',
@@ -69,6 +68,17 @@ describe('install.sh generated-catalogue block', () => {
     }
   });
 
+  it('declares no array install.sh never reads', () => {
+    // CLI_KIND and CLI_NPM were generated and read by nothing (the .mjs/docker-hosts.ts
+    // producers read the JSON's `kind`/`npmPackage` fields directly; only these two bash
+    // arrays were dead). A generated-but-unread array is a maintenance trap the generator
+    // itself cannot warn about — it has no reader to check against — so this pins the
+    // opposite of the test above: naming what must NOT come back rather than what must.
+    for (const name of ['CLI_KIND', 'CLI_NPM']) {
+      expect(new RegExp(`^${name}=\\(`, 'm').test(SOURCE), `${name} is declared but nothing reads it`).toBe(false);
+    }
+  });
+
   it('keeps no hand-written per-CLI detection behind', () => {
     // The nine `*_SEARCH_PATHS` arrays and eighteen `check_<cli>`/`get_<cli>_path` pairs are
     // what this change removes. One left behind would be a second source of truth that the
@@ -93,6 +103,16 @@ describe('install.sh generated-catalogue block', () => {
       []
     );
   });
+
+  it('keeps no dead generic-lookup helpers behind', () => {
+    // _cli_index/check_cli/get_cli_path were the ungenericized precursor to the per-CLI
+    // helpers above: same shape, one level of indirection, called from nowhere once the
+    // catalogue-driven menu and hints stopped needing a lookup-by-id. Unlike the per-CLI
+    // pairs these are exact names, not derived from the catalogue.
+    for (const fn of ['_cli_index()', 'check_cli()', 'get_cli_path()']) {
+      expect(CODE.includes(fn), `${fn} should have been removed as dead code`).toBe(false);
+    }
+  });
 });
 
 describe('install.sh trust boundary', () => {
@@ -245,3 +265,42 @@ describe('install.sh AI CLI install menu', () => {
     expect(run.status).toBe(1);
   });
 });
+
+describe('install.sh detect_all_clis and a disabled entry', () => {
+  // No stock entry ships disabled today, so this is characterization rather than a regression
+  // pin on real data: it drives the real function in a real bash with entry 0 fabricated
+  // disabled, and points its binary at `bash` — guaranteed resolvable via `command -v` — to
+  // prove the entry is genuinely never PROBED (CLI_FOUND_PATH stays empty) rather than merely
+  // filtered out downstream by every consumer's own `CLI_ENABLED` check.
+  function driveDetect(disableEntry0: boolean) {
+    const driver = `
+      set -euo pipefail
+      export CODEMAN_INSTALL_SH_LIB=1
+      . "$1"
+      k=0; while [[ $k -lt \${#CLI_ALL_BINS[@]} ]]; do CLI_ALL_BINS[$k]="codeman-test-no-such-bin-$k"; k=$((k + 1)); done
+      k=0; while [[ $k -lt \${#CLI_ALL_PATHS[@]} ]]; do CLI_ALL_PATHS[$k]="/nonexistent/codeman-test/$k"; k=$((k + 1)); done
+      # Point entry 0's first declared binary at something that WILL resolve, so a probe that
+      # runs at all finds it.
+      CLI_ALL_BINS[\${CLI_BIN_OFF[0]}]="bash"
+      ${disableEntry0 ? 'CLI_ENABLED[0]="0"' : ''}
+      CLI_DETECT_DONE=""
+      detect_all_clis
+      echo "path0=[\${CLI_FOUND_PATH[0]}]"
+      echo "found=$CLI_FOUND_COUNT"
+    `;
+    const result = spawnSync('bash', ['-c', driver, 'bash', INSTALL_SH], { encoding: 'utf-8', timeout: 30_000 });
+    return { status: result.status, stdout: result.stdout ?? '', stderr: result.stderr ?? '' };
+  }
+
+  it('probes an enabled entry (control case)', () => {
+    const run = driveDetect(false);
+    expect(run.stdout, run.stderr).not.toContain('path0=[]');
+    expect(run.stdout).toContain('found=1');
+  });
+
+  it('never probes a disabled entry', () => {
+    const run = driveDetect(true);
+    expect(run.stdout, run.stderr).toContain('path0=[]');
+    expect(run.stdout).toContain('found=0');
+  });
+});