Repository navigation
Conversation
Deicyde
left a comment
There was a problem hiding this comment.
The CAS mechanics are careful, and all 241 focused tests pass, but I found six protocol/safety gaps on the exact head:
-
A relative local
originis kept lexical, then the pinned worktree is closed beforepin_claim_repository()resolves it (__main__.py:729-732,1132-1137). An ancestor A-B-A swap in that gap madeclaim acquirewrite the claim ref to a different repository. Resolve and pin the derived local repository while the worktree binding is still retained. -
Markerless scratch migration is destructive by shape alone (
claims.py:1042-1077,1619-1631). Pointing--scratchat an unrelated empty bare repo silently installed Autoform's marker and erased custom config; a normal bare repo withrefs/heads/mainwas still modified before the command rejected it. Validate owned namespaces and legacy-specific evidence before any write, or require explicit migration. -
Claim refs may be symbolic.
ls-remoteatclaims.py:1677-1685reports the target OID without preserving that fact, then push at2326-2339dereferences the ref. In a supported local bare repository, making a claim ref point atrefs/heads/maincaused acquire to createmainand release to delete it. Refuse symbolic claim refs or use a mutation mechanism that cannot follow them. -
release()has no exact lease argument and trusts the latest receipt for the whole worktree (claims.py:2748-2768). After process A's lease expired and process B reacquired the key in the same default session, delayedA.release()returned true and deleted B's successor. Carry thelease_id/ClaimFencethrough release just as heartbeat renewal does. -
The v1 compatibility scan is not atomic with the v2 CAS (
claims.py:2541-2553,2622-2661). Installing a live historical v1 ref after the post-push scan lets acquire return true with simultaneous v1/v2 owners;holds()immediately returns false. The migration fence needs an atomic multi-ref transition, or mixed-client operation must be rejected by a stronger invariant. -
Process cleanup sends
SIGKILLonly when the direct child survivesSIGTERM(claims.py:574-590). A leader that exits promptly while its grandchild ignoresSIGTERMleaves that grandchild running after timeout/output-limit cleanup. Kill/check the process group after the grace period regardless of the leader's status.
These are independent of the #28 dependency. Validation: tests/test_claims.py tests/test_claim_cli.py passed, 241 tests.
Summary
claim acquire,renew,release,list, andcleanupcommandsDepends on #28. The reviewable change is commit
e511514; earlier commits belong to the dependency stack.Validation
file://acquire/list/release smoke testsmake lintmake check-examplegit diff --check