Hold the agent's credential while counting its caps, and keep refusal events

CONTRIBUTING.md

@@ -63,7 +63,8 @@ Core checks every call against the agent's scope and caps, so a tool and a form
 - Reading a content type or its entries needs `read` on the type, and lists leave out types the agent may not read.
 - Writing an entry needs `create`, `update` or `delete` on its type, plus `read`. Every field the write changes must be allowed by the field list; a field sent back unchanged is not a change.
 - A type's definition covers every field, so creating, revising or proposing one needs a scope that does not narrow that type to some of its fields.
-- A call past the credential's calls per period does not run. It returns an error result that names the cap, and refused calls do not count toward it.
+- A call past the credential's calls per period does not run. It returns an error result that names the cap, and refused calls do not count toward it. A call is recorded as `running` before its tool runs and counts from then, so concurrent calls of one agent take turns at the cap, as its change set opens do at the daily cap.
+- An agent reads, adds to, submits and reopens only the change sets it opened. People read every set.
 
 On a fresh database the admin asks for the first administrator at `/setup`; every other admin page needs a signed-in user. Data Protection keeps the keys that protect sign-in cookies, anti-forgery tokens and stored secrets in `DataProtection__KeysDirectory` (default `data-protection-keys` under the content root). Put it on storage that survives restarts and upgrades and that only the host's account can read; losing it signs everyone out. Sign-in, setup and password reset posts are limited per client address by `SignInRateLimit__PermitLimit` and `SignInRateLimit__Window`, ten a minute by default. A password reset link issued on `/users` works once and expires after `PasswordReset__TokenLifespan`, one day by default. Each agent connects with its own key, issued on `/agents/new` with a scope, caps and an expiry. The key is shown once, in the response that issues it, and only its SHA-256 hash is stored. A lost key cannot be recovered: issue a new one and revoke the old one on `/agents`.
 

src/FluentCMS.Core/Agents/AgentPermissions.cs

@@ -88,6 +88,9 @@ public async Task<Result> CheckNewChangeSetAsync(CancellationToken cancellationT
             return Result.Success();
         }
 
+        // Called inside the unit of work that opens the set, so concurrent opens by one agent are counted one at a time.
+        await activity.HoldAsync(credential.Id, cancellationToken);
+
         var cap = credential.Caps.ChangeSetsPerDay;
         var opened = await activity.ChangeSetsOpenedSinceAsync(credential.Id, clock.GetUtcNow() - Day, cancellationToken);
         if (opened < cap)
@@ -105,6 +108,28 @@ await RecordAsync(
             + $"(change sets per day). Add the operation to a change set that is still open, or wait until fewer than {cap} were opened in the last 24 hours.");
     }
 
