Fixed plugin installation and endless plugin reloading on Linux (#924)

app/MindWork AI Studio/Tools/PluginSystem/Assistants/PluginAssistants.cs

@@ -301,12 +301,40 @@ public sealed class PluginAssistants(bool isInternal, LuaState state, PluginType
         return fileMap.ToImmutable();
     }
 
+    /// <summary>
+    /// The audit hash of this plugin, together with the directory it was computed for.
+    /// </summary>
+    /// <remarks>
+    /// One record instead of two fields, so that a reader always sees a directory and a hash which
+    /// belong together. Recomputing the same hash twice costs nothing but time, mixing up a hash
+    /// with the wrong directory would show a wrong security state.
+    /// </remarks>
+    private sealed record AuditHashCache(string PluginPath, string Hash);
+
+    private AuditHashCache? auditHashCache;
+
     /// <summary>
     /// Computes a stable audit hash across all Lua files by hashing a canonical
     /// sequence of relative path length, relative path, content length, and content
     /// for each file in ordinal path order.
     /// </summary>
-    public string ComputeAuditHash() => AssistantPluginHash.Compute(this.PluginPath);
+    /// <remarks>
+    /// The result is kept, because computing it reads every Lua file of the plugin, and the plugins
+    /// page as well as the assistants page ask for it on every render. That is safe: the files of
+    /// one plugin instance never change. Whenever something in the plugins directory changes, the
+    /// plugin factory reloads and creates new instances, cf. PluginFactory.Starting.RestartAllPlugins.
+    /// The plugin directory is assigned after the instance was created, so the cache remembers which
+    /// directory it belongs to.
+    /// </remarks>
+    public string ComputeAuditHash()
+    {
+        if (this.auditHashCache is { } cache && string.Equals(cache.PluginPath, this.PluginPath, StringComparison.Ordinal))
+            return cache.Hash;
+
+        var hash = AssistantPluginHash.Compute(this.PluginPath);
+        this.auditHashCache = new(this.PluginPath, hash);
+        return hash;
+    }
 
     private static string BuildSecureSystemPrompt(string pluginSystemPrompt)
     {

app/MindWork AI Studio/Tools/PluginSystem/PluginFactory.HotReload.cs

@@ -1,9 +1,26 @@
+using Timer = System.Timers.Timer;
+
 namespace AIStudio.Tools.PluginSystem;
 
 public static partial class PluginFactory
 {
     private static readonly SemaphoreSlim HOT_RELOAD_SEMAPHORE = new(1, 1);
-    
+
+    /// <summary>
+    /// How long the plugins directory has to stay quiet before we reload.
+    /// </summary>
+    /// <remarks>
+    /// One change never arrives as one event: writing a single file produces several, and moving an
+    /// entire plugin directory into place produces dozens. Reloading on each of them would restart
+    /// every plugin over and over.
+    /// </remarks>
+    private static readonly TimeSpan HOT_RELOAD_DEBOUNCE_INTERVAL = TimeSpan.FromSeconds(1);
+
+    private static readonly Timer HOT_RELOAD_DEBOUNCE_TIMER = new(HOT_RELOAD_DEBOUNCE_INTERVAL)
+    {
+        AutoReset = false,
+    };
+
     public static void SetUpHotReloading()
     {
         if (!IsInitialized)
@@ -11,18 +28,26 @@ public static partial class PluginFactory
             LOG.LogError("PluginFactory is not initialized. Please call Setup() before using it.");
             return;
         }
-        
+
         LOG.LogInformation($"Start hot reloading plugins for path '{HOT_RELOAD_WATCHER.Path}'.");
         try
         {
+            HOT_RELOAD_DEBOUNCE_TIMER.Elapsed += (_, _) => _ = ReloadPluginsAsync();
+
             HOT_RELOAD_WATCHER.IncludeSubdirectories = true;
-            HOT_RELOAD_WATCHER.NotifyFilter = NotifyFilters.CreationTime
-                                              | NotifyFilters.DirectoryName
+
+            //
+            // We watch for plugins appearing, disappearing, and changing. We do not watch access
+            // times: reading a plugin is not a change, and on Linux our own reads would be
+            // reported back to us. Loading the plugins and computing the audit hash of an
+            // assistant plugin both read every Lua file in this directory, so such a filter
+            // makes each reload cause the next one:
+            //
+            HOT_RELOAD_WATCHER.NotifyFilter = NotifyFilters.DirectoryName
                                               | NotifyFilters.FileName
-                                              | NotifyFilters.LastAccess
                                               | NotifyFilters.LastWrite
                                               | NotifyFilters.Size;
-            
+
             HOT_RELOAD_WATCHER.Changed += HotReloadEventHandler;
             HOT_RELOAD_WATCHER.Deleted += HotReloadEventHandler;
             HOT_RELOAD_WATCHER.Created += HotReloadEventHandler;
@@ -42,64 +67,96 @@ public static partial class PluginFactory
             LOG.LogInformation("Hot reloading plugins set up.");
         }
     }
-    
-    private static async void HotReloadEventHandler(object _, FileSystemEventArgs args)
+
+    private static void HotReloadEventHandler(object _, FileSystemEventArgs args)
     {
         try
         {
-            var changeType = args.ChangeType.ToString().ToLowerInvariant();
-            if (!await HOT_RELOAD_SEMAPHORE.WaitAsync(0))
-            {
-                LOG.LogInformation($"File changed '{args.FullPath}' (event={changeType}). Already processing another change.");
+            //
+            // Our own lock file lives in the watched directory. Writing and removing it are not
+            // plugin changes, and reacting to them would turn every locked operation into a
+            // reload of its own:
+            //
+            if (IsHotReloadLockFile(args.FullPath))
                 return;
-            }
 
-            try
-            {
-                LOG.LogInformation($"File changed '{args.FullPath}' (event={changeType}). Reloading plugins...");
-                if (File.Exists(HOT_RELOAD_LOCK_FILE))
-                {
-                    LOG.LogInformation("Hot reload lock file exists. Waiting for it to be released before proceeding with the reload.");
+            var changeType = args.ChangeType.ToString().ToLowerInvariant();
+            LOG.LogInformation($"File changed '{args.FullPath}' (event={changeType}). Scheduling a plugin reload.");
 
-                    var lockFileCancellationTokenSource = new CancellationTokenSource(TimeSpan.FromSeconds(30));
-                    var token = lockFileCancellationTokenSource.Token;
-                    var waitTime = TimeSpan.FromSeconds(1);
-                    while (File.Exists(HOT_RELOAD_LOCK_FILE) && !token.IsCancellationRequested)
-                    {
-                        try
-                        {
-                            LOG.LogDebug("Waiting for hot reload lock to be released...");
-                            await Task.Delay(waitTime, token);
-                            waitTime = TimeSpan.FromSeconds(Math.Min(waitTime.TotalSeconds * 2, 120)); // Exponential backoff with a cap
-                        }
-                        catch (TaskCanceledException)
-                        {
-                            // Case: The cancellation token was triggered, meaning the lock file is still present.
-                            // We expect that something goes wrong. So, we try to delete the lock file:
-                            LOG.LogWarning("Hot reload lock file still exists after 30 seconds. Attempting to delete it...");
-                            UnlockHotReload();
-                            break;
-                        }
-                    }
-                    
-                    LOG.LogInformation("Hot reload lock file released. Proceeding with plugin reload.");
-                }
-
-                await LoadAll();
-                await MessageBus.INSTANCE.SendMessage<bool>(null, Event.PLUGINS_RELOADED);
-            }
-            catch(Exception e)
-            {
-                LOG.LogError(e, $"Error while reloading plugins after change in file '{args.FullPath}' with change type '{changeType}'.");
-            }
-            finally
-            {
-                HOT_RELOAD_SEMAPHORE.Release();
-            }
+            // Restart the debounce window, so that a burst of events results in one reload:
+            HOT_RELOAD_DEBOUNCE_TIMER.Stop();
+            HOT_RELOAD_DEBOUNCE_TIMER.Start();
         }
         catch (Exception e)
         {
             LOG.LogError(e, $"Error while handling hot reload event for file '{args.FullPath}' with change type '{args.ChangeType}'.");
         }
     }
+
+    private static bool IsHotReloadLockFile(string path)
+    {
+        if (string.IsNullOrWhiteSpace(path) || string.IsNullOrWhiteSpace(HOT_RELOAD_LOCK_FILE))
+            return false;
+
+        return string.Equals(path, HOT_RELOAD_LOCK_FILE, StringComparison.OrdinalIgnoreCase);
+    }
+
+    private static async Task ReloadPluginsAsync()
+    {
+        //
+        // Reloads must never overlap. When one is still running, we do not drop this one: the
+        // changes which triggered it might have arrived after the running reload had already read
+        // them. We try again after another quiet window instead:
+        //
+        if (!await HOT_RELOAD_SEMAPHORE.WaitAsync(0))
+        {
+            LOG.LogInformation("A plugin reload is already running. Waiting for it to finish before reloading again.");
+            HOT_RELOAD_DEBOUNCE_TIMER.Stop();
+            HOT_RELOAD_DEBOUNCE_TIMER.Start();
+            return;
+        }
+
+        try
+        {
+            LOG.LogInformation("Reloading plugins...");
+            if (File.Exists(HOT_RELOAD_LOCK_FILE))
+            {
+                LOG.LogInformation("Hot reload lock file exists. Waiting for it to be released before proceeding with the reload.");
+
+                var lockFileCancellationTokenSource = new CancellationTokenSource(TimeSpan.FromSeconds(30));
+                var token = lockFileCancellationTokenSource.Token;
+                var waitTime = TimeSpan.FromSeconds(1);
+                while (File.Exists(HOT_RELOAD_LOCK_FILE) && !token.IsCancellationRequested)
+                {
+                    try
+                    {
+                        LOG.LogDebug("Waiting for hot reload lock to be released...");
+                        await Task.Delay(waitTime, token);
+                        waitTime = TimeSpan.FromSeconds(Math.Min(waitTime.TotalSeconds * 2, 120)); // Exponential backoff with a cap
+                    }
+                    catch (TaskCanceledException)
+                    {
+                        // Case: The cancellation token was triggered, meaning the lock file is still present.
+                        // We expect that something goes wrong. So, we try to delete the lock file:
+                        LOG.LogWarning("Hot reload lock file still exists after 30 seconds. Attempting to delete it...");
+                        UnlockHotReload();
+                        break;
+                    }
+                }
+
+                LOG.LogInformation("Hot reload lock file released. Proceeding with plugin reload.");
+            }
+
+            // LoadAll announces the reload itself, cf. PluginFactory.Starting.RestartAllPlugins:
+            await LoadAll();
+        }
+        catch(Exception e)
+        {
+            LOG.LogError(e, "Error while reloading plugins after a change in the plugins directory.");
+        }
+        finally
+        {
+            HOT_RELOAD_SEMAPHORE.Release();
+        }
+    }
 }

app/MindWork AI Studio/Tools/PluginSystem/PluginFactory.cs

@@ -290,7 +290,25 @@ public static partial class PluginFactory
         return AVAILABLE_PLUGINS.Any(plugin => plugin.Id == configPluginId && plugin.Type is PluginType.CONFIGURATION && IsEnterpriseTestConfigurationPath(plugin.LocalPath));
     }
 
