Skip to content

fix(sync): keep lock refs fast-forwardable and push them apart from the WAL - #30

Open
tgenov wants to merge 3 commits into
neul-labs:mainfrom
tgenov:upstream/lock-ref-sync
Open

tgenov wants to merge 3 commits into
neul-labs:mainfrom
tgenov:upstream/lock-ref-sync

Conversation

@tgenov

@tgenov tgenov commented Oct 1, 2026

Copy link
Copy Markdown

Summary

Re-acquiring a lock whose ref already exists on origin broke grite sync:
the push failed with NotFastForward (-11) and, because it was all-or-nothing,
took the WAL down with it. Released and collected locks never left origin, so
the next pull brought them back, and nothing ever reconciled a diverged lock
ref. This PR keeps lock-ref history linear, pushes locks apart from the WAL,
and reconciles them by git ancestry plus lock liveness. A live lock held by
another actor keeps its mutual-exclusion meaning across clones.

Independent of #28 and #29; based directly on main (33afee0).

Root cause

  1. Every replacement lock was a new root. acquire over an expired lock
    deleted the ref and wrote a parentless commit. Once origin had the ref, any
    later acquire of that resource — any actor, including the same one after
    TTL — produced history unrelated to origin's.
  2. One push for all refs/grite/*. libgit2 checks fast-forwards
    client-side before sending anything, so one diverged lock ref failed the
    whole push — including a WAL that would have fast-forwarded — and it
    surfaced as a hard Err, not a per-ref rejection.
  3. Pull never reconciled. The non-forced fetch leaves a diverged ref where
    it is.
  4. Release/gc were local-only. Both deleted the local ref. The remote copy
    survived and came back on the next pull.

The same facts (2) and (3) meant push_with_rebase never rebased: the
push error escaped through ? before the rebase branch, and the rebase relied
on a pull that does not move a diverged WAL. Two clones that both wrote events
could not sync at all, whatever their locks were doing. It is fixed here
because the new push path makes it a few lines, and leaving it would mean "the
WAL is never blocked" was only true for one writer.

Reproduction (before, 0.5.3)

$ (cd a && grite lock acquire path:src --ttl 2 && grite issue create ... && grite sync --push)
Pushed to origin
$ sleep 3
$ (cd a && grite lock acquire path:src --ttl 60 && grite issue create ... && grite sync --push)
error: internal error: Git error: cannot push non-fastforwardable reference; class=Reference (4); code=NotFastForward (-11)
origin wal 434493e…  local wal 92f1f3e…      <-- WAL never reached origin

Fix

Lock history stays linear (lock_manager.rs).

  • acquire writes a replacement as a child of the expired or released lock,
    swapping the ref with a compare-and-swap. Previously it deleted then
    created, which also left a gap that a concurrent acquirer could slip into.
  • release writes a tombstone (expires_unix_ms = 0, exactly what
    docs/locking.md already specified) on top of the lock instead of deleting
    it, so there is something to propagate. read_lock/list_locks treat a
    tombstone as no lock.
  • gc keeps tombstones while a remote exists.

Sync decides per ref (sync.rs, new lock_sync.rs). Pull and push both
download the remote's grite objects without moving refs, then compare tips:

  • WAL and snapshots fast-forward, in their own push. A diverged WAL is
    rebased explicitly onto the remote head (local-only events re-appended, as
    the original code intended) and pushed again.
  • Lock refs go in a second push, planned by one function shared by
    pull and push:
Local vs remote Outcome
remote is ancestor of local local wins: live → push fast-forward; expired/tombstone → delete remote, then local
local is ancestor of remote remote wins: live → adopt; expired → delete both
unrelated, local live, remote expired or same owner (later expiry) re-parent local onto remote tip ([local, remote]), push fast-forward
unrelated, otherwise remote wins; a lost live local lock is reported in lock_conflicts
only local, live pushed if owned by an actor of this repository, else dropped (it came from the remote, which has since released it)
only remote, expired deleted from the remote

Mutual exclusion. Every remote change is a compare-and-swap on the tip
that was inspected. push_refs re-reads the advertisement on the connection
it pushes over
and skips a ref that moved. libgit2 sends that advertised oid
as the old value, so receive-pack rejects a ref that moves after that. A remote
lock that is live and owned by another actor is therefore never replaced or
deleted, unless the local lock was written on top of it. That only happens if
acquire/renew/release saw it as expired or as its own. A raced lock is
re-planned once on the retry.

"Actors of this repository" (from <commondir>/grite/actors) matters because
linked worktrees share refs: a sibling worktree's unpushed lock must not be
mistaken for a stale copy of a released remote lock.

Reporting. PullResult/PushResult gain lock_conflicts. It appears in
sync JSON for the CLI and the daemon, plus a Lock conflict: … line in human
output. Lock conflicts never make push_success false.

Daemon path. The daemon's Sync handler uses the same SyncManager, so
it is fixed by construction (covered by a test). Lock commands never route
through the daemon.

Changed files

File Change
crates/libgrite-git/src/lock_manager.rs linear history on acquire, CAS replace, release tombstone, gc keeps tombstones; 3 unit tests
crates/libgrite-git/src/lock_sync.rs new — per-lock-ref plan shared by pull and push
crates/libgrite-git/src/sync.rs download-then-decide pull/push, separate WAL and lock pushes, CAS push_refs, explicit WAL rebase
crates/libgrite-git/src/lib.rs export LockConflict
crates/grite/src/commands/sync.rs, crates/grite-daemon/src/worker.rs lock_conflicts in output
docs/locking.md, docs/cli-json.md document the semantics
crates/grite/tests/lock_sync.rs new — 11 integration tests

SyncManager::push/sync signatures are unchanged.

Validation

cargo test --all → 174 passed, 0 failed on this branch (3 new unit +
11 new integration tests). cargo fmt --all -- --check and
cargo clippy --all-targets --all-features -- -D warnings clean. No leaked
daemons or sockets after a full run.

Requirement Test On main (33afee0)
re-acquire, same actor reacquire_after_expiry_pushes_cleanly NotFastForward
same, through the daemon reacquire_after_expiry_pushes_cleanly_through_the_daemon NotFastForward
takeover by another actor, with/without pull another_actor_takes_over_an_expired_lock NotFastForward
negative: live lock of another actor still conflicts; WAL still pushed live_lock_of_another_actor_still_conflicts whole push fails
release propagates (origin and other clones) release_propagates_to_origin ref stays on origin
gc/expiry propagates gc_and_expiry_propagate_to_origin ref stays on origin
pull reconciles diverged expired lock pull_reconciles_a_diverged_expired_lock stays diverged
self-heal of unrelated expired refs on both sides diverged_expired_locks_self_heal_on_sync NotFastForward
healing never replaces a live legacy lock self_heal_never_replaces_a_live_legacy_lock push fails
diverged WAL rebased diverged_wal_is_rebased_on_sync NotFastForward
sibling worktree's unpushed lock survives another actor's sync sibling_worktree_lock_survives_a_sync_from_another_actor passes (guards the new rule)

10 of the 11 fail against main (verified by running this test file on 33afee0).

Manual repro (after), built binary, scratch bare origin + clones

== wait, A re-acquire + new issue, push
Pushed to origin
origin wal f47b32d…  local wal f47b32d…

Extended run (GRITE_DAEMON_SOCKET on a private socket for the daemon step):

### 1. daemon mode: original repro
{"push_success":true,"message":"Push successful","lock_conflicts":[]}
   wal a=6b3bbce origin=6b3bbce | lock a=bbeed74 origin=bbeed74
### 2. B (stale view) acquires A's live lock and pushes
{"success":true,"message":"Push successful after rebase (1 events rebased)",
 "lock_conflicts":[{"resource":"path:src","owner":"11260f53…","expires_in_ms":599784}]}
   origin lock unchanged; B now refused: "Lock on path:src is held by 11260f53… (expires in 599s)"
### 3. A releases, syncs; B pulls
   origin locks after release: []   b locks after pull: []
### 4. self-heal: state produced by the OLD binary (diverged expired roots, WAL stuck)
   old: NotFastForward (-11)
   new, plain `grite sync`: {"push_success":true,...}  locks d=[] origin=[]  wal d=origin
### 5. old client pulls and pushes after new clients changed lock refs
   old pull ok, sees the new lock; old push ok, wal c=origin

Compatibility

  • Wire/ref format unchanged. Same ref names and lock.json. A tombstone
    is an ordinary expired lock, and a re-parented lock is just a two-parent
    commit.
  • Old clients against new refs: new history only ever fast-forwards, so
    old clients' non-forced fetch follows it. They read tombstones as expired
    locks. Two limits stay as before: old clients do not prune refs the remote
    deleted, and their own acquire over an expired lock still writes a root
    commit. Their push of such a ref can therefore still fail as before — or,
    now that new clients delete stale remote refs, simply recreate an expired
    ref, which the next new-client sync deletes again.
  • Existing broken repositories heal with a plain grite sync from a new
    client. No manual ref deletion is needed.
  • Cost: pull/push now download before deciding, and lock refs are a
    second push, so a full sync makes four connections instead of two.

Notes for reviewers

  • A stale copy of a released lock is dropped on pull only if its owner is not
    an actor of this repository. For a repository's own actors, "unpushed" and
    "released elsewhere" cannot be told apart without remote-tracking state. In
    that case the lock is kept and pushed back, the conservative side, until it
    expires. This only happens when the same actor id is used from two clones.
  • The libgit2 local (path) transport used by the tests does not exercise
    receive-pack's old-oid check; the compare-and-swap on ssh/https relies on it.

🤖 Generated with Claude Code

tgenov and others added 3 commits October 1, 2026 08:36
`acquire` over an expired lock deleted the ref and wrote the new lock as a
parentless root commit. Once origin had that lock ref, every later acquire of
the same resource - by any actor, including the same actor after its TTL -
produced history unrelated to origin's, and the next `sync --push` failed:

    cannot push non-fastforwardable reference ... NotFastForward (-11)

`release` deleted only the local ref, so the remote copy outlived it and the
next pull brought the released lock back.

- `acquire` writes the replacement lock as a child of the expired or released
  one, and swaps the ref with a compare-and-swap on the tip it inspected
  instead of delete-then-create. A concurrent acquirer in the same repository
  now loses cleanly rather than racing through the gap between the two.
- `release` writes a tombstone (expiry=0, as docs/locking.md always described)
  on top of the lock instead of deleting the ref, so the release has something
  to propagate. `read_lock` and `list_locks` treat a tombstone as no lock.
- `gc` keeps tombstones while the repository has a remote: they are how a
  release reaches it, and `sync` removes them once it has.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…veness

`push` sent every `refs/grite/*` ref in one libgit2 push. libgit2 checks
fast-forwards client-side before sending anything, so one diverged lock ref
failed the whole push with `NotFastForward` - including a WAL that would have
fast-forwarded - and returned it as a hard error. `pull` used a non-forced
fetch, which leaves a diverged ref where it is, so nothing ever reconciled it.

The same two facts meant `push_with_rebase` never rebased anything: the push
error escaped through `?` before the rebase branch, and the rebase relied on
a pull that does not move a diverged WAL. Two clones that both wrote events
could not sync at all.

Both directions now download the remote's grite objects without moving any
ref, compare tips, and decide per ref:

- WAL and snapshots fast-forward, in their own push. A diverged WAL is
  rebased explicitly onto the remote head (local-only events re-appended, as
  before) and pushed again.
- Lock refs go in a second push and are planned by `lock_sync::plan`, shared
  by pull and push, from git ancestry plus lock liveness. A diverged local
  lock is re-parented onto the remote tip only when the remote lock is
  expired or has the same owner; expired and released locks are deleted on
  both sides; a lock owned by an actor outside this repository that the
  remote no longer has is dropped, never pushed back.
- Every remote change is a compare-and-swap on the tip that was inspected:
  each push re-reads the advertisement on the connection it pushes over and
  skips a ref that moved, and receive-pack rejects one that moves after that.
  A live lock held by another actor is therefore never replaced or deleted.
  It is reported as a lock conflict (`lock_conflicts` in the sync output) and
  replaces the local lock. It never fails the WAL push.

Repositories already in the broken state - unrelated, expired lock refs on
both sides - heal through a normal sync with no manual ref deletion.

The daemon runs sync through the same `SyncManager`, so it gets the fix too;
lock commands never route through the daemon.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Eleven tests driving the real binary against a bare origin and two clones
with distinct actors: re-acquire after expiry by the same actor (in-process
and through the daemon) and by another actor, with and without a pull first;
a live lock of another actor still conflicting while the WAL is pushed;
release and gc propagating to origin and to other clones; pull reconciling a
diverged expired lock; self-heal of unrelated expired lock refs on both
sides; a live legacy lock never replaced by healing; a diverged WAL rebased;
and a linked worktree's unpushed lock surviving another actor's sync.

All ten that exist before the sibling-worktree case fail against 05d9d3e.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant