Skip to content

Mirror tray notifications in app#869

Merged
shanselman merged 1 commit into
openclaw:mainfrom
RBrid:user/rbrid/notification-mirroring
Jun 27, 2026
Merged

Mirror tray notifications in app#869
shanselman merged 1 commit into
openclaw:mainfrom
RBrid:user/rbrid/notification-mirroring

Conversation

@RBrid

@RBrid RBrid commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #820.

This PR makes the companion app's in-memory Notifications page and top-of-window InfoBar reflect the same substantive tray-visible notifications that are already shown as Windows toasts.

Before this change, many notification producers only called ToastService.ShowToast(), so users could see notifications in Windows Notification Center while the companion app's Notifications page stayed empty. The page and global InfoBar were not broken; they only render items published through AppNotificationService.

What changed

  • Added a structured notification mapper for gateway and node notifications.
  • Added a shared app/toast publish path that attempts Windows toast delivery and in-app notification publishing independently.
  • Mirrored eligible current-session notifications into AppNotificationService, including:
    • gateway OpenClawNotification events
    • node system.notify events
    • node paired success
    • session command failures
    • node screen/camera failure or blocked events
  • Kept transient confirmations toast-only, including node screen/camera start/complete/capture notices.
  • Added a 100-item cap to active in-app notifications to avoid unbounded growth during long sessions.
  • Preserved chat notification routing, including session-specific chat navigation when a SessionKey is available and generic Chat fallback when it is not.
  • Added behavior tests for mapping, queue capping, and publisher channel independence.

Scope notes

This intentionally keeps app notifications current-session/in-memory only. It does not add disk-backed notification persistence. Closing or restarting the tray still clears the Notifications page, which matches the current product behavior.

Validation

  • ./build.ps1
  • dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore
  • dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore
  • auto-review & Hanselman reviews

Screenshots

After running this command in a Powershell prompt:
dotnet run --project .\src\OpenClaw.WinNode.Cli\OpenClaw.WinNode.Cli.csproj -- --command system.notify --params '{"title":"Notification mirror test","body":"This should appear in the Windows toast, top InfoBar, and Notifications page."}'

image image

@RBrid RBrid marked this pull request as draft June 26, 2026 22:08
@clawsweeper

clawsweeper Bot commented Jun 26, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed June 26, 2026, 11:17 PM ET / 03:17 UTC.

Summary
The branch mirrors selected tray/toast notifications into the in-memory AppNotificationService, adds mapper/publisher helpers, caps active notifications at 100, updates the NodeService toast event payload, and adds tray behavior tests.

Reproducibility: yes. from source. Current main's Notifications page renders AppNotificationService snapshots while system.notify and gateway notification handlers still call ToastService without adding AppNotification entries; I did not run a Windows tray smoke in this read-only review.

Review metrics: 3 noteworthy metrics.

  • Changed surface: 11 files, +517/-35. The PR spans tray event routing, notification services, a NodeService event type, and tray tests.
  • Regression coverage: 2 test files added, 1 test file changed, 1 test project changed. The tests cover mapper behavior, publisher channel independence, and the new in-memory queue cap.
  • Visible proof: 2 screenshots inspected. The media directly shows the after-change system.notify payload in the toast, top InfoBar, and Notifications page.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #820
Summary: This PR is the candidate fix for the open notification-mirroring issue; the merged background-notifications PR covers selected background states but not the broader toast-to-app mirroring problem.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster ✨ media proof bonus
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Wait for the pending Build and Test jobs to finish green before landing.

Mantis proof suggestion
A short visible desktop proof would add useful confidence for gateway/pairing/camera/session mirrored notification classes beyond the system.notify screenshots already provided. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis visual task: verify a gateway or node toast event appears in the OpenClaw in-app Notifications page and top InfoBar while transient capture or recording confirmations remain toast-only.

Risk before merge

  • [P1] At inspection, the GitHub Build and Test test job and three setup e2e jobs were still pending; this read-only review did not run the AGENTS.md validation commands locally.
  • [P1] The provided screenshots prove the node system.notify path; gateway, pairing, session failure, and camera/screen failure mirroring are supported by source inspection and tests rather than separate live media.

