Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions .console/log.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,27 @@
# Log
## 2026-08-03 — fix(reconcile): give the prune lock a Windows backend

`reconcile_lock` raised `RuntimeError` without `fcntl`, so `prune --apply` was
unrunnable on Windows — which is why consumer logs got hand-pruned instead.

`msvcrt.locking(LK_NBLCK)` is the direct analogue: non-blocking, exclusive,
released on process death, conflicts with a second handle in the same process.
One difference shapes the layout — `flock` is advisory and whole-file, but a
`msvcrt` range is mandatory, so a reader touching a locked byte gets
PermissionError. The lock claims a sentinel byte at offset 1024 while the pid
stays at 0, readable by a contending run that wants to name the holder; the pid
is a fixed-width field because truncating would cross the locked range. Both
invariants have a test. Contention now matches on errno (`flock` gives
EWOULDBLOCK, `msvcrt` EACCES/EDEADLOCK); anything else re-raises as itself, so a
bad fd is never reported as "someone else holds it".

Not just an unblock: 9 pre-existing Windows failures were all this same
RuntimeError — four lock tests, five prune tests. Suite 25 failures -> 16, the
16 a strict subset of the old set (diffed by name). 457 -> 473 = 9 fixed + 7 new.

The cross-process test has the child print its own `os.getpid()`: a venv's
python.exe can be a shim, so `Popen.pid` is not always the lock holder.

## 2026-08-03 — fix(cli): stop console encoding from failing a command that succeeded

`cl reconcile check` computed a GREEN verdict, then died printing it:
Expand Down
109 changes: 90 additions & 19 deletions src/context_lifecycle/reconcile/lock.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,34 +4,107 @@

Two prunes applying concurrently against the same repo can interleave the
archive append and the source trim (same-section archived twice; the
idempotency-by-heading guard races its own read). An exclusive ``flock`` on
idempotency-by-heading guard races its own read). An exclusive lock on
``.console/.reconcile.lock`` serializes appliers **on one host**; cross-host
runs are already serialized by git (append-only archive + idempotent re-run,
merged with ordinary rebase) — the lock closes the same-host window where no
merge point exists.

The lock file is untracked working state (like the worksheet) and is left in
place after release; holding pid is recorded for diagnostics only.

Platform backends
─────────────────
POSIX uses ``fcntl.flock``; Windows uses ``msvcrt.locking``. Both are
non-blocking, both release automatically if the holder dies, and both conflict
with a second handle opened by the *same* process — so a nested acquire raises
rather than silently succeeding.

They differ in one way that shapes the file layout. ``flock`` is advisory and
whole-file, so any reader can still read the pid. ``msvcrt.locking`` locks a
byte *range* and Windows enforces it: a reader touching a locked byte gets
``PermissionError``. The lock therefore claims a single sentinel byte at
``_LOCK_OFFSET``, clear of the pid field at offset 0, so a contending run can
still read and name the holder. Writing the pid as a fixed-width field avoids
truncating the file, which would otherwise cross the locked range.

Before 2026-08-03 this module raised on any non-POSIX host, so
``prune --apply`` was unrunnable on Windows — the reconciliation workflow
dead-ended there and logs got hand-pruned instead.
"""

from __future__ import annotations

try:
import fcntl
except ImportError: # POSIX-only. On Windows only `reconcile prune --apply` needs
fcntl = None # type: ignore[assignment] # it; importing this module must still work.
import errno
import os
from contextlib import contextmanager
from collections.abc import Iterator
from contextlib import contextmanager
from pathlib import Path

try:
import fcntl
except ImportError: # Windows — msvcrt below is the backend there.
fcntl = None # type: ignore[assignment]
try:
import msvcrt
except ImportError: # POSIX — fcntl above is the backend there.
msvcrt = None # type: ignore[assignment]

LOCK_RELPATH = Path(".console") / ".reconcile.lock"

# Pid is written as a fixed-width field at offset 0, so a shorter pid cannot
# leave a longer predecessor's tail behind and no truncation is needed.
_PID_FIELD = 32
# The byte msvcrt actually locks. Must sit clear of the pid field: on Windows a
# locked range is mandatory, and a contending run reads the pid to name the
# holder. Unused by the POSIX backend, whose flock covers the whole file.
_LOCK_OFFSET = 1024

# Errnos meaning "someone else holds it" rather than "the call was wrong".
# POSIX flock reports EWOULDBLOCK/EAGAIN; msvcrt reports EACCES, and EDEADLOCK
# once its internal retries are exhausted.
_CONTENDED = frozenset(
e for e in (
getattr(errno, "EACCES", None),
getattr(errno, "EAGAIN", None),
getattr(errno, "EWOULDBLOCK", None),
getattr(errno, "EDEADLOCK", None),
getattr(errno, "EDEADLK", None),
)
if e is not None
)


class PruneLockHeld(RuntimeError):
"""Raised when another prune --apply holds the repo's reconcile lock."""


def _acquire(fd: int) -> None:
"""Take the exclusive lock without blocking. Raises OSError if contended."""
if fcntl is not None:
fcntl.flock(fd, fcntl.LOCK_EX | fcntl.LOCK_NB)
return
os.lseek(fd, _LOCK_OFFSET, os.SEEK_SET)
msvcrt.locking(fd, msvcrt.LK_NBLCK, 1)


def _release(fd: int) -> None:
if fcntl is not None:
fcntl.flock(fd, fcntl.LOCK_UN)
return
os.lseek(fd, _LOCK_OFFSET, os.SEEK_SET)
msvcrt.locking(fd, msvcrt.LK_UNLCK, 1)


def _read_holder(fd: int) -> str:
"""Best-effort pid of the current holder, for the error message only."""
try:
os.lseek(fd, 0, os.SEEK_SET)
return os.read(fd, _PID_FIELD).decode("utf-8", "replace").strip()
except OSError:
return ""


@contextmanager
def reconcile_lock(repo_root: Path) -> Iterator[None]:
"""Hold the exclusive per-repo reconcile lock for the duration of the block.
Expand All @@ -41,33 +114,31 @@ def reconcile_lock(repo_root: Path) -> Iterator[None]:
than queue behind it (the second run is a no-op anyway once the first
lands).
"""
if fcntl is None:
if fcntl is None and msvcrt is None: # pragma: no cover - neither backend
raise RuntimeError(
"reconcile prune --apply requires POSIX fcntl file locking, "
"unavailable on this platform"
"reconcile prune --apply requires file locking (fcntl or msvcrt), "
"and neither is available on this platform"
)
lock_path = Path(repo_root) / LOCK_RELPATH
lock_path.parent.mkdir(parents=True, exist_ok=True)
fd = os.open(lock_path, os.O_RDWR | os.O_CREAT, 0o644)
try:
try:
fcntl.flock(fd, fcntl.LOCK_EX | fcntl.LOCK_NB)
except BlockingIOError:
holder = ""
try:
holder = os.read(fd, 64).decode("utf-8", "replace").strip()
except OSError:
pass
_acquire(fd)
except OSError as exc:
if exc.errno not in _CONTENDED:
raise # a real error (bad fd, I/O) — never report it as contention
holder = _read_holder(fd)
detail = f" (held by pid {holder})" if holder else ""
raise PruneLockHeld(
f"another prune --apply is running against this repo{detail}; "
f"lock: {lock_path}"
) from None
os.ftruncate(fd, 0)
os.write(fd, str(os.getpid()).encode("utf-8"))
os.lseek(fd, 0, os.SEEK_SET)
os.write(fd, str(os.getpid()).ljust(_PID_FIELD).encode("utf-8"))
try:
yield
finally:
fcntl.flock(fd, fcntl.LOCK_UN)
_release(fd)
finally:
os.close(fd)
127 changes: 121 additions & 6 deletions tests/test_reconcile_lock.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,16 @@

from __future__ import annotations

import errno
import os
import subprocess
import sys
from hashlib import sha256
from pathlib import Path

import pytest

