fix(web): closing a file tab no longer overwrites newer changes - #8632
fix(web): closing a file tab no longer overwrites newer changes#8632walid-baharwal wants to merge 1 commit into
Conversation
The file editor's save coordinator only ever incremented its edit revision, so `dispose` treated any tab that had been edited once as holding unsaved work. Closing or switching that tab re-wrote the remembered buffer, replacing whatever had changed the file since the last save. Track the revision confirmed on disk and flush on dispose only when the buffer holds a newer edit. Edits made inside the debounce window are still saved on close, and a write that failed before the close is still retried. Fixes pingdotgg#8475
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This is a narrowly scoped file-save bug fix that prevents a redundant close-time write after a revision is already persisted, while preserving unsaved-edit flushing and retry behavior. Production changes are confined to the save coordinator, with targeted tests covering the affected lifecycle cases and no product-default or schema changes. You can add or adjust custom eligibility rules. Learn more. |
What Changed
FileSaveCoordinatornow tracks the revision it last confirmed on disk and flushes ondisposeonly when the buffer holds an edit newer than that. Previouslydisposeflushed wheneverlatestRevision > 0, andlatestRevisionwas only ever incremented, never reconciled with what was actually written.Why
Closing or switching a file tab re-wrote the tab's remembered buffer even when the debounce had already saved it. If the file changed on disk after that save — an agent turn, another client, a shell — the close replaced the newer content with the stale buffer, with nothing in the UI or the event store recording the loss. Fixes #8475.
Flush-on-dispose is deliberate: it saves edits made inside the 500 ms debounce window right before a tab closes. That still works, and a write that failed before the close is still retried. A write already in flight when the tab closes is unchanged from before this PR: if it fails and no newer edit exists, it is not retried.
Full disk-staleness reconciliation is deliberately out of scope.
ProjectWriteFileInputcarries onlycwd,relativePathandcontents, so a write precondition would be a contract change across server, web, mobile and desktop. The remaining case: an edit made inside the debounce window that races an external write still flushes.Related: #8424 touches this file for a different bug and, as a side effect, would also fix #8475 by zeroing
latestRevisionon success. This PR uses a separate counter instead, becauselatestRevisionis also the in-flight ordering token — zeroing it makes therevision === latestRevisioncheck after a successful write never hold. Whichever lands second needs a rebase.UI Changes
None. Nothing rendered changes; the observable difference is the file's contents on disk after a tab close.
Before (
main) and after, same steps — openprobe.txt, type a character, wait past the debounce, delete it, wait, thenecho APPENDED_BY_AGENT >> probe.txtfrom a shell, then close the tab:Verification
does not rewrite already saved contents when the editor closesfails onmain(expected "vi.fn()" to be called once, but got 2 times) and passes here. The other new cases cover dispose inside the debounce window, dispose while a write is in flight (with and without a newer edit), and retry after a failed write. Also verified by hand in a dev client against a scratch--home-dir, per the transcript above.Checklist
Model: Claude Opus 5 (1M context). Harness: Claude Code.
Note
Medium Risk
Changes file persistence on tab close in a coordinator used by the editor; incorrect logic could lose edits or skip needed flushes, but scope is small and well-tested.
Overview
Fixes closing a file tab re-writing disk with a stale buffer after the debounce had already persisted the same revision—e.g. when the file changed externally afterward (#8475).
FileSaveCoordinatornow keeps apersistedRevisionupdated on each successfulpersistand only runs flush-on-disposewhenlatestRevision > persistedRevision, instead of wheneverlatestRevision > 0. Flush-on-close behavior is preserved for edits still inside the debounce window, newer edits while a write is in flight, and retries after a failed write; it does not trigger a second write when the last edit was already confirmed.Tests add coverage for those dispose/close scenarios.
Reviewed by Cursor Bugbot for commit 38876e5. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Fix
FileSaveCoordinator.disposeoverwriting newer changes by trackingpersistedRevisionpersistedRevisionfield toFileSaveCoordinatorto track the last successfully saved revision, set on each successful persist inpersistLatestdisposeto only flush whenlatestRevision > persistedRevision, so closing a tab no longer rewrites content that was already savedpersistedRevisionunchanged, allowingdisposeto retry pending or failed writesdisposenow skips persisting when all edits are already confirmed; previously it always persisted iflatestRevision > 0Macroscope summarized 38876e5.