diff --git a/CHANGELOG.md b/CHANGELOG.md index 20a9d233..b5d92b12 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ Full release notes with details on each version: [GitHub Releases](https://github.com/safishamsi/graphify/releases) +## Unreleased + +- Fix: `graphify-out/.rebuild.lock` no longer accumulates concatenated PIDs across post-commit rebuilds — the lock file now contains a single owning PID followed by a newline while the rebuild runs, and is unlinked when it completes so downstream tooling that polls for its absence unblocks promptly (#858) + ## 0.7.18 (2026-05-14) - Fix: `graphify update` is now idempotent — graph.json and GRAPH_REPORT.md are only rewritten when content actually changes; topology comparison short-circuits clustering entirely on unchanged graphs, eliminating residual community-count drift (#824) diff --git a/graphify/watch.py b/graphify/watch.py index c0173424..748f93ac 100644 --- a/graphify/watch.py +++ b/graphify/watch.py @@ -19,6 +19,11 @@ def _rebuild_lock(out_dir: Path, *, blocking: bool = False): ``blocking`` is False. Uses fcntl.flock so the lock is released automatically if the process is killed (no stale-lock cleanup needed). + While the lock is held, ``.rebuild.lock`` contains the owning PID followed + by a newline so external pollers (publish scripts, etc.) can read it. + On successful release the file is unlinked so downstream tooling that + waits for the lock to clear by polling for its absence unblocks promptly. + Falls back to a no-op yield(True) on platforms without fcntl (Windows). """ try: @@ -29,7 +34,11 @@ def _rebuild_lock(out_dir: Path, *, blocking: bool = False): out_dir.mkdir(parents=True, exist_ok=True) lock_path = out_dir / ".rebuild.lock" - fh = open(lock_path, "a", encoding="utf-8") + # "a+" creates the file if missing without truncating an existing holder's + # PID payload — important because another process may have already written + # its PID before we attempt the flock. + fh = open(lock_path, "a+", encoding="utf-8") + acquired = False try: flags = fcntl.LOCK_EX if blocking else (fcntl.LOCK_EX | fcntl.LOCK_NB) try: @@ -37,13 +46,29 @@ def _rebuild_lock(out_dir: Path, *, blocking: bool = False): except BlockingIOError: yield False return - yield True - finally: + acquired = True + # Replace any prior owner's PID with ours so external readers see a + # single parseable line, not a digit-concatenation across rebuilds. try: - fcntl.flock(fh.fileno(), fcntl.LOCK_UN) + fh.seek(0) + fh.truncate() + fh.write(f"{os.getpid()}\n") + fh.flush() except OSError: pass + yield True + finally: + if acquired: + try: + fcntl.flock(fh.fileno(), fcntl.LOCK_UN) + except OSError: + pass fh.close() + # Signal "rebuild done" by removing the lock file. Only the holder + # unlinks; a non-acquiring caller leaves the existing lock in place. + if acquired: + with contextlib.suppress(OSError): + lock_path.unlink() def _apply_resource_limits() -> None: diff --git a/tests/test_watch.py b/tests/test_watch.py index 5cacb018..72a7f88a 100644 --- a/tests/test_watch.py +++ b/tests/test_watch.py @@ -1,9 +1,11 @@ """Tests for watch.py - file watcher helpers (no watchdog required).""" +import os +import sys import time from pathlib import Path import pytest -from graphify.watch import _notify_only, _WATCHED_EXTENSIONS +from graphify.watch import _notify_only, _WATCHED_EXTENSIONS, _rebuild_lock # --- _notify_only --- @@ -96,6 +98,60 @@ def test_watch_raises_without_watchdog(tmp_path, monkeypatch): watch(tmp_path) +# --- _rebuild_lock (GH-858) --- + + +@pytest.mark.skipif(sys.platform == "win32", reason="fcntl-only (POSIX)") +def test_rebuild_lock_writes_pid_with_newline(tmp_path): + out = tmp_path / "graphify-out" + lock_path = out / ".rebuild.lock" + with _rebuild_lock(out) as got: + assert got is True + assert lock_path.exists() + contents = lock_path.read_text(encoding="utf-8") + assert contents == f"{os.getpid()}\n", contents + + +@pytest.mark.skipif(sys.platform == "win32", reason="fcntl-only (POSIX)") +def test_rebuild_lock_removed_after_release(tmp_path): + """GH-858: lock file must be unlinked once the rebuild completes so + downstream waiters that poll for its absence unblock promptly.""" + out = tmp_path / "graphify-out" + lock_path = out / ".rebuild.lock" + with _rebuild_lock(out) as got: + assert got is True + assert not lock_path.exists(), "lock file should be unlinked after release" + + +@pytest.mark.skipif(sys.platform == "win32", reason="fcntl-only (POSIX)") +def test_rebuild_lock_does_not_accumulate_pids_across_runs(tmp_path): + """GH-858: each acquisition truncates and rewrites the PID line rather + than appending, so the file never grows into a digit-concatenation.""" + out = tmp_path / "graphify-out" + lock_path = out / ".rebuild.lock" + expected = f"{os.getpid()}\n" + for _ in range(5): + with _rebuild_lock(out) as got: + assert got is True + assert lock_path.read_text(encoding="utf-8") == expected + assert not lock_path.exists() + + +@pytest.mark.skipif(sys.platform == "win32", reason="fcntl-only (POSIX)") +def test_rebuild_lock_non_blocking_does_not_clobber_holder(tmp_path): + """GH-858: a non-blocking caller that fails to acquire the lock must not + truncate the holder's PID payload.""" + out = tmp_path / "graphify-out" + lock_path = out / ".rebuild.lock" + with _rebuild_lock(out) as outer: + assert outer is True + held_contents = lock_path.read_text(encoding="utf-8") + with _rebuild_lock(out, blocking=False) as inner: + assert inner is False + # Holder's PID line must still be intact. + assert lock_path.read_text(encoding="utf-8") == held_contents + + def test_rebuild_code_is_idempotent_when_cluster_ids_flap(tmp_path, monkeypatch): from graphify import cluster as cluster_mod from graphify.watch import _rebuild_code