feat(management): opportunistic expiry sweep on secured-write list (arch-review S2)

Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
This commit is contained in:
Joseph Doherty
2026-07-10 05:39:29 -04:00
parent 064a7d9415
commit 03295e91bb
3 changed files with 53 additions and 1 deletions
@@ -151,7 +151,7 @@ The two-person authorization workflow for writes through the MxAccess Gateway. B
- **RejectSecuredWriteCommand** (`Verifier` role): marks a `Pending` row `Rejected` with the verifier's reason.
- **ListSecuredWritesCommand** (any of `Operator` / `Verifier` / `Administrator`): pending queue + terminal history, global or per-site. 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.
**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.
### External Systems
@@ -1395,6 +1395,22 @@ public class ManagementActor : ReceiveActor
{
var repo = sp.GetRequiredService<ISecuredWriteRepository>();
var rows = await repo.QueryAsync(cmd.Status, cmd.SiteId, skip: 0, take: 200);
// Opportunistic expiry sweep (arch-review S2): every time the list is read, walk
// the page and expire any overdue Pending row. TryExpireIfStaleAsync self-filters
// by age (fresh rows are no-ops) and CAS-guards the transition, so this is
// deliberately lazy/opportunistic — the ManagementActor already runs on every
// central node, so a dedicated expiry-timer singleton would be redundant, and the
// compare-and-swap makes two nodes sweeping the same overdue row concurrently
// harmless (exactly one wins; the loser observes false and re-projects the row's
// freshly-Expired state). The mutation lands on the in-memory row before projection,
// so the caller sees Expired in the same response.
foreach (var row in rows)
{
if (string.Equals(row.Status, "Pending", StringComparison.Ordinal))
await TryExpireIfStaleAsync(sp, repo, row);
}
return new SecuredWriteListResult(rows.Select(ToSecuredWriteDto).ToList());
}
@@ -613,6 +613,42 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable
Assert.Contains("not found", updated.ExecutionError);
}
// ------------------------------------------------------------------------
// Opportunistic expiry sweep on list — Task 16 (arch-review S2)
// ------------------------------------------------------------------------
[Fact]
public void List_SweepsStalePendingRows_MarksExpired_CallsTryMarkExpiredOnceForStaleOnly()
{
var fresh = new PendingSecuredWrite
{
Id = 1, SiteId = "SITE1", ConnectionName = "Mx1", TagPath = "Tag.A",
ValueJson = "true", ValueType = "Boolean", Status = "Pending",
OperatorUser = "alice", SubmittedAtUtc = DateTime.UtcNow.AddHours(-1)
};
var stale = new PendingSecuredWrite
{
Id = 2, SiteId = "SITE1", ConnectionName = "Mx1", TagPath = "Tag.B",
ValueJson = "false", ValueType = "Boolean", Status = "Pending",
OperatorUser = "alice", SubmittedAtUtc = DateTime.UtcNow.AddHours(-25) // TTL default 24h
};
_securedWriteRepo.QueryAsync(null, null, 0, 200, Arg.Any<CancellationToken>())
.Returns(new List<PendingSecuredWrite> { fresh, stale });
_securedWriteRepo.TryMarkExpiredAsync(2, Arg.Any<DateTime>(), Arg.Any<CancellationToken>())
.Returns(true);
var actor = CreateActor();
actor.Tell(Envelope(new ListSecuredWritesCommand(null, null), "carol", "Operator"));
var resp = ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
// The stale row surfaces as Expired in the projection; the fresh row stays Pending.
Assert.Contains("\"id\":2", resp.JsonData);
Assert.Contains("Expired", resp.JsonData);
// Exactly one CAS — only the overdue row.
_securedWriteRepo.Received(1).TryMarkExpiredAsync(2, Arg.Any<DateTime>(), Arg.Any<CancellationToken>());
_securedWriteRepo.DidNotReceive().TryMarkExpiredAsync(1, Arg.Any<DateTime>(), Arg.Any<CancellationToken>());
}
// ------------------------------------------------------------------------
// Server-side TTL expiry — Task 15 (arch-review S2)
// ------------------------------------------------------------------------