Skip to content

Fix/windows desktop notifications fixes gen2 - #3

Open
bernhardkaindl wants to merge 7 commits into
mainfrom
fix/windows-desktop-notifications-fixes-gen2
Open

bernhardkaindl wants to merge 7 commits into
mainfrom
fix/windows-desktop-notifications-fixes-gen2

Conversation

@bernhardkaindl

Copy link
Copy Markdown

Local recreation to get a Copilot review of block /buzz/pull/7614:

fix(desktop): enable Windows-native winrt notifications and make them reliable

Potentially better PR (to be tested, reads very compentent and well-tested, but needs to be verified by a 2nd party):

  • 6876

This PR combines @fatima-n09's PR

  • 6689
    with my fixes to make them reliable and fix the remaining issues found.
  • Tested on Windows 11: When clicking the notification, the correct message and if in a thread/branch, the message in the thread opens an is highlighted as having just arrived.

Fixes

  • 2445
  • 4660
  • 6982
  • 6377

Obsoletes

  • 6678
  • 6689
  • 7275

Related to

  • 7452

Problems

  1. On the current main branch, Windows notitifications are impossible to enable.
  2. Even with 6689 applied, Windows notification delivery and activation had three related gaps:
    • permission repair could still be in flight when the first notification was sent,
    • clicking a normal message notification could open an empty reply branch,
    • broad live channel subscriptions updated only the projected message cache, so an active channel could miss the message until reload.

Solution

  • Await and confirm desktop notification permission before feed, DM, and dedicated thread-reply delivery.
  • Carry explicit timeline-versus-thread intent in notification targets and channel routes.
  • Merge visible live channel messages into the authoritative channel window store before projection.
  • Add focused regression coverage for permission sequencing, activation routing, notify-while-viewing, and live window projection.

Fatima Nur and others added 2 commits September 13, 2026 00:56
The show_native_notification command only handled Linux and macOS.
On Windows it returned an error, forcing the frontend to fall through
to window.Notification (WebKit API). WebView2's Notification.permission
reports 'denied' even when the WinRT toast API is available, so the
app never appeared in Windows Settings > System > Notifications and
the settings toggle was stuck showing 'Desktop notifications are blocked.'

- Add tauri-winrt-notification as a Windows-specific dependency
- Add a windows module in notifications.rs that posts WinRT toasts
  using the app's Tauri identifier as AppUserModelID (this is what
  registers the app with Windows notification settings)
- Handle click actions through WinRT Activated handler, forwarding
  to the same native-notification-activated event that Linux uses
- Add isWindowsPlatform() helper to platform.ts
- Skip WebView2 Notification.permission check on Windows in
  getDesktopNotificationPermissionState() — use the Tauri plugin's
  isPermissionGranted() which queries native WinRT status
- Route Windows through the native show_native_notification path
  in sendDesktopNotification()
- Skip the Tauri plugin's onAction listener on Windows (click
  actions come through the native WinRT event instead)

Fixes block#6377

Signed-off-by: Fatima Nur <fatimanur424@example.com>
Problem
-------
Windows notification delivery and activation had three related gaps:

- permission repair could still be in flight when the first notification was sent,
- clicking a normal message notification could open an empty reply branch,
- broad live channel subscriptions updated only the projected message cache, so an active channel could miss the message until reload.

Solution
--------
- Await and confirm desktop notification permission before feed, DM, and dedicated thread-reply delivery.
- Carry explicit timeline-versus-thread intent in notification targets and channel routes.
- Merge visible live channel messages into the authoritative channel window store before projection.
- Add focused regression coverage for permission sequencing, activation routing, notify-while-viewing, and live window projection.

Signed-off-by: Bernhard Kaindl <bernhardkaindl7@gmail.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Critical notification delivery and activation issues, along with multiple routing and error-handling defects, remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR enables Windows-native WinRT notifications and improves permission sequencing, activation routing, and live channel projection.

Changes:

  • Adds Windows detection and native toast delivery.
  • Gates notifications on permission readiness.
  • Adds explicit timeline/thread routing.
  • Projects live channel updates into the message window.
  • Adds regression tests and Windows dependencies.
