fix(apply): reliably prompt to restart after bulk apply and keep silent settings silent

Two related bugs surfaced by running the Extreme preset on a fresh Windows 11
install:

Reboot prompt lost on Save & close. The manual Apply / preset path showed its
reboot affordance as a button inside ApplyResultsWindow, which was owned by the
Settings window. On "Save & close" the Settings window closes immediately after
showing it, and WPF destroys owned windows with their owner — so a bulk apply
wrote several reboot-required settings but never prompted to restart. Centralize
the restart prompt in a new, unowned, app-level RebootPrompt (reusing
RebootPendingWindow) that every apply path funnels through: manual Apply/preset,
the background auto-apply loop, the drift-notification Apply button, and the CPU
power-plan action. Because it is unowned it survives the close; deferred to the
end of the batch it prompts once for the whole apply, not per setting. The
now-redundant reboot button is removed from ApplyResultsWindow (the per-row
"reboot to take effect" badge stays).

Silent didn't mean silent. MonitorService's notification list excluded only the
settings being auto-applied this tick, so an auto-apply setting that failed to
verify (15-min backoff) fell through and popped a toast — on a fresh install the
services/tasks Windows resists hit this repeatedly. Notifications are now gated
on a pure, unit-tested SelectNotifiable rule: a setting the user set to
auto-apply never notifies, on any tick, for any reason.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: carterscode

src/GamerGuardian/App.xaml.cs

@@ -154,19 +154,7 @@ protected override void OnStartup(StartupEventArgs e)
             .ToArray();
         _monitor = new MonitorService(_store, _allMonitors, report => _notifier.ShowAsync(report));
         _monitor.AutoAppliedRebootRequired += items =>
-        {
-            var descriptions = items.Select(i => i.Description).ToList();
-            Dispatcher.BeginInvoke(() =>
-            {
-                try
-                {
-                    var win = new GamerGuardian.UI.RebootPendingWindow(descriptions);
-                    win.Closed += (_, _) => ReleaseWindow(win);
-                    win.Show();
-                }
-                catch { }
-            });
-        };
+            GamerGuardian.Services.RebootPrompt.Show(items.Select(i => i.Description).ToList());
 
         _tray = new TrayIconHost();
         _tray.OpenSettingsRequested += ShowSettings;

src/GamerGuardian/Services/MonitorService.cs

@@ -367,15 +367,14 @@ private async Task TickAsync(MonitorTier? tier)
                     AutoAppliedRebootRequired?.Invoke(rebootSettings);
             }
 
-            // Drifts that aren't auto-applied (or are in cooldown) still surface
-            // as a notification so the user knows something's drifting and can
-            // act manually. Settings the breaker has tripped are excluded: they're
-            // logged once (LogCircuitBreaker) and would otherwise re-notify every
-            // poll, swapping UAC spam for toast spam.
-            var prompt = drifted
-                .Where(a => !auto.Any(b => b.SettingId == a.SettingId))
-                .Where(a => !_breaker.IsTripped(a.SettingId, now))
-                .ToList();
+            // Notify only about drift the user asked to be notified about
+            // (Monitor on, Auto-apply off). A setting set to auto-apply — "silently
+            // change to my desired state" — must NEVER produce a notification, even
+            // on a tick where we couldn't apply it (verify-backoff or breaker
+            // cooldown). Surfacing a toast for a silent setting is exactly the
+            // "silent didn't mean silent" complaint: those failures are logged
+            // (LogApplyResults / LogCircuitBreaker) and retried, not shown.
+            var prompt = SelectNotifiable(drifted, id => _breaker.IsTripped(id, now));
             if (prompt.Count > 0)
                 await _onDriftAsync(new DriftReport(prompt));
 
@@ -393,6 +392,20 @@ private async Task TickAsync(MonitorTier? tier)
         }
     }
 