-    private static async Task LockHotReloadAsync()
+    /// <summary>
+    /// Counts how many operations currently write to the plugins directory.
+    /// </summary>
+    /// <remarks>
+    /// Downloading an organization's configuration and installing a plugin can run at the same
+    /// time. Without counting, whichever finishes first would unlock hot reloading while the other
+    /// is still writing.
+    /// </remarks>
+    private static int HOT_RELOAD_LOCK_COUNT;
+    private static readonly SemaphoreSlim HOT_RELOAD_LOCK_SEMAPHORE = new(1, 1);
+
+    /// <summary>
+    /// Holds back hot reloading while the caller writes to the plugins directory.
+    /// </summary>
+    /// <remarks>
+    /// Every caller has to release the lock again, so wrap the write in a try-finally block. Hot
+    /// reloading resumes once the last caller has released it.
+    /// </remarks>
+    public static async Task LockHotReloadAsync()
     {
         if (!IsInitialized)
         {
@@ -298,23 +316,28 @@ public static partial class PluginFactory
             return;
         }
 
+        await HOT_RELOAD_LOCK_SEMAPHORE.WaitAsync();
         try
         {
-            if (File.Exists(HOT_RELOAD_LOCK_FILE))
-            {
-                LOG.LogWarning("Hot reload lock file already exists.");
+            if (HOT_RELOAD_LOCK_COUNT++ > 0)
                 return;
-            }
-            
+
             await File.WriteAllTextAsync(HOT_RELOAD_LOCK_FILE, DateTime.UtcNow.ToString("o"));
         }
         catch (Exception e)
         {
             LOG.LogError(e, "An error occurred while trying to lock hot reloading.");
         }
+        finally
+        {
+            HOT_RELOAD_LOCK_SEMAPHORE.Release();
+        }
     }
 
