Summary
The fix for #5425 — the sg-inflight-<sha> markers in hooks/diffstate.py — relies on fcntl.flock. On native Windows Python, import fcntl fails, so _mark_inflight() returns without creating a marker and _load_inflight_shas() always returns an empty set. A push made while the commit's review is still running sees that commit as neither reviewed nor in flight, and reviews it again from scratch.
The code comment already notes this ("Without flock (native Windows Python, some network filesystems) there are no markers and nothing changes."), so it is a known gap rather than a regression. I'm filing it so Windows has a tracked path to the fix, with measurements and some Windows-specific notes below.
Environment
- security-guidance 2.0.16 (claude-plugins-official)
- Windows 11, native CPython 3.12.13 (no WSL)
- Agentic commit review enabled
Evidence
~/.claude/security/log.txt (repo path and sha redacted):
16:58:08.542 Commit review: detected git commit in command
16:58:08.598 Commit review: 1/1 sha(s) resolved, 16 files <- review #1 starts
16:58:10.876 Push sweep: repo resolved via 2 -> '<repo>'
16:58:10.931 Push sweep: range=1 prefix_advanced=0 base=<sha> tail=1 <- review #2 of the same commit
16:58:41.818 Commit review: no security issues found
16:58:43.746 Push sweep: no new findings- In a two-hour log window, 3 of the 5 push reviews that ran overlapped a still-running review of the same commit like this (the push came 2–10 s after the commit).
- Over five days and ~850 agentic reviews on this machine, reviews that started while another review was running in the same checkout accounted for up to 12% of review cost. This is an upper bound, because it also counts unrelated commits that legitimately overlap. Measured with a wrapper set as
SG_AGENTIC_CLI_PATHthat logs each run's start time, duration, cwd andtotal_cost_usd. - When the commit review has already finished, the push is skipped correctly (
Push sweep: every pushed commit already reviewed). Thesg-reviewed-shasrecord works on Windows through the plain-append fallback; only the in-flight path is missing.
Notes for a Windows implementation
All checked with the plugin's own venv interpreter (CPython 3.12.13, win32):
import fcntlraisesModuleNotFoundError.msvcrt.locking(fd, msvcrt.LK_NBLCK, 1)gives a non-blocking exclusive lock: a second fd on the same file getsPermissionErroruntil the first fd is closed. It has no shared mode, so_load_inflight_shas()would need to probe with an exclusive lock and release it right away, or useLockFileExvia ctypes for a real shared lock.- A file opened with
os.open()cannot be unlinked while it is open (PermissionError, WinError 32); afteros.close()it can._release_inflight()unlinks before closing, so on Windows the unlink fails and the marker stays until the 3600 s GC. The order needs to be close, then unlink. For the same reason, the "locked an already-unlinked file" case that the inode re-check in_mark_inflight()guards against cannot occur on Windows. - Alternatively, the lock-free
O_CREAT | O_EXCL+ TTL sentinel suggested in the #5425 discussion works the same on every platform.
Expected
A push made while a commit review is in flight waits for that review, as it does on macOS and Linux, and skips the commit once the commit is recorded in sg-reviewed-shas.
Activity
0No activity yet.