Skip to content

Commit f4f3c93

Browse files
authored
Merge pull request #26 from carterscode/fix/silent-apply-popup-and-race
fix: silent-apply popup + race-write + notification header
2 parents a279aac + 3202e9c commit f4f3c93

6 files changed

Lines changed: 168 additions & 3 deletions

File tree

src/GamerGuardian/Monitors/SettingsSearchAiMonitor.cs

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,18 @@ public IEnumerable<DriftItem> CheckDrift(AppConfig config)
5252
using var advanced = Registry.CurrentUser.OpenSubKey(AdvancedKey, writable: false);
5353
var disable = policy?.GetValue(PolicyVal) as int?;
5454
var companion = advanced?.GetValue(TaskbarCompanionVal) as int?;
55-
bool off = disable == 1 && companion == 0;
55+
56+
// The PolicyVal (DisableSearchBoxSuggestions) is the primary signal
57+
// and authoritative. The TaskbarCompanion value is a belt-and-
58+
// suspenders write -- on many Windows builds the value name isn't
59+
// recognized by Explorer and the write silently doesn't stick. We
60+
// treat companion as "off" when it's absent OR explicitly 0; only
61+
// companion == 1 counts as "companion still on". Without this,
62+
// ReadCurrent would forever report "on" after Apply even though
63+
// the user-visible search box AI suggestions are actually off.
64+
bool searchSuggestionsOff = disable == 1;
65+
bool companionOff = companion != 1;
66+
bool off = searchSuggestionsOff && companionOff;
5667
return !off;
5768
}
5869
catch { return null; }

src/GamerGuardian/Services/MonitorService.cs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,14 @@ private async Task TickAsync()
138138
try { drifted.AddRange(m.CheckDrift(config).Where(d => d.IsMonitored)); }
139139
catch { /* swallow per-monitor failure to keep loop alive */ }
140140
}
141-
_store.Save(config);
141+
// NOTE: do NOT _store.Save(config) here. The tick's `config` is a
142+
// local snapshot loaded at the top of TickAsync; writing it back
143+
// races with the user's Settings-window Apply: if the user saves
144+
// a new draft between our Load and Save, our save overwrites the
145+
// user's just-saved prefs (silently dropping any newly-added
146+
// properties -- e.g. the v0.1.39 Windows AI prefs). CheckDrift
147+
// doesn't mutate config anyway, so the save was a no-op except
148+
// for triggering this race. Removed in v0.1.40.
142149

143150
// External-reset detection happens BEFORE auto-apply so we log the
144151
// cause (EXTRESET) and the effect (the corrective apply) as two
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
using GamerGuardian.Models;
2+
3+
namespace GamerGuardian.Services;
4+
5+
/// <summary>
6+
/// Picks the notification window's header text based on what's actually in
7+
/// the drift report. Replaces the old hard-coded "Display settings have
8+
/// drifted" string that appeared even when no displays were involved (e.g.
9+
/// a Windows-AI policy drifting). The drift item's DisplayKey is the
10+
/// grouping signal; if a single category dominates the report we name it,
11+
/// otherwise we use a category-agnostic "Monitored settings" header.
12+
/// </summary>
13+
public static class NotificationHeader
14+
{
15+
public static string For(DriftReport report)
16+
{
17+
if (report is null || report.Items.Count == 0)
18+
return "Monitored settings have drifted";
19+
20+
var keys = report.Items.Select(i => i.DisplayKey).Distinct().ToList();
21+
if (keys.Count == 1)
22+
{
23+
var single = report.Items.Count == 1;
24+
var noun = keys[0] switch
25+
{
26+
"global" => "Global gaming setting",
27+
"ai" => "Windows AI setting",
28+
"ai-app" => "Windows AI app",
29+
"service" => "Windows service",
30+
"display" => "Display setting",
31+
_ => "Monitored setting"
32+
};
33+
// Pluralize the noun when there are multiple items, except for
34+
// proper-noun "Windows AI app" which we'll just leave as singular
35+
// since multiples are rare.
36+
var pluralNoun = single ? noun
37+
: keys[0] switch
38+
{
39+
"service" => "Windows services",
40+
"display" => "Display settings",
41+
"global" => "Global gaming settings",
42+
"ai" => "Windows AI settings",
43+
"ai-app" => "Windows AI apps",
44+
_ => "Monitored settings"
45+
};
46+
return $"{(single ? noun : pluralNoun)} {(single ? "has" : "have")} drifted";
47+
}
48+
return $"{report.Items.Count} monitored settings have drifted";
49+
}
50+
}

