fix(alarms): native ack refused off-Primary, commented re-ack reported, real UnshelveTime (#597)

- Native-condition Acknowledge on a non-Primary node returns BadInvalidState (nothing
  routed/applied/reported) via NativeAlarmAckPrimaryGate, fed from
  IDriverPrimaryRoleSource by OtOpcUaServerHostedService.
- A re-ack carrying a comment reports one comment event (Acked/Confirmed unchanged).
- TimedShelve applies its requested duration and the scripted projection carries
  UnshelveAtUtc, so ShelvingState/UnshelveTime counts down instead of ~24.8 days.
- Native re-ack still routed to the driver (deliberate: documented retry path), pinned.
- AdminUI /alerts client ack/shelve path test; docs/AlarmTracking.md decisions incl.
  accepted SDK EventsMonitored residual (item 6).
- Also #599/03/S20: RemoveEquipmentSubtree severs inverse Organizes edges.

Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
This commit is contained in:
Joseph Doherty
2026-10-08 07:12:29 -04:00
parent 0bd3ec00e4
commit bd63810645
10 changed files with 459 additions and 51 deletions
@@ -0,0 +1,81 @@
using Akka.Hosting;
using Akka.TestKit.Xunit2;
using Shouldly;
using Xunit;
using ZB.MOM.WW.OtOpcUa.AdminUI.Clients;
using ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Acls;
using ZB.MOM.WW.OtOpcUa.Commons.Messages.Admin;
using ZB.MOM.WW.OtOpcUa.Commons.Messages.Mesh;
using ZB.MOM.WW.OtOpcUa.Commons.OpcUa;
using ZB.MOM.WW.OtOpcUa.ControlPlane;
using ZB.MOM.WW.OtOpcUa.ControlPlane.AdminOperations;
using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests;
/// <summary>
/// Gitea #597 — the AdminUI <c>/alerts</c> acknowledge / shelve path, from the page's client call down to
/// the command the cluster receives: <see cref="AdminOperationsClient"/> (what <c>Alerts.razor</c>'s
/// per-row buttons call) Asks the real <see cref="AdminOperationsActor"/> registered under
/// <see cref="AdminOperationsActorKey"/>, which maps the request to an <see cref="AlarmCommand"/> and hands
/// it to the mesh transport. Until now only the actor half had a test; the client seam the page goes
/// through had none.
/// </summary>
/// <remarks>
/// The razor click handler itself is not driven here — this repo has no bUnit. What it adds on top of the
/// client call is the operator name and <c>comment: null</c>, both reproduced below exactly as the page
/// passes them.
/// </remarks>
public sealed class AlertsAckPathTests : TestKit
{
private (AdminOperationsClient Client, Akka.TestKit.TestProbe Mesh) Wire()
{
var mesh = CreateTestProbe("mesh-router");
var actor = Sys.ActorOf(AdminOperationsActor.Props(
AclScopeTestDb.Factory($"alerts-ack-{Guid.NewGuid():N}"),
coordinator: CreateTestProbe("coordinator").Ref,
probes: Enumerable.Empty<IDriverProbe>(),
meshRouter: mesh.Ref));
var registry = new ActorRegistry();
registry.Register<AdminOperationsActorKey>(actor);
return (new AdminOperationsClient(registry), mesh);
}
/// <summary>The page's Acknowledge reaches the cluster as an <c>Acknowledge</c> AlarmCommand on the
/// <c>alarm-commands</c> topic carrying the alarm id and the operator, and the page is told it was sent.</summary>
[Fact]
public async Task Acknowledge_from_the_alerts_page_dispatches_an_Acknowledge_command_and_reports_sent()
{
var (client, mesh) = Wire();
var result = await client.AcknowledgeAlarmAsync("alarm-42", "operator-jo", comment: null, CancellationToken.None);
result.Ok.ShouldBeTrue();
result.Message.ShouldBeNull();
var sent = mesh.ExpectMsg<MeshCommand>(TimeSpan.FromSeconds(5));
sent.Topic.ShouldBe(AlarmCommandsTopic.Name);
var command = sent.Message.ShouldBeOfType<AlarmCommand>();
command.AlarmId.ShouldBe("alarm-42");
command.Operation.ShouldBe("Acknowledge");
command.User.ShouldBe("operator-jo");
command.Comment.ShouldBeNull();
command.UnshelveAtUtc.ShouldBeNull();
}
/// <summary>The page's timed shelve carries its expiry through to the command — the instant the engine
/// projects as the node's <c>UnshelveTime</c>.</summary>
[Fact]
public async Task Timed_shelve_from_the_alerts_page_carries_its_expiry_to_the_command()
{
var (client, mesh) = Wire();
var until = new DateTime(2026, 10, 8, 12, 30, 0, DateTimeKind.Utc);
var result = await client.ShelveAlarmAsync(
"alarm-7", "operator-kim", ShelveKind.Timed, unshelveAtUtc: until, comment: null, CancellationToken.None);
result.Ok.ShouldBeTrue();
var command = mesh.ExpectMsg<MeshCommand>(TimeSpan.FromSeconds(5)).Message.ShouldBeOfType<AlarmCommand>();
command.Operation.ShouldBe("TimedShelve");
command.UnshelveAtUtc.ShouldBe(until);
}
}
@@ -266,10 +266,11 @@ public sealed class ConditionMethodEventTests
/// reports exactly one unshelve event after the one shelve event.
/// </summary>
/// <remarks>
/// The method path applies the shelve with <c>shelvingTime 0</c>, as the projection does, so the expiry is
/// the engine's (computed from the command's <c>UnshelveAtUtc</c>), never the SDK timer's — a node timer
/// would only route an Unshelve back to the engine anyway. The stand-in engine here schedules its expiry
/// from the real <c>UnshelveAtUtc</c> the method handler computed.
/// The expiry is the engine's (computed from the command's <c>UnshelveAtUtc</c>). Since Gitea #597 the node
/// is shelved for the real duration, so the SDK's own timer fires at the same instant — but it only routes
/// an Unshelve back to the engine (ignored by this stand-in, a no-op on an unshelved engine alarm) and
/// changes no node state, so it adds no event. The stand-in engine here schedules its expiry from the real
/// <c>UnshelveAtUtc</c> the method handler computed.
/// </remarks>
[Fact]
public async Task A_scripted_timed_shelve_expires_with_exactly_one_unshelve_event()
@@ -326,17 +327,157 @@ public sealed class ConditionMethodEventTests
}
/// <summary>
/// Acknowledging an acknowledged condition reports nothing, and does not flip a scripted condition's
/// Confirmed out of step with its engine.
/// A scripted condition's events carry the real remaining shelve time in
/// <c>ShelvingState/UnshelveTime</c> — after a client TimedShelve, after the engine's re-projection of it,
/// and after an engine-driven timed shelve (the AdminUI <c>/alerts</c> path, which never calls the method).
/// </summary>
/// <remarks>
/// The SDK accepts a re-ack and sets Confirmed false again; the engine ignores it. The scripted leg
/// acknowledges and confirms first, so the flip would be visible on the sentinel.
/// Gitea #597: both paths applied the shelve with <c>shelvingTime 0</c>, which the SDK reads as "the
/// longest shelve allowed" — <c>MaxTimeShelved</c>, <c>int.MaxValue</c> ms — so every timed shelve
/// advertised ~24.8 days remaining while the engine expired it on time. Read through the event select
/// clause rather than a Read of the child node: the condition's children carry their type's
/// instance-declaration NodeIds (<c>i=9189</c> for UnshelveTime), so a direct Read reaches the type, not
/// this condition.
/// </remarks>
[Fact]
public async Task A_scripted_timed_shelve_reports_the_real_remaining_unshelve_time()
{
var ct = TestContext.Current.CancellationToken;
var rig = await StartAsync("CondUnshelveTime", encrypted: false, ct);
try
{
rig.Enforce(true, [Key()], ClusterGrant(KeyPrincipal, "acl-key", All));
rig.MaterialiseScripted();
var current = PreState("TimedShelve", native: false) with { Message = "base" };
rig.Nm.AlarmCommandRouter = cmd =>
{
if (cmd.Operation != "TimedShelve") return;
// The engine's answer carries its own expiry — what ScriptedAlarmHostActor projects.
current = current with { Shelving = AlarmShelvingKind.Timed, UnshelveAtUtc = cmd.UnshelveAtUtc };
rig.ProjectLater(native: false, current);
};
using var session = await rig.ConnectAsync(anonymous: false, ct);
var events = new RecordedEvents();
var subscription = await RecordedEvents.SubscribeAsync(session, events, ct);
var item = RecordedEvents.AddEventItem(subscription, ObjectIds.Server);
await subscription.ApplyChangesAsync(ct);
double RemainingMs(string message) =>
(double)events.ConditionEvents(item, message)[^1][RecordedEvents.UnshelveTimeField].Value;
rig.Project(false, current);
await RecordedEvents.WaitUntilAsync(() => events.Has(item, "base"));
RemainingMs("base").ShouldBe(0d, "unshelved");
// The method's one event carries the shelve's remaining time …
await CallAsync(session, ConditionNodeId(session, false), "TimedShelve", null, ct, shelvingTimeMs: 60_000);
await rig.DrainProjectionsAsync();
await RecordedEvents.WaitUntilAsync(() => events.ConditionEvents(item, "base").Count == 2);
RemainingMs("base").ShouldBeInRange(50_000, 60_000, "the TimedShelve method event");
// … and so does a later transition, after the engine's re-projection re-applied the shelve.
rig.Project(false, current = current with { Message = "after-reprojection" });
await RecordedEvents.WaitUntilAsync(() => events.Has(item, "after-reprojection"));
RemainingMs("after-reprojection").ShouldBeInRange(50_000, 60_000, "after the engine's re-projection");
// Engine-driven (the AdminUI path): unshelve, then a timed shelve that reaches the node only as a
// projection, never through the method.
rig.Project(false, current = current with { Shelving = AlarmShelvingKind.Unshelved, UnshelveAtUtc = null, Message = "unshelved" });
await RecordedEvents.WaitUntilAsync(() => events.Has(item, "unshelved"));
RemainingMs("unshelved").ShouldBe(0d);
rig.Project(false, current with
{
Shelving = AlarmShelvingKind.Timed, UnshelveAtUtc = DateTime.UtcNow.AddSeconds(30), Message = "projected",
});
await RecordedEvents.WaitUntilAsync(() => events.Has(item, "projected"));
RemainingMs("projected").ShouldBeInRange(20_000, 30_000, "a projected timed shelve");
await subscription.DeleteAsync(true, ct);
}
finally { await rig.DisposeAsync(); }
}
/// <summary>
/// On a node that is not the redundancy Primary, an OPC UA Acknowledge of a NATIVE condition is refused
/// with <c>BadInvalidState</c>: nothing is routed to the driver, nothing is applied, nothing is reported.
/// On the Primary the same call is accepted (the positive control).
/// </summary>
/// <remarks>
/// Gitea #597 decision. The ack used to be applied locally and reported, while <c>DriverHostActor</c>'s
/// Primary gate dropped the route upstream — so the driver never acknowledged, its next transition
/// projected <c>Acked = false</c>, and a client acking against the Secondary saw acked → unacked. An
/// honest refusal replaces an ack that visibly sticks and then reverts. Scripted conditions are
/// unaffected: their engine runs on both nodes, so the Secondary's ack is real.
/// </remarks>
[Fact]
public async Task A_native_acknowledge_on_a_non_primary_node_is_refused()
{
var ct = TestContext.Current.CancellationToken;
var rig = await StartAsync("CondNativeAckSecondary", encrypted: false, ct);
try
{
rig.Enforce(true, [Key()], ClusterGrant(KeyPrincipal, "acl-key", All));
rig.MaterialiseNative();
var current = PreState("Acknowledge", native: true) with { Message = "base" };
WireSimulatedOwners(rig, native: true, () => current, s => current = s);
var routed = 0;
var driverAck = rig.Nm.NativeAlarmAckRouter!;
rig.Nm.NativeAlarmAckRouter = ack => { Interlocked.Increment(ref routed); driverAck(ack); };
var primary = false;
rig.Nm.NativeAlarmAckPrimaryGate = () => primary;
using var session = await rig.ConnectAsync(anonymous: false, ct);
var events = new RecordedEvents();
var subscription = await RecordedEvents.SubscribeAsync(session, events, ct);
var item = RecordedEvents.AddEventItem(subscription, ObjectIds.Server);
await subscription.ApplyChangesAsync(ct);
rig.Project(native: true, current);
await RecordedEvents.WaitUntilAsync(() => events.Has(item, "base"));
var condition = ConditionNodeId(session, native: true);
var refused = await Should.ThrowAsync<ServiceResultException>(
CallAsync(session, condition, "Acknowledge", events.LatestEventId(item, "base"), ct));
refused.StatusCode.ShouldBe(StatusCodes.BadInvalidState);
routed.ShouldBe(0, "a refused ack reaches no driver");
rig.Nm.TryGetAlarmCondition(RawAlarmPath, AddressSpaceRealm.Raw)!.AckedState.Id.Value
.ShouldBeFalse("a refused ack applies nothing");
// Positive control: the same call on the Primary is accepted, routed and reported.
primary = true;
await CallAsync(session, condition, "Acknowledge", events.LatestEventId(item, "base"), ct);
await rig.DrainProjectionsAsync();
routed.ShouldBe(1);
rig.Project(native: true, current = current with { Message = "sentinel" });
await RecordedEvents.WaitUntilAsync(() => events.Has(item, "sentinel"));
var baseEvents = events.ConditionEvents(item, "base");
baseEvents.Count.ShouldBe(2, "the base transition plus the Primary's one ack event; the refusal reported nothing");
baseEvents[1][RecordedEvents.AckedField].Value.ShouldBe(true);
await subscription.DeleteAsync(true, ct);
}
finally { await rig.DisposeAsync(); }
}
/// <summary>
/// Acknowledging an acknowledged condition WITHOUT a comment reports nothing, and does not flip a scripted
/// condition's Confirmed out of step with its engine. With a comment it reports exactly one event carrying
/// that comment — the comment is a Part 9 change in its own right, as AddComment is (Gitea #597) — and
/// still leaves Acked/Confirmed alone.
/// </summary>
/// <remarks>
/// The SDK accepts a re-ack, writes its comment, and sets Confirmed false again; the engine ignores it.
/// The scripted leg acknowledges and confirms first, so the flip would be visible on the sentinel. Before
/// #597 a commented re-ack wrote the comment to the node but reported nothing, so no subscriber ever saw
/// it.
/// </remarks>
[Theory]
[InlineData(false)]
[InlineData(true)]
public async Task A_re_acknowledge_reports_nothing_and_changes_nothing(bool native)
[InlineData(false, null)]
[InlineData(true, null)]
[InlineData(false, "again")]
[InlineData(true, "again")]
public async Task A_re_acknowledge_reports_only_its_comment_and_changes_nothing_else(bool native, string? reAckComment)
{
var ct = TestContext.Current.CancellationToken;
var rig = await StartAsync($"CondReAck{native}", encrypted: false, ct);
@@ -347,6 +488,9 @@ public sealed class ConditionMethodEventTests
rig.MaterialiseScripted();
var current = PreState("Acknowledge", native) with { Message = "base" };
WireSimulatedOwners(rig, native, () => current, s => current = s);
var routedAcks = 0;
var driverAck = rig.Nm.NativeAlarmAckRouter!;
rig.Nm.NativeAlarmAckRouter = ack => { Interlocked.Increment(ref routedAcks); driverAck(ack); };
using var session = await rig.ConnectAsync(anonymous: false, ct);
var events = new RecordedEvents();
@@ -370,13 +514,32 @@ public sealed class ConditionMethodEventTests
}
await RecordedEvents.WaitUntilAsync(() => events.ConditionEvents(item, "base").Count == expected);
await CallAsync(session, condition, "Acknowledge", events.LatestEventId(item, "base"), ct);
var routedBefore = routedAcks;
await CallAsync(session, condition, "Acknowledge", events.LatestEventId(item, "base"), ct, ackComment: reAckComment);
await rig.DrainProjectionsAsync();
rig.Project(native, current = current with { Message = "sentinel" });
await RecordedEvents.WaitUntilAsync(() => events.Has(item, "sentinel"));
events.ConditionEvents(item, "base").Count.ShouldBe(expected, "the re-ack must report nothing");
var baseEvents = events.ConditionEvents(item, "base");
if (reAckComment is null)
{
baseEvents.Count.ShouldBe(expected, "an uncommented re-ack must report nothing");
}
else
{
baseEvents.Count.ShouldBe(expected + 1, "a commented re-ack reports exactly one comment event");
var commented = baseEvents[^1];
(commented[RecordedEvents.CommentField].Value as LocalizedText)?.Text.ShouldBe(reAckComment);
commented[RecordedEvents.AckedField].Value.ShouldBe(true);
commented[RecordedEvents.ConfirmedField].Value.ShouldBe(true, "the commented re-ack must not unconfirm the node");
}
// A native re-ack is still routed to the driver: the route is fire-and-forget, so a node showing
// Acked does not prove the upstream system acknowledged, and re-acking is the documented retry
// (docs/AlarmTracking.md, "Best-effort during reconnect"). Pinned so it is changed deliberately.
if (native) (routedAcks - routedBefore).ShouldBe(1);
var sentinel = events.ConditionEvents(item, "sentinel");
sentinel.Count.ShouldBe(1);
sentinel[0][RecordedEvents.AckedField].Value.ShouldBe(true);
@@ -467,11 +630,13 @@ public sealed class ConditionMethodEventTests
/// condition's method declarations — the shelve methods included); throws on a bad result.
/// </summary>
private static async Task CallAsync(
ISession session, NodeId condition, string method, byte[]? eventId, CancellationToken ct, double shelvingTimeMs = 60_000)
ISession session, NodeId condition, string method, byte[]? eventId, CancellationToken ct, double shelvingTimeMs = 60_000,
string? ackComment = "ack")
{
var (methodId, args) = method switch
{
"Acknowledge" => (MethodIds.AcknowledgeableConditionType_Acknowledge, new object[] { eventId!, new LocalizedText("ack") }),
"Acknowledge" => (MethodIds.AcknowledgeableConditionType_Acknowledge,
new object[] { eventId!, ackComment is null ? LocalizedText.Null : new LocalizedText(ackComment) }),
"Confirm" => (MethodIds.AcknowledgeableConditionType_Confirm, [eventId!, new LocalizedText("confirm")]),
"AddComment" => (MethodIds.ConditionType_AddComment, [eventId!, new LocalizedText("note")]),
"Enable" => (MethodIds.ConditionType_Enable, []),
@@ -292,6 +292,7 @@ internal sealed class RecordedEvents
public const int CommentField = 7;
public const int ChangesField = 8;
public const int SourceNodeField = 9;
public const int UnshelveTimeField = 10;
private readonly object _gate = new();
private readonly List<(uint Handle, VariantCollection Fields)> _events = [];
@@ -349,6 +350,8 @@ internal sealed class RecordedEvents
filter.AddSelectClause(ObjectTypeIds.ConditionType, BrowseNames.Comment);
filter.AddSelectClause(ObjectTypeIds.GeneralModelChangeEventType, BrowseNames.Changes);
filter.AddSelectClause(ObjectTypes.BaseEventType, BrowseNames.SourceNode);
filter.SelectClauses.Add(new SimpleAttributeOperand(
ObjectTypeIds.AlarmConditionType, [BrowseNames.ShelvingState, BrowseNames.UnshelveTime]));
var item = new MonitoredItem(subscription.DefaultItem)
{