+    /// <summary>
+    /// The notification-eligibility rule, pulled out pure so it can be unit-tested
+    /// without a timer or the registry. A drifted setting is shown to the user only
+    /// when it is NOT set to auto-apply (auto-apply means "silently change", so it
+    /// never notifies) and is NOT currently tripped by the circuit breaker (tripped
+    /// settings are logged once and would otherwise re-notify every poll).
+    /// </summary>
+    public static List<DriftItem> SelectNotifiable(
+        IEnumerable<DriftItem> drifted, Func<string, bool> isTripped) =>
+        drifted
+            .Where(d => !d.AutoApply)
+            .Where(d => !isTripped(d.SettingId))
+            .ToList();
+
     public void Dispose()
     {
         UnsubscribeSystemEvents();

src/GamerGuardian/Services/RebootPrompt.cs

@@ -0,0 +1,56 @@
+using GamerGuardian.UI;
+using Application = System.Windows.Application;
+
+namespace GamerGuardian.Services;
+
+/// <summary>
+/// Shows the single, app-level "a restart is required" prompt. Every path that
+/// applies a reboot-requiring change funnels through here — the manual Settings
+/// Apply / preset flow and the background auto-apply loop — so the prompt looks
+/// and behaves the same no matter what triggered it.
+///
+/// <para>The window is deliberately <b>unowned</b>. The manual Apply path shows
+/// this prompt and then closes the Settings window; a WPF owned window is
+/// destroyed together with its owner, so an owned reboot prompt vanished the
+/// instant the user clicked "Save &amp; close" after a bulk apply (the Extreme
+/// preset is the worst case — it applies several reboot-required settings at
+/// once). That is exactly the "applied the changes but never prompted to
+/// restart" bug. Unowned, combined with the app's <c>OnExplicitShutdown</c>
+/// mode, keeps the prompt alive until the user acts on it.</para>
+///
+/// <para>Singleton: a later batch that also needs a reboot replaces the prior
+/// prompt rather than stacking a second window (same pattern as
+/// <see cref="Notifier"/>). The restart action is identical regardless of which
+/// settings triggered it, so replacing the descriptions loses nothing.</para>
+/// </summary>
+public static class RebootPrompt
+{
+    private static RebootPendingWindow? _current;
+
+    /// <summary>
+    /// Surface the restart prompt for the given human-readable setting
+    /// descriptions. No-op when the list is empty or the WPF application isn't
+    /// running (e.g. headless self-test). Safe to call from any thread — the
+    /// window is created on the UI dispatcher.
+    /// </summary>
+    public static void Show(IReadOnlyList<string> settingDescriptions)
+    {
+        if (settingDescriptions is null || settingDescriptions.Count == 0) return;
+        var app = Application.Current;
+        if (app is null) return;
+
+        app.Dispatcher.BeginInvoke(() =>
+        {
+            try
+            {
+                _current?.Close();
+                var win = new RebootPendingWindow(settingDescriptions);
+                _current = win;
+                // RebootPendingWindow.OnClosed already releases its visual tree.
+                win.Closed += (_, _) => { if (ReferenceEquals(_current, win)) _current = null; };
+                win.Show();
+            }
+            catch { /* best-effort: a failed prompt must never crash the applier */ }
+        });
+    }
+}

src/GamerGuardian/UI/ApplyResultsWindow.xaml

@@ -32,7 +32,6 @@
                 </Grid.ColumnDefinitions>
                 <ui:Button Grid.Column="0" Content="Open change log" MinWidth="140" Click="OpenLogButton_Click"/>
                 <StackPanel Grid.Column="2" Orientation="Horizontal" HorizontalAlignment="Right">
-                    <ui:Button x:Name="RebootButton" Content="Reboot now" MinWidth="110" Margin="0,0,8,0" Visibility="Collapsed" Click="RebootButton_Click"/>
                     <ui:Button x:Name="CloseButton" Content="Close" Appearance="Primary" MinWidth="90" Click="CloseButton_Click"/>
                 </StackPanel>
             </Grid>

src/GamerGuardian/UI/ApplyResultsWindow.xaml.cs

@@ -17,10 +17,10 @@ public ApplyResultsWindow(IReadOnlyList<ApplyResult> results)
             : $"Applied {ok} of {results.Count} settings ({fail} failed)";
         SubText.Text = "Each row shows the value before, what you wanted, and the actual value re-read from the OS after apply. The 'Mechanism' line tells you exactly where the change was written; the PowerShell snippet lets you verify the same value yourself outside the app.";
 
+        // The per-row "reboot to take effect" badge stays; the restart prompt
+        // itself is now the app-level, unowned RebootPrompt (raised by the caller)
+        // so it survives Save & close and matches the auto-apply path.
         ItemsList.ItemsSource = results.Select(r => new ResultRow(r)).ToList();
-
-        if (results.Any(r => r.RequiresReboot && r.Verified))
-            RebootButton.Visibility = Visibility.Visible;
     }
 
     private void CloseButton_Click(object sender, RoutedEventArgs e) => Close();
@@ -37,12 +37,6 @@ protected override void OnClosed(EventArgs e)
         catch { }
     }
 
-    private void RebootButton_Click(object sender, RoutedEventArgs e)
-    {
-        GamerGuardian.Services.RebootHelper.ForceRebootNow();
-        Close();
-    }
-
     private void CopyButton_Click(object sender, RoutedEventArgs e)
     {
         if (sender is FrameworkElement fe && fe.Tag is string s && !string.IsNullOrEmpty(s))

src/GamerGuardian/UI/NotificationWindow.xaml.cs

@@ -28,10 +28,22 @@ private async void ApplyButton_Click(object sender, RoutedEventArgs e)
     {
         ApplyButton.IsEnabled = false;
         DismissButton.IsEnabled = false;
+        // Applying from a drift notification is still an apply — if a reboot-required
+        // setting was among them, the user needs the same restart prompt they'd get
+        // from the Settings Apply button. Only items whose Apply didn't throw are
+        // flagged (a declined UAC prompt shouldn't claim a reboot is pending).
+        var rebootDescriptions = new List<string>();
         foreach (var item in _report.Items)
         {
-            try { await item.Apply(); } catch { }
+            try
+            {
+                await item.Apply();
+                if (item.RequiresReboot) rebootDescriptions.Add(item.Description);
+            }
+            catch { }
         }
+        if (rebootDescriptions.Count > 0)
+            Services.RebootPrompt.Show(rebootDescriptions);
         Close();
     }
 

src/GamerGuardian/UI/SettingsWindow.xaml.cs

@@ -1164,6 +1164,20 @@ private async Task ApplyChangesCoreAsync(bool closeAfter)
                 System.Windows.MessageBoxImage.Information);
         }
 
+        // Surface a single restart prompt for any reboot-requiring change that
+        // actually landed this pass. Deferred to the end on purpose: ApplyAndVerify
+        // ran every drifted setting first, so a bulk apply (e.g. the Extreme preset)
+        // flags reboot across the whole batch and prompts once here, not per setting.
+        // RebootPrompt shows an UNOWNED window so it survives the Close() below —
+        // the ApplyResultsWindow is owned by this window and would be destroyed with
+        // it on Save & close, which is why bulk applies used to reboot-flag nothing.
+        var rebootDescriptions = results
+            .Where(r => r.RequiresReboot && r.Verified)
+            .Select(r => r.Description)
+            .ToList();
+        if (rebootDescriptions.Count > 0)
+            RebootPrompt.Show(rebootDescriptions);
+
         if (closeAfter) Close();
     }
 
@@ -1590,6 +1604,17 @@ private async Task RunCpuActionAsync(
                         "or (on dual-CCD X3D) the BIOS/driver/Game Bar dependencies aren't in place, see the Apply Results window for details.",
                         "GamerGuardian", System.Windows.MessageBoxButton.OK, System.Windows.MessageBoxImage.Warning);
                 }
