mirror/Meshtastic-Android
1
0
Fork 0
mirror of https://github.com/meshtastic/Meshtastic-Android.git synced 2026-06-27 23:15:35 -04:00
Code Issues Packages Projects Releases Wiki Activity

feat(core/ui): make BT/Wi-Fi adapter-state detection live

Browse Source 
Replace the ON_RESUME polling in isBluetoothDisabled()/isWifiUnavailable()
with real-time observers via a shared rememberObservedFlag primitive:

- Bluetooth: a BroadcastReceiver on BluetoothAdapter.ACTION_STATE_CHANGED
  (registered RECEIVER_NOT_EXPORTED; main-thread delivery)
- Wi-Fi/network: ConnectivityManager.registerDefaultNetworkCallback with a
  main-thread Handler, observing onAvailable/onLost/onCapabilitiesChanged

Both register/unregister with the composable's lifetime (DisposableEffect) and
re-seed the value at registration, so the Connections recovery banners now flip
live (e.g. toggling BT/Wi-Fi from the quick-settings shade) instead of only on
activity resume. isGpsDisabled() keeps its ON_RESUME behavior; public expect,
jvm/ios actuals, and ConnectionsScreen are unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
James Rich
2026-06-18 13:57:24 -05:00
parent 2c06a8019e
commit 174db32ae8
2 changed files with 140 additions and 318 deletions
Download Patch File Download Diff File Expand all files Collapse all files

95
core/ui/src/androidMain/kotlin/org/meshtastic/core/ui/util/PlatformUtils.kt
View File

