ADFA-1840: Fix project deletion to remove hidden backups and handle partial failures

[Daniel-ADFA]

No description provided.

[Daniel-ADFA]

@greptile

[greptile-apps]

Greptile Overview

Greptile Summary

This PR refactors project deletion to use a rename-then-delete strategy instead of copy-then-delete, and adds cleanup of hidden backup files from previous failed deletions.

Key Changes:

• Changed FileDeleteUtils.deleteRecursive() from callback-based to suspend function returning Boolean
• Replaced file copy with renameTo() to hidden filename before deletion (more efficient)
• Added Sentry exception tracking throughout deletion process
• Made MainActivity.deleteProject() launch coroutine instead of synchronous try-catch
• Improved error handling to detect and report partial failures

Critical Issues Found:

• Ignored return value when cleaning up previous failed deletions can cause renameTo() to fail
• Race condition when file doesn't exist causes attempt to delete non-existent hidden file
• Uncontrolled concurrent deletions when multiple projects deleted (no semaphore/queue)
• Missing Sentry tracking in MainActivity's runCatching block

Regressions:

• User may see "deleted successfully" message while deletions still running in background
• Multiple concurrent I/O operations may exhaust system resources or cause slowdown
• Error reporting broken for bulk deletions (only last error shown, not first failure)

Confidence Score: 1/5

• This PR has critical logic bugs that will cause deletion failures in common scenarios
• Score reflects three critical logic errors that will cause failures: ignored cleanup return value, race condition with non-existent files, and uncontrolled concurrent deletions. These issues will manifest when users delete multiple projects, when hidden files exist from previous failures, or when files are already deleted. The fire-and-forget pattern also breaks error reporting for bulk operations.
• FileDeleteUtils.kt requires immediate fixes for cleanup logic and race condition. MainActivity.kt needs proper async coordination and Sentry tracking.

Important Files Changed

File Analysis

Filename	Score	Overview
app/src/main/java/com/itsaky/androidide/utils/FileDeleteUtils.kt	1/5	Changed from copy-then-delete to rename-then-delete strategy with proper error handling, but has critical logic bugs with cleanup and race conditions that will cause deletion failures
app/src/main/java/com/itsaky/androidide/activities/MainActivity.kt	2/5	Made deleteProject asynchronous with proper error handling, but fire-and-forget pattern causes uncontrolled concurrency when multiple projects are deleted and missing Sentry error tracking

Sequence Diagram

sequenceDiagram
participant Fragment as DeleteProjectFragment
participant Activity as MainActivity
participant Utils as FileDeleteUtils
participant FS as File System
Fragment->>Activity: deleteProject(root)
Note over Fragment,Activity: Called in loop for each selected project
Activity->>Activity: lifecycleScope.launch
Note over Activity: Fire-and-forget coroutine
Activity->>Utils: deleteRecursive(root)
alt Hidden file exists (previous failure)
Utils->>FS: Check .hiddenFile exists
FS-->>Utils: exists=true
Utils->>Utils: deleteRecursively(.hiddenFile)
Note over Utils: Return value ignored - RISK!
end
alt Original file exists
Utils->>FS: renameTo(.hiddenFile)
alt Rename fails
FS-->>Utils: false
Utils-->>Activity: return false
Activity->>Activity: flashInfo(error)
else Rename succeeds
FS-->>Utils: true
Utils->>Utils: deleteRecursively(.hiddenFile)
Utils->>FS: delete directory tree
FS-->>Utils: success/failure
Utils-->>Activity: return result
alt Delete failed
Activity->>Activity: flashInfo(error)
end
end
else File doesn't exist
Utils->>FS: Try to delete non-existent .hiddenFile
Note over Utils: RACE CONDITION - attempts to delete missing file
Utils-->>Activity: false
Activity->>Activity: flashInfo(error)
end
Note over Fragment: Multiple concurrent operations if deleting multiple projects

Loading

