From cdef59fab4e3fdde58d04ea3a1dc4e28e421f499 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 17 Apr 2026 16:12:49 +0000 Subject: [PATCH 01/20] fix: keep user on tasks screen when removing current workspace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a user deletes the current workspace from settings, the backend clears current_workspace and the frontend's hasWorkspace derived fell through to the setup screen — even if the user still had other healthy workspaces configured. Mirror the forgetMissingWorkspace flow: switch to the next available workspace automatically. --- apps/tauri/src/lib/stores/app.svelte.ts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/apps/tauri/src/lib/stores/app.svelte.ts b/apps/tauri/src/lib/stores/app.svelte.ts index 10031b6..18f17e5 100644 --- a/apps/tauri/src/lib/stores/app.svelte.ts +++ b/apps/tauri/src/lib/stores/app.svelte.ts @@ -184,11 +184,17 @@ async function removeWorkspace(id: string) { try { await invoke("remove_workspace", { id }); config = await invoke("get_config"); - if (!hasWorkspace) { + activeListId = null; + tasks = []; + lists = []; + // Switch to the next available workspace rather than dumping the user + // to the setup screen when they still have other workspaces. + const remaining = Object.keys(config?.workspaces ?? {}); + if (remaining.length > 0) { + await switchWorkspace(remaining[0]); + screen = "tasks"; + } else { screen = "setup"; - lists = []; - tasks = []; - activeListId = null; } } catch (e) { error = String(e); From 855fa46a0e232213921a6266c1f51857345b7c02 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 17 Apr 2026 16:13:46 +0000 Subject: [PATCH 02/20] refactor: simplify forgetMissingWorkspace now that removeWorkspace handles switch removeWorkspace already switches to the next available workspace (or falls back to setup). forgetMissingWorkspace can just delegate, dropping the duplicate branch that previously never ran anyway because current_workspace was always null after removal. --- apps/tauri/src/lib/stores/app.svelte.ts | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/apps/tauri/src/lib/stores/app.svelte.ts b/apps/tauri/src/lib/stores/app.svelte.ts index 18f17e5..2b57daa 100644 --- a/apps/tauri/src/lib/stores/app.svelte.ts +++ b/apps/tauri/src/lib/stores/app.svelte.ts @@ -525,22 +525,10 @@ async function addGoogleTasksWorkspace( async function forgetMissingWorkspace() { if (!missingWorkspace) return; + // removeWorkspace handles switching to the next available workspace (or + // falling back to the setup screen when none remain); just delegate. await removeWorkspace(missingWorkspace); missingWorkspace = null; - config = await invoke("get_config"); - if (hasWorkspace) { - // Switch to the next available workspace - const nextName = Object.keys(config!.workspaces)[0]; - if (nextName) { - await switchWorkspace(nextName); - screen = "tasks"; - return; - } - } - screen = "setup"; - lists = []; - tasks = []; - activeListId = null; } function setScreen(s: Screen) { From 433a9504183c2d41a6f652c712e436b3d5f3c7c2 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 17 Apr 2026 16:15:45 +0000 Subject: [PATCH 03/20] fix(cli): accept workspace name or UUID, auto-select on first add MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three related CLI bugs found during smoke testing: 1. `get_repository` used `config.get_workspace(name)` which expects the UUID string, so `onyx list create -w dev` or `onyx task add -w dev` always failed with "Workspace 'dev' not found". Unified CLI resolution into a single `resolve_workspace()` helper that accepts either the display name or the UUID; removed sync.rs's duplicated local copy. 2. `workspace switch`/`remove`/`retarget`/`migrate` only accepted the display name — the error message even suggested "Use the workspace ID instead" on ambiguous names, but IDs were then rejected. Updated `resolve_name` to try the map key first. 3. `onyx workspace add` never set `current_workspace`, so the very next command failed with "No workspace set. Use 'onyx init'..." even though a workspace was just created. Now sets the new workspace as current whenever none was previously selected, and reports the fact. Updated the error message to point at the correct `workspace add` / `workspace switch` commands instead of `init`. --- crates/onyx-cli/src/commands/mod.rs | 34 +++++++++++++++-------- crates/onyx-cli/src/commands/sync.rs | 16 +---------- crates/onyx-cli/src/commands/workspace.rs | 25 +++++++++++++---- 3 files changed, 44 insertions(+), 31 deletions(-) diff --git a/crates/onyx-cli/src/commands/mod.rs b/crates/onyx-cli/src/commands/mod.rs index c681294..7731bc0 100644 --- a/crates/onyx-cli/src/commands/mod.rs +++ b/crates/onyx-cli/src/commands/mod.rs @@ -6,6 +6,7 @@ pub mod group; pub mod sync; use onyx_core::{AppConfig, TaskRepository}; +use onyx_core::config::WorkspaceConfig; use anyhow::{Context, Result}; use std::path::PathBuf; @@ -23,18 +24,29 @@ pub fn save_config(config: &AppConfig) -> Result<()> { config.save_to_file(&path).context("Failed to save config") } -pub fn get_repository(workspace_name: Option) -> Result<(TaskRepository, String)> { - let config = load_config()?; - - let (name, workspace_config) = if let Some(name) = workspace_name { - let workspace_config = config.get_workspace(&name) - .ok_or_else(|| anyhow::anyhow!("Workspace '{}' not found", name))?; - (name, workspace_config.clone()) +/// Resolve a user-supplied identifier to (id, WorkspaceConfig). Accepts either +/// the workspace's display name or its UUID. Falls back to the current +/// workspace when `identifier` is `None`. +pub fn resolve_workspace(config: &AppConfig, identifier: Option<&str>) -> Result<(String, WorkspaceConfig)> { + if let Some(s) = identifier { + // Try by UUID first (exact match on map key), then fall back to name lookup. + if let Some(ws) = config.get_workspace(s) { + return Ok((s.to_string(), ws.clone())); + } + let (id, ws) = config.find_by_name(s) + .ok_or_else(|| anyhow::anyhow!("Workspace '{}' not found", s))?; + Ok((id.clone(), ws.clone())) } else { - let (name, workspace_config) = config.get_current_workspace() - .context("No workspace set. Use 'onyx init' to create one.")?; - (name.clone(), workspace_config.clone()) - }; + let (id, ws) = config.get_current_workspace() + .context("No workspace set. Run 'onyx workspace add ' to create one, or 'onyx workspace switch ' to select one.")?; + Ok((id.clone(), ws.clone())) + } +} + +pub fn get_repository(workspace_identifier: Option) -> Result<(TaskRepository, String)> { + let config = load_config()?; + let (_id, workspace_config) = resolve_workspace(&config, workspace_identifier.as_deref())?; + let name = workspace_config.name.clone(); let repo = TaskRepository::new(workspace_config.path.clone()) .context(format!("Failed to open workspace '{}'", name))?; diff --git a/crates/onyx-cli/src/commands/sync.rs b/crates/onyx-cli/src/commands/sync.rs index c34d502..605eaee 100644 --- a/crates/onyx-cli/src/commands/sync.rs +++ b/crates/onyx-cli/src/commands/sync.rs @@ -2,22 +2,8 @@ use anyhow::{Context, Result}; use colored::Colorize; use onyx_core::sync::{SyncMode, sync_workspace, get_sync_status}; use onyx_core::webdav::{WebDavClient, store_credentials, load_credentials}; -use onyx_core::config::AppConfig; use crate::output; -use super::{load_config, save_config}; - -/// Resolve a workspace name to (id, config). Falls back to current workspace if name is None. -fn resolve_workspace(config: &AppConfig, name: Option<&str>) -> Result<(String, onyx_core::config::WorkspaceConfig)> { - if let Some(name) = name { - let (id, ws) = config.find_by_name(name) - .ok_or_else(|| anyhow::anyhow!("Workspace '{}' not found", name))?; - Ok((id.clone(), ws.clone())) - } else { - let (id, ws) = config.get_current_workspace() - .context("No workspace set. Use 'onyx init' to create one.")?; - Ok((id.clone(), ws.clone())) - } -} +use super::{load_config, save_config, resolve_workspace}; /// Run sync setup: prompt for URL, username, password, test connection, store credentials. pub fn setup(workspace_name: Option) -> Result<()> { diff --git a/crates/onyx-cli/src/commands/workspace.rs b/crates/onyx-cli/src/commands/workspace.rs index ac10a94..18b02ba 100644 --- a/crates/onyx-cli/src/commands/workspace.rs +++ b/crates/onyx-cli/src/commands/workspace.rs @@ -30,11 +30,21 @@ pub fn add(name: String, path: String) -> Result<()> { // Add workspace let id = config.add_workspace(WorkspaceConfig::new(name.clone(), path_buf.clone())); + // Select the new workspace as current when none was previously set, so the + // very next command doesn't fail with "No workspace set". + let made_current = config.current_workspace.is_none(); + if made_current { + config.set_current_workspace(id.clone())?; + } + // Save config save_config(&config)?; output::success(&format!("Added workspace \"{}\" ({}) at {}", name, &id[..8], path_buf.display())); output::success("Created default list \"My Tasks\""); + if made_current { + output::success(&format!("Set \"{}\" as the current workspace", name)); + } Ok(()) } @@ -64,15 +74,20 @@ pub fn list() -> Result<()> { Ok(()) } -/// Resolve a workspace name to its ID. Errors if not found or ambiguous. -fn resolve_name(config: &onyx_core::config::AppConfig, name: &str) -> Result { +/// Resolve a user-supplied identifier to a workspace ID. Accepts either the +/// display name or the UUID. Errors if not found or ambiguous. +fn resolve_name(config: &onyx_core::config::AppConfig, identifier: &str) -> Result { + // Direct UUID hit on the map key — unambiguous. + if config.workspaces.contains_key(identifier) { + return Ok(identifier.to_string()); + } let matches: Vec<_> = config.workspaces.iter() - .filter(|(_, ws)| ws.name == name) + .filter(|(_, ws)| ws.name == identifier) .collect(); match matches.len() { - 0 => anyhow::bail!("Workspace '{}' not found", name), + 0 => anyhow::bail!("Workspace '{}' not found", identifier), 1 => Ok(matches[0].0.clone()), - n => anyhow::bail!("Ambiguous: {} workspaces named '{}'. Use the workspace ID instead.", n, name), + n => anyhow::bail!("Ambiguous: {} workspaces named '{}'. Use the workspace ID instead.", n, identifier), } } From 8a81f0549278d9c9b3b7b1709776b229b8945111 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 17 Apr 2026 16:16:52 +0000 Subject: [PATCH 04/20] fix(cli): print clean error chain instead of anyhow Debug with backtrace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When RUST_BACKTRACE was set in the environment, every user-facing error dumped a 20-line Rust backtrace at the user — e.g. running 'onyx list show' with no workspace gave them a stack trace through anyhow, clap, and libc start. Replace 'fn main() -> Result' with an explicit error printer that walks the anyhow cause chain using Display, and exits 1. Programming-bug panics still surface through the default panic handler. --- crates/onyx-cli/src/main.rs | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/crates/onyx-cli/src/main.rs b/crates/onyx-cli/src/main.rs index ebaae18..76518af 100644 --- a/crates/onyx-cli/src/main.rs +++ b/crates/onyx-cli/src/main.rs @@ -3,6 +3,7 @@ mod output; use anyhow::Result; use clap::{Parser, Subcommand}; +use colored::Colorize; use commands::*; #[derive(Parser)] @@ -197,7 +198,24 @@ enum GroupCommands { }, } -fn main() -> Result<()> { +fn main() { + match run() { + Ok(()) => {} + Err(e) => { + // Print user-friendly error chain (no backtrace). Programming-bug + // panics still surface through their default handler. + eprintln!("{}: {}", "Error".red().bold(), e); + let mut cause = e.source(); + while let Some(c) = cause { + eprintln!(" caused by: {}", c); + cause = c.source(); + } + std::process::exit(1); + } + } +} + +fn run() -> Result<()> { let cli = Cli::parse(); match cli.command { From a0e2bb214b3c96716da2da0a6372039bde39518e Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 17 Apr 2026 16:17:36 +0000 Subject: [PATCH 05/20] fix(date-picker): don't mark the same day in every month as selected The day cell class used `selectedDay === day`, ignoring the currently viewed month/year. After picking e.g. April 15, flipping to May still painted May 15 as the selected day; committing with Done would shift the task's date to whatever month the user happened to be viewing. Track selectedYear/selectedMonth alongside selectedDay, update them only on actual day click, and construct the committed ISO from the selection (not the view). The pre-existing isSelected() helper is now wired into the cell template. --- .../src/lib/components/DateTimePicker.svelte | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/apps/tauri/src/lib/components/DateTimePicker.svelte b/apps/tauri/src/lib/components/DateTimePicker.svelte index 92d92cf..a86d791 100644 --- a/apps/tauri/src/lib/components/DateTimePicker.svelte +++ b/apps/tauri/src/lib/components/DateTimePicker.svelte @@ -50,23 +50,28 @@ function selectDay(day: number) { selectedDay = day; + selectedYear = viewYear; + selectedMonth = viewMonth; } function isToday(day: number): boolean { return `${viewYear}-${viewMonth + 1}-${day}` === todayStr; } + let selectedYear = $state(existing ? existing.getFullYear() : now.getFullYear()); + let selectedMonth = $state(existing ? existing.getMonth() : now.getMonth()); + function isSelected(day: number): boolean { - return selectedDay === day && (!value || (() => { - const v = new Date(value); - return v.getFullYear() === viewYear && v.getMonth() === viewMonth; - })()); + return selectedDay === day && selectedYear === viewYear && selectedMonth === viewMonth; } function done() { const h = includeTime ? selectedHour : 0; const m = includeTime ? selectedMinute : 0; - const iso = new Date(viewYear, viewMonth, selectedDay, h, m).toISOString(); + // Commit based on the last-selected year/month, not the currently-viewed + // ones — users can navigate months after selecting a day without + // accidentally shifting the chosen date to the viewed month. + const iso = new Date(selectedYear, selectedMonth, selectedDay, h, m).toISOString(); onchange(iso, includeTime); dismiss(); } @@ -129,9 +134,9 @@ From 604a6058b82cc379c62c3825045d66989f4bc53c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 17 Apr 2026 16:18:09 +0000 Subject: [PATCH 06/20] fix(storage): atomic task-file writes write_task used plain fs::write for the .md payload even though every other write path in this module (metadata files, sync state, offline queue, config) uses atomic_write. A crash mid-write left a truncated .md file whose malformed YAML frontmatter then failed list_tasks for the entire list. Route through atomic_write so a failed write either leaves the old file intact or produces the full new file. --- crates/onyx-core/src/storage.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/onyx-core/src/storage.rs b/crates/onyx-core/src/storage.rs index c966af0..fbf0a1b 100644 --- a/crates/onyx-core/src/storage.rs +++ b/crates/onyx-core/src/storage.rs @@ -381,7 +381,9 @@ impl Storage for FileSystemStorage { } let content = self.write_markdown_with_frontmatter(task)?; - fs::write(&task_path, content)?; + // Atomic write: a crash mid-write must not leave a truncated .md file + // that then fails YAML parsing on the next list_tasks/read_task. + atomic_write(&task_path, content.as_bytes())?; // Update list metadata to include this task in task_order if not already present let mut list_metadata = self.read_list_metadata(list_id)?; From df66e7bc98507effd78c3b6ec742ab87287836c5 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 17 Apr 2026 16:19:03 +0000 Subject: [PATCH 07/20] fix(tauri): add_workspace must initialise the target folder The frontend currently calls init_workspace before add_workspace, but the Tauri command itself is trivially breakable by any caller that skips the pre-step or a future frontend refactor: add_workspace would save the workspace entry pointing at a non-existent directory, and every subsequent command would then fail with 'Path does not exist' via TaskRepository::new. Call TaskRepository::init inside the command so it is self-contained and idempotent. --- apps/tauri/src-tauri/src/lib.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/apps/tauri/src-tauri/src/lib.rs b/apps/tauri/src-tauri/src/lib.rs index 6db3de8..875a3e2 100644 --- a/apps/tauri/src-tauri/src/lib.rs +++ b/apps/tauri/src-tauri/src/lib.rs @@ -179,6 +179,13 @@ fn add_workspace( state: State<'_, Mutex>, ) -> Result<(), String> { validate_workspace_path(&path)?; + // Ensure the path exists and is a valid workspace before persisting the + // config. Without this, calling add_workspace directly on a missing + // directory would save the workspace but every subsequent ensure_repo + // call would fail with "Path does not exist". + TaskRepository::init(PathBuf::from(&path)) + .map(|_| ()) + .map_err(|e| e.to_string())?; let mut s = lock_state(&state)?; let ws = WorkspaceConfig::new(name, PathBuf::from(&path)); let id = s.config.add_workspace(ws); From f276233be57723d3fae521f33db147e5fbd2e392 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 17 Apr 2026 16:19:46 +0000 Subject: [PATCH 08/20] fix(tauri): cascade delete must handle the full subtree, not just direct children MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit delete_task only collected direct children when a parent was deleted, so grandchildren (and deeper descendants — the data model allows any depth even though the UI is two-level today) would be left with a parent_id pointing at a deleted task. Walk the parent-child graph to collect the full descendant set and delete children before the parent so a mid-cascade failure can't strand descendants. --- apps/tauri/src-tauri/src/lib.rs | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/apps/tauri/src-tauri/src/lib.rs b/apps/tauri/src-tauri/src/lib.rs index 875a3e2..937afc4 100644 --- a/apps/tauri/src-tauri/src/lib.rs +++ b/apps/tauri/src-tauri/src/lib.rs @@ -420,14 +420,23 @@ fn delete_task( 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 repo = repo_mut(&mut s)?; - // Cascade-delete subtasks first + // Cascade-delete the full descendant subtree (not just direct children) + // 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 child_ids: Vec = all_tasks - .iter() - .filter(|t| t.parent_id == Some(tid)) - .map(|t| t.id) - .collect(); - for child_id in child_ids { + let mut to_delete: Vec = Vec::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); + frontier.push(t.id); + } + } + } + // Delete children before the parent so a mid-cascade failure doesn't + // leave the parent removed but descendants stranded. + for child_id in to_delete { repo.delete_task(lid, child_id).map_err(|e| format!("Failed to delete subtask {}: {}", child_id, e))?; } repo.delete_task(lid, tid) From c1346248397985b200c35325322acddc880ce2c3 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 17 Apr 2026 16:20:11 +0000 Subject: [PATCH 09/20] fix(repository): saturating_add for in-memory version bump create_task used a plain += on the in-memory version returned to the caller while FileSystemStorage uses saturating_add when serialising the frontmatter. The two would disagree at u64::MAX, and in debug builds the + operator would panic on overflow. Match the storage behaviour. --- crates/onyx-core/src/repository.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/onyx-core/src/repository.rs b/crates/onyx-core/src/repository.rs index 498a542..5c18b60 100644 --- a/crates/onyx-core/src/repository.rs +++ b/crates/onyx-core/src/repository.rs @@ -26,7 +26,10 @@ impl TaskRepository { // Task operations pub fn create_task(&mut self, list_id: Uuid, mut task: Task) -> Result { self.storage.write_task(list_id, &task)?; - task.version += 1; + // Mirror the saturating increment that FileSystemStorage applies to + // the on-disk frontmatter so the in-memory Task matches what was + // written and doesn't wrap at u64::MAX. + task.version = task.version.saturating_add(1); Ok(task) } From b437b0b7b21948405090773ba29da3e14717d09b Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 17 Apr 2026 16:21:24 +0000 Subject: [PATCH 10/20] fix(sync): use atomic_write for all payload file writes during sync Sync's conflict-resolution and download paths wrote the local file with plain fs::write. A crash or I/O error mid-write left a truncated .md or .listdata.json that would then fail YAML/JSON parsing on the next list_tasks. All other callers in this crate use atomic_write; route the four sync call sites through it for consistency and crash safety. --- crates/onyx-core/src/sync.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/crates/onyx-core/src/sync.rs b/crates/onyx-core/src/sync.rs index 7f2c5e5..bfae1f4 100644 --- a/crates/onyx-core/src/sync.rs +++ b/crates/onyx-core/src/sync.rs @@ -5,7 +5,7 @@ use serde::{Deserialize, Serialize}; use sha2::{Sha256, Digest}; use uuid::Uuid; use crate::error::{Error, Result}; -use crate::storage::{ListMetadata, TaskFrontmatter}; +use crate::storage::{atomic_write, ListMetadata, TaskFrontmatter}; use crate::webdav::WebDavClient; /// File-based lock to prevent concurrent sync operations on the same workspace. @@ -743,8 +743,9 @@ async fn execute_action( } else { report(&format!(" ! Conflict: remote wins for {}, recovering local as duplicate", path)); - // Remote wins: overwrite local with remote content - std::fs::write(&local_path, &remote_data)?; + // Remote wins: overwrite local with remote content. Atomic + // so a crash mid-sync cannot leave a truncated file behind. + atomic_write(&local_path, &remote_data)?; let modified = std::fs::metadata(&local_path).ok() .and_then(|m| m.modified().ok()) .map(|t| { let dt: DateTime = t.into(); dt.to_rfc3339() }); @@ -775,7 +776,7 @@ async fn execute_action( let list_dir = workspace_path.join(parts[0]); let dup_filename = format!("{}.md", new_id); let dup_path = list_dir.join(&dup_filename); - std::fs::write(&dup_path, &new_content)?; + atomic_write(&dup_path, new_content.as_bytes())?; // Insert new task adjacent to original in .listdata.json. // If metadata update fails, remove the duplicate file to @@ -791,7 +792,7 @@ async fn execute_action( .unwrap_or(metadata.task_order.len()); metadata.task_order.insert(insert_pos, new_id); let json = serde_json::to_string_pretty(&metadata)?; - std::fs::write(&listdata_path, json)?; + atomic_write(&listdata_path, json.as_bytes())?; Ok(()) })(); if let Err(e) = metadata_updated { @@ -816,7 +817,7 @@ async fn execute_action( if let Some(parent) = local_path.parent() { std::fs::create_dir_all(parent)?; } - std::fs::write(&local_path, &data)?; + atomic_write(&local_path, &data)?; // Record remote's last_modified so next diff won't see a timestamp mismatch let modified = remote_meta.get(path.as_str()).and_then(|r| r.last_modified.clone()); From d01bd9d28042396e3e6da1d6919d63dd41cee352 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 17 Apr 2026 16:22:31 +0000 Subject: [PATCH 11/20] fix(settings): stop clobbering WebDAV edits and save without a successful test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two coupled issues in workspace settings: 1. The credentials-loading effect re-ran whenever ws.webdav_url changed, so any config mutation (e.g. changing sync interval) would trigger a re-load of the stored username/password, overwriting whatever the user was typing into those fields. Gate with a one-shot credsLoaded flag. 2. Save would persist whatever was in the URL input even if the user had never tested it — a typo'd host silently pointed the workspace at a dead server. Now saveWebdav auto-runs the connection test and bails if it fails; any edit to the three inputs clears the "ok" status via markDirty() so the next Save is forced to re-verify. Also replaces the ASCII "Failed -- Retry" with an em dash. --- .../src/lib/screens/SettingsScreen.svelte | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/apps/tauri/src/lib/screens/SettingsScreen.svelte b/apps/tauri/src/lib/screens/SettingsScreen.svelte index d63e617..9e9e581 100644 --- a/apps/tauri/src/lib/screens/SettingsScreen.svelte +++ b/apps/tauri/src/lib/screens/SettingsScreen.svelte @@ -15,14 +15,19 @@ let webdavUser = $state(""); let webdavPass = $state(""); let testStatus = $state<"idle" | "testing" | "ok" | "fail">("idle"); + let credsLoaded = $state(false); let renaming = $state(false); let renameValue = $state(""); let showKebab = $state(false); let confirmRename = $state(false); + // Load stored credentials exactly once for this workspace. Previously this + // ran on every `ws.webdav_url` change, which silently clobbered in-progress + // user edits whenever any other setting updated the config. $effect(() => { - if (!ws?.webdav_url) return; + if (credsLoaded || !ws?.webdav_url) return; + credsLoaded = true; webdavUrl = ws.webdav_url; try { const domain = new URL(ws.webdav_url).hostname; @@ -35,6 +40,12 @@ } catch {} }); + // Any edit invalidates a prior test so users can't Save a config they + // haven't validated since changing it. + function markDirty() { + if (testStatus !== "idle") testStatus = "idle"; + } + async function testConnection() { testStatus = "testing"; try { @@ -51,6 +62,12 @@ async function saveWebdav() { if (!webdavUrl.trim()) return; + // Require a successful test so a typo'd URL can't silently point the + // workspace at a dead server. + if (testStatus !== "ok") { + await testConnection(); + if (testStatus !== "ok") return; + } await invoke("set_webdav_config", { workspaceId, webdavUrl: webdavUrl.trim(), @@ -172,6 +189,7 @@ @@ -180,6 +198,7 @@ @@ -187,6 +206,7 @@ @@ -196,7 +216,7 @@ disabled={!webdavUrl.trim()} class="rounded-lg border border-border-light px-4 py-2 text-sm font-medium hover:bg-black/5 disabled:opacity-40 dark:border-border-dark dark:hover:bg-white/10" > - {testStatus === "testing" ? "Testing..." : testStatus === "ok" ? "Connected" : testStatus === "fail" ? "Failed -- Retry" : "Test Connection"} + {testStatus === "testing" ? "Testing…" : testStatus === "ok" ? "Connected" : testStatus === "fail" ? "Failed — Retry" : "Test Connection"} + {/if} {:else if !app.activeListId}
From f6c8dfc9514e9f6dd9a5ad9b7f6c6f33029d14bc Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 17 Apr 2026 16:28:20 +0000 Subject: [PATCH 18/20] fix(cli): create task-edit scratch file with mode 0600 on unix onyx task edit wrote the task body to /tmp/onyx-.md with the default umask, leaving it world-readable on shared multi-user systems for the duration of the editor session. Open with O_CREAT|O_TRUNC + mode 0600 via OpenOptionsExt on unix; Windows keeps the existing behaviour since unix-style mode bits don't apply. --- crates/onyx-cli/src/commands/task.rs | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/crates/onyx-cli/src/commands/task.rs b/crates/onyx-cli/src/commands/task.rs index 00471de..0bfd58c 100644 --- a/crates/onyx-cli/src/commands/task.rs +++ b/crates/onyx-cli/src/commands/task.rs @@ -119,13 +119,26 @@ pub fn edit(task_id_str: String, workspace: Option) -> Result<()> { let (list_id, task) = find_task(&lists, task_id) .ok_or_else(|| anyhow::anyhow!("Task not found: {}", task_id_str))?; - // Create temporary file with task content + // Create temporary file with task content. On Unix, open with 0600 so + // other local users on a shared system can't read the task body off /tmp + // while the editor is running. let temp_dir = std::env::temp_dir(); let temp_file = temp_dir.join(format!("onyx-{}.md", task.id)); - // Write current task content to temp file let content = format!("# {}\n\n{}", task.title, task.description); - std::fs::write(&temp_file, content)?; + { + use std::io::Write; + let mut opts = std::fs::OpenOptions::new(); + opts.write(true).create(true).truncate(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + opts.mode(0o600); + } + let mut f = opts.open(&temp_file) + .with_context(|| format!("Failed to create {}", temp_file.display()))?; + f.write_all(content.as_bytes())?; + } // Get editor from environment let editor = std::env::var("EDITOR").unwrap_or_else(|_| { From efb4ccaaef19f01e3a3e99da0d256def2d156fc1 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 17 Apr 2026 16:29:04 +0000 Subject: [PATCH 19/20] chore(cleanup): remove unused BottomSheet component and dead testConnection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BottomSheet.svelte is not imported anywhere — NewTaskInput hand-rolls its own sheet. SetupScreen had a standalone testConnection() function that was only ever reachable through connectAndBrowse which calls test_webdav_connection directly; the standalone variant had no callers. --- .../src/lib/components/BottomSheet.svelte | 42 ------------------- apps/tauri/src/lib/screens/SetupScreen.svelte | 14 ------- 2 files changed, 56 deletions(-) delete mode 100644 apps/tauri/src/lib/components/BottomSheet.svelte diff --git a/apps/tauri/src/lib/components/BottomSheet.svelte b/apps/tauri/src/lib/components/BottomSheet.svelte deleted file mode 100644 index 4d2aa7d..0000000 --- a/apps/tauri/src/lib/components/BottomSheet.svelte +++ /dev/null @@ -1,42 +0,0 @@ - - - -
{ if (e.key === "Escape") onclose(); }} ->
- - - - - diff --git a/apps/tauri/src/lib/screens/SetupScreen.svelte b/apps/tauri/src/lib/screens/SetupScreen.svelte index 7a1ad4f..f727487 100644 --- a/apps/tauri/src/lib/screens/SetupScreen.svelte +++ b/apps/tauri/src/lib/screens/SetupScreen.svelte @@ -77,20 +77,6 @@ // ── WebDAV handlers ─────────────────────────────────────────────── - async function testConnection() { - testStatus = "testing"; - try { - await invoke("test_webdav_connection", { - url: webdavUrl, - username: webdavUser, - password: webdavPass, - }); - testStatus = "ok"; - } catch { - testStatus = "fail"; - } - } - async function connectAndBrowse() { testStatus = "testing"; try { From a79dcc461794f216fd6cb8fc034f30708235cf7f Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 17 Apr 2026 16:32:22 +0000 Subject: [PATCH 20/20] test: cover CLI workspace resolver, date picker, saturating version Add regression tests for the bugs found in this smoke test: - resolve_workspace: by-name, by-UUID, unknown-identifier, current-fallback, actionable no-workspace message. - DateTimePicker: selected-day highlight must be month-scoped; committing after navigating months uses the selected month, not the viewed one. - create_task: version is saturating_add on u64::MAX (doesn't panic/wrap). Also fixes the three pre-existing clippy warnings (WorkspaceMode now uses #[derive(Default)] + #[default], repository test drops unused binding, sync test uses struct-update syntax instead of field-reassign-default). --- .../src/lib/components/DateTimePicker.test.ts | 74 +++++++++++++++++++ crates/onyx-cli/src/commands/mod.rs | 57 ++++++++++++++ crates/onyx-core/src/config.rs | 9 +-- crates/onyx-core/src/repository.rs | 16 +++- crates/onyx-core/src/sync.rs | 3 +- 5 files changed, 149 insertions(+), 10 deletions(-) create mode 100644 apps/tauri/src/lib/components/DateTimePicker.test.ts diff --git a/apps/tauri/src/lib/components/DateTimePicker.test.ts b/apps/tauri/src/lib/components/DateTimePicker.test.ts new file mode 100644 index 0000000..755c67a --- /dev/null +++ b/apps/tauri/src/lib/components/DateTimePicker.test.ts @@ -0,0 +1,74 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { render, screen, cleanup } from "@testing-library/svelte"; +import userEvent from "@testing-library/user-event"; +import DateTimePicker from "./DateTimePicker.svelte"; + +beforeEach(() => { + cleanup(); +}); + +describe("DateTimePicker — selected highlight", () => { + it("only marks the selected day in the month/year that was actually picked", async () => { + const user = userEvent.setup(); + // Pick a date in the current month so the component opens on it. + const now = new Date(); + const existing = new Date(now.getFullYear(), now.getMonth(), 15, 0, 0, 0).toISOString(); + + render(DateTimePicker, { + value: existing, + has_time: false, + onchange: vi.fn(), + onclose: vi.fn(), + }); + + // The "15" button for the current month should be rendered with the + // selected styling (bg-primary). + const day15 = screen.getByRole("button", { name: "15" }); + expect(day15.className).toMatch(/bg-primary/); + + // Navigate one month forward. The same "15" cell must NOT be marked as + // selected, because the user hasn't picked a day in that month yet. + const nextMonthBtn = screen.getAllByRole("button").find((b) => + b.querySelector("svg path[d*='M7.21 14.77']"), + ) as HTMLElement; + await user.click(nextMonthBtn); + + const nextMonth15 = screen.getByRole("button", { name: "15" }); + expect(nextMonth15.className).not.toMatch(/bg-primary/); + }); + + it("commits based on the last-selected month, not the currently-viewed month", async () => { + const user = userEvent.setup(); + const onchange = vi.fn(); + const onclose = vi.fn(); + + // Start with April 10 selected (use a fixed month/year so the test is stable). + const existing = new Date(2026, 3, 10, 0, 0, 0).toISOString(); + render(DateTimePicker, { + value: existing, + has_time: false, + onchange, + onclose, + }); + + // Pick the 20th while viewing April. + await user.click(screen.getByRole("button", { name: "20" })); + + // Flip to May. + const nextMonthBtn = screen.getAllByRole("button").find((b) => + b.querySelector("svg path[d*='M7.21 14.77']"), + ) as HTMLElement; + await user.click(nextMonthBtn); + + // Hit Done. + await user.click(screen.getByRole("button", { name: "Done" })); + + expect(onchange).toHaveBeenCalled(); + const committed = new Date(onchange.mock.calls[0][0] as string); + // April == month 3 (0-indexed). We navigated to May without reselecting, + // so the committed date must still be April 20. + expect(committed.getMonth()).toBe(3); + expect(committed.getDate()).toBe(20); + expect(committed.getFullYear()).toBe(2026); + }); +}); diff --git a/crates/onyx-cli/src/commands/mod.rs b/crates/onyx-cli/src/commands/mod.rs index 7731bc0..b4197f1 100644 --- a/crates/onyx-cli/src/commands/mod.rs +++ b/crates/onyx-cli/src/commands/mod.rs @@ -53,3 +53,60 @@ pub fn get_repository(workspace_identifier: Option) -> Result<(TaskRepos Ok((repo, name)) } + +#[cfg(test)] +mod tests { + use super::*; + + fn make_config_with(ws: &[(&str, &str)]) -> (AppConfig, Vec) { + let mut config = AppConfig::new(); + let ids: Vec = ws.iter() + .map(|(name, path)| config.add_workspace(WorkspaceConfig::new(name.to_string(), PathBuf::from(path)))) + .collect(); + (config, ids) + } + + #[test] + fn resolve_by_name() { + let (config, _ids) = make_config_with(&[("dev", "/tmp/dev"), ("home", "/tmp/home")]); + let (id, ws) = resolve_workspace(&config, Some("dev")).unwrap(); + assert_eq!(ws.name, "dev"); + assert!(config.workspaces.contains_key(&id)); + } + + #[test] + fn resolve_by_uuid() { + let (config, ids) = make_config_with(&[("dev", "/tmp/dev")]); + let target = ids[0].clone(); + let (id, ws) = resolve_workspace(&config, Some(&target)).unwrap(); + assert_eq!(id, target); + assert_eq!(ws.name, "dev"); + } + + #[test] + fn resolve_unknown_identifier_errors() { + let (config, _ids) = make_config_with(&[("dev", "/tmp/dev")]); + let err = resolve_workspace(&config, Some("ghost")).unwrap_err(); + assert!(err.to_string().contains("Workspace 'ghost' not found")); + } + + #[test] + fn resolve_falls_back_to_current() { + let (mut config, ids) = make_config_with(&[("a", "/tmp/a"), ("b", "/tmp/b")]); + config.set_current_workspace(ids[1].clone()).unwrap(); + let (id, ws) = resolve_workspace(&config, None).unwrap(); + assert_eq!(id, ids[1]); + assert_eq!(ws.name, "b"); + } + + #[test] + fn resolve_no_current_gives_actionable_message() { + let config = AppConfig::new(); + let err = resolve_workspace(&config, None).unwrap_err(); + let msg = err.to_string(); + // The message should point the user at the right sub-commands, not + // at the obsolete 'onyx init' suggestion. + assert!(msg.contains("workspace add") || msg.contains("workspace switch"), + "expected actionable message, got: {msg}"); + } +} diff --git a/crates/onyx-core/src/config.rs b/crates/onyx-core/src/config.rs index 4498b06..1ad7769 100644 --- a/crates/onyx-core/src/config.rs +++ b/crates/onyx-core/src/config.rs @@ -4,20 +4,15 @@ use serde::{Deserialize, Serialize}; use uuid::Uuid; use crate::error::{Error, Result}; -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Default)] #[serde(rename_all = "lowercase")] pub enum WorkspaceMode { + #[default] Local, Webdav, GoogleTasks, } -impl Default for WorkspaceMode { - fn default() -> Self { - Self::Local - } -} - #[derive(Debug, Clone, Serialize, Deserialize)] pub struct WorkspaceConfig { pub name: String, diff --git a/crates/onyx-core/src/repository.rs b/crates/onyx-core/src/repository.rs index 5c18b60..aa96764 100644 --- a/crates/onyx-core/src/repository.rs +++ b/crates/onyx-core/src/repository.rs @@ -157,7 +157,7 @@ mod tests { // Create a task let task = Task::new("Test Task".to_string()); - let created_task = repo.create_task(list.id, task).unwrap(); + let _ = repo.create_task(list.id, task).unwrap(); // List tasks let tasks = repo.list_tasks(list.id).unwrap(); @@ -165,6 +165,20 @@ mod tests { assert_eq!(tasks[0].title, "Test Task"); } + #[test] + fn test_create_task_saturates_version_at_max() { + let temp_dir = TempDir::new().unwrap(); + let mut repo = TaskRepository::init(temp_dir.path().to_path_buf()).unwrap(); + let list = repo.create_list("L".to_string()).unwrap(); + + // Simulate a task that is already at u64::MAX. A plain `+=` would + // overflow — saturating_add must clamp. + let mut task = Task::new("max".to_string()); + task.version = u64::MAX; + let created = repo.create_task(list.id, task).unwrap(); + assert_eq!(created.version, u64::MAX); + } + #[test] fn test_update_task() { let temp_dir = TempDir::new().unwrap(); diff --git a/crates/onyx-core/src/sync.rs b/crates/onyx-core/src/sync.rs index bfae1f4..17c10ae 100644 --- a/crates/onyx-core/src/sync.rs +++ b/crates/onyx-core/src/sync.rs @@ -1137,8 +1137,7 @@ mod tests { #[test] fn test_sync_state_save_load_roundtrip() { let temp_dir = TempDir::new().unwrap(); - let mut state = SyncState::default(); - state.last_sync = Some(Utc::now()); + let mut state = SyncState { last_sync: Some(Utc::now()), ..Default::default() }; state.record_file("test.md", "abc123", Some("2026-01-01T00:00:00Z"), 42); state.save(temp_dir.path()).unwrap();