Skip to content

Commit f2a08ff

Browse files
jamesarichclaude
andcommitted
fix(notifications): harden shortcut labels and on-demand pruning per review
- Never expose the raw contactKey as a user-facing label: DM shortcuts fall back to the localized unknown-username string, and the post-reply refresh uses localized fallbacks too. contactKey stays confined to internal ids and deep-link metadata (privacy-first convention). - Drop the dead "Channel N" fallback: Channel.name resolves an empty settings.name to the modem-preset display name (or "Custom"), so the effective name is never empty. - Protect on-demand shortcuts from the observer's pruning until a contacts snapshot includes their conversation, so a brand-new conversation's first notification cannot have its shortcut link severed by an in-flight publish. Adds regression tests for the pruning protection and reseeds the stale-removal test via a raw push (the in-memory protection intentionally doesn't survive a process restart, so the observer still owns cross-session cleanup). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent d7cff2d commit f2a08ff

3 files changed

Lines changed: 62 additions & 11 deletions

File tree

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

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,8 +125,9 @@ class ConversationShortcutPublisherTest {
125125

126126
@Test
127127
fun `stale shortcuts are removed when no longer part of the conversation set`() = runTest {
128-
// Simulate a leftover shortcut from a previous device/channel config.
129-
publisher.ensureConversationShortcut("9^all", "Old Channel")
128+
// Simulate a leftover shortcut from a previous session (raw push: the in-memory on-demand protection does not
129+
// survive a process restart, so the observer owns its cleanup).
130+
pushRawShortcut("9^all", "Old Channel")
130131
assertTrue(shortcutManager.dynamicShortcuts.any { it.id == "9^all" })
131132

132133
every { packetRepository.getContacts() } returns flowOf(emptyMap())
@@ -137,4 +138,29 @@ class ConversationShortcutPublisherTest {
137138
assertNull(shortcutManager.dynamicShortcuts.find { it.id == "9^all" }, "stale shortcut should be pruned")
138139
assertTrue(shortcutManager.dynamicShortcuts.any { it.id == "0^all" }, "current channels are published")
139140
}
141+
142+
@Test
143+
fun `on-demand shortcuts survive pruning until a snapshot includes their conversation`() = runTest {
144+
// A brand-new conversation's first notification publishes its shortcut before the contacts flow emits it.
145+
publisher.ensureConversationShortcut("0!000000ff", "New Peer")
146+
147+
every { packetRepository.getContacts() } returns flowOf(emptyMap())
148+
149+
publisher.startObserving(this)
150+
advanceUntilIdle()
151+
152+
assertTrue(
153+
shortcutManager.dynamicShortcuts.any { it.id == "0!000000ff" },
154+
"pending on-demand shortcut must not be pruned before the snapshot catches up",
155+
)
156+
}
157+
158+
private fun pushRawShortcut(id: String, label: String) {
159+
val shortcut =
160+
androidx.core.content.pm.ShortcutInfoCompat.Builder(context, id)
161+
.setShortLabel(label)
162+
.setIntent(android.content.Intent(android.content.Intent.ACTION_VIEW))
163+
.build()
164+
androidx.core.content.pm.ShortcutManagerCompat.pushDynamicShortcut(context, shortcut)
165+
}
140166
}

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

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,11 @@ import org.meshtastic.core.navigation.DEEP_LINK_BASE_URI
4343
import org.meshtastic.core.repository.NodeRepository
4444
import org.meshtastic.core.repository.PacketRepository
4545
import org.meshtastic.core.repository.RadioConfigRepository
46+
import org.meshtastic.core.resources.Res
47+
import org.meshtastic.core.resources.getString
48+
import org.meshtastic.core.resources.unknown_username
4649
import org.meshtastic.proto.ChannelSet
50+
import java.util.concurrent.ConcurrentHashMap
4751

4852
/**
4953
* Single owner of the dynamic conversation shortcuts that link Meshtastic conversations to their notifications.
@@ -69,6 +73,13 @@ class ConversationShortcutPublisher(
6973

7074
private var observeJob: Job? = null
7175

76+
/**
77+
* On-demand shortcut ids published ahead of the observed contacts snapshot (a brand-new conversation's first
78+
* notification). Protected from pruning until a snapshot includes them, so [publishShortcuts] can't sever a live
79+
* notification's shortcut link during the window before the flow catches up.
80+
*/
81+
private val pendingOnDemandIds = ConcurrentHashMap.newKeySet<String>()
82+
7283
fun startObserving(scope: CoroutineScope) {
7384
observeJob?.cancel()
7485
observeJob =
@@ -134,7 +145,12 @@ class ConversationShortcutPublisher(
134145

135146
try {
136147
val currentKeys = shortcuts.map { it.id }.toSet()
137-
val stale = ShortcutManagerCompat.getDynamicShortcuts(context).map { it.id }.filter { it !in currentKeys }
148+
// The snapshot now covers these conversations; normal pruning applies to them from here on.
149+
pendingOnDemandIds.removeAll(currentKeys)
150+
val stale =
151+
ShortcutManagerCompat.getDynamicShortcuts(context)
152+
.map { it.id }
153+
.filter { it !in currentKeys && it !in pendingOnDemandIds }
138154
if (stale.isNotEmpty()) {
139155
ShortcutManagerCompat.removeDynamicShortcuts(context, stale)
140156
}
@@ -151,12 +167,14 @@ class ConversationShortcutPublisher(
151167

152168
private fun buildDmShortcut(dm: Conversation.Dm, rank: Int): ShortcutInfoCompat? {
153169
val node = nodeRepository.nodeDBbyNum.value.values.find { it.user.id == dm.userId }
154-
// shortLabel is the compact 4-char short name; longLabel the full node name. Fall back sensibly when a name is
155-
// missing so the shortcut is never blank.
170+
// shortLabel is the compact 4-char short name; longLabel the full node name. Fall back to a localized generic
171+
// name when node metadata is missing: the raw contactKey is a user-traceable identifier and must stay confined
172+
// to internal ids and deep-link metadata (privacy-first convention).
156173
val shortName = node?.user?.short_name?.takeIf { it.isNotBlank() }
157174
val longName = node?.user?.long_name?.takeIf { it.isNotBlank() }
158-
val shortLabel = shortName ?: longName ?: dm.contactKey
159-
val longLabel = longName ?: shortName ?: dm.contactKey
175+
val fallbackName by lazy { getString(Res.string.unknown_username) }
176+
val shortLabel = shortName ?: longName ?: fallbackName
177+
val longLabel = longName ?: shortName ?: fallbackName
160178

161179
// A node-colored pill avatar showing the short name identifies the person and matches the in-app node chip.
162180
// Set it on the shortcut itself (not just the Person) so launchers/Android Auto render it instead of a generic
@@ -189,8 +207,9 @@ class ConversationShortcutPublisher(
189207
}
190208

191209
private fun buildChannelShortcut(channel: Conversation.Channel, rank: Int): ShortcutInfoCompat {
192-
// channel.name is already the effective name (preset-derived for an empty primary), so no placeholder here.
193-
val channelName = channel.name.ifEmpty { "Channel ${channel.index}" }
210+
// channel.name is the effective name and never empty: Channel.name resolves an empty settings.name to the
211+
// modem-preset display name (or "Custom" for non-preset configs), so no fallback placeholder is needed.
212+
val channelName = channel.name
194213
// A black rounded-square badge with the channel number visually separates channels from the node-colored DM
195214
// pills.
196215
val icon = channelIcon(channel.index)
@@ -228,6 +247,8 @@ class ConversationShortcutPublisher(
228247
* emission).
229248
*/
230249
fun ensureConversationShortcut(contactKey: String, displayName: String) {
250+
// Protect this conversation from the observer's pruning until a contacts snapshot includes it.
251+
pendingOnDemandIds += contactKey
231252
val alreadyPublished = ShortcutManagerCompat.getDynamicShortcuts(context).any { it.id == contactKey }
232253
if (alreadyPublished) return
233254
// Match the styling the observer will republish so there is no generic-head flash: rounded channel badge with

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ import org.meshtastic.core.repository.SERVICE_NOTIFY_ID
5353
import org.meshtastic.core.resources.R.drawable
5454
import org.meshtastic.core.resources.R.raw
5555
import org.meshtastic.core.resources.Res
56+
import org.meshtastic.core.resources.channel
5657
import org.meshtastic.core.resources.client_notification
5758
import org.meshtastic.core.resources.connected
5859
import org.meshtastic.core.resources.connecting
@@ -87,6 +88,7 @@ import org.meshtastic.core.resources.new_node_seen
8788
import org.meshtastic.core.resources.no_local_stats
8889
import org.meshtastic.core.resources.powered
8990
import org.meshtastic.core.resources.reply
91+
import org.meshtastic.core.resources.unknown_username
9092
import org.meshtastic.core.resources.you
9193
import org.meshtastic.core.service.MarkAsReadReceiver.Companion.MARK_AS_READ_ACTION
9294
import org.meshtastic.core.service.ReactionReceiver.Companion.REACT_ACTION
@@ -602,12 +604,14 @@ class MeshNotificationManagerImpl(
602604
val lora = channelSet.lora_config ?: Channel.default.loraConfig
603605
val index = contactKey.substringBefore(NodeAddress.ID_BROADCAST).toIntOrNull()
604606
channelName = index?.let { channelSet.settings.getOrNull(it) }?.let { Channel(it, lora).name }
605-
conversationName = channelName ?: contactKey
607+
// Never fall back to the raw contactKey for user-facing labels (privacy-first convention).
608+
conversationName = channelName ?: getString(Res.string.channel)
606609
} else {
607610
// DM contactKey is "<channelIndex><userId>" where userId starts at the '!'.
608611
val userId = "!" + contactKey.substringAfter("!", missingDelimiterValue = "")
609612
val peer = nodeRepository.value.nodeDBbyNum.value.values.find { it.user.id == userId }
610-
conversationName = peer?.user?.long_name?.takeIf { it.isNotBlank() } ?: contactKey
613+
conversationName =
614+
peer?.user?.long_name?.takeIf { it.isNotBlank() } ?: getString(Res.string.unknown_username)
611615
}
612616
showConversationNotification(contactKey, isBroadcast, channelName, conversationName, isSilent = true)
613617
}

0 commit comments

Comments
 (0)