diff --git a/src/FluentCMS.Core/Builtin/Tools/DeskTools.cs b/src/FluentCMS.Core/Builtin/Tools/DeskTools.cs index 0b8c3f9cc..eb73a543c 100644 --- a/src/FluentCMS.Core/Builtin/Tools/DeskTools.cs +++ b/src/FluentCMS.Core/Builtin/Tools/DeskTools.cs @@ -31,16 +31,18 @@ public sealed class DeskTools : IToolProvider "get_desk_counts", "Get the review desk's counts", "Returns how many items wait in each queue of the review desk: sets awaiting review, sets whose checks failed, stale facts, " - + "and sets published or reverted in the last seven days. Call list_desk_queue to see the items of one queue.", + + "and sets published or reverted in the last seven days. An agent counts only the change sets it opened, so other " + + "people's and agents' sets may wait too. Call list_desk_queue to see the items of one queue.", ToolSchemas.Object(), readOnly: true, CountsAsync), new AgentTool( "list_desk_queue", "List a review desk queue", - "Lists one queue of the review desk, as reviewers see it: awaiting_review and checks_failed put the set that has waited " - + "longest first; recently_published and reverted cover the last seven days, newest first; stale_facts lists the facts " - + "that are stale, the longest stale first. Read a set with get_change_set or get_change_set_review.", + "Lists one queue of the review desk, holding only the change sets this agent opened: awaiting_review and checks_failed " + + "put the set that has waited longest first; recently_published and reverted cover the last seven days, newest first; " + + "stale_facts lists the stale facts of the types this agent may read, the longest stale first. Read a set with " + + "get_change_set or get_change_set_review.", ToolSchemas.Object( new ToolArgument( "queue", @@ -59,9 +61,9 @@ public sealed class DeskTools : IToolProvider new AgentTool( "get_change_set_review", "Get a change set's review status", - "Returns where a change set stands in review: its state and since when, the reason if a person rejected it, the set it " - + "reverts, and every move it made, in order, with who made it and when. Use it to learn whether your set was approved, " - + "published or rejected and why; a rejected set's reason says what to change in a new set.", + "Returns where a change set this agent opened stands in review: its state and since when, the reason if a person " + + "rejected it, the set it reverts, and every move it made, in order, with who made it and when. Use it to learn whether " + + "your set was approved, published or rejected and why; a rejected set's reason says what to change in a new set.", ToolSchemas.Object(ToolSchemas.Uuid("id", "The change set's id.")), readOnly: true, ReviewAsync), @@ -139,46 +141,58 @@ private static async Task ReviewAsync(ToolInvocation call, Cancellat return ToolResult.Failure(errors); } - var read = await call.Services.GetRequiredService().GetAsync(id!.Value, cancellationToken); - if (!read.IsSuccess) + // The set and its moves are two reads. A decision that commits between them leaves the last move ahead of the set, so both + // are read again until they agree, and the answer is one moment's. + ChangeSet set; + List moves; + var attempts = 0; + do { - return ToolResult.Failure(read.Errors, "A change set's id comes back from the call that opened it, or from list_desk_queue."); - } - - var set = read.Value; - var history = call.Services.GetRequiredService().Read(new AuditQuery { Subject = new AuditSubject("change_set", set.Id) }); - var moves = new JsonArray(); - DateTimeOffset? since = null; - if (history.IsSuccess) - { - await foreach (var auditEvent in history.Value.WithCancellation(cancellationToken)) + var read = await call.Services.GetRequiredService().GetAsync(id!.Value, cancellationToken); + if (!read.IsSuccess) { - if (!ChangeSetMoves.Operations.Contains(auditEvent.Operation)) - { - continue; - } - - since = auditEvent.Timestamp; - moves.Add(new JsonObject - { - ["move"] = auditEvent.Operation["change_set.".Length..], - ["by"] = Initiator(auditEvent.Initiator), - ["at"] = auditEvent.Timestamp.ToUniversalTime().ToString("O"), - }); + return ToolResult.Failure(read.Errors, "A change set's id comes back from the call that opened it, or from list_desk_queue."); } + + set = read.Value; + moves = await MovesAsync(call.Services, set.Id, cancellationToken); } + while (moves.Count > 0 && ChangeSetMoves.StateAfter(moves[^1].Operation) != set.State && ++attempts < 3); return ToolResult.Success(new JsonObject { ["id"] = set.Id.ToString(), ["state"] = ToolJson.Name(set.State), - ["stateSince"] = since?.ToUniversalTime().ToString("O"), + ["stateSince"] = moves.Count > 0 ? moves[^1].Timestamp.ToUniversalTime().ToString("O") : null, ["rejectionReason"] = set.RejectionReason, ["revertsChangeSetId"] = set.RevertsChangeSetId?.ToString(), - ["moves"] = moves, + ["moves"] = new JsonArray([.. moves.Select(move => (JsonNode)new JsonObject + { + ["move"] = move.Operation["change_set.".Length..], + ["by"] = Initiator(move.Initiator), + ["at"] = move.Timestamp.ToUniversalTime().ToString("O"), + })]), }); } + private static async Task> MovesAsync(IServiceProvider services, Guid changeSetId, CancellationToken cancellationToken) + { + var moves = new List(); + var history = services.GetRequiredService().Read(new AuditQuery { Subject = new AuditSubject("change_set", changeSetId) }); + if (history.IsSuccess) + { + await foreach (var auditEvent in history.Value.WithCancellation(cancellationToken)) + { + if (ChangeSetMoves.Operations.Contains(auditEvent.Operation)) + { + moves.Add(auditEvent); + } + } + } + + return moves; + } + private static JsonObject Summary(ChangeSetSummary set) { return new JsonObject diff --git a/src/FluentCMS.Core/ChangeSets/ChangeSetMoves.cs b/src/FluentCMS.Core/ChangeSets/ChangeSetMoves.cs index e5ccd116e..0ead1b408 100644 --- a/src/FluentCMS.Core/ChangeSets/ChangeSetMoves.cs +++ b/src/FluentCMS.Core/ChangeSets/ChangeSetMoves.cs @@ -19,4 +19,18 @@ public static class ChangeSetMoves "change_set.rejected", "change_set.reverted", ]; + + // The state a move leaves its set in, or null for an operation that is not a move. + public static ChangeSetState? StateAfter(string operation) => operation switch + { + Opened or "change_set.reopened" => ChangeSetState.Open, + "change_set.submitted" => ChangeSetState.ChecksRunning, + "change_set.checks_failed" => ChangeSetState.ChecksFailed, + "change_set.awaiting_review" => ChangeSetState.AwaitingReview, + "change_set.approved" => ChangeSetState.Approved, + "change_set.published" => ChangeSetState.Published, + "change_set.rejected" => ChangeSetState.Rejected, + "change_set.reverted" => ChangeSetState.Reverted, + _ => null, + }; } diff --git a/tests/FluentCMS.IntegrationTests/Desk/DeskQueueTests.cs b/tests/FluentCMS.IntegrationTests/Desk/DeskQueueTests.cs index ee5615aff..2acc796bb 100644 --- a/tests/FluentCMS.IntegrationTests/Desk/DeskQueueTests.cs +++ b/tests/FluentCMS.IntegrationTests/Desk/DeskQueueTests.cs @@ -186,6 +186,73 @@ public async Task The_stale_facts_queue_lists_expired_and_unverified_facts_the_l Assert.Equal((unverified, StaleReason.Unverified, 2), (Assert.Single(second.Items).EntityId, second.Items[0].Reason, second.Total)); } + // The read sides date and describe sets by these move names, so each move the directory makes must append one of them. + [Theory] + [InlineData(DatabaseProviders.Sqlite)] + [PostgresData] + public async Task Every_move_the_directory_makes_appends_a_known_move_that_names_the_state_it_leaves(string provider) + { + await using var scenario = await ChangeSetScenario.CreateAsync(provider); + var seen = new HashSet(StringComparer.Ordinal); + var set = await scenario.OpenAsync(Create(await scenario.ContentTypeAsync("product"))); + await ExpectMoveAsync(scenario, set.Id, ChangeSetState.Open, seen, before: []); + + async Task MoveAsync(Guid id, ChangeSetState state, Func>> move, bool asPerson = true) + { + var before = await MoveEventsAsync(scenario, id); + var moved = asPerson ? await scenario.AsPersonAsync(move) : await scenario.RunAsync(move); + Assert.True(moved.IsSuccess, string.Join("; ", moved.Errors.Select(error => error.Message))); + await ExpectMoveAsync(scenario, id, state, seen, before); + } + + await MoveAsync(set.Id, ChangeSetState.ChecksRunning, directory => directory.TransitionAsync(set.Id, ChangeSetState.ChecksRunning), asPerson: false); + await MoveAsync(set.Id, ChangeSetState.ChecksFailed, directory => directory.TransitionAsync(set.Id, ChangeSetState.ChecksFailed)); + await MoveAsync(set.Id, ChangeSetState.Open, directory => directory.TransitionAsync(set.Id, ChangeSetState.Open), asPerson: false); + await MoveAsync(set.Id, ChangeSetState.ChecksRunning, directory => directory.TransitionAsync(set.Id, ChangeSetState.ChecksRunning), asPerson: false); + await MoveAsync(set.Id, ChangeSetState.AwaitingReview, directory => directory.TransitionAsync(set.Id, ChangeSetState.AwaitingReview)); + await MoveAsync(set.Id, ChangeSetState.Approved, directory => directory.TransitionAsync(set.Id, ChangeSetState.Approved)); + await MoveAsync(set.Id, ChangeSetState.Published, directory => directory.PublishAsync(set.Id)); + + var revert = (await scenario.AsPersonAsync(directory => directory.RevertAsync(set.Id, "Undo it."))).Value; + await MoveAsync(revert.Id, ChangeSetState.ChecksRunning, directory => directory.TransitionAsync(revert.Id, ChangeSetState.ChecksRunning)); + await MoveAsync(revert.Id, ChangeSetState.AwaitingReview, directory => directory.TransitionAsync(revert.Id, ChangeSetState.AwaitingReview)); + await MoveAsync(revert.Id, ChangeSetState.Approved, directory => directory.TransitionAsync(revert.Id, ChangeSetState.Approved)); + var originalBefore = await MoveEventsAsync(scenario, set.Id); + await MoveAsync(revert.Id, ChangeSetState.Published, directory => directory.PublishAsync(revert.Id)); + await ExpectMoveAsync(scenario, set.Id, ChangeSetState.Reverted, seen, originalBefore); + + var rejected = await scenario.OpenAsync(Create(await scenario.ContentTypeAsync("category"))); + await MoveAsync(rejected.Id, ChangeSetState.ChecksRunning, directory => directory.TransitionAsync(rejected.Id, ChangeSetState.ChecksRunning), asPerson: false); + await MoveAsync(rejected.Id, ChangeSetState.AwaitingReview, directory => directory.TransitionAsync(rejected.Id, ChangeSetState.AwaitingReview)); + await MoveAsync(rejected.Id, ChangeSetState.Rejected, directory => directory.RejectAsync(rejected.Id, "Not needed.")); + + Assert.Equal(ChangeSetMoves.Operations.Order(StringComparer.Ordinal), seen.Order(StringComparer.Ordinal)); + } + + private static async Task ExpectMoveAsync(ChangeSetScenario scenario, Guid id, ChangeSetState state, HashSet seen, IReadOnlyCollection before) + { + var appended = (await scenario.QueryAsync(context => context.AuditEvents + .AsNoTracking() + .Where(row => row.SubjectType == "change_set" && row.SubjectId == id && row.Operation.StartsWith("change_set.")) + .ToListAsync())) + .Where(row => !before.Contains(row.Id) && !row.Operation.StartsWith("change_set.operation_", StringComparison.Ordinal)) + .ToList(); + + var move = Assert.Single(appended); + Assert.Contains(move.Operation, ChangeSetMoves.Operations); + Assert.Equal(state, ChangeSetMoves.StateAfter(move.Operation)); + seen.Add(move.Operation); + } + + private static async Task> MoveEventsAsync(ChangeSetScenario scenario, Guid id) + { + return await scenario.QueryAsync(context => context.AuditEvents + .AsNoTracking() + .Where(row => row.SubjectType == "change_set" && row.SubjectId == id) + .Select(row => row.Id) + .ToListAsync()); + } + private static async Task AwaitingReviewAsync(ChangeSetScenario scenario, string slug) { var set = await scenario.OpenAsync(Create(await scenario.ContentTypeAsync(slug))); diff --git a/tests/FluentCMS.IntegrationTests/Mcp/ConnectedAgent.cs b/tests/FluentCMS.IntegrationTests/Mcp/ConnectedAgent.cs index 42bd6f97f..ac8b76cb9 100644 --- a/tests/FluentCMS.IntegrationTests/Mcp/ConnectedAgent.cs +++ b/tests/FluentCMS.IntegrationTests/Mcp/ConnectedAgent.cs @@ -18,7 +18,8 @@ internal sealed class ConnectedAgent( AdminHost host, Guid administrator, AgentCredential credential, - McpClient client) : IAsyncDisposable + McpClient client, + bool ownsHost = true) : IAsyncDisposable { public AgentCredential Credential { get; } = credential; @@ -101,12 +102,27 @@ public async Task QueryAsync(Func> query) return await query(scope.ServiceProvider.GetRequiredService()); } + // Another agent on the same host, with a credential of its own, for what one agent must not see of another's work. Disposing it + // leaves the host running. + public async Task AnotherAgentAsync(string displayName) + { + var issued = await McpAgents.IssueAsync(host, administrator, McpAgents.Credential(DateTimeOffset.UtcNow, displayName) with + { + Scope = AgentScope.Create([Site.Default.Id], null, null, [AgentOperation.Read, AgentOperation.Create, AgentOperation.Update]).Value, + }); + var other = await McpAgents.ConnectOverHttpAsync(host, issued.Key); + return new ConnectedAgent(install, database, host, administrator, issued.Credential, other, ownsHost: false); + } + public async ValueTask DisposeAsync() { await client.DisposeAsync(); - await host.DisposeAsync(); - await database.DisposeAsync(); - install.Dispose(); + if (ownsHost) + { + await host.DisposeAsync(); + await database.DisposeAsync(); + install.Dispose(); + } } private static string Text(CallToolResult result) diff --git a/tests/FluentCMS.IntegrationTests/Mcp/DeskToolsTests.cs b/tests/FluentCMS.IntegrationTests/Mcp/DeskToolsTests.cs index 27ef1147a..ed5a8b9a8 100644 --- a/tests/FluentCMS.IntegrationTests/Mcp/DeskToolsTests.cs +++ b/tests/FluentCMS.IntegrationTests/Mcp/DeskToolsTests.cs @@ -1,8 +1,12 @@ +using System.Text.Json; using System.Text.Json.Nodes; using FluentCMS.Core.Agents; using FluentCMS.Core.ChangeSets; +using FluentCMS.Core.Content; +using FluentCMS.Core.Schema; using FluentCMS.Core.Tools; using FluentCMS.Infrastructure.Persistence; +using Microsoft.Extensions.DependencyInjection; namespace FluentCMS.IntegrationTests.Mcp; @@ -55,6 +59,8 @@ public async Task An_agent_reads_who_rejected_its_set_and_why_and_not_the_review await agent.AsPersonAsync(directory => directory.RejectAsync(set, "The type duplicates item.")); var someoneElses = await PersonsSetAsync(agent, "page"); + var events = await agent.QueryAsync(context => Task.FromResult(context.AuditEvents.Count())); + var review = await agent.CallAsync("get_change_set_review", new() { ["id"] = set.ToString() }); var refused = await agent.FailAsync("get_change_set_review", new() { ["id"] = someoneElses.ToString() }); @@ -65,6 +71,80 @@ public async Task An_agent_reads_who_rejected_its_set_and_why_and_not_the_review Assert.Equal(("user", "Test administrator"), ((string?)moves[3]!["by"]!["kind"], (string?)moves[3]!["by"]!["name"])); Assert.Equal((string?)moves[3]!["at"], (string?)review["stateSince"]); Assert.Equal(AgentPermissionErrorCodes.NotOwner, Code(refused)); + Assert.Equal(events, await agent.QueryAsync(context => Task.FromResult(context.AuditEvents.Count()))); + } + + // The credential id separates one agent from another; a person's sets are already left out by the initiator's kind. + [Theory] + [InlineData(DatabaseProviders.Sqlite)] + [PostgresData] + public async Task Another_agents_sets_are_not_counted_listed_or_readable(string provider) + { + await using var agent = await ConnectedAgent.StartAsync(provider); + await using var other = await agent.AnotherAgentAsync("other-agent"); + var mine = await SubmitAsync(agent, "product"); + var theirs = await SubmitAsync(other, "category"); + var theirsFailed = await SubmitAsync(other, "brand", submit: false); + await MoveAsync(agent, theirsFailed, ChangeSetState.ChecksRunning); + await MoveAsync(agent, theirsFailed, ChangeSetState.ChecksFailed); + + var counts = await agent.CallAsync("get_desk_counts", []); + var awaiting = await agent.CallAsync("list_desk_queue", new() { ["queue"] = "awaiting_review" }); + var failed = await agent.CallAsync("list_desk_queue", new() { ["queue"] = "checks_failed" }); + var refused = await agent.FailAsync("get_change_set_review", new() { ["id"] = theirs.ToString() }); + var theirCounts = await other.CallAsync("get_desk_counts", []); + + Assert.Equal((1, 0), ((int)counts["counts"]!["awaiting_review"]!, (int)counts["counts"]!["checks_failed"]!)); + Assert.Equal([mine.ToString()], Ids(awaiting)); + Assert.Empty(Ids(failed)); + Assert.Equal(AgentPermissionErrorCodes.NotOwner, Code(refused)); + Assert.Equal((1, 1), ((int)theirCounts["counts"]!["awaiting_review"]!, (int)theirCounts["counts"]!["checks_failed"]!)); + } + + [Theory] + [InlineData(DatabaseProviders.Sqlite)] + [PostgresData] + public async Task Every_queue_lists_through_the_tool_in_its_order_and_window_and_no_call_appends_an_event(string provider) + { + var clock = new ManualClock(DateTimeOffset.UtcNow); + await using var agent = await ConnectedAgent.StartAsync(provider, configure: builder => builder.Services.AddSingleton(clock)); + var old = await PublishAsync(agent, await SubmitAsync(agent, "region")); + var oldRevert = await RevertAsync(agent, await PublishAsync(agent, await SubmitAsync(agent, "warehouse"))); + clock.Now = clock.Now.AddDays(8); + var first = await PublishAsync(agent, await SubmitAsync(agent, "product")); + clock.Now = clock.Now.AddMinutes(1); + var wrong = await RevertAsync(agent, await PublishAsync(agent, await SubmitAsync(agent, "category"))); + clock.Now = clock.Now.AddMinutes(1); + var alsoWrong = await RevertAsync(agent, await PublishAsync(agent, await SubmitAsync(agent, "colour"))); + clock.Now = clock.Now.AddMinutes(1); + var second = await PublishAsync(agent, await SubmitAsync(agent, "brand")); + var failedFirst = await FailedAsync(agent, "size"); + clock.Now = clock.Now.AddMinutes(1); + var failedLater = await FailedAsync(agent, "shape"); + var stale = await StaleFactsAsync(agent, 3); + var events = await agent.QueryAsync(context => Task.FromResult(context.AuditEvents.Count())); + + var counts = await agent.CallAsync("get_desk_counts", []); + var published = await agent.CallAsync("list_desk_queue", new() { ["queue"] = "recently_published" }); + var reverted = await agent.CallAsync("list_desk_queue", new() { ["queue"] = "reverted" }); + var failed = await agent.CallAsync("list_desk_queue", new() { ["queue"] = "checks_failed" }); + var staleFirst = await agent.CallAsync("list_desk_queue", new() { ["queue"] = "stale_facts", ["limit"] = 2 }); + var staleSecond = await agent.CallAsync("list_desk_queue", new() { ["queue"] = "stale_facts", ["limit"] = 2, ["page"] = 2 }); + await agent.CallAsync("get_change_set_review", new() { ["id"] = wrong.ToString() }); + + Assert.Equal( + (2, 2, 2, 3), + ((int)counts["counts"]!["recently_published"]!, (int)counts["counts"]!["reverted"]!, (int)counts["counts"]!["checks_failed"]!, (int)counts["counts"]!["stale_facts"]!)); + Assert.Equal([second.ToString(), first.ToString()], Ids(published)); + Assert.DoesNotContain(old.ToString(), Ids(published)); + Assert.Equal([alsoWrong.ToString(), wrong.ToString()], Ids(reverted)); + Assert.DoesNotContain(oldRevert.ToString(), Ids(reverted)); + Assert.Equal([failedFirst.ToString(), failedLater.ToString()], Ids(failed)); + Assert.Equal((3, 3), ((int)staleFirst["total"]!, (int)staleSecond["total"]!)); + Assert.Equal( + stale, + [.. staleFirst["facts"]!.AsArray().Concat(staleSecond["facts"]!.AsArray()).Select(fact => Guid.Parse((string)fact!["entityId"]!))]); + Assert.Equal(events, await agent.QueryAsync(context => Task.FromResult(context.AuditEvents.Count()))); } [Theory] @@ -123,6 +203,54 @@ private static async Task SubmitAsync(ConnectedAgent agent, string slug, b return Guid.Parse(id); } + // A person reverts the published set and publishes the revert, which moves the set to reverted. + private static async Task RevertAsync(ConnectedAgent agent, Guid id) + { + var revert = await agent.AsPersonAsync(directory => directory.RevertAsync(id, "It was a mistake.")); + await MoveAsync(agent, revert.Id, ChangeSetState.ChecksRunning); + await MoveAsync(agent, revert.Id, ChangeSetState.AwaitingReview); + await PublishAsync(agent, revert.Id); + return id; + } + + private static async Task FailedAsync(ConnectedAgent agent, string slug) + { + var id = await SubmitAsync(agent, slug, submit: false); + await MoveAsync(agent, id, ChangeSetState.ChecksRunning); + await MoveAsync(agent, id, ChangeSetState.ChecksFailed); + return id; + } + + // A person approves and publishes the set, as a reviewer would. + private static async Task PublishAsync(ConnectedAgent agent, Guid id) + { + await MoveAsync(agent, id, ChangeSetState.Approved); + await agent.AsPersonAsync(directory => directory.PublishAsync(id)); + return id; + } + + // Suppliers whose price expired a day apart, so the longest stale comes first; returns them in that order. + private static async Task> StaleFactsAsync(ConnectedAgent agent, int count) + { + await agent.AsPersonAsync(directory => directory.CreateAsync( + new NewContentType("Supplier", "supplier", new ContentTypeDefinition([new FieldDefinition("name", "text"), new FieldDefinition("price", "number")], isEntity: true)), + TestChangeSetTargets.ContentModel)); + var ids = new List(); + for (var day = 1; day <= count; day++) + { + const string sourced = """{"kind": "url", "reference": "https://example.com/catalog"}"""; + var fields = JsonSerializer.Deserialize($$""" + { + "name": {"value": "Supplier {{day}}", "source": {{sourced}}, "owner": null, "validFrom": null, "validUntil": null, "lastVerified": null}, + "price": {"value": 99, "source": {{sourced}}, "owner": null, "validFrom": null, "validUntil": "2020-01-0{{day}}T00:00:00Z", "lastVerified": null} + } + """); + ids.Add((await agent.AsPersonAsync(directory => directory.CreateAsync(new NewEntry("supplier", fields)))).Id); + } + + return ids; + } + // The checks and the reviewer move a set on; a person stands in for the engine here. private static Task MoveAsync(ConnectedAgent agent, Guid id, ChangeSetState state) {