File summaries
File Summary
desktop/src/shared/lib/platform.ts Windows platform detection
desktop/src/shared/lib/platform.test.mjs Platform tests
desktop/src/features/notifications/use-feed-desktop-notifications.ts Feed permission sequencing
desktop/src/features/notifications/use-feed-desktop-notifications.test.mjs Feed sequencing tests
desktop/src/features/notifications/lib/target.ts Notification routing targets
desktop/src/features/notifications/lib/target.test.mjs Target tests
desktop/src/features/notifications/lib/desktop.ts Permission and native notification handling
desktop/src/features/notifications/lib/desktop.test.mjs Permission tests
desktop/src/features/channels/useLiveChannelUpdates.ts Live window projection
desktop/src/features/channels/useLiveChannelUpdates.test.mjs Live update tests
desktop/src/features/channels/ui/useChannelRouteTarget.ts Timeline routing
desktop/src/features/channels/ui/channelSearchKeys.ts Route search keys
desktop/src/features/channels/ui/ChannelScreen.types.ts Route target prop
desktop/src/features/channels/ui/ChannelScreen.tsx Route prop wiring
desktop/src/app/useAppShellDesktopNotifications.ts Notification delivery sequencing
desktop/src/app/routes/channels.$channelId.tsx Route parsing
desktop/src/app/routes/ChannelRouteScreen.tsx Route target wiring
desktop/src/app/navigation/useAppNavigation.ts Navigation options
desktop/src/app/AppShell.helpers.ts Activation routing
desktop/src/app/AppShell.helpers.test.mjs Activation tests
desktop/src-tauri/src/commands/notifications.rs Windows toast implementation
desktop/src-tauri/Cargo.toml Native notification dependency
desktop/src-tauri/Cargo.lock Dependency lock update
Review details

Suppressed comments (8)

