max / audiofiles

Sync A+: HLC no longer silently zeros a corrupt clock or writes empty state Ultra-fuzz --deep, Sync HLC defensive trio. - load_clock: a corrupt persisted last_hlc no longer resets to the zero clock (which sends the clock backward — the one place HLC monotonicity wasn't structurally guaranteed, risking a local edit losing to an already-committed row). It now seeds from physical time (forward of any sane past HLC) and logs; observe() advances it past anything seen on the next pull. First-sync (empty) still returns zero. - store_clock / set_committed / stamp_pending: replace serialize-failure unwrap_or_default() (which wrote an empty string load_clock would then read as a corrupt clock) with expect("Hlc always serializes") — Hlc is a plain derive, so this encodes the true invariant instead of a silent-corruption fallback. - Document the device_id (HLC node) uniqueness assumption convergence rests on. 64 sync tests pass (1 new corrupt-clock test); clippy clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: Max Johnson <me@maxj.phd> · 2026-06-23 16:54 UTC

Commit: 398b48cf2f7fb01279473efd5fdef37f97549324

Parent: 3e04818

1 file changed, +50 insertions, -7 deletions

M crates/audiofiles-sync/src/service/hlc.rs +50 -7

			@@ -8,6 +8,12 @@
8	8		//! All HLC logic lives here in the sync layer; the content store and DB schema
9	9		//! stay sync-agnostic. The device's `node` is its registered `device_id`; the
10	10		//! running clock is persisted as `sync_state.last_hlc`.
	11	+	//!
	12	+	//! Convergence rests on `device_id` (the HLC `node`) being unique per install: it
	13	+	//! is the final, deterministic tiebreak when two HLCs share `wall_ms`+`counter`.
	14	+	//! A restored backup that copies the `device_id` violates this; synckit mitigates
	15	+	//! the exact-HLC collision with a canonical-payload tiebreak, but unique
	16	+	//! `device_id`s are the design assumption.
11	17
12	18		use rusqlite::{Connection, OptionalExtension};
13	19		use synckit_client::Hlc;
			@@ -21,17 +27,31 @@ fn now_ms() -> i64 {
21	27		chrono::Utc::now().timestamp_millis()
22	28		}
23	29
24		-	/// Load the persisted device clock, falling back to the zero clock for `node`
25		-	/// (first sync, or unreadable state).
	30	+	/// Load the persisted device clock. Returns the zero clock on first sync (no
	31	+	/// persisted value); on a corrupt persisted value it seeds from physical time
	32	+	/// rather than zero — see below.
26	33		pub(crate) fn load_clock(conn: &Connection, node: Uuid) -> Hlc {
27	34		match get_sync_state(conn, "last_hlc") {
28		-	Ok(s) if !s.is_empty() => serde_json::from_str(&s).unwrap_or_else(\|_\| Hlc::zero(node)),
29		-	_ => Hlc::zero(node),
	35	+	Ok(s) if !s.is_empty() => serde_json::from_str(&s).unwrap_or_else(\|e\| {
	36	+	// Corrupt persisted clock: do NOT reset to zero. Zero sends the clock
	37	+	// backward, so a subsequent local edit could mint an HLC that loses to
	38	+	// an already-committed row — the one place HLC monotonicity isn't
	39	+	// structurally guaranteed. Seed from physical time instead (forward of
	40	+	// any sanely-timestamped past HLC); `observe()` advances it past
	41	+	// anything seen on the next pull.
	42	+	tracing::warn!("corrupt last_hlc ({e}); seeding clock from physical time");
	43	+	Hlc::tick(Hlc::zero(node), now_ms(), node)
	44	+	}),
	45	+	_ => Hlc::zero(node), // first sync: genuinely no prior clock
30	46		}
31	47		}
32	48
33	49		fn store_clock(conn: &Connection, clock: &Hlc) -> Result<()> {
34		-	set_sync_state(conn, "last_hlc", &serde_json::to_string(clock).unwrap_or_default())
	50	+	// Hlc is a plain derive-Serialize struct, so serialization cannot fail; make
	51	+	// that an invariant (panic on the impossible) rather than silently persisting
	52	+	// an empty string that load_clock would then treat as a corrupt clock.
	53	+	let json = serde_json::to_string(clock).expect("Hlc always serializes");
	54	+	set_sync_state(conn, "last_hlc", &json)
35	55		}
36	56
37	57		/// The committed HLC for a row, if any — the oracle `CleanChanges::gated` uses to
			@@ -53,7 +73,11 @@ pub(crate) fn set_committed(conn: &Connection, table: &str, row_id: &str, hlc: &
53	73		conn.execute(
54	74		"INSERT INTO hlc_ledger (table_name, row_id, hlc) VALUES (?1, ?2, ?3)
55	75		ON CONFLICT(table_name, row_id) DO UPDATE SET hlc = excluded.hlc",
56		-	rusqlite::params![table, row_id, serde_json::to_string(hlc).unwrap_or_default()],
	76	+	rusqlite::params![
	77	+	table,
	78	+	row_id,
	79	+	serde_json::to_string(hlc).expect("Hlc always serializes")
	80	+	],
57	81		)?;
58	82		Ok(())
59	83		}
			@@ -79,7 +103,10 @@ pub(crate) fn stamp_pending(conn: &Connection, node: Uuid) -> Result<()> {
79	103		clock = Hlc::tick(clock, now_ms(), node);
80	104		conn.execute(
81	105		"UPDATE sync_changelog SET hlc = ?1 WHERE id = ?2",
82		-	rusqlite::params![serde_json::to_string(&clock).unwrap_or_default(), id],
	106	+	rusqlite::params![
	107	+	serde_json::to_string(&clock).expect("Hlc always serializes"),
	108	+	id
	109	+	],
83	110		)?;
84	111		}
85	112		store_clock(conn, &clock)
			@@ -149,6 +176,22 @@ mod tests {
149	176		}
150	177
151	178		#[test]
	179	+	fn corrupt_clock_seeds_forward_not_zero() {
	180	+	let conn = mem();
	181	+	let node = Uuid::from_u128(1);
	182	+	// First sync (empty) is genuinely the zero clock.
	183	+	assert_eq!(load_clock(&conn, node), Hlc::zero(node));
	184	+	// A corrupt persisted clock must NOT reset to zero (that moves the clock
	185	+	// backward); it seeds from physical time, so it's well ahead of zero.
	186	+	set_sync_state(&conn, "last_hlc", "{not valid json").unwrap();
	187	+	let recovered = load_clock(&conn, node);
	188	+	assert!(
	189	+	recovered.wall_ms > 0 && recovered > Hlc::zero(node),
	190	+	"corrupt clock seeded backward: {recovered:?}"
	191	+	);
	192	+	}
	193	+
	194	+	#[test]
152	195		fn ledger_roundtrip_and_gate() {
153	196		let conn = mem();
154	197		let node = Uuid::from_u128(7);