Fixed Semantic Search with external data sources (ERI servers) (#1090)

Co-authored-by: Thorsten Sommer <SommerEngineering@users.noreply.github.com>

app/MindWork AI Studio/Tools/Services/DataSourceService.cs

@@ -18,6 +18,7 @@ public sealed class DataSourceService
     // ReSharper disable once NotAccessedPositionalProperty.Local
     private readonly record struct ParticipatingProvider(string Role, bool IsTrusted, ConfidenceLevel ConfidenceLevel);
 
+    private readonly ERISecurityCheckCache securityChecks;
     private readonly DataSourceEmbeddingService embeddingService;
     private readonly RustService rustService;
     private readonly SettingsManager settingsManager;
@@ -29,6 +30,7 @@ public sealed class DataSourceService
         this.rustService = rustService;
         this.settingsManager = settingsManager;
         this.embeddingService = embeddingService;
+        this.securityChecks = new(this.FetchSecurityRequirementsAsync, TimeProvider.System);
 
         this.logger.LogInformation("The data source service has been initialized.");
     }
@@ -52,7 +54,7 @@ public sealed class DataSourceService
         if (selectedLLMProvider == Settings.Provider.NONE)
         {
             this.logger.LogWarning("The selected LLM provider is not set. We cannot filter the data sources by any means.");
-            return new([], [], [], []);
+            return new([], [], [], [], []);
         }
         
         var usingTrustedProvider = selectedLLMProvider.IsTrustedForDataSourceSecurityChecks(this.settingsManager);
@@ -82,7 +84,7 @@ public sealed class DataSourceService
         var usingTrustedProvider = selectedLLMProvider.IsTrustedForDataSourceSecurityChecks(this.settingsManager);
         var participatingProviders = this.GetParticipatingProviders(selectedLLMProvider.Id, dataSourceOptions, retrievalMode,
             new("chat provider", usingTrustedProvider, selectedLLMProvider.GetConfidenceLevel(this.settingsManager)));
-        var allowedDataSources = await this.GetAllowedDataSources(usingTrustedProvider, participatingProviders, requestedDataSources);
+        var allowedDataSources = (await this.GetAllowedDataSources(usingTrustedProvider, participatingProviders, requestedDataSources)).Allowed;
 
         //
         // Whoever asks this way has no list to show, so a data source which cannot be searched is
@@ -113,7 +115,7 @@ public sealed class DataSourceService
         if (selectedLLMProvider is NoProvider)
         {
             this.logger.LogWarning("The selected LLM provider is the default provider. We cannot filter the data sources by any means.");
-            return new([], [], [], []);
+            return new([], [], [], [], []);
         }
         
         var usingTrustedProvider = selectedLLMProvider.IsTrustedForDataSourceSecurityChecks(this.settingsManager);
@@ -180,7 +182,8 @@ public sealed class DataSourceService
     {
         var allDataSources = this.settingsManager.ConfigurationData.DataSources.ToList();
         var previousSelectedDataSourceIds = previousSelectedDataSources?.Select(source => source.Id).ToHashSet(StringComparer.Ordinal) ?? [];
-        var filteredDataSources = await this.GetAllowedDataSources(usingTrustedProvider, participatingProviders, allDataSources);
+        var checkedDataSources = await this.GetAllowedDataSources(usingTrustedProvider, participatingProviders, allDataSources);
+        var filteredDataSources = checkedDataSources.Allowed;
 
         //
         // Which of the sources that passed every check cannot answer a search right now. They are
@@ -202,7 +205,7 @@ public sealed class DataSourceService
         var usableDataSources = filteredDataSources.Where(source => !blockedIds.Contains(source.Id)).ToList();
         var filteredSelectedDataSources = usableDataSources.Where(source => previousSelectedDataSourceIds.Contains(source.Id)).ToList();
 
-        return new(usableDataSources, filteredSelectedDataSources, awaitingReindex, needingRepair);
+        return new(usableDataSources, filteredSelectedDataSources, awaitingReindex, needingRepair, checkedDataSources.Unreachable);
     }
 
     /// <summary>
@@ -257,10 +260,16 @@ public sealed class DataSourceService
         return needingRepair;
     }
 
-    private async Task<IReadOnlyList<IDataSource>> GetAllowedDataSources(bool usingTrustedProvider, IReadOnlyList<ParticipatingProvider> participatingProviders, IReadOnlyCollection<IDataSource> requestedDataSources)
+    /// <summary>
+    /// What the checks of several data sources found: those which may be used, and the ERI data sources whose server could not be reached.
+    /// </summary>
+    private readonly record struct CheckedDataSources(IReadOnlyList<IDataSource> Allowed, IReadOnlyList<IDataSource> Unreachable);
+
+    private async Task<CheckedDataSources> GetAllowedDataSources(bool usingTrustedProvider, IReadOnlyList<ParticipatingProvider> participatingProviders, IReadOnlyCollection<IDataSource> requestedDataSources)
     {
         var filteredDataSources = new List<IDataSource>(requestedDataSources.Count);
-        var tasks = new List<Task<IDataSource?>>(requestedDataSources.Count);
+        var unreachableDataSources = new List<IDataSource>();
+        var tasks = new List<Task<DataSourceCheck>>(requestedDataSources.Count);
 
         // Start all checks in parallel:
         foreach (var source in requestedDataSources)
@@ -270,15 +279,126 @@ public sealed class DataSourceService
         // Wait for all checks and collect the results:
         foreach (var task in tasks)
         {
-            var source = await task;
-            if (source is not null)
-                filteredDataSources.Add(source);
+            var check = await task;
+            if (check.AllowedSource is not null)
+                filteredDataSources.Add(check.AllowedSource);
+            else if (check.UnreachableSource is not null)
+                unreachableDataSources.Add(check.UnreachableSource);
         }
 
-        return filteredDataSources;
+        return new(filteredDataSources, unreachableDataSources);
     }
