feat(desktop): fix mac notifications, new desktop icons (#5403)

desktop/README.md

@@ -55,9 +55,9 @@ The module depends on the JVM variants of KMP modules:
 
 **DI:** A Koin DI graph is bootstrapped in `Main.kt` with platform-specific implementations injected.
 
-**UI:** JetBrains Compose for Desktop with Material 3 theming. Desktop acts as a thin host shell, delegating almost entirely to fully shared KMP UI modules. Includes native macOS notification support (via `TrayState` and `bundleID` identification) and a monochrome SVG tray icon for a native look and feel.
+**UI:** JetBrains Compose for Desktop with Material 3 theming. Desktop acts as a thin host shell, delegating almost entirely to fully shared KMP UI modules. Includes native OS notifications (macOS `UNUserNotificationCenter`, Linux libnotify, Windows toast) and a monochrome SVG tray icon for a native look and feel.
 
-**Notifications:** Implements the common `NotificationManager` interface via `DesktopNotificationManager`. Repository-level notifications (messages, node events, alerts) are collected in `Main.kt` and forwarded to the system tray. macOS requires a consistent `bundleID` (configured in `build.gradle.kts`) and the `NSUserNotificationAlertStyle` key in `Info.plist` for notifications to appear correctly in the distributable.
+**Notifications:** Implements the common `NotificationManager` interface via `DesktopNotificationManager`. Repository-level notifications (messages, node events, alerts) are collected in `Main.kt` and forwarded to platform-native senders; the app falls back to Compose `TrayState` notifications only if native delivery fails. macOS requires a consistent `bundleID` (configured in `build.gradle.kts`) for proper app attribution.
 
 **Localization:** Desktop exposes a language picker, persisting the selected BCP-47 tag in `UiPreferencesDataSource.locale`. `Main.kt` applies the override to the JVM default `Locale` and uses a `staticCompositionLocalOf`-backed recomposition trigger so Compose Multiplatform `stringResource()` calls update immediately without recreating the Navigation 3 backstack.
 

desktop/src/main/kotlin/org/meshtastic/desktop/DesktopNotificationManager.kt

@@ -33,9 +33,9 @@ import androidx.compose.ui.window.Notification as ComposeNotification
 /**
  * Desktop notification manager that dispatches domain [Notification] objects to native OS notifications.
  *
- * Uses platform-specific [NativeNotificationSender] implementations (notify-send on Linux, osascript on macOS,
- * PowerShell toast on Windows) for proper native look-and-feel. Falls back to Compose Desktop tray notifications (via
- * [fallbackNotifications]) when the native sender is unavailable or fails.
+ * Uses platform-specific [NativeNotificationSender] implementations (libnotify on Linux, `UNUserNotificationCenter` on
+ * macOS, PowerShell toast on Windows) for proper native look-and-feel. Falls back to Compose Desktop tray notifications
+ * (via [fallbackNotifications]) when the native sender is unavailable or fails.
  *
  * All native sends are dispatched on a background scope to avoid blocking callers.
  *

desktop/src/main/kotlin/org/meshtastic/desktop/notification/MacOSNotificationSender.kt

@@ -17,63 +17,48 @@
 package org.meshtastic.desktop.notification
 
 import co.touchlab.kermit.Logger
+import com.sun.jna.Function
+import com.sun.jna.NativeLibrary
+import com.sun.jna.Pointer
 import org.meshtastic.core.repository.Notification
-import java.util.concurrent.TimeUnit
-
-private const val PROCESS_TIMEOUT_SECONDS = 5L
+import java.util.UUID
 
 /**
- * Sends notifications via `osascript` on macOS, using AppleScript's `display notification` command.
- *
- * Content is passed as arguments to a pre-built script — never interpolated into the script source — to prevent
- * injection from untrusted message text. The `on run argv` handler receives title/message as positional args.
+ * Sends notifications through macOS UserNotifications (`UNUserNotificationCenter`) via JNA + Objective-C runtime. This
+ * ensures notifications are attributed to the app bundle instead of Script Editor.
  */
-class MacOSNotificationSender : NativeNotificationSender {
+class MacOSNotificationSender private constructor(private val bridge: MacNotificationBridge) :
+    NativeNotificationSender {
+    constructor() : this(JnaMacNotificationBridge())
 
-    override fun send(notification: Notification): Boolean = runCommand(buildCommand(notification))
+    internal constructor(bridge: MacNotificationBridge, unused: Unit = Unit) : this(bridge)
 
-    /**
-     * Builds an `osascript` command that passes title and message as arguments to a safe `on run argv` handler.
-     *
-     * AppleScript's `on run argv` receives command-line arguments as a list, avoiding any need to escape quotes or
-     * special characters in user content.
-     */
-    internal fun buildCommand(notification: Notification): List<String> = buildList {
-        add("osascript")
+    @Volatile private var authorizationRequested = false
 
-        // Build the script as a safe handler that reads argv items
-        val scriptLines = buildList {
-            add("on run argv")
-            add("  set notifTitle to item 1 of argv")
-            add("  set notifMessage to item 2 of argv")
-            add("  set notifSubtitle to item 3 of argv")
-            add("  set isSilent to item 4 of argv")
-            if (notification.isSilent) {
-                add("  display notification notifMessage with title notifTitle subtitle notifSubtitle")
-            } else {
-                add(
-                    "  display notification notifMessage with title notifTitle" +
-                        " subtitle notifSubtitle sound name \"default\"",
-                )
+    override fun send(notification: Notification): Boolean {
+        if (!bridge.isAvailable) return false
+
+        if (!authorizationRequested) {
+            synchronized(this) {
+                if (!authorizationRequested) {
+                    val authOk = bridge.requestAuthorization(DEFAULT_AUTHORIZATION_OPTIONS)
+                    if (!authOk) {
+                        Logger.w { "UNUserNotificationCenter authorization request failed" }
+                    }
+                    authorizationRequested = true
+                }
             }
-            add("end run")
         }
 
-        // Pass each line with -e
-        for (line in scriptLines) {
-            add("-e")
-            add(line)
-        }
-
-        // Positional arguments after "--"
-        add("--")
-        add(notification.title)
-        add(notification.message)
-        add(categorySubtitle(notification.category))
-        add(if (notification.isSilent) "true" else "false")
+        return bridge.post(
+            title = notification.title,
+            message = notification.message,
+            subtitle = categorySubtitle(notification.category),
+            playSound = !notification.isSilent,
+        )
     }
 
-    private fun categorySubtitle(category: Notification.Category): String = when (category) {
+    internal fun categorySubtitle(category: Notification.Category): String = when (category) {
         Notification.Category.Message -> "Message"
         Notification.Category.NodeEvent -> "Node Event"
         Notification.Category.Battery -> "Low Battery"
@@ -81,27 +66,134 @@ class MacOSNotificationSender : NativeNotificationSender {
         Notification.Category.Service -> "Service"
     }
 
-    private fun runCommand(command: List<String>): Boolean = try {
-        val process = ProcessBuilder(command).redirectErrorStream(true).start()
-        val completed = process.waitFor(PROCESS_TIMEOUT_SECONDS, TimeUnit.SECONDS)
-        if (!completed) {
-            process.destroyForcibly()
-            Logger.w { "osascript timed out after ${PROCESS_TIMEOUT_SECONDS}s" }
-            false
-        } else {
-            val exitCode = process.exitValue()
-            if (exitCode != 0) {
-                val stderr = process.inputStream.bufferedReader().readText().take(MAX_STDERR_CHARS)
-                Logger.w { "osascript exited $exitCode: $stderr" }
-            }
-            exitCode == 0
-        }
-    } catch (e: java.io.IOException) {
-        Logger.w(e) { "Failed to run osascript" }
-        false
-    }
-
     companion object {
-        private const val MAX_STDERR_CHARS = 200
+        // UNAuthorizationOptions: badge(1 << 0), sound(1 << 1), alert(1 << 2)
+        internal const val DEFAULT_AUTHORIZATION_OPTIONS = 0b111L
     }
 }
+
+internal interface MacNotificationBridge {
+    val isAvailable: Boolean
+
+    fun requestAuthorization(options: Long): Boolean
+
+    fun post(title: String, message: String, subtitle: String, playSound: Boolean): Boolean
+}
+
+private class JnaMacNotificationBridge : MacNotificationBridge {
+    private val objc: NativeLibrary?
+    private val objcGetClass: Function?
+    private val selRegisterName: Function?
+    private val objcMsgSend: Function?
+
+    init {
+        val loaded =
+            if (!isMacOs()) {
+                LoadedBridge(null, null, null, null)
+            } else {
+                try {
+                    NativeLibrary.getInstance("UserNotifications")
+                    val nativeObjc = NativeLibrary.getInstance("objc")
+                    LoadedBridge(
+                        objc = nativeObjc,
+                        objcGetClass = nativeObjc.getFunction("objc_getClass"),
+                        selRegisterName = nativeObjc.getFunction("sel_registerName"),
+                        objcMsgSend = nativeObjc.getFunction("objc_msgSend"),
+                    )
+                } catch (e: UnsatisfiedLinkError) {
+                    Logger.w(e) { "Failed to initialize macOS notification bridge" }
+                    LoadedBridge(null, null, null, null)
+                } catch (e: SecurityException) {
+                    Logger.w(e) { "Failed to initialize macOS notification bridge" }
+                    LoadedBridge(null, null, null, null)
+                }
+            }
+
+        objc = loaded.objc
+        objcGetClass = loaded.objcGetClass
+        selRegisterName = loaded.selRegisterName
+        objcMsgSend = loaded.objcMsgSend
+    }
+
+    override val isAvailable: Boolean
+        get() = objc != null && objcGetClass != null && selRegisterName != null && objcMsgSend != null
+
+    override fun requestAuthorization(options: Long): Boolean = runCatching {
+        val centerClass = classRef("UNUserNotificationCenter") ?: return false
+        val center = msg(centerClass, selector("currentNotificationCenter")) ?: return false
+        msg(center, selector("requestAuthorizationWithOptions:completionHandler:"), options, Pointer.NULL)
+        true
+    }
+        .getOrElse { e ->
+            Logger.w(e) { "Failed to request UNUserNotificationCenter authorization" }
+            false
+        }
+
+    override fun post(title: String, message: String, subtitle: String, playSound: Boolean): Boolean = runCatching {
+        val contentClass = classRef("UNMutableNotificationContent") ?: return false
+        val requestClass = classRef("UNNotificationRequest") ?: return false
+        val centerClass = classRef("UNUserNotificationCenter") ?: return false
+
+        val content = msg(msg(contentClass, selector("alloc")), selector("init")) ?: return false
+        msg(content, selector("setTitle:"), nsString(title) ?: return false)
+        msg(content, selector("setBody:"), nsString(message) ?: return false)
+        msg(content, selector("setSubtitle:"), nsString(subtitle) ?: return false)
+
+        if (playSound) {
+            val soundClass = classRef("UNNotificationSound")
+            val defaultSound = soundClass?.let { msg(it, selector("defaultSound")) }
+            if (defaultSound != null) {
+                msg(content, selector("setSound:"), defaultSound)
+            }
+        }
+
+        val request =
+            msg(
+                requestClass,
+                selector("requestWithIdentifier:content:trigger:"),
+                nsString(UUID.randomUUID().toString()) ?: return false,
+                content,
+                Pointer.NULL,
+            ) ?: return false
+
+        val center = msg(centerClass, selector("currentNotificationCenter")) ?: return false
+        msg(center, selector("addNotificationRequest:withCompletionHandler:"), request, Pointer.NULL)
+        true
+    }
+        .getOrElse { e ->
+            Logger.w(e) { "Failed to post macOS notification" }
+            false
+        }
+
+    private fun classRef(name: String): Pointer? = objcGetClass?.invoke(Pointer::class.java, arrayOf(name)) as? Pointer
+
+    private fun selector(name: String): Pointer =
+        selRegisterName?.invoke(Pointer::class.java, arrayOf(name)) as? Pointer
+            ?: error("Unable to resolve selector '$name'")
+
+    private fun nsString(value: String): Pointer? {
+        val nsStringClass = classRef("NSString") ?: return null
+        return msg(nsStringClass, selector("stringWithUTF8String:"), value)
+    }
+
+    private fun msg(receiver: Pointer?, selector: Pointer, vararg args: Any?): Pointer? {
+        val function = objcMsgSend
+        val target = receiver
+        if (function == null || target == null) return null
+        val callArgs = arrayOfNulls<Any>(args.size + 2)
+        callArgs[0] = target
+        callArgs[1] = selector
+        args.copyInto(callArgs, destinationOffset = 2)
+        return function.invoke(Pointer::class.java, callArgs) as? Pointer
+    }
+
+    private fun isMacOs(): Boolean =
+        System.getProperty("os.name", "").lowercase().let { it.contains("mac") || it.contains("darwin") }
+
+    private data class LoadedBridge(
+        val objc: NativeLibrary?,
+        val objcGetClass: Function?,
+        val selRegisterName: Function?,
+        val objcMsgSend: Function?,
+    )
+}

desktop/src/test/kotlin/org/meshtastic/desktop/notification/MacOSNotificationSenderTest.kt

@@ -24,62 +24,78 @@ import kotlin.test.assertTrue
 
 class MacOSNotificationSenderTest {
 
-    private val sender = MacOSNotificationSender()
-
     @Test
-    fun `command starts with osascript`() {
-        val notification = Notification(title = "Hi", message = "There")
-        val cmd = sender.buildCommand(notification)
-        assertEquals("osascript", cmd[0])
+    fun `returns false when bridge unavailable`() {
+        val bridge = FakeBridge(available = false)
+        val sender = MacOSNotificationSender(bridge)
+
+        val result = sender.send(Notification(title = "Hi", message = "There"))
+
+        assertFalse(result)
+        assertEquals(0, bridge.authorizationCalls)
+        assertEquals(0, bridge.postCalls.size)
     }
 
     @Test
-    fun `non-silent notification includes sound name`() {
-        val notification = Notification(title = "Hi", message = "There", isSilent = false)
-        val cmd = sender.buildCommand(notification)
-        val script = cmd.filter { it.contains("display notification") }.joinToString(" ")
-        assertTrue(script.contains("sound name \"default\""), "Expected sound name in: $script")
+    fun `requests authorization once then posts notification`() {
+        val bridge = FakeBridge(available = true)
+        val sender = MacOSNotificationSender(bridge)
+
+        val first =
+            sender.send(Notification(title = "One", message = "First", category = Notification.Category.Battery))
+        val second =
+            sender.send(Notification(title = "Two", message = "Second", category = Notification.Category.Alert))
+
+        assertTrue(first)
+        assertTrue(second)
+        assertEquals(1, bridge.authorizationCalls)
+        assertEquals(MacOSNotificationSender.DEFAULT_AUTHORIZATION_OPTIONS, bridge.lastAuthorizationOptions)
+        assertEquals(2, bridge.postCalls.size)
+        assertEquals("Low Battery", bridge.postCalls[0].subtitle)
+        assertEquals("Alert", bridge.postCalls[1].subtitle)
     }
 
     @Test
-    fun `silent notification omits sound name`() {
-        val notification = Notification(title = "Quiet", message = "Shhh", isSilent = true)
-        val cmd = sender.buildCommand(notification)
-        val script = cmd.filter { it.contains("display notification") }.joinToString(" ")
-        assertFalse(script.contains("sound name"), "Expected no sound name in: $script")
+    fun `silent notification disables sound`() {
+        val bridge = FakeBridge(available = true)
+        val sender = MacOSNotificationSender(bridge)
+
+        sender.send(Notification(title = "Quiet", message = "Shhh", isSilent = true))
+
+        assertEquals(1, bridge.postCalls.size)
+        assertFalse(bridge.postCalls[0].playSound)
     }
 
     @Test
-    fun `title and message passed as positional args after double dash`() {
-        val notification = Notification(title = "My Title", message = "My Message")
-        val cmd = sender.buildCommand(notification)
-        val dashDashIdx = cmd.indexOf("--")
-        assertTrue(dashDashIdx > 0, "Expected '--' separator in command")
-        assertEquals("My Title", cmd[dashDashIdx + 1])
-        assertEquals("My Message", cmd[dashDashIdx + 2])
+    fun `non-silent notification enables sound`() {
+        val bridge = FakeBridge(available = true)
+        val sender = MacOSNotificationSender(bridge)
+
+        sender.send(Notification(title = "Loud", message = "Ping", isSilent = false))
+
+        assertEquals(1, bridge.postCalls.size)
+        assertTrue(bridge.postCalls[0].playSound)
     }
 
-    @Test
-    fun `special characters are not interpolated into script`() {
-        val notification = Notification(title = "It's \"tricky\" & <bad>", message = "'; drop table;")
-        val cmd = sender.buildCommand(notification)
-        // Script lines should not contain the user content — only argv references
-        val scriptLines = cmd.filter { it.contains("notifTitle") || it.contains("notifMessage") }
-        for (line in scriptLines) {
-            assertFalse(line.contains("tricky"), "User content leaked into script: $line")
-            assertFalse(line.contains("drop table"), "User content leaked into script: $line")
+    private class FakeBridge(private val available: Boolean) : MacNotificationBridge {
+        override val isAvailable: Boolean
+            get() = available
+
+        var authorizationCalls: Int = 0
+        var lastAuthorizationOptions: Long? = null
+        val postCalls = mutableListOf<PostCall>()
+
+        override fun requestAuthorization(options: Long): Boolean {
+            authorizationCalls += 1
+            lastAuthorizationOptions = options
+            return true
+        }
+
+        override fun post(title: String, message: String, subtitle: String, playSound: Boolean): Boolean {
+            postCalls += PostCall(title = title, message = message, subtitle = subtitle, playSound = playSound)
+            return true
         }
-        // But args should contain the raw content
-        val dashDashIdx = cmd.indexOf("--")
-        assertEquals("It's \"tricky\" & <bad>", cmd[dashDashIdx + 1])
-        assertEquals("'; drop table;", cmd[dashDashIdx + 2])
     }
 
-    @Test
-    fun `battery category becomes subtitle`() {
-        val notification = Notification(title = "Bat", message = "Low", category = Notification.Category.Battery)
-        val cmd = sender.buildCommand(notification)
-        val dashDashIdx = cmd.indexOf("--")
-        assertEquals("Low Battery", cmd[dashDashIdx + 3])
-    }
+    private data class PostCall(val title: String, val message: String, val subtitle: String, val playSound: Boolean)
 }