fix: handle invalid Windows save paths

This commit is contained in:
lucaronin
2026-04-27 04:04:04 +02:00
parent f1bed131bf
commit fb39c6679a
8 changed files with 307 additions and 15 deletions

View File

@@ -266,6 +266,7 @@ Tolaria separates **display title** from the file identifier:
- **Explicit filename actions** (`rename_note`): breadcrumb rename/sync actions stage crash-safe note renames through a hidden `.tolaria-rename-txn/` transaction directory, recover unfinished renames on the next vault scan, update wikilinks across the vault, and surface any failed backlink rewrites instead of silently reporting partial success. The editor body remains the title editing surface.
- **Unicode-aware note stems** (`src/utils/noteSlug.ts`, `vault/rename.rs`): frontend and backend slugging preserve Unicode letters/digits in note filenames, untitled-rename detection, and fallback wikilink targets while still collapsing symbol-only titles to `untitled`.
- **Portable filename validation** (`vault/filename_rules.rs`): note filenames, folder names, and custom view filenames all reject Windows-reserved device names, invalid characters, and trailing dot/space suffixes so a vault created on macOS/Linux still clones and syncs cleanly on Windows.
- **Recoverable save failures** (`useEditorSave`, `vault/file.rs`): invalid platform path syntax is reported as a clear retryable save error, while the editor keeps the unsaved buffer intact for another attempt.
- **Untitled drafts** start as `untitled-*.md` and are auto-renamed on save once the note gains an H1.
### Title Surface (UI)

View File

