Meshtastic-Android/workpad.md

Craft Workpad: Permissions Checks UX Audit & Improvements

Generated by /craft · Started: 2026-06-18

---

Task

Check all of our permissions checks UX, ensure that we're appropriately presenting error states with corrective/recovery actions and context for the user. Research docs, ensure we're using documented guidance and best practice for Android permissions.

---

Exploration Report

Key Facts (most actionable first)

• shouldShowRequestPermissionRationale is used NOWHERE for permissions (verified: every shouldShow* hit in the repo is unrelated node-list display prefs). The app cannot distinguish "first request" from "permanently denied," which is the central Android best-practice gap (docs checklist items #2, #6, #7).
• The app is mid-migration off Accompanist, not done. #5211 ("eliminate Accompanist permissions") was partial. Native ActivityResultContracts wrappers live in core/ui/src/androidMain/.../PlatformUtils.kt, but Accompanist rememberPermissionState/rememberMultiplePermissionsState is STILL used in: feature/intro/.../AppIntroductionScreen.kt + AndroidIntroPermissions.kt, androidApp/src/google/.../map/MapView.kt, androidApp/src/fdroid/.../map/MapView.kt, core/barcode/.../BarcodeScannerProvider.kt. Catalog still pins accompanist = "0.37.3" (gradle/libs.versions.toml:7,248) and build-logic/.../KmpFeatureConventionPlugin.kt:68 injects accompanist-permissions into EVERY KMP feature module.
• No centralized permission abstraction. Each feature handles its own: intro (BT/loc/notif), TAK settings (local-network), connections (USB bonding), barcode (camera), map (location). No registry, no shared rationale/recovery component.
• Denial handling is inconsistent across features: Compass = best-in-class (distinct warning cards + recovery buttons); Map location button = snackbar feedback (map_location_unavailable, added in 4607c3f22); TAK server = silent auto-disable on denial; BT bonding = SecurityException→ service error message; USB = log only, no UI; Barcode camera = no error state if denied.
• Compass is the reference pattern to generalize — feature/node/src/commonMain/.../CompassSheetContent.kt:168-226 shows CompassWarning enum (NO_LOCATION_PERMISSION, LOCATION_DISABLED, NO_MAGNETOMETER, NO_LOCATION_FIX) with error-container cards, icons, and context-specific action buttons (request permission vs. open location settings).
• GPS-disabled is correctly distinguished from permission-denied only in Compass + google-flavor LocationHandler. androidApp/src/google/.../LocationHandler.kt uses Play Services LocationSettingsRequest resolution; ContextServices.kt exposes hasGps()/gpsDisabled()/hasLocationPermission(). Other location entry points conflate the two (docs checklist #26).
• Settings deep-links exist and use correct intents: Settings.ACTION_APPLICATION_DETAILS_SETTINGS (AndroidIntroSettingsNavigator.kt:26, AppInfoSection.kt), ACTION_LOCATION_SOURCE_SETTINGS (compass), ACTION_APP_NOTIFICATION_SETTINGS, ACTION_NFC_SETTINGS. But they are only reachable from the intro flow and the settings screen — NOT offered at the point of a mid-app permanent denial.
• rememberOnResumeState (PlatformUtils.kt:313-317) re-checks permission on ON_RESUME — correct per docs (return-from-settings refresh). Good pattern, already in place.
• API gating is solid: BT split perms gated >= S with neverForLocation; legacy BLUETOOTH/BLUETOOTH_ADMIN capped maxSdkVersion=30; POST_NOTIFICATIONS gated >= TIRAMISU; ACCESS_LOCAL_NETWORK gated API 37 (hardcoded LOCAL_NETWORK_PERMISSION_API = 37 constant, no named SDK constant yet). minSdk=26, targetSdk/compileSdk=37.
• Intro flow batch-requests BT + Location + Notifications up-front at first launch (AppIntroductionScreen.kt:40-83) — this is the up-front-batch anti-pattern Google warns against (docs anti-patterns), though it is skippable and screens carry rationale text. Camera (barcode) is NOT in intro — correctly requested in-context.
• Location requests FINE+COARSE together (PlatformUtils.kt:184-205, intro) — correct for the Android 12+ approximate/precise toggle (docs #22). But no code path treats a coarse-only grant as acceptable degraded mode (docs #23).
• USB permission uses intent-broadcast flow (UsbManager.kt:33-57, AndroidScannerViewModel.kt:95-108), denial logged only — no user-facing recovery (docs #4).
• FGS types declared (connectedDevice|location) with matching FOREGROUND_SERVICE_* perms; no explicit FGS-permission runtime request found (relies on the underlying runtime perms being granted before startForeground).

Best-Practice Checklist (from official docs research — see Agent docs report)

Sources: developer.android.com/training/permissions/requesting, .../explaining-access, .../privacy-and-security/minimize-permission-requests, .../develop/connectivity/bluetooth/bt-permissions, .../training/location/permissions, .../develop/ui/views/notifications/notification-permission, .../privacy-and-security/local-network-permission, .../develop/background-work/services/fgs/declare, google.github.io/accompanist/permissions/.

High-yield audit gaps for this app:

1. Detect permanent denial (persist a "has-been-requested" flag + shouldShowRequestPermissionRationale) and offer a user-initiated settings deep-link recovery on permanent denial. (#6,#7,#8)
2. Request in-context per feature rather than batch-up-front. (#9)
3. Function on coarse-only location grants. (#23)
4. Treat GPS-disabled vs permission-denied distinctly everywhere location is used. (#26)
5. Consistent graceful-degradation messaging on every denial path (USB, barcode currently silent). (#4)
6. Don't link to settings to "pressure" the user — recovery must be user-initiated, framed as help. (anti-pattern)

Code Snippets (exact)

Native wrapper, no rationale/permanent-denial branch — core/ui/src/androidMain/kotlin/org/meshtastic/core/ui/util/PlatformUtils.kt:218-236

@Composable
actual fun rememberRequestBluetoothPermission(onGranted: () -> Unit, onDenied: () -> Unit): () -> Unit {
    if (android.os.Build.VERSION.SDK_INT < android.os.Build.VERSION_CODES.S) {
        return remember { { onGranted() } }
    }
    val currentOnGranted = rememberUpdatedState(onGranted)
    val currentOnDenied = rememberUpdatedState(onDenied)
    val launcher =
        rememberLauncherForActivityResult(ActivityResultContracts.RequestMultiplePermissions()) { permissions ->
            if (permissions.values.all { it }) currentOnGranted.value() else currentOnDenied.value()
        }
    return remember(launcher) {
        { launcher.launch(arrayOf(BLUETOOTH_SCAN, BLUETOOTH_CONNECT)) }
    }
}

(onDenied is a single binary callback — no way for callers to know if denial is recoverable or permanent.)

Reference recovery UX (Compass) — feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/component/CompassSheetContent.kt:212-224

if (warnings.contains(CompassWarning.NO_LOCATION_PERMISSION)) {
    Button(onClick = onRequestPermission, modifier = Modifier.fillMaxWidth()) {
        Icon(MeshtasticIcons.MyLocation, contentDescription = null)
        Text(stringResource(Res.string.compass_no_location_permission))
    }
} else if (warnings.contains(CompassWarning.LOCATION_DISABLED)) {
    Button(onClick = onOpenLocationSettings, modifier = Modifier.fillMaxWidth()) {
        Icon(MeshtasticIcons.MyLocation, contentDescription = null)
        Text(stringResource(Res.string.compass_location_disabled))
    }
}

Still-on-Accompanist intro — feature/intro/src/androidMain/kotlin/org/meshtastic/feature/intro/AppIntroductionScreen.kt:45-61

val notificationPermissionState: PermissionState? =
    if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU)
        rememberPermissionState(Manifest.permission.POST_NOTIFICATIONS) else null
val locationPermissionState = rememberMultiplePermissionsState(permissions = locationPermissions)
val bluetoothPermissionState = rememberMultiplePermissionsState(permissions = bluetoothPermissions)

Key Files

• core/ui/src/androidMain/kotlin/org/meshtastic/core/ui/util/PlatformUtils.kt (184-317) — central native permission launchers + rememberOnResumeState refresh; settings deep-links. The natural home for a shared rationale/recovery helper.
• core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/util/PlatformUtils.kt — expect/actual contract; JVM/iOS stubs.
• feature/intro/src/commonMain/.../IntroPermissions.kt + androidMain/.../AndroidIntroPermissions.kt, AppIntroductionScreen.kt — intro flow, still Accompanist; PermissionScreenLayout.kt + BluetoothScreen/LocationScreen/NotificationsScreen.kt rationale screens.
• feature/node/src/commonMain/.../component/CompassSheetContent.kt — reference warning/recovery UX to generalize.
• androidApp/src/google/.../map/MapView.kt + androidApp/src/fdroid/.../map/MapView.kt — Accompanist location gating; feature/map/.../MapScreen.kt:265 snackbar feedback.
• androidApp/src/google/.../map/LocationHandler.kt — Play Services location-settings resolution (GPS vs permission).
• core/barcode/src/main/.../BarcodeScannerProvider.kt (68-105) — Accompanist camera, no denial UI.
• feature/settings/src/.../tak/TakPermissionUtil.kt + radio/component/TAKConfigItemList.kt:172 — local-network on-demand + auto-disable; test TAKConfigPermissionDeniedTest.kt.
• feature/connections/src/androidMain/.../AndroidScannerViewModel.kt (75-108) — BT bonding SecurityException + USB permission (log-only).
• core/common/src/androidMain/.../ContextServices.kt — hasGps()/gpsDisabled()/hasLocationPermission().
• androidApp/src/main/AndroidManifest.xml — all perm declarations with API gating + neverForLocation.
• gradle/libs.versions.toml:7,248 + build-logic/.../KmpFeatureConventionPlugin.kt:68 — Accompanist still pinned + injected into every KMP feature module.

Open contradiction resolved

Agent C reported Accompanist fully removed (#5211); Agent E reported it still present. Verified via grep at HEAD: BOTH partially right — removal was incomplete. Accompanist remains in intro/map/barcode + the feature convention plugin; native APIs cover the PlatformUtils-based paths only.

---

Requirements

Scope decision (human): Comprehensive standardization of permission UX + full Accompanist→native migration. Onboarding stays skippable AND features additionally request in-context when a permission isn't granted.

R1: A reusable permission-request component in core/ui detects permanent denial by persisting a "has-been-requested" flag and combining it with shouldShowRequestPermissionRationale, exposing three states to callers: not-yet-requested, denied-can-retry (show rationale), and permanently-denied (offer Settings recovery). Source: human confirmed Verification: Unit/behavior test asserting the state machine returns permanently-denied only when flag-set AND shouldShowRequestPermissionRationale==false; review confirms it lives in core/ui and is the single source for all paths.

R2: All runtime-permission paths are migrated off Accompanist to native androidx.activity ActivityResultContracts APIs; accompanist-permissions is removed from gradle/libs.versions.toml, build-logic/.../KmpFeatureConventionPlugin.kt, and any module build.gradle.kts that declared it. Source: human confirmed Verification: grep -rn "accompanist" --include=*.kt --include=*.toml --include=*.gradle.kts returns no permission usages; app compiles for google + fdroid flavors.

R3: Every runtime-permission denial path presents the user with context (why the feature needs it) and a corrective/recovery action — no silent failures. Specifically, the currently-silent USB-device and barcode/camera paths gain user-facing denial feedback. Source: human confirmed Verification: Review enumerates each permission call site and confirms each has a denied-state UI; USB and barcode paths show a message + action.

R4: On permanent denial, the app offers a user-initiated "Open app settings" recovery (Settings.ACTION_APPLICATION_DETAILS_SETTINGS) and re-checks permission state on ON_RESUME when the user returns; recovery is framed as help, never as repeated nagging. Source: human confirmed (aligns with docs items #6/#7/#8) Verification: Review confirms recovery is shown only in the permanently-denied state and re-check happens on resume; no auto-re-launch loops.

R5: At every location entry point, "location services (GPS) disabled" is treated distinctly from "permission denied," routing GPS-disabled to Settings.ACTION_LOCATION_SOURCE_SETTINGS (generalizing the existing Compass pattern) rather than re-prompting the permission. Source: craft-clarify recommendation (docs item #26) Verification: Review confirms each location consumer distinguishes the two states with distinct copy/actions.

R6: Features requiring a permission request it in-context (with rationale) at the point of use when not already granted; the onboarding/intro flow remains present and skippable. Source: human confirmed ("onboarding should be skippable, but the user should still be presented with permissions in-context if not granted") Verification: Review confirms feature entry points (BT scan/connect, location share/track, notifications, camera, local-network) trigger an in-context request when ungranted, and intro flow still skippable.

R7: Permission denials degrade gracefully without blocking the whole UI; where a permission grants only coarse/approximate location, location-dependent features still function in degraded mode rather than hard-failing on FINE. Source: craft-clarify recommendation (docs items #4/#23) Verification: Review confirms no full-screen blockers on denial and coarse-only grants are accepted where feasible.

R8: All new/changed user-facing permission strings are added as string resources (no hardcoded UI text), and python3 scripts/sort-strings.py is run; the BT-bonding SecurityException message (currently a hardcoded English string in AndroidScannerViewModel) is converted to a resource. Source: craft-clarify recommendation (project convention) Verification: grep for new literals in changed files; strings index updated; sort script produces no diff.

R9: Existing good patterns are generalized, not regressed: Compass warning/recovery cards, rememberOnResumeState refresh, TAK auto-disable-on-denial, and all API gating (BT >=S + neverForLocation, POST_NOTIFICATIONS >=33, ACCESS_LOCAL_NETWORK API 37, legacy maxSdkVersion=30) continue to work. Source: craft-clarify recommendation Verification: Existing tests (TAKConfigPermissionDeniedTest) still pass; manifest gating unchanged; Compass behavior preserved.

---

Architectural Decision

• Chosen approach: Pragmatic, enriched — extend the existing core/ui PlatformUtils expect/actual seam.
• Core idea: Add a four-state PermissionStatus enum + companion reactive rememberXxxPermissionState() composables (recomputed on ON_RESUME), backed by a synchronous SharedPreferences "has-requested" flag in core/ui androidMain + shouldShowRequestPermissionRationale, with one shared PermissionRecoveryCard lifted from Compass; migrate the 4 Accompanist sites and remove Accompanist.
• Approved: 2026-06-18

Requirement Coverage

• R1: permanent-denial detection — pure computePermissionStatus(granted, hasRequested, shouldShowRationale) in core/ui androidMain + PermissionRequestTracker (SharedPreferences). Single source.
• R2: full Accompanist removal — KmpFeatureConventionPlugin.kt:68, libs.versions.toml:7,248, core/barcode + androidApp build.gradle.kts; migrate intro/map×2/barcode; promote CAMERA to native launcher.
• R3: no silent denials — PermissionRecoveryCard in barcode dialog; USB else-branch + BT-bonding → setErrorMessage.
• R4: rememberOpenAppSettings() (ACTION_APPLICATION_DETAILS_SETTINGS) shown only in PERMANENTLY_DENIED; rememberOnResumeState recheck; no auto-relaunch.
• R5: GPS-disabled distinct via existing isGpsDisabled() + rememberOpenLocationSettings() (ACTION_LOCATION_SOURCE_SETTINGS); generalize Compass pattern.
• R6: in-context rememberXxxPermissionState() at feature entry points; intro keeps skippable flow.
• R7: location GRANTED on any { it } (coarse OK); cards are inline, never full-screen blockers.
• R8: new string resources + scripts/sort-strings.py; BT-bonding literal → resource.
• R9: Compass card lifted (behavior preserved); TAK auto-disable, API gating, rememberOnResumeState untouched.

Key trade-offs resolved

• Reactive rememberXxxPermissionState() (works at-rest) chosen over Minimal's post-request-only onResult callback.
• Flag in synchronous SharedPreferences (core/ui androidMain) chosen over core/prefs DataStore — avoids async read-after-write race vs the rationale read; flag is Android-only anyway.
• No central PermissionController/Permission registry (Robust) — keeps the per-feature expect/actual grain and bounded diff.
• Old rememberRequestXxx/isXxx wrappers kept during migration; deletion sequenced to a follow-up PR.

Deferred to Implementation

• Delete unused old rememberRequestXxx/isXxx wrappers — follow-up PR once all callers migrated.
• Possibly fold the SharedPreferences flag into core/prefs later — only if a synchronous accessor is added there.
• Whether to introduce a light Permission enum carrying rationale StringResources vs passing per-call-site — implement may decide; default is per-call-site.

Adversarial finding

• Severity: P1 — A global per-permission "has-requested" boolean set when launch() is called (not when the OS prompts) can misclassify a first-run user as PERMANENTLY_DENIED (backgrounding mid-dialog, OEM group auto-deny, or pre-12 BLE short-circuit).
• Addressed by: (1) set the flag in the launcher result callback, not at launch(); (2) pre-Android-12 rememberBluetoothPermissionState() delegates to location status (not bare GRANTED) so the correct recovery card shows; (3) explicit unit-test case (false, true, false) → PERMANENTLY_DENIED + documented invariant that hasRequested is set only from a completed request.

Files to create/modify

• core/ui/src/commonMain/.../util/PermissionStatus.kt (new) — enum + PermissionUiState holder.
• core/ui/src/commonMain/.../util/PlatformUtils.kt — add expect rememberXxxPermissionState() (bluetooth/location/notification/localNetwork/camera) + rememberOpenAppSettings().
• core/ui/src/androidMain/.../util/PlatformUtils.kt — actuals; pure computePermissionStatus; flag write in result callback; LocalActivity.current; pre-12 BT→location delegation; rememberOnResumeState made internal.
• core/ui/src/androidMain/.../util/PermissionRequestTracker.kt (new) — synchronous SharedPreferences flag.
• core/ui/src/commonMain/.../component/PermissionRecoveryCard.kt (new) — shared recovery card.
• core/ui/src/jvmMain + iosMain PlatformUtils.kt — trivial GRANTED stubs.
• build-logic/.../KmpFeatureConventionPlugin.kt, gradle/libs.versions.toml, core/barcode/build.gradle.kts, androidApp/build.gradle.kts — remove Accompanist.
• feature/intro/.../AndroidIntroPermissions.kt + AppIntroductionScreen.kt, androidApp/src/google + src/fdroid MapView.kt, core/barcode/.../BarcodeScannerProvider.kt — migrate off Accompanist.
• feature/connections/.../AndroidScannerViewModel.kt — USB + BT-bonding strings/messages.
• feature/node/.../component/CompassBottomSheet.kt — refactor onto shared card.
• core/resources/.../strings.xml — new strings + sort-strings.py.

Test strategy

• New unit test: table-drive computePermissionStatus(...) over all 8 input combinations incl. the P1 case + invariant.
• Regression: TAKConfigPermissionDeniedTest, IntroViewModelTest stay green.
• Build gates (gradle-runner): :core:ui:kmpSmokeCompile, assembleGoogleDebug + assembleFdroidDebug, detekt, spotlessCheck.
• Manual (PR Testing Performed): deny→permanent-deny camera/location/BT/notifications on API 33+; recovery card → settings → ON_RESUME recheck.

---

Acceptance Criteria

AC1: [R1] A pure computePermissionStatus(granted, hasRequested, shouldShowRationale) returns PERMANENTLY_DENIED only when !granted && hasRequested && !shouldShowRationale; NOT_REQUESTED when !hasRequested; DENIED_CAN_RETRY when hasRequested && shouldShowRationale; GRANTED when granted. Lives once in core/ui. Auto-verify: test PermissionStatusTest (table over 8 combos incl. the P1 case)

AC2: [R2] No Accompanist permissions usage remains in app code or build config. Auto-verify: grep -rn "accompanist" --include=.kt --include=.toml --include=*.gradle.kts returns no permission references; assembleGoogleDebug + assembleFdroidDebug compile

AC3: [R3] Barcode-camera denial and USB-permission denial present a user-facing message/recovery (no silent fail); BT-bonding error surfaced. Auto-verify: grep confirms PermissionRecoveryCard in BarcodeScannerProvider + setErrorMessage on USB branch in AndroidScannerViewModel; [MANUAL-VERIFY] runtime UI

AC4: [R4] Permanent-denial state offers an "Open settings" action (ACTION_APPLICATION_DETAILS_SETTINGS); state re-checks on ON_RESUME; no auto-relaunch loop. Auto-verify: grep ACTION_APPLICATION_DETAILS_SETTINGS in rememberOpenAppSettings; card shows settings button only for PERMANENTLY_DENIED; [MANUAL-VERIFY] resume recheck

AC5: [R5] Location entry points route GPS-disabled (isGpsDisabled) to ACTION_LOCATION_SOURCE_SETTINGS, distinct from permission-denied. Auto-verify: review each Permission.LOCATION consumer; [MANUAL-VERIFY]

AC6: [R6] Feature entry points request in-context when ungranted; intro flow stays skippable. Auto-verify: grep rememberXxxPermissionState at map/barcode/connections; IntroViewModelTest passes; [MANUAL-VERIFY] skippable

AC7: [R7] Location grant succeeds on coarse-only (any granted); recovery UI is inline, not a full-screen blocker. Auto-verify: code uses any { it }; review confirms inline cards; [MANUAL-VERIFY]

AC8: [R8] New user-facing strings are resources; BT-bonding literal converted; sort-strings.py produces no diff. Auto-verify: grep for new literals in changed .kt; python3 scripts/sort-strings.py clean

AC9: [R9] Compass behavior preserved (magnetometer/fix warnings intact); TAK auto-disable + API gating + rememberOnResumeState unchanged. Auto-verify: TAKConfigPermissionDeniedTest passes; manifest diff empty; review Compass parity

---

Implementation Plan

Branch: claude/gallant-thompson-d1b2ea Started: 2026-06-18 Base commit: 1125172c46

Task list

• core/ui/.../util/PermissionStatus.kt (new, commonMain) — enum + computePermissionStatus (pure) + PermissionUiState
• core/ui/.../util/PlatformUtils.kt (commonMain) — add expect rememberXxxPermissionState() + rememberOpenAppSettings()
• core/ui/.../util/PermissionRequestTracker.kt (new, androidMain) — synchronous SharedPreferences flag
• core/ui/.../util/PlatformUtils.kt (androidMain) — actuals; flag write in result callback; LocalActivity; pre-12 BT→location delegation
• core/ui/.../util/PlatformUtils.kt (jvmMain) + NoopStubs.kt (iosMain) — GRANTED stubs
• core/ui/.../component/PermissionRecoveryCard.kt (new, commonMain) — shared card
• core/ui/.../util/PermissionStatusTest.kt (new, commonTest) — table test (R1 anchor)
• core/resources/.../strings.xml — new strings + sort-strings.py
• core/barcode/.../BarcodeScannerProvider.kt — migrate camera off Accompanist + denial card
• feature/intro/.../AndroidIntroPermissions.kt + AppIntroductionScreen.kt — migrate off Accompanist
• androidApp/src/google + src/fdroid MapView.kt — migrate off Accompanist
• feature/connections/.../AndroidScannerViewModel.kt — USB + BT-bonding strings/messages
• feature/node/.../component/CompassBottomSheet.kt — refactor onto shared card (preserve behavior)
• Remove Accompanist: KmpFeatureConventionPlugin.kt, libs.versions.toml, core/barcode/build.gradle.kts, androidApp/build.gradle.kts

Pre-existing failures (do not fix — out of scope)

---

Completion Bar

All 6 must be checked before handing off to review.

1. All planned files created/modified (task list fully checked)
2. Linter clean — zero new errors (pre-existing errors logged above)
3. Tests pass — new failures logged as pre-existing above
4. Every AC has a completion note
5. No open markers remain ([NEEDS CLARIFICATION], [TODO], [TODO-IMPL], [TBD])
6. Scope discipline honored — discovered improvements in [DEFERRED] items only

---

Review

Populated by /craft-review.

Reviewers activated: [list]

Findings

---

Deferred Items

Accumulated across all phases.

---

Phase Log

• explore: done — 2026-06-18 — 5 agents (arch, change-surface, history, deps, docs-research) + file verification. Resolved Accompanist contradiction: partial migration. Central gap: no shouldShowRequestPermissionRationale/permanent-denial handling anywhere.
• clarify: done — 2026-06-18 — Comprehensive scope confirmed. Shared recovery helper + full Accompanist migration + hybrid intro (skippable + in-context). R1–R9 recorded.
• architect: done — 2026-06-18 — Pragmatic+enriched approved. 3 architect agents + adversarial (P1, mitigations folded in). PlatformUtils seam + status enum + SharedPreferences flag + shared recovery card.
• implement:
• review:
• refine:
• pr: