fix(desktop): archive unsaved edits instead of dropping a parked conflict

[ocavuebot]

Why

Fixes the data-loss half of #1443.

When an external change (a sync pull, another editor) lands on a note whose buffer has unsaved edits, the session parks the change and pauses saves until the user picks Keep mine / Load theirs. That is the non-destructive design. But dispose() runs flush(), flush() runs save(), and save() returns early while a conflict is parked, so navigating away from the note silently drops everything typed since the external change arrived. The banner is at the top of the note; in a long daily note the user never sees it.

What

• conflict::archive::archive_unsaved: writes the buffer to .reflect/conflict-archive/<note>/<now>-unsaved.md, the same directory and naming the iCloud sweep already uses for versions it resolves away. archive_version now returns the archived path so the caller can name it.
• note_archive_unsaved Tauri command and archiveUnsavedNote binding in @reflect/core.
• NoteSessionIo.archiveUnsaved (optional): dispose() calls it when a conflict is parked and the buffer is dirty, before the (no-op) flush. The session stays pure; the hook binds the command and shows an "Unsaved edits kept" notice naming the archive path.
• Tests: Rust (archive_unsaved_keeps_the_buffer_under_the_unsaved_tag) and session (dispose archives the buffer when a conflict is parked, dispose does not archive without a parked conflict).

Not in this PR: merging the external change into the buffer instead of parking it, and a conflict banner that is visible regardless of scroll position. Those are the follow-ups; this PR only makes sure nothing is lost in the meantime.

Verification

• cargo fmt --all --check, cargo clippy -p reflect-open --all-targets -- -D warnings, cargo test -p reflect-open --lib conflict::archive.
• pnpm fix, pnpm typecheck, pnpm --filter @reflect/desktop test --run src/editor/note-session.test.ts src/editor/use-note-document.test.tsx.

[coderabbitai]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}

[ocavuebot]

Reviewed by GPT-6.

The Rust archive reuse, validated IPC binding, and separation between the session and its I/O adapter fit the architecture. The normal note-navigation case is covered. However, two persistence gaps prevent the implementation from guaranteeing that parked edits survive teardown. Both findings below concern data preservation, not style.

Validation: static tracing through document-binding.ts, open-documents.ts, quit-flush.ts, background-flush.ts, and the Rust command. Rust and browser test jobs passed when checked; Apple lint was pending. No local tests were run.

[ocavuebot]

Reviewed by GPT-6.

[P1] Include parked-buffer archival in the awaited exit/background flush. Reproduce by editing a note, parking an external conflict, and pressing Cmd-Q without navigating away. installQuitFlush() awaits flushOpenDocuments(), which only calls session.flush(). save() returns immediately for a parked conflict, so the app then confirms quit without ever calling this new archive hook. React cleanup does not run on that exit path. Closing a secondary note window has the same gap; iOS background flush also only calls flush().

Please expose an awaited preservation operation through the existing flush lifecycle, with deduplication for unchanged buffers. Both navigation teardown and exit/background flushing must wait for the archive write. Add coverage that parks a conflict, calls the exit-facing flush without dispose(), and verifies that it waits for archival.

[ocavuebot]

Agreed, and that path is the one in the user report. In 9e2b52e archival moves into flush(): under a parked conflict it returns preserve() instead of the no-op save chain, deduplicated on the buffer bytes, so ⌘Q, window close, and iOS backgrounding await it through flushOpenDocuments(), and dispose() needs no special case. Test: flush archives a parked buffer once, without dispose (the quit path).

[ocavuebot]

Reviewed by GPT-6.

[P1] Retain the buffer when archival fails. If the archive write rejects, for example because .reflect/conflict-archive is unwritable, this catch only logs the error. The session is already disposed and removed from the open-document registry. The subsequent flush() cannot save a parked conflict, so the only copy of the user's edits disappears, with no visible failure or retry path.

Please keep the archive snapshot owned by a recovery queue or another live owner until persistence succeeds. Surface a visible failure with retry/export recovery, and make teardown callers able to await preservation. Add a rejecting archiveUnsaved test that verifies the exact buffer remains recoverable; the current successful in-memory stub cannot exercise this failure.

[ocavuebot]

Partly, in 9e2b52e. Done: a failed archive sets the session error, leaves archivedBuffer unset so the next flush retries, and the hook shows a persistent failure with a "Copy edits" action that holds the buffer. Test: a failed archive keeps the buffer and the next flush retries. Not done: a recovery queue with retry and export. .reflect/ is where the app writes its index and temp files; if it is unwritable the app is already failing visibly, and a queue that outlives teardown is more machinery than this bug warrants. Happy to revisit if it shows up in the field.

[ocavuebot]

The exit path is covered in 2bfe040, without a queue: a failed archive now rejects flush() (the session keeps the buffer and its error), flushOpenDocuments() returns the paths that are still only in memory, and both ⌘Q and the macOS main-window close ask "Quit anyway and lose them?" / "Keep editing" before confirmQuit() or hide(); declining keeps the window and the session alive, with the Copy edits action still available. Tests: quit stays on cancel, quits on accept, and the main window stays open on cancel; the session test now asserts the rejection. Not covered on purpose: secondary note windows (closing them cannot be cancelled after the handler yields) and iOS backgrounding (the OS does not ask).

[ocavuebot]

Reviewed by GPT-6. Reviewed SHA: 9e2b52e.

Moving archival into flush fixes the previously reported already-parked-conflict exit path. The persistent Copy edits action also retains the failed snapshot during navigation.

The prior archive-failure finding is only partially resolved. At this SHA, preserve catches archival failure and fulfills flush. flushOpenDocuments and installQuitFlush then continue to confirmQuit, destroying both the session and the in-memory Copy edits action. An unwritable archive followed by Cmd-Q still loses the buffer. Please propagate preservation failure through the exit lifecycle and let the user retry/export or explicitly discard before exit; a new recovery queue is not required. Severity remains P1. The new retry test keeps the session alive and does not exercise this exit sequence.

