fix(lock): discard stale pre-open observations - #193
Conversation
Keep ordinary Root reads and post-create admission strict while allowing bounded acquisition-only observation discard when a verified pre-open generation changes. Require exact descriptor and canonical ancestry evidence; never infer unlink, ownership, or removal authority. Add deterministic regression and boundary controls for the 0.7.1 release gate failure.
|
🦞👀 Pull request received. I will update this pull request when review starts. |
|
Independent Codex autoreview of the final frozen candidate completed scoped-clean at the configured P0 threshold, with no accepted/actionable findings. Reviewed and fully tested tree: The new receipt remains observation-only: the waiter cannot read/adopt the replacement, infer pathname absence or unlink, or acquire release/reclaim authority. Post-create admission remains on its separate unlinked-only policy. All 36 new controls and the wider 158-test owner/provenance/ancestry set passed, followed by the full 6,364-test native-backed check and 6,000 contended acquisitions. |
|
Codex review: needs maintainer review before merge. Reviewed September 1, 2026, 1:46 PM ET / 17:46 UTC. ClawSweeper reviewWhat this changesThe PR lets a Root-backed asynchronous lock contender discard a verified stale pre-open observation and retry exclusive creation without reading, adopting, or removing the successor lock. Regression provenancePossible regression — probable (reviewed change; failure trace). No predecessor PR is attributed. Merge readiness✅ Ready for maintainer review Keep open: this is a focused repair for a current-main Root-backed lock contention gap, with scoped identity checks and real-filesystem validation evidence; it remains necessary because current main lacks the new pre-open-change handling. Priority: P1 Review scores
Verification
How this fits togetherSidecar locks coordinate exclusive work on files beneath a capability-style Root. A contending acquirer samples and opens a lock record, validates its identity and ancestry, then either safely retries creation or evaluates the current holder. flowchart LR
A[Current lock holder] --> B[Contending lock waiter]
B --> C[Pre-open pathname sample]
C --> D[Opened descriptor identity check]
D --> E[Canonical ancestry recheck]
E --> F[Fresh exclusive lock creation]
Before mergeNone. Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Merge the narrowly scoped acquisition-only retry once the pending native checks complete, retaining the strict generic-read, ancestry, and creator-admission boundaries. Do we have a high-confidence way to reproduce the issue? Yes. The PR body supplies a deterministic current-main Root-async handoff scenario on real macOS and Linux filesystems, and the added focused test encodes that path; this read-only review did not execute it. Is this the best way to solve the issue? Yes. Recording a narrowly scoped stale-generation receipt for the acquisition retry path fixes the handoff without relaxing generic Root reads or granting lock ownership, reclaim, or deletion authority. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 3b425f1b2a1e. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
|
Landed as Exact-head CI, coverage, CodeQL, and benchmarks passed. This includes the macOS Node 24 fallback contention lane that failed on the preceding main commit, plus Linux, Windows, native, musl, and package-install checks. Before/after proof: deterministic pre-open handoff failed with the strict identity mismatch on macOS and isolated Linux; the final candidate passed 158 focused checks, 6,364 full native-backed tests (81 skipped), 84 security tests, 63 Rust tests, Clippy, docs, host package smoke, and 6,000 contended acquisitions. No test deadlines were increased. The final Codex autoreview was scoped-clean at the configured P0 threshold. Task-owned Linux proof was retained and its lease released. The 0.7.1 release remains untagged pending the exact merged-main gate. No published package or tag was replaced to resolve this failure. |
Problem and fix
The 0.7.1 preparation passed PR CI, but the merged-main gate caught a macOS Node 24 Root-async contention failure (
path-mismatch, 79/100 completed acquisitions). Publication is still paused: no release tag has been pushed.A deterministic regression on macOS and isolated Linux confirms a missing handoff case: a waiter samples holder A with pre-open lstat, A releases, and holder B creates the next record before the waiter opens it. Root correctly rejects the A/B identity mismatch. The lock contender previously had no way to discard this stale observation, even though it was not acquiring or removing anything from it.
Record a distinct, current-observation pre-open-change receipt only with strictly known, different, regular-file identities, no observed multiple links, and a stable descriptor reinspection. Only an EEXIST contention snapshot may consume that receipt, after the existing complete canonical-ancestry and current-leaf checks. It discards the observation and charges the normal retry budget; success still requires fresh exclusive creation. It does not read or adopt B's payload, infer that A was unlinked, or grant absence/removal/reclaim authority.
Generic Root reads remain strict. Post-create admission explicitly retains its narrower unlinked-only policy, and held/reclaim/cleanup reads do not opt into changed-observation discard. The creator's exact-byte/token check and original cleanup receipt remain unchanged. Production change: net +30 lines, with no public API, dependency, or native-code change.
Verification
pnpm check: 6,364 passed / 81 skipped.pnpm test:security: 84 passed; locked Rust tests: 63 passed; Clippy with warnings denied passed.Frozen tested tree:
f572891984b3d03d79220c4c20a4a323f580bc6a.The dated 0.7.1 changelog and lock documentation are updated as part of the explicitly requested in-progress release. All 0.7.0-and-older notes remain unchanged. The release tag will target latest main only after this fix lands and its prerequisites pass.