fix(save): ask the file who wrote it, and check before overwriting (#692) - #698
Merged
Conversation
) Whether a `file-changed` event was somebody else's write was guessed from the clock: each of our own writes opened a 400ms window in which every event for that path was discarded as ours. A clock cannot answer a question about identity, and it was wrong in both directions. Too early: another program writing inside the window had its event dropped, with nothing re-queued. The buffer then held our text and the disk held theirs, with the tab looking clean, so the next keystroke's auto-save put ours back over theirs — silent loss of somebody else's edit, which is the exact thing the guard exists to prevent. Too late: an event arriving after the window read as external, and if the user had typed since, the conflict bar asked about their own save. Zed is disliked for precisely this: a question that is sometimes false teaches people to dismiss it, including the times it is true. `originalContent` is already the text last known to be in the file, so comparing the file against it answers the question exactly and needs no window. The whole grace-window mechanism is deleted. The same comparison now also runs immediately before a write. Live Mode is off by default, its events can be dropped, and a watch does not always survive the file being renamed over — none of which reaches a check made at the moment of writing. Every editor with a claim to not losing work has one: VS Code refuses the save ("The content of the file is newer"), Vim re-stats before each write, Emacs raises file-supersession. Vim has no watcher at all and is still safe. A refusal, not a merge: the conflict bar is raised, and answering "keep mine" authorises exactly the next save. Cmd+S while the bar is up carries the same authorisation, or the bar would be a trap with no way out.
PathGao
force-pushed
the
fix/ask-the-file-not-the-clock
branch
from
August 21, 2026 18:09
ca532ef to
dc311fd
Compare
This was referenced Aug 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this is
Two failures of one mechanism, and one replacement for both.
Whether a
file-changedevent was somebody else's write was guessed from the clock: each of our own writes opened a 400ms window in which every event for that path was discarded as ours.Too early. A program writing inside that window had its event dropped, with nothing re-queued:
That is loss of somebody else's work, produced by the guard that exists to prevent it.
Too late. An event arriving after the window — a network share, a synced folder, any watcher latency over 400ms — read as external:
Nothing is lost, but the bar is now sometimes lying, and Zed is disliked for exactly this. What people learn from a question that is sometimes false is to dismiss it, including the times it is true.
Mechanism
originalContentis already the text last known to be in the file — every load sets it, every successful save sets it. So "the file differs fromoriginalContent" is the question, exactly, with no window and no clock:Equal means nobody has written since, whoever ran the last write. Different means somebody has, however long ago. Both directions above collapse.
selfWriteUntilByPath,markSelfWrite,clearSelfWrite,shouldReloadExternalChangeandSELF_WRITE_GRACE_MSare all deleted; nothing replaces them but the function above.An external write of identical bytes now resolves to
ignore, which is right — nothing changed.A truncated tab (the >5MB preview slice) always answers "changed" and needs no better answer:
ensureFullContentgates every path that can write, so a tab we have never written has no write of ours to recognise.The same question, asked before the write
The second half, and the reason this is one PR. That check now also runs immediately before
save_file_content, and refuses the save if the disk has moved.This is the guard that does not depend on the watcher, which matters because the watcher is not dependable: Live Mode is off by default, its events can be dropped, and a watch armed on a file does not always survive that file being renamed over (#697). None of that reaches a check made at the moment of writing.
Every editor with a claim to not losing work has one, and this is the shape they agree on:
autoread, opt-infile-supersession, raised as soon as you typeVim has no file watcher at all and is still safe, which is the argument for this check existing independently of the event path rather than instead of it.
A refusal, not a merge and not a retry. "Keep my version" authorises exactly the next save — one save, because a third program writing a minute later is a question the user has not answered. Cmd+S while the bar is up carries the same authorisation: without that the bar was a trap, since the disk really has changed, so the guard refuses, so the bar goes back up, and nothing the user presses gets them out.
The gap between the read and the rename is real and is the same gap Vim and VS Code have. It narrows exposure from "the whole time the document is open" to "the length of one write".
Scope
One read per watcher event and one per save. Every event that is not ignored already led to a read, so the new cost is a read per own save — while typing with auto-save on, one per 1.5s, of a file being written at the same rate. Files over 5MB take the
isTruncatedbranch and skip it.Not changed: the auto-save toast. A refused save returns
falselike a failed one, and the generic "auto-save failed" is suppressed exactly as it already is for the lossy-decode refusal — the conflict bar is on screen saying what happened and offering both ways out.Not changed, and next:
canCloseTabsaves a dirty tab without consulting the bar, so closing a tab with an unanswered conflict still silently answers "keep mine". And the bar still offers an irreversible Reload with no way to see what would be lost.Tests
scripts/externalChangeReload.spec.tsis rebuilt on a fake disk — aMapthe stubbedread_file_content_checkedandsave_file_contentshare — because every question in it is now "what does the file say?". That is also what let the fake clock go: no test here manipulates time any more.Seventeen cases. The four that name this change:
Checked by breaking what they guard: stubbing
fileDiffersFromBaselineto always answer "changed" turns four red; removing the guard from the write path turns two red.scripts/reopenDirtyDocument.spec.tsandscripts/truncatedBufferGuard.spec.tsare updated for the new call shape — the resolution isasyncnow, and it does a read of its own, which the read counters had to stop attributing to the open.Verification
npm run test:vitestalso reports 14 failures in this environment (session-restore and window-tag snapshots,assert.deepEqualreference-identity under node 26 + jsdom). Byte-identical on a clean checkout of the base branch in the same environment, verified by stashing and re-running.cargo testnot re-run: no Rust changed here. #697 covers that side.Not verified by hand: the end-to-end gesture with a real second editor. The behaviours above are pinned against a fake disk, which is where the decisions are made; what is not pinned is watcher latency in the wild, and the point of this change is that no decision depends on it any more.