ADFA-5260 | Fix edit_file to save edited file via saveFile

ai-core/src/main/AndroidManifest.xml

@@ -28,7 +28,7 @@
 
  <meta-data
  android:name="plugin.min_ide_version"
- android:value="26.32" />
+ android:value="26.35" />
 
  <meta-data
  android:name="plugin.max_ide_version"

ai-core/src/main/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/edit/EditorBufferApplier.kt

@@ -13,6 +13,7 @@ import kotlinx.coroutines.withContext
  * Applies an edit to a file **open in the editor** through the buffer, not behind it: unsaved work is
  * what changes, the change is one Ctrl+Z away, and no stale buffer can overwrite it. Owns the
  * coordinate conversion, since [IdeEditorService.replaceRange] takes 0-based (line, column) pairs.
+ * The save is file-targeted, so it never depends on - or steals - the user's tab focus.
  * @param editorService the host editor.
  * @param mainDispatcher dispatcher for editor-UI calls; overridden in unit tests.
  */
@@ -77,7 +78,8 @@ class EditorBufferApplier(
  "range=${range.startLine}:${range.startColumn}-${range.endLine}:${range.endColumn}",
  )
 
- val outcome = withContext(mainDispatcher) {
+ // Read-check and mutate in one main-thread block; a refusal short-circuits the save.
+ val refusal = withContext(mainDispatcher) {
  val current = editorService.getFileContent(file)
  if (current != matched) {
  AgentTrace.refusal(
@@ -98,21 +100,20 @@ class EditorBufferApplier(
  )
  }
 
- // saveCurrentFile() saves the FOCUSED tab, so saving unfocused persists another file.
- val focused = editorService.openFile(file)
- if (!focused) {
- AgentTrace.refusal(
- "EDIT", "apply=editor path=${file.name}",
- "openFile returned false; not saving, to avoid saving a different tab",
- )
- }
- Outcome.Applied(saved = focused && editorService.saveCurrentFile())
+ null
  }
+ refusal?.let { return it }
 
- if (outcome is Outcome.Applied) {
- Log.d(TAG, "Edited $displayPath in the editor buffer (saved=${outcome.saved})")
+ // Only a denied filesystem.write means "unsaved"; cancellation and defects must propagate.
+ val saved = try {
+ editorService.saveFile(file)
+ } catch (e: SecurityException) {
+ AgentTrace.refusal("EDIT", "apply=editor path=${file.name}", "saveFile: ${e.message}")
+ false
  }
- return outcome
+
+ Log.d(TAG, "Edited $displayPath in the editor buffer (saved=$saved)")
+ return Outcome.Applied(saved = saved)
  }
 
  /**

ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/EditFileHandlerTest.kt

@@ -5,6 +5,8 @@ import com.itsaky.androidide.plugins.ServiceRegistry
 import com.itsaky.androidide.plugins.aicore.tool.Validation
 import com.itsaky.androidide.plugins.services.IdeEditorService
 import com.itsaky.androidide.plugins.services.SelectionRange
+import io.mockk.coEvery
+import io.mockk.coVerify
 import io.mockk.every
 import io.mockk.mockk
 import io.mockk.slot
@@ -41,8 +43,8 @@ class EditFileHandlerTest {
  editorService = mockk(relaxed = true)
  // Default: nothing is open in the editor, so the disk path is exercised.
  every { editorService.getFileContent(any()) } returns null
- // Default: focusing before saving succeeds; the false case has its own test below.
-every { editorService.openFile(any()) } returns true
+ // Default: the host persists the buffer; the false case has its own test below.
+coEvery { editorService.saveFile(any()) } returns true
  services = mockk()
  context = mockk()
  every { context.services } returns services
@@ -472,7 +474,7 @@ class EditFileHandlerTest {
  val target = File(projectRoot, "Main.kt")
  every { editorService.getFileContent(target) } returns "line0\nline1\nline2\n"
  every { editorService.replaceRange(any(), any(), any()) } returns true
-every { editorService.saveCurrentFile() } returns true
+coEvery { editorService.saveFile(any()) } returns true
 
  val range = slot<SelectionRange>()
  val replacement = slot<String>()
@@ -487,7 +489,10 @@ class EditFileHandlerTest {
  assertEquals(1, range.captured.endLine)
  assertEquals(5, range.captured.endColumn)
  assertEquals("LINE1", replacement.captured)
- verify { editorService.saveCurrentFile() }
+ coVerify { editorService.saveFile(target) }
+ // The save is file-targeted: the user's tab focus must not be read or moved (ADFA-5215).
+ verify(exactly = 0) { editorService.openFile(any()) }
+ verify(exactly = 0) { editorService.saveCurrentFile() }
  }
 
  @Test
@@ -496,7 +501,7 @@ class EditFileHandlerTest {
  val file = createFile("Main.kt", "val a = 1\n")
  every { editorService.getFileContent(file) } returns "val a = 1\nval userTyped = 2\n"
  every { editorService.replaceRange(any(), any(), any()) } returns true
-every { editorService.saveCurrentFile() } returns true
+coEvery { editorService.saveFile(any()) } returns true
 
  val result = edit(
  "file_path" to "Main.kt",
@@ -518,7 +523,7 @@ class EditFileHandlerTest {
  val file = createFile("Main.kt", "a\na\n")
  every { editorService.getFileContent(file) } returns "a\na\n"
  every { editorService.replaceRange(any(), any(), any()) } returns true
-every { editorService.saveCurrentFile() } returns true
+coEvery { editorService.saveFile(any()) } returns true
 
  val range = slot<SelectionRange>()
  val replacement = slot<String>()
@@ -538,12 +543,11 @@ class EditFileHandlerTest {
  }
 
  @Test
- fun givenTheTabCannotBeFocused_whenEdited_thenNothingIsSavedAndTheResultSaysUnsaved() {
- // saveCurrentFile() saves the FOCUSED tab, so saving unfocused persists a different file.
+ fun givenTheHostCannotSaveTheFile_whenEdited_thenTheResultSaysUnsaved() {
  val file = createFile("Main.kt", "old\n")
  every { editorService.getFileContent(file) } returns "old\n"
  every { editorService.replaceRange(any(), any(), any()) } returns true
-every { editorService.openFile(any()) } returns false
+coEvery { editorService.saveFile(any()) } returns false
 
  val result = edit("file_path" to "Main.kt", "old_string" to "old", "new_string" to "new")
 
@@ -552,6 +556,8 @@ class EditFileHandlerTest {
  "Must not claim the file was saved; got: ${result.message}",
  result.message.contains("left unsaved")
  )
+ // A failed save must not be retried by focusing the tab and saving that instead.
+ verify(exactly = 0) { editorService.openFile(any()) }
  verify(exactly = 0) { editorService.saveCurrentFile() }
  assertEquals("old\n", file.readText())
  }
@@ -566,7 +572,7 @@ class EditFileHandlerTest {
 
  assertFalse(result.success)
  assertEquals("old\n", file.readText())
-verify(exactly = 0) { editorService.saveCurrentFile() }
+coVerify(exactly = 0) { editorService.saveFile(any()) }
  }
 
  @Test
@@ -589,7 +595,7 @@ class EditFileHandlerTest {
  result.message.contains("changed")
  )
  verify(exactly = 0) { editorService.replaceRange(any(), any(), any()) }
-verify(exactly = 0) { editorService.saveCurrentFile() }
+coVerify(exactly = 0) { editorService.saveFile(any()) }
  assertEquals("the disk copy must not be touched either", analysed, file.readText())
  }
 
@@ -602,7 +608,7 @@ class EditFileHandlerTest {
  "line0\nline1\n",
  )
  every { editorService.replaceRange(any(), any(), any()) } returns true
-every { editorService.saveCurrentFile() } returns true
+coEvery { editorService.saveFile(any()) } returns true
 
  val result = edit("file_path" to "Main.kt", "old_string" to "line1", "new_string" to "LINE1")
 
@@ -680,7 +686,7 @@ class EditFileHandlerTest {
 
  assertEquals("val a = 1\n", file.readText())
  verify(exactly = 0) { editorService.replaceRange(any(), any(), any()) }
-verify(exactly = 0) { editorService.saveCurrentFile() }
+coVerify(exactly = 0) { editorService.saveFile(any()) }
  }
 
  // --- Content changing while the approval dialog is open -----------------

ai-core/src/test/kotlin/com/itsaky/androidide/plugins/aicore/tool/handlers/edit/EditorBufferApplierTest.kt

@@ -0,0 +1,155 @@
+package com.itsaky.androidide.plugins.aicore.tool.handlers.edit
+
+import com.itsaky.androidide.plugins.services.IdeEditorService
+import io.mockk.coEvery
+import io.mockk.coVerify
+import io.mockk.every
+import io.mockk.mockk
+import io.mockk.verify
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.asCoroutineDispatcher
+import kotlinx.coroutines.runBlocking
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertThrows
+import org.junit.Assert.assertTrue
+import org.junit.Before
+import org.junit.Test
+import java.io.File
+import java.util.concurrent.Executors
+
+/**
+ * Unit tests for [EditorBufferApplier] — specifically that the save targets the *file* and never the
+ * focused tab. Saving by focus persisted whichever tab the user happened to be in (ADFA-5215), so the
+ * negative verifications on `openFile`/`saveCurrentFile` below are the regression lock.
+ */
+class EditorBufferApplierTest {
+
+ private companion object {
+ const val EDITOR_THREAD = "test-editor"
+ }
+
+ private lateinit var editorService: IdeEditorService
+ private lateinit var applier: EditorBufferApplier
+
+ private val target = File("/project/A.kt")
+ private val buffer = "line0\nline1\nline2\n"
+
+ @Before
+ fun setup() {
+ editorService = mockk(relaxed = true)
+ every { editorService.getFileContent(target) } returns buffer
+ every { editorService.replaceRange(any(), any(), any()) } returns true
+ coEvery { editorService.saveFile(any()) } returns true
+ applier = EditorBufferApplier(editorService, Dispatchers.Unconfined)
+ }
+
+ /** The running thread, without the ` @coroutine#N` suffix the test JVM's debug mode appends. */
+ private fun currentThreadName() = Thread.currentThread().name.substringBefore(" @")
+
+ private fun apply(
+ matched: String = buffer,
+ applier: EditorBufferApplier = this.applier,
+ ): EditorBufferApplier.Outcome =
+ runBlocking {
+ applier.apply(
+ file = target,
+ displayPath = "A.kt",
+ matched = matched,
+ updated = matched.replace("line1", "LINE1"),
+ oldString = "line1",
+ newString = "LINE1",
+ occurrences = 1,
+ )
+ }
+
+ @Test
+ fun givenAnUnfocusedFile_whenApplied_thenItIsSavedByFileWithoutStealingFocus() {
+ // The user is looking at another tab; the edit must neither follow nor move focus.
+ every { editorService.getCurrentFile() } returns File("/project/B.kt")
+
+ val outcome = apply()
+
+ assertEquals(EditorBufferApplier.Outcome.Applied(saved = true), outcome)
+ coVerify { editorService.saveFile(target) }
+ verify(exactly = 0) { editorService.openFile(any()) }
+ verify(exactly = 0) { editorService.saveCurrentFile() }
+ }
+
+ @Test
+ fun givenTheHostSavesTheFile_whenApplied_thenTheOutcomeReportsSaved() {
+ assertEquals(EditorBufferApplier.Outcome.Applied(saved = true), apply())
+ }
+
+ @Test
+ fun givenTheHostCannotSaveTheFile_whenApplied_thenTheEditStillCountsAsApplied() {
+ coEvery { editorService.saveFile(any()) } returns false
+
+ assertEquals(EditorBufferApplier.Outcome.Applied(saved = false), apply())
+ }
+
+ @Test
+ fun givenSaveDeniedByPermissions_whenApplied_thenItIsReportedAsUnsavedNotPropagated() {
+ // Letting a permission miss escape would abort the turn over an edit already in the buffer.
+ coEvery { editorService.saveFile(any()) } throws SecurityException("FILESYSTEM_WRITE denied")
+
+ assertEquals(EditorBufferApplier.Outcome.Applied(saved = false), apply())
+ }
+
+ @Test
+ fun givenSaveFailingUnexpectedly_whenApplied_thenTheErrorIsNotSwallowed() {
+ // Only SecurityException means "unsaved"; a defect must not pass as an applied-but-unsaved edit.
+ coEvery { editorService.saveFile(any()) } throws IllegalStateException("host defect")
+
+ assertThrows(IllegalStateException::class.java) { apply() }
+ }
+
+ @Test
+ fun givenAnEditorDispatcher_whenApplied_thenOnlyTheBufferEditRunsOnIt() {
+ // saveFile is a suspending host call that reaches the editor thread on its own; re-dispatching
+ // it here - or saving inside the editor block - would put the applier back in that business.
+ val editorExecutor = Executors.newSingleThreadExecutor { Thread(it, EDITOR_THREAD) }
+ var replaceThread: String? = null
+ var saveThread: String? = null
+ every { editorService.replaceRange(any(), any(), any()) } answers {
+ replaceThread = currentThreadName()
+ true
+ }
+ coEvery { editorService.saveFile(any()) } answers {
+ saveThread = currentThreadName()
+ true
+ }
+
+ try {
+ val outcome = apply(
+ applier = EditorBufferApplier(editorService, editorExecutor.asCoroutineDispatcher()),
+ )
+
+ assertEquals(EditorBufferApplier.Outcome.Applied(saved = true), outcome)
+ assertEquals(EDITOR_THREAD, replaceThread)
+ assertEquals(currentThreadName(), saveThread)
+ } finally {
+ editorExecutor.shutdownNow()
+ }
+ }
+
+ @Test
+ fun givenAStaleBuffer_whenApplied_thenNothingIsEditedOrSaved() {
+ every { editorService.getFileContent(target) } returns "the user typed this instead\n"
+
+ val outcome = apply()
+
+ assertTrue("Expected a refusal, got: $outcome", outcome is EditorBufferApplier.Outcome.Failed)
+ verify(exactly = 0) { editorService.replaceRange(any(), any(), any()) }
+ coVerify(exactly = 0) { editorService.saveFile(any()) }
+ }
+
+ @Test
+ fun givenReplaceRangeRejectingTheEdit_whenApplied_thenNothingIsSaved() {
+ every { editorService.replaceRange(any(), any(), any()) } returns false
+
+ val outcome = apply()
+
+ assertTrue("Expected a refusal, got: $outcome", outcome is EditorBufferApplier.Outcome.Failed)
+ coVerify(exactly = 0) { editorService.saveFile(any()) }
+ }
+}