fix(alarms,acl): one condition event per Part 9 method call; gate Server-object model-change/audit events (#587, #588)

#587 — a client Acknowledge/Confirm/AddComment/Shelve/Enable/Disable emitted no condition event: the SDK's
ReportStateChange is gated on AutoReportStateChanges (never set), and the engine re-projection found the node
already in the new state and was suppressed by the delta-gate. WireMethodTransitionReporting now wraps each
method's SDK handler and, on Good, reports exactly once via ReportConditionEvent, under Lock so the routed
re-projection cannot race in ahead of the SDK applying the state. Independent of the SDK's imprecise
AreEventsMonitored bookkeeping. The shelve methods' state is applied by the wrapper (the SDK never applies
it). Native conditions' Confirmed/Shelving are method-owned (NativeAlarmProjector's constants would re-report
every ack and undo every shelve). MaxTimeShelved set to int.MaxValue ms at materialise — Create() left it 0
and every TimedShelve failed BadShelvingTimeOutOfRange. Wrong "SDK auto-fires (E2)" comments fixed.

#588 — with ACL on, GeneralModelChange and AuditWriteUpdate events no longer go to Server.ReportEvent
(ungated to every i=2253 item): ReportToServerObject delivers them per item, a model change narrowed to the
Affected nodes the session may Browse (dropped when none), an audit write only with Read on its source plus
the SDK audit/encryption guard. Flag off: unchanged.