_{2 files reviewed, 4 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

logic: The return value of deleteRecursively(hiddenFile) is ignored during cleanup of previous failed deletion. If cleanup fails, the function continues and may attempt to rename to a directory that already exists, causing renameTo to fail.

Suggested change

	if (hiddenFile.exists() &&!fileOrDirectory.exists()) {
	Log.w(TAG, "Cleaning up previous failed deletion: ${hiddenFile.absolutePath}")
	deleteRecursively(hiddenFile)
	if (hiddenFile.exists() &&!fileOrDirectory.exists()) {
	Log.w(TAG, "Cleaning up previous failed deletion: ${hiddenFile.absolutePath}")
	if (!deleteRecursively(hiddenFile)) {
	Log.e(TAG, "Failed to clean up hidden file: ${hiddenFile.absolutePath}")
	return@withContext false
	}
	}

[greptile-apps]

logic: Race condition: if fileOrDirectory doesn't exist at line 24 but hiddenFile also doesn't exist, the function will attempt to delete a non-existent hiddenFile at line 37. This happens when called with a file that was already deleted or never existed.

The function should return early with an appropriate result when the file doesn't exist:

Suggested change

	if (fileOrDirectory.exists()) {
	val renamed = runCatching {
	fileOrDirectory.renameTo(hiddenFile)
	}.onFailure { t ->
	Log.e(TAG, "Failed to rename ${fileOrDirectory.absolutePath} to hidden: ${t.message}", t)
	Sentry.captureException(t)
	}.getOrDefault(false)
	if (!renamed) {
	return@withContext false
	}
	}
	if (fileOrDirectory.exists()) {
	val renamed = runCatching {
	fileOrDirectory.renameTo(hiddenFile)
	}.onFailure { t ->
	Log.e(TAG, "Failed to rename ${fileOrDirectory.absolutePath} to hidden: ${t.message}", t)
	Sentry.captureException(t)
	}.getOrDefault(false)
	if (!renamed) {
	return@withContext false
	}
	} elseif (!hiddenFile.exists()) {
	Log.d(TAG, "File does not exist: ${fileOrDirectory.absolutePath}")
	return@withContext true
	}

[greptile-apps]

logic: Fire-and-forget coroutine: the caller in DeleteProjectFragment.kt:159 iterates through multiple projects and calls deleteProject() in a loop without waiting for completion. This creates uncontrolled concurrent deletion operations that may:

1. Exhaust system resources with many parallel I/O operations
2. Prevent proper error reporting to the user (only last deletion's error is shown)
3. Show success message before deletions complete

The function should be suspend and called with proper concurrency control.

[greptile-apps]

style: Custom instruction violation: exceptions caught in runCatching should be captured with Sentry.captureException() for error tracking and monitoring.

Suggested change

	val success = runCatching {
	FileDeleteUtils.deleteRecursive(root)
	}.getOrDefault(false)
	val success = runCatching {
	FileDeleteUtils.deleteRecursive(root)
	}.onFailure { Sentry.captureException(it) }
	.getOrDefault(false)

Context Used: Rule from dashboard - Use Sentry.captureException() to capture exceptions for error tracking and monitoring. (source)

[Daniel-ADFA]

@greptile

[greptile-apps]

Greptile Overview

Greptile Summary

This PR refactors project deletion to use a rename-then-delete pattern (renaming to hidden .projectName before deletion) and adds cleanup for incomplete deletions from previous failures. The implementation improves error handling with Sentry integration and properly converts the deletion flow to use suspending functions.

Key Changes:

• Replaced async copy-then-delete with synchronous rename-then-delete in FileDeleteUtils
• Added cleanup logic for hidden backup files from failed previous deletions
• Changed MainActivity.deleteProject() to suspending function returning Boolean
• Updated DeleteProjectFragment to use concurrent deletion with async/awaitAll for multiple projects
• Improved error handling with Sentry exception tracking throughout the deletion flow
• Modernized dialog padding implementation in RecentProjectsAdapter

Potential Regressions:

• Race condition in FileDeleteUtils.kt:38-41 where an externally deleted file could trigger deletion of an unrelated hidden file
• The cleanup logic at line 19-25 doesn't verify the hidden file is actually related to the current deletion request
• Error propagation now properly bubbles up, which could expose new failure cases that were previously silently ignored

Confidence Score: 2/5

• This PR has critical logic issues in the core deletion logic that could cause data loss or deletion failures
• The fundamental approach is sound (rename-then-delete pattern), but the implementation has race conditions and logic flaws in FileDeleteUtils that could delete wrong data or fail unpredictably. The previous comments identify scenarios where the file existence checks don't properly handle all edge cases, particularly when external processes delete files concurrently or when hidden files exist from unrelated operations.
• Pay close attention to app/src/main/java/com/itsaky/androidide/utils/FileDeleteUtils.kt - the race conditions in lines 19-41 need to be resolved before merging

Important Files Changed

File Analysis

Filename	Score	Overview
app/src/main/java/com/itsaky/androidide/utils/FileDeleteUtils.kt	2/5	Rewritten deletion logic using rename-then-delete pattern, but contains race conditions and logic issues that could cause data loss or deletion failures
app/src/main/java/com/itsaky/androidide/activities/MainActivity.kt	4/5	Made deleteProject() suspending and added proper error handling with Sentry; removed unused imports and improved structure
app/src/main/java/com/itsaky/androidide/fragments/DeleteProjectFragment.kt	3/5	Improved deletion flow with concurrent deletion using async/awaitAll, proper error feedback, but has minor optimization issue with dispatcher usage
app/src/main/java/com/itsaky/androidide/adapters/RecentProjectsAdapter.kt	4/5	Updated to use new suspending deleteProject() API with proper coroutine scoping and error handling; modernized dialog padding implementation

Sequence Diagram

sequenceDiagram
participant User
participant Fragment as DeleteProjectFragment/<br/>RecentProjectsAdapter
participant Activity as MainActivity
participant Utils as FileDeleteUtils
participant FS as File System
User->>Fragment: Select project(s) & confirm delete
Fragment->>Fragment: Get selected projects
loop For each project
Fragment->>Activity: deleteProject(File) [async]
Activity->>Activity: Set project path
Activity->>Utils: deleteRecursive(File) [suspend]
Utils->>Utils: Create hidden file path (.projectName)
alt Hidden file exists & original doesn't
Utils->>Utils: Clean up previous failed deletion
Utils->>FS: deleteRecursively(hiddenFile)
alt Cleanup fails
FS-->>Utils: false
Utils-->>Activity: false
Activity-->>Fragment: false
end
end
alt Original file exists
Utils->>FS: renameTo(hiddenFile)
alt Rename fails
FS-->>Utils: Exception/false
Utils->>Utils: Log error + Sentry
Utils-->>Activity: false
Activity-->>Fragment: false
end
else Original doesn't exist & hidden doesn't exist
Utils-->>Activity: true (already deleted)
Activity-->>Fragment: true
end
Utils->>FS: deleteRecursively(hiddenFile)
FS-->>Utils: boolean result
alt Deletion fails
Utils->>Utils: Log warning about remaining hidden dir
end
Utils-->>Activity: deletion result
Activity-->>Fragment: deletion result
end
Fragment->>Fragment: awaitAll() for all deletions
alt All deletions successful
Fragment->>Fragment: Remove from UI state
Fragment->>User: Show success message
else Any deletion failed
Fragment->>User: Show error message
end

Loading

_{4 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

logic: Race condition: when fileOrDirectory doesn't exist but hiddenFile exists, the code skips renaming and proceeds to delete hiddenFile at line 43. However, this assumes hiddenFile is from a previous failed deletion. If fileOrDirectory was just deleted externally (race condition), attempting to delete an unrelated hiddenFile could delete the wrong data.

Suggested change

	} elseif (!hiddenFile.exists()) {
	Log.d(TAG, "File does not exist: ${fileOrDirectory.absolutePath}")
	return@withContext true
	}
	} elseif (!hiddenFile.exists()) {
	Log.d(TAG, "File does not exist: ${fileOrDirectory.absolutePath}")
	return@withContext true
	} else {
	Log.w(TAG, "File does not exist but hidden file remains: ${hiddenFile.absolutePath}")
	}

[greptile-apps]

style: Unnecessary dispatcher specification: async(Dispatchers.IO) is redundant since MainActivity.deleteProject() already uses withContext(Dispatchers.IO) internally at FileDeleteUtils.kt:13. The explicit dispatcher causes an extra thread context switch.

Suggested change

	val results = locations.map { location ->
	async(Dispatchers.IO) {
	(activity as?MainActivity)?.deleteProject(File(location.path)) ?:false
	}
	val names = locations?.map { it.name }
	if (names !=null) {
	}.awaitAll()
	val results = locations.map { location ->
	async {
	(activity as?MainActivity)?.deleteProject(File(location.path)) ?:false
	}
	}.awaitAll()

[Daniel-ADFA]

closing this in favour of #748