desktop/src-tauri/src/commands/notifications.rs:143

  • Toast::new(&app_id) only creates a notifier for an AppUserModelID that Windows already knows; this code never registers the identifier with a Start Menu shortcut/installer or repairs that registration. On unpackaged Tauri builds, xyz.block.buzz.app can therefore be rejected as unregistered and every toast is dropped. Register or self-heal the AUMID before using it.
        let app_id = app.config().identifier.clone();

        std::thread::spawn(move || {
            let app_clone = app.clone();
            let result = Toast::new(&app_id)

desktop/src/app/routes/ChannelRouteScreen.tsx:329

  • This forwards targetMessageView only through the ordinary ChannelScreen path. When projectHome is present, the earlier ProjectChannelHome return does not accept or forward this prop to its ChannelScreen, so a root-message notification in a project channel falls back to the default route handling and opens a reply panel instead of staying in the timeline.
      targetMessageView={targetMessageView}

desktop/src/app/useAppShellDesktopNotifications.ts:94

  • ensureDesktopNotificationPermissionGranted() can reject when the native permission request fails, but this .then has no rejection handler and the surrounding void drops it. A permission failure therefore becomes an unhandled rejection and the DM notification is lost; handle or queue the event at this boundary, or propagate it to a durable retry owner.
      void ensureDesktopNotificationPermissionGranted().then(
        async (permissionGranted) => {

desktop/src/app/useAppShellDesktopNotifications.ts:141

  • ensureDesktopNotificationPermissionGranted() can reject when the native permission request fails, but this .then has no rejection handler and the surrounding void drops it. A permission failure therefore becomes an unhandled rejection and the thread-reply notification is lost; handle or queue the event at this boundary, or propagate it to a durable retry owner.
      void ensureDesktopNotificationPermissionGranted().then(
        async (permissionGranted) => {

desktop/src/features/channels/ui/useChannelRouteTarget.ts:124

  • targetMessageView === "timeline" is checked only after !targetMessage.parentId. A notification with openInThread: false can still carry root/reply tags (the new target test does), so after the new goChannel(...messageView: "timeline") route is loaded this effect falls through to getThreadRouteTarget and opens the branch anyway. Move this guard before the parent check so the explicit timeline intent wins.
      if (targetMessageView === "timeline") {
        handledThreadRouteTargetRef.current = targetKey;
        return;

desktop/src/features/channels/useLiveChannelUpdates.test.mjs:190

  • Please assert the rendered channelMessagesKey cache here, not only liveOverlay. The production fix is projectChannelWindowMessages(...); removing that call leaves this overlay assertion green while the timeline cache remains stale, so the regression test is not falsifiable against the user-visible behavior.
    assert.deepEqual(
      h.queryClient
        .getQueryData(h.channelWindowKey("channel-0"))
        .liveOverlay.map((item) => item.id),
      ["mention"],
    );

desktop/src/features/channels/useLiveChannelUpdates.ts:372

  • The added regression only asserts liveOverlay on the window store. It never asserts the projected channelMessagesKey (nor a loaded authoritative page), so removing projectChannelWindowMessages(queryClient, channelId) would still pass while the original “miss until reload” bug returns. Assert the rendered message cache, ideally with an existing authoritative page, so this production seam is falsifiable.
        projectChannelWindowMessages(queryClient, channelId);

desktop/src/features/notifications/use-feed-desktop-notifications.ts:209

  • This new batch awaits permission repair, which can reject when the native request fails, but the returned promise is discarded here. That produces an unhandled rejection and silently loses the feed items; handle the failure at an owner that records/retries the pending batch rather than using void here.
      void deliverFeedNotificationBatch(
        newItems,
        autoRequestPermissionIfNeeded,
  • Files reviewed: 22/23 changed files
  • Comments generated: 8
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread desktop/src-tauri/src/commands/notifications.rs Outdated
Comment thread desktop/src/features/notifications/lib/desktop.ts
Comment thread desktop/src-tauri/src/commands/notifications.rs Outdated
Comment thread desktop/src/app/AppShell.helpers.ts Outdated
Comment thread desktop/src/features/channels/ui/channelSearchKeys.ts
Comment thread desktop/src/features/notifications/lib/desktop.ts Outdated
Comment thread desktop/src/features/notifications/lib/desktop.ts Outdated
Comment thread desktop/src/features/notifications/use-feed-desktop-notifications.ts Outdated
Signed-off-by: Bernhard Kaindl <bernhardkaindl7@gmail.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Six moderate review findings remain unresolved in routing, live projection, and permission handling.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (5)

Previously missed (1) — in code that hasn't changed since the last review.

desktop/src/features/channels/useLiveChannelUpdates.ts:364

  • isUnreadTriggerKind is intentionally narrower than the authoritative timeline set: it excludes visible diff/system/job/huddle rows and auxiliary events. The broad subscription still adds those events to channelMessagesKey, but this guard omits them from channelWindowKey, so the next authoritative projection can discard them and the active channel misses the event until reload. Merge every row/aux kind recognized by the channel-window subscription, not only unread-trigger kinds.

desktop/src/app/routes/ChannelRouteScreen.tsx:329

  • This forwards the new timeline intent only to the ordinary ChannelScreen branch. For a project-home channel, the earlier branch renders ProjectChannelHome with targetMessageId but no targetMessageView, and its nested ChannelScreen therefore follows the default root-message behavior and can open the reply panel. Forward the view mode through ProjectChannelHome as well so notification clicks have the same behavior in project channels.
      targetForumReplyId={targetReplyId}
      targetMessageEvents={targetMessageEvents}
      targetMessageId={targetMessageId}
      targetMessageView={targetMessageView}

desktop/src/features/notifications/lib/desktop.ts:168

  • The Windows branch collapses every isPermissionGranted() === false result to "default", but the native API only returns a boolean, so a user-denied system permission is indistinguishable from an unprompted one. After denial, every DM/thread callback will call requestPermission() again, and the settings hook never observes "denied" to disable the feature. Preserve/cache a terminal native denial (or otherwise stop retrying it per event) so a persistent block does not trigger repeated permission requests.
  if (isTauri() && isWindowsPlatform()) {
    try {
      return (await isPermissionGranted()) ? "granted" : "default";
    } catch {
      return "default";

desktop/src/features/notifications/lib/target.ts:46

  • Broadcast replies are intentionally treated as timeline rows (isThreadReply excludes isBroadcastReply and the route helper returns the reply id), but this condition only checks for a root tag. A feed notification for a broadcast reply therefore sets openInThread: true, sends activation through the thread/search path, and can open the wrong view. Exclude broadcast replies when deriving this flag, using the existing isBroadcastReply helper.
    openInThread: getThreadReference(item.tags).rootId !== null,

desktop/src/features/notifications/use-feed-desktop-notifications.ts:107

  • This catch converts a permission-state/request failure into false, but the feed effect adds every item to seenItemIdsRef and persists those IDs before awaiting the batch. A transient permission IPC failure therefore suppresses the current batch and permanently prevents those feed items from being retried. Distinguish an explicit denial from an operational error and retain or durably retry the items on the latter instead of returning a terminal false.
  } catch (error) {
    console.warn("Failed to request desktop notification permission", error);
    return false;
  • Files reviewed: 25/26 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread desktop/src/features/channels/ui/useChannelRouteTarget.ts Outdated
Signed-off-by: Bernhard Kaindl <bernhardkaindl7@gmail.com>
@bernhardkaindl

Copy link
Copy Markdown
Author

Addressed all five suppressed findings from review 5190904492 in 0f89114:

  • Live authoritative storage now uses CHANNEL_TIMELINE_CONTENT_KINDS / CHANNEL_AUX_EVENT_KINDS, independent of the narrower unread policy. The regression exercises diff, system, huddle, reaction, ordinary-reply, and subsequent projection behavior.
  • targetMessageView now flows through ChannelRouteScreen -> ProjectChannelHome -> ChannelScreen; a focused Playwright test proves a project-home timeline target does not open the thread panel.
  • Windows permission state now comes from ToastNotifier::Setting() on Tauri's initialized main thread. Only Enabled maps to granted; all native disabled states map to denied. Rust and frontend mapping tests cover this.
  • Broadcast feed replies now remain exact timeline-row targets rather than opening a thread.
  • Feed handling distinguishes terminal denial from operational error. IDs are persisted only after successful delivery or terminal denial; failed permission IPC and failed delivery remain retryable, with in-flight reservations and generation fencing preventing duplicates and stale writes.

I also made background settings refreshes catch and log native IPC failures, while explicit user toggles retain their visible error path.

Validation: 35 focused frontend tests, TypeScript, Biome, Rust formatting, native notification test, E2E build, and the focused project-home Playwright regression all pass.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved critical and moderate issues remain in Windows activation and registration, live-channel projection, and notification delivery.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (6)

desktop/src/features/channels/ui/useChannelRouteTarget.ts:127

  • This explicit timeline branch is bypassed by the handled-target guard when the same channelId:messageId was previously processed as a thread target: targetKey does not include targetMessageView, so a later notification with messageView=timeline returns at line 112 and leaves the existing thread panel open. Include the view intent in the handled key (or evaluate timeline intent before that guard) so repeated clicks with different intents are routed independently.
    // Explicit timeline intent outranks ancestry-based thread navigation.
    if (targetMessageView === "timeline") {
      handledThreadRouteTargetRef.current = targetKey;
      return;

desktop/src/features/channels/useLiveChannelUpdates.ts:382

  • This hook subscribes every member channel, so it now creates/updates a window for each background channel on every timeline or auxiliary event. mergeLiveChannelWindowEvent only deduplicates and has no retention bound, allowing liveOverlay/liveAux (and the projected message list) to grow for a busy channel for the lifetime of its cache while also doing projection work per event. Add bounded eviction or limit this merge to the active channel while preserving the race fix.
      const nextWindow = mergeLiveChannelWindowEvent(
        currentWindow,
        event,
        isTimelineRow,
      );

desktop/src/features/notifications/lib/desktop.ts:468

  • On Windows, getDesktopNotificationPermissionState() reaches the native invoke path and can reject when the WinRT query cannot be scheduled or completed. Because this await is outside the delivery try/catch, fire-and-forget callers such as reminders and community-join alerts receive an unhandled rejection instead of the promised boolean delivery result, and the notification is dropped. Treat permission-query failures as false here (and log them) so sendDesktopNotification preserves its failure contract.
  if ((await getDesktopNotificationPermissionState()) !== "granted") {
    return false;
  }

desktop/src/features/notifications/use-feed-desktop-notifications.test.mjs:104

  • This test does not bind the single-flight claim to the production request path: the injected requestAccess stub returns the same promise on every call, so it passes even if requestDesktopNotificationAccess stops sharing pendingPermissionRequest; requestCalls === 2 actually allows duplicate underlying requests. Exercise the production request function (or a seam that counts the OS request) and assert that the underlying request happens once.
test("a concurrent feed batch joins the pending permission request", async () => {
  const attempt = { hasRequested: false };
  let permissionStateChecks = 0;
  let requestCalls = 0;
  let releaseRequest;
  const request = new Promise((resolve) => {
    releaseRequest = resolve;
  });
  const getPermissionState = async () => {
    permissionStateChecks++;
    return "default";
  };
  const requestAccess = () => {
    requestCalls++;
    return request;
  };

desktop/src/features/notifications/use-feed-desktop-notifications.ts:98

  • This caught delivery failure is not durable: the hook clears inFlightItemIdsRef and only persists handledIds, while a remount treats every item currently in the feed as initial history and marks it seen. If the app exits after this exception, the item in retryableIds is therefore never retried, despite the new retry path. Persist retryable IDs (or a durable retry record) and consume them on initialization, or propagate the failure instead of relying only on the next in-process feed update.
      } catch (error) {
        console.warn("Failed to deliver feed notification", item.id, error);
        return { id: item.id, delivered: false };
      }

desktop/src/features/notifications/use-feed-desktop-notifications.ts:284

  • The generation fence is advanced only when normalizedPubkey changes, but enabled is also a dependency of this effect. If the user disables desktop notifications while the permission request is pending, the existing batch continues and can call sendDesktopNotification after the setting is off; a hung request can also leave its IDs permanently in inFlightItemIdsRef. Invalidate/cancel the generation on disable and re-check enabled before delivery.
      const generation = notificationGenerationRef.current;
      void deliverFeedNotificationBatch(
  • Files reviewed: 31/32 changed files
  • Comments generated: 3
  • Review effort level: Lite

Comment thread desktop/src-tauri/src/commands/notifications.rs
Comment thread desktop/src-tauri/src/commands/notifications.rs
Comment thread desktop/src/features/channels/useLiveChannelUpdates.ts
Signed-off-by: Bernhard Kaindl <bernhardkaindl7@gmail.com>
@bernhardkaindl

Copy link
Copy Markdown
Author

All six suppressed findings from review 5191170956 were addressed in commit 91c3809:

  • Route handling now includes targetMessageView in its deduplication key.
  • Live channel overlays are bounded.
  • Permission-query failures return false safely.
  • Single-flight coverage now exercises the production request path.
  • Retryable feed IDs persist across remounts/restarts.
  • Disabling notifications invalidates pending work and rechecks enabled.

Signed-off-by: Bernhard Kaindl <bernhardkaindl7@gmail.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved findings remain around Windows registration/FFI, permission guards, durable delivery retries, activation draining, and retry-state bounds.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (7)

Previously missed (1) — in code that hasn't changed since the last review.

desktop/src/app/useAppShellDesktopNotifications.ts:96

  • The permission promise can outlive the state that scheduled it. If the user disables desktop notifications or switches away from this mounted surface while the OS permission request is pending, this continuation still sends the DM because it never re-checks current state or a generation/cancellation token; guard the continuation before calling sendDesktopNotification.

This issue also appears on line 142 of the same file.

desktop/src-tauri/src/commands/notifications.rs:193

  • This adds a new unsafe FFI call in the production notification path. The repository's native code policy does not allow introducing unsafe production code; please route the AUMID operation through an approved safe abstraction or otherwise redesign this Windows integration before merging.
        use windows_sys::Win32::System::Registry::{

desktop/src/app/useAppShellDesktopNotifications.ts:104

  • When native delivery returns false, this handler just exits, but useLiveChannelUpdates records the event ID in seenNotificationEventIdsRef before invoking this callback. A transient Windows permission/WinRT/IPC failure therefore permanently drops a DM notification and it cannot be retried; keep a durable retry record or only mark the event seen after successful delivery.
          if (!didSend) return;

desktop/src/app/useAppShellDesktopNotifications.ts:143

  • The permission promise can outlive the state that scheduled it. If the user disables desktop notifications or switches away from this mounted surface while the OS permission request is pending, this continuation still sends the thread reply because it never re-checks current state or a generation/cancellation token; guard the continuation before calling sendDesktopNotification.
          if (!permissionGranted) return;
          const didSend = await sendDesktopNotification({

desktop/src/features/notifications/lib/desktop.ts:326

  • If the native event listener setup rejects, nativeUnlisten stays null and this initial drain is skipped. A cold-start Windows toast click can then remain in the Rust queue when the already-visible window produces no later focus or visibility event; drain the queue independently of whether the listener registration succeeded.
    if (nativeUnlisten && usesActivationQueue) {

desktop/src/features/notifications/lib/desktop.ts:497

  • This newly added Windows native post can fail transiently, but the returned false is discarded by the DM and thread-reply callers. Unlike the feed path, those live notifications have no durable retry, so a WinRT error permanently loses the event; connect this failure to a retryable delivery path instead of treating it as terminal.
  if (
    isTauri() &&
    (isLinuxPlatform() || isMacPlatform() || isWindowsPlatform())
  ) {
    try {

desktop/src/features/notifications/use-feed-desktop-notifications.ts:339

  • Only the serialized retry list is capped; this in-memory Set is never pruned, so every item whose delivery keeps failing remains resident and a long-lived desktop session can grow it without bound. Prune the set to the same 500-item limit before persisting it.
  • Files reviewed: 35/36 changed files
  • Comments generated: 4
  • Review effort level: Lite

Comment thread desktop/src-tauri/src/commands/notifications.rs Outdated
Comment thread desktop/src/features/notifications/lib/desktop.ts
Comment thread desktop/src/features/notifications/lib/desktop.ts Outdated
…delivery

Replace raw notification registration FFI with WinSafe and forbid unsafe code in the notification module. Preserve existing notification opt-outs and shortcut arguments, with native regression tests.

Fence feed and live deliveries on current settings, persist bounded DM/thread retry records, and test Windows activation drains even when listener setup fails. Revert the AGENTS.md addition from 91c3809; local Windows guidance stays excluded.

Signed-off-by: Bernhard Kaindl <bernhardkaindl7@gmail.com>
@bernhardkaindl

Copy link
Copy Markdown
Author

Review 5191367484 follow-up

Published in 3589226. The three remaining formal threads have fix-specific replies; the registration failure-propagation thread was already addressed by e520143.

All seven suppressed comments

  1. Stale DM permission continuation: live delivery now uses a session/generation fence, checks current surface/global/slot settings after permission resolves, and checks cancellation again after the send helper's final permission query. Tests cover disabling, slot changes, scope changes, and unmount.
  2. Unsafe Windows registration FFI: removed the raw registry/COM/shell/property-store calls from Buzz notification code. Windows-only winsafe 0.0.29 provides the safe API boundary and RAII cleanup; the entire notification module now has #![forbid(unsafe_code)]. No policy exception or locally relocated unsafe wrapper was added. Native tests read back registry values and a temporary shortcut AUMID, verify opt-outs and shortcut arguments survive repair, and verify shortcut failures propagate. The synchronous startup error barrier remains intact.
  3. Lost DM delivery failures: the AppShell handler persists a pending alert before native delivery. Live-event deduplication no longer erases retry responsibility; failures stay in a community/viewer-scoped journal.
  4. Stale thread-reply continuation: thread alerts use the same guarded queue and cancellation checks as DMs, with tests for both categories.
  5. Initial activation drain skipped after listener failure: queue draining no longer depends on successful native listener registration. Both Windows and macOS production listener paths have mocked IPC tests for the failed-listener case.
  6. Discarded Windows send failure: both live callers now consume the actual send result through the durable queue. Sound/bounce follows success only. Failed sends retry with exponential backoff, at most five attempts per mount; exhausted records remain for the next mount. The queue retains at most 500 pending alerts and evicts the oldest on overflow. Delivery is at-least-once across a crash between native acceptance and persisting success, as documented in code.
  7. Unbounded feed retry Set: the production persistence helper prunes the in-memory Set to the same 500-ID bound as storage, with a regression test.

Validation on Windows

  • Desktop Rust library suite: 3,038 passed, 0 failed, 15 ignored (includes 6 native notification tests). The previously missing true, env, and sh executables were supplied by Git for Windows on the test process PATH; no unrelated tests were changed or skipped.
  • Full desktop JavaScript suite: 6,558 passed, 0 failed. Focused notification group: 109 passed.
  • TypeScript, scoped Biome, desktop Rust formatting, differential desktop file-size check, and diff whitespace checks passed.
  • Not a full CI pass: strict desktop Clippy stops on five existing Windows unused-import/dead-code errors in buzz-terminal. Native just ci stops at the Unix _ensure-sidecar-stubs recipe because Bash cannot open the backslash-form Windows temporary script path (exit 127). Neither gate was bypassed or relabelled as successful.
  • Real Windows registry/shortcut behavior was tested using isolated keys and temporary directories. End-to-end OS toast delivery/click-through was not manually exercised; the activation tests mock IPC and do not prove activation of a fully terminated process.

The eight-line AGENTS.md addition from 91c3809 was reverted exactly as requested. Expanded local Windows build/test guidance remains excluded from this PR for a later update to block#7609; the earlier local AGENTS guidance remains preserved in its named stash.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved critical and moderate notification reliability, retry, routing, and cache-growth issues remain.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (5)

desktop/src/app/AppShell.helpers.ts:214

  • When openInThread is true this path still uses openSearchHit, which seeds toSearchHit(target) before ChannelRouteScreen has fetched the real reply. That synthetic event has no reply tags, so useChannelRouteTarget can classify the reply as a root, open a new/empty branch, and mark the route target handled before hydration replaces it. Defer the cache projection until the authoritative target/ancestor fetch completes (or preserve the reply ancestry in the cached event), and cover an actual thread-notification activation.
    !target.openInThread &&
    target.kind !== KIND_FORUM_POST &&
    target.kind !== KIND_FORUM_COMMENT
  ) {

desktop/src/features/channels/useLiveChannelUpdates.ts:362

  • This broad subscription updates every cached channel's channelMessagesKey with every live event, including ordinary thread replies and auxiliary events, but neither mergeTimelineCacheMessages nor this path imposes a size bound. reconcileChannelWindowMessages deliberately retains cache-only replies, so a long-running session can accumulate an unbounded per-channel array and repeatedly format those events; cap/evict this cache or keep the authoritative window as the only bounded live store.
    queryClient.setQueryData<RelayEvent[]>(
      channelMessagesKey(channelId),
      (current) => {
        if (!current) {
          return current;
        }

        return mergeTimelineCacheMessages(current, event);
      },

desktop/src/features/notifications/use-feed-desktop-notifications.ts:392

  • Retry IDs are retained until delivery succeeds or permission is denied, but no attempt count, backoff, or terminal state is stored. If native delivery remains unavailable, each feed refresh or app mount will retry the same item forever; the 500-ID cap bounds storage but not repeated delivery work. Persist retry metadata and quarantine/drop an item after a terminal limit while retaining whatever durable recovery record the product requires.
    desktop/src/features/notifications/useLiveNotificationDelivery.ts:53
  • restore() runs while the delivery effect is mounting, so a truncated or otherwise malformed localStorage journal makes this hook throw before it can deliver new notifications (and can fail the app shell's notification setup). Treat malformed JSON and invalid entries as an empty/filtered journal, matching the feed retry readers, rather than propagating storage corruption into React effects. Please also add a regression test for malformed journal data.
    desktop/src/features/notifications/useLiveNotificationDelivery.ts:55
  • The retry budget is reset to zero for every restored record, while exhausted entries remain in the journal. A permanent native failure (for example, WinRT remaining unavailable) therefore causes the same alert to be attempted five more times on every app mount indefinitely. Persist a terminal/exhausted state or otherwise quarantine records after a total retry limit so a durable journal does not become an unbounded cross-session retry loop.
  • Files reviewed: 36/37 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment on lines +28 to +29
const MAX_PENDING = 500;
const MAX_ATTEMPTS = 5;
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.

2 participants