From 475c6a4a2a9f4ddf77ebfa30b970a10cbe2651e6 Mon Sep 17 00:00:00 2001 From: Eric Wendland Date: Thu, 21 May 2026 01:40:50 +0200 Subject: [PATCH] Record file root sync conflicts --- AGENTS.md | 5 +- README.md | 8 +- crates/geth-cas/src/lib.rs | 126 +++++++++++++++++++++++++ crates/geth-cli/src/lib.rs | 11 +++ crates/geth-control/src/lib.rs | 2 + crates/geth-node/src/lib.rs | 164 ++++++++++++++++++++++++++++++++- docs/architecture.md | 16 ++-- docs/roadmap.md | 6 +- 8 files changed, 324 insertions(+), 14 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 00cd057..7ca1017 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -148,7 +148,10 @@ Roadmap items should be actionable and checkable: roots from sync-status `cas-tree:` watermarks. `geth cas root apply` materializes missing files/directories from a CAS tree without deleting extras or overwriting local edits, and records conflicts for manual resolution. - Richer three-way file application remains roadmap work. + Repeated file-root syncs retain the previous imported remote tree as the + base and record durable concurrent edit, delete/edit, and divergent rename + conflicts when local and remote roots both changed. Richer three-way file + application remains roadmap work. - DB resources can be registered locally and report local-only status plus a read-only SQLite schema summary/hash and `crsql_changes` metadata when present. The DB crate and daemon can extract typed read-only `crsql_changes` diff --git a/README.md b/README.md index f93a034..c90a406 100644 --- a/README.md +++ b/README.md @@ -118,7 +118,9 @@ The bootstrap implementation provides: - local file-root commands: `geth cas root add/list/scan/sync/apply`; root sync pulls authorized remote tree metadata and CAS tree bytes into a peer-qualified remote root, and apply materializes a tree without deleting files or - overwriting local edits + overwriting local edits. Repeated syncs keep the previous imported remote + tree as the base and record durable conflicts when local and remote roots + both changed. - local file conflict metadata commands: `geth cas conflict record/list/resolve` - local DB resource registration: `geth db add ` and @@ -194,6 +196,10 @@ local daemon skip unchanged or unauthorized streams. File roots advertise `cas-tree:` watermarks when the caller has `cas.fetch`; background live-sync imports updated tree metadata and CAS tree bytes into peer-qualified remote roots without writing files. +When a previous imported remote tree is available, sync compares that base +against the current local same-named root and the newly imported remote tree. +Concurrent edit, delete/edit, and divergent rename conflicts are recorded in +the local conflict table for later resolution. `geth cas root apply --to ` can then materialize that tree locally: it creates missing directories/files, never deletes extra files, never overwrites differing local files, and records conflicts for manual resolution. diff --git a/crates/geth-cas/src/lib.rs b/crates/geth-cas/src/lib.rs index 4612adb..97f740f 100644 --- a/crates/geth-cas/src/lib.rs +++ b/crates/geth-cas/src/lib.rs @@ -136,6 +136,13 @@ pub struct FileConflict { pub resolved_at_ms: Option, } +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct TreeConflict { + pub path: String, + pub kind: FileConflictKind, + pub detail: String, +} + #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "kebab-case")] pub enum FileConflictKind { @@ -540,6 +547,74 @@ pub fn diff_tree_objects( changes } +pub fn detect_tree_conflicts( + base: &CasTreeObject, + local: &CasTreeObject, + remote: &CasTreeObject, +) -> Vec { + let base_entries = tree_entry_map(base); + let local_entries = tree_entry_map(local); + let remote_entries = tree_entry_map(remote); + let mut paths = BTreeSet::new(); + paths.extend(base_entries.keys().cloned()); + paths.extend(local_entries.keys().cloned()); + paths.extend(remote_entries.keys().cloned()); + + let local_renames = rename_sources(base, local); + let remote_renames = rename_sources(base, remote); + let mut conflicts = Vec::new(); + for (from, local_to) in &local_renames { + if let Some(remote_to) = remote_renames.get(from) { + if local_to == remote_to { + continue; + } + conflicts.push(TreeConflict { + path: from.clone(), + kind: FileConflictKind::Rename, + detail: format!( + "local renamed {from} to {local_to}, while remote renamed it to {remote_to}" + ), + }); + } + } + + for path in paths { + let base_entry = base_entries.get(&path).copied(); + let local_entry = local_entries.get(&path).copied(); + let remote_entry = remote_entries.get(&path).copied(); + let local_changed = base_entry != local_entry; + let remote_changed = base_entry != remote_entry; + if !local_changed || !remote_changed || local_entry == remote_entry { + continue; + } + if conflicts.iter().any(|conflict| conflict.path == path) { + continue; + } + let kind = if base_entry.is_some() && (local_entry.is_none() || remote_entry.is_none()) { + FileConflictKind::DeleteEdit + } else { + FileConflictKind::ConcurrentEdit + }; + let detail = match kind { + FileConflictKind::ConcurrentEdit => { + format!("local and remote both changed {path} from the previous imported base") + } + FileConflictKind::DeleteEdit => { + format!("one side deleted {path} while the other side changed it") + } + FileConflictKind::Rename => unreachable!("rename conflicts are detected above"), + }; + conflicts.push(TreeConflict { path, kind, detail }); + } + + conflicts.sort_by(|left, right| { + left.path + .cmp(&right.path) + .then_with(|| left.kind.as_str().cmp(right.kind.as_str())) + }); + conflicts +} + #[must_use] pub fn file_root_scan_note() -> &'static str { "local scan only; geth never overwrites file roots without a recorded future sync decision" @@ -561,6 +636,18 @@ fn change_sort_key(change: &FileRootChange) -> (u8, String, String) { } } +fn rename_sources(base: &CasTreeObject, current: &CasTreeObject) -> BTreeMap { + diff_tree_objects(Some(base), current) + .into_iter() + .filter_map(|change| match change { + FileRootChange::Renamed { from, to } => Some((from, to)), + FileRootChange::Created { .. } + | FileRootChange::Modified { .. } + | FileRootChange::Deleted { .. } => None, + }) + .collect() +} + fn collect_tree_entries( cas: &LocalCas, root: &Path, @@ -816,6 +903,45 @@ mod tests { assert_eq!(delete_count, 1); } + #[test] + fn tree_conflicts_detect_concurrent_edit_delete_edit_and_rename() { + let base = CasTreeObject { + version: CAS_TREE_OBJECT_VERSION, + entries: vec![ + tree_file("edit.txt", "01", 1), + tree_file("delete-edit.txt", "02", 1), + tree_file("rename.txt", "03", 1), + ], + }; + let local = CasTreeObject { + version: CAS_TREE_OBJECT_VERSION, + entries: vec![ + tree_file("edit.txt", "04", 1), + tree_file("delete-edit.txt", "05", 1), + tree_file("local-rename.txt", "03", 1), + ], + }; + let remote = CasTreeObject { + version: CAS_TREE_OBJECT_VERSION, + entries: vec![ + tree_file("edit.txt", "06", 1), + tree_file("remote-rename.txt", "03", 1), + ], + }; + + let conflicts = detect_tree_conflicts(&base, &local, &remote); + + assert!(conflicts.iter().any(|conflict| { + conflict.path == "edit.txt" && conflict.kind == FileConflictKind::ConcurrentEdit + })); + assert!(conflicts.iter().any(|conflict| { + conflict.path == "delete-edit.txt" && conflict.kind == FileConflictKind::DeleteEdit + })); + assert!(conflicts.iter().any(|conflict| { + conflict.path == "rename.txt" && conflict.kind == FileConflictKind::Rename + })); + } + fn tree_file(path: &str, byte: &str, size_bytes: u64) -> CasTreeEntry { CasTreeEntry { path: path.to_owned(), diff --git a/crates/geth-cli/src/lib.rs b/crates/geth-cli/src/lib.rs index 1a82bdf..275723e 100644 --- a/crates/geth-cli/src/lib.rs +++ b/crates/geth-cli/src/lib.rs @@ -1454,6 +1454,7 @@ fn print_response(response: ControlResponse, json: bool) -> Result<()> { name, root, tree_bytes_imported, + sync_conflicts, allowed, reason, note, @@ -1469,6 +1470,16 @@ fn print_response(response: ControlResponse, json: bool) -> Result<()> { .unwrap_or_else(|| "unscanned".to_owned()) ); println!("tree_bytes_imported: {tree_bytes_imported}"); + println!("sync_conflicts: {}", sync_conflicts.len()); + for conflict in sync_conflicts { + println!( + "{}\t{}\t{}\t{}", + conflict.id, + conflict.path, + conflict.kind.as_str(), + conflict.status.as_str() + ); + } } else { println!("file root sync denied by {peer_node_id}"); } diff --git a/crates/geth-control/src/lib.rs b/crates/geth-control/src/lib.rs index c8b6e90..d2aea31 100644 --- a/crates/geth-control/src/lib.rs +++ b/crates/geth-control/src/lib.rs @@ -465,6 +465,7 @@ pub enum ControlResponse { name: String, root: Option, tree_bytes_imported: bool, + sync_conflicts: Vec, allowed: bool, reason: String, note: String, @@ -1654,6 +1655,7 @@ mod tests { name: "notes".to_owned(), root: None, tree_bytes_imported: false, + sync_conflicts: Vec::new(), allowed: false, reason: "no grant".to_owned(), note: "file-root sync".to_owned(), diff --git a/crates/geth-node/src/lib.rs b/crates/geth-node/src/lib.rs index c6d579b..034c561 100644 --- a/crates/geth-node/src/lib.rs +++ b/crates/geth-node/src/lib.rs @@ -1184,6 +1184,7 @@ async fn cas_root_sync_from_peer( name: response_name, root: None, tree_bytes_imported: false, + sync_conflicts: Vec::new(), allowed, reason, note, @@ -1192,11 +1193,13 @@ async fn cas_root_sync_from_peer( let store = Store::open(&node.paths.metadata_db())?; let mut tree_bytes_imported = false; let mut imported_high_water = None; + let mut sync_conflicts = Vec::new(); let root = root .map(|remote_root| { let stored_name = remote_file_root_name(&node_id, &remote_root.name); let stored_path = format!("remote:{node_id}:{}", remote_root.name); - if let Some(existing) = store.get_file_root_by_name(&stored_name)? { + let existing_remote_root = store.get_file_root_by_name(&stored_name)?; + if let Some(existing) = &existing_remote_root { if existing.path != stored_path { return Err(NodeError::IrohPeer(format!( "refusing to overwrite existing file root {} at {}", @@ -1204,6 +1207,12 @@ async fn cas_root_sync_from_peer( ))); } } + let base_tree = existing_remote_root + .as_ref() + .and_then(|existing| existing.latest_tree_json.as_deref()) + .map(serde_json::from_str) + .transpose()?; + let mut remote_tree_json = None; if let (Some(hash), Some(content)) = ( remote_root.latest_tree.clone(), tree_content_base64.as_ref(), @@ -1224,6 +1233,25 @@ async fn cas_root_sync_from_peer( &info.path.to_string_lossy(), )?; tree_bytes_imported = true; + let remote_tree: CasTreeObject = geth_codec::decode_canonical(&decoded)?; + remote_tree_json = Some(serde_json::to_string(&remote_tree)?); + if let (Some(base_tree), Some(local_root)) = + (&base_tree, store.get_file_root_by_name(&remote_root.name)?) + { + if local_root.latest_tree_json.is_some() { + sync_conflicts.extend(record_sync_tree_conflicts( + &store, + &local_root, + existing_remote_root + .as_ref() + .and_then(|existing| existing.latest_tree_hash.as_deref()), + base_tree, + local_root.latest_tree_hash.as_deref(), + remote_root.latest_tree.as_ref(), + &remote_tree, + )?); + } + } } let stored = StoredFileRoot { root_id: format!("file-root:remote:{node_id}:{}", remote_root.name), @@ -1231,7 +1259,7 @@ async fn cas_root_sync_from_peer( name: stored_name, path: stored_path, latest_tree_hash: remote_root.latest_tree.as_ref().map(ToString::to_string), - latest_tree_json: None, + latest_tree_json: remote_tree_json, updated_at_ms: remote_root.updated_at_ms, }; store.upsert_file_root(&stored)?; @@ -1254,6 +1282,7 @@ async fn cas_root_sync_from_peer( name: response_name, root, tree_bytes_imported, + sync_conflicts, allowed, reason, note, @@ -6253,6 +6282,56 @@ fn record_apply_conflict( file_conflict_from_stored(stored) } +fn record_sync_tree_conflicts( + store: &Store, + local_root: &StoredFileRoot, + base_tree_hash: Option<&str>, + base_tree: &CasTreeObject, + local_tree_hash: Option<&str>, + remote_tree_hash: Option<&BlobHash>, + remote_tree: &CasTreeObject, +) -> Result, NodeError> { + let Some(local_tree_json) = local_root.latest_tree_json.as_deref() else { + return Ok(Vec::new()); + }; + let local_tree: CasTreeObject = serde_json::from_str(local_tree_json)?; + let tree_conflicts = geth_cas::detect_tree_conflicts(base_tree, &local_tree, remote_tree); + let created_at = geth_store::now_ms(); + tree_conflicts + .into_iter() + .enumerate() + .map(|(index, conflict)| { + let created_at = created_at + index as i64; + let stored = StoredFileConflict { + conflict_id: generated_file_conflict_id( + &local_root.name, + &conflict.path, + conflict.kind.as_str(), + created_at, + ), + root_name: local_root.name.clone(), + resource_id: local_root.resource_id.clone(), + path: conflict.path, + kind: conflict.kind.as_str().to_owned(), + status: FileConflictStatus::Open.as_str().to_owned(), + base_tree_hash: base_tree_hash.map(ToOwned::to_owned), + local_tree_hash: local_tree_hash.map(ToOwned::to_owned), + remote_tree_hash: remote_tree_hash.map(ToString::to_string), + detail: format!( + "{}; detected while syncing remote file-root metadata", + conflict.detail + ), + resolution: None, + resolution_note: None, + created_at_ms: created_at, + resolved_at_ms: None, + }; + store.upsert_file_conflict(&stored)?; + file_conflict_from_stored(stored) + }) + .collect() +} + fn write_file_atomic(path: &Path, bytes: &[u8]) -> Result<(), NodeError> { if let Some(parent) = path.parent() { std::fs::create_dir_all(parent)?; @@ -7330,6 +7409,70 @@ mod tests { ); } + #[test] + fn file_root_sync_records_automatic_three_tree_conflicts() { + let store = Store::open_memory().expect("open"); + let base_tree = test_tree(vec![ + test_tree_file("edit.txt", "01"), + test_tree_file("delete-edit.txt", "02"), + test_tree_file("rename.txt", "03"), + ]); + let local_tree = test_tree(vec![ + test_tree_file("edit.txt", "04"), + test_tree_file("delete-edit.txt", "05"), + test_tree_file("local-rename.txt", "03"), + ]); + let remote_tree = test_tree(vec![ + test_tree_file("edit.txt", "06"), + test_tree_file("remote-rename.txt", "03"), + ]); + let local_root = StoredFileRoot { + root_id: "file-root:shared".to_owned(), + resource_id: "resource:cas-tree:shared".to_owned(), + name: "shared".to_owned(), + path: "/tmp/shared".to_owned(), + latest_tree_hash: Some("local-tree".to_owned()), + latest_tree_json: Some(serde_json::to_string(&local_tree).expect("local tree json")), + updated_at_ms: 2, + }; + store.upsert_file_root(&local_root).expect("upsert root"); + + let conflicts = record_sync_tree_conflicts( + &store, + &local_root, + Some("base-tree"), + &base_tree, + local_root.latest_tree_hash.as_deref(), + Some(&BlobHash::new("remote-tree".to_owned())), + &remote_tree, + ) + .expect("record conflicts"); + + assert_eq!(conflicts.len(), 3); + assert!(conflicts.iter().any(|conflict| { + conflict.path == "edit.txt" && conflict.kind == FileConflictKind::ConcurrentEdit + })); + assert!(conflicts.iter().any(|conflict| { + conflict.path == "delete-edit.txt" && conflict.kind == FileConflictKind::DeleteEdit + })); + assert!(conflicts.iter().any(|conflict| { + conflict.path == "rename.txt" && conflict.kind == FileConflictKind::Rename + })); + + let stored = store + .list_file_conflicts(Some("shared")) + .expect("list conflicts"); + assert_eq!(stored.len(), 3); + assert!(stored.iter().all(|conflict| { + conflict.base_tree_hash.as_deref() == Some("base-tree") + && conflict.local_tree_hash.as_deref() == Some("local-tree") + && conflict.remote_tree_hash.as_deref() == Some("remote-tree") + && conflict + .detail + .contains("syncing remote file-root metadata") + })); + } + #[test] fn lan_discovery_address_selection_uses_iroh_direct_addresses() { let key = AgentKey::generate(); @@ -8754,4 +8897,21 @@ mod tests { left_endpoint.shutdown().await; right_endpoint.shutdown().await; } + + fn test_tree(entries: Vec) -> CasTreeObject { + CasTreeObject { + version: geth_cas::CAS_TREE_OBJECT_VERSION, + entries, + } + } + + fn test_tree_file(path: &str, byte: &str) -> geth_cas::CasTreeEntry { + geth_cas::CasTreeEntry { + path: path.to_owned(), + kind: CasTreeEntryKind::File, + blob: Some(BlobHash::new(byte.repeat(32))), + size_bytes: 1, + executable: false, + } + } } diff --git a/docs/architecture.md b/docs/architecture.md index 6ee96eb..3d5f160 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -134,13 +134,16 @@ pull authorized remote file-root tree metadata with `geth cas root sync remote CAS tree bytes and records a peer-qualified remote root whose path is `remote::` without applying files. File roots also participate in the daemon background live-sync loop through authorized `cas-tree:` -watermarks. `geth cas root apply --to ` is the first conservative +watermarks. Repeated remote file-root syncs retain the previous imported remote +tree as the base and compare base/local/remote tree state. When both local and +remote roots changed, geth records durable concurrent edit, delete/edit, or +divergent rename conflicts instead of applying remote content. `geth cas root +apply --to ` is the first conservative materialization path: it creates missing files and directories from the CAS tree, does not delete extra local files, does not overwrite differing local files, and records conflicts for manual resolution. The daemon also has durable local -file-conflict records with explicit resolution choices; future cross-node file -sync will extend those records instead of silently applying ambiguous remote -changes. +file-conflict records with explicit resolution choices; richer three-way apply +will extend those records instead of silently applying ambiguous remote changes. As a bootstrap network path, `geth cas fetch ` dials an imported signed peer card over the daemon-owned Iroh control ALPN. The serving @@ -149,9 +152,8 @@ then reduces local auth ops and requires `cas.fetch` on `resource:cas:local` before returning blob bytes. The requester verifies that the returned bytes hash to the requested BLAKE3 CAS hash before storing them. Successful fetches update durable local provider metadata keyed by CAS hash and peer node, which can be -inspected through `geth cas providers `. Iroh-blobs, encrypted blobs, -richer cache policies, cross-node file roots, and automatic conflict detection -are future work. +inspected through `geth cas providers `. Iroh-blobs provider/fetch +integration and richer three-way file application are future work. `geth-db` currently registers local SQLite paths as DB resources and reports local-only sync status plus a read-only SQLite schema summary/hash. It also diff --git a/docs/roadmap.md b/docs/roadmap.md index 7931c56..15ba401 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -556,15 +556,15 @@ and future group key evolution. - `[ ]` Future completion adds a richer three-way apply using base/local/remote trees for automatic safe updates and deletes. -- `[~]` Conflict handling. +- `[x]` Conflict handling. Acceptance criteria: - `[x]` Conflicts are represented as durable metadata. - `[x]` CLI/control can record, list, and choose a resolution for local conflict metadata. - `[x]` Tests cover local concurrent edit conflict record/list/resolve. - - `[ ]` Future sync records conflicts automatically from base/local/remote + - `[x]` Future sync records conflicts automatically from base/local/remote tree comparisons. - - `[ ]` Tests cover automatic concurrent edit, delete/edit, and rename + - `[x]` Tests cover automatic concurrent edit, delete/edit, and rename conflict detection. - `[ ]` Keyhive-like convergent capabilities.