+
+                // Consistency with the main Apply path: any reboot-required change
+                // that landed raises the shared restart prompt. Power-plan actions
+                // don't need a reboot today, so this is a no-op guard that keeps
+                // every apply path honest if that ever changes.
+                var rebootDescriptions = results
+                    .Where(r => r.RequiresReboot && r.Verified)
+                    .Select(r => r.Description)
+                    .ToList();
+                if (rebootDescriptions.Count > 0)
+                    RebootPrompt.Show(rebootDescriptions);
             }
         }
         catch (Exception ex)

tests/GamerGuardian.Tests/MonitorServiceTests.cs

@@ -0,0 +1,81 @@
+using System.Collections.Generic;
+using System.Linq;
+using System.Threading.Tasks;
+using GamerGuardian.Models;
+using GamerGuardian.Services;
+using Xunit;
+
+namespace GamerGuardian.Tests;
+
+/// <summary>
+/// Covers <see cref="MonitorService.SelectNotifiable"/> — the pure rule that
+/// decides which drifted settings raise an on-screen notification. The key
+/// guarantee under test: "silent means silent" — a setting the user set to
+/// auto-apply must never notify, on any tick, for any reason.
+/// </summary>
+public class MonitorServiceTests
+{
+    private static DriftItem Drift(string id, bool autoApply) => new(
+        SettingId: id,
+        DisplayKey: "global",
+        DisplayLabel: "Global",
+        Description: id,
+        CurrentValue: "On",
+        DesiredValue: "Off",
+        AutoApply: autoApply,
+        Apply: () => Task.CompletedTask);
+
+    private static bool NeverTripped(string _) => false;
+
+    [Fact]
+    public void NotifyOnly_Setting_IsNotified()
+    {
+        var drifted = new[] { Drift("hags", autoApply: false) };
+        var result = MonitorService.SelectNotifiable(drifted, NeverTripped);
+        Assert.Single(result);
+        Assert.Equal("hags", result[0].SettingId);
+    }
+
+    [Fact]
+    public void AutoApply_Setting_IsNeverNotified()
+    {
+        // The whole point: a "silently change" setting must not produce a toast.
+        var drifted = new[] { Drift("vrr", autoApply: true) };
+        Assert.Empty(MonitorService.SelectNotifiable(drifted, NeverTripped));
+    }
+
+    [Fact]
+    public void AutoApply_Setting_IsNotNotified_EvenWhenNotBeingApplied()
+    {
+        // This models the verify-backoff / breaker-cooldown case: the setting is
+        // auto-apply but couldn't be applied this tick. It must STILL stay silent —
+        // this was the regression where backed-off auto-apply settings leaked into
+        // the notification path.
+        var drifted = new[] { Drift("svc:DoSvc", autoApply: true) };
+        Assert.Empty(MonitorService.SelectNotifiable(drifted, NeverTripped));
+    }
+
+    [Fact]
+    public void BreakerTripped_NotifyOnly_Setting_IsSuppressed()
+    {
+        var drifted = new[] { Drift("hags", autoApply: false) };
+        var result = MonitorService.SelectNotifiable(drifted, id => id == "hags");
+        Assert.Empty(result);
+    }
+
+    [Fact]
+    public void MixedBatch_KeepsOnlyNotifyOnlyUntrippedSettings()
+    {
+        var drifted = new[]
+        {
+            Drift("auto1", autoApply: true),    // silent -> excluded
+            Drift("notify1", autoApply: false), // notify -> kept
+            Drift("notify2", autoApply: false), // notify but tripped -> excluded
+            Drift("auto2", autoApply: true),    // silent -> excluded
+        };
+
+        var result = MonitorService.SelectNotifiable(drifted, id => id == "notify2");
+
+        Assert.Equal(new[] { "notify1" }, result.Select(d => d.SettingId).ToArray());
+    }
+}