-    
-    private async Task<IDataSource?> CheckOneDataSource(IDataSource source, bool usingTrustedProvider, IReadOnlyList<ParticipatingProvider> participatingProviders)
+
+    /// <summary>
+    /// What the check of one data source found: it may be used, its ERI server could not be reached, or neither.
+    /// </summary>
+    private readonly record struct DataSourceCheck(IDataSource? AllowedSource, IDataSource? UnreachableSource);
+
+    private async Task<DataSourceCheck> CheckOneDataSource(IDataSource source, bool usingTrustedProvider, IReadOnlyList<ParticipatingProvider> participatingProviders)
+    {
+        //
+        // Unfortunately, we have to live-check any ERI source for its security requirements.
+        // Because the ERI server operator might change the security requirements at any time.
+        // The answer is kept for a minute, see ERISecurityCheckCache for why that is acceptable.
+        //
+        SecurityRequirements? eriSourceRequirements = null;
+        if (source is DataSourceERI_V1 eriSource)
+        {
+            var securityCheck = await this.securityChecks.GetAsync(eriSource);
+            if (!securityCheck.IsReachable)
+            {
+                //
+                // The source cannot be used, but whoever asks may still tell the model or the user
+                // about it, so that its silence is not taken for finding nothing. Only a source
+                // which its own configured policy allows for this provider counts: whatever the
+                // server would demand on top of that is unknown, and it is never searched anyway.
+                //
+                return new(null, MayBeNamedWhileUnreachable(eriSource, usingTrustedProvider) ? source : null);
+            }
+
+            eriSourceRequirements = securityCheck.Requirements;
+        }
+
+        return new(this.CheckPolicies(source, usingTrustedProvider, participatingProviders, eriSourceRequirements), null);
+    }
+
+    /// <summary>
+    /// Whether an ERI data source whose server could not be reached may still be named to a provider.
+    /// </summary>
+    /// <param name="source">The ERI data source.</param>
+    /// <param name="usingTrustedProvider">Whether the chat provider is trusted for data source security checks.</param>
+    /// <returns>True when the configured security policy of the data source allows it for this provider.</returns>
+    internal static bool MayBeNamedWhileUnreachable(IExternalDataSource source, bool usingTrustedProvider) => usingTrustedProvider.AllowsDataSourceSecurity(source.SecurityPolicy);
+
+    /// <summary>
+    /// Signs in to the server of an ERI data source and asks for its security requirements.
+    /// </summary>
+    /// <returns>Whether the server answered and the sign-in succeeded, and its requirements when it did.</returns>
+    private async Task<ERISecurityCheckResult> FetchSecurityRequirementsAsync(DataSourceERI_V1 source)
+    {
+        //
+        // Leave the caller first: this starts while the cache is locked, and signing in should
+        // not happen while holding the lock:
+        //
+        await Task.Yield();
+
+        using var cancellationTokenSource = new CancellationTokenSource(TimeSpan.FromSeconds(6));
+        using var client = ERIClientFactory.Get(ERIVersion.V1, source);
+        if(client is null)
+        {
+            this.logger.LogError($"Could not create ERI client for source '{source.Name}' (id={source.Id}). We skip this source.");
+            return ERISecurityCheckResult.UNREACHABLE;
+        }
+
+        this.logger.LogInformation($"Authenticating with ERI source '{source.Name}' (id={source.Id})...");
+        var loginResult = await client.AuthenticateAsync(this.rustService, cancellationToken: cancellationTokenSource.Token);
+        if (!loginResult.Successful)
+        {
+            this.logger.LogWarning($"Authentication with ERI source '{source.Name}' (id={source.Id}) failed. We skip this source. Reason: {loginResult.Message}");
+            return ERISecurityCheckResult.UNREACHABLE;
+        }
+
+        this.logger.LogInformation($"Checking security requirements for ERI source '{source.Name}' (id={source.Id})...");
+        var securityRequest = await client.GetSecurityRequirementsAsync(cancellationTokenSource.Token);
+        if (!securityRequest.Successful)
+        {
+            this.logger.LogWarning($"Could not retrieve security requirements for ERI source '{source.Name}' (id={source.Id}). We skip this source. Reason: {securityRequest.Message}");
+            return ERISecurityCheckResult.UNREACHABLE;
+        }
+
+        this.logger.LogInformation($"Security requirements for ERI source '{source.Name}' (id={source.Id}) retrieved successfully.");
+        return new(true, securityRequest.Data);
+    }
+
+    /// <summary>
+    /// Whether the server of an ERI data source allows the provider to see its data.
+    /// </summary>
+    /// <remarks>
+    /// Its operator owns the data, so the rule of the server applies to every provider, a trusted
+    /// one included. A server which allows no provider at all closes its data source this way, as
+    /// the info dialog of the data source tells the user, e.g., to stop all access while the
+    /// operator corrects a mistake. A rule this version of AI Studio does not know closes it, too:
+    /// better safe than sorry.
+    /// </remarks>
+    /// <param name="requirements">What the server requires.</param>
+    /// <param name="usingTrustedProvider">Whether the chat provider is trusted for data source security checks.</param>
+    /// <returns>True when the server allows the provider.</returns>
+    internal static bool ServerAllows(SecurityRequirements requirements, bool usingTrustedProvider) => requirements.AllowedProviderType switch
+    {
+        ProviderType.ANY => true,
+        ProviderType.SELF_HOSTED => usingTrustedProvider,
+        _ => false,
+    };
+
+    /// <summary>
+    /// Checks a data source against the trust it asks for.
+    /// </summary>
+    /// <param name="source">The data source.</param>
+    /// <param name="usingTrustedProvider">Whether the chat provider is trusted for data source security checks.</param>
+    /// <param name="participatingProviders">Every provider which gets to see the data.</param>
+    /// <param name="eriSourceRequirements">What the server of an ERI data source requires, or null for any other data source.</param>
+    /// <returns>The data source when it may be used, otherwise null.</returns>
+    private IDataSource? CheckPolicies(IDataSource source, bool usingTrustedProvider, IReadOnlyList<ParticipatingProvider> participatingProviders, SecurityRequirements? eriSourceRequirements)
     {
         if (source is IInternalDataSource internalSource)
         {
@@ -307,85 +427,28 @@ public sealed class DataSourceService
             return source;
         }
 
-        //
-        // Unfortunately, we have to live-check any ERI source for its security requirements.
-        // Because the ERI server operator might change the security requirements at any time.
-        //
-        SecurityRequirements? eriSourceRequirements = null;
-        if (source is DataSourceERI_V1 eriSource)
-        {
-            using var cancellationTokenSource = new CancellationTokenSource(TimeSpan.FromSeconds(6));
-            using var client = ERIClientFactory.Get(ERIVersion.V1, eriSource);
-            if(client is null)
-            {
-                this.logger.LogError($"Could not create ERI client for source '{source.Name}' (id={source.Id}). We skip this source.");
-                return null;
-            }
-
-            this.logger.LogInformation($"Authenticating with ERI source '{source.Name}' (id={source.Id})...");
-            var loginResult = await client.AuthenticateAsync(this.rustService, cancellationToken: cancellationTokenSource.Token);
-            if (!loginResult.Successful)
-            {
-                this.logger.LogWarning($"Authentication with ERI source '{source.Name}' (id={source.Id}) failed. We skip this source. Reason: {loginResult.Message}");
-                return null;
-            }
-
-            this.logger.LogInformation($"Checking security requirements for ERI source '{source.Name}' (id={source.Id})...");
-            var securityRequest = await client.GetSecurityRequirementsAsync(cancellationTokenSource.Token);
-            if (!securityRequest.Successful)
-            {
-                this.logger.LogWarning($"Could not retrieve security requirements for ERI source '{source.Name}' (id={source.Id}). We skip this source. Reason: {loginResult.Message}");
-                return null;
-            }
-
-            eriSourceRequirements = securityRequest.Data;
-            this.logger.LogInformation($"Security requirements for ERI source '{source.Name}' (id={source.Id}) retrieved successfully.");
-        }
-
         if (source is not IExternalDataSource externalSource)
             return source;
 
+        //
+        // The server of an ERI data source has the last word, on top of the policy configured
+        // here: its operator owns the data and may tighten the rules at any time. This holds for a
+        // trusted provider as well, see ServerAllows:
+        //
+        if (eriSourceRequirements is { } requirements && !ServerAllows(requirements, usingTrustedProvider))
+        {
+            this.logger.LogWarning($"The ERI source '{source.Name}' (id={source.Id}) does not allow this provider by the rules of its server ('{requirements.AllowedProviderType}'). We skip this source.");
+            return null;
+        }
+
         switch (externalSource.SecurityPolicy)
         {
+            //
+            // Case: The data source allows any provider type, and the server of an ERI data source
+            //       agreed above. Accept it.
+            //
             case DataSourceSecurity.ALLOW_ANY:
-                
-                //
-                // Case: The data source allows any provider type. We want to use a trusted provider.
-                //       There is no issue with this source. Accept it.
-                //
-                if(usingTrustedProvider)
-                    return source;
-
-                //
-                // Case: This is a local data source. When the source allows any provider type, we can use it.
-                //       Accept it.
-                //
-                if(eriSourceRequirements is null)
-                    return source;
-
-                //
-                // Case: The ERI source requires a self-hosted or organization-trusted provider. This misconfiguration happens
-                //       when the ERI server operator changes the security requirements. The ERI server
-                //       operator owns the data -- we have to respect their rules. We skip this source.
-                //
-                if (eriSourceRequirements is { AllowedProviderType: ProviderType.SELF_HOSTED })
-                {
-                    this.logger.LogWarning($"The ERI source '{source.Name}' (id={source.Id}) requires a self-hosted or organization-trusted provider. We skip this source.");
-                    return null;
-                }    
-                
-                //
-                // Case: The ERI source allows any provider type. The data source configuration is correct.
-                //       Accept it.
-                //
-                if(eriSourceRequirements is { AllowedProviderType: ProviderType.ANY })
-                    return source;
-
-                //
-                // Case: Missing rules. We skip this source. Better safe than sorry.
-                //
-                this.logger.LogWarning($"The ERI source '{source.Name}' (id={source.Id}) was filtered out due to missing rules.");
-                return null;
+                return source;
 
             //
             // Case: The data source requires a trusted provider. We want to use a trusted provider.

app/MindWork AI Studio/Tools/Services/ERISecurityCheckCache.cs

@@ -0,0 +1,107 @@
+using AIStudio.Settings.DataModel;
+
+namespace AIStudio.Tools.Services;
+
+/// <summary>
+/// Keeps what the servers of ERI data sources answered to the check of their security requirements.
+/// </summary>
+/// <remarks>
+/// One message asks this several times: Semantic Search checks the data sources when it is
+/// offered and again before every search, and the classic RAG process checks them with every
+/// message. Each check signs in, and a server which limits sign-ins or sessions would start
+/// refusing. A check which is still running is joined rather than started a second time.<br/><br/>
+/// The server's operator may change the requirements at any time, so an answer is kept for one
+/// minute only. That covers the rounds of one answer, while a change still takes effect soon. A
+/// server which did not answer is asked again sooner: it may well be back, and until then, its
+/// data source is not searched anyway. A changed configuration, e.g., another server or another
+/// account, asks anew right away.<br/><br/>
+/// The minute holds for an answer which lets the data reach any provider as well, and that is a
+/// deliberate trade-off. When an operator opened a server by mistake and corrects it, every AI
+/// Studio follows within that minute rather than with its next check: until then, a chat with a
+/// cloud provider may still search the server. Nothing outlasts the running app, though, so no
+/// answer, however open, sticks for longer.<br/><br/>
+/// Whenever a check starts, the expired ones are dropped, so that data sources which were deleted
+/// or changed do not pile up.
+/// </remarks>
+/// <param name="fetch">Signs in to the server of a data source and asks for its requirements.</param>
+/// <param name="timeProvider">Tells the time the checks are started at.</param>
+internal sealed class ERISecurityCheckCache(Func<DataSourceERI_V1, Task<ERISecurityCheckResult>> fetch, TimeProvider timeProvider)
+{
+    /// <summary>
+    /// How long the answer of an ERI server to the check of its security requirements is used.
+    /// </summary>
+    private static readonly TimeSpan ANSWER_LIFETIME = TimeSpan.FromMinutes(1);
+
+    /// <summary>
+    /// How long an ERI server which did not answer the check is not asked again, counted from the start of the check.
+    /// </summary>
+    private static readonly TimeSpan NO_ANSWER_LIFETIME = TimeSpan.FromSeconds(15);
+
+    /// <summary>
+    /// A check of an ERI server, which may still be running, together with the configuration it was started with.
+    /// </summary>
+    private readonly record struct SecurityCheck(DataSourceERI_V1 DataSource, Task<ERISecurityCheckResult> Result, DateTimeOffset StartedAt);
+
+    private readonly Lock checksLock = new();
+    private readonly Dictionary<string, SecurityCheck> checks = new(StringComparer.Ordinal);
+
+    /// <summary>
+    /// What the server of an ERI data source requires, asked anew only once the last answer expired.
+    /// </summary>
+    /// <param name="source">The ERI data source.</param>
+    /// <returns>Whether the server answered, and its requirements when it did.</returns>
+    public Task<ERISecurityCheckResult> GetAsync(DataSourceERI_V1 source)
+    {
+        lock (this.checksLock)
+        {
+            var now = timeProvider.GetUtcNow();
+            if (this.checks.TryGetValue(source.Id, out var known) && known.DataSource.Equals(source) && IsValid(known.Result, now - known.StartedAt))
+                return known.Result;
+
+            this.RemoveExpired(now);
+            var check = new SecurityCheck(source, fetch(source), now);
+            this.checks[source.Id] = check;
+            return check.Result;
+        }
+    }
+
+    /// <summary>
+    /// How many checks are kept right now, expired ones included.
+    /// </summary>
+    internal int Count
+    {
+        get
+        {
+            lock (this.checksLock)
+                return this.checks.Count;
+        }
+    }
+
+    /// <summary>
+    /// Whether a check of an ERI server may still be used instead of asking the server again.
+    /// </summary>
+    /// <param name="result">The check, which may still be running.</param>
+    /// <param name="age">How long ago the check was started.</param>
+    /// <returns>True when the check is still running or its answer has not expired yet.</returns>
+    internal static bool IsValid(Task<ERISecurityCheckResult> result, TimeSpan age)
+    {
+        if (!result.IsCompleted)
+            return true;
+
+        if (!result.IsCompletedSuccessfully)
+            return false;
+
+        return age < (result.Result.IsReachable ? ANSWER_LIFETIME : NO_ANSWER_LIFETIME);
+    }
+
+    private void RemoveExpired(DateTimeOffset now)
+    {
+        var expiredIds = this.checks
+            .Where(pair => !IsValid(pair.Value.Result, now - pair.Value.StartedAt))
+            .Select(pair => pair.Key)
+            .ToList();
+
+        foreach (var id in expiredIds)
+            this.checks.Remove(id);
+    }
+}

app/MindWork AI Studio/Tools/Services/ERISecurityCheckResult.cs

@@ -0,0 +1,16 @@
+using AIStudio.Tools.ERIClient.DataModel;
+
+namespace AIStudio.Tools.Services;
+
+/// <summary>
+/// What the server of an ERI data source answered to the check of its security requirements.
+/// </summary>
+/// <param name="IsReachable">Whether the server answered and the sign-in succeeded.</param>
+/// <param name="Requirements">What the server requires, or null when it could not be reached.</param>
+internal readonly record struct ERISecurityCheckResult(bool IsReachable, SecurityRequirements? Requirements)
+{
+    /// <summary>
+    /// The server could not be reached, or it refused the sign-in.
+    /// </summary>
+    public static readonly ERISecurityCheckResult UNREACHABLE = new(false, null);
+}