fix(wiki-lock): fall back to mkdir mutex when flock(1) is unavailable - #144
Open
JesseLeeStringer wants to merge 1 commit into
Open
fix(wiki-lock): fall back to mkdir mutex when flock(1) is unavailable#144JesseLeeStringer wants to merge 1 commit into
JesseLeeStringer wants to merge 1 commit into
Conversation
Every wiki-lock.sh command routes through with_meta_lock(), which called
flock(1) unconditionally. flock ships with util-linux and is absent on
Git Bash / MSYS2 (Windows) and on a default macOS install, so on those
platforms every invocation aborted before doing any work — silently
disabling vault locking for all non-Linux users.
Repro on Git Bash (MINGW64_NT, bash 5.2.12):
$ bash scripts/wiki-lock.sh acquire wiki/concepts/Foo.md
ERR: could not acquire meta-lock within 5s
$ echo $?
1
with_meta_lock() now uses flock where available and falls back to an
atomic mkdir(2) mutex otherwise, with the same 5s ceiling and a 30s reap
for a mutex abandoned by a crashed holder. Per-path acquire is already
race-safe via `set -o noclobber` (per the Design note in the header), so
the meta-lock only serializes LOCK_DIR mutation between commands.
Two details worth reviewer attention:
- The fallback runs the payload in the current shell rather than a
subshell, so die()'s exit() would skip cleanup and leak the mutex —
which then masked later path-validation failures as exit 1 instead of
exit 4. Handled with an EXIT trap around the payload.
- Added mtime_of() because BSD/macOS stat needs `-f %m` where GNU needs
`-c %Y`, and the fallback runs on exactly the platforms where that
differs. A GNU-only stat would yield 0 there, making every mutex look
infinitely stale and reaping it immediately.
Verified on Git Bash: acquire / contend(75) / stale-reap / list /
release / peek all behave per spec; path validation returns 4 for both
absolute and `..` paths; no lockfile or mutex residue after failures;
and 12 simultaneous acquires of one path yield exactly one winner.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Every
wiki-lock.shcommand routes throughwith_meta_lock(), which calledflock(1)unconditionally.flockships with util-linux and is absent on Git Bash / MSYS2 (Windows) and on a default macOS install, so on those platforms every invocation aborts before doing any work.The effect is quiet rather than loud: v1.7's per-file advisory locking — the fix for the v1.6 multi-writer corruption hole — is a no-op for every non-Linux user. Skills that follow the documented
wiki-lock acquire/releaseprotocol get a failure they're told to interpret as "locked by another writer", so they skip the write silently.Repro on Git Bash (MINGW64_NT-10.0, bash 5.2.12):
I hit this for real: two concurrent sessions writing the same vault collided on
hot.md, which is exactly the failure the lock exists to prevent.Fix
with_meta_lock()keepsflockwhere available and falls back to an atomicmkdir(2)mutex otherwise — same 5s ceiling (25 × 0.2s), plus a 30s reap so a crashed holder can't deadlock the vault.This preserves the existing design: per-path
acquireis already race-safe viaset -o noclobber(as the header's Design note says), so the meta-lock only serializesLOCK_DIRmutation betweenacquire/release/list/clear-stale.Two details worth your attention
1. EXIT trap. The fallback runs the payload in the current shell rather than a subshell, so
die()'sexit()unwinds past the cleanup and leaks the mutex. The flock branch never had this problem —exitonly left the( … ) 9>subshell and the lock released on fd close. Symptom before I caught it: a leaked mutex made the next invocation block 5s and report exit 1, masking a path-validation error that should have been exit 4.2.
mtime_of()helper. BSD/macOSstatneeds-f %mwhere GNU needs-c %Y— and the fallback runs on precisely the platforms where that differs. A GNU-onlystatreturns empty on macOS, so the mutex age computes as 0, every mutex looks infinitely stale, and it gets reaped instantly — defeating the mutual exclusion it's meant to provide. Mirrors the existingsha1sum/shasumfallback insha1_of().Verification
On Git Bash (MINGW64, bash 5.2.12), against an isolated vault via
WIKI_LOCK_VAULT:bash -nsyntax checkpeekunheldunheld, exit 0acquireacquirewhile freshacquire --stale-after-sec 0listrelease→peekunheld..traversalacquireof one pathThe
flockbranch is untouched, so Linux behaviour is unchanged.Note on a related issue (not in this PR)
skills/save/SKILL.mddeclaresallowed-tools: Read Write Edit Glob Grep— noBash— while the skill body instructsbash scripts/wiki-lock.sh acquire "$NOTE_PATH". The skill therefore cannot acquire the lock its own documentation mandates, independently of this bug.skills/wiki-ingest/SKILL.mdmay want the same check. Happy to send that as a separate PR if you'd like it.🤖 Generated with Claude Code