Feat: Add optimistic concurrency semantics to governance storage (#13)

Author: rian-be

Tests/ModularityKit.Mutator.Governance.Tests/Lifecycle/MutationRequestLifecycleAtomicityTests.cs

@@ -1,4 +1,5 @@
 using ModularityKit.Mutator.Abstractions.Context;
+using ModularityKit.Mutator.Governance.Abstractions.Exceptions;
 using ModularityKit.Mutator.Governance.Abstractions.Lifecycle;
 using ModularityKit.Mutator.Governance.Runtime.Lifecycle;
 using ModularityKit.Mutator.Governance.Tests.TestSupport;
@@ -9,7 +10,7 @@ namespace ModularityKit.Mutator.Governance.Tests.Lifecycle;
 public sealed class MutationRequestLifecycleAtomicityTests
 {
     [Fact]
-    public async Task Stale_snapshot_transition_can_succeed_after_a_prior_lifecycle_update()
+    public async Task Stale_snapshot_transition_is_rejected_after_a_prior_lifecycle_update()
     {
         var request = MutationRequestTestFactory.CreatePendingRequest();
         var store = new StaleSnapshotMutationRequestStore(request);
@@ -19,15 +20,17 @@ public async Task Stale_snapshot_transition_can_succeed_after_a_prior_lifecycle_
             request.RequestId,
             MutationContext.User("approver", "Approver", "Approve request"));
 
-        var canceled = await manager.Cancel(
-            request.RequestId,
-            MutationContext.User("operator", "Operator", "Cancel request"));
+        var exception = await Assert.ThrowsAsync<MutationRequestConcurrencyException>(() =>
+            manager.Cancel(
+                request.RequestId,
+                MutationContext.User("operator", "Operator", "Cancel request")));
 
-        Assert.Equal(2, store.StoreCount);
+        Assert.Equal(1, store.StoreCount);
         Assert.All(store.GetSnapshots, snapshot => Assert.Equal(MutationRequestStatus.Pending, snapshot.Status));
         Assert.Equal(MutationRequestStatus.Approved, approved.Status);
-        Assert.Equal(MutationRequestStatus.Canceled, canceled.Status);
-        Assert.NotEqual(approved.Status, canceled.Status);
-        Assert.Contains(store.Current.Status, new[] { MutationRequestStatus.Approved, MutationRequestStatus.Canceled });
+        Assert.Equal(request.RequestId, exception.RequestId);
+        Assert.Equal(0, exception.ExpectedRevision);
+        Assert.Equal(MutationRequestStatus.Approved, store.Current.Status);
+        Assert.Equal(1, store.Current.Revision);
     }
 }

Tests/ModularityKit.Mutator.Governance.Tests/Lifecycle/MutationRequestStoreContractTests.cs

@@ -1,4 +1,5 @@
 using ModularityKit.Mutator.Abstractions.Context;
+using ModularityKit.Mutator.Governance.Abstractions.Exceptions;
 using ModularityKit.Mutator.Governance.Abstractions.Lifecycle;
 using ModularityKit.Mutator.Governance.Abstractions.Requests;
 using ModularityKit.Mutator.Governance.Runtime.Storage;
@@ -10,32 +11,62 @@ namespace ModularityKit.Mutator.Governance.Tests.Lifecycle;
 public sealed class MutationRequestStoreContractTests
 {
     [Fact]
-    public async Task Store_contract_allows_blind_overwrite_without_expected_revision_or_status()
+    public async Task Create_contract_rejects_duplicate_request_ids()
     {
         var store = new InMemoryMutationRequestStore();
         var request = MutationRequestTestFactory.CreatePendingRequest();
 
-        await store.Store(request);
+        var created = await store.Create(request);
 
-        var overwritten = request with
+        var exception = await Assert.ThrowsAsync<MutationRequestAlreadyExistsException>(() =>
+            store.Create(created));
+
+        Assert.Equal(request.RequestId, exception.RequestId);
+    }
+
+    [Fact]
+    public async Task TryStore_rejects_stale_revision_and_preserves_current_state()
+    {
+        var store = new InMemoryMutationRequestStore();
+        var request = MutationRequestTestFactory.CreatePendingRequest();
+        var created = await store.Create(request);
+
+        var firstUpdate = created with
+        {
+            Status = MutationRequestStatus.Approved,
+            PendingReason = null,
+            Decisions =
+            [
+                .. created.Decisions,
+                MutationRequestDecision.Create(
+                    MutationRequestDecisionType.Approved,
+                    MutationContext.User("approver", "Approver", "Approve request"))
+            ]
+        };
+
+        var persisted = await store.TryStore(firstUpdate, created.Revision);
+        Assert.NotNull(persisted);
+
+        var staleUpdate = created with
         {
             Status = MutationRequestStatus.Canceled,
             PendingReason = null,
             Decisions =
             [
-                .. request.Decisions,
+                .. created.Decisions,
                 MutationRequestDecision.Create(
                     MutationRequestDecisionType.Canceled,
-                    MutationContext.User("operator", "Operator", "Canceled without guard"))
+                    MutationContext.User("operator", "Operator", "Cancel request"))
             ]
         };
 
-        await store.Store(overwritten);
+        var rejected = await store.TryStore(staleUpdate, created.Revision);
 
         var loaded = await store.Get(request.RequestId);
 
+        Assert.Null(rejected);
         Assert.NotNull(loaded);
-        Assert.Equal(MutationRequestStatus.Canceled, loaded.Status);
-        Assert.Equal(overwritten.Decisions.Count, loaded.Decisions.Count);
+        Assert.Equal(MutationRequestStatus.Approved, loaded.Status);
+        Assert.Equal(1, loaded.Revision);
     }
 }