Maintainer options:

  1. Decide the mitigation before merge
    Land this paired fix after final validation stays green, keeping app notifications current-session-only and preserving toast settings, dedupe, chat routing, and transient-toast exclusions.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • No automated repair is needed; the remaining action is maintainer landing review after final checks complete.

Security
Cleared: No concrete security or supply-chain issue was found; the added test package reference matches the notification package/version already used by the tray app.

Review details

Best possible solution:

Land this paired fix after final validation stays green, keeping app notifications current-session-only and preserving toast settings, dedupe, chat routing, and transient-toast exclusions.

Do we have a high-confidence way to reproduce the issue?

Yes, from source. Current main's Notifications page renders AppNotificationService snapshots while system.notify and gateway notification handlers still call ToastService without adding AppNotification entries; I did not run a Windows tray smoke in this read-only review.

Is this the best way to solve the issue?

Yes. The PR adds a focused shared publisher and mapper, preserves existing toast delivery, keeps transient confirmations toast-only, and adds regression coverage for the new behavior.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against dd1c7916204c.

Label changes

Label justifications:

  • P2: This is a normal-priority user-facing tray notification bug fix with limited blast radius and no blocking code finding found.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (screenshot): The PR body screenshots show after-fix real behavior for system.notify in the Windows toast, top InfoBar, and in-app Notifications page.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body screenshots show after-fix real behavior for system.notify in the Windows toast, top InfoBar, and in-app Notifications page.
  • proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. The PR body screenshots show after-fix real behavior for system.notify in the Windows toast, top InfoBar, and in-app Notifications page.
Evidence reviewed

What I checked:

Likely related people:

  • RBrid: Authored the merged app-notifications/background-notifications surface that this PR extends and is also the prior main-history owner of AppNotificationPublisher. (role: feature owner and recent area contributor; confidence: high; commits: 1a07759a7e6e, 4e7982bafb86; files: src/OpenClaw.Tray.WinUI/Services/AppNotificationPublisher.cs, src/OpenClaw.Tray.WinUI/Services/AppNotificationService.cs, src/OpenClaw.Tray.WinUI/Pages/NotificationsPage.xaml.cs)
  • Vitor Cepeda Lopes: git blame shows the current main node system.notify and baseline gateway notification handlers in App.xaml.cs come from 0faa821. (role: introduced current toast-only handler behavior; confidence: medium; commits: 0faa8217de00; files: src/OpenClaw.Tray.WinUI/App.xaml.cs, src/OpenClaw.Tray.WinUI/Services/NodeService.cs)
  • Barbara Kudiess: Recent current-main work changed AppNotificationService, App.xaml.cs, and HubWindow notification banner behavior immediately before this PR. (role: recent adjacent notification/banner contributor; confidence: medium; commits: 977648cc9c84; files: src/OpenClaw.Tray.WinUI/App.xaml.cs, src/OpenClaw.Tray.WinUI/Services/AppNotificationService.cs, src/OpenClaw.Tray.WinUI/Windows/HubWindow.xaml.cs)
  • QQSHI13: Authored the open notification-mirroring issue and recent chat toast session routing that this PR preserves while adding in-app mirroring. (role: adjacent toast-routing contributor and canonical issue reporter; confidence: medium; commits: 6283fb174ead; files: src/OpenClaw.Tray.WinUI/App.xaml.cs, src/OpenClaw.Tray.WinUI/App.ToastActivation.cs)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 26, 2026
@RBrid

RBrid commented Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 26, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@RBrid RBrid marked this pull request as ready for review June 26, 2026 22:27
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@shanselman shanselman force-pushed the user/rbrid/notification-mirroring branch from 4a89db9 to 1da2ed8 Compare June 27, 2026 03:12
@shanselman shanselman merged commit 8b5ea32 into openclaw:main Jun 27, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Notifications tab never shows notifications despite Windows Notification Center having them

2 participants