diff --git a/apps/tauri/src-tauri/src/lib.rs b/apps/tauri/src-tauri/src/lib.rs index 6db3de8..8f512e9 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); @@ -367,6 +374,8 @@ fn create_task( title: String, description: Option, parent_id: Option, + date: Option>, + has_time: Option, state: State<'_, Mutex>, ) -> Result { let mut s = lock_state(&state)?; @@ -381,6 +390,11 @@ fn create_task( let parent_uuid = Uuid::parse_str(&pid).map_err(|e| e.to_string())?; task.parent_id = Some(parent_uuid); } + // Accept the date fields at creation time so callers don't have to do a + // second update() round-trip just to attach a date — which previously + // dropped the date entirely if the follow-up update failed. + task.date = date; + task.has_time = has_time.unwrap_or(false); repo_mut(&mut s)? .create_task(id, task) .map_err(|e| e.to_string()) @@ -413,14 +427,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) 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/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 @@ 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/apps/tauri/src/lib/components/NewTaskInput.svelte b/apps/tauri/src/lib/components/NewTaskInput.svelte index 9edc833..cf8027d 100644 --- a/apps/tauri/src/lib/components/NewTaskInput.svelte +++ b/apps/tauri/src/lib/components/NewTaskInput.svelte @@ -17,10 +17,15 @@ async function handleSubmit() { if (!title.trim()) return; - const created = await app.createTask(title.trim(), description.trim() || undefined); - if (date && created) { - await app.updateTask({ ...created, date: date, has_time: dateHasTime }); - } + // Pass date/has_time into createTask directly so the date can't be lost + // if a second round-trip to update() failed after the create succeeded. + await app.createTask( + title.trim(), + description.trim() || undefined, + undefined, + date, + dateHasTime, + ); title = ""; description = ""; date = null; diff --git a/apps/tauri/src/lib/components/TaskDetailView.svelte b/apps/tauri/src/lib/components/TaskDetailView.svelte index 39c8571..c5e0a32 100644 --- a/apps/tauri/src/lib/components/TaskDetailView.svelte +++ b/apps/tauri/src/lib/components/TaskDetailView.svelte @@ -120,7 +120,12 @@ async function executeDeleteCompletedSubtasks() { confirmDeleteCompleted = false; showSubtaskMenu = false; - for (const s of completedSubtasks) await app.deleteTask(s.id); + // Snapshot — completedSubtasks is reactive and shrinks as we delete. + // Bail on first failure so we don't silently leave a partial delete. + const targets = [...completedSubtasks]; + for (const s of targets) { + if (!(await app.deleteTask(s.id))) return; + } } function handleSubtaskMenuClickOutside(e: MouseEvent) { diff --git a/apps/tauri/src/lib/screens/SettingsScreen.svelte b/apps/tauri/src/lib/screens/SettingsScreen.svelte index d63e617..d99ff7c 100644 --- a/apps/tauri/src/lib/screens/SettingsScreen.svelte +++ b/apps/tauri/src/lib/screens/SettingsScreen.svelte @@ -15,14 +15,29 @@ 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 renameInput = $state(null); let showKebab = $state(false); let confirmRename = $state(false); + // Imperative focus — Svelte's native autofocus attribute is unreliable + // for inputs that appear only via conditional blocks. $effect(() => { - if (!ws?.webdav_url) return; + if (renaming && renameInput) { + renameInput.focus(); + renameInput.select(); + } + }); + + // 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 (credsLoaded || !ws?.webdav_url) return; + credsLoaded = true; webdavUrl = ws.webdav_url; try { const domain = new URL(ws.webdav_url).hostname; @@ -35,6 +50,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 +72,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(), @@ -116,11 +143,11 @@ {#if renaming} { if (e.key === "Enter") handleRename(); if (e.key === "Escape") { renaming = false; } }} onblur={handleRename} - autofocus /> {:else}

{ws?.name}

@@ -172,6 +199,7 @@ @@ -180,6 +208,7 @@ @@ -187,6 +216,7 @@ @@ -196,7 +226,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}
diff --git a/apps/tauri/src/lib/stores/app.svelte.ts b/apps/tauri/src/lib/stores/app.svelte.ts index 10031b6..c327c0d 100644 --- a/apps/tauri/src/lib/stores/app.svelte.ts +++ b/apps/tauri/src/lib/stores/app.svelte.ts @@ -10,10 +10,13 @@ import type { } from "../types"; import { groupTasksByDate, type TaskGroup } from "../grouping"; -// Listen for file system changes from the backend watcher. +// Listen for file system changes from the backend watcher. Guard against +// firing while the user is on the setup/missing screens — loadLists would +// fail (no workspace) and a debouncedSync against a non-synced workspace +// would be wasted work. listen("fs-changed", () => { + if (!hasWorkspace || screen !== "tasks") return; loadLists(); - // Debounced sync for WebDAV workspaces on local file changes if (isSyncedWorkspace) debouncedSync(); }); @@ -184,11 +187,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); @@ -255,7 +264,13 @@ async function deleteList(id: string) { } } -async function createTask(title: string, description?: string, parentId?: string): Promise { +async function createTask( + title: string, + description?: string, + parentId?: string, + date?: string | null, + hasTime?: boolean, +): Promise { if (!activeListId) return null; try { const task = await invoke("create_task", { @@ -263,6 +278,8 @@ async function createTask(title: string, description?: string, parentId?: string title, description: description ?? "", parentId: parentId ?? null, + date: date ?? null, + hasTime: hasTime ?? false, }); tasks = parentId ? [task, ...tasks] : [...tasks, task]; error = null; @@ -381,7 +398,11 @@ async function triggerSync() { await loadLists(); } catch (e) { const msg = String(e); - const isTransient = /timeout|connect|network|unreachable|refused/i.test(msg); + // Narrow phrases so that a legitimate server-side error containing a + // word like "network" or "refused" in its description isn't silently + // swallowed as an offline blip. Only treat obvious connectivity failures + // as transient. + const isTransient = /(^|\W)(timed? out|timeout|connection (refused|reset|timed out|aborted)|connect error|network (is )?unreachable|no route to host|host (not found|is unreachable)|dns|enotfound|econnrefused|etimedout|ehostunreach|enetunreach)(\W|$)/i.test(msg); syncStatus = isTransient ? "offline" : "error"; // Only show the error banner for non-transient failures; connectivity issues just update the status dot if (!isTransient) error = msg; @@ -519,22 +540,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) { diff --git a/crates/onyx-cli/src/commands/mod.rs b/crates/onyx-cli/src/commands/mod.rs index c681294..b4197f1 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,21 +24,89 @@ 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))?; 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-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/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(|_| { 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), } } 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 { 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 498a542..aa96764 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) } @@ -154,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(); @@ -162,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/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)?; diff --git a/crates/onyx-core/src/sync.rs b/crates/onyx-core/src/sync.rs index 7f2c5e5..17c10ae 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()); @@ -1136,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();