Tests/ModularityKit.Mutator.Governance.Tests/Resolution/MutationRequestVersionResolutionPersistenceTests.cs

@@ -17,7 +17,7 @@ public async Task Resolve_does_not_persist_decision_history_unless_caller_saves_
         var resolver = new MutationRequestVersionResolver();
         var request = MutationRequestTestFactory.CreateApprovedSecurityRequest("v10");
 
-        await store.Store(request);
+        await store.Create(request);
 
         var resolution = resolver.Resolve(
             request,

Tests/ModularityKit.Mutator.Governance.Tests/TestSupport/StaleSnapshotMutationRequestStore.cs

@@ -26,17 +26,40 @@ public MutationRequest Current
 
     public IReadOnlyList<MutationRequest> GetSnapshots => _getSnapshots;
 
-    public Task Store(
+    public Task<MutationRequest> Create(
         MutationRequest request,
         CancellationToken cancellationToken = default)
     {
         lock (_gate)
         {
             StoreCount++;
-            _current = request;
+            _current = request with
+            {
+                Revision = 0
+            };
         }
 
-        return Task.CompletedTask;
+        return Task.FromResult(_current);
+    }
+
+    public Task<MutationRequest?> TryStore(
+        MutationRequest request,
+        long expectedRevision,
+        CancellationToken cancellationToken = default)
+    {
+        lock (_gate)
+        {
+            if (_current.Revision != expectedRevision)
+                return Task.FromResult<MutationRequest?>(null);
+
+            StoreCount++;
+            _current = request with
+            {
+                Revision = expectedRevision + 1
+            };
+
+            return Task.FromResult<MutationRequest?>(_current);
+        }
     }
 
     public Task<MutationRequest?> Get(

src/Governance/Abstractions/Exceptions/MutationRequestAlreadyExistsException.cs

@@ -0,0 +1,15 @@
+using ModularityKit.Mutator.Abstractions.Exceptions;
+
+namespace ModularityKit.Mutator.Governance.Abstractions.Exceptions;
+
+/// <summary>
+/// Thrown when governance storage is asked to create a request that already exists.
+/// </summary>
+public sealed class MutationRequestAlreadyExistsException(string requestId)
+    : MutationException($"Mutation request '{requestId}' already exists.")
+{
+    /// <summary>
+    /// Stable identifier of the duplicate request.
+    /// </summary>
+    public string RequestId { get; } = requestId;
+}

src/Governance/Abstractions/Exceptions/MutationRequestConcurrencyException.cs

@@ -0,0 +1,23 @@
+using ModularityKit.Mutator.Abstractions.Exceptions;
+
+namespace ModularityKit.Mutator.Governance.Abstractions.Exceptions;
+
+/// <summary>
+/// Thrown when a governance request transition loses an optimistic concurrency race.
+/// </summary>
+public sealed class MutationRequestConcurrencyException(
+    string requestId,
+    long expectedRevision)
+    : MutationException(
+        $"Mutation request '{requestId}' could not be updated because revision '{expectedRevision}' is stale.")
+{
+    /// <summary>
+    /// Stable identifier of the request that lost the concurrency race.
+    /// </summary>
+    public string RequestId { get; } = requestId;
+
+    /// <summary>
+    /// Revision the runtime expected to update.
+    /// </summary>
+    public long ExpectedRevision { get; } = expectedRevision;
+}

src/Governance/Abstractions/Requests/MutationRequest.cs

@@ -60,6 +60,11 @@ public sealed record MutationRequest
     /// </summary>
     public IReadOnlyList<MutationRequestDecision> Decisions { get; init; } = [];
 
+    /// <summary>
+    /// Optimistic concurrency revision for the governed request.
+    /// </summary>
+    public long Revision { get; init; }
+
     /// <summary>
     /// Expected version or concurrency token for the target state.
     /// </summary>

src/Governance/Abstractions/Storage/IMutationRequestStore.cs

@@ -9,12 +9,21 @@ namespace ModularityKit.Mutator.Governance.Abstractions.Storage;
 public interface IMutationRequestStore
 {
     /// <summary>
-    /// Stores or updates a mutation request.
+    /// Creates a new governed mutation request in persistence.
     /// </summary>
-    Task Store(
+    Task<MutationRequest> Create(
         MutationRequest request,
         CancellationToken cancellationToken = default);
 
+    /// <summary>
+    /// Stores a request only when the persisted revision matches the expected revision.
+    /// Returns the persisted request with incremented revision on success, or <c>null</c> on conflict.
+    /// </summary>
+    Task<MutationRequest?> TryStore(
+        MutationRequest request,
+        long expectedRevision,
+        CancellationToken cancellationToken = default);
+
     /// <summary>
     /// Retrieves a single mutation request by its stable identifier.
     /// </summary>

src/Governance/README.md

@@ -38,6 +38,8 @@ Key types:
 - `MutationRequestVersionResolution`
 - `MutationRequestVersionResolutionOutcome`
 - `VersionedRequestResolutionStrategy`
+- `MutationRequestAlreadyExistsException`
+- `MutationRequestConcurrencyException`
 - `MutationRequestNotFoundException`
 - `InvalidMutationRequestTransitionException`