fix(installer): fix agent detection, global install, and add interactive agent selection

README.md

@@ -64,10 +64,11 @@ Slash command availability depends on the agent CLI. Claude Code, Codex, and Gem
 
 | Flag | Effect |
 |------|--------|
-| `./install.sh` | Auto-detect all agents, prompt global vs local, install all skills |
-| `./install.sh --global` | Skip prompt, install to global agent dirs (all projects) |
-| `./install.sh --local` | Skip prompt, install to `.claude/skills/` and `.agents/skills/` in cwd |
-| `./install.sh --only claude` | Claude Code only |
+| `./install.sh` | Auto-detect agents, prompt for global vs local, prompt which agents to install |
+| `./install.sh --global` | Skip location prompt, install to global agent dirs (all projects) |
+| `./install.sh --local` | Skip location prompt, install to `.claude/skills/` and `.agents/skills/` in cwd |
+| `./install.sh --non-interactive` | Skip all prompts, install globally to all detected agents (useful in CI / curl\|bash) |
+| `./install.sh --only claude` | Claude Code only (skips agent selection prompt) |
 | `./install.sh --only codex` | Codex only |
 | `./install.sh --only gemini` | Gemini CLI only |
 | `./install.sh --dry-run` | Preview, write nothing |

bin/install.js