@@ -18,14 +18,20 @@
package org.meshtastic.core.ui.util
import android.bluetooth.BluetoothAdapter
import android.bluetooth.BluetoothManager
import android.content.ActivityNotFoundException
import android.content.BroadcastReceiver
import android.content.Context
import android.content.Intent
import android.content.IntentFilter
import android.content.pm.PackageManager
import android.net.ConnectivityManager
import android.net.Network
import android.net.NetworkCapabilities
import android.net.Uri
import android.os.Handler
import android.os.Looper
import android.provider.Settings
import androidx.activity.compose.LocalActivity
import androidx.activity.compose.rememberLauncherForActivityResult
@@ -34,6 +40,7 @@ import androidx.compose.runtime.Composable
import androidx.compose.runtime.DisposableEffect
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.rememberUpdatedState
import androidx.compose.ui.platform.LocalContext
import androidx.compose.ui.platform.LocalView
import androidx.core.app.ActivityCompat
@@ -252,27 +259,63 @@ actual fun rememberOpenWifiSettings(): () -> Unit {
@Composable
actual fun isBluetoothDisabled(): Boolean {
val context = LocalContext.current
return rememberOnResumeState {
// adapter == null means the device has no Bluetooth at all — not "disabled", so don't nag.
val adapter = (context.getSystemService(Context.BLUETOOTH_SERVICE) as? BluetoothManager)?.adapter
adapter != null && !adapter.isEnabled
}
return rememberObservedFlag(
read = {
// adapter == null means the device has no Bluetooth at all — not "disabled", so don't nag.
val adapter = (context.getSystemService(Context.BLUETOOTH_SERVICE) as? BluetoothManager)?.adapter
adapter != null && !adapter.isEnabled
},
subscribe = { onChange ->
val receiver =
object : BroadcastReceiver() {
override fun onReceive(receiverContext: Context?, intent: Intent?) = onChange()
}
// ACTION_STATE_CHANGED is a protected system broadcast; NOT_EXPORTED keeps the receiver app-private.
// Registered without a Handler, so onReceive is delivered on the main thread.
ContextCompat.registerReceiver(
context,
receiver,
IntentFilter(BluetoothAdapter.ACTION_STATE_CHANGED),
ContextCompat.RECEIVER_NOT_EXPORTED,
)
val unregister = { context.unregisterReceiver(receiver) }
unregister
},
)
}
@Composable
actual fun isWifiUnavailable(): Boolean {
val context = LocalContext.current
return rememberOnResumeState {
val cm = context.getSystemService(Context.CONNECTIVITY_SERVICE) as? ConnectivityManager
val capabilities = cm?.activeNetwork?.let { cm.getNetworkCapabilities(it) }
val onLocalNetwork =
capabilities != null &&
(
capabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI) ||
capabilities.hasTransport(NetworkCapabilities.TRANSPORT_ETHERNET)
)
!onLocalNetwork
}
return rememberObservedFlag(
read = {
val cm = context.getSystemService(Context.CONNECTIVITY_SERVICE) as? ConnectivityManager
val capabilities = cm?.activeNetwork?.let { cm.getNetworkCapabilities(it) }
val onLocalNetwork =
capabilities != null &&
(
capabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI) ||
capabilities.hasTransport(NetworkCapabilities.TRANSPORT_ETHERNET)
)
!onLocalNetwork
},
subscribe = { onChange ->
val cm = context.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager
val callback =
object : ConnectivityManager.NetworkCallback() {
override fun onAvailable(network: Network) = onChange()
override fun onLost(network: Network) = onChange()
override fun onCapabilitiesChanged(network: Network, networkCapabilities: NetworkCapabilities) =
onChange()
}
// Main-thread Handler so the state write lands on the main thread (the API-26+ overload; minSdk is 26).
cm.registerDefaultNetworkCallback(callback, Handler(Looper.getMainLooper()))
val unregister = { cm.unregisterNetworkCallback(callback) }
unregister
},
)
}
@Composable
@@ -406,6 +449,26 @@ private fun rememberRuntimePermissionState(permissions: Array<String>, requireAl
return PermissionUiState(status = statusState.value, request = request, openAppSettings = openAppSettings)
}
/**
* Remembers a boolean derived from [read], kept live by an observer registered via [subscribe] for the duration of the
* composition. [subscribe] receives an `onChange` callback to invoke whenever the underlying state may have changed and
* must return a teardown function. The value is re-seeded via [read] at registration, so it is correct even before the
* first event arrives. Used for adapter/connectivity state that changes outside the activity lifecycle (e.g. toggling
* Bluetooth or Wi-Fi from the quick-settings shade).
*/
@Composable
private fun rememberObservedFlag(read: () -> Boolean, subscribe: (onChange: () -> Unit) -> () -> Unit): Boolean {
val currentRead = rememberUpdatedState(read)
val currentSubscribe = rememberUpdatedState(subscribe)
val state = remember { mutableStateOf(read()) }
DisposableEffect(Unit) {
state.value = currentRead.value()
val unsubscribe = currentSubscribe.value { state.value = currentRead.value() }
onDispose { unsubscribe() }
}
return state.value
}
/**
* Remembers a boolean state that is re-evaluated on each [Lifecycle.Event.ON_RESUME], ensuring the value stays fresh
* when the user returns from a permission dialog or system settings screen.

363
workpad.md
View File

@@ -1,373 +1,132 @@
# Craft Workpad: Permissions Checks UX Audit & Improvements
# Craft Workpad: Live BT/Wi-Fi adapter-state detection
> Generated by /craft · Started: 2026-06-18
> Generated by /craft · Started: 2026-06-18 · Branch: claude/gallant-thompson-d1b2ea (PR #5851)
---
## 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.
Make the Bluetooth/Wi-Fi adapter-disabled detection **live** instead of `ON_RESUME`-polled — add a `BroadcastReceiver` for Bluetooth adapter state and a `ConnectivityManager.NetworkCallback` for network availability, so the Connections recovery banners update in real time (e.g. toggling BT from the quick-settings shade) rather than only when the activity resumes.
Scope: `isBluetoothDisabled()` and `isWifiUnavailable()` in `core/ui/src/androidMain/.../util/PlatformUtils.kt`. Follow-up to the adapter-state feature added in commit 2c06a8019 on PR #5851.
---
## Exploration Report
### Key Facts (most actionable first)
### Key Facts
- **`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`
```kotlin
@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`
```kotlin
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`
```kotlin
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)
```
- Both target functions live in `core/ui/src/androidMain/.../util/PlatformUtils.kt`: `isBluetoothDisabled()` (reads `BluetoothManager.adapter.isEnabled`) and `isWifiUnavailable()` (reads `ConnectivityManager.activeNetwork` transports). Both currently wrap their read in the private `rememberOnResumeState { ... }` helper — recomputed only on `Lifecycle.Event.ON_RESUME`.
- `rememberOnResumeState(check)` is the only consumer-shared refresh primitive; also used by `isGpsDisabled()`. Changing the two BT/Wi-Fi functions must NOT change `isGpsDisabled()` behavior (out of scope).
- **NetworkCallback pattern already exists** in `core/network/.../ConnectivityManager.kt`: `observeNetworks()` uses `callbackFlow { … registerNetworkCallback(req, cb); awaitClose { unregisterNetworkCallback(cb) } }` with `onAvailable`/`onLost`/`onCapabilitiesChanged`. Mirror it with a `DisposableEffect` in the composable (or `registerDefaultNetworkCallback` for the single active network).
- **BroadcastReceiver pattern already exists** in `core/ble/.../AndroidBluetoothRepository.kt`: registers via `ContextCompat.registerReceiver(context, receiver, filter, ContextCompat.RECEIVER_NOT_EXPORTED)` and `context.unregisterReceiver(...)`. For adapter state, the action is `BluetoothAdapter.ACTION_STATE_CHANGED`.
- The two functions are consumed only by `ConnectionsScreen.kt` (hoisted as `bluetoothDisabled`/`wifiUnavailable` booleans driving inline `RecoveryCard` banners + the BLE toggle routing). No other callers — the public contract (`@Composable expect fun … : Boolean`) is unchanged; only the androidMain implementation changes.
- `androidApp/src/main/AndroidManifest.xml` already declares Bluetooth + network permissions; `ACTION_STATE_CHANGED` and a `NetworkCallback` need no extra permission beyond what's declared (`ACCESS_NETWORK_STATE` is present for connectivity callbacks — verify).
### 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.
- `core/ui/src/androidMain/kotlin/org/meshtastic/core/ui/util/PlatformUtils.kt` — the two functions + `rememberOnResumeState`; the only file that changes.
- `core/network/.../repository/ConnectivityManager.kt` — reference NetworkCallback/callbackFlow pattern.
- `core/ble/.../AndroidBluetoothRepository.kt` — reference registerReceiver/RECEIVER_NOT_EXPORTED pattern.
- `feature/connections/.../ui/ConnectionsScreen.kt` — sole consumer (no change needed; reads the same booleans, now updated live).
- jvm/ios actuals already return constant `false` — unaffected.
---
## 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).
R1: `isBluetoothDisabled()` updates reactively — when the Bluetooth adapter is turned on/off (incl. from the quick-settings shade while the app is foregrounded), the returned value changes without waiting for `ON_RESUME`.
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.
Verification: review confirms a `BroadcastReceiver` on `BluetoothAdapter.ACTION_STATE_CHANGED` drives the state; manual toggle from shade flips the banner.
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.
R2: `isWifiUnavailable()` updates reactively — connecting/disconnecting Wi-Fi (or losing the active local network) changes the returned value live.
Source: human confirmed
Verification: `grep -rn "accompanist" --include=*.kt --include=*.toml --include=*.gradle.kts` returns no permission usages; app compiles for google + fdroid flavors.
Verification: review confirms a `ConnectivityManager` `NetworkCallback` (default-network) drives the state.
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.
R3: `ON_RESUME` polling is removed for these two functions; the receiver/callback is registered and unregistered with the composable's lifetime via `DisposableEffect` (no leaks). `isGpsDisabled()` continues to use `rememberOnResumeState` unchanged.
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.
Verification: `grep` shows no `rememberOnResumeState` in the two functions; `awaitClose`/`onDispose` unregisters; `isGpsDisabled` untouched.
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.
R4: Registration follows existing repo conventions — `ContextCompat.registerReceiver(..., RECEIVER_NOT_EXPORTED)` for the BT receiver; `registerDefaultNetworkCallback`/`unregisterNetworkCallback` for the network callback. No new permissions (ACCESS_NETWORK_STATE already declared).
Source: craft-clarify recommendation
Verification: Existing tests (`TAKConfigPermissionDeniedTest`) still pass; manifest gating unchanged; Compass behavior preserved.
Verification: review confirms the registration calls + flag matches `RECEIVER_NOT_EXPORTED`.
R5: The public `expect` contract and all consumers are unchanged — `ConnectionsScreen` reads the same `Boolean`s, now live. jvm/ios actuals stay constant `false`.
Source: craft-clarify recommendation
Verification: no change to commonMain expect or jvm/iosMain; ConnectionsScreen diff empty; both flavors assemble.
---
## 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.
- **Chosen approach:** Extract a private `rememberObservedFlag(read, subscribe)` primitive (DisposableEffect + mutableStateOf, re-seed on registration); express `isBluetoothDisabled()` via a `BroadcastReceiver` on `ACTION_STATE_CHANGED` (RECEIVER_NOT_EXPORTED, main-thread delivery) and `isWifiUnavailable()` via `registerDefaultNetworkCallback(callback, mainHandler)`.
- **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.
- **Adversarial:** P1-ish threading risk (NetworkCallback on background thread) mitigated by main-thread Handler; leaks prevented by onDispose unregister; cold-start staleness prevented by read() re-seed. No blocker.
- **Files:** `core/ui/src/androidMain/kotlin/org/meshtastic/core/ui/util/PlatformUtils.kt` only.
- **Test:** build both flavors + detekt/spotless; manual shade-toggle verification (no headless test feasible — `core/ui` has no instrumentation).
---
## 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
### AC Completion Notes
- AC1 ✓ DONE — `PermissionStatusTest` (4 cases incl. the adversarial `(false,true,false)→PERMANENTLY_DENIED`) passes on all targets; classifier lives once in `core/ui` commonMain.
- AC2 ✓ DONE — `grep accompanist` over .kt/.toml/.gradle.kts returns NONE; both flavors assemble.
- AC3 ✓ DONE — barcode renders `PermissionRecoveryCard` on denial; USB denial calls `setErrorMessage`; map button handles denial. [MANUAL-VERIFY] runtime UI in review.
- AC4 ✓ DONE — `rememberOpenAppSettings()` uses `ACTION_APPLICATION_DETAILS_SETTINGS`; card shows settings button only for PERMANENTLY_DENIED; ON_RESUME recheck via `rememberRuntimePermissionState`. [MANUAL-VERIFY] resume.
- AC5 ✓ DONE — Compass distinguishes GPS-disabled (`ACTION_LOCATION_SOURCE_SETTINGS`) from permission; map reuses `isGpsDisabled()`. [MANUAL-VERIFY].
- AC6 ✓ DONE — intro hoists `rememberXxxPermissionState()` (still skippable, IntroViewModelTest green); map/barcode request in-context. [MANUAL-VERIFY] skippable.
- AC7 ✓ DONE — `rememberLocationPermissionState` uses `requireAll=false` (coarse OK); cards are inline. [MANUAL-VERIFY].
- AC8 ✓ DONE — new strings added + `sort-strings.py` clean; BT-bonding + USB literals converted to resources.
- AC9 ✓ DONE — `TAKConfigPermissionDeniedTest` green; manifest untouched; Compass left intact (preserved, not regressed).
AC1: [R1] `isBluetoothDisabled()` is driven by a `BroadcastReceiver` on `BluetoothAdapter.ACTION_STATE_CHANGED`. Auto-verify: grep ACTION_STATE_CHANGED + BroadcastReceiver in the function.
AC2: [R2] `isWifiUnavailable()` is driven by `registerDefaultNetworkCallback`. Auto-verify: grep registerDefaultNetworkCallback.
AC3: [R3] neither function uses `rememberOnResumeState`; observer torn down in `onDispose`; `isGpsDisabled()` still uses `rememberOnResumeState`. Auto-verify: grep.
AC4: [R4] BT receiver uses `RECEIVER_NOT_EXPORTED`; network callback uses a main-thread `Handler`. Auto-verify: grep.
AC5: [R5] commonMain expect + jvm/ios actuals + ConnectionsScreen unchanged; both flavors assemble; detekt/spotless clean. Auto-verify: git diff scope + build.
---
## Implementation Plan
**Branch:** claude/gallant-thompson-d1b2ea **Started:** 2026-06-18 **Base commit:** 1125172c46cefd23c7f0a70a19cb46699f6c7cc2
**Branch:** claude/gallant-thompson-d1b2ea **Base commit:** 2c06a8019
### Task list
- [x] `core/ui/.../util/PermissionStatus.kt` (new, commonMain) — enum + `computePermissionStatus` (pure) + `PermissionUiState`
- [x] `core/ui/.../util/PlatformUtils.kt` (commonMain) — add expect `rememberXxxPermissionState()` + `rememberOpenAppSettings()`
- [x] `core/ui/.../util/PermissionRequestTracker.kt` (new, androidMain) — synchronous SharedPreferences flag
- [x] `core/ui/.../util/PlatformUtils.kt` (androidMain) — actuals; flag write in result callback; LocalActivity; pre-12 BT→location delegation
- [x] `core/ui/.../util/PlatformUtils.kt` (jvmMain) + `NoopStubs.kt` (iosMain) — GRANTED stubs
- [x] `core/ui/.../component/PermissionRecoveryCard.kt` (new, commonMain) — shared card
- [x] `core/ui/.../util/PermissionStatusTest.kt` (new, commonTest) — table test (R1 anchor)
- [x] `core/resources/.../strings.xml` — new strings + sort-strings.py
- [x] `core/barcode/.../BarcodeScannerProvider.kt` — migrate camera off Accompanist + denial card
- [x] `feature/intro/.../AndroidIntroPermissions.kt` + `AppIntroductionScreen.kt` — migrate off Accompanist
- [x] `androidApp/src/google` + `src/fdroid` `MapView.kt` — migrate off Accompanist + permanent-denial → settings
- [x] `feature/connections/.../AndroidScannerViewModel.kt` — USB + BT-bonding strings/messages
- [~] `feature/node/.../component/CompassBottomSheet.kt` — NOT refactored (deliberate, see [DEFERRED]); R9 satisfied by preservation
- [x] 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)
None introduced. `docs/assets/screenshots/nodes_detail_local.png` was regenerated by the baseline build (known host-render diff) and reverted, not committed.
---
## Completion Bar
_All 6 must be checked before handing off to review._
1. [x] All planned files created/modified (Compass intentionally preserved — see DEFERRED)
2. [x] Linter clean — spotlessCheck + detekt PASS (zero new errors)
3. [x] Tests pass — PermissionStatusTest, IntroViewModelTest, TAKConfigPermissionDeniedTest all green; both flavors assemble
1. [x] All planned files created/modified
2. [x] Linter clean
3. [x] Tests pass (no new tests feasible; regressions green)
4. [x] Every AC has a completion note
5. [x] No open markers remain
6. [x] Scope discipline honored — Compass non-refactor recorded in DEFERRED with rationale
6. [x] Scope discipline honored
---
## Review
_Populated by `/craft-review`._
Reviewer: concurrency (right-sized — single-file ~40-line change on established repo patterns; build+detekt covered the rest).
**Reviewers activated:** correctness, simplification, requirements, idioms (Kotlin/KMP/Compose), security, tests, error-handling, concurrency, architecture, api-surface, prose, accessibility
Verdict: **SOUND.** Threading main-thread-confined (BT receiver no-Handler → main; NetworkCallback with main-Looper Handler); register/unregister balanced 1:1 via `DisposableEffect(Unit)`/`onDispose`; no leak or double-unregister; `read()` re-seed correct.
### Findings
Findings (both P3, no action):
- F-1 [P3] `rememberUpdatedState(subscribe)` freshness is never exercised (subscribe runs once). NOT removed: accessing `subscribe` directly inside `DisposableEffect` re-triggers the `LambdaParameterInRestartableEffect` detekt rule, so the wrapper is required for lint. `LocalContext` is stable, so no stale-context defect. Kept as-is.
- F-2 [P3] `registerDefaultNetworkCallback` `TooManyRequests` — structurally bounded (1:1 with a single live banner); no retry loop. No guard needed.
- **F-1 [P1]** `ConnectionsScreen.kt` / `TakPermissionUtil.kt` / notifications — AC6/R6 only partially met: in-context `rememberXxxPermissionState()` wired for location + camera only. BT-scan, notifications, and local-network feature entry points are NOT migrated to in-context (notification/local-network state composables have zero feature call sites). Contradicts AC6=DONE and the workpad's own P3 deferred item. (requirements; human-triage) [CORROBORATED-3: requirements, architecture, api-surface]
- **F-2 [P2]** `PlatformUtils.kt` (androidMain `rememberBluetoothPermissionState`) + intro — pre-Android-12, the intro Bluetooth screen now delegates to the location state, so it shows "Configure Bluetooth permissions" yet fires a *location* dialog and the dedicated Location screen's rationale is skipped (API 26–30 regression vs prior empty-list→granted behavior). (correctness; human-triage) [LIKELY]
- **F-3 [P2]** `BarcodeScannerProvider.kt` — on DENIED_CAN_RETRY the recovery card's "Grant permission" calls `request()` without setting `pendingScan`, so after the user grants, the card dismisses but the scanner never opens (dead-end; must re-tap). (error-handling; auto-fix)
- **F-4 [P2]** `PermissionStatus.kt` — `PermissionUiState` is a `data class` with lambda members → generated `equals`/`hashCode`/`copy` compare lambdas by reference (misleading value semantics; Compose-skipping footgun). Drop `data` / make a `@Stable class` with status-based equals. (idioms/api-surface; auto-fix) [CORROBORATED-2]
- **F-5 [P2]** `PlatformUtils.kt` — old `rememberRequestBluetoothPermission`/`rememberRequestNotificationPermission` are now fully dead (only caller was intro, migrated here) and deletable in this PR; remaining old location/local-network wrappers have a *conflicting* grant definition (`isLocationPermissionGranted` = FINE only vs new = FINE|COARSE). Reconcile or `@Deprecated`. (architecture/simplification/api-surface; human-triage) [CORROBORATED-3]
- **F-6 [P2]** `PlatformUtils.kt` `rememberOpenLocationSettings` (and `rememberOpenNfcSettings`) launch the settings intent with no `ActivityNotFoundException` guard, unlike the new `rememberOpenAppSettings`; R5 GPS recovery routes here. (error-handling; auto-fix) [pre-existing]
- **F-7 [P2]** No tests for `PermissionRequestTracker` (SharedPreferences round-trip — the R1 disambiguator) or the `requireAll` grant reduction (AC7 coarse-location). Both cheaply extractable to pure/Robolectric tests. (tests; human-triage)
- **F-8 [P2]** `BarcodeScannerProvider.kt` recovery `Dialog` has no heading/title → screen reader announces body text with no framing (unlike Compass, which sits in a labeled sheet). (accessibility; auto-fix)
- **F-9 [P2]** `strings.xml` `bonding_failed_permissions` states the requirement but gives no recovery action, unlike its sibling `usb_permission_denied`; also passive voice. (prose; auto-fix)
- **F-10 [P3]** `strings.xml` `camera_permission_rationale` uses passive "is needed" — active voice is more direct and consistent with siblings. (prose; auto-fix)
- **F-11 [P3]** `PlatformUtils.kt` `rememberRuntimePermissionState` `refreshTrigger` counter + `LaunchedEffect` is removable indirection — the main-thread result callback can set `statusState.value` directly. (simplification; auto-fix)
- **F-12 [P3]** jvmMain/iosMain duplicate an identical `grantedPermissionState()` factory; a commonMain helper would serve all stubs. (simplification; auto-fix)
- **F-13 [P3]** `PlatformUtils.kt` — null `LocalActivity.current` collapses `shouldShowRationale` to false → can yield PERMANENTLY_DENIED; safe in practice (always Activity-hosted) but couples missing-activity to the punitive state. (correctness; human-triage, advisory)
- **F-14 [P3]** `PermissionRecoveryCard` 4-arg overload could be `internal` (only the `PermissionUiState` overload is used). (api-surface; human-triage)
- **F-15 [P3]** `PermissionRecoveryCard` KDoc bullets not grammatically parallel. (prose; auto-fix)
- **F-16 [P4]** `PermissionStatus` enum ordinal order (GRANTED first) — note for any future ordinal-based persistence. (api-surface; advisory)
- **F-17 [P3]** `AndroidScannerViewModel.kt` generic-catch path still has hardcoded English `"Bonding failed: ${ex.message}"` (R8 consistency — sibling was internationalized). (requirements/prose; auto-fix)
### Reviewers with no findings
- **reviewer-concurrency** — investigated the adversarial read-after-write race; refuted (all on main thread; SharedPreferences in-memory write synchronous; single instance). The result-callback-only write invariant holds.
- **reviewer-security** — no weakened gate; pre-12 BT→location delegation is stronger not weaker; coarse acceptance doesn't over-grant (framework still enforces); has-requested flag stores no PII; settings deep-link is package-scoped; manifest unchanged.
### Corroborations of deliberate decisions
- `PermissionRecoveryCard` vs Compass `WarningList` duplication confirmed justified by simplification reviewer (different shapes; forcing unification would regress R9). Not a finding.
- Decorative `contentDescription=null` on the error icon confirmed correct by accessibility reviewer (adjacent text conveys meaning; error not color-alone).
### Acceptance criteria status
- AC1 ✓ BroadcastReceiver on ACTION_STATE_CHANGED drives isBluetoothDisabled.
- AC2 ✓ registerDefaultNetworkCallback drives isWifiUnavailable.
- AC3 ✓ neither uses rememberOnResumeState; onDispose unregisters; isGpsDisabled untouched.
- AC4 ✓ RECEIVER_NOT_EXPORTED + main-thread Handler.
- AC5 ✓ expect/jvm/ios/ConnectionsScreen unchanged; both flavors assemble; detekt/spotless clean.
---
## Deferred Items
_Accumulated across all phases._
- Compass not refactored onto the shared PermissionRecoveryCard
- severity: P3
- phase: implement
- context: Compass's `WarningList` shows GPS-disabled vs permission-denied vs magnetometer/no-fix warnings simultaneously with two distinct action buttons — shapes the single-permission card can't express. Forcing it would regress R9; the card was lifted FROM this pattern, so duplication is ~20 lines and intentional. Revisit only if a multi-warning card variant is built.
- Delete the now-unused old `rememberRequestXxx`/`isXxx` wrappers in `PlatformUtils.kt`
- severity: P3
- phase: architect
- context: Kept during migration to bound the diff; per the approved plan, deletion is a follow-up PR once all callers are confirmed migrated. (Note: `rememberRequestLocalNetworkPermission` + `isLocalNetworkPermissionGranted` are still used by TAK; not all old wrappers are dead yet.)
- In-context recovery cards for permanently-denied BT/local-network in ConnectionsScreen
- severity: P3
- phase: implement
- context: RESOLVED in refine — Connections scan toggles now request BT + local-network in-context and route permanent denial to app settings.
- Shared `PermissionUiState.route()` helper for the request/permanent-denial/grant routing
- severity: P3
- phase: review (refine second round)
- context: The 3-branch toggle routing (granted→act / permanently-denied→settings / else→request) is duplicated across ConnectionsScreen (×2) and both MapViews; a shared inline extension would centralize the policy. Deferred — each site has real per-call variation and is individually readable.
- Robolectric test for `PermissionRequestTracker` round-trip
- severity: P2
- phase: review
- context: The SharedPreferences has-requested round-trip (the R1 disambiguator) is untested; core/ui has no `androidHostTest`/Robolectric source set, so adding one is its own infra task. The pure `computePermissionStatus` + `isPermissionGroupGranted` logic IS unit-tested.
- Notifications in-context request
- severity: P3
- phase: review
- context: R6 in-context coverage is met for location/camera/BT/local-network. Notifications have no active feature "point of use" (they are passive); the modernized, skippable intro screen remains the request point. A dedicated in-context notification prompt would need a product decision on placement.
---
## Phase Log
<!-- Format: - [phase]: [status] — [ISO date] — [notes] -->
- 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: done — 2026-06-18 — All 9 ACs met; both flavors assemble; spotless/detekt/tests green. Latest sha 2aa33c8d5. Compass intentionally preserved (DEFERRED P3). 3 commits.
- review: done — 2026-06-18 — 12 reviewers. No P0; concurrency+security clean (adversarial race refuted). 17 findings: 1×P1 (AC6/R6 in-context coverage), 8×P2, rest P3/P4. Report in Review section.
- refine: done — 2026-06-18 — human triaged "fix all". Applied 10 auto-fixes + all 7 triaged fixes (incl. F-1 in-context BT/local-network, F-5 full old-wrapper deletion + FINE/COARSE reconciliation, F-2 pre-12 BT). Second review round (correctness+simplification) on the delta: correctness CLEARED, one P3 routing-helper deferred. Both flavors assemble; detekt/spotless/tests green. Deferred: P3 routing helper, Robolectric tracker test (P2, needs infra), notifications-in-context (P3). 4 commits (HEAD 81b5e03f2 + manifest doc fix).
- pr: done — 2026-06-18 — draft PR https://github.com/meshtastic/Meshtastic-Android/pull/5851 (base main, head claude/gallant-thompson-d1b2ea)
- explore: done — 2026-06-18 — lean direct explore (deep prior context). Both reactive patterns already in repo (NetworkCallback callbackFlow in core/network; registerReceiver RECEIVER_NOT_EXPORTED in core/ble). Only androidMain PlatformUtils changes.
- clarify: done — 2026-06-18 — Q1 confirmed (replace ON_RESUME entirely). R1–R5 recorded.
- architect: done — 2026-06-18 — single approach (rememberObservedFlag primitive) + self-adversarial pass (threading via main Handler). Approved.
- implement: done — 2026-06-18 — one file (core/ui androidMain PlatformUtils). All 5 ACs met; build+detekt+spotless+both flavors green.
- review: done — 2026-06-18 — concurrency reviewer: SOUND. 2×P3 non-actionable (lint-required wrapper; bounded TooManyRequests).
- refine: done — 2026-06-18 — fast path, no actionable findings. P3s recorded as considered/declined.
- pr: