ADFA-4326: Guard uninitialized projectPath on process-death recreation#1414
Conversation
📝 Walkthrough
Risks / best-practice notes:
WalkthroughThe PR makes project-path restoration resilient during editor recreation, avoids uninitialized project-path crashes in project accessors, and updates editor name lookup to return an empty string when no project path is available. ChangesProject path recovery
Sequence Diagram(s)sequenceDiagram
participant BaseEditorActivity
participant GeneralPreferences
participant ProjectManagerImpl
participant MainActivity
BaseEditorActivity->>GeneralPreferences: read lastOpenedProject when saved state and intent are blank
BaseEditorActivity->>ProjectManagerImpl: assign restored projectPath
BaseEditorActivity->>ProjectManagerImpl: read projectDirPath after super.onCreate
alt projectDirPath is blank
BaseEditorActivity->>MainActivity: navigate back
BaseEditorActivity->>BaseEditorActivity: finish()
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Sentry APPDEVFORALL-QZ. Restore ProjectManagerImpl.projectPath from savedInstanceState -> intent PROJECT_PATH extra -> lastOpenedProject; guard projectDirPath getter; route back to MainActivity if still unset. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds ProjectManagerImplUninitializedPathTest covering the root cause of the EditorActivity crash after process-death recreation: reading ProjectManagerImpl.projectDirPath while the lateinit projectPath is unset. Pre-fix the getter delegated directly to the lateinit (get() = projectPath), throwing UninitializedPropertyAccessException; the fix guards with isInitialized and returns "" so the caller can route back to MainActivity. The test fails (UninitializedPropertyAccessException) on the pre-fix baseline and passes on the fix branch. A third case asserts the getter still reflects an assigned path, guarding against an always-blank regression. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
c2b5184 to
ea4b254
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt (1)
645-652: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the
"PROJECT_PATH"intent-extra key into a shared constant.This literal is the producer/consumer contract with
MainActivity(putExtra("PROJECT_PATH", ...)atMainActivity.kt:427). Duplicating the raw string in both places risks silent drift if either side is renamed. Consider a singleconst valreferenced by both.🤖 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/activities/editor/BaseEditorActivity.kt` around lines 645 - 652, The PROJECT_PATH intent extra is duplicated as a raw string in the editor restore flow, which can drift from the sender in MainActivity. Introduce a shared constant for the intent-extra key and update BaseEditorActivity’s restoredProjectPath lookup and MainActivity’s putExtra call to use that symbol consistently, so the producer/consumer contract stays centralized and easy to rename safely.
🤖 Prompt for all review comments with 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.
Nitpick comments:
In
`@app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt`:
- Around line 645-652: The PROJECT_PATH intent extra is duplicated as a raw
string in the editor restore flow, which can drift from the sender in
MainActivity. Introduce a shared constant for the intent-extra key and update
BaseEditorActivity’s restoredProjectPath lookup and MainActivity’s putExtra call
to use that symbol consistently, so the producer/consumer contract stays
centralized and easy to rename safely.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 248653b1-f8b7-4793-92e8-c7b478044b1c
📒 Files selected for processing (4)
app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.ktapp/src/main/java/com/itsaky/androidide/viewmodel/EditorViewModel.ktsubprojects/projects/src/main/java/com/itsaky/androidide/projects/ProjectManagerImpl.ktsubprojects/projects/src/test/java/com/itsaky/androidide/projects/ProjectManagerImplUninitializedPathTest.kt
Jira Ticket: https://appdevforall.atlassian.net/browse/ADFA-4326
Sentry Issue: https://appdevforall-inc-9p.sentry.io/issues/APPDEVFORALL-QZ
Reproduction Details
After process death the OS can recreate
EditorActivitywithout routing throughMainActivity, leavingProjectManagerImpl'slateinit projectPathunset.EditorActivityreadsprojectDirPathduringonCreate(viaEditorViewModel.getProjectName → projectDir → projectDirPath); the getter delegated straight to the lateinit and threwUninitializedPropertyAccessException, failing activity start.Stack Trace
User Steps
User steps leading up to crash, based on Sentry breadcrumbs:
BaseApplication: Creating safe context because user is not unlocked→ direct-boot / credential-protected storage), thenEditorActivity/EditorSidebarFragmentare recreated with no project context → crash on firstprojectPathaccess.Was able to reproduce in a unit test?
Yes.
ProjectManagerImplUninitializedPathTest(:subprojects:projects, Robolectric) constructs a freshProjectManagerImpl(projectPath never set = the process-death state) and readsprojectDirPath/projectDir. Baseline: 2/3 cases FAIL withUninitializedPropertyAccessException(the 3rd, initialized, passes — not a tautology). Branch: 3/3 pass.What Was Fixed
Guarded getter —
get() = if (this::projectPath.isInitialized) projectPath else ""— andEditorViewModel/BaseEditorActivityroute back toMainActivityinstead of crashing when there's no project context.Testing
:subprojects:projects:testV8DebugUnitTest→ 3/3 green (red on baseline). Local CodeRabbit review: no findings. Reviewer notes (local): thefinish()+returnonly exitsBaseEditorActivity.onCreate— subclassonCreate(services/LSP/initializeProject) still runs on the finishing activity; consider short-circuiting earlier /isFinishingguards. TheEditorViewModelblank-path guard is redundant with the getter fix. Recommend an android-qa smoke of the process-death → editor-recreation path before close.Fixes APPDEVFORALL-QZ