dohertj2/lmxopcua
1
0
Fork 0
Code Issues 14 Pull Requests Actions Packages Projects Releases Wiki Activity

bug(alarms): an active scripted alarm under-reports as normal after a full-rebuild deploy #487

New Issue
Closed
opened 2026-07-21 03:36:40 -04:00 by dohertj2 · 1 comment
dohertj2 commented 2026-07-21 03:36:40 -04:00
Owner
Copy Link
Copy Source

Confirmed pre-existing bug, deferred out of v3 Batch 4 because the fix carries real risk (see below). Recorded in docs/plans/2026-07-16-v3-batch4-PR.md under "Documented follow-ups"; filed now so it is tracked rather than living in a plan doc.

What happens

The scripted-alarm Part 9 condition node has the same redeploy-reset race the VirtualTag ReassertValue fix closed:

  • RebuildAddressSpace clears _alarmConditions, and MaterialiseScriptedAlarms recreates each condition fresh/normal.
  • ScriptedAlarmEngine.LoadAsync reloads from persisted state and yields EmissionKind.None for a still-active condition, which OnEngineEmission drops.

So an active scripted alarm with static dependencies under-reports as normal after a full-rebuild deploy, until its next real transition. This is the known "snapshot-is-one-shot / restart-to-re-deliver" behaviour, not a Batch 4 regression.

Why it was deferred

The fix is a Core-engine emission-contract change carrying double-alarm-history risk — a naive re-emit would append a duplicate historian/AVEVA row per deploy. That is the alarm analogue of the VirtualTag M1 historian issue.

Proposed shape

A host-driven, node-only AlarmStateUpdate re-assert in OnAlarmsLoaded, using the same safe post-materialise ordering as the VirtualTag fix, reusing _engine.GetState(alarmId) + LoadedAlarmIds, and never touching the alerts topic (that is what would duplicate history).

Verified still unfixed as of master 7808faa7: ReassertValue exists only in VirtualTagActor/VirtualTagHostActor; there is no OnAlarmsLoaded equivalent.

Live-verify this one — the VirtualTag analogue was only caught on the rig, and the failure mode (an alarm silently reading normal) is invisible to unit tests that drive a transition.

**Confirmed pre-existing bug**, deferred out of v3 Batch 4 because the fix carries real risk (see below). Recorded in `docs/plans/2026-07-16-v3-batch4-PR.md` under "Documented follow-ups"; filed now so it is tracked rather than living in a plan doc. ## What happens The scripted-alarm Part 9 condition node has the same redeploy-reset race the VirtualTag `ReassertValue` fix closed: - `RebuildAddressSpace` clears `_alarmConditions`, and `MaterialiseScriptedAlarms` recreates each condition **fresh/normal**. - `ScriptedAlarmEngine.LoadAsync` reloads from persisted state and yields `EmissionKind.None` for a still-active condition, which `OnEngineEmission` drops. So an **active scripted alarm with static dependencies under-reports as normal after a full-rebuild deploy**, until its next real transition. This is the known "snapshot-is-one-shot / restart-to-re-deliver" behaviour, not a Batch 4 regression. ## Why it was deferred The fix is a Core-engine emission-contract change carrying **double-alarm-history risk** — a naive re-emit would append a duplicate historian/AVEVA row per deploy. That is the alarm analogue of the VirtualTag M1 historian issue. ## Proposed shape A host-driven, **node-only** `AlarmStateUpdate` re-assert in `OnAlarmsLoaded`, using the same safe post-materialise ordering as the VirtualTag fix, reusing `_engine.GetState(alarmId)` + `LoadedAlarmIds`, and **never touching the `alerts` topic** (that is what would duplicate history). Verified still unfixed as of `master` `7808faa7`: `ReassertValue` exists only in `VirtualTagActor`/`VirtualTagHostActor`; there is no `OnAlarmsLoaded` equivalent. Live-verify this one — the VirtualTag analogue was only caught on the rig, and the failure mode (an alarm silently reading normal) is invisible to unit tests that drive a transition.
dohertj2 commented 2026-07-26 09:07:27 -04:00
Author
Owner
Copy Link
Copy Source

Fixed on branch fix/487-scripted-alarm-reassert (commit 549c6564). Not merged yet.

What landed

