Skip to content

Commit 778a0b4

Browse files
committed
Fix macOS notifications visibility and API usage
Updates notifier.py to correctly use the async API of desktop-notifier 6.x via asyncio.run. Re-introduces osascript as a robust fallback for macOS to ensure notifications appear even if the high-level API fails. Updated unit tests.
1 parent c62ee22 commit 778a0b4

File tree

2 files changed

+56
-16
lines changed

2 files changed

+56
-16
lines changed

lb_ui/services/notifier.py

Lines changed: 40 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import logging
22
import platform
33
import os
4+
import subprocess
5+
import asyncio
46
from pathlib import Path
57
from typing import Optional
68

@@ -30,21 +32,39 @@ def _get_cache_dir() -> Path:
3032
def _resolve_icon_path() -> str | None:
3133
"""Resolve the path to the best available application icon."""
3234
try:
33-
# Priority 1: Optimized cached icon (v4, cropped, 64x64)
34-
cache_icon = _get_cache_dir() / "tray_icon_v4_64.png"
35+
# Priority 1: Optimized cached icon (v7, logo_sys2)
36+
cache_icon = _get_cache_dir() / "tray_icon_v7_64.png"
3537
if cache_icon.exists():
3638
return str(cache_icon.absolute())
3739

3840
# Priority 2: Older cached versions
39-
for old_name in ["tray_icon_128.png", "tray_icon_64.png"]:
41+
for old_name in ["tray_icon_v6_64.png", "tray_icon_v5_64.png", "tray_icon_v4_64.png"]:
4042
old_path = _get_cache_dir() / old_name
4143
if old_path.exists():
4244
return str(old_path.absolute())
45+
46+
# Priority 3: Original source icon
47+
current_file = Path(__file__)
48+
project_root = current_file.parents[2]
49+
source_icon = project_root / "docs" / "img" / "logo_sys2.png"
50+
if source_icon.exists():
51+
return str(source_icon.absolute())
4352
except Exception:
4453
pass
4554
return None
4655

4756

57+
def _send_macos_osascript(title: str, message: str, icon_path: str | None = None) -> None:
58+
"""Fallback macOS notification using osascript."""
59+
safe_title = title.replace('"', '\\"')
60+
safe_message = message.replace('"', '\\"')
61+
script = f'display notification "{safe_message}" with title "{safe_title}"'
62+
if icon_path:
63+
script += f' with icon file (POSIX file "{icon_path}")'
64+
65+
subprocess.run(["osascript", "-e", script], capture_output=True, timeout=5)
66+
67+
4868
def send_notification(
4969
title: str,
5070
message: str,
@@ -64,15 +84,23 @@ def send_notification(
6484
try:
6585
icon_path = _resolve_icon_path()
6686

67-
# macOS specific implementation using desktop-notifier (Native API)
68-
if platform.system() == "Darwin" and DesktopNotifier is not None:
69-
notifier = DesktopNotifier(app_name=app_name)
70-
# send_sync is perfect for CLI tools
71-
notifier.send_sync(
72-
title=title,
73-
message=message,
74-
icon=icon_path,
75-
)
87+
# macOS specific implementation
88+
if platform.system() == "Darwin":
89+
if DesktopNotifier is not None:
90+
try:
91+
notifier = DesktopNotifier(app_name=app_name)
92+
# desktop-notifier 6.x is async only
93+
asyncio.run(notifier.send(
94+
title=title,
95+
message=message,
96+
icon=icon_path,
97+
))
98+
return
99+
except Exception as exc:
100+
logger.debug(f"desktop-notifier failed, falling back to osascript: {exc}")
101+
102+
# Robust fallback for macOS
103+
_send_macos_osascript(title, message, icon_path)
76104
return
77105

78106
# Fallback to plyer for Linux/Windows

tests/unit/lb_ui/test_notifier.py

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11

22
import pytest
3-
from unittest.mock import patch, MagicMock
3+
from unittest.mock import patch, MagicMock, ANY
44
import logging
55

66
# We import the module under test inside tests to facilitate patching imports if needed,
@@ -30,17 +30,29 @@ def test_send_notification_success_linux(self):
3030
def test_send_notification_macos(self):
3131
"""Test notification via desktop-notifier on macOS."""
3232
with patch("platform.system", return_value="Darwin"), \
33-
patch("lb_ui.services.notifier.DesktopNotifier") as mock_notifier_cls:
33+
patch("lb_ui.services.notifier.DesktopNotifier") as mock_notifier_cls, \
34+
patch("asyncio.run") as mock_asyncio_run:
3435

3536
mock_instance = mock_notifier_cls.return_value
3637
notifier.send_notification("Title", "Message")
3738

3839
mock_notifier_cls.assert_called_once()
39-
mock_instance.send_sync.assert_called_once()
40-
_, kwargs = mock_instance.send_sync.call_args
40+
mock_asyncio_run.assert_called_once()
41+
# The argument to asyncio.run is the coroutine from mock_instance.send
42+
mock_instance.send.assert_called_once()
43+
_, kwargs = mock_instance.send.call_args
4144
assert kwargs["title"] == "Title"
4245
assert kwargs["message"] == "Message"
4346

47+
def test_send_notification_macos_fallback(self):
48+
"""Test fallback to osascript if desktop-notifier fails."""
49+
with patch("platform.system", return_value="Darwin"), \
50+
patch("lb_ui.services.notifier.DesktopNotifier", side_effect=Exception("API Error")), \
51+
patch("lb_ui.services.notifier._send_macos_osascript") as mock_osascript:
52+
53+
notifier.send_notification("Title", "Message")
54+
mock_osascript.assert_called_once_with("Title", "Message", ANY)
55+
4456
def test_send_notification_plyer_missing(self):
4557
"""Test behavior when plyer is not installed (notification module is None) on Linux."""
4658
with patch("lb_ui.services.notifier.notification", None), \

0 commit comments

Comments
 (0)