From 4decd2e3b26f6c17e458fcdcf2dd60aef8566f3e Mon Sep 17 00:00:00 2001 From: Mohamed Hegazy Date: Wed, 27 May 2026 14:50:51 -0700 Subject: [PATCH] Enable agentic commit reviewer on Windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The agentic reviewer is silently no-op on Windows today. SessionStart bootstrap (ensure_agent_sdk.py) short-circuits with SKIP_WIN32 because the consumer glob in llm.py only matches POSIX venv layout (lib/pythonX.Y/site-packages). On Windows, venvs use Lib/site-packages (capital L, no pythonX.Y subdir), so even if a venv got built the glob wouldn't find its contents. Result: Windows users on default installs (no system-wide claude_agent_sdk) get layer 1 (pattern warnings) and layer 2 (single- shot LLM diff review) but not layer 3 — the cross-file agentic review that catches IDOR, auth-bypass, cross-file SSRF, and other things that need to read related files. Plugin description claims layer 3 but it silently doesn't run. Three changes: 1. llm.py — extend the consumer glob (2 sites: 3P SDK fallback at ~L297, agentic_review fallback at ~L1090) to also match the Windows Lib/site-packages layout, so a venv built on Windows is actually discoverable. 2. ensure_agent_sdk.py — remove the sys.platform == 'win32' early-exit so the SessionStart bootstrap builds the venv on Windows too. Outcome code 4 (formerly SKIP_WIN32) is retired but not reused so pre-fix telemetry rows still decode correctly. 3. ensure_agent_sdk.py — venv_py path now branches on sys.platform: Windows venvs put the interpreter at Scripts\python.exe; POSIX uses bin/python. Previously assumed POSIX, so even with the glob fix, the post-build SDK-importability probe would fail on Windows. Verified locally on macOS: - glob test: both layouts now match (POSIX venv detected, simulated Windows venv also detected via the new Lib/site-packages branch) - both files pass py_compile - POSIX path unchanged (sys.platform != 'win32' so old branch runs) Not verified on Windows in this commit — needs an actual Windows runner to confirm the venv build + SDK import + subprocess plumbing all work end-to-end. The SDK spawns a child claude.exe; Windows process plumbing has its own quirks (shell semantics, path escaping) that may surface separately. Worth a controlled rollout (one-week soak under env-var opt-in before flipping default). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../hooks/ensure_agent_sdk.py | 17 ++++++++--------- plugins/security-guidance/hooks/llm.py | 18 ++++++++++++------ 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/plugins/security-guidance/hooks/ensure_agent_sdk.py b/plugins/security-guidance/hooks/ensure_agent_sdk.py index 4e34f176..697b5cce 100644 --- a/plugins/security-guidance/hooks/ensure_agent_sdk.py +++ b/plugins/security-guidance/hooks/ensure_agent_sdk.py @@ -28,7 +28,9 @@ NOOP_SYSTEM = 0 # claude_agent_sdk already importable in system python NOOP_VENV = 1 # venv already built and SDK imports from it BUILT = 2 # venv created + SDK pip-installed this run BUILD_FAILED = 3 # venv create or pip install raised/timed out -SKIP_WIN32 = 4 # Windows; consumer glob doesn't handle Lib/ layout +# Outcome 4 was previously SKIP_WIN32; retired now that the consumer glob in +# llm.py also matches Windows venv layout (Lib/site-packages). Don't reuse the +# value — telemetry rows from older plugin builds still emit 4. SKIP_SENTINEL = 5 # another SessionStart is currently building @@ -60,13 +62,6 @@ def main() -> tuple[int, str, str]: err_phase / err_kind are non-empty only on BUILD_FAILED — they let telemetry split bootstrap failures by root cause. """ - # Windows venv layout (Lib/site-packages, no python* subdir) isn't - # handled by the consumer's glob in security_reminder_hook.py; skip the - # bootstrap entirely rather than build a venv that's never read. - if sys.platform == "win32": - return SKIP_WIN32, "", "" - - if _sdk_on_syspath(): return NOOP_SYSTEM, "", "" @@ -75,7 +70,11 @@ def main() -> tuple[int, str, str]: or os.path.expanduser("~/.claude/security") ) venv = state_dir / "agent-sdk-venv" - venv_py = venv / "bin" / "python" + # Windows venvs put the interpreter at Scripts\python.exe; POSIX uses bin/python. + if sys.platform == "win32": + venv_py = venv / "Scripts" / "python.exe" + else: + venv_py = venv / "bin" / "python" # Another SessionStart (concurrent CC instance, same plugin) may already # be building. The sentinel lives NEXT TO the venv, not inside it — diff --git a/plugins/security-guidance/hooks/llm.py b/plugins/security-guidance/hooks/llm.py index eff56de7..11ef6ce9 100644 --- a/plugins/security-guidance/hooks/llm.py +++ b/plugins/security-guidance/hooks/llm.py @@ -298,9 +298,12 @@ def _call_claude_via_sdk(prompt, output_schema, *, max_tokens=16000, model=None) "SECURITY_WARNINGS_STATE_DIR", os.path.expanduser("~/.claude/security"), ) - for _sp in glob.glob( - os.path.join(_state_dir, "agent-sdk-venv", "lib", - "python*", "site-packages") + # POSIX venv: lib/pythonX.Y/site-packages + # Windows venv: Lib/site-packages (capital L, no pythonX.Y subdir) + _venv_root = os.path.join(_state_dir, "agent-sdk-venv") + for _sp in ( + glob.glob(os.path.join(_venv_root, "lib", "python*", "site-packages")) + + glob.glob(os.path.join(_venv_root, "Lib", "site-packages")) ): if os.path.isdir(_sp) and _sp not in sys.path: sys.path.insert(0, _sp) @@ -1094,9 +1097,12 @@ def agentic_review( "SECURITY_WARNINGS_STATE_DIR", os.path.expanduser("~/.claude/security"), ) - for _sp in glob.glob( - os.path.join(_state_dir, "agent-sdk-venv", "lib", - "python*", "site-packages") + # POSIX venv: lib/pythonX.Y/site-packages + # Windows venv: Lib/site-packages (capital L, no pythonX.Y subdir) + _venv_root = os.path.join(_state_dir, "agent-sdk-venv") + for _sp in ( + glob.glob(os.path.join(_venv_root, "lib", "python*", "site-packages")) + + glob.glob(os.path.join(_venv_root, "Lib", "site-packages")) ): if os.path.isdir(_sp) and _sp not in sys.path: sys.path.insert(0, _sp)