ADFA-3489 | Fix onboarding blocked by unsupported overlay permission#1157
Merged
ADFA-3489 | Fix onboarding blocked by unsupported overlay permission#1157
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an overlay-permission state enum and helper, gates debug pre-exec on overlay state with an early-return/request flow, updates onboarding and overlay manager to distinguish unsupported vs requestable overlays, and adjusts onboarding UI to disable unsupported overlay actions. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Activity
participant DebugAction
participant PermissionsHelper
participant SystemSettings
participant Shizuku
User->>Activity: trigger debug action
Activity->>DebugAction: preExec()
DebugAction->>PermissionsHelper: getOverlayPermissionState(activity)
PermissionsHelper-->>DebugAction: OverlayPermissionState
alt GRANTED
DebugAction->>Shizuku: pingBinder()
Shizuku-->>DebugAction: response
DebugAction->>Activity: proceed with debug flow
else REQUESTABLE
DebugAction->>SystemSettings: launch ACTION_MANAGE_OVERLAY_PERMISSION (package/data)
SystemSettings-->>PermissionsHelper: user toggles overlay permission
PermissionsHelper-->>Activity: updated overlay state / notify
else UNSUPPORTED
DebugAction-->>Activity: flash unsupported hint / Toast
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/com/itsaky/androidide/actions/build/DebugAction.kt`:
- Around line 103-108: The preExec() path in DebugAction.kt currently requests
overlay permission via PermissionsHelper.requestOverlayPermission(...) and
returns false if not granted, but never wires up
PermissionsHelper.handlePostOverlayPermissionRequest(...) or a resume hook so
the flow can continue when the user returns from settings; add an
awaiting-overlay-result mechanism: when requestOverlayPermission triggers an
external settings flow from preExec() (in DebugAction.preExec), set a resume
flag or register a continuation (e.g., awaitingOverlayGrantResult tied to the
DebugAction instance) and ensure
PermissionsHelper.handlePostOverlayPermissionRequest(...) is invoked on activity
resume to clear the flag and re-run or schedule preExec()'s post-permission
logic (or surface the fallback UI on the next invocation) so restricted-settings
fallback (hint/App info) is reached instead of aborting immediately.
In `@app/src/main/java/com/itsaky/androidide/utils/PermissionsHelper.kt`:
- Around line 4-13: In PermissionsHelper.kt replace the broad catch(Throwable)
used when launching the settings/activity intent with a catch specifically for
android.content.ActivityNotFoundException so only the expected "activity not
found" case is handled; update the catch block in the method that opens the app
settings (the function that builds and starts the Intent to Settings and
currently swallows all Throwables) to catch ActivityNotFoundException and
handle/log it, removing the generic Throwable catch so other runtime errors
surface.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f5f732d1-8b18-44e4-8879-bcc0380cdcf3
📒 Files selected for processing (5)
app/src/main/java/com/itsaky/androidide/actions/build/DebugAction.ktapp/src/main/java/com/itsaky/androidide/fragments/onboarding/PermissionsFragment.ktapp/src/main/java/com/itsaky/androidide/services/debug/DebugOverlayManager.ktapp/src/main/java/com/itsaky/androidide/utils/PermissionsHelper.ktresources/src/main/res/values/strings.xml
app/src/main/java/com/itsaky/androidide/actions/build/DebugAction.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/itsaky/androidide/utils/PermissionsHelper.kt
Outdated
Show resolved
Hide resolved
jatezzz
force-pushed
the
fix/ADFA-3489-overlay-permissions
branch
from
29de3b1 to
7f6e464
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/main/java/com/itsaky/androidide/utils/PermissionsHelper.kt (1)
142-149: Reasonable heuristic for detecting unsupported overlay devices.The check for Android 10 + low RAM (Go edition-like) devices is a pragmatic approach. Note that this heuristic may not catch all devices where overlay permission is disabled or problematic, but it addresses the primary reported issue. Consider adding a comment documenting why this specific combination is targeted.
📝 Suggested documentation
+ /** + * Checks if overlay permission is supported on the device. + * Android 10 Go edition (low RAM) devices often have overlay permission + * disabled or restricted at the system level, causing onboarding to block. + */ private fun isOverlayPermissionSupported(context: Context): Boolean {🤖 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/utils/PermissionsHelper.kt` around lines 142 - 149, The isOverlayPermissionSupported(Context) function uses a heuristic that treats Android 10 (Build.VERSION_CODES.Q) devices with ActivityManager.isLowRamDevice as Go-like and returns false; add a concise comment above the isOverlayPermissionSupported method explaining the rationale (Android 10 + low-RAM/Go-like devices often have problematic overlay permission), mention this is a heuristic that may not cover all cases, and note why we check SDK == Q and isLowRamDevice via ActivityManager to aid future maintainers (refer to isOverlayPermissionSupported, isAndroid10, isLowRamDevice, and isAndroid10GoLike).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/src/main/java/com/itsaky/androidide/utils/PermissionsHelper.kt`:
- Around line 142-149: The isOverlayPermissionSupported(Context) function uses a
heuristic that treats Android 10 (Build.VERSION_CODES.Q) devices with
ActivityManager.isLowRamDevice as Go-like and returns false; add a concise
comment above the isOverlayPermissionSupported method explaining the rationale
(Android 10 + low-RAM/Go-like devices often have problematic overlay
permission), mention this is a heuristic that may not cover all cases, and note
why we check SDK == Q and isLowRamDevice via ActivityManager to aid future
maintainers (refer to isOverlayPermissionSupported, isAndroid10, isLowRamDevice,
and isAndroid10GoLike).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cd4e57f3-8c74-448a-abf2-e47b4cd6a49a
📒 Files selected for processing (5)
app/src/main/java/com/itsaky/androidide/actions/build/DebugAction.ktapp/src/main/java/com/itsaky/androidide/fragments/onboarding/PermissionsFragment.ktapp/src/main/java/com/itsaky/androidide/services/debug/DebugOverlayManager.ktapp/src/main/java/com/itsaky/androidide/utils/PermissionsHelper.ktresources/src/main/res/values/strings.xml
✅ Files skipped from review due to trivial changes (1)
- resources/src/main/res/values/strings.xml
🚧 Files skipped from review as they are similar to previous changes (3)
- app/src/main/java/com/itsaky/androidide/services/debug/DebugOverlayManager.kt
- app/src/main/java/com/itsaky/androidide/actions/build/DebugAction.kt
- app/src/main/java/com/itsaky/androidide/fragments/onboarding/PermissionsFragment.kt
jatezzz
force-pushed
the
fix/ADFA-3489-overlay-permissions
branch
from
7f6e464 to
3535221
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src/main/java/com/itsaky/androidide/adapters/onboarding/OnboardingPermissionsAdapter.kt (1)
67-74: Optional UX clarity: avoid “Grant” label when permission is unsupported.When
isSupportedOnDevice == false, consider a non-action label (e.g., “Unsupported”) to reduce ambiguity.🤖 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/adapters/onboarding/OnboardingPermissionsAdapter.kt` around lines 67 - 74, The button currently always shows the "Grant" label even when permission.isSupportedOnDevice is false; update the OnboardingPermissionsAdapter binding so that after setting isEnabled = permission.isSupportedOnDevice you set text to a non-action label when unsupported (e.g., context.getString(R.string.title_unsupported) or similar) instead of R.string.title_grant, and keep icon/iconTint/iconPadding/iconSize behavior consistent for the unsupported state so the UI clearly communicates that the permission cannot be granted.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/com/itsaky/androidide/utils/PermissionsHelper.kt`:
- Around line 29-35: Replace the unreliable isLowRamDevice check in the overlay
permission logic: instead of using ActivityManager.isLowRamDevice, build an
intent with ACTION_MANAGE_OVERLAY_PERMISSION for the app package and verify
intent.resolveActivity(packageManager) to determine if the settings page is
requestable; use Settings.canDrawOverlays(context) (or the existing
canDrawOverlays(context)) to detect current GRANTED state, and add an explicit
check for Android 10 Go edition to mark OverlayPermissionState.UNSUPPORTED;
update the return branches that reference canDrawOverlays(context) and
OverlayPermissionState to use these intent-resolvability and Go-edition checks.
---
Nitpick comments:
In
`@app/src/main/java/com/itsaky/androidide/adapters/onboarding/OnboardingPermissionsAdapter.kt`:
- Around line 67-74: The button currently always shows the "Grant" label even
when permission.isSupportedOnDevice is false; update the
OnboardingPermissionsAdapter binding so that after setting isEnabled =
permission.isSupportedOnDevice you set text to a non-action label when
unsupported (e.g., context.getString(R.string.title_unsupported) or similar)
instead of R.string.title_grant, and keep icon/iconTint/iconPadding/iconSize
behavior consistent for the unsupported state so the UI clearly communicates
that the permission cannot be granted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2ecfda09-f82b-4d71-bdcd-8ded1df3a42c
📒 Files selected for processing (7)
app/src/main/java/com/itsaky/androidide/actions/build/DebugAction.ktapp/src/main/java/com/itsaky/androidide/adapters/onboarding/OnboardingPermissionsAdapter.ktapp/src/main/java/com/itsaky/androidide/fragments/onboarding/PermissionsFragment.ktapp/src/main/java/com/itsaky/androidide/models/OnboardingPermissionItem.ktapp/src/main/java/com/itsaky/androidide/services/debug/DebugOverlayManager.ktapp/src/main/java/com/itsaky/androidide/utils/PermissionsHelper.ktresources/src/main/res/values/strings.xml
✅ Files skipped from review due to trivial changes (1)
- resources/src/main/res/values/strings.xml
🚧 Files skipped from review as they are similar to previous changes (3)
- app/src/main/java/com/itsaky/androidide/services/debug/DebugOverlayManager.kt
- app/src/main/java/com/itsaky/androidide/actions/build/DebugAction.kt
- app/src/main/java/com/itsaky/androidide/fragments/onboarding/PermissionsFragment.kt
app/src/main/java/com/itsaky/androidide/utils/PermissionsHelper.kt
Outdated
Show resolved
Hide resolved
jatezzz
requested review from
a team,
Daniel-ADFA,
dara-abijo-adfa,
itsaky-adfa and
jomen-adfa
jomen-adfa
approved these changes
Apr 7, 2026
jatezzz
force-pushed
the
fix/ADFA-3489-overlay-permissions
branch
from
73ff502 to
338ab77
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
app/src/main/java/com/itsaky/androidide/actions/build/DebugAction.kt (1)
109-111:⚠️ Potential issue | 🟠 MajorThe requestable path still has no post-return handling.
preExec()returnsfalseright after this helper, andAbstractCancellableRunAction.execAction()stops on that result. In theREQUESTABLEbranch, users who come back from Settings with the toggle still blocked never get any follow-up handling here; the next tap just reopens Settings. Wire a resume/post-request hook into this flow before returning.Also applies to: 134-140
🤖 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/actions/build/DebugAction.kt` around lines 109 - 111, preExec() exits early when overlayState != PermissionsHelper.OverlayPermissionState.GRANTED by calling handleMissingOverlayPermission(activity, overlayState) and returning false, but the REQUESTABLE branch lacks any post-request/resume handling so users returning from Settings receive no follow-up; update preExec() to register a resume/post-request callback (or enqueue a continuation) when handleMissingOverlayPermission is called for the REQUESTABLE case so that after the Settings flow finishes you re-check overlayState and proceed with the original exec path (mirroring the handling in AbstractCancellableRunAction.execAction()), and apply the same change for the other occurrence around the 134-140 block to ensure both requestable flows resume correctly.
🧹 Nitpick comments (2)
app/src/main/java/com/itsaky/androidide/actions/build/DebugAction.kt (1)
141-143: Catch the settings-launch failure specifically.The rest of this file already uses
ActivityNotFoundExceptionfor settings intents. CatchingExceptionhere will also swallow unrelated bugs in the overlay flow.♻️ Narrow the catch
- } catch (e: Exception) { + } catch (e: ActivityNotFoundException) {Based on learnings: in Kotlin code across AndroidIDE, prefer narrow exception handling that catches only the specific exception type reported in crashes instead of a broad catch-all.
🤖 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/actions/build/DebugAction.kt` around lines 141 - 143, The catch block in DebugAction.kt that wraps the overlay settings launch is too broad; replace the generic "catch (e: Exception)" with a narrow "catch (e: ActivityNotFoundException)" (importing android.content.ActivityNotFoundException if needed) so only the settings-intent failure is caught, keep the existing log.error(...) and activity.flashError(...) calls using the same exception variable to preserve logging and user feedback.app/src/main/java/com/itsaky/androidide/services/debug/DebugOverlayManager.kt (1)
115-124: Gateshow()from the centralized overlay state.This branch still decides eligibility from
Settings.canDrawOverlays(ctx)and only usesPermissionsHelperfor the message. ReusingOverlayPermissionStatefor both the guard and the toast keeps one source of truth if the helper’s heuristics change later.♻️ Minimal refactor
- if (!Settings.canDrawOverlays(ctx)) { + val state = PermissionsHelper.getOverlayPermissionState(ctx) + if (state != PermissionsHelper.OverlayPermissionState.GRANTED) { logger.warn("Overlay permission denied. Skipping debugger overlay window.") - - val state = PermissionsHelper.getOverlayPermissionState(ctx) val message = if (state == PermissionsHelper.OverlayPermissionState.UNSUPPORTED) { ctx.getString(R.string.permission_overlay_unsupported_hint) } else {🤖 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/services/debug/DebugOverlayManager.kt` around lines 115 - 124, The current guard in DebugOverlayManager.show() uses Settings.canDrawOverlays(ctx) while PermissionsHelper.getOverlayPermissionState(ctx) is only used for the message; change the eligibility check to use PermissionsHelper.getOverlayPermissionState(ctx) (the OverlayPermissionState enum) and gate the early return/logging on the non-GRANTED states so the same state drives both behavior and toast text; update references in the method (replace Settings.canDrawOverlays(ctx) with the state check, keep ctx.getString(...) logic using the same state value and leave logger.warn/message behavior intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@app/src/main/java/com/itsaky/androidide/actions/build/DebugAction.kt`:
- Around line 109-111: preExec() exits early when overlayState !=
PermissionsHelper.OverlayPermissionState.GRANTED by calling
handleMissingOverlayPermission(activity, overlayState) and returning false, but
the REQUESTABLE branch lacks any post-request/resume handling so users returning
from Settings receive no follow-up; update preExec() to register a
resume/post-request callback (or enqueue a continuation) when
handleMissingOverlayPermission is called for the REQUESTABLE case so that after
the Settings flow finishes you re-check overlayState and proceed with the
original exec path (mirroring the handling in
AbstractCancellableRunAction.execAction()), and apply the same change for the
other occurrence around the 134-140 block to ensure both requestable flows
resume correctly.
---
Nitpick comments:
In `@app/src/main/java/com/itsaky/androidide/actions/build/DebugAction.kt`:
- Around line 141-143: The catch block in DebugAction.kt that wraps the overlay
settings launch is too broad; replace the generic "catch (e: Exception)" with a
narrow "catch (e: ActivityNotFoundException)" (importing
android.content.ActivityNotFoundException if needed) so only the settings-intent
failure is caught, keep the existing log.error(...) and activity.flashError(...)
calls using the same exception variable to preserve logging and user feedback.
In
`@app/src/main/java/com/itsaky/androidide/services/debug/DebugOverlayManager.kt`:
- Around line 115-124: The current guard in DebugOverlayManager.show() uses
Settings.canDrawOverlays(ctx) while
PermissionsHelper.getOverlayPermissionState(ctx) is only used for the message;
change the eligibility check to use
PermissionsHelper.getOverlayPermissionState(ctx) (the OverlayPermissionState
enum) and gate the early return/logging on the non-GRANTED states so the same
state drives both behavior and toast text; update references in the method
(replace Settings.canDrawOverlays(ctx) with the state check, keep
ctx.getString(...) logic using the same state value and leave
logger.warn/message behavior intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e57f96b4-e572-4520-8306-9029904c98eb
📒 Files selected for processing (7)
app/src/main/java/com/itsaky/androidide/actions/build/DebugAction.ktapp/src/main/java/com/itsaky/androidide/adapters/onboarding/OnboardingPermissionsAdapter.ktapp/src/main/java/com/itsaky/androidide/fragments/onboarding/PermissionsFragment.ktapp/src/main/java/com/itsaky/androidide/models/OnboardingPermissionItem.ktapp/src/main/java/com/itsaky/androidide/services/debug/DebugOverlayManager.ktapp/src/main/java/com/itsaky/androidide/utils/PermissionsHelper.ktresources/src/main/res/values/strings.xml
✅ Files skipped from review due to trivial changes (2)
- resources/src/main/res/values/strings.xml
- app/src/main/java/com/itsaky/androidide/fragments/onboarding/PermissionsFragment.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/com/itsaky/androidide/utils/PermissionsHelper.kt
jatezzz
force-pushed
the
fix/ADFA-3489-overlay-permissions
branch
from
c4948e8 to
74bbdb6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR introduces an
OverlayPermissionStatecheck to gracefully handle devices (such as Android Go or low-RAM devices) that do not support theSYSTEM_ALERT_WINDOWpermission. It prevents users from getting stuck in an infinite onboarding loop by marking the permission as optional when unsupported, dynamically disabling the corresponding UI elements, and displaying informative hints instead of errors.Details
ActivityManager.isLowRamDeviceas the official heuristic to determine if overlays are unsupported. IntroducedOverlayPermissionState(GRANTED,REQUESTABLE,UNSUPPORTED) to cleanly map out scenarios.OnboardingPermissionsAdapternow dynamically applies a 38% alpha color state to gray out the permission title, description, and grant button if unsupported.DebugActionandDebugOverlayManagerwhen attempting to open the debugger on restricted devices.32 Bits device test
screen-20260407-131025.mp4
screen-20260407-133312.mp4
64 Bits device test
Screen.Recording.2026-04-07.at.1.22.55.PM.mov
Ticket
ADFA-3489
Observation
SYSTEM_ALERT_WINDOWis now evaluated dynamically. If it is disabled due to hardware restrictions, it correctly bypasses theareAllPermissionsGrantedblock, allowing users to proceed to the IDE seamlessly. Ensure any future overlay features also verifyPermissionsHelper.getOverlayPermissionStatebefore executing.