merge origin/feature/sh-196-wo-uplifts

SeaHaven.Services/Implementation/WorkOrderBoardCancelService.cs

@@ -35,41 +35,39 @@ namespace SeaHaven.Services.Implementation
             ClaimsPrincipal user,
             string? actorId)
         {
-            return await _upliftData.ExecuteWorkOrderMutationAsync(
+            // SH-196: run the whole cancel under the same per-work-order gate that uplift
+            // create uses. Previously the pending-uplift read happened outside any gate, so
+            // an in-flight create could commit after that read and leave a pending uplift on
+            // a Canceled work order, breaking "cancelling a WO with a pending uplift cancels
+            // that uplift in the same action".
+            await _upliftData.ExecuteWorkOrderMutationAsync(
                 workOrderId,
-                ct => CancelLockedAsync(workOrderId, user, actorId, ct),
+                async ct =>
+                {
+                    var workOrder = await _mutationData.GetTrackedWorkOrderAsync(workOrderId, ct);
+
+                    if (workOrder == null)
+                        throw new WorkOrderBoardValidationException("NotFound", "Work order not found.");
+
+                    // Already canceled: nothing to mutate, the row is returned below.
+                    if (workOrder.LifecycleStatus == LifecycleStatus.Canceled)
+                        return false;
+
+                    if (workOrder.LifecycleStatus == LifecycleStatus.Completed)
+                        throw new WorkOrderBoardValidationException("CancelNotAllowed", "Work order cannot be canceled in its current status.");
+
+                    var oldStatus = workOrder.LifecycleStatus?.ToString() ?? workOrder.Status ?? "";
+                    workOrder.LifecycleStatus = LifecycleStatus.Canceled;
+                    workOrder.Status = WorkOrderDerivedFields.GetLifecycleStatusLabel(LifecycleStatus.Canceled);
+                    if (workOrder.LegacyStatus == null && workOrder.Status != null)
+                        workOrder.LegacyStatus = workOrder.Status;
+
+                    await _upliftService.WithdrawPendingForWorkOrderAsync(workOrderId, actorId, ct);
+                    await _auditService.StageStatusChangedAsync(workOrderId, oldStatus, LifecycleStatus.Canceled.ToString(), actorId);
+                    await _mutationData.SaveAsync(ct);
+                    return true;
+                },
                 CancellationToken.None);
-        }
-
-        private async Task<WorkOrderBoardRowDto> CancelLockedAsync(
-            int workOrderId,
-            ClaimsPrincipal user,
-            string? actorId,
-            CancellationToken cancellationToken)
-        {
-            var workOrder = await _mutationData.GetTrackedWorkOrderAsync(workOrderId, cancellationToken);
-
-            if (workOrder == null)
-                throw new WorkOrderBoardValidationException("NotFound", "Work order not found.");
-
-            if (workOrder.LifecycleStatus == LifecycleStatus.Canceled)
-            {
-                var existing = await _boardService.GetBoardRowAsync(workOrderId, user);
-                return existing ?? throw new WorkOrderBoardValidationException("NotFound", "Work order not found.");
-            }
-
-            if (workOrder.LifecycleStatus == LifecycleStatus.Completed)
-                throw new WorkOrderBoardValidationException("CancelNotAllowed", "Work order cannot be canceled in its current status.");
-
-            var oldStatus = workOrder.LifecycleStatus?.ToString() ?? workOrder.Status ?? "";
-            workOrder.LifecycleStatus = LifecycleStatus.Canceled;
-            workOrder.Status = WorkOrderDerivedFields.GetLifecycleStatusLabel(LifecycleStatus.Canceled);
-            if (workOrder.LegacyStatus == null && workOrder.Status != null)
-                workOrder.LegacyStatus = workOrder.Status;
-
-            await _upliftService.WithdrawPendingForWorkOrderAsync(workOrderId, actorId, cancellationToken);
-            await _auditService.StageStatusChangedAsync(workOrderId, oldStatus, LifecycleStatus.Canceled.ToString(), actorId);
-            await _mutationData.SaveAsync(cancellationToken);
 
             var row = await _boardService.GetBoardRowAsync(workOrderId, user);
             return row ?? throw new WorkOrderBoardValidationException("NotFound", "Work order not found.");

SeaHaven.Services/Implementation/WorkOrderUpliftService.cs

@@ -101,7 +101,7 @@ namespace SeaHaven.Services.Implementation
             if (workOrder.LifecycleStatus is LifecycleStatus.Completed or LifecycleStatus.Canceled)
             {
                 throw new InvalidOperationException(
-                    $"Cannot change uplifts on a '{workOrder.LifecycleStatus}' work order");
+                    $"Cannot create an uplift on a '{workOrder.LifecycleStatus}' work order");
             }
 
             var dispatch = await _dispatchData.GetByIdAsync(dispatchId);
@@ -166,8 +166,6 @@ namespace SeaHaven.Services.Implementation
             if (!await HasWorkOrderAccessAsync(workOrderId, user, cancellationToken))
                 return null;
 
-            await EnsureWorkOrderAllowsUpliftMutationAsync(workOrderId, user, cancellationToken);
-
             var req = await _upliftData.GetByIdAndWorkOrderAsync(upliftId, workOrderId, cancellationToken);
             if (req == null)
                 throw new KeyNotFoundException("Uplift request not found");
@@ -179,6 +177,10 @@ namespace SeaHaven.Services.Implementation
             if (dispatch == null)
                 throw new KeyNotFoundException("Dispatch not found");
 
+            // SH-196: same terminal-lifecycle guard as revoke — the read-only dialog state
+            // is UX only and does not stop a direct API call.
+            await EnsureWorkOrderAcceptsUpliftMutationAsync(workOrderId, user, "cancel", cancellationToken);
+
             var userId = user.FindFirstValue(ClaimTypes.NameIdentifier);
             var now = _timeProvider.GetUtcNow().UtcDateTime;
             var previous = UpliftStatus.ToCanonical(req.Status);
@@ -213,8 +215,6 @@ namespace SeaHaven.Services.Implementation
             if (!await HasWorkOrderAccessAsync(workOrderId, user, cancellationToken))
                 return null;
 
-            await EnsureWorkOrderAllowsUpliftMutationAsync(workOrderId, user, cancellationToken);
-
             var req = await _upliftData.GetByIdAndWorkOrderAsync(upliftId, workOrderId, cancellationToken);
             if (req == null)
                 throw new KeyNotFoundException("Uplift request not found");
@@ -247,9 +247,20 @@ namespace SeaHaven.Services.Implementation
             if (dispatch == null)
                 throw new KeyNotFoundException("Dispatch not found");
 
+            // SH-196: revoking is blocked once the work order is Completed or Canceled.
+            // The board dialog enforces this in the UI only, so without a service-side
+            // guard a direct API call could still mutate uplifts on a terminal WO.
+            await EnsureWorkOrderAcceptsUpliftMutationAsync(workOrderId, user, "revoke", cancellationToken);
+
             var now = _timeProvider.GetUtcNow().UtcDateTime;
             var previous = canonical;
 
+            // SH-196: revoking must free the work order's uplift capacity again. Create
+            // raises the dispatch NTE for BOTH auto-approved and approved uplifts, so
+            // revoke has to compensate symmetrically. Restoring only on Approved left the
+            // NTE raised while SumAutoApprovedAmountForWorkOrderAsync stopped counting the
+            // revoked request, so every create -> auto-approve -> revoke cycle compounded
+            // the inflation and handed back allowance that was never actually released.
             if (canonical == UpliftStatus.Approved || canonical == UpliftStatus.NoApprovalRequired)
             {
                 var oldNte = dispatch.NTEAmount ?? 0m;
@@ -359,24 +370,24 @@ namespace SeaHaven.Services.Implementation
             return WorkOrderUpliftContractMapper.MapItem(created, requesterName, null);
         }
 
-        private async Task EnsureWorkOrderAllowsUpliftMutationAsync(
+        /// <summary>
+        /// SH-196: uplifts may not be mutated once the work order reaches a terminal
+        /// lifecycle state. Enforced in the service so a direct API call is rejected too,
+        /// not only the board dialog's read-only state.
+        /// </summary>
+        private async Task EnsureWorkOrderAcceptsUpliftMutationAsync(
             int workOrderId,
             ClaimsPrincipal user,
+            string action,
             CancellationToken cancellationToken)
         {
             var accountFilter = _accountResolver.ResolveAccountFilter(user);
-            var workOrder = await _detailData.GetWorkOrderForMediaAsync(
+            var workOrder = await _detailData.GetWorkOrderForMediaAsync(workOrderId, cancellationToken, accountFilter);
-                workOrderId,
+            var lifecycle = workOrder?.LifecycleStatus;
-                cancellationToken,
-                accountFilter);
-            if (workOrder == null)
-                throw new KeyNotFoundException("Work order not found");
 
-            if (workOrder.LifecycleStatus is LifecycleStatus.Completed or LifecycleStatus.Canceled)
+            if (lifecycle == LifecycleStatus.Completed || lifecycle == LifecycleStatus.Canceled)
-            {
                 throw new InvalidOperationException(
-                    $"Cannot change uplifts on a '{workOrder.LifecycleStatus}' work order");
+                    $"Cannot {action} an uplift on a '{lifecycle}' work order");
-            }
         }
 
         private async Task<bool> HasWorkOrderAccessAsync(

SeaHavenIndustries.Tests/WorkOrderBoardCancelServiceTests.cs

@@ -26,12 +26,7 @@ public class WorkOrderBoardCancelServiceTests
         var boardService = new WorkOrderBoardService(boardData, WorkOrderAccountTestHelpers.Resolver(context));
         var fieldLocks = new WorkOrderFieldLockService(new WorkOrderFieldLockDataService(context));
         var audit = new WorkOrderAuditService(new WorkOrderAuditDataService(context), fieldLocks);
-        var cancel = new WorkOrderBoardCancelService(
+        var cancel = new WorkOrderBoardCancelService(mutationData, boardService, audit, new NoOpUpliftService(), new PassThroughUpliftData());
-            mutationData,
-            boardService,
-            audit,
-            new NoOpUpliftService(),
-            new UpliftDataService(context));
         var update = new WorkOrderBoardUpdateService(boardData, mutationData, audit);
         return (context, cancel, update);
     }
@@ -210,7 +205,7 @@ public class WorkOrderBoardCancelServiceTests
             new UserDataService(context),
             TimeProvider.System,
             Options.Create(new ApprovalsOptions()));