+    public async Task<Result> CheckChangeSetReadAsync(ChangeSet changeSet, CancellationToken cancellationToken = default)
+    {
+        ArgumentNullException.ThrowIfNull(changeSet);
+
+        if (currentInitiator.Current is SystemInitiator)
+        {
+            return Result.Success();
+        }
+
+        var agent = await AgentAsync(cancellationToken);
+        if (!agent.IsSuccess)
+        {
+            return Result.Failure(agent.Errors);
+        }
+
+        return agent.Value is { } credential && !(changeSet.Initiator is AgentInitiator owner && owner.CredentialId == credential.Id)
+            ? Result.Failure(
+                AgentPermissionErrorCodes.NotOwner,
+                $"Change set {changeSet.Id} is not one the credential '{credential.DisplayName}' opened; an agent reads its own change sets only.")
+            : Result.Success();
+    }
+
     public async Task<Result> CheckChangeSetSizeAsync(ChangeSet changeSet, int adding = 0, CancellationToken cancellationToken = default)
     {
         ArgumentNullException.ThrowIfNull(changeSet);
@@ -253,34 +278,48 @@ public async Task<Func<string, bool>> ReadableTypesAsync(CancellationToken cance
         return scope.AllowsOperation(AgentOperation.Read) ? scope.AllowsType : _ => false;
     }
 
-    public async Task<Result> CheckToolCallAsync(CancellationToken cancellationToken = default)
+    public async Task<Result> ClaimToolCallAsync(ToolCall call, CancellationToken cancellationToken = default)
     {
+        ArgumentNullException.ThrowIfNull(call);
+
         if (currentInitiator.Current is SystemInitiator system)
         {
             return SystemRefused(system, "call a tool");
         }
 
+        await using var scope = await unitOfWork.BeginAsync(cancellationToken);
+
         var agent = await AgentAsync(cancellationToken);
         if (!agent.IsSuccess)
         {
+            await activity.StartToolCallAsync(call with { Outcome = ToolCallOutcome.Refused }, cancellationToken);
+            await scope.CommitAsync(cancellationToken);
             return Result.Failure(agent.Errors);
         }
 
         if (agent.Value is not { } credential)
         {
+            await scope.CommitAsync(cancellationToken);
             return Result.Success();
         }
 
+        // One credential's calls take turns from here to the commit, so each count sees every call admitted before it.
+        await activity.HoldAsync(credential.Id, cancellationToken);
+
         var caps = credential.Caps;
         var made = await activity.ToolCallsSinceAsync(credential.Id, clock.GetUtcNow() - caps.CallPeriod, cancellationToken);
         if (made < caps.CallsPerPeriod)
         {
+            await activity.StartToolCallAsync(call with { Outcome = ToolCallOutcome.Running }, cancellationToken);
+            await scope.CommitAsync(cancellationToken);
             return Result.Success();
         }
 
-        await RecordAsync(
+        await activity.StartToolCallAsync(call with { Outcome = ToolCallOutcome.Refused }, cancellationToken);
+        await auditLog.AppendAsync(
             new NewAuditEvent("agent.calls_per_period_exceeded") { SubjectType = "agent_credential", SubjectId = credential.Id },
             cancellationToken);
+        await scope.CommitAsync(cancellationToken);
 
         return Result.Failure(
             AgentPermissionErrorCodes.CapExceeded,
@@ -390,10 +429,17 @@ private static Result Owns(AgentCredential credential, ChangeSet changeSet)
         return Result<AgentCredential?>.Success(credential.Value);
     }
 
-    // The refused write leaves nothing, so the event commits on its own, or with the caller's unit of work when there is one.
+    // The refused write leaves nothing, so the event commits on its own, or with the caller's unit of work when there is one. That
+    // unit of work usually rolls back because of the refusal, so the event is appended again once it has, in a unit of work of its own.
     private async Task RecordAsync(NewAuditEvent refusal, CancellationToken cancellationToken)
     {
         await using var scope = await unitOfWork.BeginAsync(cancellationToken);
+        unitOfWork.AfterRollback(async afterwards =>
+        {
+            await using var again = await unitOfWork.BeginAsync(afterwards);
+            await auditLog.AppendAsync(refusal, afterwards);
+            await again.CommitAsync(afterwards);
+        });
         await auditLog.AppendAsync(refusal, cancellationToken);
         await scope.CommitAsync(cancellationToken);
     }

src/FluentCMS.Core/Agents/IAgentActivity.cs

@@ -5,6 +5,13 @@ public interface IAgentActivity
 {
     Task<int> ChangeSetsOpenedSinceAsync(Guid credentialId, DateTimeOffset since, CancellationToken cancellationToken = default);
 
-    // Calls a refusal stopped are left out, so an agent over its cap is not kept there by its own retries.
+    // Calls a refusal stopped are left out, so an agent over its cap is not kept there by its own retries; running ones count.
     Task<int> ToolCallsSinceAsync(Guid credentialId, DateTimeOffset since, CancellationToken cancellationToken = default);
+
+    // Holds the credential until the current unit of work ends, so concurrent capped actions of one agent are counted one at a time.
+    // Called first in the unit of work, before the count it protects.
+    Task HoldAsync(Guid credentialId, CancellationToken cancellationToken = default);
+
+    // Adds the call to the current unit of work, so it is recorded in the same transaction as the count that let it run.
+    Task StartToolCallAsync(ToolCall call, CancellationToken cancellationToken = default);
 }

src/FluentCMS.Core/Agents/IAgentPermissions.cs

@@ -13,8 +13,13 @@ public interface IAgentPermissions
     // An agent submits its own set and reopens it after failed checks; every other move is the checks', routing's or a person's.
     Task<Result> CheckTransitionAsync(ChangeSet changeSet, ChangeSetState target, CancellationToken cancellationToken = default);
 
+    // Called inside the unit of work that opens the set: it holds the agent's credential until that unit of work ends, so concurrent
+    // opens by one agent are counted one at a time. Outside one it is only a pre-check, and the open counts again.
     Task<Result> CheckNewChangeSetAsync(CancellationToken cancellationToken = default);
 
+    // An agent reads the change sets it opened, as it writes only those; people and the system read every set.
+    Task<Result> CheckChangeSetReadAsync(ChangeSet changeSet, CancellationToken cancellationToken = default);
+
     // Called with the agent's own set as read for the write and the new operation added in memory, before anything is written,
     // or ahead of a write with the number of operations it will add.
     Task<Result> CheckChangeSetSizeAsync(ChangeSet changeSet, int adding = 0, CancellationToken cancellationToken = default);
@@ -33,6 +38,7 @@ Task<Result> CheckContentAsync(
     // For lists: every type for a person, the types in scope for an agent that may read, and none for a refused credential.
     Task<Func<string, bool>> ReadableTypesAsync(CancellationToken cancellationToken = default);
 
-    // One more tool call within the credential's calls per period (SEC-04), before the tool runs.
-    Task<Result> CheckToolCallAsync(CancellationToken cancellationToken = default);
+    // Records the call before its tool runs: as running when it is within the credential's calls per period (SEC-04), or as refused,
+    // with its security event, when it is not. The count and the record share one transaction that one credential's calls take turns on.
+    Task<Result> ClaimToolCallAsync(ToolCall call, CancellationToken cancellationToken = default);
 }

src/FluentCMS.Core/Agents/IToolCallLog.cs

@@ -5,4 +5,7 @@ public interface IToolCallLog
 {
     // Writes in a transaction of its own, never in a caller's unit of work, so a call whose own changes rolled back is still on record.
     Task AppendAsync(ToolCall call, CancellationToken cancellationToken = default);
+
+    // The outcome of a call recorded as running when it started, in a transaction of its own.
+    Task FinishAsync(Guid callId, ToolCallOutcome outcome, TimeSpan duration, CancellationToken cancellationToken = default);
 }

src/FluentCMS.Core/Agents/ToolCall.cs

@@ -13,6 +13,9 @@ public enum ToolCallOutcome
 
     // The tool never ran because the call was over the credential's calls per period; not counted toward that cap.
     Refused,
+
+    // The tool is running; the outcome replaces this when it ends. It counts toward the cap, so calls made at once take their places in turn.
+    Running,
 }
 
 // One tool call an agent made (MCP-07). Arguments and results are not kept: they hold content, and the audit log records every

src/FluentCMS.Core/Persistence/IUnitOfWork.cs

@@ -5,6 +5,10 @@ public interface IUnitOfWork
 {
     // Nested calls join the outer unit of work; only the outermost scope commits the transaction.
     Task<IUnitOfWorkScope> BeginAsync(CancellationToken cancellationToken = default);
+
+    // Runs the step once the outermost scope ends without committing, in a unit of work of its own, for what must outlive a
+    // rolled-back change, such as the security event of a refusal (SEC-08). Without an open scope, or once it commits, it never runs.
+    void AfterRollback(Func<CancellationToken, Task> step);
 }
 
 public interface IUnitOfWorkScope : IAsyncDisposable

src/FluentCMS.Infrastructure/Agents/AgentActivity.cs

@@ -32,4 +32,18 @@ public Task<int> ToolCallsSinceAsync(Guid credentialId, DateTimeOffset since, Ca
                 """)
             .CountAsync(cancellationToken);
     }
+
+    // A write to the credential's row: PostgreSQL holds the row lock and SQLite the write lock until the transaction ends.
+    public async Task HoldAsync(Guid credentialId, CancellationToken cancellationToken = default)
+    {
+        await context.Database.ExecuteSqlAsync($"""UPDATE "AgentCredentials" SET "DisplayName" = "DisplayName" WHERE "Id" = {credentialId}""", cancellationToken);
+    }
+
+    public Task StartToolCallAsync(ToolCall call, CancellationToken cancellationToken = default)
+    {
+        ArgumentNullException.ThrowIfNull(call);
+
+        context.ToolCalls.Add(ToolCallRow.From(call));
+        return Task.CompletedTask;
+    }
 }

src/FluentCMS.Infrastructure/Agents/ToolCallLog.cs

@@ -1,5 +1,6 @@
 using FluentCMS.Core.Agents;
 using FluentCMS.Infrastructure.Persistence;
+using Microsoft.EntityFrameworkCore;
 using Microsoft.Extensions.DependencyInjection;
 
 namespace FluentCMS.Infrastructure.Agents;
@@ -16,4 +17,12 @@ public async Task AppendAsync(ToolCall call, CancellationToken cancellationToken
         context.ToolCalls.Add(ToolCallRow.From(call));
         await context.SaveChangesAsync(cancellationToken);
     }
+
+    public async Task FinishAsync(Guid callId, ToolCallOutcome outcome, TimeSpan duration, CancellationToken cancellationToken = default)
+    {
+        await using var scope = scopes.CreateAsyncScope();
+        await scope.ServiceProvider.GetRequiredService<FluentCmsDbContext>().ToolCalls
+            .Where(row => row.Id == callId && row.Outcome == ToolCallOutcome.Running)
+            .ExecuteUpdateAsync(setters => setters.SetProperty(row => row.Outcome, outcome).SetProperty(row => row.Duration, duration), cancellationToken);
+    }
 }

src/FluentCMS.Infrastructure/Agents/ToolCallRow.cs

@@ -2,7 +2,8 @@
 
 namespace FluentCMS.Infrastructure.Agents;
 
-// The flat shape of a ToolCall in the ToolCalls table; rows are inserted and never changed.
+// The flat shape of a ToolCall in the ToolCalls table; a row is inserted when its call starts, and only a running call's outcome and
+// duration change after that.
 public sealed class ToolCallRow
 {
     public Guid Id { get; set; }

src/FluentCMS.Infrastructure/ChangeSets/ChangeSetDirectory.cs

@@ -13,7 +13,8 @@
 
 namespace FluentCMS.Infrastructure.ChangeSets;
 
-// Expected failures are found before the unit of work begins, so they leave a caller's outer unit of work able to commit.
+// Expected failures are found before the unit of work begins, so they leave a caller's outer unit of work able to commit. An agent's
+// daily cap is the exception: it is counted inside, so concurrent opens take turns, and its refusal rolls the caller's unit of work back.
 // Reads never track rows, and updates go straight to the database, so a long-lived context always sees what is stored.
 internal sealed class ChangeSetDirectory(
     FluentCmsDbContext context,
@@ -33,8 +34,13 @@ internal sealed class ChangeSetDirectory(
     public async Task<Result<ChangeSet>> GetAsync(Guid id, CancellationToken cancellationToken = default)
     {
         var stored = await ReadAsync(id, cancellationToken);
+        if (stored is null)
+        {
+            return NotFound(id);
+        }
 
-        return stored is null ? NotFound(id) : Result<ChangeSet>.Success(stored.ChangeSet);
+        var readable = await permissions.CheckChangeSetReadAsync(stored.ChangeSet, cancellationToken);
+        return readable.IsSuccess ? Result<ChangeSet>.Success(stored.ChangeSet) : Result<ChangeSet>.Failure(readable.Errors);
     }
 
     public async Task<Result<ChangeSetDiff>> DiffAsync(Guid id, CancellationToken cancellationToken = default)
@@ -45,6 +51,12 @@ public async Task<Result<ChangeSetDiff>> DiffAsync(Guid id, CancellationToken ca
             return Result<ChangeSetDiff>.Failure(NotFound(id).Errors);
         }
 
+        var readable = await permissions.CheckChangeSetReadAsync(stored.ChangeSet, cancellationToken);
+        if (!readable.IsSuccess)
+        {
+            return Result<ChangeSetDiff>.Failure(readable.Errors);
+        }
+
         var set = stored.ChangeSet;
         var definitions = await DefinitionsAsync(set.Operations, cancellationToken);
         var entries = await EntryDiffsAsync(set, cancellationToken);
@@ -85,13 +97,14 @@ public async Task<Result<ChangeSet>> OpenAsync(NewChangeSet changeSet, Cancellat
             return opened;
         }
 
+        // Counted inside the unit of work, after holding the agent's credential, so concurrent opens cannot all pass the cap.
+        await using var scope = await unitOfWork.BeginAsync(cancellationToken);
         var allowed = await permissions.CheckNewChangeSetAsync(cancellationToken);
         if (!allowed.IsSuccess)
         {
             return Result<ChangeSet>.Failure(allowed.Errors);
         }
 
-        await using var scope = await unitOfWork.BeginAsync(cancellationToken);
         await InsertAsync(opened.Value, cancellationToken);
 
         return await CommitAsync(scope, opened.Value, Conflict(opened.Value.Id), cancellationToken);
@@ -138,13 +151,13 @@ public async Task<Result<ChangeSet>> AddToSessionAsync(
             return Result<ChangeSet>.Failure(previousVersion.Errors);
         }
 
+        await using var scope = await unitOfWork.BeginAsync(cancellationToken);
         var allowed = await permissions.CheckNewChangeSetAsync(cancellationToken);
         if (!allowed.IsSuccess)
         {
             return Result<ChangeSet>.Failure(allowed.Errors);
         }
 
-        await using var scope = await unitOfWork.BeginAsync(cancellationToken);
         await InsertAsync(set, cancellationToken);
         await InsertOperationAsync(set, set.Operations.Count - 1, previousVersion.Value, cancellationToken);
 

src/FluentCMS.Infrastructure/Persistence/UnitOfWork.cs

@@ -1,11 +1,13 @@
 using FluentCMS.Core.Persistence;
 using Microsoft.EntityFrameworkCore.Storage;
+using Microsoft.Extensions.Logging;
 
 namespace FluentCMS.Infrastructure.Persistence;
 
 // One database transaction per outermost scope; inner scopes flush into it and cannot commit it on their own.
-internal sealed class UnitOfWork(FluentCmsDbContext context) : IUnitOfWork
+internal sealed class UnitOfWork(FluentCmsDbContext context, ILogger<UnitOfWork> logger) : IUnitOfWork
 {
+    private readonly List<Func<CancellationToken, Task>> _afterRollback = [];
     private IDbContextTransaction? _transaction;
     private int _depth;
     private bool _abandoned;
@@ -22,6 +24,16 @@ public async Task<IUnitOfWorkScope> BeginAsync(CancellationToken cancellationTok
         return new Scope(this);
     }
 
+    public void AfterRollback(Func<CancellationToken, Task> step)
+    {
+        ArgumentNullException.ThrowIfNull(step);
+
+        if (_depth > 0)
+        {
+            _afterRollback.Add(step);
+        }
+    }
+
     private async Task CommitAsync(CancellationToken cancellationToken)
     {
         if (_abandoned)
@@ -47,6 +59,9 @@ private async ValueTask EndAsync(bool committed)
             return;
         }
 
+        List<Func<CancellationToken, Task>> steps = _abandoned ? [.. _afterRollback] : [];
+        _afterRollback.Clear();
+
         // Disposing an uncommitted transaction rolls it back; the tracker is cleared so nothing from it leaks into the next scope.
         await _transaction!.DisposeAsync();
         _transaction = null;
@@ -55,6 +70,19 @@ private async ValueTask EndAsync(bool committed)
         {
             context.ChangeTracker.Clear();
         }
+
+        // Each step begins a unit of work of its own; a failing one is logged, because this runs while the caller's scope ends.
+        foreach (var step in steps)
+        {
+            try
+            {
+                await step(CancellationToken.None);
+            }
+            catch (Exception exception)
+            {
+                logger.LogError(exception, "A step that runs after a rolled-back unit of work failed.");
+            }
+        }
     }
 
     private sealed class Scope(UnitOfWork owner) : IUnitOfWorkScope