Add multiple windows, workspaces, and per-window sessions - #197
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds workspace-aware runtime state, revision-checked persistence, draft checkpoints, conflict recovery, scoped settings, window sessions, editor preferences, workspace navigation, and Vitest infrastructure. ChangesWorkspace and persistence flow
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/notes/NoteList.tsx (1)
297-299: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd error handling to
refreshSettings.
refreshSettingscallsnotesService.getSettings().then(setSettings)without a.catch(). The mount-time load at lines 265-272 catches and logs errors, but this callback does not. IfgetSettings()rejects, the failure is silent and the promise rejection is unhandled.Add the same error handling used elsewhere in this file.
🩹 Proposed fix
const refreshSettings = useCallback(() => { - notesService.getSettings().then(setSettings); + notesService.getSettings().then(setSettings).catch((error) => { + console.error("Failed to refresh settings:", error); + }); }, []);As per path instructions for
src/**/*.{ts,tsx}: "Implement error handling with user-friendly messages".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/notes/NoteList.tsx` around lines 297 - 299, Update the refreshSettings callback to handle rejected notesService.getSettings() promises, matching the existing mount-time error handling in NoteList and reporting a user-friendly error through the file’s established logging or notification mechanism.Source: Path instructions
src/components/editor/Editor.tsx (1)
872-901: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAlign the formatted-editor auto-save debounce with the project value.
scheduleSaveuses a 500ms debounce.handleSourceChange(Line 2393) uses 300ms. The two edit paths persist the same note with different latencies.Based on coding guidelines: "Debounce user-triggered operations: auto-save 300ms, search 150ms, file watcher 500ms, git status 1000ms".
⏱️ Proposed fix
- }, 500); + }, 300);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/editor/Editor.tsx` around lines 872 - 901, Update the debounce delay in scheduleSave’s window.setTimeout from 500ms to the project-standard 300ms, matching handleSourceChange while preserving the existing save-generation and persistence logic.Source: Coding guidelines
src/context/NotesContext.tsx (1)
489-498: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winMove the ref writes and nested setters out of the state updater.
React can call an updater more than once. The updater writes
selectedNoteIdRef,currentNoteRef, and queuessetCurrentNote/setNoteConflict. Compute the condition fromselectedNoteIdRef.currentand perform the side effects outside the setter.🛠️ Proposed fix
- setSelectedNoteId((prevId) => { - if (prevId === id) { - selectedNoteIdRef.current = null; - currentNoteRef.current = null; - setCurrentNote(null); - setNoteConflict(null); - return null; - } - return prevId; - }); + if (selectedNoteIdRef.current === id) { + selectedNoteIdRef.current = null; + currentNoteRef.current = null; + setSelectedNoteId(null); + setCurrentNote(null); + setNoteConflict(null); + }Based on learnings from static analysis: "This state updater performs the captured value selectedNoteIdRef" and "This side-effecting call runs inside a state updater".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/context/NotesContext.tsx` around lines 489 - 498, Update the note-selection logic around setSelectedNoteId so it checks selectedNoteIdRef.current against id before invoking the state setter, then performs the selectedNoteIdRef, currentNoteRef, setCurrentNote, and setNoteConflict updates outside the updater. Keep the setter limited to returning the previous or new selected note ID without side effects.Source: Linters/SAST tools
🧹 Nitpick comments (12)
src/components/layout/Sidebar.tsx (1)
262-271: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winFilter the settings event by scope.
handleSettingsChangedignores the event detail, so the sidebar reloads settings for everysettings-changedevent, including workspace-scoped events from other workspaces.src/lib/settingsScope.tsalready exportsshouldApplySettingsChange, andsrc/lib/settingsScope.test.tsasserts that a workspace event must not apply to a different workspace. Use that helper withnotesFolderto avoid redundant IPC calls on unrelated workspace changes.♻️ Proposed scope filter
- // Workspace settings update live in every window bound to that workspace. useEffect(() => { void loadWorkspaceSettings(); - const handleSettingsChanged = () => void loadWorkspaceSettings(); + const handleSettingsChanged = (event: Event) => { + const detail = (event as CustomEvent<SettingsChangedEvent>).detail; + if (detail && !shouldApplySettingsChange(detail, notesFolder)) return; + void loadWorkspaceSettings(); + }; window.addEventListener(SETTINGS_CHANGED_DOM_EVENT, handleSettingsChanged); return () => window.removeEventListener( SETTINGS_CHANGED_DOM_EVENT, handleSettingsChanged, ); - }, [loadWorkspaceSettings]); + }, [loadWorkspaceSettings, notesFolder]);Import the helper and the type:
import { SETTINGS_CHANGED_DOM_EVENT, shouldApplySettingsChange, type SettingsChangedEvent, } from "../../lib/settingsScope";🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/layout/Sidebar.tsx` around lines 262 - 271, Update the settings-change effect in Sidebar.tsx to type the event as SettingsChangedEvent and call shouldApplySettingsChange with the event detail and notesFolder before invoking loadWorkspaceSettings. Import the helper and type from settingsScope, while preserving the initial load and listener cleanup behavior.src/components/layout/WorkspaceMenu.tsx (1)
107-121: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd arrow-key navigation and initial focus for the
role="menu"container.The container declares
role="menu", so assistive technology expects item navigation with ArrowUp, ArrowDown, Home, and End, plus focus inside the menu after it opens. The current implementation relies on Tab only. Keyboard users can still reach every item, so this degrades the experience instead of blocking it.Either implement roving focus for the
menuitemradioitems, or change the container to a plain listbox-free composite that matches the actual keyboard behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/layout/WorkspaceMenu.tsx` around lines 107 - 121, The role="menu" container in WorkspaceMenu must support menu keyboard behavior rather than relying on Tab navigation. Implement roving focus for its menuitemradio children, including initial focus on open and ArrowUp, ArrowDown, Home, and End handling, while preserving selection behavior; alternatively, remove the menu role and use semantics matching the existing focus behavior.src-tauri/src/lib.rs (2)
1257-1274: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe final branch in
workspace_for_windowrepeats a lookup that already failed.For a label that is not a fallback label,
workspace_session(window_label)already returnedNoneat line 1261. Line 1271 repeats the same lookup and can only returnNone. Return the error directly to make the control flow explicit.♻️ Proposed simplification
- self.workspace_session(window_label) - .map(WorkspaceRuntime::Session) - .ok_or_else(|| format!("Workspace session not found for window: {}", window_label)) + Err(format!( + "Workspace session not found for window: {}", + window_label + ))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src-tauri/src/lib.rs` around lines 1257 - 1274, Update workspace_for_window so the non-fallback path returns the workspace-session-not-found error directly after the initial workspace_session lookup fails; remove the redundant final workspace_session(window_label) call while preserving the fallback behavior and existing error message.
3630-3652: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
update_git_enabledmutates memory before persistence can fail.The command sets
settings.git_enabledin a first lock scope, then saves in a second scope. Ifsave_settingsfails, the in-memory value already differs from disk, and the error returned to the caller suggests nothing changed. Build the updated value, persist it, and commit to memory only after the save succeeds, asupdate_workspace_settingsdoes at line 3576.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src-tauri/src/lib.rs` around lines 3630 - 3652, Update update_git_enabled to modify a temporary settings value and persist it before changing the workspace’s in-memory settings. Follow the transaction pattern used by update_workspace_settings: save the updated value first, then acquire the write lock and assign git_enabled only after save_settings succeeds.src-tauri/src/persistence.rs (2)
121-153: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
lock_keycan produce two keys for one file when the parent is missing.
lock_keycanonicalizes the parent directory. If the parent does not exist yet, it falls back to the non-canonical absolute path. Two callers that reach the same file through different path spellings then take different locks, so the per-path serialization is lost for that save. Create-only publication still protects the file, so the impact is limited.Consider canonicalizing the nearest existing ancestor and appending the remaining components.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src-tauri/src/persistence.rs` around lines 121 - 153, Update lock_key to canonicalize the nearest existing ancestor when the file’s immediate parent does not exist, then append the remaining path components so equivalent spellings produce one key. Preserve the existing absolute-path handling and file-name behavior while ensuring create-only paths resolve consistently before lock_for_path uses them.
270-311: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winOne hand-rolled SHA-256 implementation exists twice. Both modules carry their own copy of the compression function and the digest driver because the crate has no shared hashing utility. The copies can drift, and each copy carries its own correctness risk.
src-tauri/src/persistence.rs#L270-L311: remove the localsha256andsha256_compressand call one shared hashing helper, or thesha2crate.src-tauri/src/draft_checkpoint.rs#L314-L362: remove the localhex_sha256,sha256, andsha256_compressand call the same shared helper.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src-tauri/src/persistence.rs` around lines 270 - 311, Replace the duplicated hand-rolled hashing implementations with one shared hashing helper or the existing sha2 crate. In src-tauri/src/persistence.rs lines 270-311, remove sha256 and sha256_compress and update callers to use the shared implementation; in src-tauri/src/draft_checkpoint.rs lines 314-362, remove hex_sha256, sha256, and sha256_compress and update callers to use that same implementation, preserving the current digest and hexadecimal output behavior.src/lib/useWindowShortcuts.ts (1)
14-17: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider moving ref updates for "latest value" tracking out of render.
Line 16 and Line 17 write to
interfaceZoomRef.currentandopenPreferencesRef.currentdirectly in the render body. React Doctor'sno-ref-current-in-renderrule flags this because React can replay or discard render work, so ref writes during render are not guaranteed to be pure.This "latest ref" idiom is already used elsewhere in the codebase (for example
currentNoteRef.current = currentNoteinsrc/App.tsx), so this is consistent with existing conventions and unlikely to cause a bug today. If the project later adopts the React Compiler, consider moving these assignments into auseEffect(or a custom "insertion effect"-style hook) to keep render pure.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/useWindowShortcuts.ts` around lines 14 - 17, Move the latest-value assignments for interfaceZoomRef and openPreferencesRef out of the render body into an appropriate effect or insertion-effect-style hook within useWindowShortcuts, while preserving their updates whenever interfaceZoom or onOpenPreferences changes.Source: Linters/SAST tools
src/lib/serializedWriter.ts (1)
20-30: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the swallowed rejection in
createSerializedWriter.
createSerializedWriterreturns the promise produced after.catch(onError), so the returned promise always fulfills. A caller that awaits it and expects a rejection on failure will treat a failed write as a success.createSerializedTaskQueuein the same file rejects to the caller, so the two utilities have opposite error contracts.Add a short doc comment that states the contract.
📝 Proposed change
+/** + * Serializes writes. Failures are reported through `onError` only. The + * returned promise always fulfills, so callers cannot detect a failed write. + * Use `createSerializedTaskQueue` when the caller must observe rejections. + */ export function createSerializedWriter<T>(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/serializedWriter.ts` around lines 20 - 30, Add a concise documentation comment above createSerializedWriter stating that write errors are passed to onError and swallowed, so the returned promise fulfills rather than rejects; distinguish this contract from createSerializedTaskQueue without changing the implementation.src/lib/windowSession.test.ts (1)
38-87: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test for a workspace mismatch.
restoreWindowSessiondiscards a saved session whensaved.workspacedoes not equal the requestedworkspace. That branch is the workspace isolation guarantee of this change, and no test covers it. Add one case that loadssavedSessionwith a differentworkspacevalue and asserts safe defaults.A test for the write-failure restore path in
createWindowSessionPatchWriterwould also be useful.💚 Proposed test to cover workspace isolation
+ it("ignores a session saved for a different workspace", async () => { + const restored = await restoreWindowSession({ + isPreview: false, + workspace: "/notes/personal", + noteIds: ["projects/plan"], + load: async () => savedSession, + }); + + expect(restored).toEqual({ + selectedNoteId: null, + sidebarVisible: true, + focusMode: false, + geometry: null, + }); + }); + it("uses safe defaults when the session cannot be loaded", async () => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/windowSession.test.ts` around lines 38 - 87, Add a workspace-isolation test alongside the existing restoreWindowSession tests: load savedSession while requesting a workspace different from savedSession.workspace, then assert the safe-default result (no selected note, default sidebar/focus values, and null geometry). Do not add the optional createWindowSessionPatchWriter write-failure test unless required separately.src/lib/windowSession.ts (1)
132-144: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winRetry the failed window-session patch if the writer is not cancelled.
catchrestores the failed patch topending, but the timer is only created fromqueue(). A caller offlush()or an unmount flush has no later activity to re-send the patch. Scheduleflush()again after a failed write unless cancellation has occurred.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/windowSession.ts` around lines 132 - 144, Update the failed-write handling in the activeWrite catch path to schedule another flush when the writer is not cancelled, ensuring patches failed during flush() or unmount cleanup are retried without requiring a new queue() call. Preserve the existing pending merge and rethrow behavior, and avoid scheduling retries after cancellation.src/lib/windowCloseCallsites.test.ts (1)
14-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winWiden the negative assertion to any window receiver.
The test only blocks the literal
appWindow.close()andappWindow.destroy(). A regression that callsgetCurrentWindow().close()orwin.destroy()still passes. Use a regular expression on the method call.♻️ Proposed refactor
expect(source).toContain("closeWindowAfterSave"); - expect(source).not.toContain("appWindow.close()"); - expect(source).not.toContain("appWindow.destroy()"); + expect(source).not.toMatch(/\.\s*(close|destroy)\s*\(\s*\)/);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/windowCloseCallsites.test.ts` around lines 14 - 19, Update the negative assertions in the window source test around closeWindowAfterSave to reject close() and destroy() calls on any receiver, using regular expressions rather than matching only appWindow.close() and appWindow.destroy().src/context/NotesContext.tsx (1)
1042-1096: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winRead
noteConflictfrom a ref to stop watcher listener churn.The effect depends on
noteConflict, so every conflict change unregisters and re-registers thefile-changelistener. Events that arrive during the gap are lost, and the asynclistenround trip repeats. Keep the effect dependency list free ofnoteConflictand read the value from a ref inside the handler.♻️ Proposed refactor
const [noteConflict, setNoteConflict] = useState<NoteSyncConflict | null>( null, ); + const noteConflictRef = useRef<NoteSyncConflict | null>(null);Update the ref in an effect, then use it in the handler:
- conflict: noteConflict, + conflict: noteConflictRef.current,- }, [noteConflict, refreshNotes]); + }, [refreshNotes]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/context/NotesContext.tsx` around lines 1042 - 1096, Update the file-change listener flow in the effect containing reconcileRemoteNote to read noteConflict through a ref instead of the captured state value. Keep that ref synchronized in a separate effect, remove noteConflict from the listener effect dependency array, and use the ref’s current value when constructing syncState so the listener remains registered across conflict changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src-tauri/capabilities/default.json`:
- Around line 13-16: Update the opener:allow-open-path capability in
default.json so reveal-in-file-manager supports workspace folders outside $HOME,
including paths on external or other mounted volumes. Extend the allowed scope
to the appropriate platform mount roots, or replace this capability path with a
Rust command that validates paths against the bound workspace.
In `@src-tauri/src/draft_checkpoint.rs`:
- Around line 120-139: Update list_checkpoints so each JSON entry’s file read,
deserialization, and ensure_identity_matches validation are handled per entry;
skip entries that fail any of these steps instead of propagating the error.
Continue collecting and sorting all valid checkpoints, while preserving existing
entry and extension handling.
In `@src-tauri/src/lib.rs`:
- Around line 446-457: Replace the DefaultHasher-based implementation of
workspace_path_key with a stable digest whose output remains consistent across
Rust toolchain versions, preserving the workspace- prefix format used by
get_workspace_search_index_path. Update forget_workspace to remove the
corresponding workspace search-index directory before or alongside deleting its
configuration entry, using the same key/path derivation so stale indexes are
cleaned up.
- Around line 186-207: Update the filter in restorable_window_sessions to
exclude the exact "preferences" label in addition to labels beginning with
"preview-". Preserve the existing workspace availability check and restoration
ordering for all other records.
- Around line 409-444: Update emit_native_new_window_request to emit a distinct
event or action payload identifying the native “New Window” intent, while
keeping emit_native_open_folder_request mapped to the folder-opening behavior.
Ensure the frontend can distinguish the new-window request from the open-folder
request and create a workspace window for the former.
- Around line 2822-2848: Update the rename flow around save_if_revision and
fs::remove_file so a source-removal failure does not leave both old_file_path
and file_path present. If removing the source fails after the target is saved,
attempt to delete the newly created target before returning the error, while
preserving the existing error context and successful rename behavior.
- Around line 2097-2124: Make set_notes_folder and switch_workspace asynchronous
commands, moving WorkspaceSession::initialize and the existing workspace
initialization work into tauri::async_runtime::spawn_blocking. Await the task
result, propagate initialization errors, then perform the existing config
persistence, state registration, and event emission only after completion; keep
main-window handling consistent with its current behavior.
In `@src-tauri/src/persistence.rs`:
- Around line 181-203: Update atomic_create_new to fall back when fs::hard_link
reports an unsupported-operation error: open the destination with
create_new(true), write and flush the same bytes directly, and preserve the
existing AlreadyExists behavior so no entry is replaced. Keep the hard-link path
and temporary-file cleanup for filesystems that support links, and ensure the
fallback handles the temporary file safely before returning.
In `@src/App.tsx`:
- Around line 92-145: Update the close handling around runSafeWindowClose and
its catch so draft-save/recovery failures remain reported as draft-save errors,
while failures from flushWindowSession or closeWindowAfterSave use a distinct
window-session/native-close failure message. Preserve resetting
closeInProgressRef and keeping the window open for either failure source.
In `@src/components/editor/Editor.tsx`:
- Around line 2522-2532: Remove the role="status" attribute from the conflict
trigger button in the hasSaveConflict/resolveNoteConflict DropdownMenu.Trigger.
Preserve the native button semantics and ensure the conflict status wording is
included in the trigger’s accessible name rather than exposed as a status role.
- Around line 1823-1836: Update the cleanup returned by the useEffect around
checkpointCaptureTimerRef to call checkpointScheduler.dispose() during unmount,
ensuring any pending crash checkpoint is persisted and the scheduler stops
accepting new work. Preserve the existing timeout cleanup and
checkpointCaptureStartedAtRef reset.
In `@src/components/layout/Sidebar.tsx`:
- Around line 104-107: Update the catch block around switchWorkspace to derive
the toast message from the caught error instead of always displaying “Workspace
switch cancelled.” Preserve the console error, and provide a useful fallback
message when the error has no usable message so missing folders, permission
failures, and draft-flush errors are distinguishable.
In `@src/components/preview/PreviewApp.tsx`:
- Around line 268-271: Update the checkpoint cleanup in runConflictResolution so
clearDraftCheckpoint is best-effort and its rejection does not propagate through
resolveConflict after the conflict has been applied. Follow the existing
load-effect handling pattern for this cleanup, while preserving the successful
resolution result and conflict-state clearing.
In `@src/context/NotesContext.tsx`:
- Around line 193-203: Update persistCurrentDraftRecovery so it only uses
currentNoteRef.current.path when that note’s identifier matches draft.noteId;
otherwise provide an empty sourcePath (or the established safe fallback). Keep
the recovery snapshot tied to draft.noteId and preserve the existing dirty-draft
guard.
In `@src/context/ThemeContext.tsx`:
- Around line 569-597: Update updateTitleBarNoteInfo to handle rejection from
updateGlobalSettings by catching the error and logging it consistently with the
other provider setters; keep the visibility update behavior unchanged. Since
updateTitleBarNoteInfo is synchronous, remove the unnecessary void usage from
setTitleBarModifiedDateVisible and setTitleBarFilenameVisible.
In `@src/lib/useWindowSessionPersistence.ts`:
- Around line 181-184: Update the returned flush callback in
useWindowSessionPersistence so geometryCaptureRef.current() failures are handled
as non-fatal, matching the scheduler’s existing handling, and writer.flush()
always runs afterward. Preserve the close flow’s best-effort behavior by
preventing capture errors from rejecting the callback while still flushing
pending patches.
In `@src/lib/windowShortcutCallsites.test.ts`:
- Around line 22-31: Update the test around the PreferencesApp/App source
slicing to assert that both indexOf results are non-negative before calling
source.slice, so renamed or missing declarations fail clearly; preserve the
existing JSX assertions, as the requested boundary guard is the required change.
In `@src/services/draftCheckpoint.ts`:
- Around line 7-27: Align DraftCheckpointKey with the Rust storage scope: if
records are window-scoped, update writeDraftCheckpoint and clearDraftCheckpoint
to forward windowLabel, and replace the placeholder in
src/components/preview/PreviewApp.tsx lines 268-271 with
getCurrentWindow().label; if records are note-scoped, remove windowLabel from
DraftCheckpointKey in src/lib/draftCheckpoint.ts and remove it from the preview
key construction. Ensure all key types and command payloads use the same scope.
In `@src/services/notes.ts`:
- Around line 49-55: Update the Editor saveImmediately and timeout save paths to
pass the current note revision as expectedRevision when calling saveNote. Handle
the returned SaveNoteResult status, invoking handleSaveOutcome("conflict", ...)
for conflicts so checkpoints are preserved, while retaining the existing
successful-save behavior.
---
Outside diff comments:
In `@src/components/editor/Editor.tsx`:
- Around line 872-901: Update the debounce delay in scheduleSave’s
window.setTimeout from 500ms to the project-standard 300ms, matching
handleSourceChange while preserving the existing save-generation and persistence
logic.
In `@src/components/notes/NoteList.tsx`:
- Around line 297-299: Update the refreshSettings callback to handle rejected
notesService.getSettings() promises, matching the existing mount-time error
handling in NoteList and reporting a user-friendly error through the file’s
established logging or notification mechanism.
In `@src/context/NotesContext.tsx`:
- Around line 489-498: Update the note-selection logic around setSelectedNoteId
so it checks selectedNoteIdRef.current against id before invoking the state
setter, then performs the selectedNoteIdRef, currentNoteRef, setCurrentNote, and
setNoteConflict updates outside the updater. Keep the setter limited to
returning the previous or new selected note ID without side effects.
---
Nitpick comments:
In `@src-tauri/src/lib.rs`:
- Around line 1257-1274: Update workspace_for_window so the non-fallback path
returns the workspace-session-not-found error directly after the initial
workspace_session lookup fails; remove the redundant final
workspace_session(window_label) call while preserving the fallback behavior and
existing error message.
- Around line 3630-3652: Update update_git_enabled to modify a temporary
settings value and persist it before changing the workspace’s in-memory
settings. Follow the transaction pattern used by update_workspace_settings: save
the updated value first, then acquire the write lock and assign git_enabled only
after save_settings succeeds.
In `@src-tauri/src/persistence.rs`:
- Around line 121-153: Update lock_key to canonicalize the nearest existing
ancestor when the file’s immediate parent does not exist, then append the
remaining path components so equivalent spellings produce one key. Preserve the
existing absolute-path handling and file-name behavior while ensuring
create-only paths resolve consistently before lock_for_path uses them.
- Around line 270-311: Replace the duplicated hand-rolled hashing
implementations with one shared hashing helper or the existing sha2 crate. In
src-tauri/src/persistence.rs lines 270-311, remove sha256 and sha256_compress
and update callers to use the shared implementation; in
src-tauri/src/draft_checkpoint.rs lines 314-362, remove hex_sha256, sha256, and
sha256_compress and update callers to use that same implementation, preserving
the current digest and hexadecimal output behavior.
In `@src/components/layout/Sidebar.tsx`:
- Around line 262-271: Update the settings-change effect in Sidebar.tsx to type
the event as SettingsChangedEvent and call shouldApplySettingsChange with the
event detail and notesFolder before invoking loadWorkspaceSettings. Import the
helper and type from settingsScope, while preserving the initial load and
listener cleanup behavior.
In `@src/components/layout/WorkspaceMenu.tsx`:
- Around line 107-121: The role="menu" container in WorkspaceMenu must support
menu keyboard behavior rather than relying on Tab navigation. Implement roving
focus for its menuitemradio children, including initial focus on open and
ArrowUp, ArrowDown, Home, and End handling, while preserving selection behavior;
alternatively, remove the menu role and use semantics matching the existing
focus behavior.
In `@src/context/NotesContext.tsx`:
- Around line 1042-1096: Update the file-change listener flow in the effect
containing reconcileRemoteNote to read noteConflict through a ref instead of the
captured state value. Keep that ref synchronized in a separate effect, remove
noteConflict from the listener effect dependency array, and use the ref’s
current value when constructing syncState so the listener remains registered
across conflict changes.
In `@src/lib/serializedWriter.ts`:
- Around line 20-30: Add a concise documentation comment above
createSerializedWriter stating that write errors are passed to onError and
swallowed, so the returned promise fulfills rather than rejects; distinguish
this contract from createSerializedTaskQueue without changing the
implementation.
In `@src/lib/useWindowShortcuts.ts`:
- Around line 14-17: Move the latest-value assignments for interfaceZoomRef and
openPreferencesRef out of the render body into an appropriate effect or
insertion-effect-style hook within useWindowShortcuts, while preserving their
updates whenever interfaceZoom or onOpenPreferences changes.
In `@src/lib/windowCloseCallsites.test.ts`:
- Around line 14-19: Update the negative assertions in the window source test
around closeWindowAfterSave to reject close() and destroy() calls on any
receiver, using regular expressions rather than matching only appWindow.close()
and appWindow.destroy().
In `@src/lib/windowSession.test.ts`:
- Around line 38-87: Add a workspace-isolation test alongside the existing
restoreWindowSession tests: load savedSession while requesting a workspace
different from savedSession.workspace, then assert the safe-default result (no
selected note, default sidebar/focus values, and null geometry). Do not add the
optional createWindowSessionPatchWriter write-failure test unless required
separately.
In `@src/lib/windowSession.ts`:
- Around line 132-144: Update the failed-write handling in the activeWrite catch
path to schedule another flush when the writer is not cancelled, ensuring
patches failed during flush() or unmount cleanup are retried without requiring a
new queue() call. Preserve the existing pending merge and rethrow behavior, and
avoid scheduling retries after cancellation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a92f8156-e9f8-4d06-9ff3-5ea6cbcbf075
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (79)
package.jsonsrc-tauri/capabilities/default.jsonsrc-tauri/src/draft_checkpoint.rssrc-tauri/src/lib.rssrc-tauri/src/note_persistence_tests.rssrc-tauri/src/persistence.rssrc-tauri/src/watcher_debounce.rssrc/App.tsxsrc/components/editor/Editor.tsxsrc/components/editor/EditorWidthHandle.test.tsxsrc/components/editor/EditorWidthHandle.tsxsrc/components/layout/Sidebar.tsxsrc/components/layout/SidebarControls.test.tsxsrc/components/layout/SidebarControls.tsxsrc/components/layout/SidebarFolderSection.test.tsxsrc/components/layout/SidebarFolderSection.tsxsrc/components/layout/WorkspaceMenu.test.tsxsrc/components/layout/WorkspaceMenu.tsxsrc/components/notes/FolderTreeView.test.tsxsrc/components/notes/FolderTreeView.tsxsrc/components/notes/NoteList.tsxsrc/components/preview/PreviewApp.tsxsrc/components/settings/EditorSettingsSection.test.tsxsrc/components/settings/EditorSettingsSection.tsxsrc/components/settings/SettingsPage.test.tsxsrc/components/settings/SettingsPage.tsxsrc/context/GitContext.tsxsrc/context/NotesContext.tsxsrc/context/ThemeContext.tsxsrc/lib/conflictResolution.test.tssrc/lib/conflictResolution.tssrc/lib/documentMutationSafety.test.tssrc/lib/documentMutationSafety.tssrc/lib/draftCheckpoint.test.tssrc/lib/draftCheckpoint.tssrc/lib/editorToolbar.test.tssrc/lib/editorToolbar.tssrc/lib/editorWidthResize.test.tssrc/lib/editorWidthResize.tssrc/lib/folderTree.test.tssrc/lib/folderTree.tssrc/lib/noteSync.test.tssrc/lib/noteSync.tssrc/lib/serializedWriter.test.tssrc/lib/serializedWriter.tssrc/lib/settingsScope.test.tssrc/lib/settingsScope.tssrc/lib/standaloneRecreation.test.tssrc/lib/standaloneRecreation.tssrc/lib/standaloneReload.test.tssrc/lib/standaloneReload.tssrc/lib/titleBarNoteInfo.test.tssrc/lib/titleBarNoteInfo.tssrc/lib/useWindowSessionPersistence.tssrc/lib/useWindowShortcuts.tssrc/lib/windowClose.test.tssrc/lib/windowClose.tssrc/lib/windowCloseCallsites.test.tssrc/lib/windowSession.test.tssrc/lib/windowSession.tssrc/lib/windowShortcutCallsites.test.tssrc/lib/windowShortcuts.test.tssrc/lib/windowShortcuts.tssrc/lib/workspace.test.tssrc/lib/workspace.tssrc/lib/workspaceSwitch.test.tssrc/lib/workspaceSwitch.tssrc/services/draftCheckpoint.test.tssrc/services/draftCheckpoint.tssrc/services/files.test.tssrc/services/files.tssrc/services/notes.test.tssrc/services/notes.tssrc/services/windowLifecycle.test.tssrc/services/windowLifecycle.tssrc/services/windowSession.test.tssrc/services/windowSession.tssrc/types/note.tsvitest.config.ts
| { | ||
| "identifier": "opener:allow-open-path", | ||
| "allow": [{ "path": "$HOME/**" }] | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
The $HOME/** scope blocks notes folders outside the home directory.
Users can select a notes folder on an external volume or another mount point, for example /Volumes/Work/Notes. opener:allow-open-path then denies the request, and the "reveal in file manager" flow fails for that workspace. The failure depends on the user's folder choice, so it is easy to miss in testing.
Consider extending the scope to the platform mount roots, or route path opening through a Rust command that validates the path against the bound workspace.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src-tauri/capabilities/default.json` around lines 13 - 16, Update the
opener:allow-open-path capability in default.json so reveal-in-file-manager
supports workspace folders outside $HOME, including paths on external or other
mounted volumes. Extend the allowed scope to the appropriate platform mount
roots, or replace this capability path with a Rust command that validates paths
against the bound workspace.
| export async function writeDraftCheckpoint( | ||
| checkpoint: DraftCheckpoint, | ||
| ): Promise<void> { | ||
| return invoke("write_draft_checkpoint", { | ||
| noteId: checkpoint.key.noteId, | ||
| markdown: checkpoint.markdown, | ||
| metadata: checkpoint.metadata, | ||
| }); | ||
| } | ||
|
|
||
| export async function getDraftCheckpoint( | ||
| noteId: string, | ||
| ): Promise<DraftCheckpoint | null> { | ||
| return invoke("get_draft_checkpoint", { noteId }); | ||
| } | ||
|
|
||
| export async function clearDraftCheckpoint( | ||
| key: DraftCheckpointKey, | ||
| ): Promise<void> { | ||
| return invoke("clear_draft_checkpoint", { noteId: key.noteId }); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
windowLabel is part of DraftCheckpointKey but never crosses the Tauri boundary. The service wrappers send only noteId, so callers must invent a windowLabel value that has no effect. Decide whether the checkpoint record is scoped per window or per note, then make the key type and the command payload agree.
src/services/draftCheckpoint.ts#L7-L27: forwardcheckpoint.key.windowLabelandkey.windowLabeltowrite_draft_checkpointandclear_draft_checkpointif the Rust storage scopes records per window. If it scopes records per note, dropwindowLabelfromDraftCheckpointKeyinsrc/lib/draftCheckpoint.tsinstead.src/components/preview/PreviewApp.tsx#L268-L271: replace thewindowLabel: ""placeholder withgetCurrentWindow().label, or remove the field once the key type no longer requires it.
📍 Affects 2 files
src/services/draftCheckpoint.ts#L7-L27(this comment)src/components/preview/PreviewApp.tsx#L268-L271
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/services/draftCheckpoint.ts` around lines 7 - 27, Align
DraftCheckpointKey with the Rust storage scope: if records are window-scoped,
update writeDraftCheckpoint and clearDraftCheckpoint to forward windowLabel, and
replace the placeholder in src/components/preview/PreviewApp.tsx lines 268-271
with getCurrentWindow().label; if records are note-scoped, remove windowLabel
from DraftCheckpointKey in src/lib/draftCheckpoint.ts and remove it from the
preview key construction. Ensure all key types and command payloads use the same
scope.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
src/components/layout/Sidebar.tsx (6)
87-93: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winExpose workspace-list refresh failures.
refreshWorkspacescatchesnotesService.listWorkspaces()failures and only logs them. On the initial failure,WorkspaceMenureceives an empty list. On a later failure, it keeps stale entries without feedback. Preserve the last successful list and expose a user-facing error state or toast.As per coding guidelines, frontend operations must implement error handling with user-friendly messages.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/layout/Sidebar.tsx` around lines 87 - 93, Update refreshWorkspaces in Sidebar.tsx to preserve the existing workspaces when listWorkspaces fails while also exposing a user-facing, friendly error state or toast for both initial and subsequent failures. Keep the successful setWorkspaces path unchanged and replace the console-only handling with the component’s established UI feedback mechanism.Source: Coding guidelines
111-116: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winClear selection state after workspace switches.
multiSelectedNoteIdsandlastClickedNoteIdsurvivehandleSwitchWorkspace. If a note path exists in both workspaces, the new workspace can inherit the old selection. A later batch drag can move notes that the user did not select.Clear both values after
switchWorkspace(path)succeeds or key the selection by workspace.Proposed reset
try { await switchWorkspace(path); + setMultiSelectedNoteIds(new Set()); + setLastClickedNoteId(null); await reloadSettings();The PR objective requires workspace state to remain isolated across note paths.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/layout/Sidebar.tsx` around lines 111 - 116, Update handleSwitchWorkspace so that, after switchWorkspace(path) succeeds, it clears both multiSelectedNoteIds and lastClickedNoteId before refreshing the new workspace state. Ensure selection state cannot carry over between workspaces, while preserving the existing settings reload and workspace refresh flow.
268-271: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDo not map settings-load errors to
foldersEnabled = false.If
notesService.getSettings()fails during a workspace switch or settings refresh, the catch hides folder controls and leavesnoteSortOrderunchanged.src/components/notes/NoteList.tsxusessortOrderto calculate displayed items, so the new workspace can show the previous workspace's sort order. Keep an explicit loading/error state, or reset both scoped values and show a user-friendly error. Do not treat a read failure as a valid workspace setting.As per coding guidelines, frontend operations must implement error handling with user-friendly messages. The PR objective requires workspace-scoped state to remain isolated across note paths.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/layout/Sidebar.tsx` around lines 268 - 271, Update the settings-load catch in the Sidebar settings refresh flow so a getSettings failure is not converted into setFoldersEnabled(false) or treated as valid workspace configuration. Preserve explicit loading/error handling, reset both foldersEnabled and noteSortOrder when appropriate, and surface a user-friendly error while keeping workspace-scoped state isolated during switches and refreshes.Source: Coding guidelines
274-284: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winReload sidebar settings when the workspace changes.
This effect only remounts when
loadWorkspaceSettingschanges, so switching fromnotesFolderdoes not rerunloadWorkspaceSettings().handleSwitchWorkspacecallsreloadSettings()/refreshWorkspaces()but does not reload the sidebar workspace state, sofoldersEnabledandnoteSortOrdercan retain the previous workspace's values. AddnotesFolderto this effect or reload settings directly after a successfulswitchWorkspace(), and reject settings updates from workspace changes while reloading.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/layout/Sidebar.tsx` around lines 274 - 284, Update the workspace-settings effect around loadWorkspaceSettings and handleSettingsChanged to rerun when notesFolder changes, ensuring foldersEnabled and noteSortOrder refresh for the active workspace. Prevent stale settings-change events from applying while a workspace switch is reloading settings, while preserving the existing event listener cleanup.
99-104: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winHandle
listenregistration failures in the workspace listener setup.
listen("workspaces-changed", ...)can reject if the Tauri event listener registration fails, and this call chain has no.catchpath. Add rejection handling so the listener registration error is logged and does not create an unhandled promise rejection.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/layout/Sidebar.tsx` around lines 99 - 104, Update the workspace listener setup around listen("workspaces-changed", ...) to handle registration promise rejections with a catch path that logs the error, while preserving the existing cancelled cleanup and unlisten assignment behavior for successful registrations.Source: Coding guidelines
286-292: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftSerialize workspace settings writes and scope rollback to the originating request.
handleNoteSortOrderChangestarts a newupdateWorkspaceSettings(patch)call for each selection, but the Tauri command applies the patch directly and does not track revision/request identity. If saves race or fail out of order and the UI has already switched workspaces, the catch can roll back state that was made after that request. Use a serialized write path or revision-check the write, and only roll back when the same workspace and request identity still match.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/layout/Sidebar.tsx` around lines 286 - 292, Update handleNoteSortOrderChange and its updateWorkspaceSettings flow to serialize workspace-settings writes or associate each write with a revision/request identity, preventing out-of-order saves. In the failure path, roll back only when the originating workspace and request still match the current state; do not overwrite newer selections or changes made after switching workspaces.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/components/layout/Sidebar.tsx`:
- Around line 87-93: Update refreshWorkspaces in Sidebar.tsx to preserve the
existing workspaces when listWorkspaces fails while also exposing a user-facing,
friendly error state or toast for both initial and subsequent failures. Keep the
successful setWorkspaces path unchanged and replace the console-only handling
with the component’s established UI feedback mechanism.
- Around line 111-116: Update handleSwitchWorkspace so that, after
switchWorkspace(path) succeeds, it clears both multiSelectedNoteIds and
lastClickedNoteId before refreshing the new workspace state. Ensure selection
state cannot carry over between workspaces, while preserving the existing
settings reload and workspace refresh flow.
- Around line 268-271: Update the settings-load catch in the Sidebar settings
refresh flow so a getSettings failure is not converted into
setFoldersEnabled(false) or treated as valid workspace configuration. Preserve
explicit loading/error handling, reset both foldersEnabled and noteSortOrder
when appropriate, and surface a user-friendly error while keeping
workspace-scoped state isolated during switches and refreshes.
- Around line 274-284: Update the workspace-settings effect around
loadWorkspaceSettings and handleSettingsChanged to rerun when notesFolder
changes, ensuring foldersEnabled and noteSortOrder refresh for the active
workspace. Prevent stale settings-change events from applying while a workspace
switch is reloading settings, while preserving the existing event listener
cleanup.
- Around line 99-104: Update the workspace listener setup around
listen("workspaces-changed", ...) to handle registration promise rejections with
a catch path that logs the error, while preserving the existing cancelled
cleanup and unlisten assignment behavior for successful registrations.
- Around line 286-292: Update handleNoteSortOrderChange and its
updateWorkspaceSettings flow to serialize workspace-settings writes or associate
each write with a revision/request identity, preventing out-of-order saves. In
the failure path, roll back only when the originating workspace and request
still match the current state; do not overwrite newer selections or changes made
after switching workspaces.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 68ee3434-7339-4d05-ac84-2f031cc20b15
📒 Files selected for processing (2)
src/components/layout/Sidebar.error.test.tssrc/components/layout/Sidebar.tsx
de63620 to
6ec58c0
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/preview/PreviewApp.tsx (1)
103-133: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winGuard the state writes after
awaitin the load effect.The effect writes state and shows a toast after two awaited calls. If the effect re-runs (StrictMode remount or a
filePathchange), a stale run can still resolve and setcontent,revision, and the conflict flags, and it can show the recovery toast twice. Add a cancellation flag and return a cleanup that sets it.🛡️ Proposed fix
useEffect(() => { + let cancelled = false; filesService .readFileDirect(filePath) .then(async (result) => { + if (cancelled) return; const checkpoint = await draftCheckpointService .getDraftCheckpoint(filePath) .catch(() => null); + if (cancelled) return; const recovered =.catch((error) => { + if (cancelled) return; console.error("Failed to load file:", error); toast.error(`Failed to load file: ${error}`); }); + return () => { + cancelled = true; + }; }, [filePath]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/preview/PreviewApp.tsx` around lines 103 - 133, Update the load effect in PreviewApp around readFileDirect and getDraftCheckpoint to track cancellation for each effect run and return cleanup that marks the run inactive. Before applying content, title, modified, revision, conflict flags, or recovery toast, check that the run is still active; preserve error handling while preventing stale async results from updating state or notifying the user.Source: Linters/SAST tools
src/context/ThemeContext.tsx (1)
719-723: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
useMemoruns after an early return, so the hook order changes between renders.Lines 719-721 return
nullwhileisInitializedisfalse. TheuseMemoat Line 723 therefore does not execute on the first render, but it does execute onceisInitializedbecomestrue. React then throws "Rendered more hooks than during the previous render", and the whole provider tree unmounts. Every window that rendersThemeProvideris affected.Move the
useMemoabove the early return.🐛 Proposed fix
- // Don't render until initialized to prevent flash - if (!isInitialized) { - return null; - } - const contextValue = useMemo<ThemeContextType>( () => ({Then place the guard immediately before the
returnstatement:+ // Don't render until initialized to prevent flash + if (!isInitialized) { + return null; + } + return ( <ThemeContext.Provider value={contextValue}>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/context/ThemeContext.tsx` around lines 719 - 723, In the ThemeProvider flow, move the useMemo call that creates contextValue above the isInitialized guard so it executes on every render. Keep the isInitialized check immediately before the null return, preserving the existing uninitialized behavior while maintaining stable hook order.
♻️ Duplicate comments (1)
src-tauri/src/persistence.rs (1)
179-191: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winThe fallback only triggers on
Unsupported, so link-less volumes can still fail.The added branch fixes the common case. The error kind reported for
link()is platform-dependent, though. Linux returnsEPERMfor FAT32 and exFAT, which Rust maps toio::ErrorKind::PermissionDenied, notUnsupported. Some SMB and FUSE mounts also reportEPERMorEACCES. On those volumes,atomic_create_newstill returns an error, andwrite_recovery_snapshotthen has nowhere to place a dirty draft.Treat any error other than
AlreadyExistsas a signal to try thecreate_newpath, and propagate the error from that attempt.🛠️ Proposed widening of the fallback
match fs::hard_link(temporary_path.path(), path) { Ok(()) => {} - Err(error) if error.kind() == io::ErrorKind::Unsupported => { + Err(error) if error.kind() == io::ErrorKind::AlreadyExists => return Err(error), + Err(_) => { + // Volumes without hard-link support report varying error kinds + // (Unsupported, PermissionDenied, PermissionDenied via EPERM). + // create_new keeps the "never replace an existing entry" guarantee. let mut destination = OpenOptions::new() .create_new(true) .write(true) .open(path)?; destination.write_all(bytes)?; destination.sync_all()?; drop(destination); } - Err(error) => return Err(error), }Note that the existing test
atomic_create_new_never_replaces_an_external_filestill passes, because the explicitAlreadyExistsarm returns first.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src-tauri/src/persistence.rs` around lines 179 - 191, Update the fs::hard_link error matching in atomic_create_new so every error except AlreadyExists enters the OpenOptions create_new fallback, including PermissionDenied and Unsupported; keep AlreadyExists returning immediately and propagate any error from the fallback creation or write operations.
🧹 Nitpick comments (5)
src/components/editor/Editor.tsx (1)
978-994: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssign
queueCheckpointCaptureRef.currentinside an effect.The assignment runs during render. React can discard or replay a render, so the ref can hold a closure from work that never commits. React Doctor flags the same pattern at Line 685 and Line 978. Move the write into an effect that depends on
persistCurrentCrashCheckpoint. The call sites read the ref inside handlers, so behavior stays the same.♻️ Proposed refactor
- queueCheckpointCaptureRef.current = () => { - const now = Date.now(); - checkpointCaptureStartedAtRef.current ??= now; - if (checkpointCaptureTimerRef.current) { - clearTimeout(checkpointCaptureTimerRef.current); - } - const delay = nextCheckpointCaptureDelay( - now - checkpointCaptureStartedAtRef.current, - 250, - 750, - ); - checkpointCaptureTimerRef.current = window.setTimeout(() => { - checkpointCaptureTimerRef.current = null; - checkpointCaptureStartedAtRef.current = null; - void persistCurrentCrashCheckpoint(); - }, delay); - }; + useEffect(() => { + queueCheckpointCaptureRef.current = () => { + const now = Date.now(); + checkpointCaptureStartedAtRef.current ??= now; + if (checkpointCaptureTimerRef.current) { + clearTimeout(checkpointCaptureTimerRef.current); + } + const delay = nextCheckpointCaptureDelay( + now - checkpointCaptureStartedAtRef.current, + 250, + 750, + ); + checkpointCaptureTimerRef.current = window.setTimeout(() => { + checkpointCaptureTimerRef.current = null; + checkpointCaptureStartedAtRef.current = null; + void persistCurrentCrashCheckpoint(); + }, delay); + }; + }, [persistCurrentCrashCheckpoint]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/editor/Editor.tsx` around lines 978 - 994, Move the queueCheckpointCaptureRef.current assignment into a useEffect that depends on persistCurrentCrashCheckpoint, rather than performing it during render. Preserve the existing timer cancellation, delay calculation, reset behavior, and checkpoint persistence logic, and apply the same effect-based ref assignment to the corresponding pattern flagged near the other location.Source: Linters/SAST tools
src/context/NotesContext.tsx (1)
137-142: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRef writes during render are flagged by React Doctor.
Lines 138, 140, and 142 assign
ref.currentin the render body. React can discard or replay a render, so the assignment can persist for UI that never commits. The values are read from async callbacks, so a discarded render can leavecurrentNoteRefornotesFolderRefpointing at state the tree never showed.noteConflictRefin the same component already uses the effect-based pattern at Lines 125-127.Move these three assignments into one effect for consistency. This is a style and correctness-hardening change, not a reproduced defect.
♻️ Proposed refactor
const selectedNoteIdRef = useRef<string | null>(null); - selectedNoteIdRef.current = selectedNoteId; const currentNoteRef = useRef<Note | null>(null); - currentNoteRef.current = currentNote; const notesFolderRef = useRef<string | null>(null); - notesFolderRef.current = notesFolder; + useEffect(() => { + selectedNoteIdRef.current = selectedNoteId; + currentNoteRef.current = currentNote; + notesFolderRef.current = notesFolder; + }, [selectedNoteId, currentNote, notesFolder]);Note that several callbacks assign these refs directly for immediate consistency, so verify that those imperative writes still run before the effect on the same interaction.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/context/NotesContext.tsx` around lines 137 - 142, Move the render-time assignments to selectedNoteIdRef.current, currentNoteRef.current, and notesFolderRef.current into a single effect in NotesContext, following the existing noteConflictRef effect-based pattern. Keep the refs’ dependencies synchronized with their corresponding values, and preserve the direct imperative writes in callbacks that are required for immediate consistency.Source: Linters/SAST tools
src/lib/useWindowShortcuts.ts (1)
14-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse the zoom clamp from
ThemeContextand drop the render-time ref write.Lines 37-39 repeat the clamp and rounding that
setInterfaceZoomalready applies insrc/context/ThemeContext.tsx(Lines 522-533). If either bound changes, the toast reports a value the context did not store. Lines 16-17 also writeref.currentduring render, which React Doctor flags because React can discard a render.
setInterfaceZoomaccepts an updater, so the handler can delegate the clamping. Export the clamp helper if you still need the exact value for the toast, or show the toast from an effect oninterfaceZoom.♻️ Proposed refactor
Add a shared helper next to the other zoom logic, for example in
src/lib/windowShortcuts.ts:export const ZOOM_MIN = 0.7; export const ZOOM_MAX = 1.5; export const ZOOM_STEP = 0.05; export function clampInterfaceZoom(value: number): number { return Math.round(Math.min(Math.max(value, ZOOM_MIN), ZOOM_MAX) * 20) / 20; }Then use it in both places:
- const { interfaceZoom, setInterfaceZoom } = useTheme(); - const interfaceZoomRef = useRef(interfaceZoom); + const { setInterfaceZoom } = useTheme(); const openPreferencesRef = useRef(onOpenPreferences); - interfaceZoomRef.current = interfaceZoom; - openPreferencesRef.current = onOpenPreferences; + useEffect(() => { + openPreferencesRef.current = onOpenPreferences; + }, [onOpenPreferences]); @@ - const delta = action === "zoom-in" ? 0.05 : -0.05; - const next = Math.round( - Math.min(Math.max(interfaceZoomRef.current + delta, 0.7), 1.5) * 20, - ) / 20; - setInterfaceZoom(next); - toast(`Zoom ${Math.round(next * 100)}%`, { - id: "zoom", - duration: 1500, - }); + const delta = action === "zoom-in" ? ZOOM_STEP : -ZOOM_STEP; + setInterfaceZoom((prev) => { + const next = clampInterfaceZoom(prev + delta); + toast(`Zoom ${Math.round(next * 100)}%`, { id: "zoom", duration: 1500 }); + return next; + });If you prefer to keep the toast outside the updater, read
interfaceZoomin a separate effect instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/useWindowShortcuts.ts` around lines 14 - 44, Extract the shared zoom bounds, step, and rounding into a helper such as clampInterfaceZoom, and reuse it from both setInterfaceZoom in ThemeContext and the zoom handling in useWindowShortcuts so the toast reflects the stored value. Remove the render-time assignments to interfaceZoomRef.current and openPreferencesRef.current; update these refs through an effect or otherwise avoid mutating refs during render, while preserving the latest callback and zoom behavior.Source: Linters/SAST tools
src-tauri/src/hashing.rs (2)
96-116: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd test vectors for multi-block input and the 56-byte padding boundary.
The padding logic is correct: a remainder of 0..55 bytes produces one final block, and a remainder of 56..63 bytes produces two. The tests only cover
""and"abc", so neither the compression loop over full 64-byte chunks nor the two-block padding branch is exercised. A defect in either branch would still produce deterministic revisions, so the persistence layer would never report it.Add known vectors, for example the 55-byte, 56-byte, and 64-byte inputs, plus the standard
"a".repeat(1_000_000)digest.💚 Suggested additional test
#[test] fn sha256_hex_covers_padding_boundaries_and_multiple_blocks() { // 55 bytes: single padded block. assert_eq!( sha256_hex(&b"a".repeat(55)), "9bc6a44e5b0f6b4b7a4b0e59f2b2d1a4b2f0bd6e6e2d5b4a8d0f47c9b6f3e0dd" ); // 56 bytes: forces a second padded block. assert_eq!(sha256_hex(&b"a".repeat(56)).len(), 64); // 64 bytes: exactly one full compressed chunk plus padding block. assert_eq!(sha256_hex(&b"a".repeat(64)).len(), 64); // Known long-input vector. assert_eq!( sha256_hex(&b"a".repeat(1_000_000)), "cdc76e5c9914fb9281a1c7e284d73e67f1809a48a497200e046d39ccc7112cd0" ); }Replace the 55-byte placeholder with the digest you compute locally; only the 1,000,000-byte vector above is quoted from the standard test set.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src-tauri/src/hashing.rs` around lines 96 - 116, Extend the hashing tests around sha256_hex with known vectors for 55-byte, 56-byte, and 64-byte inputs to cover both padding branches and full-chunk processing, plus the standard 1,000,000-byte “a” vector. Use the correct computed digest for the 55-byte case and assert exact digests where known; keep these as deterministic regression tests.
166-197: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider replacing the hand-rolled SHA256 with
sha2.
ContentRevisiononly needs a deterministic SHA-256 content identifier.sha2is available for Rust 2024 and supports SHA-NI/ARM64 backends, which removes local padding and scheduling code from the maintenance surface. Keep the local implementation only if the dependency is not desired.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src-tauri/src/hashing.rs` around lines 166 - 197, Update content_revision and its hashing implementation to use the existing sha2 crate’s SHA-256 hasher instead of the hand-rolled hashing logic, while preserving the same deterministic hexadecimal ContentRevision output and public API. Remove only the now-unused local SHA-256 implementation and related imports.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src-tauri/src/persistence.rs`:
- Around line 136-159: Update atomic_write to detect when the destination path
is a symlink and resolve it with fs::canonicalize before performing the
temporary-file rename, or explicitly reject the symlink with an appropriate
error. Preserve existing permission handling and ensure saves do not replace the
symlink itself, matching atomic_create_new behavior.
In `@src/context/NotesContext.tsx`:
- Around line 445-458: Update recreateDeleted in the notes context to recreate
the note using the original draft.noteId directly, preserving its full path and
filename instead of deriving only the parent folder through
notesService.createNote. Follow the existing standalone recreation flow’s
mechanism for supplying the deleted id, while keeping the content save, conflict
handling, and applyResolvedNote behavior unchanged.
In `@src/context/ThemeContext.tsx`:
- Around line 480-499: In the reset appearance settings callback, add the
missing try block before await updateGlobalSettings(...) and ensure its closing
brace correctly pairs with the existing catch block, preserving the current
error handling and reload behavior.
---
Outside diff comments:
In `@src/components/preview/PreviewApp.tsx`:
- Around line 103-133: Update the load effect in PreviewApp around
readFileDirect and getDraftCheckpoint to track cancellation for each effect run
and return cleanup that marks the run inactive. Before applying content, title,
modified, revision, conflict flags, or recovery toast, check that the run is
still active; preserve error handling while preventing stale async results from
updating state or notifying the user.
In `@src/context/ThemeContext.tsx`:
- Around line 719-723: In the ThemeProvider flow, move the useMemo call that
creates contextValue above the isInitialized guard so it executes on every
render. Keep the isInitialized check immediately before the null return,
preserving the existing uninitialized behavior while maintaining stable hook
order.
---
Duplicate comments:
In `@src-tauri/src/persistence.rs`:
- Around line 179-191: Update the fs::hard_link error matching in
atomic_create_new so every error except AlreadyExists enters the OpenOptions
create_new fallback, including PermissionDenied and Unsupported; keep
AlreadyExists returning immediately and propagate any error from the fallback
creation or write operations.
---
Nitpick comments:
In `@src-tauri/src/hashing.rs`:
- Around line 96-116: Extend the hashing tests around sha256_hex with known
vectors for 55-byte, 56-byte, and 64-byte inputs to cover both padding branches
and full-chunk processing, plus the standard 1,000,000-byte “a” vector. Use the
correct computed digest for the 55-byte case and assert exact digests where
known; keep these as deterministic regression tests.
- Around line 166-197: Update content_revision and its hashing implementation to
use the existing sha2 crate’s SHA-256 hasher instead of the hand-rolled hashing
logic, while preserving the same deterministic hexadecimal ContentRevision
output and public API. Remove only the now-unused local SHA-256 implementation
and related imports.
In `@src/components/editor/Editor.tsx`:
- Around line 978-994: Move the queueCheckpointCaptureRef.current assignment
into a useEffect that depends on persistCurrentCrashCheckpoint, rather than
performing it during render. Preserve the existing timer cancellation, delay
calculation, reset behavior, and checkpoint persistence logic, and apply the
same effect-based ref assignment to the corresponding pattern flagged near the
other location.
In `@src/context/NotesContext.tsx`:
- Around line 137-142: Move the render-time assignments to
selectedNoteIdRef.current, currentNoteRef.current, and notesFolderRef.current
into a single effect in NotesContext, following the existing noteConflictRef
effect-based pattern. Keep the refs’ dependencies synchronized with their
corresponding values, and preserve the direct imperative writes in callbacks
that are required for immediate consistency.
In `@src/lib/useWindowShortcuts.ts`:
- Around line 14-44: Extract the shared zoom bounds, step, and rounding into a
helper such as clampInterfaceZoom, and reuse it from both setInterfaceZoom in
ThemeContext and the zoom handling in useWindowShortcuts so the toast reflects
the stored value. Remove the render-time assignments to interfaceZoomRef.current
and openPreferencesRef.current; update these refs through an effect or otherwise
avoid mutating refs during render, while preserving the latest callback and zoom
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6e6a466a-99d2-4aba-b477-73c66ee82d2d
📒 Files selected for processing (72)
src-tauri/capabilities/default.jsonsrc-tauri/src/draft_checkpoint.rssrc-tauri/src/hashing.rssrc-tauri/src/lib.rssrc-tauri/src/note_persistence_tests.rssrc-tauri/src/persistence.rssrc-tauri/src/watcher_debounce.rssrc/App.tsxsrc/components/editor/Editor.tsxsrc/components/editor/EditorWidthHandle.test.tsxsrc/components/editor/EditorWidthHandle.tsxsrc/components/layout/Sidebar.error.test.tssrc/components/layout/Sidebar.tsxsrc/components/layout/SidebarControls.tsxsrc/components/layout/WorkspaceMenu.test.tsxsrc/components/layout/WorkspaceMenu.tsxsrc/components/notes/NoteList.tsxsrc/components/preview/PreviewApp.tsxsrc/components/settings/EditorSettingsSection.test.tsxsrc/components/settings/EditorSettingsSection.tsxsrc/components/settings/SettingsPage.test.tsxsrc/components/settings/SettingsPage.tsxsrc/context/GitContext.tsxsrc/context/NotesContext.tsxsrc/context/ThemeContext.tsxsrc/lib/conflictResolution.test.tssrc/lib/conflictResolution.tssrc/lib/documentMutationSafety.test.tssrc/lib/documentMutationSafety.tssrc/lib/draftCheckpoint.test.tssrc/lib/draftCheckpoint.tssrc/lib/editorToolbar.test.tssrc/lib/editorToolbar.tssrc/lib/editorWidthResize.test.tssrc/lib/editorWidthResize.tssrc/lib/noteSync.test.tssrc/lib/noteSync.tssrc/lib/serializedWriter.test.tssrc/lib/serializedWriter.tssrc/lib/settingsScope.test.tssrc/lib/settingsScope.tssrc/lib/standaloneRecreation.test.tssrc/lib/standaloneRecreation.tssrc/lib/standaloneReload.test.tssrc/lib/standaloneReload.tssrc/lib/titleBarNoteInfo.test.tssrc/lib/titleBarNoteInfo.tssrc/lib/useWindowSessionPersistence.tssrc/lib/useWindowShortcuts.tssrc/lib/windowClose.test.tssrc/lib/windowClose.tssrc/lib/windowCloseCallsites.test.tssrc/lib/windowSession.test.tssrc/lib/windowSession.tssrc/lib/windowShortcutCallsites.test.tssrc/lib/windowShortcuts.test.tssrc/lib/windowShortcuts.tssrc/lib/workspace.test.tssrc/lib/workspace.tssrc/lib/workspaceSwitch.test.tssrc/lib/workspaceSwitch.tssrc/services/draftCheckpoint.test.tssrc/services/draftCheckpoint.tssrc/services/files.test.tssrc/services/files.tssrc/services/notes.test.tssrc/services/notes.tssrc/services/windowLifecycle.test.tssrc/services/windowLifecycle.tssrc/services/windowSession.test.tssrc/services/windowSession.tssrc/types/note.ts
💤 Files with no reviewable changes (1)
- src/components/layout/SidebarControls.tsx
🚧 Files skipped from review as they are similar to previous changes (57)
- src/lib/documentMutationSafety.test.ts
- src/lib/editorWidthResize.ts
- src/services/windowSession.test.ts
- src/lib/editorWidthResize.test.ts
- src/lib/windowCloseCallsites.test.ts
- src/lib/workspace.ts
- src/services/notes.test.ts
- src/lib/documentMutationSafety.ts
- src/lib/workspaceSwitch.test.ts
- src/lib/standaloneRecreation.test.ts
- src/lib/noteSync.test.ts
- src/services/windowSession.ts
- src/lib/standaloneReload.ts
- src/lib/editorToolbar.ts
- src/lib/workspace.test.ts
- src/services/files.test.ts
- src/lib/editorToolbar.test.ts
- src/lib/windowShortcutCallsites.test.ts
- src/services/draftCheckpoint.test.ts
- src/lib/windowSession.test.ts
- src/lib/windowClose.test.ts
- src/lib/workspaceSwitch.ts
- src/lib/titleBarNoteInfo.test.ts
- src/services/files.ts
- src/services/windowLifecycle.ts
- src/lib/conflictResolution.test.ts
- src/components/editor/EditorWidthHandle.tsx
- src/services/windowLifecycle.test.ts
- src/components/editor/EditorWidthHandle.test.tsx
- src/lib/standaloneReload.test.ts
- src/lib/windowClose.ts
- src/types/note.ts
- src/context/GitContext.tsx
- src/components/settings/EditorSettingsSection.test.tsx
- src-tauri/src/draft_checkpoint.rs
- src/lib/windowShortcuts.test.ts
- src/components/settings/EditorSettingsSection.tsx
- src/lib/windowSession.ts
- src/components/notes/NoteList.tsx
- src/lib/windowShortcuts.ts
- src/lib/serializedWriter.test.ts
- src-tauri/src/watcher_debounce.rs
- src/lib/noteSync.ts
- src/components/settings/SettingsPage.test.tsx
- src/components/layout/Sidebar.error.test.ts
- src/lib/settingsScope.ts
- src/components/layout/WorkspaceMenu.tsx
- src/lib/draftCheckpoint.test.ts
- src/lib/draftCheckpoint.ts
- src/lib/titleBarNoteInfo.ts
- src/App.tsx
- src/lib/serializedWriter.ts
- src/components/layout/WorkspaceMenu.test.tsx
- src/components/settings/SettingsPage.tsx
- src/lib/standaloneRecreation.ts
- src/components/layout/Sidebar.tsx
- src-tauri/src/lib.rs
| fn atomic_write(path: &Path, bytes: &[u8]) -> io::Result<()> { | ||
| let parent = path.parent().ok_or_else(|| { | ||
| io::Error::new( | ||
| io::ErrorKind::InvalidInput, | ||
| "atomic save target has no parent directory", | ||
| ) | ||
| })?; | ||
| let existing_permissions = fs::metadata(path) | ||
| .ok() | ||
| .map(|metadata| metadata.permissions()); | ||
| let (mut temporary_file, mut temporary_path) = create_temporary_file(path, parent)?; | ||
|
|
||
| temporary_file.write_all(bytes)?; | ||
| temporary_file.flush()?; | ||
| if let Some(permissions) = existing_permissions { | ||
| temporary_file.set_permissions(permissions)?; | ||
| } | ||
| temporary_file.sync_all()?; | ||
| drop(temporary_file); | ||
|
|
||
| fs::rename(temporary_path.path(), path)?; | ||
| temporary_path.commit(); | ||
| sync_parent_directory(parent)?; | ||
| Ok(()) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
atomic_write replaces a symlinked note instead of writing through it.
fs::metadata at Line 143 follows the symlink, but fs::rename at Line 156 replaces the link itself. If a user symlinks a note into their notes folder, the first save silently detaches the link, and the original target keeps the old content. atomic_create_new already protects a dangling symlink, and there is a test for it, so the two paths behave differently.
Resolve the destination with fs::canonicalize before the rename when the entry is a symlink, or reject symlinked destinations explicitly.
🛠️ Proposed fix
fn atomic_write(path: &Path, bytes: &[u8]) -> io::Result<()> {
+ // Follow a symlinked note so the save reaches the real target.
+ let resolved = fs::symlink_metadata(path)
+ .ok()
+ .filter(|metadata| metadata.file_type().is_symlink())
+ .and_then(|_| fs::canonicalize(path).ok());
+ let path = resolved.as_deref().unwrap_or(path);
let parent = path.parent().ok_or_else(|| {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src-tauri/src/persistence.rs` around lines 136 - 159, Update atomic_write to
detect when the destination path is a symlink and resolve it with
fs::canonicalize before performing the temporary-file rename, or explicitly
reject the symlink with an appropriate error. Preserve existing permission
handling and ensure saves do not replace the symlink itself, matching
atomic_create_new behavior.
1fd5867 to
32e7ec5
Compare
Verify request_full_window_closure_for_preview uses window.close() instead of window.destroy(). Verify is_full_editor_window only matches main and workspace-*. Add Rust tests proving only main and workspace-* are targeted, preview windows are ignored, and Preferences is not treated as a full editor window.
Move ContentRevision to a dedicated hashing module and use it from persistence. This stabilizes note identity across workspace switches.
Wire native menu actions to their own events. File > New Window opens a new workspace window. File > Open Folder… targets the focused window.
Stabilize window session persistence with best-effort geometry capture and serialized writers that don't swallow failures silently.
Import completion and preview actions target the invoking window label. Best-effort cleanup for preview draft checkpoints.
32e7ec5 to
e6f3861
Compare
1 participant
Summary
Stack
This is PR 4 of a four-PR dependency stack. It currently includes the three preceding commits because those PRs are not merged yet:
Merge or rebase the stack in order. This PR diff will shrink as PRs 1–3 land.
Verification
npm test -- --run: 31 files, 122 tests passed.npm run build: passed.cargo test --manifest-path src-tauri/Cargo.toml --quiet: 89 tests passed.cargo clippy --manifest-path src-tauri/Cargo.toml --all-targets --all-features -- -D warnings: passed.git diff --check: passed.The Rust suite covers same-named notes in separate roots, concurrent saves without cross-talk, shared-runtime lifetime, per-window restoration, and workspace rebinding.
A macOS isolated app launch/restoration was exercised during development, but the complete two-window manual scenario was not repeated after the final scope cleanup. This is not a zero-bug guarantee.
Scope
The unrelated donor "Reveal in Finder/Explorer" command was intentionally removed from this PR. Editor selection, tables, image drag/drop, and block drag/drop remain outside this change.
Formatting note
cargo fmt --checkis already red on the parent PR 3 because the repository and donor snapshot do not match the installed rustfmt version. This PR does not apply a repository-wide formatting rewrite. Clippy with warnings denied passes.Summary by CodeRabbit