fix(guard): shared lock for guards.json write-race - #288
Merged
Conversation
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Phase 1 of the guards.json write-race fix. mkdir is atomic on both macOS and Linux (flock has no CLI on macOS, only the syscall), so it's used as the lock primitive with a staleness timeout so a crashed holder can't wedge the lock permanently. Standalone script (lib/guards-lock.sh), not a sourced function -- Operation 12's guards.json mutation is inline bash issued per-turn by the LLM with no backing script, so it can't share a shell process with install-guards.sh's separate invocation.
Phase 2 of the guards.json write-race fix. enable/disable/profile's jq-mutate step and the mute-expiry auto-sweep (when it actually clears a mute) now acquire lib/guards-lock.sh before the read-modify-write and release immediately after, with a trap EXIT cleanup so a mid-write failure can't leak the lock.
Phase 3 of the guards.json write-race fix -- the second writer found during adversarial review. Both the fresh-create branch and the per-guard merge loop now acquire lib/guards-lock.sh before their read-modify-write and release immediately after, with trap EXIT cleanup. The merge loop keeps a separate acquire/release per guard iteration rather than one lock held across the whole loop, so a concurrent Operation 12 mutation isn't blocked for the full duration. Verified against a scratch HOME (fresh-create + idempotent re-run).
Phase 4 of the guards.json write-race fix. Formalizes this session's ad hoc falsification test (2 concurrent jq-mutate loops, 40 writes each, scratch copy) as tests/test_guards_registry_concurrency.sh: - Case 1: two concurrent writers, distinct fields -- zero lost updates. - Case 2: an Operation-12-style writer concurrent with an install-guards.sh-style merge writer against the same scratch file -- proves the shared lock, not just per-writer self-consistency. - Case 3: a pre-staged stale lock dir is force-broken within the timeout rather than hanging. Every case runs against a mktemp -d scratch copy; a hard assert_not_real guard aborts the suite if any path ever matches the real ~/.claude/guards.json. Red-first verified: MODE=unlocked reproduces lost updates on the pre-fix code path across repeated runs; MODE=locked (default) passes 7/7 with zero lost updates. Also fixes a bug found while red-lining the suite: lib/guards-lock.sh logged a spurious "breaking stale lock" warning when a lock dir vanished between its existence check and its stat call (the holder releasing normally, not a crash) -- now retries immediately instead of misreporting staleness.
Phase 5 of the guards.json write-race fix. skills/dev/git/SKILL.md Operation 12 no longer claims to be the sole sanctioned mutator of guards.json -- corrected to name both writers (Operation 12 and install-guards.sh) and the shared lib/guards-lock.sh lock that now coordinates them, per the second-writer finding from adversarial review. Mirrored [Unreleased] Fixed entries added to CHANGELOG.md and docs/CHANGELOG.md.
2 tasks
Data-Wise
added a commit
that referenced
this pull request
Jul 15, 2026
All three guard-hardening PRs merged to dev with green CI: #287 (cd-target resolution), #288 (guards.json lock, no CHANGELOG conflict with #287 despite both touching it), #289 (orchestrate-dispatch hardening). Worktrees removed, local branches deleted via safe `-d` (squash-merge content recognized without needing force). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Data-Wise
added a commit
that referenced
this pull request
Jul 16, 2026
Three claims in GRILL-branch-guard-target-resolution-2026-07-14.md are now stale given this session's guard-hardening work: the disproven no-switch-guard "compound-string-match/worktree-cleanup" framing, the concurrency-safety item marked deferred (now fixed in #288, broader than originally scoped), and the cd/-C cumulative-tracking question marked unresolved (now locked and shipped in #287). Added forward pointers to the actual resolutions rather than leaving a future session to re-derive them from a stale doc. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Data-Wise
added a commit
that referenced
this pull request
Jul 16, 2026
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
1 participant
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.
Summary
PR B of
SPEC-guard-hardening-adversarial-review-2026-07-15.md. Closes a confirmed lost-update race in~/.claude/guards.json's mutation path.jq ... > tmp && mv tmp filepattern with no locking.scripts/install-guards.sh's seed/merge path uses the identical unsafe pattern —skills/dev/git/SKILL.mdOperation 12's "sole sanctioned mutator" claim was not accurate.Changes
lib/guards-lock.sh— mkdir-based lock (atomic on both macOS and Linux;flockhas no CLI on macOS, confirmed) with a staleness timeout so a crashed caller can't wedge the lock permanently.skills/dev/git/SKILL.mdOperation 12 (enable/disable/profile) andscripts/install-guards.sh(fresh-create + per-guard merge loop, each iteration independently lock-guarded so a concurrent Operation 12 mutation isn't blocked for the full loop).tests/test_guards_registry_concurrency.sh— 3 cases, all against amktemp -dscratch copy (hardassert_not_realguard, never the real~/.claude/guards.json): concurrent-writer race, cross-writer (Operation 12 + install-guards.sh) shared-lock proof, stale-lock-timeout recovery.skills/dev/git/SKILL.md— corrected the "sole sanctioned mutator" claim to reflect the shared lock and both writers.CHANGELOG.md+docs/CHANGELOG.md)[Unreleased]entries.Test plan
tests/test_guards_registry_concurrency.sh— 7/7, red-first verified:MODE=unlockedreliably reproduces lost updates (e.g. 17/40+23/40, never 40/40+40/40),MODE=locked(default) always lands 40/40+40/40 across repeated runspython3 -m pytest tests/— 1678 passed, 33 skipped, 1 xpassed, 1 failed (pre-existing, environmental — same ambientguards.jsonmute unrelated to this branch, confirmed by inspection)stat, not an actual crash)SendMessagerather than a fresh dispatch, which committed cleanly🤖 Generated with Claude Code