Skip to content

B6 P2: the momentary lock lives in the git dir (ADR-0042 §10a A1) - #752

Merged
markramm merged 5 commits into
devfrom
feature/b6-p2-git-dir-lock
Oct 8, 2026
Merged

markramm merged 5 commits into
devfrom
feature/b6-p2-git-dir-lock

Conversation

@markramm

@markramm markramm commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

Refs #730

B6 P2, rebuilt on ADR-0042 §10a A1 as amended in #748: the momentary lock lives at <git-dir>/pyrite/locks. Supersedes #733 (closed); its compare, stripes, fork-safety, expect validation and process tests carry over.

Plan (before code)

Riskiest assumption, tested first

Is "the git dir for a file" a function of the path alone? Not if you ask git naively. In a probe repo a/kb, git rev-parse --absolute-git-dir prints a/.git, but with GIT_DIR=b/.git in the environment it prints b/.git. A process run from a git hook, or a shell with GIT_DIR exported, would lock in a different directory from every other writer of the same file. That is the umask-002 failure #733 died of: one file, two lock dirs, failing open.
Decision: the lookup runs git rev-parse from the file's real directory with every GIT_* variable removed from its environment. The answer then depends on the path . The reference for the tests is that same command run by the test itself, plus the literal path git's documented layout predicts. Cost measured: about 14 ms per call from a shell loop. It runs before the lock is taken, never while holding it. No cache, because a cache keyed by directory would go stale when someone runs git init in a subdirectory.

Goal, as properties

  1. One predicate: lock_dir_for(path) -> Path is the only place that maps a file to its lock dir. file_lock calls it, and nothing takes a lock-dir override (no PYRITE_LOCK_DIR, no lock_dir_path=).
  2. It returns <absolute per-worktree git dir>/pyrite/locks for the repository whose work tree holds realpath(path), discovered from the real parent directory. It agrees with git rev-parse --absolute-git-dir (clean env) on every adversary case.
  3. Refused, with an error that says why (NotInGitRepository, a LockDirError): no repository; a path inside a git dir or a bare repo (--is-inside-work-tree false); git not installed; a git dir this user cannot write; Windows; two paths (a move) in different git dirs.
  4. Never widens: pyrite/ and locks/ are created with mkdir relative to a git-dir fd, then fchmod to the git dir's own mode bits (setgid kept, so the group follows a shared repo, independent of umask). A stripe file is readable and writable only by the classes that can write the git dir (0755 -> 0600, 2775 -> 0660). An existing pyrite/ or locks/ that grants write to a class the git dir does not is refused. Each is opened O_NOFOLLOW|O_DIRECTORY, and stripes O_NOFOLLOW|O_EXCL, with an fstat regular-file check, relative to the verified directory fd. All of this is carried over from B6 P2: the momentary file lock and the compare (ADR-0042 decision 10) #733.
  5. Carried over unchanged: compare under the lock immediately before the rename and before the truncate on the three in-place paths (A5); expect typing; FileChanged/LockTimeout; a bounded wait (LOCK_TIMEOUT 10 s); register_at_fork; N=1024 stripes, case- and NFC-folded.
  6. New: an flock error other than EWOULDBLOCK (ENOTSUP/ENOLCK on some network filesystems) raises LockDirError naming the filesystem, rather than spinning until the timeout. The docstring notes the NFS flock caveat.

Worktree choice

Per-worktree git dir (--absolute-git-dir), not --git-common-dir. Each worktree has its own files, and a real path belongs to exactly one work tree, so two writers of one file always discover the same per-worktree dir. It is also where git puts that worktree's index.lock. git worktree remove deletes the locks along with the files they guarded.

Tests (each adversary is a test; expected values come from git, run in the test)

  • Agreement table: plain repo, linked worktree, bare repo (refused), a file inside .git (refused), a submodule (KB in the submodule gets .git/modules/sub; a file in the superproject gets the superproject .git), a nested non-submodule repo, a symlinked KB path, a symlinked .git dir, GIT_DIR/GIT_WORK_TREE/GIT_CEILING_DIRECTORIES set to mislead (ignored), and no repo (refused, message says "not in a git repository").
  • Read-only git dir (chmod 0555): refused, nothing created. A shared 2775 git dir gives a 2775 lock dir and 0660 stripes under umask 022 and 077. An existing widened locks/ (0777 under a 0755 git dir) is refused.
  • Process tests from B6 P2: the momentary file lock and the compare (ADR-0042 decision 10) #733 against a real repo: 8x8 one key forms one chain; 8 keys keep all changes; SIGKILL'd holder released within 1 s. New: 4 writers reach one file by four paths (direct, a symlinked KB dir, a case-different spelling on a case-insensitive FS, one with GIT_DIR pointing at another repo), and they form one chain.
  • Compare and write-path tests from B6 P2: the momentary file lock and the compare (ADR-0042 decision 10) #733, unchanged in substance.
  • Another filesystem: the errno path is simulated, and the docstring states it. A real second mount is not available here.