from context_lifecycle.reconcile import lock as lock_mod
from context_lifecycle.reconcile.lock import (
LOCK_RELPATH,
PruneLockHeld,
Expand Down Expand Up @@ -56,9 +61,8 @@ def test_lock_is_exclusive_and_reentrant_after_release(tmp_path):
entered = 0
with reconcile_lock(repo):
entered += 1
with pytest.raises(PruneLockHeld):
with reconcile_lock(repo):
pytest.fail("second acquire must not enter the block")
with pytest.raises(PruneLockHeld), reconcile_lock(repo):
pytest.fail("second acquire must not enter the block")
with reconcile_lock(repo): # released → reacquirable
entered += 1
assert entered == 2
Expand All @@ -74,9 +78,8 @@ def test_apply_refused_while_lock_held(tmp_path, monkeypatch):
plan = build_plan(repo, private_root=private)

log_before = _checksum(repo / ".console" / "log.md")
with reconcile_lock(repo): # another applier
with pytest.raises(PruneLockHeld):
apply_plan(repo, plan)
with reconcile_lock(repo), pytest.raises(PruneLockHeld): # another applier
apply_plan(repo, plan)
# Refused apply mutated nothing.
assert _checksum(repo / ".console" / "log.md") == log_before
assert not private.exists()
Expand All @@ -96,3 +99,115 @@ def test_lock_released_after_apply(tmp_path, monkeypatch):
with reconcile_lock(repo): # would raise PruneLockHeld if apply leaked it
reacquired = True
assert reacquired


# ── cross-platform backend ───────────────────────────────────────────────────
# Until 2026-08-03 this module raised on any non-POSIX host, so `prune --apply`
# was unrunnable on Windows. These pin that both backends actually lock, rather
# than that the module merely imports.


def test_a_backend_is_available_on_this_platform():
"""No supported platform may fall through to the 'neither backend' error."""
assert (lock_mod.fcntl is not None) or (lock_mod.msvcrt is not None)


def test_holder_pid_is_recorded_and_readable_while_held(tmp_path):
"""The pid field must stay readable while the lock is held.

On Windows the locked range is mandatory, so this fails if the sentinel byte
is ever moved on top of the pid field — a contending run could then not name
the holder.
"""
(tmp_path / ".console").mkdir()
with reconcile_lock(tmp_path):
raw = (tmp_path / LOCK_RELPATH).read_bytes()
assert raw[:lock_mod._PID_FIELD].decode().strip() == str(os.getpid())


def test_pid_field_and_lock_byte_do_not_overlap():
"""Layout invariant the Windows backend depends on."""
assert lock_mod._LOCK_OFFSET >= lock_mod._PID_FIELD


def test_shorter_pid_cannot_leave_a_longer_predecessors_tail(tmp_path):
"""Fixed-width field instead of truncate — truncation would cross the lock."""
(tmp_path / ".console").mkdir()
lock_file = tmp_path / LOCK_RELPATH
lock_file.parent.mkdir(parents=True, exist_ok=True)
lock_file.write_bytes(b"9" * lock_mod._PID_FIELD) # a long stale pid
with reconcile_lock(tmp_path):
pass
recorded = lock_file.read_bytes()[: lock_mod._PID_FIELD].decode().strip()
assert recorded == str(os.getpid())
assert "9" * 8 not in recorded


# The child reports its OWN pid rather than the test 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.
_CHILD = """
import os, sys, time
from pathlib import Path
sys.path.insert(0, {src!r})
from context_lifecycle.reconcile.lock import reconcile_lock
with reconcile_lock(Path({repo!r})):
print("HELD", os.getpid(), flush=True)
time.sleep(float({hold!r}))
"""


def test_lock_excludes_a_separate_process(tmp_path):
"""The real contract: another *process* is refused while the lock is held.

The same-process test above passes on any backend that tracks handles; only
a second process proves the OS is enforcing it.
"""
(tmp_path / ".console").mkdir()
src = str(Path(lock_mod.__file__).parents[3])
child = subprocess.Popen(
[sys.executable, "-c", _CHILD.format(src=src, repo=str(tmp_path), hold="5")],
stdout=subprocess.PIPE, text=True,
)
try:
held, _, child_pid = child.stdout.readline().strip().partition(" ")
assert held == "HELD", "child never acquired the lock"
with pytest.raises(PruneLockHeld) as exc, reconcile_lock(tmp_path):
pytest.fail("acquired a lock another process holds")
# The holder's pid is reported, which is the whole point of the layout.
assert child_pid and child_pid in str(exc.value)
assert str(os.getpid()) != child_pid, "child must be a separate process"
finally:
child.kill()
child.wait()


def test_lock_is_reacquirable_after_the_holding_process_dies(tmp_path):
"""Both backends release on process death — no stale lock survives a crash."""
(tmp_path / ".console").mkdir()
src = str(Path(lock_mod.__file__).parents[3])
child = subprocess.Popen(
[sys.executable, "-c", _CHILD.format(src=src, repo=str(tmp_path), hold="30")],
stdout=subprocess.PIPE, text=True,
)
assert child.stdout.readline().split()[0] == "HELD"
child.kill()
child.wait()
acquired = False
with reconcile_lock(tmp_path): # stale lock would raise here
acquired = True
assert acquired


def test_unexpected_oserror_is_not_reported_as_contention(tmp_path, monkeypatch):
"""A real failure must surface as itself, not as 'someone else holds it'."""
(tmp_path / ".console").mkdir()

def _boom(fd):
raise OSError(errno.EIO, "disk fell over")

monkeypatch.setattr(lock_mod, "_acquire", _boom)
with pytest.raises(OSError) as exc, reconcile_lock(tmp_path):
pytest.fail("must not enter the block")
assert not isinstance(exc.value, PruneLockHeld)
assert exc.value.errno == errno.EIO
Loading