Skip to content

fix(notifications): real OS permission gate + delivery for #1152#1247

Merged
senamakel merged 2 commits intotinyhumansai:mainfrom
CodeGhost21:fix/issue-1152-test-notification
May 5, 2026
Merged

fix(notifications): real OS permission gate + delivery for #1152#1247
senamakel merged 2 commits intotinyhumansai:mainfrom
CodeGhost21:fix/issue-1152-test-notification

Conversation

@CodeGhost21
Copy link
Copy Markdown
Contributor

@CodeGhost21 CodeGhost21 commented May 5, 2026

Summary

  • Fixes Fix Send test notification not showing OS notification #1152. Clicking Send test notification previously reported success but no banner appeared.
  • Two root causes in the bundled tauri-plugin-notification: permission_state() is hardcoded to Granted on desktop, and .show() spawns notify-rust on a background task and discards the result. So a denied user sailed through the permission gate, the OS silently dropped the banner, and the UI happily declared "sent."
  • Routed the bridge through dedicated Rust commands and replaced the macOS delivery path with a direct UNUserNotificationCenter.add(...) call so the completion handler propagates real errors instead of getting swallowed.

Changes

  • New app/src-tauri/src/native_notifications/ module:
    • notification_permission_state and notification_permission_request drive UNUserNotificationCenter directly so the bridge sees the real OS authorization state (the plugin's hardcoded Granted is no longer trusted).
    • show_native_notification re-checks authorization, builds a UNNotificationRequest, and waits for addNotificationRequest:withCompletionHandler: so a successful return means the system accepted the request, not just that an async dispatch was scheduled.
    • Linux/Windows fall back to the existing plugin path.
  • Frontend bridge (app/src/lib/nativeNotifications/tauriBridge.ts) switched from plugin:notification|* to the dedicated commands.
  • Allow-listed the three custom commands in app/src-tauri/permissions/allow-core-process.toml (Tauri 2 ACL requires explicit entries; the old plugin commands were covered by notification:default).
  • Lifted ~129 lines of inline notification code out of lib.rs (was 2100 lines, project rule prefers ≤500).

Test plan

  • pnpm debug unit nativeNotifications — 23 new bridge tests for the new contract (uses dedicated commands, maps backend states correctly, surfaces Rust errors).
  • pnpm debug unit OpenhumanLinkModal.notifications — existing 4 modal-flow tests still pass against the new bridge.
  • cargo test --manifest-path app/src-tauri/Cargo.toml --lib native_notifications — Rust unit tests for the macOS state mapping helpers.
  • cargo check --manifest-path app/src-tauri/Cargo.toml — Tauri shell compiles clean.
  • Hands-on macOS verification with pnpm dev:app:
    • Scenario 1 (denied): UI surfaces "Notification permission is off…" + Retry button — no false "sent" message.
    • Scenario 2 (granted): banner "OpenHuman is good to go" actually appears top-right.
    • Scenario 3 (repeated taps): each click produces a fresh banner (unique-id fallback).

Note: an unrelated pre-existing test failure in src/components/walkthrough/__tests__/AppWalkthrough.test.tsx (missing react-joyride resolution) exists on main and is not introduced by this PR.

Closes #1152

Summary by CodeRabbit

  • New Features

    • Added native notification permission state checking and requesting capabilities
    • Enhanced native notification system with improved permission handling
  • Refactor

    • Reorganized notification code into a dedicated module for better maintainability
  • Tests

    • Added comprehensive test coverage for notification permission and delivery functionality

…ai#1152

The "Send test notification" flow reported success but no banner
appeared because the bundled tauri-plugin-notification's
permission_state() / request_permission() are hardcoded to Granted
on desktop, and its .show() spawns notify-rust on a background task
and discards the result. So a denied user passed the permission
gate, the OS silently dropped the banner, and the UI happily
declared "sent."

- Move notification IPC into app/src-tauri/src/native_notifications/
  and route show_native_notification through UNUserNotificationCenter
  on macOS: real authorization check, real
  addNotificationRequest:withCompletionHandler: dispatch, with
  delivery errors surfaced back to the caller.
- Switch the frontend bridge from plugin:notification|* to the
  dedicated notification_permission_state /
  notification_permission_request / show_native_notification
  commands so the UI sees the real OS state.
- Cover the new contract with bridge unit tests and small Rust unit
  tests for the macOS state mapping.

Closes tinyhumansai#1152
The capability ACL rejected notification_permission_state /
notification_permission_request / show_native_notification with
"Command not found" because Tauri 2 requires custom commands to be
explicitly listed under permissions/. The previous commit moved
these into a dedicated module and switched the frontend bridge to
call them directly (the old plugin commands plugin:notification|*
were already covered by notification:default), but never updated
the allowlist — verified end-to-end on macOS by clicking
"Send test notification" and watching the banner appear.
@CodeGhost21 CodeGhost21 requested a review from a team May 5, 2026 18:16
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 5, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3598ede6-2b77-429b-9976-e7e8ee0ff12e

📥 Commits

Reviewing files that changed from the base of the PR and between 932703f and 63e1c78.

📒 Files selected for processing (5)
  • app/src-tauri/permissions/allow-core-process.toml
  • app/src-tauri/src/lib.rs
  • app/src-tauri/src/native_notifications/mod.rs
  • app/src/lib/nativeNotifications/__tests__/tauriBridge.test.ts
  • app/src/lib/nativeNotifications/tauriBridge.ts

📝 Walkthrough

Walkthrough

This PR refactors native notification handling by extracting Tauri command handlers from the main library into a dedicated native_notifications module, routing permission queries and notifications through OS-level APIs on macOS while falling back to the notification plugin on other platforms, and updating the TypeScript bridge to invoke the new Rust commands.

Changes

Native Notifications Refactor

Layer / File(s) Summary
Permission Configuration
app/src-tauri/permissions/allow-core-process.toml
Three Tauri commands are added to the allowlist: notification_permission_state, notification_permission_request, and show_native_notification.
Rust Module Declaration & Wiring
app/src-tauri/src/lib.rs
Module native_notifications is declared and the three notification commands are wired from lib.rs to handlers in the new module.
macOS & Cross-Platform Backend
app/src-tauri/src/native_notifications/mod.rs
Three Tauri commands are implemented: permission state/request use macOS OS APIs (with 2–5s timeouts and authorization status mapping), returning "granted" on non-macOS; notification dispatch verifies permission and submits UNNotificationRequest on macOS, delegating to tauri_plugin_notification elsewhere, with deduplication via tag-derived or timestamp-based identifiers. Includes unit tests for authorization state mapping.
TypeScript Bridge & Integration
app/src/lib/nativeNotifications/tauriBridge.ts
Invocations are rerouted from plugin permission gates to dedicated Rust commands; a mapBackendState helper normalizes backend status strings (provisional/ephemeral → granted, prompt/default/not_determined → prompt, etc.); permission request and notification submission preserve the existing return shape.
Tests
app/src/lib/nativeNotifications/__tests__/tauriBridge.test.ts
Comprehensive unit tests verify command selection (avoiding plugin routing), permission state normalization, conditional prompting behavior, non-Tauri fallback, error mapping, and notification invocation payload shape and handling.

Sequence Diagram

sequenceDiagram
    participant UI as Frontend<br/>(TypeScript)
    participant Bridge as Tauri Bridge<br/>(tauriBridge.ts)
    participant TauriRuntime as Tauri Runtime<br/>(invoke)
    participant RustBackend as Native Notifications<br/>(Rust)
    participant macOSAPI as macOS API<br/>(UNUserNotificationCenter)

    UI->>Bridge: getNotificationPermissionState(requestIfNeeded=true)
    Bridge->>TauriRuntime: invoke("notification_permission_state")
    TauriRuntime->>RustBackend: notification_permission_state()
    RustBackend->>macOSAPI: UNNotificationSettings query
    macOSAPI-->>RustBackend: authorization status
    RustBackend-->>TauriRuntime: "granted"|"denied"|"prompt"
    TauriRuntime-->>Bridge: backend state string
    Bridge->>Bridge: mapBackendState(raw)
    Bridge-->>UI: NotificationPermissionState

    UI->>Bridge: showNativeNotification(title, body, tag?)
    alt permission not granted
        Bridge->>Bridge: ensureNotificationPermission()
        Bridge->>TauriRuntime: invoke("notification_permission_request")
        RustBackend->>macOSAPI: requestAuthorizationWithOptions
        macOSAPI-->>RustBackend: user response
    end
    Bridge->>TauriRuntime: invoke("show_native_notification", {title, body, tag})
    TauriRuntime->>RustBackend: show_native_notification(...)
    RustBackend->>macOSAPI: UNMutableNotificationContent + UNNotificationRequest
    macOSAPI->>macOSAPI: add notification request
    macOSAPI-->>RustBackend: dispatch result (with timeout)
    RustBackend-->>TauriRuntime: Ok(()) | Err(NSError message)
    TauriRuntime-->>Bridge: result
    Bridge-->>UI: {delivered: true} | {delivered: false, reason, error}
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • tinyhumansai/openhuman#998: Touches the same native notification commands and permission flow implementation.
  • tinyhumansai/openhuman#1024: Modifies the TypeScript bridge layer (tauriBridge.ts) to call the new Rust commands and return structured permission/notification results.

Suggested reviewers

  • senamakel

Poem

🐰 Whiskers twitch with glee as notifications take flight,
macOS APIs dance, permission prompts shine bright,
From Rust to TypeScript, commands bridge the way,
Test notifications now appear—hooray, hooray! 🔔

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the fix addresses the core issue from #1152: implementing real OS permission handling and proper notification delivery instead of false success reports.
Linked Issues check ✅ Passed The pull request directly addresses all coding requirements from #1152: real OS permission detection via dedicated commands, accurate permission state via UNUserNotificationCenter on macOS, delivery verification with error surfacing, and retry support after permission grant.
Out of Scope Changes check ✅ Passed All changes are scoped to fixing notification permission handling and delivery: new native_notifications module, permission configuration, bridge routing, and tests directly support the #1152 objectives with no unrelated modifications.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@senamakel senamakel merged commit ec4d3de into tinyhumansai:main May 5, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix Send test notification not showing OS notification

2 participants