From 6728e241bb19e4a318f0d44a2225771b1ea0c06f Mon Sep 17 00:00:00 2001 From: Bob Date: Wed, 16 Sep 2026 07:52:41 +0000 Subject: [PATCH 1/5] fix(aw-sync): sanitize whitespace hostnames on import; make per-bucket errors non-fatal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two issues block pulling peers whose bucket hostname contains whitespace (e.g. "POCO F8 Ultra" from Android devices before aw-android#272): 1. `get_or_create_sync_bucket` derived the destination ID from the raw hostname. On aw-server-rust destinations, `create_bucket` rejects new buckets with whitespace hostnames (#658), so the first pull of such a peer returns HTTP 400, which propagated as `?` and aborted the whole sync pass (every subsequent peer also skipped). Fix: before creating a new bucket, sanitize whitespace → `_` in both the bucket ID and the hostname field. The legacy (unsanitized) ID is checked first so any existing pre-sanitization import is reused, avoiding the full re-import fork (#1373). 2. A single bucket failure with `?` in `sync_datastores` aborted the entire pass. All remaining peers were skipped. Blast radius: daemon returns Err → exits → supervisor restarts → same failure → budget exhausted → #688. Fix: per-bucket errors in `sync_datastores` are now non-fatal (warn + continue). A broken or invalid bucket is skipped; healthy buckets still sync. Updated two tests that relied on the old fatal-error behavior. Fixes #692. Related: #658, #685, #688, ActivityWatch/aw-android#272. Git-Session-Id: c8bb --- aw-sync/src/sync.rs | 85 ++++++++++++++++------- aw-sync/tests/sync.rs | 152 +++++++++++++++++++++++++++++++++++++++--- 2 files changed, 203 insertions(+), 34 deletions(-) diff --git a/aw-sync/src/sync.rs b/aw-sync/src/sync.rs index c128b167..5b6382d4 100644 --- a/aw-sync/src/sync.rs +++ b/aw-sync/src/sync.rs @@ -376,32 +376,56 @@ fn get_or_create_sync_bucket( ) }; + // Look up the unsanitized ID first. Any device that was synced before + // aw-android added hostname sanitization (ActivityWatch/aw-android#272) may + // have left a local bucket whose ID and hostname contain whitespace (e.g. + // `…-synced-from-POCO F8 Ultra`). Keep using that ID to avoid a fork that + // would cause a full re-import (ActivityWatch/activitywatch#1373). match ds_to.get_bucket(new_id.as_str()) { - Ok(bucket) => Ok(bucket), - Err(DatastoreError::NoSuchBucket(_)) => { - let mut bucket_new = bucket_from.clone(); - bucket_new.id = new_id.clone(); - // Only stamp $aw.sync.origin on pull/import. The derived origin already handles - // the legacy case: hostname is used when the source bucket has no metadata field. - if let Some(origin) = sync_origin { - bucket_new - .data - .insert("$aw.sync.origin".to_string(), serde_json::json!(origin)); - } else { - // Push path: strip any stale $aw.sync.origin that bucket_from may carry - // (e.g. if it was previously imported by a pull). Staging copies must - // never look like synced-from-remote buckets. - bucket_new.data.remove("$aw.sync.origin"); - } - ds_to - .create_bucket(&bucket_new) - .map_err(|e| format!("Failed to create bucket '{new_id}': {e:?}"))?; - ds_to - .get_bucket(new_id.as_str()) - .map_err(|e| format!("Failed to read back bucket '{new_id}': {e:?}")) + Ok(bucket) => return Ok(bucket), + Err(DatastoreError::NoSuchBucket(_)) => {} + Err(e) => return Err(format!("Failed to get bucket '{new_id}': {e:?}")), + } + + // The bucket does not exist yet. If the ID or hostname contains whitespace, + // sanitize before creating: aw-server-rust rejects new buckets with whitespace + // hostnames (#658). We must also check whether a sanitized bucket was already + // created by a previous sync session so we don't open a second fork. + let (final_id, final_hostname) = if new_id.contains(char::is_whitespace) { + let sanitized_id = new_id.replace(char::is_whitespace, "_"); + let sanitized_hostname = bucket_from.hostname.replace(char::is_whitespace, "_"); + // If a sanitized bucket already exists, use it. + match ds_to.get_bucket(sanitized_id.as_str()) { + Ok(bucket) => return Ok(bucket), + Err(DatastoreError::NoSuchBucket(_)) => {} + Err(e) => return Err(format!("Failed to get bucket '{sanitized_id}': {e:?}")), } - Err(e) => Err(format!("Failed to get bucket '{new_id}': {e:?}")), + (sanitized_id, sanitized_hostname) + } else { + (new_id.clone(), bucket_from.hostname.clone()) + }; + + let mut bucket_new = bucket_from.clone(); + bucket_new.id = final_id.clone(); + bucket_new.hostname = final_hostname; + // Only stamp $aw.sync.origin on pull/import. The derived origin already handles + // the legacy case: hostname is used when the source bucket has no metadata field. + if let Some(origin) = sync_origin { + bucket_new + .data + .insert("$aw.sync.origin".to_string(), serde_json::json!(origin)); + } else { + // Push path: strip any stale $aw.sync.origin that bucket_from may carry + // (e.g. if it was previously imported by a pull). Staging copies must + // never look like synced-from-remote buckets. + bucket_new.data.remove("$aw.sync.origin"); } + ds_to + .create_bucket(&bucket_new) + .map_err(|e| format!("Failed to create bucket '{final_id}': {e:?}"))?; + ds_to + .get_bucket(final_id.as_str()) + .map_err(|e| format!("Failed to read back bucket '{final_id}': {e:?}")) } /// Number of events fetched per page in the chunked-fetch loop in `sync_one`. @@ -530,8 +554,19 @@ pub fn sync_datastores( let mut buckets = Vec::with_capacity(buckets_from.len()); for bucket_from in buckets_from { - let bucket_to = get_or_create_sync_bucket(&bucket_from, ds_to, is_push)?; - buckets.push(sync_one(ds_from, ds_to, bucket_from, bucket_to, sync_spec)?); + let bucket_to = match get_or_create_sync_bucket(&bucket_from, ds_to, is_push) { + Ok(b) => b, + Err(e) => { + // Non-fatal: log and skip this bucket so a bad peer does not + // abort the entire pass and leave other buckets un-synced (#692). + warn!(" ! Skipping bucket '{}': {}", bucket_from.id, e); + continue; + } + }; + match sync_one(ds_from, ds_to, bucket_from, bucket_to, sync_spec) { + Ok(synced) => buckets.push(synced), + Err(e) => warn!(" ! Skipping sync for bucket: {}", e), + } } Ok(buckets) diff --git a/aw-sync/tests/sync.rs b/aw-sync/tests/sync.rs index add2937b..951f6d5c 100644 --- a/aw-sync/tests/sync.rs +++ b/aw-sync/tests/sync.rs @@ -96,14 +96,18 @@ mod sync_tests { .collect() } - /// A datastore failure must be *returned*, never panicked. + /// A datastore failure must not panic. /// /// On Android the sync step runs inside a JNI `extern "C"` frame, where an /// unwinding panic aborts the whole app process rather than surfacing as an /// exception — the SIGABRT in ActivityWatch/aw-android#220. `sync_datastores` /// used to `unwrap()` every datastore call, so any failure here was fatal. + /// + /// Since the per-bucket non-fatal change (#692), sync_datastores returns + /// Ok(()) and warns when individual buckets fail, so a broken peer does not + /// abort the whole pass. No panic is still the key invariant. #[test] - fn test_unusable_datastore_returns_error_instead_of_panicking() { + fn test_unusable_datastore_does_not_panic() { let state = init_teststate(); create_bucket(&state.ds_src, 0); @@ -114,6 +118,9 @@ mod sync_tests { )) .expect("path is valid UTF-8"); + // Previously this panicked (unwrap on datastore failure); later it + // returned Err; now it returns Ok(()) after skipping the broken bucket. + // The key property: it must not panic. let result = aw_sync::sync_datastores( &state.ds_src, &ds_broken, @@ -122,16 +129,135 @@ mod sync_tests { &SyncSpec::default(), ); assert!( - result.is_err(), - "an unusable destination datastore must return Err, got {result:?}" + !result.is_err() || result.is_ok(), + "sync_datastores must not panic; got {result:?}" + ); + // With non-fatal per-bucket errors, the function now returns Ok(()) + // and logs a warning rather than propagating the per-bucket failure. + assert!( + result.is_ok(), + "a per-bucket failure must not abort the whole pass; got {result:?}" + ); + } + + /// Pulling a peer whose bucket hostname contains whitespace must not fail with + /// a 400: aw-server-rust rejects new buckets with whitespace hostnames (#658). + /// `get_or_create_sync_bucket` must sanitize the hostname (and derived ID) + /// before creating, while still re-using any legacy unsanitized bucket that + /// was imported before the sanitization was added. + #[test] + fn test_whitespace_hostname_pull_creates_sanitized_bucket() { + let state = init_teststate(); + + // Source bucket whose hostname contains a space, as produced by Android + // devices that were named before aw-android added hostname sanitization + // (ActivityWatch/aw-android#272). + let bucket: Bucket = serde_json::from_value(serde_json::json!({ + "id": "aw-watcher-android", + "type": "currentwindow", + "hostname": "POCO F8 Ultra", + "client": "aw-android" + })) + .unwrap(); + state.ds_src.create_bucket(&bucket).unwrap(); + + // In-memory datastore does not enforce the server-side whitespace check, + // so the sync completes without 400. + aw_sync::sync_datastores( + &state.ds_src, + &state.ds_dest, + false, // pull + None, + &SyncSpec::default(), + ) + .unwrap(); + + let dest_buckets = state.ds_dest.get_buckets().unwrap(); + + // The destination bucket ID must use underscores, not spaces. + let sanitized_id = "aw-watcher-android-synced-from-POCO_F8_Ultra"; + assert!( + dest_buckets.contains_key(sanitized_id), + "expected sanitized bucket id '{sanitized_id}', got: {:?}", + dest_buckets.keys().collect::>() + ); + // No whitespace bucket must have been created. + let whitespace_id = "aw-watcher-android-synced-from-POCO F8 Ultra"; + assert!( + !dest_buckets.contains_key(whitespace_id), + "whitespace bucket id '{whitespace_id}' must not be created" + ); + + // The hostname field on the destination bucket must also be sanitized. + let dest_bucket = dest_buckets.get(sanitized_id).unwrap(); + assert!( + !dest_bucket.hostname.contains(char::is_whitespace), + "destination hostname must not contain whitespace, got: {:?}", + dest_bucket.hostname + ); + } + + /// If a legacy unsanitized bucket already exists in the destination (imported + /// before the sanitization was added), re-use it instead of creating a new + /// sanitized one. Creating a sanitized copy forks the destination and causes + /// a full re-import (ActivityWatch/activitywatch#1373). + #[test] + fn test_whitespace_hostname_pull_reuses_legacy_unsanitized_bucket() { + let state = init_teststate(); + + let bucket: Bucket = serde_json::from_value(serde_json::json!({ + "id": "aw-watcher-android", + "type": "currentwindow", + "hostname": "POCO F8 Ultra", + "client": "aw-android" + })) + .unwrap(); + state.ds_src.create_bucket(&bucket).unwrap(); + + // Simulate a pre-existing legacy destination bucket with unsanitized ID. + let legacy_id = "aw-watcher-android-synced-from-POCO F8 Ultra"; + let legacy_bucket: Bucket = serde_json::from_value(serde_json::json!({ + "id": legacy_id, + "type": "currentwindow", + "hostname": "POCO F8 Ultra", + "client": "aw-android" + })) + .unwrap(); + state.ds_dest.create_bucket(&legacy_bucket).unwrap(); + + aw_sync::sync_datastores( + &state.ds_src, + &state.ds_dest, + false, // pull + None, + &SyncSpec::default(), + ) + .unwrap(); + + let dest_buckets = state.ds_dest.get_buckets().unwrap(); + + // The legacy bucket must be present and re-used — not replaced. + assert!( + dest_buckets.contains_key(legacy_id), + "legacy unsanitized bucket must be preserved" + ); + // No new sanitized duplicate must have been created. + let sanitized_id = "aw-watcher-android-synced-from-POCO_F8_Ultra"; + assert!( + !dest_buckets.contains_key(sanitized_id), + "a sanitized fork must not be created when legacy bucket exists, got: {:?}", + dest_buckets.keys().collect::>() ); } - /// Bucket metadata of an unexpected shape must also be an error, not a panic: + /// Bucket metadata of an unexpected shape must not panic: /// `$aw.sync.origin` is read from data written by another host, so it is not /// under this host's control. + /// + /// With the per-bucket non-fatal change (#692), a malformed bucket is now + /// skipped (warn + continue) rather than aborting the whole sync pass. #[test] - fn test_non_string_sync_origin_returns_error_instead_of_panicking() { + fn test_non_string_sync_origin_does_not_panic() { let state = init_teststate(); let bucket: Bucket = serde_json::from_str( r#"{ @@ -145,6 +271,8 @@ mod sync_tests { .unwrap(); state.ds_src.create_bucket(&bucket).unwrap(); + // Previously this panicked; later it returned Err; now it returns Ok(()) + // after skipping the malformed bucket. No panic is the key invariant. let result = aw_sync::sync_datastores( &state.ds_src, &state.ds_dest, @@ -152,10 +280,16 @@ mod sync_tests { None, &SyncSpec::default(), ); - let err = result.expect_err("a non-string $aw.sync.origin must return Err"); assert!( - err.contains("$aw.sync.origin"), - "error should name the offending field, got: {err}" + result.is_ok(), + "a malformed bucket must not abort the whole pass; got {result:?}" + ); + // The malformed bucket must have been skipped, not imported. + let dest_buckets = state.ds_dest.get_buckets().unwrap(); + assert!( + dest_buckets.is_empty(), + "skipped bucket must not appear in destination, got: {:?}", + dest_buckets.keys().collect::>() ); } From 1d688364ff5e651b077b7caa9cb0186d5d230748 Mon Sep 17 00:00:00 2001 From: Bob Date: Wed, 16 Sep 2026 08:10:57 +0000 Subject: [PATCH 2/5] fix(aw-sync): match Android hostname sanitizer; isolate per-peer errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Whitespace-only replace ("POCO F8 Ultra" → "POCO_F8_Ultra") would fork the destination the day aw-android#272 migrates the phone's hostname column to poco_f8_ultra. Use Android's exact algorithm (lowercase, [^a-z0-9_-]+ → _, trim _) via a shared sanitize_hostname() that must stay byte-identical to DeviceHostname.kt. Also: - Trigger sanitization when the source hostname has whitespace even if the derived ID is already clean ($aw.sync.origin can be sanitized while bucket.hostname is not; create_bucket would 400 on the field). - pull_all: per-peer warn+continue so a failed open does not abort the pass and skip every peer after it (#688 / #693). Git-Session-Id: 2097767 --- aw-sync/src/lib.rs | 1 + aw-sync/src/sync.rs | 92 ++++++++++++++++++++++++++++++++++--- aw-sync/src/sync_wrapper.rs | 15 ++++-- aw-sync/tests/sync.rs | 65 +++++++++++++++++++++++--- 4 files changed, 154 insertions(+), 19 deletions(-) diff --git a/aw-sync/src/lib.rs b/aw-sync/src/lib.rs index d165dceb..3a85988a 100644 --- a/aw-sync/src/lib.rs +++ b/aw-sync/src/lib.rs @@ -12,6 +12,7 @@ pub use report::{ mod sync; pub use sync::create_datastore; +pub use sync::sanitize_hostname; pub use sync::sync_datastores; pub use sync::sync_run; pub use sync::SyncSpec; diff --git a/aw-sync/src/sync.rs b/aw-sync/src/sync.rs index 5b6382d4..079a49ed 100644 --- a/aw-sync/src/sync.rs +++ b/aw-sync/src/sync.rs @@ -337,6 +337,42 @@ fn utf8_db_path(path: &Path) -> Result<&str, String> { .ok_or_else(|| format!("Sync database path is not valid UTF-8: {}", path.display())) } +/// Sanitize a device hostname so it is a legal bucket hostname and a stable +/// sync-ID suffix. +/// +/// Must stay byte-identical to aw-android's `sanitizeDeviceHostname` +/// (`mobile/src/main/java/net/activitywatch/android/DeviceHostname.kt`): +/// trim, lowercase, replace `[^a-z0-9_-]+` with `_`, trim `_`. Empty result +/// becomes `"unknown"`. +/// +/// Divergence here forks destination buckets the day Android migrates its +/// hostname column (ActivityWatch/aw-android#272) onto a different +/// `-synced-from-` ID (ActivityWatch/activitywatch#1373). +pub fn sanitize_hostname(raw: &str) -> String { + let value = raw.trim(); + if value.is_empty() { + return "unknown".to_string(); + } + let lower = value.to_lowercase(); + let mut out = String::with_capacity(lower.len()); + let mut in_run = false; + for c in lower.chars() { + if c.is_ascii_lowercase() || c.is_ascii_digit() || c == '_' || c == '-' { + out.push(c); + in_run = false; + } else if !in_run { + out.push('_'); + in_run = true; + } + } + let trimmed = out.trim_matches('_'); + if trimmed.is_empty() { + "unknown".to_string() + } else { + trimmed.to_string() + } +} + /// Returns the sync-destination bucket for a given bucket, creates it if it doesn't exist. /// /// Returns an error rather than panicking on a datastore failure or on bucket @@ -387,13 +423,31 @@ fn get_or_create_sync_bucket( Err(e) => return Err(format!("Failed to get bucket '{new_id}': {e:?}")), } - // The bucket does not exist yet. If the ID or hostname contains whitespace, - // sanitize before creating: aw-server-rust rejects new buckets with whitespace - // hostnames (#658). We must also check whether a sanitized bucket was already - // created by a previous sync session so we don't open a second fork. - let (final_id, final_hostname) = if new_id.contains(char::is_whitespace) { - let sanitized_id = new_id.replace(char::is_whitespace, "_"); - let sanitized_hostname = bucket_from.hostname.replace(char::is_whitespace, "_"); + // The bucket does not exist yet. If the ID *or the source hostname* contains + // whitespace, sanitize before creating: aw-server-rust rejects new buckets + // with whitespace hostnames (#658). The ID-only check is not enough — + // `$aw.sync.origin` can already be clean while `bucket_from.hostname` still + // has spaces, and `create_bucket` would 400 on the hostname field. + // + // Sanitization uses Android's algorithm (not a whitespace-only replace) so + // today's desktop creates `…-synced-from-poco_f8_ultra` and Android's + // hostname-column migration lands on the same ID. + let (final_id, final_hostname) = if bucket_from.hostname.contains(char::is_whitespace) + || new_id.contains(char::is_whitespace) + { + let sanitized_hostname = sanitize_hostname(&bucket_from.hostname); + let sanitized_id = if let Some(ref origin) = sync_origin { + let orig_bucketid = bucket_from + .id + .split("-synced-from-") + .next() + .unwrap_or(bucket_from.id.as_str()); + format!("{orig_bucketid}-synced-from-{}", sanitize_hostname(origin)) + } else { + // Push path: keep the original bucket ID; only the hostname field + // needs to be a legal create_bucket value. + new_id.clone() + }; // If a sanitized bucket already exists, use it. match ds_to.get_bucket(sanitized_id.as_str()) { Ok(bucket) => return Ok(bucket), @@ -956,3 +1010,27 @@ mod pull_only_staging_tests { let _ = fs::remove_dir_all(&dir); } } + +#[cfg(test)] +mod hostname_sanitize_tests { + use super::sanitize_hostname; + + #[test] + fn poco_f8_ultra_matches_android() { + // The contract Erik asked for on ActivityWatch/aw-server-rust#697: + // whitespace-only replace would produce "POCO_F8_Ultra" and fork the + // day aw-android#272 migrates the phone's hostname column. + assert_eq!(sanitize_hostname("POCO F8 Ultra"), "poco_f8_ultra"); + } + + #[test] + fn android_device_hostname_contract() { + // Byte-identical to aw-android DeviceHostnameTest.kt. + assert_eq!(sanitize_hostname("Pixel 8"), "pixel_8"); + assert_eq!(sanitize_hostname("My-Phone_1"), "my-phone_1"); + assert_eq!(sanitize_hostname(" Pixel 8 "), "pixel_8"); + assert_eq!(sanitize_hostname(""), "unknown"); + assert_eq!(sanitize_hostname(" "), "unknown"); + assert_eq!(sanitize_hostname("***"), "unknown"); + } +} diff --git a/aw-sync/src/sync_wrapper.rs b/aw-sync/src/sync_wrapper.rs index b14d2542..9c73d12f 100644 --- a/aw-sync/src/sync_wrapper.rs +++ b/aw-sync/src/sync_wrapper.rs @@ -45,17 +45,22 @@ pub fn pull_all(client: &AwClient) -> Result> { match pull_db(client, &remote.hostname, &remote.path) { Ok(one) => report.merge(one), Err(e) => { + // Per-peer isolation: a peer that fails to open must not abort + // the pass and skip every peer after it. Bucket-level errors are + // already non-fatal in `sync_datastores`; peer-level open + // failures need the same warn+continue so a later + // skip-on-mismatch (#693) has somewhere to go instead of + // becoming "abort pass" (#688). + warn!( + "Skipping peer '{}' ({:?}): {e}", + remote.hostname, remote.path + ); report.peers.push(PeerReport::failed( remote.device_id, remote.hostname, remote.path, e.to_string(), )); - report.finish(); - // Persist the aggregate (earlier peers + this failure), not - // only the phase-local report from the failing `sync_run`. - crate::report::persist_last_report_warn(&report); - return Err(e); } } } diff --git a/aw-sync/tests/sync.rs b/aw-sync/tests/sync.rs index 951f6d5c..7ccd7351 100644 --- a/aw-sync/tests/sync.rs +++ b/aw-sync/tests/sync.rs @@ -174,8 +174,9 @@ mod sync_tests { let dest_buckets = state.ds_dest.get_buckets().unwrap(); - // The destination bucket ID must use underscores, not spaces. - let sanitized_id = "aw-watcher-android-synced-from-POCO_F8_Ultra"; + // Must match aw-android's sanitizeDeviceHostname, not a whitespace-only + // replace ("POCO_F8_Ultra" would fork when Android migrates). + let sanitized_id = "aw-watcher-android-synced-from-poco_f8_ultra"; assert!( dest_buckets.contains_key(sanitized_id), "expected sanitized bucket id '{sanitized_id}', got: {:?}", @@ -187,13 +188,26 @@ mod sync_tests { !dest_buckets.contains_key(whitespace_id), "whitespace bucket id '{whitespace_id}' must not be created" ); + assert!( + !dest_buckets.contains_key("aw-watcher-android-synced-from-POCO_F8_Ultra"), + "whitespace-only replace must not be used; got: {:?}", + dest_buckets.keys().collect::>() + ); // The hostname field on the destination bucket must also be sanitized. let dest_bucket = dest_buckets.get(sanitized_id).unwrap(); - assert!( - !dest_bucket.hostname.contains(char::is_whitespace), - "destination hostname must not contain whitespace, got: {:?}", - dest_bucket.hostname + assert_eq!( + dest_bucket.hostname, "poco_f8_ultra", + "destination hostname must match Android's sanitizer" + ); + // $aw.sync.origin keeps the raw hostname so the pre-migration phone + // identity is still recoverable. + assert_eq!( + dest_bucket + .data + .get("$aw.sync.origin") + .and_then(|v| v.as_str()), + Some("POCO F8 Ultra") ); } @@ -242,7 +256,7 @@ mod sync_tests { "legacy unsanitized bucket must be preserved" ); // No new sanitized duplicate must have been created. - let sanitized_id = "aw-watcher-android-synced-from-POCO_F8_Ultra"; + let sanitized_id = "aw-watcher-android-synced-from-poco_f8_ultra"; assert!( !dest_buckets.contains_key(sanitized_id), "a sanitized fork must not be created when legacy bucket exists, got: {:?}", @@ -250,6 +264,43 @@ mod sync_tests { ); } + /// If `$aw.sync.origin` is already clean while `bucket.hostname` still has + /// whitespace, the sanitizer must still run: otherwise `create_bucket` 400s + /// on the hostname field even though the derived ID is legal. + #[test] + fn test_whitespace_hostname_sanitizes_even_when_id_is_clean() { + let state = init_teststate(); + + let bucket: Bucket = serde_json::from_value(serde_json::json!({ + "id": "aw-watcher-android", + "type": "currentwindow", + "hostname": "POCO F8 Ultra", + "client": "aw-android", + "data": {"$aw.sync.origin": "poco_f8_ultra"} + })) + .unwrap(); + state.ds_src.create_bucket(&bucket).unwrap(); + + aw_sync::sync_datastores( + &state.ds_src, + &state.ds_dest, + false, // pull + None, + &SyncSpec::default(), + ) + .unwrap(); + + let dest_buckets = state.ds_dest.get_buckets().unwrap(); + let sanitized_id = "aw-watcher-android-synced-from-poco_f8_ultra"; + let dest_bucket = dest_buckets.get(sanitized_id).unwrap_or_else(|| { + panic!( + "expected sanitized bucket id '{sanitized_id}', got: {:?}", + dest_buckets.keys().collect::>() + ) + }); + assert_eq!(dest_bucket.hostname, "poco_f8_ultra"); + } + /// Bucket metadata of an unexpected shape must not panic: /// `$aw.sync.origin` is read from data written by another host, so it is not /// under this host's control. From 3690511ce4e2734bd6687c5e28975442d8413196 Mon Sep 17 00:00:00 2001 From: Bob Date: Wed, 16 Sep 2026 08:40:55 +0000 Subject: [PATCH 3/5] fix(aw-sync): refuse pull when hostname sanitizes to unknown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Android maps empty/punctuation-only names to the "unknown" sentinel. A hostname like " * " contains whitespace so the sanitizer runs, then becomes "unknown", and get_or_create would create -synced-from-unknown — mixing every such remote into one destination. The hostname=="unknown" guard in sync_datastores never sees this because the source hostname is not yet the sentinel. Refuse on pull; per-bucket warn+continue skips the junk bucket and a healthy sibling still syncs. Git-Session-Id: 2679931 --- aw-sync/src/sync.rs | 17 ++++++++++++++ aw-sync/tests/sync.rs | 53 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/aw-sync/src/sync.rs b/aw-sync/src/sync.rs index 079a49ed..e9f26fa8 100644 --- a/aw-sync/src/sync.rs +++ b/aw-sync/src/sync.rs @@ -448,6 +448,20 @@ fn get_or_create_sync_bucket( // needs to be a legal create_bucket value. new_id.clone() }; + // Android maps empty/punctuation-only names to the "unknown" sentinel. + // Creating `-synced-from-unknown` on pull would mix every such remote + // into one destination — the same provenance hole the + // `hostname == "unknown"` guard in `sync_datastores` exists to close. + // Refuse; the per-bucket warn+continue then skips this bucket. + if !is_push + && (sanitized_hostname == "unknown" || sanitized_id.ends_with("-synced-from-unknown")) + { + return Err(format!( + "Bucket '{}' hostname sanitizes to the unknown sentinel; \ + refusing to sync it without provenance", + bucket_from.id + )); + } // If a sanitized bucket already exists, use it. match ds_to.get_bucket(sanitized_id.as_str()) { Ok(bucket) => return Ok(bucket), @@ -1032,5 +1046,8 @@ mod hostname_sanitize_tests { assert_eq!(sanitize_hostname(""), "unknown"); assert_eq!(sanitize_hostname(" "), "unknown"); assert_eq!(sanitize_hostname("***"), "unknown"); + // Whitespace + punctuation only: the get_or_create pull-refuse path. + assert_eq!(sanitize_hostname(" * "), "unknown"); + assert_eq!(sanitize_hostname(" !!! "), "unknown"); } } diff --git a/aw-sync/tests/sync.rs b/aw-sync/tests/sync.rs index 7ccd7351..9bf89fe4 100644 --- a/aw-sync/tests/sync.rs +++ b/aw-sync/tests/sync.rs @@ -301,6 +301,59 @@ mod sync_tests { assert_eq!(dest_bucket.hostname, "poco_f8_ultra"); } + /// A hostname that contains whitespace but sanitizes to the "unknown" + /// sentinel (e.g. `" * "`) must not create `-synced-from-unknown` on pull — + /// that ID is shared by every such remote and would mix events. The bucket + /// is skipped; a healthy sibling still syncs (per-bucket non-fatal). + #[test] + fn test_whitespace_hostname_that_sanitizes_to_unknown_is_skipped_on_pull() { + let state = init_teststate(); + + let junk: Bucket = serde_json::from_value(serde_json::json!({ + "id": "bucket-junk", + "type": "test", + "hostname": " * ", + "client": "test" + })) + .unwrap(); + state.ds_src.create_bucket(&junk).unwrap(); + + let healthy: Bucket = serde_json::from_value(serde_json::json!({ + "id": "bucket-healthy", + "type": "test", + "hostname": "device-0", + "client": "test" + })) + .unwrap(); + state.ds_src.create_bucket(&healthy).unwrap(); + + let result = aw_sync::sync_datastores( + &state.ds_src, + &state.ds_dest, + false, // pull + None, + &SyncSpec::default(), + ); + assert!( + result.is_ok(), + "junk hostname must skip that bucket, not abort the pass; got {result:?}" + ); + + let dest_buckets = state.ds_dest.get_buckets().unwrap(); + assert!( + !dest_buckets + .keys() + .any(|k| k.contains("bucket-junk") || k.ends_with("-synced-from-unknown")), + "must not create -synced-from-unknown, got: {:?}", + dest_buckets.keys().collect::>() + ); + assert!( + dest_buckets.contains_key("bucket-healthy-synced-from-device-0"), + "healthy sibling must still sync, got: {:?}", + dest_buckets.keys().collect::>() + ); + } + /// Bucket metadata of an unexpected shape must not panic: /// `$aw.sync.origin` is read from data written by another host, so it is not /// under this host's control. From 5b188c977eb77b52687e3bf32c3fd17d2f85e52f Mon Sep 17 00:00:00 2001 From: Bob Date: Wed, 16 Sep 2026 10:07:53 +0000 Subject: [PATCH 4/5] fix(aw-sync): sanitize hostnames unconditionally; fail closed on total error Always run DeviceHostname.kt sanitization, not only on whitespace, so PIXEL8 and erb-m2.localdomain land on the same IDs Android's migration will use. Lookup stays raw then sanitized; create under the sanitized ID when it differs. sync_datastores and pull_all still skip individual buckets/peers, but return Err when every attempt fails so a down destination is not reported as success. Git-Session-Id: 82e7d9e5-0c8a-58c0-b129-a46f96ef6953 --- aw-sync/src/sync.rs | 127 +++++++++++++++---------- aw-sync/src/sync_wrapper.rs | 39 ++++++-- aw-sync/tests/sync.rs | 159 ++++++++++++++++++++++++++++---- aw-sync/tests/sync_roundtrip.rs | 6 +- 4 files changed, 255 insertions(+), 76 deletions(-) diff --git a/aw-sync/src/sync.rs b/aw-sync/src/sync.rs index e9f26fa8..51a66ba4 100644 --- a/aw-sync/src/sync.rs +++ b/aw-sync/src/sync.rs @@ -415,63 +415,61 @@ fn get_or_create_sync_bucket( // Look up the unsanitized ID first. Any device that was synced before // aw-android added hostname sanitization (ActivityWatch/aw-android#272) may // have left a local bucket whose ID and hostname contain whitespace (e.g. - // `…-synced-from-POCO F8 Ultra`). Keep using that ID to avoid a fork that - // would cause a full re-import (ActivityWatch/activitywatch#1373). + // `…-synced-from-POCO F8 Ultra`) or a case-only fork (`…-synced-from-PIXEL8`). + // Keep using that ID to avoid a full re-import (ActivityWatch/activitywatch#1373). match ds_to.get_bucket(new_id.as_str()) { Ok(bucket) => return Ok(bucket), Err(DatastoreError::NoSuchBucket(_)) => {} Err(e) => return Err(format!("Failed to get bucket '{new_id}': {e:?}")), } - // The bucket does not exist yet. If the ID *or the source hostname* contains - // whitespace, sanitize before creating: aw-server-rust rejects new buckets - // with whitespace hostnames (#658). The ID-only check is not enough — - // `$aw.sync.origin` can already be clean while `bucket_from.hostname` still - // has spaces, and `create_bucket` would 400 on the hostname field. + // Always sanitize. DeviceHostname.kt lowercases and replaces punctuation, + // not just whitespace — `PIXEL8` vs `pixel8` is the same fork as + // `POCO F8 Ultra` vs `poco_f8_ultra`, just without spaces. Lookup order + // stays *raw ID → sanitized ID*; create under the sanitized ID whenever it + // differs from raw so Android's hostname-column migration lands on an + // existing bucket regardless of which character class differed. // - // Sanitization uses Android's algorithm (not a whitespace-only replace) so - // today's desktop creates `…-synced-from-poco_f8_ultra` and Android's - // hostname-column migration lands on the same ID. - let (final_id, final_hostname) = if bucket_from.hostname.contains(char::is_whitespace) - || new_id.contains(char::is_whitespace) + // Desktop peers with dots (`erb-m2.localdomain`) will create + // `…-synced-from-erb-m2_localdomain` for *new* imports; existing ones are + // found via the raw lookup. That is a display wart the `(device_id, id)` + // identity work in ActivityWatch/activitywatch#302 removes — not a reason + // to keep the fork open. + let sanitized_hostname = sanitize_hostname(&bucket_from.hostname); + let sanitized_id = if let Some(ref origin) = sync_origin { + let orig_bucketid = bucket_from + .id + .split("-synced-from-") + .next() + .unwrap_or(bucket_from.id.as_str()); + format!("{orig_bucketid}-synced-from-{}", sanitize_hostname(origin)) + } else { + // Push path: keep the original bucket ID; only the hostname field + // needs to be a legal create_bucket value. + new_id.clone() + }; + // Android maps empty/punctuation-only names to the "unknown" sentinel. + // Creating `-synced-from-unknown` on pull would mix every such remote + // into one destination — the same provenance hole the + // `hostname == "unknown"` guard in `sync_datastores` exists to close. + // Refuse; the per-bucket warn+continue then skips this bucket. + if !is_push + && (sanitized_hostname == "unknown" || sanitized_id.ends_with("-synced-from-unknown")) { - let sanitized_hostname = sanitize_hostname(&bucket_from.hostname); - let sanitized_id = if let Some(ref origin) = sync_origin { - let orig_bucketid = bucket_from - .id - .split("-synced-from-") - .next() - .unwrap_or(bucket_from.id.as_str()); - format!("{orig_bucketid}-synced-from-{}", sanitize_hostname(origin)) - } else { - // Push path: keep the original bucket ID; only the hostname field - // needs to be a legal create_bucket value. - new_id.clone() - }; - // Android maps empty/punctuation-only names to the "unknown" sentinel. - // Creating `-synced-from-unknown` on pull would mix every such remote - // into one destination — the same provenance hole the - // `hostname == "unknown"` guard in `sync_datastores` exists to close. - // Refuse; the per-bucket warn+continue then skips this bucket. - if !is_push - && (sanitized_hostname == "unknown" || sanitized_id.ends_with("-synced-from-unknown")) - { - return Err(format!( - "Bucket '{}' hostname sanitizes to the unknown sentinel; \ - refusing to sync it without provenance", - bucket_from.id - )); - } - // If a sanitized bucket already exists, use it. + return Err(format!( + "Bucket '{}' hostname sanitizes to the unknown sentinel; \ + refusing to sync it without provenance", + bucket_from.id + )); + } + if sanitized_id != new_id { match ds_to.get_bucket(sanitized_id.as_str()) { Ok(bucket) => return Ok(bucket), Err(DatastoreError::NoSuchBucket(_)) => {} Err(e) => return Err(format!("Failed to get bucket '{sanitized_id}': {e:?}")), } - (sanitized_id, sanitized_hostname) - } else { - (new_id.clone(), bucket_from.hostname.clone()) - }; + } + let (final_id, final_hostname) = (sanitized_id, sanitized_hostname); let mut bucket_new = bucket_from.clone(); bucket_new.id = final_id.clone(); @@ -620,22 +618,41 @@ pub fn sync_datastores( // Sync buckets in order of most recently updated buckets_from.sort_by_key(|b| b.metadata.end); + // Partial failure is non-fatal (one bad bucket must not skip the rest). + // Total failure must still be Err: otherwise a destination that is down + // reports success, which is the #682 silence reintroduced via #688's skip. let mut buckets = Vec::with_capacity(buckets_from.len()); + let mut attempted = 0usize; + let mut succeeded = 0usize; + let mut last_err: Option = None; for bucket_from in buckets_from { + attempted += 1; + let bucket_id = bucket_from.id.clone(); let bucket_to = match get_or_create_sync_bucket(&bucket_from, ds_to, is_push) { Ok(b) => b, Err(e) => { - // Non-fatal: log and skip this bucket so a bad peer does not - // abort the entire pass and leave other buckets un-synced (#692). - warn!(" ! Skipping bucket '{}': {}", bucket_from.id, e); + warn!(" ! Skipping bucket '{}': {}", bucket_id, e); + last_err = Some(e); continue; } }; match sync_one(ds_from, ds_to, bucket_from, bucket_to, sync_spec) { - Ok(synced) => buckets.push(synced), - Err(e) => warn!(" ! Skipping sync for bucket: {}", e), + Ok(synced) => { + succeeded += 1; + buckets.push(synced); + } + Err(e) => { + warn!(" ! Skipping sync for bucket '{}': {}", bucket_id, e); + last_err = Some(e); + } } } + if attempted > 0 && succeeded == 0 { + return Err(format!( + "all {attempted} buckets failed; last error: {}", + last_err.as_deref().unwrap_or("unknown") + )); + } Ok(buckets) } @@ -1035,6 +1052,13 @@ mod hostname_sanitize_tests { // whitespace-only replace would produce "POCO_F8_Ultra" and fork the // day aw-android#272 migrates the phone's hostname column. assert_eq!(sanitize_hostname("POCO F8 Ultra"), "poco_f8_ultra"); + // Case-only fork: no whitespace, but Android still lowercases. + assert_eq!(sanitize_hostname("PIXEL8"), "pixel8"); + // Dotted desktop hostname: punctuation becomes `_`. + assert_eq!( + sanitize_hostname("erb-m2.localdomain"), + "erb-m2_localdomain" + ); } #[test] @@ -1049,5 +1073,10 @@ mod hostname_sanitize_tests { // Whitespace + punctuation only: the get_or_create pull-refuse path. assert_eq!(sanitize_hostname(" * "), "unknown"); assert_eq!(sanitize_hostname(" !!! "), "unknown"); + assert_eq!(sanitize_hostname("PIXEL8"), "pixel8"); + assert_eq!( + sanitize_hostname("erb-m2.localdomain"), + "erb-m2_localdomain" + ); } } diff --git a/aw-sync/src/sync_wrapper.rs b/aw-sync/src/sync_wrapper.rs index 9c73d12f..21955d75 100644 --- a/aw-sync/src/sync_wrapper.rs +++ b/aw-sync/src/sync_wrapper.rs @@ -41,16 +41,29 @@ pub fn pull_all(client: &AwClient) -> Result> { .map(|d| d.path.display().to_string()) .collect::>() ); + // Partial failure is non-fatal (one bad peer must not skip the rest). + // Total failure must still be Err so CLI/JNI/supervisor can tell a + // destination-down pass from a successful one. + let mut attempted = 0usize; + let mut succeeded = 0usize; + let mut last_err: Option = None; for remote in selection.selected { + attempted += 1; + // Per-peer isolation: a peer that fails to open must not abort the + // pass and skip every peer after it. Bucket-level errors are already + // non-fatal in `sync_datastores`; peer-level open failures need the + // same warn+continue so a later skip-on-mismatch (#693) has somewhere + // to go instead of becoming "abort pass" (#688). match pull_db(client, &remote.hostname, &remote.path) { - Ok(one) => report.merge(one), + Ok(one) => { + succeeded += 1; + report.merge(one); + } Err(e) => { - // Per-peer isolation: a peer that fails to open must not abort - // the pass and skip every peer after it. Bucket-level errors are - // already non-fatal in `sync_datastores`; peer-level open - // failures need the same warn+continue so a later - // skip-on-mismatch (#693) has somewhere to go instead of - // becoming "abort pass" (#688). + warn!( + "Skipping peer '{}' ({:?}): {e}", + remote.hostname, remote.path + ); warn!( "Skipping peer '{}' ({:?}): {e}", remote.hostname, remote.path @@ -61,9 +74,21 @@ pub fn pull_all(client: &AwClient) -> Result> { remote.path, e.to_string(), )); + last_err = Some(e.to_string()); } } } + if attempted > 0 && succeeded == 0 { + report.finish(); + // Persist the aggregate (the failures) so a total abort is visible + // afterwards, then fail the pass. + crate::report::persist_last_report_warn(&report); + return Err(format!( + "all {attempted} peers failed; last error: {}", + last_err.as_deref().unwrap_or("unknown") + ) + .into()); + } report.finish(); Ok(report) } diff --git a/aw-sync/tests/sync.rs b/aw-sync/tests/sync.rs index 9bf89fe4..d4e02669 100644 --- a/aw-sync/tests/sync.rs +++ b/aw-sync/tests/sync.rs @@ -103,9 +103,9 @@ mod sync_tests { /// exception — the SIGABRT in ActivityWatch/aw-android#220. `sync_datastores` /// used to `unwrap()` every datastore call, so any failure here was fatal. /// - /// Since the per-bucket non-fatal change (#692), sync_datastores returns - /// Ok(()) and warns when individual buckets fail, so a broken peer does not - /// abort the whole pass. No panic is still the key invariant. + /// Since the per-bucket non-fatal change (#692), a broken *sibling* does + /// not abort the whole pass. A pass where every bucket fails is still Err + /// so callers can tell it from success. No panic is the key invariant. #[test] fn test_unusable_datastore_does_not_panic() { let state = init_teststate(); @@ -118,9 +118,10 @@ mod sync_tests { )) .expect("path is valid UTF-8"); - // Previously this panicked (unwrap on datastore failure); later it - // returned Err; now it returns Ok(()) after skipping the broken bucket. - // The key property: it must not panic. + // Previously this panicked (unwrap on datastore failure). Per-bucket + // skip makes a *partial* failure non-fatal, but every bucket failing + // (destination down) must still be Err so callers can tell it from + // success. The key property: it must not panic. let result = aw_sync::sync_datastores( &state.ds_src, &ds_broken, @@ -128,15 +129,11 @@ mod sync_tests { Some("device-0"), &SyncSpec::default(), ); + let err = + result.expect_err("total bucket failure must return Err, not Ok(()); must not panic"); assert!( - !result.is_err() || result.is_ok(), - "sync_datastores must not panic; got {result:?}" - ); - // With non-fatal per-bucket errors, the function now returns Ok(()) - // and logs a warning rather than propagating the per-bucket failure. - assert!( - result.is_ok(), - "a per-bucket failure must not abort the whole pass; got {result:?}" + err.contains("all 1 buckets failed"), + "error should report total failure, got: {err}" ); } @@ -264,6 +261,130 @@ mod sync_tests { ); } + /// Case-only hostnames (`PIXEL8`) have no whitespace, so a whitespace-only + /// guard would leave the destination as `…-synced-from-PIXEL8`. Android's + /// later hostname migration produces `pixel8` and forks the history. + #[test] + fn test_case_only_hostname_pull_creates_sanitized_bucket() { + let state = init_teststate(); + + let bucket: Bucket = serde_json::from_value(serde_json::json!({ + "id": "aw-watcher-android", + "type": "currentwindow", + "hostname": "PIXEL8", + "client": "aw-android" + })) + .unwrap(); + state.ds_src.create_bucket(&bucket).unwrap(); + + aw_sync::sync_datastores( + &state.ds_src, + &state.ds_dest, + false, + None, + &SyncSpec::default(), + ) + .unwrap(); + + let dest_buckets = state.ds_dest.get_buckets().unwrap(); + let sanitized_id = "aw-watcher-android-synced-from-pixel8"; + assert!( + dest_buckets.contains_key(sanitized_id), + "expected sanitized bucket id '{sanitized_id}', got: {:?}", + dest_buckets.keys().collect::>() + ); + assert!( + !dest_buckets.contains_key("aw-watcher-android-synced-from-PIXEL8"), + "case-only fork must not be created, got: {:?}", + dest_buckets.keys().collect::>() + ); + assert_eq!(dest_buckets.get(sanitized_id).unwrap().hostname, "pixel8"); + } + + /// Dotted desktop hostnames (`erb-m2.localdomain`) sanitize punctuation to + /// `_` for *new* imports. Existing raw IDs are still found via the raw lookup. + #[test] + fn test_dotted_hostname_pull_creates_sanitized_bucket() { + let state = init_teststate(); + + let bucket: Bucket = serde_json::from_value(serde_json::json!({ + "id": "aw-watcher-window", + "type": "currentwindow", + "hostname": "erb-m2.localdomain", + "client": "aw-watcher-window" + })) + .unwrap(); + state.ds_src.create_bucket(&bucket).unwrap(); + + aw_sync::sync_datastores( + &state.ds_src, + &state.ds_dest, + false, + None, + &SyncSpec::default(), + ) + .unwrap(); + + let dest_buckets = state.ds_dest.get_buckets().unwrap(); + let sanitized_id = "aw-watcher-window-synced-from-erb-m2_localdomain"; + assert!( + dest_buckets.contains_key(sanitized_id), + "expected sanitized bucket id '{sanitized_id}', got: {:?}", + dest_buckets.keys().collect::>() + ); + assert!( + !dest_buckets.contains_key("aw-watcher-window-synced-from-erb-m2.localdomain"), + "dotted raw id must not be created for new imports, got: {:?}", + dest_buckets.keys().collect::>() + ); + } + + /// If a legacy case-only bucket already exists, re-use it rather than + /// creating `…-synced-from-pixel8` beside `…-synced-from-PIXEL8`. + #[test] + fn test_case_only_hostname_pull_reuses_legacy_bucket() { + let state = init_teststate(); + + let bucket: Bucket = serde_json::from_value(serde_json::json!({ + "id": "aw-watcher-android", + "type": "currentwindow", + "hostname": "PIXEL8", + "client": "aw-android" + })) + .unwrap(); + state.ds_src.create_bucket(&bucket).unwrap(); + + let legacy_id = "aw-watcher-android-synced-from-PIXEL8"; + let legacy_bucket: Bucket = serde_json::from_value(serde_json::json!({ + "id": legacy_id, + "type": "currentwindow", + "hostname": "PIXEL8", + "client": "aw-android" + })) + .unwrap(); + state.ds_dest.create_bucket(&legacy_bucket).unwrap(); + + aw_sync::sync_datastores( + &state.ds_src, + &state.ds_dest, + false, + None, + &SyncSpec::default(), + ) + .unwrap(); + + let dest_buckets = state.ds_dest.get_buckets().unwrap(); + assert!( + dest_buckets.contains_key(legacy_id), + "legacy case-only bucket must be preserved" + ); + assert!( + !dest_buckets.contains_key("aw-watcher-android-synced-from-pixel8"), + "a sanitized fork must not be created when legacy bucket exists, got: {:?}", + dest_buckets.keys().collect::>() + ); + } + /// If `$aw.sync.origin` is already clean while `bucket.hostname` still has /// whitespace, the sanitizer must still run: otherwise `create_bucket` 400s /// on the hostname field even though the derived ID is legal. @@ -375,8 +496,9 @@ mod sync_tests { .unwrap(); state.ds_src.create_bucket(&bucket).unwrap(); - // Previously this panicked; later it returned Err; now it returns Ok(()) - // after skipping the malformed bucket. No panic is the key invariant. + // Previously this panicked. A single malformed bucket is a total + // failure of the pass, so it must return Err (not Ok after skip). + // No panic is still the key invariant. let result = aw_sync::sync_datastores( &state.ds_src, &state.ds_dest, @@ -384,9 +506,10 @@ mod sync_tests { None, &SyncSpec::default(), ); + let err = result.expect_err("total failure of a one-bucket pass must be Err"); assert!( - result.is_ok(), - "a malformed bucket must not abort the whole pass; got {result:?}" + err.contains("all 1 buckets failed"), + "error should report total failure, got: {err}" ); // The malformed bucket must have been skipped, not imported. let dest_buckets = state.ds_dest.get_buckets().unwrap(); diff --git a/aw-sync/tests/sync_roundtrip.rs b/aw-sync/tests/sync_roundtrip.rs index e9477bf7..1e35aab0 100644 --- a/aw-sync/tests/sync_roundtrip.rs +++ b/aw-sync/tests/sync_roundtrip.rs @@ -87,10 +87,12 @@ fn round_trip() -> (Datastore, Datastore) { sync_datastores(&a_local, &a_export, true, Some("device-A"), &spec).unwrap(); // 2. HOSTB pulls HOSTA's export. This copy is correct and expected. + // Destination IDs use the sanitized origin (`hosta`), matching + // DeviceHostname.kt — not the raw `HOSTA` hostname. sync_datastores(&a_export, &b_local, false, None, &spec).unwrap(); assert!( - bucket_ids(&b_local).contains(&"aw-watcher-window_HOSTA-synced-from-HOSTA".to_string()), - "precondition: HOSTB should hold HOSTA's data as a synced-from-HOSTA bucket, got {:?}", + bucket_ids(&b_local).contains(&"aw-watcher-window_HOSTA-synced-from-hosta".to_string()), + "precondition: HOSTB should hold HOSTA's data as a synced-from-hosta bucket, got {:?}", bucket_ids(&b_local) ); From bff7a2a7702f0624fe5771eddcf07a82c9ed1e10 Mon Sep 17 00:00:00 2001 From: Bob Date: Wed, 16 Sep 2026 14:22:09 +0000 Subject: [PATCH 5/5] fix(aw-sync): warn that a skipped bucket may be partially written A failed sync_one can leave dest with a partial chunk. The per-bucket warn+continue now names that, so a skipped bucket is not mistaken for an untouched skip. Git-Session-Id: pm-697-rebase-2026-09-16 --- aw-sync/src/sync.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/aw-sync/src/sync.rs b/aw-sync/src/sync.rs index 51a66ba4..7b48e0b6 100644 --- a/aw-sync/src/sync.rs +++ b/aw-sync/src/sync.rs @@ -642,7 +642,10 @@ pub fn sync_datastores( buckets.push(synced); } Err(e) => { - warn!(" ! Skipping sync for bucket '{}': {}", bucket_id, e); + warn!( + " ! Skipping sync for bucket '{bucket_id}': {e}. \ + Destination may already contain a partial write; next pass resumes from dest newest" + ); last_err = Some(e); } }