fix Windows claude-cli WinError 2 and post-commit hook silent drop on rapid commits

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

graphify/llm.py

@@ -490,10 +490,27 @@ def _call_claude_cli(user_message: str, max_tokens: int = 8192, *, deep_mode: bo
     ANTHROPIC_API_KEY. Useful for Pro/Max subscribers who don't want to provision
     a pay-as-you-go API key just to run graphify's semantic pass.
     """
+    import platform
     import shutil
     import subprocess
 
-    if shutil.which("claude") is None:
+    # 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."
@@ -508,7 +525,7 @@ def _call_claude_cli(user_message: str, max_tokens: int = 8192, *, deep_mode: bo
     # Replacing the default prompt eliminates the conflict at the source.
     # Side benefit: cache-creation tokens per call drop ~19% in practice.
     cli_args = [
-        "claude", "-p",
+        claude_cmd, "-p",
         "--output-format", "json",
         "--no-session-persistence",
         "--system-prompt", _extraction_system(deep=deep_mode),

graphify/watch.py

@@ -9,6 +9,83 @@ import time
 from pathlib import Path
 
 _GRAPHIFY_OUT = os.environ.get("GRAPHIFY_OUT", "graphify-out")
+_PENDING_FILENAME = ".pending_changes"
+_PENDING_DRAIN_MAX_PASSES = 20
+
+
+def _queue_pending(out_dir: Path, changed_paths: list[Path]) -> None:
+    """Append ``changed_paths`` to ``out_dir/.pending_changes`` (one per line).
+
+    Used by a post-commit hook process that cannot acquire ``_rebuild_lock``
+    so its change set is not silently dropped (#1059). The lock-holding
+    process drains this file before and after its rebuild and merges the
+    contents with its own change set.
+
+    Opened in append mode so concurrent writers do not clobber each other on
+    POSIX; each ``write()`` of a small payload is effectively atomic. A
+    trailing newline is always written so partial-line corruption stays
+    confined to the offending entry and is skipped on drain.
+    """
+    if not changed_paths:
+        return
+    out_dir.mkdir(parents=True, exist_ok=True)
+    pending = out_dir / _PENDING_FILENAME
+    payload = "".join(f"{os.fspath(p)}\n" for p in changed_paths)
+    with open(pending, "a", encoding="utf-8") as fh:
+        fh.write(payload)
+
+
+def _drain_pending(out_dir: Path) -> list[Path]:
+    """Read + unlink ``out_dir/.pending_changes`` and return deduplicated paths.
+
+    Returns an empty list if the file does not exist. Empty/whitespace lines
+    are silently skipped so a partial concurrent write that left only a
+    fragment cannot poison the merge.
+    """
+    pending = out_dir / _PENDING_FILENAME
+    if not pending.exists():
+        return []
+    try:
+        raw = pending.read_text(encoding="utf-8")
+    except OSError:
+        return []
+    # Unlink BEFORE returning so a crash between read and process retains the
+    # data in the next caller's view via the lines we are about to return —
+    # i.e. losing the file after reading is fine, losing it before would be a
+    # bug. Use missing_ok to tolerate a racing drain on platforms where
+    # rename/unlink may interleave.
+    with contextlib.suppress(FileNotFoundError):
+        pending.unlink()
+    seen: set[str] = set()
+    out: list[Path] = []
+    for line in raw.splitlines():
+        s = line.strip()
+        if not s or s in seen:
+            continue
+        seen.add(s)
+        out.append(Path(s))
+    return out
+
+
+def _merge_changed_paths(*sources: "list[Path] | None") -> list[Path]:
+    """Concatenate path lists, preserving order and dropping duplicates.
+
+    Used to combine a hook process's own ``changed_paths`` with the drained
+    contents of ``.pending_changes`` so the lock-holding rebuild covers
+    every queued commit's worth of files (#1059).
+    """
+    seen: set[str] = set()
+    out: list[Path] = []
+    for src in sources:
+        if not src:
+            continue
+        for p in src:
+            key = os.fspath(p)
+            if key in seen:
+                continue
+            seen.add(key)
+            out.append(p)
+    return out
 
 
 @contextlib.contextmanager
@@ -320,19 +397,55 @@ def _rebuild_code(
     """
     out = watch_path / _GRAPHIFY_OUT
     if acquire_lock:
+        # #1059: incremental (changed_paths is not None) hooks must not drop
+        # their change set when another rebuild is already running. Queue
+        # before attempting the lock so a non-blocking failure still records
+        # the work; the lock-holder drains the queue and merges it in. Full-
+        # corpus rebuilds skip the queue entirely — they already cover every
+        # file, so there is nothing to merge.
+        if changed_paths is not None and not block_on_lock:
+            _queue_pending(out, list(changed_paths))
         with _rebuild_lock(out, blocking=block_on_lock) as got:
             if not got:
                 print("[graphify watch] Rebuild already in progress for "
-                      f"{watch_path.resolve()} - skipping.")
+                      f"{watch_path.resolve()} - changes queued.")
                 return False
-            return _rebuild_code(
+            # Lock acquired. Drain anything queued by earlier contenders
+            # (including, importantly, the paths we just queued ourselves)
+            # and merge with our own change set so a single rebuild covers
+            # everything outstanding.
+            if changed_paths is not None:
+                merged = _merge_changed_paths(changed_paths, _drain_pending(out))
+            else:
+                # Full-corpus rebuild supersedes any queued incremental work.
+                _drain_pending(out)
+                merged = None
+            ok = _rebuild_code(
                 watch_path,
-                changed_paths=changed_paths,
+                changed_paths=merged,
                 follow_symlinks=follow_symlinks,
                 force=force,
                 no_cluster=no_cluster,
                 acquire_lock=False,
             )
+            # Late-arrival drain: another hook may have queued work while we
+            # were rebuilding. Loop up to _PENDING_DRAIN_MAX_PASSES times so a
+            # storm of commits eventually quiesces without livelocking. A full
+            # rebuild already saw everything, so skip this for changed_paths is None.
+            if merged is not None:
+                for _ in range(_PENDING_DRAIN_MAX_PASSES):
+                    late = _drain_pending(out)
+                    if not late:
+                        break
+                    ok = _rebuild_code(
+                        watch_path,
+                        changed_paths=late,
+                        follow_symlinks=follow_symlinks,
+                        force=force,
+                        no_cluster=no_cluster,
+                        acquire_lock=False,
+                    ) and ok
+            return ok
 
     watch_root = watch_path.resolve()
     project_root = Path.cwd().resolve() if not watch_path.is_absolute() else watch_root

tests/test_claude_cli_backend.py

@@ -115,3 +115,81 @@ def test_no_session_persistence_flag_in_subprocess(fake_claude):
     llm._call_claude_cli("dummy", max_tokens=8192)
     call_args = fake_claude.call_args[0][0]
     assert "--no-session-persistence" in call_args
+
+
+# ---------- Windows path resolution (#1072) ----------
+
+
+def test_windows_prefers_claude_cmd_over_bare_claude(monkeypatch):
+    """On Windows, npm installs `claude.ps1` alongside `claude.cmd`.
+    `CreateProcess` cannot execute `.ps1` directly (raises WinError 2),
+    so we must explicitly resolve `claude.cmd` and pass its full path
+    to subprocess.run. See issue #1072."""
+    completed = MagicMock(returncode=0, stdout=json.dumps(_ENVELOPE), stderr="")
+    monkeypatch.setattr(llm, "_response_is_hollow", lambda raw, parsed: False)
+
+    def fake_which(name):
+        # Simulate Windows PATHEXT=.PS1;.CMD ordering: bare "claude"
+        # resolves to the .ps1 (unexecutable by CreateProcess), while
+        # "claude.cmd" resolves to the .cmd shim.
+        return {
+            "claude": r"C:\Users\u\AppData\Roaming\npm\claude.ps1",
+            "claude.cmd": r"C:\Users\u\AppData\Roaming\npm\claude.cmd",
+        }.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] == r"C:\Users\u\AppData\Roaming\npm\claude.cmd", (
+        f"Expected full path to claude.cmd on Windows, got {argv[0]!r}"
+    )
+
+
+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."""
+    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
+
+    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"
+
+
+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."""
+    with patch("platform.system", return_value="Windows"), \
+         patch("shutil.which", return_value=None):
+        with pytest.raises(RuntimeError, match="Claude Code CLI not found"):
+            llm._call_claude_cli("dummy", max_tokens=8192)
+
+
+def test_non_windows_uses_bare_claude(monkeypatch):
+    """On non-Windows platforms, behaviour is unchanged: bare `claude`
+    is passed to subprocess.run (shell resolves it via PATH)."""
+    completed = MagicMock(returncode=0, stdout=json.dumps(_ENVELOPE), stderr="")
+    monkeypatch.setattr(llm, "_response_is_hollow", lambda raw, parsed: False)
+
+    with patch("platform.system", return_value="Linux"), \
+         patch("shutil.which", return_value="/usr/local/bin/claude"), \
+         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"

tests/test_watch.py

@@ -484,3 +484,213 @@ def test_rebuild_code_prunes_deleted_file_nodes(tmp_path):
         assert "keep.py" in after_sources, "untouched file's nodes should survive"
     finally:
         os.chdir(cwd)
+
+
+# --- #1059: pending-changes queue prevents commit drops under lock contention ---
+
+
+def test_queue_and_drain_pending_round_trip(tmp_path):
+    """_queue_pending writes one path per line; _drain_pending reads + unlinks
+    and returns the same set of paths."""
+    from graphify.watch import _queue_pending, _drain_pending, _PENDING_FILENAME
+
+    out = tmp_path / "graphify-out"
+    paths = [Path("a.py"), Path("sub/b.py"), Path("c.md")]
+    _queue_pending(out, paths)
+
+    pending_file = out / _PENDING_FILENAME
+    assert pending_file.exists()
+    # Each path written on its own line.
+    assert pending_file.read_text(encoding="utf-8").splitlines() == [
+        "a.py", "sub/b.py", "c.md",
+    ]
+
+    drained = _drain_pending(out)
+    assert drained == paths
+    # Drain unlinks so subsequent callers see an empty queue.
+    assert not pending_file.exists()
+    assert _drain_pending(out) == []
+
+
+def test_drain_pending_dedupes_and_skips_blank_lines(tmp_path):
+    """Repeated appends across concurrent contenders must dedupe; partial
+    writes leaving blank lines must not poison the merge."""
+    from graphify.watch import _queue_pending, _drain_pending
+
+    out = tmp_path / "graphify-out"
+    _queue_pending(out, [Path("a.py"), Path("b.py")])
+    _queue_pending(out, [Path("b.py"), Path("c.py")])
+    # Simulate a torn write leaving an empty line.
+    with open(out / ".pending_changes", "a", encoding="utf-8") as fh:
+        fh.write("\n   \n")
+
+    drained = _drain_pending(out)
+    assert drained == [Path("a.py"), Path("b.py"), Path("c.py")]
+
+
+def test_queue_pending_noop_on_empty_list(tmp_path):
+    """Empty change set must not create an empty .pending_changes file."""
+    from graphify.watch import _queue_pending, _PENDING_FILENAME
+
+    out = tmp_path / "graphify-out"
+    _queue_pending(out, [])
+    assert not (out / _PENDING_FILENAME).exists()
+
+
+@pytest.mark.skipif(sys.platform == "win32", reason="fcntl-only (POSIX)")
+def test_rebuild_code_queues_on_lock_contention(tmp_path, monkeypatch, capsys):
+    """#1059: when the rebuild lock is held, an incremental hook must queue
+    its changed_paths to .pending_changes and print 'queued' instead of
+    silently dropping the change set."""
+    from graphify.watch import _rebuild_code, _rebuild_lock, _PENDING_FILENAME
+
+    out = tmp_path / "graphify-out"
+    out.mkdir()
+
+    # Hold the lock so the next non-blocking attempt fails. Use a real
+    # _rebuild_lock context manager in this same process — flock on the same
+    # file descriptor would otherwise be re-entrant on Linux, so we open
+    # the file ourselves via the lock helper.
+    with _rebuild_lock(out, blocking=False) as outer_got:
+        assert outer_got is True
+
+        ok = _rebuild_code(
+            tmp_path,
+            changed_paths=[Path("a.py"), Path("b.py")],
+        )
+        assert ok is False
+
+        # Output should say "queued", not "skipping".
+        captured = capsys.readouterr().out
+        assert "queued" in captured.lower()
+        assert "skipping" not in captured.lower()
+
+        # And the paths must have been written to the pending file so the
+        # eventual lock-holder can drain them.
+        pending = out / _PENDING_FILENAME
+        assert pending.exists()
+        assert pending.read_text(encoding="utf-8").splitlines() == ["a.py", "b.py"]
+
+
+@pytest.mark.skipif(sys.platform == "win32", reason="fcntl-only (POSIX)")
+def test_rebuild_code_merges_pending_on_acquire(tmp_path, monkeypatch):
+    """#1059: the process that acquires the lock must drain .pending_changes
+    and pass the merged change set to the inner rebuild call."""
+    from graphify import watch as watch_mod
+
+    out = tmp_path / "graphify-out"
+    out.mkdir()
+    # Pre-populate the queue as if an earlier contender had dropped its paths.
+    watch_mod._queue_pending(out, [Path("queued1.py"), Path("queued2.py")])
+
+    # Snapshot the original BEFORE monkeypatching so we can drive the outer
+    # dispatch path while the inner recursive call resolves to our spy.
+    orig_rebuild = watch_mod._rebuild_code
+    inner_calls: list[list[str]] = []
+
+    def recording_inner(watch_path, **kwargs):
+        if kwargs.get("acquire_lock") is False:
+            paths = kwargs.get("changed_paths") or []
+            inner_calls.append([p.as_posix() for p in paths])
+        return True
+
+    monkeypatch.setattr(watch_mod, "_rebuild_code", recording_inner)
+
+    ok = orig_rebuild(
+        tmp_path,
+        changed_paths=[Path("own.py"), Path("queued1.py")],
+    )
+    assert ok is True
+
+    # The first inner call must have received the merged + deduped set:
+    # own.py first (caller's order preserved), then drained queued1/queued2,
+    # with queued1.py deduped against own's prior occurrence.
+    assert inner_calls, "inner _rebuild_code should have been called"
+    assert inner_calls[0] == ["own.py", "queued1.py", "queued2.py"]
+
+    # And .pending_changes was drained.
+    assert not (out / watch_mod._PENDING_FILENAME).exists()
+
+
+@pytest.mark.skipif(sys.platform == "win32", reason="fcntl-only (POSIX)")
+def test_rebuild_code_drains_late_arrivals(tmp_path, monkeypatch):
+    """#1059: after the primary rebuild, the lock-holder must loop and drain
+    any paths queued by hooks that arrived mid-rebuild."""
+    from graphify import watch as watch_mod
+    from graphify.watch import _rebuild_code as orig_rebuild
+
+    out = tmp_path / "graphify-out"
+    out.mkdir()
+
+    inner_calls: list[list[str]] = []
+    call_state = {"i": 0}
+
+    def fake_inner(watch_path, **kwargs):
+        if kwargs.get("acquire_lock") is False:
+            paths = [p.as_posix() for p in (kwargs.get("changed_paths") or [])]
+            inner_calls.append(paths)
+            # Simulate a late-arriving hook that queues during the FIRST
+            # inner rebuild only. The outer drain loop must see it.
+            call_state["i"] += 1
+            if call_state["i"] == 1:
+                watch_mod._queue_pending(out, [Path("late.py")])
+        return True
+
+    monkeypatch.setattr(watch_mod, "_rebuild_code", fake_inner)
+
+    ok = orig_rebuild(tmp_path, changed_paths=[Path("own.py")])
+    assert ok is True
+
+    # First inner call covers our own change set; second is the late-drain
+    # pass that picks up "late.py".
+    assert len(inner_calls) >= 2
+    assert inner_calls[0] == ["own.py"]
+    assert inner_calls[1] == ["late.py"]
+    # And the queue is now empty (no further late drains).
+    assert not (out / watch_mod._PENDING_FILENAME).exists()
+
+
+def test_rebuild_code_full_corpus_skips_pending_queue(tmp_path, monkeypatch):
+    """#1059: changed_paths=None means a full-corpus rebuild — the queue
+    must not be touched on the failure path because there is nothing
+    incremental to preserve."""
+    from graphify import watch as watch_mod
+    from graphify.watch import _rebuild_code as orig_rebuild
+
+    out = tmp_path / "graphify-out"
+    out.mkdir()
+
+    # Pre-existing queued paths from an earlier incremental hook.
+    watch_mod._queue_pending(out, [Path("earlier.py")])
+
+    # Force the inner call to record what it saw.
+    seen: list = []
+
+    def fake_inner(watch_path, **kwargs):
+        if kwargs.get("acquire_lock") is False:
+            seen.append(kwargs.get("changed_paths"))
+        return True
+
+    monkeypatch.setattr(watch_mod, "_rebuild_code", fake_inner)
+
+    ok = orig_rebuild(tmp_path, changed_paths=None)
+    assert ok is True
+    # Full-corpus rebuild passes None to the inner call (does not merge in
+    # the queued paths — a full rebuild already covers them).
+    assert seen == [None]
+    # The queue still gets drained on entry so stale entries don't leak,
+    # but no late-arrival loop runs for the full-corpus path.
+    assert not (out / watch_mod._PENDING_FILENAME).exists()
+
+
+def test_merge_changed_paths_dedupes_in_order():
+    """_merge_changed_paths preserves first-seen order and drops dupes."""
+    from graphify.watch import _merge_changed_paths
+
+    merged = _merge_changed_paths(
+        [Path("a.py"), Path("b.py")],
+        None,
+        [Path("b.py"), Path("c.py")],
+        [Path("a.py")],
+    )
+    assert [p.as_posix() for p in merged] == ["a.py", "b.py", "c.py"]

uv.lock

@@ -1109,7 +1109,7 @@ wheels = [
 
 [[package]]
 name = "graphifyy"
-version = "0.8.23"
+version = "0.8.24"
 source = { editable = "." }
 dependencies = [
     { name = "datasketch" },