ADFA-5260 | Fix edit_file to save edited file via saveFile - #79
Open
jatezzz wants to merge 1 commit into
Open
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
jatezzz
requested review from
a team,
Daniel-ADFA,
dara-abijo-adfa and
itsaky-adfa
August 26, 2026 20:58
EditorBufferApplier persisted its edit by focusing the target tab and calling saveCurrentFile(). Both are fire-and-forget: openFile() only posts the tab switch, so saveCurrentFile() read the previous tab index and saved whatever the user was looking at - force-saving their in-progress work while the target file stayed dirty and unchanged on disk. It returned true either way, so the model was told "saved" and later builds compiled the stale copy. Use the host's new file-targeted IdeEditorService.saveFile(File) instead. The buffer read and the replaceRange still share one main-thread block; the save moves to ioDispatcher, because saveFile blocks until the bytes are on disk and the write itself runs on main. openFile is gone from this path entirely - the agent no longer takes the user's focus to persist a file. saveFile throws on a permission miss rather than returning false, so the call is wrapped: an exception escaping here would abort the whole agent turn over an edit that already landed in the buffer. Swallowing it into saved=false yields "applied, left unsaved", which is true and actionable. New EditorBufferApplierTest covers the unfocused-file case with negative verifications on openFile/saveCurrentFile - the lock against reintroducing the focus dependency - plus save success, failure, throwing, and the two refusal paths. EditFileHandlerTest moves off the removed stubs. Requires the host release that ships saveFile (26.35); min_ide_version bumped, though the host records rather than enforces it.
jatezzz
force-pushed
the
fix/ADFA-5260-file-targeted-save
branch
from
August 27, 2026 13:26
e46ab31 to
229cdbd
Compare
itsaky-adfa
approved these changes
Aug 27, 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
2 participants
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.
Description
This PR updates
EditorBufferApplierto persist the specific file it modifies, preventing a severe code loss and desynchronization issue. Previously, the applier would callopenFile(which merely posted a tab switch) followed immediately bysaveCurrentFile(). This race condition caused the IDE to force-save whichever tab the user was actively looking at, leaving the agent's target file dirty on disk while falsely reporting a successful save to the model.To fix this, we:
openFileandsaveCurrentFileinEditorBufferApplier.saveFile(file)call executed off the main thread.SecurityExceptionduring the save process, so a denied filesystem write gracefully reportssaved=falseto the model without throwing an escaping exception that would abort the agent's turn.EditorBufferApplierTestand migratedEditFileHandlerTestto verify unfocused file scenarios, save success/failure states, and to guarantee tab focus is never stolen.Details
Logic-related changes. Please review the newly added
EditorBufferApplierTestsuite which includes strict negative verifications to lock in the regression fix (ensuringopenFileandsaveCurrentFileare never invoked).Before
Screen_Recording_20260824_161034_Code.on.the.Go.mp4
After
Screen_Recording_20260824_161341_Code.on.the.Go.mp4
Ticket
ADFA-5260
Parent: ADFA-5215
Observation
The plugin version in
AndroidManifest.xmlhas been bumped to26.35. The tests confirm the requirements for regression testREG-TOOL-EDIT-006, ensuring that agent edits to unfocused files keep the user's currently focused unsaved changes strictly untouched.This PR depends on appdevforall/CodeOnTheGo#1750