From a1fe22ac66587969bbe3117d5c1e2f86ac2e63d1 Mon Sep 17 00:00:00 2001 From: Peer Hogeterp <20603780+peerschuett@users.noreply.github.com> Date: Mon, 14 Sep 2026 14:46:42 +0200 Subject: [PATCH] Fix lost policy renames and unguarded settings reads --- .../DocumentAnalysisAssistant.razor.cs | 105 +++++++++++------- .../Components/ProviderSelection.razor | 4 +- .../Components/ProviderSelection.razor.cs | 22 ++++ .../Settings/SettingsManager.cs | 29 ++++- 4 files changed, 115 insertions(+), 45 deletions(-) diff --git a/app/MindWork AI Studio/Assistants/DocumentAnalysis/DocumentAnalysisAssistant.razor.cs b/app/MindWork AI Studio/Assistants/DocumentAnalysis/DocumentAnalysisAssistant.razor.cs index b9adc668..f2a7ad96 100644 --- a/app/MindWork AI Studio/Assistants/DocumentAnalysis/DocumentAnalysisAssistant.razor.cs +++ b/app/MindWork AI Studio/Assistants/DocumentAnalysis/DocumentAnalysisAssistant.razor.cs @@ -180,7 +180,6 @@ public partial class DocumentAnalysisAssistant : AssistantBaseCore + /// Takes the form values over into the given policy. + /// + /// The policy to write the form values to. + /// Whether the protected fields may be written as well. + /// True when this changed anything about the policy, false otherwise. + private bool ApplyFormToPolicy(DataDocumentAnalysisPolicy policy, bool force) + { + // The preselected profile is always user-adjustable, even for protected policies and enterprise configurations: + var hasChanges = policy.PreselectedProfile != this.policyPreselectedProfile; + policy.PreselectedProfile = this.policyPreselectedProfile; + + // Enterprise configurations cannot be modified at all: + if(policy.IsEnterpriseConfiguration) + return false; + + var canEditProtectedFields = force || (!policy.IsProtected && !this.policyIsProtected); + if (!canEditProtectedFields) + return hasChanges; + + hasChanges = hasChanges + || policy.PolicyName != this.policyName + || policy.PreselectedProvider != this.policyPreselectedProviderId + || policy.PolicyDescription != this.policyDescription + || policy.IsProtected != this.policyIsProtected + || policy.HidePolicyDefinition != this.policyHidePolicyDefinition + || policy.AnalysisRules != this.policyAnalysisRules + || policy.OutputRules != this.policyOutputRules + || policy.MinimumProviderConfidence != this.policyMinimumProviderConfidence + || !policy.AllowedToolIds.SetEquals(this.policyAllowedToolIds); + + policy.PreselectedProvider = this.policyPreselectedProviderId; + policy.PolicyName = this.policyName; + policy.PolicyDescription = this.policyDescription; + policy.IsProtected = this.policyIsProtected; + policy.HidePolicyDefinition = this.policyHidePolicyDefinition; + policy.AnalysisRules = this.policyAnalysisRules; + policy.OutputRules = this.policyOutputRules; + policy.MinimumProviderConfidence = this.policyMinimumProviderConfidence; + policy.AllowedToolIds = [..this.policyAllowedToolIds]; + return hasChanges; } private DataDocumentAnalysisPolicy? selectedPolicy; @@ -316,7 +331,17 @@ public partial class DocumentAnalysisAssistant : AssistantBaseCore private bool documentSelectionExpanded; private string policyName = string.Empty; - private bool policyNameWasEdited; + + /// + /// Whether a typed policy name still waits to be written to the settings file. + /// + /// + /// Typing a name applies it to its policy at once, so that the policy list shows the new name + /// right away -- which leaves nothing for the auto-save to compare the form against. This flag + /// is what tells it that a store is nevertheless due. It belongs to no particular policy: the + /// name has long arrived where it belongs, only the file has not caught up yet. + /// + private bool policyNameStorePending; private string policyDescription = string.Empty; private string policyAnalysisRules = string.Empty; private string policyOutputRules = string.Empty; @@ -496,7 +521,7 @@ public partial class DocumentAnalysisAssistant : AssistantBaseCore } -@if (availableProviderItems.Count is 0) +@if (availableProviderItems.Count is 0 && this.GetEmptySelectionHint() is { } emptySelectionHint) { - @T("No LLM providers meet the confidence requirements. Configure an eligible provider in the app settings.") + @emptySelectionHint } diff --git a/app/MindWork AI Studio/Components/ProviderSelection.razor.cs b/app/MindWork AI Studio/Components/ProviderSelection.razor.cs index 0847111c..b40175f6 100644 --- a/app/MindWork AI Studio/Components/ProviderSelection.razor.cs +++ b/app/MindWork AI Studio/Components/ProviderSelection.razor.cs @@ -53,6 +53,28 @@ public partial class ProviderSelection : MSGComponentBase yield return new(provider, this.GetCapabilityIcons(provider)); } + /// + /// Says why there is nothing to choose from, or nothing at all when that is not the user's doing. + /// + /// + /// An empty list has two causes the user can act on, and they lead to different places in the + /// settings: there is no provider yet, or none of the configured ones reaches the confidence + /// this component asks for. Naming the wrong one sends the user looking in the wrong place -- + /// a first start has nobody to blame for a confidence level it never set. A missing or invalid + /// component is a third case and neither of those: it is a defect, it was logged as one, and + /// any explanation offered to the user here would be a guess. + /// + private string? GetEmptySelectionHint() + { + if (this.Component is null or Tools.Components.NONE) + return null; + + if (!this.SettingsManager.GetAllProviders().Any(x => x.UsedLLMProvider is not LLMProviders.NONE)) + return this.T("No LLM providers are configured yet. Add a provider in the app settings."); + + return this.T("No LLM providers meet the confidence requirements. Configure an eligible provider in the app settings."); + } + private IReadOnlyList GetCapabilityIcons(AIStudio.Settings.Provider provider) { var profile = provider.GetModelProfile(); diff --git a/app/MindWork AI Studio/Settings/SettingsManager.cs b/app/MindWork AI Studio/Settings/SettingsManager.cs index aeb6f31b..7dfceb31 100644 --- a/app/MindWork AI Studio/Settings/SettingsManager.cs +++ b/app/MindWork AI Studio/Settings/SettingsManager.cs @@ -33,7 +33,17 @@ public sealed class SettingsManager private readonly ILogger logger; private readonly RustService rustService; - private readonly SemaphoreSlim settingsWriteSemaphore = new(1, 1); + + /// + /// Lets only one operation at a time touch the settings files. + /// + /// + /// Reading takes this as well as writing does, for two reasons. A read migrates and backs up + /// what it found, so it writes the very files a store writes. And it re-evaluates whether + /// writes are blocked at all, starting out by clearing that block: a store slipping through + /// that moment would overwrite the settings the block exists to protect. + /// + private readonly SemaphoreSlim settingsFileSemaphore = new(1, 1); /// /// The settings manager. @@ -104,6 +114,19 @@ public sealed class SettingsManager /// /// A (migrated) settings snapshot, or null if it could not be read. public async Task TryReadSettingsSnapshot() + { + await this.settingsFileSemaphore.WaitAsync(); + try + { + return await this.ReadSettingsSnapshot(); + } + finally + { + this.settingsFileSemaphore.Release(); + } + } + + private async Task ReadSettingsSnapshot() { this.SettingsWriteBlockReason = SettingsWriteBlockReason.NONE; if(!this.IsSetUp) @@ -295,7 +318,7 @@ public sealed class SettingsManager /// public async Task StoreSettings() { - await this.settingsWriteSemaphore.WaitAsync(); + await this.settingsFileSemaphore.WaitAsync(); try { if(!this.IsSetUp) @@ -317,7 +340,7 @@ public sealed class SettingsManager } finally { - this.settingsWriteSemaphore.Release(); + this.settingsFileSemaphore.Release(); } }