ScriptedAlarmHostActor.ReassertConditionNodes, called from OnAlarmsLoaded, node-only, exactly as this issue proposed. Two deliberate deviations from the sketch:

  1. It reads a new ScriptedAlarmEngine.GetProjections() rather than GetState(alarmId) + LoadedAlarmIds. GetState returns only the Part 9 condition — no severity, no message, no input quality — so re-asserting from it would have clobbered the node's severity and Message. ScriptedAlarmProjection bundles the condition with the definition's severity, the message template resolved against the live value cache, and the last-observed worst-input quality. It carries no EmissionKind, so it is structurally impossible for the emission machinery (or a future refactor) to mistake it for a transition — that is the double-history guard, made a type property rather than a convention.
  2. Alarms in the Part 9 no-event position are skipped. One already matches what the materialise built, so writing it would be a no-op on state but would still overwrite the node's Message (the materialise seeds it with the display name). Skipping keeps a never-fired alarm byte-identical to a deploy without this pass, and an all-normal deploy writes nothing at all.

Also: the timestamp is the condition's own LastTransitionUtc, not the deploy instant. And no duplicate Part 9 events — WriteAlarmCondition's delta-gate compares against the node's current state, so a re-assert onto a freshly materialised node is a genuine delta (fires once, correctly, since the rebuild reset what clients see) while one onto a surgically-preserved node is suppressed.

Correction to this issue's premise

The issue describes the symptom as "an active scripted alarm ... under-reports as normal". On a real node that specific framing does not hold, because Active is deliberately not persisted — neither LocalDbAlarmConditionStateStore's alarm_condition_state table nor the documented contract in docs/ScriptedAlarms.md §"State persistence" carries it; the engine re-derives Active from the predicate at every LoadAsync. So a still-active predicate-driven alarm does re-emit Activated on reload and its node recovers on its own.

The bug is real, but it bites the fields that are persisted: Acked, Confirmed, Shelving, Enabled. The sharpest case, and the one used for the live gate:

An alarm that fired, cleared, and is still unacknowledged. On reload the predicate is false, so there is no transition, so EmissionKind.None. The node comes back from the materialise as acknowledged, which makes Retain = Active || !Acked false — so the condition disappears from ConditionRefresh entirely. The operator's outstanding alarm silently vanishes from the alarm list on the next deploy, while the engine still holds Unacknowledged.

That is a worse symptom than the one originally described, and it is what the fix is gated against.

Live gate — PASSED

docker-dev rig, central-2, pre-fix image vs. the rebuilt one. Scripted alarm on gateequip (MAIN), authored through the real /uns equipment Alarms modal; driven to active-then-cleared-unacked; rebuild forced by a node restart (RestoreApplied re-materialises everything).

leg pre-fix post-fix
ConditionRefresh after rebuild SA-alwaysactive absent — Retain=false present, Unacknowledged, Retain: True
timestamp — 12:55:38 = the condition's own LastTransitionUtc (matches updated_at_utc in LocalDb exactly), not the restart instant
/alerts during re-assert — empty across two restarts ⇒ no duplicate historian row

The /alerts leg was explicitly falsified rather than accepted as a bare absence: immediately afterwards the predicate was flipped back to true and a real ACTIVATED row appeared on the same open panel, proving the feed was live the whole time.

Tests

6 new host tests (ScriptedAlarmHostActorTests) + 6 new engine tests (ScriptedAlarmProjectionTests). Falsifiability checked by disabling the call: 4 of the 6 host tests go red. The other 2 (A_normal_alarm_is_not_reasserted, A_disabled_plan_is_not_reasserted) are absence guards against an over-broad fix and pass either way by design.

Full solution builds clean; Runtime.Tests 504/504, Core.ScriptedAlarms.Tests 82/82.

Docs: docs/ScriptedAlarms.md §"Post-(re)load condition-node re-assert (#487)"; the deferral note in docs/plans/2026-07-16-v3-batch4-PR.md updated to point at it.

Side-discovery filed separately as #504 (/scripts is a hard 500 when any Script.SourceHash is shorter than 12 chars — the docker-dev seed trips it).

