[snowshadow]

What this fixes

build, update and postprocess now take an exclusive lock for the whole write window. A caller that cannot take it stands down with status: contended and the holder's PID instead of repeating the work.

This is the mechanism behind #1073. The report there is about correctness — one writer post-processing a graph another writer has only partly written — and it is also a CPU problem, which is how I hit it:

An agent tool hook ran code-review-graph update --skip-flows after every tool call. Ten of them overlapped on one repository, each spawning its own process pool (~90 Python workers on 18 cores), load average 44, 63 of 64 GB RAM in use. Every duplicate parse was thrown away at the SQLite write lock — after it had already been paid for.

What changed

• code_review_graph/locking.py (new): WriteLock, an advisory exclusive flock on <data dir>/write.lock.
• build_or_update_graph takes it before the first parse; run_postprocess takes it before rebuilding flows/communities/FTS. Both share one helper so they cannot drift.
• A lost race returns {"status": "contended", "skipped": true, "holder_pid": n, "lock_path": ...} with zeroed counts, so callers that unpack the existing keys keep working.
• CLI (build, update, postprocess): prints Skipped: another code-review-graph ... is already running ... (PID n) and exits EX_TEMPFAIL (75). --wait blocks instead of skipping.
• wait_for_lock on build_or_update_graph, run_postprocess, and their MCP tools' docs.

Why the choices

Exit 75, not 0 or 1. A skip is not success — a caller must not believe a graph was refreshed when it was not — but it is not the failure 1 reports either (bad config, corrupt database). EX_TEMPFAIL is exactly this condition, so a hook script can branch on it without matching strings, and || true hooks are unaffected.

A skip by default, --wait on request. The caller that overlaps is nearly always a hook, and the in-flight run covers the same working tree, so skipping is free there. A human running update by hand usually wants the wait.

Lock named from store.db_path, not a second path resolution. The file guards the database that is actually about to be written, including under CRG_DATA_DIR and --data-dir; two resolutions of "the" repo root can disagree.

Fixed name and location (<data dir>/write.lock). As the issue asked: external tooling can exclude itself with flock -n .code-review-graph/write.lock ... and both sides exclude each other. One database, one lock.

flock, not lockf. The lock belongs to the open file description, so a probe from a second descriptor cannot silently drop a lock this process holds, and the kernel releases it on SIGKILL — a build killed by a hook timeout never wedges the repository.

Failure to open the lock file degrades to the old behaviour rather than failing the command; Windows (no flock) does the same, so behaviour there is unchanged.

Scope

This covers the two entry points that own the long parse plus the stage that rewrites derived data. Deliberately left out, because they need a different treatment and I did not want to widen the diff:

• watch writes through incremental_update directly. Handing it a non-blocking lock is wrong — a skipped batch would silently drop file-change events — so it needs "block or retry this batch", which is its own change.
• embed and forget are single short writes that SQLite already serialises transaction by transaction, with no long parse to duplicate.

Happy to extend this PR to cover them, or to file follow-ups if you prefer to land this first.

Tests

tests/test_concurrency_and_daemon.py (-m concurrency), 10 new tests:

test	asserts
test_the_lock_lives_in_the_data_directory	fixed write.lock path, so flock -n interop holds
test_second_update_skips_while_the_first_holds_the_lock	exit 75, no anchor written, no nodes written
test_the_skipped_run_names_the_process_that_owns_the_work	the message carries the holder PID
test_a_finished_build_leaves_the_lock_free	a completed build releases it
test_a_killed_holder_does_not_wedge_the_repository	SIGKILL the holder, next run builds
test_wait_blocks_until_the_holder_finishes	--wait waits, then does the work
test_builds_of_different_repositories_do_not_block_each_other	per-database, not global
test_the_tool_entry_point_reports_contention_instead_of_parsing	MCP path returns contended
test_postprocess_stands_down_for_a_running_build	the other half of #1073
test_a_burst_of_concurrent_updates_builds_the_graph_once	8 concurrent updates, 1 build, graph correct

8 of the 9 behaviour tests fail on staging (verified by reverting tools/build.py and re-running); the burst test is the regression for the reported symptom.

Two existing tests were updated for the new contract: test_concurrent_builds_do_not_corrupt_or_lose_files now expects a loser to exit 75 with the skip message rather than die on the busy timeout (the timeout path itself is still covered by the _LockHeld tests, which hold the SQLite lock without holding ours), and the exact-kwargs assertions in tests/test_cli.py and tests/test_embedding_refresh.py gained wait_for_lock=False.

Verification

uv run pytest tests/ -q                 # 5046 passed, 890 skipped
uv run pytest tests/test_concurrency_and_daemon.py -m concurrency   # 50 passed
uv run ruff check code_review_graph/    # clean
uv run mypy code_review_graph/ --ignore-missing-imports --no-strict-optional   # clean
bandit -r code_review_graph/ -c pyproject.toml   # clean

Two tests fail in my sandbox both before and after this change, for environmental reasons: test_released_shapes.py::test_recorded_hook_blocks_match_the_tags (the clone has no release tags) and test_e2e_real_repo.py::test_hybrid_search_ranks_impact_radius_nodes_first (no embedding model available). Confirmed by stashing the branch and re-running them on staging.

Docs

CHANGELOG.md under Unreleased, docs/COMMANDS.md (--wait), and a new docs/TROUBLESHOOTING.md section covering the skip message, the exit code and the flock interop.

Refs #1073