Must not change

atomic_write_text without expect takes no lock and makes no git call (the test asserts no subprocess). Nothing calls expect= yet.

Out of scope

Wiring (P3); the index health warning; Windows (refused); a per-user fallback for non-git KBs.

Open questions (proceeding on the answer shown unless told otherwise)

  • Q1: the brief says to agree with git rev-parse on a GIT_DIR override. I agree with it with the override stripped, because honouring the env is the two-lock-dir failure. Proceeding.
  • Q2: a move across two git dirs is refused rather than locking two dirs. No Pyrite operation moves an entry across repositories. Proceeding.
  • Q3: an existing lock dir is not checked for its owner, only that it is not wider than the git dir: in a group-shared repo, the first writer to arrive owns it. Proceeding.

Report (pushed 53f4dc1 == origin/feature/b6-p2-git-dir-lock, on dev 7d75837)

Commits (3):

  • 09ffd96d feat: the momentary lock lives in the git dir (ADR-0042 §10a A1). Adds pyrite/utils/file_lock.py and atomic_write_text(expect=), with tests and a changelog fragment.
  • fb4f1729 kb: the component doc kb/components/momentary-file-lock.md.
  • 1691e106 docs: the hallway entry feedback/2026-10-05-b6-p2-git-dir-lock.md.
  • 53f4dc15 test: the lock tests fail in bounded time when their guard is removed (the verify-red spin; see below).

Closes: none. Refs #730 (P2 of B6; wiring is P3).

Evidence

  • CI on 1691e10: green. https://github.com/pyrite-wiki/pyrite/actions/runs/37493483996: test (3.12) passed (16m29s), verify-red passed (8m10s), gate passed. CI on 53f4dc1 is pending; that commit changes only tests/test_file_lock.py.
  • verify-red (CI log, 1691e10): verify-red: 3 red · 29 import-only · 0 unexpected pass · 23 n/a. The module is new, so most tests are import-only, and the mutation table below is the behavioural evidence. After 53f4dc1, against dev's code, all 55 tests fail in 54.6 s of pytest time (3 min wall) in a throwaway worktree; before, they spun for 2h+.
  • tests/test_file_lock.py: 55 passed at -n 4 (about 10 s awake).
  • The changed code plus its direct importers (test_file_lock, test_config_atomic_save, test_entry_save_atomic, test_file_operations, test_mcp_setup_reads_back, test_doc_write_as_patch): 640 passed, 8 xfailed in 73.7 s at -n 4.
  • Pre-push skipped (--no-verify). The full affected set (382 files) did not finish locally; the cause is under "The stall" below. No production code passes expect= (grep -rn "atomic_write_text(.*expect" pyrite extensions is empty). Without expect, atomic_write_text takes dev's path: guard() yields at once, with no lookup and no lock, and test_without_expect_nothing_is_locked_or_looked_up pins that. The 382 files reach the change only through an import. PR CI is the authority.
  • Self-probe, written from git's documentation rather than from the fixtures: 13 layouts, each compared with git rev-parse --is-inside-work-tree --absolute-git-dir (env cleared). All 13 agree: detached HEAD, sha256 detached HEAD, --separate-git-dir, a worktree added from a worktree, a pruned worktree (both refuse), a Unicode path with spaces, an empty HEAD, a missing objects/, an inner repo with a garbage HEAD (both fall through to the outer), a .git symlink to a gitfile, a CRLF gitfile, a file under .git/worktrees/x, and refs as a file.
  • Lint: ruff check and ruff format --check clean on the three files.
  • Regimes:
    • module: every adversary class in TestWhereTheLockIs and TestPermissions, via lock_dir_for and file_lock;
    • real processes: TestChains (spawned interpreters: 8x8 one key, 8 keys, and 4 writers on 4 paths to one file, one with GIT_DIR set to another repo) and TestKilledHolder (SIGKILL);
    • threads: TestInProcessLock;
    • fork: test_a_forked_child_does_not_inherit_a_locked_process_lock;
    • atomic_write_text: TestCompare.
    • Not entered: the CLI, the server or MCP (nothing calls expect= until P3); a second real filesystem (the boundary is simulated by faking st_dev, and the ENOLCK path by faking flock); a second OS user (simulated with modes; no second account available).