@@ -266,6 +266,28 @@ mod tests {
.contains("title: Command Note"));
}
#[test]
fn note_content_commands_accept_windows_sensitive_valid_segments() {
let dir = TempDir::new().unwrap();
let root = vault_root(&dir);
let note = root
.join("@raflymln")
.join("notes with spaces")
.join("résumé note.md");
save_note_content(
note.clone(),
"# Windows-Sensitive Path\n\nBody\n".to_string(),
Some(root.clone()),
)
.unwrap();
assert_eq!(
get_note_content(note, Some(root)).unwrap(),
"# Windows-Sensitive Path\n\nBody\n"
);
}
#[test]
fn folder_and_listing_commands_use_expanded_vault_root() {
let dir = TempDir::new().unwrap();

View File

@@ -1,5 +1,5 @@
use std::fs;
use std::io::{ErrorKind, Write};
use std::io::{Error, ErrorKind, Write};
use std::path::Path;
use std::time::UNIX_EPOCH;
@@ -25,6 +25,49 @@ fn invalid_utf8_text_error(path: &Path) -> String {
format!("File is not valid UTF-8 text: {}", path.display())
}
fn is_invalid_platform_path_error(error: &Error) -> bool {
error.kind() == ErrorKind::InvalidInput || error.raw_os_error() == Some(123)
}
#[derive(Clone, Copy)]
enum NoteIoOperation {
Save,
Create,
}
#[derive(Clone, Copy)]
struct NotePathDisplay<'a> {
value: &'a str,
}
impl<'a> NotePathDisplay<'a> {
fn new(value: &'a str) -> Self {
Self { value }
}
}
impl NoteIoOperation {
fn verb(self) -> &'static str {
match self {
Self::Save => "save",
Self::Create => "create",
}
}
}
fn note_io_error(operation: NoteIoOperation, path: NotePathDisplay<'_>, error: &Error) -> String {
let verb = operation.verb();
if is_invalid_platform_path_error(error) {
let path = path.value;
format!(
"Failed to {verb} note: the path is invalid on this platform. Rename the note or move it to a valid folder, then try again. Path: {path}"
)
} else {
let path = path.value;
format!("Failed to {verb} {path}: {error}")
}
}
/// Read the content of a single note file.
pub fn get_note_content(path: &Path) -> Result<String, String> {
if !path.exists() {
@@ -62,12 +105,14 @@ pub fn save_note_content(path: &str, content: &str) -> Result<(), String> {
let file_path = Path::new(path);
if let Some(parent) = file_path.parent() {
if !parent.exists() {
fs::create_dir_all(parent)
.map_err(|e| format!("Failed to create directory {}: {}", parent.display(), e))?;
fs::create_dir_all(parent).map_err(|e| {
note_io_error(NoteIoOperation::Save, NotePathDisplay::new(path), &e)
})?;
}
}
validate_save_path(file_path, path)?;
fs::write(file_path, content).map_err(|e| format!("Failed to save {}: {}", path, e))
fs::write(file_path, content)
.map_err(|e| note_io_error(NoteIoOperation::Save, NotePathDisplay::new(path), &e))
}
/// Create a new note file without overwriting any existing file.
@@ -75,8 +120,9 @@ pub fn create_note_content(path: &str, content: &str) -> Result<(), String> {
let file_path = Path::new(path);
if let Some(parent) = file_path.parent() {
if !parent.exists() {
fs::create_dir_all(parent)
.map_err(|e| format!("Failed to create directory {}: {}", parent.display(), e))?;
fs::create_dir_all(parent).map_err(|e| {
note_io_error(NoteIoOperation::Create, NotePathDisplay::new(path), &e)
})?;
}
}
validate_save_path(file_path, path)?;
@@ -86,8 +132,27 @@ pub fn create_note_content(path: &str, content: &str) -> Result<(), String> {
.open(file_path)
.map_err(|e| match e.kind() {
ErrorKind::AlreadyExists => format!("File already exists: {}", path),
_ => format!("Failed to create {}: {}", path, e),
_ => note_io_error(NoteIoOperation::Create, NotePathDisplay::new(path), &e),
})?;
file.write_all(content.as_bytes())
.map_err(|e| format!("Failed to save {}: {}", path, e))
.map_err(|e| note_io_error(NoteIoOperation::Save, NotePathDisplay::new(path), &e))
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn formats_windows_invalid_path_syntax_as_recoverable_save_error() {
let path = r"C:\Users\@raflymln\notes\untitled-note-1777236475.md";
let message = note_io_error(
NoteIoOperation::Save,
NotePathDisplay::new(path),
&Error::from_raw_os_error(123),
);
assert!(message.contains("path is invalid on this platform"));
assert!(message.contains("Rename the note or move it to a valid folder"));
assert!(!message.contains("os error 123"));
}
}

View File

@@ -169,6 +169,35 @@ describe('useAppSave', () => {
// Should complete without error
})
it('handles Windows invalid path save failures without clearing unsaved content', async () => {
vi.mocked(isTauri).mockReturnValue(true)
const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
const path = 'C:\\Users\\@raflymln\\notes\\untitled-note-1777236475.md'
const entry = makeEntry(path, 'Untitled Note 1777236475', 'untitled-note-1777236475.md')
vi.mocked(invoke).mockRejectedValueOnce(
new Error('The filename, directory name, or volume label syntax is incorrect. (os error 123)'),
)
const { result } = renderSave({
tabs: [{ entry, content: '# Draft\n\nBody' }],
activeTabPath: path,
unsavedPaths: new Set([path]),
})
let saved = true
await act(async () => {
saved = await result.current.handleSave()
})
expect(saved).toBe(false)
expect(deps.setToastMessage).toHaveBeenCalledWith(
'Save failed: The note path is invalid on this platform. Rename the note or move it to a valid folder, then try again.',
)
expect(deps.clearUnsaved).not.toHaveBeenCalled()
expect(vi.mocked(invoke)).not.toHaveBeenCalledWith('auto_rename_untitled', expect.anything())
consoleSpy.mockRestore()
})
it('handleContentChange is a function', () => {
const { result } = renderSave()
expect(typeof result.current.handleContentChange).toBe('function')

View File

@@ -565,7 +565,7 @@ function useHandleSaveAction({
flushPendingUntitledRename,
resolveCurrentPath,
}: {
handleSaveRaw: (unsavedFallback?: { path: string; content: string }) => Promise<void>
handleSaveRaw: (unsavedFallback?: { path: string; content: string }) => Promise<boolean>
tabs: TabState[]
activeTabPath: string | null
unsavedPaths: Set<string>
@@ -574,12 +574,14 @@ function useHandleSaveAction({
}) {
return useCallback(async () => {
const resolvedActiveTabPath = activeTabPath ? resolveCurrentPath(activeTabPath) : null
await handleSaveRaw(findUnsavedFallback({
const saveCompleted = await handleSaveRaw(findUnsavedFallback({
tabs,
activeTabPath: resolvedActiveTabPath,
unsavedPaths,
}))
if (!saveCompleted) return false
await flushPendingUntitledRename(resolvedActiveTabPath ?? undefined)
return true
}, [handleSaveRaw, tabs, activeTabPath, unsavedPaths, flushPendingUntitledRename, resolveCurrentPath])
}
@@ -696,7 +698,7 @@ function useAppSaveHandlers({
resolvedPath: string
replaceRenamedEntry: (oldPath: string, newEntry: Partial<VaultEntry> & { path: string }, newContent: string) => void
loadModifiedFiles: AppSaveDeps['loadModifiedFiles']
handleSaveRaw: (unsavedFallback?: { path: string; content: string }) => Promise<void>
handleSaveRaw: (unsavedFallback?: { path: string; content: string }) => Promise<boolean>
tabs: TabState[]
unsavedPaths: Set<string>
}) {

View File

@@ -84,6 +84,42 @@ describe('useEditorSave', () => {
consoleSpy.mockRestore()
})
it('keeps failed Windows path saves pending with a recoverable error toast', async () => {
const path = 'C:\\Users\\@raflymln\\notes\\untitled-note-1777236475.md'
mockInvokeFn.mockRejectedValueOnce(
new Error(`Failed to save ${path}: The filename, directory name, or volume label syntax is incorrect. (os error 123)`),
)
const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
const { result } = renderSaveHook()
act(() => {
result.current.handleContentChange(path, '# Draft\n\nUnsaved body')
})
let saved = true
await act(async () => {
saved = await result.current.handleSave()
})
expect(saved).toBe(false)
expect(setToastMessage).toHaveBeenCalledWith(
'Save failed: The note path is invalid on this platform. Rename the note or move it to a valid folder, then try again.',
)
expect(updateVaultContent).not.toHaveBeenCalled()
await act(async () => {
saved = await result.current.handleSave()
})
expect(saved).toBe(true)
expect(mockInvokeFn).toHaveBeenLastCalledWith('save_note_content', {
path,
content: '# Draft\n\nUnsaved body',
})
expect(updateVaultContent).toHaveBeenCalledWith(path, '# Draft\n\nUnsaved body')
consoleSpy.mockRestore()
})
it('savePendingForPath saves content only for the matching path', async () => {
const { result } = renderSaveHook()
@@ -320,6 +356,32 @@ describe('useEditorSave', () => {
expect(setToastMessage).not.toHaveBeenCalled()
})
it('auto-save reports invalid path failures and leaves content retryable', async () => {
const path = 'C:\\Users\\@raflymln\\notes\\untitled-note-1777236475.md'
mockInvokeFn.mockRejectedValueOnce(new Error('The filename, directory name, or volume label syntax is incorrect. (os error 123)'))
const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
const { result } = renderHook(() =>
useEditorSave({ updateVaultContent, setTabs, setToastMessage })
)
act(() => { result.current.handleContentChange(path, 'draft from auto-save') })
await act(async () => { await vi.advanceTimersByTimeAsync(500) })
expect(setToastMessage).toHaveBeenCalledWith(
'Save failed: The note path is invalid on this platform. Rename the note or move it to a valid folder, then try again.',
)
expect(updateVaultContent).not.toHaveBeenCalled()
await act(async () => { await result.current.handleSave() })
expect(mockInvokeFn).toHaveBeenLastCalledWith('save_note_content', {
path,
content: 'draft from auto-save',
})
expect(updateVaultContent).toHaveBeenCalledWith(path, 'draft from auto-save')
consoleSpy.mockRestore()
})
it('Cmd+S cancels pending auto-save and saves immediately', async () => {
const { result } = renderHook(() =>
useEditorSave({ updateVaultContent, setTabs, setToastMessage })

View File

@@ -28,12 +28,31 @@ interface EditorSaveConfig {
const noop = () => {}
const AUTO_SAVE_DEBOUNCE_MS = 500
const INVALID_PATH_SAVE_MESSAGE = 'Save failed: The note path is invalid on this platform. Rename the note or move it to a valid folder, then try again.'
interface PendingContent {
path: string
content: string
}
function errorMessage(error: unknown): string {
if (error instanceof Error) return error.message
return String(error)
}
function isInvalidPathSaveError(message: string): boolean {
const normalized = message.toLowerCase()
return normalized.includes('os error 123')
|| normalized.includes('filename, directory name, or volume label syntax is incorrect')
|| normalized.includes('path is invalid on this platform')
}
function formatSaveFailureMessage(error: unknown): string {
const message = errorMessage(error)
if (isInvalidPathSaveError(message)) return INVALID_PATH_SAVE_MESSAGE
return `Save failed: ${message}`
}
function resolveBufferedPath(path: string, resolvePath?: EditorSaveConfig['resolvePath']): string {
return resolvePath?.(path) ?? path
}
@@ -91,10 +110,12 @@ function scheduleAutoSave({
autoSaveTimerRef,
flushPending,
onAfterSaveRef,
setToastMessage,
}: {
autoSaveTimerRef: MutableRefObject<ReturnType<typeof setTimeout> | null>
flushPending: () => Promise<boolean>
onAfterSaveRef: MutableRefObject<() => void>
setToastMessage: EditorSaveConfig['setToastMessage']
}): void {
autoSaveTimerRef.current = setTimeout(async () => {
autoSaveTimerRef.current = null
@@ -103,6 +124,7 @@ function scheduleAutoSave({
if (saved) onAfterSaveRef.current()
} catch (err) {
console.error('Auto-save failed:', err)
setToastMessage(formatSaveFailureMessage(err))
}
}, AUTO_SAVE_DEBOUNCE_MS)
}
@@ -198,7 +220,7 @@ function useImmediateSaveCommands({
resolvePath?: EditorSaveConfig['resolvePath']
resolvePathBeforeSave?: EditorSaveConfig['resolvePathBeforeSave']
}) {
const handleSave = useCallback(async (unsavedFallback?: { path: string; content: string }) => {
const handleSave = useCallback(async (unsavedFallback?: { path: string; content: string }): Promise<boolean> => {
cancelAutoSave()
try {
const saved = await flushPending()
@@ -211,9 +233,11 @@ function useImmediateSaveCommands({
})
setToastMessage(saved || savedFallback ? 'Saved' : 'Nothing to save')
onAfterSave()
return true
} catch (err) {
console.error('Save failed:', err)
setToastMessage(`Save failed: ${err}`)
setToastMessage(formatSaveFailureMessage(err))
return false
}
}, [cancelAutoSave, flushPending, onAfterSave, onNotePersisted, resolvePath, resolvePathBeforeSave, saveNote, setToastMessage])
@@ -231,6 +255,7 @@ function useContentChangeCommand({
pendingContentRef,
autoSaveTimerRef,
setTabs,
setToastMessage,
cancelAutoSave,
flushPending,
onAfterSaveRef,
@@ -238,6 +263,7 @@ function useContentChangeCommand({
pendingContentRef: MutableRefObject<PendingContent | null>
autoSaveTimerRef: MutableRefObject<ReturnType<typeof setTimeout> | null>
setTabs: EditorSaveConfig['setTabs']
setToastMessage: EditorSaveConfig['setToastMessage']
cancelAutoSave: () => void
flushPending: () => Promise<boolean>
onAfterSaveRef: MutableRefObject<() => void>
@@ -246,8 +272,8 @@ function useContentChangeCommand({
pendingContentRef.current = { path, content }
applyTabContent(setTabs, path, content)
cancelAutoSave()
scheduleAutoSave({ autoSaveTimerRef, flushPending, onAfterSaveRef })
}, [autoSaveTimerRef, cancelAutoSave, flushPending, onAfterSaveRef, pendingContentRef, setTabs])
scheduleAutoSave({ autoSaveTimerRef, flushPending, onAfterSaveRef, setToastMessage })
}, [autoSaveTimerRef, cancelAutoSave, flushPending, onAfterSaveRef, pendingContentRef, setTabs, setToastMessage])
}
function useEditorSaveCommands({
@@ -295,6 +321,7 @@ function useEditorSaveCommands({
pendingContentRef,
autoSaveTimerRef,
setTabs,
setToastMessage,
cancelAutoSave,
flushPending: () => flushPending(),
onAfterSaveRef,

View File

@@ -0,0 +1,84 @@
import { test, expect, type Page } from '@playwright/test'
import { executeCommand, openCommandPalette, sendShortcut } from './helpers'
const RAW_EDITOR = '.cm-content'
type MockHandler = (args?: Record<string, unknown>) => unknown
async function openFirstNote(page: Page) {
await page.waitForSelector('[data-testid="sidebar-top-nav"]', { timeout: 10_000 })
const noteList = page.locator('[data-testid="note-list-container"]')
await noteList.waitFor({ timeout: 5_000 })
await noteList.locator('.cursor-pointer').first().click()
await expect(page.locator('.bn-editor')).toBeVisible({ timeout: 5_000 })
}
async function setRawEditorContent(page: Page, content: string) {
await page.evaluate((nextContent) => {
const el = document.querySelector('.cm-content')
if (!el) throw new Error('CodeMirror content element is missing')
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const view = (el as any).cmTile?.view
if (!view) throw new Error('CodeMirror view is missing')
view.dispatch({
changes: { from: 0, to: view.state.doc.length, insert: nextContent },
})
view.focus()
}, content)
}
async function openRawMode(page: Page) {
await openCommandPalette(page)
await executeCommand(page, 'Toggle Raw')
await expect(page.locator(RAW_EDITOR)).toBeVisible({ timeout: 5_000 })
}
async function installFailOnceSaveMock(page: Page) {
await page.waitForFunction(() => Boolean(window.__mockHandlers?.save_note_content))
await page.evaluate(() => {
const handlers = window.__mockHandlers as Record<string, MockHandler>
const originalSaveNoteContent = handlers.save_note_content
let shouldFail = true
window.__laputaTest = {
...window.__laputaTest,
saveAttempts: [],
}
handlers.save_note_content = (args?: Record<string, unknown>) => {
window.__laputaTest?.saveAttempts?.push(args)
if (shouldFail) {
shouldFail = false
throw new Error('The filename, directory name, or volume label syntax is incorrect. (os error 123)')
}
return originalSaveNoteContent?.(args)
}
})
}
test('failed Windows path saves show a recoverable toast and retry the draft', async ({ page }) => {
const pageErrors: string[] = []
page.on('pageerror', (err) => { pageErrors.push(err.message) })
await page.route('**/api/vault/ping', async (route) => {
await route.fulfill({ status: 404, body: '' })
})
await page.goto('/')
await openFirstNote(page)
await installFailOnceSaveMock(page)
await openRawMode(page)
await setRawEditorContent(page, '# Retryable Windows Save\n\nDraft that must survive failure')
await page.waitForTimeout(550)
await sendShortcut(page, 's', ['Control'])
await expect(page.locator('.fixed.bottom-8')).toContainText('note path is invalid on this platform', { timeout: 5_000 })
expect(pageErrors.filter((message) => message.includes('os error 123'))).toHaveLength(0)
await sendShortcut(page, 's', ['Control'])
await expect(page.locator('.fixed.bottom-8')).toContainText('Saved', { timeout: 5_000 })
const saveAttempts = await page.evaluate(() => window.__laputaTest?.saveAttempts ?? [])
expect(saveAttempts).toHaveLength(2)
expect(saveAttempts[1]).toEqual(expect.objectContaining({
content: '# Retryable Windows Save\n\nDraft that must survive failure',
}))
})