fix: audit — debounced save data loss, silent workspace errors, code duplication
- Fix debouncedSave in TaskDetailView losing edits when title and description are changed within 400ms (shared timer only saved the last-changed field) - Return errors from Tauri commands when workspace ID doesn't exist instead of silently succeeding (set_webdav_config, set_workspace_theme, set_sync_interval, set_sync_interval_unfocused) - Remove duplicate atomic_write_bytes in google_tasks.rs; reuse pub(crate) atomic_write from storage.rs - Fix failing test using wrong frontmatter field name (due → date) - Add Audit.md log https://claude.ai/code/session_0186pnnUJxj2uv1KhHjWoAGA
This commit is contained in:
parent
35db093b79
commit
125f1e19ac
10
Audit.md
Normal file
10
Audit.md
Normal file
|
|
@ -0,0 +1,10 @@
|
||||||
|
# Audit Log
|
||||||
|
|
||||||
|
## 2026-04-15
|
||||||
|
|
||||||
|
Found and fixed 4 issues:
|
||||||
|
|
||||||
|
1. **Bug: debouncedSave shared timer loses edits** (TaskDetailView.svelte) - When user edits both title and description within 400ms, only the last-edited field was saved. Fixed by always saving both fields in the debounced callback.
|
||||||
|
2. **Code duplication: atomic_write_bytes** (google_tasks.rs) - Identical copy of `atomic_write` from storage.rs. Removed duplicate and reused the shared `pub(crate)` function.
|
||||||
|
3. **Bug: silent success on missing workspace** (lib.rs) - Four Tauri commands (`set_webdav_config`, `set_workspace_theme`, `set_sync_interval`, `set_sync_interval_unfocused`) silently succeeded when given a nonexistent workspace ID. Fixed to return an error.
|
||||||
|
4. **Bug: failing test due to wrong frontmatter field name** (storage.rs) - `test_parse_frontmatter_with_optional_fields` used `due:` instead of `date:` in frontmatter YAML, causing the assertion on `fm.date.is_some()` to fail.
|
||||||
|
|
@ -549,9 +549,9 @@ fn set_webdav_config(
|
||||||
state: State<'_, Mutex<AppState>>,
|
state: State<'_, Mutex<AppState>>,
|
||||||
) -> Result<(), String> {
|
) -> Result<(), String> {
|
||||||
let mut s = lock_state(&state)?;
|
let mut s = lock_state(&state)?;
|
||||||
if let Some(ws) = s.config.workspaces.get_mut(&workspace_id) {
|
let ws = s.config.workspaces.get_mut(&workspace_id)
|
||||||
|
.ok_or_else(|| format!("Workspace '{}' not found", workspace_id))?;
|
||||||
ws.webdav_url = Some(webdav_url);
|
ws.webdav_url = Some(webdav_url);
|
||||||
}
|
|
||||||
s.save_config()
|
s.save_config()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -562,9 +562,9 @@ fn set_workspace_theme(
|
||||||
state: State<'_, Mutex<AppState>>,
|
state: State<'_, Mutex<AppState>>,
|
||||||
) -> Result<(), String> {
|
) -> Result<(), String> {
|
||||||
let mut s = lock_state(&state)?;
|
let mut s = lock_state(&state)?;
|
||||||
if let Some(ws) = s.config.workspaces.get_mut(&workspace_id) {
|
let ws = s.config.workspaces.get_mut(&workspace_id)
|
||||||
|
.ok_or_else(|| format!("Workspace '{}' not found", workspace_id))?;
|
||||||
ws.theme = theme;
|
ws.theme = theme;
|
||||||
}
|
|
||||||
s.save_config()
|
s.save_config()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -575,9 +575,9 @@ fn set_sync_interval(
|
||||||
state: State<'_, Mutex<AppState>>,
|
state: State<'_, Mutex<AppState>>,
|
||||||
) -> Result<(), String> {
|
) -> Result<(), String> {
|
||||||
let mut s = lock_state(&state)?;
|
let mut s = lock_state(&state)?;
|
||||||
if let Some(ws) = s.config.workspaces.get_mut(&workspace_id) {
|
let ws = s.config.workspaces.get_mut(&workspace_id)
|
||||||
|
.ok_or_else(|| format!("Workspace '{}' not found", workspace_id))?;
|
||||||
ws.sync_interval_secs = interval_secs;
|
ws.sync_interval_secs = interval_secs;
|
||||||
}
|
|
||||||
s.save_config()
|
s.save_config()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -588,9 +588,9 @@ fn set_sync_interval_unfocused(
|
||||||
state: State<'_, Mutex<AppState>>,
|
state: State<'_, Mutex<AppState>>,
|
||||||
) -> Result<(), String> {
|
) -> Result<(), String> {
|
||||||
let mut s = lock_state(&state)?;
|
let mut s = lock_state(&state)?;
|
||||||
if let Some(ws) = s.config.workspaces.get_mut(&workspace_id) {
|
let ws = s.config.workspaces.get_mut(&workspace_id)
|
||||||
|
.ok_or_else(|| format!("Workspace '{}' not found", workspace_id))?;
|
||||||
ws.sync_interval_unfocused_secs = interval_secs;
|
ws.sync_interval_unfocused_secs = interval_secs;
|
||||||
}
|
|
||||||
s.save_config()
|
s.save_config()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -32,19 +32,19 @@
|
||||||
if (isDesktop) appWindow.startDragging();
|
if (isDesktop) appWindow.startDragging();
|
||||||
}
|
}
|
||||||
|
|
||||||
function debouncedSave(fields: Partial<Task>) {
|
function debouncedSave() {
|
||||||
clearTimeout(saveTimer);
|
clearTimeout(saveTimer);
|
||||||
saveTimer = setTimeout(() => {
|
saveTimer = setTimeout(() => {
|
||||||
app.updateTask({ ...task, ...fields });
|
app.updateTask({ ...task, title: title.trim() || task.title, description });
|
||||||
}, 400);
|
}, 400);
|
||||||
}
|
}
|
||||||
|
|
||||||
function handleTitleInput() {
|
function handleTitleInput() {
|
||||||
debouncedSave({ title: title.trim() || task.title });
|
debouncedSave();
|
||||||
}
|
}
|
||||||
|
|
||||||
function handleDescInput() {
|
function handleDescInput() {
|
||||||
debouncedSave({ description });
|
debouncedSave();
|
||||||
}
|
}
|
||||||
|
|
||||||
function handleDateChange(iso: string | null, hasTime: boolean = false) {
|
function handleDateChange(iso: string | null, hasTime: boolean = false) {
|
||||||
|
|
|
||||||
|
|
@ -14,7 +14,7 @@ use uuid::Uuid;
|
||||||
|
|
||||||
use crate::error::{Error, Result};
|
use crate::error::{Error, Result};
|
||||||
use crate::models::{Task, TaskStatus};
|
use crate::models::{Task, TaskStatus};
|
||||||
use crate::storage::{ListMetadata, RootMetadata};
|
use crate::storage::{ListMetadata, RootMetadata, atomic_write};
|
||||||
|
|
||||||
const REQUEST_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(30);
|
const REQUEST_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(30);
|
||||||
const CONNECT_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10);
|
const CONNECT_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10);
|
||||||
|
|
@ -348,7 +348,7 @@ pub async fn sync_google_tasks(
|
||||||
};
|
};
|
||||||
|
|
||||||
let content = render_task_markdown(&task);
|
let content = render_task_markdown(&task);
|
||||||
if let Err(e) = atomic_write_bytes(&task_path, content.as_bytes()) {
|
if let Err(e) = atomic_write(&task_path, content.as_bytes()) {
|
||||||
errors.push(format!("Failed to write task '{}': {}", task.title, e));
|
errors.push(format!("Failed to write task '{}': {}", task.title, e));
|
||||||
} else {
|
} else {
|
||||||
downloaded += 1;
|
downloaded += 1;
|
||||||
|
|
@ -359,7 +359,7 @@ pub async fn sync_google_tasks(
|
||||||
list_meta.updated_at = Utc::now();
|
list_meta.updated_at = Utc::now();
|
||||||
|
|
||||||
if let Ok(meta_content) = serde_json::to_string_pretty(&list_meta) {
|
if let Ok(meta_content) = serde_json::to_string_pretty(&list_meta) {
|
||||||
let _ = atomic_write_bytes(&listdata_path, meta_content.as_bytes());
|
let _ = atomic_write(&listdata_path, meta_content.as_bytes());
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -375,7 +375,7 @@ pub async fn sync_google_tasks(
|
||||||
};
|
};
|
||||||
root_meta.list_order = new_list_order;
|
root_meta.list_order = new_list_order;
|
||||||
if let Ok(meta_content) = serde_json::to_string_pretty(&root_meta) {
|
if let Ok(meta_content) = serde_json::to_string_pretty(&root_meta) {
|
||||||
let _ = atomic_write_bytes(&root_meta_path, meta_content.as_bytes());
|
let _ = atomic_write(&root_meta_path, meta_content.as_bytes());
|
||||||
}
|
}
|
||||||
|
|
||||||
Ok(GoogleSyncResult { downloaded, errors })
|
Ok(GoogleSyncResult { downloaded, errors })
|
||||||
|
|
@ -464,13 +464,3 @@ fn sanitize_name(name: &str) -> String {
|
||||||
if s.is_empty() { "Untitled".to_string() } else { s }
|
if s.is_empty() { "Untitled".to_string() } else { s }
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Write bytes to a file atomically (write to `.tmp`, then rename).
|
|
||||||
fn atomic_write_bytes(path: &Path, content: &[u8]) -> std::io::Result<()> {
|
|
||||||
let temp = path.with_extension("tmp");
|
|
||||||
std::fs::write(&temp, content)?;
|
|
||||||
if let Err(e) = std::fs::rename(&temp, path) {
|
|
||||||
let _ = std::fs::remove_file(&temp);
|
|
||||||
return Err(e);
|
|
||||||
}
|
|
||||||
Ok(())
|
|
||||||
}
|
|
||||||
|
|
|
||||||
|
|
@ -27,7 +27,7 @@ const DEFAULT_TASK_VERSION: u64 = 1;
|
||||||
/// Write data to a temporary file then atomically rename to the target path.
|
/// Write data to a temporary file then atomically rename to the target path.
|
||||||
/// Prevents corruption from partial writes on crash. Cleans up temp file on
|
/// Prevents corruption from partial writes on crash. Cleans up temp file on
|
||||||
/// rename failure to prevent accumulation.
|
/// rename failure to prevent accumulation.
|
||||||
fn atomic_write(path: &Path, content: &[u8]) -> std::io::Result<()> {
|
pub(crate) fn atomic_write(path: &Path, content: &[u8]) -> std::io::Result<()> {
|
||||||
let temp = path.with_extension("tmp");
|
let temp = path.with_extension("tmp");
|
||||||
fs::write(&temp, content)?;
|
fs::write(&temp, content)?;
|
||||||
if let Err(e) = fs::rename(&temp, path) {
|
if let Err(e) = fs::rename(&temp, path) {
|
||||||
|
|
@ -759,7 +759,7 @@ mod tests {
|
||||||
let temp_dir = TempDir::new().unwrap();
|
let temp_dir = TempDir::new().unwrap();
|
||||||
let storage = init_storage(&temp_dir);
|
let storage = init_storage(&temp_dir);
|
||||||
|
|
||||||
let content = "---\nid: 550e8400-e29b-41d4-a716-446655440000\nstatus: backlog\ndue: 2026-06-15T12:00:00Z\nversion: 2\nparent: 660e8400-e29b-41d4-a716-446655440001\n---\n\nNotes";
|
let content = "---\nid: 550e8400-e29b-41d4-a716-446655440000\nstatus: backlog\ndate: 2026-06-15T12:00:00Z\nversion: 2\nparent: 660e8400-e29b-41d4-a716-446655440001\n---\n\nNotes";
|
||||||
let (fm, _) = storage.parse_markdown_with_frontmatter(content).unwrap();
|
let (fm, _) = storage.parse_markdown_with_frontmatter(content).unwrap();
|
||||||
assert!(fm.date.is_some());
|
assert!(fm.date.is_some());
|
||||||
assert!(fm.parent.is_some());
|
assert!(fm.parent.is_some());
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue