diff --git a/bin/install.js b/bin/install.js index 8d37188..1f6957c 100755 --- a/bin/install.js +++ b/bin/install.js @@ -420,6 +420,15 @@ function captureSpawn(cmd, args) { catch (_) { return { status: 1, stdout: '', stderr: '' }; } } +// spawnSync reports a missing binary as { status: null, error }, so the old +// `(r.status || 0) === 0` checks read ENOENT as success — a machine without +// the `claude` CLI got "installed: claude" with nothing installed and the +// standalone-hook fallback skipped (issue #592). Every spawn result must pass +// through here before being treated as "it worked". +function spawnOk(r) { + return !!r && !r.error && r.status === 0; +} + function absoluteNodePath() { return process.execPath; } @@ -444,10 +453,13 @@ async function installClaude(ctx) { } else { const r1 = runSpawn('claude', ['plugin', 'marketplace', 'add', REPO], null, opts.dryRun); const r2 = runSpawn('claude', ['plugin', 'install', 'caveman@caveman'], null, opts.dryRun); - if ((r1.status || 0) === 0 && (r2.status || 0) === 0) { + if (spawnOk(r1) && spawnOk(r2)) { results.installed.push('claude'); pluginInstallSucceeded = true; } else { + if (r1.error || r2.error) { + warn(' claude CLI not found on PATH (or could not be spawned)'); + } results.failed.push(['claude', 'claude plugin install failed']); } } @@ -536,7 +548,7 @@ function installGemini(ctx) { } } const r = runSpawn('gemini', ['extensions', 'install', `https://github.com/${REPO}`], null, opts.dryRun); - if ((r.status || 0) === 0) results.installed.push('gemini'); + if (spawnOk(r)) results.installed.push('gemini'); else results.failed.push(['gemini', 'gemini extensions install failed']); process.stdout.write('\n'); } @@ -557,7 +569,7 @@ function installViaSkills(ctx, prov) { // documented form for "install every skill into a specific agent". const args = ['-y', 'skills', 'add', REPO, '--skill', '*', '-a', prov.profile, '--yes']; const r = runSpawn('npx', args, null, opts.dryRun); - if ((r.status || 0) === 0) results.installed.push(prov.id); + if (spawnOk(r)) results.installed.push(prov.id); else results.failed.push([prov.id, `npx skills add (${prov.profile}) failed`]); process.stdout.write('\n'); } @@ -1024,7 +1036,7 @@ function installMcpShrink(ctx) { ['mcp', 'add', 'caveman-shrink', '--', 'npx', '-y', MCP_SHRINK_PKG, ...upstream], null, opts.dryRun ); - if ((r.status || 0) === 0) { + if (spawnOk(r)) { note(` registered, wrapping: ${upstream.join(' ')}`); note(` Edit ~/.claude.json mcpServers["caveman-shrink"] to change the upstream,`); note(' or `claude mcp remove caveman-shrink` to drop it.'); @@ -1043,7 +1055,7 @@ async function runInit(ctx) { if (opts.force) args.push('--force'); if (local && fs.existsSync(local)) { const r = runSpawn(absoluteNodePath(), [local, ...args], null, opts.dryRun); - return (r.status || 0) === 0; + return spawnOk(r); } // Curl-pipe fallback if (opts.dryRun) { @@ -1055,7 +1067,7 @@ async function runInit(ctx) { await downloadTo(INIT_SCRIPT_URL, tmp); const r = child_process.spawnSync(absoluteNodePath(), [tmp, ...args], { stdio: 'inherit' }); try { fs.unlinkSync(tmp); } catch (_) {} - return (r.status || 0) === 0; + return spawnOk(r); } catch (e) { warn(' ' + e.message); return false; @@ -1159,7 +1171,7 @@ function uninstall(ctx) { const probe = captureSpawn('claude', ['plugin', 'list']); if (probe.status === 0 && /caveman/i.test(probe.stdout || '')) { const r = runSpawn('claude', ['plugin', 'uninstall', 'caveman@caveman'], null, opts.dryRun); - if ((r.status || 0) === 0) ok(' removed claude plugin'); + if (spawnOk(r)) ok(' removed claude plugin'); } else { note(' claude plugin not installed — skipping'); } @@ -1450,7 +1462,7 @@ async function main() { // --yes --all for the same reason as installViaSkills above (issue #370): // skip the interactive skill picker so curl|bash actually installs. const r = runSpawn('npx', ['-y', 'skills', 'add', REPO, '--yes', '--all'], null, opts.dryRun); - if ((r.status || 0) === 0) ctx.results.installed.push('skills-auto'); + if (spawnOk(r)) ctx.results.installed.push('skills-auto'); else ctx.results.failed.push(['skills-auto', 'npx skills add (auto) failed']); process.stdout.write('\n'); } diff --git a/tests/installer/e2e.freshinstall.test.mjs b/tests/installer/e2e.freshinstall.test.mjs index d7b54e1..515c6b6 100644 --- a/tests/installer/e2e.freshinstall.test.mjs +++ b/tests/installer/e2e.freshinstall.test.mjs @@ -519,3 +519,38 @@ test('openclaw: append on a well-formed block stays a no-op', () => { fs.rmSync(dir, { recursive: true, force: true }); } }); + +// ── Test: missing `claude` CLI must be a FAILURE, not silent success (#592) +// spawnSync reports ENOENT as { status: null, error }; the old +// `(r.status || 0) === 0` coerced that to success, so the installer printed +// "installed: claude", skipped the standalone-hook fallback, and left the +// machine with nothing installed. Always runs: an empty PATH guarantees the +// claude lookup fails even on machines that do have the CLI. +test('missing claude CLI: reports failure and falls back to standalone hook wiring', () => { + const dir = freshTmpDir(); + const emptyBin = path.join(dir, 'empty-bin'); + fs.mkdirSync(emptyBin); + const configDir = path.join(dir, 'claude-config'); + try { + // process.execPath instead of 'node': the stripped PATH must not break + // the test's own ability to launch the installer. + const r = spawnSync(process.execPath, [ + INSTALLER, '--only', 'claude', '--skip-skills', + '--config-dir', configDir, '--non-interactive', '--no-mcp-shrink', + ], { + env: { ...process.env, PATH: emptyBin, CLAUDE_CONFIG_DIR: configDir, NO_COLOR: '1' }, + encoding: 'utf8', + }); + const out = (r.stdout || '') + (r.stderr || ''); + assert.match(out, /plugin install did not succeed; falling back to standalone wiring/, + `fallback to standalone hooks did not trigger:\n${out}`); + assert.match(out, /claude plugin install failed/, 'claude was not reported as failed'); + assert.ok(!/• claude\n/.test(out), 'claude must not be listed as installed'); + assert.ok(fs.existsSync(path.join(configDir, 'hooks', 'caveman-activate.js')), + 'standalone hooks were not written'); + const settings = JSON.parse(fs.readFileSync(path.join(configDir, 'settings.json'), 'utf8')); + assert.ok(settings.hooks && settings.hooks.SessionStart, 'SessionStart hook not wired'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } +});