Skip to content

fix(ADFA-2584): Allow file rename for case-only changes #1359

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dara-abijo-adfa wants to merge 1 commit into
base: stage
Choose a base branch
from
+8 −2
Open

fix(ADFA-2584): Allow file rename for case-only changes #1359

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions app/src/main/java/com/itsaky/androidide/viewmodel/FileManagerViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,17 @@ class FileManagerViewModel : ViewModel() {

fun renameFile(file: File, newName: String, context: Context? = null, onResult: ((Boolean) -> Unit)? = null) {
viewModelScope.launch {
val destFile = File(file.parentFile, newName)
val renamed = withContext(Dispatchers.IO) {
newName.length in 1..40 && FileUtils.rename(file, newName)
if (file.name.equals(newName, ignoreCase = true)) {
val tempFile = File(file.parentFile, "$newName.tmp")
file.renameTo(tempFile) && tempFile.renameTo(destFile)
Comment on lines +34 to +35
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Jun 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Critical: Check for temporary file collision before rename.

The temporary file "$newName.tmp" could already exist in the directory. If it does, renameTo will either fail (leaving the original file unchanged) or potentially overwrite existing data on some file systems, causing data loss.

🛡️ Proposed fix to check for temp file existence 
 if (file.name.equals(newName, ignoreCase = true)) {
     val tempFile = File(file.parentFile, "$newName.tmp")
+    if (tempFile.exists()) {
+        return@withContext false
+    }
     file.renameTo(tempFile) && tempFile.renameTo(destFile)
 } else {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
val tempFile = File(file.parentFile, "$newName.tmp")
file.renameTo(tempFile) && tempFile.renameTo(destFile)
val tempFile = File(file.parentFile, "$newName.tmp")
if (tempFile.exists()) {
return@withContext false
}
file.renameTo(tempFile) && tempFile.renameTo(destFile)
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/main/java/com/itsaky/androidide/viewmodel/FileManagerViewModel.kt`
around lines 34 - 35, The rename sequence uses a fixed temp filename
("$newName.tmp") which can collide; update the logic in FileManagerViewModel
around variables tempFile, file.renameTo(tempFile) and destFile to first ensure
a unique temporary path (e.g., use File.createTempFile or loop to append an
increasing suffix until a non-existent temp is found), fail fast if you cannot
reserve a unique temp, perform file.renameTo(tempFile) then
tempFile.renameTo(destFile), and ensure you clean up the tempFile on any failure
to avoid leaving or overwriting files.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Major: Add rollback mechanism for failed two-step rename.

If file.renameTo(tempFile) succeeds but tempFile.renameTo(destFile) fails, the file will be left with the .tmp extension. There is no cleanup or rollback, leaving the file system in an inconsistent state.

🔄 Proposed fix to add rollback logic 
 if (file.name.equals(newName, ignoreCase = true)) {
     val tempFile = File(file.parentFile, "$newName.tmp")
-    file.renameTo(tempFile) && tempFile.renameTo(destFile)
+    val firstRename = file.renameTo(tempFile)
+    if (firstRename) {
+        val secondRename = tempFile.renameTo(destFile)
+        if (!secondRename) {
+            // Rollback: restore original name
+            tempFile.renameTo(file)
+        }
+        secondRename
+    } else {
+        false
+    }
 } else {
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/main/java/com/itsaky/androidide/viewmodel/FileManagerViewModel.kt`
around lines 34 - 35, The two-step rename in FileManagerViewModel (where it
creates tempFile and calls file.renameTo(tempFile) &&
tempFile.renameTo(destFile)) lacks rollback: if the first rename succeeds but
the second fails the temp file remains. Change the logic in the renaming method
in FileManagerViewModel to perform the second rename in an if block so you can
detect failure; on failure attempt to move tempFile back to the original file
name (rollback), catch/log any exceptions (use whatever logger is available in
this class), and return false; ensure the method returns true only if both
renames succeed and that all filesystem operations are guarded and error-handled
to avoid leaving a .tmp file behind.

} else {
FileUtils.rename(file, newName)
}
}

if (renamed) {
if (newName.length in 1..40 && renamed) {
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Jun 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Critical: Validate name before performing rename operation.

The validation of newName.length in 1..40 now occurs AFTER the rename operation completes. This means if the name length is invalid (empty, or >40 characters), the file will still be renamed, but the operation will be reported as failed. This creates an inconsistent state where the file system changes but the operation is reported as an error.

Validation must occur before attempting the rename to prevent this inconsistency.

🛡️ Proposed fix to validate before rename 
 fun renameFile(file: File, newName: String, context: Context? = null, onResult: ((Boolean) -> Unit)? = null) {
     viewModelScope.launch {
+        if (newName.length !in 1..40) {
+            _operationResult.emit(FileOpResult.Error(R.string.rename_failed))
+            onResult?.invoke(false)
+            return@launch
+        }
         val destFile = File(file.parentFile, newName)
         val renamed = withContext(Dispatchers.IO) {
             if (file.name.equals(newName, ignoreCase = true)) {
                 val tempFile = File(file.parentFile, "$newName.tmp")
                 file.renameTo(tempFile) && tempFile.renameTo(destFile)
             } else {
                 FileUtils.rename(file, newName)
             }
         }

-        if (newName.length in 1..40 && renamed) {
+        if (renamed) {
             // Notify system of the rename
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/main/java/com/itsaky/androidide/viewmodel/FileManagerViewModel.kt` at
line 41, The rename validation is happening after the file is already renamed,
causing filesystem changes when the new name is invalid; in FileManagerViewModel
(the method handling rename—e.g., renameFile or the function where "renamed" and
"newName" are used) move the check newName.length in 1..40 to before any
file.renameTo / rename operation, return early or set an error/result when the
name is invalid, and only perform the actual rename when the name passes
validation (then update the existing condition that currently reads "if
(newName.length in 1..40 && renamed)" so it only evaluates renamed after a
successful pre-check).

// Notify system of the rename
val renameEvent = FileRenameEvent(file, File(file.parent, newName))
if (context != null) {
Expand Down
Loading