Skip to content

agent: a lock file is recognized by being locked, not only by its name - #153

Merged
jaredLunde merged 2 commits into
mainfrom
jared/lock-file-gaps
Oct 7, 2026
Merged

jaredLunde merged 2 commits into
mainfrom
jared/lock-file-gaps

Conversation

@jaredLunde

@jaredLunde jaredLunde commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

This PR closes the three LOW gaps from #144's final check and fixes a doc. Each fix has a test that fails without it; I checked that by reverting each fix with temporary toggles.

Revised after audit. The first version of this PR found locks by opening the target and probing it. The audit found three problems with that, all now fixed:

  • HIGH: opening a FIFO with no writer hung write_atomic. Opening a device could act, and a hung NFS mount could block.
  • MED: the test flock made other programs' own non-blocking lock attempts fail (2071 of 20000).
  • MED: legitimate edits were refused, for example a SQLite WAL database that another program holds mid-transaction.

The probe (is_locked) is deleted.

The policy now: file_lock::is_lock_file only stats. It never opens the target and never locks it. A target is a lock file only when one of these holds:

  1. Its name says so, compared without regard to ASCII case. That means a record lock file (*.beyond-lock), or a legacy lock / <f>.lock beside one.

  2. This process holds a lock on it. try_lock adds the (dev, inode) of each lock file it holds to a small counted set, and Drop removes them after unlocking. is_lock_file looks up the target's stat there. This covers Target::Itself keys and our own lock files under any name, hard link or folded spelling.

  3. It is an old binary's legacy lock with no record file beside it, judged by where it sits:

    • a lock in a session directory (one holding 000001.jsonl, which is never deleted), or
    • a <f>.lock beside a session file (*.jsonl) or the MCP manifest.

    Nobody's lock is tested.

Anything else is an ordinary file, and an edit of it goes through. That includes a SQLite DB someone holds, a daemon's pid file, Cargo.lock, and a lock in a directory with no session.

Claim Proving test Fails without the fix (checked)
A FIFO target doesn't hang; a device is only stat'ed tools::tests::write_atomic_never_opens_its_target_to_decide the old probe times out at 10s
Another program's flock attempts never fail while we check its file file_lock::tests::asking_never_makes_another_programs_lock_fail (python, 20000 attempts, 0 failures) the old probe: 1410–1517 of 20000 failed
A SQLite WAL database held mid-transaction stays editable tools::tests::write_atomic_edits_a_sqlite_database_another_program_holds the old probe refuses it
Gap 1, casefold: .BEYOND-LOCK, S2.BEYOND-LOCK, and LOCK in an old session dir file_lock::tests::a_lock_file_is_one_by_its_name_in_any_case_or_by_where_it_sits, tools::tests::write_atomic_never_replaces_a_lock_file_a_record_beside_it_cannot_show, and …through_a_folded_spelling (run on a real casefold ext4 directory) case-sensitive compare: all three fail (on casefold ext4 too)
Gap 2, an old binary's legacy lock in a session dir with no record beside it the same two tests removing rule 3: both fail
Gap 3, an Itself key, and a hard link to a held lock under any name file_lock::tests::a_held_lock_file_is_recognized_under_any_name, tools::tests::write_atomic_never_replaces_a_lock_file_a_record_beside_it_cannot_show removing the held set: both fail

Doc fixes.

  • FileLock::release_and_remove_files and ARCHITECTURE.md now say what actually matters: the descriptors must be closed before remove_dir. An NFS client turns the unlink of a file it still has open into a rename to .nfs*, which stays until the last close and makes remove_dir fail with ENOTEMPTY.
  • ARCHITECTURE.md's Locks section now describes the stat-only policy.

The worktree seed still skips record lock files by name, now case-insensitively.

Local checks:

  • All mcp_*, serve_* and run_* suites plus lib units: 2392 passed.
  • Casefold ext4 run: 19/19.
  • Clippy --workspace --all-targets -D warnings, cargo fmt --check and dprint are clean.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk

jaredLunde and others added 2 commits October 7, 2026 04:51
The rename-target check in write_atomic was name-based, which missed:
a spelling a case-insensitive filesystem folds onto a lock file
(`.BEYOND-LOCK`, `LOCK`); an old binary's legacy `lock` held with no
record lock file beside it; and a journal key held through
Target::Itself.

file_lock::is_lock_file is now: named like a record lock file (ASCII
case-insensitive), or locked right now — asked of the file itself
(file_lock::is_locked: F_OFD_GETLK, which also reports this process's
other descriptions, plus a non-blocking test flock). A lock nobody holds
needs no protecting; Cargo.lock is written as ever. Verified on a casefold
ext4 directory too. The release doc now says what matters: the
descriptors are closed before remove_dir (an unlink of an open file on
NFS becomes a .nfs* rename until the last close).

Tests (each fails with its fix reverted): held legacy lock with no
record beside it, a key held Itself, and a case-folded spelling are
refused by write_atomic; a hard link under any name is recognized; the
folded-spelling test exercises a real case-insensitive dir when the temp
dir is one.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk
… target

write_atomic's lock-file check opened its target read-only and took a test
flock on it. That hung on a FIFO (and could act on a device or block on a hung
mount), made other programs' own non-blocking flocks fail (2071/20000), and
refused legitimate edits of files others hold locked (a SQLite WAL database).

is_lock_file now only stats. A target is a lock file when:
- its name says so, ASCII case-insensitively (record lock file, or a legacy
  lock beside one);
- its (dev, inode) is one this process holds: try_lock records each held lock
  file in a counted set, Drop removes them after unlocking (covers Itself keys,
  hard links, folded spellings of our own locks);
- it is an old binary's legacy lock by where it sits: `lock` in a session dir
  (000001.jsonl present) or `<f>.lock` beside a *.jsonl or the MCP manifest.

is_locked's probe is deleted.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk
@jaredLunde
jaredLunde merged commit b64cefc into main Oct 7, 2026
21 checks passed
@jaredLunde
jaredLunde deleted the jared/lock-file-gaps branch October 7, 2026 13:00
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