-    private static void UnlockHotReload()
+    /// <summary>
+    /// Releases the hot reload lock of one caller, see LockHotReloadAsync.
+    /// </summary>
+    public static void UnlockHotReload()
     {
         if (!IsInitialized)
         {
@@ -322,8 +345,20 @@ public static partial class PluginFactory
             return;
         }
 
+        HOT_RELOAD_LOCK_SEMAPHORE.Wait();
         try
         {
+            //
+            // The count can be zero when the reload gave up waiting and removed the lock file
+            // itself. We must not go negative, because that would keep the next lock from ever
+            // writing the file again:
+            //
+            if (HOT_RELOAD_LOCK_COUNT > 0)
+                HOT_RELOAD_LOCK_COUNT--;
+
+            if (HOT_RELOAD_LOCK_COUNT > 0)
+                return;
+
             if(File.Exists(HOT_RELOAD_LOCK_FILE))
                 File.Delete(HOT_RELOAD_LOCK_FILE);
             else
@@ -333,14 +368,19 @@ public static partial class PluginFactory
         {
             LOG.LogError(e, "An error occurred while trying to unlock hot reloading.");
         }
+        finally
+        {
+            HOT_RELOAD_LOCK_SEMAPHORE.Release();
+        }
     }
     
     public static void Dispose()
     {
         if(!IsInitialized)
             return;
-        
+
         HOT_RELOAD_WATCHER.Dispose();
+        HOT_RELOAD_DEBOUNCE_TIMER.Dispose();
     }
 
     public static IReadOnlyList<DataMandatoryInfo> GetMandatoryInfos()