#586 review: materialise raises only the new condition's monitored unit (was a full O(N) reconcile per
condition ⇒ O(N²) deploys under Lock).

Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
This commit is contained in:
Joseph Doherty
2026-10-08 00:54:08 -04:00
parent 390c8269a0
commit 167a48bffa
10 changed files with 1454 additions and 66 deletions
+47 -5
View File
@@ -371,11 +371,53 @@ hosted service.
`OnTimedUnshelve` (the SDK's automatic unshelve timer) bypasses the
operator gate — it is system-initiated.
`WriteAlarmCondition` fires the Part 9 condition event only when the
incoming state differs from the node's current live state (delta-gate),
preventing the double-emit that would otherwise occur when the SDK
auto-applies the acked state and the engine re-projection fires a
duplicate event immediately after.
### One condition event per method call (Gitea #587)
Every accepted Part 9 method call — Acknowledge, Confirm, AddComment,
OneShotShelve, TimedShelve, Unshelve, Enable, Disable, on a native or a
scripted condition — reports **exactly one** condition event carrying the
new state, to folder subscribers and (through the ACL) Server-object
subscribers alike. A refused call (bad status, e.g. a denied gate, an
already-acknowledged condition, Enable/Disable on a native condition)
reports nothing.
- **The SDK does not report it.** `ConditionState.ReportStateChange` is
gated on `AutoReportStateChanges`, which this repo leaves `false` — so
before #587 a client Acknowledge produced no condition event at all (an
older comment claimed "the SDK auto-fires its own event"; it never did).
Instead the node manager wraps each method's SDK handler
(`WireMethodTransitionReporting`) and, once the handler returns Good,
reports the condition itself. That report does not depend on the SDK's
`AreEventsMonitored` bookkeeping, which is imprecise for conditions
materialised under an already-subscribed folder.
- **The SDK's shelve methods never apply shelving state** (they only call
`OnShelve`), so the wrapper applies it — with the same
`(shelved, oneShot, 0)` mapping the projection uses — before reporting.
- **The re-projection does not duplicate it.** The engine's (or driver's)
answer to the routed command reaches `WriteAlarmCondition`'s delta-gate,
which compares against the node's current state — already the new state —
and suppresses. The whole method call runs under the node-manager lock, so
a fast re-projection cannot slip in between the SDK applying the state and
the report.
- **Native conditions: Confirmed and Shelving are method-owned.**
`NativeAlarmProjector` models neither (it always says `Confirmed = true`,
`Unshelved`), while the SDK's Acknowledge sets `Confirmed = false`. So for a
native condition `WriteAlarmCondition` neither projects nor compares those
two fields — otherwise the driver's echo of an ack would report a second
event, and the next driver transition would undo (and report) a local
shelve. A native condition starts Confirmed; Confirm and the shelve methods
are still not routed to the driver (local only, as before), but they now
stick. Scripted conditions are unchanged: the engine models both fields.
- **TimedShelve works.** `Create()` left `MaxTimeShelved` at 0, and the SDK
rejects any shelving time above it, so every client TimedShelve failed with
`BadShelvingTimeOutOfRange`. It is now `int.MaxValue` ms (≈24.8 days — the
longest the SDK's own shelving timer can hold).
- Method **audit** events (`AuditConditionAcknowledgeEventType`, …) are still
the SDK's, delivered only while the server is auditing over an encrypted
channel.
Engine-driven transitions are unaffected: `WriteAlarmCondition` still fires
whenever the incoming state differs from the node's current state.
### AdminUI path
+1 -1
View File
@@ -358,7 +358,7 @@ On allow, the handler publishes a `Commons.OpcUa.AlarmCommand` (containing comma
### Delta-gate de-duplication
`WriteAlarmCondition` fires a Part 9 condition event only when the incoming state differs from the node's current live state. This suppresses the double-emit that would otherwise occur when the OPC UA SDK auto-applies the acked state on the node and the engine's re-projection then attempts to fire a duplicate event.
`WriteAlarmCondition` fires a Part 9 condition event only when the incoming state differs from the node's current live state. For an OPC UA method call (Acknowledge, Confirm, AddComment, the shelve methods, Enable/Disable) the SDK applies the new state to the node, and the node manager then reports **the one** condition event for it from the method path (Gitea #587 — the SDK reports none of its own, since `AutoReportStateChanges` stays false; the shelve methods' state is applied by the node manager, because the SDK's never applies it). The engine's later re-projection of the same transition finds the node already in that state and is suppressed, so each accepted call yields exactly one event. See `docs/AlarmTracking.md` §"One condition event per method call".
### AdminUI path
+27 -8
View File
@@ -564,18 +564,37 @@ Behaviours worth knowing before enabling it:
condition reports through itself (`AuditConditionAcknowledgeEventType`, Comment, Enable, Shelving)
reach a Server-object item only while the server is auditing, over SignAndEncrypt or https-binary,
which is exactly the SDK's own guard. And while any Server-object item exists, every condition counts as
**monitored** (`ConditionState.EventsMonitored()`), so the SDK still reports its method-driven state
changes on a server whose only subscribers are Server-object items. Both were caught in review: the
**monitored** (`ConditionState.EventsMonitored()`), which gates the SDK's method **audit** events on a
server whose only subscribers are Server-object items. Both were caught in review: the
first cut queued `i=8944` to every Server-object item with auditing off, and left conditions
unmonitored. The second also needed the SDK's `AddRootNotifier` neutralised: its "subscribe existing
all-events items with `unsubscribe: true`" bug would otherwise take the unit back from every
condition rebuilt under a live item.
condition rebuilt under a live item. (Method-driven **state-change** events no longer depend on this
bookkeeping at all — the node manager reports them itself, Gitea #587; see `docs/AlarmTracking.md`.
The SDK's counter stays imprecise in one known way — a folder subscriber's unit never reaches a
condition materialised under that folder later, yet its unsubscribe still lowers it — which can now
only cost an audit event, and only while auditing is on.)
- Materialising a condition raises only **that** condition's monitored unit; the full reconcile (every
condition) runs on removal, rebuild and Server-object subscribe/delete. A deploy of N alarms under a
live Server-object item is therefore N bookkeeping steps, not N²/2 under the node-manager lock.
Residual, not closed by #586: **audit and model-change events** (`AuditWriteUpdateEvent`,
`GeneralModelChangeEvent`, …) are still reported straight to the Server object and reach every
Server-object item ungated. They are delivered only when auditing / model-change reporting is on (and
audit events only over an encrypted channel), and a node manager has no seam on the core Server
object's fan-out to filter them.
**Model-change and audit events on the Server object are gated too** (Gitea #588). The node manager's
own server-wide events — `GeneralModelChangeEvent` (NodeAdded / NodeDeleted / DataTypeChanged) and the
`AuditWriteUpdateEvent` of a rejected device write — used to go straight to `Server.ReportEvent`, where
the core Server object queued them to every Server-object item ungated: a session with no grant learned
the RawPath / UNS path of every node added, deleted or retyped, and (with auditing on) the written and
prior values of a rejected write. With the flag **on**, the node manager now delivers them itself, item by
item, to the Server-object items it already holds for #586 (the same audience — only Server-object items
ever received them):
- A **model-change** event keeps only the changes whose `Affected` node the session may **Browse**
(ancestors of a granted node included, as for Browse itself). Every event the server raises today carries
one change, so in practice it is delivered whole or not at all; a multi-change event would be delivered
as a copy holding only the visible changes, with a neutral `Message` (the original names the nodes).
- An **audit write** event needs `Read` on its source node — it carries the node's prior value and the
attempted one — on top of the SDK's own guard (server auditing on, SignAndEncrypt or https-binary).
Auditing stays **off by default**: this repo never sets `ServerConfiguration.AuditingEnabled`.
- Any other server-wide event type is withheld (fail closed); the node manager raises none today.
- With the flag **off**, these events go to `Server.ReportEvent` exactly as before.
- **An ancestor of a granted node is browsable**, or no narrow grant would be reachable: a client
arrives at a deep node by browsing its ancestors, and the trie otherwise inherits downward only. Note
what this necessarily discloses — the *names* of the folders on the path to a granted node are visible