Prevent stale agent edits from silently dirtying the buffer on chat session restore#313793
Open
jeffreybulanadi wants to merge 1 commit intomicrosoft:mainfrom
Open
Conversation
…ession restore When a chat session is restored, _initEntries unconditionally applied the agent's snapshot edits back into the editor buffer whenever the entry state was Modified, even if the file had been saved with newer content after the snapshot was persisted. This caused the buffer to be overwritten with stale agent content and marked dirty, so any subsequent save (including autosave) would silently discard the user's newer changes. The fix reads the current on-disk content before the restore and compares it to the snapshot's original baseline. If the two differ the restore is skipped and the buffer is left showing whatever is on disk. UTF-8 BOM and CRLF line endings are normalised prior to comparison so encoding artefacts VS Code handles transparently are never mistaken for an external change. Affected: chatEditingSession.ts (_initEntries, new _isSnapshotCurrentOnDisk) Fixes: microsoft#313703
jeffreybulanadi
force-pushed
the
dev/jeffreyb/prevent-stale-agent-edit-restore
branch
from
09107fc to
e493712
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Prevents stale chat-edit snapshots from being restored into editor buffers on session reopen when the underlying file has changed on disk, reducing risk of silently dirtying buffers and overwriting newer user changes.
Changes:
- Adds a guard in
_initEntriesto only restore in-buffer edits when the on-disk content still matches the snapshot baseline. - Introduces
_isSnapshotCurrentOnDiskto read/normalize disk content (BOM + CRLF) and compare tosnapshotEntry.original. - Logs an info-level diagnostic when an in-buffer restore is skipped due to divergence.
Comment on lines
+1261
to
+1263
| const { value } = await this._fileService.readFile(snapshotEntry.resource); | ||
| const normalise = (s: string): string => s.replace(/^\uFEFF/, '').replace(/\r\n/g, '\n'); | ||
| return normalise(value.toString()) === normalise(snapshotEntry.original); |
Comment on lines
+1007
to
+1019
| let restoreToDisk = snapshotEntry.state === ModifiedFileEntryState.Modified; | ||
| if (restoreToDisk) { | ||
| // Guard against overwriting changes made to the file after the | ||
| // snapshot was persisted (e.g. the user edited and saved the file | ||
| // in a prior VS Code session or an external tool wrote to it). | ||
| // If the on-disk content no longer matches the snapshot baseline | ||
| // we skip restoring the agent edits into the buffer so we never | ||
| // silently dirty the buffer with stale content. | ||
| restoreToDisk = await this._isSnapshotCurrentOnDisk(snapshotEntry); | ||
| if (!restoreToDisk) { | ||
| this._logService.info(`chatEditingSession: skipping in-buffer restore for ${snapshotEntry.resource.toString()} - on-disk content has changed since snapshot was persisted`); | ||
| } | ||
| } |
Comment on lines
+1007
to
+1018
| let restoreToDisk = snapshotEntry.state === ModifiedFileEntryState.Modified; | ||
| if (restoreToDisk) { | ||
| // Guard against overwriting changes made to the file after the | ||
| // snapshot was persisted (e.g. the user edited and saved the file | ||
| // in a prior VS Code session or an external tool wrote to it). | ||
| // If the on-disk content no longer matches the snapshot baseline | ||
| // we skip restoring the agent edits into the buffer so we never | ||
| // silently dirty the buffer with stale content. | ||
| restoreToDisk = await this._isSnapshotCurrentOnDisk(snapshotEntry); | ||
| if (!restoreToDisk) { | ||
| this._logService.info(`chatEditingSession: skipping in-buffer restore for ${snapshotEntry.resource.toString()} - on-disk content has changed since snapshot was persisted`); | ||
| } |
Comment on lines
+1246
to
+1263
| * Returns `true` when the on-disk content of the file still matches the | ||
| * pre-edit baseline recorded in the snapshot ({@link ISnapshotEntry.original}), | ||
| * meaning no external change has occurred since the snapshot was persisted | ||
| * and it is therefore safe to restore the agent's in-progress edits into the buffer. | ||
| * | ||
| * Returns `false` when the file's on-disk content has diverged from the snapshot | ||
| * baseline (edited by the user, another tool, or saved from a prior VS Code session). | ||
| * The restore is skipped to prevent silently overwriting newer content and marking | ||
| * the buffer dirty. | ||
| * | ||
| * UTF-8 BOM and CRLF line endings are normalised before comparison so encoding | ||
| * artefacts that VS Code handles transparently are never treated as external changes. | ||
| */ | ||
| private async _isSnapshotCurrentOnDisk(snapshotEntry: ISnapshotEntry): Promise<boolean> { | ||
| try { | ||
| const { value } = await this._fileService.readFile(snapshotEntry.resource); | ||
| const normalise = (s: string): string => s.replace(/^\uFEFF/, '').replace(/\r\n/g, '\n'); | ||
| return normalise(value.toString()) === normalise(snapshotEntry.original); |
Comment on lines
+1259
to
+1263
| private async _isSnapshotCurrentOnDisk(snapshotEntry: ISnapshotEntry): Promise<boolean> { | ||
| try { | ||
| const { value } = await this._fileService.readFile(snapshotEntry.resource); | ||
| const normalise = (s: string): string => s.replace(/^\uFEFF/, '').replace(/\r\n/g, '\n'); | ||
| return normalise(value.toString()) === normalise(snapshotEntry.original); |
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.
Closes #313703
Problem
When a chat session is reopened, _initEntries\ unconditionally restores the agent's snapshot edits into the editor buffer whenever \snapshotEntry.state === Modified\ - with no check on whether the file has changed since the snapshot was persisted.
This means if the user edited and saved the file between two VS Code sessions, reopening the chat overwrites their newer content with the old agent content and marks the buffer dirty. Any subsequent save (or autosave) then silently discards the user's work.
The same data-loss window exists for files modified by external tools or synced from another machine.
What changed
*\chatEditingSession.ts\ - _initEntries*
Before attempting a disk restore, the code now calls a new helper _isSnapshotCurrentOnDisk\ that reads the file's current on-disk content and compares it to \snapshotEntry.original\ (the baseline captured before the agent started editing). Only when the two match - meaning the file is unchanged - does the restore proceed.
\chatEditingSession.ts\ - _isSnapshotCurrentOnDisk\ (new)
Reads the file via \IFileService.readFile, normalises UTF-8 BOM and CRLF line endings on both sides before comparing, then returns:
On read error (file gone, permission denied, etc.) the method returns \ rue\ so the existing missing-file handling in the entry is not bypassed.
A diagnostic log line is emitted at \info\ level when a restore is skipped, making the decision visible in the extension host log.
Behaviour matrix