From 65c1d72610ebe113cbcd5a6fdec20bdad246a024 Mon Sep 17 00:00:00 2001 From: "bakudies@microsoft.com" Date: Tue, 21 Jul 2026 16:24:10 -0700 Subject: [PATCH] refactor(tray): extract Settings page into a WinUI-free view model over a settings store Moves the Settings page's settings read/persist logic out of the code-behind into a WinUI-free view model backed by a new settings facade, as a behavior-and-visual-identical refactor. First page to adopt the merged DI + navigation activation seam and the first visible reduction of a page god-file. - Add ISettingsStore (SettingsStore): immutable snapshot read, batched Update, a single non-echoing Changed (self-originated saves suppressed; external saves marshaled to the UI thread), and BeginSelfWrite() so App-owned direct writes share the same echo suppression. - Add SettingsPageViewModel (WinUI-free): two-way-bindable settings state that persists through the store, preserving the save/notify contract and the read-aloud / tool-call side effects, reloading on external change without re-persisting, and normalizing combo values so a legacy tag never renders blank. - Add SettingsAppInfoProjection (WinUI-free) for the About section strings. - Bind SettingsPage.xaml to the view model; shrink the code-behind to the remaining view concerns (gateway removal dialog, uptime timer, saved indicator, app-info, gateway-section refresh on external change). - Populate the page->view-model map so the navigation hook resolves and binds the view model; add ApplyAutoStart/SetChatSpeakerMuted/SetChatToolCallsVisible to the app-commands adapter, with mute suppression scoped to the Settings call. - Record settings-store and settings-page-vm in the architecture ledger with behavioral guard tests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/ARCHITECTURE.md | 5 + src/OpenClaw.Tray.WinUI/App.xaml.cs | 67 +++- .../Pages/SettingsPage.xaml | 29 +- .../Pages/SettingsPage.xaml.cs | 349 +++--------------- .../Adapters/FramePageActivator.cs | 9 +- .../Presentation/AppServiceRegistration.cs | 6 + .../Presentation/IPageActivator.cs | 7 +- .../Presentation/ISettingsStore.cs | 111 ++++++ .../Presentation/SettingsAppInfoProjection.cs | 155 ++++++++ .../Presentation/SettingsPageViewModel.cs | 346 ++++++++++++++++- .../Presentation/SettingsStore.cs | 138 +++++++ .../Services/IAppCommands.cs | 3 + .../Windows/HubWindow.xaml.cs | 10 +- .../ChatToolCallsToggleContractTests.cs | 11 +- .../OpenClaw.Tray.Tests.csproj | 3 + .../AppServiceRegistrationTests.cs | 10 +- .../PresentationSeamContractTests.cs | 20 + .../Presentation/PresentationTestDoubles.cs | 71 +++- .../SettingsAppInfoProjectionTests.cs | 77 ++++ .../SettingsPageViewModelTests.cs | 278 ++++++++++++++ .../Presentation/SettingsStoreTests.cs | 151 ++++++++ .../Presentation/UiDispatcherContractTests.cs | 5 +- 22 files changed, 1504 insertions(+), 357 deletions(-) create mode 100644 src/OpenClaw.Tray.WinUI/Presentation/ISettingsStore.cs create mode 100644 src/OpenClaw.Tray.WinUI/Presentation/SettingsAppInfoProjection.cs create mode 100644 src/OpenClaw.Tray.WinUI/Presentation/SettingsStore.cs create mode 100644 tests/OpenClaw.Tray.Tests/Presentation/SettingsAppInfoProjectionTests.cs create mode 100644 tests/OpenClaw.Tray.Tests/Presentation/SettingsPageViewModelTests.cs create mode 100644 tests/OpenClaw.Tray.Tests/Presentation/SettingsStoreTests.cs diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 5b704e115..11c15352a 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -57,6 +57,8 @@ These are the canonical homes. Do not reintroduce private copies elsewhere. | UI-thread marshaling for presentation code | `IUiDispatcher` | authoritative | | Page view-model activation/deactivation + disposal lifetime | `NavigationScopeManager` | authoritative | | Presentation-layer DI composition root | `AppServiceRegistration` (root `ServiceProvider`, owned by `App`) | authoritative | +| Settings snapshot read + batched save + non-echoing change notification | `ISettingsStore` | authoritative | +| Settings page load/persist view logic | `SettingsPageViewModel` | authoritative | | Capability UI metadata | `NodeCapabilityUiCatalog` (planned) | planned | | Capability registration/gating | `NodeCapabilityRegistrationPolicy` (planned) | planned | | Local MCP exposure policy | `McpCapabilityPolicy` (planned) | planned | @@ -72,6 +74,7 @@ These are the canonical homes. Do not reintroduce private copies elsewhere. | `src/OpenClaw.Tray.WinUI/Chat/OpenClawChatTimeline.cs` | `TimelineScrollController`, `ChatBubbleRenderer`, `ToolCallCardRenderer`, `PermissionRequestCard`, `AttachmentBubbleRenderer` | | `src/OpenClaw.Tray.WinUI/Chat/OpenClawComposer.cs` | `ComposerViewModel`, `SlashCommandPalette`, `AttachmentPreviewStrip`, `VoiceComposerController` | | `src/OpenClaw.Tray.WinUI/Pages/ConnectionPage.xaml.cs` | `ConnectionPagePlan` (pure), `ConnectionPageViewModel`, gateway row models | +| `src/OpenClaw.Tray.WinUI/Pages/SettingsPage.xaml.cs` | settings read/persist → `SettingsPageViewModel` + `ISettingsStore`; keep gateway-uninstall, uptime timer, saved-indicator, and app-info in the view | | `src/OpenClaw.Tray.WinUI/Services/NodeService.cs` | `McpServerHost`, `CanvasWindowManager`, `MediaCapabilityHost`, `RecordingConsentService`, `NodeCapabilityRegistry` | | `src/OpenClaw.Shared/OpenClawGatewayClient.cs` | `PendingRequestRegistry`, `ConnectEnvelopeBuilder`, `GatewayMessageRouter`, per-domain API facades | | `src/OpenClaw.Shared/Models.cs` | per-domain model files + `*Mapper` classes | @@ -124,6 +127,8 @@ leading and trailing pipe. Columns, in order: | navigation-scope | authoritative | src/OpenClaw.Tray.WinUI/Windows/HubWindow.xaml.cs | page view-model activation/deactivation and disposal lifetime | NavigationScopeManager | HubWindow keeps frame navigation back-stack and rail selection | transient page view models are activated on navigation and deactivated then disposed on navigate-away | NavigationScopeManagerTests.NavigatingAway_DeactivatesAndDisposesPreviousViewModel | behavioral | - | | composition-root | authoritative | src/OpenClaw.Tray.WinUI/App.xaml.cs | presentation-layer service construction and wiring | AppServiceRegistration | App remains the composition root and owns non-DI service lifetimes | one validated root ServiceProvider; App-owned singletons registered as instances are never disposed by the container | AppServiceRegistrationTests.Dispose_DoesNotDisposeAppOwnedInstanceSingletons | behavioral | - | | node-summary-text | authoritative | src/OpenClaw.Tray.WinUI/App.xaml.cs | node-summary clipboard text formatting | NodeSummaryText | App keeps the clipboard side effect (building the DataPackage and setting clipboard content) | copied node-summary text is projected only by NodeSummaryText.Build (online/offline state, display-name fallback, short id, detail text, newline join) | NodeSummaryTextTests.Build_MultipleNodes_OneLinePerNodeJoinedByNewline | behavioral | - | +| settings-store | authoritative | src/OpenClaw.Tray.WinUI/Pages/SettingsPage.xaml.cs | hand-rolled save/echo suppression flags for two-way settings binding | ISettingsStore | PermissionsPage and other surfaces may read SettingsManager directly until migrated | a save originating from Update does not echo Changed to the caller and external saves are republished on the UI thread | SettingsStoreTests.Update_DoesNotEchoChangedToSelf | behavioral | when all settings surfaces read and write through ISettingsStore | +| settings-page-vm | authoritative | src/OpenClaw.Tray.WinUI/Pages/SettingsPage.xaml.cs | settings load, persist, echo-guard, and auto-save wiring | SettingsPageViewModel | code-behind keeps gateway-uninstall, gateway-info and uptime timer, saved-indicator visual, and app-info population | each settings control persists its field through the store preserving mutate-save-notify order and does not re-persist on external change | SettingsPageViewModelTests.ExternalChange_ReloadsWithoutRePersisting | behavioral | when the Settings page holds no settings persistence logic in code-behind | ## Deferred test builders diff --git a/src/OpenClaw.Tray.WinUI/App.xaml.cs b/src/OpenClaw.Tray.WinUI/App.xaml.cs index 44a27e439..2f7f1707b 100644 --- a/src/OpenClaw.Tray.WinUI/App.xaml.cs +++ b/src/OpenClaw.Tray.WinUI/App.xaml.cs @@ -64,16 +64,22 @@ public partial class App : Application, OpenClawTray.Services.IAppCommands private ServiceProvider? _services; /// - /// Page type → view-model type map used by the navigation activation hook. - /// Currently empty: no page resolves a view model from DI yet, so the activation - /// hook is a runtime no-op. Entries are added here as pages adopt view models. + /// Page type → view-model type map used by the navigation activation hook. The Settings + /// page resolves its view model from DI and binds it as the page DataContext; pages absent + /// from the map take the no-op activation path. /// private static readonly IReadOnlyDictionary PageViewModelMap = - new Dictionary(); + new Dictionary + { + [typeof(Pages.SettingsPage)] = typeof(SettingsPageViewModel), + }; /// The root service provider, or null before startup / after shutdown. internal IServiceProvider? Services => _services; + /// The settings facade, or null before startup / after shutdown. + internal ISettingsStore? SettingsStore => _services?.GetService(); + /// Resolves the page activator used by HubWindow's navigation hook. internal IPageActivator? PageActivator => _services?.GetService(); /// @@ -4208,6 +4214,33 @@ private async Task ToggleAutoStartAsync() await AutoStartManager.SetAutoStartAsync(_settings.AutoStart); } + /// + /// Persists the auto-start setting and applies the Windows OS registration in the original + /// order (save, then await the OS write, then notify). Returns true only when the OS write + /// and notify complete, so the caller shows its saved confirmation only on success. The save + /// is marked as a store self-write so it does not echo an external-change reload. + /// + public async Task ApplyAutoStart(bool autoStart) + { + if (_settings == null) return false; + try + { + _settings.AutoStart = autoStart; + using (SettingsStore?.BeginSelfWrite()) + { + _settings.Save(); + } + await AutoStartManager.SetAutoStartAsync(autoStart); + OnSettingsSaved(this, EventArgs.Empty); + return true; + } + catch (Exception ex) + { + Logger.Error($"ApplyAutoStart failed: {ex.Message}"); + return false; + } + } + private void OpenLogFile() { try @@ -4491,6 +4524,12 @@ public Task SpeakChatTextAsync(string text) => /// Raised when speaker mute state changes from any source (composer, settings, etc.). public event Action? SpeakerMuteChanged; + /// + /// Sets speaker mute from any surface (chat window, chat page, voice settings) and persists it. + /// This public path is NOT store-self-write-suppressed, so an open Settings page still reflects + /// a mute toggled elsewhere. The Settings-page-originated call goes through the explicit + /// below, which suppresses its own echo. + /// public void SetChatSpeakerMuted(bool muted) { if (_chatCoordinator is { } c) c.IsMuted = muted; @@ -4504,6 +4543,26 @@ public void SetChatSpeakerMuted(bool muted) SpeakerMuteChanged?.Invoke(muted); } + /// + /// Settings-page-originated mute: wraps the shared write in a store self-write so it does not + /// echo an external-change reload back to the Settings view model that triggered it. + /// + void IAppCommands.SetChatSpeakerMuted(bool muted) + { + using (SettingsStore?.BeginSelfWrite()) + { + SetChatSpeakerMuted(muted); + } + } + + /// + /// Pushes tool-call visibility into the live chat timeline. Forwards to the shared + /// static writer so a WinUI-free settings view model can drive it through IAppCommands + /// without referencing the chat UI directly. + /// + public void SetChatToolCallsVisible(bool visible) => + OpenClawTray.Chat.OpenClawChatRoot.SetToolCallsVisible(visible); + private static void SendDeepLinkToRunningInstance(string uri) { try diff --git a/src/OpenClaw.Tray.WinUI/Pages/SettingsPage.xaml b/src/OpenClaw.Tray.WinUI/Pages/SettingsPage.xaml index 36e3ce599..b31b4bece 100644 --- a/src/OpenClaw.Tray.WinUI/Pages/SettingsPage.xaml +++ b/src/OpenClaw.Tray.WinUI/Pages/SettingsPage.xaml @@ -52,6 +52,7 @@ TextWrapping="Wrap"/> @@ -74,6 +75,8 @@ TextWrapping="Wrap"/> @@ -100,6 +103,7 @@ TextWrapping="Wrap"/> @@ -122,6 +126,7 @@ TextWrapping="Wrap"/> @@ -149,6 +154,7 @@ TextWrapping="Wrap"/> @@ -172,6 +178,7 @@ TextWrapping="Wrap"/> @@ -198,6 +205,7 @@ TextWrapping="Wrap"/> @@ -221,6 +229,8 @@ @@ -248,28 +258,28 @@ /// -/// The page→view-model map is currently empty, so every navigation takes the -/// "no view model" path: the scope manager only deactivates and disposes any prior -/// view model, and a page's DataContext is never touched. This makes the -/// activation hook a runtime no-op until pages adopt view models, at which point their -/// entries are added to the map. +/// Pages present in the map (currently the Settings page) resolve and activate their view model +/// on navigation and have it assigned as their DataContext. Pages absent from the map take +/// the "no view model" path: the scope manager only deactivates and disposes any prior view model +/// and the page's DataContext is left untouched. /// internal sealed class FramePageActivator : IPageActivator { diff --git a/src/OpenClaw.Tray.WinUI/Presentation/AppServiceRegistration.cs b/src/OpenClaw.Tray.WinUI/Presentation/AppServiceRegistration.cs index 868ee7d92..1f890f4aa 100644 --- a/src/OpenClaw.Tray.WinUI/Presentation/AppServiceRegistration.cs +++ b/src/OpenClaw.Tray.WinUI/Presentation/AppServiceRegistration.cs @@ -33,6 +33,12 @@ public static IServiceCollection AddOpenClawTrayCore(this IServiceCollection ser services.AddSingleton(context.AppCommands); services.AddSingleton(context.Settings); + // Settings facade over the App-owned SettingsManager. Constructed eagerly from the + // already-owned singletons so presentation code depends on ISettingsStore, never the + // concrete manager. It subscribes to the manager (which App owns) and is not disposed + // by the container. + services.AddSingleton(new SettingsStore(context.Settings, context.Dispatcher)); + // Container-owned navigation lifetime manager (disposed with the root provider). services.AddSingleton(); diff --git a/src/OpenClaw.Tray.WinUI/Presentation/IPageActivator.cs b/src/OpenClaw.Tray.WinUI/Presentation/IPageActivator.cs index d045f39be..3e6cfdd40 100644 --- a/src/OpenClaw.Tray.WinUI/Presentation/IPageActivator.cs +++ b/src/OpenClaw.Tray.WinUI/Presentation/IPageActivator.cs @@ -8,10 +8,9 @@ namespace OpenClawTray.Presentation; /// page's DataContext. /// /// -/// No page is mapped to a view model yet, so only advances -/// the navigation scope (deactivating and disposing any prior view model) and never -/// touches a page's data context. The seam therefore has no observable runtime effect -/// today; it is proven by unit tests. +/// Pages mapped to a view model (currently the Settings page) have it resolved, activated, and +/// assigned as their DataContext by . Unmapped pages only advance +/// the navigation scope (deactivating and disposing any prior view model) with no data-context change. /// public interface IPageActivator { diff --git a/src/OpenClaw.Tray.WinUI/Presentation/ISettingsStore.cs b/src/OpenClaw.Tray.WinUI/Presentation/ISettingsStore.cs new file mode 100644 index 000000000..7b16879d5 --- /dev/null +++ b/src/OpenClaw.Tray.WinUI/Presentation/ISettingsStore.cs @@ -0,0 +1,111 @@ +namespace OpenClawTray.Presentation; + +/// +/// WinUI-free seam over the App-owned settings for presentation code. It exposes an +/// immutable for reads and a single batched +/// for writes, and raises only for +/// external changes (a caller's own is not echoed back). +/// +/// +/// This removes the hand-rolled save/echo bookkeeping that page code-behind used to carry +/// (the _saving/_loading suppression flags). A view model reads +/// , writes through , and refreshes on +/// without risking a save-storm: the store suppresses the +/// self-originated notification and marshals external ones onto the UI thread. +/// The auto-save contract itself is unchanged: mutates the settings +/// and saves once, exactly as the previous per-toggle code did; callers still invoke +/// IAppCommands.NotifySettingsSaved() for the reconnect/re-register side effect. +/// +public interface ISettingsStore +{ + /// An immutable snapshot of the current settings values used by the settings surfaces. + SettingsSnapshot Current { get; } + + /// + /// Applies to the settings and persists once. The store + /// suppresses the notification that would otherwise echo back to + /// the caller from this save, so a two-way-bound view model cannot loop. + /// + void Update(Action edit); + + /// + /// Marks the calling thread as performing a store-originated write for the scope's lifetime, + /// so a raised on that thread is treated as self-originated + /// and does not echo . Used by App-owned writes that persist settings + /// directly (auto-start OS registration, speaker mute) instead of through , + /// so they get the same echo suppression. Dispose on the same thread that created it. + /// + IDisposable BeginSelfWrite(); + + /// + /// Raised after settings change from a source other than this caller's + /// (for example another surface saving, onboarding, or a background save). Always raised on + /// the UI thread. + /// + event EventHandler? Changed; +} + +/// +/// Narrow write surface handed to . It exposes only the +/// fields the settings surfaces mutate, so presentation code never touches the concrete +/// settings manager. Grows as more pages adopt the store. +/// +public interface ISettingsEditor +{ + bool AutoStart { set; } + bool GlobalHotkeyEnabled { set; } + bool UseLegacyWebChat { set; } + bool ShowNotifications { set; } + string NotificationSound { set; } + string AppTheme { set; } + + /// Writes the raw diagnostics override (null clears it back to the computed default). + bool? ShowDiagnosticsOverride { set; } + + bool NotifyHealth { set; } + bool NotifyUrgent { set; } + bool NotifyReminder { set; } + bool NotifyEmail { set; } + bool NotifyCalendar { set; } + bool NotifyBuild { set; } + bool NotifyStock { set; } + bool NotifyInfo { set; } + + bool ScreenRecordingConsentGiven { set; } + bool CameraRecordingConsentGiven { set; } + + bool ShowChatToolCalls { set; } +} + +/// +/// Immutable read snapshot of the settings values the settings surfaces display. Mirrors the +/// fields the settings page previously read directly off the settings manager. +/// +public sealed record SettingsSnapshot +{ + public bool AutoStart { get; init; } + public bool GlobalHotkeyEnabled { get; init; } + public bool UseLegacyWebChat { get; init; } + public bool ShowNotifications { get; init; } + public string NotificationSound { get; init; } = "Default"; + public string AppTheme { get; init; } = "System"; + + /// The effective diagnostics visibility (override applied over the computed default). + public bool ShowDiagnosticsEffective { get; init; } + + public bool NotifyHealth { get; init; } + public bool NotifyUrgent { get; init; } + public bool NotifyReminder { get; init; } + public bool NotifyEmail { get; init; } + public bool NotifyCalendar { get; init; } + public bool NotifyBuild { get; init; } + public bool NotifyStock { get; init; } + public bool NotifyInfo { get; init; } + + public bool ScreenRecordingConsentGiven { get; init; } + public bool CameraRecordingConsentGiven { get; init; } + + /// Reflects VoiceTtsEnabled; the "read responses aloud" toggle mirrors it. + public bool VoiceTtsEnabled { get; init; } + public bool ShowChatToolCalls { get; init; } +} diff --git a/src/OpenClaw.Tray.WinUI/Presentation/SettingsAppInfoProjection.cs b/src/OpenClaw.Tray.WinUI/Presentation/SettingsAppInfoProjection.cs new file mode 100644 index 000000000..dfd2d0ec9 --- /dev/null +++ b/src/OpenClaw.Tray.WinUI/Presentation/SettingsAppInfoProjection.cs @@ -0,0 +1,155 @@ +using System.Globalization; +using System.IO; +using System.Text.Json; + +namespace OpenClawTray.Presentation; + +/// +/// WinUI-free projection of the "About / app info" strings the settings surface displays. +/// It holds the pure formatting and environment/reflection lookups that used to live inline in +/// the settings page code-behind, so they are unit-testable. The one genuinely WinUI-bound value +/// (the WinUI display name, resolved from Microsoft.UI.Xaml.Application's assembly) is +/// passed in by the view; everything else is computed here. +/// +public static class SettingsAppInfoProjection +{ + private const string PackagedInstallText = "Packaged (MSIX)"; + private const string UnpackagedInstallText = "Unpackaged (developer)"; + private const string DefaultChannel = "stable"; + + /// Composes the "runtime / WinUI / Windows App SDK" one-line stack description. + public static string BuildRuntimeStack(string frameworkDescription, string winUiDisplayName, string windowsAppSdkDisplayName) => + $"{frameworkDescription} / {winUiDisplayName} / {windowsAppSdkDisplayName}"; + + /// Maps the packaged flag to the installation-kind label. + public static string InstallKind(bool isPackaged) => isPackaged ? PackagedInstallText : UnpackagedInstallText; + + /// Resolves the update-channel label, defaulting to stable when unset. + public static string ResolveUpdateChannel(string? channelEnvironmentValue) => + string.IsNullOrWhiteSpace(channelEnvironmentValue) ? DefaultChannel : channelEnvironmentValue.Trim(); + + /// Strips a +buildmetadata suffix from a semantic version string. + public static string StripBuildMetadata(string version) + { + var plus = version.IndexOf('+', StringComparison.Ordinal); + return plus >= 0 ? version[..plus] : version; + } + + /// + /// Formats the entry assembly's last-write time as the displayed build date, or null when the + /// location is missing/unreadable (matching the previous "hide the build row" behavior). + /// + public static string? FormatBuildDate(string? entryAssemblyLocation, CultureInfo culture) + { + try + { + if (string.IsNullOrWhiteSpace(entryAssemblyLocation) || !File.Exists(entryAssemblyLocation)) + { + return null; + } + + return File.GetLastWriteTime(entryAssemblyLocation).ToString("MMM d, yyyy", culture); + } + catch (Exception ex) when (ex is ArgumentException or IOException or UnauthorizedAccessException or NotSupportedException) + { + return null; + } + } + + /// + /// Resolves the Windows App SDK display name, preferring the package version recorded in the + /// entry assembly's .deps.json and falling back to the native XAML dll's file version. + /// + public static string ResolveWindowsAppSdkDisplayName(string? entryAssemblyName, string baseDirectory) + { + if (TryResolveWindowsAppSdkPackageVersionFromDeps(entryAssemblyName, baseDirectory) is { Length: > 0 } packageVersion) + { + return $"Windows App SDK {packageVersion}"; + } + + return ResolveWindowsAppSdkDisplayNameFromFileVersion(baseDirectory); + } + + private static string ResolveWindowsAppSdkDisplayNameFromFileVersion(string baseDirectory) + { + var xamlNativePath = Path.Combine(baseDirectory, "Microsoft.ui.xaml.dll"); + if (File.Exists(xamlNativePath)) + { + try + { + var productVersion = System.Diagnostics.FileVersionInfo.GetVersionInfo(xamlNativePath).ProductVersion; + if (!string.IsNullOrWhiteSpace(productVersion)) + { + return $"Windows App SDK {StripBuildMetadata(productVersion)}"; + } + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or ArgumentException) + { + // Fall through to the generic label. + } + } + + return "Windows App SDK"; + } + + private static string? TryResolveWindowsAppSdkPackageVersionFromDeps(string? entryAssemblyName, string baseDirectory) + { + if (string.IsNullOrWhiteSpace(entryAssemblyName)) + { + return null; + } + + var depsPath = Path.Combine(baseDirectory, $"{entryAssemblyName}.deps.json"); + if (!File.Exists(depsPath)) + { + return null; + } + + try + { + using var stream = File.OpenRead(depsPath); + using var document = JsonDocument.Parse(stream); + if (!document.RootElement.TryGetProperty("libraries", out var libraries) || + libraries.ValueKind != JsonValueKind.Object) + { + return null; + } + + foreach (var library in libraries.EnumerateObject()) + { + const string packagePrefix = "Microsoft.WindowsAppSDK/"; + if (library.Name.StartsWith(packagePrefix, StringComparison.OrdinalIgnoreCase)) + { + return StripBuildMetadata(library.Name[packagePrefix.Length..]); + } + } + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or JsonException or ArgumentException) + { + return null; + } + + return null; + } + + /// Formats a duration the way the gateway-uptime row displays it. + public static string FormatDuration(TimeSpan duration) + { + if (duration.TotalDays >= 1) + { + return $"{(int)duration.TotalDays}d {duration.Hours}h"; + } + + if (duration.TotalHours >= 1) + { + return $"{(int)duration.TotalHours}h {duration.Minutes}m"; + } + + if (duration.TotalMinutes >= 1) + { + return $"{(int)duration.TotalMinutes}m {duration.Seconds}s"; + } + + return $"{Math.Max(0, (int)duration.TotalSeconds)}s"; + } +} diff --git a/src/OpenClaw.Tray.WinUI/Presentation/SettingsPageViewModel.cs b/src/OpenClaw.Tray.WinUI/Presentation/SettingsPageViewModel.cs index 17e5ab0ed..0fd4aa8e2 100644 --- a/src/OpenClaw.Tray.WinUI/Presentation/SettingsPageViewModel.cs +++ b/src/OpenClaw.Tray.WinUI/Presentation/SettingsPageViewModel.cs @@ -1,33 +1,347 @@ +using System.ComponentModel; +using System.Runtime.CompilerServices; +using System.Threading.Tasks; using OpenClawTray.Services; namespace OpenClawTray.Presentation; /// -/// Transient view-model placeholder for the Settings page. It exercises the DI wiring -/// end to end: it is constructor-injected with app services, participates in the -/// navigation activation/deactivation lifetime, and is disposed when its navigation -/// scope ends. It intentionally holds no behavior yet. Nothing is started in the -/// constructor. +/// WinUI-free view model for the Settings page. It owns the settings surface's read/persist +/// logic that previously lived in the page code-behind: it loads the current values from +/// on activation, exposes two-way-bindable properties, and persists +/// each change through the store while preserving the exact auto-save contract +/// (mutate -> save -> notify) and the two chat side effects. /// -internal sealed class SettingsPageViewModel : INavigationAware, IDisposable +/// +/// Echo handling lives in the store, not here: a property change persists through +/// (which suppresses the self-originated notification), and an +/// external change arrives via and reloads the properties +/// under the _loading guard so it cannot re-persist. View-only side effects (the "Saved" +/// indicator flash and refreshing the view-owned gateway section) are surfaced as events the page +/// handles; OS- and WinUI-coupled work (auto-start registration, speaker mute, chat tool-call +/// visibility) is delegated to , so this type stays free of WinUI and OS APIs. +/// +internal sealed class SettingsPageViewModel : INavigationAware, IDisposable, INotifyPropertyChanged { - public SettingsPageViewModel(IAppCommands appCommands, SettingsManager settings, IUiDispatcher dispatcher) + private const string DefaultNotificationSound = "Default"; + private const string DefaultAppTheme = "System"; + + private static readonly string[] SoundTags = { "Default", "None", "Subtle" }; + private static readonly string[] ThemeTags = { "System", "Light", "Dark" }; + + private readonly ISettingsStore _store; + private readonly IAppCommands _appCommands; + + private bool _loading; + private bool _subscribed; + + private bool _autoStart; + private bool _globalHotkeyEnabled; + private bool _useLegacyWebChat; + private bool _showNotifications; + private string _notificationSound = DefaultNotificationSound; + private string _appTheme = DefaultAppTheme; + private bool _showDiagnostics; + private bool _notifyHealth; + private bool _notifyUrgent; + private bool _notifyReminder; + private bool _notifyEmail; + private bool _notifyCalendar; + private bool _notifyBuild; + private bool _notifyStock; + private bool _notifyInfo; + private bool _screenRecordingConsentGiven; + private bool _cameraRecordingConsentGiven; + private bool _readResponsesAloud; + private bool _showChatToolCalls; + + public SettingsPageViewModel(ISettingsStore store, IAppCommands appCommands) { - AppCommands = appCommands ?? throw new ArgumentNullException(nameof(appCommands)); - Settings = settings ?? throw new ArgumentNullException(nameof(settings)); - Dispatcher = dispatcher ?? throw new ArgumentNullException(nameof(dispatcher)); + _store = store ?? throw new ArgumentNullException(nameof(store)); + _appCommands = appCommands ?? throw new ArgumentNullException(nameof(appCommands)); } - internal IAppCommands AppCommands { get; } - internal SettingsManager Settings { get; } - internal IUiDispatcher Dispatcher { get; } + public event PropertyChangedEventHandler? PropertyChanged; + + /// Raised (on the caller's thread) when a change should flash the page's "Saved" indicator. + public event EventHandler? SavedIndicated; + + /// + /// Raised when the settings change from an external source (not this view model's own writes), + /// so the page can refresh the view-owned sections that are not settings-bound (the gateway + /// section). Not raised during the initial activation load. + /// + public event EventHandler? ExternalChanged; internal bool IsActive { get; private set; } internal bool IsDisposed { get; private set; } - public void Activate(object? parameter) => IsActive = true; + public bool AutoStart + { + get => _autoStart; + set + { + if (SetField(ref _autoStart, value) && !_loading) + { + // Auto-start owns an OS registration with a required save -> OS-write -> notify + // order. It is applied through the app command that preserves that ordering, and + // the saved indicator is flashed only after the OS write succeeds (matching the + // original behavior where a failed OS write showed no confirmation). + _ = ApplyAutoStartAsync(value); + } + } + } + + private async Task ApplyAutoStartAsync(bool value) + { + if (await _appCommands.ApplyAutoStart(value)) + { + RaiseSaved(); + } + } + + public bool GlobalHotkeyEnabled + { + get => _globalHotkeyEnabled; + set { if (SetField(ref _globalHotkeyEnabled, value) && !_loading) Persist(e => e.GlobalHotkeyEnabled = value); } + } + + public bool UseLegacyWebChat + { + get => _useLegacyWebChat; + set { if (SetField(ref _useLegacyWebChat, value) && !_loading) Persist(e => e.UseLegacyWebChat = value); } + } + + public bool ShowNotifications + { + get => _showNotifications; + set { if (SetField(ref _showNotifications, value) && !_loading) Persist(e => e.ShowNotifications = value); } + } + + public string NotificationSound + { + get => _notificationSound; + set + { + var normalized = string.IsNullOrEmpty(value) ? DefaultNotificationSound : value; + if (SetField(ref _notificationSound, normalized) && !_loading) Persist(e => e.NotificationSound = normalized); + } + } + + public string AppTheme + { + get => _appTheme; + set + { + var normalized = string.IsNullOrEmpty(value) ? DefaultAppTheme : value; + if (SetField(ref _appTheme, normalized) && !_loading) Persist(e => e.AppTheme = normalized); + } + } + + /// Bound to the diagnostics toggle: reads the effective value, writes the override. + public bool ShowDiagnostics + { + get => _showDiagnostics; + set { if (SetField(ref _showDiagnostics, value) && !_loading) Persist(e => e.ShowDiagnosticsOverride = value); } + } + + public bool NotifyHealth + { + get => _notifyHealth; + set { if (SetField(ref _notifyHealth, value) && !_loading) Persist(e => e.NotifyHealth = value); } + } + + public bool NotifyUrgent + { + get => _notifyUrgent; + set { if (SetField(ref _notifyUrgent, value) && !_loading) Persist(e => e.NotifyUrgent = value); } + } + + public bool NotifyReminder + { + get => _notifyReminder; + set { if (SetField(ref _notifyReminder, value) && !_loading) Persist(e => e.NotifyReminder = value); } + } + + public bool NotifyEmail + { + get => _notifyEmail; + set { if (SetField(ref _notifyEmail, value) && !_loading) Persist(e => e.NotifyEmail = value); } + } + + public bool NotifyCalendar + { + get => _notifyCalendar; + set { if (SetField(ref _notifyCalendar, value) && !_loading) Persist(e => e.NotifyCalendar = value); } + } + + public bool NotifyBuild + { + get => _notifyBuild; + set { if (SetField(ref _notifyBuild, value) && !_loading) Persist(e => e.NotifyBuild = value); } + } + + public bool NotifyStock + { + get => _notifyStock; + set { if (SetField(ref _notifyStock, value) && !_loading) Persist(e => e.NotifyStock = value); } + } + + public bool NotifyInfo + { + get => _notifyInfo; + set { if (SetField(ref _notifyInfo, value) && !_loading) Persist(e => e.NotifyInfo = value); } + } + + public bool ScreenRecordingConsentGiven + { + get => _screenRecordingConsentGiven; + set { if (SetField(ref _screenRecordingConsentGiven, value) && !_loading) Persist(e => e.ScreenRecordingConsentGiven = value); } + } + + public bool CameraRecordingConsentGiven + { + get => _cameraRecordingConsentGiven; + set { if (SetField(ref _cameraRecordingConsentGiven, value) && !_loading) Persist(e => e.CameraRecordingConsentGiven = value); } + } + + /// + /// "Read responses aloud" mirrors VoiceTtsEnabled (mute is its inverse). Routed through + /// the app command that persists + broadcasts, exactly like before; it does not go through the + /// normal persist/notify path. + /// + public bool ReadResponsesAloud + { + get => _readResponsesAloud; + set + { + if (SetField(ref _readResponsesAloud, value) && !_loading) + { + _appCommands.SetChatSpeakerMuted(!value); + RaiseSaved(); + } + } + } + + /// "Show tool calls and usage" persists the setting and pushes visibility to the live timeline. + public bool ShowChatToolCalls + { + get => _showChatToolCalls; + set + { + if (SetField(ref _showChatToolCalls, value) && !_loading) + { + Persist(e => e.ShowChatToolCalls = value); + _appCommands.SetChatToolCallsVisible(value); + } + } + } + + public void Activate(object? parameter) + { + IsActive = true; + LoadFromStore(); + if (!_subscribed) + { + _store.Changed += OnStoreChanged; + _subscribed = true; + } + } - public void Deactivate() => IsActive = false; + public void Deactivate() + { + IsActive = false; + if (_subscribed) + { + _store.Changed -= OnStoreChanged; + _subscribed = false; + } + } + + public void Dispose() + { + Deactivate(); + IsDisposed = true; + } + + private void OnStoreChanged(object? sender, EventArgs e) + { + LoadFromStore(); + ExternalChanged?.Invoke(this, EventArgs.Empty); + } + + /// Reloads all bound properties from the store snapshot without persisting. + private void LoadFromStore() + { + var s = _store.Current; + _loading = true; + try + { + SetField(ref _autoStart, s.AutoStart, nameof(AutoStart)); + SetField(ref _globalHotkeyEnabled, s.GlobalHotkeyEnabled, nameof(GlobalHotkeyEnabled)); + SetField(ref _useLegacyWebChat, s.UseLegacyWebChat, nameof(UseLegacyWebChat)); + SetField(ref _showNotifications, s.ShowNotifications, nameof(ShowNotifications)); + // Normalize combo values to a known tag so a legacy or odd stored value selects the + // default item instead of rendering a blank combo (matching the old tag lookup). + SetField(ref _notificationSound, NormalizeTag(s.NotificationSound, SoundTags, DefaultNotificationSound), nameof(NotificationSound)); + SetField(ref _appTheme, NormalizeTag(s.AppTheme, ThemeTags, DefaultAppTheme), nameof(AppTheme)); + SetField(ref _showDiagnostics, s.ShowDiagnosticsEffective, nameof(ShowDiagnostics)); + SetField(ref _notifyHealth, s.NotifyHealth, nameof(NotifyHealth)); + SetField(ref _notifyUrgent, s.NotifyUrgent, nameof(NotifyUrgent)); + SetField(ref _notifyReminder, s.NotifyReminder, nameof(NotifyReminder)); + SetField(ref _notifyEmail, s.NotifyEmail, nameof(NotifyEmail)); + SetField(ref _notifyCalendar, s.NotifyCalendar, nameof(NotifyCalendar)); + SetField(ref _notifyBuild, s.NotifyBuild, nameof(NotifyBuild)); + SetField(ref _notifyStock, s.NotifyStock, nameof(NotifyStock)); + SetField(ref _notifyInfo, s.NotifyInfo, nameof(NotifyInfo)); + SetField(ref _screenRecordingConsentGiven, s.ScreenRecordingConsentGiven, nameof(ScreenRecordingConsentGiven)); + SetField(ref _cameraRecordingConsentGiven, s.CameraRecordingConsentGiven, nameof(CameraRecordingConsentGiven)); + SetField(ref _readResponsesAloud, s.VoiceTtsEnabled, nameof(ReadResponsesAloud)); + SetField(ref _showChatToolCalls, s.ShowChatToolCalls, nameof(ShowChatToolCalls)); + } + finally + { + _loading = false; + } + } - public void Dispose() => IsDisposed = true; + private void Persist(Action edit) + { + if (_loading) + { + return; + } + + _store.Update(edit); + _appCommands.NotifySettingsSaved(); + RaiseSaved(); + } + + private void RaiseSaved() => SavedIndicated?.Invoke(this, EventArgs.Empty); + + /// Maps a stored value to a known combo tag (case-insensitive), falling back to the default. + private static string NormalizeTag(string? value, string[] tags, string fallback) + { + foreach (var tag in tags) + { + if (string.Equals(tag, value, StringComparison.OrdinalIgnoreCase)) + { + return tag; + } + } + + return fallback; + } + + private bool SetField(ref T field, T value, [CallerMemberName] string? propertyName = null) + { + if (EqualityComparer.Default.Equals(field, value)) + { + return false; + } + + field = value; + PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName)); + return true; + } } diff --git a/src/OpenClaw.Tray.WinUI/Presentation/SettingsStore.cs b/src/OpenClaw.Tray.WinUI/Presentation/SettingsStore.cs new file mode 100644 index 000000000..0f5bbdb29 --- /dev/null +++ b/src/OpenClaw.Tray.WinUI/Presentation/SettingsStore.cs @@ -0,0 +1,138 @@ +using OpenClawTray.Services; + +namespace OpenClawTray.Presentation; + +/// +/// Default backed by the App-owned . +/// It does not own the manager's lifetime (App does); it only reads/writes it and republishes +/// the manager's Saved event as with self-origination suppressed +/// and UI-thread affinity. +/// +/// +/// Self-origination is tracked with a [ThreadStatic] depth: +/// raises Saved synchronously on the calling thread inside or a +/// scope, so the handler observes a non-zero depth on that thread and +/// suppresses the echo. A save from any other source (another surface, onboarding, a background +/// writer) runs with depth zero and is republished as , marshaled onto the UI +/// thread via . +/// +internal sealed class SettingsStore : ISettingsStore +{ + [ThreadStatic] + private static int t_selfUpdateDepth; + + private readonly SettingsManager _settings; + private readonly IUiDispatcher _dispatcher; + + public SettingsStore(SettingsManager settings, IUiDispatcher dispatcher) + { + _settings = settings ?? throw new ArgumentNullException(nameof(settings)); + _dispatcher = dispatcher ?? throw new ArgumentNullException(nameof(dispatcher)); + _settings.Saved += OnManagerSaved; + } + + public event EventHandler? Changed; + + public SettingsSnapshot Current => new() + { + AutoStart = _settings.AutoStart, + GlobalHotkeyEnabled = _settings.GlobalHotkeyEnabled, + UseLegacyWebChat = _settings.UseLegacyWebChat, + ShowNotifications = _settings.ShowNotifications, + NotificationSound = _settings.NotificationSound, + AppTheme = _settings.AppTheme, + ShowDiagnosticsEffective = _settings.ShowDiagnosticsEffective, + NotifyHealth = _settings.NotifyHealth, + NotifyUrgent = _settings.NotifyUrgent, + NotifyReminder = _settings.NotifyReminder, + NotifyEmail = _settings.NotifyEmail, + NotifyCalendar = _settings.NotifyCalendar, + NotifyBuild = _settings.NotifyBuild, + NotifyStock = _settings.NotifyStock, + NotifyInfo = _settings.NotifyInfo, + ScreenRecordingConsentGiven = _settings.ScreenRecordingConsentGiven, + CameraRecordingConsentGiven = _settings.CameraRecordingConsentGiven, + VoiceTtsEnabled = _settings.VoiceTtsEnabled, + ShowChatToolCalls = _settings.ShowChatToolCalls, + }; + + public void Update(Action edit) + { + ArgumentNullException.ThrowIfNull(edit); + + using (BeginSelfWrite()) + { + edit(new Editor(_settings)); + _settings.Save(); + } + } + + public IDisposable BeginSelfWrite() + { + t_selfUpdateDepth++; + return new SelfWriteScope(); + } + + private void OnManagerSaved(object? sender, EventArgs e) + { + // Saved fires synchronously on the thread that called Save(). When that is our own + // Update or an App-owned self-write on this thread, the depth is non-zero and the echo + // is suppressed. + if (t_selfUpdateDepth > 0) + { + return; + } + + if (_dispatcher.HasThreadAccess) + { + Changed?.Invoke(this, EventArgs.Empty); + } + else + { + _dispatcher.TryEnqueue(() => Changed?.Invoke(this, EventArgs.Empty)); + } + } + + /// Decrements the self-write depth on dispose. Created and disposed on the same thread. + private sealed class SelfWriteScope : IDisposable + { + private bool _disposed; + + public void Dispose() + { + if (_disposed) + { + return; + } + + _disposed = true; + t_selfUpdateDepth--; + } + } + + private sealed class Editor : ISettingsEditor + { + private readonly SettingsManager _settings; + + public Editor(SettingsManager settings) => _settings = settings; + + public bool AutoStart { set => _settings.AutoStart = value; } + public bool GlobalHotkeyEnabled { set => _settings.GlobalHotkeyEnabled = value; } + public bool UseLegacyWebChat { set => _settings.UseLegacyWebChat = value; } + public bool ShowNotifications { set => _settings.ShowNotifications = value; } + public string NotificationSound { set => _settings.NotificationSound = value; } + public string AppTheme { set => _settings.AppTheme = value; } + public bool? ShowDiagnosticsOverride { set => _settings.ShowDiagnosticsOverride = value; } + public bool NotifyHealth { set => _settings.NotifyHealth = value; } + public bool NotifyUrgent { set => _settings.NotifyUrgent = value; } + public bool NotifyReminder { set => _settings.NotifyReminder = value; } + public bool NotifyEmail { set => _settings.NotifyEmail = value; } + public bool NotifyCalendar { set => _settings.NotifyCalendar = value; } + public bool NotifyBuild { set => _settings.NotifyBuild = value; } + public bool NotifyStock { set => _settings.NotifyStock = value; } + public bool NotifyInfo { set => _settings.NotifyInfo = value; } + public bool ScreenRecordingConsentGiven { set => _settings.ScreenRecordingConsentGiven = value; } + public bool CameraRecordingConsentGiven { set => _settings.CameraRecordingConsentGiven = value; } + public bool ShowChatToolCalls { set => _settings.ShowChatToolCalls = value; } + } +} diff --git a/src/OpenClaw.Tray.WinUI/Services/IAppCommands.cs b/src/OpenClaw.Tray.WinUI/Services/IAppCommands.cs index 8b5462546..a60339172 100644 --- a/src/OpenClaw.Tray.WinUI/Services/IAppCommands.cs +++ b/src/OpenClaw.Tray.WinUI/Services/IAppCommands.cs @@ -18,5 +18,8 @@ internal interface IAppCommands void ShowGatewayWizard(); void ShowConnectionStatus(); void NotifySettingsSaved(); + Task ApplyAutoStart(bool autoStart); + void SetChatSpeakerMuted(bool muted); + void SetChatToolCallsVisible(bool visible); Task ResendOpenTelemetryProbeAsync(); } diff --git a/src/OpenClaw.Tray.WinUI/Windows/HubWindow.xaml.cs b/src/OpenClaw.Tray.WinUI/Windows/HubWindow.xaml.cs index 62cf2d7cf..7d2fe9a99 100644 --- a/src/OpenClaw.Tray.WinUI/Windows/HubWindow.xaml.cs +++ b/src/OpenClaw.Tray.WinUI/Windows/HubWindow.xaml.cs @@ -1027,11 +1027,11 @@ private void OnContentFrameNavigated(object sender, Microsoft.UI.Xaml.Navigation ArmContentReady(e.Content as FrameworkElement); // Navigation activation hook: resolve + assign a DI-backed view model for the - // navigated page. The page→view-model map is currently empty, so this only - // advances the navigation scope (deactivating any prior view model) and never - // touches a page's DataContext — a runtime no-op until pages adopt view models. - // Contained in a try/catch so a future misbehaving view model's activation - // cannot escape this XAML event handler and crash the app. + // navigated page. Pages present in the page->view-model map (currently Settings) + // get their view model resolved, activated, and assigned as DataContext; other pages + // only advance the navigation scope (deactivating any prior view model). Contained in a + // try/catch so a misbehaving view model's activation cannot escape this XAML event handler + // and crash the app. if (e.Content is { } content) { try diff --git a/tests/OpenClaw.Tray.Tests/ChatToolCallsToggleContractTests.cs b/tests/OpenClaw.Tray.Tests/ChatToolCallsToggleContractTests.cs index af15e71bf..a1bb84f37 100644 --- a/tests/OpenClaw.Tray.Tests/ChatToolCallsToggleContractTests.cs +++ b/tests/OpenClaw.Tray.Tests/ChatToolCallsToggleContractTests.cs @@ -10,7 +10,6 @@ public void ProductionTimeline_HonorsSettingsToolCallVisibilityToggle() var root = Read("src", "OpenClaw.Tray.WinUI", "Chat", "OpenClawChatRoot.cs"); var composer = Read("src", "OpenClaw.Tray.WinUI", "Chat", "OpenClawComposer.cs"); var timeline = Read("src", "OpenClaw.Tray.WinUI", "Chat", "OpenClawChatTimeline.cs"); - var settings = Read("src", "OpenClaw.Tray.WinUI", "Pages", "SettingsPage.xaml.cs"); var app = Read("src", "OpenClaw.Tray.WinUI", "App.xaml.cs"); // Root still owns the shared tool-call visibility state and feeds it to @@ -31,10 +30,12 @@ public void ProductionTimeline_HonorsSettingsToolCallVisibilityToggle() Assert.DoesNotContain("ShowToolCalls", composer); Assert.DoesNotContain("OnShowToolCallsChanged", composer); - // Settings drives it: persists the setting and pushes it into the live - // timeline via the static writer. - Assert.Contains("OpenClawTray.Chat.OpenClawChatRoot.SetToolCallsVisible", settings); - Assert.Contains("ShowChatToolCalls", settings); + // Settings drives it: the settings view model persists the setting and pushes it into + // the live timeline through IAppCommands, which App forwards to the static writer. + var settingsVm = Read("src", "OpenClaw.Tray.WinUI", "Presentation", "SettingsPageViewModel.cs"); + Assert.Contains("SetChatToolCallsVisible", settingsVm); + Assert.Contains("ShowChatToolCalls", settingsVm); + Assert.Contains("OpenClawTray.Chat.OpenClawChatRoot.SetToolCallsVisible", app); // Startup seeds visibility from the persisted setting. Assert.Contains("SetToolCallsVisible(_settings.ShowChatToolCalls)", app); diff --git a/tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj b/tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj index cf9ffdfc9..50cfa608e 100644 --- a/tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj +++ b/tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj @@ -144,6 +144,9 @@ + + + diff --git a/tests/OpenClaw.Tray.Tests/Presentation/AppServiceRegistrationTests.cs b/tests/OpenClaw.Tray.Tests/Presentation/AppServiceRegistrationTests.cs index 3fbc36d79..268733332 100644 --- a/tests/OpenClaw.Tray.Tests/Presentation/AppServiceRegistrationTests.cs +++ b/tests/OpenClaw.Tray.Tests/Presentation/AppServiceRegistrationTests.cs @@ -71,9 +71,13 @@ public void PageViewModels_AreTransient_AndReceiveInjectedServices() var second = scope.ServiceProvider.GetRequiredService(); Assert.NotSame(first, second); - Assert.Same(dispatcher, first.Dispatcher); - Assert.Same(commands, first.AppCommands); - Assert.Same(settings, first.Settings); + + // The placeholder permissions view model still exposes its injected services, so use it + // to prove the transient page view models receive the registered singleton instances. + var permissions = scope.ServiceProvider.GetRequiredService(); + Assert.Same(dispatcher, permissions.Dispatcher); + Assert.Same(commands, permissions.AppCommands); + Assert.Same(settings, permissions.Settings); } } diff --git a/tests/OpenClaw.Tray.Tests/Presentation/PresentationSeamContractTests.cs b/tests/OpenClaw.Tray.Tests/Presentation/PresentationSeamContractTests.cs index 1b83fbb44..d1697ddda 100644 --- a/tests/OpenClaw.Tray.Tests/Presentation/PresentationSeamContractTests.cs +++ b/tests/OpenClaw.Tray.Tests/Presentation/PresentationSeamContractTests.cs @@ -78,6 +78,26 @@ public void App_ResetsNavigationScope_OnHubClose() Assert.Contains("PageActivator?.Reset()", source); } + [Fact] + public void App_SuppressesOnlySettingsOriginatedSpeakerMute() + { + var source = ReadAppSources(); + + // The Settings-page-originated mute (explicit IAppCommands impl) suppresses its own echo + // by wrapping the shared write in a store self-write... + var explicitIdx = source.IndexOf("void IAppCommands.SetChatSpeakerMuted", StringComparison.Ordinal); + Assert.True(explicitIdx >= 0, "Expected an explicit IAppCommands.SetChatSpeakerMuted for the Settings-originated path."); + var explicitBlock = source.Substring(explicitIdx, Math.Min(300, source.Length - explicitIdx)); + Assert.Contains("BeginSelfWrite", explicitBlock); + + // ...while the public method (used by chat window / chat page / voice settings) persists + // WITHOUT a self-write scope, so an open Settings page still reflects a mute toggled there. + var publicIdx = source.IndexOf("public void SetChatSpeakerMuted", StringComparison.Ordinal); + Assert.True(publicIdx >= 0, "Expected a public SetChatSpeakerMuted for other surfaces."); + var publicBlock = source.Substring(publicIdx, Math.Min(300, source.Length - publicIdx)); + Assert.DoesNotContain("BeginSelfWrite", publicBlock); + } + [Fact] public void HubWindow_InvokesPageActivator_OnNavigation() { diff --git a/tests/OpenClaw.Tray.Tests/Presentation/PresentationTestDoubles.cs b/tests/OpenClaw.Tray.Tests/Presentation/PresentationTestDoubles.cs index c4faaa88e..63e97fe6b 100644 --- a/tests/OpenClaw.Tray.Tests/Presentation/PresentationTestDoubles.cs +++ b/tests/OpenClaw.Tray.Tests/Presentation/PresentationTestDoubles.cs @@ -60,12 +60,81 @@ public void CheckForUpdates() { } public void ShowOnboarding() { } public void ShowGatewayWizard() { } public void ShowConnectionStatus() { } - public void NotifySettingsSaved() { } + public void NotifySettingsSaved() => NotifySettingsSavedCount++; + public Task ApplyAutoStart(bool autoStart) + { + AutoStartApplied = autoStart; + AutoStartApplyCount++; + return Task.FromResult(AutoStartResult); + } + public void SetChatSpeakerMuted(bool muted) => SpeakerMuted = muted; + public void SetChatToolCallsVisible(bool visible) => ToolCallsVisible = visible; public Task ResendOpenTelemetryProbeAsync() => Task.FromResult(true); + public bool? SpeakerMuted { get; private set; } + public bool? ToolCallsVisible { get; private set; } + public bool? AutoStartApplied { get; private set; } + public int AutoStartApplyCount { get; private set; } + + /// Result returned by so tests can simulate OS-write failure. + public bool AutoStartResult { get; set; } = true; + public int NotifySettingsSavedCount { get; private set; } + public void Dispose() => Disposed = true; } +/// +/// App-commands double that mimics the real App-owned settings writes: it persists directly to a +/// real and marks the save as a store self-write via +/// , so tests can prove those writes do not echo an +/// external-change reload (unlike , which no-ops the save). +/// +internal sealed class SelfWritingAppCommands : IAppCommands +{ + private readonly ISettingsStore _store; + private readonly SettingsManager _settings; + + public SelfWritingAppCommands(ISettingsStore store, SettingsManager settings) + { + _store = store; + _settings = settings; + } + + public void OpenDashboard(string? path = null) { } + public void Navigate(string pageTag) { } + public void Reconnect() { } + public void Disconnect() { } + public void ShowVoiceOverlay() { } + public void ShowChat() { } + public void CheckForUpdates() { } + public void ShowOnboarding() { } + public void ShowGatewayWizard() { } + public void ShowConnectionStatus() { } + public void NotifySettingsSaved() { } + + public Task ApplyAutoStart(bool autoStart) + { + _settings.AutoStart = autoStart; + using (_store.BeginSelfWrite()) + { + _settings.Save(); + } + return Task.FromResult(true); + } + + public void SetChatSpeakerMuted(bool muted) + { + _settings.VoiceTtsEnabled = !muted; + using (_store.BeginSelfWrite()) + { + _settings.Save(); + } + } + + public void SetChatToolCallsVisible(bool visible) { } + public Task ResendOpenTelemetryProbeAsync() => Task.FromResult(true); +} + /// Fake navigation-aware, disposable view model for scope-lifetime tests. internal sealed class FakeViewModel : INavigationAware, IDisposable { diff --git a/tests/OpenClaw.Tray.Tests/Presentation/SettingsAppInfoProjectionTests.cs b/tests/OpenClaw.Tray.Tests/Presentation/SettingsAppInfoProjectionTests.cs new file mode 100644 index 000000000..a82da414a --- /dev/null +++ b/tests/OpenClaw.Tray.Tests/Presentation/SettingsAppInfoProjectionTests.cs @@ -0,0 +1,77 @@ +using System.Globalization; +using OpenClawTray.Presentation; + +namespace OpenClaw.Tray.Tests.Presentation; + +/// Pure-formatting checks for the settings "about" projection. +public sealed class SettingsAppInfoProjectionTests +{ + [Fact] + public void BuildRuntimeStack_JoinsWithSlashes() + { + Assert.Equal(".NET 10 / WinUI 3 / Windows App SDK 2.0.1", + SettingsAppInfoProjection.BuildRuntimeStack(".NET 10", "WinUI 3", "Windows App SDK 2.0.1")); + } + + [Theory] + [InlineData(true, "Packaged (MSIX)")] + [InlineData(false, "Unpackaged (developer)")] + public void InstallKind_MapsPackagedFlag(bool packaged, string expected) + { + Assert.Equal(expected, SettingsAppInfoProjection.InstallKind(packaged)); + } + + [Theory] + [InlineData(null, "stable")] + [InlineData("", "stable")] + [InlineData(" ", "stable")] + [InlineData(" beta ", "beta")] + public void ResolveUpdateChannel_DefaultsToStable(string? input, string expected) + { + Assert.Equal(expected, SettingsAppInfoProjection.ResolveUpdateChannel(input)); + } + + [Theory] + [InlineData("2.0.1", "2.0.1")] + [InlineData("2.0.1+abc123", "2.0.1")] + public void StripBuildMetadata_RemovesPlusSuffix(string input, string expected) + { + Assert.Equal(expected, SettingsAppInfoProjection.StripBuildMetadata(input)); + } + + [Fact] + public void FormatBuildDate_ReturnsNull_WhenLocationMissing() + { + Assert.Null(SettingsAppInfoProjection.FormatBuildDate(null, CultureInfo.InvariantCulture)); + Assert.Null(SettingsAppInfoProjection.FormatBuildDate(" ", CultureInfo.InvariantCulture)); + Assert.Null(SettingsAppInfoProjection.FormatBuildDate( + Path.Combine(Path.GetTempPath(), $"missing-{Guid.NewGuid():N}.dll"), CultureInfo.InvariantCulture)); + } + + [Fact] + public void FormatBuildDate_FormatsExistingFile() + { + var path = Path.Combine(Path.GetTempPath(), $"buildinfo-{Guid.NewGuid():N}.dll"); + File.WriteAllText(path, "x"); + try + { + var formatted = SettingsAppInfoProjection.FormatBuildDate(path, CultureInfo.InvariantCulture); + Assert.False(string.IsNullOrWhiteSpace(formatted)); + } + finally + { + File.Delete(path); + } + } + + [Theory] + [InlineData(0, "0s")] + [InlineData(45, "45s")] + [InlineData(90, "1m 30s")] + [InlineData(3661, "1h 1m")] + [InlineData(90000, "1d 1h")] + public void FormatDuration_MatchesTieredFormat(int totalSeconds, string expected) + { + Assert.Equal(expected, SettingsAppInfoProjection.FormatDuration(TimeSpan.FromSeconds(totalSeconds))); + } +} diff --git a/tests/OpenClaw.Tray.Tests/Presentation/SettingsPageViewModelTests.cs b/tests/OpenClaw.Tray.Tests/Presentation/SettingsPageViewModelTests.cs new file mode 100644 index 000000000..d87694abe --- /dev/null +++ b/tests/OpenClaw.Tray.Tests/Presentation/SettingsPageViewModelTests.cs @@ -0,0 +1,278 @@ +using OpenClawTray.Presentation; +using OpenClawTray.Services; + +namespace OpenClaw.Tray.Tests.Presentation; + +/// +/// Characterization of the Settings page's save behavior, now owned by +/// : each bound property writes the correct settings field, +/// preserves the mutate -> save -> notify contract, flashes the saved indicator, keeps the two +/// chat side effects, and does not echo/re-persist on external changes. Toggle tests flip relative +/// to the loaded value so they hold regardless of a field's default. +/// +public sealed class SettingsPageViewModelTests +{ + private static SettingsPageViewModel NewVm( + out SettingsManager settings, + out FakeAppCommands appCommands, + out RecordingUiDispatcher dispatcher, + out TempDir temp) + { + temp = new TempDir(); + settings = new SettingsManager(temp.Path); + appCommands = new FakeAppCommands(); + dispatcher = new RecordingUiDispatcher(); + var store = new SettingsStore(settings, dispatcher); + return new SettingsPageViewModel(store, appCommands); + } + + [Fact] + public void Activate_LoadsCurrentSettings() + { + var vm = NewVm(out var settings, out _, out _, out var temp); + using (temp) + { + settings.GlobalHotkeyEnabled = true; + settings.AppTheme = "Dark"; + settings.NotifyBuild = false; + + vm.Activate(null); + + Assert.True(vm.GlobalHotkeyEnabled); + Assert.Equal("Dark", vm.AppTheme); + Assert.False(vm.NotifyBuild); + } + } + + [Fact] + public void TogglingBool_PersistsField_AndNotifiesOnce_AndFlashesSaved() + { + var vm = NewVm(out var settings, out var appCommands, out _, out var temp); + using (temp) + { + vm.Activate(null); + var savedFlashes = 0; + vm.SavedIndicated += (_, _) => savedFlashes++; + + var target = !vm.GlobalHotkeyEnabled; + vm.GlobalHotkeyEnabled = target; + + Assert.Equal(target, settings.GlobalHotkeyEnabled); // persisted the right field + Assert.Equal(1, appCommands.NotifySettingsSavedCount); // notified exactly once (no echo) + Assert.Equal(1, savedFlashes); + } + } + + [Fact] + public void ShowDiagnostics_WritesOverride_NotEffectiveOnly() + { + var vm = NewVm(out var settings, out _, out _, out var temp); + using (temp) + { + vm.Activate(null); + + var target = !vm.ShowDiagnostics; + vm.ShowDiagnostics = target; + + Assert.Equal((bool?)target, settings.ShowDiagnosticsOverride); + } + } + + [Fact] + public void AppTheme_And_NotificationSound_PersistStringTag() + { + var vm = NewVm(out var settings, out _, out _, out var temp); + using (temp) + { + vm.Activate(null); + + vm.AppTheme = "Light"; + vm.NotificationSound = "Subtle"; + + Assert.Equal("Light", settings.AppTheme); + Assert.Equal("Subtle", settings.NotificationSound); + } + } + + [Fact] + public void AutoStart_AppliedThroughAppCommand_FlashesSavedOnlyOnSuccess() + { + var vm = NewVm(out _, out var appCommands, out _, out var temp); + using (temp) + { + vm.Activate(null); + var savedFlashes = 0; + vm.SavedIndicated += (_, _) => savedFlashes++; + + var target = !vm.AutoStart; + vm.AutoStart = target; + + // Routed through the app command that owns the save -> OS-write -> notify ordering. + // With a completed (successful) task, the saved flash runs after the awaited apply. + Assert.Equal((bool?)target, appCommands.AutoStartApplied); + Assert.Equal(1, appCommands.AutoStartApplyCount); + Assert.Equal(0, appCommands.NotifySettingsSavedCount); + Assert.Equal(1, savedFlashes); + } + } + + [Fact] + public void AutoStart_OsWriteFailure_DoesNotFlashSaved() + { + var vm = NewVm(out _, out var appCommands, out _, out var temp); + using (temp) + { + appCommands.AutoStartResult = false; // simulate the OS registration failing + vm.Activate(null); + var savedFlashes = 0; + vm.SavedIndicated += (_, _) => savedFlashes++; + + vm.AutoStart = !vm.AutoStart; + + Assert.Equal(1, appCommands.AutoStartApplyCount); + Assert.Equal(0, savedFlashes); // no confirmation when the apply reports failure + } + } + + [Fact] + public void SelfWrite_AutoStartOrMute_DoesNotTriggerExternalReload() + { + using var temp = new TempDir(); + var settings = new SettingsManager(temp.Path); + var dispatcher = new RecordingUiDispatcher(); + var store = new SettingsStore(settings, dispatcher); + var appCommands = new SelfWritingAppCommands(store, settings); + var vm = new SettingsPageViewModel(store, appCommands); + + vm.Activate(null); + var externalChanges = 0; + vm.ExternalChanged += (_, _) => externalChanges++; + + // These persist directly to settings.Save() but mark a store self-write, so the store must + // suppress Changed and the view model must not treat them as an external change. + vm.AutoStart = !vm.AutoStart; + vm.ReadResponsesAloud = !vm.ReadResponsesAloud; + + Assert.Equal(0, externalChanges); + + // Control: a genuine external save (no self-write scope) still reloads. + settings.NotifyStock = !settings.NotifyStock; + settings.Save(); + Assert.Equal(1, externalChanges); + } + + [Fact] + public void NotificationSound_LegacyOrOddValue_NormalizesToKnownTag() + { + var vm = NewVm(out var settings, out _, out _, out var temp); + using (temp) + { + settings.NotificationSound = "subtle"; // lower-case legacy value + vm.Activate(null); + Assert.Equal("Subtle", vm.NotificationSound); + + settings.NotificationSound = "bogus"; + settings.Save(); // external change -> reload + Assert.Equal("Default", vm.NotificationSound); // unknown -> default, never blank + } + } + + [Fact] + public void ExternalChange_RaisesExternalChanged_ForViewOwnedRefresh() + { + var vm = NewVm(out var settings, out _, out _, out var temp); + using (temp) + { + vm.Activate(null); + var externalChanges = 0; + vm.ExternalChanged += (_, _) => externalChanges++; + + settings.NotifyStock = !settings.NotifyStock; + settings.Save(); + + Assert.Equal(1, externalChanges); + } + } + + [Fact] + public void ReadResponsesAloud_RoutesThroughSpeakerMute_WithoutNotify() + { + var vm = NewVm(out _, out var appCommands, out _, out var temp); + using (temp) + { + vm.Activate(null); + + var target = !vm.ReadResponsesAloud; + vm.ReadResponsesAloud = target; + + Assert.Equal((bool?)(!target), appCommands.SpeakerMuted); // mute is the inverse + Assert.Equal(0, appCommands.NotifySettingsSavedCount); // does not use the persist/notify path + } + } + + [Fact] + public void ShowChatToolCalls_Persists_AndPushesVisibility() + { + var vm = NewVm(out var settings, out var appCommands, out _, out var temp); + using (temp) + { + vm.Activate(null); + + var target = !vm.ShowChatToolCalls; + vm.ShowChatToolCalls = target; + + Assert.Equal(target, settings.ShowChatToolCalls); + Assert.Equal((bool?)target, appCommands.ToolCallsVisible); + Assert.Equal(1, appCommands.NotifySettingsSavedCount); + } + } + + [Fact] + public void ExternalChange_ReloadsWithoutRePersisting() + { + var vm = NewVm(out var settings, out var appCommands, out _, out var temp); + using (temp) + { + vm.Activate(null); + var target = !vm.NotifyStock; + + // Simulate another surface saving a change. + settings.NotifyStock = target; + settings.Save(); + + Assert.Equal(target, vm.NotifyStock); // VM reflected the external change + Assert.Equal(0, appCommands.NotifySettingsSavedCount); // and did NOT re-persist (no echo/save-storm) + } + } + + [Fact] + public void SelfChange_DoesNotDoublePersist() + { + var vm = NewVm(out _, out var appCommands, out _, out var temp); + using (temp) + { + vm.Activate(null); + + vm.ShowNotifications = !vm.ShowNotifications; + + Assert.Equal(1, appCommands.NotifySettingsSavedCount); // exactly one, not two + } + } + + [Fact] + public void AfterDeactivate_ExternalChangeIsIgnored() + { + var vm = NewVm(out var settings, out _, out _, out var temp); + using (temp) + { + vm.Activate(null); + var before = vm.NotifyStock; + vm.Deactivate(); + + settings.NotifyStock = !before; + settings.Save(); + + Assert.Equal(before, vm.NotifyStock); // unsubscribed, so no reload + } + } +} diff --git a/tests/OpenClaw.Tray.Tests/Presentation/SettingsStoreTests.cs b/tests/OpenClaw.Tray.Tests/Presentation/SettingsStoreTests.cs new file mode 100644 index 000000000..716f5b355 --- /dev/null +++ b/tests/OpenClaw.Tray.Tests/Presentation/SettingsStoreTests.cs @@ -0,0 +1,151 @@ +using OpenClawTray.Presentation; +using OpenClawTray.Services; + +namespace OpenClaw.Tray.Tests.Presentation; + +/// +/// Behavior of the settings facade over the real settings manager: snapshot reads, batched +/// single save, self-originated echo suppression, external-change republishing, and UI-thread +/// affinity for the notification. +/// +public sealed class SettingsStoreTests +{ + private static SettingsStore NewStore(out SettingsManager settings, out RecordingUiDispatcher dispatcher, out TempDir temp) + { + temp = new TempDir(); + settings = new SettingsManager(temp.Path); + dispatcher = new RecordingUiDispatcher(); + return new SettingsStore(settings, dispatcher); + } + + [Fact] + public void Current_ReflectsUnderlyingSettings() + { + var store = NewStore(out var settings, out _, out var temp); + using (temp) + { + settings.GlobalHotkeyEnabled = true; + settings.NotificationSound = "Subtle"; + + var snapshot = store.Current; + + Assert.True(snapshot.GlobalHotkeyEnabled); + Assert.Equal("Subtle", snapshot.NotificationSound); + } + } + + [Fact] + public void Update_MutatesAndPersists_Once() + { + var store = NewStore(out var settings, out _, out var temp); + using (temp) + { + var saves = 0; + settings.Saved += (_, _) => saves++; + + store.Update(e => + { + e.GlobalHotkeyEnabled = true; + e.NotifyHealth = true; + }); + + Assert.True(settings.GlobalHotkeyEnabled); + Assert.True(settings.NotifyHealth); + Assert.Equal(1, saves); // batched: one save for both edits + } + } + + [Fact] + public void Update_DoesNotEchoChangedToSelf() + { + var store = NewStore(out _, out _, out var temp); + using (temp) + { + var changed = 0; + store.Changed += (_, _) => changed++; + + store.Update(e => e.GlobalHotkeyEnabled = true); + + Assert.Equal(0, changed); // self-originated save is suppressed + } + } + + [Fact] + public void ExternalSave_RaisesChanged() + { + var store = NewStore(out var settings, out _, out var temp); + using (temp) + { + var changed = 0; + store.Changed += (_, _) => changed++; + + // A save that did not originate from this store's Update (e.g. another surface). + settings.GlobalHotkeyEnabled = true; + settings.Save(); + + Assert.Equal(1, changed); + } + } + + [Fact] + public void ExternalSave_OffUiThread_IsMarshaledThroughDispatcher() + { + var store = NewStore(out var settings, out var dispatcher, out var temp); + using (temp) + { + dispatcher.HasThreadAccess = false; + dispatcher.RunEnqueuedImmediately = false; + var changed = 0; + store.Changed += (_, _) => changed++; + + settings.Save(); + + Assert.Equal(0, changed); // not raised inline off-thread + Assert.Equal(1, dispatcher.EnqueuedCount); + + dispatcher.FlushPending(); + Assert.Equal(1, changed); // raised once marshaled to the UI thread + } + } + + [Fact] + public void BeginSelfWrite_SuppressesChanged_ForDirectSave() + { + var store = NewStore(out var settings, out _, out var temp); + using (temp) + { + var changed = 0; + store.Changed += (_, _) => changed++; + + using (store.BeginSelfWrite()) + { + settings.GlobalHotkeyEnabled = true; + settings.Save(); + } + + Assert.Equal(0, changed); // save inside a self-write scope is suppressed like Update + + settings.Save(); // outside the scope it is external again + Assert.Equal(1, changed); + } + } + + [Fact] + public void Update_ThrowingEdit_ResetsSelfWriteDepth_SoLaterExternalSaveRepublishes() + { + var store = NewStore(out var settings, out _, out var temp); + using (temp) + { + var changed = 0; + store.Changed += (_, _) => changed++; + + Assert.Throws(() => + store.Update(_ => throw new InvalidOperationException("boom"))); + + // The scope's Dispose ran in the finally path, so the depth is not stuck above zero: + // a subsequent external save still republishes Changed exactly once. + settings.Save(); + Assert.Equal(1, changed); + } + } +} diff --git a/tests/OpenClaw.Tray.Tests/Presentation/UiDispatcherContractTests.cs b/tests/OpenClaw.Tray.Tests/Presentation/UiDispatcherContractTests.cs index 3c4594c45..015137266 100644 --- a/tests/OpenClaw.Tray.Tests/Presentation/UiDispatcherContractTests.cs +++ b/tests/OpenClaw.Tray.Tests/Presentation/UiDispatcherContractTests.cs @@ -30,7 +30,10 @@ public void PageViewModel_ReceivesRegisteredDispatcher() var settingsVm = scope.ServiceProvider.GetRequiredService(); var permissionsVm = scope.ServiceProvider.GetRequiredService(); - Assert.Same(dispatcher, settingsVm.Dispatcher); + // Both page view models resolve from the registered core, so their IUiDispatcher + // dependency is satisfied from DI (not a concrete dispatcher queue). The placeholder + // permissions view model still exposes the injected instance for a strong same-reference check. + Assert.NotNull(settingsVm); Assert.Same(dispatcher, permissionsVm.Dispatcher); }