From 7f665bf25b76974c9691216f956650353db7a549 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 08:23:26 -0400 Subject: [PATCH 1/3] docs(contracts): document unpropagated operator_full_name, reserved enum values, correlation echo --- docs/MxAccessWorkerInstanceDesign.md | 6 +++--- .../Generated/MxaccessGateway.cs | 19 ++++++++++++++++++- .../Generated/MxaccessWorker.cs | 6 ++++++ .../Protos/mxaccess_gateway.proto | 11 ++++++++++- .../Protos/mxaccess_worker.proto | 4 ++++ 5 files changed, 41 insertions(+), 5 deletions(-) diff --git a/docs/MxAccessWorkerInstanceDesign.md b/docs/MxAccessWorkerInstanceDesign.md index 4124bb0..2c0f4b3 100644 --- a/docs/MxAccessWorkerInstanceDesign.md +++ b/docs/MxAccessWorkerInstanceDesign.md @@ -1019,10 +1019,10 @@ and relies on the gateway to kill the process. ## Fault Handling -Worker fault categories: +Worker fault categories (`InvalidArguments` and `GatewayAuthenticationFailed` +are reserved and never emitted: bootstrap failures exit via `WorkerExitCode` +before the pipe exists): -- `InvalidArguments` -- `GatewayAuthenticationFailed` - `ProtocolMismatch` - `ProtocolViolation` - `PipeDisconnected` diff --git a/src/ZB.MOM.WW.MxGateway.Contracts/Generated/MxaccessGateway.cs b/src/ZB.MOM.WW.MxGateway.Contracts/Generated/MxaccessGateway.cs index 9e1229e..0bbd29f 100644 --- a/src/ZB.MOM.WW.MxGateway.Contracts/Generated/MxaccessGateway.cs +++ b/src/ZB.MOM.WW.MxGateway.Contracts/Generated/MxaccessGateway.cs @@ -727,6 +727,10 @@ namespace ZB.MOM.WW.MxGateway.Contracts.Proto { [pbr::OriginalName("ALARM_TRANSITION_KIND_RAISE")] Raise = 1, [pbr::OriginalName("ALARM_TRANSITION_KIND_ACKNOWLEDGE")] Acknowledge = 2, [pbr::OriginalName("ALARM_TRANSITION_KIND_CLEAR")] Clear = 3, + /// + /// Reserved: never produced by the gateway or worker. Retained so the numeric + /// value is not reused. + /// [pbr::OriginalName("ALARM_TRANSITION_KIND_RETRIGGER")] Retrigger = 4, } @@ -12714,6 +12718,11 @@ namespace ZB.MOM.WW.MxGateway.Contracts.Proto { /// Field number for the "operator_full_name" field. public const int OperatorFullNameFieldNumber = 6; private string operatorFullName_ = ""; + /// + /// Accepted for wire compatibility but NOT propagated to MXAccess: the 8-arg + /// AlarmAckByName that carried it returns -55. operator_domain IS used (the + /// AuthenticateUser identity is "<operator_domain>\<operator_user>"). + /// [global::System.Diagnostics.DebuggerNonUserCodeAttribute] [global::System.CodeDom.Compiler.GeneratedCode("protoc", null)] public string OperatorFullName { @@ -13384,6 +13393,11 @@ namespace ZB.MOM.WW.MxGateway.Contracts.Proto { /// Field number for the "operator_full_name" field. public const int OperatorFullNameFieldNumber = 8; private string operatorFullName_ = ""; + /// + /// Accepted for wire compatibility but NOT propagated to MXAccess: the 8-arg + /// AlarmAckByName that carried it returns -55. operator_domain IS used (the + /// AuthenticateUser identity is "<operator_domain>\<operator_user>"). + /// [global::System.Diagnostics.DebuggerNonUserCodeAttribute] [global::System.CodeDom.Compiler.GeneratedCode("protoc", null)] public string OperatorFullName { @@ -17527,6 +17541,9 @@ namespace ZB.MOM.WW.MxGateway.Contracts.Proto { /// Field number for the "correlation_id" field. public const int CorrelationIdFieldNumber = 2; private string correlationId_ = ""; + /// + /// Echoes the request's client_correlation_id. + /// [global::System.Diagnostics.DebuggerNonUserCodeAttribute] [global::System.CodeDom.Compiler.GeneratedCode("protoc", null)] public string CorrelationId { @@ -26125,7 +26142,7 @@ namespace ZB.MOM.WW.MxGateway.Contracts.Proto { /// /// When the alarm originally entered the active state. Preserved across /// acknowledge transitions so the Part 9 condition keeps the original raise - /// time. Unset on retrigger from a previously-cleared condition. + /// time. Unset when the raise time is unknown (RETRIGGER is reserved and never produced). /// [global::System.Diagnostics.DebuggerNonUserCodeAttribute] [global::System.CodeDom.Compiler.GeneratedCode("protoc", null)] diff --git a/src/ZB.MOM.WW.MxGateway.Contracts/Generated/MxaccessWorker.cs b/src/ZB.MOM.WW.MxGateway.Contracts/Generated/MxaccessWorker.cs index 892348a..2863490 100644 --- a/src/ZB.MOM.WW.MxGateway.Contracts/Generated/MxaccessWorker.cs +++ b/src/ZB.MOM.WW.MxGateway.Contracts/Generated/MxaccessWorker.cs @@ -132,6 +132,12 @@ namespace ZB.MOM.WW.MxGateway.Contracts.Proto { public enum WorkerFaultCategory { [pbr::OriginalName("WORKER_FAULT_CATEGORY_UNSPECIFIED")] Unspecified = 0, + /// + /// Values 1 and 2 are reserved: never emitted. Bootstrap failures (bad + /// arguments, gateway authentication) exit via WorkerExitCode before the pipe + /// exists, so there is no channel to carry a fault. Names kept so the numbers + /// are not reused. + /// [pbr::OriginalName("WORKER_FAULT_CATEGORY_INVALID_ARGUMENTS")] InvalidArguments = 1, [pbr::OriginalName("WORKER_FAULT_CATEGORY_GATEWAY_AUTHENTICATION_FAILED")] GatewayAuthenticationFailed = 2, [pbr::OriginalName("WORKER_FAULT_CATEGORY_PROTOCOL_MISMATCH")] ProtocolMismatch = 3, diff --git a/src/ZB.MOM.WW.MxGateway.Contracts/Protos/mxaccess_gateway.proto b/src/ZB.MOM.WW.MxGateway.Contracts/Protos/mxaccess_gateway.proto index 915b1e8..edfdca6 100644 --- a/src/ZB.MOM.WW.MxGateway.Contracts/Protos/mxaccess_gateway.proto +++ b/src/ZB.MOM.WW.MxGateway.Contracts/Protos/mxaccess_gateway.proto @@ -405,6 +405,9 @@ message AcknowledgeAlarmCommand { string operator_user = 3; string operator_node = 4; string operator_domain = 5; + // Accepted for wire compatibility but NOT propagated to MXAccess: the 8-arg + // AlarmAckByName that carried it returns -55. operator_domain IS used (the + // AuthenticateUser identity is "\"). string operator_full_name = 6; // Operator credential used for AuthenticateUser before the AckMsg write. // Implementations must keep this field out of logs, metrics labels, command @@ -435,6 +438,9 @@ message AcknowledgeAlarmByNameCommand { string operator_user = 5; string operator_node = 6; string operator_domain = 7; + // Accepted for wire compatibility but NOT propagated to MXAccess: the 8-arg + // AlarmAckByName that carried it returns -55. operator_domain IS used (the + // AuthenticateUser identity is "\"). string operator_full_name = 8; // Operator credential used for AuthenticateUser before the AckMsg write. // Implementations must keep this field out of logs, metrics labels, command @@ -549,6 +555,7 @@ message ShutdownWorkerCommand { message MxCommandReply { string session_id = 1; + // Echoes the request's client_correlation_id. string correlation_id = 2; MxCommandKind kind = 3; ProtocolStatus protocol_status = 4; @@ -880,7 +887,7 @@ message OnAlarmTransitionEvent { // When the alarm originally entered the active state. Preserved across // acknowledge transitions so the Part 9 condition keeps the original raise - // time. Unset on retrigger from a previously-cleared condition. + // time. Unset when the raise time is unknown (RETRIGGER is reserved and never produced). google.protobuf.Timestamp original_raise_timestamp = 6; // When this specific transition occurred (raise time on Raise, ack time on @@ -930,6 +937,8 @@ enum AlarmTransitionKind { ALARM_TRANSITION_KIND_RAISE = 1; ALARM_TRANSITION_KIND_ACKNOWLEDGE = 2; ALARM_TRANSITION_KIND_CLEAR = 3; + // Reserved: never produced by the gateway or worker. Retained so the numeric + // value is not reused. ALARM_TRANSITION_KIND_RETRIGGER = 4; } diff --git a/src/ZB.MOM.WW.MxGateway.Contracts/Protos/mxaccess_worker.proto b/src/ZB.MOM.WW.MxGateway.Contracts/Protos/mxaccess_worker.proto index 7a8a5e7..da894a3 100644 --- a/src/ZB.MOM.WW.MxGateway.Contracts/Protos/mxaccess_worker.proto +++ b/src/ZB.MOM.WW.MxGateway.Contracts/Protos/mxaccess_worker.proto @@ -127,6 +127,10 @@ enum WorkerState { enum WorkerFaultCategory { WORKER_FAULT_CATEGORY_UNSPECIFIED = 0; + // Values 1 and 2 are reserved: never emitted. Bootstrap failures (bad + // arguments, gateway authentication) exit via WorkerExitCode before the pipe + // exists, so there is no channel to carry a fault. Names kept so the numbers + // are not reused. WORKER_FAULT_CATEGORY_INVALID_ARGUMENTS = 1; WORKER_FAULT_CATEGORY_GATEWAY_AUTHENTICATION_FAILED = 2; WORKER_FAULT_CATEGORY_PROTOCOL_MISMATCH = 3; From dd3ea056b0e85eeff1c422ae0a6c848982e84d60 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 08:24:04 -0400 Subject: [PATCH 2/3] docs: note operator_full_name not propagated; refresh stillpending and deferred-followups status --- clients/dotnet/README.md | 2 +- clients/go/README.md | 2 +- clients/java/README.md | 2 +- clients/python/README.md | 2 +- clients/rust/README.md | 2 +- docs/Grpc.md | 1 + docs/plans/2026-06-14-deferred-followups.md | 2 +- stillpending.md | 15 +++++++-------- 8 files changed, 14 insertions(+), 14 deletions(-) diff --git a/clients/dotnet/README.md b/clients/dotnet/README.md index d651ddf..22eb9bb 100644 --- a/clients/dotnet/README.md +++ b/clients/dotnet/README.md @@ -146,7 +146,7 @@ the active alarms the gateway's central monitor currently holds), keyed by the same monitor), and `AcknowledgeAlarmAsync` (acknowledges as the operator: the gateway authenticates `OperatorUser`/`OperatorPassword` and writes the comment to the alarm's `AckMsg`, so the Historian records the -operator). All three accept a cancellation +operator). The operator's full name is not sent by the client and is not propagated to MXAccess (`operator_full_name` on the worker commands is accepted but ignored); the Historian takes it from the authenticated Galaxy account. All three accept a cancellation token and pass through the `MxGateway:Alarms` configuration on the server — when alarms are disabled, the gateway returns an empty list / empty stream rather than failing. diff --git a/clients/go/README.md b/clients/go/README.md index 5e5fa0f..9fbc670 100644 --- a/clients/go/README.md +++ b/clients/go/README.md @@ -144,7 +144,7 @@ snapshots, `Client.StreamAlarms` for the server-streaming feed, and operator's Galaxy credentials (`OperatorUser` required, `OperatorPassword`, optional `OperatorDomain`) so the gateway authenticates as the operator and the Historian records that operator; an empty password is allowed, and the password -is scrubbed from any returned error. The streaming +is scrubbed from any returned error. The operator's full name is not sent by the client and is not propagated to MXAccess (`operator_full_name` on the worker commands is accepted but ignored); the Historian takes it from the authenticated Galaxy account. The streaming call returns a `StreamAlarmsClient`; cancel its context to terminate the stream. All three pass straight through to the gateway's central alarm monitor. diff --git a/clients/java/README.md b/clients/java/README.md index 05e6c08..9c98f69 100644 --- a/clients/java/README.md +++ b/clients/java/README.md @@ -118,7 +118,7 @@ yields alarm-feed messages from the gateway's central monitor), and The ack carries operator credentials: `operator_user` is required, and `operator_password` / `operator_domain` let the gateway authenticate as that operator so the Historian records the real operator. An empty password is -allowed. The password is scrubbed from exceptions the client throws. Close the subscription to cancel the underlying gRPC stream. +allowed. The password is scrubbed from exceptions the client throws. The operator's full name is not sent by the client and is not propagated to MXAccess (`operator_full_name` on the worker commands is accepted but ignored); the Historian takes it from the authenticated Galaxy account. Close the subscription to cancel the underlying gRPC stream. `ActiveAlarmSnapshot.getFromTruncatedSnapshot()` reports that the record came from a provider fetch which hit the per-fetch cap: the snapshot set may omit diff --git a/clients/python/README.md b/clients/python/README.md index 64ddd9b..ecdbc7c 100644 --- a/clients/python/README.md +++ b/clients/python/README.md @@ -116,7 +116,7 @@ comment). The ack is made as the operator: set `operator_user` (required) and `operator_password` (may be empty) and optionally `operator_domain` on `AcknowledgeAlarmRequest`. The gateway authenticates the operator and writes the comment to the alarm's `AckMsg`, so the alarm history records the operator. The -password is redacted from any raised error. +password is redacted from any raised error. The operator's full name is not sent by the client and is not propagated to MXAccess (`operator_full_name` on the worker commands is accepted but ignored); the Historian takes it from the authenticated Galaxy account. Cancel the surrounding task or `aclose()` an alarm iterator to terminate the stream. diff --git a/clients/rust/README.md b/clients/rust/README.md index 963f915..457bf71 100644 --- a/clients/rust/README.md +++ b/clients/rust/README.md @@ -127,7 +127,7 @@ returns an `AlarmFeedStream` async stream of alarm-feed messages and shares the gateway's central alarm monitor with every other client. `acknowledge_alarm` requires `operator_user` and carries `operator_password` (may be empty) and `operator_domain`; the gateway authenticates as that operator so the Historian -records them. The password is scrubbed from any returned error's `Display`/`Debug`. +records them. The operator's full name is not sent by the client and is not propagated to MXAccess (`operator_full_name` on the worker commands is accepted but ignored); the Historian takes it from the authenticated Galaxy account. The password is scrubbed from any returned error's `Display`/`Debug`. The `from_truncated_snapshot` field on `ActiveAlarmSnapshot` reports that the record came from a provider fetch which hit the per-fetch cap: the snapshot set may omit active diff --git a/docs/Grpc.md b/docs/Grpc.md index 28d1cc4..851aa7f 100644 --- a/docs/Grpc.md +++ b/docs/Grpc.md @@ -103,6 +103,7 @@ Request fields that identify the acknowledging operator: - `operator_user` (required): the Galaxy account name. An empty value is rejected with `InvalidArgument` so the real operator is always recorded. - `operator_password` (field 6): the operator's Galaxy password. May be empty because Galaxy accounts can have empty passwords. It is a credential and is never logged. - `operator_domain` (field 7): optional. When set, the worker authenticates `domain\user`; when empty it authenticates `user`. +- `operator_full_name`: not a field on `AcknowledgeAlarmRequest`. The worker commands (`AcknowledgeAlarmCommand` field 6, `AcknowledgeAlarmByNameCommand` field 8) carry one, but it is accepted and **not propagated** to MXAccess: the 8-argument `AlarmAckByName` that carried it returns -55 (see [Alarm Client Discovery](./AlarmClientDiscovery.md)). `operator_domain` is used. The worker calls `AuthenticateUser` as the operator, then writes `operator_comment` to `.AckMsg` (subtag mode: the configured ack-comment subtag) on a dedicated MXAccess server registered as `MxGateway:Alarms:AckClientName`, and waits a bounded time for `OnWriteComplete`. The AVEVA runtime treats that write as the acknowledgement, and the Historian (`Runtime.dbo.v_AlarmEventHistory2`) records the operator name, domain and full name from the authenticated account, with the ack client name as `OperatorNode`. The alarm-client ack calls (`AlarmAckByName`, `AlarmAckByGUID`) are not used because a headless consumer's acks are accepted and dropped; see [Alarm Probe Findings](./AlarmProbeFindings.md). diff --git a/docs/plans/2026-06-14-deferred-followups.md b/docs/plans/2026-06-14-deferred-followups.md index 1670b25..fb08c9a 100644 --- a/docs/plans/2026-06-14-deferred-followups.md +++ b/docs/plans/2026-06-14-deferred-followups.md @@ -1,7 +1,7 @@ # Deferred Follow-ups Implementation Plan **Date:** 2026-06-14 -**Status:** D1 executed (commit 4af24b9 — `mxgateway.alarms.provider_switches` emitted in `DashboardSnapshotService.cs:198`). D2 resolved as no-op (see resolution section below). D3–D5 remain pending (ops/validation, no code). +**Status:** D1 executed (commit 4af24b9 — `mxgateway.alarms.provider_switches` emitted in `DashboardSnapshotService.cs:198`). D2 resolved as no-op (see resolution section below). D3 (ForceSubtag validation) was exercised through the forced-subtag work (`2026-06-15-forced-subtag-mode-fix.md`); D4 (windev backup prune) has no completion record in the repo and is treated as pending; D5 (wonder redeploy) was superseded by later wonder deployments. D3–D5 are ops/validation with no code. **Context:** After the alarm-subtag-fallback cleanup (merged `5976770`) and its redeploy to windev (10.100.0.48), five items remain deferred. This plan handles all five. They are independent — execute in any order, or cherry-pick. Items D1–D2 are code (branch off `main`); diff --git a/stillpending.md b/stillpending.md index 0d6923b..428566a 100644 --- a/stillpending.md +++ b/stillpending.md @@ -42,10 +42,9 @@ Items are graded by what they actually are, because most "pending" surface in th - **Location:** metric emitted in `Alarms/GatewayAlarmMonitor.cs` (failover/failback path); residual recorded in `docs/plans/2026-06-14-deferred-followups.md:124-125`. - **Evidence:** *"that counter's live exercise remains the one gap; record it explicitly rather than claiming coverage."* Unit-tested (Tests-032) but the dev rig can't drive a real alarmmgr→subtag failover (`project_rig_alarms_object_driven`), so the counter's `reason` tagging is unproven in production. -### 🟠 1.4 — Worker 8-arg alarm ack silently discards operator domain / full name -- **Location:** `src/ZB.MOM.WW.MxGateway.Worker/MxAccess/WnWrapAlarmConsumer.cs:261-278` (`_ = ackOperatorDomain; _ = ackOperatorFullName;`). -- **Evidence:** *"the IwwAlarmConsumer2 8-arg AlarmAckByName returns -55 on this AVEVA build (looks like a stub) … fields are accepted by the proto for forward-compat but are not propagated to AVEVA today."* -- **Impact:** two contract fields are accepted on the wire and silently not delivered. Root cause is the vendor stub (see 3.5), but the drop is currently invisible to callers. +### 🟡 1.4 — Operator full name is accepted but not propagated (domain is now consumed) +- **Status (2026-10-22):** half-resolved. Acks now authenticate as the operator and write `.AckMsg` (`Worker/MxAccess/AlarmAckOperator.cs`), and **`operator_domain` is consumed** (`domain\user` for `AuthenticateUser`). The older `WnWrapAlarmConsumer` 8-arg path that discarded both fields (`_ = ackOperatorDomain; _ = ackOperatorFullName;`) is no longer the ack path. +- **Remaining:** `operator_full_name` on `AcknowledgeAlarmCommand` (field 6) / `AcknowledgeAlarmByNameCommand` (field 8) is still accepted and not propagated to MXAccess; the 8-arg `AlarmAckByName` that carried it returns -55 (`docs/AlarmClientDiscovery.md:743-748`). The public `AcknowledgeAlarmRequest` has no such field. The proto comments and `docs/Grpc.md` now state this. --- @@ -76,8 +75,8 @@ Intentionally incomplete, awaiting a live capture or an AVEVA fix; raw payload/m - 🟡 **3.1 `OperationComplete` native trigger condition unknown** — modeled and emitted only from the real event (no synthesis), but the runtime condition that fires it isn't captured. `docs/DesignDecisions.md:280-289`, `gateway.md:1094`, `docs/MxAccessWorkerInstanceDesign.md:341,366`. - 🟡 **3.2 `OnBufferedDataChange` multi-sample conversion unvalidated — STILL OPEN (residual after B8)** — `AddBufferedItem`/`SetBufferedUpdateInterval` are now implemented and live-confirmed (§1.1), and the live B8 test (`f7ada90`) confirms the worker receives and cleanly converts the empty `NoData` bootstrap `OnBufferedDataChange` (no crash, no dropped payload). But the rig's object logic does not drive a multi-sample buffered batch on demand (same limitation as the alarm rig), so a real parallel quality/timestamp sample array (length > 1) has never been observed live — it is exercised only by the B-bundle unit tests against a fake `IMxAccessServer`. Re-run `GatewaySession_WithLiveWorker_BufferedItem_*` against a fast-changing simulated tag to close this. `docs/DesignDecisions.md:291-297`. - 🟡 **3.3 Completion-only status → `MXSTATUS_PROXY[]` mapping unproven** — completion-only operation-status bytes are kept as raw diagnostic metadata until the analysis proves an exact mapping. `docs/DesignDecisions.md:299-306`. -- 🟡 **3.4 `AlarmAckByGUID` is `E_NOTIMPL` on this AVEVA build** — throws `NotImplementedException`; all acks route through `AlarmAckByName`. Proto/worker keep the path for forward-compat but it is dead today. `docs/AlarmClientDiscovery.md:750-763`. -- 🟡 **3.5 8-arg `AlarmAckByName` v2 is a vendor stub (returns -55)** — worker uses the 6-arg method; the 8-arg `domain`/`full_name` fields are carried for forward-compat only (see 1.4). `docs/AlarmClientDiscovery.md:743-748`. +- 🟡 **3.4 `AlarmAckByGUID` is `E_NOTIMPL` on this AVEVA build (historical; note corrected 2026-10-22)** — it throws `NotImplementedException`, but the worker no longer calls it. `MxCommandKind.AcknowledgeAlarm` has live dispatch (`Worker/MxAccess/MxAccessCommandExecutor.cs` `ExecuteAcknowledgeAlarm`, ~895-936): it parses `alarm_guid` and calls `alarmCommandHandler.Acknowledge(guid, comment, AlarmAckOperator)`, which acknowledges by writing `AckMsg` as the authenticated operator. A GUID ack therefore works; only the vendor `AlarmAckByGUID` call is dead. `docs/AlarmClientDiscovery.md:750-763`. +- 🟡 **3.5 8-arg `AlarmAckByName` v2 is a vendor stub (returns -55)** — no longer on the ack path (acks write `AckMsg`); `operator_full_name` is carried for forward-compat only and is not propagated (see 1.4). `docs/AlarmClientDiscovery.md:743-748`. - 🟡 **3.6 Subtag degraded-mode fidelity limits** — `category`, `description`, `alarm_type_name`, operator fields, and `retrigger` are not populated/synthesized in subtag fallback (no subtag exists for them). Documented, by design. `docs/AlarmClientDiscovery.md:913-931`, `docs/plans/2026-06-13-alarm-subtag-fallback-design.md:292-298`. - 🟡 **3.7 Subtag `Clear` transition unvalidated live** — Raise/Ack/AckMsg are live-confirmed; Clear is externally undrivable on the rig (object logic owns alarm state). Environmental, not code. (`project_alarm_subtag_fallback`, `project_rig_alarms_object_driven`.) @@ -132,7 +131,7 @@ No placeholder/empty/`Assert.True(true)` tests were found anywhere. ## 7. Stale docs, dead code, accepted gaps -- 📄 **7.1 D1 plan header stale** — `docs/plans/2026-06-14-deferred-followups.md:4` still says *"Plan only — NOT yet executed,"* but D1 is **done** (`Dashboard/DashboardSnapshotService.cs:198`, commit `4af24b9`). Update the plan status. +- ✅ **7.1 D1 plan header stale — RESOLVED** (2026-10-22): `docs/plans/2026-06-14-deferred-followups.md` status line now records D1 executed (`4af24b9`) and D2 resolved as no-op, and no longer claims the plan is unexecuted. - 📄 **7.2 `AlarmClientDiscovery.md` STA "production fix needed" prose is stale** — `docs/AlarmClientDiscovery.md:765-774` reads as a pending follow-up, but alarms now run through the worker STA / `GatewayAlarmMonitor` (merged). Re-check against current code. - 📄 **7.3 EventsHub "publisher side is a follow-up" comment is stale** — `Dashboard/Hubs/EventsHub.cs:9-17`; the `DashboardEventBroadcaster` exists, is DI-registered (`Dashboard/DashboardServiceCollectionExtensions.cs:47`), runs in the live loop (`Grpc/EventStreamService.cs:133`), and `SessionDetailsPage.razor` renders the feed. - ✅ **7.4 CLAUDE.md project-name drift — RESOLVED** (verified 2026-08-07): CLAUDE.md now uses the actual `src/ZB.MOM.WW.MxGateway.*` project paths throughout. @@ -161,7 +160,7 @@ No placeholder/empty/`Assert.True(true)` tests were found anywhere. **Still open (all deliberate or environment/vendor-gated):** - **§1.3** — `provider_switches` counter still only unit-tested; the dev rig can't drive a real alarmmgr→subtag failover, so live `reason`-tag coverage remains a recorded residual. -- **§1.4 / §3.4 / §3.5** — the AVEVA 8-arg `AlarmAckByName` is a vendor stub (−55) and `AlarmAckByGUID` is `E_NOTIMPL`; the `domain`/`full_name` fields stay forward-compat-only until AVEVA implements them. +- **§1.4 / §3.4 / §3.5** — the AVEVA 8-arg `AlarmAckByName` is a vendor stub (−55) and `AlarmAckByGUID` is `E_NOTIMPL`; `operator_domain` is consumed by the `AckMsg` path; `operator_full_name` stays forward-compat-only (not propagated). - **§3.2** — buffered commands work and the empty bootstrap converts cleanly live, but a multi-sample buffered batch is undrivable on the rig (unit-tested only). - **§3.1 / §3.3 / §3.6 / §3.7** — await live MXAccess captures. - **§2** — deliberate v1 scope. Multi-subscriber fan-out, reconnect, and the per-session dashboard ACL are resolved; orphan-worker reattach remains deliberately deferred and not planned. **§5** — opt-in verification gates. **§7.6** — accepted `Won't Fix` review findings. From 908cf7cccaacea3096345a8c9de556873b9995b9 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 08:25:11 -0400 Subject: [PATCH 3/3] test(server): cover GatewayConfigPathRules, DashboardRedactor, EventSubscriberReplayAttachment --- .../GatewayConfigPathRulesTests.cs | 122 ++++++++++++++++++ .../Dashboard/DashboardRedactorTests.cs | 51 ++++++++ .../EventSubscriberReplayAttachmentTests.cs | 63 +++++++++ 3 files changed, 236 insertions(+) create mode 100644 src/ZB.MOM.WW.MxGateway.Tests/Configuration/GatewayConfigPathRulesTests.cs create mode 100644 src/ZB.MOM.WW.MxGateway.Tests/Dashboard/DashboardRedactorTests.cs create mode 100644 src/ZB.MOM.WW.MxGateway.Tests/Sessions/EventSubscriberReplayAttachmentTests.cs diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Configuration/GatewayConfigPathRulesTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Configuration/GatewayConfigPathRulesTests.cs new file mode 100644 index 0000000..085d4ac --- /dev/null +++ b/src/ZB.MOM.WW.MxGateway.Tests/Configuration/GatewayConfigPathRulesTests.cs @@ -0,0 +1,122 @@ +using ZB.MOM.WW.Configuration; +using ZB.MOM.WW.MxGateway.Server.Configuration; + +namespace ZB.MOM.WW.MxGateway.Tests.Configuration; + +/// +/// Direct tests of the shared path-validation primitives so the rules are pinned independently of +/// the options validators that call them. +/// +public sealed class GatewayConfigPathRulesTests +{ + private const string Message = "path rule failed"; + + private static readonly string Root = Path.Combine(Path.GetTempPath(), "mxgw-app"); + + /// Verifies blank values are ignored by every rule. + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + public void BlankValue_RecordsNoFailure(string? value) + { + ValidationBuilder builder = new(); + + GatewayConfigPathRules.AddIfNotRooted(value, Message, builder); + GatewayConfigPathRules.AddIfInvalidPath(value, Message, builder); + GatewayConfigPathRules.AddIfUnderContentRoot(value, Root, Message, builder); + + Assert.True(builder.IsValid); + } + + /// Verifies a relative path is rejected as not rooted. + [Fact] + public void AddIfNotRooted_RecordsFailure_ForRelativePath() + { + ValidationBuilder builder = new(); + + GatewayConfigPathRules.AddIfNotRooted(Path.Combine("data", "auth.db"), Message, builder); + + Assert.Equal([Message], builder.Failures); + } + + /// Verifies a rooted path passes. + [Fact] + public void AddIfNotRooted_Passes_ForRootedPath() + { + ValidationBuilder builder = new(); + + GatewayConfigPathRules.AddIfNotRooted(Path.Combine(Root, "auth.db"), Message, builder); + + Assert.True(builder.IsValid); + } + + /// Verifies a NUL character makes the path invalid. + [Fact] + public void AddIfInvalidPath_RecordsFailure_ForEmbeddedNul() + { + ValidationBuilder builder = new(); + + GatewayConfigPathRules.AddIfInvalidPath("a\0b", Message, builder); + + Assert.Equal([Message], builder.Failures); + } + + /// Verifies an ordinary path is valid. + [Fact] + public void AddIfInvalidPath_Passes_ForOrdinaryPath() + { + ValidationBuilder builder = new(); + + GatewayConfigPathRules.AddIfInvalidPath(Path.Combine(Root, "auth.db"), Message, builder); + + Assert.True(builder.IsValid); + } + + /// Verifies paths inside the content root, and the root itself, are rejected. + [Fact] + public void AddIfUnderContentRoot_RecordsFailure_ForPathInsideRoot() + { + ValidationBuilder inside = new(); + ValidationBuilder equal = new(); + ValidationBuilder trailing = new(); + + GatewayConfigPathRules.AddIfUnderContentRoot(Path.Combine(Root, "sub", "auth.db"), Root, Message, inside); + GatewayConfigPathRules.AddIfUnderContentRoot(Root, Root, Message, equal); + GatewayConfigPathRules.AddIfUnderContentRoot( + Path.Combine(Root, "auth.db"), + Root + Path.DirectorySeparatorChar, + Message, + trailing); + + Assert.Equal([Message], inside.Failures); + Assert.Equal([Message], equal.Failures); + Assert.Equal([Message], trailing.Failures); + } + + /// Verifies a sibling directory sharing the root's name prefix is not treated as inside it. + [Fact] + public void AddIfUnderContentRoot_Passes_ForSiblingWithSharedPrefix() + { + ValidationBuilder builder = new(); + + GatewayConfigPathRules.AddIfUnderContentRoot( + Root + "-data" + Path.DirectorySeparatorChar + "auth.db", + Root, + Message, + builder); + + Assert.True(builder.IsValid); + } + + /// Verifies a malformed value reports nothing here, leaving it to the path-validity rule. + [Fact] + public void AddIfUnderContentRoot_Passes_ForMalformedValue() + { + ValidationBuilder builder = new(); + + GatewayConfigPathRules.AddIfUnderContentRoot("a\0b", Root, Message, builder); + + Assert.True(builder.IsValid); + } +} diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Dashboard/DashboardRedactorTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Dashboard/DashboardRedactorTests.cs new file mode 100644 index 0000000..197486d --- /dev/null +++ b/src/ZB.MOM.WW.MxGateway.Tests/Dashboard/DashboardRedactorTests.cs @@ -0,0 +1,51 @@ +using ZB.MOM.WW.MxGateway.Server.Dashboard; +using ZB.MOM.WW.MxGateway.Server.Diagnostics; + +namespace ZB.MOM.WW.MxGateway.Tests.Dashboard; + +/// Tests for . +public sealed class DashboardRedactorTests +{ + /// Verifies null, empty and whitespace values pass through unchanged. + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + public void Redact_ReturnsBlankValueUnchanged(string? value) + { + Assert.Equal(value, DashboardRedactor.Redact(value)); + } + + /// Verifies harmless text is returned unchanged. + [Fact] + public void Redact_ReturnsOrdinaryTextUnchanged() + { + Assert.Equal("Session opened for Plant1", DashboardRedactor.Redact("Session opened for Plant1")); + } + + /// Verifies text containing a sensitive marker is replaced whole, case-insensitively. + [Theory] + [InlineData("apikey=abc")] + [InlineData("API_KEY abc")] + [InlineData("Authorization: Basic abc")] + [InlineData("user credential rejected")] + [InlineData("bad Password supplied")] + [InlineData("client SECRET")] + [InlineData("hub Token minted")] + public void Redact_ReplacesTextContainingSensitiveMarker(string value) + { + Assert.Equal(GatewayLogRedactor.RedactedValue, DashboardRedactor.Redact(value)); + } + + /// Verifies a gateway API key goes through client-identity redaction and its secret never survives. + [Fact] + public void Redact_DelegatesGatewayKeysToClientIdentityRedaction() + { + const string Value = "Bearer mxgw_dev_topsecretvalue"; + + string? result = DashboardRedactor.Redact(Value); + + Assert.Equal(GatewayLogRedactor.RedactClientIdentity(Value), result); + Assert.DoesNotContain("topsecretvalue", result); + } +} diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Sessions/EventSubscriberReplayAttachmentTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Sessions/EventSubscriberReplayAttachmentTests.cs new file mode 100644 index 0000000..47caabd --- /dev/null +++ b/src/ZB.MOM.WW.MxGateway.Tests/Sessions/EventSubscriberReplayAttachmentTests.cs @@ -0,0 +1,63 @@ +using System.Threading.Channels; +using ZB.MOM.WW.MxGateway.Contracts.Proto; +using ZB.MOM.WW.MxGateway.Server.Sessions; + +namespace ZB.MOM.WW.MxGateway.Tests.Sessions; + +/// Tests for the value shape. +public sealed class EventSubscriberReplayAttachmentTests +{ + /// Verifies the constructor exposes every component unchanged. + [Fact] + public void Constructor_ExposesComponents() + { + StubLease lease = new(); + MxEvent[] replay = [new MxEvent { WorkerSequence = 5 }, new MxEvent { WorkerSequence = 6 }]; + + EventSubscriberReplayAttachment attachment = new(lease, replay, true, 5, 6); + + Assert.Same(lease, attachment.Lease); + Assert.Same(replay, attachment.ReplayedEvents); + Assert.True(attachment.Gap); + Assert.Equal(5UL, attachment.OldestAvailableSequence); + Assert.Equal(6UL, attachment.LiveResumeSequence); + } + + /// Verifies value equality holds for identical components and breaks when a watermark differs. + [Fact] + public void Equality_ComparesComponents() + { + StubLease lease = new(); + IReadOnlyList replay = []; + + EventSubscriberReplayAttachment a = new(lease, replay, false, 0, 10); + EventSubscriberReplayAttachment same = new(lease, replay, false, 0, 10); + EventSubscriberReplayAttachment other = a with { LiveResumeSequence = 11 }; + + Assert.Equal(a, same); + Assert.NotEqual(a, other); + } + + /// Verifies disposing the lease through the attachment releases the subscriber. + [Fact] + public void Lease_DisposeIsObservableThroughAttachment() + { + StubLease lease = new(); + EventSubscriberReplayAttachment attachment = new(lease, [], false, 0, 0); + + attachment.Lease.Dispose(); + + Assert.True(lease.Disposed); + } + + private sealed class StubLease : IEventSubscriberLease + { + private readonly Channel channel = Channel.CreateUnbounded(); + + public bool Disposed { get; private set; } + + public ChannelReader Reader => channel.Reader; + + public void Dispose() => Disposed = true; + } +}