diff --git a/docs/requirements/Component-ManagementService.md b/docs/requirements/Component-ManagementService.md index 4e077dce..1cdcf1bf 100644 --- a/docs/requirements/Component-ManagementService.md +++ b/docs/requirements/Component-ManagementService.md @@ -168,7 +168,7 @@ The two-person authorization workflow for writes through the MxAccess Gateway. B - **SubmitSecuredWriteCommand** (`Operator` role): validates the target connection exists and uses the **MxGateway** protocol (rejected otherwise), then inserts a `Pending` row capturing site / connection / tag / typed value / operator + comment. Nothing is written yet. - **ApproveSecuredWriteCommand** (`Verifier` role): enforces **no self-approval** (`VerifierUser ≠ OperatorUser`, checked **before** any state change) and a **compare-and-swap** race guard (`TryMarkApprovedAsync` — exactly one concurrent approver flips `Pending → Approved`; the loser is rejected). On winning, it validates the value type, decodes the value (guarded — a bad value fails the row deterministically rather than leaving it stuck `Approved`), and relays a `WriteTagRequest` to the site MxGateway via the Communication Layer, recording the outcome as `Executed` or `Failed`. -- **RejectSecuredWriteCommand** (`Verifier` role): marks a `Pending` row `Rejected` with the verifier's reason. +- **RejectSecuredWriteCommand** (`Verifier` role): marks a `Pending` row `Rejected` with the verifier's reason. Like approve, the transition is **compare-and-swap guarded** (`TryMarkRejectedAsync` — a conditional `UPDATE … WHERE Status = 'Pending'`, so approve, reject and the TTL expiry can never both transition one row); a loser records **nothing** — no status write and no `SecuredWriteReject` audit row — and is answered with the status that actually won. The blind full-row update it replaced could land after a concurrent approve had won its own CAS and while that approve's multi-second `WriteTagRequest` relay was still in flight, stamping the row `Rejected` for a write that physically executed at the plant (arch-review 2026-08-19 P6-1). Winning the approve CAS therefore gives that leg exclusive ownership of the row, which is what makes its post-relay `Executed`/`Failed` update safe as an unconditional write. - **ListSecuredWritesCommand** (any of `Operator` / `Verifier` / `Administrator`): pending queue + terminal history, global or per-site, **offset-paged** via `Skip` / `Take` (arch-review P3; additive — the defaults `Skip=0` / `Take=200` preserve the historical unpaged wire behaviour). The handler clamps `Take` to `1..500` and `Skip` to `>= 0`, calls `QueryAsync(Status, SiteId, skip, take)` for the page and `CountAsync(Status, SiteId)` for the unpaged total, and returns a `SecuredWriteListResult { Items, TotalCount }` so a client can render a pager. Although read-only, the history table exposes process-sensitive tag values, so it is **not** open to any authenticated user — it is gated to the two Secured Write roles plus `Administrator` (any-of), aligning with the `RequireSecuredWriteAccess` Central UI policy (arch-review UA1). **Server-side pending TTL (arch-review S2).** A `Pending` secured write is not valid indefinitely. Both `ApproveSecuredWriteCommand` and `RejectSecuredWriteCommand` call a shared `TryExpireIfStaleAsync` helper immediately after the `Pending`-status check: if the row's `SubmittedAtUtc` is older than the configured `SecuredWritePendingTtl` (`ManagementServiceOptions`, default **24 h**), the helper atomically transitions it `Pending → Expired` via `ISecuredWriteRepository.TryMarkExpiredAsync` (compare-and-swap — no verifier recorded, `VerifierUser` stays `NULL`), emits a best-effort `SecuredWriteExpire` audit row (system actor, `Discarded` status — the closest terminal `AuditStatus`), and the command fails with an "expired" error. A stale setpoint therefore can **never** be approved or executed days later, and no `WriteTagRequest` is relayed. A non-positive TTL disables expiry. `ListSecuredWritesCommand` additionally runs an **opportunistic sweep**: on every read it walks the returned page and calls the same helper for each `Pending` row, so overdue writes surface as `Expired` the next time anyone views the queue — no dedicated expiry-timer singleton is needed because the ManagementActor already runs on every central node and the CAS makes concurrent multi-node sweeps idempotent. diff --git a/src/ZB.MOM.WW.ScadaBridge.Commons/Interfaces/Repositories/ISecuredWriteRepository.cs b/src/ZB.MOM.WW.ScadaBridge.Commons/Interfaces/Repositories/ISecuredWriteRepository.cs index 52f7c71f..4e6aae96 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Commons/Interfaces/Repositories/ISecuredWriteRepository.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Commons/Interfaces/Repositories/ISecuredWriteRepository.cs @@ -86,6 +86,35 @@ public interface ISecuredWriteRepository DateTime decidedAtUtc, CancellationToken ct = default); + /// + /// Atomically flips a row from Pending to Rejected, stamping the + /// verifier identity, comment, and decision time, but ONLY if the row is still + /// Pending. The reject counterpart of , and + /// the compare-and-swap guard for the approve-vs-reject race: a stale-read reject + /// used to land a blind full-row update while a concurrent approve's multi-second + /// site relay was still in flight, marking the row Rejected while the plant + /// write physically executed (arch-review 2026-08-19 P6-1). The conditional + /// WHERE Status='Pending' makes exactly one of approve/reject/expiry win. + /// The loser observes false and must record nothing — no status change and no + /// reject audit row. + /// + /// Identity of the pending secured write. + /// The rejecting verifier's username. + /// Optional free-text comment from the verifier. + /// UTC instant the rejection decision was made. + /// Cancellation token. + /// + /// A task resolving to true if this caller won the rejection (exactly one + /// row transitioned), or false if the row was no longer Pending + /// (already approved, rejected, or expired). + /// + Task TryMarkRejectedAsync( + long id, + string verifierUser, + string? verifierComment, + DateTime decidedAtUtc, + CancellationToken ct = default); + /// /// Atomically flips a row from Pending to Expired, stamping the /// expiry as the decision time, but ONLY if the row is still Pending. diff --git a/src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase/Repositories/SecuredWriteRepository.cs b/src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase/Repositories/SecuredWriteRepository.cs index 1ad871e1..f1fb9c3f 100644 --- a/src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase/Repositories/SecuredWriteRepository.cs +++ b/src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase/Repositories/SecuredWriteRepository.cs @@ -113,6 +113,35 @@ WHERE Id = {id} return rowsAffected == 1; } + /// + public async Task TryMarkRejectedAsync( + long id, + string verifierUser, + string? verifierComment, + DateTime decidedAtUtc, + CancellationToken ct = default) + { + // Single-statement compare-and-swap, mirroring TryMarkApprovedAsync exactly. + // A blind full-row UpdateAsync here used to be able to land AFTER a concurrent + // approve had won its own CAS and while its multi-second WriteTagAsync relay was + // still in flight — stamping the row Rejected even though the plant write was + // physically executing, i.e. falsifying the two-person-control record + // (arch-review 2026-08-19 P6-1). The conditional WHERE Status='Pending' makes + // reject win only from Pending, so approve and reject can never both transition + // one row. Only the columns the reject transition owns are written. + var rowsAffected = await _context.Database.ExecuteSqlInterpolatedAsync( + $@"UPDATE dbo.PendingSecuredWrites +SET Status = 'Rejected', + VerifierUser = {verifierUser}, + VerifierComment = {verifierComment}, + DecidedAtUtc = {decidedAtUtc} +WHERE Id = {id} + AND Status = 'Pending';", + ct); + + return rowsAffected == 1; + } + /// public async Task TryMarkExpiredAsync(long id, DateTime expiredAtUtc, CancellationToken ct = default) { diff --git a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs index 3b145dc1..dcfabc61 100644 --- a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs @@ -1604,6 +1604,14 @@ public class ManagementActor : ReceiveActor row.ExecutionError = error; // UpdateAsync overwrites all columns -> pass the fully-populated entity. + // Safe as a blind write (arch-review 2026-08-19 P6-1): winning the + // Pending->Approved CAS above gives this leg EXCLUSIVE ownership of the row from + // that point on. Every other writer transitions only out of Pending — + // TryMarkApprovedAsync, TryMarkRejectedAsync and TryMarkExpiredAsync (both the + // approve/reject inline expiry and the opportunistic list sweep) are all + // conditional on Status='Pending' — so nothing can overwrite the row while the + // multi-second relay above is in flight. Do not reintroduce an unconditional + // transition on this table. await repo.UpdateAsync(row); // Terminal execute outcome: Delivered (relay succeeded) maps to canonical @@ -1646,15 +1654,37 @@ public class ManagementActor : ReceiveActor throw new ManagementCommandException( "A secured write cannot be verified by its submitter."); + // Compare-and-swap: guards the approve-vs-reject race exactly as approve's own + // CAS guards the two-verifier race (arch-review 2026-08-19 P6-1). The status + // check above is a stale read — commands run concurrently, each on its own DI + // scope and across both central nodes on one shared MS SQL — so a concurrent + // approve can win its CAS and enter its multi-second WriteTagAsync relay in the + // gap. The blind full-row UpdateAsync that used to sit here would then land + // mid-relay and stamp the row Rejected while the plant write physically + // executed, falsifying the two-person-control record. Reject now transitions + // ONLY from Pending; the loser records nothing. + var decidedAtUtc = DateTime.UtcNow; + if (!await repo.TryMarkRejectedAsync(cmd.Id, user.Username, cmd.Comment, decidedAtUtc)) + { + // Re-read so the conflict names the status that actually won (approve, + // another reject, or the TTL expiry) — same UX shape as the stale-read + // guard above and approve's own lost-race error. + var current = await repo.GetAsync(cmd.Id); + throw new ManagementCommandException(current is null + ? $"Secured write {cmd.Id} is no longer pending — already decided." + : $"Secured write {cmd.Id} is '{current.Status}', not Pending; it cannot be rejected."); + } + + // We won the race. Stamp the decision locally so the returned DTO carries the + // same values the CAS committed. entity.Status = "Rejected"; entity.VerifierUser = user.Username; entity.VerifierComment = cmd.Comment; - entity.DecidedAtUtc = DateTime.UtcNow; + entity.DecidedAtUtc = decidedAtUtc; - // UpdateAsync overwrites all columns -> pass the fully-populated entity. - await repo.UpdateAsync(entity); - - // Reject is a terminal lifecycle event (canonical Discarded outcome). + // Reject is a terminal lifecycle event (canonical Discarded outcome). Emitted + // ONLY on a won CAS, so a lost race never leaves a reject row in the audit log + // for a write that was actually approved and executed. // Actor = the verifier; the operator is carried in the row's extra detail. await EmitSecuredWriteAuditAsync( sp, AuditKind.SecuredWriteReject, AuditStatus.Discarded, entity, actor: user.Username); diff --git a/tests/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests/SecuredWriteRepositoryTests.cs b/tests/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests/SecuredWriteRepositoryTests.cs index 2788fac8..db02de37 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests/SecuredWriteRepositoryTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests/SecuredWriteRepositoryTests.cs @@ -148,6 +148,108 @@ public class SecuredWriteRepositoryTests : IClassFixture Assert.False(await repo.TryMarkExpiredAsync(id, DateTime.UtcNow)); } + [SkippableFact] + public async Task TryMarkRejectedAsync_PendingRow_FlipsToRejected_Once() + { + Skip.IfNot(_fixture.Available, _fixture.SkipReason); + + var siteId = NewSiteId(); + await using var context = CreateContext(); + var repo = new SecuredWriteRepository(context); + + var id = await repo.AddAsync(NewRow(siteId, status: "Pending")); + + Assert.True(await repo.TryMarkRejectedAsync(id, "ver.bob", "not authorized", DateTime.UtcNow)); + // CAS: a second caller loses because the row is no longer Pending. + Assert.False(await repo.TryMarkRejectedAsync(id, "ver.carol", "also no", DateTime.UtcNow)); + + await using var readContext = CreateContext(); + var row = await new SecuredWriteRepository(readContext).GetAsync(id); + Assert.NotNull(row); + Assert.Equal("Rejected", row!.Status); + Assert.Equal("ver.bob", row.VerifierUser); + Assert.Equal("not authorized", row.VerifierComment); + Assert.NotNull(row.DecidedAtUtc); + // Reject writes only the columns its transition owns — the operator's submission + // and the (never-run) execution columns are untouched. + Assert.Equal("op.alice", row.OperatorUser); + Assert.Null(row.ExecutedAtUtc); + Assert.Null(row.ExecutionError); + } + + [SkippableFact] + public async Task ApproveThenReject_OnOnePendingRow_ApproveWins_RejectReturnsFalse() + { + // arch-review 2026-08-19 P6-1: approve and reject both transition out of Pending, + // so exactly one may win. This is the ordering that used to falsify the + // two-person-control record — a blind reject update landing after approve had + // already committed (and, in production, while its multi-second site relay was + // still in flight) rewrote the row to Rejected for a write that executed. + Skip.IfNot(_fixture.Available, _fixture.SkipReason); + + var siteId = NewSiteId(); + await using var context = CreateContext(); + var repo = new SecuredWriteRepository(context); + + var id = await repo.AddAsync(NewRow(siteId, status: "Pending")); + + Assert.True(await repo.TryMarkApprovedAsync(id, "ver.bob", "ok", DateTime.UtcNow)); + Assert.False(await repo.TryMarkRejectedAsync(id, "ver.carol", "no", DateTime.UtcNow)); + + await using var readContext = CreateContext(); + var row = await new SecuredWriteRepository(readContext).GetAsync(id); + Assert.NotNull(row); + // The row reflects the winner ONLY — no trace of the losing reject. + Assert.Equal("Approved", row!.Status); + Assert.Equal("ver.bob", row.VerifierUser); + Assert.Equal("ok", row.VerifierComment); + } + + [SkippableFact] + public async Task RejectThenApprove_OnOnePendingRow_RejectWins_ApproveReturnsFalse() + { + // The opposite order: once rejected, the row can never be approved (and therefore + // never relayed to the plant). + Skip.IfNot(_fixture.Available, _fixture.SkipReason); + + var siteId = NewSiteId(); + await using var context = CreateContext(); + var repo = new SecuredWriteRepository(context); + + var id = await repo.AddAsync(NewRow(siteId, status: "Pending")); + + Assert.True(await repo.TryMarkRejectedAsync(id, "ver.carol", "no", DateTime.UtcNow)); + Assert.False(await repo.TryMarkApprovedAsync(id, "ver.bob", "ok", DateTime.UtcNow)); + + await using var readContext = CreateContext(); + var row = await new SecuredWriteRepository(readContext).GetAsync(id); + Assert.NotNull(row); + Assert.Equal("Rejected", row!.Status); + Assert.Equal("ver.carol", row.VerifierUser); + Assert.Equal("no", row.VerifierComment); + } + + [SkippableFact] + public async Task TryMarkRejectedAsync_ExpiredRow_ReturnsFalse() + { + Skip.IfNot(_fixture.Available, _fixture.SkipReason); + + var siteId = NewSiteId(); + await using var context = CreateContext(); + var repo = new SecuredWriteRepository(context); + + var id = await repo.AddAsync(NewRow(siteId, status: "Pending")); + Assert.True(await repo.TryMarkExpiredAsync(id, DateTime.UtcNow)); + + // Already Expired -> the conditional WHERE Status='Pending' matches no row. + Assert.False(await repo.TryMarkRejectedAsync(id, "ver.bob", null, DateTime.UtcNow)); + + await using var readContext = CreateContext(); + var row = await new SecuredWriteRepository(readContext).GetAsync(id); + Assert.Equal("Expired", row!.Status); + Assert.Null(row.VerifierUser); + } + [SkippableFact] public async Task CountAsync_FiltersLikeQueryAsync() { diff --git a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/SecuredWriteHandlerTests.cs b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/SecuredWriteHandlerTests.cs index 05c6dc22..0c6cf54c 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/SecuredWriteHandlerTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/SecuredWriteHandlerTests.cs @@ -270,6 +270,29 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable return row; } + /// + /// Arms the reject compare-and-swap to succeed (this verifier wins the + /// Pending->Rejected transition). NSubstitute's unarmed default for the + /// bool-returning CAS is false, i.e. "lost the race", so every reject test + /// that expects the decision to land must arm it explicitly (arch-review + /// 2026-08-19 P6-1). + /// + private void ArmRejectCasSuccess(long id) => + _securedWriteRepo.TryMarkRejectedAsync( + id, Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(true); + + /// + /// Lets the best-effort audit path settle, then asserts NOTHING was written. Audit + /// emission is awaited inline before the handler replies, so the reply itself is the + /// happens-before edge; the short sleep is belt-and-braces against a late writer. + /// + private static void AssertNoAuditRows(List captured) + { + Thread.Sleep(250); + Assert.Empty(captured); + } + [Fact] public void Reject_BySubmittingUser_ReturnsError_StatusUnchanged() { @@ -284,17 +307,15 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable var response = ExpectMsg(TimeSpan.FromSeconds(5)); Assert.Equal("COMMAND_FAILED", response.ErrorCode); _securedWriteRepo.DidNotReceiveWithAnyArgs().UpdateAsync(default!, default); + _securedWriteRepo.DidNotReceiveWithAnyArgs().TryMarkRejectedAsync( + default, default!, default, default, default); } [Fact] - public void Reject_ByDifferentUser_FlipsStatusToRejected() + public void Reject_ByDifferentUser_CasFlipsStatusToRejected_NoBlindUpdate() { SeedPendingWrite(7, operatorUser: "alice"); - - PendingSecuredWrite? updated = null; - _securedWriteRepo - .When(r => r.UpdateAsync(Arg.Any(), Arg.Any())) - .Do(ci => updated = ci.Arg()); + ArmRejectCasSuccess(7); var actor = CreateActor(); var envelope = Envelope(new RejectSecuredWriteCommand(7, "not authorized"), "bob", "Verifier"); @@ -303,13 +324,21 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable var response = ExpectMsg(TimeSpan.FromSeconds(5)); Assert.Equal(envelope.CorrelationId, response.CorrelationId); - Assert.NotNull(updated); - Assert.Equal("Rejected", updated!.Status); - Assert.Equal("bob", updated.VerifierUser); - Assert.Equal("not authorized", updated.VerifierComment); - Assert.NotNull(updated.DecidedAtUtc); - // Operator fields are preserved (fully-populated update). - Assert.Equal("alice", updated.OperatorUser); + + // The decision goes through the conditional Pending->Rejected CAS, exactly once, + // carrying the verifier identity + comment (arch-review 2026-08-19 P6-1). + _securedWriteRepo.Received(1).TryMarkRejectedAsync( + 7, "bob", "not authorized", Arg.Any(), Arg.Any()); + // The blind full-row overwrite is gone — it could land mid-relay on a row a + // concurrent approve already owned. + _securedWriteRepo.DidNotReceiveWithAnyArgs().UpdateAsync(default!, default); + + // The projected row reflects the committed decision. + Assert.Contains("Rejected", response.JsonData); + Assert.Contains("bob", response.JsonData); + Assert.Contains("not authorized", response.JsonData); + // Operator fields are preserved. + Assert.Contains("alice", response.JsonData); } [Fact] @@ -317,6 +346,7 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable { var row = SeedPendingWrite(7, operatorUser: "alice"); row.Status = "Approved"; + var captured = CaptureAuditEvents(); var actor = CreateActor(); var envelope = Envelope(new RejectSecuredWriteCommand(7, null), "bob", "Verifier"); @@ -326,6 +356,79 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable var response = ExpectMsg(TimeSpan.FromSeconds(5)); Assert.Equal("COMMAND_FAILED", response.ErrorCode); _securedWriteRepo.DidNotReceiveWithAnyArgs().UpdateAsync(default!, default); + _securedWriteRepo.DidNotReceiveWithAnyArgs().TryMarkRejectedAsync( + default, default!, default, default, default); + // Nothing was decided, so no Reject audit row may exist. + AssertNoAuditRows(captured); + } + + [Fact] + public void Reject_WhenCasLosesRace_ReturnsConflict_WritesNothing_NoRejectAudit() + { + // The P6-1 race, from the reject side: the status read at the top of the handler + // saw Pending, but a concurrent approve won the Pending->Approved CAS and entered + // its multi-second WriteTagAsync relay before this reject reached the store. The + // reject must lose (CAS returns false), record NOTHING — no status write, no + // audit row — and report the status that actually won, so the two-person-control + // record can never say Rejected for a write that physically executed. + SeedPendingWrite(7, operatorUser: "alice"); + _securedWriteRepo.TryMarkRejectedAsync( + 7, Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(false); + // First read (the handler's stale-read guard) sees Pending; the post-CAS re-read + // observes the status the concurrent approve committed. + var stalePending = new PendingSecuredWrite + { + Id = 7, SiteId = "SITE1", ConnectionName = "Mx1", TagPath = "Tag.A", + ValueJson = "true", ValueType = "Boolean", Status = "Pending", + OperatorUser = "alice", SubmittedAtUtc = DateTime.UtcNow + }; + var approvedByRival = new PendingSecuredWrite + { + Id = 7, SiteId = "SITE1", ConnectionName = "Mx1", TagPath = "Tag.A", + ValueJson = "true", ValueType = "Boolean", Status = "Approved", + OperatorUser = "alice", VerifierUser = "carol", + SubmittedAtUtc = DateTime.UtcNow, DecidedAtUtc = DateTime.UtcNow + }; + _securedWriteRepo.GetAsync(7, Arg.Any()) + .Returns(stalePending, approvedByRival); + var captured = CaptureAuditEvents(); + + var actor = CreateActor(); + actor.Tell(Envelope(new RejectSecuredWriteCommand(7, "no"), "bob", "Verifier")); + + var response = ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.Equal("COMMAND_FAILED", response.ErrorCode); + // The conflict names the status that won the race. + Assert.Contains("Approved", response.Error); + Assert.Contains("cannot be rejected", response.Error); + + _securedWriteRepo.Received(1).TryMarkRejectedAsync( + 7, "bob", "no", Arg.Any(), Arg.Any()); + _securedWriteRepo.DidNotReceiveWithAnyArgs().UpdateAsync(default!, default); + AssertNoAuditRows(captured); + } + + [Fact] + public void Approve_OnAlreadyRejectedRow_ReturnsError_NoCas_NoRelay_NoAudit() + { + // The mirror case: a rejected row can never be approved/executed afterwards. + var row = SeedPendingWrite(7, operatorUser: "alice"); + row.Status = "Rejected"; + row.VerifierUser = "carol"; + var captured = CaptureAuditEvents(); + + var actor = CreateActor(); + actor.Tell(Envelope(new ApproveSecuredWriteCommand(7, "ok"), "bob", "Verifier")); + + var response = ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.Equal("COMMAND_FAILED", response.ErrorCode); + Assert.Contains("Rejected", response.Error); + _securedWriteRepo.DidNotReceiveWithAnyArgs().TryMarkApprovedAsync( + default, default!, default, default, default); + _securedWriteRepo.DidNotReceiveWithAnyArgs().UpdateAsync(default!, default); + Assert.Equal(0, _comms.CallCount); // the device write is NEVER relayed + AssertNoAuditRows(captured); } [Fact] @@ -503,6 +606,7 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable _securedWriteRepo.TryMarkApprovedAsync( 7, Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) .Returns(false); + var captured = CaptureAuditEvents(); var actor = CreateActor(); var envelope = Envelope(new ApproveSecuredWriteCommand(7, "race"), "bob", "Verifier"); @@ -513,6 +617,9 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable Assert.Equal("COMMAND_FAILED", response.ErrorCode); Assert.Contains("already decided", response.Error); Assert.Equal(0, _comms.CallCount); + _securedWriteRepo.DidNotReceiveWithAnyArgs().UpdateAsync(default!, default); + // A lost approve decides nothing, so it may leave no Approve audit row behind. + AssertNoAuditRows(captured); } [Fact] @@ -751,8 +858,10 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable var resp = ExpectMsg(TimeSpan.FromSeconds(5)); Assert.Contains("expired", resp.Error, StringComparison.OrdinalIgnoreCase); - // The reject path never overwrites the row (the CAS expiry owns the transition). + // The reject path never writes the row (the CAS expiry owns the transition). _securedWriteRepo.DidNotReceiveWithAnyArgs().UpdateAsync(default!, default); + _securedWriteRepo.DidNotReceiveWithAnyArgs().TryMarkRejectedAsync( + default, default!, default, default, default); _securedWriteRepo.Received(1).TryMarkExpiredAsync(1, Arg.Any(), Arg.Any()); } @@ -829,6 +938,7 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable public void Reject_EmitsOneRejectAuditRow_WithVerifier() { SeedPendingWrite(7, operatorUser: "alice"); + ArmRejectCasSuccess(7); var captured = CaptureAuditEvents(); var actor = CreateActor(); @@ -1003,6 +1113,8 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable actor.Tell(ScopedEnvelope(new RejectSecuredWriteCommand(42, "no"), "bob", ["3"], "Verifier")); ExpectMsg(TimeSpan.FromSeconds(5)); _securedWriteRepo.DidNotReceiveWithAnyArgs().UpdateAsync(default!, default); + _securedWriteRepo.DidNotReceiveWithAnyArgs().TryMarkRejectedAsync( + default, default!, default, default, default); } [Fact]