fix(notifications): address review feedback on threading, privacy, and cancellation

- ConversationShortcutPublisher observes on the IO dispatcher: the caller's
  scope is Main-bound (MeshService), and publishing does ShortcutManager IPC
  plus avatar bitmap rasterization on every recency-affecting DB change.
- Drop the contactKey from the on-demand shortcut failure logs; node/channel
  identifiers are user-traceable (privacy-first convention).
- ReplyReceiver rethrows CancellationException instead of treating it as a
  failed send, and uses safeCatching (which preserves cancellation) rather than
  runCatching for the notification calls.
- Strengthen the DM refresh test to assert the resolved peer name on the
  published shortcut label, not just the absent conversation title.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: jamesarich

core/service/src/androidHostTest/kotlin/org/meshtastic/core/service/ConversationShortcutPublisherTest.kt

@@ -24,13 +24,15 @@ import dev.mokkery.MockMode
 import dev.mokkery.answering.returns
 import dev.mokkery.every
 import dev.mokkery.mock
+import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.flowOf
 import kotlinx.coroutines.test.advanceUntilIdle
 import kotlinx.coroutines.test.runTest
 import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
+import org.meshtastic.core.di.CoroutineDispatchers
 import org.meshtastic.core.model.DataPacket
 import org.meshtastic.core.model.Node
 import org.meshtastic.core.repository.NodeRepository