-        var cancel = new WorkOrderBoardCancelService(mutationData, boardService, audit, uplifts, new UpliftDataService(context));
+        var cancel = new WorkOrderBoardCancelService(mutationData, boardService, audit, uplifts, new PassThroughUpliftData());
 
         var result = await cancel.CancelAsync(1, WorkOrderAccountTestHelpers.OrgWideAdmin(), "actor-1");
 
@@ -276,7 +271,7 @@ public class WorkOrderBoardCancelServiceTests
             boardService,
             audit,
             uplifts,
-            new UpliftDataService(context));
+            new PassThroughUpliftData());
 
         await Assert.ThrowsAsync<InvalidOperationException>(() =>
             cancel.CancelAsync(1, WorkOrderAccountTestHelpers.OrgWideAdmin(), "actor-1"));
@@ -419,6 +414,41 @@ public class WorkOrderBoardCancelServiceTests
             => throw new InvalidOperationException("forced late failure");
     }
 
+    /// <summary>
+    /// SH-196: the cancel flow now runs inside the per-work-order gate. Tests only need the
+    /// gate to invoke the work; the real serialization is exercised against SQL Server.
+    /// </summary>
+    private sealed class PassThroughUpliftData : IUpliftDataService
+    {
+        public Task<T> ExecuteWorkOrderMutationAsync<T>(
+            int workOrderId,
+            Func<CancellationToken, Task<T>> work,
+            CancellationToken cancellationToken) => work(cancellationToken);
+
+        public Task<(int TotalCount, IReadOnlyList<UpliftListItemData> Items)> GetPagedAsync(
+            string? status, int? tier, int page, int pageSize, CancellationToken cancellationToken) =>
+            throw new NotSupportedException();
+        public Task<IReadOnlyList<UpliftForDispatchData>> GetForDispatchAsync(int dispatchId, CancellationToken cancellationToken) => throw new NotSupportedException();
+        public Task<IReadOnlyList<UpliftForWorkOrderData>> GetForWorkOrderAsync(int workOrderId, CancellationToken cancellationToken) => throw new NotSupportedException();
+        public Task<DispatchUpliftRequest?> GetByIdAndWorkOrderAsync(int requestId, int workOrderId, CancellationToken cancellationToken) => throw new NotSupportedException();
+        public Task<IReadOnlyList<PortalUpliftData>> GetForVendorDispatchAsync(int dispatchId, CancellationToken cancellationToken) => throw new NotSupportedException();
+        public Task<DispatchUpliftRequest?> GetByIdAsync(int id, CancellationToken cancellationToken) => throw new NotSupportedException();
+        public Task<DispatchUpliftRequest?> GetByIdAndDispatchAsync(int requestId, int dispatchId, CancellationToken cancellationToken) => throw new NotSupportedException();
+        public Task<UpliftEvidenceDownloadData?> GetEvidenceForInternalDownloadAsync(int upliftRequestId, CancellationToken cancellationToken) => throw new NotSupportedException();
+        public Task<bool> HasPendingAsync(int dispatchId, CancellationToken cancellationToken) => throw new NotSupportedException();
+        public Task<bool> HasPendingForWorkOrderAsync(int workOrderId, CancellationToken cancellationToken) => throw new NotSupportedException();
+        public Task<decimal> SumAutoApprovedAmountForWorkOrderAsync(int workOrderId, CancellationToken cancellationToken) => throw new NotSupportedException();
+        public Task<List<DispatchUpliftRequest>> GetPendingForWorkOrderAsync(int workOrderId, CancellationToken cancellationToken) => throw new NotSupportedException();
+        public Task<bool> HasActiveAsync(int dispatchId, CancellationToken cancellationToken) => throw new NotSupportedException();
+        public Task<DispatchUpliftRequest?> GetActiveRequestAsync(int dispatchId, CancellationToken cancellationToken) => throw new NotSupportedException();
+        public Task<DispatchUpliftRequest?> GetByRequestKeyAsync(int dispatchId, string requestKey, CancellationToken cancellationToken) => throw new NotSupportedException();
+        public Task<List<DispatchUpliftRequest>> GetDueForExpiryAsync(DateTime utcNow, int count, CancellationToken cancellationToken) => throw new NotSupportedException();
+        public Task<List<DispatchUpliftRequest>> GetDueForInitialNotificationAsync(int count, CancellationToken cancellationToken) => throw new NotSupportedException();
+        public Task<List<DispatchUpliftRequest>> GetDueForEscalationAsync(int count, CancellationToken cancellationToken) => throw new NotSupportedException();
+        public Task StageAsync(DispatchUpliftRequest request, CancellationToken cancellationToken) => throw new NotSupportedException();
+        public Task SaveChangesAsync(CancellationToken cancellationToken) => throw new NotSupportedException();
+    }
+
     private sealed class NoOpUpliftService : IWorkOrderUpliftService
     {
         public Task<IReadOnlyList<WorkOrderUpliftDto>?> ListAsync(

SeaHavenIndustries.Tests/WorkOrderUpliftServiceTests.cs

@@ -340,6 +340,111 @@ public sealed class WorkOrderUpliftServiceTests
         Assert.Equal(1000m, dispatch.NTEAmount);
     }
 
+    [Fact]
+    public async Task RevokeAsync_AutoApproved_RestoresNteAndFreesAllowance()
+    {
+        await using var context = CreateContext();
+        var (workOrder, dispatch) = await SeedWorkOrderAsync(context);
+        var service = NewService(context);
+
+        // Auto-approved create raises the dispatch NTE by the requested amount.
+        var created = await service.CreateAsync(
+            workOrder.Id,
+            new CreateWorkOrderUpliftRequestDto { Amount = 400m, Notes = "Within limit" },
+            Dispatcher(),
+            CancellationToken.None);
+        Assert.Equal("auto_approved", created!.Status);
+        Assert.Equal(1400m, context.Dispatches.Single(d => d.Id == 10).NTEAmount);
+
+        var revoked = await service.RevokeAsync(
+            workOrder.Id,
+            created.Id,
+            new RevokeWorkOrderUpliftRequestDto(),
+            Dispatcher(),
+            CancellationToken.None);
+
+        // SH-196: revoking frees the allowance, so it must release the NTE too. Otherwise
+        // every create -> revoke cycle compounds NTE inflation.
+        Assert.Equal("revoked", revoked!.Status);
+        Assert.Equal(1000m, context.Dispatches.Single(d => d.Id == 10).NTEAmount);
+
+        // The freed allowance is reusable at the full cap.
+        var second = await service.CreateAsync(
+            workOrder.Id,
+            new CreateWorkOrderUpliftRequestDto { Amount = 500m, Notes = "Reuses freed allowance" },
+            Dispatcher(),
+            CancellationToken.None);
+        Assert.Equal("auto_approved", second!.Status);
+        Assert.Equal(1500m, context.Dispatches.Single(d => d.Id == 10).NTEAmount);
+    }
+
+    [Theory]
+    [InlineData(LifecycleStatus.Completed)]
+    [InlineData(LifecycleStatus.Canceled)]
+    public async Task RevokeAsync_TerminalWorkOrder_IsRejected(LifecycleStatus lifecycle)
+    {
+        await using var context = CreateContext();
+        var (workOrder, dispatch) = await SeedWorkOrderAsync(context);
+        dispatch.NTEAmount = 1500m;
+        context.DispatchUpliftRequests.Add(new DispatchUpliftRequest
+        {
+            Id = 100,
+            DispatchId = dispatch.Id,
+            RequestedNTE = 1500m,
+            CurrentNTE = 1000m,
+            Status = "Approved",
+            RequiredTier = 1,
+            NotificationStatus = "Sent",
+        });
+        workOrder.LifecycleStatus = lifecycle;
+        await context.SaveChangesAsync();
+
+        var service = NewService(context);
+
+        // SH-196: the board dialog's read-only state is UX only; a direct API call must
+        // still be rejected server-side.
+        await Assert.ThrowsAsync<InvalidOperationException>(() => service.RevokeAsync(
+            workOrder.Id,
+            100,
+            new RevokeWorkOrderUpliftRequestDto { Reason = "Policy change" },
+            WorkOrderAccountTestHelpers.OrgWideAdmin("admin-1"),
+            CancellationToken.None));
+
+        Assert.Equal(1500m, context.Dispatches.Single(d => d.Id == 10).NTEAmount);
+        Assert.Equal("Approved", context.DispatchUpliftRequests.Single(r => r.Id == 100).Status);
+    }
+
+    [Theory]
+    [InlineData(LifecycleStatus.Completed)]
+    [InlineData(LifecycleStatus.Canceled)]
+    public async Task CancelAsync_TerminalWorkOrder_IsRejected(LifecycleStatus lifecycle)
+    {
+        await using var context = CreateContext();
+        var (workOrder, dispatch) = await SeedWorkOrderAsync(context);
+        context.DispatchUpliftRequests.Add(new DispatchUpliftRequest
+        {
+            Id = 101,
+            DispatchId = dispatch.Id,
+            RequestedNTE = 1600m,
+            CurrentNTE = 1000m,
+            Status = "Pending",
+            RequiredTier = 1,
+            NotificationStatus = "Sent",
+        });
+        workOrder.LifecycleStatus = lifecycle;
+        await context.SaveChangesAsync();
+
+        var service = NewService(context);
+
+        await Assert.ThrowsAsync<InvalidOperationException>(() => service.CancelAsync(
+            workOrder.Id,
+            101,
+            Dispatcher(),
+            CancellationToken.None));
+
+        Assert.Equal("Pending", context.DispatchUpliftRequests.Single(r => r.Id == 101).Status);
+    }
+
     [Fact]
     public async Task CreateAsync_ConcurrentRequests_PreserveOnePendingAndCap()
     {