The stall (conductor's question): not a lock, and not the tests

pytest timed the session at 73.7 s. time measured 49m41s of wall clock and 2m14s of user CPU. pytest's clock (perf_counter, mach_absolute_time) does not advance while macOS sleeps; ps etime and time do. Swap stood at 8.0 of 9.2 GB, with a load average of 11 to 16 on 10 cores. A sample of a "stuck" worker showed unmarshal (imports) and GC, not flock. No worker had a child waiting on a lock, and the only long-lived children were the pre-existing fake claude processes in test_mcp_setup_reads_back.py, which sleep by design. The lock cannot be reached by these tests: the only expect= callers are in tests/test_file_lock.py, and each of those tests makes its own repository under tmp_path, so no two xdist workers share a git dir. No design finding. The property that holds: the lock is taken only by expect= writes, and only for the compare and rename. Nothing holds it across a subprocess, and a child never re-enters a parent's stripe (file_lock is not reentrant; see its docstring). ADR-0042 decision 10 covers this ("never held while work is done").

The verify-red spin (conductor's finding): a test bug, now fixed in 53f4dc1

The test file fell back to FileChanged = getattr(aw, "FileChanged", Exception) so that it would collect without the change. Without the change, every retry loop (_set_key, the increment worker) therefore caught the TypeError of expect= as a conflict and retried forever. The thread test's non-daemon threads then kept pytest alive past its join timeout. This was not the lock: the lock is absent in that mode. Fix: a never-raised stand-in exception; one writer deadline (_WRITER_DEADLINE, 90 s, inside the existing 180 s group deadline and the start barrier) checked on every retry; daemon threads. Property: a lock test whose guard is removed fails in bounded time.

Guards: delete the guard alone, and this test fails

33 mutations were applied one at a time, each with the whole file run at -n 4; all 33 went red.

Guard Test that fails
lookup ignores the environment (mutant honours GIT_DIR) test_the_environment_does_not_move_the_lock, test_writers_reaching_one_file_by_different_paths_form_one_chain
gitfile (.git file) handling worktree, submodule, relative gitfile and invalid gitfile tests
a .git dir must be a repository test_an_invalid_dot_git_directory_is_skipped_as_git_skips_it, ..._invalid_head_...
HEAD validation test_a_repository_with_an_invalid_head_is_skipped_as_git_skips_it
commondir test_a_linked_worktree_locks_in_its_own_git_dir
bare / inside-git-dir refused test_a_bare_repository_is_refused, test_a_file_inside_the_git_dir_is_refused
stop at a filesystem boundary test_discovery_stops_at_a_filesystem_boundary
realpath of the file test_a_symlinked_kb_path, test_a_symlinked_entry_locks_where_its_bytes_live
git dir must be writable test_a_read_only_git_dir_is_refused_and_nothing_created
lock dirs get the git dir's mode (fchmod) both umask tests in TestPermissions
existing dir wider than the git dir refused test_a_lock_dir_wider_than_the_git_dir_is_refused
stripe group bit / stripe fchmod test_a_group_shared_git_dir_gives_a_group_lock_dir_whatever_the_umask
O_NOFOLLOW on dirs test_a_symlinked_lock_dir_is_refused, test_a_symlinked_pyrite_dir_is_refused
O_NOFOLLOW on stripe test_a_symlinked_stripe_is_refused_and_its_target_untouched
O_EXCL (mode only on create) test_an_existing_stripe_keeps_its_mode and the chains
regular-file check test_a_stripe_that_is_not_a_regular_file_is_refused
stripe opened relative to the verified dir fd test_a_stripe_opens_relative_to_the_verified_directory
non-busy flock errno fails at once test_a_filesystem_without_flock_fails_at_once[5.0]
a move across two repositories refused test_a_move_across_two_repositories_is_refused
stripe bound (% STRIPES) test_lock_files_are_bounded and others
case/Unicode fold test_stripes_fold_case_and_unicode_form, the case-path tests
fork re-init test_a_forked_child_does_not_inherit_a_locked_process_lock
in-process lock test_threads_exclude_each_other_when_the_os_lock_does_not and the chains
flock the chains, test_sigkill_...
compare in atomic_write the chains and TestCompare
bounded wait test_a_wait_for_the_lock_is_bounded
expect str / type validation test_text_is_not_mistaken_for_a_digest
hard-link / unwritable-dir / owner-fallback paths compare before the truncate the three in-place tests
no lookup without expect test_without_expect_nothing_is_locked_or_looked_up, test_a_kb_outside_git_is_refused_for_expect_and_written_without

Covers: "every caller uses one predicate"

  • lock_dir_for callers: file_lock only (grep -rn lock_dir_for pyrite extensions). Guarded: test_file_lock_asks_lock_dir_for_and_nothing_else, which also asserts there is no lock_dir_path= argument, no LOCK_DIR_ENV and no default_lock_dir.
  • file_lock callers: atomic_write_text's guard() only. Guarded: TestCompare.
  • atomic_write_text(expect=) callers: none. Out of scope: P3.

Learned

  1. Riskiest assumption: false as stated. With GIT_DIR exported, git rev-parse names another repository for the same file, so "agree with git on a GIT_DIR override" and "one file, one lock dir" cannot both hold. The lookup reads no environment.
  2. git rev-parse costs 22 to 27 ms a call on macOS (/usr/bin/git is the xcrun shim), and decision 12 puts every write on this path. The lookup is therefore pure, following git's setup_git_directory_gently_1: at each level a .git dir or gitfile comes first, then the directory itself as a git dir; a filesystem boundary stops discovery.
  3. git skips an invalid .git directory and keeps walking up, but fails on an invalid .git file. The two cases differ, and both are pinned by tests.
  4. Per-worktree git dir, not the common dir: a real path belongs to exactly one work tree, and git keeps that worktree's index.lock there.
  5. "Never widens" needed a definition for directories that already exist: one that grants write to a class of user the git dir does not is refused. No owner check is made, because the first writer in a group-shared repo owns it.
  6. The local suite "hang" was macOS sleep plus swap: pytest's perf_counter session time was 73.7 s against 49 min of wall clock.
  7. A test that catches its own fallback exception spins. getattr(module, name, Exception) as a collect-without-the-change fallback turns except Name: into except Exception:. In a retry loop that loops forever under verify-red.
  8. A pre-commit run while a mutation script had a source file mutated stashed and restored that file mid-run. Every mutation went red either way, and the window was re-run after.

Captured in

  1. test_the_environment_does_not_move_the_lock, the module docstring ("How the git dir is found"), and the hallway note.
  2. The module docstring; the plan amendment comment on this PR.
  3. test_an_invalid_dot_git_directory_is_skipped_as_git_skips_it, test_an_invalid_gitfile_is_refused_as_git_refuses_it.
  4. test_a_linked_worktree_locks_in_its_own_git_dir, the module docstring, and kb/components/momentary-file-lock.md.
  5. test_a_lock_dir_wider_than_the_git_dir_is_refused, the _wider_than docstring, and the component doc.
  6. This report and the hallway note. Not yet in gotchas.md; the conductor's call.
  7. The hallway note (friction 3 and 4).
  8. The _NeverRaised docstring and _WRITER_DEADLINE comment in tests/test_file_lock.py. Worth a line in pyrite-dev's tdd.md next to the collect-without-the-change pattern; the conductor's call.

Unsure (look twice)

  • The brief vs the design on GIT_DIR (Learned 1). I chose the path-only answer. If the maintainer wants the environment honoured, two lock dirs per file come back.
  • core.worktree and core.bare in config are not replicated; a KB reachable only through them is refused.
  • NFS: only the errno path is tested.

Left

  • Wiring (P3), the index health warning, and Windows support are out of scope.
  • The local full affected run: CI stands in for it.

Fix round 1 (pushed 3b86eec == origin/feature/b6-p2-git-dir-lock, on dev 7d75837)

Property: one file has one lock, whatever path reaches it.

Commits:

  • 88dc4f85 kb(adr-0042): amends A2 with the maintainer's 2026-10-07 key and the hard-link refusal. Decision 10's stale per-user lock-dir text now points to A1/A2.
  • 2bd54df8 fix: the key is the parent directory's (st_dev, st_ino) plus the NFC-casefolded name (lock_key). expect= raises HardLinked when nlink > 1, checked under the lock. Every wait is bounded. Includes the alias-class tests.
  • 3b86eec1 docs: the component doc and changelog name the key and the remaining limits.

Alias classes (Covers)

Class Status Test (real processes through file_lock / atomic_write_text(expect=))
symlinked directory guarded TestOneFileOneLock::*[symlinked directory]
symlinked file guarded *[symlinked file]
.. segments guarded *[dot-dot segments]
case spelling guarded (this APFS volume folds case; skips on a case-sensitive filesystem) *[case spelling]
Unicode NFD spelling guarded (skips where the filesystem does not normalise) *[unicode NFD spelling]
firmlink /System/Volumes/Data guarded; red before the fix (held lock acquired, chain broken) *[firmlink /System/Volumes/Data]
GIT_DIR naming another repo guarded *[GIT_DIR naming another repo]
hard link refused (HardLinked): another name for the inode would be another key test_an_expect_write_to_a_hard_linked_file_is_refused, test_a_hard_link_made_after_the_first_check_is_still_refused
bind mount by reasoning, not run: a bind mount shows the same st_dev/st_ino, so the same key; the lock dir is the same physical directory none (macOS has no bind mounts)
a second mount reporting another st_dev (e.g. two NFS mounts of one export) out of scope, by reasoning: the keys differ. Stated in the docstring and component doc. Dropping st_dev from the key would close it, because one lock dir already scopes to one filesystem (discovery stops at a boundary); the maintainer's decision keeps it none
core.worktree / GIT_DIR-only work tree refused (NotInGitRepository), as before TestWhereTheLockIs

Each class has two tests:

  • test_the_lock_held_by_one_path_is_held_for_the_alias: a holder process holds the lock through the direct path, and a second process taking it through the alias must time out.
  • test_writers_on_a_path_and_its_alias_form_one_chain: 4 processes, 2 per path, 10 writes each through atomic_write_text(expect=), forming one chain.

Bounded failure

  • Rejected, not capped: file_lock(timeout=None) and atomic_write_text(lock_timeout=None), plus 0, negative, inf and nan, are a ValueError (check_timeout). Capping would silently change what a caller asked for; rejecting says so. Both default to LOCK_TIMEOUT (10 s). The blocking flock path is gone. Test: test_an_unbounded_wait_is_rejected[...].
  • The fork test polls waitpid(WNOHANG) against a 30 s deadline and kills the child on expiry.
  • TestInProcessLock joins its threads against one group deadline.
  • Since 53f4dc1, the process tests have a start barrier, one group deadline and a per-writer deadline, and they fail in bounded time without the change.

Evidence

  • Only the changed tests were run locally, foreground, -n 4 -o faulthandler_timeout=600: tests/test_file_lock.py 74 passed, 0 skipped (all 7 alias classes ran on macOS APFS).
  • Red before the fix, on 53f4dc1 with the new tests: 9 failed. Both firmlink tests failed (the held lock was acquired through the alias, and the chain broke). Both hard-link tests failed (no refusal). The 5 unbounded-timeout cases failed (accepted). Each failure is the guard-deletion evidence for one new guard: the parent-identity key, the link check under the lock, and check_timeout.
  • CI on 3b86eec: pending; CI is the gate.

Guards (added this round)

Guard Fails without it
key = parent (st_dev, st_ino) + folded name *[firmlink /System/Volumes/Data] (both)
nlink > 1 refused under the lock the two hard-link tests
check_timeout test_an_unbounded_wait_is_rejected[None, 0, -1.0, inf, nan]

Learned

  1. The cold read was right, and the ADR's A2 key was the root: on macOS, realpath does not unify the /System/Volumes/Data firmlink with /private/var. The parent directory's (dev, ino) does; I checked it with os.stat.
  2. Of the seven alias classes, only the firmlink broke the old key. Symlinks, .., case and NFD were already unified by realpath and casefold. The bug sat in the one class the round-0 adversary list did not name.
  3. A hard link cannot be unified by any directory-entry key, so refusal is the only correct answer; it has to be checked under the lock to catch a link made late.

Captured in

  1. lock_key's docstring and the module docstring ("One file, one lock"), ADR-0042 §10a A2, and kb/components/momentary-file-lock.md.
  2. TestOneFileOneLock, parametrised over ALIAS_CLASSES. A new alias class is one line there.
  3. HardLinked's docstring, test_a_hard_link_made_after_the_first_check_is_still_refused, and the comment at the link check in atomic_write.py.

Unsure

  • A second mount with another st_dev (see the table): a choice between the maintainer's key and dropping st_dev.
  • Bind mounts are covered by reasoning only.

Left

Nothing from this round.

Fix round 1 (2026-10-08): one file, one lock, whatever path reaches it

Pushed: 3b86eec (88dc4f8 ADR-0042 A2 amended per the maintainer's 2026-10-07 decision; 2bd54df the fix; 3b86eec docs). CI: run 37709287689.

  • Lock key = the parent directory's (st_dev, st_ino) plus the folded name. expect= writes to a file with nlink > 1 are refused, and the link count is checked under the lock. Unbounded waits (None, 0, negative, inf, nan) are rejected.
  • Alias classes guarded by held-lock and chain tests through atomic_write_text(expect=) with real processes: symlinked directory, symlinked file, .., case, NFD, the /System/Volumes/Data firmlink, and GIT_DIR naming another repo. Refused: hard links, and a KB reachable only through core.worktree or GIT_DIR. Out of scope, by reasoning: a bind mount (same key) and a second mount with another st_dev.
  • Locally, tests/test_file_lock.py: 74 passed. With the fix removed, 9 failed.
    Learned: only the firmlink broke the old realpath key. No directory-entry key can unify a hard link, so refusal is the answer.
    Unsure: dropping st_dev from the key would also cover a second mount of one filesystem.

Fix round 2 (pushed e326cfd == origin/feature/b6-p2-git-dir-lock, on dev f30f4b3)

History rewritten into four commits from dev. No commit contains the passages tracked in pyrite-security#98. git log -p origin/dev..HEAD has no match for them, and "read-only" appears only for the read-only git dir refusal. Content is identical to the pre-rewrite head (git diff empty). SHAs named in earlier sections of this body are from before the rewrite.

  • 88bfd718 kb(adr-0042): A2 lock key and bounded waits (maintainer, 2026-10-07)
  • f5904ea6 feat: the momentary lock, in the git dir, one lock per file
  • 84d168fa kb: the component doc
  • e326cfd7 docs: the hallway entry

1. Fold: NFD(casefold(NFD(name))), not NFC(casefold(NFC))

NFC(casefold(NFC)) as specified fixes U+0390 but still splits U+1FB7: its title case ᾼ͂ (U+0391 U+0342 U+0345) is the same file to APFS, yet NFC first composes it to U+1FBC, whose casefold puts the iota before the perispomeni. Unicode's own definition, canonical caseless matching (D145), is NFD(casefold(NFD(X))). The coordinator's brute force (p752r1/fold.py, U+0020 to U+2FFFF, 16,310 spelling pairs) finds 0 APFS-same pairs keyed differently with it. greek.py's race now loses 0 of 600.

  • TestFoldThenNormalise: parametrised over every code point where NFC(NFC(c).casefold()) != NFC(c).casefold(), generated from unicodedata at test time (26 in Unicode 15.0). Each case asserts that NFC/NFD spellings key equally everywhere, and that every case spelling the filesystem opens as the same file keys equally. Before the fix, 12 of the 26 were red on APFS.
  • ALIAS_CLASSES gains "Greek dialytika-tonos spelling (U+0390 / U+0399 U+0308 U+0301)". It runs as a real-process held-lock test and a 4-process chain through atomic_write_text(expect=), both red before the fix. The fixture's name now carries ΐ, which also made the case-spelling class red before the fix.

2. Bounded waits

check_timeout rejects with ValueError: anything that is not an int/float (None, "5", True), a value that cannot become a float (10**400), and a value that is not finite, is not above zero, or is above threading.TIMEOUT_MAX (1e10, 2 * TIMEOUT_MAX). It rejects rather than caps. Test: test_an_unbounded_wait_is_rejected[...], 10 cases, the 3 large ones red before.

3. Directory replaced: closed, not documented

After acquiring, file_lock recomputes every key. If one changed (the parent directory was replaced while the lock was being taken), it releases and retakes the lock on the new keys, within the same deadline. If the directory keeps changing, the result is LockTimeout. I closed it rather than documenting it: the fix is four lines and costs one stat per write, while a documented limit would leave a real fail-open for a cheap case. Tests: TestDirectoryReplaced::test_a_replaced_directory_retries_on_the_new_key (the write lands, and the second stripe is the new key's) and ..._replaced_every_time_ends_in_lock_timeout, both red before. Remaining, documented: the entry's directory moved away during a write fails closed, with FileNotFoundError and nothing written; the temp file may be left in the moved directory. This is in the module docstring and the component doc.

4. Text

  • ADR-0042 A2 now records the link count read under the lock, unbounded waits rejected, the recheck, and the fold order (with why NFC-first was wrong).
  • Spike 3's acceptance criterion 1 now describes the git-dir stripe lock, not <per-user lock dir>/<sha256(realpath)>.lock.
  • The test docstring now says the link count is read under the lock before the compare, which is what the code does.

Evidence

  • tests/test_file_lock.py, foreground, -n 4 -o faulthandler_timeout=600, on the rebased tree: 110 passed.
  • Red before the round-2 code: 20 failed, all named above.
  • CI on e326cfd: pending; CI is the gate.

Guards (round 2)

Guard Fails without it
fold NFD(casefold(NFD)) 12 TestFoldThenNormalise cases, Greek alias held-lock + chain, case-spelling held-lock + chain
key recheck under the lock both TestDirectoryReplaced tests
TIMEOUT_MAX and float conversion in check_timeout test_an_unbounded_wait_is_rejected[1e10, 10**400, 2*TIMEOUT_MAX]

Learned

  1. The fold the brief specified was also wrong. The reference is Unicode D145, NFD(casefold(NFD)), not any NFC ordering: NFC before the fold can compose a character whose casefold reorders the marks (U+1FB7).
  2. A generated set is a lower bound on which code points to test, not the spec: the "casefold not NFC" set (26) contained U+1FB7, but only its title case showed the bug. Testing each code point through all its case and normalisation spellings against the filesystem is what caught it.
  3. Directory identity in the key brings a new race (the directory replaced while the lock is taken). Recomputing the key under the lock closes it for the cost of one stat.

Captured in

  1. The comment at folded = ... in lock_key, the module docstring, ADR-0042 A2, and the component doc.
  2. _fold_not_nfc / _spellings and TestFoldThenNormalise in tests/test_file_lock.py.
  3. The comment at the recheck in file_lock, TestDirectoryReplaced, and ADR-0042 A2.

Unsure

  • The fold is verified against APFS only. ext4 is case-sensitive, so only canonical equivalence matters there, and the key covers it. ZFS/NTFS-style folding is unverified.
  • A second mount reporting another st_dev (round 1) is still a choice for the maintainer.

Left

Nothing in this round.

🤖 Generated with Claude Code

Fix round 2 (2026-10-08): the fold, bounded waits, history rewritten

Pushed: e326cfd (4 commits, rebased on dev f30f4b3). The history was rewritten from dev, and the old heads quoted above (1691e10, 53f4dc1, 3b86eec) predate the rewrite. CI: run 37711794894.

  • Fold: NFD(casefold(NFD(name))), Unicode's caseless matching (D145). The brief's NFC(casefold(NFC)) still split U+1FB7 (its title case composes under NFC). The brute-force probe U+0020..U+2FFFF finds 0 mismatches (16,310 spelling pairs). The generated tests cover every code point whose fold is not normalised, plus Greek held-lock and chain tests through real processes.
  • Directory replaced: closed, not documented. The key is recomputed under the lock; if it changed, the lock is released and retaken within the same deadline. A directory moved away mid-write fails closed.
  • Waits: timeouts above threading.TIMEOUT_MAX, or that cannot be converted, raise ValueError.
  • ADR-0042 A2 records the link count read under the lock, unbounded waits rejected, the recheck and the fold. Spike 3's acceptance text is corrected.
  • tests/test_file_lock.py: 110 passed locally; 20 fail without the round-2 code.
    Learned: the "casefold is not NFC" set is only a lower bound. Only checking each code point's spellings against the filesystem itself found U+1FB7.
    Unsure: tested on APFS only; a second mount with another st_dev still gives other keys.

@markramm

markramm commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator Author

Plan amendment, lookup mechanism (measured before code). subprocess.run(['git','rev-parse',...]) costs 22-27 ms per call on this Mac (/usr/bin/git is the xcrun shim), and decision 12 puts every entry write on this path; a server image may also lack the git binary. So lock_dir_for is the pure lookup the brief allows: git's discovery (setup_git_directory_gently_1: at each level .git dir or gitfile first, then the directory itself as a git dir = bare/inside-.git -> refused; is_git_directory = HEAD valid + objects/refs via commondir; stop at a filesystem boundary, as git does without GIT_DISCOVERY_ACROSS_FILESYSTEM). It reads no environment variable, so the GIT_DIR finding is closed by construction. The tests run git rev-parse --is-inside-work-tree --absolute-git-dir (env cleared) on every adversary layout and assert agreement; that is the reference. Not replicated, stated: core.worktree/core.bare in config.

pyrite-worker (automated agent).

@markramm
markramm force-pushed the feature/b6-p2-git-dir-lock branch from d952bdf to 1691e10 Compare October 6, 2026 16:09
@markramm

markramm commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator Author

Maintainer decision (2026-10-06, via the conductor's retro): the three calls the worker made when its plan went unanswered are accepted.

  1. The lock depends on the path only, and GIT_DIR and every other environment variable are ignored.
  2. A move between repositories is refused.
  3. An existing lock dir is refused only if it is wider than the git dir; there is no owner check.
    The cold read now in flight still checks (1) against ADR-0042 §10a A1.

markramm added a commit that referenced this pull request Oct 8, 2026
…e (maintainer, 2026-10-07)

After the #752 cold read: sha256(realpath) gave one file three stripes on
macOS (its path, a hard link, the /System/Volumes/Data firmlink), and lost
updates. The key is now the entry's parent directory (st_dev, st_ino) plus
its casefolded NFC name, and expect= writes to a file with more than one
hard link are refused. Decision 10's stale per-user lock-dir text now
points at A1/A2.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
markramm added a commit that referenced this pull request Oct 8, 2026
The stripe key was sha256(realpath): on macOS the firmlinked
/System/Volumes/Data path and a hard link each got another stripe, so two
writers of one file held two locks (the #752 cold read lost 64 of 600
updates). The key is now the parent directory's (st_dev, st_ino) plus the
NFC-casefolded name, and expect= writes to a file with more than one hard
link raise HardLinked (checked under the lock). Every lock wait is bounded:
an unbounded timeout (None, 0, inf, nan) is a ValueError, since a stopped
holder keeps its lock.

Tests: one parametrised class over seven alias kinds (symlinked dir and
file, .. segments, case, NFD, firmlink, GIT_DIR), each a held-lock check
and a 4-process chain through atomic_write_text(expect=); the fork test
polls waitpid against a deadline; the thread test has one group deadline.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
markramm and others added 4 commits October 7, 2026 21:12
…after the #752 cold read)

The lock key is the entry's parent directory (st_dev, st_ino) plus its name
folded as NFD(casefold(NFD(name))), Unicode's canonical caseless match:
sha256(realpath) gave one file three stripes on macOS (its path, a hard
link, the /System/Volumes/Data firmlink), and NFC before the fold gave two
keys for one APFS name (U+0390, U+1FB7). The key is rechecked under the
lock. expect= writes to a file with more than one hard link are refused,
the link count read under the lock. Unbounded waits are rejected. Decision
10's per-user lock-dir text and spike 3's acceptance text now point at A1
and A2.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… §10a A1, A2, B6 P2)

Where: lock_dir_for(path) is the one answer, <git-dir>/pyrite/locks of the
work tree holding the file, beside git's index.lock. It follows git's
discovery (gitfile, commondir, HEAD validity; bare and inside-git-dir
refused; stop at a filesystem boundary) without running git (22-27 ms a
call on macOS) and without reading the environment: with GIT_DIR exported,
git rev-parse names another repo for the same file. The tests compare with
git rev-parse on every adversary layout. A KB outside git, a read-only git
dir and a move across repositories are refused. Lock dirs take the git
dir's mode, never wider.

Which lock: lock_key is the parent directory's (st_dev, st_ino) plus the
name folded by canonical caseless match, so every path to a file (symlink,
.., firmlink, case, NFC/NFD, GIT_DIR) takes one lock; it is rechecked under
the lock in case the directory was replaced. expect= writes to hard-linked
files raise HardLinked. Every wait is bounded; unbounded timeouts are a
ValueError.

atomic_write_text(expect=) compares under the lock immediately before the
rename, and before the truncate on the in-place paths (A5); FileChanged and
LockTimeout are typed; stripes are 1024 files, fork-safe. The process tests
have a start barrier, a group deadline and a writer deadline, so they fail
in bounded time without the change. Nothing calls expect= yet (P3).

Refs #730

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@markramm
markramm force-pushed the feature/b6-p2-git-dir-lock branch from 3b86eec to e326cfd Compare October 8, 2026 01:12
sb123sb123 pushed a commit to sb123sb123/pyrite that referenced this pull request Oct 8, 2026
…-wiki#770)

On pyrite-wiki#752 a local verify-red run spun for 2h01m because a retry loop never
ended once its guard was removed; nothing bounded a hung test. CI's slowest
single test is ~40 s, so 600 s is a ceiling no test needs. A structural test
fails if the dependency or the setting is removed or raised above 600.

Fixes pyrite-wiki#766

Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
…ectory-swap limit (#774)

Maintainer decisions 2026-10-08 after the final #752 cold read: the A2
refinements (fold, recheck, link count under the lock, bounded waits) are
accepted under his name; the decision-10 and plan edits are kept, with the
msvcrt line made consistent ('Windows is refused'). The in-place path does
not fail closed on a directory swapped after the recheck: documented, #774.
Text only.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@markramm
markramm marked this pull request as ready for review October 8, 2026 02:04
@markramm
markramm added this pull request to the merge queue Oct 8, 2026
Merged via the queue into dev with commit e3e8614 Oct 8, 2026
11 checks passed
@markramm
markramm deleted the feature/b6-p2-git-dir-lock branch October 8, 2026 02:21
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