Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 37 additions & 3 deletions apps/desktop/src-tauri/src/conflict/archive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,15 @@ const MAX_PER_NOTE: usize = 20;
/// Archived versions older than this are pruned regardless of count.
const MAX_AGE_MS: u64 = 90 * 24 * 60 * 60 * 1000;

/// Archive one version's full content before it is resolved away.
/// Archive one version's full content before it is resolved away. Returns
/// the archive file's path relative to the graph root.
pub fn archive_version(
root: &Path,
rel: &str,
device: Option<&str>,
modified_ms: u64,
bytes: &[u8],
) -> AppResult<()> {
) -> AppResult<String> {
let dir = note_archive_dir(root, rel)
.ok_or_else(|| AppError::io(format!("invalid archive path: {rel}")))?;
fs::create_dir_all(&dir)?;
Expand All @@ -40,7 +41,24 @@ pub fn archive_version(
target = dir.join(format!("{stem}-{attempt}.{ext}"));
}
crate::fs::atomic_write_bytes(root, &target, bytes)?;
Ok(())
let archived = target
.strip_prefix(root)
.map(|rel| rel.to_string_lossy().replace('\\', "/"))
.unwrap_or_else(|_| target.to_string_lossy().into_owned());
Ok(archived)
}

/// Archive an open note's unsaved buffer: the user typed into a note while
/// an external change was parked against it, then closed the note before
/// choosing a side. Saves are paused behind a parked conflict, so without
/// this the buffer would die with the session (#1443). Stamped with the
/// current time and the `unsaved` tag, next to the sync versions.
pub fn archive_unsaved(root: &Path, rel: &str, bytes: &[u8]) -> AppResult<String> {
let now_ms = SystemTime::now()
.duration_since(UNIX_EPOCH)
.map(|dur| dur.as_millis() as u64)
.unwrap_or(0);
archive_version(root, rel, Some("unsaved"), now_ms, bytes)
}

/// Prune the archive: per note, keep the newest [`MAX_PER_NOTE`] versions and
Expand Down Expand Up @@ -158,6 +176,22 @@ mod tests {
.exists());
}

#[test]
fn archive_unsaved_keeps_the_buffer_under_the_unsaved_tag() {
let root = tempdir().unwrap();
fs::create_dir_all(root.path().join(".reflect")).unwrap();
let archived = archive_unsaved(root.path(), "daily/2026-10-07.md", b"# typed\n").unwrap();
assert!(
archived.starts_with(".reflect/conflict-archive/daily/2026-10-07.md/"),
"{archived}"
);
assert!(archived.ends_with("-unsaved.md"), "{archived}");
assert_eq!(
fs::read_to_string(root.path().join(&archived)).unwrap(),
"# typed\n"
);
}

#[test]
fn traversal_shapes_are_refused() {
let root = tempdir().unwrap();
Expand Down
19 changes: 19 additions & 0 deletions apps/desktop/src-tauri/src/fs/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,25 @@ pub async fn note_read(
crate::blocking::run_blocking(move || Ok(io::read_note_no_follow(&root, &abs)?)).await
}

/// Archive an open note's unsaved buffer under `.reflect/conflict-archive/`
/// (see [`crate::conflict::archive::archive_unsaved`]). The editor calls
/// this when a session is torn down while an external change is parked
/// against unsaved edits: the buffer must outlive the session. Returns the
/// graph-relative archive path for the notice.
#[tauri::command]
pub async fn note_archive_unsaved(
path: String,
contents: String,
generation: u64,
state: State<'_, GraphState>,
) -> AppResult<String> {
let root = root_for_generation(&state, generation)?;
crate::blocking::run_blocking(move || {
crate::conflict::archive::archive_unsaved(&root, &path, contents.as_bytes())
})
.await
}

/// How a [`note_read_local`] request found the note on disk.
#[derive(Debug, Serialize)]
#[serde(
Expand Down
1 change: 1 addition & 0 deletions apps/desktop/src-tauri/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,7 @@ pub fn run() {
fs::note_read_local,
fs::note_create,
fs::note_write,
fs::note_archive_unsaved,
fs::asset_write,
fs::asset_read,
fs::asset_read_binary,
Expand Down
8 changes: 8 additions & 0 deletions apps/desktop/src/editor/note-session-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,14 @@ export function createNoteSession(options: NoteSessionOptions): NoteSession {
// reads the (now frozen) buffer, so pending edits persist to this
// session's path even after the UI moves on.
if (!discarded) {
if (conflict !== null && dirty && io.archiveUnsaved !== undefined) {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

// A parked conflict pauses saves, so the flush below writes nothing
// and the buffer would die with the session. Keep it recoverable
// instead; the user chooses a side (or merges by hand) later.
void io.archiveUnsaved(path, header + buffer).catch((cause: unknown) => {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

console.error('failed to archive unsaved edits:', cause)
})
}
void flush()
}
disposed = true
Expand Down
7 changes: 7 additions & 0 deletions apps/desktop/src/editor/note-session-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,13 @@ export interface NoteSessionIo {
write:
| ((path: string, contents: string, expectedContents?: string | null) => Promise<void>)
| null
/**
* Keep an unsaved buffer somewhere recoverable. Called by `dispose()` when
* an external change is still parked against unsaved edits: saves are
* paused in that state, so without this the buffer would be lost with the
* session. Optional — a host without a graph generation cannot archive.
*/
archiveUnsaved?: ((path: string, contents: string) => Promise<void>) | undefined
}

/** Why {@link NoteSessionOptions.onContent} fired. */
Expand Down
39 changes: 39 additions & 0 deletions apps/desktop/src/editor/note-session.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ interface Harness {
snapshots: NoteSessionSnapshot[]
expectedContents: (string | null | undefined)[]
writes: Array<{ path: string; contents: string }>
archived: Array<{ path: string; contents: string }>
applied: string[]
contents: Array<{ content: string; origin: string }>
/** `null` deletes the file: subsequent reads throw the notFound AppError. */
Expand All @@ -49,6 +50,7 @@ function harness(options?: {
const snapshots: NoteSessionSnapshot[] = []
const expectedContents: (string | null | undefined)[] = []
const writes: Array<{ path: string; contents: string }> = []
const archived: Array<{ path: string; contents: string }> = []
const applied: string[] = []
const contents: Array<{ content: string; origin: string }> = []
let disk = options?.disk === undefined ? '# Hello\n' : options.disk
Expand Down Expand Up @@ -77,6 +79,9 @@ function harness(options?: {
writes.push({ path, contents })
disk = contents
},
archiveUnsaved: async (path, contents) => {
archived.push({ path, contents })
},
},
classify: options?.classify ?? (() => 'exact'),
onSnapshot: (snapshot) => {
Expand All @@ -99,6 +104,7 @@ function harness(options?: {
snapshots,
expectedContents,
writes,
archived,
applied,
contents,
setDisk: (contents) => {
Expand Down Expand Up @@ -213,6 +219,39 @@ describe('createNoteSession', () => {
expect(snapshots.length).toBe(afterLoad + 1) // one dirty transition, not two
})

it('dispose archives the buffer when a conflict is parked', async () => {
// #1443: a parked conflict pauses saves, so leaving the note would drop
// everything typed since the external change arrived.
const { session, writes, archived, snapshots, setDisk } = harness()
session.load()
await settled()

session.editorChanged('# Mine\n')
setDisk('# Theirs\n')
session.externalChanged()
await settled()
expect(snapshots.at(-1)?.conflict).toBe('# Theirs\n')

session.dispose()
await settled()

expect(writes).toEqual([]) // the conflict still blocks the write
expect(archived).toEqual([{ path: 'notes/a.md', contents: '# Mine\n' }])
})

it('dispose does not archive without a parked conflict', async () => {
const { session, writes, archived } = harness()
session.load()
await settled()

session.editorChanged('# Mine\n')
session.dispose()
await settled()

expect(writes).toEqual([{ path: 'notes/a.md', contents: '# Mine\n' }])
expect(archived).toEqual([])
})

it('keepMine rewrites the file even when the conflict content equals the buffer', async () => {
const { session, writes, snapshots, setDisk } = harness()
session.load()
Expand Down
15 changes: 14 additions & 1 deletion apps/desktop/src/editor/use-note-document.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { useCallback, useEffect, useRef, useState } from 'react'
import { readNote, writeNote, type FileChange } from '@reflect/core'
import { archiveUnsavedNote, readNote, writeNote, type FileChange } from '@reflect/core'
import { startOperation } from '@/lib/operations.ts'
import { useFileChanges } from '@/lib/use-file-changes.ts'
import { createDocumentBinding, type DocumentBinding } from './document-binding.ts'
import type { NoteEditorHandle } from './note-editor.tsx'
Expand Down Expand Up @@ -128,6 +129,18 @@ export function useNoteDocument(
return writeNote(forPath, contents, current, expectedContents)
}
: null,
archiveUnsaved: canWrite
? async (forPath, contents) => {
const current = generationRef.current
if (current === null) {
return
}
const archived = await archiveUnsavedNote(forPath, contents, current)
startOperation('Unsaved edits kept').warn(
`${forPath} changed on disk while you were editing. Your version is at ${archived}.`,
)
}
: undefined,
},
classify: checkRoundTrip,
onSnapshot: (next) => {
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/exports/platform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ export {
readNoteLocal,
type LocalNoteRead,
writeNote,
archiveUnsavedNote,
createNoteIfAbsent,
writeAsset,
readAsset,
Expand Down
15 changes: 15 additions & 0 deletions packages/core/src/graph/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,21 @@ export async function writeNote(
echoLocalWrite({ path, kind: 'upsert', modifiedMs: modifiedMs ?? Date.now() })
}

/**
* Archive an open note's unsaved buffer under `.reflect/conflict-archive/`.
* The note session calls this when it is torn down while an external change
* is parked against unsaved edits: saves are paused behind a parked
* conflict, so the buffer would otherwise die with the session. Returns the
* graph-relative archive path.
*/
export async function archiveUnsavedNote(
path: string,
contents: string,
generation: number,
): Promise<string> {
return await call('note_archive_unsaved', { path, contents, generation }, z.string())
}

/**
* Atomically create a note only if `path` is still unoccupied. A collision is
* returned as data and never overwrites the winner, closing the race between a
Expand Down
Loading