From 22e2a0ca5d7e69932d2b1b822262a3321b509ce7 Mon Sep 17 00:00:00 2001 From: Owais Jamil Date: Mon, 20 Jul 2026 00:48:05 -0500 Subject: [PATCH] fix: file lock & dialogs --- Cargo.lock | 70 ++++++ TODO.md | 7 +- apps/desktop/src-tauri/Cargo.toml | 2 + .../src-tauri/capabilities/default.json | 2 +- apps/desktop/src-tauri/src/files.rs | 51 ++++- apps/desktop/src-tauri/src/lib.rs | 8 +- apps/desktop/src-tauri/src/menu.rs | 23 +- apps/desktop/src-tauri/src/session.rs | 45 +++- apps/desktop/src/lib/fileops.ts | 212 +++++++++--------- .../desktop-session.invoke.test.ts | 131 +++++++++++ .../src/lib/persistence/desktop-session.ts | 91 +++++--- apps/desktop/src/lib/platform.ts | 8 +- crates/inkfinite-core/src/file/persistence.rs | 46 ++-- crates/inkfinite-core/src/file/tests.rs | 24 ++ 14 files changed, 542 insertions(+), 178 deletions(-) create mode 100644 apps/desktop/src/lib/persistence/desktop-session.invoke.test.ts diff --git a/Cargo.lock b/Cargo.lock index d49ee7e..24676bb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -32,6 +32,23 @@ dependencies = [ "alloc-no-stdlib", ] +[[package]] +name = "android_log-sys" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "84521a3cf562bc62942e294181d9eef17eb38ceb8c68677bc49f144e4c3d4f8d" + +[[package]] +name = "android_logger" +version = "0.15.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dbb4e440d04be07da1f1bf44fb4495ebd58669372fe0cffa6e48595ac5bd88a3" +dependencies = [ + "android_log-sys", + "env_filter", + "log", +] + [[package]] name = "android_system_properties" version = "0.1.5" @@ -876,11 +893,13 @@ name = "desktop" version = "0.1.0" dependencies = [ "inkfinite-core", + "log", "serde", "serde_json", "tauri", "tauri-build", "tauri-plugin-dialog", + "tauri-plugin-log", "tauri-plugin-opener", "tauri-plugin-store", "tokio", @@ -1093,6 +1112,16 @@ dependencies = [ "syn 2.0.119", ] +[[package]] +name = "env_filter" +version = "0.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1bf3c259d255ca70051b30e2e95b5446cdb8949ac4cd22c0d7fd634d89f568e2" +dependencies = [ + "log", + "regex", +] + [[package]] name = "equivalent" version = "1.0.2" @@ -1156,6 +1185,15 @@ dependencies = [ "simd-adler32", ] +[[package]] +name = "fern" +version = "0.7.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4316185f709b23713e41e3195f90edef7fb00c3ed4adc79769cf09cc762a3b29" +dependencies = [ + "log", +] + [[package]] name = "field-offset" version = "0.3.6" @@ -2347,6 +2385,15 @@ dependencies = [ "syn 2.0.119", ] +[[package]] +name = "num_threads" +version = "0.1.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5c7398b9c8b70908f6371f47ed36737907c87c52af34c268fed0bf0ceb92ead9" +dependencies = [ + "libc", +] + [[package]] name = "objc2" version = "0.6.4" @@ -3883,6 +3930,27 @@ dependencies = [ "url", ] +[[package]] +name = "tauri-plugin-log" +version = "2.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6792296e6f389268016c77db21ebae1fc0568f2fccf88b1ec7e2ea71330afb4c" +dependencies = [ + "android_logger", + "fern", + "log", + "objc2", + "objc2-foundation", + "serde", + "serde_json", + "serde_repr", + "swift-rs", + "tauri", + "tauri-plugin", + "thiserror 2.0.18", + "time", +] + [[package]] name = "tauri-plugin-opener" version = "2.5.4" @@ -4099,7 +4167,9 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "18dfaaeddcb932337b5e7866ee7d0ce9b76d2fd092997146f187ec09b4558a50" dependencies = [ "deranged", + "libc", "num-conv", + "num_threads", "powerfmt", "serde_core", "time-core", diff --git a/TODO.md b/TODO.md index d518cfd..d962621 100644 --- a/TODO.md +++ b/TODO.md @@ -258,7 +258,8 @@ Acceptance criteria: ### QA -- We don't expose agent editable in the UI -- Save As doesn't work +- [x] Expose agent-editable state in the UI. +- [x] Make Save As open the native dialog and persist to the selected path. - We don't expose dirty when creating a new board -- Saving doesn't work +- [x] Make New Board create and persist the selected `.inkfinite` file. +- [x] Recover app-managed drafts after a crash leaves the lock sidecar behind. diff --git a/apps/desktop/src-tauri/Cargo.toml b/apps/desktop/src-tauri/Cargo.toml index ab2a538..be79b1b 100644 --- a/apps/desktop/src-tauri/Cargo.toml +++ b/apps/desktop/src-tauri/Cargo.toml @@ -21,8 +21,10 @@ tauri-build = { version = "2", features = [] } tauri = { version = "2", features = [] } tauri-plugin-opener = "2" tauri-plugin-dialog = "2" +tauri-plugin-log = "2" tauri-plugin-store = "2" inkfinite-core.workspace = true +log = "0.4" serde = { version = "1", features = ["derive"] } serde_json = "1" tokio.workspace = true diff --git a/apps/desktop/src-tauri/capabilities/default.json b/apps/desktop/src-tauri/capabilities/default.json index b31fc83..ae3a0ed 100644 --- a/apps/desktop/src-tauri/capabilities/default.json +++ b/apps/desktop/src-tauri/capabilities/default.json @@ -3,5 +3,5 @@ "identifier": "default", "description": "Capability for the main window", "windows": ["main"], - "permissions": ["core:default", "opener:default", "dialog:default", "store:default"] + "permissions": ["core:default", "opener:default", "dialog:default", "log:default", "store:default"] } diff --git a/apps/desktop/src-tauri/src/files.rs b/apps/desktop/src-tauri/src/files.rs index 8b4547f..8dde5e2 100644 --- a/apps/desktop/src-tauri/src/files.rs +++ b/apps/desktop/src-tauri/src/files.rs @@ -1,6 +1,7 @@ use std::fs; use std::path::Path; use tauri::AppHandle; +use tauri_plugin_dialog::DialogExt; #[derive(serde::Serialize, serde::Deserialize)] pub struct FileEntry { @@ -89,11 +90,57 @@ pub fn delete_file(file_path: String) -> Result<(), String> { Ok(()) } +/// Opens the native document picker and returns the selected Inkfinite path. +#[tauri::command] +pub async fn pick_open_document(app: AppHandle) -> Result, String> { + log::info!("opening native document picker"); + let selected = app + .dialog() + .file() + .add_filter("Inkfinite Files", &["inkfinite"]) + .blocking_pick_file(); + let path = selected + .map(|path| path.into_path().map(|path| path.to_string_lossy().into_owned())) + .transpose() + .map_err(|error| { + log::error!("failed to resolve the selected document path: {error}"); + format!("Failed to resolve selected document: {error}") + })?; + match &path { + Some(path) => log::info!("native document picker selected {path}"), + None => log::info!("native document picker was cancelled"), + } + Ok(path) +} + +/// Opens the native Save dialog and returns the selected Inkfinite path. +#[tauri::command] +pub async fn pick_save_document(app: AppHandle, default_name: Option) -> Result, String> { + log::info!("opening native Save dialog with default name {:?}", default_name); + let dialog = app.dialog().file().add_filter("Inkfinite Files", &["inkfinite"]); + let selected = match default_name { + Some(name) => dialog.set_file_name(name), + None => dialog, + } + .blocking_save_file(); + let path = selected + .map(|path| path.into_path().map(|path| path.to_string_lossy().into_owned())) + .transpose() + .map_err(|error| { + log::error!("failed to resolve the selected Save path: {error}"); + format!("Failed to resolve selected Save path: {error}") + })?; + match &path { + Some(path) => log::info!("native Save dialog selected {path}"), + None => log::info!("native Save dialog was cancelled"), + } + Ok(path) +} + /// Pick a workspace directory using the system folder picker #[tauri::command] pub async fn pick_workspace_directory(app: AppHandle) -> Result, String> { - use tauri_plugin_dialog::DialogExt; - + log::info!("opening native workspace directory picker"); let result = app.dialog().file().blocking_pick_folder(); match result { diff --git a/apps/desktop/src-tauri/src/lib.rs b/apps/desktop/src-tauri/src/lib.rs index adbfcfe..7021a18 100644 --- a/apps/desktop/src-tauri/src/lib.rs +++ b/apps/desktop/src-tauri/src/lib.rs @@ -8,6 +8,7 @@ use tauri::Manager; #[cfg_attr(mobile, tauri::mobile_entry_point)] pub fn run() { let builder = tauri::Builder::default() + .plugin(tauri_plugin_log::Builder::new().level(log::LevelFilter::Debug).build()) .menu(menu::build) .on_menu_event(menu::handle_event) .manage(session::DesktopState::default()) @@ -18,6 +19,7 @@ pub fn run() { let service = app.state::().service_handle(); let server = tauri::async_runtime::block_on(ipc::start(app.handle().clone(), service))?; app.manage(server); + log::info!("desktop application setup completed"); Ok(()) }) .invoke_handler(tauri::generate_handler![ @@ -43,9 +45,13 @@ pub fn run() { session::sync_receive, session::close, files::read_directory, + files::pick_open_document, + files::pick_save_document, files::rename_file, files::delete_file, - files::pick_workspace_directory + files::pick_workspace_directory, + menu::record_renderer_event, + menu::record_renderer_error ]); let app = builder .build(tauri::generate_context!()) diff --git a/apps/desktop/src-tauri/src/menu.rs b/apps/desktop/src-tauri/src/menu.rs index 62004b3..eca04c8 100644 --- a/apps/desktop/src-tauri/src/menu.rs +++ b/apps/desktop/src-tauri/src/menu.rs @@ -24,6 +24,9 @@ pub fn build(app: &AppHandle) -> tauri::Result> { let separator = PredefinedMenuItem::separator(app)?; let items: [&dyn IsMenuItem; 4] = [&new_board, &open_board, &save_board_as, &separator]; file_menu.insert_items(&items, 0)?; + log::info!("installed native File menu commands"); + } else { + log::warn!("native File menu was unavailable; document commands were not installed"); } Ok(menu) @@ -31,11 +34,27 @@ pub fn build(app: &AppHandle) -> tauri::Result> { /// Forwards native File menu commands to the active editor webview. pub fn handle_event(app: &AppHandle, event: tauri::menu::MenuEvent) { - let action = match event.id().as_ref() { + let menu_id = event.id().as_ref(); + let action = match menu_id { NEW_BOARD => "new", OPEN_BOARD => "open", SAVE_BOARD_AS => "save-as", _ => return, }; - let _ = app.emit(FILE_MENU_EVENT, action); + log::info!("native File menu selected: id={menu_id}, action={action}"); + if let Err(error) = app.emit(FILE_MENU_EVENT, action) { + log::error!("failed to emit {FILE_MENU_EVENT} for {action}: {error}"); + } +} + +/// Records that the renderer received a native File menu event. +#[tauri::command] +pub fn record_renderer_event(action: String) { + log::info!("renderer received native File menu action: {action}"); +} + +/// Persists a renderer-side command failure alongside the native application logs. +#[tauri::command] +pub fn record_renderer_error(message: String) { + log::error!("renderer command failure: {message}"); } diff --git a/apps/desktop/src-tauri/src/session.rs b/apps/desktop/src-tauri/src/session.rs index b13d5e9..b50cf89 100644 --- a/apps/desktop/src-tauri/src/session.rs +++ b/apps/desktop/src-tauri/src/session.rs @@ -39,14 +39,18 @@ impl DesktopState { } fn lock_service(state: &DesktopState) -> Result> { - state.service.lock().map_err(|_| ProtocolError { - code: "session_service_unavailable".into(), - message: "the desktop session service lock is poisoned".into(), - details: None, + state.service.lock().map_err(|_| { + log::error!("desktop session service lock is poisoned"); + ProtocolError { + code: "session_service_unavailable".into(), + message: "the desktop session service lock is poisoned".into(), + details: None, + } }) } fn to_protocol_error(error: SessionError) -> ProtocolError { + log::error!("desktop session command failed: {error}"); inkfinite_core::ipc::session_protocol_error(&error) } @@ -84,14 +88,21 @@ fn same_path(left: &str, right: &Path) -> bool { pub fn create_document( state: State<'_, DesktopState>, path: String, document_id: String, actor_id: String, page_name: Option, ) -> Result { - lock_service(&state)? + log::info!("creating document at {path}"); + let opened = lock_service(&state)? .create( path, DocumentId::new(document_id), ActorId::new(actor_id), page_name.as_deref(), ) - .map_err(to_protocol_error) + .map_err(to_protocol_error)?; + log::info!( + "created document session {} at {}", + opened.session_id.0, + opened.status.path.0 + ); + Ok(opened) } /// Opens a canonical `.inkfinite` document. @@ -112,6 +123,7 @@ pub fn open_or_create_draft( app: AppHandle, state: State<'_, DesktopState>, document_id: String, actor_id: String, ) -> Result { let path = draft_document_path(&app)?; + log::info!("opening desktop draft at {}", path.display()); let promotion_path = draft_promotion_path(&path); if !path.exists() && promotion_path.exists() { fs::rename(&promotion_path, &path).map_err(|error| ProtocolError { @@ -246,9 +258,12 @@ pub fn redo(state: State<'_, DesktopState>, session_id: String, actor_id: String pub fn save( state: State<'_, DesktopState>, session_id: String, expected_heads: Vec, ) -> Result { - lock_service(&state)? + log::info!("saving document session {session_id}"); + let saved = lock_service(&state)? .save(&SessionId(session_id), &expected_heads) - .map_err(to_protocol_error) + .map_err(to_protocol_error)?; + log::info!("saved document at {}", saved.status.path.0); + Ok(saved) } /// Saves the current session at a replacement path after checking its heads. @@ -256,9 +271,12 @@ pub fn save( pub fn save_as( state: State<'_, DesktopState>, session_id: String, path: DocumentPath, expected_heads: Vec, ) -> Result { - lock_service(&state)? + log::info!("saving document session {session_id} as {}", path.0); + let saved = lock_service(&state)? .save_as(&SessionId(session_id), path.0, &expected_heads) - .map_err(to_protocol_error) + .map_err(to_protocol_error)?; + log::info!("saved document as {}", saved.status.path.0); + Ok(saved) } /// Promotes the app-managed draft to a user-selected document path. @@ -271,6 +289,7 @@ pub fn save_draft_as( expected_heads: Vec, ) -> Result { let draft_path = draft_document_path(&app)?; + log::info!("promoting desktop draft session {session_id} to {}", path.0); let promotion_path = draft_promotion_path(&draft_path); let session_id = SessionId(session_id); let mut service = lock_service(&state)?; @@ -308,6 +327,7 @@ pub fn save_draft_as( // reopened as a draft or shown in the document browser. } } + log::info!("promoted desktop draft to {}", saved.status.path.0); Ok(saved) } @@ -372,9 +392,12 @@ pub fn sync_receive( /// Closes a session and releases its advisory file lock. #[tauri::command] pub fn close(state: State<'_, DesktopState>, session_id: String) -> Result<()> { + log::info!("closing document session {session_id}"); lock_service(&state)? .close(&SessionId(session_id)) - .map_err(to_protocol_error) + .map_err(to_protocol_error)?; + log::info!("closed document session"); + Ok(()) } #[cfg(test)] diff --git a/apps/desktop/src/lib/fileops.ts b/apps/desktop/src/lib/fileops.ts index e71f532..01b8d59 100644 --- a/apps/desktop/src/lib/fileops.ts +++ b/apps/desktop/src/lib/fileops.ts @@ -1,13 +1,12 @@ -import { invoke } from "@tauri-apps/api/core"; -import { open, save } from "@tauri-apps/plugin-dialog"; -import { load } from "@tauri-apps/plugin-store"; -import type { DesktopFileOps, DirectoryEntry, FileHandle } from "@inkfinite/core"; +import { invoke } from '@tauri-apps/api/core'; +import { load } from '@tauri-apps/plugin-store'; +import type { DesktopFileOps, DirectoryEntry, FileHandle } from '@inkfinite/core'; export type { DesktopFileOps }; -const STORE_NAME = "inkfinite-desktop.json"; -const RECENT_FILES_KEY = "recentFiles"; -const WORKSPACE_DIR_KEY = "workspaceDir"; +const STORE_NAME = 'inkfinite-desktop.json'; +const RECENT_FILES_KEY = 'recentFiles'; +const WORKSPACE_DIR_KEY = 'workspaceDir'; const MAX_RECENT_FILES = 10; type FileEntry = { path: string; name: string; is_dir: boolean }; @@ -16,109 +15,98 @@ type FileEntry = { path: string; name: string; is_dir: boolean }; * Create desktop file operations using Tauri APIs */ export function createDesktopFileOps(): DesktopFileOps { - let storePromise: Promise>> | null = null; - - async function getStore() { - if (!storePromise) { - storePromise = load(STORE_NAME); - } - return storePromise; - } - - async function showOpenDialog(): Promise { - const result = await open({ - multiple: false, - directory: false, - filters: [{ name: "Inkfinite Files", extensions: ["inkfinite"] }], - }); - - return result; - } - - async function showSaveDialog(defaultName?: string): Promise { - const result = await save({ - defaultPath: defaultName || "Untitled.inkfinite", - filters: [{ name: "Inkfinite Files", extensions: ["inkfinite"] }], - }); - - return result; - } - - async function getRecentFiles(): Promise { - const store = await getStore(); - const recent = (await store.get(RECENT_FILES_KEY)) || []; - return recent; - } - - async function addRecentFile(handle: FileHandle): Promise { - const store = await getStore(); - const recent = (await store.get(RECENT_FILES_KEY)) || []; - const filtered = recent.filter((f) => f.path !== handle.path); - const updated = [handle, ...filtered].slice(0, MAX_RECENT_FILES); - await store.set(RECENT_FILES_KEY, updated); - await store.save(); - } - - async function removeRecentFile(path: string): Promise { - const store = await getStore(); - const recent = (await store.get(RECENT_FILES_KEY)) || []; - const filtered = recent.filter((f) => f.path !== path); - await store.set(RECENT_FILES_KEY, filtered); - await store.save(); - } - - async function clearRecentFiles(): Promise { - const store = await getStore(); - await store.set(RECENT_FILES_KEY, []); - await store.save(); - } - - async function getWorkspaceDir(): Promise { - const store = await getStore(); - const workspace = (await store.get(WORKSPACE_DIR_KEY)) || null; - return workspace; - } - - async function setWorkspaceDir(path: string | null): Promise { - const store = await getStore(); - await store.set(WORKSPACE_DIR_KEY, path); - await store.save(); - } - - async function pickWorkspaceDir(): Promise { - const result = await invoke("pick_workspace_directory"); - if (result) { - await setWorkspaceDir(result); - } - return result; - } - - async function readDirectory(directory: string, pattern?: string): Promise { - const entries = await invoke("read_directory", { directory, pattern: pattern || "*.inkfinite" }); - - return entries.map((e) => ({ path: e.path, name: e.name, isDir: e.is_dir })); - } - - async function renameFile(oldPath: string, newPath: string): Promise { - await invoke("rename_file", { oldPath, newPath }); - } - - async function deleteFile(path: string): Promise { - await invoke("delete_file", { filePath: path }); - } - - return { - showOpenDialog, - showSaveDialog, - getRecentFiles, - addRecentFile, - removeRecentFile, - clearRecentFiles, - getWorkspaceDir, - setWorkspaceDir, - pickWorkspaceDir, - readDirectory, - renameFile, - deleteFile, - }; + let storePromise: Promise>> | null = null; + + async function getStore() { + if (!storePromise) { + storePromise = load(STORE_NAME); + } + return storePromise; + } + + async function showOpenDialog(): Promise { + return invoke('pick_open_document'); + } + + async function showSaveDialog(defaultName?: string): Promise { + return invoke('pick_save_document', { defaultName: defaultName || 'Untitled.inkfinite' }); + } + + async function getRecentFiles(): Promise { + const store = await getStore(); + const recent = (await store.get(RECENT_FILES_KEY)) || []; + return recent; + } + + async function addRecentFile(handle: FileHandle): Promise { + const store = await getStore(); + const recent = (await store.get(RECENT_FILES_KEY)) || []; + const filtered = recent.filter((f) => f.path !== handle.path); + const updated = [handle, ...filtered].slice(0, MAX_RECENT_FILES); + await store.set(RECENT_FILES_KEY, updated); + await store.save(); + } + + async function removeRecentFile(path: string): Promise { + const store = await getStore(); + const recent = (await store.get(RECENT_FILES_KEY)) || []; + const filtered = recent.filter((f) => f.path !== path); + await store.set(RECENT_FILES_KEY, filtered); + await store.save(); + } + + async function clearRecentFiles(): Promise { + const store = await getStore(); + await store.set(RECENT_FILES_KEY, []); + await store.save(); + } + + async function getWorkspaceDir(): Promise { + const store = await getStore(); + const workspace = (await store.get(WORKSPACE_DIR_KEY)) || null; + return workspace; + } + + async function setWorkspaceDir(path: string | null): Promise { + const store = await getStore(); + await store.set(WORKSPACE_DIR_KEY, path); + await store.save(); + } + + async function pickWorkspaceDir(): Promise { + const result = await invoke('pick_workspace_directory'); + if (result) { + await setWorkspaceDir(result); + } + return result; + } + + async function readDirectory(directory: string, pattern?: string): Promise { + const entries = await invoke('read_directory', { directory, pattern: pattern || '*.inkfinite' }); + + return entries.map((e) => ({ path: e.path, name: e.name, isDir: e.is_dir })); + } + + async function renameFile(oldPath: string, newPath: string): Promise { + await invoke('rename_file', { oldPath, newPath }); + } + + async function deleteFile(path: string): Promise { + await invoke('delete_file', { filePath: path }); + } + + return { + showOpenDialog, + showSaveDialog, + getRecentFiles, + addRecentFile, + removeRecentFile, + clearRecentFiles, + getWorkspaceDir, + setWorkspaceDir, + pickWorkspaceDir, + readDirectory, + renameFile, + deleteFile + }; } diff --git a/apps/desktop/src/lib/persistence/desktop-session.invoke.test.ts b/apps/desktop/src/lib/persistence/desktop-session.invoke.test.ts new file mode 100644 index 0000000..7ea6bd0 --- /dev/null +++ b/apps/desktop/src/lib/persistence/desktop-session.invoke.test.ts @@ -0,0 +1,131 @@ +import type { DesktopFileOps } from '@inkfinite/core'; +import type { DocumentSnapshot } from '@inkfinite/bindings'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const tauri = vi.hoisted(() => ({ invoke: vi.fn(), listen: vi.fn(async () => () => undefined) })); + +vi.mock('@tauri-apps/api/core', () => ({ invoke: tauri.invoke })); +vi.mock('@tauri-apps/api/event', () => ({ listen: tauri.listen })); + +import { createDesktopSessionRepo, type SessionOpened, type SessionSaved } from './desktop-session'; + +function snapshot(documentId: string): DocumentSnapshot { + const pageId = `page:${documentId}:1`; + const layerId = `layer:${documentId}:1`; + return { + format: 'inkfinite', + format_version: 2, + document_id: documentId, + heads: ['head:1'], + document: { + pages: { [pageId]: { id: pageId, name: 'Page 1', layer_ids: [layerId], version: 1 } }, + page_ids: [pageId], + layers: { + [layerId]: { + id: layerId, + page_id: pageId, + name: 'Default', + shape_ids: [], + visible: true, + locked: false, + opacity: 1, + version: 1 + } + }, + shapes: {}, + bindings: {}, + assets: {} + } + }; +} + +function fileOps() { + let savePath = '/tmp/Untitled.inkfinite'; + const ops: DesktopFileOps = { + showOpenDialog: async () => null, + showSaveDialog: async () => savePath, + getRecentFiles: async () => [], + addRecentFile: async () => undefined, + removeRecentFile: async () => undefined, + clearRecentFiles: async () => undefined, + getWorkspaceDir: async () => null, + setWorkspaceDir: async () => undefined, + pickWorkspaceDir: async () => null, + readDirectory: async () => [], + renameFile: async () => undefined, + deleteFile: async () => undefined + }; + return { ops, setSavePath: (path: string) => (savePath = path) }; +} + +describe('Tauri desktop session command boundary', () => { + beforeEach(() => { + tauri.invoke.mockReset(); + tauri.listen.mockClear(); + }); + + it('uses camelCase command arguments for New Board and Save As', async () => { + const files = fileOps(); + let currentPath = '/tmp/Untitled.inkfinite'; + const document = snapshot('board:test'); + tauri.invoke.mockImplementation(async (command: string, args: Record) => { + if (command === 'create_document') { + expect(args).toMatchObject({ + path: currentPath, + documentId: expect.stringMatching(/^board:/), + actorId: 'actor:desktop', + pageName: 'Page 1' + }); + expect(args).not.toHaveProperty('document_id'); + return { + session_id: 'session:1', + status: { + session_id: 'session:1', + path: currentPath, + actor_id: 'actor:desktop', + snapshot: document, + dirty: false, + lock_held: true, + recovery_available: false, + can_undo: false, + can_redo: false, + sync: { status: 'disabled' } + } + } satisfies SessionOpened; + } + if (command === 'save_as') { + expect(args).toEqual({ + sessionId: 'session:1', + path: '/tmp/Renamed.inkfinite', + expectedHeads: ['head:1'] + }); + expect(args).not.toHaveProperty('session_id'); + currentPath = '/tmp/Renamed.inkfinite'; + return { + save: { path: currentPath, heads: ['head:1'] }, + status: { + session_id: 'session:1', + path: currentPath, + actor_id: 'actor:desktop', + snapshot: document, + dirty: false, + lock_held: true, + recovery_available: false, + can_undo: false, + can_redo: false, + sync: { status: 'disabled' } + } + } satisfies SessionSaved; + } + throw new Error(`Unexpected command: ${command}`); + }); + + const repo = createDesktopSessionRepo(files.ops); + await repo.createBoard('Untitled'); + files.setSavePath('/tmp/Renamed.inkfinite'); + await repo.saveAs(); + + expect(tauri.invoke).toHaveBeenCalledWith('create_document', expect.any(Object)); + expect(tauri.invoke).toHaveBeenCalledWith('save_as', expect.any(Object)); + }); +}); diff --git a/apps/desktop/src/lib/persistence/desktop-session.ts b/apps/desktop/src/lib/persistence/desktop-session.ts index cef2d9d..0bcee7e 100644 --- a/apps/desktop/src/lib/persistence/desktop-session.ts +++ b/apps/desktop/src/lib/persistence/desktop-session.ts @@ -141,30 +141,74 @@ export interface SessionApi { function createSessionApi(): SessionApi { return { - createDocument: (args) => invoke('create_document', args), - openDocument: (args) => invoke('open_document', args), - openOrCreateDraft: (args) => invoke('open_or_create_draft', args), - snapshot: (args) => invoke('snapshot', args), - commit: (args) => invoke('commit', args), - propose: (args) => invoke('propose', args), - acceptProposal: (args) => invoke('accept_proposal', args), - rejectProposal: (args) => invoke('reject_proposal', args), - authorizeApply: (args) => invoke('authorize_apply', args), - undo: (args) => invoke('undo', args), - redo: (args) => invoke('redo', args), - save: (args) => invoke('save', args), - saveAs: (args) => invoke('save_as', args), - saveDraftAs: (args) => invoke('save_draft_as', args), - query: (args) => invoke('query', args), - validate: (args) => invoke('validate', args), - syncConnect: (args) => invoke('sync_connect', args), - syncDisconnect: (args) => invoke('sync_disconnect', args), - syncNext: (args) => invoke('sync_next', args), - syncReceive: (args) => invoke('sync_receive', args), - close: (args) => invoke('close', args) + createDocument: (args) => + invokeSession('create_document', { + path: args.path, + documentId: args.document_id, + actorId: args.actor_id, + pageName: args.page_name + }), + openDocument: (args) => + invokeSession('open_document', { path: args.path, actorId: args.actor_id }), + openOrCreateDraft: (args) => + invokeSession('open_or_create_draft', { + documentId: args.document_id, + actorId: args.actor_id + }), + snapshot: (args) => invokeSession('snapshot', { sessionId: args.session_id }), + commit: (args) => + invokeSession('commit', { sessionId: args.session_id, transaction: args.transaction }), + propose: (args) => + invokeSession('propose', { sessionId: args.session_id, transaction: args.transaction }), + acceptProposal: (args) => + invokeSession('accept_proposal', { + sessionId: args.session_id, + proposalId: args.proposal_id, + operationPositions: args.operation_positions + }), + rejectProposal: (args) => + invokeSession('reject_proposal', { sessionId: args.session_id, proposalId: args.proposal_id }), + authorizeApply: (args) => invokeSession('authorize_apply', { sessionId: args.session_id }), + undo: (args) => invokeSession('undo', { sessionId: args.session_id, actorId: args.actor_id }), + redo: (args) => invokeSession('redo', { sessionId: args.session_id, actorId: args.actor_id }), + save: (args) => + invokeSession('save', { sessionId: args.session_id, expectedHeads: args.expected_heads }), + saveAs: (args) => + invokeSession('save_as', { + sessionId: args.session_id, + path: args.path, + expectedHeads: args.expected_heads + }), + saveDraftAs: (args) => + invokeSession('save_draft_as', { + sessionId: args.session_id, + path: args.path, + expectedHeads: args.expected_heads + }), + query: (args) => invokeSession('query', { sessionId: args.session_id, query: args.query }), + validate: (args) => invokeSession('validate', { sessionId: args.session_id }), + syncConnect: (args) => + invokeSession('sync_connect', { sessionId: args.session_id, peerId: args.peer_id }), + syncDisconnect: (args) => + invokeSession('sync_disconnect', { sessionId: args.session_id, peerId: args.peer_id }), + syncNext: (args) => + invokeSession('sync_next', { sessionId: args.session_id, peerId: args.peer_id }), + syncReceive: (args) => + invokeSession('sync_receive', { sessionId: args.session_id, message: args.message }), + close: (args) => invokeSession('close', { sessionId: args.session_id }) }; } +function invokeSession(command: string, args: Record): Promise { + return invoke(command, args).catch((error: unknown) => { + const detail = + error instanceof Error ? error.message : typeof error === 'string' ? error : JSON.stringify(error); + const message = `${command} failed: ${detail}`; + void invoke('record_renderer_error', { message }).catch(() => undefined); + throw error; + }); +} + /** Persistent document repository backed by one backend/tauri-owned session. */ export type DesktopSessionRepo = PersistentDocRepo & { kind: 'desktop'; @@ -495,10 +539,7 @@ export function createDesktopSessionRepo(fileOps: DesktopFileOps, opts: { api?: if (!currentStatus || !currentBoard || !currentDoc) throw new Error('No board loaded'); const saved = path === currentStatus.path - ? await api.save({ - session_id: currentStatus.session_id, - expected_heads: currentStatus.snapshot.heads - }) + ? await api.save({ session_id: currentStatus.session_id, expected_heads: currentStatus.snapshot.heads }) : await (currentIsDraft ? api.saveDraftAs : api.saveAs)({ session_id: currentStatus.session_id, path, diff --git a/apps/desktop/src/lib/platform.ts b/apps/desktop/src/lib/platform.ts index bfb04b5..a9deb81 100644 --- a/apps/desktop/src/lib/platform.ts +++ b/apps/desktop/src/lib/platform.ts @@ -1,3 +1,4 @@ +import { invoke } from '@tauri-apps/api/core'; import { listen } from '@tauri-apps/api/event'; import { createStatusStore, type EditorPlatformAdapter, type NativeFileMenuAction } from '@inkfinite/ui/editor'; import { createDesktopFileOps } from './fileops'; @@ -18,7 +19,12 @@ export function createDesktopPlatformAdapter(): EditorPlatformAdapter { subscribeFileMenu(listener) { let active = true; let stop: (() => void) | undefined; - void listen('inkfinite-file-menu', (event) => listener(event.payload)) + void listen('inkfinite-file-menu', (event) => { + void invoke('record_renderer_event', { action: event.payload }).catch((error) => + console.error('Failed to record native File menu command', error) + ); + listener(event.payload); + }) .then((unlisten) => { if (active) stop = unlisten; else unlisten(); diff --git a/crates/inkfinite-core/src/file/persistence.rs b/crates/inkfinite-core/src/file/persistence.rs index 7222b86..96c06d0 100644 --- a/crates/inkfinite-core/src/file/persistence.rs +++ b/crates/inkfinite-core/src/file/persistence.rs @@ -2,7 +2,7 @@ use std::collections::{BTreeMap, btree_map::Entry}; use std::fmt::Write as _; -use std::fs::{self, File, OpenOptions}; +use std::fs::{self, File, OpenOptions, TryLockError}; use std::io::Write; use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicU64, Ordering}; @@ -971,33 +971,39 @@ fn lock_path(path: &Path) -> PathBuf { .join(format!(".{file_name}.lock")) } +/// Holds an OS-exclusive lock on a stable sidecar inode for one document. +/// +/// The sidecar remains after clean shutdown so another process cannot create a +/// different inode while a waiting writer still holds the previous one open. +/// Closing the file releases the kernel lock, including when the process exits +/// after a crash. struct AdvisoryLock { - path: PathBuf, _file: File, } impl AdvisoryLock { fn acquire(document_path: &Path) -> Result { let path = lock_path(document_path); - let mut file = match OpenOptions::new().write(true).create_new(true).open(&path) { - Ok(file) => file, - Err(error) if error.kind() == std::io::ErrorKind::AlreadyExists => { - return Err(FileError::Locked { path: document_path.to_owned() }); - } - Err(error) => return Err(io_error("create document lock", path, error)), - }; - let owner = format!("pid={}\n", std::process::id()); - if let Err(error) = file.write_all(owner.as_bytes()).and_then(|()| file.sync_all()) { - let _ = fs::remove_file(&path); - return Err(io_error("write document lock", path, error)); + let mut file = OpenOptions::new() + .read(true) + .write(true) + .create(true) + .truncate(false) + .open(&path) + .map_err(|error| io_error("open document lock", path.clone(), error))?; + match file.try_lock() { + Ok(()) => {} + Err(TryLockError::WouldBlock) => return Err(FileError::Locked { path: document_path.to_owned() }), + Err(TryLockError::Error(error)) => return Err(io_error("lock document", path, error)), } - Ok(Self { path, _file: file }) - } -} - -impl Drop for AdvisoryLock { - fn drop(&mut self) { - let _ = fs::remove_file(&self.path); + if let Err(error) = file + .set_len(0) + .and_then(|()| file.write_all(format!("pid={}\n", std::process::id()).as_bytes())) + .and_then(|()| file.sync_all()) + { + return Err(io_error("write document lock", path, error)); + }; + Ok(Self { _file: file }) } } diff --git a/crates/inkfinite-core/src/file/tests.rs b/crates/inkfinite-core/src/file/tests.rs index c7a8d50..0d81cfb 100644 --- a/crates/inkfinite-core/src/file/tests.rs +++ b/crates/inkfinite-core/src/file/tests.rs @@ -58,6 +58,30 @@ fn canonical_sessions_lock_save_reopen_and_export_deterministically() { ); } +#[test] +fn stale_lock_sidecar_does_not_block_a_new_writer() { + let temporary = TestDirectory::new(); + let canonical = temporary.path.join("board.inkfinite"); + let lock = temporary.path.join(".board.inkfinite.lock"); + fs::write(&lock, b"pid=999999999\n").expect("write abandoned lock sidecar"); + + let session = DocumentFile::create( + &canonical, + DocumentId::from("document:stale-lock"), + ActorId::from("actor:first"), + simple_document(), + ) + .expect("reclaim stale lock sidecar"); + assert!(matches!( + DocumentFile::open(&canonical, ActorId::from("actor:blocked")), + Err(FileError::Locked { .. }) + )); + + drop(session); + assert!(lock.exists()); + DocumentFile::open(&canonical, ActorId::from("actor:restart")).expect("open after the previous writer exits"); +} + #[test] fn rejects_invalid_canonical_bytes_without_replacing_the_file() { let temporary = TestDirectory::new(); -- 2.51.2