From 174db32ae8d88827b455fb1d6ebca39227b1202d Mon Sep 17 00:00:00 2001 From: James Rich Date: Thu, 18 Jun 2026 13:57:24 -0500 Subject: [PATCH] feat(core/ui): make BT/Wi-Fi adapter-state detection live 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 --- .../meshtastic/core/ui/util/PlatformUtils.kt | 95 ++++- workpad.md | 363 +++--------------- 2 files changed, 140 insertions(+), 318 deletions(-) diff --git a/core/ui/src/androidMain/kotlin/org/meshtastic/core/ui/util/PlatformUtils.kt b/core/ui/src/androidMain/kotlin/org/meshtastic/core/ui/util/PlatformUtils.kt index 41285f373..d538f69b5 100644 --- a/core/ui/src/androidMain/kotlin/org/meshtastic/core/ui/util/PlatformUtils.kt +++ b/core/ui/src/androidMain/kotlin/org/meshtastic/core/ui/util/PlatformUtils.kt @@ -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, 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. diff --git a/workpad.md b/workpad.md index 4ee86d563..856c4325e 100644 --- a/workpad.md +++ b/workpad.md @@ -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 - - -- 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: