fix(llm): probe Windows Claude CLI candidates before spawning

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])

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,32 +345,118 @@ 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), \
          patch("subprocess.run", return_value=completed) as run:
         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"):