@@ -151,7 +151,7 @@ const PROVIDERS = [
 
 const IS_WIN = process.platform === 'win32';
 
-function shellEscape(s) { return `'${String(s).replace(/'/g, `'\\''`)}`; }
+function shellEscape(s) { return `'${String(s).replace(/'/g, "'\\''")}'`; }
 
 function expandHome(p) {
   return String(p).replace(/^\$HOME(?=\/|$)/, os.homedir()).replace(/^~(?=\/|$)/, os.homedir());
@@ -317,6 +317,31 @@ async function promptGlobalOrLocal(c) {
   });
 }
 
+// ── Agent selection prompt ────────────────────────────────────────────────
+
+// Returns the subset of `detected` the user wants to install.
+// Skipped when: no TTY, ≤1 agent, or --only/--non-interactive already constrains the set.
+async function promptAgentSelection(detected, c) {
+  return new Promise((resolve) => {
+    const rl = readline.createInterface({ input: process.stdin, output: process.stdout });
+    rl.on('SIGINT', () => { rl.close(); resolve([]); });
+
+    process.stdout.write('\nDetected agents:\n');
+    detected.forEach((p, i) => {
+      process.stdout.write(`  [${i + 1}] ${c.bold(p.label)}\n`);
+    });
+    process.stdout.write('\n');
+    rl.question(`Install which? (${c.dim('all')} / space-separated numbers / none, default: all): `, (answer) => {
+      rl.close();
+      const a = (answer || '').trim().toLowerCase();
+      if (!a || a === 'all') { resolve(detected); return; }
+      if (a === 'none' || a === '0') { resolve([]); return; }
+      const indices = a.split(/\s+/).map(n => parseInt(n, 10) - 1).filter(i => i >= 0 && i < detected.length);
+      resolve([...new Set(indices)].map(i => detected[i]));
+    });
+  });
+}
+
 // ── Local install helpers ─────────────────────────────────────────────────
 
 // GET a raw text resource over HTTPS, following redirects up to MAX_REDIRECTS.
@@ -348,9 +373,13 @@ function fetchRawText(url) {
 }
 
 async function fetchSkill(slug, repoRoot) {
-  if (repoRoot) {
-    const p = path.join(repoRoot, 'skills', slug, 'SKILL.md');
-    try { return fs.readFileSync(p, 'utf8'); } catch (_) { return null; }
+  // Try local sources first: explicit repoRoot, then the package bundled alongside this script.
+  const localRoots = [];
+  if (repoRoot) localRoots.push(repoRoot);
+  localRoots.push(path.resolve(__dirname, '..'));
+  for (const root of localRoots) {
+    const p = path.join(root, 'skills', slug, 'SKILL.md');
+    try { return fs.readFileSync(p, 'utf8'); } catch (_) { /* try next */ }
   }
   return fetchRawText(`${RAW_BASE}/skills/${slug}/SKILL.md`);
 }
@@ -475,12 +504,17 @@ function installGemini(opts, results, c) {
   else results.failed.push({ id: 'gemini', why: 'gemini extensions install failed' });
 }
 
-function installViaSkills(prov, opts, results, c) {
+// Exported for tests — pure, no side-effects.
+function buildSkillsAddArgs(profile, installMode) {
+  const args = ['-y', 'skills', 'add', REPO, '--agent', profile, '--skill', '*', '--yes'];
+  if (installMode === 'global') args.push('-g');
+  return args;
+}
+
+function installViaSkills(prov, opts, results, c, installMode) {
   results.detected++;
   process.stdout.write(`\n${c.bold(`→ ${prov.label}`)}\n`);
-  // --yes --all: skip the upstream skill-selection TUI. Without these, curl|bash
-  // (no TTY on stdin) renders an empty checkbox list and exits 0 with nothing installed.
-  const args = ['-y', 'skills', 'add', REPO, '-a', prov.profile, '--yes', '--all'];
+  const args = buildSkillsAddArgs(prov.profile, installMode);
   const r = runSpawn('npx', args, opts.dryRun, c);
   if ((r.status || 0) === 0) results.installed.push(prov.id);
   else results.failed.push({ id: prov.id, why: `npx skills add (${prov.profile}) failed` });
@@ -498,9 +532,11 @@ Usage:
 Install location:
   --global           Install to global agent dirs, available in all projects (default)
   --local            Install to .claude/skills/ and .agents/skills/ in the current directory
-  --non-interactive  Skip prompt and default to global (useful in CI / curl|bash)
+  --non-interactive  Skip all prompts: default to global, install all detected agents (CI / curl|bash)
 
-If none of the above are given and stdin+stdout are TTYs, an interactive prompt appears.
+If none of the above are given and stdin+stdout are TTYs, two prompts appear:
+  1. global vs local install location
+  2. which detected agents to install to (when more than one is found)
 
 Other flags:
   --all              Install to every detected agent (default mode)
@@ -617,24 +653,31 @@ async function main() {
     return;
   }
 
-  // Global install: existing agent-detection logic.
-  process.stdout.write(c.bold('codeforerunner') + c.dim(' — installing skills into detected agents\n'));
-  if (opts.dryRun) process.stdout.write(c.yellow('  (dry-run — no files written)\n'));
-
+  // Global install: detect, prompt for selection, then install.
+  // Collect every provider that passes the hard filters.
+  let candidates = [];
   for (const prov of PROVIDERS) {
-    // soft providers only install when explicitly requested via --only
     if (prov.soft && !opts.only.includes(prov.id)) continue;
-    // --only filter
     if (opts.only.length && !opts.only.includes(prov.id)) continue;
+    if (!detectMatch(prov.detect)) continue;
+    if (opts.skipSkills && prov.profile) continue;
+    candidates.push(prov);
+  }
 
-    const detected = detectMatch(prov.detect);
-    if (!detected) continue;
+  // Interactive agent selection when TTY and no explicit --only / --non-interactive.
+  let selected = candidates;
+  if (candidates.length > 1 && !opts.only.length && !opts.nonInteractive &&
+      process.stdin.isTTY && process.stdout.isTTY) {
+    selected = await promptAgentSelection(candidates, c);
+  }
 
-    if (opts.skipSkills && prov.profile) continue;
+  process.stdout.write(c.bold('\ncodeforerunner') + c.dim(' — installing skills into detected agents\n'));
+  if (opts.dryRun) process.stdout.write(c.yellow('  (dry-run — no files written)\n'));
 
-    if (prov.id === 'claude')  { installClaude(opts, results, c);          continue; }
-    if (prov.id === 'gemini')  { installGemini(opts, results, c);          continue; }
-    if (prov.profile)          { installViaSkills(prov, opts, results, c); continue; }
+  for (const prov of selected) {
+    if (prov.id === 'claude')  { installClaude(opts, results, c);                      continue; }
+    if (prov.id === 'gemini')  { installGemini(opts, results, c);                      continue; }
+    if (prov.profile)          { installViaSkills(prov, opts, results, c, installMode); continue; }
   }
 
   printSummary(results, c, installMode);
@@ -646,4 +689,4 @@ if (require.main === module) {
 }
 
 // Exported for tests (tests/install.test.js, tests/test_installer.py) — kept minimal.
-module.exports = { loadTaskSkillSlugs, slugsFromTasksJson, fetchRawText };
+module.exports = { loadTaskSkillSlugs, slugsFromTasksJson, fetchRawText, shellEscape, buildSkillsAddArgs };

tests/install.test.js

@@ -20,7 +20,7 @@ const INSTALL_JS = path.join(REPO_ROOT, 'bin', 'install.js');
 
 const https = require('node:https');
 
-const { slugsFromTasksJson, loadTaskSkillSlugs, fetchRawText } = require(INSTALL_JS);
+const { slugsFromTasksJson, loadTaskSkillSlugs, fetchRawText, shellEscape, buildSkillsAddArgs } = require(INSTALL_JS);
 
 // Swap https.get for a fake; returns a restore fn. install.js holds the same
 // cached https module object, so mutating .get here is visible to fetchRawText.
@@ -158,6 +158,140 @@ test('loadTaskSkillSlugs reads the real tasks.json from a local checkout', async
   assert.ok(slugs.includes('forerunner-scan'));
 });
 
+// ── shellEscape ────────────────────────────────────────────────────────────
+
+test('shellEscape wraps plain string in single quotes', () => {
+  assert.equal(shellEscape('claude'), "'claude'");
+});
+
+test('shellEscape escapes embedded single quotes', () => {
+  // "it's" → 'it'"'"'s'   (end quote, escaped quote, re-open quote)
+  assert.equal(shellEscape("it's"), "'it'\\''s'");
+});
+
+test('shellEscape output is syntactically valid shell (sh -c)', () => {
+  const { spawnSync: spawn } = require('node:child_process');
+  for (const input of ['hello', "with spaces", "single'quote", "double\"quote", "semi;colon"]) {
+    const quoted = shellEscape(input);
+    // printf '%s' <quoted> prints the value without a newline; compare to input.
+    const r = spawn('sh', ['-c', `printf '%s' ${quoted}`], { encoding: 'utf8' });
+    assert.equal(r.status, 0, `sh syntax error for input: ${JSON.stringify(input)}`);
+    assert.equal(r.stdout, input, `round-trip failed for: ${JSON.stringify(input)}`);
+  }
+});
+
+test('shellEscape output allows command -v to find an existing binary', () => {
+  const { spawnSync: spawn } = require('node:child_process');
+  // 'sh' is always present; use it as a known-good detection target.
+  const r = spawn('sh', ['-c', `command -v ${shellEscape('sh')}`], { stdio: 'ignore' });
+  assert.equal(r.status, 0, 'command -v sh should exit 0 with correct shellEscape');
+});
+
+// ── buildSkillsAddArgs ─────────────────────────────────────────────────────
+// Pure unit tests — no agent detection, no file system, no CLI invocation.
+
+test('buildSkillsAddArgs global: includes --agent <profile>, --skill *, and -g', () => {
+  const args = buildSkillsAddArgs('cursor', 'global');
+  assert.ok(args.includes('--agent'), 'expected --agent flag');
+  assert.equal(args[args.indexOf('--agent') + 1], 'cursor', 'expected cursor as agent value');
+  assert.ok(args.includes('--skill'), 'expected --skill flag');
+  assert.equal(args[args.indexOf('--skill') + 1], '*', 'expected * as skill value');
+  assert.ok(args.includes('-g'), 'expected -g for global install');
+  assert.ok(!args.includes('--all'), '--all must not appear (overrides agent filter)');
+});
+
+test('buildSkillsAddArgs local: includes --agent <profile> and --skill * but no -g', () => {
+  const args = buildSkillsAddArgs('cursor', 'local');
+  assert.ok(args.includes('--agent'), 'expected --agent flag');
+  assert.equal(args[args.indexOf('--agent') + 1], 'cursor');
+  assert.ok(args.includes('--skill'), 'expected --skill flag');
+  assert.equal(args[args.indexOf('--skill') + 1], '*');
+  assert.ok(!args.includes('-g'), '-g must not appear for local install');
+});
+
+test('buildSkillsAddArgs: profile is preserved per provider (not overridden to *)', () => {
+  for (const profile of ['opencode', 'codex', 'roo', 'amp']) {
+    const args = buildSkillsAddArgs(profile, 'global');
+    assert.equal(args[args.indexOf('--agent') + 1], profile, `expected profile ${profile} in args`);
+  }
+});
+
+test('local dry-run does NOT pass -g to npx skills add', () => {
+  // Local mode calls writeSkillsLocal (direct file writes), not installViaSkills.
+  // So -g must never appear in local dry-run output.
+  const r = spawnSync(
+    process.execPath,
+    [INSTALL_JS, '--dry-run', '--local', '--non-interactive', '--no-color'],
+    { encoding: 'utf8', cwd: os.tmpdir() },
+  );
+  assert.doesNotMatch((r.stdout || ''), / -g(\s|$)/, '-g must not appear in local dry-run');
+});
+
+// ── CLI dry-run: local install paths ──────────────────────────────────────
+
+test('local dry-run writes to cwd not home dir', () => {
+  const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cfr-test-'));
+  try {
+    const r = spawnSync(
+      process.execPath,
+      [INSTALL_JS, '--dry-run', '--local', '--non-interactive', '--no-color'],
+      { encoding: 'utf8', cwd: tmpDir },
+    );
+    const out = r.stdout || '';
+    assert.ok(out.includes(tmpDir), `expected cwd (${tmpDir}) in local dry-run output`);
+    assert.ok(!out.includes(os.homedir() + path.sep + '.claude'), 'local install must not target home .claude');
+  } finally {
+    fs.rmSync(tmpDir, { recursive: true, force: true });
+  }
+});
+
+test('local dry-run targets .claude/skills/ and .agents/skills/', () => {
+  const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cfr-test-'));
+  try {
+    const r = spawnSync(
+      process.execPath,
+      [INSTALL_JS, '--dry-run', '--local', '--non-interactive', '--no-color'],
+      { encoding: 'utf8', cwd: tmpDir },
+    );
+    const out = r.stdout || '';
+    assert.match(out, /\.claude[/\\]skills/, 'expected .claude/skills path in local dry-run');
+    assert.match(out, /\.agents[/\\]skills/, 'expected .agents/skills path in local dry-run');
+  } finally {
+    fs.rmSync(tmpDir, { recursive: true, force: true });
+  }
+});
+
+// ── CLI: --only filter ─────────────────────────────────────────────────────
+
+test('--only with unknown agent id exits non-zero with error', () => {
+  const r = spawnSync(
+    process.execPath,
+    [INSTALL_JS, '--only', 'does-not-exist', '--no-color'],
+    { encoding: 'utf8' },
+  );
+  assert.notEqual(r.status, 0);
+  assert.match((r.stderr || ''), /unknown agent/);
+});
+
+// ── CLI: non-interactive defaults ─────────────────────────────────────────
+
+test('--non-interactive with --local does not prompt and exits cleanly', () => {
+  const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cfr-test-'));
+  try {
+    const r = spawnSync(
+      process.execPath,
+      [INSTALL_JS, '--dry-run', '--local', '--non-interactive', '--no-color'],
+      { encoding: 'utf8', cwd: tmpDir },
+    );
+    assert.equal(r.status, 0);
+    assert.match(r.stdout || '', /local install/);
+  } finally {
+    fs.rmSync(tmpDir, { recursive: true, force: true });
+  }
+});
+
+// ── local install exits non-zero with a clear error when the registry is unreadable ──
+
 test('local install exits non-zero with a clear error when the registry is unreadable', () => {
   // Preload stub: poison every tasks.json read path (local file + HTTPS) so
   // loadTaskSkillSlugs() resolves null, exercising writeSkillsLocal's guard.