Merge pull request #52 from SteelDynamite/claude/smoke-test-and-fixes-TwfSh

This commit is contained in:
SteelDynamite 2026-04-18 09:49:21 +01:00 committed by GitHub
commit 065118789f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
19 changed files with 390 additions and 157 deletions

View file

@ -179,6 +179,13 @@ fn add_workspace(
state: State<'_, Mutex<AppState>>,
) -> 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<String>,
parent_id: Option<String>,
date: Option<chrono::DateTime<chrono::Utc>>,
has_time: Option<bool>,
state: State<'_, Mutex<AppState>>,
) -> Result<Task, String> {
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<Uuid> = 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<Uuid> = Vec::new();
let mut frontier: Vec<Uuid> = 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)

View file

@ -1,42 +0,0 @@
<script lang="ts">
import type { Snippet } from "svelte";
let { onclose, children }: { onclose: () => void; children: Snippet } = $props();
</script>
<!-- Backdrop -->
<div
class="fixed inset-0 z-40 bg-black/40"
role="button"
tabindex="-1"
aria-label="Close sheet"
onclick={onclose}
onkeydown={(e) => { if (e.key === "Escape") onclose(); }}
></div>
<!-- Sheet -->
<div
role="dialog"
aria-modal="true"
class="fixed bottom-0 left-0 right-0 z-50 max-h-[70vh] overflow-y-auto rounded-t-2xl bg-surface-light shadow-xl dark:bg-card-dark animate-slide-up"
>
<!-- Drag handle -->
<div class="flex justify-center py-2">
<div class="h-1 w-8 rounded-full bg-gray-300 dark:bg-gray-600"></div>
</div>
{@render children()}
<div class="h-[env(safe-area-inset-bottom)]"></div>
</div>
<style>
@keyframes slide-up {
from {
transform: translateY(100%);
}
to {
transform: translateY(0);
}
}
.animate-slide-up {
animation: slide-up 0.25s ease-out;
}
</style>

View file

@ -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 @@
<button
onclick={() => selectDay(day)}
class="mx-auto flex h-8 w-8 items-center justify-center rounded-full text-sm transition-colors
{selectedDay === day ? 'bg-primary text-white' : ''}
{isToday(day) && selectedDay !== day ? 'font-bold text-primary' : ''}
{selectedDay !== day && !isToday(day) ? 'hover:bg-black/5 dark:hover:bg-white/10' : ''}"
{isSelected(day) ? 'bg-primary text-white' : ''}
{isToday(day) && !isSelected(day) ? 'font-bold text-primary' : ''}
{!isSelected(day) && !isToday(day) ? 'hover:bg-black/5 dark:hover:bg-white/10' : ''}"
>
{day}
</button>

View file

@ -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);
});
});

View file

@ -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;

View file

