From ae610bbba50d3296fc7a7f4bd20db41162b9953d Mon Sep 17 00:00:00 2001
From: James Rich <2199651+jamesarich@users.noreply.github.com>
Date: Wed, 22 Apr 2026 11:04:57 -0500
Subject: [PATCH] refactor: eliminate Accompanist permissions library (#5211)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: garthvh <1795163+garthvh@users.noreply.github.com>
---
 AGENTS.md                                     |  5 +++
 app/src/main/AndroidManifest.xml              | 11 ++++-
 .../meshtastic/core/ui/util/PlatformUtils.kt  | 30 ++++++++++++++
 .../meshtastic/core/ui/util/PlatformUtils.kt  | 10 +++++
 .../org/meshtastic/core/ui/util/NoopStubs.kt  |  5 +++
 .../meshtastic/core/ui/util/PlatformUtils.kt  |  9 ++++
 .../feature/settings/tak/TakPermissionUtil.kt | 41 ++++++-------------
 7 files changed, 81 insertions(+), 30 deletions(-)

diff --git a/AGENTS.md b/AGENTS.md
index c1bafdd96..e22717f9c 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -72,6 +72,11 @@ Do NOT duplicate content into agent-specific files. When you modify architecture
 - **Dependency Discipline:** Never add a library without first checking `libs.versions.toml` and justifying its inclusion against the project's size and complexity goals. Prefer removing dependencies over adding them.
 - **Zero Lint Tolerance:** A task is incomplete if `detekt` fails or `spotlessCheck` does not pass for touched modules.
 - **Read Before Refactoring:** When a pattern contradicts best practices, analyze whether it is legacy debt or a deliberate architectural choice before proposing a change.
+- **Verify Before Push:** Treat any "push", "commit and push", or "push and pr" request as **verify-then-push**. Before `git push`, run `./gradlew spotlessApply detekt` (and the relevant `:module:test` / `:module:lint<Flavor>Debug` for touched modules). CI has repeatedly failed on `UnusedParameter`, `CyclomaticComplexMethod`, and `MagicNumber` from skipping this step. Only push on green; if a check fails, fix it before pushing.
+- **Never Touch Protos or Secrets:** `core/proto/src/main/proto` is an upstream submodule — **do not modify** any `.proto` file. If a feature request requires a proto change, stop and report it as upstream (label issue `upstream`, point at `meshtastic/protobufs`). Likewise, never `git add` `app/google-services.json`, `local.properties`, `secrets.properties`, or any `*.keystore` / `*.jks` file — these are gitignored and contain secrets.
+- **Multi-Flavor Install Hygiene:** When using the `android` CLI MCP to install/run on a connected device, the `fdroid` (`com.geeksville.mesh`) and `google` (`com.geeksville.mesh.google`) flavors have different signatures and **cannot coexist**. Before any install: pick a flavor explicitly, force-stop and uninstall the other flavor on every connected device, then install. Stale installs of the other flavor are a recurring source of "the fix didn't work" red herrings.
+- **Verify UI With Annotated Screenshots:** For any UI/UX task, do **not** claim a fix works based on logs or assumed state. Capture an annotated screenshot via the `android` CLI MCP (or its annotated-screenshot tool) on a real connected device, and inspect the result before reporting back.
+- **Branch Scope Discipline:** If a working branch grows beyond ~5 logical commits, crosses unrelated concerns, or accumulates a large blast radius, proactively propose a fresh branch off `upstream/main` and cherry-pick only the high-signal, low-risk changes (see `.skills/new-branch/SKILL.md`). Don't keep piling onto a sprawling branch.
 </rules>
 
 <copilot_cli_workflow>
diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml
index f7d2ce900..81b8f8d86 100644
--- a/app/src/main/AndroidManifest.xml
+++ b/app/src/main/AndroidManifest.xml
@@ -49,11 +49,18 @@
 
     <!-- This permission is required for analytics - and soon the MQTT gateway -->
     <uses-permission android:name="android.permission.INTERNET" />
-    <!-- Required for Android 17+ (API 37) Local Networking for TAK Server localhost loopback -->
-    <uses-permission android:name="android.permission.ACCESS_LOCAL_NETWORK" />
 
     <uses-permission android:name="android.permission.WAKE_LOCK" />
 
+    <!--
+      Android 17 (API 37) Local Network Protection: targetSdk=37 apps are blocked
+      from local-network access by default. Required for both NSD/mDNS device
+      discovery on the Connections screen and the built-in TAK Server's localhost
+      loopback binding. Requested at runtime via rememberRequestLocalNetworkPermission.
+      See: https://developer.android.com/privacy-and-security/local-network-permission
+    -->
+    <uses-permission android:name="android.permission.ACCESS_LOCAL_NETWORK" />
+
     <!--
       This permission is optional but recommended so we can be smart
       about when to send data.
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 5365ab95e..889e4545e 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
@@ -256,6 +256,36 @@ actual fun rememberRequestNotificationPermission(onGranted: () -> Unit, onDenied
     return remember(launcher) { { launcher.launch(android.Manifest.permission.POST_NOTIFICATIONS) } }
 }
 
+@Composable
+actual fun rememberRequestLocalNetworkPermission(onGranted: () -> Unit, onDenied: () -> Unit): () -> Unit {
+    if (android.os.Build.VERSION.SDK_INT < android.os.Build.VERSION_CODES.S) {
+        // Pre-Android 12, no local network permission required
+        return remember { { onGranted() } }
+    }
+    val currentOnGranted = rememberUpdatedState(onGranted)
+    val currentOnDenied = rememberUpdatedState(onDenied)
+    val launcher =
+        rememberLauncherForActivityResult(ActivityResultContracts.RequestPermission()) { granted ->
+            if (granted) currentOnGranted.value() else currentOnDenied.value()
+        }
+    return remember(launcher) { { launcher.launch(android.Manifest.permission.ACCESS_LOCAL_NETWORK) } }
+}
+
+@Composable
+actual fun isLocalNetworkPermissionGranted(): Boolean {
+    if (android.os.Build.VERSION.SDK_INT < android.os.Build.VERSION_CODES.S) {
+        // Pre-Android 12, no runtime local-network permission exists; access is implicit via INTERNET.
+        return true
+    }
+    val context = LocalContext.current
+    return rememberOnResumeState {
+        androidx.core.content.ContextCompat.checkSelfPermission(
+            context,
+            android.Manifest.permission.ACCESS_LOCAL_NETWORK,
+        ) == android.content.pm.PackageManager.PERMISSION_GRANTED
+    }
+}
+
 @Composable
 actual fun isLocationPermissionGranted(): Boolean {
     val context = LocalContext.current
diff --git a/core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/util/PlatformUtils.kt b/core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/util/PlatformUtils.kt
index 9d3169c1a..c12dc5471 100644
--- a/core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/util/PlatformUtils.kt
+++ b/core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/util/PlatformUtils.kt
@@ -67,6 +67,16 @@ expect fun rememberSaveFileLauncher(
 /** Returns a launcher to request Bluetooth scan + connect permissions. No-op on platforms without runtime BLE perms. */
 @Composable expect fun rememberRequestBluetoothPermission(onGranted: () -> Unit, onDenied: () -> Unit = {}): () -> Unit
 
+/** Returns a launcher to request the ACCESS_LOCAL_NETWORK permission. No-op on platforms that don't require it. */
+@Composable
+expect fun rememberRequestLocalNetworkPermission(onGranted: () -> Unit, onDenied: () -> Unit = {}): () -> Unit
+
+/**
+ * Returns whether ACCESS_LOCAL_NETWORK is currently granted. Always `true` on platforms / API levels that don't gate
+ * local-network access behind a runtime permission.
+ */
+@Composable expect fun isLocalNetworkPermissionGranted(): Boolean
+
 /** Returns a launcher to request the POST_NOTIFICATIONS permission. No-op on platforms that don't require it. */
 @Composable
 expect fun rememberRequestNotificationPermission(onGranted: () -> Unit, onDenied: () -> Unit = {}): () -> Unit
diff --git a/core/ui/src/iosMain/kotlin/org/meshtastic/core/ui/util/NoopStubs.kt b/core/ui/src/iosMain/kotlin/org/meshtastic/core/ui/util/NoopStubs.kt
index ebe791f8e..bcaab5b39 100644
--- a/core/ui/src/iosMain/kotlin/org/meshtastic/core/ui/util/NoopStubs.kt
+++ b/core/ui/src/iosMain/kotlin/org/meshtastic/core/ui/util/NoopStubs.kt
@@ -58,6 +58,11 @@ actual fun rememberOpenFileLauncher(onUriReceived: (CommonUri?) -> Unit): (mimeT
 
 @Composable actual fun rememberRequestBluetoothPermission(onGranted: () -> Unit, onDenied: () -> Unit): () -> Unit = {}
 
+@Composable
+actual fun rememberRequestLocalNetworkPermission(onGranted: () -> Unit, onDenied: () -> Unit): () -> Unit = {}
+
+@Composable actual fun isLocalNetworkPermissionGranted(): Boolean = true
+
 @Composable
 actual fun rememberRequestNotificationPermission(onGranted: () -> Unit, onDenied: () -> Unit): () -> Unit = {}
 
diff --git a/core/ui/src/jvmMain/kotlin/org/meshtastic/core/ui/util/PlatformUtils.kt b/core/ui/src/jvmMain/kotlin/org/meshtastic/core/ui/util/PlatformUtils.kt
index a938f92ea..d10532b47 100644
--- a/core/ui/src/jvmMain/kotlin/org/meshtastic/core/ui/util/PlatformUtils.kt
+++ b/core/ui/src/jvmMain/kotlin/org/meshtastic/core/ui/util/PlatformUtils.kt
@@ -134,6 +134,15 @@ actual fun rememberOpenLocationSettings(): () -> Unit = { Logger.w { "Location s
 @Composable
 actual fun rememberRequestBluetoothPermission(onGranted: () -> Unit, onDenied: () -> Unit): () -> Unit = { onGranted() }
 
+/** JVM no-op — Desktop does not require runtime local network permissions. */
+@Composable
+actual fun rememberRequestLocalNetworkPermission(onGranted: () -> Unit, onDenied: () -> Unit): () -> Unit = {
+    onGranted()
+}
+
+/** JVM — local network permission is always considered granted on Desktop. */
+@Composable actual fun isLocalNetworkPermissionGranted(): Boolean = true
+
 /** JVM no-op — Desktop does not require runtime notification permissions. */
 @Composable
 actual fun rememberRequestNotificationPermission(onGranted: () -> Unit, onDenied: () -> Unit): () -> Unit = {
diff --git a/feature/settings/src/androidMain/kotlin/org/meshtastic/feature/settings/tak/TakPermissionUtil.kt b/feature/settings/src/androidMain/kotlin/org/meshtastic/feature/settings/tak/TakPermissionUtil.kt
index 723448897..b9c118824 100644
--- a/feature/settings/src/androidMain/kotlin/org/meshtastic/feature/settings/tak/TakPermissionUtil.kt
+++ b/feature/settings/src/androidMain/kotlin/org/meshtastic/feature/settings/tak/TakPermissionUtil.kt
@@ -16,38 +16,23 @@
  */
 package org.meshtastic.feature.settings.tak
 
-import android.os.Build
 import androidx.compose.runtime.Composable
-import androidx.compose.runtime.LaunchedEffect
-import com.google.accompanist.permissions.ExperimentalPermissionsApi
-import com.google.accompanist.permissions.isGranted
-import com.google.accompanist.permissions.rememberPermissionState
+import org.meshtastic.core.ui.util.rememberRequestLocalNetworkPermission
 
-private val SDK_INT_ANDROID_16 = Build.VERSION_CODES.BAKLAVA
-
-@OptIn(ExperimentalPermissionsApi::class)
 @Composable
 actual fun TakPermissionHandler(isTakServerEnabled: Boolean, onPermissionResult: (Boolean) -> Unit) {
-    if (Build.VERSION.SDK_INT >= SDK_INT_ANDROID_16) {
-        val permissionState =
-            rememberPermissionState("android.permission.ACCESS_LOCAL_NETWORK") { granted ->
-                // Callback fires after the system dialog is dismissed — report the result
-                // directly so onPermissionResult is the single authority for grant/deny.
-                if (isTakServerEnabled) onPermissionResult(granted)
-            }
+    // ACCESS_LOCAL_NETWORK runtime permission (Android 17 / API 37+) is required for the TAK Server's
+    // localhost socket binding (127.0.0.1:8087). It is also required globally for NSD/mDNS device discovery
+    // when targetSdk >= 37, and is requested up-front from the Connections screen, so it will usually
+    // already be granted by the time the user enables TAK. This composable handles the standalone case
+    // (e.g. user opens TAK settings before ever tapping the network-scan toggle).
+    val requestPermission =
+        rememberRequestLocalNetworkPermission(
+            onGranted = { onPermissionResult(true) },
+            onDenied = { onPermissionResult(false) },
+        )
 
-        LaunchedEffect(isTakServerEnabled) {
-            if (isTakServerEnabled) {
-                if (permissionState.status.isGranted) {
-                    // Already granted — confirm immediately so the orchestrator may proceed.
-                    onPermissionResult(true)
-                } else {
-                    // Show system dialog; result is delivered via the callback above.
-                    permissionState.launchPermissionRequest()
-                }
-            }
-        }
-    } else {
-        LaunchedEffect(isTakServerEnabled) { onPermissionResult(true) }
+    if (isTakServerEnabled) {
+        requestPermission()
     }
 }