fix(save): let one write per tab be in flight at a time

[PathGao]

#436 fixed one direction of the save race and, in its own "Not covered", named the other and left it:

A write slower than the 1.5s debounce. The timer can still be re-armed by typing and fire during a very long explicit save. Part 1 makes that safe; the residual is the transient stale-on-disk described above, which heals on the next keystroke. Serialising writes per path would close it, at the cost of a promise-chain and snapshot relocation — not taken, as the remaining exposure is narrow and no longer unsafe.

That was the right call on the evidence available. This picks it up, having measured what the residual actually does — which is less bad than #436 assumed on every path but one, and worse than it assumed on that one.

1. The mechanism, and why cancelPendingAutoSave cannot reach it

The auto-save timer callback (MarkdownViewer.svelte) deletes itself from autoSaveTimers as its first statement, then calls saveContent:

const timer = setTimeout(() => {
    autoSaveTimers.delete(s.id);
    saveContent(s.id).then(...)
}, AUTO_SAVE_DEBOUNCE_MS);

So #436's fix covers exactly the window before the timer fires. Once it has fired, options.cancelPendingAutoSave(tab.id) finds nothing to cancel, and saveContent had no in-flight guard of its own. A Cmd+S landing while the auto-save write is still awaiting invoke('save_file_content') started a second concurrent write to the same path.