@@ -61,9 +63,15 @@ class ConversationShortcutPublisherTest {
     private val packetRepository: PacketRepository = mock(MockMode.autofill)
     private val nodeRepository: NodeRepository = mock(MockMode.autofill)
     private val radioConfigRepository: RadioConfigRepository = mock(MockMode.autofill)
+    private val dispatchers =
+        CoroutineDispatchers(
+            io = Dispatchers.Unconfined,
+            main = Dispatchers.Unconfined,
+            default = Dispatchers.Unconfined,
+        )
 
     private val publisher =
-        ConversationShortcutPublisher(context, nodeRepository, packetRepository, radioConfigRepository)
+        ConversationShortcutPublisher(context, nodeRepository, packetRepository, radioConfigRepository, dispatchers)
 
     @Before
     fun setUp() {

core/service/src/androidHostTest/kotlin/org/meshtastic/core/service/MeshNotificationManagerImplConversationTest.kt

@@ -27,12 +27,14 @@ import dev.mokkery.every
 import dev.mokkery.everySuspend
 import dev.mokkery.matcher.any
 import dev.mokkery.mock
+import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.flowOf
 import kotlinx.coroutines.test.runTest
 import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
+import org.meshtastic.core.di.CoroutineDispatchers
 import org.meshtastic.core.model.ConnectionState
 import org.meshtastic.core.model.Message
 import org.meshtastic.core.model.MyNodeInfo
@@ -74,7 +76,17 @@ class MeshNotificationManagerImplConversationTest {
             nodeRepository = lazy { nodeRepository },
             conversationShortcutPublisher =
             lazy {
-                ConversationShortcutPublisher(context, nodeRepository, packetRepository, radioConfigRepository)
+                ConversationShortcutPublisher(
+                    context,
+                    nodeRepository,
+                    packetRepository,
+                    radioConfigRepository,
+                    CoroutineDispatchers(
+                        io = Dispatchers.Unconfined,
+                        main = Dispatchers.Unconfined,
+                        default = Dispatchers.Unconfined,
+                    ),
+                )
             },
             radioConfigRepository = lazy { radioConfigRepository },
         )
@@ -211,5 +223,11 @@ class MeshNotificationManagerImplConversationTest {
         val posted = activeByTag("message").single().notification
         // A DM is not a group conversation, so no conversation title is set.
         assertNull(posted.extras.getCharSequence(Notification.EXTRA_CONVERSATION_TITLE))
+        // The on-demand shortcut published for the notification carries the peer's resolved name.
+        val shortcut =
+            context.getSystemService(android.content.pm.ShortcutManager::class.java)!!.dynamicShortcuts.single {
+                it.id == "0!00000007"
+            }
+        assertEquals("Hawk Ridge", shortcut.shortLabel)
     }
 }

core/service/src/androidMain/kotlin/org/meshtastic/core/service/ConversationShortcutPublisher.kt

@@ -34,6 +34,7 @@ import kotlinx.coroutines.flow.distinctUntilChanged
 import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.launch
 import org.koin.core.annotation.Single
+import org.meshtastic.core.di.CoroutineDispatchers
 import org.meshtastic.core.model.Channel
 import org.meshtastic.core.model.DataPacket
 import org.meshtastic.core.model.NodeAddress
@@ -63,14 +64,17 @@ class ConversationShortcutPublisher(
     private val nodeRepository: NodeRepository,
     private val packetRepository: PacketRepository,
     private val radioConfigRepository: RadioConfigRepository,
+    private val dispatchers: CoroutineDispatchers,
 ) {
 
     private var observeJob: Job? = null
 
     fun startObserving(scope: CoroutineScope) {
         observeJob?.cancel()
         observeJob =
-            scope.launch {
+            // The caller's scope may be Main-bound (MeshService); shortcut publishing does ShortcutManager IPC and
+            // rasterizes avatar bitmaps on every recency-affecting DB change, so keep it off the Main thread.
+            scope.launch(dispatchers.io) {
                 // Combine the message DB (recency + DM peers) with the channel config (names). Ranking both by
                 // last-activity time lets launchers / Android Auto surface the *recently active* conversations first
                 // (the shortcut display cap is small), instead of an arbitrary slice of stale channels.
@@ -252,9 +256,10 @@ class ConversationShortcutPublisher(
         try {
             ShortcutManagerCompat.pushDynamicShortcut(context, shortcut)
         } catch (e: IllegalArgumentException) {
-            Logger.e(tag = TAG, throwable = e) { "Failed to publish on-demand shortcut $contactKey" }
+            // Don't log the contactKey: node/channel identifiers are user-traceable (privacy-first convention).
+            Logger.e(tag = TAG, throwable = e) { "Failed to publish on-demand shortcut" }
         } catch (e: IllegalStateException) {
-            Logger.e(tag = TAG, throwable = e) { "Failed to publish on-demand shortcut $contactKey" }
+            Logger.e(tag = TAG, throwable = e) { "Failed to publish on-demand shortcut" }
         }
     }
 

core/service/src/androidMain/kotlin/org/meshtastic/core/service/ReplyReceiver.kt

@@ -21,12 +21,14 @@ import android.content.Context
 import android.content.Intent
 import androidx.core.app.RemoteInput
 import co.touchlab.kermit.Logger
+import kotlinx.coroutines.CancellationException
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.SupervisorJob
 import kotlinx.coroutines.launch
 import org.koin.core.component.KoinComponent
 import org.koin.core.component.inject
 import org.meshtastic.core.common.util.nowMillis
+import org.meshtastic.core.common.util.safeCatching
 import org.meshtastic.core.di.CoroutineDispatchers
 import org.meshtastic.core.repository.MeshNotificationManager
 import org.meshtastic.core.repository.PacketRepository
@@ -84,15 +86,18 @@ class ReplyReceiver :
                 // Re-post the conversation silently with the sent reply appended — the MessagingStyle confirmation
                 // flow. This resolves the RemoteInput spinner with visible feedback instead of the notification
                 // vanishing. Fall back to dismissal so the spinner never hangs if the refresh itself fails.
-                runCatching { meshServiceNotifications.refreshConversationAfterReply(contactKey) }
+                safeCatching { meshServiceNotifications.refreshConversationAfterReply(contactKey) }
                     .onFailure {
                         Logger.e(tag = TAG, throwable = it) { "refresh after reply failed" }
-                        runCatching { meshServiceNotifications.cancelMessageNotification(contactKey) }
+                        safeCatching { meshServiceNotifications.cancelMessageNotification(contactKey) }
                     }
+            } catch (e: CancellationException) {
+                // Preserve structured concurrency — never treat cancellation as a failed send.
+                throw e
             } catch (e: Exception) {
                 Logger.e(tag = TAG, throwable = e) { "reply send failed" }
                 // The send failed; dismiss so the RemoteInput spinner resolves rather than hanging forever.
-                runCatching { meshServiceNotifications.cancelMessageNotification(contactKey) }
+                safeCatching { meshServiceNotifications.cancelMessageNotification(contactKey) }
                     .onFailure { Logger.e(tag = TAG, throwable = it) { "cancel notification failed" } }
             } finally {
                 pendingResult?.finish()