Fixed settings keeping their plugin value after a configuration was withdrawn (#903)

app/MindWork AI Studio/Settings/ConfigMeta.cs

@@ -1,4 +1,5 @@
 using System.Linq.Expressions;
+using System.Text.Json;
 
 using AIStudio.Settings.DataModel;
 
@@ -57,6 +58,31 @@ public record ConfigMeta<TClass, TValue> : ConfigMetaBase
     /// <inheritdoc/>
     public override bool RemovePluginContribution(Guid configPluginId) => this.pluginContributions.Remove(configPluginId);
 
+    /// <inheritdoc/>
+    public override string SerializeCurrentValue() => ManagedConfiguration.SerializeManagedScalarValue(this.GetValue());
+
+    /// <inheritdoc/>
+    protected override string SerializeCurrentValueAsJson() => JsonSerializer.Serialize(this.GetValue(), SettingsManager.JSON_OPTIONS);
+
+    /// <inheritdoc/>
+    protected override bool TrySetValueFromJson(string json)
+    {
+        try
+        {
+            var value = JsonSerializer.Deserialize<TValue>(json, SettingsManager.JSON_OPTIONS);
+            if (value is null)
+                return false;
+
+            this.SetValue(value);
+            return true;
+        }
+        catch (Exception e)
+        {
+            Log.LogWarning(e, $"Was not able to restore the value of the setting '{this.SettingName}' from its snapshot '{json}'. Using the default value instead.");
+            return false;
+        }
+    }
+
     /// <inheritdoc/>
     protected override void Reset()
     {

app/MindWork AI Studio/Settings/ConfigMetaBase.cs

@@ -12,6 +12,8 @@ public abstract record ConfigMetaBase(string SettingName) : IConfig
 {
     protected static SettingsManager SettingsManagerAccess => Program.SERVICE_PROVIDER.GetRequiredService<SettingsManager>();
 
+    protected static ILogger Log => Program.LOGGER_FACTORY.CreateLogger(nameof(ConfigMetaBase));
+
     /// <summary>
     /// The persisted name of the configuration setting.
     /// </summary>
@@ -89,14 +91,14 @@ public abstract record ConfigMetaBase(string SettingName) : IConfig
     public void ResetLockedConfiguration()
     {
         SettingsManagerAccess.ConfigurationData.ManagedLockedConfigurations.Remove(this.SettingName);
-        
+
         this.IsLocked = false;
         this.LockedByConfigPluginId = Guid.Empty;
-        
+
         if (this.ManagedMode is ManagedConfigurationMode.LOCKED)
             this.ManagedMode = null;
 
-        this.Reset();
+        this.RestoreUserValueOrDefault();
     }
 
     /// <summary>
@@ -137,6 +139,29 @@ public abstract record ConfigMetaBase(string SettingName) : IConfig
         this.EditableDefaultByConfigPluginId = Guid.Empty;
     }
 
+    /// <summary>
+    /// Clears the editable-default state and hands the setting back to the user.
+    /// </summary>
+    /// <remarks>
+    /// Without a snapshot of the user's value, the current value stays as it is. That is the
+    /// difference to a locked setting: the user was allowed to change an editable default all
+    /// along, so its value is a plausible choice of theirs. Resetting it to the app's default would
+    /// take away something nobody asked us to remove.
+    /// </remarks>
+    /// <param name="keepCurrentValue">
+    /// True when the user has changed the value in the meantime. Their decision outlives the
+    /// configuration plugin, so the snapshot is dropped instead of applied.
+    /// </param>
+    public void ResetEditableDefaultConfiguration(bool keepCurrentValue)
+    {
+        this.ClearEditableDefaultConfiguration();
+
+        if (keepCurrentValue)
+            this.ClearUserValueSnapshot();
+        else
+            this.TryRestoreUserValueSnapshot();
+    }
+
     /// <summary>
     /// Removes the contribution of one configuration plugin without changing the current value.
     /// </summary>
@@ -144,6 +169,96 @@ public abstract record ConfigMetaBase(string SettingName) : IConfig
     /// <returns>True when that plugin had a contribution, otherwise false.</returns>
     public abstract bool RemovePluginContribution(Guid configPluginId);
 
+    /// <summary>
+    /// Indicates whether the value the user had chosen before a configuration plugin took over
+    /// this setting is still available.
+    /// </summary>
+    public bool HasUserValueSnapshot => SettingsManagerAccess.ConfigurationData.ManagedUserValueSnapshots.ContainsKey(this.SettingName);
+
+    /// <summary>
+    /// Remembers the current value as the user's value, so that it can be restored once no
+    /// configuration plugin manages this setting anymore.
+    /// </summary>
+    /// <remarks>
+    /// Only an unmanaged setting holds a value which belongs to the user. When one configuration
+    /// plugin takes a setting over from another, the current value belongs to the previous plugin,
+    /// so the snapshot of the user's value must survive that handover untouched.<br/><br/>
+    /// The persisted editable default counts as managed as well: unlike a locked setting, it is not
+    /// restored into the in-memory state when the settings are loaded, so right after a start it is
+    /// the only evidence that a configuration plugin is already in charge.
+    /// </remarks>
+    public void CaptureUserValueSnapshot()
+    {
+        if (this.ManagedMode is not null || SettingsManagerAccess.ConfigurationData.ManagedEditableDefaults.ContainsKey(this.SettingName))
+            return;
+
+        var snapshots = SettingsManagerAccess.ConfigurationData.ManagedUserValueSnapshots;
+        if (snapshots.ContainsKey(this.SettingName))
+            return;
+
+        snapshots[this.SettingName] = this.SerializeCurrentValueAsJson();
+    }
+
+    /// <summary>
+    /// Restores the value the user had chosen before a configuration plugin took over this setting.
+    /// </summary>
+    /// <remarks>
+    /// The snapshot is consumed either way: when it cannot be applied, keeping it would mean trying
+    /// the same broken value again on every start.
+    /// </remarks>
+    /// <returns>True when a snapshot was available and could be applied, otherwise false.</returns>
+    private bool TryRestoreUserValueSnapshot()
+    {
+        var snapshots = SettingsManagerAccess.ConfigurationData.ManagedUserValueSnapshots;
+        if (!snapshots.Remove(this.SettingName, out var snapshot))
+            return false;
+
+        return this.TrySetValueFromJson(snapshot);
+    }
+
+    /// <summary>
+    /// Drops the snapshot of the user's value without changing the current value.
+    /// </summary>
+    /// <returns>True when a snapshot was dropped, otherwise false.</returns>
+    public bool ClearUserValueSnapshot() => SettingsManagerAccess.ConfigurationData.ManagedUserValueSnapshots.Remove(this.SettingName);
+
+    /// <summary>
+    /// Serializes the current value the same way the managed states record it.
+    /// </summary>
+    /// <remarks>
+    /// This is meant for comparisons, e.g. to tell whether the user has changed an editable default
+    /// in the meantime. It is not meant for restoring a value: the representation is lossy.
+    /// </remarks>
+    public abstract string SerializeCurrentValue();
+
+    /// <summary>
+    /// Restores the user's value, or falls back to the default value when no snapshot is available.
+    /// </summary>
+    /// <remarks>
+    /// Settings which a configuration plugin managed before this app version has no snapshot, and
+    /// neither has a setting whose value the user never changed. The default value is the best
+    /// answer in both cases.
+    /// </remarks>
+    private void RestoreUserValueOrDefault()
+    {
+        if (this.TryRestoreUserValueSnapshot())
+            return;
+
+        this.Reset();
+    }
+
+    /// <summary>
+    /// Serializes the current value as JSON, so that it can be restored without losing information.
+    /// </summary>
+    protected abstract string SerializeCurrentValueAsJson();
+
+    /// <summary>
+    /// Applies a value which was serialized by SerializeCurrentValueAsJson.
+    /// </summary>
+    /// <param name="json">The serialized value.</param>
+    /// <returns>True when the value could be applied, otherwise false.</returns>
+    protected abstract bool TrySetValueFromJson(string json);
+
     /// <summary>
     /// Resets the configuration property to its default value.
     /// </summary>

app/MindWork AI Studio/Settings/DataModel/Data.cs

@@ -68,6 +68,18 @@ public sealed class Data
     /// </summary>
     public Dictionary<string, Guid> ManagedLockedConfigurations { get; set; } = [];
 
+    /// <summary>
+    /// The value each managed setting had before a configuration plugin took it over, as JSON.
+    /// </summary>
+    /// <remarks>
+    /// A configuration plugin might be removed later, e.g. when a test configuration ends or when an
+    /// organization withdraws its configuration. The value the user had chosen before belongs to the
+    /// user, so we keep it here and restore it instead of falling back to the app's default value.
+    /// The snapshot is taken once, when a setting becomes managed, and is consumed when no
+    /// configuration plugin manages that setting anymore.
+    /// </remarks>
+    public Dictionary<string, string> ManagedUserValueSnapshots { get; set; } = [];
+
     /// <summary>
     /// Cached audit results for assistant plugins.
     /// </summary>

app/MindWork AI Studio/Settings/ManagedConfiguration.Parsing.cs

@@ -913,6 +913,13 @@ public static partial class ManagedConfiguration
         if (!MayManageSetting(configPluginId, configMeta))
             return false;
 
+        //
+        // Remember the value the user had chosen before any configuration plugin took this setting
+        // over. Once no plugin manages it anymore, we hand that value back to the user:
+        //
+        if (successful)
+            configMeta.CaptureUserValueSnapshot();
+
         switch (successful)
         {
             case true:
@@ -967,6 +974,15 @@ public static partial class ManagedConfiguration
         if (!MayManageSetting(configPluginId, configMeta))
             return false;
 
+        //
+        // Remember the value the user had chosen before any configuration plugin took this setting
+        // over. Once no plugin manages it anymore, we hand that value back to the user. This has to
+        // happen before the managed state below changes, because only an unmanaged setting holds a
+        // value which belongs to the user:
+        //
+        if (successful)
+            configMeta.CaptureUserValueSnapshot();
+
         switch (successful)
         {
             case true when managedMode is ManagedConfigurationMode.LOCKED:
@@ -1008,7 +1024,7 @@ public static partial class ManagedConfiguration
             case false when configMeta.ManagedMode is ManagedConfigurationMode.EDITABLE_DEFAULT
                             && TryGetEditableDefaultState(settingName, out var editableDefaultStateToRemove)
                             && editableDefaultStateToRemove.ConfigPluginId == configPluginId:
-                configMeta.ClearEditableDefaultConfiguration();
+                configMeta.ResetEditableDefaultConfiguration(HasUserChangedEditableDefault(configMeta, editableDefaultStateToRemove));
                 ClearEditableDefaultState(settingName);
                 break;
         }
@@ -1033,7 +1049,7 @@ public static partial class ManagedConfiguration
         return ManagedConfigurationMode.LOCKED;
     }
 
-    private static string SerializeManagedScalarValue<TValue>(TValue value) => value switch
+    internal static string SerializeManagedScalarValue<TValue>(TValue value) => value switch
     {
         null => string.Empty,
         string text => text,

app/MindWork AI Studio/Settings/ManagedConfiguration.cs

@@ -361,6 +361,19 @@ public static partial class ManagedConfiguration
                 configMeta.RemovePluginContribution(contributingConfigPluginId);
                 wasChanged = true;
             }
+
+            //
+            // Finally, drop any snapshot of the user's value which nobody claims anymore. Without
+            // this, a setting which stopped being managed outside of the paths above would keep its
+            // snapshot in the settings file forever. The persisted editable default counts as a
+            // claim as well: it survives a configuration plugin which is deployed but could not be
+            // loaded, and that plugin is still in charge:
+            //
+            if (configMeta.ManagedMode is null && !TryGetEditableDefaultState(configMeta.SettingName, out _) && configMeta.ClearUserValueSnapshot())
+            {
+                Log.LogInformation($"Dropping the snapshot of the user's value for the setting '{configMeta.SettingName}': no configuration plugin manages it anymore.");
+                wasChanged = true;
+            }
         }
 
         // Remove persisted states which belong to settings that do not exist anymore:
@@ -405,6 +418,13 @@ public static partial class ManagedConfiguration
             wasChanged = true;
         }
 
