Skip to content

tcfs-sync: record Conflict for out-of-band-diverged refs (TIN-2584)#540

Merged
Jesssullivan merged 2 commits into
mainfrom
fix/tin-2584-divergent-ref-conflict
Jul 7, 2026
Merged

tcfs-sync: record Conflict for out-of-band-diverged refs (TIN-2584)#540
Jesssullivan merged 2 commits into
mainfrom
fix/tin-2584-divergent-ref-conflict

Conversation

@Jesssullivan

Copy link
Copy Markdown
Owner

TIN-2584: out-of-band-diverged refs never record a Conflict

Defect (traced at origin/main e5d913d)

A .git/refs/heads/* file rewritten out-of-band by git commit never ticks the TCFS vclock, so the diverged file carries its stale, last-synced clock into reconciliation. Three layers turned that into a silent, invisible divergence:

  1. ROOTcompare_both_exist (crates/tcfs-sync/src/reconcile.rs) freshly hashes the local file but read the tracked vclock verbatim (let local_vclock = tracked.map(|s| s.vclock.clone())). The fresh hash was never compared against tracked.blake3, so a locally-diverged ref was classified on its stale, possibly-dominated clock.
  2. AMPLIFIERcompare_clocks (crates/tcfs-sync/src/conflict.rs:205-214): with differing hashes, {neo:1} vs {neo:2} = Ordering::LessRemoteNewer; Conflict fires only on Equal | None. A diverged-but-dominated ref was structurally unreachable as a Conflict, so the incoming pull path parked it with no ConflictInfo recordedtcfs conflicts stayed empty and tcfs resolve keep-both had nothing to act on.
  3. RACEclassify_path's (Some, None, Some) arm (reconcile.rs) classified UpToDate even when the live local hash ≠ tracked.blake3: a diverged ref became a silent no-op whenever the peer's freshly-pushed index entry was not yet visible in the bulk LIST (S3 read-after-write lag, observed live at +0.5s after the peer's push).

Canary provenance (neo/honey, 2026-07-07)

Baseline {neo:1} pulled to honey; honey commits; neo commits {neo:2} and pushes; honey reconciles 0.5s later → pushed=1 pulled=4 conflicts=0, ref state frozen at baseline blake3 bc0807b8 / {neo:1}. Divergence invisible on both hosts.

Fix (mirrors engine.rs:1554 local_edit_inferred -> tick)

  1. compare_both_exist — when tracked.blake3 != live local hash and the tracked clock is strictly dominated by the remote (Ordering::Less), tick a clone of the clock before compare_clocks. {neo:1}{neo:1,honey:1}, incomparable with {neo:2}Conflict, recorded → keep-both works. The tick lands only on the comparison clone; the stored state clock is untouched, so re-recording each cycle stays idempotent. reclassify_git_ff_conflicts decides on git merge-base --is-ancestor (git SHAs), not clocks, so it is unaffected.
  2. (Some, None, Some) arm — a ref-class path whose live content diverges from tracked routes to a conflict-aware Push instead of a silent UpToDate. The engine push path re-resolves the exact index key via a direct GET (stronger read-after-write than the bulk LIST that missed it) and records a Conflict (remote concurrently ahead) or uploads (remote genuinely dropped it) — never a silent freeze, never a blind clobber. The hash-equal case keeps the existing orphan behavior.

Deliberate deviation from the prescribed fix shape

The ticket prescribed ticking whenever tracked.blake3 != local_hash (unconditionally). I narrowed the tick to the strictly-dominated (Ordering::Less) case. The unconditional tick also fires on the equal-clock divergence and promotes a divergent non-head ref (tag / submodule head / detached HEAD) from Conflict to LocalNewerPush, which defeats the fail-closed fast-forward veto and breaks three security-regression tests (git_ff_divergent_tag_vetoes_whole_group, git_ff_divergent_submodule_ref_vetoes_whole_group, git_ff_divergent_detached_head_vetoes_whole_group). Those tests rely on an equal-clock divergent ref staying a Conflict. Narrowing to Less:

  • fixes the exact TIN-2584 defect (the dominated case that was mis-read as RemoteNewer / silently parked);
  • leaves the equal-clock divergence as-is — it was already a recorded Conflict, which is the FF-veto precondition and already satisfies TIN-2584's acceptance (divergence recorded, not silent);
  • changes zero existing test behavior — no security guard modified;
  • keeps the blast radius minimal for a .git write path.

Test evidence (cargo test -p tcfs-sync, hermetic: GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null)

New tests:

  • 5 hermetic classifier unit tests in reconcile.rs: dominated -> Conflict, equal-clock -> stays Conflict (no LocalNewer promotion), local-clock-ahead -> Push, remote-absent + diverged -> not UpToDate (Push), remote-absent + unchanged -> UpToDate.
  • 1 integration test in git_ff_resolution.rs: git_out_of_band_dominated_ref_records_conflict — the canary shape end-to-end (dominated head ref → Conflict recorded, zero writes to the ref), FF reclassifier off so the Conflict can only originate at the vclock layer.
lib:                 test result: ok. 267 passed; 0 failed; 0 ignored
  (incl. tin2584 subset) test result: ok. 5 passed; 0 failed

git_ff_resolution:   test result: ok. 13 passed; 0 failed
  test git_ff_divergent_tag_vetoes_whole_group ... ok
  test git_ff_divergent_submodule_ref_vetoes_whole_group ... ok
  test git_ff_divergent_detached_head_vetoes_whole_group ... ok
  test git_ff_converges_push_then_pull ... ok
  test git_divergent_stays_conflict ... ok
  test git_ff_concurrent_clocks_push_uploads_and_converges ... ok
  test git_ff_remote_ahead_concurrent_clocks_reclassifies_to_pull ... ok
  test git_ff_push_defers_when_local_ref_moves_after_plan ... ok
  test git_ff_pull_skips_when_local_ref_moves_after_plan ... ok
  test git_object_failure_bars_ref_actions_sequential ... ok
  test git_object_failure_bars_ref_pull_concurrent ... ok
  test git_ff_planning_fabricates_nothing_under_root ... ok
  test git_out_of_band_dominated_ref_records_conflict ... ok   <-- new

full suite:          exit 0 (all integration suites green: e2e_*, push_pull,
                     resolve_conflict, two_device_sync, symlink, state_cache, ...)

cargo clippy -p tcfs-sync --tests: clean apart from one pre-existing warning at reconcile.rs .unwrap_or_else(VectorClock::new) (present verbatim on origin/main, in code this PR does not touch).

Do NOT merge yet — this is a .git write path and warrants an adversarial review pass first.


This PR owns crates/tcfs-sync — coordinate before stacking commits (AGENTS.md Agent Coordination).

A `.git/refs/heads/*` file rewritten out-of-band by `git commit` never
ticks the TCFS vclock, so the diverged file carries its stale, last-synced
clock into reconciliation. Three layers turned that into a silent,
invisible divergence:

  ROOT: compare_both_exist (reconcile.rs) freshly hashes the local file
  but read the tracked vclock verbatim, so a locally-diverged ref was
  classified on a stale, possibly-dominated clock -- the fresh hash was
  never compared against tracked.blake3.

  AMPLIFIER: compare_clocks (conflict.rs) maps {neo:1} vs {neo:2} to
  Ordering::Less -> RemoteNewer; Conflict fires only on Equal|None. A
  diverged-but-dominated ref was structurally unreachable as a Conflict,
  so the incoming pull path parked it with no ConflictInfo recorded --
  `tcfs conflicts` stayed empty and keep-both had nothing to act on.

  RACE: classify_path's (Some, None, Some) arm classified UpToDate even
  when the live local hash != tracked.blake3, so a diverged ref became a
  silent no-op whenever the peer's freshly-pushed index entry was not yet
  visible in the bulk LIST (S3 read-after-write lag).

Fix (mirrors engine.rs local_edit_inferred -> tick):

  1. compare_both_exist: when tracked.blake3 != live local hash AND the
     tracked clock is strictly dominated by the remote (Ordering::Less),
     tick a CLONE of the clock before compare_clocks. {neo:1} becomes
     {neo:1,honey:1}, incomparable with {neo:2} -> Conflict, recorded.
     Narrowed to the strictly-dominated case on purpose: the equal-clock
     divergence is already a Conflict, and ticking it would promote a
     divergent non-head ref to LocalNewer, defeating the fail-closed
     fast-forward veto (git_ff_resolution BLOCKER-1/3). The tick lands
     only on the comparison clone, so re-recording stays idempotent
     (no per-cycle clock growth) and reclassify_git_ff_conflicts, which
     decides on git-SHA ancestry rather than clocks, is unaffected.

  2. (Some, None, Some) arm: a ref-class path whose live content diverges
     from tracked routes to a conflict-aware Push instead of a silent
     UpToDate; the engine push path re-resolves the exact index key via a
     direct GET (stronger read-after-write than the bulk LIST that missed
     it) and records a Conflict or uploads -- never a silent freeze. The
     hash-equal case keeps the existing orphan behavior.

Deviation from the prescribed fix: the ticket asked to tick whenever
tracked.blake3 != local_hash. That unconditional tick also fires on the
equal-clock divergence and promotes a divergent non-head ref (tag,
submodule head, detached HEAD) from Conflict to LocalNewer -> Push,
breaking the three fail-closed FF-veto regression tests. Narrowing the
tick to Ordering::Less fixes the exact TIN-2584 defect, changes zero
existing test behavior, and touches no security guard.

Tests: 5 hermetic classifier unit tests in reconcile.rs
(dominated->Conflict, equal-clock->stays Conflict, local-clock-ahead->
Push, remote-absent+diverged->not UpToDate, remote-absent+unchanged->
UpToDate) plus a git_ff_resolution.rs integration test reproducing the
canary shape (dominated head ref -> Conflict recorded, zero writes to
the ref, FF reclassifier off so the Conflict can only come from the
vclock layer). All existing git_ff_resolution (12) and loser_guard tests
stay green.

Canary provenance: neo/honey 2026-07-07 -- baseline {neo:1} pulled to
honey; neo pushed {neo:2}; honey reconciled 0.5s later with pushed=1
pulled=4 conflicts=0, ref frozen at baseline. Divergence invisible on
both hosts.
@Jesssullivan

Copy link
Copy Markdown
Owner Author

Adversarial merge-gate (4 lenses + judge) verdict: MERGE. Key confirmations: the plan path hashes unconditionally (mtime/size quick-check is execute-fast-path only), so the fix demonstrably catches the live canary shape — revert-proven by git_out_of_band_dominated_ref_records_conflict failing on base / passing with the fix. Hermetic re-run: 267 lib + 13 git_ff_resolution green; no test weakened (the one replaced classifier test was strengthened).

Scope honesty (per gate): this fix covers the common dominated-clock divergence (the canary case). One pre-existing residual corner remains: if a device's own tracked self-clock has regressed below the remote's record of it (state.db reset/recovery), the post-tick clock can still compare Less → RemoteNewer silent pull. Fast-follow filed to force incomparability (insert(device, max(remote[dev], local[dev])+1)) + regression test. Additional follow-ups tracked: needs_sync same-second/size-equal blindness (affects tcfs unsync dirty-check, not this planner), non-git-file coverage test for the generalized tick, symlink-arm asymmetry.

Latest push: fmt fix + crossbeam-epoch 0.9.18→0.9.20 (RUSTSEC-2026-0204, new upstream advisory, lockfile-only).

@Jesssullivan Jesssullivan merged commit b566985 into main Jul 7, 2026
10 checks passed
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