diff --git a/code-reviews/Contracts/findings.md b/code-reviews/Contracts/findings.md
index f3e6b1b..b1890c3 100644
--- a/code-reviews/Contracts/findings.md
+++ b/code-reviews/Contracts/findings.md
@@ -7,7 +7,7 @@
| Review date | 2026-05-18 |
| Commit reviewed | `6c64030` |
| Status | Reviewed |
-| Open findings | 8 |
+| Open findings | 7 |
## Checklist coverage
@@ -48,13 +48,13 @@
| Severity | Medium |
| Category | Error handling & resilience |
| Location | `src/MxGateway.Contracts/Protos/mxaccess_gateway.proto:384-385`, `:95` |
-| Status | Open |
+| Status | Resolved |
**Description:** `MxCommandKind` includes `MX_COMMAND_KIND_ACKNOWLEDGE_ALARM_BY_NAME = 29` and `MxCommand.payload` carries `AcknowledgeAlarmByNameCommand acknowledge_alarm_by_name_command = 38`, but `MxCommandReply.payload` has only `acknowledge_alarm = 34` and `query_active_alarms = 35` — there is no by-name reply case. The by-name ack must reuse `AcknowledgeAlarmReplyPayload` or rely on the top-level `hresult`. The command/reply payload asymmetry is undocumented and easy to dispatch incorrectly.
**Recommendation:** Either add an explicit comment to `MxCommandReply` stating that by-name ack reuses the `acknowledge_alarm` payload case, or add a dedicated payload case for symmetry, and document the chosen contract in `docs/Contracts.md` / `AlarmClientDiscovery.md`.
-**Resolution:** _(open)_
+**Resolution:** _(2026-05-18)_ Verified against both the `.proto` and the dispatch code. The asymmetry is intentional and the code is correct: the worker's `MxAccessCommandExecutor.ExecuteAcknowledgeAlarmByName` builds `reply.AcknowledgeAlarm = new AcknowledgeAlarmReplyPayload { NativeStatus = rc }` — deliberately reusing the `acknowledge_alarm` payload case — and the gateway's `WorkerAlarmRpcDispatcher.AcknowledgeAsync` only reads the top-level `hresult`/`protocol_status`, so both ack arms work. The gap was documentation only. Took the finding's preferred option (a) — comment-only, no wire-format or generated-type change: added explicit comments to the `acknowledge_alarm` reply-payload case and to the `AcknowledgeAlarmReplyPayload` message in `mxaccess_gateway.proto` stating both ack kinds reuse this case and consumers must dispatch on `MxCommandReply.kind`, and documented the contract in `docs/AlarmClientDiscovery.md` section 4. Added regression test `ProtobufContractRoundTripTests.MxCommandReply_AcknowledgeAlarmByName_ReusesAcknowledgeAlarmPayloadCase` pinning the by-name-ack → `acknowledge_alarm` reuse and asserting no by-name-specific reply oneof case exists.
### Contracts-003
diff --git a/docs/AlarmClientDiscovery.md b/docs/AlarmClientDiscovery.md
index da565bb..a74eca5 100644
--- a/docs/AlarmClientDiscovery.md
+++ b/docs/AlarmClientDiscovery.md
@@ -762,6 +762,20 @@ in the codebase for the forward-compat shape, but the gateway-side
`AcknowledgeAlarmByName` when the public RPC supplies a recognizable
`Provider!Group.Tag` reference.
+**Command/reply payload reuse.** `MxCommand.payload` has a dedicated
+`acknowledge_alarm_by_name_command` field, but `MxCommandReply.payload`
+intentionally has **no** by-name-specific case. The by-name ack carries
+no outcome detail beyond the native return code, so the worker's
+`ExecuteAcknowledgeAlarmByName` sets the same `acknowledge_alarm`
+(`AcknowledgeAlarmReplyPayload`) reply case used by the GUID arm, with
+`native_status` = the `AlarmAckByName` return code (also echoed into the
+top-level `MxCommandReply.hresult`). Reply consumers must dispatch on
+`MxCommandReply.kind` (`MX_COMMAND_KIND_ACKNOWLEDGE_ALARM` vs.
+`MX_COMMAND_KIND_ACKNOWLEDGE_ALARM_BY_NAME`), not on the payload oneof
+case, to distinguish the two acks. `WorkerAlarmRpcDispatcher` reads only
+the top-level `hresult`/`protocol_status`, so it handles both arms
+without unpacking the payload.
+
### 5. STA / threading — production fix needed
The wnwrap COM is `ThreadingModel=Apartment`. The consumer's
diff --git a/src/MxGateway.Contracts/Generated/MxaccessGateway.cs b/src/MxGateway.Contracts/Generated/MxaccessGateway.cs
index ed8b018..8b64186 100644
--- a/src/MxGateway.Contracts/Generated/MxaccessGateway.cs
+++ b/src/MxGateway.Contracts/Generated/MxaccessGateway.cs
@@ -13388,6 +13388,17 @@ namespace MxGateway.Contracts.Proto {
/// Field number for the "acknowledge_alarm" field.
public const int AcknowledgeAlarmFieldNumber = 34;
+ ///
+ /// Reply payload for BOTH MX_COMMAND_KIND_ACKNOWLEDGE_ALARM (by GUID)
+ /// and MX_COMMAND_KIND_ACKNOWLEDGE_ALARM_BY_NAME. There is intentionally
+ /// no by-name-specific reply case: the by-name ack carries no outcome
+ /// detail beyond the native ack return code, so the worker reuses this
+ /// `acknowledge_alarm` payload for both command kinds (the worker's
+ /// MxAccessCommandExecutor sets `acknowledge_alarm` for the by-name arm
+ /// too). Consumers must dispatch on MxCommandReply.kind, not on the
+ /// payload case, to tell the two acks apart. The top-level `hresult`
+ /// mirrors AcknowledgeAlarmReplyPayload.native_status and is preferred.
+ ///
[global::System.Diagnostics.DebuggerNonUserCodeAttribute]
[global::System.CodeDom.Compiler.GeneratedCode("protoc", null)]
public global::MxGateway.Contracts.Proto.AcknowledgeAlarmReplyPayload AcknowledgeAlarm {
@@ -17339,12 +17350,16 @@ namespace MxGateway.Contracts.Proto {
}
///
- /// Reply payload for AcknowledgeAlarmCommand. Surfaces AVEVA's native
- /// AlarmAckByGUID return code; 0 means success. The MxCommandReply's
- /// hresult field carries the same value and is preferred for protocol
- /// consumers — this payload exists so the gateway-side
- /// WorkerAlarmRpcDispatcher can echo native_status into
- /// AcknowledgeAlarmReply.hresult without unpacking the outer envelope.
+ /// Reply payload for AcknowledgeAlarmCommand AND
+ /// AcknowledgeAlarmByNameCommand — both ack command kinds reuse this
+ /// payload case (`MxCommandReply.acknowledge_alarm`); there is no
+ /// dedicated by-name reply case. Surfaces AVEVA's native ack return
+ /// code (AlarmAckByGUID for the GUID arm, AlarmAckByName for the
+ /// by-name arm); 0 means success. The MxCommandReply's hresult field
+ /// carries the same value and is preferred for protocol consumers —
+ /// this payload exists so the gateway-side WorkerAlarmRpcDispatcher
+ /// can echo native_status into AcknowledgeAlarmReply.hresult without
+ /// unpacking the outer envelope.
///
[global::System.Diagnostics.DebuggerDisplayAttribute("{ToString(),nq}")]
public sealed partial class AcknowledgeAlarmReplyPayload : pb::IMessage
diff --git a/src/MxGateway.Contracts/Protos/mxaccess_gateway.proto b/src/MxGateway.Contracts/Protos/mxaccess_gateway.proto
index fa71b6b..6d75b7d 100644
--- a/src/MxGateway.Contracts/Protos/mxaccess_gateway.proto
+++ b/src/MxGateway.Contracts/Protos/mxaccess_gateway.proto
@@ -381,6 +381,15 @@ message MxCommandReply {
BulkSubscribeReply un_advise_item_bulk = 31;
BulkSubscribeReply subscribe_bulk = 32;
BulkSubscribeReply unsubscribe_bulk = 33;
+ // Reply payload for BOTH MX_COMMAND_KIND_ACKNOWLEDGE_ALARM (by GUID)
+ // and MX_COMMAND_KIND_ACKNOWLEDGE_ALARM_BY_NAME. There is intentionally
+ // no by-name-specific reply case: the by-name ack carries no outcome
+ // detail beyond the native ack return code, so the worker reuses this
+ // `acknowledge_alarm` payload for both command kinds (the worker's
+ // MxAccessCommandExecutor sets `acknowledge_alarm` for the by-name arm
+ // too). Consumers must dispatch on MxCommandReply.kind, not on the
+ // payload case, to tell the two acks apart. The top-level `hresult`
+ // mirrors AcknowledgeAlarmReplyPayload.native_status and is preferred.
AcknowledgeAlarmReplyPayload acknowledge_alarm = 34;
QueryActiveAlarmsReplyPayload query_active_alarms = 35;
SessionStateReply session_state = 100;
@@ -448,12 +457,16 @@ message DrainEventsReply {
repeated MxEvent events = 1;
}
-// Reply payload for AcknowledgeAlarmCommand. Surfaces AVEVA's native
-// AlarmAckByGUID return code; 0 means success. The MxCommandReply's
-// hresult field carries the same value and is preferred for protocol
-// consumers — this payload exists so the gateway-side
-// WorkerAlarmRpcDispatcher can echo native_status into
-// AcknowledgeAlarmReply.hresult without unpacking the outer envelope.
+// Reply payload for AcknowledgeAlarmCommand AND
+// AcknowledgeAlarmByNameCommand — both ack command kinds reuse this
+// payload case (`MxCommandReply.acknowledge_alarm`); there is no
+// dedicated by-name reply case. Surfaces AVEVA's native ack return
+// code (AlarmAckByGUID for the GUID arm, AlarmAckByName for the
+// by-name arm); 0 means success. The MxCommandReply's hresult field
+// carries the same value and is preferred for protocol consumers —
+// this payload exists so the gateway-side WorkerAlarmRpcDispatcher
+// can echo native_status into AcknowledgeAlarmReply.hresult without
+// unpacking the outer envelope.
message AcknowledgeAlarmReplyPayload {
int32 native_status = 1;
}
diff --git a/src/MxGateway.Tests/Contracts/ProtobufContractRoundTripTests.cs b/src/MxGateway.Tests/Contracts/ProtobufContractRoundTripTests.cs
index f746766..e430326 100644
--- a/src/MxGateway.Tests/Contracts/ProtobufContractRoundTripTests.cs
+++ b/src/MxGateway.Tests/Contracts/ProtobufContractRoundTripTests.cs
@@ -342,6 +342,56 @@ public sealed class ProtobufContractRoundTripTests
Assert.True(parsed.HasHresult);
}
+ ///
+ /// Pins the documented command/reply payload-reuse contract: an
+ /// ACKNOWLEDGE_ALARM_BY_NAME command's reply intentionally has no
+ /// by-name-specific payload case and instead reuses the
+ /// acknowledge_alarm ()
+ /// case. A future change that adds a separate by-name reply case — or
+ /// drops the reuse — breaks this test. See Contracts-002 and
+ /// docs/AlarmClientDiscovery.md section 4.
+ ///
+ [Fact]
+ public void MxCommandReply_AcknowledgeAlarmByName_ReusesAcknowledgeAlarmPayloadCase()
+ {
+ // The reply oneof must NOT have a by-name-specific case. If a future
+ // edit adds one, this assertion fails and forces the doc/test contract
+ // to be revisited deliberately.
+ foreach (MxCommandReply.PayloadOneofCase value in
+ System.Enum.GetValues())
+ {
+ Assert.NotEqual("AcknowledgeAlarmByName", value.ToString());
+ }
+
+ var original = new MxCommandReply
+ {
+ SessionId = "session-1",
+ CorrelationId = "gateway-correlation-7",
+ Kind = MxCommandKind.AcknowledgeAlarmByName,
+ ProtocolStatus = new ProtocolStatus { Code = ProtocolStatusCode.Ok },
+ Hresult = 0,
+ // By-name ack reuses the acknowledge_alarm payload case; see the
+ // worker's MxAccessCommandExecutor.ExecuteAcknowledgeAlarmByName.
+ AcknowledgeAlarm = new AcknowledgeAlarmReplyPayload
+ {
+ NativeStatus = 0,
+ },
+ };
+
+ var parsed = MxCommandReply.Parser.ParseFrom(original.ToByteArray());
+
+ Assert.Equal(original, parsed);
+ // Kind distinguishes the by-name ack; the payload case is shared.
+ Assert.Equal(MxCommandKind.AcknowledgeAlarmByName, parsed.Kind);
+ Assert.Equal(MxCommandReply.PayloadOneofCase.AcknowledgeAlarm, parsed.PayloadCase);
+ Assert.Equal(0, parsed.AcknowledgeAlarm.NativeStatus);
+ // The by-name command has its own command payload case — the asymmetry
+ // with the reply oneof is the documented contract under test.
+ Assert.Contains(
+ MxCommand.PayloadOneofCase.AcknowledgeAlarmByNameCommand,
+ System.Enum.GetValues());
+ }
+
/// Verifies that ActiveAlarmSnapshot round-trips with current state and operator metadata.
[Fact]
public void ActiveAlarmSnapshot_RoundTripsAllFields()