Skip to content

fix(reconcile): give the prune lock a Windows backend - #52

Merged
ProtocolWarden merged 1 commit into
mainfrom
claude/windows-reconcile-lock
Aug 3, 2026
Merged

fix(reconcile): give the prune lock a Windows backend#52
ProtocolWarden merged 1 commit into
mainfrom
claude/windows-reconcile-lock

Conversation

@ProtocolWarden

Copy link
Copy Markdown
Owner

reconcile_lock raised RuntimeError on any host without fcntl, so cl reconcile prune --apply was unrunnable on Windows:

RuntimeError: reconcile prune --apply requires POSIX fcntl file locking,
              unavailable on this platform

The whole reconciliation workflow dead-ended there — which is likely why consumer .console/log.md files kept getting hand-pruned instead of reconciled.

The fix

msvcrt.locking(LK_NBLCK) is the direct analogue of flock(LOCK_EX|LOCK_NB): non-blocking, exclusive, released on process death, and it conflicts with a second handle opened by the same process — so the existing nested-acquire test still holds unchanged.

One difference shapes the file layout. flock is advisory and whole-file, so any reader can read the pid. A msvcrt byte range is mandatory: a reader touching a locked byte gets PermissionError. So the lock claims a sentinel byte at offset 1024 while the pid field stays at offset 0, where a contending run can still read it to name the holder. The pid is written as a fixed-width field rather than truncate+write, because truncating would cross the locked range. Both invariants have their own test.

Contention is now matched on errno rather than exception type (flock reports EWOULDBLOCK, msvcrt reports EACCES/EDEADLOCK). An OSError outside that set is re-raised unchanged — a bad fd must never be reported as "someone else holds it", which a broad except OSError would have got wrong.

It fixes 9 pre-existing failures, not just the CLI

Every one was the same RuntimeError:

tests/test_reconcile_cli.py::test_prune_apply_exits_3_when_locked
tests/test_reconcile_lock.py::test_apply_refused_while_lock_held
tests/test_reconcile_lock.py::test_lock_is_exclusive_and_reentrant_after_release
tests/test_reconcile_lock.py::test_lock_released_after_apply
tests/test_reconcile_prune.py::test_ac9_apply_moves_and_is_idempotent
tests/test_reconcile_prune.py::test_ac10_post_prune_check_green_and_clean
tests/test_reconcile_prune.py::test_private_manifest_root_prune_keeps_names
tests/test_reconcile_prune.py::test_private_repo_prune_keeps_own_name
tests/test_reconcile_prune.py::test_public_repo_prune_still_scrubs

Suite: 25 failures → 16, 457 → 473 passed (9 fixed + 7 new). The remaining 16 are a strict subset of the previous set — diffed by name, not counted, so no failure was swapped for another.

New tests (7)

A backend exists on this platform; the pid stays readable while the lock is held (fails if the sentinel byte ever moves onto the pid field); the layout invariant; a stale longer pid cannot leave a tail; cross-process exclusion; reacquire after the holder is killed; and an unexpected OSError not being laundered into PruneLockHeld.

The cross-process test has the child print its own os.getpid() rather than trusting Popen.pid — a venv's python.exe can be a launcher shim, so the process that takes the lock isn't always the one Popen returns. That cost a false failure before it was understood.

Verification

  • ruff clean on both changed files (repo-wide ruff is red on main at 204 findings — unrelated, unchanged)
  • Custodian audit: 0 findings
  • Verified the worktree source was the one under test, since the venv installs CL against the main checkout

🤖 Generated with Claude Code

@ProtocolWarden
ProtocolWarden force-pushed the claude/windows-reconcile-lock branch from 5138b0e to 6976b73 Compare August 3, 2026 19:38
`reconcile_lock` raised RuntimeError on any host without `fcntl`, so
`prune --apply` was unrunnable on Windows. The reconciliation workflow
dead-ended there, which is why consumer logs kept getting hand-pruned instead.

`msvcrt.locking(LK_NBLCK)` is the direct analogue: non-blocking, exclusive,
released on process death, and it conflicts with a second handle opened by the
same process, so the existing nested-acquire test still holds.

One difference shapes the file layout. `flock` is advisory and whole-file, but a
`msvcrt` byte range is mandatory: a reader touching a locked byte gets
PermissionError. The lock therefore claims a sentinel byte at offset 1024 while
the pid stays at offset 0, where a contending run can still read it to name the
holder. The pid is written as a fixed-width field rather than truncate+write,
because truncation would cross the locked range. Both invariants have a test.

Contention is matched on errno rather than exception type (`flock` reports
EWOULDBLOCK, `msvcrt` reports EACCES and EDEADLOCK). An OSError outside that set
is re-raised unchanged -- a bad fd must never be reported as "someone else holds
it", which the previous BlockingIOError-only catch got right by accident and a
broad `except OSError` would have got wrong.

Not merely an unblock: this fixes 9 pre-existing Windows test failures, all the
same RuntimeError -- four lock tests and five prune tests. Suite goes 25 failures
-> 16, and the remaining 16 are a strict subset of the previous set (diffed by
name, not counted). 457 -> 473 passed = 9 fixed + 7 new.

7 new tests: a backend exists on this platform; the pid stays readable while the
lock is held (fails if the sentinel byte ever moves onto the pid field); the
layout invariant; a stale longer pid cannot leave a tail; cross-process
exclusion; reacquire after the holder is killed; and an unexpected OSError not
being laundered into PruneLockHeld.

The cross-process test has the child print its own `os.getpid()` rather than
trusting `Popen.pid` -- a venv's python.exe can be a launcher shim, so the
process that takes the lock is not always the one Popen returns.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@ProtocolWarden
ProtocolWarden force-pushed the claude/windows-reconcile-lock branch from 6976b73 to bc2062a Compare August 3, 2026 19:40
@ProtocolWarden
ProtocolWarden merged commit 8d2c115 into main Aug 3, 2026
5 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant