Show a dead check job as ended and let its set recover from the set view

src/FluentCMS.Admin/Components/Pages/ChangeSetView.razor

@@ -4,6 +4,8 @@
 @using FluentCMS.Core.Audit
 @using FluentCMS.Core.ChangeSets
 @using FluentCMS.Core.Desk
+@using FluentCMS.Core.Identity
+@using FluentCMS.Core.Jobs
 @inject IChangeSetDirectory Directory
 @inject IChangeSetLists Lists
 @inject IDeskQueues Queues
@@ -72,7 +74,7 @@
     <Card Class="mt-6" data-testid="change-set-review">
         <CardHeader>
             <CardTitle><h2>@L["Review"]</h2></CardTitle>
-            <CardDescription>@ReviewHint(set.State)</CardDescription>
+            <CardDescription>@(CheckJobEnded ? L["The check run ended without a result, so the set cannot be reviewed yet. Whoever opened it can run the checks again."] : ReviewHint(set.State))</CardDescription>
         </CardHeader>
         <CardContent Class="grid gap-4">
             @if (_message is not null)
@@ -92,12 +94,19 @@
                         <Button Type="submit" data-action="approve" aria-keyshortcuts="A">@L["Approve"] <Kbd Class="bg-primary-foreground/20 text-primary-foreground" aria-hidden="true">A</Kbd></Button>
                     </form>
                 }
-                @if (set.State == ChangeSetState.Open)
+                @if (OffersSubmit)
                 {
                     <form method="post" @onsubmit="SubmitAsync" @formname="submit">
                         <AntiforgeryToken />
                         <input type="hidden" name="Submit.Seen" value="@_seen" />
-                        <Button Type="submit" data-action="submit" aria-keyshortcuts="S">@L["Submit for checks"] <Kbd Class="bg-primary-foreground/20 text-primary-foreground" aria-hidden="true">S</Kbd></Button>
+                        @if (set.State == ChangeSetState.Open)
+                        {
+                            <Button Type="submit" data-action="submit" aria-keyshortcuts="S">@L["Submit for checks"] <Kbd Class="bg-primary-foreground/20 text-primary-foreground" aria-hidden="true">S</Kbd></Button>
+                        }
+                        else
+                        {
+                            <Button Type="submit" data-action="submit">@L["Run the checks again"]</Button>
+                        }
                     </form>
                 }
                 @if (set.State == ChangeSetState.Approved)
@@ -142,7 +151,7 @@
                 }
             </div>
             @* A form posted from a page shown before the set moved on must still find its handler, which then says the set moved on. *@
-            @if (set.State != ChangeSetState.Open)
+            @if (!OffersSubmit)
             {
                 <form method="post" @onsubmit="SubmitAsync" @formname="submit" hidden></form>
             }
@@ -218,14 +227,22 @@
                                 }
                             </span>
                         }
-                        else if (_checks?.Job is { } job)
+                        else if (CheckJobEnded)
+                        {
+                            <span data-testid="check-job-ended">@L["The check run ended without a result. Whoever opened the set can run the checks again."]</span>
+                        }
+                        else if (_checks?.Job is { State: JobState.Queued or JobState.Running } job)
                         {
                             @L["The checks are running: {0} of {1} items done.", job.Progress.Done, job.Progress.Total?.ToString(CultureInfo.CurrentCulture) ?? "?"]
                         }
-                        else
+                        else if (set.State == ChangeSetState.Open)
                         {
                             @L["The checks run when the set is submitted."]
                         }
+                        else
+                        {
+                            @L["No check results are stored for this set."]
+                        }
                     </CardDescription>
                 </CardHeader>
                 @if (_checks?.Run is { } results && (results.Problems.Count > 0 || results.Items.Any(item => item.Results.Count > 0)))
@@ -255,15 +272,16 @@
                                             </span>
                                             @foreach (var finding in result.Findings)
                                             {
+                                                @* Each part is isolated, so a right-to-left override in a judgement's text cannot reorder the others. *@
                                                 <span class="ps-2 text-muted-foreground" data-finding>
-                                                    @finding.Message
+                                                    <bdi>@finding.Message</bdi>
                                                     @if (finding.Pointer.Length > 0)
                                                     {
-                                                        <code class="ms-1 font-mono text-xs" dir="ltr">@finding.Pointer</code>
+                                                        <bdi><code class="ms-1 font-mono text-xs" dir="ltr">@finding.Pointer</code></bdi>
                                                     }
                                                     @if (finding.Excerpt is { } excerpt)
                                                     {
-                                                        <q class="ms-1">@excerpt</q>
+                                                        <q class="ms-1"><bdi>@excerpt</bdi></q>
                                                     }
                                                 </span>
                                             }
@@ -466,13 +484,17 @@
 
     private async Task SubmitAsync()
     {
-        await ActAsync(Submit, [ChangeSetState.Open], async () =>
+        // Running the checks again on a set whose job ended recovers it, which fails the set rather than submitting it.
+        var done = CheckJobEnded
+            ? L["The check run had ended without a result, so the set now shows its checks as failed."]
+            : L["Submitted. The checks run on the set before it is reviewed."];
+        await ActAsync(Submit, OffersSubmit && _set is { } set ? [set.State] : [], async () =>
         {
             var submitted = await Checks.SubmitAsync(Id);
             return submitted.IsSuccess
                 ? FluentCMS.Core.Results.Result<ChangeSet>.Success(submitted.Value.ChangeSet)
                 : FluentCMS.Core.Results.Result<ChangeSet>.Failure(submitted.Errors);
-        }, L["Submitted. The checks run on the set before it is reviewed."]);
+        }, done);
     }
 
     private async Task ApproveAsync()
@@ -526,6 +548,13 @@
         }
     }
 
+    // A person submits a set they opened, and runs its checks again when its check job ended without moving it; an agent submits its
+    // own sets by tool, and a person pressing S on one would split its work in two.
+    private bool OffersSubmit => _set is { Initiator: UserInitiator } set
+        && (set.State == ChangeSetState.Open || (set.State == ChangeSetState.ChecksRunning && CheckJobEnded));
+
+    private bool CheckJobEnded => _set?.State == ChangeSetState.ChecksRunning && _checks is { Run: null, Job.State: JobState.Succeeded or JobState.Failed or JobState.Dead };
+
     private static readonly ChangeSetState[] RejectableStates = [ChangeSetState.AwaitingReview, ChangeSetState.ChecksFailed, ChangeSetState.Approved];
 
     // A revert set that may still publish; a rejected one leaves room for another.

tests/FluentCMS.IntegrationTests/Browser/AccessibilityTests.cs

@@ -35,9 +35,10 @@ public async Task The_showcase_desk_and_set_view_have_no_wcag_21_aa_violations()
         {
             await session.Host.AsUserAsync(session.OwnerId, async users => await users.CreateAsync(new FluentCMS.Core.Identity.NewUser($"editor{user}@example.com", $"Editor {user}", "An0ther!Passw0rd")));
         }
+        var failing = await Desk.CheckedSets.FailingAsync(session.Host, session.OwnerId);
         await session.SignInWithKeyboardAsync();
 
-        string[] paths = ["/ui", "/", "/?queue=stale-facts", $"/sets/{set.Id}?queue=awaiting-me", "/security", "/media", $"/media/{asset}", "/content", "/content/product", "/content/product/new", "/content/supplier/new", "/content/kitchen-sink/new"];
+        string[] paths = ["/ui", "/", "/?queue=stale-facts", $"/sets/{set.Id}?queue=awaiting-me", $"/sets/{failing}", "/security", "/media", $"/media/{asset}", "/content", "/content/product", "/content/product/new", "/content/supplier/new", "/content/kitchen-sink/new"];
         var violations = new List<string>();
         foreach (var theme in new[] { "light", "dark" })
         {

tests/FluentCMS.IntegrationTests/Desk/CheckCardTests.cs

@@ -0,0 +1,108 @@
+using System.Text.RegularExpressions;
+using FluentCMS.Core.ChangeSets;
+using FluentCMS.Core.Schema;
+using FluentCMS.Infrastructure.Persistence;
+using Microsoft.EntityFrameworkCore;
+using Microsoft.Extensions.DependencyInjection;
+
+namespace FluentCMS.IntegrationTests.Desk;
+
+public sealed class CheckCardTests
+{
+    // A check job that ended without moving its set leaves the set in checks running; the person who opened it recovers it from the
+    // set view, which fails the set so it can be reopened.
+    [Theory]
+    [InlineData(DatabaseProviders.Sqlite)]
+    [PostgresData]
+    public async Task A_set_whose_check_job_ended_says_so_and_its_opener_recovers_it(string provider)
+    {
+        using var install = new TemporaryInstall();
+        await using var database = await TemporaryDatabase.CreateAsync(provider);
+        await using var host = await AdminHost.StartAsync(install, settings: database.Configuration);
+        var ownerId = await host.CreateFirstAdministratorAsync();
+        var type = await host.AsUserAsync<IContentTypeDirectory, ContentType>(ownerId, async types => (await types.CreateAsync(
+            new NewContentType("Brand", "brand", new ContentTypeDefinition([new FieldDefinition("name", "text")])), TestChangeSetTargets.ContentModel)).Value);
+        var setId = await CheckedSets.DeadJobAsync(host, ownerId, type);
+        await host.SignInAsync(AdminHost.Owner);
+
+        var page = await host.Client.GetStringAsync($"/sets/{setId}");
+        Assert.Contains("data-testid=\"check-job-ended\"", page);
+        Assert.DoesNotContain("The checks are running, so it cannot be reviewed yet.", page);
+        Assert.DoesNotContain("items done", page);
+        Assert.Contains("Run the checks again", page);
+
+        var seen = Regex.Match(page, "name=\"Submit.Seen\" value=\"([0-9a-f]+)\"").Groups[1].Value;
+        var recovered = await host.SubmitAsync($"/sets/{setId}", "submit", new Dictionary<string, string> { ["Submit.Seen"] = seen });
+
+        var after = await recovered.Content.ReadAsStringAsync();
+        Assert.Contains("data-state=\"checks-failed\"", after);
+        Assert.Contains("now shows its checks as failed", after);
+        Assert.DoesNotContain("Submitted. The checks run", after);
+        var set = await host.AsUserAsync<IChangeSetDirectory, ChangeSet>(ownerId, async sets => (await sets.GetAsync(setId)).Value);
+        Assert.Equal(ChangeSetState.ChecksFailed, set.State);
+    }
+
+    // An agent submits its own sets by tool: a person's post of Submit for one is refused, whatever the form claims.
+    [Theory]
+    [InlineData(DatabaseProviders.Sqlite)]
+    [PostgresData]
+    public async Task A_post_submitting_an_agents_open_set_is_refused(string provider)
+    {
+        using var install = new TemporaryInstall();
+        await using var database = await TemporaryDatabase.CreateAsync(provider);
+        await using var host = await AdminHost.StartAsync(install, settings: database.Configuration);
+        var ownerId = await host.CreateFirstAdministratorAsync();
+        var issued = await Mcp.McpAgents.IssueAsync(host, ownerId, Mcp.McpAgents.Credential(DateTimeOffset.UtcNow, "catalog-agent") with
+        {
+            Scope = FluentCMS.Core.Agents.AgentScope.Create(
+                [FluentCMS.Core.Sites.Site.Default.Id], null, null, [FluentCMS.Core.Agents.AgentOperation.Read, FluentCMS.Core.Agents.AgentOperation.Create]).Value,
+        });
+        Guid setId;
+        await using (var client = await Mcp.McpAgents.ConnectOverHttpAsync(host, issued.Key))
+        {
+            var opened = await client.CallToolAsync("open_change_set", new Dictionary<string, object?> { ["rationale"] = "Add the brand type." });
+            setId = Guid.Parse((string)System.Text.Json.Nodes.JsonNode.Parse(((ModelContextProtocol.Protocol.TextContentBlock)opened.Content.Single()).Text)!["changeSet"]!["id"]!);
+        }
+
+        await host.SignInAsync(AdminHost.Owner);
+        var page = await host.Client.GetStringAsync($"/sets/{setId}");
+        Assert.DoesNotContain("data-action=\"submit\"", page);
+        var seen = await LastMoveAsync(host, setId);
+        var refused = await host.SubmitAsync($"/sets/{setId}", "submit", new Dictionary<string, string> { ["Submit.Seen"] = seen });
+
+        Assert.Contains("data-testid=\"review-refusal\"", await refused.Content.ReadAsStringAsync());
+        var set = await host.AsUserAsync<IChangeSetDirectory, ChangeSet>(ownerId, async sets => (await sets.GetAsync(setId)).Value);
+        Assert.Equal(ChangeSetState.Open, set.State);
+    }
+
+    [Theory]
+    [InlineData(DatabaseProviders.Sqlite)]
+    [PostgresData]
+    public async Task A_failing_run_shows_its_findings_and_each_severity(string provider)
+    {
+        using var install = new TemporaryInstall();
+        await using var database = await TemporaryDatabase.CreateAsync(provider);
+        await using var host = await AdminHost.StartAsync(install, settings: database.Configuration);
+        var ownerId = await host.CreateFirstAdministratorAsync();
+        var setId = await CheckedSets.FailingAsync(host, ownerId);
+        await host.SignInAsync(AdminHost.Owner);
+
+        var page = await host.Client.GetStringAsync($"/sets/{setId}");
+
+        Assert.Contains("data-state=\"checks-failed\"", page);
+        Assert.Contains("data-check=\"no-superlatives\" data-status=\"fail\"", page);
+        Assert.Contains("blocks", page[page.IndexOf("data-check=\"no-superlatives\"", StringComparison.Ordinal)..]);
+        var finding = page[page.IndexOf("data-finding", StringComparison.Ordinal)..];
+        Assert.StartsWith("data-finding><bdi>", finding.Replace(" ", string.Empty).Replace("\n", string.Empty).Replace("\r", string.Empty));
+    }
+
+    private static async Task<string> LastMoveAsync(AdminHost host, Guid id)
+    {
+        await using var scope = host.App.Services.CreateAsyncScope();
+        var rows = await scope.ServiceProvider.GetRequiredService<FluentCmsDbContext>().AuditEvents
+            .AsNoTracking()
+            .Where(row => row.SubjectType == "change_set" && row.SubjectId == id)
+            .ToListAsync();
+        return rows.OrderBy(row => row.Timestamp).Last(row => ChangeSetMoves.Operations.Contains(row.Operation)).Id.ToString("N");
+    }
+}

tests/FluentCMS.IntegrationTests/Desk/CheckedSets.cs

@@ -0,0 +1,81 @@
+using System.Text.Json;
+using FluentCMS.Core.ChangeSets;
+using FluentCMS.Core.Checks;
+using FluentCMS.Core.Content;
+using FluentCMS.Core.Jobs;
+using FluentCMS.Core.Schema;
+using FluentCMS.Infrastructure.Jobs;
+using FluentCMS.Infrastructure.Persistence;
+using Microsoft.Extensions.DependencyInjection;
+
+namespace FluentCMS.IntegrationTests.Desk;
+
+// Sets in the states the checks card shows, made through the code that makes them.
+internal static class CheckedSets
+{
+    // A product page whose headline says "best in class", submitted while a terms check against it is active: its run fails.
+    public static async Task<Guid> FailingAsync(AdminHost host, Guid ownerId)
+    {
+        await host.AsUserAsync<IContentTypeDirectory, ContentType>(ownerId, async types => (await types.CreateAsync(
+            new NewContentType("Product page", "product-page", new ContentTypeDefinition([new FieldDefinition("headline", "text")])),
+            TestChangeSetTargets.ContentModel)).Value);
+        await host.AsUserAsync<ICheckDirectory, Check>(ownerId, async checks =>
+        {
+            var created = (await checks.CreateAsync(new NewCheck("no-superlatives", new CheckDefinition(
+                "terms",
+                "Headlines make no claims they cannot prove.",
+                Json("""{"terms": ["best in class"]}"""),
+                [
+                    new CheckExample("passes", CheckVerdict.Pass, Json("""{"headline": "A sturdy kettle"}"""), contentType: "product-page"),
+                    new CheckExample("fails", CheckVerdict.Fail, Json("""{"headline": "The best in class kettle"}"""), contentType: "product-page"),
+                ],
+                scope: new CheckScope(["product-page"], null))))).Value;
+            return (await checks.ActivateAsync(created.Name, created.Version)).Value;
+        });
+        var entry = await host.AsUserAsync<IEntryDirectory, Entry>(ownerId, async entries =>
+            (await entries.CreateAsync(new NewEntry("product-page", Json("""{"headline": "The best in class kettle"}""")))).Value);
+
+        var set = await host.AsUserAsync<IChangeSetDirectory, ChangeSet>(ownerId, async directory =>
+        {
+            var opened = (await directory.OpenAsync(new NewChangeSet("Add the kettle page.", []))).Value;
+            return (await directory.AddOperationAsync(opened.Id, ChangeOperation.Create(OperationKind.Create, ItemKind.Entry, entry.Id, null, 1).Value)).Value;
+        });
+        return await host.AsUserAsync<IChangeSetChecks, Guid>(ownerId, async checks => (await checks.SubmitAsync(set.Id)).Value.ChangeSet.Id);
+    }
+
+    // A set a person submitted whose check job ended without moving it, as when every attempt failed.
+    public static async Task<Guid> DeadJobAsync(AdminHost host, Guid ownerId, ContentType type)
+    {
+        var set = await host.AsUserAsync<IChangeSetDirectory, ChangeSet>(ownerId, async directory =>
+        {
+            var opened = (await directory.OpenAsync(new NewChangeSet("Add the brand type.", []))).Value;
+            await directory.AddOperationAsync(opened.Id, ChangeOperation.Create(OperationKind.Create, ItemKind.Schema, type.Id, null, 1).Value);
+            return (await directory.TransitionAsync(opened.Id, ChangeSetState.ChecksRunning)).Value;
+        });
+
+        await using var scope = host.App.Services.CreateAsyncScope();
+        var context = scope.ServiceProvider.GetRequiredService<FluentCmsDbContext>();
+        var now = DateTimeOffset.UtcNow;
+        context.Add(new JobRow
+        {
+            Id = Guid.CreateVersion7(),
+            Type = "checks.run",
+            Payload = JsonSerializer.Serialize(new { changeSetId = set.Id }),
+            SubjectType = "change_set",
+            SubjectId = set.Id,
+            State = JobState.Dead,
+            Attempts = 6,
+            MaxAttempts = 6,
+            CreatedAt = now,
+            ScheduledAt = now,
+            FinishedAt = now,
+        });
+        await context.SaveChangesAsync();
+        return set.Id;
+    }
+
+    private static JsonElement Json(string json)
+    {
+        return JsonSerializer.Deserialize<JsonElement>(json);
+    }
+}