From 70fe7420cd88db985cc393a5521b868aedde64fe Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 20 Apr 2026 07:33:12 +0000 Subject: [PATCH 1/5] refactor(sync): remove dead .listdata.json guard in conflict path The `.listdata.json` check was unreachable: the branch is already gated on `parts[1].ends_with(".md")`, which `.listdata.json` fails. --- crates/onyx-core/src/sync.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/onyx-core/src/sync.rs b/crates/onyx-core/src/sync.rs index 17c10ae..6eb2b57 100644 --- a/crates/onyx-core/src/sync.rs +++ b/crates/onyx-core/src/sync.rs @@ -753,7 +753,7 @@ async fn execute_action( // For .md task files inside a list dir, create a duplicate of the local version let parts: Vec<&str> = path.split('/').collect(); - if parts.len() == 2 && parts[1].ends_with(".md") && parts[1] != ".listdata.json" { + if parts.len() == 2 && parts[1].ends_with(".md") { let local_content = String::from_utf8_lossy(&local_data); if let Ok((frontmatter, description)) = parse_frontmatter_for_conflict(&local_content) { let original_id = frontmatter.id; From 6abe95692e04b6ed0c756715ec3f37502455a5e8 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 20 Apr 2026 07:34:52 +0000 Subject: [PATCH 2/5] perf(tauri): use HashSet for cascade-delete dedup Descendant walking in delete_task called Vec::contains in the inner loop, making the traversal O(n^2) in the number of tasks. Swap the visited set to HashSet so membership tests are O(1); HashSet::insert also folds the contains-check and record-new steps into one call. --- apps/tauri/src-tauri/src/lib.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/apps/tauri/src-tauri/src/lib.rs b/apps/tauri/src-tauri/src/lib.rs index 154f6d0..82d50dc 100644 --- a/apps/tauri/src-tauri/src/lib.rs +++ b/apps/tauri/src-tauri/src/lib.rs @@ -450,12 +450,11 @@ fn delete_task( // so deleting a parent can't leave grandchildren orphaned with a // parent_id pointing at a deleted task. let all_tasks = repo.list_tasks(lid).map_err(|e| e.to_string())?; - let mut to_delete: Vec = Vec::new(); + let mut to_delete: std::collections::HashSet = std::collections::HashSet::new(); let mut frontier: Vec = vec![tid]; while let Some(parent) = frontier.pop() { for t in &all_tasks { - if t.parent_id == Some(parent) && !to_delete.contains(&t.id) { - to_delete.push(t.id); + if t.parent_id == Some(parent) && to_delete.insert(t.id) { frontier.push(t.id); } } From 7754ea4b45bbd07be17c660a9232e896b8e4e27c Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 20 Apr 2026 07:35:12 +0000 Subject: [PATCH 3/5] fix(tauri): surface errors from toggle_task cascade When a parent task was toggled, `update_task` failures on child tasks were silently swallowed with `let _ = ...`, leaving subtasks out of sync with the parent's status and giving the user no feedback. Map the error and propagate so the UI can show it and the user can retry. --- apps/tauri/src-tauri/src/lib.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/tauri/src-tauri/src/lib.rs b/apps/tauri/src-tauri/src/lib.rs index 82d50dc..3d7c4be 100644 --- a/apps/tauri/src-tauri/src/lib.rs +++ b/apps/tauri/src-tauri/src/lib.rs @@ -495,7 +495,9 @@ fn toggle_task( TaskStatus::Backlog => child.uncomplete(), TaskStatus::Completed => child.complete(), } - let _ = repo.update_task(lid, child); + let child_id = child.id; + repo.update_task(lid, child) + .map_err(|e| format!("Failed to cascade to subtask {}: {}", child_id, e))?; } } Ok(task) From f42697f4ed85f3b4b419c91b5a7b5558bdda1c23 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 20 Apr 2026 07:35:50 +0000 Subject: [PATCH 4/5] refactor(tauri): extract parse_uuid helper 17 Tauri commands repeated `Uuid::parse_str(&s).map_err(|e| e.to_string())` for each UUID argument. Collapse the pattern into a `parse_uuid` helper so callers read as `let id = parse_uuid(&list_id)?;`. --- apps/tauri/src-tauri/src/lib.rs | 39 +++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/apps/tauri/src-tauri/src/lib.rs b/apps/tauri/src-tauri/src/lib.rs index 3d7c4be..d71e981 100644 --- a/apps/tauri/src-tauri/src/lib.rs +++ b/apps/tauri/src-tauri/src/lib.rs @@ -60,6 +60,11 @@ fn lock_state(state: &Mutex) -> Result Result { + Uuid::parse_str(s).map_err(|e| e.to_string()) +} + impl AppState { /// Persist config to disk, converting errors to String for Tauri commands. fn save_config(&self) -> Result<(), String> { @@ -366,7 +371,7 @@ fn delete_list( let mut s = lock_state(&state)?; ensure_repo(&mut s)?; mute_watcher(&mut s); - let id = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; + let id = parse_uuid(&list_id)?; repo_mut(&mut s)? .delete_list(id) .map_err(|e| e.to_string()) @@ -381,7 +386,7 @@ fn list_tasks( ) -> Result, String> { let mut s = lock_state(&state)?; ensure_repo(&mut s)?; - let id = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; + let id = parse_uuid(&list_id)?; repo_ref(&s)? .list_tasks(id) .map_err(|e| e.to_string()) @@ -400,13 +405,13 @@ fn create_task( let mut s = lock_state(&state)?; ensure_repo(&mut s)?; mute_watcher(&mut s); - let id = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; + let id = parse_uuid(&list_id)?; let mut task = Task::new(title); if let Some(desc) = description.filter(|d| !d.is_empty()) { task.description = desc; } if let Some(pid) = parent_id { - let parent_uuid = Uuid::parse_str(&pid).map_err(|e| e.to_string())?; + let parent_uuid = parse_uuid(&pid)?; task.parent_id = Some(parent_uuid); } // Accept the date fields at creation time so callers don't have to do a @@ -428,7 +433,7 @@ fn update_task( let mut s = lock_state(&state)?; ensure_repo(&mut s)?; mute_watcher(&mut s); - let id = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; + let id = parse_uuid(&list_id)?; repo_mut(&mut s)? .update_task(id, task) .map_err(|e| e.to_string()) @@ -443,8 +448,8 @@ fn delete_task( let mut s = lock_state(&state)?; ensure_repo(&mut s)?; mute_watcher(&mut s); - let lid = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; - let tid = Uuid::parse_str(&task_id).map_err(|e| e.to_string())?; + let lid = parse_uuid(&list_id)?; + let tid = parse_uuid(&task_id)?; let repo = repo_mut(&mut s)?; // Cascade-delete the full descendant subtree (not just direct children) // so deleting a parent can't leave grandchildren orphaned with a @@ -477,8 +482,8 @@ fn toggle_task( let mut s = lock_state(&state)?; ensure_repo(&mut s)?; mute_watcher(&mut s); - let lid = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; - let tid = Uuid::parse_str(&task_id).map_err(|e| e.to_string())?; + let lid = parse_uuid(&list_id)?; + let tid = parse_uuid(&task_id)?; let repo = repo_mut(&mut s)?; let mut task = repo.get_task(lid, tid).map_err(|e| e.to_string())?; match task.status { @@ -513,8 +518,8 @@ fn reorder_task( let mut s = lock_state(&state)?; ensure_repo(&mut s)?; mute_watcher(&mut s); - let lid = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; - let tid = Uuid::parse_str(&task_id).map_err(|e| e.to_string())?; + let lid = parse_uuid(&list_id)?; + let tid = parse_uuid(&task_id)?; repo_mut(&mut s)? .reorder_task(lid, tid, new_position) .map_err(|e| e.to_string()) @@ -532,9 +537,9 @@ fn move_task( let mut s = lock_state(&state)?; ensure_repo(&mut s)?; mute_watcher(&mut s); - let from = Uuid::parse_str(&from_list_id).map_err(|e| e.to_string())?; - let to = Uuid::parse_str(&to_list_id).map_err(|e| e.to_string())?; - let tid = Uuid::parse_str(&task_id).map_err(|e| e.to_string())?; + let from = parse_uuid(&from_list_id)?; + let to = parse_uuid(&to_list_id)?; + let tid = parse_uuid(&task_id)?; repo_mut(&mut s)? .move_task(from, to, tid) .map_err(|e| e.to_string()) @@ -549,7 +554,7 @@ fn rename_list( let mut s = lock_state(&state)?; ensure_repo(&mut s)?; mute_watcher(&mut s); - let id = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; + let id = parse_uuid(&list_id)?; repo_mut(&mut s)? .rename_list(id, new_name) .map_err(|e| e.to_string()) @@ -564,7 +569,7 @@ fn set_group_by_date( let mut s = lock_state(&state)?; ensure_repo(&mut s)?; mute_watcher(&mut s); - let id = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; + let id = parse_uuid(&list_id)?; repo_mut(&mut s)? .set_group_by_date(id, enabled) .map_err(|e| e.to_string()) @@ -577,7 +582,7 @@ fn get_group_by_date( ) -> Result { let mut s = lock_state(&state)?; ensure_repo(&mut s)?; - let id = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; + let id = parse_uuid(&list_id)?; repo_ref(&s)? .get_group_by_date(id) .map_err(|e| e.to_string()) From 890f0c21263d60941b87ee69c6f533445ddaf662 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 20 Apr 2026 07:37:54 +0000 Subject: [PATCH 5/5] docs(audit): log 2026-04-20 findings --- Audit.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Audit.md b/Audit.md index 3a28260..0204e0b 100644 --- a/Audit.md +++ b/Audit.md @@ -1,5 +1,14 @@ # Audit Log +## 2026-04-20 + +Found and fixed 4 issues: + +1. **Dead code in conflict recovery** (sync.rs:756) — `parts[1] != ".listdata.json"` was unreachable because the branch is already gated on `parts[1].ends_with(".md")`, which `.listdata.json` cannot satisfy. Removed the redundant check. +2. **O(n²) cascade delete** (tauri/lib.rs) — descendant traversal in `delete_task` used `Vec::contains` inside the inner loop, making it quadratic in the number of tasks per list. Swapped the visited set to `HashSet`; `HashSet::insert` folds the contains+push into one call. +3. **Silent cascade failure in toggle_task** (tauri/lib.rs) — subtask `update_task` errors were discarded with `let _ = ...`, leaving subtasks stuck at the old status with no UI feedback. Propagate the error so the frontend can surface it. +4. **Duplicated UUID-parse boilerplate** (tauri/lib.rs) — 17 commands repeated `Uuid::parse_str(&x).map_err(|e| e.to_string())?`. Extracted a `parse_uuid` helper so callers read as `let id = parse_uuid(&list_id)?;`. + ## 2026-04-15 Found and fixed 4 issues: