From fdc7d8fd844ae3116a55677f0b1008d3c09aa4a5 Mon Sep 17 00:00:00 2001 From: lucaronin Date: Thu, 30 Apr 2026 12:09:35 +0200 Subject: [PATCH] fix: guard note suggestions during reload --- src-tauri/src/ai_agents.rs | 94 +++++++++++++++++++ src-tauri/src/app_updater.rs | 53 ++++++++++- .../src/commands/vault/lifecycle_cmds.rs | 64 +++++++++++++ src-tauri/src/commands/vault/view_cmds.rs | 47 ++++++++++ src-tauri/src/window_state.rs | 59 +++++++++++- src/components/SingleEditorView.test.tsx | 19 ++++ src/components/SingleEditorView.tsx | 43 +++++++-- .../missing-string-metadata-open-note.spec.ts | 24 ++++- 8 files changed, 389 insertions(+), 14 deletions(-) diff --git a/src-tauri/src/ai_agents.rs b/src-tauri/src/ai_agents.rs index abfa198d..236debc1 100644 --- a/src-tauri/src/ai_agents.rs +++ b/src-tauri/src/ai_agents.rs @@ -191,6 +191,30 @@ fn map_claude_event(event: crate::claude_cli::ClaudeStreamEvent) -> Option, + ) -> AiAgentStreamRequest { + AiAgentStreamRequest { + agent: AiAgentId::Codex, + message: "Summarize this vault".into(), + system_prompt: None, + vault_path: "/tmp/vault".into(), + permission_mode, + } + } + + #[test] + fn stream_request_uses_default_or_explicit_permission_mode() { + assert_eq!( + request_with_permission(None).permission_mode(), + AiAgentPermissionMode::Safe + ); + assert_eq!( + request_with_permission(Some(AiAgentPermissionMode::PowerUser)).permission_mode(), + AiAgentPermissionMode::PowerUser + ); + } + #[test] fn normalize_status_contains_all_agents() { let status = get_ai_agents_status(); @@ -214,6 +238,66 @@ mod tests { assert!(matches!(mapped, Some(AiAgentStreamEvent::Done))); } + #[test] + fn map_claude_text_events_preserve_stream_data() { + assert!(matches!( + map_claude_event(crate::claude_cli::ClaudeStreamEvent::Init { + session_id: "session-1".into(), + }), + Some(AiAgentStreamEvent::Init { session_id }) if session_id == "session-1" + )); + assert!(matches!( + map_claude_event(crate::claude_cli::ClaudeStreamEvent::TextDelta { + text: "visible output".into(), + }), + Some(AiAgentStreamEvent::TextDelta { text }) if text == "visible output" + )); + assert!(matches!( + map_claude_event(crate::claude_cli::ClaudeStreamEvent::ThinkingDelta { + text: "thinking".into(), + }), + Some(AiAgentStreamEvent::ThinkingDelta { text }) if text == "thinking" + )); + } + + #[test] + fn map_claude_tool_events_preserve_stream_data() { + let started = map_claude_event(crate::claude_cli::ClaudeStreamEvent::ToolStart { + tool_name: "Read".into(), + tool_id: "tool-1".into(), + input: Some("{\"file\":\"note.md\"}".into()), + }); + let finished = map_claude_event(crate::claude_cli::ClaudeStreamEvent::ToolDone { + tool_id: "tool-1".into(), + output: Some("done".into()), + }); + + assert!(matches!( + started, + Some(AiAgentStreamEvent::ToolStart { tool_name, tool_id, input }) + if tool_name == "Read" + && tool_id == "tool-1" + && input.as_deref() == Some("{\"file\":\"note.md\"}") + )); + assert!(matches!( + finished, + Some(AiAgentStreamEvent::ToolDone { tool_id, output }) + if tool_id == "tool-1" && output.as_deref() == Some("done") + )); + } + + #[test] + fn map_claude_error_event_preserves_message() { + let mapped = map_claude_event(crate::claude_cli::ClaudeStreamEvent::Error { + message: "missing auth".into(), + }); + + assert!(matches!( + mapped, + Some(AiAgentStreamEvent::Error { message }) if message == "missing auth" + )); + } + #[test] fn map_claude_result_event_preserves_final_text() { let mapped = map_claude_event(crate::claude_cli::ClaudeStreamEvent::Result { @@ -226,4 +310,14 @@ mod tests { Some(AiAgentStreamEvent::TextDelta { text }) if text == "Final answer from Claude" )); } + + #[test] + fn map_claude_empty_result_event_is_ignored() { + let mapped = map_claude_event(crate::claude_cli::ClaudeStreamEvent::Result { + text: String::new(), + session_id: "session-1".into(), + }); + + assert!(mapped.is_none()); + } } diff --git a/src-tauri/src/app_updater.rs b/src-tauri/src/app_updater.rs index 196e497e..99ab94dd 100644 --- a/src-tauri/src/app_updater.rs +++ b/src-tauri/src/app_updater.rs @@ -131,7 +131,8 @@ pub async fn download_and_install_app_update( #[cfg(test)] mod tests { - use super::ReleaseChannel; + use super::{AppUpdateDownloadEvent, AppUpdateMetadata, ReleaseChannel}; + use serde_json::json; #[test] fn release_channel_defaults_to_stable() { @@ -176,4 +177,54 @@ mod tests { "https://refactoringhq.github.io/tolaria/stable/latest.json" ); } + + #[test] + fn update_metadata_serializes_for_frontend_consumers() { + let metadata = AppUpdateMetadata { + current_version: "2026.4.1".into(), + version: "2026.4.2".into(), + date: Some("2026-04-30T12:00:00Z".into()), + body: Some("Bug fixes".into()), + }; + + assert_eq!( + serde_json::to_value(metadata).unwrap(), + json!({ + "currentVersion": "2026.4.1", + "version": "2026.4.2", + "date": "2026-04-30T12:00:00Z", + "body": "Bug fixes" + }) + ); + } + + #[test] + fn download_events_serialize_as_tagged_frontend_events() { + let events = [ + ( + AppUpdateDownloadEvent::Started { + content_length: Some(4096), + }, + json!({ + "event": "Started", + "data": { "contentLength": 4096 } + }), + ), + ( + AppUpdateDownloadEvent::Progress { chunk_length: 512 }, + json!({ + "event": "Progress", + "data": { "chunkLength": 512 } + }), + ), + ( + AppUpdateDownloadEvent::Finished, + json!({ "event": "Finished" }), + ), + ]; + + for (event, expected) in events { + assert_eq!(serde_json::to_value(event).unwrap(), expected); + } + } } diff --git a/src-tauri/src/commands/vault/lifecycle_cmds.rs b/src-tauri/src/commands/vault/lifecycle_cmds.rs index e1105456..ddedad7f 100644 --- a/src-tauri/src/commands/vault/lifecycle_cmds.rs +++ b/src-tauri/src/commands/vault/lifecycle_cmds.rs @@ -90,3 +90,67 @@ pub fn repair_vault(vault_path: String) -> Result { git::ensure_gitignore(&vault_path)?; Ok("Vault repaired".to_string()) } + +#[cfg(test)] +mod tests { + use super::*; + use std::fs; + + #[test] + fn empty_vault_target_validation_allows_missing_or_empty_directories() { + let dir = tempfile::TempDir::new().unwrap(); + let missing = dir.path().join("new-vault"); + let empty = dir.path().join("empty-vault"); + fs::create_dir(&empty).unwrap(); + + assert_eq!(ensure_directory_is_missing_or_empty(&missing), Ok(())); + assert_eq!(ensure_directory_is_missing_or_empty(&empty), Ok(())); + } + + #[test] + fn empty_vault_target_validation_rejects_files_and_nonempty_directories() { + let dir = tempfile::TempDir::new().unwrap(); + let file = dir.path().join("vault.md"); + let nonempty = dir.path().join("vault"); + fs::write(&file, "# Not a folder").unwrap(); + fs::create_dir(&nonempty).unwrap(); + fs::write(nonempty.join("note.md"), "# Existing note").unwrap(); + + assert_eq!( + ensure_directory_is_missing_or_empty(&file), + Err("Choose a folder path for the new vault".to_string()) + ); + assert_eq!( + ensure_directory_is_missing_or_empty(&nonempty), + Err("Choose an empty folder to create a new vault".to_string()) + ); + } + + #[test] + fn canonical_vault_path_uses_existing_canonical_path_or_original_path() { + let dir = tempfile::TempDir::new().unwrap(); + let existing = dir.path().join("existing"); + let missing = dir.path().join("missing"); + fs::create_dir(&existing).unwrap(); + + assert_eq!( + canonical_vault_path_string(&existing), + existing.canonicalize().unwrap().to_string_lossy() + ); + assert_eq!( + canonical_vault_path_string(&missing), + missing.to_string_lossy() + ); + } + + #[test] + fn getting_started_target_uses_explicit_path_when_provided() { + let dir = tempfile::TempDir::new().unwrap(); + let explicit = dir.path().join("starter"); + + assert_eq!( + resolve_getting_started_target(explicit.to_str()), + Ok(explicit.to_string_lossy().to_string()) + ); + } +} diff --git a/src-tauri/src/commands/vault/view_cmds.rs b/src-tauri/src/commands/vault/view_cmds.rs index 192a65ba..1bbdaf2b 100644 --- a/src-tauri/src/commands/vault/view_cmds.rs +++ b/src-tauri/src/commands/vault/view_cmds.rs @@ -35,3 +35,50 @@ pub fn delete_view_cmd(vault_path: String, filename: String) -> Result<(), Strin }, ) } + +#[cfg(test)] +mod tests { + use super::*; + use crate::vault::{FilterCondition, FilterGroup, FilterNode, FilterOp}; + + fn definition(name: &str) -> ViewDefinition { + ViewDefinition { + name: name.to_string(), + icon: Some("star".to_string()), + color: None, + order: None, + sort: Some("modified:desc".to_string()), + list_properties_display: vec!["Priority".to_string()], + filters: FilterGroup::All(vec![FilterNode::Condition(FilterCondition { + field: "type".to_string(), + op: FilterOp::Equals, + value: Some(serde_yaml::Value::String("Project".to_string())), + regex: false, + })]), + } + } + + #[test] + fn view_commands_roundtrip_through_validated_vault_paths() { + let dir = tempfile::TempDir::new().unwrap(); + let vault_path = dir.path().to_string_lossy().to_string(); + + assert!(list_views(vault_path.clone()).unwrap().is_empty()); + + save_view_cmd( + vault_path.clone(), + "active-projects.yml".to_string(), + definition("Active Projects"), + ) + .unwrap(); + + let views = list_views(vault_path.clone()).unwrap(); + assert_eq!(views.len(), 1); + assert_eq!(views[0].filename, "active-projects.yml"); + assert_eq!(views[0].definition.name, "Active Projects"); + + delete_view_cmd(vault_path.clone(), "active-projects.yml".to_string()).unwrap(); + + assert!(list_views(vault_path).unwrap().is_empty()); + } +} diff --git a/src-tauri/src/window_state.rs b/src-tauri/src/window_state.rs index 99a245bc..3532b2f7 100644 --- a/src-tauri/src/window_state.rs +++ b/src-tauri/src/window_state.rs @@ -1,6 +1,6 @@ use serde::{Deserialize, Serialize}; use std::fs; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::sync::Mutex; use tauri::{ App, AppHandle, LogicalPosition, LogicalSize, Manager, Position, RunEvent, Size, WebviewWindow, @@ -263,7 +263,11 @@ fn window_state_path() -> Result { } fn read_main_window_frame(scale_factor: f64) -> Option { - let content = fs::read_to_string(window_state_path().ok()?).ok()?; + read_main_window_frame_at(&window_state_path().ok()?, scale_factor) +} + +fn read_main_window_frame_at(path: &Path, scale_factor: f64) -> Option { + let content = fs::read_to_string(path).ok()?; let persisted: PersistedWindowState = serde_json::from_str(&content).ok()?; persisted .main @@ -276,7 +280,10 @@ fn read_main_window_frame(scale_factor: f64) -> Option { } fn write_main_window_frame(frame: WindowFrame) -> Result<(), String> { - let path = window_state_path()?; + write_main_window_frame_at(&window_state_path()?, frame) +} + +fn write_main_window_frame_at(path: &Path, frame: WindowFrame) -> Result<(), String> { if let Some(parent) = path.parent() { fs::create_dir_all(parent) .map_err(|e| format!("Failed to create window state directory: {e}"))?; @@ -511,4 +518,50 @@ mod tests { &RunEvent::Resumed )); } + + #[test] + fn persists_and_reads_logical_frame_from_disk() { + let dir = tempfile::TempDir::new().unwrap(); + let path = dir.path().join("nested/window-state.json"); + let saved = frame(80, 120, 1100, 700); + + write_main_window_frame_at(&path, saved).unwrap(); + + let json = std::fs::read_to_string(&path).unwrap(); + assert!(json.contains("\"coordinate_space\": \"logical\"")); + assert_eq!(read_main_window_frame_at(&path, 2.0), Some(saved)); + } + + #[test] + fn reads_legacy_physical_frame_from_disk_as_logical_points() { + let dir = tempfile::TempDir::new().unwrap(); + let path = dir.path().join("window-state.json"); + std::fs::write( + &path, + r#"{ + "main": { "x": 160, "y": 240, "width": 2200, "height": 1400 }, + "coordinate_space": "physical" + }"#, + ) + .unwrap(); + + assert_eq!( + read_main_window_frame_at(&path, 2.0), + Some(frame(80, 120, 1100, 700)) + ); + } + + #[test] + fn ignores_missing_corrupt_or_tiny_window_state_files() { + let dir = tempfile::TempDir::new().unwrap(); + let path = dir.path().join("window-state.json"); + + assert_eq!(read_main_window_frame_at(&path, 1.0), None); + + std::fs::write(&path, "not json").unwrap(); + assert_eq!(read_main_window_frame_at(&path, 1.0), None); + + std::fs::write(&path, r#"{"main":{"x":0,"y":0,"width":100,"height":100}}"#).unwrap(); + assert_eq!(read_main_window_frame_at(&path, 1.0), None); + } } diff --git a/src/components/SingleEditorView.test.tsx b/src/components/SingleEditorView.test.tsx index c659e3aa..268e9013 100644 --- a/src/components/SingleEditorView.test.tsx +++ b/src/components/SingleEditorView.test.tsx @@ -440,6 +440,25 @@ describe('SingleEditorView', () => { expect(onMentionItemClick).toHaveBeenCalledOnce() }) + it('renders when a reload returns an entry with missing suggestion metadata', () => { + const reloadedEntry = { + ...makeEntry({ path: '/vault/project/reloaded.md', title: 'Reloaded' }), + filename: undefined, + aliases: undefined, + isA: undefined, + } as unknown as VaultEntry + + expect(() => { + render( + , + ) + }).not.toThrow() + }) + it('ignores stale suggestion item clicks after the editor DOM disconnects', () => { const editor = createEditor() editor.domElement = document.createElement('div') diff --git a/src/components/SingleEditorView.tsx b/src/components/SingleEditorView.tsx index 14b04b36..95fb0d54 100644 --- a/src/components/SingleEditorView.tsx +++ b/src/components/SingleEditorView.tsx @@ -451,15 +451,42 @@ function handleCodeBlockCopy(event: React.ClipboardEvent) { event.preventDefault() } +function nonEmptyString(value: unknown): string | null { + return typeof value === 'string' && value.trim().length > 0 ? value : null +} + +function markdownStem(value: string): string { + return value.replace(/\.md$/i, '') +} + +function pathStem(path: string): string { + return markdownStem(path.split('/').pop() ?? path) +} + +function safeStringArray(value: unknown): string[] { + return Array.isArray(value) + ? value.filter((item): item is string => nonEmptyString(item) !== null) + : [] +} + function buildBaseSuggestionItems(entries: VaultEntry[]) { - return deduplicateByPath(entries.map(entry => ({ - title: entry.title, - aliases: [...new Set([entry.filename.replace(/\.md$/, ''), ...entry.aliases])], - group: entry.isA || 'Note', - entryType: entry.isA, - entryTitle: entry.title, - path: entry.path, - }))) + return deduplicateByPath(entries.flatMap(entry => { + const path = nonEmptyString(entry.path) + if (!path) return [] + + const filename = nonEmptyString(entry.filename) + const filenameStem = filename ? markdownStem(filename) : pathStem(path) + const title = nonEmptyString(entry.title) ?? filenameStem + const entryType = nonEmptyString(entry.isA) + return [{ + title, + aliases: [...new Set([filenameStem, ...safeStringArray(entry.aliases)])], + group: entryType ?? 'Note', + entryType, + entryTitle: title, + path, + }] + })) } function useInsertWikilink( diff --git a/tests/smoke/missing-string-metadata-open-note.spec.ts b/tests/smoke/missing-string-metadata-open-note.spec.ts index e03f1da6..8c7ef6ac 100644 --- a/tests/smoke/missing-string-metadata-open-note.spec.ts +++ b/tests/smoke/missing-string-metadata-open-note.spec.ts @@ -1,9 +1,10 @@ import { test, expect, type Page } from '@playwright/test' import { createFixtureVaultCopy, - openFixtureVault, + openFixtureVaultDesktopHarness, removeFixtureVaultCopy, } from '../helpers/fixtureVault' +import { executeCommand, openCommandPalette } from './helpers' let tempVaultDir: string @@ -46,6 +47,12 @@ function removeAlphaProjectStringMetadata(entries: Array }) } +async function reloadVaultFromCommandPalette(page: Page): Promise { + await openCommandPalette(page) + await executeCommand(page, 'Reload Vault') + await expect(page.locator('input[placeholder="Type a command..."]')).not.toBeVisible() +} + test.beforeEach(async ({ page }, testInfo) => { testInfo.setTimeout(60_000) tempVaultDir = createFixtureVaultCopy() @@ -62,7 +69,7 @@ test.beforeEach(async ({ page }, testInfo) => { json: removeAlphaProjectStringMetadata(entries), }) }) - await openFixtureVault(page, tempVaultDir, { + await openFixtureVaultDesktopHarness(page, tempVaultDir, { expectedReadyTitle: 'alpha-project', }) await page.setViewportSize({ width: 1180, height: 760 }) @@ -85,3 +92,16 @@ test('@smoke note open tolerates missing string metadata from the vault scan', a expect(errors).toHaveLength(0) }) + +test('note open after vault reload tolerates missing suggestion metadata', async ({ page }) => { + const errors = collectMissingMetadataCrashes(page) + const noteList = page.getByTestId('note-list-container') + + await reloadVaultFromCommandPalette(page) + + await noteList.getByText('Note B', { exact: true }).click() + await noteList.getByText('alpha-project', { exact: true }).click() + await expect(page.getByRole('heading', { name: 'Alpha Project', level: 1 })).toBeVisible({ timeout: 5_000 }) + + expect(errors).toHaveLength(0) +})