Meshtastic-Android/workpad.md

Craft Workpad: Permissions Checks UX Audit & Improvements

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

---

Task

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

---

Exploration Report

Key Facts (most actionable first)

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

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

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

High-yield audit gaps for this app:

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

Code Snippets (exact)

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

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

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

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

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

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

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

Key Files

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

Open contradiction resolved

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

---

Requirements

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

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

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

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

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

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

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

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

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

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

---

Architectural Decision

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

Requirement Coverage

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

Key trade-offs resolved

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

Deferred to Implementation

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

Adversarial finding

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

Files to create/modify

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

Test strategy

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

---

Acceptance Criteria

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

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

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

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

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

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

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

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

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

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).

---

Implementation Plan

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

Task list

• core/ui/.../util/PermissionStatus.kt (new, commonMain) — enum + computePermissionStatus (pure) + PermissionUiState
• core/ui/.../util/PlatformUtils.kt (commonMain) — add expect rememberXxxPermissionState() + rememberOpenAppSettings()
• core/ui/.../util/PermissionRequestTracker.kt (new, androidMain) — synchronous SharedPreferences flag
• core/ui/.../util/PlatformUtils.kt (androidMain) — actuals; flag write in result callback; LocalActivity; pre-12 BT→location delegation
• core/ui/.../util/PlatformUtils.kt (jvmMain) + NoopStubs.kt (iosMain) — GRANTED stubs
• core/ui/.../component/PermissionRecoveryCard.kt (new, commonMain) — shared card
• core/ui/.../util/PermissionStatusTest.kt (new, commonTest) — table test (R1 anchor)
• core/resources/.../strings.xml — new strings + sort-strings.py
• core/barcode/.../BarcodeScannerProvider.kt — migrate camera off Accompanist + denial card
• feature/intro/.../AndroidIntroPermissions.kt + AppIntroductionScreen.kt — migrate off Accompanist
• androidApp/src/google + src/fdroid MapView.kt — migrate off Accompanist + permanent-denial → settings
• feature/connections/.../AndroidScannerViewModel.kt — USB + BT-bonding strings/messages
• [~] feature/node/.../component/CompassBottomSheet.kt — NOT refactored (deliberate, see [DEFERRED]); R9 satisfied by preservation
• 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. All planned files created/modified (Compass intentionally preserved — see DEFERRED)
2. Linter clean — spotlessCheck + detekt PASS (zero new errors)
3. Tests pass — PermissionStatusTest, IntroViewModelTest, TAKConfigPermissionDeniedTest all green; both flavors assemble
4. Every AC has a completion note
5. No open markers remain
6. Scope discipline honored — Compass non-refactor recorded in DEFERRED with rationale

---

Review

Populated by /craft-review.

Reviewers activated: correctness, simplification, requirements, idioms (Kotlin/KMP/Compose), security, tests, error-handling, concurrency, architecture, api-surface, prose, accessibility

Findings

• 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).

---

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)