atomic_write (#436 part 1) makes that safe — neither writer can corrupt the file or fail the other. It is a safety property, not an ordering one. The older snapshot can still rename last.

The trigger window is one disk write. The auto-save debounce fires 1.5s after the last keystroke, so reaching this by hand means going idle for 1.5s and then pressing Cmd+S inside the few hundred microseconds the write takes. On a local SSD with a small file that is effectively unreachable. It widens with the write: a large document, a network volume, a busy or encrypted filesystem.

2. Severity: narrower than #436's, with one exception that is not

#436's bug was silent data loss — a clean-looking buffer over an older file. This one is not, on almost every path, and that is measured, not reasoned.

Driving the real documentSession and TabManager with a stubbed backend: auto-save takes snapshot A, the user types B, Cmd+S takes snapshot B, and the first (older) write is made to rename last:

OVERLAP: true
DISK:    "A"
BUFFER:  "B"   isDirty: true   originalContent: "A"

await invoke(...) resolves after Rust's rename, so resolution order tracks rename order — which means the continuation that lands last is the one whose snapshot is on disk:

tab.originalContent = snapshot;
tab.isDirty = tab.rawContent !== snapshot;

The end state is self-consistent. isDirty is not a spurious flag; it correctly says "the buffer and the disk disagree", and the dirty dot tells the user. Reading the auto-save effect, it also self-heals: isDirty flipping true with no timer armed re-arms the debounce, so the buffer is rewritten ~1.5s later without the user touching anything. #436's residual note said "heals on the next keystroke" — it is better than that; no keystroke is needed.

The exception is the close path, and it is silent loss. With auto-save on and confirm-before-save off, canCloseTab saves and closes if the save reports success and the tab reads clean. Measured:

canClose: true | at close time disk: "B" isDirty: false
AFTER the older write lands -> disk: "A" | tab still open: false

The user closes the tab, everything looks clean, and the older snapshot then renames on top. There is no tab left to raise the dirty flag, and no effect left to re-arm. The user's last edits are gone with no indication. That single path is the reason this is worth fixing; without it I would have argued for closing this as not worth the code, in the spirit of #430 and #431.

3. markSelfWrite under overlap — the more interesting consequence

markSelfWrite(path) is called both before and after the invoke, to stop the file watcher treating our own write as an external change. Two questions, both measured.

Both writes succeed: the four markSelfWrite calls only ever push the deadline further out, so the suppression window is extended, not broken. The mechanism degrades safely.

BOTH-OK -> our own write treated as external? false

One fails, the other succeeds: clearSelfWrite(path) in the catch deletes the entry unconditionally — including the one the successful write had just installed.

ONE-FAILS -> our own write treated as external? true
             tab isDirty: false => resolveExternalChange: {"action":"reload",...}

The app re-reads the file it just wrote. With a clean tab that is a spurious reload; with a dirty tab resolveExternalChange returns conflict, i.e. the "this file changed on disk" bar raised about our own write. #436's atomic_write means concurrent writers no longer fail each other, so this now needs an independent failure (ENOSPC, permissions, a disconnected volume) landing on one of the two — narrow, but real.

Serialising removes it structurally rather than by special-casing: ordered, the failing write's catch runs before the next write's markSelfWrite, so a clear can never reach a later write's guard.

4. The fix

writeExclusively(tabId, write) in documentSession.svelte.ts. Three choices, each of which could have gone the other way:

Wait and then write, rather than return the in-flight promise. They are different answers, not two spellings of one. The second caller pressed Cmd+S after typing more, so handing back the running write would report success for a file that does not contain those keystrokes — and would leave the tab dirty while telling the caller it saved. Waiting costs one disk write and ends with the text the user asked to save actually on disk. The snapshot is therefore taken inside the queued closure, on the far side of the wait; taking it before would serialise the writes and still publish stale text.

Keyed by tab, not by path. The state these two writes corrupt is the tab's own — originalContent, isDirty, its path — and the two racers are by construction one tab's, since both come from the same tab's debounce and the same tab's Cmd+S. A path key also could not be taken where it is needed: an untitled tab has no path until its Save dialog closes, which is an await. Two tabs pointing at one file still write concurrently; that is last-writer-wins between two documents, which the app permits by allowing the second tab at all, and is a different question. This is deliberately not a general per-path write queue — the window is one disk write, and the fix is sized to it.

A chain, not a bare await inFlight. Several callers awaiting one promise all wake in the same microtask and then all write at once — the race again, with extra steps. The chain is ~10 lines and self-clearing: only the tail deletes the map entry, so a link finishing cannot drop a queue others are still behind.

saveContentAs shares the same guard. Its target is usually a different file, so the renames need not collide — but both continuations write the same tab's originalContent, isDirty and path, and picking the tab's own file in the dialog is an allowed overwrite (as its existing comment notes) that collides outright.

The guard sits after cancelPendingAutoSave and after the Save dialog, so #436's two contracts are untouched: the cancel still precedes the write, and nothing is disarmed before a modal the user can still back out of.

Tests

scripts/oneWritePerTabInFlight.test.ts, five cases against the real TabManager and the real documentSession. The stub gives the first write the longer duration, which is what makes the race observable rather than merely possible.

test	asserts
a second save waits for the write in flight	no overlap; the newest text survives on disk; the tab reads clean
the snapshot is taken after the wait	text typed during the wait is what gets written
three saves stacked on one tab	still one write at a time, and all three still write — ordered, not dropped
saveContentAs shares the guard	no overlap; the older write does not leave the moved tab dirty
closing a tab mid-write	the closed tab's newest text is what is left on disk

Counter-proofs:

change	result
neutralise the wait (const result = write())	5 of 5 fail, first on two writes to one file were in flight at once
hoist the snapshot back outside the wait	only the snapshot-placement test fails

The second is there to show the tests are not all pinning one thing.

All four of #436's explicitSaveCancelsAutoSave.test.ts cases still pass and still mean what they meant — including its two source-text assertions, which require cancelPendingAutoSave to precede invoke('save_file_content') inside saveContent and forbid a call site taking the duty back. Both remain true.

Verification

On upstream/master (b63ff2e):

npm test        533 / 533   (528 before, 5 new)
npm run check   0 errors, 0 warnings   (632 files)
npm run build   clean

cargo test not run: no Rust changed. #436's atomic_write hardening is what this rests on and is not touched.

Not covered

• Cross-window and cross-process writers. Two windows can hold the same document with independent timers and independent isDirty, and the guard is per-session state that does not reach across them — the same limit fix(save): stop two writers on one file from breaking each other #436 recorded. This is exactly why atomic_write must stay the safety layer; the frontend can only supply ordering within one window.
• Two tabs on one file in one window. Serialised per tab, not per path, so they still write concurrently. Left deliberately: that is two documents disagreeing about one file, which the app already permits, and closing it means the per-path queue this change is explicitly sized not to build.
• The self-heal claim is read, not measured. That isDirty going true re-arms the debounce comes from reading the auto-save effect in MarkdownViewer.svelte; the tests stub $effect out, so no test exercises it. It affects the severity argument, not the fix.
• A tab closed while a save is queued behind another. The queued write now runs slightly later than it used to, possibly after the tab is gone. It writes the buffer as of that moment — the newest text, which is more correct than before, not less — and the discard path does not call saveContent at all, so no reverted buffer can be queued. No guard was added for it; noting it as the one ordering consequence a reviewer might want to push back on.
• No live app run. Verified against the real documentSession with a stubbed Tauri backend, not by hand in the running app on a slow volume.

🤖 Generated with Claude Code