mirror of
https://github.com/github/spec-kit.git
synced 2026-03-27 15:53:07 +00:00
Fix Claude Code CLI detection for npm-local installs (#1978)
* Fix Claude Code CLI detection for npm-local installs `specify check` reports "Claude Code CLI (not found)" for users who installed Claude Code via npm-local (the default installer path, common with nvm). The binary lives at ~/.claude/local/node_modules/.bin/claude which was not checked. Add CLAUDE_NPM_LOCAL_PATH as a second well-known location alongside the existing migrate-installer path. Fixes https://github.com/github/spec-kit/issues/550 * Address Copilot review feedback - Remove unused pytest import from test_check_tool.py - Use tmp_path instead of hardcoded /nonexistent/claude for hermetic tests - Simplify redundant exists() + is_file() to just is_file() AI-assisted: Changes applied with Claude Code. * Update tests/test_check_tool.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update tests/test_check_tool.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -345,6 +345,7 @@ AI_ASSISTANT_HELP = _build_ai_assistant_help()
|
||||
SCRIPT_TYPE_CHOICES = {"sh": "POSIX Shell (bash/zsh)", "ps": "PowerShell"}
|
||||
|
||||
CLAUDE_LOCAL_PATH = Path.home() / ".claude" / "local" / "claude"
|
||||
CLAUDE_NPM_LOCAL_PATH = Path.home() / ".claude" / "local" / "node_modules" / ".bin" / "claude"
|
||||
|
||||
BANNER = """
|
||||
███████╗██████╗ ███████╗ ██████╗██╗███████╗██╗ ██╗
|
||||
@@ -605,13 +606,15 @@ def check_tool(tool: str, tracker: StepTracker = None) -> bool:
|
||||
Returns:
|
||||
True if tool is found, False otherwise
|
||||
"""
|
||||
# Special handling for Claude CLI after `claude migrate-installer`
|
||||
# Special handling for Claude CLI local installs
|
||||
# See: https://github.com/github/spec-kit/issues/123
|
||||
# The migrate-installer command REMOVES the original executable from PATH
|
||||
# and creates an alias at ~/.claude/local/claude instead
|
||||
# This path should be prioritized over other claude executables in PATH
|
||||
# See: https://github.com/github/spec-kit/issues/550
|
||||
# Claude Code can be installed in two local paths:
|
||||
# 1. ~/.claude/local/claude (after `claude migrate-installer`)
|
||||
# 2. ~/.claude/local/node_modules/.bin/claude (npm-local install, e.g. via nvm)
|
||||
# Neither path may be on the system PATH, so we check them explicitly.
|
||||
if tool == "claude":
|
||||
if CLAUDE_LOCAL_PATH.exists() and CLAUDE_LOCAL_PATH.is_file():
|
||||
if CLAUDE_LOCAL_PATH.is_file() or CLAUDE_NPM_LOCAL_PATH.is_file():
|
||||
if tracker:
|
||||
tracker.complete(tool, "available")
|
||||
return True
|
||||
|
||||
96
tests/test_check_tool.py
Normal file
96
tests/test_check_tool.py
Normal file
@@ -0,0 +1,96 @@
|
||||
"""Tests for check_tool() — Claude Code CLI detection across install methods.
|
||||
|
||||
Covers issue https://github.com/github/spec-kit/issues/550:
|
||||
`specify check` reports "Claude Code CLI (not found)" even when claude is
|
||||
installed via npm-local (the default `claude` installer path).
|
||||
"""
|
||||
|
||||
from unittest.mock import patch, MagicMock
|
||||
|
||||
from specify_cli import check_tool
|
||||
|
||||
|
||||
class TestCheckToolClaude:
|
||||
"""Claude CLI detection must work for all install methods."""
|
||||
|
||||
def test_detected_via_migrate_installer_path(self, tmp_path):
|
||||
"""claude migrate-installer puts binary at ~/.claude/local/claude."""
|
||||
fake_claude = tmp_path / "claude"
|
||||
fake_claude.touch()
|
||||
|
||||
# Ensure npm-local path is missing so we only exercise migrate-installer path
|
||||
fake_missing = tmp_path / "nonexistent" / "claude"
|
||||
|
||||
with patch("specify_cli.CLAUDE_LOCAL_PATH", fake_claude), \
|
||||
patch("specify_cli.CLAUDE_NPM_LOCAL_PATH", fake_missing), \
|
||||
patch("shutil.which", return_value=None):
|
||||
assert check_tool("claude") is True
|
||||
|
||||
def test_detected_via_npm_local_path(self, tmp_path):
|
||||
"""npm-local install puts binary at ~/.claude/local/node_modules/.bin/claude."""
|
||||
fake_npm_claude = tmp_path / "node_modules" / ".bin" / "claude"
|
||||
fake_npm_claude.parent.mkdir(parents=True)
|
||||
fake_npm_claude.touch()
|
||||
|
||||
# Neither the migrate-installer path nor PATH has claude
|
||||
fake_migrate = tmp_path / "nonexistent" / "claude"
|
||||
|
||||
with patch("specify_cli.CLAUDE_LOCAL_PATH", fake_migrate), \
|
||||
patch("specify_cli.CLAUDE_NPM_LOCAL_PATH", fake_npm_claude), \
|
||||
patch("shutil.which", return_value=None):
|
||||
assert check_tool("claude") is True
|
||||
|
||||
def test_detected_via_path(self, tmp_path):
|
||||
"""claude on PATH (global npm install) should still work."""
|
||||
fake_missing = tmp_path / "nonexistent" / "claude"
|
||||
|
||||
with patch("specify_cli.CLAUDE_LOCAL_PATH", fake_missing), \
|
||||
patch("specify_cli.CLAUDE_NPM_LOCAL_PATH", fake_missing), \
|
||||
patch("shutil.which", return_value="/usr/local/bin/claude"):
|
||||
assert check_tool("claude") is True
|
||||
|
||||
def test_not_found_when_nowhere(self, tmp_path):
|
||||
"""Should return False when claude is genuinely not installed."""
|
||||
fake_missing = tmp_path / "nonexistent" / "claude"
|
||||
|
||||
with patch("specify_cli.CLAUDE_LOCAL_PATH", fake_missing), \
|
||||
patch("specify_cli.CLAUDE_NPM_LOCAL_PATH", fake_missing), \
|
||||
patch("shutil.which", return_value=None):
|
||||
assert check_tool("claude") is False
|
||||
|
||||
def test_tracker_updated_on_npm_local_detection(self, tmp_path):
|
||||
"""StepTracker should be marked 'available' for npm-local installs."""
|
||||
fake_npm_claude = tmp_path / "node_modules" / ".bin" / "claude"
|
||||
fake_npm_claude.parent.mkdir(parents=True)
|
||||
fake_npm_claude.touch()
|
||||
|
||||
fake_missing = tmp_path / "nonexistent" / "claude"
|
||||
tracker = MagicMock()
|
||||
|
||||
with patch("specify_cli.CLAUDE_LOCAL_PATH", fake_missing), \
|
||||
patch("specify_cli.CLAUDE_NPM_LOCAL_PATH", fake_npm_claude), \
|
||||
patch("shutil.which", return_value=None):
|
||||
result = check_tool("claude", tracker=tracker)
|
||||
|
||||
assert result is True
|
||||
tracker.complete.assert_called_once_with("claude", "available")
|
||||
|
||||
|
||||
class TestCheckToolOther:
|
||||
"""Non-Claude tools should be unaffected by the fix."""
|
||||
|
||||
def test_git_detected_via_path(self):
|
||||
with patch("shutil.which", return_value="/usr/bin/git"):
|
||||
assert check_tool("git") is True
|
||||
|
||||
def test_missing_tool(self):
|
||||
with patch("shutil.which", return_value=None):
|
||||
assert check_tool("nonexistent-tool") is False
|
||||
|
||||
def test_kiro_fallback(self):
|
||||
"""kiro-cli detection should try both kiro-cli and kiro."""
|
||||
def fake_which(name):
|
||||
return "/usr/bin/kiro" if name == "kiro" else None
|
||||
|
||||
with patch("shutil.which", side_effect=fake_which):
|
||||
assert check_tool("kiro-cli") is True
|
||||
Reference in New Issue
Block a user