@ -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) {

View file

@ -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<HTMLInputElement | null>(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}
<input
type="text"
bind:this={renameInput}
bind:value={renameValue}
class="w-full bg-transparent text-xl font-bold outline-none"
onkeydown={(e) => { if (e.key === "Enter") handleRename(); if (e.key === "Escape") { renaming = false; } }}
onblur={handleRename}
autofocus
/>
{:else}
<p class="text-xl font-bold">{ws?.name}</p>
@ -172,6 +199,7 @@
<input
type="url"
bind:value={webdavUrl}
oninput={markDirty}
placeholder="https://dav.example.com/tasks/"
class="mb-3 w-full rounded-lg border border-border-light bg-transparent px-3 py-2 text-sm outline-none focus:border-primary dark:border-border-dark"
/>
@ -180,6 +208,7 @@
<input
type="text"
bind:value={webdavUser}
oninput={markDirty}
class="mb-3 w-full rounded-lg border border-border-light bg-transparent px-3 py-2 text-sm outline-none focus:border-primary dark:border-border-dark"
/>
@ -187,6 +216,7 @@
<input
type="password"
bind:value={webdavPass}
oninput={markDirty}
class="mb-4 w-full rounded-lg border border-border-light bg-transparent px-3 py-2 text-sm outline-none focus:border-primary dark:border-border-dark"
/>
@ -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"}
</button>
<button
onclick={saveWebdav}

View file

@ -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 {

View file

@ -58,6 +58,7 @@
let completedVisible = $state(false);
let renamingListId = $state<string | null>(null);
let renameValue = $state("");
let renameListInput = $state<HTMLInputElement | null>(null);
let showListMenu = $state(false);
let showSubtasks = $state(false);
let confirmDeleteList = $state(false);
@ -85,6 +86,14 @@
if (showNewList && newListInput) newListInput.focus();
});
// Same imperative-focus trick for the inline list-rename input.
$effect(() => {
if (renamingListId && renameListInput) {
renameListInput.focus();
renameListInput.select();
}
});
async function handleNewList() {
if (!newListName.trim()) return;
@ -100,7 +109,12 @@
async function executeDeleteCompleted() {
confirmDeleteCompleted = false;
for (var t of app.completedTasks) await app.deleteTask(t.id);
// Snapshot targets first — deletes mutate app.completedTasks reactively.
// Bail on first failure so we don't silently leave a partial delete.
const targets = [...app.completedTasks];
for (const t of targets) {
if (!(await app.deleteTask(t.id))) return;
}
}
function promptDeleteList() {
@ -626,11 +640,11 @@
{#if renamingListId === app.activeListId}
<input
type="text"
bind:this={renameListInput}
bind:value={renameValue}
class="w-full bg-transparent text-xl font-bold outline-none"
onkeydown={(e) => { if (e.key === "Enter") handleRenameList(); if (e.key === "Escape") renamingListId = null; }}
onblur={handleRenameList}
autofocus
/>
{:else}
<p class="text-xl font-bold">{app.activeList?.title ?? "Tasks"}</p>
@ -643,7 +657,16 @@
{#if app.lists.length === 0}
<div class="flex h-full flex-col items-center justify-center p-8 text-center">
<p class="text-lg font-medium opacity-60">No lists yet</p>
<p class="mt-1 text-sm opacity-40">Tap the list name above to create one</p>
{#if app.isGoogleTasks}
<p class="mt-1 text-sm opacity-40">Lists will appear after your next sync.</p>
{:else}
<button
onclick={() => { showDrawer = true; showNewList = true; }}
class="mt-4 rounded-lg bg-primary px-4 py-2 text-sm font-medium text-white hover:bg-primary-hover"
>
Create a list
</button>
{/if}
</div>
{:else if !app.activeListId}
<div class="flex h-full items-center justify-center opacity-40">

View file

@ -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<AppConfig>("get_config");
if (!hasWorkspace) {
screen = "setup";
lists = [];
tasks = [];
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";
}
} catch (e) {
error = String(e);
@ -255,7 +264,13 @@ async function deleteList(id: string) {
}
}
async function createTask(title: string, description?: string, parentId?: string): Promise<Task | null> {
async function createTask(
title: string,
description?: string,
parentId?: string,
date?: string | null,
hasTime?: boolean,
): Promise<Task | null> {
if (!activeListId) return null;
try {
const task = await invoke<Task>("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<AppConfig>("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) {

View file

@ -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<String>) -> 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 <name> <path>' to create one, or 'onyx workspace switch <name>' to select one.")?;
Ok((id.clone(), ws.clone()))
}
}
pub fn get_repository(workspace_identifier: Option<String>) -> 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<String>) {
let mut config = AppConfig::new();
let ids: Vec<String> = 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}");
}
}

View file

@ -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<String>) -> Result<()> {

View file

@ -119,13 +119,26 @@ pub fn edit(task_id_str: String, workspace: Option<String>) -> 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(|_| {

View file

@ -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<String> {
/// 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<String> {
// 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),
}
}

View file

@ -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 {

View file

@ -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,

View file

@ -26,7 +26,10 @@ impl TaskRepository {
// Task operations
pub fn create_task(&mut self, list_id: Uuid, mut task: Task) -> Result<Task> {
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();

View file

@ -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)?;

View file

@ -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<Utc> = 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();