Fixed on branch `fix/487-scripted-alarm-reassert` (commit `549c6564`). Not merged yet. ## What landed `ScriptedAlarmHostActor.ReassertConditionNodes`, called from `OnAlarmsLoaded`, node-only, exactly as this issue proposed. Two deliberate deviations from the sketch: 1. **It reads a new `ScriptedAlarmEngine.GetProjections()` rather than `GetState(alarmId)` + `LoadedAlarmIds`.** `GetState` returns only the Part 9 condition — no severity, no message, no input quality — so re-asserting from it would have clobbered the node's severity and Message. `ScriptedAlarmProjection` bundles the condition with the definition's severity, the message template resolved against the live value cache, and the last-observed worst-input quality. It carries **no `EmissionKind`**, so it is structurally impossible for the emission machinery (or a future refactor) to mistake it for a transition — that is the double-history guard, made a type property rather than a convention. 2. **Alarms in the Part 9 no-event position are skipped.** One already matches what the materialise built, so writing it would be a no-op on state but would still overwrite the node's Message (the materialise seeds it with the display name). Skipping keeps a never-fired alarm byte-identical to a deploy without this pass, and an all-normal deploy writes nothing at all. Also: the timestamp is the condition's own `LastTransitionUtc`, not the deploy instant. And no duplicate Part 9 events — `WriteAlarmCondition`'s delta-gate compares against the node's *current* state, so a re-assert onto a freshly materialised node is a genuine delta (fires once, correctly, since the rebuild reset what clients see) while one onto a surgically-preserved node is suppressed. ## Correction to this issue's premise The issue describes the symptom as *"an active scripted alarm ... under-reports as normal"*. On a real node that specific framing does not hold, because **`Active` is deliberately not persisted** — neither `LocalDbAlarmConditionStateStore`'s `alarm_condition_state` table nor the documented contract in `docs/ScriptedAlarms.md` §"State persistence" carries it; the engine re-derives Active from the predicate at every `LoadAsync`. So a still-active predicate-driven alarm *does* re-emit `Activated` on reload and its node recovers on its own. The bug is real, but it bites the fields that **are** persisted: `Acked`, `Confirmed`, `Shelving`, `Enabled`. The sharpest case, and the one used for the live gate: > An alarm that fired, cleared, and is still **unacknowledged**. On reload the predicate is false, so there is no transition, so `EmissionKind.None`. The node comes back from the materialise as *acknowledged*, which makes `Retain = Active || !Acked` false — so the condition **disappears from `ConditionRefresh` entirely**. The operator's outstanding alarm silently vanishes from the alarm list on the next deploy, while the engine still holds Unacknowledged. That is a worse symptom than the one originally described, and it is what the fix is gated against. ## Live gate — PASSED docker-dev rig, `central-2`, pre-fix image vs. the rebuilt one. Scripted alarm on `gateequip` (MAIN), authored through the real `/uns` equipment Alarms modal; driven to active-then-cleared-unacked; rebuild forced by a node restart (`RestoreApplied` re-materialises everything). | leg | pre-fix | post-fix | |---|---|---| | `ConditionRefresh` after rebuild | `SA-alwaysactive` **absent** — Retain=false | **present**, `Unacknowledged`, `Retain: True` | | timestamp | — | `12:55:38` = the condition's own `LastTransitionUtc` (matches `updated_at_utc` in LocalDb exactly), not the restart instant | | `/alerts` during re-assert | — | **empty across two restarts** ⇒ no duplicate historian row | The `/alerts` leg was explicitly falsified rather than accepted as a bare absence: immediately afterwards the predicate was flipped back to true and a real `ACTIVATED` row appeared on the same open panel, proving the feed was live the whole time. ## Tests 6 new host tests (`ScriptedAlarmHostActorTests`) + 6 new engine tests (`ScriptedAlarmProjectionTests`). Falsifiability checked by disabling the call: 4 of the 6 host tests go red. The other 2 (`A_normal_alarm_is_not_reasserted`, `A_disabled_plan_is_not_reasserted`) are absence guards against an over-broad fix and pass either way by design. Full solution builds clean; `Runtime.Tests` 504/504, `Core.ScriptedAlarms.Tests` 82/82. Docs: `docs/ScriptedAlarms.md` §"Post-(re)load condition-node re-assert (#487)"; the deferral note in `docs/plans/2026-07-16-v3-batch4-PR.md` updated to point at it. Side-discovery filed separately as #504 (`/scripts` is a hard 500 when any `Script.SourceHash` is shorter than 12 chars — the docker-dev seed trips it).
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
auto-managed
queue management
 cross-driver
queue management
 driver/abcip
queue management
 driver/ablegacy
queue management
 driver/focas
queue management
 driver/mqtt
MQTT / Sparkplug B broker-subscribe driver
 driver/opcuaclient
queue management
 driver/s7
queue management
 driver/twincat
queue management
 phase/1
queue management
 phase/2
queue management
 phase/3
queue management
 phase/4
queue management
 phase/5
queue management
 phase/6
queue management
 queue/blocked
queue management
 queue/done
queue management
 queue/failed
queue management
 queue/in-progress
queue management
 queue/queued
queue management
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
dohertj2 (dohertj2)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: dohertj2/lmxopcua#487
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?