diff --git a/graphify/llm.py b/graphify/llm.py index 46a4785d3..21499f2c3 100644 --- a/graphify/llm.py +++ b/graphify/llm.py @@ -400,6 +400,59 @@ def _no_window_kwargs() -> dict: return {} +_CLAUDE_CLI_PROBE_TIMEOUT = 30 + + +def _resolve_claude_cli() -> str: + """Return a runnable Claude CLI executable for the current platform. + + Windows installations can leave a broken npm ``claude.cmd`` shim beside a + working native ``claude.exe``. Resolve supported candidates in a stable + order and probe each one before using it so a stale shim cannot shadow the + native installation. Non-Windows keeps the existing bare ``claude`` + invocation unchanged. + """ + import platform + import shutil + import subprocess + + if platform.system() != "Windows": + if shutil.which("claude") is None: + raise RuntimeError( + "Claude Code CLI not found on $PATH. Install from " + "https://claude.ai/code and run `claude` once to authenticate." + ) + return "claude" + + candidates: list[str] = [] + for name in ("claude.cmd", "claude.exe", "claude"): + path = shutil.which(name) + if path and path not in candidates: + candidates.append(path) + + for candidate in candidates: + try: + probe = subprocess.run( + [candidate, "--version"], + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + timeout=_CLAUDE_CLI_PROBE_TIMEOUT, + check=False, + **_no_window_kwargs(), + ) + except (OSError, subprocess.SubprocessError): + continue + if probe.returncode == 0: + return candidate + + raise RuntimeError( + "Claude Code CLI not found or not executable on $PATH. Install from " + "https://claude.ai/code and run `claude` once to authenticate." + ) + + def _resolve_api_timeout(default: float = 600.0) -> float: """Honour GRAPHIFY_API_TIMEOUT env var override, else use default (seconds).""" raw = os.environ.get("GRAPHIFY_API_TIMEOUT", "").strip() @@ -1473,31 +1526,9 @@ def _call_claude_cli(user_message: str, max_tokens: int = 8192, *, deep_mode: bo the model to open each one with its Read tool, and each containing directory is allowlisted with `--add-dir` so the read is permitted. """ - import platform - import shutil import subprocess - # On Windows, npm installs `claude` as both `claude.ps1` and `claude.cmd` - # alongside each other. When PATHEXT lists `.PS1` before `.CMD`, - # `shutil.which("claude")` returns `claude.ps1`, which `CreateProcess` - # cannot execute directly — it raises `[WinError 2] The system cannot - # find the file specified`. `claude.cmd` IS executable by CreateProcess, - # so prefer it explicitly on Windows. See issue #1072. - claude_cmd = "claude" - if platform.system() == "Windows": - cmd_path = shutil.which("claude.cmd") - if cmd_path: - claude_cmd = cmd_path - elif shutil.which("claude") is None: - raise RuntimeError( - "Claude Code CLI not found on $PATH. Install from " - "https://claude.ai/code and run `claude` once to authenticate." - ) - elif shutil.which("claude") is None: - raise RuntimeError( - "Claude Code CLI not found on $PATH. Install from " - "https://claude.ai/code and run `claude` once to authenticate." - ) + claude_cmd = _resolve_claude_cli() # Deliver the extraction instructions in the USER turn rather than via # --system-prompt. Newer Claude Code CLIs (>= ~2.1) do not treat a @@ -2605,19 +2636,9 @@ def _rec(inp, out) -> None: return resp.content[0].text if resp.content else "" if backend == "claude-cli": - import platform, shutil, subprocess - # Mirror the extraction-path resolution: on Windows the npm shim is - # claude.cmd, which CreateProcess can't resolve from a bare "claude" - # (PATHEXT doesn't apply), so pass the resolved .cmd path explicitly. - claude_cmd = "claude" - if platform.system() == "Windows": - cmd_path = shutil.which("claude.cmd") - if cmd_path: - claude_cmd = cmd_path - elif shutil.which("claude") is None: - raise RuntimeError("Claude Code CLI not found on $PATH") - elif shutil.which("claude") is None: - raise RuntimeError("Claude Code CLI not found on $PATH") + import subprocess + + claude_cmd = _resolve_claude_cli() cli_args = [claude_cmd, "-p", "--output-format", "json", "--no-session-persistence"] if model is not None: cli_args.extend(["--model", mdl]) diff --git a/tests/test_claude_cli_backend.py b/tests/test_claude_cli_backend.py index 4f5187ab4..5a0c40024 100644 --- a/tests/test_claude_cli_backend.py +++ b/tests/test_claude_cli_backend.py @@ -81,8 +81,14 @@ def test_raises_when_cli_missing(): def test_raises_on_nonzero_exit(): completed = MagicMock(returncode=2, stdout="", stderr="auth failed") + + def fake_run(args, **kwargs): + if args[1] in ("--version", "--help"): + return MagicMock(returncode=0, stdout="Claude 1.0", stderr="") + return completed + with patch("shutil.which", return_value="/fake/bin/claude"), \ - patch("subprocess.run", return_value=completed): + patch("subprocess.run", side_effect=fake_run): with pytest.raises(RuntimeError, match="exited 2"): llm._call_claude_cli("dummy", max_tokens=8192) @@ -104,8 +110,14 @@ def test_nonzero_exit_surfaces_envelope_error_when_stderr_empty(): completed = MagicMock( returncode=1, stdout=json.dumps(_ERROR_ENVELOPE), stderr="", ) + + def fake_run(args, **kwargs): + if args[1] in ("--version", "--help"): + return MagicMock(returncode=0, stdout="Claude 1.0", stderr="") + return completed + with patch("shutil.which", return_value="/fake/bin/claude"), \ - patch("subprocess.run", return_value=completed): + patch("subprocess.run", side_effect=fake_run): with pytest.raises(RuntimeError, match="Rate limit reached"): llm._call_claude_cli("dummy", max_tokens=8192) @@ -156,8 +168,14 @@ def test_call_llm_nonzero_exit_surfaces_envelope_error(): completed = MagicMock( returncode=1, stdout=json.dumps(_ERROR_ENVELOPE), stderr="", ) + + def fake_run(args, **kwargs): + if args[1] == "--version": + return MagicMock(returncode=0, stdout="Claude 1.0", stderr="") + return completed + with patch("shutil.which", return_value="/fake/bin/claude"), \ - patch("subprocess.run", return_value=completed): + patch("subprocess.run", side_effect=fake_run): with pytest.raises(RuntimeError, match="Rate limit reached"): llm._call_llm("dummy", backend="claude-cli") @@ -327,19 +345,106 @@ def fake_which(name): ) -def test_windows_falls_back_to_bare_claude_when_cmd_missing(monkeypatch): - """If `claude.cmd` is somehow unavailable but `claude` resolves - (e.g. WSL-style install), fall back to the bare name so the - existing behaviour is preserved.""" +def test_windows_skips_broken_cmd_and_uses_working_exe_for_extraction(monkeypatch): + """A broken npm shim must not shadow a working native CLI for extraction.""" + cmd_path = r"C:\npm\claude.cmd" + exe_path = r"C:\Users\u\.local\bin\claude.exe" + paths = { + "claude.cmd": cmd_path, + "claude.exe": exe_path, + "claude": r"C:\npm\claude.ps1", + } + calls = [] + + def fake_run(args, **kwargs): + calls.append(args) + proc = MagicMock(stderr="") + proc.returncode = 0 + if args[1] == "--version": + proc.returncode = int(args[0] == cmd_path) + proc.stdout = "broken shim" if proc.returncode else "Claude 1.0" + elif args[1] == "--help": + proc.stdout = "" + else: + proc.stdout = json.dumps(_ENVELOPE) + return proc + + monkeypatch.setattr(llm, "_response_is_hollow", lambda raw, parsed: False) + llm._JSON_SCHEMA_SUPPORT.clear() + with patch("platform.system", return_value="Windows"), \ + patch("shutil.which", side_effect=paths.get), \ + patch("subprocess.run", side_effect=fake_run): + llm._call_claude_cli("dummy", max_tokens=8192) + + assert calls[0][:2] == [cmd_path, "--version"] + assert calls[1][:2] == [exe_path, "--version"] + assert calls[-1][0] == exe_path + assert calls[-1][1] == "-p" + + +def test_windows_skips_broken_cmd_and_uses_working_exe_for_labeling(monkeypatch): + """The lightweight labeling path must use the same resolved executable.""" + cmd_path = r"C:\npm\claude.cmd" + exe_path = r"C:\Users\u\.local\bin\claude.exe" + paths = { + "claude.cmd": cmd_path, + "claude.exe": exe_path, + "claude": r"C:\npm\claude.ps1", + } + calls = [] + + def fake_run(args, **kwargs): + calls.append(args) + proc = MagicMock(stderr="") + proc.returncode = int(args[0] == cmd_path) + proc.stdout = "broken shim" if proc.returncode else json.dumps({"result": "ok"}) + return proc + + with patch("platform.system", return_value="Windows"), \ + patch("shutil.which", side_effect=paths.get), \ + patch("subprocess.run", side_effect=fake_run): + out = llm._call_llm("hi", backend="claude-cli") + + assert out == "ok" + assert calls[0][:2] == [cmd_path, "--version"] + assert calls[1][:2] == [exe_path, "--version"] + assert calls[-1][0] == exe_path + assert calls[-1][1] == "-p" + + +def test_windows_raises_when_all_cli_candidates_fail(): + """A failed probe across all supported candidates reports an actionable error.""" + paths = { + "claude.cmd": r"C:\npm\claude.cmd", + "claude.exe": r"C:\Users\u\.local\bin\claude.exe", + "claude": r"C:\npm\claude.ps1", + } + calls = [] + + def fake_run(args, **kwargs): + calls.append(args) + return MagicMock(returncode=1, stdout="", stderr="not executable") + + with patch("platform.system", return_value="Windows"), \ + patch("shutil.which", side_effect=paths.get), \ + patch("subprocess.run", side_effect=fake_run): + with pytest.raises(RuntimeError, match="not found or not executable"): + llm._call_claude_cli("dummy", max_tokens=8192) + + assert [call[1] for call in calls] == ["--version"] * 3 + + +def test_windows_uses_resolved_bare_claude_when_other_candidates_missing(monkeypatch): + """If only the bare Windows candidate resolves, use its resolved path.""" completed = MagicMock(returncode=0, stdout=json.dumps(_ENVELOPE), stderr="") monkeypatch.setattr(llm, "_response_is_hollow", lambda raw, parsed: False) def fake_which(name): - if name == "claude.cmd": - return None - if name == "claude": - return "/usr/local/bin/claude" - return None + return { + "claude.cmd": None, + "claude.exe": None, + "claude": "/usr/local/bin/claude", + }.get(name) with patch("platform.system", return_value="Windows"), \ patch("shutil.which", side_effect=fake_which), \ @@ -347,12 +452,11 @@ def fake_which(name): llm._call_claude_cli("dummy", max_tokens=8192) argv = run.call_args.args[0] - assert argv[0] == "claude" + assert argv[0] == "/usr/local/bin/claude" def test_windows_raises_when_neither_cmd_nor_bare_claude_present(): - """If neither `claude.cmd` nor `claude` are on PATH on Windows, - raise the standard not-found error.""" + """If no supported Claude candidate is on PATH, raise the not-found error.""" with patch("platform.system", return_value="Windows"), \ patch("shutil.which", return_value=None): with pytest.raises(RuntimeError, match="Claude Code CLI not found"):