src/GamerGuardian/UI/NotificationWindow.xaml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@
2525
</ui:TitleBar>
2626

2727
<StackPanel Grid.Row="1" Margin="20,8,20,16">
28-
<TextBlock Text="Display settings have drifted"
28+
<TextBlock x:Name="HeaderText"
29+
Text="Monitored settings have drifted"
2930
FontSize="16" FontWeight="SemiBold"
3031
Margin="0,0,0,12"/>
3132
<ItemsControl x:Name="ItemsList" Margin="0,0,0,12">

src/GamerGuardian/UI/NotificationWindow.xaml.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ public NotificationWindow(DriftReport report)
1313
InitializeComponent();
1414
_report = report;
1515
ItemsList.ItemsSource = report.Items;
16+
HeaderText.Text = Services.NotificationHeader.For(report);
1617
Loaded += OnLoaded;
1718
}
1819

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
using GamerGuardian.Models;
2+
using GamerGuardian.Services;
3+
using Xunit;
4+
5+
namespace GamerGuardian.Tests;
6+
7+
public class NotificationHeaderTests
8+
{
9+
private static DriftItem Item(string displayKey, string label = "x") => new(
10+
SettingId: $"{displayKey}:test",
11+
DisplayKey: displayKey,
12+
DisplayLabel: label,
13+
Description: $"{label} -- test",
14+
CurrentValue: "On",
15+
DesiredValue: "Off",
16+
AutoApply: false,
17+
Apply: () => Task.CompletedTask);
18+
19+
[Fact]
20+
public void EmptyReport_GenericHeader()
21+
{
22+
var h = NotificationHeader.For(new DriftReport(Array.Empty<DriftItem>()));
23+
Assert.Equal("Monitored settings have drifted", h);
24+
}
25+
26+
[Fact]
27+
public void NullReport_GenericHeader()
28+
{
29+
Assert.Equal("Monitored settings have drifted", NotificationHeader.For(null!));
30+
}
31+
32+
[Fact]
33+
public void SingleAiPolicy_SaysWindowsAi()
34+
{
35+
// The bug we're fixing: this used to say "Display settings have drifted"
36+
// even for an AI policy drift. Verify the AI category is named correctly.
37+
var h = NotificationHeader.For(new DriftReport(new[] { Item("ai") }));
38+
Assert.Equal("Windows AI setting has drifted", h);
39+
Assert.DoesNotContain("Display", h);
40+
}
41+
42+
[Fact]
43+
public void MultipleAiPolicies_PluralForm()
44+
{
45+
var h = NotificationHeader.For(new DriftReport(new[] { Item("ai"), Item("ai") }));
46+
Assert.Equal("Windows AI settings have drifted", h);
47+
}
48+
49+
[Fact]
50+
public void SingleService_SingularForm()
51+
{
52+
var h = NotificationHeader.For(new DriftReport(new[] { Item("service") }));
53+
Assert.Equal("Windows service has drifted", h);
54+
}
55+
56+
[Fact]
57+
public void MultipleServices_PluralForm()
58+
{
59+
var h = NotificationHeader.For(new DriftReport(new[] { Item("service"), Item("service") }));
60+
Assert.Equal("Windows services have drifted", h);
61+
}
62+
63+
[Fact]
64+
public void SingleDisplay_SingularForm()
65+
{
66+
var h = NotificationHeader.For(new DriftReport(new[] { Item("display") }));
67+
Assert.Equal("Display setting has drifted", h);
68+
}
69+
70+
[Fact]
71+
public void GlobalGaming_KnownLabel()
72+
{
73+
var h = NotificationHeader.For(new DriftReport(new[] { Item("global") }));
74+
Assert.Equal("Global gaming setting has drifted", h);
75+
}
76+
77+
[Fact]
78+
public void MixedCategories_GenericCountHeader()
79+
{
80+
var h = NotificationHeader.For(new DriftReport(new[]
81+
{
82+
Item("ai"), Item("service"), Item("display")
83+
}));
84+
Assert.Equal("3 monitored settings have drifted", h);
85+
}
86+
87+
[Fact]
88+
public void UnknownCategory_FallsBackGracefully()
89+
{
90+
var h = NotificationHeader.For(new DriftReport(new[] { Item("madeup") }));
91+
Assert.Contains("drifted", h);
92+
// Doesn't crash or say "Display"
93+
Assert.DoesNotContain("Display", h);
94+
}
95+
}

0 commit comments

Comments
 (0)