From 970210b647c7a941bbac808d9f73b316a9aee282 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 24 Apr 2026 07:36:28 +0000 Subject: [PATCH 1/4] refactor(sync): destructure remote in deleted-local branch The `(None, Some(_), Some(b))` arm re-checked the already-matched `remote` via `remote.is_some_and(...)`, which obscures intent and compiles to redundant None-branch code. Bind `Some(r)` in the match and use `r` directly. No behavior change. --- crates/onyx-core/src/sync.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/onyx-core/src/sync.rs b/crates/onyx-core/src/sync.rs index 6eb2b57..58d9945 100644 --- a/crates/onyx-core/src/sync.rs +++ b/crates/onyx-core/src/sync.rs @@ -204,8 +204,9 @@ pub fn compute_sync_actions( } // Remote present, local gone, base known: local was deleted - (None, Some(_), Some(b)) => { - let remote_changed = remote.is_some_and(|r| r.size != b.size || !timestamps_equal(r.last_modified.as_deref(), b.modified_at.as_deref())); + (None, Some(r), Some(b)) => { + let remote_changed = r.size != b.size + || !timestamps_equal(r.last_modified.as_deref(), b.modified_at.as_deref()); if remote_changed { // deleted locally + modified remotely -> download (remote wins) actions.push(SyncAction::Download { path: path.to_string() }); From 1fcc6e7f6dde9747b0f89023076e3df4b062e8d0 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 24 Apr 2026 07:37:39 +0000 Subject: [PATCH 2/4] fix(sync): purge orphan base entries when both sides deleted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `compute_sync_actions` emits no action for files that are missing from both local and remote but still tracked in the sync base (the `(None, None, Some(_))` arm). Nothing else cleaned those entries, so `.syncstate.json` grew forever every time a file was deleted both locally and remotely — and on each subsequent sync the same no-op match fired again. Add a `prune_orphan_bases` pass that runs before `compute_sync_actions` in `sync_workspace_inner`, dropping any base entry whose path is in neither the local nor remote scan. Unit-tested in isolation. --- crates/onyx-core/src/sync.rs | 38 ++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/crates/onyx-core/src/sync.rs b/crates/onyx-core/src/sync.rs index 58d9945..a21cf9d 100644 --- a/crates/onyx-core/src/sync.rs +++ b/crates/onyx-core/src/sync.rs @@ -230,6 +230,22 @@ pub fn compute_sync_actions( actions } +/// Remove base entries for files that are gone from both local and remote. +/// `compute_sync_actions` emits no action for the both-deleted case, so without +/// this pass those entries would persist in `.syncstate.json` indefinitely. +fn prune_orphan_bases( + sync_state: &mut SyncState, + local_files: &[LocalFileInfo], + remote_files: &[RemoteFileSnapshot], +) { + let live_paths: std::collections::HashSet<&str> = local_files + .iter() + .map(|f| f.path.as_str()) + .chain(remote_files.iter().map(|f| f.path.as_str())) + .collect(); + sync_state.files.retain(|p, _| live_paths.contains(p.as_str())); +} + /// Compare two timestamps for equality by parsing both, tolerating format differences. fn timestamps_equal(a: Option<&str>, b: Option<&str>) -> bool { match (a, b) { @@ -605,6 +621,12 @@ async fn sync_workspace_inner( } }; + // Purge orphan base entries: files we previously tracked that are now gone + // from both local and remote. Without this, `.syncstate.json` accumulates + // ghost entries forever because the both-deleted diff case emits no action + // and so nothing else would clean them. + prune_orphan_bases(&mut sync_state, &local_files, &remote_files); + // Compute actions from three-way diff let fresh_actions = compute_sync_actions(&local_files, &remote_files, &sync_state); @@ -1107,6 +1129,22 @@ mod tests { assert!(actions.is_empty()); } + #[test] + fn test_prune_orphan_bases() { + let mut state = SyncState::default(); + state.files.insert("kept_local.md".to_string(), make_base("a")); + state.files.insert("kept_remote.md".to_string(), make_base("b")); + state.files.insert("orphan.md".to_string(), make_base("c")); + + let local = vec![make_local("kept_local.md", "a")]; + let remote = vec![make_remote("kept_remote.md")]; + prune_orphan_bases(&mut state, &local, &remote); + + assert!(state.files.contains_key("kept_local.md")); + assert!(state.files.contains_key("kept_remote.md")); + assert!(!state.files.contains_key("orphan.md")); + } + #[test] fn test_multiple_files_mixed() { let local = vec![ From a9fac2c1d8839bcee50967b8747979f7506aff23 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 24 Apr 2026 07:38:18 +0000 Subject: [PATCH 3/4] refactor(storage): drop single-caller sanitize_filename wrapper `FileSystemStorage::sanitize_filename` was a one-line forwarder to `crate::sanitize_filename` with a single call site in `task_file_path`. The extra method added a layer of indirection without value. Inline the crate-level call. --- crates/onyx-core/src/storage.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/crates/onyx-core/src/storage.rs b/crates/onyx-core/src/storage.rs index 380e757..7f8bc8b 100644 --- a/crates/onyx-core/src/storage.rs +++ b/crates/onyx-core/src/storage.rs @@ -236,12 +236,8 @@ impl FileSystemStorage { Ok(path) } - fn sanitize_filename(name: &str) -> String { - crate::sanitize_filename(name) - } - fn task_file_path(&self, list_dir: &Path, task: &Task) -> PathBuf { - let safe_title = Self::sanitize_filename(&task.title); + let safe_title = crate::sanitize_filename(&task.title); let filename = if safe_title.is_empty() { task.id.to_string() } else { From 8611f555732fa2bd0c6e37077aa1973bb325f558 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 24 Apr 2026 07:38:54 +0000 Subject: [PATCH 4/4] docs(audit): log 2026-04-24 findings --- Audit.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Audit.md b/Audit.md index 0204e0b..977a2c7 100644 --- a/Audit.md +++ b/Audit.md @@ -1,5 +1,13 @@ # Audit Log +## 2026-04-24 + +Found and fixed 3 issues: + +1. **Bug: orphan base entries never cleaned from sync state** (sync.rs) — when a file was deleted both locally and remotely, `compute_sync_actions` emitted no action (the `(None, None, Some(_))` arm), so the base entry in `.syncstate.json` persisted forever. On each subsequent sync the same no-op case fired and the state file grew. Added `prune_orphan_bases` pass in `sync_workspace_inner` that drops base entries not present in either scan. +2. **Code quality: redundant is_some_and on already-matched Option** (sync.rs:208) — the `(None, Some(_), Some(b))` arm re-checked `remote` via `remote.is_some_and(|r| ...)` even though the pattern had just proven `remote` is `Some(_)`. Bound the inner value with `Some(r)` in the pattern and used `r` directly. +3. **Code quality: single-caller sanitize_filename wrapper** (storage.rs) — `FileSystemStorage::sanitize_filename` was a one-line forwarder to `crate::sanitize_filename` with one call site. Inlined the crate call and removed the method. + ## 2026-04-20 Found and fixed 4 issues: