fix(recents): merge the recent-files list instead of overwriting it by PathGao · Pull Request #405 · sftwrdotdev/Markpad

PathGao · 2026-08-02T23:01:52Z

1 / 5 of a stack over MarkdownViewer.svelte. Merge bottom-up.

The defect

Three call sites — saveRecentFile, deleteRecentFile, and the rename sync — each did JSON.stringify(recentFiles) from the window's own in-memory copy. No re-read, no storage listener. With two windows open, the last one to touch the list silently discarded what the other had recorded.

Reuses #370 rather than inventing a second mechanism

Same shape #370 already solved for settings:

New src/lib/utils/recentFiles.ts holds the pure list transforms plus updateStoredRecentFiles(mutate) — re-read, apply, write.
The write goes through fix(settings): persist each setting on its own key and sync across windows #370's writeStoredSetting, which is compare-and-set: a no-op write fires no storage event, so propagation terminates after one hop. fix(settings): persist each setting on its own key and sync across windows #370's reasoning for why an isApplyingRemote flag cannot work applies unchanged and is cited, not restated.
A storage listener in the viewer folds in what sibling windows record.

No change to settings.svelte.ts — writeStoredSetting was already exported.

Tests

scripts/recentFilesMultiWindow.test.ts drives two simulated windows against one shared localStorage and runs the real merge, rather than asserting on source text.


Module absent (true `master` state)	suite fails to load
Module present, viewer wiring reverted	3 red / 9 green
Final	12 / 12

npm run check   0 errors
npm test        433 / 433
cargo test      131 / 131   (untouched)

Not covered

Two windows writing in the same tick — localStorage is synchronous per document, so writes are serialised per webview; not tested.
Paths differing only by case are distinct entries.

🤖 Generated with Claude Code

Three call sites serialised the window's own copy of the list back to localStorage with no re-read and no `storage` listener, so with two windows open the last one to touch the list discarded whatever the other had recorded. #370 already solved this shape for settings, so the fix reuses it rather than inventing a second mechanism: a new `recentFiles.ts` holds the pure list transforms plus `updateStoredRecentFiles(mutate)`, which re-reads, applies, and writes through #370's `writeStoredSetting`. That write is compare-and-set, so a no-op fires no `storage` event and the propagation loop terminates after one hop - #370's argument for why an `isApplyingRemote` flag cannot work applies here unchanged. A `storage` listener in the viewer folds in what sibling windows record. No change to settings.svelte.ts was needed; `writeStoredSetting` was already exported. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

`save_pinned_tag` and `remove_pinned_tag` each read all of `pinned-tags.json`, edit the list in memory, and write the whole thing back. Nothing serialised that cycle. Tauri dispatches commands on a thread pool and every window can call these, so two windows can both read the same list and both write back a full copy - and the second write silently drops whatever the first one recorded. `atomic_write` does not cover this, which is the easy assumption to make. Its temp-file-fsync-rename ruled out a *torn* file: no reader ever sees half a JSON document. A lost update produces two whole, valid files in sequence; the second is simply built from a snapshot taken before the first one landed. The realistic trigger is not exotic. Each window saves its pinned tag from its own close handler (`appExit`, `destroyWindowAfterTabsClosed`, and the close-requested path), so quitting two tagged windows with Cmd-Q runs both cycles at once. `TitleBar.togglePinnedTag` and `clearTag` also fire their invoke without awaiting it. This is the same defect class as #405, which fixed recent-files being clobbered by re-reading live storage instead of an in-memory snapshot. A re-read alone was sufficient there because `localStorage` is per-document and single-threaded, so an RMW cycle is atomic by construction. Rust commands have no such property, so the cycle needs an explicit lock. The cycle now runs inside `update_pinned_tags`, holding a new `AppState.pinned_tags: Mutex<()>` - the same shape and the same `lock_recover` poison handling as the existing `window_registry`. Recovering from poisoning is right here too: `atomic_write` publishes by rename, so a panic inside the cycle leaves the previous file intact rather than a half-applied one, and propagating the poison would instead disable pinning for the rest of the session. `list_pinned_tags` deliberately does not lock. A plain read of a file that is only ever replaced by rename returns either the whole old list or the whole new one, both of which Markpad wrote. The cycle is what is unsafe, not the read. Measured with the lock removed, 8 writers x 4 rounds: 1-4 of 8 pins survived and 3-7 of 8 unpins came back from under a stale snapshot. The unlocked runs also failed outright with `File exists` and `No such file or directory` - concurrent `atomic_write` calls on one target can pick the same temp name (target name + pid + nanosecond clock), and the loser's cleanup deletes the file the winner was about to rename. Serialising removes that exposure for this file as well. Co-authored-by: PathGao <gaoyanbo@gaoyanbodeMacBook-Air.local> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

`save_pinned_tag` and `remove_pinned_tag` each read all of `pinned-tags.json`, edit the list in memory, and write the whole thing back. Nothing serialised that cycle. Tauri dispatches commands on a thread pool and every window can call these, so two windows can both read the same list and both write back a full copy - and the second write silently drops whatever the first one recorded. `atomic_write` does not cover this, which is the easy assumption to make. Its temp-file-fsync-rename ruled out a *torn* file: no reader ever sees half a JSON document. A lost update produces two whole, valid files in sequence; the second is simply built from a snapshot taken before the first one landed. The realistic trigger is not exotic. Each window saves its pinned tag from its own close handler (`appExit`, `destroyWindowAfterTabsClosed`, and the close-requested path), so quitting two tagged windows with Cmd-Q runs both cycles at once. `TitleBar.togglePinnedTag` and `clearTag` also fire their invoke without awaiting it. This is the same defect class as #405, which fixed recent-files being clobbered by re-reading live storage instead of an in-memory snapshot. A re-read alone was sufficient there because `localStorage` is per-document and single-threaded, so an RMW cycle is atomic by construction. Rust commands have no such property, so the cycle needs an explicit lock. The cycle now runs inside `update_pinned_tags`, holding a new `AppState.pinned_tags: Mutex<()>` - the same shape and the same `lock_recover` poison handling as the existing `window_registry`. Recovering from poisoning is right here too: `atomic_write` publishes by rename, so a panic inside the cycle leaves the previous file intact rather than a half-applied one, and propagating the poison would instead disable pinning for the rest of the session. `list_pinned_tags` deliberately does not lock. A plain read of a file that is only ever replaced by rename returns either the whole old list or the whole new one, both of which Markpad wrote. The cycle is what is unsafe, not the read. Measured with the lock removed, 8 writers x 4 rounds: 1-4 of 8 pins survived and 3-7 of 8 unpins came back from under a stale snapshot. The unlocked runs also failed outright with `File exists` and `No such file or directory` - concurrent `atomic_write` calls on one target can pick the same temp name (target name + pid + nanosecond clock), and the loser's cleanup deletes the file the winner was about to rename. Serialising removes that exposure for this file as well. Co-authored-by: PathGao <gaoyanbo@gaoyanbodeMacBook-Air.local> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

PathGao mentioned this pull request Aug 2, 2026

fix(documents): read through the checked command everywhere, and stop the auto-save toast repeating #406

Merged

PathGao force-pushed the fix/recent-files-across-windows branch from 934d9bb to 55ff270 Compare August 2, 2026 23:11

PathGao merged commit 55899c7 into master Aug 3, 2026
4 checks passed

PathGao deleted the fix/recent-files-across-windows branch August 3, 2026 02:05

PathGao mentioned this pull request Aug 3, 2026

fix(tags): stop two windows from erasing each other's pinned tags #424

Merged

This was referenced Aug 3, 2026

fix(markdown): make the code-region order an invariant instead of a sort #434

Merged

Index: open questions from the recent fix series, and where things stand #396

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(recents): merge the recent-files list instead of overwriting it - #405

fix(recents): merge the recent-files list instead of overwriting it#405
PathGao merged 1 commit into
masterfrom
fix/recent-files-across-windows

PathGao commented Aug 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

PathGao commented Aug 2, 2026

The defect

Reuses #370 rather than inventing a second mechanism

Tests

Not covered

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant