fix(syncer): a behind sync clock no longer eats local edits (BEA-196)

[ssowonny]

TL;DR

• bdrive sync could report remote: pushed for a file, exit 0, and revert that file to the old bytes in the same run. An agent reports the task done; the work is gone. That's fixed.
• Root cause: nothing ever re-derived this device's lamport clock. Once it fell behind, it stayed behind forever — and since ordering puts lamport before wall-clock time, every op it wrote lost replay.
• restore now exits non-zero if the file doesn't hold the version you asked for, instead of printing a success line naming one it doesn't.
• restore --list marks exactly one row *. Starring every op that shared the current blob is what made this look like "duplicate head records per path" — it was one head rendered twice.
• Known gap: one acceptance criterion (an out-of-band delete coming back on the next sync) contradicts the delete design and is not implemented — details at the bottom, it needs a product call.

The fault

journal.Less orders on lamport before wall-clock time. So an op written today at a low lamport loses to a peer's op from this morning at a high one. Replay resolves the path to the peer's version, and materialize writes it back over your file — in the same cycle that really did push yours.

It was permanent because of where the clock was raised. st.Lamport was only ever raised from a pull's new tail, and pull returns nothing once a peer's journal is fully downloaded: it skips an object no larger than the local copy (o.Size <= localSize) and one whose bytes are identical (bytes.Equal). So a device whose sync.json sat below the journals in its own volume dir never caught up, however many times it synced.

clock behind  ──>  scan + push        ──>  replay              ──>  materialize
                   edit journalled at       peer's older op          head bytes written
                   a LOSING lamport,        still wins the path,     back over your file,
                   then really pushed       head never moves         in the SAME run

The cycle now opens by folding every op already in the volume's journal dir through absorbLamport. A derivation, not a fix at one site, because any desync produces this state: a crash between AppendOps and SaveSync, a restored backup, a pull that died on a 403. Two things it is careful about:

• Folded through absorbLamport, not taken as a raw max — a journal on disk holds a peer's bytes, and the maxLamport ceiling is what stops a hostile peer installing a permanent write lock.
• Derived after joining is read. joining gates the adoption path, so deriving first would turn a device whose first cycle crashed into a non-joiner and change what bdrive init adopts.

Second route in: the ErrForbidden arm of the pull returned before the absorb loop that sat below it — having already written the peer journals it did get, and then committing local ops under them. The absorb moved above the switch.

Third: conflict copies required a pull. A local op can lose to an op already on this disk — a peer op above maxLamport, which absorbLamport refuses by design — and a settled peer journal yields nothing on every later cycle, so that loss was reverted with no copy and no message, forever. The candidate set is now every unpushed local op that replay didn't resolve to, deduped on the copy's own path (a losing op stays unpushed and is re-examined every cycle, so without the dedupe it re-journals the same bytes forever).

Telling the user

Every surface said "healthy" throughout the incident, and each for a correct reason:

Surface	Why it couldn't say it	Now
status pending	len(myOps) - PushedOps — zero by construction after a successful push	a warning: line when the clock is below the local journals
status local (Drift)	compares disk against the cache materialize just made agree	unchanged; the warning is the new signal
restore success line	printed unconditionally after the cycle	re-reads the file, exits non-zero on mismatch
restore --list	starred every op sharing the current blob	exactly one *

Testing

Every fix has a test that was verified to fail first — I wrote them against unmodified code and confirmed each reproduces, including reverting individual fixes to check the test catches it:

• TestBehindClockConverges — the reported loss verbatim: before the fix, LocalOps=1 PulledOps=0 Materialized=1 and the file holds the peer's old bytes.
• TestBehindClockIsRederivedFromLocalJournals — asserts the clock comes back with PulledOps == 0, i.e. from disk and nothing else.
• TestForbiddenPullStillAbsorbsTheClock — re-verified against a temporary revert of that hunk: clock 0 is below the ops the aborted pull already wrote to disk (1).
• TestLosingEditIsPreservedWithoutAPull — the unabsorbable-lamport case, plus a second cycle asserting no duplicate copy.
• TestListMarksExactlyOneCurrentRow — re-verified against a temporary revert: prints two * rows, the reporter's "two head records" exactly.
• TestRestoreReportsWhatIsActuallyOnDisk, TestStatusNamesABehindClock — real cobra commands over a real folder.

go test ./... green (full suite, including internal/webapp). The acceptance criterion's go test ./internal/syncer/... ./cmd/bdrive/... is green. The whole existing syncer suite — adoption, conflict, merge, read-only, and the security suites — passes unchanged, which is the signal that mattered for a change to clock semantics.

Two things in the spec I did not implement

1. "An out-of-band delete is restored by the next sync, reporting files updated: 1." This contradicts the delete design: scan journals a delete for every cached path the walk did not see, which is how deleting a file locally deletes it for the team. I wrote this test, watched it fail, and traced the failure to the scan correctly minting a delete. Implementing the criterion as worded would mean a locally deleted file coming back — I'm not making that call in a bug fix. Needs a product decision; the clock fix stops new occurrences of the stuck-subtree symptom, and bdrive restore recovers an existing one.

2. Fault B — materializeFile trusting the cache before os.Stat. Not reachable on current main. scan drops the cache entry whenever it journals a delete, so cache-says-present + file-absent can't survive a scan — except on a read-only folder, where the entry is deliberately kept, and that path is already handled by readOnlyDrifted with TestReadOnlyFolderRestoresALocalDelete covering it. An unconditional stat per path per cycle (the daemon ticks every 3s) would be cost with no bug behind it.

Both are written up on the issue rather than shipped around.

Architecture changes

No types, fields, seams or relationships changed — the fix is behaviour inside cycleLocked and conflictCopies. But architecture/cli-sync.md draws the canonical cycle sequence in a note, and the cycle gained a step at the front, so that note is updated (and a second note added for why). Excerpt of the change:

✅ added · ❌ removed (strikethrough) · unmarked = unchanged

flowchart TB
    Session["<div style='text-align:left'><b>Session</b><br/>-cycleLocked(ctx) Result<br/>-conflictCopies(...)<br/>-pull(ctx, cache)</div>"]
    Seq["<div style='text-align:left'>internal/syncer — <span style='background:#22c55e55;padding:0 4px;border-radius:3px'>✅ re-derive the clock →</span> scan → commit local ops →<br/>pull peer journals → adopt on join → re-assert withdrawn ops →<br/>preserve conflicts → refresh rules → prune → materialize →<br/>push blobs then own journal</div>"]
    Why["<div style='text-align:left'>✅ The cycle OPENS by folding every op already in this volume's<br/>journal dir through absorbLamport. Nothing else re-derives<br/>st.Lamport — the pull absorbs only its NEW tail, and a settled<br/>peer journal yields none — so a device whose sync.json fell below<br/>the journals beside it stayed there forever, and since Less orders<br/>on lamport before time, every op it wrote lost replay (BEA-196).<br/>Derived AFTER `joining` is read, or adoption changes.</div>"]
    Session -.- Seq
    Session -.- Why
    classDef added fill:#22c55e22,stroke:#22c55e,stroke-width:2px
    classDef noteBox fill:#88888822,stroke:#888888,stroke-dasharray:2 2
    class Seq noteBox
    class Why added

Loading

Docs updated: README.md (restore + status rows) and web/docs/.../reference/cli.md (restore's new failure mode, the warning: line).

Closes BEA-196.

🤖 Generated with Claude Code