fix(lock): retry unlinked sidecar snapshots - #185
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
|
@clawsweeper automerge Special instructions: Require deterministic proof for unlink before and after identity admission, handle closure, a real two-manager owner-release/waiter-acquire handoff, replacement preservation, changed-root rejection, symlink/non-file/hardlink and operational probe failures, and create-only retry safety. Keep raw-path snapshots and generic Root behavior unchanged. Require fresh exact-head Node 22/24 Linux/macOS/Windows checks, three-OS coverage, native/package/audit/analysis/security gates, and no actionable review finding before merge. |
|
🦞✅ Source: What merged:
Automerge notes:
The automerge loop is complete. Automerge progress:
|
clawsweeper
Bot
added
clawsweeper:automerge
|
Codex review: passed. Reviewed August 31, 2026, 8:07 AM ET / 12:07 UTC. ClawSweeper reviewWhat this changesThe branch lets Root-backed sidecar-lock acquisition retry after a concurrently unlinked ownership record only when the same Root capability proves the path absent, with regressions and CI proof for handoff and replacement cases. Regression provenancePossible regression — probable (reviewed change; failure trace). No predecessor PR is attributed. Merge readinessKeep open: current main still propagates the Root Priority: P2 Review scores
Verification
How this fits togetherThe file-lock layer publishes a sidecar ownership record next to a protected target file. When exclusive creation collides, it reads that record through a Root capability, which enforces confinement and filesystem identity before acquisition either retries or fails closed. flowchart TD
A[Caller requests file lock] --> B[Exclusive sidecar create]
B -->|record exists| C[Root-backed snapshot read]
C --> D[Path and root identity checks]
D -->|absence proven| E[Retry exclusive create]
D -->|replacement or unsafe state| F[Fail closed]
E --> G[Verified held lock]
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Land the narrow sidecar-layer exception after the remaining exact-head hosted gates complete, retaining generic Do we have a high-confidence way to reproduce the issue? Yes. The introduced deterministic POSIX tests and compiled public-API trace define a high-confidence contention path, although this read-only review did not execute the pre-fix failure on current main. Is this the best way to solve the issue? Yes. The exception is placed in the sidecar ownership layer, not generic Root opening, and reuses the same guarded capability to distinguish confirmed absence from replacements or unsafe states. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against f4c5b321ea9f. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
History |
clawsweeper
Bot
added
the
clawsweeper:human-review
|
🦞✅ Source: Why human review is needed: What the maintainer can do as a next step: I added |
clawsweeper
Bot
removed
the
status: 📣 needs proof
|
@clawsweeper automerge Special instructions: Require exact-head hosted Node 24 Linux and macOS proof showing: one post-identity hook call; owner release while the waiter holds the opened descriptor; Root.open records Keep the production owner boundary unchanged in |
clawsweeper
Bot
added
proof: sufficient
|
Landed proof for exact head Local verification:
Exact-head hosted verification:
Merged by |
1 participant
Summary
path-mismatchRoot cause
A waiter could open and identity-check an existing sidecar lock while its owner was releasing it. If the owner unlinked the pathname before
Root.open()completed opened-path resolution, the still-valid descriptor had no resolvable name andRoot.open()correctly threwFsSafeError("path-mismatch").readSidecarLockSnapshot()propagated that generic Root policy error, so normal ownership turnover could abort acquisition instead of behaving like an absent lock and retrying create-only publication.The ownership distinction belongs in the sidecar layer, not in
Root.open(): generic Root callers must continue to reject unlinked or ambiguously named descriptors.Fix
For Root-backed snapshots,
readSidecarLockSnapshot()now catches onlyFsSafeError("path-mismatch")from the initial open and callslockRoot.stat(relative)through the same capability. It returnsnullonly when that second operation provesFsSafeError("not-found")under the still-valid root. Any existing replacement, changed root, symlink/non-file/hardlink policy result, rawENOENT, or operational failure preserves the original mismatch.Returning
nullafter an observed absence is safe even if the name is recreated immediately afterward: the acquisition loop retries exclusive create, which either creates the new ownership record or rejects the replacement as contention.Compiled runtime proof
A standalone proof now runs the compiled public file-lock and Root APIs against real filesystem operations. It pauses the waiter after pathname identity admission, releases the owner, verifies same-capability absence recovery and create-only reacquisition, then separately installs a replacement after open and verifies
path-mismatchplus exact preservation:{"proof":"sidecar-unlinked-snapshot","platform":"darwin","handoff":{"hookCalls":1,"waiterOwner":"waiter","payload":{"owner":"waiter","scenario":"handoff"},"verified":true,"openedHandleClosed":true,"finalMissing":true},"replacement":{"hookCalls":1,"error":{"name":"FsSafeError","code":"path-mismatch"},"openedHandleClosed":true,"replacementPreserved":true,"originalPreserved":true}}The same proof runs on exact-head Node 24 Linux and macOS CI.
Verification
CI=1 pnpm check: 6,086 tests passed, 76 skipped; build, docs, filesystem-boundary lint, file-size budget, package, and API checks passedgit diff --check: passedExact-head hosted Linux, macOS, Windows, coverage, package, analysis, and ClawSweeper review remain the landing gates. No release or package publication is part of this PR.