+        foreach (var settingName in configurationData.ManagedUserValueSnapshots.Keys.Where(x => !registeredSettingNames.Contains(x)).ToList())
+        {
+            Log.LogInformation($"Removing the snapshot of the user's value for the setting '{settingName}': this setting does not exist anymore.");
+            configurationData.ManagedUserValueSnapshots.Remove(settingName);
+            wasChanged = true;
+        }
+
         return wasChanged;
     }
 
@@ -447,7 +467,7 @@ public static partial class ManagedConfiguration
             if (configMeta.ManagedMode is not ManagedConfigurationMode.EDITABLE_DEFAULT)
                 return false;
 
-            configMeta.ClearEditableDefaultConfiguration();
+            configMeta.ResetEditableDefaultConfiguration(keepCurrentValue: false);
             return true;
         }
 
@@ -455,7 +475,17 @@ public static partial class ManagedConfiguration
             return false;
 
         Log.LogInformation($"Clearing the editable default of the setting '{configMeta.SettingName}': the configuration plugin '{editableDefaultState.ConfigPluginId}' is not available anymore.");
-        configMeta.ClearEditableDefaultConfiguration();
+        configMeta.ResetEditableDefaultConfiguration(HasUserChangedEditableDefault(configMeta, editableDefaultState));
         return ClearEditableDefaultState(configMeta.SettingName);
     }
+
+    /// <summary>
+    /// Checks whether the user has changed an editable default themselves.
+    /// </summary>
+    /// <remarks>
+    /// The user may change an editable default at any time. When the current value is not the one
+    /// the configuration plugin applied last, the user decided against that value, and their
+    /// decision outlives the plugin.
+    /// </remarks>
+    private static bool HasUserChangedEditableDefault(ConfigMetaBase configMeta, ManagedEditableDefaultState editableDefaultState) => !string.Equals(configMeta.SerializeCurrentValue(), editableDefaultState.LastAppliedValue, StringComparison.Ordinal);
 }

app/MindWork AI Studio/Settings/SettingsManager.cs

@@ -23,7 +23,7 @@ public sealed class SettingsManager
     
     private readonly record struct CurrentSettingsReadResult(Data? SettingsData, SettingsWriteBlockReason FailureReason);
     
-    private static readonly JsonSerializerOptions JSON_OPTIONS = new()
+    internal static readonly JsonSerializerOptions JSON_OPTIONS = new()
     {
         WriteIndented = true,
         Converters = { new TolerantEnumConverter() },