Skip to content

[Repo Assist] fix: respect sound=false in system.notify node notifications#122

Closed
github-actions[bot] wants to merge 1 commit intomasterfrom
repo-assist/fix-system-notify-playsound-ignored-10c7f97e6ac8d2e8
Closed

[Repo Assist] fix: respect sound=false in system.notify node notifications#122
github-actions[bot] wants to merge 1 commit intomasterfrom
repo-assist/fix-system-notify-playsound-ignored-10c7f97e6ac8d2e8

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

🤖 This is an automated pull request from Repo Assist.

Summary

When an agent invokes system.notify with sound=false, the PlaySound field in SystemNotifyArgs was correctly populated by SystemCapability — but silently ignored in OnNodeNotificationRequested. The toast was always shown with sound regardless of the per-notification flag.

Root Cause

SystemCapability.HandleNotifyAsync correctly reads the sound argument:

var sound = GetBoolArg(request.Args, "sound", true);
// ...
NotifyRequested?.Invoke(this, new SystemNotifyArgs { PlaySound = sound, ... });

But OnNodeNotificationRequested in App.xaml.cs never read args.PlaySound:

new ToastContentBuilder()
    .AddText(args.Title)
    .AddText(args.Body)
    .Show();  // ← PlaySound completely ignored

The existing test (CapabilityTests.Notify_RaisesEvent_WithArgs) already verifies that PlaySound=false is propagated to the event args — the gap is the UI handler.

Fix

var builder = new ToastContentBuilder()
    .AddText(args.Title)
    .AddText(args.Body);

if (!args.PlaySound)
    builder.AddAudio(silent: true);

builder.Show();

Relationship to PR #95

PR #95 applies the user's global NotificationSound preference to all toasts. This PR handles a different concern: respecting the per-notification sound=false flag sent by the agent. Both fixes are complementary and non-conflicting.

Test Status

  • ✅ All 503 shared tests pass, 18 skipped (Windows-only infrastructure): dotnet test tests/OpenClaw.Shared.Tests/
  • ⚠️ WinUI app cannot be built on the Linux CI runner (NETSDK1100), so App.xaml.cs changes are verified by inspection. The logic is a straightforward null-guard on a boolean field already confirmed by the existing capability test.

Generated by Repo Assist ·

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@cbb46ab386962aa371045839fc9998ee4e97ca64

When an agent invokes system.notify with sound=false, the PlaySound field
in SystemNotifyArgs was correctly populated by SystemCapability but then
silently ignored in OnNodeNotificationRequested. The toast was always
shown with sound regardless of the per-notification flag.

Fix: check args.PlaySound before showing the toast and add a silent
audio element when the agent has explicitly requested no sound.

This is orthogonal to the user-level NotificationSound preference
(addressed in PR #95): PlaySound=false suppresses sound for that
specific notification only, regardless of the global sound setting.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions github-actions bot added automation bug Something isn't working repo-assist labels Mar 31, 2026
@github-actions github-actions bot mentioned this pull request Mar 31, 2026
36 tasks
@shanselman
Copy link
Copy Markdown
Collaborator

Superseded by PR #125 which applied NotificationSound to all toast call sites including system.notify.

@shanselman shanselman closed this Apr 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automation bug Something isn't working repo-assist

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant