diff --git a/BotSharp.sln b/BotSharp.sln index 20bfeb54e..a497c5f84 100644 --- a/BotSharp.sln +++ b/BotSharp.sln @@ -165,6 +165,8 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "BotSharp.Plugin.MicrosoftTe EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "BotSharp.Core.UnitTests", "tests\BotSharp.Core.UnitTests\BotSharp.Core.UnitTests.csproj", "{3585AE68-43CE-B724-72C1-579D9FFE1D6E}" EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "BotSharp.Plugin.AgentTesting", "src\Plugins\BotSharp.Plugin.AgentTesting\BotSharp.Plugin.AgentTesting.csproj", "{57CE6EAE-8957-435A-8A14-5A693EA8B520}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -955,6 +957,18 @@ Global {3585AE68-43CE-B724-72C1-579D9FFE1D6E}.Release|x64.Build.0 = Release|Any CPU {3585AE68-43CE-B724-72C1-579D9FFE1D6E}.Release|x86.ActiveCfg = Release|Any CPU {3585AE68-43CE-B724-72C1-579D9FFE1D6E}.Release|x86.Build.0 = Release|Any CPU + {57CE6EAE-8957-435A-8A14-5A693EA8B520}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {57CE6EAE-8957-435A-8A14-5A693EA8B520}.Debug|Any CPU.Build.0 = Debug|Any CPU + {57CE6EAE-8957-435A-8A14-5A693EA8B520}.Debug|x64.ActiveCfg = Debug|Any CPU + {57CE6EAE-8957-435A-8A14-5A693EA8B520}.Debug|x64.Build.0 = Debug|Any CPU + {57CE6EAE-8957-435A-8A14-5A693EA8B520}.Debug|x86.ActiveCfg = Debug|Any CPU + {57CE6EAE-8957-435A-8A14-5A693EA8B520}.Debug|x86.Build.0 = Debug|Any CPU + {57CE6EAE-8957-435A-8A14-5A693EA8B520}.Release|Any CPU.ActiveCfg = Release|Any CPU + {57CE6EAE-8957-435A-8A14-5A693EA8B520}.Release|Any CPU.Build.0 = Release|Any CPU + {57CE6EAE-8957-435A-8A14-5A693EA8B520}.Release|x64.ActiveCfg = Release|Any CPU + {57CE6EAE-8957-435A-8A14-5A693EA8B520}.Release|x64.Build.0 = Release|Any CPU + {57CE6EAE-8957-435A-8A14-5A693EA8B520}.Release|x86.ActiveCfg = Release|Any CPU + {57CE6EAE-8957-435A-8A14-5A693EA8B520}.Release|x86.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -1034,6 +1048,7 @@ Global {58D3A2C3-F96F-5E57-2C6B-ECE59D6A18FC} = {827E0CD3-B72D-47B6-A68D-7590B98EB39B} {19FA8311-69D1-4729-AE9B-D10979C099EC} = {58D3A2C3-F96F-5E57-2C6B-ECE59D6A18FC} {3585AE68-43CE-B724-72C1-579D9FFE1D6E} = {32FAFFFE-A4CB-4FEE-BF7C-84518BBC6DCC} + {57CE6EAE-8957-435A-8A14-5A693EA8B520} = {58D3A2C3-F96F-5E57-2C6B-ECE59D6A18FC} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {A9969D89-C98B-40A5-A12B-FC87E55B3A19} diff --git a/src/Infrastructure/BotSharp.Abstraction/Routing/Executor/IFunctionExecutorFactory.cs b/src/Infrastructure/BotSharp.Abstraction/Routing/Executor/IFunctionExecutorFactory.cs new file mode 100644 index 000000000..8adecb889 --- /dev/null +++ b/src/Infrastructure/BotSharp.Abstraction/Routing/Executor/IFunctionExecutorFactory.cs @@ -0,0 +1,11 @@ +namespace BotSharp.Abstraction.Routing.Executor; + +/// +/// Decides who executes a given function name. Every function-call path has to go through here, +/// or IFunctionExecutorProvider gets bypassed -- BotSharp.Core.Rules' ToolCallAction used to be +/// exactly such a bypass. +/// +public interface IFunctionExecutorFactory +{ + IFunctionExecutor? Create(string functionName, Agent agent); +} diff --git a/src/Infrastructure/BotSharp.Abstraction/Routing/Executor/IFunctionExecutorProvider.cs b/src/Infrastructure/BotSharp.Abstraction/Routing/Executor/IFunctionExecutorProvider.cs new file mode 100644 index 000000000..15f6f6fce --- /dev/null +++ b/src/Infrastructure/BotSharp.Abstraction/Routing/Executor/IFunctionExecutorProvider.cs @@ -0,0 +1,14 @@ +namespace BotSharp.Abstraction.Routing.Executor; + +/// +/// Lets an external component take over the execution of a function. Returning null means "not +/// mine" and hands off to the next provider, or to the built-in resolution chain. The typical use +/// is swapping a real tool for a fake during a test, or blocking a function by policy. +/// +public interface IFunctionExecutorProvider +{ + /// Lower is asked first. + int Order => 0; + + IFunctionExecutor? TryResolve(string functionName, Agent agent); +} diff --git a/src/Infrastructure/BotSharp.Core.Rules/Actions/ToolCallAction.cs b/src/Infrastructure/BotSharp.Core.Rules/Actions/ToolCallAction.cs index 1cd2e62b2..958a3f584 100644 --- a/src/Infrastructure/BotSharp.Core.Rules/Actions/ToolCallAction.cs +++ b/src/Infrastructure/BotSharp.Core.Rules/Actions/ToolCallAction.cs @@ -1,5 +1,6 @@ using BotSharp.Abstraction.Functions; using BotSharp.Abstraction.Graph.Models; +using BotSharp.Abstraction.Routing.Executor; namespace BotSharp.Core.Rules.Actions; @@ -43,9 +44,27 @@ public async Task ExecuteAsync( RuleFlowContext context) { var funcName = context.Parameters.TryGetValue("function_name", out var fName) ? fName : null; - var func = _services.GetServices().FirstOrDefault(x => x.Name.IsEqualTo(funcName)); - if (func == null) + // A missing/blank function_name has to fail gracefully. The old IsEqualTo lookup was + // null-safe -- it simply matched no callback. Going through the factory instead means null + // reaches IFunctionExecutorProvider.TryResolve, whose contract declares a non-null string, + // and some implementations (a mock/blocking provider doing a Dictionary lookup by name, for + // instance) would throw ArgumentNullException rather than returning Success = false the way + // this used to. So it is rejected before the factory is touched at all. + string? canonicalName = null; + IFunctionExecutor? executor = null; + if (!string.IsNullOrWhiteSpace(funcName)) + { + // Registered callbacks are used only to resolve the CANONICAL name, preserving the + // original case-insensitive semantics; the execution itself must go through the factory, + // or IFunctionExecutorProvider is bypassed on the rule path. + canonicalName = _services.GetServices() + .FirstOrDefault(x => x.Name.IsEqualTo(funcName))?.Name ?? funcName; + executor = _services.GetRequiredService() + .Create(canonicalName, agent); + } + + if (executor == null || canonicalName == null) { var errorMsg = $"Unable to find function '{funcName}' when running action {agent.Name}-{trigger.Name}"; _logger.LogWarning(errorMsg); @@ -57,7 +76,7 @@ public async Task ExecuteAsync( } var funcArg = context.Parameters.TryGetObjectValueOrDefault("function_argument", new()) ?? new(); - await func.Execute(funcArg); + await executor.ExecuteAsync(funcArg); return new RuleNodeResult { @@ -65,7 +84,7 @@ public async Task ExecuteAsync( Response = funcArg?.RichContent?.Message?.Text ?? funcArg?.Content, Data = new() { - ["function_name"] = func.Name!, + ["function_name"] = canonicalName, ["function_argument"] = funcArg?.ConvertToString() ?? "{}", ["function_call_result"] = funcArg?.RichContent?.Message?.Text ?? funcArg?.Content ?? string.Empty } diff --git a/src/Infrastructure/BotSharp.Core/Routing/Executor/FunctionExecutorFactory.cs b/src/Infrastructure/BotSharp.Core/Routing/Executor/FunctionExecutorFactory.cs index 8a4a54865..f0d2030e1 100644 --- a/src/Infrastructure/BotSharp.Core/Routing/Executor/FunctionExecutorFactory.cs +++ b/src/Infrastructure/BotSharp.Core/Routing/Executor/FunctionExecutorFactory.cs @@ -3,11 +3,45 @@ namespace BotSharp.Core.Routing.Executor; -internal class FunctionExecutorFactory +public class FunctionExecutorFactory : IFunctionExecutorFactory { - public static IFunctionExecutor? Create(IServiceProvider services, string functionName, Agent agent) + private readonly IServiceProvider _services; + + public FunctionExecutorFactory(IServiceProvider services) + { + _services = services; + } + + public IFunctionExecutor? Create(string functionName, Agent agent) { - var functionCall = services.GetServices().FirstOrDefault(x => x.Name == functionName); + // This is the one place in the repo that decides who executes a given function name (see + // the interface's own doc comment), so it cannot rely on every caller validating first -- + // RoutingService.InvokeFunction has no guard at all and passes `name` straight through. A + // null/blank function name reaching one of the registered IFunctionExecutorProvider + // implementations below would make some of them throw ArgumentNullException instead of + // returning null the way "no such function" does: a mock/blocking provider doing a + // Dictionary lookup by name, for one, and ToolCallActionTests' NullIntolerantProvider + // already proves that shape exists. Guarding once here saves every caller from repeating + // the same check. + if (string.IsNullOrWhiteSpace(functionName)) + { + return null; + } + + // Give external providers first refusal. Order is stable (ascending Order), never the DI + // registration order. + var providers = _services.GetServices().OrderBy(x => x.Order); + foreach (var provider in providers) + { + var claimed = provider.TryResolve(functionName, agent); + if (claimed != null) + { + return claimed; + } + } + + // The three stages below are the pre-existing logic verbatim -- same order, same semantics. + var functionCall = _services.GetServices().FirstOrDefault(x => x.Name == functionName); if (functionCall != null) { return new FunctionCallbackExecutor(functionCall); @@ -17,15 +51,15 @@ internal class FunctionExecutorFactory var funcDef = functions.FirstOrDefault(x => x.Name == functionName); if (!string.IsNullOrWhiteSpace(funcDef?.Output)) { - return new DummyFunctionExecutor(services, funcDef); + return new DummyFunctionExecutor(_services, funcDef); } var mcpServerId = agent?.McpTools?.Where(x => x.Functions.Any(y => y.Name == funcDef?.Name))?.FirstOrDefault()?.ServerId; if (!string.IsNullOrWhiteSpace(mcpServerId)) { - return new McpToolExecutor(services, mcpServerId, functionName); + return new McpToolExecutor(_services, mcpServerId, functionName); } - + return null; } } diff --git a/src/Infrastructure/BotSharp.Core/Routing/RoutingPlugin.cs b/src/Infrastructure/BotSharp.Core/Routing/RoutingPlugin.cs index 2e409fcf3..c352ad641 100644 --- a/src/Infrastructure/BotSharp.Core/Routing/RoutingPlugin.cs +++ b/src/Infrastructure/BotSharp.Core/Routing/RoutingPlugin.cs @@ -1,6 +1,8 @@ +using BotSharp.Abstraction.Routing.Executor; using BotSharp.Abstraction.Routing.Reasoning; using BotSharp.Abstraction.Routing.Settings; using BotSharp.Abstraction.Settings; +using BotSharp.Core.Routing.Executor; using BotSharp.Core.Routing.Hooks; using BotSharp.Core.Routing.Reasoning; using Microsoft.Extensions.Configuration; @@ -31,6 +33,7 @@ public void RegisterDI(IServiceCollection services, IConfiguration config) }); services.AddScoped(); + services.AddScoped(); services.AddScoped(); services.AddScoped(); diff --git a/src/Infrastructure/BotSharp.Core/Routing/RoutingService.InvokeFunction.cs b/src/Infrastructure/BotSharp.Core/Routing/RoutingService.InvokeFunction.cs index dc6aeacb4..d319d282d 100644 --- a/src/Infrastructure/BotSharp.Core/Routing/RoutingService.InvokeFunction.cs +++ b/src/Infrastructure/BotSharp.Core/Routing/RoutingService.InvokeFunction.cs @@ -1,3 +1,4 @@ +using BotSharp.Abstraction.Routing.Executor; using BotSharp.Abstraction.Routing.Models; using BotSharp.Core.MessageHub; using BotSharp.Core.Routing.Executor; @@ -13,7 +14,7 @@ public async Task InvokeFunction(string name, RoleDialogModel message, Inv var agentService = _services.GetRequiredService(); var agent = await agentService.GetAgent(currentAgentId); - var funcExecutor = FunctionExecutorFactory.Create(_services, name, agent); + var funcExecutor = _services.GetRequiredService().Create(name, agent); if (funcExecutor == null) { message.StopCompletion = true; diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/AgentTestingPlugin.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/AgentTestingPlugin.cs new file mode 100644 index 000000000..865423073 --- /dev/null +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/AgentTestingPlugin.cs @@ -0,0 +1,89 @@ +using BotSharp.Abstraction.Agents; +using BotSharp.Abstraction.Repositories.Settings; +using BotSharp.Abstraction.Users.Enums; +using BotSharp.Plugin.AgentTesting.Repositories; +using BotSharp.Plugin.AgentTesting.Runtime; +using BotSharp.Plugin.AgentTesting.Services; + +namespace BotSharp.Plugin.AgentTesting; + +public class AgentTestingPlugin : IBotSharpPlugin +{ + public string Id => "5c1f4d38-9a2e-4b7c-8f61-2d0e7a9c4b13"; + public string Name => "Agent Testing"; + public string Description => "Per-agent regression test sets: scripted multi-turn cases, mocked tools, deterministic assertions."; + + /// + /// Without this the four pages exist but nothing links to them -- the suite list is only + /// reachable by typing the URL. Same shape as QaAutomationPlugin: sit under the "One Brain" + /// header when it is there, fall back to "Apps" (seeded by BotSharp.OpenAPI's + /// PluginController), and position with the section's own weight. + /// + /// Roles matches the gate that actually matters here: TriggerRun and RecordCase carry + /// [BotSharpAuth], which is Root/Admin only. The read endpoints are plain [Authorize], but a + /// menu entry leading to a page whose primary buttons 401 is worse than no entry. + /// + public bool AttachMenu(List menu) + { + var section = menu.FirstOrDefault(x => x.Label == "One Brain") + ?? menu.FirstOrDefault(x => x.Label == "Apps"); + + if (section != null) + { + menu.Add(new PluginMenuDef("Agent Testing", icon: "bx bx-test-tube", link: "page/agent-test", weight: section.Weight + 2) + { + Roles = new List { UserRole.Root, UserRole.Admin } + }); + } + + return true; + } + + public void RegisterDI(IServiceCollection services, IConfiguration config) + { + // Singleton: the test context has to be visible across requests and across threads. + services.AddSingleton(); + + // The seam that takes over function execution. Lose this line and mocking silently stops + // working -- the runner's canary self-check is what catches that. + services.AddScoped(); + + // The model-override seam, which lets one run sweep the same agent across several models. + // Losing this line does not raise an error: every model would run on the agent's own + // LlmConfig and the comparison grid would show them all behaving identically. + services.AddScoped(); + + services.AddScoped(); + + // Registered by its ICaseRunner interface rather than its concrete type, so that + // AgentTestRunQueue can substitute a wrapper that opens a fresh DI scope per call in + // production (see AgentTestRunQueue.ScopedCaseRunner). The behaviour itself -- multi-turn, + // canary, timeout -- is unchanged by that substitution. + services.AddScoped(); + + // The harness owns its four collections, so it also owns the Mongo connection to them. + // Singleton because MongoClient is thread-safe and holds a connection pool -- one per DI + // scope would open a fresh pool for every case a run executes. Bound here rather than taken + // from the container because BotSharpDatabaseSettings is not registered as a service on + // every host configuration, while `config` always is. + var dbSettings = new BotSharpDatabaseSettings(); + config.Bind("Database", dbSettings); + services.AddSingleton(new AgentTestMongoDbContext(dbSettings)); + + services.AddScoped(); + + // Task 9: no custom interface -- injected by its own concrete type into + // AgentTestController, so it needs an explicit registration the same way every other + // service in this method does (there is no auto-discovery mechanism in this plugin). + services.AddScoped(); + services.AddScoped(); + + // AgentTestRunQueue is both a singleton and a BackgroundService: all three lines point at + // the same instance, sharing one Channel. Same shape as WeChatBackgroundService elsewhere in + // this repo -- AddSingleton(), AddHostedService forwarding to that same instance, then + // the interface type forwarding to it as well. + services.AddSingleton(); + services.AddHostedService(s => s.GetRequiredService()); + services.AddSingleton(s => s.GetRequiredService()); + } +} diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/BotSharp.Plugin.AgentTesting.csproj b/src/Plugins/BotSharp.Plugin.AgentTesting/BotSharp.Plugin.AgentTesting.csproj new file mode 100644 index 000000000..842b5bc53 --- /dev/null +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/BotSharp.Plugin.AgentTesting.csproj @@ -0,0 +1,27 @@ + + + + $(TargetFramework) + $(LangVersion) + enable + enable + $(BotSharpVersion) + $(GeneratePackageOnBuild) + $(GenerateDocumentationFile) + $(SolutionDir)packages + + + + + + + + + + + + + + + + diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Controllers/AgentTestController.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Controllers/AgentTestController.cs new file mode 100644 index 000000000..1d6315c17 --- /dev/null +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Controllers/AgentTestController.cs @@ -0,0 +1,493 @@ +using System.Security.Claims; +using BotSharp.Abstraction.Agents; +using BotSharp.Abstraction.MLTasks; +using BotSharp.Abstraction.Infrastructures.Attributes; +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.ModelBinding; +using BotSharp.Plugin.AgentTesting.Models; +using BotSharp.Plugin.AgentTesting.Repositories; +using BotSharp.Plugin.AgentTesting.Services; + +namespace BotSharp.Plugin.AgentTesting.Controllers; + +/// +/// CRUD for suites and cases, triggering a run (asynchronous -- returns the runId immediately), +/// reading or cancelling a run, and the mock-target candidate list the case editor needs. All +/// literal absolute routes, all requiring authentication. +/// +[Authorize] +[ApiController] +[Route("agent-test")] +public class AgentTestController : ControllerBase +{ + private readonly IAgentTestRepository _repo; + private readonly IAgentTestRunQueue _queue; + private readonly IAgentService _agents; + private readonly AgentTestRecorder _recorder; + private readonly ILlmProviderService _llmProviders; + + public AgentTestController( + IAgentTestRepository repo, + IAgentTestRunQueue queue, + IAgentService agents, + AgentTestRecorder recorder, + ILlmProviderService llmProviders) + { + _repo = repo; + _queue = queue; + _agents = agents; + _recorder = recorder; + _llmProviders = llmProviders; + } + + [HttpGet("suites")] + public async Task> ListSuites([FromQuery] string? agentId) + => await _repo.ListSuitesAsync(agentId); + + [HttpPost("suites")] + public async Task CreateSuite([FromBody] AgentTestSuiteUpsertRequest request) + { + var suite = new AgentTestSuite(); + ApplySuite(suite, request); + + await _repo.UpsertSuiteAsync(suite); + return suite; + } + + [HttpGet("suites/{id}")] + public async Task> GetSuite(string id) + { + var suite = await _repo.GetSuiteAsync(id); + if (suite == null) + { + return NotFound($"agent test suite {id} not found"); + } + + return suite; + } + + [HttpPut("suites/{id}")] + public async Task> UpdateSuite(string id, [FromBody] AgentTestSuiteUpsertRequest request) + { + var suite = await _repo.GetSuiteAsync(id); + if (suite == null) + { + return NotFound($"agent test suite {id} not found"); + } + + // A blank agentId/name in the request means "leave it where it is," not "clear it" -- + // ApplySuite below copies every field unconditionally (matching CreateSuite's full-replace + // semantics), and without this fallback a PUT body that omits either field would silently + // blank it. Mirrors UpdateCase's SuiteId fallback below. + if (string.IsNullOrWhiteSpace(request.AgentId)) + { + request.AgentId = suite.AgentId; + } + if (string.IsNullOrWhiteSpace(request.Name)) + { + request.Name = suite.Name; + } + + ApplySuite(suite, request); + + await _repo.UpsertSuiteAsync(suite); + return suite; + } + + [HttpDelete("suites/{id}")] + public async Task DeleteSuite(string id) + { + var suite = await _repo.GetSuiteAsync(id); + if (suite == null) + { + return NotFound($"agent test suite {id} not found"); + } + + await _repo.DeleteSuiteAsync(id); + return Ok(); + } + + [HttpGet("cases")] + public async Task>> ListCases([FromQuery] string? suiteId) + { + if (string.IsNullOrWhiteSpace(suiteId)) + { + return BadRequest("suiteId is required"); + } + + return await _repo.ListCasesAsync(suiteId); + } + + [HttpPost("cases")] + public async Task> CreateCase([FromBody] AgentTestCaseUpsertRequest request) + { + if (string.IsNullOrWhiteSpace(request.SuiteId)) + { + return BadRequest("suiteId is required"); + } + + if (ValidateCasePayload(request) is { } validationError) + { + return BadRequest(validationError); + } + + var suite = await _repo.GetSuiteAsync(request.SuiteId); + if (suite == null) + { + return NotFound($"agent test suite {request.SuiteId} not found"); + } + + var testCase = new AgentTestCase(); + ApplyCase(testCase, request); + + await _repo.UpsertCaseAsync(testCase); + return testCase; + } + + [HttpGet("cases/{id}")] + public async Task> GetCase(string id) + { + var testCase = await _repo.GetCaseAsync(id); + if (testCase == null) + { + return NotFound($"agent test case {id} not found"); + } + + return testCase; + } + + [HttpPut("cases/{id}")] + public async Task> UpdateCase(string id, [FromBody] AgentTestCaseUpsertRequest request) + { + var testCase = await _repo.GetCaseAsync(id); + if (testCase == null) + { + return NotFound($"agent test case {id} not found"); + } + + if (ValidateCasePayload(request) is { } validationError) + { + return BadRequest(validationError); + } + + // A blank SuiteId in the request means "leave it where it is" -- ApplyCase below copies + // every field unconditionally (matching CreateCase's full-replace semantics), and without + // this fallback a PUT body that omits suiteId would silently clear it to string.Empty, + // orphaning the case from ListCasesAsync(suiteId) queries. Only re-validate existence when + // the target actually differs from what the case already points at. + var targetSuiteId = string.IsNullOrWhiteSpace(request.SuiteId) ? testCase.SuiteId : request.SuiteId; + if (targetSuiteId != testCase.SuiteId) + { + var suite = await _repo.GetSuiteAsync(targetSuiteId); + if (suite == null) + { + return NotFound($"agent test suite {targetSuiteId} not found"); + } + } + + request.SuiteId = targetSuiteId; + ApplyCase(testCase, request); + + await _repo.UpsertCaseAsync(testCase); + return testCase; + } + + [HttpDelete("cases/{id}")] + public async Task DeleteCase(string id) + { + var testCase = await _repo.GetCaseAsync(id); + if (testCase == null) + { + return NotFound($"agent test case {id} not found"); + } + + await _repo.DeleteCaseAsync(id); + return Ok(); + } + + /// + /// Records a draft case from a real conversation: real function returns become mocks, real + /// state deltas become StateWrites/InitialStates, and the stable assertions + /// (toolCalled/stateEquals) are generated -- so nobody has to hand-write a work order agent's + /// mock JSON. The draft is stored with Enabled = false and has to be reviewed and enabled by + /// hand before it joins a real run. + /// + /// [BotSharpAuth]: this endpoint copies the raw contents of a real conversation (potentially + /// phone numbers, addresses, tenant names) into the test store, and the conversationId comes + /// from the caller with no ownership check -- the highest PII-escalation risk on this whole + /// surface. Restricted to admin/root rather than any authenticated user being able to call it + /// against any conversation. + /// + [BotSharpAuth] + [HttpPost("record")] + public async Task>> RecordCase([FromBody] AgentTestRecordRequest request) + { + if (string.IsNullOrWhiteSpace(request.SuiteId) || string.IsNullOrWhiteSpace(request.ConversationId)) + { + return BadRequest("suiteId and conversationId are required"); + } + + // Same guard as TriggerRun: a model this host cannot run must fail here with a readable + // message, not deep inside the segmenter's completion call. + if (ValidateRequestedModels(request.Model == null ? null : [request.Model]) is { } modelError) + { + return BadRequest(modelError); + } + + var suite = await _repo.GetSuiteAsync(request.SuiteId); + if (suite == null) + { + return NotFound($"agent test suite {request.SuiteId} not found"); + } + + try + { + return await _recorder.LoadAndBuildManyAsync(request.SuiteId, request.ConversationId, request.Model); + } + catch (InvalidOperationException ex) + { + // The segmenter rejects its own model's output rather than silently cutting cases in + // the wrong place (see LlmCaseSegmenter.Parse). Surface that verbatim -- "the model + // left turns 3..5 uncovered" tells the caller to just retry, which a 500 would not. + return BadRequest($"AI extraction failed: {ex.Message}"); + } + } + + /// + /// [BotSharpAuth]: every trigger really calls the model and really spends token quota, with no + /// usage throttling anywhere -- a cost-escalation surface just like RecordCase, so it is + /// restricted to admin/root. + /// + [BotSharpAuth] + [HttpPost("suites/{id}/run")] + public async Task> TriggerRun( + string id, + [FromBody(EmptyBodyBehavior = EmptyBodyBehavior.Allow)] AgentTestRunTriggerRequest? request) + { + var suite = await _repo.GetSuiteAsync(id); + if (suite == null) + { + return NotFound($"agent test suite {id} not found"); + } + + // Reject an unknown provider/model here rather than letting every case in the run die + // deep inside model resolution. Measured: an unregistered model name surfaces as a bare + // "Object reference not set to an instance of an object." on each case result, which tells + // the author nothing about what they actually got wrong. + if (ValidateRequestedModels(request?.Models) is { } modelError) + { + return BadRequest(modelError); + } + + if (!suite.Enabled) + { + // The suite's own Enabled flag is advertised in the design as a way to turn a whole + // suite off; AgentTestCase.Enabled is honoured by the executor, but nothing previously + // checked the suite's own flag anywhere, so disabling a suite silently did nothing -- + // a caller (or a scheduled re-run) could still trigger it. Reject at the one place this + // whole path runs inside a real HTTP request, before a run row is ever created. + return BadRequest($"agent test suite {id} is disabled"); + } + + var run = new AgentTestRun + { + SuiteId = id, + Status = AgentTestStatus.Pending, + // Re-running only a caller-selected subset (e.g. "just the cases that failed last + // time") is core value for a regression harness -- AgentTestRunExecutor filters the + // suite's enabled cases down to this list when it's non-empty. + CaseIds = request?.CaseIds, + // Empty list and null mean the same thing downstream ("agent's own LlmConfig"), so + // normalise to null rather than persisting an empty array that reads like a choice. + Models = request?.Models is { Count: > 0 } ? request.Models : null, + // This controller action is the only place in the whole trigger-to-execute path that + // ever runs inside a real HTTP request -- AgentTestRunQueue's background loop has no + // HttpContext at all, so if the triggering identity isn't captured here it can never + // be recovered later. ClaimTypes.NameIdentifier matches what this codebase's own + // ICurrentUser/CurrentUserIdentity.Id already reads as "who is this" elsewhere + // (BusinessCore.Abstraction.CurrentUserIdentity), read directly off User here instead + // of taking on that whole service as a new dependency for one claim. + TriggeredBy = User.FindFirstValue(ClaimTypes.NameIdentifier) + }; + + await _repo.CreateRunAsync(run); + + // Drop the runId on the queue and return immediately -- nothing waits for it here. The + // actual execution happens in AgentTestRunQueue's background loop. + _queue.Enqueue(run.Id); + + return run; + } + + [HttpGet("runs")] + public async Task> ListRuns([FromQuery] string? suiteId) + => await _repo.ListRunsAsync(suiteId); + + [HttpGet("runs/{id}")] + public async Task> GetRun(string id) + { + var run = await _repo.GetRunAsync(id); + if (run == null) + { + return NotFound($"agent test run {id} not found"); + } + + var results = await _repo.ListCaseResultsAsync(id); + return new AgentTestRunDetailDto { Run = run, Results = results }; + } + + [HttpPost("runs/{id}/cancel")] + public async Task CancelRun(string id) + { + var run = await _repo.GetRunAsync(id); + if (run == null) + { + return NotFound($"agent test run {id} not found"); + } + + if (IsTerminalStatus(run.Status)) + { + // A run that already finished has nothing left to cancel; silently accepting this + // (the old behaviour) makes a stale/duplicate cancel click on a finished run look + // successful with no signal that it did nothing. + return Conflict($"agent test run {id} has already finished ({run.Status}) and cannot be cancelled"); + } + + run.CancelRequested = true; + await _repo.UpdateRunAsync(run); + return Ok(); + } + + [HttpGet("mock-targets")] + public async Task>> GetMockTargets([FromQuery] string? agentId) + { + if (string.IsNullOrWhiteSpace(agentId)) + { + return BadRequest("agentId is required"); + } + + var agent = await _agents.GetAgent(agentId); + if (agent == null) + { + return NotFound($"agent {agentId} not found"); + } + + var names = new List(); + names.AddRange((agent.Functions ?? []).Select(f => f.Name)); + names.AddRange((agent.SecondaryFunctions ?? []).Select(f => f.Name)); + names.AddRange((agent.McpTools ?? []).SelectMany(t => t.Functions ?? []).Select(f => f.Name)); + + return names + .Where(n => !string.IsNullOrWhiteSpace(n)) + .Distinct(StringComparer.OrdinalIgnoreCase) + .OrderBy(n => n, StringComparer.OrdinalIgnoreCase) + .ToList(); + } + + /// + /// Null when every requested model is registered (or none was requested); otherwise a + /// caller-facing 400 message naming the first offender. + /// + private string? ValidateRequestedModels(List? models) + { + if (models is not { Count: > 0 }) + { + return null; + } + + foreach (var model in models) + { + if (string.IsNullOrWhiteSpace(model.Provider) || string.IsNullOrWhiteSpace(model.Model)) + { + return "each entry in 'models' needs both a provider and a model"; + } + + if (_llmProviders.GetSetting(model.Provider, model.Model) == null) + { + return $"model '{model.Provider}/{model.Model}' is not registered; " + + "see GET /llm-configs for what this host can actually run"; + } + } + + // Two identical entries would run the same case twice under the same label and make the + // comparison grid collapse two results into one cell -- the second silently overwriting + // the first. + var duplicate = models + .GroupBy(m => $"{m.Provider}/{m.Model}", StringComparer.OrdinalIgnoreCase) + .FirstOrDefault(g => g.Count() > 1); + + return duplicate == null ? null : $"model '{duplicate.Key}' is listed more than once"; + } + + private static void ApplySuite(AgentTestSuite suite, AgentTestSuiteUpsertRequest request) + { + suite.AgentId = request.AgentId; + suite.Name = request.Name; + suite.Description = request.Description; + // request.Enabled is null when the request body omits "enabled" -- keep whatever the + // suite already had (== the entity's own default of true, for a brand-new suite created + // via CreateSuite's `new AgentTestSuite()`) rather than defaulting to false or, worse, + // silently re-enabling a suite someone had deliberately disabled via a partial PUT that + // only meant to change some other field. + suite.Enabled = request.Enabled ?? suite.Enabled; + suite.JudgeProvider = request.JudgeProvider; + suite.JudgeModel = request.JudgeModel; + suite.ExtraAllowedFunctions = request.ExtraAllowedFunctions ?? []; + suite.ForceBlockedFunctions = request.ForceBlockedFunctions ?? []; + suite.CaseTimeoutSeconds = request.CaseTimeoutSeconds; + } + + private static void ApplyCase(AgentTestCase testCase, AgentTestCaseUpsertRequest request) + { + testCase.SuiteId = request.SuiteId; + testCase.Name = request.Name; + testCase.Enabled = request.Enabled; + testCase.Turns = request.Turns ?? []; + testCase.Assertions = request.Assertions ?? []; + testCase.InitialStates = request.InitialStates ?? []; + testCase.Mocks = request.Mocks ?? []; + testCase.UnmockedToolPolicy = request.UnmockedToolPolicy; + testCase.SourceConversationId = request.SourceConversationId; + } + + /// + /// Shared create/update validation for a case payload. Null means the payload is acceptable; + /// otherwise the string is a caller-facing 400 message. + /// + private static string? ValidateCasePayload(AgentTestCaseUpsertRequest request) + { + if (IsUnsupportedUnmockedToolPolicy(request.UnmockedToolPolicy)) + { + return "Passthrough is not supported in P1"; + } + + var allAssertions = (request.Turns ?? []) + .SelectMany(t => t.Assertions ?? []) + .Concat(request.Assertions ?? []); + + foreach (var assertion in allAssertions) + { + var error = AssertionValidation.Validate(assertion); + if (error != null) + { + return error; + } + } + + return null; + } + + /// + /// Passthrough was specified in the design/plan and even had a (dead) code path, but nothing + /// ever back-fills an ObservedToolCall for a tool the provider let run for real -- under it, + /// toolNotCalled always vacuously passed against a tool that genuinely executed with real side + /// effects. Rejected here rather than implementing the back-fill (project owner decision). + /// + private static bool IsUnsupportedUnmockedToolPolicy(string? policy) + => string.Equals(policy, "Passthrough", StringComparison.OrdinalIgnoreCase); + + private static bool IsTerminalStatus(string status) => + status is AgentTestStatus.Passed or AgentTestStatus.Failed + or AgentTestStatus.Error or AgentTestStatus.Cancelled; +} diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestCase.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestCase.cs new file mode 100644 index 000000000..1bc7db5cb --- /dev/null +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestCase.cs @@ -0,0 +1,116 @@ +using MongoDB.Bson.Serialization.Attributes; + +namespace BotSharp.Plugin.AgentTesting.Models; + +public class AgentTestCase : MongoBase +{ + public string SuiteId { get; set; } = default!; + public string Name { get; set; } = default!; + public bool Enabled { get; set; } = true; + + /// A length of 1 is a single-turn case. + public List Turns { get; set; } = []; + + /// Case-level assertions, evaluated once every turn has run. + public List Assertions { get; set; } = []; + + /// Injected before the conversation starts; maps to BotSharp's MessageState. + public List InitialStates { get; set; } = []; + + public List Mocks { get; set; } = []; + + /// + /// See . Blocks by default: better a failing case than a + /// real tool call. + /// + public string UnmockedToolPolicy { get; set; } = UnmockedToolPolicies.Block; + + /// The conversation this was recorded from, for traceability; null when hand-written. + public string? SourceConversationId { get; set; } + + public DateTime CreateDate { get; set; } = DateTime.UtcNow; + public DateTime UpdateDate { get; set; } = DateTime.UtcNow; +} + +[BsonIgnoreExtraElements(Inherited = true)] +public class TestTurn +{ + public int Index { get; set; } + public string UserMessage { get; set; } = default!; + public List Assertions { get; set; } = []; +} + +[BsonIgnoreExtraElements(Inherited = true)] +public class TestState +{ + public string Key { get; set; } = default!; + public string Value { get; set; } = default!; + public int ActiveRounds { get; set; } = -1; + public bool Global { get; set; } +} + +[BsonIgnoreExtraElements(Inherited = true)] +public class TestToolMock +{ + public string FunctionName { get; set; } = default!; + + /// + /// Optional argument-subset match, for giving different returns to repeated calls of the same + /// tool. + /// + public string? ArgsMatchJson { get; set; } + + /// Optional: match only the Nth call (0-based). + public int? CallIndex { get; set; } + + /// The faked return, written to message.Content. + public string ResultContent { get; set; } = string.Empty; + + /// Reproduces a real tool's "stop this turn's LLM completion" behaviour. + public bool StopCompletion { get; set; } + + /// + /// A mock has to be able to write conversation state too. Plenty of IFunctionCallback + /// implementations ignore the LLM's arguments entirely and pass data across turns purely + /// through IConversationStateService, so mocking only the return value leaves every later + /// function unable to read what it expects and the whole case collapses. + /// + public List? StateWrites { get; set; } +} + +[BsonIgnoreExtraElements(Inherited = true)] +public class TestAssertion +{ + /// outputContains|outputNotContains|outputRegex|toolCalled|toolNotCalled|stateEquals|routedToAgent|llmJudge + public string Type { get; set; } = default!; + + /// Function name / state key / agent name. + public string? Target { get; set; } + + /// Expected value / regex / judging criteria. + public string? Expected { get; set; } + + /// Argument-subset match for toolCalled. + public string? ArgsMatchJson { get; set; } + + /// Pass threshold for llmJudge. + public double? MinScore { get; set; } + + /// On failure, abort the remaining turns of this case. + public bool Fatal { get; set; } +} + +public static class UnmockedToolPolicies +{ + public const string Block = "Block"; +} + +public static class AgentTestStatus +{ + public const string Pending = "Pending"; + public const string Running = "Running"; + public const string Passed = "Passed"; + public const string Failed = "Failed"; + public const string Error = "Error"; + public const string Cancelled = "Cancelled"; +} diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestCaseResult.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestCaseResult.cs new file mode 100644 index 000000000..bea9793ed --- /dev/null +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestCaseResult.cs @@ -0,0 +1,75 @@ +using MongoDB.Bson.Serialization.Attributes; + +namespace BotSharp.Plugin.AgentTesting.Models; + +public class AgentTestCaseResult : MongoBase +{ + public string RunId { get; set; } = default!; + public string CaseId { get; set; } = default!; + public string CaseName { get; set; } = default!; + + /// Passed | Failed | Error | Cancelled -- see . + public string Status { get; set; } = AgentTestStatus.Pending; + + /// The conversation this execution created; live conversations are never reused. + public string? ConversationId { get; set; } + + /// + /// Which model produced this result. Null = the agent's own LlmConfig was used (the run named + /// no models). Unlike the deleted AgentTestRunTriggerRequest.Provider/Model, these values were + /// genuinely applied to this execution by AgentTestModelOverrideHook -- they are not a + /// decorative record of something that never took effect. + /// + public string? Provider { get; set; } + public string? Model { get; set; } + + public long DurationMs { get; set; } + + /// + /// Infrastructure-level reason for failure (a timeout, a dead canary), kept distinct from an + /// assertion failure. + /// + public string? Error { get; set; } + + public List Turns { get; set; } = []; + + /// Case-level assertion results. + public List Assertions { get; set; } = []; + + public List ObservedToolCalls { get; set; } = []; + + public DateTime CreateDate { get; set; } = DateTime.UtcNow; +} + +[BsonIgnoreExtraElements(Inherited = true)] +public class TurnResult +{ + public int Index { get; set; } + public string UserMessage { get; set; } = default!; + public string? Output { get; set; } + public List Assertions { get; set; } = []; +} + +[BsonIgnoreExtraElements(Inherited = true)] +public class AssertionResult +{ + public string Type { get; set; } = default!; + public string? Target { get; set; } + public string? Expected { get; set; } + public string? Actual { get; set; } + public bool Passed { get; set; } + public string? Message { get; set; } +} + +[BsonIgnoreExtraElements(Inherited = true)] +public class ObservedToolCall +{ + public int TurnIndex { get; set; } + public string FunctionName { get; set; } = default!; + public string? ArgsJson { get; set; } + + /// Mocked | Blocked + public string Outcome { get; set; } = default!; + + public string? ResultContent { get; set; } +} diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestDtos.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestDtos.cs new file mode 100644 index 000000000..e8b6c2b9a --- /dev/null +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestDtos.cs @@ -0,0 +1,113 @@ +namespace BotSharp.Plugin.AgentTesting.Models; + +/// +/// Body for creating/updating a suite via POST/PUT. Id/CreateDate/UpdateDate are server-owned and +/// deliberately absent: the repository generates them on create, and on update the controller +/// carries them over from the stored entity. +/// +public class AgentTestSuiteUpsertRequest +{ + public string AgentId { get; set; } = string.Empty; + public string Name { get; set; } = string.Empty; + public string? Description { get; set; } + + /// + /// Nullable so a request that omits "enabled" (a partial PUT, e.g. one that only means to + /// change caseTimeoutSeconds) can be told apart from one that explicitly sends + /// "enabled": true -- a non-nullable bool defaulting to true made both cases look + /// identical, so a partial PUT against a suite someone had deliberately disabled silently + /// re-enabled it (see AgentTestController.ApplySuite, which now does + /// request.Enabled ?? suite.Enabled). Null on create still means "enabled" -- a brand + /// new AgentTestSuite's own Enabled default (true) is what the null falls back to there. + /// + public bool? Enabled { get; set; } + public string? JudgeProvider { get; set; } + public string? JudgeModel { get; set; } + public List ExtraAllowedFunctions { get; set; } = []; + public List ForceBlockedFunctions { get; set; } = []; + public int CaseTimeoutSeconds { get; set; } = 120; +} + +/// +/// Body for creating/updating a case via POST/PUT; the fields map straight onto the writable part +/// of AgentTestCase. +/// +public class AgentTestCaseUpsertRequest +{ + public string SuiteId { get; set; } = string.Empty; + public string Name { get; set; } = string.Empty; + public bool Enabled { get; set; } = true; + public List Turns { get; set; } = []; + public List Assertions { get; set; } = []; + public List InitialStates { get; set; } = []; + public List Mocks { get; set; } = []; + public string UnmockedToolPolicy { get; set; } = UnmockedToolPolicies.Block; + public string? SourceConversationId { get; set; } +} + +/// +/// Body of POST /agent-test/record -- record a draft case from a real conversation, see +/// . +/// +public class AgentTestRecordRequest +{ + public string SuiteId { get; set; } = string.Empty; + public string ConversationId { get; set; } = string.Empty; + + /// + /// Optional: use this model to split the conversation into one or more scenarios, each becoming + /// its own draft case. Null calls no model at all and falls back to the deterministic recorder + /// (whole conversation = one case). + /// + /// The model ONLY decides where to split and what to name each case. Mock return values, + /// toolCalled/stateEquals assertions and state writes still come verbatim from the real + /// conversation -- see ICaseSegmenter for why that boundary is drawn there. + /// + /// Note that setting this sends the conversation's user messages and tool names to that model + /// vendor (tool arguments and results are withheld). The deterministic recorder never leaves + /// this system, so using this field means accepting one data egress. + /// + public TestModel? Model { get; set; } +} + +/// +/// Body of POST /agent-test/suites/{id}/run. +/// +/// CaseIds lands on AgentTestRun.CaseIds, which AgentTestRunExecutor uses to narrow the suite's +/// enabled cases further (null/empty = no filter, run every enabled case, identical to the field not +/// existing). "Re-run just the ones that failed" is a core regression-harness scenario, not a +/// nice-to-have. +/// +/// Fix wave (project owner decision): this used to also accept Provider/Model, stored verbatim +/// on AgentTestRun.Provider/Model for "which model did this run use" auditing -- but nothing ever +/// read them back to actually apply a model override to execution (the run always executed with +/// the agent's own LlmConfig regardless of what was passed here). A permanent record that +/// CLAIMS a model was used, when it wasn't, is worse than no field at all, so both fields were +/// deleted rather than left to silently lie. Implementing a real override would need a channel +/// through IAgentConversationDriver, which is out of scope here. +/// +public class AgentTestRunTriggerRequest +{ + public List? CaseIds { get; set; } + + /// + /// Models to sweep for this run; null/empty = one pass on the agent's own LlmConfig (the + /// existing behaviour). + /// + /// This is not a revival of the two deleted Provider/Model fields above: those were stored and + /// never read back, so they lied. This one genuinely takes effect -- + /// AgentTestModelOverrideHook rewrites agent.LlmConfig to the named provider/model as the agent + /// loads, so the main conversation path (RoutingService.InvokeAgent) reads the rewritten value, + /// and every AgentTestCaseResult records which model produced it. + /// + public List? Models { get; set; } +} + +/// +/// Body of GET /agent-test/runs/{id}: one run plus every AgentTestCaseResult belonging to it. +/// +public class AgentTestRunDetailDto +{ + public AgentTestRun Run { get; set; } = default!; + public List Results { get; set; } = []; +} diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestMongoDbContext.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestMongoDbContext.cs new file mode 100644 index 000000000..5529efbb2 --- /dev/null +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestMongoDbContext.cs @@ -0,0 +1,44 @@ +using BotSharp.Abstraction.Repositories.Settings; +using MongoDB.Driver; + +namespace BotSharp.Plugin.AgentTesting.Models; + +/// +/// The four collections this harness owns. Mirrors BotSharp.Plugin.MongoStorage.MongoDbContext: +/// same connection-string setting (Database:BotSharpMongoDb), same "derive the database name +/// from the connection string" rule, same TablePrefix convention -- so a host that already has Mongo +/// storage configured needs no additional configuration for the test set. +/// +/// Kept separate from BotSharp's own storage abstraction on purpose. Adding these collections to +/// IBotSharpRepository would mean ~20 new members that FileRepository and BotSharpDbContext would +/// each have to implement, for data no other feature reads. +/// +public class AgentTestMongoDbContext +{ + private const string DefaultTablePrefix = "BotSharp"; + + private readonly IMongoDatabase _database; + private readonly string _collectionPrefix; + + public AgentTestMongoDbContext(BotSharpDatabaseSettings dbSettings) + { + var connectionString = dbSettings?.BotSharpMongoDb; + if (string.IsNullOrWhiteSpace(connectionString)) + { + throw new InvalidOperationException( + "The agent testing plugin needs Database:BotSharpMongoDb to be configured."); + } + + var url = new MongoUrl(connectionString); + var databaseName = string.IsNullOrEmpty(url.DatabaseName) ? url.AuthenticationSource : url.DatabaseName; + _database = new MongoClient(connectionString).GetDatabase(databaseName); + _collectionPrefix = string.IsNullOrEmpty(dbSettings?.TablePrefix) ? DefaultTablePrefix : dbSettings.TablePrefix; + } + + private IMongoCollection Collection(string name) => _database.GetCollection($"{_collectionPrefix}_{name}"); + + public IMongoCollection AgentTestSuites => Collection("AgentTestSuites"); + public IMongoCollection AgentTestCases => Collection("AgentTestCases"); + public IMongoCollection AgentTestRuns => Collection("AgentTestRuns"); + public IMongoCollection AgentTestCaseResults => Collection("AgentTestCaseResults"); +} diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestRun.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestRun.cs new file mode 100644 index 000000000..31b3f6af5 --- /dev/null +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestRun.cs @@ -0,0 +1,65 @@ +using MongoDB.Bson.Serialization.Attributes; + +namespace BotSharp.Plugin.AgentTesting.Models; + +/// +/// One model to run against. Absent/empty means "use the agent's own LlmConfig" -- which was the +/// only behaviour before multi-model existed, so a historical Run document without this field keeps +/// its original meaning and needs no migration. +/// +[BsonIgnoreExtraElements(Inherited = true)] +public class TestModel +{ + public string Provider { get; set; } = default!; + public string Model { get; set; } = default!; + + /// "provider/model" -- used for logging and for de-duplicating result columns. + public override string ToString() => $"{Provider}/{Model}"; +} + +public class AgentTestRun : MongoBase +{ + public string SuiteId { get; set; } = default!; + + /// See . + public string Status { get; set; } = AgentTestStatus.Pending; + + public string? TriggeredBy { get; set; } + + /// + /// Run only these case ids; null/empty means every enabled case in the suite (the original + /// behaviour). This is what makes "re-run just the failures" -- a core regression-harness + /// scenario -- possible. Mongo is schemaless, so no migration was needed to add it. + /// + public List? CaseIds { get; set; } + + /// + /// Models this run sweeps. Null/empty = a single pass on the agent's own LlmConfig (the + /// behaviour from before multi-model). When set, the executor runs the cartesian product of + /// cases x models and every AgentTestCaseResult records which model produced it -- so + /// TotalCount is "cases x models" and no longer equals the case count. + /// + public List? Models { get; set; } + + public int TotalCount { get; set; } + public int PassedCount { get; set; } + public int FailedCount { get; set; } + public int ErrorCount { get; set; } + + /// + /// Why a run ended as -- an infrastructure stop that + /// happened before or instead of executing cases (suite gone, suite disabled, the CaseIds + /// filter matched nothing, the host died mid-run, an unhandled exception). + /// + /// Distinct from AgentTestCaseResult.Error, which explains one case. A run can fail with ZERO + /// case results, and until this field existed the reason lived only in the server log: the API + /// returned status=Error with 0/0/0/0 and an empty result list, so no UI could ever say why. + /// + public string? Error { get; set; } + + public bool CancelRequested { get; set; } + + public DateTime? StartedAt { get; set; } + public DateTime? CompletedAt { get; set; } + public DateTime CreateDate { get; set; } = DateTime.UtcNow; +} diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestSuite.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestSuite.cs new file mode 100644 index 000000000..ce511d220 --- /dev/null +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestSuite.cs @@ -0,0 +1,27 @@ +namespace BotSharp.Plugin.AgentTesting.Models; + +public class AgentTestSuite : MongoBase +{ + public string AgentId { get; set; } = default!; + public string Name { get; set; } = default!; + public string? Description { get; set; } + public bool Enabled { get; set; } = true; + + /// + /// Model used by llmJudge. When unconfigured, llmJudge assertions fail outright rather than + /// passing silently. + /// + public string? JudgeProvider { get; set; } + public string? JudgeModel { get; set; } + + /// Functions let through on top of the default control-flow allow list. + public List ExtraAllowedFunctions { get; set; } = []; + + /// Functions blocked outright; wins over the allow list. + public List ForceBlockedFunctions { get; set; } = []; + + public int CaseTimeoutSeconds { get; set; } = 120; + + public DateTime CreateDate { get; set; } = DateTime.UtcNow; + public DateTime UpdateDate { get; set; } = DateTime.UtcNow; +} diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Models/MongoBase.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Models/MongoBase.cs new file mode 100644 index 000000000..8ed8aa7bf --- /dev/null +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Models/MongoBase.cs @@ -0,0 +1,25 @@ +using MongoDB.Bson.Serialization; +using MongoDB.Bson.Serialization.Attributes; + +namespace BotSharp.Plugin.AgentTesting.Models; + +/// +/// Deliberately a local copy of BotSharp.Plugin.MongoStorage's own MongoBase/StringGuidIdGenerator +/// pair rather than a reference to that plugin. Two plugins referencing each other would mean this +/// harness cannot be enabled without also enabling Mongo storage, which is not a real dependency -- +/// the test set keeps its own four collections and does not touch BotSharp's storage backend at all. +/// Eight lines of duplication is the cheaper side of that trade. +/// +[BsonIgnoreExtraElements(Inherited = true)] +public abstract class MongoBase +{ + [BsonId(IdGenerator = typeof(StringGuidIdGenerator))] + public string Id { get; set; } = default!; +} + +public class StringGuidIdGenerator : IIdGenerator +{ + public object GenerateId(object container, object document) => Guid.NewGuid().ToString(); + + public bool IsEmpty(object id) => id == null || string.IsNullOrEmpty(id.ToString()); +} diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Repositories/AgentTestRepository.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Repositories/AgentTestRepository.cs new file mode 100644 index 000000000..d2dcbeb88 --- /dev/null +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Repositories/AgentTestRepository.cs @@ -0,0 +1,172 @@ +using MongoDB.Driver; + +namespace BotSharp.Plugin.AgentTesting.Repositories; + +/// +/// The Mongo repository contract for this plugin's four document types (Suite/Case/Run/CaseResult). +/// The signatures -- in particular which parameters are string? rather than string -- match the +/// InMemoryRepo fake used by the tests, which is the specification this interface follows. +/// +public interface IAgentTestRepository +{ + Task GetSuiteAsync(string id); + Task> ListSuitesAsync(string? agentId); + Task UpsertSuiteAsync(AgentTestSuite suite); + Task DeleteSuiteAsync(string id); + + Task GetCaseAsync(string id); + Task> ListCasesAsync(string suiteId); + Task UpsertCaseAsync(AgentTestCase testCase); + Task DeleteCaseAsync(string id); + + Task CreateRunAsync(AgentTestRun run); + Task GetRunAsync(string id); + Task> ListRunsAsync(string? suiteId); + + /// + /// Bounded counterpart to for the startup reconciliation sweep + /// (AgentTestRunQueue.ReconcileStaleRunningRunsAsync), which only ever cares about one status + /// (AgentTestStatus.Running) across every suite -- ListRunsAsync(null) would otherwise pull + /// the entire, ever-growing AgentTestRuns collection into memory on every host startup just to + /// filter it down to a handful of rows client-side. + /// + Task> ListRunsByStatusAsync(string status); + + Task UpdateRunAsync(AgentTestRun run); + + Task AddCaseResultAsync(AgentTestCaseResult result); + Task> ListCaseResultsAsync(string runId); +} + +public class AgentTestRepository : IAgentTestRepository +{ + private readonly AgentTestMongoDbContext _mongoDbContext; + + public AgentTestRepository(AgentTestMongoDbContext mongoDbContext) + { + _mongoDbContext = mongoDbContext; + } + + public async Task GetSuiteAsync(string id) + => await _mongoDbContext.AgentTestSuites.Find(x => x.Id == id).FirstOrDefaultAsync(); + + public async Task> ListSuitesAsync(string? agentId) + { + var filter = string.IsNullOrWhiteSpace(agentId) + ? Builders.Filter.Empty + : Builders.Filter.Eq(x => x.AgentId, agentId); + + return await _mongoDbContext.AgentTestSuites + .Find(filter) + .SortByDescending(x => x.CreateDate) + .ToListAsync(); + } + + public async Task UpsertSuiteAsync(AgentTestSuite suite) + { + // ReplaceOneAsync(upsert:true) does not run the [BsonId(IdGenerator=...)] hook the way + // InsertOneAsync does -- a brand-new document with a null/empty Id must get a real one + // here, or the driver would send an upsert whose replacement document has no _id at all + // (see the same fix already applied in UnableToValidateJobMongoRepository.UpsertAsync). + if (string.IsNullOrEmpty(suite.Id)) + { + suite.Id = Guid.NewGuid().ToString(); + } + + suite.UpdateDate = DateTime.UtcNow; + + await _mongoDbContext.AgentTestSuites.ReplaceOneAsync( + x => x.Id == suite.Id, + suite, + new ReplaceOptions { IsUpsert = true }); + } + + public async Task DeleteSuiteAsync(string id) + => await _mongoDbContext.AgentTestSuites.DeleteOneAsync(x => x.Id == id); + + public async Task GetCaseAsync(string id) + => await _mongoDbContext.AgentTestCases.Find(x => x.Id == id).FirstOrDefaultAsync(); + + public async Task> ListCasesAsync(string suiteId) + => await _mongoDbContext.AgentTestCases + .Find(x => x.SuiteId == suiteId) + .SortByDescending(x => x.CreateDate) + .ToListAsync(); + + public async Task UpsertCaseAsync(AgentTestCase testCase) + { + if (string.IsNullOrEmpty(testCase.Id)) + { + testCase.Id = Guid.NewGuid().ToString(); + } + + testCase.UpdateDate = DateTime.UtcNow; + + await _mongoDbContext.AgentTestCases.ReplaceOneAsync( + x => x.Id == testCase.Id, + testCase, + new ReplaceOptions { IsUpsert = true }); + } + + public async Task DeleteCaseAsync(string id) + => await _mongoDbContext.AgentTestCases.DeleteOneAsync(x => x.Id == id); + + public async Task CreateRunAsync(AgentTestRun run) + { + // InsertOneAsync DOES run the StringGuidIdGenerator hook for a null/empty Id, unlike the + // upsert path above -- but setting it explicitly here as well costs nothing and means the + // caller (the controller) can read back run.Id immediately after this call returns, before + // the insert has even happened, e.g. to log it. + if (string.IsNullOrEmpty(run.Id)) + { + run.Id = Guid.NewGuid().ToString(); + } + + await _mongoDbContext.AgentTestRuns.InsertOneAsync(run); + return run; + } + + public async Task GetRunAsync(string id) + => await _mongoDbContext.AgentTestRuns.Find(x => x.Id == id).FirstOrDefaultAsync(); + + public async Task> ListRunsAsync(string? suiteId) + { + var filter = string.IsNullOrWhiteSpace(suiteId) + ? Builders.Filter.Empty + : Builders.Filter.Eq(x => x.SuiteId, suiteId); + + return await _mongoDbContext.AgentTestRuns + .Find(filter) + .SortByDescending(x => x.CreateDate) + .ToListAsync(); + } + + public async Task> ListRunsByStatusAsync(string status) + => await _mongoDbContext.AgentTestRuns + .Find(x => x.Status == status) + .ToListAsync(); + + public async Task UpdateRunAsync(AgentTestRun run) + => await _mongoDbContext.AgentTestRuns.ReplaceOneAsync(x => x.Id == run.Id, run); + + public async Task AddCaseResultAsync(AgentTestCaseResult result) + { + if (string.IsNullOrEmpty(result.Id)) + { + result.Id = Guid.NewGuid().ToString(); + } + + await _mongoDbContext.AgentTestCaseResults.InsertOneAsync(result); + } + + public async Task> ListCaseResultsAsync(string runId) + // Ascending, deliberately unlike every other List*Async method in this file: those are + // "list of independent rows for an admin screen," newest-first. This one is "the case + // results FOR ONE RUN," where the natural read order is execution order -- newest-first + // would show a run's cases back-to-front, and ties within the same millisecond would sort + // nondeterministically either way. + => await _mongoDbContext.AgentTestCaseResults + .Find(x => x.RunId == runId) + .SortBy(x => x.CreateDate) + .ToListAsync(); +} diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Runtime/AgentTestModelOverrideHook.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Runtime/AgentTestModelOverrideHook.cs new file mode 100644 index 000000000..2891ea8a1 --- /dev/null +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Runtime/AgentTestModelOverrideHook.cs @@ -0,0 +1,71 @@ +using BotSharp.Abstraction.Agents; +using BotSharp.Abstraction.Agents.Settings; + +namespace BotSharp.Plugin.AgentTesting.Runtime; + +/// +/// Lets one test run force a specific model, which is what makes "sweep the same agent across +/// several models and compare them" possible. +/// +/// Why an agent-load hook and not somewhere else: the main conversation path, +/// RoutingService.InvokeAgent, reads agent.LlmConfig.Provider/Model and passes them +/// EXPLICITLY to CompletionProvider.GetChatCompletion; and +/// CompletionProvider.GetProviderAndModel only consults the provider/model +/// conversation-state override when what it was passed is empty. In other words, writing those two +/// keys into conversation state has no effect on the main path -- the only moment left where an +/// override still takes is after LoadAgent has produced the agent and before InvokeAgent runs. +/// +/// Structurally identical to TestMockExecutorProvider: look the conversation up in the registry, act +/// only on a hit, leave every other conversation untouched. No AsyncLocal, for the same reason as +/// MockFunctionExecutor (it is silently lost across a background queue or SideCar boundary). +/// +/// SelfId is overridden to the empty string: AgentHookBase's default implementation throws, and this +/// hook has to apply to ANY agent under test -- including downstream agents a test conversation +/// reaches via route_to_agent. Those must run on the requested model too, otherwise a model +/// comparison only swaps the entry agent and the results mean nothing. +/// +public class AgentTestModelOverrideHook : AgentHookBase +{ + private readonly IAgentTestRunRegistry _registry; + private readonly IConversationService _conversations; + private readonly ILogger _logger; + + public override string SelfId => string.Empty; + + public AgentTestModelOverrideHook( + IServiceProvider services, + AgentSettings settings, + IAgentTestRunRegistry registry, + IConversationService conversations, + ILogger logger) + : base(services, settings) + { + _registry = registry; + _conversations = conversations; + _logger = logger; + } + + public override Task OnAgentLoaded(Agent agent) + { + var active = _registry.TryGet(_conversations.ConversationId); + var over = active?.ModelOverride; + if (over == null || agent == null) + { + // Not a conversation under test, or this run named no model: touch nothing. + return Task.CompletedTask; + } + + // LlmConfig can be null (an agent.json with no llmConfig block) and the override still has + // to apply there -- otherwise the agents that most need to be told which model to use are + // exactly the ones it silently skips. + agent.LlmConfig ??= new AgentLlmConfig(); + agent.LlmConfig.Provider = over.Provider; + agent.LlmConfig.Model = over.Model; + + _logger.LogDebug( + "Agent test run overrode agent {AgentId} to {Provider}/{Model} for conversation {ConversationId}.", + agent.Id, over.Provider, over.Model, active!.ConversationId); + + return Task.CompletedTask; + } +} diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Runtime/AgentTestRunRegistry.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Runtime/AgentTestRunRegistry.cs new file mode 100644 index 000000000..53e288a05 --- /dev/null +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Runtime/AgentTestRunRegistry.cs @@ -0,0 +1,146 @@ +using System.Collections.Concurrent; + +namespace BotSharp.Plugin.AgentTesting.Runtime; + +/// +/// One test case currently executing. Indexed by conversationId because the test context has to be +/// reliable across threads: AsyncLocal is silently lost across a background-queue or SideCar +/// boundary, and losing it means real tools get executed. +/// +public class ActiveTestRun +{ + public string ConversationId { get; set; } = default!; + public string CaseId { get; set; } = default!; + + /// + /// The model this execution is forced onto; null = no override, use the agent's own LlmConfig. + /// + /// Applied by AgentTestModelOverrideHook, writing into agent.LlmConfig at the moment the agent + /// finishes loading. That moment is the only one that works: the main conversation path, + /// RoutingService.InvokeAgent, passes agent.LlmConfig.Provider/Model to CompletionProvider + /// EXPLICITLY, and CompletionProvider.GetProviderAndModel only consults the conversation-state + /// override when what it was passed is empty. So writing provider/model into conversation state + /// has no effect on the main path, and overriding any later than agent load is already too late. + /// + public TestModel? ModelOverride { get; set; } + + public IReadOnlyList Mocks { get; set; } = []; + public string UnmockedToolPolicy { get; set; } = UnmockedToolPolicies.Block; + public ISet AllowedFunctions { get; set; } = new HashSet(StringComparer.OrdinalIgnoreCase); + public ISet ForceBlockedFunctions { get; set; } = new HashSet(StringComparer.OrdinalIgnoreCase); + + /// Which turn is currently running, so tool calls can be attributed to a turn. + public int CurrentTurnIndex { get; set; } + + /// + /// Whether the canary was intercepted. At run time the verdict does NOT come from this flag but + /// from the content the canary call returns (see AgentTestCaseRunner); the flag exists so a test + /// can assert directly that MockFunctionExecutor recognises the canary function name. + /// + public bool CanaryIntercepted { get; set; } + + private readonly List _observed = []; + private readonly ConcurrentDictionary _ordinals = new(StringComparer.OrdinalIgnoreCase); + + public IReadOnlyList ObservedCalls + { + get { lock (_observed) return _observed.ToList(); } + } + + public void Record(ObservedToolCall call) + { + lock (_observed) _observed.Add(call); + } + + /// + /// Which call this is for a given function name (0-based), for matching TestToolMock.CallIndex. + /// + public int NextCallOrdinal(string functionName) + => _ordinals.AddOrUpdate(functionName, 0, (_, prev) => prev + 1); +} + +public interface IAgentTestRunRegistry +{ + void Register(ActiveTestRun run); + void Unregister(string conversationId); + ActiveTestRun? TryGet(string? conversationId); +} + +public class AgentTestRunRegistry : IAgentTestRunRegistry +{ + private readonly ConcurrentDictionary _runs = new(); + + public void Register(ActiveTestRun run) => _runs[run.ConversationId] = run; + + public void Unregister(string conversationId) => _runs.TryRemove(conversationId, out _); + + public ActiveTestRun? TryGet(string? conversationId) + => string.IsNullOrEmpty(conversationId) ? null + : _runs.TryGetValue(conversationId, out var run) ? run : null; +} + +/// +/// Control-flow functions allowed through by default. **Do not turn this into a `util-` prefix +/// match**: `util-email-handle_email_sender`, `util-twilio-outbound_phone_call`, +/// `util-twilio-text_message`, `util-http-handle_http_request` and `util-db-sql_select` all start +/// with `util-` and all have real side effects. Allowing by prefix means one test run really sends +/// the emails and really places the calls. +/// +public static class ControlFlowFunctions +{ + public static readonly IReadOnlySet Default = new HashSet(StringComparer.OrdinalIgnoreCase) + { + "route_to_agent", + "response_to_user", + "human_intervention_needed", + "util-routing-fallback_to_router", + "util-instruct-execute_template" + }; +} + +public static class AgentTestCanary +{ + /// + /// The runner calls this function name once before starting, to prove the seam is actually + /// live. If BotSharp.Core resolves to an older package without IFunctionExecutorProvider + /// support, mocking fails silently -- the canary turns that into an explicit failure. + /// + public const string FunctionName = "__agent_test_canary__"; + + /// + /// The one sentinel value written by the interceptor (MockFunctionExecutor) and compared by the + /// verifier (BotSharpAgentConversationDriver). Keeping a bare "canary" literal in both places + /// used to be a hazard in this safety-critical check -- drifting apart would only ever make + /// every case report Error rather than pass silently, but there should still be exactly one + /// definition. + /// + public const string ExpectedContent = "canary"; +} + +/// +/// Marks every conversation this harness creates as synthetic, so an operator who stumbles on a +/// strange conversation in the admin UI (or an analytics job aggregating over the collection) can +/// tell it apart from a genuine customer conversation. AgentTestCaseResult.ConversationId already +/// gives forward traceability (result -> conversation); this is the reverse direction. +/// +/// Deliberately just ONE marker (the tag), not a Channel/state-key marker too -- a prior version +/// of this class also declared a Channel constant seeded as the "channel" conversation state, but +/// that key (CustomStateKeys.Channel) is load-bearing production state with ~30 readers across +/// this codebase (routing hooks, visibility gates, etc.), none of which recognize a synthetic +/// value, and AgentTestRecorder.BuildDraft copies every seeded state into a recorded case's own +/// InitialStates -- so it also silently overwrote a recorded case's REAL channel on every +/// recording. The tag alone already satisfies the "make it identifiable after the fact" goal +/// without touching anything BotSharp/onebrain branches on. Do not add another state-key marker +/// without grepping every reader of that key first. +/// +public static class AgentTestConversationMarker +{ + /// + /// The SAME tag value BotSharp-UI's chat window already writes when a human manually tags a + /// real conversation as a test conversation (ConversationTag.Test in the UI's + /// src/lib/helpers/enums.js) -- reusing it means any current or future code that filters this + /// tag out of normal conversation views/analytics already covers harness-created conversations + /// too, with nothing new to teach it. + /// + public const string Tag = "test-set"; +} diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Runtime/MockFunctionExecutor.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Runtime/MockFunctionExecutor.cs new file mode 100644 index 000000000..bde36b629 --- /dev/null +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Runtime/MockFunctionExecutor.cs @@ -0,0 +1,75 @@ +namespace BotSharp.Plugin.AgentTesting.Runtime; + +public class MockFunctionExecutor : IFunctionExecutor +{ + public const string BlockedPrefix = "[agent-test] blocked unmocked tool"; + + private readonly ActiveTestRun _run; + private readonly string _functionName; + private readonly IConversationStateService _state; + private readonly ILogger _logger; + + public MockFunctionExecutor( + ActiveTestRun run, + string functionName, + IConversationStateService state, + ILogger logger) + { + _run = run; + _functionName = functionName; + _state = state; + _logger = logger; + } + + public Task GetIndicatorAsync(RoleDialogModel message) => Task.FromResult(string.Empty); + + public Task ExecuteAsync(RoleDialogModel message) + { + if (_functionName == AgentTestCanary.FunctionName) + { + _run.CanaryIntercepted = true; + message.Content = AgentTestCanary.ExpectedContent; + return Task.FromResult(true); + } + + var ordinal = _run.NextCallOrdinal(_functionName); + var mock = ToolMockMatcher.Match(_run.Mocks, _functionName, message.FunctionArgs, ordinal); + + if (mock == null) + { + message.Content = $"{BlockedPrefix}: {_functionName}"; + message.StopCompletion = true; + _run.Record(new ObservedToolCall + { + TurnIndex = _run.CurrentTurnIndex, + FunctionName = _functionName, + ArgsJson = message.FunctionArgs, + Outcome = "Blocked", + ResultContent = message.Content + }); + return Task.FromResult(false); + } + + message.Content = mock.ResultContent; + if (mock.StopCompletion) + { + message.StopCompletion = true; + } + + foreach (var write in mock.StateWrites ?? []) + { + _state.SetState(write.Key, write.Value, activeRounds: write.ActiveRounds); + } + + _run.Record(new ObservedToolCall + { + TurnIndex = _run.CurrentTurnIndex, + FunctionName = _functionName, + ArgsJson = message.FunctionArgs, + Outcome = "Mocked", + ResultContent = mock.ResultContent + }); + + return Task.FromResult(true); + } +} diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Runtime/TestMockExecutorProvider.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Runtime/TestMockExecutorProvider.cs new file mode 100644 index 000000000..25b0e1be8 --- /dev/null +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Runtime/TestMockExecutorProvider.cs @@ -0,0 +1,53 @@ +namespace BotSharp.Plugin.AgentTesting.Runtime; + +public class TestMockExecutorProvider : IFunctionExecutorProvider +{ + private readonly IAgentTestRunRegistry _registry; + private readonly IConversationService _conversations; + private readonly IConversationStateService _state; + private readonly ILogger _logger; + + public TestMockExecutorProvider( + IAgentTestRunRegistry registry, + IConversationService conversations, + IConversationStateService state, + ILogger logger) + { + _registry = registry; + _conversations = conversations; + _state = state; + _logger = logger; + } + + /// Must be asked before the built-in resolution chain. + public int Order => -1000; + + public IFunctionExecutor? TryResolve(string functionName, Agent agent) + { + var run = _registry.TryGet(_conversations.ConversationId); + if (run == null) + { + return null; // Not a conversation under test: pass through untouched. + } + + if (run.ForceBlockedFunctions.Contains(functionName)) + { + return new MockFunctionExecutor(run, functionName, _state, _logger); + } + + if (run.AllowedFunctions.Contains(functionName)) + { + return null; // Control flow: leave it to the real implementation, or the agent cannot move. + } + + // P1 only ever ships UnmockedToolPolicies.Block: every other function inside a test + // conversation is taken over, mocked if a TestToolMock claims it, blocked otherwise (see + // MockFunctionExecutor). A Passthrough policy existed here once -- it let an unmocked call + // straight through to the real implementation on the theory that the runner would back-fill + // an ObservedToolCall for it afterward from the conversation's dialogs. Nothing ever + // implemented that back-fill, so toolNotCalled assertions vacuously passed against tools + // that had genuinely executed with real side effects; the policy is now rejected at + // case create/update (AgentTestController) rather than reachable here. + return new MockFunctionExecutor(run, functionName, _state, _logger); + } +} diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Runtime/ToolMockMatcher.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Runtime/ToolMockMatcher.cs new file mode 100644 index 000000000..a07559598 --- /dev/null +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Runtime/ToolMockMatcher.cs @@ -0,0 +1,110 @@ +using System.Text.Json; +using System.Text.Json.Nodes; + +namespace BotSharp.Plugin.AgentTesting.Runtime; + +public static class ToolMockMatcher +{ + /// + /// Picks the most specific mock: argument-subset match beats call ordinal, which beats function + /// name alone. The argument JSON comes from model output and may be malformed; this always + /// degrades to a mock without argument conditions and never throws -- throwing would record the + /// case as an infrastructure Error and hide the real problem. + /// + public static TestToolMock? Match( + IReadOnlyList mocks, + string functionName, + string? argsJson, + int callOrdinal) + { + var candidates = mocks + .Where(m => string.Equals(m.FunctionName, functionName, StringComparison.OrdinalIgnoreCase)) + .ToList(); + + if (candidates.Count == 0) + { + return null; + } + + var actual = ParseOrNull(argsJson); + + var byArgs = candidates.FirstOrDefault(m => + !string.IsNullOrWhiteSpace(m.ArgsMatchJson) + && actual != null + && IsSubset(ParseOrNull(m.ArgsMatchJson), actual)); + if (byArgs != null) + { + return byArgs; + } + + var byOrdinal = candidates.FirstOrDefault(m => m.CallIndex == callOrdinal); + if (byOrdinal != null) + { + return byOrdinal; + } + + return candidates.FirstOrDefault(m => + string.IsNullOrWhiteSpace(m.ArgsMatchJson) && m.CallIndex == null); + } + + /// + /// Public because AssertionEvaluator's toolCalled branch reuses the duplicate-top-level-key + /// materialisation fix below rather than writing its own JsonNode.Parse wrapper -- that trap + /// should be fixed in exactly one place. + /// + public static JsonObject? ParseOrNull(string? json) + { + if (string.IsNullOrWhiteSpace(json)) + { + return null; + } + + try + { + var node = JsonNode.Parse(json) as JsonObject; + + // JsonObject materialises its backing dictionary lazily: Parse itself does not + // complain about duplicate top-level keys, and the ArgumentException only surfaces on + // first access (foreach/TryGetPropertyValue/indexer). Forcing materialisation here puts + // "duplicate keys" and "syntax error" through the same try block and the same handling, + // rather than leaving the exception for a caller to hit unexpectedly inside IsSubset. + _ = node?.Count; + + return node; + } + catch (JsonException) + { + return null; + } + catch (ArgumentException) + { + return null; + } + } + + /// + /// Every key in expected is present in actual, with an equal textual representation of the value. + /// + public static bool IsSubset(JsonObject? expected, JsonObject actual) + { + if (expected == null) + { + return false; + } + + foreach (var (key, value) in expected) + { + if (!actual.TryGetPropertyValue(key, out var actualValue)) + { + return false; + } + + if (value?.ToJsonString() != actualValue?.ToJsonString()) + { + return false; + } + } + + return true; + } +} diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Services/AgentTestCaseRunner.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/AgentTestCaseRunner.cs new file mode 100644 index 000000000..bbe3d7070 --- /dev/null +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/AgentTestCaseRunner.cs @@ -0,0 +1,302 @@ +using System.Diagnostics; +using BotSharp.Plugin.AgentTesting.Runtime; + +namespace BotSharp.Plugin.AgentTesting.Services; + +public class AgentTestCaseRunner : ICaseRunner +{ + private readonly IAgentTestRunRegistry _registry; + private readonly IAgentConversationDriver _driver; + private readonly ILogger _logger; + + public AgentTestCaseRunner( + IAgentTestRunRegistry registry, + IAgentConversationDriver driver, + ILogger logger) + { + _registry = registry; + _driver = driver; + _logger = logger; + } + + public async Task RunAsync( + AgentTestSuite suite, + AgentTestCase testCase, + string runId, + TestModel? model, + CancellationToken ct) + { + var conversationId = Guid.NewGuid().ToString(); + var result = new AgentTestCaseResult + { + RunId = runId, + CaseId = testCase.Id, + CaseName = testCase.Name, + ConversationId = conversationId, + // Stamped up front so even the early-return paths below (no turns, canary failure, + // timeout) still say which model they were meant to run under -- a result that cannot + // be attributed to a model is useless in a comparison run. + Provider = model?.Provider, + Model = model?.Model + }; + + // Turns.SelectMany(...).Concat(caseAssertions).All(a => a.Passed) is vacuously true on an + // empty sequence, so a case with no turns would otherwise execute nothing and still report + // Passed. Catch it here, before the driver is touched at all: no PrepareAsync, no canary, + // no conversation ever opened for a case that was never going to run anything. + if (testCase.Turns.Count == 0) + { + result.Status = AgentTestStatus.Error; + result.Error = "the case has no turns"; + return result; + } + + var active = new ActiveTestRun + { + ConversationId = conversationId, + CaseId = testCase.Id, + ModelOverride = model, + Mocks = testCase.Mocks, + UnmockedToolPolicy = testCase.UnmockedToolPolicy, + AllowedFunctions = BuildAllowList(suite), + ForceBlockedFunctions = new HashSet(suite.ForceBlockedFunctions, StringComparer.OrdinalIgnoreCase) + }; + + var stopwatch = Stopwatch.StartNew(); + using var timeout = CancellationTokenSource.CreateLinkedTokenSource(ct); + timeout.CancelAfter(TimeSpan.FromSeconds(Math.Max(1, suite.CaseTimeoutSeconds))); + + // Fix round 1, Finding 1. BotSharp's SendMessage/InvokeFunction have no CancellationToken + // of their own, so a driver call can at best be RACED against `timeout` (see + // AwaitOrHandOffAsync below) -- it can never actually be aborted. When OUR OWN timeout is + // what fired, the real call is very likely still running server-side, still walking + // through BotSharp's routing/tool-invocation loop. Unregistering the conversation right + // now (the old, unconditional `finally` behavior) would remove the only thing making + // TestMockExecutorProvider intercept its NEXT function call -- the mock seam disappears + // out from under a still-running case, and its next tool call falls through to the REAL + // implementation (a real phone call, a real email). So on that path specifically, cleanup + // is handed off to a continuation on the raw driver task, and the `finally` below must NOT + // unregister inline while that hand-off is pending. + var orphanHandedOff = false; + + async Task AwaitOrHandOffAsync(Task driverTask) + { + try + { + return await driverTask.WaitAsync(timeout.Token); + } + catch (OperationCanceledException) when (timeout.IsCancellationRequested) + { + orphanHandedOff = true; + _ = driverTask.ContinueWith(t => + { + // Observe the antecedent's exception, if any, before anything else: an + // orphaned real driver call that later throws (e.g. the underlying BotSharp + // SendMessage/InvokeFunction call itself fails after we already gave up + // waiting on it) previously vanished completely -- no case result reflects it + // (the case already recorded "timed out" and returned), and the exception was + // never even observed, so at best it surfaces as an UnobservedTaskException at + // GC. This is the cheapest available diagnostic for the single highest-risk + // unverified property in this whole feature: a real call still running after + // RunAsync itself has already returned. + if (t.IsFaulted) + { + _logger.LogError(t.Exception, + "Orphaned agent test driver call for case {CaseId} (conversation {ConversationId}) " + + "faulted after the case's own timeout had already elapsed. The case result already " + + "recorded a timeout; this log is diagnostic-only.", + testCase.Id, conversationId); + } + + try { _registry.Unregister(conversationId); } + catch { /* best-effort cleanup of an orphaned call; nothing else to do here */ } + }, TaskScheduler.Default); + throw; + } + } + + _registry.Register(active); + try + { + await _driver.PrepareAsync(conversationId, suite.AgentId, testCase.InitialStates); + + // Prove the seam is live first. A dead seam means mocking silently does nothing and + // real tools execute, so this has to happen before a single user message is sent. + // + // The verdict comes only from the driver's return value: the real driver derives that + // bool from whether the canary call's content was replaced with 'canary', and that + // content only ever appears when MockFunctionExecutor took over. Also checking + // active.CanaryIntercepted would look stricter but checks the same fact twice, and it + // would stop a fake driver from unit-testing the orchestration at all -- a fake driver + // has no ActiveTestRun and can never set that flag. + if (!await AwaitOrHandOffAsync(_driver.RunCanaryAsync(conversationId, suite.AgentId, timeout.Token))) + { + result.Status = AgentTestStatus.Error; + result.Error = "the mock seam is not live: IFunctionExecutorProvider was not consulted. " + + "Check that the build uses DebugBrain.sln (BotSharp from source) and that " + + "BotSharp.Plugin.AgentTesting is listed in PluginLoader:Assemblies."; + return result; + } + + var fatalStop = false; + foreach (var turn in testCase.Turns.OrderBy(t => t.Index)) + { + if (fatalStop) break; + + active.CurrentTurnIndex = turn.Index; + var output = await AwaitOrHandOffAsync( + _driver.SendAsync(conversationId, suite.AgentId, turn.UserMessage, timeout.Token)); + + var turnResult = new TurnResult + { + Index = turn.Index, + UserMessage = turn.UserMessage, + Output = output + }; + + var turnContext = new AssertionContext + { + Output = output, + ToolCalls = active.ObservedCalls.Where(c => c.TurnIndex == turn.Index).ToList(), + States = await _driver.ReadStatesAsync(conversationId), + RoutedToAgent = await _driver.ReadRoutedAgentNameAsync(conversationId) + }; + + foreach (var assertion in turn.Assertions) + { + var evaluated = AssertionEvaluator.Evaluate(assertion, turnContext); + turnResult.Assertions.Add(evaluated); + if (!evaluated.Passed && assertion.Fatal) + { + fatalStop = true; + } + } + + result.Turns.Add(turnResult); + } + + var finalContext = new AssertionContext + { + Output = result.Turns.LastOrDefault()?.Output, + ToolCalls = active.ObservedCalls, + States = await _driver.ReadStatesAsync(conversationId), + RoutedToAgent = await _driver.ReadRoutedAgentNameAsync(conversationId) + }; + + foreach (var assertion in testCase.Assertions) + { + result.Assertions.Add(AssertionEvaluator.Evaluate(assertion, finalContext)); + } + + result.ObservedToolCalls = active.ObservedCalls.ToList(); + + AddBlockedToolFailure(result); + + var allAssertions = result.Turns.SelectMany(t => t.Assertions).Concat(result.Assertions); + result.Status = allAssertions.All(a => a.Passed) ? AgentTestStatus.Passed : AgentTestStatus.Failed; + } + // Fix round 1, Finding 3. The old `when (!ct.IsCancellationRequested)` guard mislabeled ANY + // OperationCanceledException that wasn't the caller's own cancellation as "the case timed + // out" -- including, say, an HttpClient timeout raised deep inside a passthrough tool call, + // which has nothing to do with `timeout`/CaseTimeoutSeconds at all. Tightened to require + // OUR OWN timeout to actually be the one that fired; anything else (`ct` cancelled, or some + // unrelated cancellation) falls through to the clauses below, which record what really + // happened instead of a misleading "timed out". + catch (OperationCanceledException) when (timeout.IsCancellationRequested && !ct.IsCancellationRequested) + { + // This case's own timeout, not a cancellation of the whole run. "Could not run" and + // "ran and came out wrong" have to stay distinguishable. + result.Status = AgentTestStatus.Error; + result.Error = $"the case timed out after {suite.CaseTimeoutSeconds}s"; + result.ObservedToolCalls = active.ObservedCalls.ToList(); + } + catch (OperationCanceledException) when (ct.IsCancellationRequested) + { + result.Status = AgentTestStatus.Cancelled; + result.ObservedToolCalls = active.ObservedCalls.ToList(); + } + catch (Exception ex) + { + // Reaches here for any OperationCanceledException that was neither our own timeout nor + // the caller's cancellation too (e.g. an unrelated cancellation raised inside a + // passthrough tool call), as well as every other exception -- both get the real + // message instead of being folded into "timed out"/"cancelled". + _logger.LogError(ex, "Agent test case {CaseId} crashed.", testCase.Id); + result.Status = AgentTestStatus.Error; + result.Error = ex.Message; + result.ObservedToolCalls = active.ObservedCalls.ToList(); + } + finally + { + // Leaking a registry entry would mean every later tool call on that conversationId is + // still intercepted as a test -- unless a timeout already handed the removal to the + // ContinueWith above, in which case removing it here early is exactly what must not + // happen. + if (!orphanHandedOff) + { + _registry.Unregister(conversationId); + } + stopwatch.Stop(); + result.DurationMs = stopwatch.ElapsedMilliseconds; + } + + return result; + } + + /// + /// A blocked tool call fails the case, as a synthetic case-level assertion. + /// + /// Blocking is the mock seam working correctly -- the agent reached for a tool this case does + /// not mock, and executing it for real could have sent an email or created a work order. But + /// the block also truncates that turn (StopCompletion), so everything the agent would have done + /// afterwards never happened and every later assertion is evaluated against a conversation that + /// stopped early. Reporting Passed there is the "executed nothing, reports green" defect this + /// harness guards against everywhere else (a case with no turns, a dead canary, a CaseIds filter + /// that matched nothing). + /// + /// Modelled as an assertion rather than as result.Error so it renders in the ordinary assertion + /// table with expected/actual, and so the existing all-assertions-passed rule decides the status + /// with no special case. Error stays reserved for "the harness itself did not work", which is + /// the opposite of what happened here. + /// + private static void AddBlockedToolFailure(AgentTestCaseResult result) + { + var blocked = result.ObservedToolCalls + .Where(c => string.Equals(c.Outcome, "Blocked", StringComparison.Ordinal)) + .ToList(); + + if (blocked.Count == 0) + { + return; + } + + var named = string.Join(", ", blocked + .Select(c => $"{c.FunctionName} (turn {c.TurnIndex + 1})") + .Distinct(StringComparer.Ordinal)); + + result.Assertions.Add(new AssertionResult + { + Type = AssertionTypes.NoBlockedTools, + Target = null, + Expected = "every tool the agent calls is mocked by this case", + Actual = named, + Passed = false, + Message = blocked.Count == 1 + ? "The agent called a tool this case does not mock, so it was blocked and the turn " + + "stopped there. Add a mock for it, or change the case so the agent does not need it." + : $"The agent called {blocked.Count} tools this case does not mock, so they were " + + "blocked and their turns stopped there. Add mocks for them, or change the case " + + "so the agent does not need them." + }); + } + + private static HashSet BuildAllowList(AgentTestSuite suite) + { + var allow = new HashSet(ControlFlowFunctions.Default, StringComparer.OrdinalIgnoreCase); + foreach (var extra in suite.ExtraAllowedFunctions) + { + allow.Add(extra); + } + return allow; + } +} diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Services/AgentTestRecorder.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/AgentTestRecorder.cs new file mode 100644 index 000000000..aebfabdd0 --- /dev/null +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/AgentTestRecorder.cs @@ -0,0 +1,603 @@ +using BotSharp.Abstraction.Agents.Enums; +using BotSharp.Abstraction.Repositories; +using BotSharp.Plugin.AgentTesting.Repositories; + +namespace BotSharp.Plugin.AgentTesting.Services; + +/// +/// Records an editable draft case from a real BotSharp conversation. This is what decides whether +/// QA and PM can actually use this feature: hand-writing a work order agent's mock JSON is not +/// realistic. +/// +/// is a pure function: the same (suiteId, conversationId, dialogs, states) +/// always yields the same with no I/O (the logger is an optional +/// diagnostic outlet that affects logging only, never the return value), so it can be unit-tested +/// without Mongo or BotSharp. is the thin layer that touches the +/// real data source: read , map to +/// /, call , store. +/// +/// Two deliberate limitations -- change the spec before "fixing" either of them here: +/// 1) state writes can only be extracted as a whole-turn delta, attached to that turn's last mock. +/// StateValueMongoElement carries only MessageId (which locates a turn) and Source (one of +/// external/application/user), never a function name, so splitting the delta across individual +/// mocks automatically is not possible; +/// 2) no output-text assertions (outputContains/outputRegex) and no llmJudge are generated. Using +/// the model's exact wording as a baseline is extremely brittle -- any rephrasing goes red, and +/// recording one case would mean hand-editing ten assertions. Only the two stable kinds, +/// toolCalled and stateEquals, are generated. +/// +public class AgentTestRecorder +{ + private readonly IBotSharpRepository _repository; + private readonly IAgentTestRepository _testRepository; + private readonly ILogger _logger; + + /// + /// Optional: when null this recorder only has the deterministic path (which is how the existing + /// unit tests construct it). + /// + private readonly ICaseSegmenter? _segmenter; + + public AgentTestRecorder( + IBotSharpRepository repository, + IAgentTestRepository testRepository, + ILogger logger, + ICaseSegmenter? segmenter = null) + { + _repository = repository; + _testRepository = testRepository; + _logger = logger; + _segmenter = segmenter; + } + + /// + /// Reads the real conversation, builds the draft, stores it, and returns the new draft case + /// (with its Id already assigned). + /// + public async Task LoadAndBuildAsync(string suiteId, string conversationId) + { + var (dialogs, states) = await LoadAsync(conversationId); + + var draft = BuildDraft(suiteId, conversationId, dialogs, states, _logger); + + await _testRepository.UpsertCaseAsync(draft); + return draft; + } + + /// Reads the raw conversation data recording needs. Shared by both recording paths. + private async Task<(List Dialogs, List States)> LoadAsync(string conversationId) + { + var dialogElements = await _repository.GetConversationDialogs(conversationId); + var conversationStates = await _repository.GetConversationStates(conversationId); + + var dialogs = dialogElements + .Select(d => new RecordedDialog + { + Role = d.MetaData?.Role ?? string.Empty, + Content = d.Content, + FunctionName = d.MetaData?.FunctionName, + FunctionArgs = d.MetaData?.FunctionArgs, + MessageId = d.MetaData?.MessageId + }) + .ToList(); + + // ConversationState is a ConcurrentDictionary -- the dictionary + // KEY and StateKeyValue.Key are the same thing by construction (see + // BotSharp.Plugin.MongoStorage's StateMongoElement round trip); reading it off the value + // rather than the pair's own Key is just slightly more defensive. + var states = conversationStates + .Select(pair => new RecordedState + { + Key = pair.Value.Key, + Values = pair.Value.Values + .Select(v => new RecordedStateValue + { + MessageId = v.MessageId, + Data = v.Data, + ActiveRounds = v.ActiveRounds + }) + .ToList() + }) + .ToList(); + + return (dialogs, states); + } + + /// + /// As above, but first asks to split the conversation into one or + /// more scenarios, producing one draft per scenario. A null calls no + /// model at all and falls through to (returning a single-element + /// list) -- so "do not use AI" is not a degraded branch, it is literally the original path. + /// + public async Task> LoadAndBuildManyAsync( + string suiteId, + string conversationId, + TestModel? model, + CancellationToken ct = default) + { + if (model == null || _segmenter == null) + { + return [await LoadAndBuildAsync(suiteId, conversationId)]; + } + + var (dialogs, states) = await LoadAsync(conversationId); + + var turns = ToSegmentableTurns(dialogs); + if (turns.Count == 0) + { + // Nothing to segment and nothing to record; let the deterministic path produce the + // same (empty-turn) draft it always would rather than special-casing it here. + return [await LoadAndBuildAsync(suiteId, conversationId)]; + } + + var segments = await _segmenter.SegmentAsync(turns, model, ct); + var drafts = BuildDrafts(suiteId, conversationId, dialogs, states, segments, _logger); + + foreach (var draft in drafts) + { + await _testRepository.UpsertCaseAsync(draft); + } + + return drafts; + } + + /// + /// Pure function: turns already-loaded conversation data into one disabled draft case. The + /// is used only for the single edge case of "this turn's state delta + /// has no mock to attach to"; omitting it (as the unit tests do) means no log output and changes + /// nothing about the return value. + /// + public static AgentTestCase BuildDraft( + string suiteId, + string conversationId, + IReadOnlyList dialogs, + IReadOnlyList states, + ILogger? logger = null) + { + var draft = new AgentTestCase + { + SuiteId = suiteId, + Name = $"Recorded from {conversationId}", + Enabled = false, // Enabled only after a human reviews it + SourceConversationId = conversationId, + UnmockedToolPolicy = UnmockedToolPolicies.Block + }; + + // TestTurn itself has no slot for the BotSharp MessageId that opened it (see + // Repository/Mongo/AgentTestCase.cs) -- tracked here only for the duration of this call, + // to compute each turn's own state delta (step 4) below. + var turnMessageIds = new Dictionary(); + + // turn.Index -> every mock created while that turn was open, in dialog order. Used both + // to attach a turn's state delta to its LAST mock (step 4) and to generate one toolCalled + // assertion per mock (step 5a). + var mocksByTurn = new Dictionary>(); + + // FunctionName -> FunctionArgs of the nearest preceding assistant call for that function, + // scoped to the CURRENTLY OPEN TURN ONLY -- cleared every time a new turn opens, per the + // "nearest preceding call within the same turn" rule. + // + // OrdinalIgnoreCase, matching ToolMockMatcher.Match/ActiveTestRun's own call-ordinal + // tracker (both compare function names case-insensitively): two differently-cased + // recordings of the same real function (e.g. a dialog logged "Get_Work_Order" once and + // "get_work_order" another time) must be tracked as ONE function here too, or the args/ + // ordinal correction below silently stops being unambiguous -- the recorder would see two + // single-occurrence functions (each kept its own ArgsMatchJson, each CallIndex 0), while + // ToolMockMatcher.Match sees one function called twice and could resolve either replay + // call to either mock. + var pendingArgsByFunction = new Dictionary(StringComparer.OrdinalIgnoreCase); + + // FunctionName -> how many times it has been recorded as a "function" dialog so far. + // Doubles as: (a) during the loop, the next CallIndex to assign for that function; (b) + // after the loop, its final value is the TOTAL number of times that function was recorded + // across the whole conversation, which is exactly what the args-omission correction below + // needs. OrdinalIgnoreCase for the same reason as pendingArgsByFunction above. + var callCountByFunction = new Dictionary(StringComparer.OrdinalIgnoreCase); + + TestTurn? currentTurn = null; + + foreach (var dialog in dialogs) + { + if (dialog.Role == AgentRole.User) + { + currentTurn = new TestTurn + { + Index = draft.Turns.Count, + UserMessage = dialog.Content ?? string.Empty + }; + draft.Turns.Add(currentTurn); + turnMessageIds[currentTurn.Index] = dialog.MessageId; + pendingArgsByFunction.Clear(); + continue; + } + + if (dialog.Role == AgentRole.Assistant && !string.IsNullOrEmpty(dialog.FunctionName)) + { + pendingArgsByFunction[dialog.FunctionName] = dialog.FunctionArgs; + continue; + } + + if (dialog.Role == AgentRole.Function) + { + if (currentTurn == null) + { + // A function result with no user turn open yet can't happen in a + // well-formed recording; there is nowhere sane to attach it, so it is + // skipped rather than fabricating a turn for it. + continue; + } + + var functionName = dialog.FunctionName ?? string.Empty; + var argsMatchJson = pendingArgsByFunction.TryGetValue(functionName, out var pendingArgs) + ? pendingArgs + : null; + var callIndex = callCountByFunction.TryGetValue(functionName, out var count) ? count : 0; + callCountByFunction[functionName] = callIndex + 1; + + var mock = new TestToolMock + { + FunctionName = functionName, + ArgsMatchJson = argsMatchJson, + CallIndex = callIndex, + ResultContent = dialog.Content ?? string.Empty + }; + + draft.Mocks.Add(mock); + + if (!mocksByTurn.TryGetValue(currentTurn.Index, out var turnMocks)) + { + turnMocks = []; + mocksByTurn[currentTurn.Index] = turnMocks; + } + turnMocks.Add(mock); + } + } + + // Step 5a: one toolCalled assertion per mock, generated from each mock's CURRENTLY + // recorded ArgsMatchJson -- deliberately BEFORE the args/ordinal correction below, not + // after. Fix round 1: an earlier version of this method ran the correction first and + // built assertions by reading mock.ArgsMatchJson back off the (by then corrected) + // TestToolMock objects mocksByTurn holds references to -- so a repeated function's + // assertion silently lost its argument check too, even though the correction is only + // actually needed for ToolMockMatcher.Match, which dispatches against the WHOLE case's + // mock list (MockFunctionExecutor.ExecuteAsync: Match(_run.Mocks, ...)) and is therefore + // genuinely ambiguous across turns. AssertionEvaluator's toolCalled case, by contrast, is + // evaluated per turn against ONLY that turn's observed calls + // (AgentTestCaseRunner.cs: active.ObservedCalls.Where(c => c.TurnIndex == turn.Index)) -- + // there is no cross-turn collision for an assertion to guard against, so it must keep + // whatever argument was actually recorded for that one call. TestAssertion.ArgsMatchJson + // is a plain string copy at the moment this loop runs, so building assertions here and + // nulling the MOCKS' own ArgsMatchJson afterward cannot un-set what was already copied. + foreach (var turn in draft.Turns) + { + if (!mocksByTurn.TryGetValue(turn.Index, out var turnMocks)) + { + continue; + } + + foreach (var mock in turnMocks) + { + if (string.IsNullOrWhiteSpace(mock.FunctionName)) + { + // A Role=Function dialog with no FunctionName (malformed/incomplete recorded + // data) would otherwise produce a toolCalled assertion with a blank Target -- + // AssertionValidation (fix wave item 4) now rejects that at save time, so + // RecordCase would persist a draft that the very next UpdateCase (even one + // editing an unrelated field) refuses to save, with a 400 that names the type + // but not which assertion. An assertion that pins nothing isn't worth + // recording; skip it. The mock itself is still recorded (draft.Mocks, added + // above) so a human reviewing the draft can see and fix the gap. + logger?.LogWarning( + "Agent test recorder: turn {TurnIndex} of conversation {ConversationId} " + + "recorded a function dialog with no function name; skipping the " + + "toolCalled assertion that would otherwise have an empty target.", + turn.Index, conversationId); + continue; + } + + turn.Assertions.Add(new TestAssertion + { + Type = AssertionTypes.ToolCalled, + Target = mock.FunctionName, + ArgsMatchJson = mock.ArgsMatchJson + }); + } + } + + // Correction (see the task-9 brief's correction section): ToolMockMatcher.Match (Task 5) + // tries an args-subset match BEFORE falling back to CallIndex. Once a function has been + // recorded more than once, keeping ArgsMatchJson on its mocks risks a LATER replay call + // whose real arguments happen to match an EARLIER mock's recorded arguments -- that later + // call would then resolve to the earlier mock via the args branch, before the ordinal + // branch is ever consulted, and the case could never reproduce the different result that + // was actually recorded for it. Ordinal alone is unambiguous once a function repeats. A + // function recorded exactly once has no later call to collide with, so its ArgsMatchJson + // stays -- unambiguous, and useful context when a human reviews the draft. This mutates + // the SAME TestToolMock instances already referenced by draft.Mocks/mocksByTurn AND + // already copied into the Step 5a assertions above -- it must run AFTER Step 5a, never + // before (see that step's comment for the bug this ordering fixes). + foreach (var mock in draft.Mocks) + { + if (callCountByFunction.GetValueOrDefault(mock.FunctionName) > 1) + { + mock.ArgsMatchJson = null; + } + } + + // Step 3: a state value with no MessageId was never written by any turn -- it was seeded + // before the conversation started. + foreach (var state in states) + { + foreach (var value in state.Values.Where(v => v.MessageId == null)) + { + draft.InitialStates.Add(new TestState + { + Key = state.Key, + Value = value.Data ?? string.Empty, + ActiveRounds = value.ActiveRounds + }); + } + } + + // Step 4: each turn's OWN state delta (values whose MessageId matches that turn's own + // MessageId) attaches to the LAST mock created during that turn -- the mock whose + // (mocked) return is what the real function call actually returned when that state got + // written. A turn that wrote state but called no mockable function has nowhere to hang + // the delta -- drop it and say so (there is no other turn it would be correct to + // attach to). + foreach (var turn in draft.Turns) + { + var turnMessageId = turnMessageIds.GetValueOrDefault(turn.Index); + if (turnMessageId == null) + { + // A turn opened by a user dialog with no MessageId of its own can't be matched + // against anything -- matching on null here would wrongly pull in the + // MessageId == null "initial state" values from step 3 above. + continue; + } + + var delta = ComputeDelta(states, v => v.MessageId == turnMessageId); + if (delta.Count == 0) + { + continue; + } + + if (mocksByTurn.TryGetValue(turn.Index, out var turnMocks) && turnMocks.Count > 0) + { + turnMocks[^1].StateWrites = delta; + } + else + { + logger?.LogWarning( + "Agent test recorder: turn {TurnIndex} of conversation {ConversationId} wrote " + + "{StateCount} state value(s) but called no mockable function during that " + + "turn; the delta has nowhere to attach and was dropped.", + turn.Index, conversationId, delta.Count); + } + } + + // Step 5b: one case-level stateEquals per key that was actually WRITTEN by some turn + // (i.e. has at least one value with a non-null MessageId), using that key's most recent + // such write as the expected "final" value the whole recorded conversation left it at. A + // key that was only ever seeded (every value has MessageId == null, e.g. an auth flag + // nothing in the conversation ever changes) is already covered by InitialStates above and + // was never actually incremented by this case, so it is not asserted again here. + foreach (var state in states) + { + var written = state.Values.Where(v => v.MessageId != null).ToList(); + if (written.Count == 0) + { + continue; + } + + draft.Assertions.Add(new TestAssertion + { + Type = AssertionTypes.StateEquals, + Target = state.Key, + Expected = written[^1].Data + }); + } + + return draft; + } + + /// + /// What the segmenter is allowed to see: each turn's user message plus the NAMES of the + /// functions that turn called. Deliberately carries no function arguments and no return + /// content -- see on what does and does not leave this process. + /// + public static List ToSegmentableTurns(IReadOnlyList dialogs) + { + var turns = new List(); + List? currentTools = null; + + foreach (var dialog in dialogs) + { + if (dialog.Role == AgentRole.User) + { + currentTools = []; + turns.Add(new SegmentableTurn + { + Index = turns.Count, + UserMessage = dialog.Content ?? string.Empty, + ToolNames = currentTools + }); + continue; + } + + if (dialog.Role == AgentRole.Function && currentTools != null && !string.IsNullOrWhiteSpace(dialog.FunctionName)) + { + currentTools.Add(dialog.FunctionName); + } + } + + return turns; + } + + /// + /// Pure function: cuts one conversation into several draft cases according to the segmentation. + /// Each segment runs through on its own, so mock return values, + /// toolCalled assertions and per-turn state writes still come verbatim from the real + /// conversation -- the segmenter only chose the boundaries and the names. + /// + /// Slicing introduces two things cannot see and that must be corrected + /// here: + /// + /// 1) **Carried-in state.** A case cut at turn k should start with state as it stood "as of turn + /// k-1", but BuildDraft only treats seeded values (MessageId == null) as InitialStates. + /// Without this, a later segment starts unable to read the location_id/wo_num the earlier + /// turns wrote, and the whole segment runs a path the recording never took. + /// + /// 2) **Case-level stateEquals expectations.** BuildDraft's step 5b takes a key's last write + /// across the WHOLE conversation. For a segment covering only the first two turns that is a + /// final value it never reaches -- the assertion would fail on every single run. Recomputed + /// here over the segment's own turns. + /// + public static List BuildDrafts( + string suiteId, + string conversationId, + IReadOnlyList dialogs, + IReadOnlyList states, + IReadOnlyList segments, + ILogger? logger = null) + { + // Position in `dialogs` where each user turn opens; a segment's turn indices index into this. + var turnStarts = new List(); + for (var i = 0; i < dialogs.Count; i++) + { + if (dialogs[i].Role == AgentRole.User) + { + turnStarts.Add(i); + } + } + + var drafts = new List(); + foreach (var segment in segments) + { + if (segment.FirstTurn < 0 || segment.LastTurn >= turnStarts.Count || segment.LastTurn < segment.FirstTurn) + { + // The segmenter validates its own output, so reaching here means a caller built + // segments some other way. Skip rather than throw: the segments that ARE valid are + // still worth producing. + logger?.LogWarning( + "Agent test recorder: dropping segment '{Name}' ({First}..{Last}) -- conversation " + + "{ConversationId} only has {TurnCount} turn(s).", + segment.Name, segment.FirstTurn, segment.LastTurn, conversationId, turnStarts.Count); + continue; + } + + var start = turnStarts[segment.FirstTurn]; + var end = segment.LastTurn + 1 < turnStarts.Count + ? turnStarts[segment.LastTurn + 1] - 1 + : dialogs.Count - 1; + + var slice = new List(); + for (var i = start; i <= end; i++) + { + slice.Add(dialogs[i]); + } + + var draft = BuildDraft(suiteId, conversationId, slice, states, logger); + draft.Name = segment.Name; + + // State is keyed by the USER turn's own MessageId (see step 4 above), so both fixes + // below are computed from turn message ids, not from every dialog's id. + if (segment.FirstTurn > 0) + { + var priorTurnIds = turnStarts + .Take(segment.FirstTurn) + .Select(pos => dialogs[pos].MessageId) + .Where(id => id != null) + .ToHashSet(StringComparer.Ordinal); + + // Superset of what BuildDraft computed (seeded values), with later writes winning, + // so replacing wholesale is correct rather than merging. + draft.InitialStates = ComputeDelta( + states, v => v.MessageId == null || (v.MessageId != null && priorTurnIds.Contains(v.MessageId))); + } + + var sliceTurnIds = turnStarts + .Skip(segment.FirstTurn) + .Take(segment.LastTurn - segment.FirstTurn + 1) + .Select(pos => dialogs[pos].MessageId) + .Where(id => id != null) + .ToHashSet(StringComparer.Ordinal); + + draft.Assertions = states + .Select(state => new + { + state.Key, + Written = state.Values + .Where(v => v.MessageId != null && sliceTurnIds.Contains(v.MessageId)) + .ToList() + }) + .Where(x => x.Written.Count > 0) + .Select(x => new TestAssertion + { + Type = AssertionTypes.StateEquals, + Target = x.Key, + Expected = x.Written[^1].Data + }) + .ToList(); + + drafts.Add(draft); + } + + return drafts; + } + + private static List ComputeDelta(IReadOnlyList states, Func predicate) + { + var delta = new List(); + + foreach (var state in states) + { + var match = state.Values.LastOrDefault(predicate); + if (match != null) + { + delta.Add(new TestState + { + Key = state.Key, + Value = match.Data ?? string.Empty, + ActiveRounds = match.ActiveRounds + }); + } + } + + return delta; + } +} + +/// +/// One dialog entry as recording sees it -- the fields recording cares about, picked out of the real +/// DialogElement/DialogMetaData. +/// +public class RecordedDialog +{ + public string Role { get; set; } = string.Empty; + public string? Content { get; set; } + public string? FunctionName { get; set; } + public string? FunctionArgs { get; set; } + public string? MessageId { get; set; } +} + +/// +/// One state key and its full history of values as recording sees it -- the fields recording cares +/// about, picked out of the real StateKeyValue. +/// +public class RecordedState +{ + public string Key { get; set; } = string.Empty; + public List Values { get; set; } = []; +} + +/// +/// See -- one historical value, corresponding to a real +/// StateValue/StateValueMongoElement. +/// +public class RecordedStateValue +{ + public string? MessageId { get; set; } + public string? Data { get; set; } + public int ActiveRounds { get; set; } +} diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Services/AgentTestRunExecutor.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/AgentTestRunExecutor.cs new file mode 100644 index 000000000..9faef9f05 --- /dev/null +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/AgentTestRunExecutor.cs @@ -0,0 +1,239 @@ +using BotSharp.Plugin.AgentTesting.Repositories; + +namespace BotSharp.Plugin.AgentTesting.Services; + +/// +/// Run-level orchestration: turns an already-created AgentTestRun into serial execution of every +/// enabled case in its suite. +/// +/// This class knows nothing about DI scopes -- it calls the single ICaseRunner instance handed to +/// its constructor, and the loop below decides how often and when. "A fresh DI scope per case" is +/// not done here; AgentTestRunQueue achieves it by injecting an ICaseRunner decorator that wraps +/// IServiceScopeFactory/IServiceProvider (see AgentTestRunQueue.ScopedCaseRunner). That decorator's +/// RunAsync opens a new scope on every call -- that is, per case -- resolves the real +/// AgentTestCaseRunner from it, and disposes the scope when the case finishes. As a result BotSharp +/// scoped services such as IConversationService, IConversationStateService and +/// TestMockExecutorProvider are never the same instance across two cases of one run. Keeping scope +/// creation out of this class is also what lets a DI-unaware DelegatingCaseRunner unit-test the +/// orchestration directly. +/// +/// One field, CancelRequested, has a writer outside this class (POST .../runs/{id}/cancel), which +/// makes it the only field a whole-document ReplaceOneAsync could revert to a stale value -- +/// TotalCount/PassedCount/... are written by this class alone. Hence the re-read AFTER each case +/// rather than before: the object it returns becomes the `run` this method then mutates and +/// persists, so both "should the next case run" and "will this write clobber an external one" are +/// decided from the state as of the moment that case finished, not from the copy read at the very +/// top and never refreshed. One read covers both questions, so no separate check is needed before +/// it. +/// +public class AgentTestRunExecutor +{ + private readonly IAgentTestRepository _repo; + private readonly ICaseRunner _caseRunner; + private readonly ILogger _logger; + + public AgentTestRunExecutor( + IAgentTestRepository repo, + ICaseRunner caseRunner, + ILogger logger) + { + _repo = repo; + _caseRunner = caseRunner; + _logger = logger; + } + + public async Task ExecuteAsync(string runId, CancellationToken ct) + { + var run = await _repo.GetRunAsync(runId); + if (run == null) + { + _logger.LogError("Agent test run {RunId} was not found; nothing to execute.", runId); + return; + } + + var suite = await _repo.GetSuiteAsync(run.SuiteId); + if (suite == null) + { + _logger.LogError( + "Agent test run {RunId} references suite {SuiteId}, which no longer exists.", + runId, run.SuiteId); + run.Status = AgentTestStatus.Error; + run.Error = $"The suite this run belongs to ({run.SuiteId}) no longer exists."; + run.StartedAt ??= DateTime.UtcNow; + run.CompletedAt = DateTime.UtcNow; + await _repo.UpdateRunAsync(run); + return; + } + + // The trigger endpoint (POST .../suites/{id}/run) is the primary place a disabled suite + // gets rejected -- a 400, before a run row even exists. This is defense-in-depth for the + // race where a suite is disabled AFTER a run was already queued: same shape as the + // suite-no-longer-exists branch just above, since "the suite this run belongs to says + // don't run me" is the same kind of infrastructure-level stop. + if (!suite.Enabled) + { + _logger.LogError( + "Agent test run {RunId} references suite {SuiteId}, which is disabled.", + runId, run.SuiteId); + run.Status = AgentTestStatus.Error; + run.Error = "The suite was disabled after this run was queued, so nothing ran."; + run.StartedAt ??= DateTime.UtcNow; + run.CompletedAt = DateTime.UtcNow; + await _repo.UpdateRunAsync(run); + return; + } + + var cases = await _repo.ListCasesAsync(run.SuiteId); + var enabledCases = cases.Where(c => c.Enabled).ToList(); + + // A caller-selected subset (POST .../run's optional caseIds -- e.g. "re-run only the + // cases that just failed") narrows the enabled set further. Null/empty means every + // enabled case, unchanged from before this field existed. + if (run.CaseIds is { Count: > 0 }) + { + var allowed = new HashSet(run.CaseIds, StringComparer.Ordinal); + enabledCases = enabledCases.Where(c => allowed.Contains(c.Id)).ToList(); + + if (enabledCases.Count == 0) + { + // Same "nothing executed, reports green" defect AgentTestCaseRunner already + // guards for a case with zero turns: FailedCount == 0 && ErrorCount == 0 below is + // vacuously true when the loop never runs at all, which is exactly what happens + // when every id named by a non-empty CaseIds filter is unknown or disabled in + // this suite. End the run as an infrastructure Error instead of a false Passed. + _logger.LogError( + "Agent test run {RunId} named {CaseIdCount} case id(s) via CaseIds, but none " + + "of them matched an enabled case in suite {SuiteId}.", + runId, run.CaseIds.Count, run.SuiteId); + run.Status = AgentTestStatus.Error; + run.Error = + $"None of the {run.CaseIds.Count} selected case(s) could run: each one is either " + + "disabled or no longer in this suite. Enable them and run again."; + run.StartedAt ??= DateTime.UtcNow; + run.CompletedAt = DateTime.UtcNow; + await _repo.UpdateRunAsync(run); + return; + } + } + + run.Status = AgentTestStatus.Running; + run.StartedAt = DateTime.UtcNow; + run.TotalCount = 0; + run.PassedCount = 0; + run.FailedCount = 0; + run.ErrorCount = 0; + await _repo.UpdateRunAsync(run); + + var cancelled = false; + + // The model dimension. An absent/empty Models list means "one pass, using each agent's own + // LlmConfig" -- byte-for-byte the behavior from before multi-model existed, which is what + // keeps every historical run document and every caller that omits the field working. + // Otherwise each enabled case runs once per model, so one run yields a case x model grid. + // + // Case-major order (case1/modelA, case1/modelB, case2/modelA, ...) so that a long run shows + // a complete comparison for the first case early instead of only after the first model has + // swept the whole suite. + var models = run.Models is { Count: > 0 } + ? run.Models.Cast().ToList() + : [null]; + + var workItems = enabledCases + .SelectMany(testCase => models.Select(model => (Case: testCase, Model: model))) + .ToList(); + + foreach (var (testCase, model) in workItems) + { + if (ct.IsCancellationRequested) + { + // The host itself is shutting down (BackgroundService's stoppingToken), not a + // user-requested cancel of THIS run. Leave the row as Running rather than racing + // the shutdown grace period to persist a different terminal status here -- the + // queue's own startup reconciliation step sweeps any Running row a killed process + // left behind into Error the next time it boots. + return; + } + + // `run` here is either the initial load (for the first case) or the post-case read + // from the PREVIOUS iteration (see below) -- either way it is the freshest state this + // method has seen, so this check can never miss a cancel that arrived any time up to + // "the moment the previous case finished." + if (run.CancelRequested) + { + cancelled = true; + break; + } + + AgentTestCaseResult result; + try + { + result = await _caseRunner.RunAsync(suite, testCase, runId, model, ct); + } + catch (Exception ex) + { + // A single case crashing must not abort the run -- record it as Error and move on + // to the next (case, model) pair. One model blowing up (a bad deployment name, a + // revoked key) must not cost the comparison the other models' results. + _logger.LogError( + ex, "Agent test case {CaseId} crashed under test run {RunId} with model {Model}.", + testCase.Id, runId, model?.ToString() ?? ""); + result = new AgentTestCaseResult + { + RunId = runId, + CaseId = testCase.Id, + CaseName = testCase.Name, + // Same reason the runner stamps these up front: an unattributed row is dead + // weight in a grid keyed by model. + Provider = model?.Provider, + Model = model?.Model, + Status = AgentTestStatus.Error, + Error = ex.Message + }; + } + + await _repo.AddCaseResultAsync(result); + + // Re-read AFTER the case ran, not before. This is what picks up a concurrent + // POST /runs/{id}/cancel that landed WHILE the case was executing -- both for the + // NEXT iteration's check above, and so the persist a few lines down (a whole-document + // ReplaceOneAsync) never overwrites that external write with a stale + // CancelRequested=false. `run` becomes this fresh object for the remainder of the + // method (including the terminal write after the loop, if this was the last case) -- + // that's what keeps a cancel that arrives during the very last case's own execution + // from being silently erased, with no extra read needed after the loop. + run = await _repo.GetRunAsync(runId) ?? run; + + run.TotalCount++; + switch (result.Status) + { + case AgentTestStatus.Passed: + run.PassedCount++; + break; + case AgentTestStatus.Failed: + run.FailedCount++; + break; + default: + // Error, Cancelled (e.g. a per-case timeout inside the real case runner), or + // any other non-Passed/Failed status all count against ErrorCount -- AgentTestRun + // has no separate CancelledCount field, and a per-case timeout is an + // infrastructure failure for THAT case, not a graceful run-level cancellation. + run.ErrorCount++; + break; + } + + // Real-time accumulation, per case -- not just at the very end. Without this, GET + // /agent-test/runs/{id} shows 0/0/0/0 for the run's entire duration, and a process + // death mid-run leaves the startup sweep's Error row claiming zero cases ran even + // though N AgentTestCaseResult rows already exist for it. + await _repo.UpdateRunAsync(run); + } + + run.Status = cancelled + ? AgentTestStatus.Cancelled + : run.FailedCount == 0 && run.ErrorCount == 0 + ? AgentTestStatus.Passed + : AgentTestStatus.Failed; + run.CompletedAt = DateTime.UtcNow; + await _repo.UpdateRunAsync(run); + } +} diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Services/AgentTestRunQueue.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/AgentTestRunQueue.cs new file mode 100644 index 000000000..d21c690ea --- /dev/null +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/AgentTestRunQueue.cs @@ -0,0 +1,187 @@ +using System.Threading.Channels; +using Microsoft.Extensions.Hosting; +using BotSharp.Plugin.AgentTesting.Repositories; + +namespace BotSharp.Plugin.AgentTesting.Services; + +public interface IAgentTestRunQueue +{ + void Enqueue(string runId); +} + +/// +/// An in-process, unbounded, single-consumer run queue: POST .../run only drops a Pending run's id +/// in here and returns immediately, and the real execution happens serially in this +/// BackgroundService's loop. +/// +/// The DI shape follows WeChatBackgroundService elsewhere in this repo -- the one existing +/// precedent for "both a singleton and a BackgroundService": register the concrete type, have +/// AddHostedService forward to that same instance, then forward the interface type to it as well. +/// All three point at one object, so Enqueue and the background loop share a Channel. +/// +/// A fresh DI scope per CASE, not per RUN -- see the comment on ScopedCaseRunner. Each dequeue here +/// opens only an outer scope that lives for the whole run, used solely to resolve +/// IAgentTestRepository and ILogger<AgentTestRunExecutor>, neither of which carries dangerous +/// cross-case state. The layer that would actually leak BotSharp scoped services between two cases +/// (IConversationService/IConversationStateService, and TestMockExecutorProvider's ambient +/// conversation id) is isolated in ScopedCaseRunner's own inner scope, reopened on every RunAsync. +/// +public class AgentTestRunQueue : BackgroundService, IAgentTestRunQueue +{ + private readonly Channel _queue = Channel.CreateUnbounded(); + private readonly IServiceProvider _serviceProvider; + private readonly ILogger _logger; + + public AgentTestRunQueue(IServiceProvider serviceProvider, ILogger logger) + { + _serviceProvider = serviceProvider; + _logger = logger; + } + + public void Enqueue(string runId) + { + if (!_queue.Writer.TryWrite(runId)) + { + _logger.LogError("Failed to enqueue agent test run {RunId}: the queue's writer rejected it.", runId); + } + } + + protected override async Task ExecuteAsync(CancellationToken stoppingToken) + { + // The queue is in-process, so a restart wipes out every run still in flight. Without + // sweeping them to Error they stay Running forever and the admin page keeps claiming they + // are still going. Runs once per host start. + await ReconcileStaleRunningRunsAsync(); + + while (!stoppingToken.IsCancellationRequested) + { + try + { + var runId = await _queue.Reader.ReadAsync(stoppingToken); + await ProcessAsync(runId, stoppingToken); + } + catch (OperationCanceledException) + { + break; + } + catch (Exception ex) + { + _logger.LogError(ex, "Unexpected failure in the agent test run queue loop."); + } + } + } + + private async Task ReconcileStaleRunningRunsAsync() + { + try + { + using var scope = _serviceProvider.CreateScope(); + var repo = scope.ServiceProvider.GetRequiredService(); + + // Status-filtered at the query level, not ListRunsAsync(null) filtered in memory -- + // this collection only ever grows, and every host restart used to pull the entire + // thing into memory just to find a handful of stale Running rows. + var runs = await repo.ListRunsByStatusAsync(AgentTestStatus.Running); + foreach (var run in runs) + { + run.Status = AgentTestStatus.Error; + run.Error = "The host restarted while this run was still going, so it was abandoned. " + + "The queue is in-process and does not survive a restart -- trigger the run again."; + run.CompletedAt = DateTime.UtcNow; + await repo.UpdateRunAsync(run); + _logger.LogWarning( + "Agent test run {RunId} was left Running by a previous process; marked Error on startup.", + run.Id); + } + } + catch (Exception ex) + { + _logger.LogError(ex, "Failed to reconcile leftover Running agent test runs on startup."); + } + } + + private async Task ProcessAsync(string runId, CancellationToken ct) + { + try + { + using var scope = _serviceProvider.CreateScope(); + var repo = scope.ServiceProvider.GetRequiredService(); + var executorLogger = scope.ServiceProvider.GetRequiredService>(); + + var executor = new AgentTestRunExecutor(repo, new ScopedCaseRunner(_serviceProvider), executorLogger); + await executor.ExecuteAsync(runId, ct); + } + catch (Exception ex) + { + _logger.LogError(ex, "Agent test run {RunId} crashed outside of case-level handling.", runId); + await TryMarkRunAsErrorAsync(runId, ex.Message); + } + } + + /// + /// Surfaced on the run itself. A crash outside case-level handling produces no case results at + /// all, so without this the API reports Error with an empty result list and the reason exists + /// only in this process's log. + /// + private async Task TryMarkRunAsErrorAsync(string runId, string? reason = null) + { + try + { + using var scope = _serviceProvider.CreateScope(); + var repo = scope.ServiceProvider.GetRequiredService(); + + var run = await repo.GetRunAsync(runId); + if (run != null && run.Status != AgentTestStatus.Error) + { + run.Status = AgentTestStatus.Error; + run.Error = string.IsNullOrWhiteSpace(reason) + ? "The run crashed before it could record a result." + : $"The run crashed before it could record a result: {reason}"; + run.CompletedAt = DateTime.UtcNow; + await repo.UpdateRunAsync(run); + } + } + catch (Exception ex) + { + _logger.LogError(ex, "Failed to mark agent test run {RunId} as Error after a queue-level crash.", runId); + } + } + + /// + /// This is where "a fresh DI scope per case" actually happens. + /// AgentTestRunExecutor.ExecuteAsync calls _caseRunner.RunAsync once per enabled case and + /// neither knows nor cares whether the same ICaseRunner instance is behind it. This wrapper + /// exploits that: it performs no orchestration of its own, and on every RunAsync call it opens + /// a brand-new DI scope, resolves the real ICaseRunner from it (AgentTestCaseRunner, along with + /// the whole scoped dependency chain it drags in -- IAgentConversationDriver, + /// IConversationService, IConversationStateService, TestMockExecutorProvider), and disposes the + /// scope once that one case is done. + /// + /// Why this is necessary rather than fastidious: TestMockExecutorProvider.TryResolve finds + /// mocks by the ambient ConversationService._conversationId, not by an explicit argument, and + /// ConversationStateService caches cross-turn state in memory. If several cases in one run + /// shared a scope -- and therefore one IConversationService/IConversationStateService instance + /// -- the next case's PrepareAsync would repoint the ambient conversation id at itself, and an + /// orphaned call left over from a previous case's timeout could then unregister the entry the + /// NEW case had just registered. The mock seam disappears and the orphaned call lands on the + /// real tool implementation: a real phone call, a real email. A scope per case means those two + /// BotSharp scoped services are never the same object across two cases, and the path does not + /// exist. + /// + private sealed class ScopedCaseRunner : ICaseRunner + { + private readonly IServiceProvider _serviceProvider; + + public ScopedCaseRunner(IServiceProvider serviceProvider) + { + _serviceProvider = serviceProvider; + } + + public async Task RunAsync(AgentTestSuite suite, AgentTestCase testCase, string runId, TestModel? model, CancellationToken ct) + { + using var scope = _serviceProvider.CreateScope(); + var runner = scope.ServiceProvider.GetRequiredService(); + return await runner.RunAsync(suite, testCase, runId, model, ct); + } + } +} diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Services/AssertionContext.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/AssertionContext.cs new file mode 100644 index 000000000..6234163e7 --- /dev/null +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/AssertionContext.cs @@ -0,0 +1,13 @@ +namespace BotSharp.Plugin.AgentTesting.Services; + +/// +/// Everything an assertion is evaluated against. Turn-level and case-level share this shape and +/// differ only in how much of it is populated. +/// +public class AssertionContext +{ + public string? Output { get; set; } + public IReadOnlyList ToolCalls { get; set; } = []; + public IReadOnlyDictionary States { get; set; } = new Dictionary(); + public string? RoutedToAgent { get; set; } +} diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Services/AssertionEvaluator.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/AssertionEvaluator.cs new file mode 100644 index 000000000..6041beb7c --- /dev/null +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/AssertionEvaluator.cs @@ -0,0 +1,260 @@ +using System.Text.RegularExpressions; +using BotSharp.Plugin.AgentTesting.Runtime; + +namespace BotSharp.Plugin.AgentTesting.Services; + +public static class AssertionTypes +{ + public const string OutputContains = "outputContains"; + public const string OutputNotContains = "outputNotContains"; + public const string OutputRegex = "outputRegex"; + public const string ToolCalled = "toolCalled"; + public const string ToolNotCalled = "toolNotCalled"; + public const string StateEquals = "stateEquals"; + public const string RoutedToAgent = "routedToAgent"; + public const string LlmJudge = "llmJudge"; + + /// + /// Result-only. Never authored on a case and never evaluated -- AgentTestCaseRunner synthesises + /// it when the mock seam blocked a tool, so the block surfaces in the ordinary assertion table + /// instead of only in Observed Tool Calls. Deliberately absent from AssertionValidation's + /// Requirements map, which covers the eight authorable types. + /// + public const string NoBlockedTools = "noBlockedTools"; +} + +/// +/// Evaluating an assertion is a pure function: the same (assertion, context) always yields the same +/// AssertionResult, with no I/O and no service dependencies. The runner calls it once per turn, and +/// again for case-level assertions after every turn has run. That purity is exactly what makes it +/// usable as the pass/fail verdict -- reproducible and explainable. +/// +public static class AssertionEvaluator +{ + public static AssertionResult Evaluate(TestAssertion assertion, AssertionContext context) + { + var result = new AssertionResult + { + Type = assertion.Type, + Target = assertion.Target, + Expected = assertion.Expected + }; + + switch (assertion.Type) + { + case AssertionTypes.OutputContains: + result.Actual = context.Output; + if (string.IsNullOrEmpty(assertion.Expected)) + { + // Contains("") is vacuously true for any non-null string -- a blank/omitted + // Expected must not read as "the output contains nothing," which verifies + // nothing and would always report Passed. + result.Passed = false; + result.Message = "outputContains requires a non-empty 'expected' value"; + } + else + { + result.Passed = context.Output?.Contains(assertion.Expected, + StringComparison.OrdinalIgnoreCase) == true; + if (!result.Passed) result.Message = "output does not contain the expected text"; + } + break; + + case AssertionTypes.OutputNotContains: + result.Actual = context.Output; + result.Passed = context.Output?.Contains(assertion.Expected ?? string.Empty, + StringComparison.OrdinalIgnoreCase) != true; + if (!result.Passed) result.Message = "output contains text that should not appear"; + break; + + case AssertionTypes.OutputRegex: + result.Actual = context.Output; + if (string.IsNullOrEmpty(assertion.Expected)) + { + // An empty pattern matches everything -- a blank/omitted Expected must not + // read as "match anything," which verifies nothing and would always pass. + result.Passed = false; + result.Message = "outputRegex requires a non-empty 'expected' pattern"; + break; + } + try + { + result.Passed = Regex.IsMatch(context.Output ?? string.Empty, assertion.Expected, + RegexOptions.IgnoreCase, TimeSpan.FromSeconds(1)); + if (!result.Passed) result.Message = "output does not match the pattern"; + } + catch (ArgumentException) + { + // The regex is user input and being malformed is routine. Fail the assertion + // and say why, rather than recording the whole case as an infrastructure error. + result.Passed = false; + result.Message = $"invalid regular expression: {assertion.Expected}"; + } + catch (RegexMatchTimeoutException) + { + result.Passed = false; + result.Message = "regular expression timed out"; + } + break; + + case AssertionTypes.ToolCalled: + { + var matches = context.ToolCalls + .Where(c => string.Equals(c.FunctionName, assertion.Target, StringComparison.OrdinalIgnoreCase)) + .ToList(); + result.Actual = string.Join(", ", matches.Select(m => m.ArgsJson ?? "{}")); + + if (matches.Count == 0) + { + result.Passed = false; + result.Message = "the tool was never called"; + } + else if (string.IsNullOrWhiteSpace(assertion.ArgsMatchJson)) + { + result.Passed = true; + } + else + { + // ArgsMatchJson comes from the test author and ArgsJson from model output -- + // either can be blank, syntactically invalid, or syntactically valid but with + // duplicate top-level keys (JsonObject's dictionary materialises lazily, so the + // ArgumentException only fires on first access inside IsSubset). ParseOrNull + // already collapses that whole family of failures into "returns null", so the + // parsing is not rewritten here. + var expected = ToolMockMatcher.ParseOrNull(assertion.ArgsMatchJson); + if (expected == null) + { + result.Passed = false; + result.Message = "argument json could not be parsed"; + } + else + { + result.Passed = matches.Any(m => + { + var actual = ToolMockMatcher.ParseOrNull(m.ArgsJson); + return actual != null && ToolMockMatcher.IsSubset(expected, actual); + }); + if (!result.Passed) result.Message = "the tool was called with different arguments"; + } + } + break; + } + + case AssertionTypes.ToolNotCalled: + { + if (string.IsNullOrWhiteSpace(assertion.Target)) + { + // A null/blank Target matches no real call's FunctionName, so this would + // otherwise always report "not called" -- vacuously passing without ever + // naming a tool to check. + result.Passed = false; + result.Message = "toolNotCalled requires a non-empty 'target' function name"; + break; + } + + // A blocked call still counts as "called": the agent did try to call it, and that + // attempt is exactly the behaviour being asserted on. + var called = context.ToolCalls + .Any(c => string.Equals(c.FunctionName, assertion.Target, StringComparison.OrdinalIgnoreCase)); + result.Passed = !called; + result.Actual = called ? "called" : "not called"; + if (!result.Passed) result.Message = "the tool should not have been called"; + break; + } + + case AssertionTypes.StateEquals: + if (assertion.Target != null && context.States.TryGetValue(assertion.Target, out var value)) + { + result.Actual = value; + result.Passed = string.Equals(value, assertion.Expected, StringComparison.Ordinal); + if (!result.Passed) result.Message = "state value differs from the expected value"; + } + else + { + result.Passed = false; + result.Message = $"state '{assertion.Target}' is not set"; + } + break; + + case AssertionTypes.RoutedToAgent: + result.Actual = context.RoutedToAgent; + if (string.IsNullOrWhiteSpace(assertion.Expected)) + { + // A null Expected compares equal to a null RoutedToAgent (e.g. the canary/no + // routing information case) -- a blank/omitted Expected must not read as + // "expect no routing," which verifies nothing and would always pass. + result.Passed = false; + result.Message = "routedToAgent requires a non-empty 'expected' agent name"; + break; + } + + result.Passed = string.Equals(context.RoutedToAgent, assertion.Expected, + StringComparison.OrdinalIgnoreCase); + if (!result.Passed) result.Message = "the conversation was handled by a different agent"; + break; + + case AssertionTypes.LlmJudge: + // P2 will wire this to an IInstructService judge. P1 fails explicitly and never + // passes silently -- passing silently would show a case that verified nothing as + // green. + result.Passed = false; + result.Message = "llmJudge is not available in P1"; + break; + + default: + result.Passed = false; + result.Message = $"unknown assertion type '{assertion.Type}'"; + break; + } + + return result; + } +} + +/// +/// Save-time counterpart to the four fixed branches above +/// (outputContains/outputRegex/toolNotCalled/routedToAgent): an assertion missing the one field +/// its type actually needs to verify anything is rejected at case create/update +/// (AgentTestController), not just left to fail at run time. toolCalled/stateEquals already +/// fail safe on a null Target at evaluation time; they get the same save-time guard here for +/// consistency, so a typo'd blank field is caught at authoring time for every assertion type, +/// not only the four that would otherwise vacuously pass. +/// +public static class AssertionValidation +{ + private enum RequiredField { Expected, Target } + + // One row per AssertionTypes constant -- eight total. + private static readonly Dictionary Requirements = new(StringComparer.Ordinal) + { + [AssertionTypes.OutputContains] = RequiredField.Expected, + [AssertionTypes.OutputNotContains] = RequiredField.Expected, + [AssertionTypes.OutputRegex] = RequiredField.Expected, + [AssertionTypes.ToolCalled] = RequiredField.Target, + [AssertionTypes.ToolNotCalled] = RequiredField.Target, + [AssertionTypes.StateEquals] = RequiredField.Target, + [AssertionTypes.RoutedToAgent] = RequiredField.Expected, + [AssertionTypes.LlmJudge] = RequiredField.Expected, + }; + + /// Null when the assertion is well-formed; otherwise a caller-facing error message. + public static string? Validate(TestAssertion assertion) + { + // An unrecognized type is not this method's job to reject -- AssertionEvaluator's own + // `default` branch already fails it loudly at evaluation time, and rejecting it here too + // would block saving a case authored against a not-yet-released assertion type. + if (!Requirements.TryGetValue(assertion.Type, out var required)) + { + return null; + } + + return required switch + { + RequiredField.Expected when string.IsNullOrWhiteSpace(assertion.Expected) + => $"assertion '{assertion.Type}' requires a non-empty 'expected' value", + RequiredField.Target when string.IsNullOrWhiteSpace(assertion.Target) + => $"assertion '{assertion.Type}' requires a non-empty 'target' value", + _ => null + }; + } +} diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Services/BotSharpAgentConversationDriver.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/BotSharpAgentConversationDriver.cs new file mode 100644 index 000000000..75ff79711 --- /dev/null +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/BotSharpAgentConversationDriver.cs @@ -0,0 +1,198 @@ +using BotSharp.Abstraction.Agents; +using BotSharp.Abstraction.Agents.Enums; +using BotSharp.Abstraction.Models; +using BotSharp.Abstraction.Repositories; +using BotSharp.Abstraction.Routing; +using BotSharp.Plugin.AgentTesting.Runtime; + +namespace BotSharp.Plugin.AgentTesting.Services; + +/// +/// The layer that talks to a real BotSharp conversation. Deliberately untested by unit tests -- +/// testing it would amount to testing BotSharp itself, and its correctness is established by the +/// end-to-end smoke run instead. Before changing anything here, check the real signatures in the +/// BotSharp source: they have changed between versions, and this implementation was written against +/// the sibling worktree's source rather than from documentation. +/// +/// How `ct` is handled: neither SendMessage nor InvokeFunction takes a CancellationToken, and +/// BotSharp's internal routing/tool-invocation loop ignores cancellation entirely. So this only +/// fails fast with a single ct.ThrowIfCancellationRequested() at method entry, before any real call +/// has gone out and while there is therefore no orphan risk. It must never wrap the returned Task +/// in .WaitAsync(ct): that would make "the caller stopped waiting" look identical to "the call +/// actually stopped", and AgentTestCaseRunner needs the raw Task itself so that on a timeout it can +/// keep it running in the background and only remove the registry entry once it truly finishes -- +/// otherwise the mock seam vanishes while an orphaned call is still running and its next tool call +/// lands on the real implementation. Anyone about to add a .WaitAsync to these two methods should +/// read this paragraph again first. +/// +public class BotSharpAgentConversationDriver : IAgentConversationDriver +{ + private readonly IConversationService _conversations; + private readonly IRoutingService _routing; + private readonly IBotSharpRepository _repository; + private readonly IAgentService _agents; + private readonly ILogger _logger; + + // Set at most once per instance: this driver is scoped per-case (a fresh DI scope per case, + // see AgentTestRunQueue.ScopedCaseRunner), so exactly one conversation ever passes through it. + private bool _conversationTagged; + + public BotSharpAgentConversationDriver( + IConversationService conversations, + IRoutingService routing, + IBotSharpRepository repository, + IAgentService agents, + ILogger logger) + { + _conversations = conversations; + _routing = routing; + _repository = repository; + _agents = agents; + _logger = logger; + } + + public async Task PrepareAsync(string conversationId, string agentId, IReadOnlyList initialStates) + { + // SendMessage (called later, per turn) is what actually creates the conversation row via + // GetConversationRecordOrCreateNew(agentId) -- this step only has to bind the ambient + // conversation id and seed its initial states before that happens. + // + // Deliberately does NOT seed a "channel" state. An earlier version of this method did, + // to stamp Conversation.Channel as synthetic before the row was created -- but "channel" + // is CustomStateKeys.Channel, a load-bearing PRODUCTION state key with ~30 readers across + // this codebase (IntelligentDiagnosisRoutingHook.OnRoutingRulesLoaded branches on it + // against ConversationChannel.OpenAPI before routing rules even load; several Functions + // gate visibility/behavior on it too), none of which recognize "agent-test" as a value -- + // forcing it changed real routing/business-logic branches mid-case. It also silently + // corrupted recording: AgentTestRecorder.BuildDraft copies every seeded (MessageId == null) + // state into InitialStates, so a recorded case's OWN real channel was overwritten by this + // marker on every recording, and replaying that case then ran a DIFFERENT code path than + // the one actually recorded. The "test-set" conversation tag (EnsureConversationTaggedAsync + // below) already satisfies "make a harness conversation identifiable after the fact" on its + // own -- see AgentTestConversationMarker's remaining doc comment. Do not reintroduce a + // channel/other state-key marker without first grepping every reader of that key. + var states = initialStates + .Select(s => new MessageState(s.Key, s.Value, s.ActiveRounds, s.Global)) + .ToList(); + + await _conversations.SetConversationId(conversationId, states); + } + + public async Task SendAsync(string conversationId, string agentId, string userMessage, CancellationToken ct) + { + // Fail fast only if the case had already timed out before we ever got here (e.g. a prior + // turn ran long). Once the real call below starts, there is no cancellation hook left to + // reach -- see the class-level note. + ct.ThrowIfCancellationRequested(); + + RoleDialogModel? last = null; + + await _conversations.SendMessage( + agentId, + new RoleDialogModel(AgentRole.User, userMessage), + replyMessage: null, + onResponseReceived: r => + { + last = r; + return Task.CompletedTask; + }); + + // The conversation row is only guaranteed to exist once the first SendMessage call above + // has returned (see PrepareAsync's note) -- tagging any earlier would either no-op against + // a row that doesn't exist yet or race its creation. Tagging is best-effort: a failure here + // must never fail the case itself, since the case's own assertions -- not a housekeeping + // label -- are what decide pass/fail. + await EnsureConversationTaggedAsync(conversationId); + + return last?.RichContent?.Message?.Text ?? last?.Content; + } + + private async Task EnsureConversationTaggedAsync(string conversationId) + { + if (_conversationTagged) + { + return; + } + + _conversationTagged = true; + try + { + await _repository.AppendConversationTags(conversationId, [AgentTestConversationMarker.Tag]); + } + catch (Exception ex) + { + _logger.LogWarning(ex, + "Failed to tag agent test conversation {ConversationId} with {Tag}; the case's own " + + "result is unaffected, but this conversation will not be reverse-traceable as synthetic.", + conversationId, AgentTestConversationMarker.Tag); + } + } + + public async Task RunCanaryAsync(string conversationId, string agentId, CancellationToken ct) + { + ct.ThrowIfCancellationRequested(); + + var message = new RoleDialogModel(AgentRole.Assistant, string.Empty) + { + FunctionName = AgentTestCanary.FunctionName, + CurrentAgentId = agentId + }; + + // The bool RoutingService.InvokeFunction returns is not the signal we want here: it is + // whether some executor ran, not whether OUR mock seam was the one that ran it. The seam + // is only proven live by the content it stamps onto the message -- see MockFunctionExecutor. + await _routing.InvokeFunction(AgentTestCanary.FunctionName, message); + + return message.Content == AgentTestCanary.ExpectedContent; + } + + public Task> ReadStatesAsync(string conversationId) + { + // Fix round 1, Finding 2: MockFunctionExecutor's StateWrites (and PrepareAsync's + // InitialStates) go through IConversationStateService.SetState, which only mutates the + // in-memory _curStates dictionary. The single writer of the PERSISTED store -- + // ConversationStateService.Save() -> IBotSharpRepository.UpdateConversationStates -- is + // never called anywhere on the SendMessage/InstructDirect/InstructLoop/InvokeFunction call + // chain this driver drives (confirmed by reading every caller of Save()/ + // UpdateConversationStates in both BotSharp.Core and the Mongo storage plugin: it is + // transport middleware, controllers, the rule engine, and onebrain's own hooks/functions -- + // none of which run here). So reading via IBotSharpRepository.GetConversationStates (the + // original implementation) would see a brand-new conversation's states as permanently + // empty, and every stateEquals assertion would report "state 'X' is not set" against a + // correctly-behaving agent. + // + // IConversationService.States is the SAME IConversationStateService instance + // (ConversationService.States => _state, constructor-injected) that TestMockExecutorProvider + // handed to MockFunctionExecutor -- both come out of the one DI scope this whole + // conversation turn runs in, so GetStates() observes those writes directly, with no + // persistence round-trip and no risk of a silent no-op (ConversationStateService.Save() + // early-returns under some conditions -- e.g. sidecar mode -- that reading live state + // sidesteps entirely). GetStates() also correctly excludes values whose ActiveRounds window + // has already expired (StateValue.Active == false), which the old repository-based read did + // not. + var states = _conversations.States.GetStates(); + + var result = new Dictionary(StringComparer.OrdinalIgnoreCase); + foreach (var pair in states) + { + result[pair.Key] = pair.Value; + } + + return Task.FromResult>(result); + } + + public async Task ReadRoutedAgentNameAsync(string conversationId) + { + var dialogs = await _repository.GetConversationDialogs(conversationId); + var lastAssistantDialog = dialogs.LastOrDefault(d => d.MetaData?.Role == AgentRole.Assistant); + + var agentId = lastAssistantDialog?.MetaData?.AgentId; + if (string.IsNullOrEmpty(agentId)) + { + return null; + } + + var agent = await _agents.GetAgent(agentId); + return agent?.Name; + } +} diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Services/IAgentConversationDriver.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/IAgentConversationDriver.cs new file mode 100644 index 000000000..4226daebb --- /dev/null +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/IAgentConversationDriver.cs @@ -0,0 +1,21 @@ +namespace BotSharp.Plugin.AgentTesting.Services; + +/// +/// Puts "talking to a BotSharp conversation" behind a seam, which is what makes the runner's +/// orchestration unit-testable at all -- otherwise testing multi-turn orchestration once would +/// require a real Mongo and a real model. +/// +public interface IAgentConversationDriver +{ + Task PrepareAsync(string conversationId, string agentId, IReadOnlyList initialStates); + + /// Drives one turn and returns that turn's output text. + Task SendAsync(string conversationId, string agentId, string userMessage, CancellationToken ct); + + /// Calls the canary function once; returns whether the mock seam took it over. + Task RunCanaryAsync(string conversationId, string agentId, CancellationToken ct); + + Task> ReadStatesAsync(string conversationId); + + Task ReadRoutedAgentNameAsync(string conversationId); +} diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Services/ICaseRunner.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/ICaseRunner.cs new file mode 100644 index 000000000..3c4ce3737 --- /dev/null +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/ICaseRunner.cs @@ -0,0 +1,17 @@ +namespace BotSharp.Plugin.AgentTesting.Services; + +/// +/// The seam for running one case. Extracted so AgentTestRunExecutor's orchestration -- serial +/// execution, one crashing case not aborting the run, cancellation taking effect promptly -- can be +/// unit-tested without a real BotSharp, and so the production implementation can be swapped for a +/// wrapper that opens a fresh DI scope per call without changing AgentTestRunExecutor's constructor +/// signature. +/// +public interface ICaseRunner +{ + /// + /// The model this execution is forced onto; null = use the agent's own LlmConfig (the existing + /// behaviour when no models were requested). + /// + Task RunAsync(AgentTestSuite suite, AgentTestCase testCase, string runId, TestModel? model, CancellationToken ct); +} diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Services/ICaseSegmenter.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/ICaseSegmenter.cs new file mode 100644 index 000000000..0ffaa2e0a --- /dev/null +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/ICaseSegmenter.cs @@ -0,0 +1,44 @@ +namespace BotSharp.Plugin.AgentTesting.Services; + +/// +/// One scenario inside a recorded conversation: turns through +/// inclusive (0-based, counted by user message), to become its own case. +/// +public record CaseSegment(string Name, int FirstTurn, int LastTurn); + +/// One turn as the segmenter sees it -- deliberately carries no function arguments and no +/// return content, see . +public class SegmentableTurn +{ + public int Index { get; set; } + public string UserMessage { get; set; } = string.Empty; + + /// Names (only the names) of the functions this turn called. + public IReadOnlyList ToolNames { get; set; } = []; +} + +/// +/// Splits a conversation into one or more scenarios. A segmenter decides ONLY where to cut and what +/// to call each piece -- mock return values, assertions and state are still generated verbatim from +/// the real conversation by , which no model touches. +/// +/// That boundary is deliberate, not laziness: +/// 1) the moment a mock's ResultContent is written by a model, the case stops being a replay of a +/// call that really happened -- and replay is the entire reason recording exists; +/// 2) letting a model write assertions inevitably produces output-text assertions like +/// outputContains, which go red on any rewording -- exactly what the AgentTestRecorder class +/// comment already rules out. +/// +/// An implementation only ever sees the user messages and which function NAMES each turn called -- +/// function arguments and return content do not leave this process. The densest PII in a +/// conversation (addresses, phone numbers, work order details) usually sits in those arguments and +/// results, and segmentation does not need them. User messages can still contain PII; that cannot be +/// removed, and callers need to know it. +/// +public interface ICaseSegmenter +{ + Task> SegmentAsync( + IReadOnlyList turns, + TestModel model, + CancellationToken ct); +} diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Services/LlmCaseSegmenter.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/LlmCaseSegmenter.cs new file mode 100644 index 000000000..f57b841f5 --- /dev/null +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/LlmCaseSegmenter.cs @@ -0,0 +1,215 @@ +using System.Text; +using System.Text.Json; +using BotSharp.Abstraction.Agents.Enums; +using BotSharp.Abstraction.MLTasks; + +namespace BotSharp.Plugin.AgentTesting.Services; + +/// +/// The model-backed . Contract and boundary live on the interface -- +/// this class only asks a model where to cut, and extends its answer no trust: out-of-range, +/// overlapping, out-of-order or unnamed segments are all rejected. Better to fail and let a human +/// retry than to accept a result that looks usable but cut the cases in the wrong places. +/// +public class LlmCaseSegmenter : ICaseSegmenter +{ + private readonly IServiceProvider _services; + private readonly ILogger _logger; + + public LlmCaseSegmenter(IServiceProvider services, ILogger logger) + { + _services = services; + _logger = logger; + } + + public async Task> SegmentAsync( + IReadOnlyList turns, + TestModel model, + CancellationToken ct) + { + if (turns.Count == 0) + { + return []; + } + + // Nothing to decide with a single turn, and it saves a model call on the commonest case. + if (turns.Count == 1) + { + return [new CaseSegment(FallbackName(turns[0]), 0, 0)]; + } + + ct.ThrowIfCancellationRequested(); + + // Resolved straight from DI rather than through BotSharp.Core's CompletionProvider helper, + // for two reasons: this plugin only references BotSharp.Abstraction (adding a BotSharp.Core + // project reference for one call would change its build topology, which differs between + // DebugBrain.sln and OneBrain.sln in the consuming host), and that helper also writes provider/model into the + // ambient conversation state -- unwanted here, since segmentation is a one-off call that + // belongs to no conversation. + var completion = _services.GetServices() + .FirstOrDefault(x => string.Equals(x.Provider, model.Provider, StringComparison.OrdinalIgnoreCase)); + + if (completion == null) + { + throw new InvalidOperationException($"no chat completion provider is registered for '{model.Provider}'"); + } + + completion.SetModelName(model.Model); + + var promptAgent = new Agent + { + Id = Guid.Empty.ToString(), + Name = "AgentTestCaseSegmenter", + Instruction = BuildInstruction() + }; + + var response = await completion.GetChatCompletions( + promptAgent, + [new RoleDialogModel(AgentRole.User, BuildTranscript(turns))]); + + var raw = response?.Content ?? string.Empty; + var segments = Parse(raw, turns.Count); + + _logger.LogInformation( + "Case segmenter split a {TurnCount}-turn conversation into {SegmentCount} case(s) using {Model}.", + turns.Count, segments.Count, model); + + return segments; + } + + private static string FallbackName(SegmentableTurn turn) + { + var text = turn.UserMessage.Trim(); + return string.IsNullOrEmpty(text) ? "Recorded case" : Truncate(text, 60); + } + + private static string Truncate(string text, int max) + => text.Length <= max ? text : text[..max].TrimEnd() + "..."; + + private static string BuildInstruction() => + """ + You split a recorded support conversation into independent regression test cases. + + A case is one self-contained thing the user wanted. Start a new case when the user moves on + to a different goal (for example: from "where is my technician" to "reschedule my + appointment"). Follow-up turns that refine, confirm or correct the SAME goal belong to the + same case. + + Rules: + - Cover every turn exactly once. Segments must be contiguous, in order, non-overlapping, + and together span turn 0 through the last turn. + - Prefer few, meaningful cases. If the whole conversation is one goal, return one segment. + - `name` is a short human label for the case, at most 60 characters, in the language the + user wrote in. Describe the user's goal, not the tools that were called. + + Reply with JSON only, no prose and no code fence: + {"segments":[{"name":"...","firstTurn":0,"lastTurn":2}]} + """; + + private static string BuildTranscript(IReadOnlyList turns) + { + var builder = new StringBuilder(); + foreach (var turn in turns) + { + builder.Append("Turn ").Append(turn.Index).Append(": ").AppendLine(turn.UserMessage); + if (turn.ToolNames.Count > 0) + { + // Names only. Arguments and results stay in this process -- see ICaseSegmenter. + builder.Append(" tools called: ").AppendLine(string.Join(", ", turn.ToolNames)); + } + } + return builder.ToString(); + } + + /// + /// Parses and validates the model's output. Any single violation rejects the whole thing: a + /// half-correct segmentation cuts cases in the wrong place, and that kind of error looks + /// entirely normal in the UI (plausible names, real turns) until somebody actually runs it. + /// + public static IReadOnlyList Parse(string raw, int turnCount) + { + var json = ExtractJson(raw); + if (json == null) + { + throw new InvalidOperationException( + $"the segmenter model did not return JSON. First 200 chars: {Truncate(raw, 200)}"); + } + + SegmentEnvelope? envelope; + try + { + envelope = JsonSerializer.Deserialize(json, new JsonSerializerOptions + { + PropertyNameCaseInsensitive = true + }); + } + catch (JsonException ex) + { + throw new InvalidOperationException($"the segmenter model returned malformed JSON: {ex.Message}"); + } + + var parsed = envelope?.Segments; + if (parsed is not { Count: > 0 }) + { + throw new InvalidOperationException("the segmenter model returned no segments"); + } + + var expectedNext = 0; + var result = new List(); + foreach (var segment in parsed) + { + if (segment.FirstTurn != expectedNext) + { + throw new InvalidOperationException( + $"the segmenter model returned a gap or overlap: expected the next segment to start at " + + $"turn {expectedNext}, got {segment.FirstTurn}"); + } + + if (segment.LastTurn < segment.FirstTurn || segment.LastTurn >= turnCount) + { + throw new InvalidOperationException( + $"the segmenter model returned an out-of-range segment {segment.FirstTurn}..{segment.LastTurn} " + + $"for a conversation with {turnCount} turn(s)"); + } + + var name = string.IsNullOrWhiteSpace(segment.Name) + ? $"Turns {segment.FirstTurn}-{segment.LastTurn}" + : Truncate(segment.Name.Trim(), 60); + + result.Add(new CaseSegment(name, segment.FirstTurn, segment.LastTurn)); + expectedNext = segment.LastTurn + 1; + } + + if (expectedNext != turnCount) + { + throw new InvalidOperationException( + $"the segmenter model left turn(s) {expectedNext}..{turnCount - 1} uncovered"); + } + + return result; + } + + /// + /// Tolerates a model wrapping its JSON in a ```json fence or a sentence of prose -- the most + /// common and most harmless way to disobey "JSON only", and not worth failing over. First '{' + /// through last '}'. + /// + private static string? ExtractJson(string raw) + { + var start = raw.IndexOf('{'); + var end = raw.LastIndexOf('}'); + return start >= 0 && end > start ? raw[start..(end + 1)] : null; + } + + private sealed class SegmentEnvelope + { + public List? Segments { get; set; } + } + + private sealed class SegmentDto + { + public string? Name { get; set; } + public int FirstTurn { get; set; } + public int LastTurn { get; set; } + } +} diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Using.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Using.cs new file mode 100644 index 000000000..127db0d37 --- /dev/null +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Using.cs @@ -0,0 +1,10 @@ +global using BotSharp.Abstraction.Agents.Models; +global using BotSharp.Abstraction.Conversations; +global using BotSharp.Abstraction.Conversations.Models; +global using BotSharp.Abstraction.Plugins; +global using BotSharp.Abstraction.Plugins.Models; +global using BotSharp.Abstraction.Routing.Executor; +global using Microsoft.Extensions.Configuration; +global using Microsoft.Extensions.DependencyInjection; +global using Microsoft.Extensions.Logging; +global using BotSharp.Plugin.AgentTesting.Models; diff --git a/tests/BotSharp.Core.UnitTests/AgentTesting/AgentTestCaseRunnerTests.cs b/tests/BotSharp.Core.UnitTests/AgentTesting/AgentTestCaseRunnerTests.cs new file mode 100644 index 000000000..373aecf63 --- /dev/null +++ b/tests/BotSharp.Core.UnitTests/AgentTesting/AgentTestCaseRunnerTests.cs @@ -0,0 +1,560 @@ +using System; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; +using BotSharp.Plugin.AgentTesting.Runtime; +using BotSharp.Plugin.AgentTesting.Services; +using BotSharp.Plugin.AgentTesting.Models; +using Xunit; + +namespace BotSharp.Core.UnitTests.AgentTesting; + +/// +/// The runner turns one case into one real conversation. A fake driver is used here to unit-test the +/// orchestration itself: whether multiple turns are driven in order, whether a Fatal assertion really +/// aborts the remaining turns, whether a timeout records Error rather than Failed, and most +/// importantly the canary -- if the seam is not live (a build resolving an unpatched BotSharp +/// package, say) the case has to fail explicitly instead of running against real tools and +/// "passing". +/// +public class AgentTestCaseRunnerTests +{ + private sealed class FakeDriver : IAgentConversationDriver + { + public List Sent { get; } = []; + public Queue Replies { get; } = new(); + public bool CanaryResult { get; set; } = true; + public Dictionary States { get; } = new(); + public string? RoutedAgent { get; set; } + public TimeSpan SendDelay { get; set; } = TimeSpan.Zero; + + // Lets the empty-Turns guard test assert that the seam was never touched at all, not merely + // that no message was sent. + public bool PrepareCalled { get; private set; } + public bool CanaryCalled { get; private set; } + + // Fix round 1, Finding 1's orphan-survival test needs a delay that keeps running past the + // runner's own timeout -- exactly like the real driver, whose underlying BotSharp call has + // no cancellation hook at all. Default true preserves every pre-existing test's behavior + // (SendDelay races against `ct`, so it stops as soon as the runner's timeout fires). + public bool HonorCancellationInSend { get; set; } = true; + + // Fix round 1, Finding 3's test: simulates a cancellation that has nothing to do with the + // runner's own timeout or the caller's ct (e.g. an HttpClient timeout inside a passthrough + // tool call). + public bool ThrowUnrelatedCancellation { get; set; } + + // Fix wave item 7's test: simulates the orphaned real driver call itself failing (not + // just running long) after the runner has already given up waiting on it -- e.g. the + // underlying BotSharp SendMessage call throws once it finally does return. + public Exception? ThrowAfterDelay { get; set; } + + public Task PrepareAsync(string conversationId, string agentId, IReadOnlyList initialStates) + { + PrepareCalled = true; + return Task.CompletedTask; + } + + /// + /// Registry + tool names the seam should report as blocked on the first turn. The real + /// blocking happens inside MockFunctionExecutor, which no fake driver goes through, so the + /// observed call it would have recorded is reproduced here instead. + /// + public IAgentTestRunRegistry? Registry { get; set; } + public List BlockedOnFirstSend { get; } = []; + + public async Task SendAsync(string conversationId, string agentId, string userMessage, CancellationToken ct) + { + Sent.Add(userMessage); + + if (BlockedOnFirstSend.Count > 0 && Sent.Count == 1) + { + var active = Registry?.TryGet(conversationId); + foreach (var name in BlockedOnFirstSend) + { + active?.Record(new ObservedToolCall + { + TurnIndex = active.CurrentTurnIndex, + FunctionName = name, + Outcome = "Blocked", + ResultContent = $"[agent-test] blocked unmocked tool: {name}" + }); + } + } + + if (ThrowUnrelatedCancellation) + { + throw new OperationCanceledException( + "simulated unrelated cancellation, e.g. an internal deadline inside a passthrough tool call"); + } + + if (SendDelay > TimeSpan.Zero) + { + if (HonorCancellationInSend) + { + await Task.Delay(SendDelay, ct); + } + else + { + // Deliberately does NOT observe ct -- simulates BotSharp's real SendMessage, + // which keeps running after the runner gives up waiting on it. + await Task.Delay(SendDelay); + } + } + + if (ThrowAfterDelay != null) + { + throw ThrowAfterDelay; + } + + return Replies.Count > 0 ? Replies.Dequeue() : string.Empty; + } + + public Task RunCanaryAsync(string conversationId, string agentId, CancellationToken ct) + { + CanaryCalled = true; + return Task.FromResult(CanaryResult); + } + + public Task> ReadStatesAsync(string conversationId) + => Task.FromResult>(States); + + public Task ReadRoutedAgentNameAsync(string conversationId) + => Task.FromResult(RoutedAgent); + } + + private static AgentTestCaseRunner Build(FakeDriver driver, out AgentTestRunRegistry registry) + { + registry = new AgentTestRunRegistry(); + return new AgentTestCaseRunner(registry, driver, NullLogger.Instance); + } + + private static AgentTestSuite Suite(int timeoutSeconds = 120) => new() + { + Id = "suite-1", + AgentId = "agent-1", + Name = "s", + CaseTimeoutSeconds = timeoutSeconds + }; + + [Fact] + public async Task A_blocked_tool_fails_the_case_even_when_every_authored_assertion_passed() + { + // Blocking is the seam working: the agent reached for a tool this case does not mock, and + // running it for real could have sent an email. But the block also stops that turn, so the + // rest of the conversation never happened -- reporting Passed would be the same + // "executed nothing, reports green" defect the no-turns and canary guards exist to prevent. + var driver = new FakeDriver(); + driver.Replies.Enqueue("ok"); + var runner = Build(driver, out var registry); + driver.Registry = registry; + driver.BlockedOnFirstSend.Add("get_estimate_arrival_time"); + + var result = await runner.RunAsync(Suite(), new AgentTestCase + { + Id = "case-1", + Name = "no assertion covers the blocked tool", + Turns = [new TestTurn { Index = 0, UserMessage = "when is the tech arriving" }] + }, "run-1", model: null, CancellationToken.None); + + Assert.Equal(AgentTestStatus.Failed, result.Status); + + // Reported as an assertion so it lands in the ordinary results table, and as Failed rather + // than Error because the harness did exactly its job. + var blocked = Assert.Single(result.Assertions, a => a.Type == AssertionTypes.NoBlockedTools); + Assert.False(blocked.Passed); + Assert.Contains("get_estimate_arrival_time", blocked.Actual!); + Assert.Null(result.Error); + } + + [Fact] + public async Task A_case_with_no_blocked_tools_gains_no_synthetic_assertion() + { + var driver = new FakeDriver(); + driver.Replies.Enqueue("ok"); + var runner = Build(driver, out _); + + var result = await runner.RunAsync(Suite(), new AgentTestCase + { + Id = "case-1", + Name = "clean", + Turns = [new TestTurn { Index = 0, UserMessage = "hello" }] + }, "run-1", model: null, CancellationToken.None); + + Assert.Equal(AgentTestStatus.Passed, result.Status); + Assert.DoesNotContain(result.Assertions, a => a.Type == AssertionTypes.NoBlockedTools); + } + + [Fact] + public async Task Drives_every_turn_in_order() + { + // Fix round 1, Finding 5: turns are enqueued OUT of index order (1 before 0) so that + // removing the runner's `.OrderBy(t => t.Index)` sort would send "my sink leaks" first and + // dequeue the replies in the wrong order, failing the assertions below. The original + // fixture (already-ascending turns) let a missing sort go unnoticed. + var driver = new FakeDriver(); + driver.Replies.Enqueue("first"); + driver.Replies.Enqueue("second"); + var runner = Build(driver, out _); + + var result = await runner.RunAsync(Suite(), new AgentTestCase + { + Id = "case-1", + Name = "two turns", + Turns = + [ + new TestTurn { Index = 1, UserMessage = "my sink leaks" }, + new TestTurn { Index = 0, UserMessage = "hello" } + ] + }, "run-1", model: null, CancellationToken.None); + + Assert.Equal(["hello", "my sink leaks"], driver.Sent); + Assert.Equal(AgentTestStatus.Passed, result.Status); + Assert.Equal("first", result.Turns[0].Output); + Assert.Equal("second", result.Turns[1].Output); + } + + [Fact] + public async Task Fails_the_case_without_running_the_agent_when_the_seam_is_not_live() + { + // The most dangerous silent failure in this whole feature: seam not live -> mocking does + // nothing -> real tools get called. + var driver = new FakeDriver { CanaryResult = false }; + var runner = Build(driver, out _); + + var result = await runner.RunAsync(Suite(), new AgentTestCase + { + Id = "case-1", + Name = "c", + Turns = [new TestTurn { Index = 0, UserMessage = "hello" }] + }, "run-1", model: null, CancellationToken.None); + + Assert.Equal(AgentTestStatus.Error, result.Status); + Assert.Contains("mock seam", result.Error!); + Assert.Empty(driver.Sent); // not a single message may go out + } + + [Fact] + public async Task A_case_with_no_turns_is_recorded_as_error_without_touching_the_driver() + { + // Turns.SelectMany(...).Concat(...).All(a => a.Passed) is vacuously true on an empty + // sequence, and a case that ran no turns must never be reported as Passed. The check has to + // come before the canary: if no turn will run, no conversation should be opened only to + // report an error afterwards. + var driver = new FakeDriver(); + var runner = Build(driver, out var registry); + + var result = await runner.RunAsync(Suite(), new AgentTestCase + { + Id = "case-1", + Name = "no turns", + Turns = [] + }, "run-1", model: null, CancellationToken.None); + + Assert.Equal(AgentTestStatus.Error, result.Status); + Assert.Equal("the case has no turns", result.Error); + Assert.False(driver.PrepareCalled); + Assert.False(driver.CanaryCalled); + Assert.Empty(driver.Sent); + Assert.Null(registry.TryGet(result.ConversationId)); + } + + [Fact] + public async Task A_failing_turn_assertion_fails_the_case_but_later_turns_still_run() + { + var driver = new FakeDriver(); + driver.Replies.Enqueue("nope"); + driver.Replies.Enqueue("second"); + var runner = Build(driver, out _); + + var result = await runner.RunAsync(Suite(), new AgentTestCase + { + Id = "case-1", + Name = "c", + Turns = + [ + new TestTurn + { + Index = 0, UserMessage = "a", + Assertions = [new TestAssertion { Type = AssertionTypes.OutputContains, Expected = "yes" }] + }, + new TestTurn { Index = 1, UserMessage = "b" } + ] + }, "run-1", model: null, CancellationToken.None); + + Assert.Equal(AgentTestStatus.Failed, result.Status); + Assert.Equal(2, driver.Sent.Count); + } + + [Fact] + public async Task A_fatal_assertion_stops_the_remaining_turns() + { + var driver = new FakeDriver(); + driver.Replies.Enqueue("nope"); + var runner = Build(driver, out _); + + var result = await runner.RunAsync(Suite(), new AgentTestCase + { + Id = "case-1", + Name = "c", + Turns = + [ + new TestTurn + { + Index = 0, UserMessage = "a", + Assertions = [new TestAssertion + { + Type = AssertionTypes.OutputContains, Expected = "yes", Fatal = true + }] + }, + new TestTurn { Index = 1, UserMessage = "b" } + ] + }, "run-1", model: null, CancellationToken.None); + + Assert.Equal(AgentTestStatus.Failed, result.Status); + Assert.Single(driver.Sent); + } + + [Fact] + public async Task Case_level_assertions_see_the_final_state_and_routed_agent() + { + var driver = new FakeDriver { RoutedAgent = "Work Order Creator" }; + driver.States["wo_id"] = "123"; + driver.Replies.Enqueue("done"); + var runner = Build(driver, out _); + + var result = await runner.RunAsync(Suite(), new AgentTestCase + { + Id = "case-1", + Name = "c", + Turns = [new TestTurn { Index = 0, UserMessage = "a" }], + Assertions = + [ + new TestAssertion { Type = AssertionTypes.StateEquals, Target = "wo_id", Expected = "123" }, + new TestAssertion { Type = AssertionTypes.RoutedToAgent, Expected = "work order creator" } + ] + }, "run-1", model: null, CancellationToken.None); + + Assert.Equal(AgentTestStatus.Passed, result.Status); + Assert.All(result.Assertions, a => Assert.True(a.Passed)); + } + + [Fact] + public async Task A_timeout_is_recorded_as_error_not_as_a_failed_assertion() + { + var driver = new FakeDriver { SendDelay = TimeSpan.FromSeconds(5) }; + var runner = Build(driver, out _); + + var result = await runner.RunAsync(Suite(timeoutSeconds: 1), new AgentTestCase + { + Id = "case-1", + Name = "c", + Turns = [new TestTurn { Index = 0, UserMessage = "a" }] + }, "run-1", model: null, CancellationToken.None); + + Assert.Equal(AgentTestStatus.Error, result.Status); + Assert.Contains("timed out", result.Error!); + } + + [Fact] + public async Task An_unrelated_cancellation_is_recorded_with_its_real_message_not_mislabeled_as_a_timeout() + { + // Fix round 1, Finding 3. The old guard (`when (!ct.IsCancellationRequested)`) caught ANY + // OperationCanceledException that wasn't the caller's own cancellation and stamped it "the + // case timed out after Ns" -- even one that has nothing to do with the runner's own + // per-case deadline, like an HttpClient timeout raised inside a passthrough tool call. This + // pins that such a cancellation now falls through to the generic handler and keeps its own + // message. + var driver = new FakeDriver { ThrowUnrelatedCancellation = true }; + var runner = Build(driver, out _); + + var result = await runner.RunAsync(Suite(), new AgentTestCase + { + Id = "case-1", + Name = "c", + Turns = [new TestTurn { Index = 0, UserMessage = "a" }] + }, "run-1", model: null, CancellationToken.None); + + Assert.Equal(AgentTestStatus.Error, result.Status); + Assert.DoesNotContain("timed out", result.Error!); + Assert.Contains("simulated unrelated cancellation", result.Error!); + } + + [Fact] + public async Task The_conversation_is_unregistered_immediately_on_the_happy_path() + { + // Fix round 1, Finding 1 explicitly asks to keep a case proving the happy path still + // unregisters synchronously -- none of the pre-existing tests actually asserted this + // (the one that touched the registry at all did so only on the timeout path, and has been + // replaced below by a pair of tests covering the new orphan-safe behavior). + var driver = new FakeDriver(); + driver.Replies.Enqueue("done"); + var runner = Build(driver, out var registry); + + var result = await runner.RunAsync(Suite(), new AgentTestCase + { + Id = "case-1", + Name = "c", + Turns = [new TestTurn { Index = 0, UserMessage = "a" }] + }, "run-1", model: null, CancellationToken.None); + + Assert.Equal(AgentTestStatus.Passed, result.Status); + Assert.Null(registry.TryGet(result.ConversationId)); + } + + [Fact] + public async Task The_registry_entry_survives_an_orphaned_send_and_is_removed_once_it_actually_finishes() + { + // Fix round 1, Finding 1 (replaces the old The_conversation_is_always_unregistered_afterwards, + // which only proved eventual cleanup -- a property the ORIGINAL, buggy code already + // satisfied trivially by unregistering immediately). HonorCancellationInSend = false makes + // the fake behave like the real driver: BotSharp's SendMessage has no cancellation hook, so + // the runner's 1s timeout only stops the RUNNER from waiting; the "call" itself keeps + // running for the full 2s. If the registry entry were removed the moment RunAsync returns + // (the old behavior), TestMockExecutorProvider would stop intercepting for the ~1s the + // orphan is still in flight -- exactly the window a real tool call could slip through in. + var driver = new FakeDriver + { + // Raised from 2s to 5s (Task 8 pre-work): RunAsync returns at ~1s and the orphan + // finished at ~2s, leaving only ~1s of slack before the Assert.NotNull below under + // UnitTest's parallel-collection thread-pool contention (no xunit.runner.json here, + // unlike the sequential AiPlatform-style suite). 5s widens that to ~4s of slack. + // Fix round 1, finding 8: raising SendDelay alone only moved the tight margin rather + // than removing it -- the trailing poll loop's deadline was left at .AddSeconds(5) + // against an orphan that itself now takes ~5s, leaving only ~1s of slack there while + // the first assert gained 3s. See the deadline below, now .AddSeconds(10), for both + // asserts to have comparable (~4-5s) margin. + SendDelay = TimeSpan.FromSeconds(5), + HonorCancellationInSend = false + }; + var runner = Build(driver, out var registry); + + var result = await runner.RunAsync(Suite(timeoutSeconds: 1), new AgentTestCase + { + Id = "case-1", + Name = "c", + Turns = [new TestTurn { Index = 0, UserMessage = "a" }] + }, "run-1", model: null, CancellationToken.None); + + Assert.Equal(AgentTestStatus.Error, result.Status); + Assert.Contains("timed out", result.Error!); + + // RunAsync returned at ~1s; the orphaned send does not finish until ~2s, so the entry MUST + // still be there right now. + Assert.NotNull(registry.TryGet(result.ConversationId)); + + // ...and must be gone once that orphaned call has actually finished. Widened from 5s to + // 10s alongside SendDelay's 2s->5s raise above (fix round 1, finding 8) -- the orphan now + // finishes at ~5s (measured from this poll starting at ~1s), so a 5s deadline left only + // ~1s of margin here, same tightness as before, just moved instead of removed. + var deadline = DateTime.UtcNow.AddSeconds(10); + while (registry.TryGet(result.ConversationId) != null && DateTime.UtcNow < deadline) + { + await Task.Delay(50); + } + Assert.Null(registry.TryGet(result.ConversationId)); + } + + [Fact] + public async Task An_orphaned_call_that_later_faults_is_logged_instead_of_vanishing_silently() + { + // Fix wave item 7: the ContinueWith that unregisters an orphaned call never observed the + // antecedent task's exception -- a real driver call that throws AFTER the case's own + // timeout had already elapsed vanished with no trace beyond an UnobservedTaskException at + // GC. This is the cheapest available diagnostic for the branch's single highest-risk + // unverified property (a real call still running after RunAsync itself already returned). + var driver = new FakeDriver + { + SendDelay = TimeSpan.FromSeconds(2), + HonorCancellationInSend = false, + ThrowAfterDelay = new InvalidOperationException( + "simulated real-driver failure after the runner gave up waiting") + }; + var logger = new CapturingLogger(); + var runner = new AgentTestCaseRunner(new AgentTestRunRegistry(), driver, logger); + + var result = await runner.RunAsync(Suite(timeoutSeconds: 1), new AgentTestCase + { + Id = "case-1", + Name = "c", + Turns = [new TestTurn { Index = 0, UserMessage = "a" }] + }, "run-1", model: null, CancellationToken.None); + + // The case itself still reports its own timeout -- the orphan's eventual fault is + // diagnostic-only and must never retroactively change what the case already reported. + Assert.Equal(AgentTestStatus.Error, result.Status); + Assert.Contains("timed out", result.Error!); + + var deadline = DateTime.UtcNow.AddSeconds(10); + while (!logger.Entries.Any(e => e.Exception != null) && DateTime.UtcNow < deadline) + { + await Task.Delay(50); + } + + var logged = Assert.Single(logger.Entries, e => e.Exception != null); + Assert.Equal(LogLevel.Error, logged.Level); + Assert.Contains("case-1", logged.Message); + Assert.Contains("simulated real-driver failure", logged.Exception!.ToString()); + } + + [Fact] + public async Task The_suite_allow_list_overrides_are_applied_to_the_active_run() + { + AgentTestCase testCase = new() + { + Id = "case-1", Name = "c", + Turns = [new TestTurn { Index = 0, UserMessage = "a" }] + }; + var suite = Suite(); + suite.ExtraAllowedFunctions.Add("util-db-sql_select"); + suite.ForceBlockedFunctions.Add("response_to_user"); + + ActiveTestRun? captured = null; + var driver = new FakeDriver(); + var registry = new AgentTestRunRegistry(); + var runner = new AgentTestCaseRunner( + new CapturingRegistry(registry, r => captured = r), + driver, + NullLogger.Instance); + + await runner.RunAsync(suite, testCase, "run-1", model: null, CancellationToken.None); + + Assert.NotNull(captured); + Assert.Contains("util-db-sql_select", captured!.AllowedFunctions); + Assert.Contains("route_to_agent", captured.AllowedFunctions); // default allow list intact + Assert.Contains("response_to_user", captured.ForceBlockedFunctions); + } + + private sealed class CapturingRegistry(AgentTestRunRegistry inner, Action onRegister) + : IAgentTestRunRegistry + { + public void Register(ActiveTestRun run) { onRegister(run); inner.Register(run); } + public void Unregister(string conversationId) => inner.Unregister(conversationId); + public ActiveTestRun? TryGet(string? conversationId) => inner.TryGet(conversationId); + } + + /// + /// Records every Log call so a test can assert an exception was actually observed and + /// logged. ConcurrentBag, not List: the continuation that logs the orphan's fault runs on a + /// ThreadPool thread (TaskScheduler.Default) racing the test's own polling read. + /// + private sealed class CapturingLogger : ILogger + { + public ConcurrentBag<(LogLevel Level, Exception? Exception, string Message)> Entries { get; } = []; + + public IDisposable? BeginScope(TState state) where TState : notnull => null; + + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log( + LogLevel logLevel, EventId eventId, TState state, Exception? exception, + Func formatter) + { + Entries.Add((logLevel, exception, formatter(state, exception))); + } + } +} diff --git a/tests/BotSharp.Core.UnitTests/AgentTesting/AgentTestControllerTests.cs b/tests/BotSharp.Core.UnitTests/AgentTesting/AgentTestControllerTests.cs new file mode 100644 index 000000000..bac90f1ab --- /dev/null +++ b/tests/BotSharp.Core.UnitTests/AgentTesting/AgentTestControllerTests.cs @@ -0,0 +1,613 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Reflection; +using System.Security.Claims; +using System.Threading.Tasks; +using BotSharp.Abstraction.Agents; +using BotSharp.Abstraction.Infrastructures.Attributes; +using BotSharp.Abstraction.MLTasks; +using BotSharp.Abstraction.MLTasks.Settings; +using BotSharp.Abstraction.Repositories; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using BotSharp.Plugin.AgentTesting.Controllers; +using BotSharp.Plugin.AgentTesting.Models; +using BotSharp.Plugin.AgentTesting.Repositories; +using BotSharp.Plugin.AgentTesting.Services; +using Xunit; + +namespace BotSharp.Core.UnitTests.AgentTesting; + +/// +/// Fix-wave coverage for the controller-level guards added on top of the ten already-shipped +/// tasks: rejecting the never-implemented Passthrough policy and a malformed assertion at case +/// create/update (before either can ever run and vacuously pass), rejecting a trigger against a +/// disabled suite, keeping UpdateSuite from blanking agentId/name on a partial body, rejecting a +/// cancel against an already-terminal run, and the [BotSharpAuth] gate on the two endpoints that +/// are this feature's PII/cost surfaces (RecordCase/TriggerRun). +/// +public class AgentTestControllerTests +{ + private sealed class InMemoryRepo : IAgentTestRepository + { + public Dictionary Suites { get; } = []; + public Dictionary Cases { get; } = []; + public Dictionary Runs { get; } = []; + + public Task GetSuiteAsync(string id) + => Task.FromResult(Suites.TryGetValue(id, out var s) ? s : null); + public Task> ListSuitesAsync(string? agentId) => Task.FromResult(Suites.Values.ToList()); + public Task UpsertSuiteAsync(AgentTestSuite suite) + { + if (string.IsNullOrEmpty(suite.Id)) + { + suite.Id = Guid.NewGuid().ToString(); + } + Suites[suite.Id] = suite; + return Task.CompletedTask; + } + public Task DeleteSuiteAsync(string id) { Suites.Remove(id); return Task.CompletedTask; } + + public Task GetCaseAsync(string id) + => Task.FromResult(Cases.TryGetValue(id, out var c) ? c : null); + public Task> ListCasesAsync(string suiteId) + => Task.FromResult(Cases.Values.Where(c => c.SuiteId == suiteId).ToList()); + public Task UpsertCaseAsync(AgentTestCase testCase) + { + // Mirrors the real AgentTestRepository: ReplaceOneAsync(upsert:true) does not run the + // [BsonId(IdGenerator=...)] hook, so a brand-new case with no Id must get one here too, + // or this fake diverges from production behaviour on exactly the create path these + // tests exercise. + if (string.IsNullOrEmpty(testCase.Id)) + { + testCase.Id = Guid.NewGuid().ToString(); + } + Cases[testCase.Id] = testCase; + return Task.CompletedTask; + } + public Task DeleteCaseAsync(string id) { Cases.Remove(id); return Task.CompletedTask; } + + public Task CreateRunAsync(AgentTestRun run) + { + if (string.IsNullOrEmpty(run.Id)) + { + run.Id = Guid.NewGuid().ToString(); + } + Runs[run.Id] = run; + return Task.FromResult(run); + } + public Task GetRunAsync(string id) => Task.FromResult(Runs.TryGetValue(id, out var r) ? r : null); + public Task> ListRunsAsync(string? suiteId) => Task.FromResult(Runs.Values.ToList()); + public Task> ListRunsByStatusAsync(string status) + => Task.FromResult(Runs.Values.Where(r => r.Status == status).ToList()); + public Task UpdateRunAsync(AgentTestRun run) { Runs[run.Id] = run; return Task.CompletedTask; } + + public Task AddCaseResultAsync(AgentTestCaseResult result) => Task.CompletedTask; + public Task> ListCaseResultsAsync(string runId) => Task.FromResult(new List()); + } + + private sealed class RecordingQueue : IAgentTestRunQueue + { + public List Enqueued { get; } = []; + public void Enqueue(string runId) => Enqueued.Add(runId); + } + + /// + /// A provider service that recognises exactly the models named here. The default (nothing + /// registered) leaves every existing test unaffected -- validation short-circuits on an empty + /// model list -- while forcing any test that DOES request a model to say so explicitly rather + /// than passing against a permissive mock. + /// + private static ILlmProviderService ProviderServiceKnowing(params string[] providerSlashModel) + { + var known = new HashSet(providerSlashModel, StringComparer.OrdinalIgnoreCase); + var mock = new Mock(); + mock.Setup(x => x.GetSetting(It.IsAny(), It.IsAny())) + .Returns((string p, string m) => known.Contains($"{p}/{m}") ? new LlmModelSetting { Name = m } : null); + return mock.Object; + } + + private static AgentTestController BuildController( + InMemoryRepo repo, out RecordingQueue queue, ILlmProviderService? llmProviders = null) + { + var recorder = new AgentTestRecorder( + Mock.Of(), + repo, + NullLogger.Instance); + queue = new RecordingQueue(); + + var controller = new AgentTestController( + repo, queue, Mock.Of(), recorder, llmProviders ?? ProviderServiceKnowing()); + + // TriggerRun reads User.FindFirstValue(ClaimTypes.NameIdentifier) -- a directly-constructed + // controller (no MVC pipeline/TestServer) has no HttpContext at all by default, and + // ControllerBase.User dereferences it, so this must be wired up even for an anonymous test + // caller (an identity with no NameIdentifier claim is fine; FindFirstValue just returns null + // for that, same as a real anonymous-but-authenticated request would). + controller.ControllerContext = new ControllerContext + { + HttpContext = new DefaultHttpContext { User = new ClaimsPrincipal(new ClaimsIdentity()) } + }; + + return controller; + } + + private static AgentTestCaseUpsertRequest ValidRequest(string suiteId = "suite-1") => new() + { + SuiteId = suiteId, + Name = "case", + Turns = [new TestTurn { Index = 0, UserMessage = "hi" }] + }; + + private static AgentTestSuite EnabledSuite(string id = "suite-1") => new() + { + Id = id, AgentId = "agent-1", Name = "s", Enabled = true + }; + + // ---- Item 1: Passthrough is rejected, Block still works -------------------------------- + + [Fact] + public async Task Create_rejects_the_passthrough_policy() + { + var repo = new InMemoryRepo(); + repo.Suites["suite-1"] = EnabledSuite(); + var controller = BuildController(repo, out _); + + var request = ValidRequest(); + request.UnmockedToolPolicy = "Passthrough"; + + var result = await controller.CreateCase(request); + + Assert.IsType(result.Result); + Assert.Empty(repo.Cases); + } + + [Fact] + public async Task Update_rejects_the_passthrough_policy_and_leaves_the_stored_case_untouched() + { + var repo = new InMemoryRepo(); + repo.Suites["suite-1"] = EnabledSuite(); + repo.Cases["case-1"] = new AgentTestCase + { + Id = "case-1", SuiteId = "suite-1", Name = "original", + UnmockedToolPolicy = UnmockedToolPolicies.Block + }; + var controller = BuildController(repo, out _); + + var request = ValidRequest(); + request.UnmockedToolPolicy = "Passthrough"; + + var result = await controller.UpdateCase("case-1", request); + + Assert.IsType(result.Result); + Assert.Equal("original", repo.Cases["case-1"].Name); + Assert.Equal(UnmockedToolPolicies.Block, repo.Cases["case-1"].UnmockedToolPolicy); + } + + [Fact] + public async Task Passthrough_rejection_is_case_insensitive() + { + var repo = new InMemoryRepo(); + repo.Suites["suite-1"] = EnabledSuite(); + var controller = BuildController(repo, out _); + + var request = ValidRequest(); + request.UnmockedToolPolicy = "passthrough"; + + var result = await controller.CreateCase(request); + + Assert.IsType(result.Result); + } + + [Fact] + public async Task Create_accepts_the_default_block_policy() + { + var repo = new InMemoryRepo(); + repo.Suites["suite-1"] = EnabledSuite(); + var controller = BuildController(repo, out _); + + var result = await controller.CreateCase(ValidRequest()); + + Assert.Null(result.Result); + Assert.NotNull(result.Value); + Assert.Single(repo.Cases); + } + + // ---- Item 4: an assertion missing its required field is rejected at save time ----------- + + [Theory] + [InlineData(AssertionTypes.OutputContains)] + [InlineData(AssertionTypes.OutputNotContains)] + [InlineData(AssertionTypes.OutputRegex)] + [InlineData(AssertionTypes.RoutedToAgent)] + [InlineData(AssertionTypes.LlmJudge)] + public async Task Create_rejects_a_case_level_assertion_missing_its_required_expected_value(string type) + { + var repo = new InMemoryRepo(); + repo.Suites["suite-1"] = EnabledSuite(); + var controller = BuildController(repo, out _); + + var request = ValidRequest(); + request.Assertions = [new TestAssertion { Type = type }]; + + var result = await controller.CreateCase(request); + + Assert.IsType(result.Result); + Assert.Empty(repo.Cases); + } + + [Theory] + [InlineData(AssertionTypes.ToolCalled)] + [InlineData(AssertionTypes.ToolNotCalled)] + [InlineData(AssertionTypes.StateEquals)] + public async Task Create_rejects_a_turn_level_assertion_missing_its_required_target(string type) + { + var repo = new InMemoryRepo(); + repo.Suites["suite-1"] = EnabledSuite(); + var controller = BuildController(repo, out _); + + var request = ValidRequest(); + request.Turns[0].Assertions = [new TestAssertion { Type = type }]; + + var result = await controller.CreateCase(request); + + Assert.IsType(result.Result); + Assert.Empty(repo.Cases); + } + + [Fact] + public async Task Update_rejects_a_case_missing_a_required_assertion_field_too() + { + var repo = new InMemoryRepo(); + repo.Suites["suite-1"] = EnabledSuite(); + repo.Cases["case-1"] = new AgentTestCase { Id = "case-1", SuiteId = "suite-1", Name = "original" }; + var controller = BuildController(repo, out _); + + var request = ValidRequest(); + request.Assertions = [new TestAssertion { Type = AssertionTypes.StateEquals, Target = "" }]; + + var result = await controller.UpdateCase("case-1", request); + + Assert.IsType(result.Result); + Assert.Equal("original", repo.Cases["case-1"].Name); + } + + [Fact] + public async Task Create_accepts_well_formed_assertions_of_every_type() + { + var repo = new InMemoryRepo(); + repo.Suites["suite-1"] = EnabledSuite(); + var controller = BuildController(repo, out _); + + var request = ValidRequest(); + request.Assertions = + [ + new TestAssertion { Type = AssertionTypes.OutputContains, Expected = "ok" }, + new TestAssertion { Type = AssertionTypes.OutputNotContains, Expected = "sorry" }, + new TestAssertion { Type = AssertionTypes.OutputRegex, Expected = "^ok$" }, + new TestAssertion { Type = AssertionTypes.ToolCalled, Target = "get_work_order" }, + new TestAssertion { Type = AssertionTypes.ToolNotCalled, Target = "send_text_message" }, + new TestAssertion { Type = AssertionTypes.StateEquals, Target = "wo_id", Expected = "1" }, + new TestAssertion { Type = AssertionTypes.RoutedToAgent, Expected = "Router" }, + new TestAssertion { Type = AssertionTypes.LlmJudge, Expected = "criteria", MinScore = 0.8 } + ]; + + var result = await controller.CreateCase(request); + + Assert.Null(result.Result); + Assert.NotNull(result.Value); + } + + // ---- Item 8a: a disabled suite cannot be triggered -------------------------------------- + + [Fact] + public async Task TriggerRun_rejects_a_disabled_suite_and_never_enqueues_it() + { + var repo = new InMemoryRepo(); + repo.Suites["suite-1"] = new AgentTestSuite { Id = "suite-1", AgentId = "agent-1", Name = "s", Enabled = false }; + var controller = BuildController(repo, out var queue); + + var result = await controller.TriggerRun("suite-1", null); + + Assert.IsType(result.Result); + Assert.Empty(queue.Enqueued); + Assert.Empty(repo.Runs); + } + + [Fact] + public async Task TriggerRun_enqueues_an_enabled_suite() + { + var repo = new InMemoryRepo(); + repo.Suites["suite-1"] = EnabledSuite(); + var controller = BuildController(repo, out var queue); + + var result = await controller.TriggerRun("suite-1", null); + + Assert.Null(result.Result); + Assert.NotNull(result.Value); + Assert.Single(queue.Enqueued); + } + + [Fact] + public async Task TriggerRun_persists_the_requested_models_on_the_run() + { + var repo = new InMemoryRepo(); + repo.Suites["suite-1"] = EnabledSuite(); + var controller = BuildController( + repo, out _, ProviderServiceKnowing("openai/gpt-4o", "anthropic/claude-3-7-sonnet-20250219")); + + var result = await controller.TriggerRun("suite-1", new AgentTestRunTriggerRequest + { + Models = + [ + new TestModel { Provider = "openai", Model = "gpt-4o" }, + new TestModel { Provider = "anthropic", Model = "claude-3-7-sonnet-20250219" } + ] + }); + + Assert.Null(result.Result); + Assert.Equal(2, result.Value!.Models!.Count); + } + + [Fact] + public async Task TriggerRun_normalises_an_empty_model_list_to_null() + { + // Empty and absent both mean "the agent's own LlmConfig"; persisting [] would read like a + // deliberate (and impossible) choice of zero models. + var repo = new InMemoryRepo(); + repo.Suites["suite-1"] = EnabledSuite(); + var controller = BuildController(repo, out _); + + var result = await controller.TriggerRun("suite-1", new AgentTestRunTriggerRequest { Models = [] }); + + Assert.Null(result.Value!.Models); + } + + [Fact] + public async Task TriggerRun_rejects_a_model_this_host_cannot_run() + { + // Measured before this guard existed: an unregistered model name reached the provider and + // every case in the run died with a bare "Object reference not set to an instance of an + // object." -- no run should be queued, and no tokens spent, for a typo. + var repo = new InMemoryRepo(); + repo.Suites["suite-1"] = EnabledSuite(); + var controller = BuildController(repo, out var queue, ProviderServiceKnowing("openai/gpt-4o")); + + var result = await controller.TriggerRun("suite-1", new AgentTestRunTriggerRequest + { + Models = [new TestModel { Provider = "openai", Model = "definitely-not-a-real-model" }] + }); + + Assert.IsType(result.Result); + Assert.Empty(queue.Enqueued); + Assert.Empty(repo.Runs); + } + + [Fact] + public async Task TriggerRun_rejects_the_same_model_listed_twice() + { + // Two identical columns would collapse into one cell in the comparison grid, the second + // result silently overwriting the first. + var repo = new InMemoryRepo(); + repo.Suites["suite-1"] = EnabledSuite(); + var controller = BuildController(repo, out var queue, ProviderServiceKnowing("openai/gpt-4o")); + + var result = await controller.TriggerRun("suite-1", new AgentTestRunTriggerRequest + { + Models = + [ + new TestModel { Provider = "openai", Model = "gpt-4o" }, + new TestModel { Provider = "openai", Model = "gpt-4o" } + ] + }); + + Assert.IsType(result.Result); + Assert.Empty(queue.Enqueued); + } + + // ---- Item 3: the PII/cost surfaces require BotSharp's admin/root gate ------------------- + + [Fact] + public void RecordCase_and_TriggerRun_carry_the_BotSharpAuth_admin_gate() + { + // RecordCase copies a real conversation's raw content (possibly PII: phone numbers, + // addresses, tenant names) into the test store; TriggerRun spends real model tokens with + // no quota. Both must be gated tighter than the controller's own class-level [Authorize], + // using the SAME mechanism BotSharp's own UserController/AgentController/PluginController/ + // RoleController already use for their sensitive actions -- BotSharpAuthAttribute checks + // the caller is an admin/root user (see IsAdminUser). + var recordMethod = typeof(AgentTestController).GetMethod(nameof(AgentTestController.RecordCase))!; + var triggerMethod = typeof(AgentTestController).GetMethod(nameof(AgentTestController.TriggerRun))!; + + Assert.NotNull(recordMethod.GetCustomAttribute()); + Assert.NotNull(triggerMethod.GetCustomAttribute()); + } + + [Fact] + public void Plain_read_and_CRUD_endpoints_do_not_carry_the_admin_only_gate() + { + // The gate is deliberately narrow: everything else (list/get/create/update/delete suites + // and cases, list runs, mock-targets) stays under the controller's plain [Authorize] -- + // over-gating would block ordinary QA/PM test authoring, not just the PII/cost surfaces. + var otherMethods = new[] + { + nameof(AgentTestController.ListSuites), + nameof(AgentTestController.CreateSuite), + nameof(AgentTestController.UpdateSuite), + nameof(AgentTestController.DeleteSuite), + nameof(AgentTestController.ListCases), + nameof(AgentTestController.CreateCase), + nameof(AgentTestController.UpdateCase), + nameof(AgentTestController.DeleteCase), + nameof(AgentTestController.ListRuns), + nameof(AgentTestController.GetRun), + nameof(AgentTestController.CancelRun), + nameof(AgentTestController.GetMockTargets), + }; + + foreach (var name in otherMethods) + { + var method = typeof(AgentTestController).GetMethod(name)!; + Assert.Null(method.GetCustomAttribute()); + } + } + + // ---- Item 8b: UpdateSuite no longer blanks agentId/name on a partial body --------------- + + [Fact] + public async Task UpdateSuite_keeps_the_existing_agentId_and_name_when_the_request_omits_them() + { + var repo = new InMemoryRepo(); + repo.Suites["suite-1"] = new AgentTestSuite + { + Id = "suite-1", AgentId = "agent-1", Name = "original name", Enabled = true + }; + var controller = BuildController(repo, out _); + + // A partial body: AgentId/Name are left at their DTO defaults (empty string), as a + // caller that only means to flip, say, CaseTimeoutSeconds would send. + var request = new AgentTestSuiteUpsertRequest { CaseTimeoutSeconds = 30 }; + + var result = await controller.UpdateSuite("suite-1", request); + + Assert.Null(result.Result); + Assert.Equal("agent-1", repo.Suites["suite-1"].AgentId); + Assert.Equal("original name", repo.Suites["suite-1"].Name); + Assert.Equal(30, repo.Suites["suite-1"].CaseTimeoutSeconds); + } + + [Fact] + public async Task UpdateSuite_still_applies_an_explicit_agentId_and_name() + { + var repo = new InMemoryRepo(); + repo.Suites["suite-1"] = new AgentTestSuite { Id = "suite-1", AgentId = "agent-1", Name = "old", Enabled = true }; + var controller = BuildController(repo, out _); + + var request = new AgentTestSuiteUpsertRequest { AgentId = "agent-2", Name = "new" }; + + await controller.UpdateSuite("suite-1", request); + + Assert.Equal("agent-2", repo.Suites["suite-1"].AgentId); + Assert.Equal("new", repo.Suites["suite-1"].Name); + } + + // ---- Coordinator re-review item 2: a partial UpdateSuite must not silently re-enable a + // suite someone deliberately disabled (Enabled is now bool?; null means "omitted") ---------- + + [Fact] + public async Task UpdateSuite_preserves_a_disabled_suite_when_the_request_omits_enabled() + { + var repo = new InMemoryRepo(); + repo.Suites["suite-1"] = new AgentTestSuite + { + Id = "suite-1", AgentId = "agent-1", Name = "s", Enabled = false + }; + var controller = BuildController(repo, out _); + + // A partial body that only means to change CaseTimeoutSeconds -- Enabled is omitted + // (null), not explicitly re-enabled. Before item 2's fix, AgentTestSuiteUpsertRequest. + // Enabled was a non-nullable bool defaulting to true, so this exact request would have + // silently flipped the suite back on. + var request = new AgentTestSuiteUpsertRequest + { + AgentId = "agent-1", Name = "s", CaseTimeoutSeconds = 30 + }; + + var result = await controller.UpdateSuite("suite-1", request); + + Assert.Null(result.Result); + Assert.False(repo.Suites["suite-1"].Enabled); + Assert.Equal(30, repo.Suites["suite-1"].CaseTimeoutSeconds); + } + + [Fact] + public async Task UpdateSuite_still_applies_an_explicit_enabled_override() + { + var repo = new InMemoryRepo(); + repo.Suites["suite-1"] = new AgentTestSuite + { + Id = "suite-1", AgentId = "agent-1", Name = "s", Enabled = false + }; + var controller = BuildController(repo, out _); + + var request = new AgentTestSuiteUpsertRequest { AgentId = "agent-1", Name = "s", Enabled = true }; + + await controller.UpdateSuite("suite-1", request); + + Assert.True(repo.Suites["suite-1"].Enabled); + } + + [Fact] + public async Task UpdateSuite_can_still_explicitly_disable_an_enabled_suite() + { + var repo = new InMemoryRepo(); + repo.Suites["suite-1"] = EnabledSuite(); + var controller = BuildController(repo, out _); + + var request = new AgentTestSuiteUpsertRequest { AgentId = "agent-1", Name = "s", Enabled = false }; + + await controller.UpdateSuite("suite-1", request); + + Assert.False(repo.Suites["suite-1"].Enabled); + } + + [Fact] + public async Task CreateSuite_defaults_to_enabled_when_the_request_omits_it() + { + var repo = new InMemoryRepo(); + var controller = BuildController(repo, out _); + + var result = await controller.CreateSuite(new AgentTestSuiteUpsertRequest { AgentId = "agent-1", Name = "s" }); + + Assert.True(result.Enabled); + } + + [Fact] + public async Task CreateSuite_honors_an_explicit_disabled_flag() + { + var repo = new InMemoryRepo(); + var controller = BuildController(repo, out _); + + var result = await controller.CreateSuite( + new AgentTestSuiteUpsertRequest { AgentId = "agent-1", Name = "s", Enabled = false }); + + Assert.False(result.Enabled); + } + + // ---- Item 8c: cancelling an already-terminal run is rejected ---------------------------- + + [Theory] + [InlineData(AgentTestStatus.Passed)] + [InlineData(AgentTestStatus.Failed)] + [InlineData(AgentTestStatus.Error)] + [InlineData(AgentTestStatus.Cancelled)] + public async Task CancelRun_returns_conflict_for_a_run_that_already_finished(string terminalStatus) + { + var repo = new InMemoryRepo(); + repo.Runs["run-1"] = new AgentTestRun { Id = "run-1", SuiteId = "suite-1", Status = terminalStatus }; + var controller = BuildController(repo, out _); + + var result = await controller.CancelRun("run-1"); + + Assert.IsType(result); + Assert.False(repo.Runs["run-1"].CancelRequested); + } + + [Theory] + [InlineData(AgentTestStatus.Pending)] + [InlineData(AgentTestStatus.Running)] + public async Task CancelRun_still_accepts_a_run_that_has_not_finished(string liveStatus) + { + var repo = new InMemoryRepo(); + repo.Runs["run-1"] = new AgentTestRun { Id = "run-1", SuiteId = "suite-1", Status = liveStatus }; + var controller = BuildController(repo, out _); + + var result = await controller.CancelRun("run-1"); + + Assert.IsType(result); + Assert.True(repo.Runs["run-1"].CancelRequested); + } +} diff --git a/tests/BotSharp.Core.UnitTests/AgentTesting/AgentTestDocumentTests.cs b/tests/BotSharp.Core.UnitTests/AgentTesting/AgentTestDocumentTests.cs new file mode 100644 index 000000000..ac34c49d1 --- /dev/null +++ b/tests/BotSharp.Core.UnitTests/AgentTesting/AgentTestDocumentTests.cs @@ -0,0 +1,84 @@ +using MongoDB.Bson; +using MongoDB.Bson.Serialization; +using BotSharp.Plugin.AgentTesting.Models; +using Xunit; + +namespace BotSharp.Core.UnitTests.AgentTesting; + +/// +/// A case document has to round-trip through BSON losslessly. Not ceremony: a mock's fake return and +/// an assertion's expected value are arbitrary user-supplied strings, JSON fragments included, and +/// if the Mongo serialiser swallows or reshapes one of those fields the symptom is "the case looks +/// different after saving and reopening it", not an exception. +/// +public class AgentTestDocumentTests +{ + [Fact] + public void A_case_round_trips_through_bson_without_losing_nested_content() + { + var original = new AgentTestCase + { + SuiteId = "suite-1", + // Deliberately non-ASCII: this test exists to prove the Mongo serialiser does not + // swallow or reshape user-supplied text, and an all-ASCII fixture would not show that. + Name = "Tenant reports a leaking sink — ünïcode ✓", + // UnmockedToolPolicies only ever has one member (Block) as of the P1 fix wave that + // rejected Passthrough -- this round trip only cares that whatever string is stored + // survives BSON serialization unchanged, so any non-default literal proves the point. + UnmockedToolPolicy = "SomeFutureNonDefaultPolicy", + SourceConversationId = "conv-9", + InitialStates = [new TestState { Key = "user_authenticated", Value = "true" }], + Mocks = + [ + new TestToolMock + { + FunctionName = "get_work_order", + ArgsMatchJson = """{"woNum":"B9897413"}""", + CallIndex = 1, + ResultContent = """{"status":"Open","trade":"Plumbing"}""", + StopCompletion = true, + StateWrites = [new TestState { Key = "wo_id", Value = "123", ActiveRounds = 5 }] + } + ], + Turns = + [ + new TestTurn + { + Index = 0, + UserMessage = "my sink is leaking", + Assertions = [new TestAssertion { Type = "toolCalled", Target = "get_work_order", Fatal = true }] + } + ], + Assertions = [new TestAssertion { Type = "stateEquals", Target = "wo_id", Expected = "123" }] + }; + + var bson = original.ToBson(); + var restored = BsonSerializer.Deserialize(bson); + + Assert.Equal("Tenant reports a leaking sink — ünïcode ✓", restored.Name); + Assert.Equal("SomeFutureNonDefaultPolicy", restored.UnmockedToolPolicy); + var mock = Assert.Single(restored.Mocks); + Assert.Equal("""{"woNum":"B9897413"}""", mock.ArgsMatchJson); + Assert.Equal(1, mock.CallIndex); + Assert.True(mock.StopCompletion); + Assert.Equal("123", Assert.Single(mock.StateWrites!).Value); + var turn = Assert.Single(restored.Turns); + Assert.True(Assert.Single(turn.Assertions).Fatal); + Assert.Equal("stateEquals", Assert.Single(restored.Assertions).Type); + } + + [Fact] + public void Optional_fields_survive_being_absent() + { + // Hand-written and AI-generated cases routinely fill in only some fields, and a missing one + // must not fail deserialisation. + var minimal = new AgentTestCase { SuiteId = "s", Name = "n" }; + + var restored = BsonSerializer.Deserialize(minimal.ToBson()); + + Assert.Empty(restored.Turns); + Assert.Empty(restored.Mocks); + Assert.Null(restored.SourceConversationId); + Assert.Equal(UnmockedToolPolicies.Block, restored.UnmockedToolPolicy); // must default to Block + } +} diff --git a/tests/BotSharp.Core.UnitTests/AgentTesting/AgentTestRecorderTests.cs b/tests/BotSharp.Core.UnitTests/AgentTesting/AgentTestRecorderTests.cs new file mode 100644 index 000000000..c482f1492 --- /dev/null +++ b/tests/BotSharp.Core.UnitTests/AgentTesting/AgentTestRecorderTests.cs @@ -0,0 +1,289 @@ +using System.Collections.Generic; +using System.Linq; +using BotSharp.Plugin.AgentTesting.Services; +using BotSharp.Plugin.AgentTesting.Models; +using Xunit; + +namespace BotSharp.Core.UnitTests.AgentTesting; + +/// +/// Recording is what decides whether QA and PM can actually use this feature: hand-writing a work +/// order agent's mock JSON is not realistic. +/// +/// Two deliberate limitations are pinned here; changing either means changing the spec first: +/// 1) state writes can only be extracted as a whole-turn delta -- StateValueMongoElement carries only +/// MessageId (which locates a turn) and a Source of external/application/user, never a function +/// name, so splitting across individual mocks automatically is not possible; +/// 2) no outputContains-style assertions are generated -- using the model's exact wording as a +/// baseline is extremely brittle, any rephrasing goes red, and recording one case would mean +/// hand-editing ten assertions. +/// +public class AgentTestRecorderTests +{ + private static readonly List Dialogs = + [ + new() { Role = "user", Content = "my sink is leaking", MessageId = "m1" }, + new() { Role = "assistant", FunctionName = "get_work_order", FunctionArgs = """{"woNum":"B1"}""", MessageId = "m1" }, + new() { Role = "function", FunctionName = "get_work_order", Content = """{"status":"Open"}""", MessageId = "m1" }, + new() { Role = "assistant", Content = "I found work order B1.", MessageId = "m1" }, + new() { Role = "user", Content = "please schedule it", MessageId = "m2" }, + new() { Role = "function", FunctionName = "get_work_order", Content = """{"status":"Scheduled"}""", MessageId = "m2" }, + new() { Role = "assistant", Content = "Scheduled for tomorrow.", MessageId = "m2" } + ]; + + private static readonly List States = + [ + new() { Key = "user_authenticated", Values = [new() { MessageId = null, Data = "true", ActiveRounds = -1 }] }, + new() { Key = "wo_id", Values = [new() { MessageId = "m1", Data = "123", ActiveRounds = -1 }] } + ]; + + private static AgentTestCase Draft() => AgentTestRecorder.BuildDraft("suite-1", "conv-9", Dialogs, States); + + [Fact] + public void Takes_the_user_messages_as_turns_in_order() + { + var draft = Draft(); + + Assert.Equal(2, draft.Turns.Count); + Assert.Equal("my sink is leaking", draft.Turns[0].UserMessage); + Assert.Equal("please schedule it", draft.Turns[1].UserMessage); + Assert.Equal([0, 1], draft.Turns.Select(t => t.Index)); + } + + [Fact] + public void Turns_the_real_function_results_into_mocks() + { + var draft = Draft(); + + Assert.Equal(2, draft.Mocks.Count); + Assert.All(draft.Mocks, m => Assert.Equal("get_work_order", m.FunctionName)); + Assert.Equal("""{"status":"Open"}""", draft.Mocks[0].ResultContent); + Assert.Equal("""{"status":"Scheduled"}""", draft.Mocks[1].ResultContent); + } + + [Fact] + public void Numbers_repeated_calls_of_the_same_function_so_they_can_be_told_apart() + { + // The same function called twice with different returns: without an ordinal both calls would + // resolve to the same fake return. + var draft = Draft(); + + Assert.Equal(0, draft.Mocks[0].CallIndex); + Assert.Equal(1, draft.Mocks[1].CallIndex); + } + + [Fact] + public void Omits_recorded_arguments_for_a_repeated_function_but_keeps_them_for_a_function_recorded_once() + { + // Correction to the original brief: ToolMockMatcher.Match (Task 5) tries an args-subset + // match BEFORE falling back to CallIndex. get_work_order is recorded twice in the shared + // fixture above (Dialogs) -- if Mocks[0] kept ArgsMatchJson = {"woNum":"B1"}, a replay + // call in turn 2 that happens to carry the SAME arguments (very plausible: the agent is + // very likely to still be talking about the same work order) would match Mocks[0] via + // the args branch before the ordinal branch is ever reached, and the case could never + // reproduce the different result ("Scheduled") that was actually recorded for that + // second call. Once a function repeats, only CallIndex is unambiguous. + // + // This uses its own local fixture (rather than extending the shared Dialogs/States + // above) specifically so it does NOT shift Mocks[]/Turns[] indices or counts out from + // under every other test in this file that pins them (e.g. Assert.Equal(2, + // draft.Mocks.Count) in Turns_the_real_function_results_into_mocks). + List dialogs = + [ + new() { Role = "user", Content = "my sink is leaking, I'm customer C9", MessageId = "m1" }, + new() { Role = "assistant", FunctionName = "check_customer", FunctionArgs = """{"customerId":"C9"}""", MessageId = "m1" }, + new() { Role = "function", FunctionName = "check_customer", Content = """{"tier":"gold"}""", MessageId = "m1" }, + new() { Role = "assistant", FunctionName = "get_work_order", FunctionArgs = """{"woNum":"B1"}""", MessageId = "m1" }, + new() { Role = "function", FunctionName = "get_work_order", Content = """{"status":"Open"}""", MessageId = "m1" }, + new() { Role = "assistant", Content = "I found work order B1.", MessageId = "m1" }, + new() { Role = "user", Content = "please schedule it", MessageId = "m2" }, + new() { Role = "function", FunctionName = "get_work_order", Content = """{"status":"Scheduled"}""", MessageId = "m2" }, + new() { Role = "assistant", Content = "Scheduled for tomorrow.", MessageId = "m2" } + ]; + + var draft = AgentTestRecorder.BuildDraft("suite-1", "conv-9", dialogs, []); + + // check_customer was recorded exactly once: unambiguous, so its recorded arguments are + // kept (useful context when a human reviews the draft in the editor). + var checkCustomer = draft.Mocks.Single(m => m.FunctionName == "check_customer"); + Assert.Equal("""{"customerId":"C9"}""", checkCustomer.ArgsMatchJson); + + // get_work_order was recorded twice: BOTH of its mocks must drop ArgsMatchJson so that + // ToolMockMatcher.Match can only ever tell them apart by CallIndex. + var getWorkOrders = draft.Mocks.Where(m => m.FunctionName == "get_work_order").ToList(); + Assert.Equal(2, getWorkOrders.Count); + Assert.All(getWorkOrders, m => Assert.Null(m.ArgsMatchJson)); + Assert.Equal([0, 1], getWorkOrders.Select(m => m.CallIndex)); + } + + [Fact] + public void Only_states_without_a_message_id_become_initial_states() + { + var draft = Draft(); + + var initial = Assert.Single(draft.InitialStates); + Assert.Equal("user_authenticated", initial.Key); + Assert.Equal("true", initial.Value); + } + + [Fact] + public void The_turns_state_delta_is_attached_to_the_last_mock_of_that_turn() + { + var draft = Draft(); + + var write = Assert.Single(draft.Mocks[0].StateWrites!); + Assert.Equal("wo_id", write.Key); + Assert.Equal("123", write.Value); + Assert.Null(draft.Mocks[1].StateWrites); + } + + [Fact] + public void Keeps_the_recorded_arguments_on_the_toolCalled_assertion_even_when_the_mock_omits_them() + { + // Fix round 1: the args/ordinal correction (see + // Omits_recorded_arguments_for_a_repeated_function_but_keeps_them_for_a_function_recorded_once) + // is needed because ToolMockMatcher.Match dispatches against the WHOLE case's mock list + // (MockFunctionExecutor.ExecuteAsync calls Match(_run.Mocks, ...), not scoped to one + // turn) -- that really is ambiguous across turns once a function repeats. But + // AssertionEvaluator's toolCalled case is evaluated per turn, against ONLY that turn's + // observed calls (AgentTestCaseRunner.cs: active.ObservedCalls.Where(c => c.TurnIndex == + // turn.Index)) -- there is no cross-turn collision for an assertion to guard against, so + // it must keep the argument actually recorded for THIS turn's call even though the mock + // behind it had to drop it to stay unambiguous for dispatch. + var draft = Draft(); + + // The mock lost its args (get_work_order is recorded twice across the whole case). + Assert.Null(draft.Mocks[0].ArgsMatchJson); + + // The toolCalled assertion generated for that same turn 1 call did not. + var assertion = Assert.Single(draft.Turns[0].Assertions); + Assert.Equal(AssertionTypes.ToolCalled, assertion.Type); + Assert.Equal("get_work_order", assertion.Target); + Assert.Equal("""{"woNum":"B1"}""", assertion.ArgsMatchJson); + } + + [Fact] + public void Keeps_each_calls_own_arguments_when_the_same_function_is_recorded_twice_in_one_turn() + { + // Fix round 1, small item 2: the only existing coverage for "recorded more than once" + // spans two DIFFERENT turns. This pins the same mechanism within a SINGLE turn: two calls + // to the same function, each with its own preceding assistant FunctionArgs entry. + // pendingArgsByFunction's last-write-wins is exactly "nearest preceding" per call, and the + // correction (and the fix above, which generates assertions before it runs) is turn- + // agnostic, so both calls should keep their own distinct recorded arguments on their + // toolCalled assertions while both mocks omit ArgsMatchJson and rely on CallIndex alone. + List dialogs = + [ + new() { Role = "user", Content = "check work orders B1 and B2", MessageId = "m1" }, + new() { Role = "assistant", FunctionName = "get_work_order", FunctionArgs = """{"woNum":"B1"}""", MessageId = "m1" }, + new() { Role = "function", FunctionName = "get_work_order", Content = """{"status":"Open"}""", MessageId = "m1" }, + new() { Role = "assistant", FunctionName = "get_work_order", FunctionArgs = """{"woNum":"B2"}""", MessageId = "m1" }, + new() { Role = "function", FunctionName = "get_work_order", Content = """{"status":"Closed"}""", MessageId = "m1" }, + new() { Role = "assistant", Content = "B1 is open, B2 is closed.", MessageId = "m1" } + ]; + + var draft = AgentTestRecorder.BuildDraft("suite-1", "conv-9", dialogs, []); + + Assert.Equal(2, draft.Mocks.Count); + Assert.All(draft.Mocks, m => Assert.Null(m.ArgsMatchJson)); + Assert.Equal([0, 1], draft.Mocks.Select(m => m.CallIndex)); + Assert.Equal("""{"status":"Open"}""", draft.Mocks[0].ResultContent); + Assert.Equal("""{"status":"Closed"}""", draft.Mocks[1].ResultContent); + + var turn0Assertions = Assert.Single(draft.Turns).Assertions; + Assert.Equal(2, turn0Assertions.Count); + Assert.Equal("""{"woNum":"B1"}""", turn0Assertions[0].ArgsMatchJson); + Assert.Equal("""{"woNum":"B2"}""", turn0Assertions[1].ArgsMatchJson); + } + + [Fact] + public void Treats_differently_cased_recordings_of_the_same_function_as_one_function() + { + // Fix wave item 8d: pendingArgsByFunction/callCountByFunction used to compare function + // names Ordinal while ToolMockMatcher.Match (and ActiveTestRun's own call-ordinal + // tracker) compare OrdinalIgnoreCase -- two differently-cased recordings of the SAME real + // function were two functions to the recorder (each counted once, so neither's + // ArgsMatchJson got stripped and both got CallIndex 0) but one function to the matcher, + // which could then resolve either replay call to either mock. This dialog fixture is + // otherwise identical to the two-calls-same-function case already covered above, just + // with the second call's FunctionName differently cased. + List dialogs = + [ + new() { Role = "user", Content = "check work order B1 then again", MessageId = "m1" }, + new() { Role = "assistant", FunctionName = "Get_Work_Order", FunctionArgs = """{"woNum":"B1"}""", MessageId = "m1" }, + new() { Role = "function", FunctionName = "Get_Work_Order", Content = """{"status":"Open"}""", MessageId = "m1" }, + new() { Role = "assistant", FunctionName = "get_work_order", FunctionArgs = """{"woNum":"B1"}""", MessageId = "m1" }, + new() { Role = "function", FunctionName = "get_work_order", Content = """{"status":"Scheduled"}""", MessageId = "m1" }, + new() { Role = "assistant", Content = "done", MessageId = "m1" } + ]; + + var draft = AgentTestRecorder.BuildDraft("suite-1", "conv-9", dialogs, []); + + Assert.Equal(2, draft.Mocks.Count); + // Recognized as the SAME function called twice -> ordinal-only, ArgsMatchJson dropped on + // both, exactly like the case-consistent repeated-function fixture above. Before the fix, + // each cased variant was counted once and both mocks would have kept their ArgsMatchJson. + Assert.All(draft.Mocks, m => Assert.Null(m.ArgsMatchJson)); + Assert.Equal([0, 1], draft.Mocks.Select(m => m.CallIndex)); + Assert.Equal("""{"status":"Open"}""", draft.Mocks[0].ResultContent); + Assert.Equal("""{"status":"Scheduled"}""", draft.Mocks[1].ResultContent); + } + + [Fact] + public void A_function_dialog_with_no_function_name_produces_a_mock_but_no_pinning_assertion() + { + // Coordinator re-review item 3: a Role=Function dialog with a null/blank FunctionName + // used to still generate a toolCalled assertion with a blank Target -- which + // AssertionValidation (fix wave item 4) now rejects at save time, so RecordCase could + // persist a draft that the very next UpdateCase (even one editing an unrelated field) + // refused to save, from a 400 that names the type but not which assertion. An assertion + // that pins nothing isn't worth recording; the fix skips generating it while still + // recording the mock itself, so a human reviewing the draft can see and fix the gap. + List dialogs = + [ + new() { Role = "user", Content = "do something", MessageId = "m1" }, + new() { Role = "function", FunctionName = null, Content = "some result", MessageId = "m1" }, + new() { Role = "assistant", Content = "done", MessageId = "m1" } + ]; + + var draft = AgentTestRecorder.BuildDraft("suite-1", "conv-9", dialogs, []); + + var mock = Assert.Single(draft.Mocks); + Assert.Equal(string.Empty, mock.FunctionName); + Assert.Empty(Assert.Single(draft.Turns).Assertions); + } + + [Fact] + public void Suggests_only_stable_assertion_types() + { + var draft = Draft(); + + var types = draft.Turns.SelectMany(t => t.Assertions).Concat(draft.Assertions) + .Select(a => a.Type).Distinct().ToList(); + + Assert.Contains(AssertionTypes.ToolCalled, types); + Assert.Contains(AssertionTypes.StateEquals, types); + Assert.DoesNotContain(AssertionTypes.OutputContains, types); + Assert.DoesNotContain(AssertionTypes.OutputRegex, types); + Assert.DoesNotContain(AssertionTypes.LlmJudge, types); + } + + [Fact] + public void The_draft_is_disabled_and_remembers_where_it_came_from() + { + var draft = Draft(); + + Assert.False(draft.Enabled); // enabled only after a human reviews it + Assert.Equal("conv-9", draft.SourceConversationId); + Assert.Equal(UnmockedToolPolicies.Block, draft.UnmockedToolPolicy); + } + + [Fact] + public void A_conversation_with_no_user_message_produces_an_empty_draft_rather_than_throwing() + { + var draft = AgentTestRecorder.BuildDraft("suite-1", "conv-x", + [new RecordedDialog { Role = "assistant", Content = "hello" }], []); + + Assert.Empty(draft.Turns); + } +} diff --git a/tests/BotSharp.Core.UnitTests/AgentTesting/AgentTestRunExecutorTests.cs b/tests/BotSharp.Core.UnitTests/AgentTesting/AgentTestRunExecutorTests.cs new file mode 100644 index 000000000..eee4129f6 --- /dev/null +++ b/tests/BotSharp.Core.UnitTests/AgentTesting/AgentTestRunExecutorTests.cs @@ -0,0 +1,414 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Extensions.Logging.Abstractions; +using BotSharp.Plugin.AgentTesting.Repositories; +using BotSharp.Plugin.AgentTesting.Services; +using BotSharp.Plugin.AgentTesting.Models; +using Xunit; + +namespace BotSharp.Core.UnitTests.AgentTesting; + +/// +/// Run-level orchestration: cases run serially, one failing case does not affect the rest, the counts +/// are right, and cancellation takes effect promptly. Serial execution is a safety choice rather than +/// a performance one -- cases share external dependencies, and running them concurrently would let +/// them pollute each other's state. +/// +public class AgentTestRunExecutorTests +{ + private sealed class InMemoryRepo : IAgentTestRepository + { + public AgentTestSuite Suite = new() { Id = "suite-1", AgentId = "agent-1", Name = "s" }; + public List Cases = []; + public AgentTestRun Run = new() { Id = "run-1", SuiteId = "suite-1" }; + public List Results = []; + + public Task GetSuiteAsync(string id) => Task.FromResult(Suite); + public Task> ListSuitesAsync(string? agentId) => Task.FromResult(new List { Suite }); + public Task UpsertSuiteAsync(AgentTestSuite suite) => Task.CompletedTask; + public Task DeleteSuiteAsync(string id) => Task.CompletedTask; + public Task GetCaseAsync(string id) => Task.FromResult(Cases.FirstOrDefault(c => c.Id == id)); + public Task> ListCasesAsync(string suiteId) => Task.FromResult(Cases); + public Task UpsertCaseAsync(AgentTestCase testCase) => Task.CompletedTask; + public Task DeleteCaseAsync(string id) => Task.CompletedTask; + public Task CreateRunAsync(AgentTestRun run) => Task.FromResult(run); + public Task GetRunAsync(string id) => Task.FromResult(Run); + public Task> ListRunsAsync(string? suiteId) => Task.FromResult(new List { Run }); + public Task> ListRunsByStatusAsync(string status) + => Task.FromResult(Run.Status == status ? new List { Run } : []); + public Task UpdateRunAsync(AgentTestRun run) { Run = run; return Task.CompletedTask; } + public Task AddCaseResultAsync(AgentTestCaseResult result) { Results.Add(result); return Task.CompletedTask; } + public Task> ListCaseResultsAsync(string runId) => Task.FromResult(Results); + } + + /// + /// Fix round 1. Unlike InMemoryRepo -- which always hands back the SAME AgentTestRun object + /// reference, so "a stale local copy" and "a fresh read" are literally one object in every + /// InMemoryRepo-based test -- this fake mimics what a real Mongo ReplaceOneAsync/Find round + /// trip actually does: GetRunAsync always returns a brand-new CLONE of whatever was last + /// written, so mutating a previously-returned object never affects what a LATER read sees; + /// only UpdateRunAsync does. Only this fake can actually distinguish "the executor kept + /// mutating/persisting a copy taken before a case ran" from "the executor picked up what + /// changed while that case was executing." + /// + private sealed class CloningRunRepo : IAgentTestRepository + { + public AgentTestSuite Suite = new() { Id = "suite-1", AgentId = "agent-1", Name = "s" }; + public List Cases = []; + public List Results = []; + + private AgentTestRun _stored = new() { Id = "run-1", SuiteId = "suite-1" }; + + /// The current backing state, for assertions -- a clone, so tests can't cheat by mutating it directly. + public AgentTestRun Stored => Clone(_stored); + + /// Simulates a concurrent POST /runs/{id}/cancel landing directly in the backing store. + public void SetCancelRequestedInBackingStore(bool value) => _stored.CancelRequested = value; + + public Task GetSuiteAsync(string id) => Task.FromResult(Suite); + public Task> ListSuitesAsync(string? agentId) => Task.FromResult(new List { Suite }); + public Task UpsertSuiteAsync(AgentTestSuite suite) => Task.CompletedTask; + public Task DeleteSuiteAsync(string id) => Task.CompletedTask; + public Task GetCaseAsync(string id) => Task.FromResult(Cases.FirstOrDefault(c => c.Id == id)); + public Task> ListCasesAsync(string suiteId) => Task.FromResult(Cases); + public Task UpsertCaseAsync(AgentTestCase testCase) => Task.CompletedTask; + public Task DeleteCaseAsync(string id) => Task.CompletedTask; + public Task CreateRunAsync(AgentTestRun run) => Task.FromResult(run); + public Task GetRunAsync(string id) => Task.FromResult(Clone(_stored)); + public Task> ListRunsAsync(string? suiteId) => Task.FromResult(new List { Clone(_stored) }); + public Task> ListRunsByStatusAsync(string status) + => Task.FromResult(_stored.Status == status ? new List { Clone(_stored) } : []); + public Task UpdateRunAsync(AgentTestRun run) { _stored = Clone(run); return Task.CompletedTask; } + public Task AddCaseResultAsync(AgentTestCaseResult result) { Results.Add(result); return Task.CompletedTask; } + public Task> ListCaseResultsAsync(string runId) => Task.FromResult(Results); + + private static AgentTestRun Clone(AgentTestRun source) => new() + { + Id = source.Id, + SuiteId = source.SuiteId, + Status = source.Status, + TriggeredBy = source.TriggeredBy, + CaseIds = source.CaseIds, + TotalCount = source.TotalCount, + PassedCount = source.PassedCount, + FailedCount = source.FailedCount, + ErrorCount = source.ErrorCount, + CancelRequested = source.CancelRequested, + StartedAt = source.StartedAt, + CompletedAt = source.CompletedAt, + CreateDate = source.CreateDate + }; + } + + private static AgentTestCase CaseNamed(string id) => new() + { + Id = id, SuiteId = "suite-1", Name = id, + Turns = [new TestTurn { Index = 0, UserMessage = "hi" }] + }; + + private static AgentTestRunExecutor Build(InMemoryRepo repo, Func run) + => BuildWithRunner(repo, run).Executor; + + private static (AgentTestRunExecutor Executor, DelegatingCaseRunner Runner) BuildWithRunner( + InMemoryRepo repo, Func run) + { + var runner = new DelegatingCaseRunner(run); + return (new AgentTestRunExecutor(repo, runner, NullLogger.Instance), runner); + } + + private sealed class DelegatingCaseRunner(Func run) : ICaseRunner + { + /// Every (case, model) pair the executor asked for, in the order it asked. + public List<(string CaseId, string? Model)> Invocations { get; } = []; + + public Task RunAsync( + AgentTestSuite suite, AgentTestCase testCase, string runId, TestModel? model, CancellationToken ct) + { + Invocations.Add((testCase.Id, model?.Model)); + var result = run(testCase); + // The real runner stamps these onto the result; mirror it so tests can assert that a + // result can be attributed back to the model that produced it. + result.Provider ??= model?.Provider; + result.Model ??= model?.Model; + return Task.FromResult(result); + } + } + + [Fact] + public async Task Counts_passed_failed_and_errored_cases() + { + // Fix round 1, finding 4: the brief's original 1-passed/1-failed/1-error mix can't tell a + // Passed/Failed arm-swap bug apart from correct code (both produce 1/1/1 either way). + // Two passed cases makes that class of bug visible (a swap would report 1 passed, not 2). + var repo = new InMemoryRepo { Cases = [CaseNamed("a"), CaseNamed("b"), CaseNamed("c"), CaseNamed("d")] }; + var executor = Build(repo, c => new AgentTestCaseResult + { + CaseId = c.Id, + Status = c.Id switch + { + "a" => AgentTestStatus.Passed, + "d" => AgentTestStatus.Passed, + "b" => AgentTestStatus.Failed, + _ => AgentTestStatus.Error + } + }); + + await executor.ExecuteAsync("run-1", CancellationToken.None); + + Assert.Equal(4, repo.Run.TotalCount); + Assert.Equal(2, repo.Run.PassedCount); + Assert.Equal(1, repo.Run.FailedCount); + Assert.Equal(1, repo.Run.ErrorCount); + Assert.NotNull(repo.Run.CompletedAt); + } + + [Fact] + public async Task Every_enabled_case_runs_once_per_requested_model() + { + // The whole point of the model dimension: one run has to produce a full case x model grid, + // otherwise "compare gpt-4o against claude on the same suite" needs two runs and a human + // diffing two pages. + var repo = new InMemoryRepo { Cases = [CaseNamed("a"), CaseNamed("b")] }; + repo.Run.Models = + [ + new TestModel { Provider = "openai", Model = "gpt-4o" }, + new TestModel { Provider = "anthropic", Model = "claude-3-7-sonnet-20250219" } + ]; + var (executor, runner) = BuildWithRunner(repo, c => new AgentTestCaseResult + { + CaseId = c.Id, + Status = AgentTestStatus.Passed + }); + + await executor.ExecuteAsync("run-1", CancellationToken.None); + + // Case-major order, so the first case's comparison is complete before the second starts. + Assert.Equal( + [("a", "gpt-4o"), ("a", "claude-3-7-sonnet-20250219"), ("b", "gpt-4o"), ("b", "claude-3-7-sonnet-20250219")], + runner.Invocations); + + // TotalCount is cases x models, not cases -- a run of 2 cases over 2 models is 4 executions + // and every one of them costs real tokens. + Assert.Equal(4, repo.Run.TotalCount); + Assert.Equal(4, repo.Run.PassedCount); + Assert.Equal(4, repo.Results.Count); + + // Attribution: without provider/model on the result the grid cannot be built at all. + Assert.Equal(2, repo.Results.Count(r => r.Model == "gpt-4o")); + Assert.All(repo.Results.Where(r => r.Model == "gpt-4o"), r => Assert.Equal("openai", r.Provider)); + } + + [Fact] + public async Task No_requested_model_still_runs_each_case_exactly_once() + { + // Back-compat: every run document written before the model dimension existed has no Models + // field, and every caller that omits it must keep the old one-pass behaviour. + var repo = new InMemoryRepo { Cases = [CaseNamed("a"), CaseNamed("b")] }; + repo.Run.Models = null; + var (executor, runner) = BuildWithRunner(repo, c => new AgentTestCaseResult + { + CaseId = c.Id, + Status = AgentTestStatus.Passed + }); + + await executor.ExecuteAsync("run-1", CancellationToken.None); + + Assert.Equal([("a", null), ("b", null)], runner.Invocations); + Assert.Equal(2, repo.Run.TotalCount); + Assert.All(repo.Results, r => Assert.Null(r.Model)); + } + + [Fact] + public async Task One_crashing_case_does_not_abort_the_rest_of_the_run() + { + var repo = new InMemoryRepo { Cases = [CaseNamed("a"), CaseNamed("b")] }; + var executor = Build(repo, c => c.Id == "a" + ? throw new InvalidOperationException("boom") + : new AgentTestCaseResult { CaseId = c.Id, Status = AgentTestStatus.Passed }); + + await executor.ExecuteAsync("run-1", CancellationToken.None); + + Assert.Equal(2, repo.Results.Count); + Assert.Equal(AgentTestStatus.Error, repo.Results[0].Status); + Assert.Equal(AgentTestStatus.Passed, repo.Results[1].Status); + } + + [Fact] + public async Task A_disabled_suite_ends_the_run_as_error_without_running_any_case() + { + // Fix wave item 8a: POST .../suites/{id}/run is the primary place a disabled suite is + // rejected (400, before a run row exists at all -- see AgentTestControllerTests). This + // pins the executor's own defense-in-depth for the race where a suite is disabled AFTER a + // run was already queued: same "infrastructure stop" shape as the pre-existing + // suite-no-longer-exists handling right above this check in the production code. + var repo = new InMemoryRepo { Cases = [CaseNamed("a")] }; + repo.Suite.Enabled = false; + var executor = Build(repo, c => new AgentTestCaseResult { CaseId = c.Id, Status = AgentTestStatus.Passed }); + + await executor.ExecuteAsync("run-1", CancellationToken.None); + + Assert.Empty(repo.Results); + Assert.Equal(AgentTestStatus.Error, repo.Run.Status); + Assert.NotNull(repo.Run.CompletedAt); + } + + [Fact] + public async Task Skips_disabled_cases() + { + var disabled = CaseNamed("b"); + disabled.Enabled = false; + var repo = new InMemoryRepo { Cases = [CaseNamed("a"), disabled] }; + var executor = Build(repo, c => new AgentTestCaseResult { CaseId = c.Id, Status = AgentTestStatus.Passed }); + + await executor.ExecuteAsync("run-1", CancellationToken.None); + + Assert.Single(repo.Results); + // Fix round 1, finding 4: Assert.Single alone can't tell "kept the enabled case" apart + // from "kept the disabled one by an inverted filter" -- both leave exactly one result. + Assert.Equal("a", repo.Results[0].CaseId); + } + + [Fact] + public async Task Stops_between_cases_once_cancellation_is_requested() + { + var repo = new InMemoryRepo { Cases = [CaseNamed("a"), CaseNamed("b"), CaseNamed("c")] }; + var executor = Build(repo, c => + { + repo.Run.CancelRequested = true; // cancel requested once the first case finished + return new AgentTestCaseResult { CaseId = c.Id, Status = AgentTestStatus.Passed }; + }); + + await executor.ExecuteAsync("run-1", CancellationToken.None); + + Assert.Single(repo.Results); + Assert.Equal(AgentTestStatus.Cancelled, repo.Run.Status); + } + + [Fact] + public async Task Runs_only_the_cases_named_by_CaseIds_when_specified() + { + // Important 2: re-running only a caller-selected subset (e.g. "just the cases that failed + // last time") is core value for a regression harness, not a nice-to-have. + var repo = new InMemoryRepo { Cases = [CaseNamed("a"), CaseNamed("b"), CaseNamed("c")] }; + repo.Run.CaseIds = ["b"]; + var executor = Build(repo, c => new AgentTestCaseResult { CaseId = c.Id, Status = AgentTestStatus.Passed }); + + await executor.ExecuteAsync("run-1", CancellationToken.None); + + Assert.Single(repo.Results); + Assert.Equal("b", repo.Results[0].CaseId); + Assert.Equal(1, repo.Run.TotalCount); + } + + [Fact] + public async Task A_CaseIds_list_that_names_no_runnable_case_ends_the_run_as_Error_not_Passed() + { + // Task 9 fix: same "nothing executed, reports green" defect class as + // AgentTestCaseRunner's "the case has no turns" guard -- FailedCount == 0 && + // ErrorCount == 0 a few lines below is vacuously true when the loop never runs at all, + // which is exactly what happens when every id named by a non-empty CaseIds filter is + // unknown (or names a disabled case) in this suite. Before this fix the run persisted + // Status = Passed with TotalCount = 0 and zero result rows. + var repo = new InMemoryRepo { Cases = [CaseNamed("a"), CaseNamed("b")] }; + repo.Run.CaseIds = ["does-not-exist"]; + var executor = Build(repo, c => new AgentTestCaseResult { CaseId = c.Id, Status = AgentTestStatus.Passed }); + + await executor.ExecuteAsync("run-1", CancellationToken.None); + + Assert.Empty(repo.Results); + Assert.Equal(AgentTestStatus.Error, repo.Run.Status); + Assert.Equal(0, repo.Run.TotalCount); + Assert.NotNull(repo.Run.CompletedAt); + } + + [Fact] + public async Task An_empty_CaseIds_list_still_runs_every_enabled_case() + { + // CaseIds is meant to narrow the run; an empty (as opposed to null) list must not be + // read as "run nothing" -- that would make triggering a run with a request body like + // `{}` (which model-binds CaseIds to null, not []) behave differently from one that + // explicitly sends `{"caseIds": []}`, which is not the contract. + var repo = new InMemoryRepo { Cases = [CaseNamed("a"), CaseNamed("b")] }; + repo.Run.CaseIds = []; + var executor = Build(repo, c => new AgentTestCaseResult { CaseId = c.Id, Status = AgentTestStatus.Passed }); + + await executor.ExecuteAsync("run-1", CancellationToken.None); + + Assert.Equal(2, repo.Results.Count); + } + + [Fact] + public async Task Persists_the_running_totals_after_each_case_not_only_at_the_end() + { + // Important 1: GET /agent-test/runs/{id} is the only progress surface while a run is in + // flight. If counters are only written once at the very end, a 10-case suite at the + // default 120s-per-case timeout would show 0/0/0/0 for up to 20 minutes, and a process + // death mid-run would leave the startup sweep's Error row claiming zero cases ran even + // though N AgentTestCaseResult rows already exist. + // + // This has to use CloningRunRepo, not InMemoryRepo: InMemoryRepo's GetRunAsync/Run field + // is one shared object, so `run.TotalCount++` inside the executor and `repo.Run.TotalCount` + // read from a test are the SAME mutation observed instantly either way -- that would make + // this test pass identically whether or not the executor ever actually calls + // UpdateRunAsync per case (confirmed live: this exact test, written against InMemoryRepo, + // passed against the round-1 code that only persists once at the very end). CloningRunRepo + // only updates its backing state -- what repo.Stored reads -- when UpdateRunAsync is + // actually called, so it is the only fake that can tell "mutated a local object" apart from + // "persisted." + var repo = new CloningRunRepo { Cases = [CaseNamed("a"), CaseNamed("b")] }; + var observedTotalBeforeSecondCase = -1; + var executor = new AgentTestRunExecutor( + repo, + new DelegatingCaseRunner(c => + { + if (c.Id == "b") + { + observedTotalBeforeSecondCase = repo.Stored.TotalCount; + } + return new AgentTestCaseResult { CaseId = c.Id, Status = AgentTestStatus.Passed }; + }), + NullLogger.Instance); + + await executor.ExecuteAsync("run-1", CancellationToken.None); + + Assert.Equal(1, observedTotalBeforeSecondCase); + Assert.Equal(2, repo.Stored.TotalCount); + } + + [Fact] + public async Task A_cancel_that_arrives_while_the_only_case_is_executing_still_survives_the_terminal_write() + { + // Fix round 1, "Minor 3": a cancel landing directly in the backing store WHILE the last + // (here, only) case is running has no further loop iteration left to notice it via the + // normal CancelRequested check. Proving the fix needs a repo that returns a genuinely + // DIFFERENT object per read (CloningRunRepo) -- InMemoryRepo's shared-reference behavior + // means "stale copy" and "fresh read" are the same object, so it can't distinguish a + // correct terminal persist from one that clobbers the flag with a stale in-memory copy. + var repo = new CloningRunRepo { Cases = [CaseNamed("a")] }; + var executor = new AgentTestRunExecutor( + repo, + new DelegatingCaseRunner(c => + { + // Simulate the concurrent POST /runs/{id}/cancel landing in the backing store + // while this case is "executing" -- after this case's own CancelRequested check + // already read false, with no case after it to re-check before the run ends. + repo.SetCancelRequestedInBackingStore(true); + return new AgentTestCaseResult { CaseId = c.Id, Status = AgentTestStatus.Passed }; + }), + NullLogger.Instance); + + await executor.ExecuteAsync("run-1", CancellationToken.None); + + // The run legitimately finished the only case it was ever going to run; Status reflects + // that real outcome, not a retroactive cancellation it never actually detected in time to + // act on. + Assert.Equal(AgentTestStatus.Passed, repo.Stored.Status); + // But the flag an external caller set mid-flight must survive the terminal whole-document + // replace, not get silently overwritten back to false by a copy read before that write + // happened. + Assert.True(repo.Stored.CancelRequested); + } +} diff --git a/tests/BotSharp.Core.UnitTests/AgentTesting/AssertionEvaluatorTests.cs b/tests/BotSharp.Core.UnitTests/AgentTesting/AssertionEvaluatorTests.cs new file mode 100644 index 000000000..fc9f01c11 --- /dev/null +++ b/tests/BotSharp.Core.UnitTests/AgentTesting/AssertionEvaluatorTests.cs @@ -0,0 +1,374 @@ +using System.Collections.Generic; +using BotSharp.Plugin.AgentTesting.Services; +using BotSharp.Plugin.AgentTesting.Models; +using Xunit; + +namespace BotSharp.Core.UnitTests.AgentTesting; + +/// +/// Assertion evaluation IS the pass/fail verdict, so it has to be pure and exhaustively testable. +/// Pinned type by type, including the edges that are easy to get wrong: an invalid regex must not +/// blow the case up into an Error (users do write bad regexes), toolCalled's argument matching is a +/// subset rather than an exact match (otherwise the author would have to list every argument the +/// model passes), and stateEquals distinguishes "the value differs" from "the key is not set at +/// all". +/// +public class AssertionEvaluatorTests +{ + private static AssertionContext Context( + string? output = null, + IReadOnlyList? calls = null, + IReadOnlyDictionary? states = null, + string? routedTo = null) => new() + { + Output = output, + ToolCalls = calls ?? [], + States = states ?? new Dictionary(), + RoutedToAgent = routedTo + }; + + [Fact] + public void Output_contains_passes_on_a_substring_and_reports_the_actual_output() + { + var result = AssertionEvaluator.Evaluate( + new TestAssertion { Type = AssertionTypes.OutputContains, Expected = "work order" }, + Context(output: "I created the work order for you.")); + + Assert.True(result.Passed); + Assert.Equal("I created the work order for you.", result.Actual); + } + + [Fact] + public void Output_contains_is_case_insensitive() + { + var result = AssertionEvaluator.Evaluate( + new TestAssertion { Type = AssertionTypes.OutputContains, Expected = "WORK ORDER" }, + Context(output: "I created the work order.")); + + Assert.True(result.Passed); + } + + [Fact] + public void Output_not_contains_fails_when_the_phrase_appears() + { + var result = AssertionEvaluator.Evaluate( + new TestAssertion { Type = AssertionTypes.OutputNotContains, Expected = "sorry" }, + Context(output: "Sorry, I cannot help.")); + + Assert.False(result.Passed); + } + + [Fact] + public void Output_regex_reports_a_bad_pattern_as_a_failed_assertion_not_an_exception() + { + var result = AssertionEvaluator.Evaluate( + new TestAssertion { Type = AssertionTypes.OutputRegex, Expected = "([unclosed" }, + Context(output: "anything")); + + Assert.False(result.Passed); + Assert.Contains("invalid regular expression", result.Message!); + } + + [Fact] + public void Tool_called_matches_arguments_as_a_subset() + { + var calls = new List + { + new() { FunctionName = "get_work_order", ArgsJson = """{"woNum":"B1","notes":true}""", Outcome = "Mocked" } + }; + + var result = AssertionEvaluator.Evaluate( + new TestAssertion + { + Type = AssertionTypes.ToolCalled, + Target = "get_work_order", + ArgsMatchJson = """{"woNum":"B1"}""" + }, + Context(calls: calls)); + + Assert.True(result.Passed); + } + + [Fact] + public void Tool_called_fails_when_arguments_do_not_match() + { + var calls = new List + { + new() { FunctionName = "get_work_order", ArgsJson = """{"woNum":"OTHER"}""", Outcome = "Mocked" } + }; + + var result = AssertionEvaluator.Evaluate( + new TestAssertion + { + Type = AssertionTypes.ToolCalled, + Target = "get_work_order", + ArgsMatchJson = """{"woNum":"B1"}""" + }, + Context(calls: calls)); + + Assert.False(result.Passed); + } + + [Fact] + public void Tool_not_called_passes_when_the_tool_never_appears() + { + var result = AssertionEvaluator.Evaluate( + new TestAssertion { Type = AssertionTypes.ToolNotCalled, Target = "send_text_message" }, + Context(calls: [new ObservedToolCall { FunctionName = "get_work_order", Outcome = "Mocked" }])); + + Assert.True(result.Passed); + } + + [Fact] + public void Tool_not_called_counts_a_blocked_call_as_called() + { + // Being blocked proves the agent did try to call it, which is precisely the behaviour + // toolNotCalled exists to catch. + var result = AssertionEvaluator.Evaluate( + new TestAssertion { Type = AssertionTypes.ToolNotCalled, Target = "send_text_message" }, + Context(calls: [new ObservedToolCall { FunctionName = "send_text_message", Outcome = "Blocked" }])); + + Assert.False(result.Passed); + } + + [Fact] + public void State_equals_distinguishes_a_wrong_value_from_a_missing_key() + { + var wrong = AssertionEvaluator.Evaluate( + new TestAssertion { Type = AssertionTypes.StateEquals, Target = "wo_id", Expected = "123" }, + Context(states: new Dictionary { ["wo_id"] = "456" })); + Assert.False(wrong.Passed); + Assert.Equal("456", wrong.Actual); + Assert.Contains("differs", wrong.Message!); + + var missing = AssertionEvaluator.Evaluate( + new TestAssertion { Type = AssertionTypes.StateEquals, Target = "wo_id", Expected = "123" }, + Context(states: new Dictionary())); + Assert.False(missing.Passed); + Assert.Contains("not set", missing.Message!); + } + + [Fact] + public void Routed_to_agent_compares_case_insensitively() + { + var result = AssertionEvaluator.Evaluate( + new TestAssertion { Type = AssertionTypes.RoutedToAgent, Expected = "Work Order Creator" }, + Context(routedTo: "work order creator")); + + Assert.True(result.Passed); + } + + [Fact] + public void An_unknown_assertion_type_fails_loudly() + { + var result = AssertionEvaluator.Evaluate( + new TestAssertion { Type = "outputLooksNice" }, + Context(output: "hi")); + + Assert.False(result.Passed); + Assert.Contains("unknown assertion type", result.Message!); + } + + [Fact] + public void Llm_judge_is_reported_as_unavailable_in_p1_rather_than_silently_passing() + { + var result = AssertionEvaluator.Evaluate( + new TestAssertion { Type = AssertionTypes.LlmJudge, Expected = "confirms the address before quoting", MinScore = 0.8 }, + Context(output: "whatever")); + + Assert.False(result.Passed); + Assert.Contains("not available in P1", result.Message!); + } + + /// + /// System.Text.Json.Nodes.JsonObject materialises its key/value dictionary lazily: JsonNode.Parse + /// does not complain about duplicate top-level keys ("woNum" appearing twice), and the + /// ArgumentException only fires on first access (TryGetPropertyValue/foreach inside IsSubset). + /// The ArgsJson here is the model's own arguments, so it has to go through + /// ToolMockMatcher.ParseOrNull -- catching JsonException alone is not enough, or this assertion + /// blows the whole case up into an infrastructure Error instead of evaluating to an ordinary + /// failure. + /// + [Fact] + public void Tool_called_fails_instead_of_throwing_when_the_actual_args_have_a_duplicate_top_level_key() + { + var calls = new List + { + new() + { + FunctionName = "get_work_order", + ArgsJson = """{"woNum":"B1","woNum":"B2"}""", + Outcome = "Mocked" + } + }; + + var result = AssertionEvaluator.Evaluate( + new TestAssertion + { + Type = AssertionTypes.ToolCalled, + Target = "get_work_order", + ArgsMatchJson = """{"woNum":"B1"}""" + }, + Context(calls: calls)); + + Assert.False(result.Passed); + } + + /// + /// The regression above only pressed duplicate keys on the ArgsJson side (the model's own + /// arguments); the ArgsMatchJson side (what the test author wrote) was never covered. Production + /// code calls ToolMockMatcher.ParseOrNull on both sides, but if someone later reverted the + /// ArgsMatchJson side to an unguarded parse, every earlier test here would stay green and say + /// nothing. This is the mirror case, so both sides are pinned. + /// + // ---- Fix wave: a blank/omitted required field must fail, never vacuously pass ----------- + // (outputContains/outputRegex/toolNotCalled/routedToAgent verified nothing without this; + // toolCalled/stateEquals already failed safe -- see AssertionEvaluatorTests above.) + + [Theory] + [InlineData(null)] + [InlineData("")] + public void Output_contains_fails_rather_than_vacuously_passing_on_a_blank_expected(string? expected) + { + // Contains("") is true for any non-null string -- without the guard this would pass + // against ANY output, verifying nothing. + var result = AssertionEvaluator.Evaluate( + new TestAssertion { Type = AssertionTypes.OutputContains, Expected = expected }, + Context(output: "anything at all")); + + Assert.False(result.Passed); + Assert.Contains("requires a non-empty", result.Message!); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + public void Output_regex_fails_rather_than_vacuously_passing_on_a_blank_pattern(string? expected) + { + // An empty regex pattern matches every string -- without the guard this would pass + // against ANY output, verifying nothing. + var result = AssertionEvaluator.Evaluate( + new TestAssertion { Type = AssertionTypes.OutputRegex, Expected = expected }, + Context(output: "anything at all")); + + Assert.False(result.Passed); + Assert.Contains("requires a non-empty", result.Message!); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + public void Tool_not_called_fails_rather_than_vacuously_passing_on_a_blank_target(string? target) + { + // A null/blank Target matches no real call's FunctionName -- without the guard this would + // report "not called" regardless of what the agent actually did, verifying nothing. + var result = AssertionEvaluator.Evaluate( + new TestAssertion { Type = AssertionTypes.ToolNotCalled, Target = target }, + Context(calls: [new ObservedToolCall { FunctionName = "create_work_order", Outcome = "Mocked" }])); + + Assert.False(result.Passed); + Assert.Contains("requires a non-empty", result.Message!); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + public void Routed_to_agent_fails_rather_than_vacuously_passing_on_a_blank_expected(string? expected) + { + // A null Expected compares equal to a null RoutedToAgent -- without the guard a case with + // no routing information at all (or a typo'd blank field) would silently pass. + var result = AssertionEvaluator.Evaluate( + new TestAssertion { Type = AssertionTypes.RoutedToAgent, Expected = expected }, + Context(routedTo: null)); + + Assert.False(result.Passed); + Assert.Contains("requires a non-empty", result.Message!); + } + + [Fact] + public void Tool_called_fails_instead_of_throwing_when_the_expected_args_match_json_has_a_duplicate_top_level_key() + { + var calls = new List + { + new() { FunctionName = "get_work_order", ArgsJson = """{"woNum":"B1"}""", Outcome = "Mocked" } + }; + + var result = AssertionEvaluator.Evaluate( + new TestAssertion + { + Type = AssertionTypes.ToolCalled, + Target = "get_work_order", + ArgsMatchJson = """{"woNum":"B1","woNum":"B2"}""" + }, + Context(calls: calls)); + + Assert.False(result.Passed); + Assert.Contains("could not be parsed", result.Message!); + } +} + +/// +/// Direct unit coverage for the save-time counterpart to the runtime fixes above: +/// is what AgentTestController's create/update path +/// calls before a case is ever persisted. The eight-row type->required-field mapping is +/// exercised here directly (pure function, no controller/repo plumbing needed); +/// AgentTestControllerTests additionally proves the controller actually calls it and turns a +/// non-null result into a 400. +/// +public class AssertionValidationTests +{ + [Theory] + [InlineData(AssertionTypes.OutputContains)] + [InlineData(AssertionTypes.OutputNotContains)] + [InlineData(AssertionTypes.OutputRegex)] + [InlineData(AssertionTypes.RoutedToAgent)] + [InlineData(AssertionTypes.LlmJudge)] + public void Rejects_a_missing_expected_for_types_that_require_it(string type) + { + Assert.NotNull(AssertionValidation.Validate(new TestAssertion { Type = type })); + Assert.NotNull(AssertionValidation.Validate(new TestAssertion { Type = type, Expected = "" })); + Assert.NotNull(AssertionValidation.Validate(new TestAssertion { Type = type, Expected = " " })); + } + + [Theory] + [InlineData(AssertionTypes.OutputContains)] + [InlineData(AssertionTypes.OutputNotContains)] + [InlineData(AssertionTypes.OutputRegex)] + [InlineData(AssertionTypes.RoutedToAgent)] + [InlineData(AssertionTypes.LlmJudge)] + public void Accepts_a_non_blank_expected_for_types_that_require_it(string type) + { + Assert.Null(AssertionValidation.Validate(new TestAssertion { Type = type, Expected = "x" })); + } + + [Theory] + [InlineData(AssertionTypes.ToolCalled)] + [InlineData(AssertionTypes.ToolNotCalled)] + [InlineData(AssertionTypes.StateEquals)] + public void Rejects_a_missing_target_for_types_that_require_it(string type) + { + Assert.NotNull(AssertionValidation.Validate(new TestAssertion { Type = type })); + Assert.NotNull(AssertionValidation.Validate(new TestAssertion { Type = type, Target = "" })); + Assert.NotNull(AssertionValidation.Validate(new TestAssertion { Type = type, Target = " " })); + } + + [Theory] + [InlineData(AssertionTypes.ToolCalled)] + [InlineData(AssertionTypes.ToolNotCalled)] + [InlineData(AssertionTypes.StateEquals)] + public void Accepts_a_non_blank_target_for_types_that_require_it(string type) + { + Assert.Null(AssertionValidation.Validate(new TestAssertion { Type = type, Target = "x" })); + } + + [Fact] + public void Does_not_reject_an_unrecognized_type() + { + // Not this validator's job -- AssertionEvaluator's own `default` branch already fails an + // unknown type loudly at evaluation time. Rejecting it here too would block saving a case + // authored against a not-yet-released assertion type. + Assert.Null(AssertionValidation.Validate(new TestAssertion { Type = "somethingNotYetSupported" })); + } +} diff --git a/tests/BotSharp.Core.UnitTests/AgentTesting/CaseSegmentationTests.cs b/tests/BotSharp.Core.UnitTests/AgentTesting/CaseSegmentationTests.cs new file mode 100644 index 000000000..50bcafad0 --- /dev/null +++ b/tests/BotSharp.Core.UnitTests/AgentTesting/CaseSegmentationTests.cs @@ -0,0 +1,180 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using BotSharp.Abstraction.Agents.Enums; +using BotSharp.Plugin.AgentTesting.Services; +using BotSharp.Plugin.AgentTesting.Models; +using Xunit; + +namespace BotSharp.Core.UnitTests.AgentTesting; + +/// +/// AI extraction is allowed to decide exactly two things: where to cut and what to name each piece. +/// These tests pin both sides of that boundary -- how far the parser distrusts the model's output, +/// and the corrections slicing needs that BuildDraft cannot see for itself. +/// +public class CaseSegmentationTests +{ + // ---- LlmCaseSegmenter.Parse: extend the model's answer no trust ------------------------- + + [Fact] + public void Parse_accepts_a_well_formed_contiguous_split() + { + var segments = LlmCaseSegmenter.Parse( + """{"segments":[{"name":"ETA","firstTurn":0,"lastTurn":1},{"name":"Reschedule","firstTurn":2,"lastTurn":2}]}""", + turnCount: 3); + + Assert.Equal(2, segments.Count); + Assert.Equal(new CaseSegment("ETA", 0, 1), segments[0]); + Assert.Equal(new CaseSegment("Reschedule", 2, 2), segments[1]); + } + + [Fact] + public void Parse_tolerates_a_code_fence_and_surrounding_prose() + { + // The commonest and most harmless way for a model to disobey "JSON only". Failing the whole + // extraction over a ``` would just make the feature look flaky. + var segments = LlmCaseSegmenter.Parse( + "Sure! Here you go:\n```json\n{\"segments\":[{\"name\":\"All\",\"firstTurn\":0,\"lastTurn\":0}]}\n```", + turnCount: 1); + + Assert.Single(segments); + } + + [Theory] + // A gap: turn 1 belongs to no case, so it silently disappears from the test set. + [InlineData("""{"segments":[{"name":"a","firstTurn":0,"lastTurn":0},{"name":"b","firstTurn":2,"lastTurn":2}]}""", 3)] + // An overlap: turn 1 lands in two cases. + [InlineData("""{"segments":[{"name":"a","firstTurn":0,"lastTurn":1},{"name":"b","firstTurn":1,"lastTurn":2}]}""", 3)] + // Out of range. + [InlineData("""{"segments":[{"name":"a","firstTurn":0,"lastTurn":9}]}""", 3)] + // Trailing turns left uncovered. + [InlineData("""{"segments":[{"name":"a","firstTurn":0,"lastTurn":0}]}""", 3)] + // Not JSON at all. + [InlineData("I could not determine the segments.", 3)] + // Structurally fine, semantically empty. + [InlineData("""{"segments":[]}""", 3)] + public void Parse_rejects_anything_it_cannot_fully_verify(string raw, int turnCount) + { + // Half-right segmentation is the dangerous outcome: the drafts look plausible in the UI + // (real names, real turns) and only misbehave when someone runs them. + Assert.Throws(() => LlmCaseSegmenter.Parse(raw, turnCount)); + } + + [Fact] + public void Parse_falls_back_to_a_positional_name_when_the_model_leaves_it_blank() + { + var segments = LlmCaseSegmenter.Parse( + """{"segments":[{"name":" ","firstTurn":0,"lastTurn":1}]}""", turnCount: 2); + + Assert.Equal("Turns 0-1", segments[0].Name); + } + + // ---- AgentTestRecorder.BuildDrafts: the two corrections slicing needs ------------------- + + private static List TwoScenarioConversation() => + [ + new() { Role = AgentRole.User, Content = "where is my tech", MessageId = "m0" }, + new() { Role = AgentRole.Assistant, FunctionName = "get_eta", FunctionArgs = """{"wo_num":"B1"}""" }, + new() { Role = AgentRole.Function, FunctionName = "get_eta", Content = """{"eta":"2pm"}""" }, + new() { Role = AgentRole.User, Content = "reschedule it", MessageId = "m1" }, + new() { Role = AgentRole.Assistant, FunctionName = "reschedule", FunctionArgs = """{"wo_num":"B1"}""" }, + new() { Role = AgentRole.Function, FunctionName = "reschedule", Content = """{"ok":true}""" } + ]; + + private static List TwoScenarioStates() => + [ + new() + { + Key = "wo_num", + Values = + [ + new RecordedStateValue { MessageId = null, Data = "B1", ActiveRounds = -1 }, + ] + }, + new() + { + Key = "stage", + Values = + [ + new RecordedStateValue { MessageId = "m0", Data = "eta-shown", ActiveRounds = -1 }, + new RecordedStateValue { MessageId = "m1", Data = "rescheduled", ActiveRounds = -1 } + ] + } + ]; + + [Fact] + public void BuildDrafts_gives_each_segment_only_its_own_turns_and_mocks() + { + var drafts = AgentTestRecorder.BuildDrafts( + "suite-1", "conv-1", TwoScenarioConversation(), TwoScenarioStates(), + [new CaseSegment("ETA", 0, 0), new CaseSegment("Reschedule", 1, 1)]); + + Assert.Equal(2, drafts.Count); + Assert.Equal("ETA", drafts[0].Name); + Assert.Equal("where is my tech", Assert.Single(drafts[0].Turns).UserMessage); + Assert.Equal("get_eta", Assert.Single(drafts[0].Mocks).FunctionName); + + Assert.Equal("reschedule it", Assert.Single(drafts[1].Turns).UserMessage); + Assert.Equal("reschedule", Assert.Single(drafts[1].Mocks).FunctionName); + + // The mock payload is the real recorded return value, not something a model wrote. + Assert.Equal("""{"ok":true}""", drafts[1].Mocks[0].ResultContent); + + // Every draft still lands disabled, exactly like the single-case recorder. + Assert.All(drafts, d => Assert.False(d.Enabled)); + Assert.All(drafts, d => Assert.Equal("conv-1", d.SourceConversationId)); + } + + [Fact] + public void BuildDrafts_carries_earlier_state_into_a_segment_that_does_not_start_at_turn_zero() + { + var drafts = AgentTestRecorder.BuildDrafts( + "suite-1", "conv-1", TwoScenarioConversation(), TwoScenarioStates(), + [new CaseSegment("ETA", 0, 0), new CaseSegment("Reschedule", 1, 1)]); + + // Segment 0 starts the conversation, so only the seeded value is initial state. + var first = drafts[0].InitialStates; + Assert.Equal("B1", Assert.Single(first, s => s.Key == "wo_num").Value); + Assert.DoesNotContain(first, s => s.Key == "stage"); + + // Segment 1 starts mid-conversation. Without the carry-in it would begin with `stage` + // unset and run a path the recording never took. + var second = drafts[1].InitialStates; + Assert.Equal("B1", Assert.Single(second, s => s.Key == "wo_num").Value); + Assert.Equal("eta-shown", Assert.Single(second, s => s.Key == "stage").Value); + } + + [Fact] + public void BuildDrafts_asserts_the_state_value_its_own_segment_reaches_not_the_conversations_final_one() + { + var drafts = AgentTestRecorder.BuildDrafts( + "suite-1", "conv-1", TwoScenarioConversation(), TwoScenarioStates(), + [new CaseSegment("ETA", 0, 0), new CaseSegment("Reschedule", 1, 1)]); + + // BuildDraft's own step 5b would put the whole conversation's LAST value ("rescheduled") + // on both drafts -- guaranteeing that the first one fails on every run, because it stops + // before that write ever happens. + var firstStage = Assert.Single(drafts[0].Assertions, a => a.Target == "stage"); + Assert.Equal("eta-shown", firstStage.Expected); + + var secondStage = Assert.Single(drafts[1].Assertions, a => a.Target == "stage"); + Assert.Equal("rescheduled", secondStage.Expected); + } + + [Fact] + public void ToSegmentableTurns_exposes_tool_names_but_not_their_arguments_or_results() + { + // The egress boundary: a change here silently starts shipping work order payloads -- + // addresses, phone numbers -- to a model vendor. + var turns = AgentTestRecorder.ToSegmentableTurns(TwoScenarioConversation()); + + Assert.Equal(2, turns.Count); + Assert.Equal(["get_eta"], turns[0].ToolNames); + Assert.Equal(["reschedule"], turns[1].ToolNames); + + var serialised = string.Join("|", turns.Select(t => t.UserMessage + string.Join(",", t.ToolNames))); + Assert.DoesNotContain("B1", serialised); + Assert.DoesNotContain("2pm", serialised); + } +} diff --git a/tests/BotSharp.Core.UnitTests/AgentTesting/MockFunctionExecutorTests.cs b/tests/BotSharp.Core.UnitTests/AgentTesting/MockFunctionExecutorTests.cs new file mode 100644 index 000000000..3b2335b23 --- /dev/null +++ b/tests/BotSharp.Core.UnitTests/AgentTesting/MockFunctionExecutorTests.cs @@ -0,0 +1,153 @@ +using System.Threading.Tasks; +using BotSharp.Abstraction.Agents.Enums; +using BotSharp.Abstraction.Conversations; +using BotSharp.Abstraction.Conversations.Models; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using BotSharp.Plugin.AgentTesting.Runtime; +using BotSharp.Plugin.AgentTesting.Models; +using Xunit; + +namespace BotSharp.Core.UnitTests.AgentTesting; + +/// +/// Three things about mock execution semantics have to be right: +/// 1) on a mock hit, the fake return is written to message.Content, which is what the LLM reads next +/// turn; +/// 2) when unmocked under the Block policy, the real implementation must never be reached, and the +/// turn has to fail explicitly rather than continue silently; +/// 3) on a mock hit it must be able to write conversation state -- for plenty of real functions the +/// actual "output" IS the state write, and returning only a value leaves later functions unable to +/// read what they need and the whole case collapses. +/// +public class MockFunctionExecutorTests +{ + private static (MockFunctionExecutor executor, Mock state) Build( + ActiveTestRun run, string functionName) + { + var state = new Mock(); + var executor = new MockFunctionExecutor(run, functionName, state.Object, NullLogger.Instance); + return (executor, state); + } + + private static ActiveTestRun Run(string policy, params TestToolMock[] mocks) => new() + { + ConversationId = "conv-1", + CaseId = "case-1", + Mocks = mocks, + UnmockedToolPolicy = policy + }; + + private static RoleDialogModel Call(string function, string? args = null) => + new(AgentRole.Assistant, string.Empty) { FunctionName = function, FunctionArgs = args }; + + [Fact] + public async Task Writes_the_canned_result_into_the_message() + { + var run = Run(UnmockedToolPolicies.Block, + new TestToolMock { FunctionName = "get_work_order", ResultContent = """{"status":"Open"}""" }); + var (executor, _) = Build(run, "get_work_order"); + var message = Call("get_work_order"); + + var ok = await executor.ExecuteAsync(message); + + Assert.True(ok); + Assert.Equal("""{"status":"Open"}""", message.Content); + } + + [Fact] + public async Task Applies_stop_completion_when_the_mock_says_so() + { + var run = Run(UnmockedToolPolicies.Block, + new TestToolMock { FunctionName = "ask_user", ResultContent = "ok", StopCompletion = true }); + var (executor, _) = Build(run, "ask_user"); + var message = Call("ask_user"); + + await executor.ExecuteAsync(message); + + Assert.True(message.StopCompletion); + } + + [Fact] + public async Task Applies_the_mocks_state_writes() + { + var run = Run(UnmockedToolPolicies.Block, new TestToolMock + { + FunctionName = "get_work_order", + ResultContent = "ok", + StateWrites = [new TestState { Key = "wo_id", Value = "123", ActiveRounds = 5 }] + }); + var (executor, state) = Build(run, "get_work_order"); + + await executor.ExecuteAsync(Call("get_work_order")); + + state.Verify(s => s.SetState( + "wo_id", "123", It.IsAny(), 5, It.IsAny(), It.IsAny(), It.IsAny()), + Times.Once); + } + + [Fact] + public async Task Blocks_an_unmocked_tool_under_the_block_policy() + { + var run = Run(UnmockedToolPolicies.Block); + var (executor, _) = Build(run, "send_text_message"); + var message = Call("send_text_message"); + + var ok = await executor.ExecuteAsync(message); + + Assert.False(ok); + Assert.Equal("[agent-test] blocked unmocked tool: send_text_message", message.Content); + Assert.True(message.StopCompletion); + } + + [Fact] + public async Task Records_every_call_it_handled_with_the_right_outcome() + { + var run = Run(UnmockedToolPolicies.Block, + new TestToolMock { FunctionName = "get_work_order", ResultContent = "ok" }); + run.CurrentTurnIndex = 2; + + await Build(run, "get_work_order").executor.ExecuteAsync(Call("get_work_order", """{"woNum":"B1"}""")); + await Build(run, "send_text_message").executor.ExecuteAsync(Call("send_text_message")); + + var calls = run.ObservedCalls; + Assert.Equal(2, calls.Count); + Assert.Equal("Mocked", calls[0].Outcome); + Assert.Equal("""{"woNum":"B1"}""", calls[0].ArgsJson); + Assert.Equal(2, calls[0].TurnIndex); + Assert.Equal("Blocked", calls[1].Outcome); + } + + [Fact] + public async Task Records_the_canary_so_the_runner_can_prove_the_seam_is_live() + { + var run = Run(UnmockedToolPolicies.Block); + var (executor, _) = Build(run, AgentTestCanary.FunctionName); + + await executor.ExecuteAsync(Call(AgentTestCanary.FunctionName)); + + Assert.True(run.CanaryIntercepted); + } + + [Fact] + public async Task Advances_the_call_ordinal_across_successive_calls_to_the_same_function() + { + // Unit-testing Match(..., callOrdinal) over in ToolMockMatcherTests is not enough. This + // proves the executor itself really wires _run.NextCallOrdinal(_functionName) into Match's + // ordinal parameter and that nobody quietly replaced it with a hardcoded 0 -- a hardcoded 0 + // would make the second call below resolve to the first mock too, and the assertion would + // catch it. + var run = Run(UnmockedToolPolicies.Block, + new TestToolMock { FunctionName = "get_work_order", CallIndex = 0, ResultContent = "first-call" }, + new TestToolMock { FunctionName = "get_work_order", CallIndex = 1, ResultContent = "second-call" }); + + var first = Call("get_work_order"); + await Build(run, "get_work_order").executor.ExecuteAsync(first); + + var second = Call("get_work_order"); + await Build(run, "get_work_order").executor.ExecuteAsync(second); + + Assert.Equal("first-call", first.Content); + Assert.Equal("second-call", second.Content); + } +} diff --git a/tests/BotSharp.Core.UnitTests/AgentTesting/TestMockExecutorProviderTests.cs b/tests/BotSharp.Core.UnitTests/AgentTesting/TestMockExecutorProviderTests.cs new file mode 100644 index 000000000..38c9dc3c0 --- /dev/null +++ b/tests/BotSharp.Core.UnitTests/AgentTesting/TestMockExecutorProviderTests.cs @@ -0,0 +1,149 @@ +using System; +using System.Collections.Generic; +using BotSharp.Abstraction.Agents.Models; +using BotSharp.Abstraction.Conversations; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using BotSharp.Plugin.AgentTesting.Runtime; +using BotSharp.Plugin.AgentTesting.Models; +using Xunit; + +namespace BotSharp.Core.UnitTests.AgentTesting; + +/// +/// The provider does exactly one thing: decide whether the current conversation is running a test +/// case, take over if it is, and pass through completely if it is not. +/// +/// It looks the conversation up in the registry by conversationId rather than using AsyncLocal. +/// AsyncLocal depends on ExecutionContext flowing, and is silently lost the moment an execution path +/// crosses a background queue or SideCar -- and losing it does not produce a failing test, it +/// produces unmocked tools executing for real: a real phone call, a real work order. These tests pin +/// both edges: never take over a non-test conversation, always take over a test one. +/// +public class TestMockExecutorProviderTests +{ + private const string TestConversation = "3f1c9d2e-0000-4a11-9b21-aaaaaaaaaaaa"; + private const string NormalConversation = "9a2b8c7d-1111-4c22-8f33-bbbbbbbbbbbb"; + + private static TestMockExecutorProvider Build(IAgentTestRunRegistry registry, string conversationId) + { + var conversations = new Mock(); + conversations.SetupGet(c => c.ConversationId).Returns(conversationId); + return new TestMockExecutorProvider( + registry, + conversations.Object, + new Mock().Object, + NullLogger.Instance); + } + + private static ActiveTestRun RunFor(string conversationId, params TestToolMock[] mocks) => new() + { + ConversationId = conversationId, + CaseId = "case-1", + Mocks = mocks, + UnmockedToolPolicy = UnmockedToolPolicies.Block, + AllowedFunctions = new HashSet(ControlFlowFunctions.Default, StringComparer.OrdinalIgnoreCase), + ForceBlockedFunctions = new HashSet(StringComparer.OrdinalIgnoreCase) + }; + + [Fact] + public void Does_not_take_over_a_conversation_that_is_not_under_test() + { + var registry = new AgentTestRunRegistry(); + registry.Register(RunFor(TestConversation)); + + var provider = Build(registry, NormalConversation); + + Assert.Null(provider.TryResolve("create_work_order", new Agent())); + } + + [Fact] + public void Takes_over_every_function_inside_a_conversation_under_test() + { + var registry = new AgentTestRunRegistry(); + registry.Register(RunFor(TestConversation, new TestToolMock { FunctionName = "create_work_order" })); + + var provider = Build(registry, TestConversation); + + Assert.NotNull(provider.TryResolve("create_work_order", new Agent())); + } + + [Fact] + public void Takes_over_an_unmocked_function_too_so_that_it_can_be_blocked() + { + // The crucial one: an unmocked function must be taken over too, or it falls through to the + // built-in chain and executes for real. + var registry = new AgentTestRunRegistry(); + registry.Register(RunFor(TestConversation)); + + var provider = Build(registry, TestConversation); + + Assert.NotNull(provider.TryResolve("send_text_message", new Agent())); + } + + [Fact] + public void Leaves_control_flow_functions_to_the_real_implementation() + { + // Blocking route_to_agent means the agent cannot move, and the case never reaches its + // assertions at all. + var registry = new AgentTestRunRegistry(); + registry.Register(RunFor(TestConversation)); + + var provider = Build(registry, TestConversation); + + Assert.Null(provider.TryResolve("route_to_agent", new Agent())); + Assert.Null(provider.TryResolve("util-routing-fallback_to_router", new Agent())); + } + + [Fact] + public void Takes_over_a_util_prefixed_function_that_is_not_control_flow() + { + // The allow list must be those exact five names and must never degrade into "anything + // prefixed util- passes". util-twilio-outbound_phone_call really places a phone call, so a + // prefix rule would wave it through and one test run would really dial out. This assertion + // fails under a prefix rule -- prefix matching would return null here, whereas the correct + // implementation has to take over. + var registry = new AgentTestRunRegistry(); + registry.Register(RunFor(TestConversation)); + + var provider = Build(registry, TestConversation); + + Assert.NotNull(provider.TryResolve("util-twilio-outbound_phone_call", new Agent())); + } + + [Fact] + public void A_force_blocked_function_is_taken_over_even_if_it_is_on_the_allow_list() + { + var run = RunFor(TestConversation); + run.ForceBlockedFunctions.Add("response_to_user"); + var registry = new AgentTestRunRegistry(); + registry.Register(run); + + var provider = Build(registry, TestConversation); + + Assert.NotNull(provider.TryResolve("response_to_user", new Agent())); + } + + [Fact] + public void Unregister_returns_the_conversation_to_normal_behaviour() + { + var registry = new AgentTestRunRegistry(); + registry.Register(RunFor(TestConversation)); + registry.Unregister(TestConversation); + + var provider = Build(registry, TestConversation); + + Assert.Null(provider.TryResolve("create_work_order", new Agent())); + } + + [Fact] + public void A_null_conversation_id_never_matches() + { + var registry = new AgentTestRunRegistry(); + registry.Register(RunFor(TestConversation)); + + var provider = Build(registry, null!); + + Assert.Null(provider.TryResolve("create_work_order", new Agent())); + } +} diff --git a/tests/BotSharp.Core.UnitTests/AgentTesting/ToolMockMatcherTests.cs b/tests/BotSharp.Core.UnitTests/AgentTesting/ToolMockMatcherTests.cs new file mode 100644 index 000000000..89ff7c584 --- /dev/null +++ b/tests/BotSharp.Core.UnitTests/AgentTesting/ToolMockMatcherTests.cs @@ -0,0 +1,114 @@ +using BotSharp.Plugin.AgentTesting.Runtime; +using BotSharp.Plugin.AgentTesting.Models; +using Xunit; + +namespace BotSharp.Core.UnitTests.AgentTesting; + +/// +/// In a real work order flow the same lookup function is called several times in one conversation +/// with different arguments, and each call has to return something different. Selecting a mock by +/// function name alone means the very first batch of real cases hits "three calls, one fake return". +/// Match priority, most specific first: argument-subset match, then call ordinal, then function name +/// alone. +/// +public class ToolMockMatcherTests +{ + private static readonly TestToolMock ByName = new() + { + FunctionName = "get_work_order", + ResultContent = "generic" + }; + + private static readonly TestToolMock ByArgs = new() + { + FunctionName = "get_work_order", + ArgsMatchJson = """{"woNum":"B9897413"}""", + ResultContent = "specific" + }; + + private static readonly TestToolMock ByOrdinal = new() + { + FunctionName = "get_work_order", + CallIndex = 1, + ResultContent = "second-call" + }; + + /// + /// ArgsMatchJson itself contains duplicate top-level keys -- syntactically valid, since + /// JsonNode.Parse does not object, but System.Text.Json.Nodes.JsonObject materialises its + /// backing dictionary lazily and the duplicate only throws ArgumentException on first access + /// (foreach/TryGetPropertyValue). + /// + private static readonly TestToolMock ByArgsWithDuplicateKey = new() + { + FunctionName = "get_work_order", + ArgsMatchJson = """{"woNum":"B1","woNum":"B2"}""", + ResultContent = "should-never-match" + }; + + [Fact] + public void Matches_on_function_name_when_nothing_more_specific_is_configured() + { + var hit = ToolMockMatcher.Match([ByName], "get_work_order", null, 0); + Assert.Equal("generic", hit!.ResultContent); + } + + [Fact] + public void An_args_subset_match_beats_a_name_only_mock() + { + var hit = ToolMockMatcher.Match([ByName, ByArgs], "get_work_order", + """{"woNum":"B9897413","includeNotes":true}""", 0); + Assert.Equal("specific", hit!.ResultContent); + } + + [Fact] + public void An_args_mock_does_not_match_different_arguments() + { + var hit = ToolMockMatcher.Match([ByName, ByArgs], "get_work_order", """{"woNum":"OTHER"}""", 0); + Assert.Equal("generic", hit!.ResultContent); + } + + [Fact] + public void Call_index_selects_a_different_mock_for_a_later_call() + { + Assert.Equal("generic", ToolMockMatcher.Match([ByName, ByOrdinal], "get_work_order", null, 0)!.ResultContent); + Assert.Equal("second-call", ToolMockMatcher.Match([ByName, ByOrdinal], "get_work_order", null, 1)!.ResultContent); + } + + [Fact] + public void Returns_null_when_the_function_has_no_mock_at_all() + { + Assert.Null(ToolMockMatcher.Match([ByName], "send_text_message", null, 0)); + } + + [Fact] + public void Malformed_argument_json_falls_back_to_the_name_only_mock_instead_of_throwing() + { + // The arguments come from model output and may not be valid JSON. This has to degrade + // gracefully rather than blowing the whole case up into an Error. + var hit = ToolMockMatcher.Match([ByName, ByArgs], "get_work_order", "{not json", 0); + Assert.Equal("generic", hit!.ResultContent); + } + + [Fact] + public void Duplicate_key_in_the_actual_arguments_falls_back_to_the_name_only_mock_instead_of_throwing() + { + // Duplicate top-level keys in the model's own arguments: JsonNode.Parse does not object, + // but actual.TryGetPropertyValue inside IsSubset throws ArgumentException the first time it + // touches actual's dictionary. This has to fall back to the function-name-only mock rather + // than blowing the whole case up into an infrastructure Error. + var hit = ToolMockMatcher.Match([ByName, ByArgs], "get_work_order", + """{"woNum":"B9897413","woNum":"DUPLICATE"}""", 0); + Assert.Equal("generic", hit!.ResultContent); + } + + [Fact] + public void Duplicate_key_in_the_mocks_own_args_match_json_falls_back_to_the_name_only_mock_instead_of_throwing() + { + // The mirror case: the duplicate key is in the ArgsMatchJson the test author wrote, and the + // throw happens where IsSubset's foreach first touches expected's dictionary. + var hit = ToolMockMatcher.Match([ByName, ByArgsWithDuplicateKey], "get_work_order", + """{"woNum":"B1"}""", 0); + Assert.Equal("generic", hit!.ResultContent); + } +} diff --git a/tests/BotSharp.Core.UnitTests/BotSharp.Core.UnitTests.csproj b/tests/BotSharp.Core.UnitTests/BotSharp.Core.UnitTests.csproj index 85eef91b0..4dd287f90 100644 --- a/tests/BotSharp.Core.UnitTests/BotSharp.Core.UnitTests.csproj +++ b/tests/BotSharp.Core.UnitTests/BotSharp.Core.UnitTests.csproj @@ -10,6 +10,7 @@ + runtime; build; native; contentfiles; analyzers; buildtransitive @@ -27,6 +28,8 @@ + + diff --git a/tests/BotSharp.Core.UnitTests/Routing/FunctionExecutorFactoryTests.cs b/tests/BotSharp.Core.UnitTests/Routing/FunctionExecutorFactoryTests.cs new file mode 100644 index 000000000..3863e96e4 --- /dev/null +++ b/tests/BotSharp.Core.UnitTests/Routing/FunctionExecutorFactoryTests.cs @@ -0,0 +1,141 @@ +using BotSharp.Abstraction.Agents.Models; +using BotSharp.Abstraction.Conversations.Models; +using BotSharp.Abstraction.Functions; +using BotSharp.Abstraction.Routing.Executor; +using BotSharp.Core.Routing.Executor; +using Microsoft.Extensions.DependencyInjection; +using Xunit; + +namespace BotSharp.Core.UnitTests.Routing; + +/// +/// This factory is the one place in the repo that decides who executes a given function name, and +/// therefore the only place a tool can be swapped for a fake. It used to be internal static, so +/// nothing outside could take part in resolution -- and the test-set feature has to intercept real +/// tools inside a test conversation, or running the regression suite once really does send emails +/// and really does create work orders. +/// +/// These tests also pin compatibility: with no provider registered, resolution must match the +/// pre-change behaviour exactly. That is the premise on which touching this core path was +/// acceptable at all, and the one way it could introduce a silent regression. +/// +public class FunctionExecutorFactoryTests +{ + private sealed class StubCallback(string name) : IFunctionCallback + { + public string Name => name; + public Task Execute(RoleDialogModel message) => Task.FromResult(true); + } + + private sealed class StubExecutor : IFunctionExecutor + { + public Task ExecuteAsync(RoleDialogModel message) => Task.FromResult(true); + public Task GetIndicatorAsync(RoleDialogModel message) => Task.FromResult(string.Empty); + } + + private sealed class StubProvider(int order, string? claims) : IFunctionExecutorProvider + { + public int Order => order; + public IFunctionExecutor? TryResolve(string functionName, Agent agent) + => functionName == claims ? Executor : null; + public StubExecutor Executor { get; } = new(); + } + + /// + /// Mirrors BotSharp.Core.Rules.ToolCallActionTests.NullIntolerantProvider: a case-insensitive + /// Dictionary<string,...> lookup, the shape a real mock/blocking provider plausibly takes. + /// Dictionary.ContainsKey/TryGetValue throw ArgumentNullException on a null key even for a + /// read-only lookup, so this proves the factory's own guard -- not just a lucky provider + /// implementation -- is what keeps a null/blank functionName from reaching TryResolve at all. + /// + private sealed class NullIntolerantProvider : IFunctionExecutorProvider + { + private static readonly Dictionary _mockedNames = + new(StringComparer.OrdinalIgnoreCase) { ["create_work_order"] = true }; + + public int Order => 0; + public bool WasAsked { get; private set; } + + public IFunctionExecutor? TryResolve(string functionName, Agent agent) + { + WasAsked = true; + return _mockedNames.ContainsKey(functionName) ? throw new InvalidOperationException("unreachable") : null; + } + } + + private static IFunctionExecutorFactory BuildFactory( + IEnumerable callbacks, + IEnumerable providers) + { + var services = new ServiceCollection(); + foreach (var cb in callbacks) services.AddSingleton(cb); + foreach (var p in providers) services.AddSingleton(p); + var sp = services.BuildServiceProvider(); + return new FunctionExecutorFactory(sp); + } + + [Fact] + public void Provider_takes_precedence_over_a_registered_callback() + { + var provider = new StubProvider(0, "create_work_order"); + var factory = BuildFactory([new StubCallback("create_work_order")], [provider]); + + var executor = factory.Create("create_work_order", new Agent()); + + Assert.Same(provider.Executor, executor); + } + + [Fact] + public void Falls_back_to_the_registered_callback_when_no_provider_claims_it() + { + // Compatibility guard: this was the only behaviour before the change, and must survive it. + var factory = BuildFactory([new StubCallback("create_work_order")], []); + + var executor = factory.Create("create_work_order", new Agent()); + + Assert.IsType(executor); + } + + [Fact] + public void Providers_are_asked_in_ascending_order_and_the_first_claim_wins() + { + var early = new StubProvider(-100, "create_work_order"); + var late = new StubProvider(100, "create_work_order"); + // Registered late-first on purpose, so a passing test proves the ordering is real rather + // than an accident of registration order. + var factory = BuildFactory([], [late, early]); + + var executor = factory.Create("create_work_order", new Agent()); + + Assert.Same(early.Executor, executor); + } + + [Fact] + public void Returns_null_when_nobody_can_execute_the_function() + { + var factory = BuildFactory([], []); + + Assert.Null(factory.Create("no_such_function", new Agent())); + } + + /// + /// The factory is documented as the single trusted seam every function-call path must go + /// through -- it cannot depend on every current and future caller pre-validating for it. + /// RoutingService.InvokeFunction has no guard of its own before calling Create(name, ...), so + /// a null/blank name is a real, reachable input here, not a hypothetical. + /// + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + public void Returns_null_for_a_null_or_blank_function_name_without_asking_any_provider(string? functionName) + { + var provider = new NullIntolerantProvider(); + var factory = BuildFactory([], [provider]); + + var executor = factory.Create(functionName!, new Agent()); + + Assert.Null(executor); + Assert.False(provider.WasAsked); + } +} diff --git a/tests/BotSharp.Core.UnitTests/Rules/ToolCallActionTests.cs b/tests/BotSharp.Core.UnitTests/Rules/ToolCallActionTests.cs new file mode 100644 index 000000000..4c94918a3 --- /dev/null +++ b/tests/BotSharp.Core.UnitTests/Rules/ToolCallActionTests.cs @@ -0,0 +1,161 @@ +using BotSharp.Abstraction.Agents.Models; +using BotSharp.Abstraction.Conversations.Models; +using BotSharp.Abstraction.Functions; +using BotSharp.Abstraction.Routing.Executor; +using BotSharp.Abstraction.Rules; +using BotSharp.Abstraction.Rules.Models; +using BotSharp.Core.Rules.Actions; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using Xunit; + +namespace BotSharp.Core.UnitTests.Rules; + +/// +/// The rule engine used to be the only function-execution path that did not go through +/// FunctionExecutorFactory. The test set depends on every tool call being interceptable, and +/// missing this path means rule-triggered tools execute for real during a test. +/// +/// The second test pins the thing this change could most easily have broken quietly: the original +/// matched names with IsEqualTo, which is case-insensitive. Switching to the factory's +/// case-sensitive comparison would turn a rule whose configured casing differs from "works" into +/// "no such function". +/// +public class ToolCallActionTests +{ + private sealed class StubCallback(string name) : IFunctionCallback + { + public string Name => name; + public bool Executed { get; private set; } + public Task Execute(RoleDialogModel message) + { + Executed = true; + message.Content = "real"; + return Task.FromResult(true); + } + } + + private sealed class ClaimingProvider : IFunctionExecutorProvider + { + public int Order => -1000; + public bool Claimed { get; private set; } + public IFunctionExecutor? TryResolve(string functionName, Agent agent) + { + Claimed = true; + return new MockExecutor(); + } + + private sealed class MockExecutor : IFunctionExecutor + { + public Task ExecuteAsync(RoleDialogModel message) + { + message.Content = "mocked"; + return Task.FromResult(true); + } + public Task GetIndicatorAsync(RoleDialogModel message) => Task.FromResult(string.Empty); + } + } + + /// + /// Minimal implementation for these tests. EntityType/EntityId have + /// no default implementation on the interface and must be supplied; Name is overridden because + /// ToolCallAction's failure path formats an error message with trigger.Name, and the interface's + /// default Name getter throws NotImplementedException. + /// + private sealed class StubTrigger : IRuleTrigger + { + public string EntityType { get; set; } = "test"; + public string EntityId { get; set; } = "test"; + public string Name => "stub_trigger"; + } + + /// + /// Mirrors a plausible shape for a future mock/blocking provider keyed by function name (plan + /// Task 5): a case-insensitive Dictionary lookup. Unlike HashSet<T>.Contains (which + /// tolerates a null reference-type item via its own internal guard, verified separately - + /// see the fix report), Dictionary<TKey,TValue> explicitly rejects a null key even for a + /// read-only lookup: both TryGetValue and ContainsKey throw ArgumentNullException. So a null + /// function_name reaching this provider crashes before it ever gets a chance to decide whether + /// it claims the function - exactly the failure mode the null guard in ToolCallAction prevents. + /// + private sealed class NullIntolerantProvider : IFunctionExecutorProvider + { + private static readonly Dictionary _mockedNames = + new(StringComparer.OrdinalIgnoreCase) { ["create_work_order"] = true }; + + public int Order => 0; + + public IFunctionExecutor? TryResolve(string functionName, Agent agent) + => _mockedNames.ContainsKey(functionName) ? throw new InvalidOperationException("unreachable") : null; + } + + private static (ToolCallAction action, IServiceProvider services) Build( + IEnumerable callbacks, + IEnumerable providers) + { + var services = new ServiceCollection(); + foreach (var cb in callbacks) services.AddSingleton(cb); + foreach (var p in providers) services.AddSingleton(p); + services.AddScoped(); + var sp = services.BuildServiceProvider(); + return (new ToolCallAction(sp, NullLogger.Instance), sp); + } + + private static RuleFlowContext ContextFor(string functionName) => new() + { + Parameters = new() { ["function_name"] = functionName } + }; + + [Fact] + public async Task A_provider_can_take_over_a_rule_triggered_tool_call() + { + var real = new StubCallback("create_work_order"); + var provider = new ClaimingProvider(); + var (action, _) = Build([real], [provider]); + + var result = await action.ExecuteAsync(new Agent { Name = "a" }, new StubTrigger(), ContextFor("create_work_order")); + + Assert.True(provider.Claimed); + Assert.False(real.Executed); // the real implementation must never be reached + Assert.True(result.Success); + } + + [Fact] + public async Task Function_name_matching_stays_case_insensitive() + { + var real = new StubCallback("create_work_order"); + var (action, _) = Build([real], []); + + var result = await action.ExecuteAsync(new Agent { Name = "a" }, new StubTrigger(), ContextFor("Create_Work_Order")); + + Assert.True(real.Executed); + Assert.True(result.Success); + } + + [Fact] + public async Task Reports_failure_when_the_function_cannot_be_resolved() + { + var (action, _) = Build([], []); + + var result = await action.ExecuteAsync(new Agent { Name = "a" }, new StubTrigger(), ContextFor("nope")); + + Assert.False(result.Success); + } + + /// + /// A missing function_name must fail gracefully instead of reaching the factory seam with a + /// null - the old IsEqualTo-based lookup was null-safe and simply matched nothing, and that + /// graceful-failure contract must survive routing execution through the factory. The + /// NullIntolerantProvider proves this isn't just an assertion on Success: without the guard, + /// this test throws ArgumentNullException instead of returning a result at all. + /// + [Fact] + public async Task Reports_failure_without_throwing_when_function_name_is_missing() + { + var (action, _) = Build([], [new NullIntolerantProvider()]); + + var result = await action.ExecuteAsync(new Agent { Name = "a" }, new StubTrigger(), new RuleFlowContext()); + + Assert.False(result.Success); + } +}