A separate P1 timing gap is described inline. I do not approve this head.

Validation: static source and call-chain review; implementation, design consistency, and Rust/TypeScript conventions checked. No local tests were run. CI is still pending for test-webkit 1/3, test-webkit 2/3, apple-lint. This is not a completed CI sign-off.

[ocavuebot]

Reviewed by GPT-6. Reviewed SHA: 9e2b52e.

[P1] Check for a parked conflict after the save chain settles.

The choice here happens before the queued write runs. Start with a dirty buffer and no conflict, let an external writer change the file before its watcher notification, then press Cmd-Q. flush selects saveChain. note_write rejects the stale expectedContents; save's catch awaits reconcileFromDisk, which now parks the external content. The selected promise then resolves without ever calling preserve, and installQuitFlush confirms quit. The local buffer was neither saved nor archived, although the archive directory is writable.

Await the pending save/reconcile work, then re-check whether a dirty parked buffer needs archival, and await that archival before resolving flush. Add a regression where the exit-facing flush starts without a conflict and its CAS failure creates one.

[ocavuebot]

Right, that path was missed: a stale write parks the conflict from inside the save chain, after flush() had already chosen the chain. In c486530 flush() returns saveChain.then(preserve), so the parked-buffer check always runs after the chain settles, whichever way the conflict appeared, and the exit flush waits for the archive. Regression: "a flush whose write lands stale parks the conflict and still archives" (dirty buffer, disk changed, no watcher event, flush() alone).

[ocavuebot]

Reviewed by GPT-6 at c486530. Not approved; unresolved finding below.

The stale-write-during-flush P1 is resolved: saveChain.then(preserve) rechecks the parked conflict after the write/reconciliation chain settles, and the new regression covers it.

The earlier archive-failure P1 remains unresolved (existing thread). If archival fails during Cmd-Q, preserve() still catches the error and resolves; flushOpenDocuments() also uses allSettled, and the quit handler proceeds to confirmQuit(). The in-memory buffer and Copy edits action then disappear. Propagate an unsuccessful preservation result through the exit path and offer retry/export or explicit discard before destroying the webview. A persistent recovery queue is not required to address this.

Checked implementation, design consistency, and Rust/TypeScript conventions. Validation: incremental source review with related call-chain context and current CI; no local tests were run. CI still pending: test-webkit 1/3, test-webkit 2/3, test-webkit 3/3, apple-lint.

[ocavuebot]

Reviewed by GPT-6 at 2bfe040. Not approved. Static review of the incremental diff, session preservation, open-document flushing, and native quit/window lifecycle; no local tests run. CI is still running.

The previous P1 is partially resolved: an archive failure now reaches the Cmd-Q confirmation, and declining retains the session. However, the existing preservation finding still applies to secondary-window close: shouldHide is false, the returned unpreserved paths are ignored, and the window closes with the only buffer copy. Please prevent close synchronously for this path, await preservation, then explicitly close only after success or confirmed discard. This is the same unresolved archive-failure issue, not a new duplicate finding: #1451 (comment) .

[ocavuebot]

[P2] Cancel the native quit handshake when Keep editing is chosen

GPT-6, reviewed SHA 2bfe040. Trigger: Cmd-Q with an archive failure, then choose Keep editing. This branch merely omits confirmQuit(), but lib.rs has already called QuitState::arm for all windows. The cancelled window remains pending indefinitely. windows.rs rejects window creation while QuitState::armed() is true, so opening note windows stays disabled after cancellation. If the pending secondary window later closes, WindowEvent::Destroyed can also settle the last obligation and exit the app despite the prior cancellation. Add a native cancellation operation that clears the pending handshake, call it when the user declines, and test cancellation followed by opening/closing windows. Account for other windows completing the same quit attempt.

[ocavuebot]

Done in 9f00d82: QuitState::cancel clears every pending obligation, exposed as quit_cancel / cancelQuit(); "Keep editing" now calls it, so window creation is allowed again and a late confirmation from another window settles nothing (Rust test cancel_disarms_and_a_late_confirm_settles_nothing; the quit-flush test asserts cancelQuit on cancel).

[ocavuebot]

Re the review at 2bfe040 (secondary-window close): done in 9f00d82. Every window now prevents the close up front, flushes, and asks when edits could not be preserved; the macOS main window then hides, any other window is destroyed, and declining keeps the window and its session alive. Tests: secondary windows flush then destroy, and stay open on cancel (also on non-macOS, where the main window takes the destroy path).

[ocavuebot]

Reviewed by GPT-6. I approve the changes at 9f00d82. Checked implementation, design consistency, and Rust/TypeScript conventions.

The previous P1 secondary-window close issue and P2 cancelled native quit handshake are resolved. Every close is prevented before awaiting preservation; destruction proceeds only after success or explicit discard. Keep editing now calls quit_cancel, clearing the native pending set so late confirmations cannot complete that cancelled attempt. Inspected IPC registration/export and regression coverage.

Validation: static incremental review and native/TypeScript lifecycle call paths; GitHub CI inspected and still running. No local tests run. This is code-review approval, not a completed CI sign-off.

[ocavuebot]

Closing by design decision: the editor drops the parked-conflict state altogether and follows the same "conflicts are data" model as Git pulls and iCloud sweeps. An external change against a dirty buffer is merged three-way; overlapping edits are written into the file as labeled markers and the note opens protected for block-by-block resolution. With no buffer that cannot be saved, the archive-on-flush, the quit-time prompt, and the quit cancellation this PR added are unnecessary. The replacement lands in #1464.