Features/aii/gtr 13556

src/Infrastructure/BotSharp.Abstraction/Agents/Models/Agent.cs

@@ -165,6 +165,12 @@ public override string ToString()
         => $"{Name} {Id}";
 
 
+    /// <summary>
+    /// Copies every member, including the ones <see cref="Clone"/> leaves out. Lists and nested objects
+    /// are still shared with the source: replace any of them before changing it.
+    /// </summary>
+    public Agent ShallowCopy() => (Agent)MemberwiseClone();
+
     public static Agent Clone(Agent agent)
     {
         return new Agent

src/Infrastructure/BotSharp.Abstraction/Infrastructures/SharpCacheAttribute.cs

@@ -2,15 +2,33 @@
 using BotSharp.Abstraction.MultiTenancy;
 using Microsoft.AspNetCore.Http;
 using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
 using Rougamo;
+using System.Collections.Concurrent;
+using System.Globalization;
+using System.Reflection;
+using System.Runtime.CompilerServices;
+using System.Security.Cryptography;
+using System.Text.Json;
 using Rougamo.Context;
 
 namespace BotSharp.Core.Infrastructures;
 
 public class SharpCacheAttribute : AsyncMoAttribute
 {
     public static IServiceProvider Services { get; set; } = null!;
-    private static readonly object NullMarker = new { __is_null = "$_is_null" };
+    private const string NullKey = "$_is_null";
+    private const int MaxRawStringKeyLength = 128;
+    private static readonly JsonSerializerOptions KeyJsonOptions = new() { WriteIndented = false };
+
+    /// <summary>
+    /// Entries for <c>perInstanceCache</c>, attached to the instance they belong to. Such an entry can only
+    /// ever be hit through that instance, so it is collected together with it. In the shared cache it stayed
+    /// for the full expiry after a scoped service's scope had ended, one dead copy per scope.
+    /// </summary>
+    private static readonly ConditionalWeakTable<object, ConcurrentDictionary<string, InstanceCacheEntry>> InstanceCaches = new();
+
+    private sealed record InstanceCacheEntry(object Value, DateTime ExpiresAt);
 
     private readonly int _minutes;
     private readonly bool _perInstanceCache;
@@ -33,7 +51,14 @@ public override async ValueTask OnEntryAsync(MethodContext context)
         }
 
         var key = GetCacheKey(context);
-        var value = await _cache.GetAsync(key, context.TaskReturnType);
+        if (key == null)
+        {
+            return;
+        }
+
+        var value = IsInstanceScoped(context)
+            ? GetInstanceValue(context.Target!, key)
+            : await _cache.GetAsync(key, context.TaskReturnType);
         if (value != null)
         {
             // check if the cache is out of date
@@ -64,8 +89,37 @@ public override async ValueTask OnSuccessAsync(MethodContext context)
         if (context.ReturnValue != null)
         {
             var key = GetCacheKey(context);
-            await _cache.SetAsync(key, context.ReturnValue, new TimeSpan(0, _minutes, 0));
+            if (key == null)
+            {
+                return;
+            }
+
+            var expiry = new TimeSpan(0, _minutes, 0);
+            if (IsInstanceScoped(context))
+            {
+                InstanceCaches.GetOrCreateValue(context.Target!)[key] = new InstanceCacheEntry(context.ReturnValue, DateTime.UtcNow + expiry);
+            }
+            else
+            {
+                await _cache.SetAsync(key, context.ReturnValue, expiry);
+            }
+        }
+    }
+
+    private bool IsInstanceScoped(MethodContext context)
+        => _perInstanceCache && context.Target != null;
+
+    private static object? GetInstanceValue(object target, string key)
+    {
+        if (InstanceCaches.TryGetValue(target, out var entries) && entries.TryGetValue(key, out var entry))
+        {
+            if (entry.ExpiresAt > DateTime.UtcNow)
+            {
+                return entry.Value;
+            }
+            entries.TryRemove(key, out _);
         }
+        return null;
     }
 
     public virtual Task<bool> IsOutOfDate(MethodContext context, object value)
@@ -74,23 +128,36 @@ public virtual Task<bool> IsOutOfDate(MethodContext context, object value)
     }
 
 
-    private string GetCacheKey(MethodContext context)
+    /// <summary>
+    /// Returns null when an argument cannot be turned into a stable key; the call then bypasses the cache
+    /// rather than risk a key that never hits or that collides with another caller's entry.
+    /// </summary>
+    private string? GetCacheKey(MethodContext context)
     {
-        var prefixKey = GetPrefixKey(context.Method.Name);
-        var argsKey = string.Join("_", context.Arguments.Select(arg => GetCacheKeyByArg(arg)));
-
-        if (_perInstanceCache && context.Target != null)
+        var argKeys = new List<string>(context.Arguments.Length);
+        foreach (var arg in context.Arguments)
         {
-            return $"{prefixKey}-{context.Target.GetHashCode()}_{argsKey}";
-        }
-        else
-        {
-            return $"{prefixKey}_{argsKey}";
+            if (!TryGetCacheKeyByArg(arg, out var argKey))
+            {
+                Services.GetService<ILogger<SharpCacheAttribute>>()?.LogDebug(
+                    "SharpCache bypassed for {Method}: argument of type {ArgType} has no stable cache key.",
+                    context.Method.Name, arg?.GetType().FullName);
+                return null;
+            }
+            argKeys.Add(argKey);
         }
+
+        // Instance-scoped entries live on the instance itself, so the key needs no instance part.
+        return $"{GetPrefixKey(context.Method)}_{string.Join("_", argKeys)}";
     }
 
-    private string GetPrefixKey(string name)
+    /// <summary>
+    /// Includes the declaring type: keyed by method name alone, two classes with a same-named method and equal
+    /// arguments shared one entry, e.g. two IEntityDataLoader implementations' LoadVocabularyAsync().
+    /// </summary>
+    private string GetPrefixKey(MethodBase method)
     {
+        var name = $"{method.DeclaringType?.FullName ?? method.DeclaringType?.Name}.{method.Name}";
         var tenantId = GetTenantId();
         if (!string.IsNullOrWhiteSpace(tenantId))
         {
@@ -113,27 +180,65 @@ private string GetPrefixKey(string name)
         }
     }
 
-    private string GetCacheKeyByArg(object? arg)
+    /// <summary>
+    /// Builds the key from the argument's content. Reference-type arguments used to be keyed by
+    /// <c>GetHashCode()</c>, i.e. object identity: a freshly built filter or list never hit the cache,
+    /// so every call parked another copy of the result, and a 32-bit hash collision could hand one
+    /// caller another caller's entry.
+    /// </summary>
+    private static bool TryGetCacheKeyByArg(object? arg, out string key)
     {
-        if (arg is null)
+        switch (arg)
         {
-            return NullMarker.GetHashCode().ToString();
+            case null:
+                key = NullKey;
+                return true;
+            case ICacheKey withCacheKey:
+                key = withCacheKey.GetCacheKey();
+                return true;
+            case string text:
+                // Length-prefixed so "a_b" + "c" and "a" + "b_c" cannot join into the same key.
+                key = text.Length <= MaxRawStringKeyLength ? $"s{text.Length}:{text}" : $"h:{Sha256(text)}";
+                return true;
+            case DateTime or DateTimeOffset:
+                key = ((IFormattable)arg).ToString("o", CultureInfo.InvariantCulture);
+                return true;
         }
-        else if (arg is ICacheKey withCacheKey)
+
+        var type = arg.GetType();
+        if (type.IsPrimitive || type.IsEnum || arg is decimal or Guid or TimeSpan)
+        {
+            key = Convert.ToString(arg, CultureInfo.InvariantCulture) ?? string.Empty;
+            return true;
+        }
+
+        try
         {
-            return withCacheKey.GetCacheKey();
+            var json = JsonSerializer.Serialize(arg, type, KeyJsonOptions);
+            key = $"j:{Sha256(type.FullName + json)}";
+            return true;
         }
-        else
+        catch (Exception ex) when (ex is NotSupportedException or JsonException or InvalidOperationException)
         {
-            return arg.GetHashCode().ToString();
+            key = string.Empty;
+            return false;
         }
     }
 
+    private static string Sha256(string value)
+        => Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(value)));
+
     public async Task ClearCacheAsync()
     {
         var tenantId = GetTenantId();
         var prefix = string.IsNullOrWhiteSpace(tenantId) ? _settings.Prefix : $"{_settings.Prefix}:{tenantId}";
         await _cache.ClearCacheAsync(prefix);
+
+        // Instance-scoped entries were cleared along with the shared cache before they moved out of it.
+        foreach (var (_, entries) in InstanceCaches)
+        {
+            entries.Clear();
+        }
     }
 
 }

src/Infrastructure/BotSharp.Core/Agents/Services/AgentService.GetAgents.cs

@@ -61,7 +61,8 @@ public async Task<Agent> GetAgent(string id)
         var agentSetting = _services.GetRequiredService<AgentSettings>();
         if (profile.LlmConfig == null)
         {
-            profile.LlmConfig = agentSetting.LlmConfig;
+            // A copy: flagging IsInherit on the shared settings instance marked the default config itself as inherited.
+            profile.LlmConfig = agentSetting.LlmConfig.DeepClone() ?? new AgentLlmConfig();
             profile.LlmConfig.IsInherit = true;
         }
         else if (string.IsNullOrEmpty(profile.LlmConfig?.Provider) || string.IsNullOrEmpty(profile.LlmConfig?.Model))

src/Infrastructure/BotSharp.Core/Routing/Hooks/RoutingAgentHook.cs

@@ -1,3 +1,4 @@
+using BotSharp.Abstraction.Routing.Models;
 using BotSharp.Abstraction.Routing.Settings;
 
 namespace BotSharp.Core.Routing.Hooks;
@@ -32,30 +33,19 @@ public override async Task<bool> OnInstructionLoaded(string template, IDictionar
             agents = agents.Except(planners).ToArray();
         }
 
-        // Postprocess agent required fields, remove it if the states exists
+        // GetRoutableAgents is cached and shared by every conversation: build this conversation's view
+        // without the fields it already has, instead of removing them from the cached agents.
         var states = _services.GetRequiredService<IConversationStateService>();
-        foreach (var agent in agents)
+        agents = agents.Select(x => new RoutableAgent
         {
-            var fields = agent.RequiredFields.ToArray();
-            foreach (var field in fields)
-            {
-                if (states.ContainsState(field.Name))
-                {
-                    var requiredField = agent.RequiredFields.First(x => x.Name == field.Name);
-                    agent.RequiredFields.Remove(requiredField);
-                }
-            }
-
-            fields = agent.OptionalFields.ToArray();
-            foreach (var field in fields)
-            {
-                if (states.ContainsState(field.Name))
-                {
-                    var optionalField = agent.OptionalFields.First(x => x.Name == field.Name);
-                    agent.OptionalFields.Remove(optionalField);
-                }
-            }
-        }
+            AgentId = x.AgentId,
+            Name = x.Name,
+            Description = x.Description,
+            Type = x.Type,
+            Profiles = x.Profiles,
+            RequiredFields = x.RequiredFields.Where(f => !states.ContainsState(f.Name)).ToList(),
+            OptionalFields = x.OptionalFields.Where(f => !states.ContainsState(f.Name)).ToList()
+        }).ToArray();
 
         dict["routing_agents"] = agents;
 

src/Infrastructure/BotSharp.OpenAPI/Controllers/Agent/AgentController.cs

@@ -40,8 +40,15 @@ public AgentSettings GetSettings()
             AgentIds = [id]
         });
 
-        var foundAgent = pagedAgents.Items.FirstOrDefault();
-        if (foundAgent == null) return null;
+        var cachedAgent = pagedAgents.Items.FirstOrDefault();
+        if (cachedAgent == null) return null;
+
+        // GetAgents is cached and shared with every other caller. Inherit into a copy with its own lists,
+        // and give the view its own routing rules, because RedirectToAgentName is written to them below.
+        var foundAgent = cachedAgent.ShallowCopy();
+        foundAgent.Templates = cachedAgent.Templates?.ToList() ?? [];
+        foundAgent.Functions = cachedAgent.Functions?.ToList() ?? [];
+        foundAgent.RoutingRules = cachedAgent.RoutingRules?.Select(x => x.Clone()).ToList() ?? [];
 
         await _agentService.InheritAgent(foundAgent);
         var targetAgent = AgentViewModel.FromAgent(foundAgent);

tests/BotSharp.Core.UnitTests/BotSharp.Core.UnitTests.csproj

@@ -11,6 +11,8 @@
   <ItemGroup>
     <PackageReference Include="Microsoft.NET.Test.Sdk" />
     <PackageReference Include="Moq" />
+    <!-- Weaves [SharpCache] onto test-only classes, so the cache key can be exercised on methods the tests define. -->
+    <PackageReference Include="Rougamo.Fody" PrivateAssets="all" />
     <PackageReference Include="xunit" />
     <PackageReference Include="xunit.runner.visualstudio">
       <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
@@ -34,6 +36,8 @@
     <ProjectReference Include="..\..\src\Plugins\BotSharp.Plugin.AgentTesting\BotSharp.Plugin.AgentTesting.csproj" />
     <!-- For ChatEventDispatcherTests: chat events must never make their producer wait on a client. -->
     <ProjectReference Include="..\..\src\Plugins\BotSharp.Plugin.ChatHub\BotSharp.Plugin.ChatHub.csproj" />
+    <!-- For SharpCacheSharedResultTests: AgentController must not mutate the cached agents it reads. -->
+    <ProjectReference Include="..\..\src\Infrastructure\BotSharp.OpenAPI\BotSharp.OpenAPI.csproj" />
     <!-- For StreamingToolCallReconstructionTests: the streamed tool-call reconstruction lives here. -->
     <ProjectReference Include="..\..\src\Plugins\BotSharp.Plugin.OpenAI\BotSharp.Plugin.OpenAI.csproj" />
   </ItemGroup>