Compare commits

...

10 Commits

Author SHA1 Message Date
Joseph Doherty cd92048f4e Regenerate stale Java client protobuf code
The checked-in generated Java sources under clients/java/src/main/generated/
were out of sync with both the .proto contracts and the configured
protobuf 4.33.1 toolchain: they were missing the alarm command kinds
(MX_COMMAND_KIND_SUBSCRIBE_ALARMS..ACKNOWLEDGE_ALARM_BY_NAME, 25-29), the
alarm/galaxy message additions, and the protobuf 4.x generated-code layout.
Regenerated via `gradle generateProto`; `gradle test` passes against the
refreshed sources. No hand edits — pure protoc/protoc-gen-grpc-java output.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-18 23:21:13 -04:00
Joseph Doherty 964b40dcbc Fix stale WorkerProjectReferenceTests MXAccess-interop assertion
MxAccessInteropReference_ExistsOnlyInWorkerProject asserted the MXAccess COM
interop was referenced only by MxGateway.Worker. The worker test project now
legitimately references ArchestrA.MxAccess and Interop.WNWRAPCONSUMERLib so it
can exercise the COM-facing worker code (WnWrapAlarmConsumer, the alarm
tests). Renamed to ..._ExistsOnlyInWorkerAndWorkerTestProjects, updated the
assertion to expect both projects, and made it order-independent. The
architecture invariant the test protects — the gateway/contracts never
reference MXAccess COM — still holds.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-18 23:19:32 -04:00
Joseph Doherty bb5603b7ec Fix flaky GalaxyHierarchyRefreshServiceTests timing race
ExecuteAsync_WhenFirstRefreshThrowsNonCancellationException_DoesNotFault
BackgroundService cancelled the service immediately after StartAsync, so
under parallel load the first RefreshAsync could be skipped (RefreshCallCount
0) and `await executeTask` rethrew TaskCanceledException before the IsFaulted
assertion. The test now waits for a TaskCompletionSource signal that the
first refresh was attempted before cancelling, and uses Task.WhenAny so a
Canceled ExecuteTask does not rethrow. Confirmed stable across full-suite
runs (408/408).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-18 23:14:47 -04:00
Joseph Doherty 24de7e21d9 Regenerate code-reviews index after Low findings Batch 3
Reflects resolution of Contracts-001/004/005/006/007/008 (and Contracts-003
re-triaged Won't Fix). All code-review findings across every module are now
closed. Also normalizes the Contracts-003 Status to the canonical
`Won't Fix` value the index generator expects.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-18 23:12:34 -04:00
Joseph Doherty ee959e46e6 Resolve Contracts-001/004/005/006/007/008 code-review findings
Contracts-001: docs/Grpc.md still described "four MxAccessGateway RPCs" —
updated to the actual six (adding AcknowledgeAlarm and QueryActiveAlarms to
the handler and validation-rule sections).

Contracts-003 (Won't Fix): the finding is factually wrong — the <Protobuf>
item for mxaccess_worker.proto already sets ProtoRoot="Protos"; all three
items are consistent (confirmed back to the reviewed commit).

Contracts-004: corrected the stale GatewayContractInfo XML summary
("before generated protobuf contracts are introduced").

Contracts-005: no proto field/enum value was ever removed, so no reserved
ranges were invented. Added a wire-compatibility policy comment to all three
.proto files instructing future editors to reserve removed numbers.

Contracts-006: documented MxStatusProxy.success — it mirrors the COM
MXSTATUS_PROXY numeric success member, is not a boolean, and clients should
branch on category.

Contracts-007: added 13 round-trip tests covering galaxy_repository.proto
messages, bulk-subscribe payloads, and raw-value/IPC worker bodies.

Contracts-008: WorkerAlarmRpcDispatcher never assigns AcknowledgeAlarmReply.
status, so the old "native status" proto comment was misleading. Corrected
the hresult/status proto comments and documented the worker native_status →
public reply mapping in AlarmClientDiscovery.md.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-18 23:12:00 -04:00
Joseph Doherty 771229b39f Regenerate code-reviews index after Low findings Batch 2
Reflects resolution of Tests-007..012, Worker.Tests-008..015,
IntegrationTests-007..010, Client.Python-001/002/004/006/007/008/010/011/012.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-18 22:59:35 -04:00
Joseph Doherty a7bf1ef95d Resolve Client.Python-001/002/004/006/007/008/010/011/012 findings
Client.Python-001: dropped "scaffold" from the stale pyproject description.
Client.Python-002 (re-triaged): stale finding — MxGatewayCommandError is
already exported and in __all__; no change needed.
Client.Python-004: removed the dead `closed` variable in _smoke; the CLI
smoke now uses `async with session`.
Client.Python-006: close() on both clients and Session had an unlocked
check-then-set race; `_closed` is now set before the await.
Client.Python-007: gateway stream iterators now share one helper that
explicitly catches CancelledError and cancels the call.
Client.Python-008: to_mx_value now rejects nan/inf; float/bytes mapping
documented.
Client.Python-010: removed the circular-import-workaround late imports in
favour of TYPE_CHECKING / module-scope imports.
Client.Python-011: ensure_mxaccess_success no longer treats a proto3-default
success==0 with an unset category as a failure.
Client.Python-012 (Won't Fix): invoke_raw deliberately skips MXAccess-failure
detection for parity tests; documented the contract instead.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-18 22:59:24 -04:00
Joseph Doherty b4f5e8eb48 Resolve IntegrationTests-007..010 code-review findings
IntegrationTests-007: the three live test classes contend for shared
singletons (one MXAccess COM, one ZB SQL DB, one GLAuth). Added
LiveResourcesCollection with DisableParallelization and applied it to all
three so they no longer run concurrently.

IntegrationTests-008: the three live fact attributes each re-implemented the
env-var check. Added IntegrationTestEnvironment.IsEnabled and all three now
delegate to it.

IntegrationTests-009: reworded the misleading "Mock server call context" XML
doc — it is a hand-written stub with no verification behavior.

IntegrationTests-010: WaitForMessageAsync ignored cancellation. It now takes
an optional CancellationToken linked with the timeout; the smoke test shares
one cancellation source with the StreamEvents call context.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-18 22:59:18 -04:00
Joseph Doherty 371bcb3f91 Resolve Worker.Tests-008..015 code-review findings
Worker.Tests-008: moved the misplaced WorkerLogRedactor test out of
VariantConverterTests into Bootstrap/WorkerLogRedactorTests.

Worker.Tests-009: renamed 46 snake_case alarm-test methods to PascalCase
Method_Scenario_Expectation.

Worker.Tests-010: replaced a weak Assert.Contains with an exact assertion
against the real diagnostic message and corrected the XML doc.

Worker.Tests-011: renamed and re-documented a cancellation test that
overstated what it proved.

Worker.Tests-012: added an oversized-frame (MessageTooLarge) test; renamed
the mislabeled zero-length-payload test.

Worker.Tests-013: removed the fixed-100ms ThrowIfCompletedAsync helper; the
caller now races runTask deterministically.

Worker.Tests-014: consolidated duplicated test fakes/helpers
(FakeRuntimeSession, NoopComApartmentInitializer, NoopEventSink, frame
helpers) into a shared TestSupport namespace.

Worker.Tests-015: added MxAccessEventQueue coverage for drain-all (maxEvents
0), empty-queue drain, and enqueue-after-fault.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-18 22:59:07 -04:00
Joseph Doherty 9582de077b Resolve Tests-007..012 code-review findings
Tests-007: TestServerCallContext and stream-writer/constraint helpers were
copy-pasted across five test files. Consolidated into a shared
MxGateway.Tests.TestSupport namespace; duplicates deleted.

Tests-008: renamed snake_case alarm-test methods to PascalCase
Method_Condition_Result and dropped redundant usings. Re-triaged two
inaccurate sub-claims (the "wnwrap" name and a required CompilerServices
using).

Tests-009: corrected three copy-paste-mismatched XML <summary> comments in
SessionManagerTests.

Tests-010: added the missing anonymous-localhost security negatives —
bypass disallowed, and loopback-allowed from a remote address.

Tests-011: SessionWorkerClientFactoryFakeWorkerTests discarded worker tasks.
The test class now tracks each launcher and observes its task in DisposeAsync.

Tests-012: added xunit.runner.json pinning collection parallelism and
documented the ephemeral-port convention.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-18 22:59:01 -04:00
69 changed files with 19833 additions and 1662 deletions
@@ -139,6 +139,68 @@ public final class MxAccessGatewayGrpc {
return getStreamEventsMethod; return getStreamEventsMethod;
} }
private static volatile io.grpc.MethodDescriptor<mxaccess_gateway.v1.MxaccessGateway.AcknowledgeAlarmRequest,
mxaccess_gateway.v1.MxaccessGateway.AcknowledgeAlarmReply> getAcknowledgeAlarmMethod;
@io.grpc.stub.annotations.RpcMethod(
fullMethodName = SERVICE_NAME + '/' + "AcknowledgeAlarm",
requestType = mxaccess_gateway.v1.MxaccessGateway.AcknowledgeAlarmRequest.class,
responseType = mxaccess_gateway.v1.MxaccessGateway.AcknowledgeAlarmReply.class,
methodType = io.grpc.MethodDescriptor.MethodType.UNARY)
public static io.grpc.MethodDescriptor<mxaccess_gateway.v1.MxaccessGateway.AcknowledgeAlarmRequest,
mxaccess_gateway.v1.MxaccessGateway.AcknowledgeAlarmReply> getAcknowledgeAlarmMethod() {
io.grpc.MethodDescriptor<mxaccess_gateway.v1.MxaccessGateway.AcknowledgeAlarmRequest, mxaccess_gateway.v1.MxaccessGateway.AcknowledgeAlarmReply> getAcknowledgeAlarmMethod;
if ((getAcknowledgeAlarmMethod = MxAccessGatewayGrpc.getAcknowledgeAlarmMethod) == null) {
synchronized (MxAccessGatewayGrpc.class) {
if ((getAcknowledgeAlarmMethod = MxAccessGatewayGrpc.getAcknowledgeAlarmMethod) == null) {
MxAccessGatewayGrpc.getAcknowledgeAlarmMethod = getAcknowledgeAlarmMethod =
io.grpc.MethodDescriptor.<mxaccess_gateway.v1.MxaccessGateway.AcknowledgeAlarmRequest, mxaccess_gateway.v1.MxaccessGateway.AcknowledgeAlarmReply>newBuilder()
.setType(io.grpc.MethodDescriptor.MethodType.UNARY)
.setFullMethodName(generateFullMethodName(SERVICE_NAME, "AcknowledgeAlarm"))
.setSampledToLocalTracing(true)
.setRequestMarshaller(io.grpc.protobuf.ProtoUtils.marshaller(
mxaccess_gateway.v1.MxaccessGateway.AcknowledgeAlarmRequest.getDefaultInstance()))
.setResponseMarshaller(io.grpc.protobuf.ProtoUtils.marshaller(
mxaccess_gateway.v1.MxaccessGateway.AcknowledgeAlarmReply.getDefaultInstance()))
.setSchemaDescriptor(new MxAccessGatewayMethodDescriptorSupplier("AcknowledgeAlarm"))
.build();
}
}
}
return getAcknowledgeAlarmMethod;
}
private static volatile io.grpc.MethodDescriptor<mxaccess_gateway.v1.MxaccessGateway.QueryActiveAlarmsRequest,
mxaccess_gateway.v1.MxaccessGateway.ActiveAlarmSnapshot> getQueryActiveAlarmsMethod;
@io.grpc.stub.annotations.RpcMethod(
fullMethodName = SERVICE_NAME + '/' + "QueryActiveAlarms",
requestType = mxaccess_gateway.v1.MxaccessGateway.QueryActiveAlarmsRequest.class,
responseType = mxaccess_gateway.v1.MxaccessGateway.ActiveAlarmSnapshot.class,
methodType = io.grpc.MethodDescriptor.MethodType.SERVER_STREAMING)
public static io.grpc.MethodDescriptor<mxaccess_gateway.v1.MxaccessGateway.QueryActiveAlarmsRequest,
mxaccess_gateway.v1.MxaccessGateway.ActiveAlarmSnapshot> getQueryActiveAlarmsMethod() {
io.grpc.MethodDescriptor<mxaccess_gateway.v1.MxaccessGateway.QueryActiveAlarmsRequest, mxaccess_gateway.v1.MxaccessGateway.ActiveAlarmSnapshot> getQueryActiveAlarmsMethod;
if ((getQueryActiveAlarmsMethod = MxAccessGatewayGrpc.getQueryActiveAlarmsMethod) == null) {
synchronized (MxAccessGatewayGrpc.class) {
if ((getQueryActiveAlarmsMethod = MxAccessGatewayGrpc.getQueryActiveAlarmsMethod) == null) {
MxAccessGatewayGrpc.getQueryActiveAlarmsMethod = getQueryActiveAlarmsMethod =
io.grpc.MethodDescriptor.<mxaccess_gateway.v1.MxaccessGateway.QueryActiveAlarmsRequest, mxaccess_gateway.v1.MxaccessGateway.ActiveAlarmSnapshot>newBuilder()
.setType(io.grpc.MethodDescriptor.MethodType.SERVER_STREAMING)
.setFullMethodName(generateFullMethodName(SERVICE_NAME, "QueryActiveAlarms"))
.setSampledToLocalTracing(true)
.setRequestMarshaller(io.grpc.protobuf.ProtoUtils.marshaller(
mxaccess_gateway.v1.MxaccessGateway.QueryActiveAlarmsRequest.getDefaultInstance()))
.setResponseMarshaller(io.grpc.protobuf.ProtoUtils.marshaller(
mxaccess_gateway.v1.MxaccessGateway.ActiveAlarmSnapshot.getDefaultInstance()))
.setSchemaDescriptor(new MxAccessGatewayMethodDescriptorSupplier("QueryActiveAlarms"))
.build();
}
}
}
return getQueryActiveAlarmsMethod;
}
/** /**
* Creates a new async stub that supports all call types for the service * Creates a new async stub that supports all call types for the service
*/ */
@@ -232,6 +294,20 @@ public final class MxAccessGatewayGrpc {
io.grpc.stub.StreamObserver<mxaccess_gateway.v1.MxaccessGateway.MxEvent> responseObserver) { io.grpc.stub.StreamObserver<mxaccess_gateway.v1.MxaccessGateway.MxEvent> responseObserver) {
io.grpc.stub.ServerCalls.asyncUnimplementedUnaryCall(getStreamEventsMethod(), responseObserver); io.grpc.stub.ServerCalls.asyncUnimplementedUnaryCall(getStreamEventsMethod(), responseObserver);
} }
/**
*/
default void acknowledgeAlarm(mxaccess_gateway.v1.MxaccessGateway.AcknowledgeAlarmRequest request,
io.grpc.stub.StreamObserver<mxaccess_gateway.v1.MxaccessGateway.AcknowledgeAlarmReply> responseObserver) {
io.grpc.stub.ServerCalls.asyncUnimplementedUnaryCall(getAcknowledgeAlarmMethod(), responseObserver);
}
/**
*/
default void queryActiveAlarms(mxaccess_gateway.v1.MxaccessGateway.QueryActiveAlarmsRequest request,
io.grpc.stub.StreamObserver<mxaccess_gateway.v1.MxaccessGateway.ActiveAlarmSnapshot> responseObserver) {
io.grpc.stub.ServerCalls.asyncUnimplementedUnaryCall(getQueryActiveAlarmsMethod(), responseObserver);
}
} }
/** /**
@@ -298,6 +374,22 @@ public final class MxAccessGatewayGrpc {
io.grpc.stub.ClientCalls.asyncServerStreamingCall( io.grpc.stub.ClientCalls.asyncServerStreamingCall(
getChannel().newCall(getStreamEventsMethod(), getCallOptions()), request, responseObserver); getChannel().newCall(getStreamEventsMethod(), getCallOptions()), request, responseObserver);
} }
/**
*/
public void acknowledgeAlarm(mxaccess_gateway.v1.MxaccessGateway.AcknowledgeAlarmRequest request,
io.grpc.stub.StreamObserver<mxaccess_gateway.v1.MxaccessGateway.AcknowledgeAlarmReply> responseObserver) {
io.grpc.stub.ClientCalls.asyncUnaryCall(
getChannel().newCall(getAcknowledgeAlarmMethod(), getCallOptions()), request, responseObserver);
}
/**
*/
public void queryActiveAlarms(mxaccess_gateway.v1.MxaccessGateway.QueryActiveAlarmsRequest request,
io.grpc.stub.StreamObserver<mxaccess_gateway.v1.MxaccessGateway.ActiveAlarmSnapshot> responseObserver) {
io.grpc.stub.ClientCalls.asyncServerStreamingCall(
getChannel().newCall(getQueryActiveAlarmsMethod(), getCallOptions()), request, responseObserver);
}
} }
/** /**
@@ -348,6 +440,22 @@ public final class MxAccessGatewayGrpc {
return io.grpc.stub.ClientCalls.blockingV2ServerStreamingCall( return io.grpc.stub.ClientCalls.blockingV2ServerStreamingCall(
getChannel(), getStreamEventsMethod(), getCallOptions(), request); getChannel(), getStreamEventsMethod(), getCallOptions(), request);
} }
/**
*/
public mxaccess_gateway.v1.MxaccessGateway.AcknowledgeAlarmReply acknowledgeAlarm(mxaccess_gateway.v1.MxaccessGateway.AcknowledgeAlarmRequest request) throws io.grpc.StatusException {
return io.grpc.stub.ClientCalls.blockingV2UnaryCall(
getChannel(), getAcknowledgeAlarmMethod(), getCallOptions(), request);
}
/**
*/
@io.grpc.ExperimentalApi("https://github.com/grpc/grpc-java/issues/10918")
public io.grpc.stub.BlockingClientCall<?, mxaccess_gateway.v1.MxaccessGateway.ActiveAlarmSnapshot>
queryActiveAlarms(mxaccess_gateway.v1.MxaccessGateway.QueryActiveAlarmsRequest request) {
return io.grpc.stub.ClientCalls.blockingV2ServerStreamingCall(
getChannel(), getQueryActiveAlarmsMethod(), getCallOptions(), request);
}
} }
/** /**
@@ -397,6 +505,21 @@ public final class MxAccessGatewayGrpc {
return io.grpc.stub.ClientCalls.blockingServerStreamingCall( return io.grpc.stub.ClientCalls.blockingServerStreamingCall(
getChannel(), getStreamEventsMethod(), getCallOptions(), request); getChannel(), getStreamEventsMethod(), getCallOptions(), request);
} }
/**
*/
public mxaccess_gateway.v1.MxaccessGateway.AcknowledgeAlarmReply acknowledgeAlarm(mxaccess_gateway.v1.MxaccessGateway.AcknowledgeAlarmRequest request) {
return io.grpc.stub.ClientCalls.blockingUnaryCall(
getChannel(), getAcknowledgeAlarmMethod(), getCallOptions(), request);
}
/**
*/
public java.util.Iterator<mxaccess_gateway.v1.MxaccessGateway.ActiveAlarmSnapshot> queryActiveAlarms(
mxaccess_gateway.v1.MxaccessGateway.QueryActiveAlarmsRequest request) {
return io.grpc.stub.ClientCalls.blockingServerStreamingCall(
getChannel(), getQueryActiveAlarmsMethod(), getCallOptions(), request);
}
} }
/** /**
@@ -441,12 +564,22 @@ public final class MxAccessGatewayGrpc {
return io.grpc.stub.ClientCalls.futureUnaryCall( return io.grpc.stub.ClientCalls.futureUnaryCall(
getChannel().newCall(getInvokeMethod(), getCallOptions()), request); getChannel().newCall(getInvokeMethod(), getCallOptions()), request);
} }
/**
*/
public com.google.common.util.concurrent.ListenableFuture<mxaccess_gateway.v1.MxaccessGateway.AcknowledgeAlarmReply> acknowledgeAlarm(
mxaccess_gateway.v1.MxaccessGateway.AcknowledgeAlarmRequest request) {
return io.grpc.stub.ClientCalls.futureUnaryCall(
getChannel().newCall(getAcknowledgeAlarmMethod(), getCallOptions()), request);
}
} }
private static final int METHODID_OPEN_SESSION = 0; private static final int METHODID_OPEN_SESSION = 0;
private static final int METHODID_CLOSE_SESSION = 1; private static final int METHODID_CLOSE_SESSION = 1;
private static final int METHODID_INVOKE = 2; private static final int METHODID_INVOKE = 2;
private static final int METHODID_STREAM_EVENTS = 3; private static final int METHODID_STREAM_EVENTS = 3;
private static final int METHODID_ACKNOWLEDGE_ALARM = 4;
private static final int METHODID_QUERY_ACTIVE_ALARMS = 5;
private static final class MethodHandlers<Req, Resp> implements private static final class MethodHandlers<Req, Resp> implements
io.grpc.stub.ServerCalls.UnaryMethod<Req, Resp>, io.grpc.stub.ServerCalls.UnaryMethod<Req, Resp>,
@@ -481,6 +614,14 @@ public final class MxAccessGatewayGrpc {
serviceImpl.streamEvents((mxaccess_gateway.v1.MxaccessGateway.StreamEventsRequest) request, serviceImpl.streamEvents((mxaccess_gateway.v1.MxaccessGateway.StreamEventsRequest) request,
(io.grpc.stub.StreamObserver<mxaccess_gateway.v1.MxaccessGateway.MxEvent>) responseObserver); (io.grpc.stub.StreamObserver<mxaccess_gateway.v1.MxaccessGateway.MxEvent>) responseObserver);
break; break;
case METHODID_ACKNOWLEDGE_ALARM:
serviceImpl.acknowledgeAlarm((mxaccess_gateway.v1.MxaccessGateway.AcknowledgeAlarmRequest) request,
(io.grpc.stub.StreamObserver<mxaccess_gateway.v1.MxaccessGateway.AcknowledgeAlarmReply>) responseObserver);
break;
case METHODID_QUERY_ACTIVE_ALARMS:
serviceImpl.queryActiveAlarms((mxaccess_gateway.v1.MxaccessGateway.QueryActiveAlarmsRequest) request,
(io.grpc.stub.StreamObserver<mxaccess_gateway.v1.MxaccessGateway.ActiveAlarmSnapshot>) responseObserver);
break;
default: default:
throw new AssertionError(); throw new AssertionError();
} }
@@ -527,6 +668,20 @@ public final class MxAccessGatewayGrpc {
mxaccess_gateway.v1.MxaccessGateway.StreamEventsRequest, mxaccess_gateway.v1.MxaccessGateway.StreamEventsRequest,
mxaccess_gateway.v1.MxaccessGateway.MxEvent>( mxaccess_gateway.v1.MxaccessGateway.MxEvent>(
service, METHODID_STREAM_EVENTS))) service, METHODID_STREAM_EVENTS)))
.addMethod(
getAcknowledgeAlarmMethod(),
io.grpc.stub.ServerCalls.asyncUnaryCall(
new MethodHandlers<
mxaccess_gateway.v1.MxaccessGateway.AcknowledgeAlarmRequest,
mxaccess_gateway.v1.MxaccessGateway.AcknowledgeAlarmReply>(
service, METHODID_ACKNOWLEDGE_ALARM)))
.addMethod(
getQueryActiveAlarmsMethod(),
io.grpc.stub.ServerCalls.asyncServerStreamingCall(
new MethodHandlers<
mxaccess_gateway.v1.MxaccessGateway.QueryActiveAlarmsRequest,
mxaccess_gateway.v1.MxaccessGateway.ActiveAlarmSnapshot>(
service, METHODID_QUERY_ACTIVE_ALARMS)))
.build(); .build();
} }
@@ -579,6 +734,8 @@ public final class MxAccessGatewayGrpc {
.addMethod(getCloseSessionMethod()) .addMethod(getCloseSessionMethod())
.addMethod(getInvokeMethod()) .addMethod(getInvokeMethod())
.addMethod(getStreamEventsMethod()) .addMethod(getStreamEventsMethod())
.addMethod(getAcknowledgeAlarmMethod())
.addMethod(getQueryActiveAlarmsMethod())
.build(); .build();
} }
} }
File diff suppressed because it is too large Load Diff
+16
View File
@@ -95,6 +95,22 @@ async with await GatewayClient.connect(
events available for parity tests. `Session` helpers call the method-specific events available for parity tests. `Session` helpers call the method-specific
MXAccess commands and preserve raw replies on typed command exceptions. MXAccess commands and preserve raw replies on typed command exceptions.
`*_raw` methods (`GatewayClient.invoke_raw`, `Session.invoke_raw`) surface
gateway protocol failures by raising the typed `MxGateway*` exceptions, but
they deliberately do **not** run MXAccess-failure detection: an MXAccess
HRESULT or `MxStatusProxy` status failure is left embedded in the returned
reply and no `MxAccessError` is raised. `Session.invoke` adds that check on
top. Parity-test callers using `invoke_raw` must inspect the reply's
`protocol_status`, `hresult`, and `statuses` themselves. The non-raw `Session`
helpers (`register`, `add_item`, `write`, the bulk methods, etc.) run the
check and raise `MxAccessError`.
Value conversion (`to_mx_value`, used by `Session.write`/`write2` and the
bulk helpers) rejects non-finite floats — `nan`, `inf`, and `-inf` raise
`ValueError` rather than being forwarded to MXAccess, which has no defined
wire representation for them. Python `bytes` values are an opaque
`VT_RECORD` pass-through that MXAccess does not interpret.
Canceling a Python task cancels the client-side gRPC call or stream wait. It Canceling a Python task cancels the client-side gRPC call or stream wait. It
does not abort an in-flight MXAccess COM call inside the worker process. does not abort an in-flight MXAccess COM call inside the worker process.
+1 -1
View File
@@ -5,7 +5,7 @@ build-backend = "setuptools.build_meta"
[project] [project]
name = "mxaccess-gateway-client" name = "mxaccess-gateway-client"
version = "0.1.0" version = "0.1.0"
description = "Async Python client scaffold for MXAccess Gateway." description = "Async Python client for MXAccess Gateway."
readme = "README.md" readme = "README.md"
requires-python = ">=3.12" requires-python = ">=3.12"
dependencies = [ dependencies = [
+37 -21
View File
@@ -72,14 +72,20 @@ class GatewayClient:
await self.close() await self.close()
async def close(self) -> None: async def close(self) -> None:
"""Close the owned gRPC channel.""" """Close the owned gRPC channel.
Idempotent, including under concurrent calls: ``_closed`` is set
before the ``await`` so a second coroutine entering ``close()``
while the first is still awaiting the channel close returns
immediately instead of issuing a second ``channel.close()``.
"""
if self._closed: if self._closed:
return return
self._closed = True
if self._channel is not None: if self._channel is not None:
await self._channel.close() await self._channel.close()
self._closed = True
async def open_session( async def open_session(
self, self,
@@ -117,7 +123,15 @@ class GatewayClient:
return reply return reply
async def invoke_raw(self, request: pb.MxCommandRequest) -> pb.MxCommandReply: async def invoke_raw(self, request: pb.MxCommandRequest) -> pb.MxCommandReply:
"""Send an `Invoke` RPC and return the raw reply.""" """Send an `Invoke` RPC and return the raw reply.
Enforces gateway protocol success only. MXAccess HRESULT/status
failures are left embedded in the reply and do not raise
`MxAccessError` — parity-test callers must inspect the reply's
`protocol_status`, `hresult`, and `statuses` themselves. Use
`Session.invoke` for the variant that also raises on MXAccess
failure.
"""
reply = await self._unary("invoke", self.raw_stub.Invoke, request) reply = await self._unary("invoke", self.raw_stub.Invoke, request)
ensure_protocol_success("invoke", reply.protocol_status, reply) ensure_protocol_success("invoke", reply.protocol_status, reply)
return reply return reply
@@ -134,7 +148,7 @@ class GatewayClient:
if self.options.stream_timeout is not None: if self.options.stream_timeout is not None:
kwargs["timeout"] = self.options.stream_timeout kwargs["timeout"] = self.options.stream_timeout
call = _open_stream(self.raw_stub.StreamEvents, request, kwargs) call = _open_stream(self.raw_stub.StreamEvents, request, kwargs)
return _canceling_iterator(call) return _canceling_iterator(call, "stream events")
async def acknowledge_alarm( async def acknowledge_alarm(
self, self,
@@ -170,7 +184,7 @@ class GatewayClient:
if self.options.stream_timeout is not None: if self.options.stream_timeout is not None:
kwargs["timeout"] = self.options.stream_timeout kwargs["timeout"] = self.options.stream_timeout
call = _open_stream(self.raw_stub.QueryActiveAlarms, request, kwargs) call = _open_stream(self.raw_stub.QueryActiveAlarms, request, kwargs)
return _canceling_active_alarms_iterator(call) return _canceling_iterator(call, "query active alarms")
async def _unary( async def _unary(
self, self,
@@ -218,24 +232,26 @@ def _open_stream(method: Any, request: Any, kwargs: dict[str, Any]) -> Any:
return method(request, **kwargs) return method(request, **kwargs)
async def _canceling_iterator(call: Any) -> AsyncIterator[pb.MxEvent]: async def _canceling_iterator(call: Any, operation: str) -> AsyncIterator[Any]:
"""Yield from a server-streaming call and cancel it when iteration stops.
Explicitly catches :class:`asyncio.CancelledError` to cancel the
underlying call before re-raising, then repeats the cancel in the
``finally`` block so the call is also cancelled on a clean break or an
``aclose()``. ``galaxy._canceling_iterator`` delegates here so the
gateway and Galaxy stream helpers stay identical.
"""
try: try:
async for event in call: async for item in call:
yield event yield item
except asyncio.CancelledError:
cancel = getattr(call, "cancel", None)
if cancel is not None:
cancel()
raise
except grpc.RpcError as error: except grpc.RpcError as error:
raise map_rpc_error("stream events", error) from error raise map_rpc_error(operation, error) from error
finally:
cancel = getattr(call, "cancel", None)
if cancel is not None:
cancel()
async def _canceling_active_alarms_iterator(call: Any) -> AsyncIterator[pb.ActiveAlarmSnapshot]:
try:
async for snapshot in call:
yield snapshot
except grpc.RpcError as error:
raise map_rpc_error("query active alarms", error) from error
finally: finally:
cancel = getattr(call, "cancel", None) cancel = getattr(call, "cancel", None)
if cancel is not None: if cancel is not None:
+23 -1
View File
@@ -138,7 +138,7 @@ def ensure_mxaccess_success(operation: str, reply: pb.MxCommandReply) -> pb.MxCo
) )
for mx_status in reply.statuses: for mx_status in reply.statuses:
if mx_status.success == 0: if _is_mxaccess_status_failure(mx_status):
raise MxAccessError( raise MxAccessError(
_mxaccess_message(operation, reply), _mxaccess_message(operation, reply),
protocol_status=status, protocol_status=status,
@@ -148,6 +148,28 @@ def ensure_mxaccess_success(operation: str, reply: pb.MxCommandReply) -> pb.MxCo
return reply return reply
def _is_mxaccess_status_failure(mx_status: pb.MxStatusProxy) -> bool:
"""Return ``True`` only for a populated MXAccess status reporting failure.
MXAccess uses ``success == 0`` as the failure flag, but ``0`` is also the
proto3 scalar default. The gateway emits placeholder ``MxStatusProxy``
entries with ``success`` unset for null ``MXSTATUS_PROXY`` COM entries
(see ``MxStatusProxyConverter.ConvertMany``); such an entry has
``category`` of ``UNSPECIFIED`` or ``UNKNOWN``. Treating it as a failure
would raise ``MxAccessError`` for a reply that carries no real failure,
so failure is keyed on ``success == 0`` together with a populated,
non-OK status category.
"""
if mx_status.success != 0:
return False
return mx_status.category not in (
pb.MX_STATUS_CATEGORY_UNSPECIFIED,
pb.MX_STATUS_CATEGORY_UNKNOWN,
pb.MX_STATUS_CATEGORY_OK,
)
def _mxaccess_message(operation: str, reply: pb.MxCommandReply) -> str: def _mxaccess_message(operation: str, reply: pb.MxCommandReply) -> str:
status_text = reply.protocol_status.message or "MXAccess command failed" status_text = reply.protocol_status.message or "MXAccess command failed"
hresult = reply.hresult if reply.HasField("hresult") else None hresult = reply.hresult if reply.HasField("hresult") else None
+10 -20
View File
@@ -18,6 +18,7 @@ import grpc
from google.protobuf.timestamp_pb2 import Timestamp from google.protobuf.timestamp_pb2 import Timestamp
from .auth import merge_metadata from .auth import merge_metadata
from .client import _canceling_iterator
from .errors import MxGatewayError, map_rpc_error from .errors import MxGatewayError, map_rpc_error
from .generated import galaxy_repository_pb2 as galaxy_pb from .generated import galaxy_repository_pb2 as galaxy_pb
from .generated import galaxy_repository_pb2_grpc as galaxy_pb_grpc from .generated import galaxy_repository_pb2_grpc as galaxy_pb_grpc
@@ -83,14 +84,20 @@ class GalaxyRepositoryClient:
await self.close() await self.close()
async def close(self) -> None: async def close(self) -> None:
"""Close the owned gRPC channel.""" """Close the owned gRPC channel.
Idempotent, including under concurrent calls: ``_closed`` is set
before the ``await`` so a second coroutine entering ``close()``
while the first is still awaiting the channel close returns
immediately instead of issuing a second ``channel.close()``.
"""
if self._closed: if self._closed:
return return
self._closed = True
if self._channel is not None: if self._channel is not None:
await self._channel.close() await self._channel.close()
self._closed = True
async def test_connection(self) -> bool: async def test_connection(self) -> bool:
"""Return ``True`` when the gateway can reach the Galaxy Repository DB.""" """Return ``True`` when the gateway can reach the Galaxy Repository DB."""
@@ -189,7 +196,7 @@ class GalaxyRepositoryClient:
kwargs.pop("timeout") kwargs.pop("timeout")
call = self.raw_stub.WatchDeployEvents(request, **kwargs) call = self.raw_stub.WatchDeployEvents(request, **kwargs)
return _canceling_iterator(call) return _canceling_iterator(call, "watch deploy events")
async def _unary( async def _unary(
self, self,
@@ -218,20 +225,3 @@ class GalaxyRepositoryClient:
raise raise
except grpc.RpcError as error: except grpc.RpcError as error:
raise map_rpc_error(operation, error) from error raise map_rpc_error(operation, error) from error
async def _canceling_iterator(call: Any) -> AsyncIterator[galaxy_pb.DeployEvent]:
try:
async for event in call:
yield event
except asyncio.CancelledError:
cancel = getattr(call, "cancel", None)
if cancel is not None:
cancel()
raise
except grpc.RpcError as error:
raise map_rpc_error("watch deploy events", error) from error
finally:
cancel = getattr(call, "cancel", None)
if cancel is not None:
cancel()
+22 -8
View File
@@ -3,11 +3,15 @@
from __future__ import annotations from __future__ import annotations
from collections.abc import AsyncIterator, Sequence from collections.abc import AsyncIterator, Sequence
from typing import TYPE_CHECKING
from .errors import ensure_mxaccess_success from .errors import ensure_mxaccess_success
from .generated import mxaccess_gateway_pb2 as pb from .generated import mxaccess_gateway_pb2 as pb
from .values import MxValueInput, to_mx_value from .values import MxValueInput, to_mx_value
if TYPE_CHECKING:
from .client import GatewayClient
MAX_BULK_ITEMS = 1000 MAX_BULK_ITEMS = 1000
@@ -36,7 +40,13 @@ class Session:
await self.close() await self.close()
async def close(self, *, client_correlation_id: str = "") -> pb.CloseSessionReply: async def close(self, *, client_correlation_id: str = "") -> pb.CloseSessionReply:
"""Close the gateway session. Repeated calls return a local closed reply.""" """Close the gateway session. Repeated calls return a local closed reply.
Idempotent, including under concurrent calls: ``_closed`` is set
before the ``CloseSession`` RPC is awaited so a second coroutine
entering ``close()`` while the first RPC is in flight returns the
local closed reply instead of issuing a second ``CloseSession``.
"""
if self._closed: if self._closed:
return pb.CloseSessionReply( return pb.CloseSessionReply(
@@ -44,15 +54,14 @@ class Session:
final_state=pb.SESSION_STATE_CLOSED, final_state=pb.SESSION_STATE_CLOSED,
protocol_status=pb.ProtocolStatus(code=pb.PROTOCOL_STATUS_CODE_OK), protocol_status=pb.ProtocolStatus(code=pb.PROTOCOL_STATUS_CODE_OK),
) )
self._closed = True
reply = await self.client.close_session_raw( return await self.client.close_session_raw(
pb.CloseSessionRequest( pb.CloseSessionRequest(
session_id=self.session_id, session_id=self.session_id,
client_correlation_id=client_correlation_id, client_correlation_id=client_correlation_id,
), ),
) )
self._closed = True
return reply
async def invoke(self, command: pb.MxCommand, *, correlation_id: str = "") -> pb.MxCommandReply: async def invoke(self, command: pb.MxCommand, *, correlation_id: str = "") -> pb.MxCommandReply:
"""Invoke a raw command and enforce gateway and MXAccess success.""" """Invoke a raw command and enforce gateway and MXAccess success."""
@@ -66,7 +75,15 @@ class Session:
*, *,
correlation_id: str = "", correlation_id: str = "",
) -> pb.MxCommandReply: ) -> pb.MxCommandReply:
"""Invoke a raw command and preserve the raw reply.""" """Invoke a raw command and preserve the raw reply.
Enforces gateway protocol success only — unlike :meth:`invoke`, it
does not run MXAccess-failure detection. An MXAccess HRESULT or
``MxStatusProxy`` status failure is left embedded in the returned
reply and no ``MxAccessError`` is raised. Parity-test callers must
inspect ``protocol_status``, ``hresult``, and ``statuses`` on the
reply themselves.
"""
return await self.client.invoke_raw( return await self.client.invoke_raw(
pb.MxCommandRequest( pb.MxCommandRequest(
@@ -399,6 +416,3 @@ class Session:
def _ensure_bulk_size(name: str, count: int) -> None: def _ensure_bulk_size(name: str, count: int) -> None:
if count > MAX_BULK_ITEMS: if count > MAX_BULK_ITEMS:
raise ValueError(f"{name} bulk commands are limited to {MAX_BULK_ITEMS} item(s)") raise ValueError(f"{name} bulk commands are limited to {MAX_BULK_ITEMS} item(s)")
from .client import GatewayClient # noqa: E402
+26 -1
View File
@@ -1,7 +1,20 @@
"""MXAccess value conversion helpers.""" """MXAccess value conversion helpers.
Value-mapping assumptions (see ``to_mx_value``):
* A Python ``float`` maps to ``VT_R8`` / ``MX_DATA_TYPE_DOUBLE``. Only finite
values are accepted — ``nan``, ``inf`` and ``-inf`` raise ``ValueError``
rather than being forwarded to MXAccess, which has no defined wire
representation for non-finite doubles.
* A Python ``bytes`` value maps to ``VT_RECORD`` / ``MX_DATA_TYPE_UNKNOWN``
and is carried in ``raw_value``. This is an opaque pass-through: MXAccess
does not interpret the bytes. Pass ``data_type`` explicitly when a concrete
MXAccess type is required.
"""
from __future__ import annotations from __future__ import annotations
import math
from collections.abc import Sequence from collections.abc import Sequence
from dataclasses import dataclass from dataclasses import dataclass
from datetime import datetime, timezone from datetime import datetime, timezone
@@ -60,6 +73,7 @@ def to_mx_value(value: MxValueInput, *, data_type: str | None = None) -> pb.MxVa
) )
if isinstance(value, float): if isinstance(value, float):
_ensure_finite(value)
return pb.MxValue( return pb.MxValue(
data_type=_data_type(data_type, pb.MX_DATA_TYPE_DOUBLE), data_type=_data_type(data_type, pb.MX_DATA_TYPE_DOUBLE),
variant_type="VT_R8", variant_type="VT_R8",
@@ -177,6 +191,8 @@ def _sequence_to_mx_value(
return pb.MxValue(data_type=pb.MX_DATA_TYPE_INTEGER, array_value=array) return pb.MxValue(data_type=pb.MX_DATA_TYPE_INTEGER, array_value=array)
if all(isinstance(item, float) for item in sequence): if all(isinstance(item, float) for item in sequence):
for item in sequence:
_ensure_finite(item)
array = pb.MxArray( array = pb.MxArray(
element_data_type=pb.MX_DATA_TYPE_DOUBLE, element_data_type=pb.MX_DATA_TYPE_DOUBLE,
variant_type="VT_ARRAY|VT_R8", variant_type="VT_ARRAY|VT_R8",
@@ -232,3 +248,12 @@ def _data_type(name: str | None, default: int) -> int:
if name is None: if name is None:
return default return default
return pb.MxDataType.Value(name) return pb.MxDataType.Value(name)
def _ensure_finite(value: float) -> None:
"""Reject non-finite doubles, which MXAccess cannot represent on the wire."""
if not math.isfinite(value):
raise ValueError(
f"MxValue double inputs must be finite; got {value!r}",
)
+3 -8
View File
@@ -18,6 +18,7 @@ from mxgateway.client import GatewayClient
from mxgateway.errors import MxGatewayError from mxgateway.errors import MxGatewayError
from mxgateway.generated import mxaccess_gateway_pb2 as pb from mxgateway.generated import mxaccess_gateway_pb2 as pb
from mxgateway.options import ClientOptions from mxgateway.options import ClientOptions
from mxgateway.session import Session
from mxgateway.values import MxValueInput from mxgateway.values import MxValueInput
MAX_AGGREGATE_EVENTS = 10_000 MAX_AGGREGATE_EVENTS = 10_000
@@ -383,8 +384,7 @@ async def _write2(**kwargs: Any) -> dict[str, Any]:
async def _smoke(**kwargs: Any) -> dict[str, Any]: async def _smoke(**kwargs: Any) -> dict[str, Any]:
async with await _connect(kwargs) as client: async with await _connect(kwargs) as client:
session = await client.open_session(client_session_name=kwargs["client_name"]) session = await client.open_session(client_session_name=kwargs["client_name"])
closed = False async with session:
try:
server_handle = await session.register(kwargs["client_name"]) server_handle = await session.register(kwargs["client_name"])
item_handle = await session.add_item(server_handle, kwargs["item"]) item_handle = await session.add_item(server_handle, kwargs["item"])
await session.advise(server_handle, item_handle) await session.advise(server_handle, item_handle)
@@ -399,9 +399,6 @@ async def _smoke(**kwargs: Any) -> dict[str, Any]:
"itemHandle": item_handle, "itemHandle": item_handle,
"events": [_message_dict(event) for event in events], "events": [_message_dict(event) for event in events],
} }
finally:
if not closed:
await session.close()
async def _connect(kwargs: dict[str, Any]) -> GatewayClient: async def _connect(kwargs: dict[str, Any]) -> GatewayClient:
@@ -419,9 +416,7 @@ async def _connect(kwargs: dict[str, Any]) -> GatewayClient:
) )
def _session(client: GatewayClient, session_id: str): def _session(client: GatewayClient, session_id: str) -> Session:
from mxgateway.session import Session
return Session(client=client, session_id=session_id) return Session(client=client, session_id=session_id)
@@ -0,0 +1,228 @@
"""Regression tests for Client.Python low-severity code-review findings.
Covers Client.Python-006 (concurrent-close idempotency),
Client.Python-007 (shared cancelling stream helper),
Client.Python-008 (non-finite float / bytes value mapping), and
Client.Python-011 (`success == 0` proto3-default ambiguity).
"""
from __future__ import annotations
import asyncio
import math
from typing import Any
import pytest
from mxgateway import ClientOptions, GalaxyRepositoryClient, GatewayClient
from mxgateway.errors import ensure_mxaccess_success, MxAccessError
from mxgateway.generated import mxaccess_gateway_pb2 as pb
from mxgateway.values import to_mx_value
# --- Client.Python-006: concurrent close() is idempotent -------------------
class CountingChannel:
"""A fake gRPC channel that records and stalls on close()."""
def __init__(self) -> None:
self.close_calls = 0
self._gate = asyncio.Event()
async def close(self) -> None:
self.close_calls += 1
# Yield control so a second concurrent close() can interleave at the
# exact point a check-then-set guard would have left the window open.
await self._gate.wait()
@pytest.mark.asyncio
async def test_gateway_client_concurrent_close_closes_channel_once() -> None:
channel = CountingChannel()
client = GatewayClient(
options=ClientOptions(endpoint="fake", plaintext=True),
stub=object(),
channel=channel, # type: ignore[arg-type]
)
first = asyncio.create_task(client.close())
second = asyncio.create_task(client.close())
await asyncio.sleep(0) # let both coroutines pass the guard if racy
channel._gate.set()
await asyncio.gather(first, second)
assert channel.close_calls == 1
@pytest.mark.asyncio
async def test_galaxy_client_concurrent_close_closes_channel_once() -> None:
channel = CountingChannel()
client = GalaxyRepositoryClient(
options=ClientOptions(endpoint="fake", plaintext=True),
stub=object(),
channel=channel, # type: ignore[arg-type]
)
first = asyncio.create_task(client.close())
second = asyncio.create_task(client.close())
await asyncio.sleep(0)
channel._gate.set()
await asyncio.gather(first, second)
assert channel.close_calls == 1
@pytest.mark.asyncio
async def test_session_concurrent_close_sends_one_close_session_rpc() -> None:
gate = asyncio.Event()
rpc_calls = 0
class StallingClient:
async def close_session_raw(self, request: Any) -> pb.CloseSessionReply:
nonlocal rpc_calls
rpc_calls += 1
await gate.wait()
return pb.CloseSessionReply(
session_id=request.session_id,
final_state=pb.SESSION_STATE_CLOSED,
protocol_status=pb.ProtocolStatus(code=pb.PROTOCOL_STATUS_CODE_OK),
)
from mxgateway.session import Session
session = Session(client=StallingClient(), session_id="session-1") # type: ignore[arg-type]
first = asyncio.create_task(session.close())
second = asyncio.create_task(session.close())
await asyncio.sleep(0)
gate.set()
await asyncio.gather(first, second)
assert rpc_calls == 1
# --- Client.Python-007: shared cancelling stream helper --------------------
@pytest.mark.asyncio
async def test_gateway_stream_iterator_cancels_call_on_task_cancellation() -> None:
"""A cancelled gateway stream iterator must explicitly cancel the call."""
class CancellableStream:
def __init__(self) -> None:
self.cancelled = False
def __aiter__(self) -> "CancellableStream":
return self
async def __anext__(self) -> pb.MxEvent:
await asyncio.Event().wait() # blocks until cancelled
raise AssertionError("unreachable")
def cancel(self) -> None:
self.cancelled = True
from mxgateway.client import _canceling_iterator
stream = CancellableStream()
iterator = _canceling_iterator(stream, "stream events")
task = asyncio.create_task(anext(iterator))
await asyncio.sleep(0)
task.cancel()
with pytest.raises(asyncio.CancelledError):
await task
# aclose() unwinds the generator's finally block.
await iterator.aclose()
assert stream.cancelled
# --- Client.Python-008: non-finite float and bytes value mapping -----------
def test_to_mx_value_rejects_nan() -> None:
with pytest.raises(ValueError, match="finite"):
to_mx_value(float("nan"))
def test_to_mx_value_rejects_positive_infinity() -> None:
with pytest.raises(ValueError, match="finite"):
to_mx_value(float("inf"))
def test_to_mx_value_rejects_negative_infinity() -> None:
with pytest.raises(ValueError, match="finite"):
to_mx_value(float("-inf"))
def test_to_mx_value_accepts_finite_float() -> None:
assert to_mx_value(3.5).double_value == 3.5
def test_to_mx_value_rejects_non_finite_float_in_sequence() -> None:
with pytest.raises(ValueError, match="finite"):
to_mx_value([1.0, math.inf])
# --- Client.Python-011: success == 0 proto3-default ambiguity --------------
def test_ensure_mxaccess_success_ignores_unpopulated_status_entry() -> None:
"""A status entry left at proto3 defaults is not a real MXAccess failure.
The gateway emits such a placeholder for a null MXSTATUS_PROXY COM entry
(``MxStatusProxyConverter.ConvertMany``): ``success`` stays 0 but the
entry carries no failure category. It must not raise ``MxAccessError``.
"""
reply = pb.MxCommandReply(
session_id="session-1",
kind=pb.MX_COMMAND_KIND_SUBSCRIBE_BULK,
protocol_status=pb.ProtocolStatus(code=pb.PROTOCOL_STATUS_CODE_OK),
statuses=[
pb.MxStatusProxy(), # all-default: success == 0, category UNSPECIFIED
pb.MxStatusProxy( # the gateway's null-entry placeholder
category=pb.MX_STATUS_CATEGORY_UNKNOWN,
detected_by=pb.MX_STATUS_SOURCE_UNKNOWN,
),
],
)
assert ensure_mxaccess_success("subscribe bulk", reply) is reply
def test_ensure_mxaccess_success_raises_on_populated_failure_status() -> None:
"""A populated failure status (success == 0 with a failure category) raises."""
reply = pb.MxCommandReply(
session_id="session-1",
kind=pb.MX_COMMAND_KIND_WRITE,
protocol_status=pb.ProtocolStatus(code=pb.PROTOCOL_STATUS_CODE_OK),
statuses=[
pb.MxStatusProxy(
success=0,
category=pb.MX_STATUS_CATEGORY_COMMUNICATION_ERROR,
),
],
)
with pytest.raises(MxAccessError):
ensure_mxaccess_success("write", reply)
def test_ensure_mxaccess_success_passes_when_status_reports_success() -> None:
reply = pb.MxCommandReply(
session_id="session-1",
kind=pb.MX_COMMAND_KIND_WRITE,
protocol_status=pb.ProtocolStatus(code=pb.PROTOCOL_STATUS_CODE_OK),
statuses=[
pb.MxStatusProxy(success=1, category=pb.MX_STATUS_CATEGORY_OK),
],
)
assert ensure_mxaccess_success("write", reply) is reply
+19 -19
View File
@@ -7,7 +7,7 @@
| Review date | 2026-05-18 | | Review date | 2026-05-18 |
| Commit reviewed | `3cc53a8` | | Commit reviewed | `3cc53a8` |
| Status | Reviewed | | Status | Reviewed |
| Open findings | 9 | | Open findings | 0 |
## Checklist coverage ## Checklist coverage
@@ -33,13 +33,13 @@
| Severity | Low | | Severity | Low |
| Category | Documentation & comments | | Category | Documentation & comments |
| Location | `clients/python/pyproject.toml:8,25`, `clients/python/src/mxgateway_cli/commands.py:25` | | Location | `clients/python/pyproject.toml:8,25`, `clients/python/src/mxgateway_cli/commands.py:25` |
| Status | Open | | Status | Resolved |
**Description:** The package `description` in `pyproject.toml` still says "Async Python client *scaffold*" even though the client is fully implemented. Stale "scaffold" wording misrepresents maturity to anyone reading PyPI metadata. (The `mxgw-py` console-script name is itself consistent between `pyproject.toml` and the README.) **Description:** The package `description` in `pyproject.toml` still says "Async Python client *scaffold*" even though the client is fully implemented. Stale "scaffold" wording misrepresents maturity to anyone reading PyPI metadata. (The `mxgw-py` console-script name is itself consistent between `pyproject.toml` and the README.)
**Recommendation:** Update the `pyproject.toml` description to drop "scaffold"; keep README CLI examples in sync with the actual `mxgw-py` entry point. **Recommendation:** Update the `pyproject.toml` description to drop "scaffold"; keep README CLI examples in sync with the actual `mxgw-py` entry point.
**Resolution:** _(open)_ **Resolution:** 2026-05-18 — Confirmed: `pyproject.toml:8` `description` read "Async Python client scaffold for MXAccess Gateway." Changed to "Async Python client for MXAccess Gateway." The `mxgw-py` console-script name was already consistent with the README, so no README change was needed. Pure metadata fix — no test required.
### Client.Python-002 ### Client.Python-002
@@ -48,13 +48,13 @@
| Severity | Low | | Severity | Low |
| Category | Code organization & conventions | | Category | Code organization & conventions |
| Location | `clients/python/src/mxgateway/__init__.py:27` | | Location | `clients/python/src/mxgateway/__init__.py:27` |
| Status | Open | | Status | Resolved |
**Description:** `MxGatewayCommandError` is imported into `__init__.py` and is a documented public exception, but it is missing from `__all__`. It is the parent of `MxAccessError` and a meaningful catch target, so omitting it from the public surface is inconsistent — `from mxgateway import *` will not expose it and tooling that respects `__all__` treats it as private. **Description:** `MxGatewayCommandError` is imported into `__init__.py` and is a documented public exception, but it is missing from `__all__`. It is the parent of `MxAccessError` and a meaningful catch target, so omitting it from the public surface is inconsistent — `from mxgateway import *` will not expose it and tooling that respects `__all__` treats it as private.
**Recommendation:** Add `"MxGatewayCommandError"` to the `__all__` list. **Recommendation:** Add `"MxGatewayCommandError"` to the `__all__` list.
**Resolution:** _(open)_ **Resolution:** 2026-05-18 — Re-triaged: this finding is stale against the reviewed source. `clients/python/src/mxgateway/__init__.py` already imports `MxGatewayCommandError` (line 16) **and** lists `"MxGatewayCommandError"` in `__all__` (line 38). `from mxgateway import *` exposes it correctly. Verified at runtime (`'MxGatewayCommandError' in mxgateway.__all__` is `True`). No source change required — the defect described no longer exists.
### Client.Python-003 ### Client.Python-003
@@ -78,13 +78,13 @@
| Severity | Low | | Severity | Low |
| Category | Correctness & logic bugs | | Category | Correctness & logic bugs |
| Location | `clients/python/src/mxgateway_cli/commands.py:386,402-404` | | Location | `clients/python/src/mxgateway_cli/commands.py:386,402-404` |
| Status | Open | | Status | Resolved |
**Description:** In `_smoke`, the local variable `closed` is set to `False` and never reassigned; the `finally` block's `if not closed:` is therefore always true. This is dead/misleading code suggesting a removed early-close path. **Description:** In `_smoke`, the local variable `closed` is set to `False` and never reassigned; the `finally` block's `if not closed:` is therefore always true. This is dead/misleading code suggesting a removed early-close path.
**Recommendation:** Remove the `closed` variable and the `if not closed:` guard; call `await session.close()` directly in the `finally` block (or use `async with session:`). **Recommendation:** Remove the `closed` variable and the `if not closed:` guard; call `await session.close()` directly in the `finally` block (or use `async with session:`).
**Resolution:** _(open)_ **Resolution:** 2026-05-18 — Confirmed: `closed = False` was set and never reassigned, making `if not closed:` dead code. Replaced the `try/finally` with `async with session:` so the session is closed via the documented async context manager — `Session` already implements `__aexit__``close()`. Behaviour is unchanged (the session is still closed on every exit path); no test needed for the dead-code removal — exercised by the existing CLI smoke test.
### Client.Python-005 ### Client.Python-005
@@ -108,13 +108,13 @@
| Severity | Low | | Severity | Low |
| Category | Concurrency & thread safety | | Category | Concurrency & thread safety |
| Location | `clients/python/src/mxgateway/client.py:74-82`, `clients/python/src/mxgateway/galaxy.py:85-93`, `clients/python/src/mxgateway/session.py:38-55` | | Location | `clients/python/src/mxgateway/client.py:74-82`, `clients/python/src/mxgateway/galaxy.py:85-93`, `clients/python/src/mxgateway/session.py:38-55` |
| Status | Open | | Status | Resolved |
**Description:** `close()` on the clients and `Session.close()` use a plain `self._closed` check-then-set with an `await` between, with no lock. If two coroutines call `close()` concurrently both can pass the guard before either sets it, causing a double `channel.close()` / double `CloseSession` RPC. Single-task usage is the documented contract, so impact is low, but the idempotency guarantee asserted in docstrings only holds for sequential calls. **Description:** `close()` on the clients and `Session.close()` use a plain `self._closed` check-then-set with an `await` between, with no lock. If two coroutines call `close()` concurrently both can pass the guard before either sets it, causing a double `channel.close()` / double `CloseSession` RPC. Single-task usage is the documented contract, so impact is low, but the idempotency guarantee asserted in docstrings only holds for sequential calls.
**Recommendation:** Set `self._closed = True` before the `await`, or guard with an `asyncio.Lock`, so the idempotency claim holds under concurrent close. **Recommendation:** Set `self._closed = True` before the `await`, or guard with an `asyncio.Lock`, so the idempotency claim holds under concurrent close.
**Resolution:** _(open)_ **Resolution:** 2026-05-18 — Confirmed the check-then-set window. Fixed `GatewayClient.close`, `GalaxyRepositoryClient.close`, and `Session.close` to set `self._closed = True` *before* the `await` (channel close / `CloseSession` RPC). A second coroutine entering `close()` while the first is still awaiting now hits the early-return guard and does not issue a second `channel.close()` / `CloseSession`. Docstrings updated to state the idempotency holds under concurrent calls. TDD: regression tests in `tests/test_low_severity_findings.py` (`test_gateway_client_concurrent_close_closes_channel_once`, `test_galaxy_client_concurrent_close_closes_channel_once`, `test_session_concurrent_close_sends_one_close_session_rpc`) — each uses a fake channel/client that stalls inside `close`/`close_session_raw` so two concurrent `close()` calls interleave at the exact race window; they failed before the fix and pass after.
### Client.Python-007 ### Client.Python-007
@@ -123,13 +123,13 @@
| Severity | Low | | Severity | Low |
| Category | Error handling & resilience | | Category | Error handling & resilience |
| Location | `clients/python/src/mxgateway/client.py:204-213` | | Location | `clients/python/src/mxgateway/client.py:204-213` |
| Status | Open | | Status | Resolved |
**Description:** `_canceling_iterator` (gateway event stream) does not catch `asyncio.CancelledError` to invoke `call.cancel()` explicitly — it relies on the `finally` block. `galaxy.py:_canceling_iterator` *does* explicitly catch `CancelledError`, cancel, and re-raise. The two are functionally equivalent today, but the inconsistency between near-identical helpers invites future divergence. **Description:** `_canceling_iterator` (gateway event stream) does not catch `asyncio.CancelledError` to invoke `call.cancel()` explicitly — it relies on the `finally` block. `galaxy.py:_canceling_iterator` *does* explicitly catch `CancelledError`, cancel, and re-raise. The two are functionally equivalent today, but the inconsistency between near-identical helpers invites future divergence.
**Recommendation:** Make the two `_canceling_iterator` helpers identical, ideally by factoring a single shared helper. **Recommendation:** Make the two `_canceling_iterator` helpers identical, ideally by factoring a single shared helper.
**Resolution:** _(open)_ **Resolution:** 2026-05-18 — Confirmed the divergence. Factored a single shared helper: `client._canceling_iterator(call, operation)` now takes the `map_rpc_error` operation string as a parameter, explicitly catches `asyncio.CancelledError` (cancels the call, re-raises) and `grpc.RpcError`, and repeats the cancel in `finally`. This replaces both the gateway `_canceling_iterator` and the gateway `_canceling_active_alarms_iterator`; `galaxy.py` now imports and delegates to the same helper instead of defining its own, so the gateway and Galaxy stream helpers are byte-for-byte identical. TDD: `tests/test_low_severity_findings.py::test_gateway_stream_iterator_cancels_call_on_task_cancellation` drives a cancellable fake stream and asserts the gateway iterator cancels the underlying call on task cancellation. All existing stream-cancellation tests still pass.
### Client.Python-008 ### Client.Python-008
@@ -138,13 +138,13 @@
| Severity | Low | | Severity | Low |
| Category | Correctness & logic bugs | | Category | Correctness & logic bugs |
| Location | `clients/python/src/mxgateway/values.py:62-67,83-88` | | Location | `clients/python/src/mxgateway/values.py:62-67,83-88` |
| Status | Open | | Status | Resolved |
**Description:** `to_mx_value` maps any Python `float` to `VT_R8`/`MX_DATA_TYPE_DOUBLE` with no handling for `nan`/`inf`, which are serialised and forwarded to MXAccess which may reject or mis-handle them. `bytes` is mapped to `VT_RECORD`/`MX_DATA_TYPE_UNKNOWN`, a questionable default. The `data_type` keyword exists but `Session.write` never forwards it. **Description:** `to_mx_value` maps any Python `float` to `VT_R8`/`MX_DATA_TYPE_DOUBLE` with no handling for `nan`/`inf`, which are serialised and forwarded to MXAccess which may reject or mis-handle them. `bytes` is mapped to `VT_RECORD`/`MX_DATA_TYPE_UNKNOWN`, a questionable default. The `data_type` keyword exists but `Session.write` never forwards it.
**Recommendation:** Document the float/bytes mapping assumptions, optionally validate finiteness, and consider plumbing the `data_type` keyword through `Session.write`/`write2`. **Recommendation:** Document the float/bytes mapping assumptions, optionally validate finiteness, and consider plumbing the `data_type` keyword through `Session.write`/`write2`.
**Resolution:** _(open)_ **Resolution:** 2026-05-18 — Confirmed the non-finite-float hazard. Added an `_ensure_finite` guard in `values.py`: `to_mx_value` now raises `ValueError` for `nan`/`inf`/`-inf`, both for a scalar `float` and for a non-finite element inside a float sequence — MXAccess has no defined wire representation for non-finite doubles, so rejecting client-side is the correct fail-fast. The `float`/`bytes` mapping assumptions (finite-only doubles; `bytes` as an opaque `VT_RECORD` pass-through) are now documented in the `values.py` module docstring and `clients/python/README.md`. Plumbing `data_type` through `Session.write`/`write2` was deliberately *not* done: it is a larger public-API surface change the finding only marks as "consider", and the documented MXAccess-parity convention is type-by-Python-value; the `data_type` keyword stays available on `to_mx_value` for callers that build the `MxValue` directly. TDD: `tests/test_low_severity_findings.py` adds `test_to_mx_value_rejects_nan`, `test_to_mx_value_rejects_positive_infinity`, `test_to_mx_value_rejects_negative_infinity`, `test_to_mx_value_rejects_non_finite_float_in_sequence`, and `test_to_mx_value_accepts_finite_float`. README updated since `to_mx_value` (used by `Session.write`/`write2`) now rejects an input it previously accepted.
### Client.Python-009 ### Client.Python-009
@@ -168,13 +168,13 @@
| Severity | Low | | Severity | Low |
| Category | Code organization & conventions | | Category | Code organization & conventions |
| Location | `clients/python/src/mxgateway/session.py:404`, `clients/python/src/mxgateway_cli/commands.py:422-425` | | Location | `clients/python/src/mxgateway/session.py:404`, `clients/python/src/mxgateway_cli/commands.py:422-425` |
| Status | Open | | Status | Resolved |
**Description:** `session.py` ends with a module-level late import `from .client import GatewayClient # noqa: E402` purely to satisfy a string type hint, and `commands.py:_session` does a function-local import. Both work around a circular dependency that `from __future__ import annotations` (already in effect) makes unnecessary. `_session` also lacks a return type annotation. **Description:** `session.py` ends with a module-level late import `from .client import GatewayClient # noqa: E402` purely to satisfy a string type hint, and `commands.py:_session` does a function-local import. Both work around a circular dependency that `from __future__ import annotations` (already in effect) makes unnecessary. `_session` also lacks a return type annotation.
**Recommendation:** Drop the runtime late import in `session.py` and use a `TYPE_CHECKING`-guarded import for the hint; add the `-> Session` return annotation to `commands.py:_session`. **Recommendation:** Drop the runtime late import in `session.py` and use a `TYPE_CHECKING`-guarded import for the hint; add the `-> Session` return annotation to `commands.py:_session`.
**Resolution:** _(open)_ **Resolution:** 2026-05-18 — Confirmed: with `from __future__ import annotations` in effect all annotations are strings, so the runtime late import was unnecessary. Removed the trailing `from .client import GatewayClient # noqa: E402` in `session.py` and replaced it with a top-of-file `if TYPE_CHECKING:` import that satisfies the `GatewayClient` hint without a runtime dependency (no import cycle: `client.py` does not import `session` at module scope). In `commands.py`, hoisted the function-local `from mxgateway.session import Session` to a module-level import and added the `-> Session` return annotation to `_session`. Verified `import mxgateway` and `import mxgateway_cli.commands` succeed with no circular-import error. Pure refactor — covered by the existing import and CLI tests; no new test needed.
### Client.Python-011 ### Client.Python-011
@@ -183,13 +183,13 @@
| Severity | Low | | Severity | Low |
| Category | Error handling & resilience | | Category | Error handling & resilience |
| Location | `clients/python/src/mxgateway/errors.py:122-148` | | Location | `clients/python/src/mxgateway/errors.py:122-148` |
| Status | Open | | Status | Resolved |
**Description:** `ensure_mxaccess_success` raises `MxAccessError` if any `mx_status.success == 0`. This treats `success == 0` as the failure sentinel, but `0` is also the proto3 scalar default for an unset `MxStatusProxy`. If the gateway ever returns a reply with an unpopulated status entry (e.g. a partially-filled bulk result), the client raises `MxAccessError` even though no real failure occurred. **Description:** `ensure_mxaccess_success` raises `MxAccessError` if any `mx_status.success == 0`. This treats `success == 0` as the failure sentinel, but `0` is also the proto3 scalar default for an unset `MxStatusProxy`. If the gateway ever returns a reply with an unpopulated status entry (e.g. a partially-filled bulk result), the client raises `MxAccessError` even though no real failure occurred.
**Recommendation:** Confirm against the proto/gateway contract whether `success` is guaranteed populated for every `statuses` entry; if not, key the failure decision on an explicit failure field rather than the `success == 0` default. **Recommendation:** Confirm against the proto/gateway contract whether `success` is guaranteed populated for every `statuses` entry; if not, key the failure decision on an explicit failure field rather than the `success == 0` default.
**Resolution:** _(open)_ **Resolution:** 2026-05-18 — Confirmed against the gateway contract: `success` is **not** guaranteed populated for every `statuses` entry. `src/MxGateway.Worker/Conversion/MxStatusProxyConverter.cs::ConvertMany` emits a placeholder `MxStatusProxy` for a null `MXSTATUS_PROXY` COM array entry, setting `Category`/`DetectedBy` to `Unknown` but **leaving `Success` at its proto3 default of 0**. A fully-default proto entry likewise has `success == 0`. Under the old client logic either placeholder would falsely raise `MxAccessError`. Fixed `ensure_mxaccess_success` to key the per-status failure decision on a new `_is_mxaccess_status_failure` helper that requires `success == 0` **and** a populated, non-OK `category` — a status with `category` of `MX_STATUS_CATEGORY_UNSPECIFIED` (default proto) or `MX_STATUS_CATEGORY_UNKNOWN` (the null-entry placeholder) is treated as unpopulated and ignored. `MX_STATUS_CATEGORY_OK` is also excluded so a genuine success entry never raises. Real failures (categories `WARNING` and the error categories, raw value ≥ 2) still raise as before — the existing `write.mxaccess-failure` fixture (`SECURITY_ERROR`/`OPERATIONAL_ERROR` statuses) and the `MXACCESS_FAILURE` protocol-status path are unaffected. TDD: `tests/test_low_severity_findings.py` adds `test_ensure_mxaccess_success_ignores_unpopulated_status_entry` (default + null-placeholder entries, no raise), `test_ensure_mxaccess_success_raises_on_populated_failure_status` (populated `COMMUNICATION_ERROR`, raises), and `test_ensure_mxaccess_success_passes_when_status_reports_success`. No public-behaviour change for genuine replies, so no README update.
### Client.Python-012 ### Client.Python-012
@@ -198,10 +198,10 @@
| Severity | Low | | Severity | Low |
| Category | mxaccessgw conventions | | Category | mxaccessgw conventions |
| Location | `clients/python/src/mxgateway/client.py:84-108`, `clients/python/src/mxgateway/session.py:57-77` | | Location | `clients/python/src/mxgateway/client.py:84-108`, `clients/python/src/mxgateway/session.py:57-77` |
| Status | Open | | Status | Won't Fix |
**Description:** `Session.invoke_raw` does not run `ensure_mxaccess_success` while `Session.invoke` does, so a caller using `invoke_raw` for parity tests gets a reply where an MXAccess HRESULT failure is silently embedded with no exception. This is by design but under-documented — the README's "preserve raw replies" sentence does not state that `*_raw` methods skip MXAccess-failure detection entirely. **Description:** `Session.invoke_raw` does not run `ensure_mxaccess_success` while `Session.invoke` does, so a caller using `invoke_raw` for parity tests gets a reply where an MXAccess HRESULT failure is silently embedded with no exception. This is by design but under-documented — the README's "preserve raw replies" sentence does not state that `*_raw` methods skip MXAccess-failure detection entirely.
**Recommendation:** Document explicitly (README + docstring) that `*_raw` methods surface MXAccess HRESULT/status failures only inside the reply and do not raise `MxAccessError`, so parity-test callers know to inspect `protocol_status`/`hresult`/`statuses` themselves. **Recommendation:** Document explicitly (README + docstring) that `*_raw` methods surface MXAccess HRESULT/status failures only inside the reply and do not raise `MxAccessError`, so parity-test callers know to inspect `protocol_status`/`hresult`/`statuses` themselves.
**Resolution:** _(open)_ **Resolution:** 2026-05-18 — Won't Fix (no behaviour change). Confirmed this is intentional, correct parity behaviour: the `*_raw` methods exist precisely so parity-test callers can inspect an unmodified gateway reply, including embedded MXAccess HRESULT/status failures, without an exception masking them. Changing `invoke_raw` to raise `MxAccessError` would defeat its purpose and duplicate `Session.invoke`. The finding's only actionable point is the documentation gap, which has been addressed: `clients/python/README.md` now states explicitly that `*_raw` methods enforce gateway protocol success only and do **not** run MXAccess-failure detection, and the docstrings of `GatewayClient.invoke_raw` and `Session.invoke_raw` say the same and point callers to inspect `protocol_status`/`hresult`/`statuses` (and to `Session.invoke` for the checked variant). No code/test change — the runtime contract is unchanged and correct.
+16 -16
View File
@@ -7,7 +7,7 @@
| Review date | 2026-05-18 | | Review date | 2026-05-18 |
| Commit reviewed | `6c64030` | | Commit reviewed | `6c64030` |
| Status | Reviewed | | Status | Reviewed |
| Open findings | 7 | | Open findings | 0 |
## Checklist coverage ## Checklist coverage
@@ -20,7 +20,7 @@
| 5 | Security | Credential-sensitive fields are clearly commented; no secrets forced into loggable shapes. No issues found. | | 5 | Security | Credential-sensitive fields are clearly commented; no secrets forced into loggable shapes. No issues found. |
| 6 | Performance & resource management | `DiscoverHierarchy` is paged; alarm-snapshot streams are server-streamed; no bloat issues. No issues found. | | 6 | Performance & resource management | `DiscoverHierarchy` is paged; alarm-snapshot streams are server-streamed; no bloat issues. No issues found. |
| 7 | Design-document adherence | `.proto` files match design intent but `docs/Grpc.md` is stale (Contracts-001); worker vs public alarm-status shapes unreconciled in docs (Contracts-008). | | 7 | Design-document adherence | `.proto` files match design intent but `docs/Grpc.md` is stale (Contracts-001); worker vs public alarm-status shapes unreconciled in docs (Contracts-008). |
| 8 | Code organization & conventions | Package/file layout correct; `mxaccess_worker.proto` Protobuf item missing `ProtoRoot` (Contracts-003); stale class summary (Contracts-004). | | 8 | Code organization & conventions | Package/file layout correct; stale class summary (Contracts-004). Contracts-003 (`mxaccess_worker.proto` Protobuf item missing `ProtoRoot`) was re-triaged as not-a-defect — the attribute is already present. |
| 9 | Testing coverage | Gateway/worker/alarm round-trips covered; Galaxy Repository protos and raw `MxArray` paths untested (Contracts-007). | | 9 | Testing coverage | Gateway/worker/alarm round-trips covered; Galaxy Repository protos and raw `MxArray` paths untested (Contracts-007). |
| 10 | Documentation & comments | Proto comments accurate and domain-rich; one stale class summary (Contracts-004). | | 10 | Documentation & comments | Proto comments accurate and domain-rich; one stale class summary (Contracts-004). |
@@ -33,13 +33,13 @@
| Severity | Low | | Severity | Low |
| Category | Design-document adherence | | Category | Design-document adherence |
| Location | `docs/Grpc.md:13` (and `:3`, `:32`, `:39`) | | Location | `docs/Grpc.md:13` (and `:3`, `:32`, `:39`) |
| Status | Open | | Status | Resolved |
**Description:** `mxaccess_gateway.proto` now declares six RPCs on `MxAccessGateway` (`OpenSession`, `CloseSession`, `Invoke`, `StreamEvents`, `AcknowledgeAlarm`, `QueryActiveAlarms`). `docs/Grpc.md` still describes "the four `MxAccessGateway` RPCs" in its type table and omits `AcknowledgeAlarm`/`QueryActiveAlarms` from the Validation Rules table. CLAUDE.md requires docs to change in the same commit as the contract; the alarm RPC commits left this doc stale and misleading about the public surface. **Description:** `mxaccess_gateway.proto` now declares six RPCs on `MxAccessGateway` (`OpenSession`, `CloseSession`, `Invoke`, `StreamEvents`, `AcknowledgeAlarm`, `QueryActiveAlarms`). `docs/Grpc.md` still describes "the four `MxAccessGateway` RPCs" in its type table and omits `AcknowledgeAlarm`/`QueryActiveAlarms` from the Validation Rules table. CLAUDE.md requires docs to change in the same commit as the contract; the alarm RPC commits left this doc stale and misleading about the public surface.
**Recommendation:** Update `docs/Grpc.md` to enumerate all six RPCs and add `AcknowledgeAlarm`/`QueryActiveAlarms` to the type/handler and validation tables, or explicitly cross-reference `AlarmClientDiscovery.md`. **Recommendation:** Update `docs/Grpc.md` to enumerate all six RPCs and add `AcknowledgeAlarm`/`QueryActiveAlarms` to the type/handler and validation tables, or explicitly cross-reference `AlarmClientDiscovery.md`.
**Resolution:** _(open)_ **Resolution:** _(2026-05-18)_ Confirmed against `mxaccess_gateway.proto` — six RPCs declared, doc said "four". Updated `docs/Grpc.md`: the collaborator table now says "six `MxAccessGateway` RPCs", the RPC Handlers intro enumerates all six, added dedicated `AcknowledgeAlarm` and `QueryActiveAlarms` handler subsections (noting the alarm surface routes through `IAlarmRpcDispatcher` and is validated inline rather than via `MxAccessGrpcRequestValidator`, with a cross-reference to `AlarmClientDiscovery.md`), and added both alarm RPCs to the Validation Rules table.
### Contracts-002 ### Contracts-002
@@ -63,13 +63,13 @@
| Severity | Low | | Severity | Low |
| Category | Code organization & conventions | | Category | Code organization & conventions |
| Location | `src/MxGateway.Contracts/MxGateway.Contracts.csproj:10` | | Location | `src/MxGateway.Contracts/MxGateway.Contracts.csproj:10` |
| Status | Open | | Status | Won't Fix |
**Description:** The `<Protobuf>` item for `mxaccess_worker.proto` omits `ProtoRoot="Protos"`, while the items for `mxaccess_gateway.proto` (line 9) and `galaxy_repository.proto` (line 11) both set it. `mxaccess_worker.proto` does `import "mxaccess_gateway.proto"`, which resolves only because Grpc.Tools adds the importing file's own directory to the proto path. The inconsistency is fragile — tooling changes to ProtoRoot handling could break import resolution. **Description:** The `<Protobuf>` item for `mxaccess_worker.proto` omits `ProtoRoot="Protos"`, while the items for `mxaccess_gateway.proto` (line 9) and `galaxy_repository.proto` (line 11) both set it. `mxaccess_worker.proto` does `import "mxaccess_gateway.proto"`, which resolves only because Grpc.Tools adds the importing file's own directory to the proto path. The inconsistency is fragile — tooling changes to ProtoRoot handling could break import resolution.
**Recommendation:** Add `ProtoRoot="Protos"` to the `mxaccess_worker.proto` `<Protobuf>` item so all three entries are consistent. **Recommendation:** Add `ProtoRoot="Protos"` to the `mxaccess_worker.proto` `<Protobuf>` item so all three entries are consistent.
**Resolution:** _(open)_ **Resolution:** _(2026-05-18)_ Re-triaged as not-a-defect: the finding's premise is factually wrong. Line 10 of `MxGateway.Contracts.csproj` already carries `ProtoRoot="Protos"` — all three `<Protobuf>` items are already consistent. `git show 6c64030:src/MxGateway.Contracts/MxGateway.Contracts.csproj` (the reviewed commit) confirms the attribute was present at review time too; the csproj has not been touched since `133c830`. No code change made. Status set to Won't Fix because there is nothing to fix.
### Contracts-004 ### Contracts-004
@@ -78,13 +78,13 @@
| Severity | Low | | Severity | Low |
| Category | Documentation & comments | | Category | Documentation & comments |
| Location | `src/MxGateway.Contracts/GatewayContractInfo.cs:3-6` | | Location | `src/MxGateway.Contracts/GatewayContractInfo.cs:3-6` |
| Status | Open | | Status | Resolved |
**Description:** The XML summary says the class exposes version metadata "before generated protobuf contracts are introduced." Generated protobuf contracts have long been introduced and are consumed across the solution. The comment is stale; the class now holds the authoritative `GatewayProtocolVersion`/`WorkerProtocolVersion` advertised in `OpenSessionReply` and used to validate `WorkerEnvelope` framing. **Description:** The XML summary says the class exposes version metadata "before generated protobuf contracts are introduced." Generated protobuf contracts have long been introduced and are consumed across the solution. The comment is stale; the class now holds the authoritative `GatewayProtocolVersion`/`WorkerProtocolVersion` advertised in `OpenSessionReply` and used to validate `WorkerEnvelope` framing.
**Recommendation:** Reword the summary to describe the current purpose — version constants advertised in `OpenSessionReply` and used to validate `WorkerEnvelope` protocol framing. **Recommendation:** Reword the summary to describe the current purpose — version constants advertised in `OpenSessionReply` and used to validate `WorkerEnvelope` protocol framing.
**Resolution:** _(open)_ **Resolution:** _(2026-05-18)_ Confirmed stale — the class is consumed by `GatewayApplication`/`OpenSessionReply` and `WorkerEnvelope` framing checks across the solution. Reworded the XML summary on `GatewayContractInfo` to describe the actual current purpose: `GatewayProtocolVersion` is advertised to clients in `OpenSessionReply`, and `WorkerProtocolVersion` validates `WorkerEnvelope` protocol framing on the gateway↔worker pipe.
### Contracts-005 ### Contracts-005
@@ -93,13 +93,13 @@
| Severity | Low | | Severity | Low |
| Category | mxaccessgw conventions | | Category | mxaccessgw conventions |
| Location | `src/MxGateway.Contracts/Protos/mxaccess_gateway.proto`, `src/MxGateway.Contracts/Protos/mxaccess_worker.proto` | | Location | `src/MxGateway.Contracts/Protos/mxaccess_gateway.proto`, `src/MxGateway.Contracts/Protos/mxaccess_worker.proto` |
| Status | Open | | Status | Resolved |
**Description:** The ProtobufStyleGuide mandates reserving removed field numbers / enum values. Evolution to date has been purely additive, so this is not a current violation — but none of the `.proto` files contain any `reserved` declarations, leaving no in-file guardrail for the first removal. This is a latent maintainability gap. **Description:** The ProtobufStyleGuide mandates reserving removed field numbers / enum values. Evolution to date has been purely additive, so this is not a current violation — but none of the `.proto` files contain any `reserved` declarations, leaving no in-file guardrail for the first removal. This is a latent maintainability gap.
**Recommendation:** When any field or enum value is eventually removed, add a `reserved` range/name in the same change. Consider a short comment block in each message documenting the policy so future editors apply `reserved` rather than reusing tags. **Recommendation:** When any field or enum value is eventually removed, add a `reserved` range/name in the same change. Consider a short comment block in each message documenting the policy so future editors apply `reserved` rather than reusing tags.
**Resolution:** _(open)_ **Resolution:** _(2026-05-18)_ Confirmed: no field or enum value has ever been removed, so adding `reserved` ranges now would be incorrect (there are no retired tags to reserve, and inventing ranges for never-used numbers would itself violate the contract). Took the finding's least-invasive option — added a short wire-compatibility policy comment block at the top of all three `.proto` files (`mxaccess_gateway.proto`, `mxaccess_worker.proto`, `galaxy_repository.proto`) stating the additive-only rule and instructing future editors to add a `reserved` range + name in the same change as any removal. Comment-only, no wire-format or generated-type change. The `reserved` declarations themselves remain correctly deferred to the first actual removal.
### Contracts-006 ### Contracts-006
@@ -108,13 +108,13 @@
| Severity | Low | | Severity | Low |
| Category | Correctness & logic bugs | | Category | Correctness & logic bugs |
| Location | `src/MxGateway.Contracts/Protos/mxaccess_gateway.proto:647` | | Location | `src/MxGateway.Contracts/Protos/mxaccess_gateway.proto:647` |
| Status | Open | | Status | Resolved |
**Description:** `MxStatusProxy.success` is declared `int32 success = 1` with no comment. The name reads like a boolean flag but the type is a 32-bit integer (mirroring MXAccess `MXSTATUS_PROXY`, which stores a numeric success/HResult-like value). Without a comment a client author can reasonably misinterpret the field (treat non-1 as failure, or expect only 0/1). **Description:** `MxStatusProxy.success` is declared `int32 success = 1` with no comment. The name reads like a boolean flag but the type is a 32-bit integer (mirroring MXAccess `MXSTATUS_PROXY`, which stores a numeric success/HResult-like value). Without a comment a client author can reasonably misinterpret the field (treat non-1 as failure, or expect only 0/1).
**Recommendation:** Add a comment clarifying the semantic — what range of values it carries and how 0 vs non-zero map to MXAccess status — per the style guide rule to comment fields carrying raw MXAccess status detail. **Recommendation:** Add a comment clarifying the semantic — what range of values it carries and how 0 vs non-zero map to MXAccess status — per the style guide rule to comment fields carrying raw MXAccess status detail.
**Resolution:** _(open)_ **Resolution:** _(2026-05-18)_ Confirmed: `int32 success = 1` had no comment. Cross-checked against the worker `MxStatusProxyConverter`, which reads the COM struct's `success` field verbatim (a 16-bit signed value) without reinterpretation, and against the MXAccess analysis (`MXAccess-Public-API.md`: `MxStatus`/`MXSTATUS_PROXY` are identical structs with a `short success` member). Added a field comment to `MxStatusProxy.success` stating it mirrors the COM struct's numeric `success` member (NOT a boolean), is carried verbatim for diagnostics, and that clients should branch on `category` (`MX_STATUS_CATEGORY_OK` marks success) — deliberately avoiding an over-specified 0-vs-1 claim, since the gateway never maps `success` to an outcome and `category` is the authoritative field. Comment-only change.
### Contracts-007 ### Contracts-007
@@ -123,13 +123,13 @@
| Severity | Low | | Severity | Low |
| Category | Testing coverage | | Category | Testing coverage |
| Location | `src/MxGateway.Tests/Contracts/ProtobufContractRoundTripTests.cs` | | Location | `src/MxGateway.Tests/Contracts/ProtobufContractRoundTripTests.cs` |
| Status | Open | | Status | Resolved |
**Description:** `ProtobufContractRoundTripTests` covers gateway command/reply/event, alarm transition, alarm ack request/reply, active-alarm snapshot, and the worker envelope. It has no coverage for: (a) any `galaxy_repository.proto` message (`DiscoverHierarchy*`, `GalaxyObject`, `GalaxyAttribute`, `DeployEvent`, the `root` oneof, wrapper-typed fields); (b) `BulkSubscribeReply`/`SubscribeResult` and the bulk command kinds; (c) `MxValue`/`MxArray` `raw_value`/`RawArray` (`bytes`) paths and the `WorkerFault`/`WorkerHeartbeat` IPC bodies. **Description:** `ProtobufContractRoundTripTests` covers gateway command/reply/event, alarm transition, alarm ack request/reply, active-alarm snapshot, and the worker envelope. It has no coverage for: (a) any `galaxy_repository.proto` message (`DiscoverHierarchy*`, `GalaxyObject`, `GalaxyAttribute`, `DeployEvent`, the `root` oneof, wrapper-typed fields); (b) `BulkSubscribeReply`/`SubscribeResult` and the bulk command kinds; (c) `MxValue`/`MxArray` `raw_value`/`RawArray` (`bytes`) paths and the `WorkerFault`/`WorkerHeartbeat` IPC bodies.
**Recommendation:** Add round-trip tests for the Galaxy Repository messages (including the `root` oneof and proto wrapper fields), the bulk-subscribe reply, and the remaining `WorkerEnvelope` body cases. **Recommendation:** Add round-trip tests for the Galaxy Repository messages (including the `root` oneof and proto wrapper fields), the bulk-subscribe reply, and the remaining `WorkerEnvelope` body cases.
**Resolution:** _(open)_ **Resolution:** _(2026-05-18)_ Confirmed the listed gaps and added round-trip tests to `ProtobufContractRoundTripTests` covering all three areas: (a) Galaxy Repository — `GalaxyRepositoryDescriptor_ContainsBrowseServiceMethods`, `DiscoverHierarchyRequest_RoundTripsRootOneofAndWrapperFields` (a `[Theory]` exercising all three `root` oneof arms plus the `Int32Value` wrapper `max_depth`), `DiscoverHierarchyReply_RoundTripsObjectAndAttributeGraph`, `DeployEvent_RoundTripsTimestampAndCounters`, `GalaxyConnectionReplies_RoundTrip`; (b) `BulkSubscribeReply_RoundTripsSubscribeResults` and `MxCommandReply_RoundTripsBulkSubscribePayload` (bulk-subscribe command kind + payload case); (c) `MxValue_RoundTripsRawValueBytesPayload`, `MxArray_RoundTripsRawArrayPayload`, `WorkerEnvelope_RoundTripsWorkerFaultBody`, `WorkerEnvelope_RoundTripsWorkerHeartbeatBody`. All new tests pass; the full `ProtobufContractRoundTripTests` class is 27 tests green.
### Contracts-008 ### Contracts-008
@@ -138,10 +138,10 @@
| Severity | Low | | Severity | Low |
| Category | Design-document adherence | | Category | Design-document adherence |
| Location | `src/MxGateway.Contracts/Protos/mxaccess_gateway.proto:451-459`, `:627-636` | | Location | `src/MxGateway.Contracts/Protos/mxaccess_gateway.proto:451-459`, `:627-636` |
| Status | Open | | Status | Resolved |
**Description:** The worker-side `AcknowledgeAlarmReplyPayload` carries the alarm-ack outcome as `int32 native_status`, while the public `AcknowledgeAlarmReply` carries it as `MxStatusProxy status` plus `optional int32 hresult`. The comment explains the worker echoes `native_status` into `AcknowledgeAlarmReply.hresult`, but the two outcome shapes (raw `int32` vs structured `MxStatusProxy`) are not reconciled in `docs/Contracts.md` / `AlarmClientDiscovery.md`. A reader cannot tell whether `MxStatusProxy status` is always populated or only on COM-layer failure. **Description:** The worker-side `AcknowledgeAlarmReplyPayload` carries the alarm-ack outcome as `int32 native_status`, while the public `AcknowledgeAlarmReply` carries it as `MxStatusProxy status` plus `optional int32 hresult`. The comment explains the worker echoes `native_status` into `AcknowledgeAlarmReply.hresult`, but the two outcome shapes (raw `int32` vs structured `MxStatusProxy`) are not reconciled in `docs/Contracts.md` / `AlarmClientDiscovery.md`. A reader cannot tell whether `MxStatusProxy status` is always populated or only on COM-layer failure.
**Recommendation:** Document in `docs/Contracts.md` (or `AlarmClientDiscovery.md`) how the worker `native_status` maps onto the public reply's `status`/`hresult` pair so client authors know which field is authoritative. **Recommendation:** Document in `docs/Contracts.md` (or `AlarmClientDiscovery.md`) how the worker `native_status` maps onto the public reply's `status`/`hresult` pair so client authors know which field is authoritative.
**Resolution:** _(open)_ **Resolution:** _(2026-05-18)_ Verified against `WorkerAlarmRpcDispatcher.AcknowledgeAsync`. The asymmetry is larger than the finding implies: the dispatcher copies the worker `MxCommandReply.hresult` into `AcknowledgeAlarmReply.hresult` but **never** assigns `AcknowledgeAlarmReply.status` — the `MxStatusProxy status` field is left UNSET on every reply. The proto comment on `status` ("Native MxAccess status describing the outcome of the ack") was therefore actively misleading. Fixed: (1) reworded the `mxaccess_gateway.proto` comments on `AcknowledgeAlarmReply.hresult` (now identifies it as the authoritative native-return-code field) and `AcknowledgeAlarmReply.status` (now states it is reserved/unset and clients must not depend on it); (2) extended `docs/AlarmClientDiscovery.md` section 4 with a "Worker `native_status` → public `AcknowledgeAlarmReply` mapping" subsection spelling out that `hresult` is authoritative (`0` = success) and `status` is always unset, and that clients should branch on `protocol_status` then `hresult`, never `status`.
+12 -10
View File
@@ -7,13 +7,13 @@
| Review date | 2026-05-18 | | Review date | 2026-05-18 |
| Commit reviewed | `6c64030` | | Commit reviewed | `6c64030` |
| Status | Reviewed | | Status | Reviewed |
| Open findings | 4 | | Open findings | 0 |
## Checklist coverage ## Checklist coverage
| # | Category | Result | | # | Category | Result |
|---|---|---| |---|---|---|
| 1 | Correctness & logic bugs | Issues found: IntegrationTests-003 (asserts only on first event), IntegrationTests-010 (`WaitForFirstMessageAsync` ignores cancellation). | | 1 | Correctness & logic bugs | Issues found: IntegrationTests-003 (asserts only on first event), IntegrationTests-010 (`WaitForMessageAsync` ignores cancellation). |
| 2 | mxaccessgw conventions | Live tests correctly gated and skip (not fail) when prerequisites are absent; `LiveGalaxyRepositoryFactAttribute` undocumented in the opt-in matrix. | | 2 | mxaccessgw conventions | Live tests correctly gated and skip (not fail) when prerequisites are absent; `LiveGalaxyRepositoryFactAttribute` undocumented in the opt-in matrix. |
| 3 | Concurrency & thread safety | Issue found: IntegrationTests-007 (no `[Collection]`/parallelism guard for shared MXAccess/ZB/GLAuth). | | 3 | Concurrency & thread safety | Issue found: IntegrationTests-007 (no `[Collection]`/parallelism guard for shared MXAccess/ZB/GLAuth). |
| 4 | Error handling & resilience | Issue found: IntegrationTests-004 (cleanup `WaitAsync` can mask the original failure). | | 4 | Error handling & resilience | Issue found: IntegrationTests-004 (cleanup `WaitAsync` can mask the original failure). |
@@ -123,13 +123,13 @@
| Severity | Low | | Severity | Low |
| Category | Concurrency & thread safety | | Category | Concurrency & thread safety |
| Location | `src/MxGateway.IntegrationTests/WorkerLiveMxAccessSmokeTests.cs:20`, `src/MxGateway.IntegrationTests/Galaxy/GalaxyRepositoryLiveTests.cs:5`, `src/MxGateway.IntegrationTests/DashboardLdapLiveTests.cs:9` | | Location | `src/MxGateway.IntegrationTests/WorkerLiveMxAccessSmokeTests.cs:20`, `src/MxGateway.IntegrationTests/Galaxy/GalaxyRepositoryLiveTests.cs:5`, `src/MxGateway.IntegrationTests/DashboardLdapLiveTests.cs:9` |
| Status | Open | | Status | Resolved |
**Description:** The live test classes contend for genuinely shared singletons — one MXAccess COM provider, one ZB SQL database, one GLAuth instance with a 3-fail/10-minute per-IP lockout. No `[Collection]` annotation or `DisableTestParallelization` is declared, so xUnit's default cross-class parallelism could run the Galaxy tests concurrently or interleave an LDAP failure burst that trips the GLAuth lockout. **Description:** The live test classes contend for genuinely shared singletons — one MXAccess COM provider, one ZB SQL database, one GLAuth instance with a 3-fail/10-minute per-IP lockout. No `[Collection]` annotation or `DisableTestParallelization` is declared, so xUnit's default cross-class parallelism could run the Galaxy tests concurrently or interleave an LDAP failure burst that trips the GLAuth lockout.
**Recommendation:** Place the live test classes in a shared `[Collection]`, or set `[assembly: CollectionBehavior(DisableTestParallelization = true)]` for this opt-in project, so live external resources are accessed serially. **Recommendation:** Place the live test classes in a shared `[Collection]`, or set `[assembly: CollectionBehavior(DisableTestParallelization = true)]` for this opt-in project, so live external resources are accessed serially.
**Resolution:** _(open)_ **Resolution:** Resolved 2026-05-18: Confirmed — no `[Collection]` or assembly-level `CollectionBehavior` existed. Added `LiveResourcesCollection.cs` with a `[CollectionDefinition(Name, DisableParallelization = true)]` and applied `[Collection(LiveResourcesCollection.Name)]` to `WorkerLiveMxAccessSmokeTests`, `GalaxyRepositoryLiveTests`, and `DashboardLdapLiveTests`. A named collection (rather than an assembly-wide `DisableTestParallelization`) was chosen so the live classes serialize against each other and within themselves while non-live tests (`IntegrationTestEnvironmentTests`) keep parallelizing. Verified by build; live tests not executed (no MXAccess COM / live LDAP in this environment).
### IntegrationTests-008 ### IntegrationTests-008
@@ -138,13 +138,13 @@
| Severity | Low | | Severity | Low |
| Category | Code organization & conventions | | Category | Code organization & conventions |
| Location | `src/MxGateway.IntegrationTests/LiveLdapFactAttribute.cs`, `src/MxGateway.IntegrationTests/Galaxy/LiveGalaxyRepositoryFactAttribute.cs`, `src/MxGateway.IntegrationTests/LiveMxAccessFactAttribute.cs` | | Location | `src/MxGateway.IntegrationTests/LiveLdapFactAttribute.cs`, `src/MxGateway.IntegrationTests/Galaxy/LiveGalaxyRepositoryFactAttribute.cs`, `src/MxGateway.IntegrationTests/LiveMxAccessFactAttribute.cs` |
| Status | Open | | Status | Resolved |
**Description:** Three near-identical fact attributes each re-implement the same "compare env var to `1` with `StringComparison.Ordinal`, set `Skip` otherwise" pattern. `LiveMxAccessFactAttribute` delegates to `IntegrationTestEnvironment` while the other two inline the logic, so the project has two divergent styles for the same concern. **Description:** Three near-identical fact attributes each re-implement the same "compare env var to `1` with `StringComparison.Ordinal`, set `Skip` otherwise" pattern. `LiveMxAccessFactAttribute` delegates to `IntegrationTestEnvironment` while the other two inline the logic, so the project has two divergent styles for the same concern.
**Recommendation:** Extract a shared helper (e.g. `IntegrationTestEnvironment.IsEnabled(string variableName)`) and have all three attributes call it. **Recommendation:** Extract a shared helper (e.g. `IntegrationTestEnvironment.IsEnabled(string variableName)`) and have all three attributes call it.
**Resolution:** _(open)_ **Resolution:** Resolved 2026-05-18: Confirmed — `LiveLdapFactAttribute.Enabled` and `LiveGalaxyRepositoryFactAttribute.Enabled` each inlined the ordinal `== "1"` comparison while `LiveMxAccessFactAttribute` delegated to `IntegrationTestEnvironment`. Added `IntegrationTestEnvironment.IsEnabled(string variableName)` as the single implementation; `LiveMxAccessTestsEnabled`, `LiveLdapFactAttribute.Enabled`, and `LiveGalaxyRepositoryFactAttribute.Enabled` now all call it. Verified by build.
### IntegrationTests-009 ### IntegrationTests-009
@@ -153,13 +153,13 @@
| Severity | Low | | Severity | Low |
| Category | Documentation & comments | | Category | Documentation & comments |
| Location | `src/MxGateway.IntegrationTests/WorkerLiveMxAccessSmokeTests.cs:372-375` | | Location | `src/MxGateway.IntegrationTests/WorkerLiveMxAccessSmokeTests.cs:372-375` |
| Status | Open | | Status | Resolved |
**Description:** `TestServerCallContext` is XML-documented as a "Mock server call context," but it is a hand-written stub/fake with no mocking framework and no verification behavior. Per the style guides (accurate naming; explain why not what), calling it a mock misleads readers who may expect verifiable interactions. **Description:** `TestServerCallContext` is XML-documented as a "Mock server call context," but it is a hand-written stub/fake with no mocking framework and no verification behavior. Per the style guides (accurate naming; explain why not what), calling it a mock misleads readers who may expect verifiable interactions.
**Recommendation:** Reword the summary to "test stub" / "minimal `ServerCallContext` implementation for in-process gRPC calls." **Recommendation:** Reword the summary to "test stub" / "minimal `ServerCallContext` implementation for in-process gRPC calls."
**Resolution:** _(open)_ **Resolution:** Resolved 2026-05-18: Confirmed — the summary read "Mock server call context for testing gRPC calls." Reworded to "Minimal `ServerCallContext` stub for invoking the gRPC service in-process," noting it is a hand-written fake with no verification behavior. No mocking framework is involved; this is a documentation-only fix. Verified by build.
### IntegrationTests-010 ### IntegrationTests-010
@@ -168,10 +168,12 @@
| Severity | Low | | Severity | Low |
| Category | Correctness & logic bugs | | Category | Correctness & logic bugs |
| Location | `src/MxGateway.IntegrationTests/WorkerLiveMxAccessSmokeTests.cs:366-369` | | Location | `src/MxGateway.IntegrationTests/WorkerLiveMxAccessSmokeTests.cs:366-369` |
| Status | Open | | Status | Resolved |
**Description:** `WaitForFirstMessageAsync` accepts only a `timeout` and never observes a `CancellationToken`. There is no per-test cancellation propagation, so if the gateway/worker hangs without writing an event the test relies solely on the 15s `WaitAsync` timeout and gives no contextual diagnostics. Combined with IntegrationTests-004, a hung live worker produces a bare `TimeoutException`. **Description:** `WaitForFirstMessageAsync` accepts only a `timeout` and never observes a `CancellationToken`. There is no per-test cancellation propagation, so if the gateway/worker hangs without writing an event the test relies solely on the 15s `WaitAsync` timeout and gives no contextual diagnostics. Combined with IntegrationTests-004, a hung live worker produces a bare `TimeoutException`.
**Recommendation:** Accept a `CancellationToken` (linked to `TestServerCallContext`'s token), pass it to `firstMessage.Task.WaitAsync(timeout, token)`, and on timeout emit the recorded `Messages` count via `output.WriteLine` before throwing. **Recommendation:** Accept a `CancellationToken` (linked to `TestServerCallContext`'s token), pass it to `firstMessage.Task.WaitAsync(timeout, token)`, and on timeout emit the recorded `Messages` count via `output.WriteLine` before throwing.
**Resolution:** _(open)_ **Re-triage:** The named method `WaitForFirstMessageAsync` no longer exists — IntegrationTests-003's resolution renamed/replaced it with `RecordingServerStreamWriter.WaitForMessageAsync(predicate, timeout)`, which scans recorded messages and blocks on a `SemaphoreSlim`. The underlying defect still held: that replacement method also took only a `timeout` and never observed a `CancellationToken`. The finding remains valid (Low, Correctness) against the renamed method; the recommendation's `firstMessage.Task.WaitAsync` detail is stale but the intent (thread a token, surface a count on timeout) is unchanged.
**Resolution:** Resolved 2026-05-18: Added an optional `CancellationToken` parameter to `WaitForMessageAsync`, linked with the existing timeout source via `CancellationTokenSource.CreateLinkedTokenSource`, so a per-test cancellation aborts the wait promptly. `GatewaySession_WithLiveWorker_RegistersAdvisesStreamsDataAndCloses` now creates a `CancellationTokenSource`, passes its token into the `StreamEvents` `TestServerCallContext` and into `WaitForMessageAsync`, so the stream call and the wait share one cancellation source. On timeout the method already throws a `TimeoutException` whose message includes the scanned message count, satisfying the "emit recorded count" intent (the count surfaces in the test failure rather than via a separate `output.WriteLine`). Verified by build; live tests not executed.
+40 -41
View File
@@ -13,55 +13,20 @@ Each module's `findings.md` is the source of truth; this file is generated from
| [Client.Dotnet](Client.Dotnet/findings.md) | Claude Code | 2026-05-18 | `3cc53a8` | Reviewed | 0 | 8 | | [Client.Dotnet](Client.Dotnet/findings.md) | Claude Code | 2026-05-18 | `3cc53a8` | Reviewed | 0 | 8 |
| [Client.Go](Client.Go/findings.md) | Claude Code | 2026-05-18 | `3cc53a8` | Reviewed | 0 | 10 | | [Client.Go](Client.Go/findings.md) | Claude Code | 2026-05-18 | `3cc53a8` | Reviewed | 0 | 10 |
| [Client.Java](Client.Java/findings.md) | Claude Code | 2026-05-18 | `3cc53a8` | Reviewed | 0 | 12 | | [Client.Java](Client.Java/findings.md) | Claude Code | 2026-05-18 | `3cc53a8` | Reviewed | 0 | 12 |
| [Client.Python](Client.Python/findings.md) | Claude Code | 2026-05-18 | `3cc53a8` | Reviewed | 9 | 12 | | [Client.Python](Client.Python/findings.md) | Claude Code | 2026-05-18 | `3cc53a8` | Reviewed | 0 | 12 |
| [Client.Rust](Client.Rust/findings.md) | Claude Code | 2026-05-18 | `3cc53a8` | Reviewed | 0 | 12 | | [Client.Rust](Client.Rust/findings.md) | Claude Code | 2026-05-18 | `3cc53a8` | Reviewed | 0 | 12 |
| [Contracts](Contracts/findings.md) | Claude Code | 2026-05-18 | `6c64030` | Reviewed | 7 | 8 | | [Contracts](Contracts/findings.md) | Claude Code | 2026-05-18 | `6c64030` | Reviewed | 0 | 8 |
| [IntegrationTests](IntegrationTests/findings.md) | Claude Code | 2026-05-18 | `6c64030` | Reviewed | 4 | 10 | | [IntegrationTests](IntegrationTests/findings.md) | Claude Code | 2026-05-18 | `6c64030` | Reviewed | 0 | 10 |
| [Server](Server/findings.md) | Claude Code | 2026-05-18 | `6c64030` | Reviewed | 0 | 14 | | [Server](Server/findings.md) | Claude Code | 2026-05-18 | `6c64030` | Reviewed | 0 | 14 |
| [Tests](Tests/findings.md) | Claude Code | 2026-05-18 | `6c64030` | Reviewed | 6 | 12 | | [Tests](Tests/findings.md) | Claude Code | 2026-05-18 | `6c64030` | Reviewed | 0 | 12 |
| [Worker](Worker/findings.md) | Claude Code | 2026-05-18 | `6c64030` | Reviewed | 0 | 15 | | [Worker](Worker/findings.md) | Claude Code | 2026-05-18 | `6c64030` | Reviewed | 0 | 15 |
| [Worker.Tests](Worker.Tests/findings.md) | Claude Code | 2026-05-18 | `6c64030` | Reviewed | 8 | 15 | | [Worker.Tests](Worker.Tests/findings.md) | Claude Code | 2026-05-18 | `6c64030` | Reviewed | 0 | 15 |
## Pending findings ## Pending findings
Findings with status `Open` or `In Progress`, ordered by severity. Findings with status `Open` or `In Progress`, ordered by severity.
| ID | Severity | Category | Location | Description | _No pending findings._
|---|---|---|---|---|
| Client.Python-001 | Low | Documentation & comments | `clients/python/pyproject.toml:8,25`, `clients/python/src/mxgateway_cli/commands.py:25` | The package `description` in `pyproject.toml` still says "Async Python client *scaffold*" even though the client is fully implemented. Stale "scaffold" wording misrepresents maturity to anyone reading PyPI metadata. (The `mxgw-py` console-… |
| Client.Python-002 | Low | Code organization & conventions | `clients/python/src/mxgateway/__init__.py:27` | `MxGatewayCommandError` is imported into `__init__.py` and is a documented public exception, but it is missing from `__all__`. It is the parent of `MxAccessError` and a meaningful catch target, so omitting it from the public surface is inc… |
| Client.Python-004 | Low | Correctness & logic bugs | `clients/python/src/mxgateway_cli/commands.py:386,402-404` | In `_smoke`, the local variable `closed` is set to `False` and never reassigned; the `finally` block's `if not closed:` is therefore always true. This is dead/misleading code suggesting a removed early-close path. |
| Client.Python-006 | Low | Concurrency & thread safety | `clients/python/src/mxgateway/client.py:74-82`, `clients/python/src/mxgateway/galaxy.py:85-93`, `clients/python/src/mxgateway/session.py:38-55` | `close()` on the clients and `Session.close()` use a plain `self._closed` check-then-set with an `await` between, with no lock. If two coroutines call `close()` concurrently both can pass the guard before either sets it, causing a double `… |
| Client.Python-007 | Low | Error handling & resilience | `clients/python/src/mxgateway/client.py:204-213` | `_canceling_iterator` (gateway event stream) does not catch `asyncio.CancelledError` to invoke `call.cancel()` explicitly — it relies on the `finally` block. `galaxy.py:_canceling_iterator` *does* explicitly catch `CancelledError`, cancel,… |
| Client.Python-008 | Low | Correctness & logic bugs | `clients/python/src/mxgateway/values.py:62-67,83-88` | `to_mx_value` maps any Python `float` to `VT_R8`/`MX_DATA_TYPE_DOUBLE` with no handling for `nan`/`inf`, which are serialised and forwarded to MXAccess which may reject or mis-handle them. `bytes` is mapped to `VT_RECORD`/`MX_DATA_TYPE_UNK… |
| Client.Python-010 | Low | Code organization & conventions | `clients/python/src/mxgateway/session.py:404`, `clients/python/src/mxgateway_cli/commands.py:422-425` | `session.py` ends with a module-level late import `from .client import GatewayClient # noqa: E402` purely to satisfy a string type hint, and `commands.py:_session` does a function-local import. Both work around a circular dependency that `… |
| Client.Python-011 | Low | Error handling & resilience | `clients/python/src/mxgateway/errors.py:122-148` | `ensure_mxaccess_success` raises `MxAccessError` if any `mx_status.success == 0`. This treats `success == 0` as the failure sentinel, but `0` is also the proto3 scalar default for an unset `MxStatusProxy`. If the gateway ever returns a rep… |
| Client.Python-012 | Low | mxaccessgw conventions | `clients/python/src/mxgateway/client.py:84-108`, `clients/python/src/mxgateway/session.py:57-77` | `Session.invoke_raw` does not run `ensure_mxaccess_success` while `Session.invoke` does, so a caller using `invoke_raw` for parity tests gets a reply where an MXAccess HRESULT failure is silently embedded with no exception. This is by desi… |
| Contracts-001 | Low | Design-document adherence | `docs/Grpc.md:13` (and `:3`, `:32`, `:39`) | `mxaccess_gateway.proto` now declares six RPCs on `MxAccessGateway` (`OpenSession`, `CloseSession`, `Invoke`, `StreamEvents`, `AcknowledgeAlarm`, `QueryActiveAlarms`). `docs/Grpc.md` still describes "the four `MxAccessGateway` RPCs" in its… |
| Contracts-003 | Low | Code organization & conventions | `src/MxGateway.Contracts/MxGateway.Contracts.csproj:10` | The `<Protobuf>` item for `mxaccess_worker.proto` omits `ProtoRoot="Protos"`, while the items for `mxaccess_gateway.proto` (line 9) and `galaxy_repository.proto` (line 11) both set it. `mxaccess_worker.proto` does `import "mxaccess_gateway… |
| Contracts-004 | Low | Documentation & comments | `src/MxGateway.Contracts/GatewayContractInfo.cs:3-6` | The XML summary says the class exposes version metadata "before generated protobuf contracts are introduced." Generated protobuf contracts have long been introduced and are consumed across the solution. The comment is stale; the class now… |
| Contracts-005 | Low | mxaccessgw conventions | `src/MxGateway.Contracts/Protos/mxaccess_gateway.proto`, `src/MxGateway.Contracts/Protos/mxaccess_worker.proto` | The ProtobufStyleGuide mandates reserving removed field numbers / enum values. Evolution to date has been purely additive, so this is not a current violation — but none of the `.proto` files contain any `reserved` declarations, leaving no… |
| Contracts-006 | Low | Correctness & logic bugs | `src/MxGateway.Contracts/Protos/mxaccess_gateway.proto:647` | `MxStatusProxy.success` is declared `int32 success = 1` with no comment. The name reads like a boolean flag but the type is a 32-bit integer (mirroring MXAccess `MXSTATUS_PROXY`, which stores a numeric success/HResult-like value). Without… |
| Contracts-007 | Low | Testing coverage | `src/MxGateway.Tests/Contracts/ProtobufContractRoundTripTests.cs` | `ProtobufContractRoundTripTests` covers gateway command/reply/event, alarm transition, alarm ack request/reply, active-alarm snapshot, and the worker envelope. It has no coverage for: (a) any `galaxy_repository.proto` message (`DiscoverHie… |
| Contracts-008 | Low | Design-document adherence | `src/MxGateway.Contracts/Protos/mxaccess_gateway.proto:451-459`, `:627-636` | The worker-side `AcknowledgeAlarmReplyPayload` carries the alarm-ack outcome as `int32 native_status`, while the public `AcknowledgeAlarmReply` carries it as `MxStatusProxy status` plus `optional int32 hresult`. The comment explains the wo… |
| IntegrationTests-007 | Low | Concurrency & thread safety | `src/MxGateway.IntegrationTests/WorkerLiveMxAccessSmokeTests.cs:20`, `src/MxGateway.IntegrationTests/Galaxy/GalaxyRepositoryLiveTests.cs:5`, `src/MxGateway.IntegrationTests/DashboardLdapLiveTests.cs:9` | The live test classes contend for genuinely shared singletons — one MXAccess COM provider, one ZB SQL database, one GLAuth instance with a 3-fail/10-minute per-IP lockout. No `[Collection]` annotation or `DisableTestParallelization` is dec… |
| IntegrationTests-008 | Low | Code organization & conventions | `src/MxGateway.IntegrationTests/LiveLdapFactAttribute.cs`, `src/MxGateway.IntegrationTests/Galaxy/LiveGalaxyRepositoryFactAttribute.cs`, `src/MxGateway.IntegrationTests/LiveMxAccessFactAttribute.cs` | Three near-identical fact attributes each re-implement the same "compare env var to `1` with `StringComparison.Ordinal`, set `Skip` otherwise" pattern. `LiveMxAccessFactAttribute` delegates to `IntegrationTestEnvironment` while the other t… |
| IntegrationTests-009 | Low | Documentation & comments | `src/MxGateway.IntegrationTests/WorkerLiveMxAccessSmokeTests.cs:372-375` | `TestServerCallContext` is XML-documented as a "Mock server call context," but it is a hand-written stub/fake with no mocking framework and no verification behavior. Per the style guides (accurate naming; explain why not what), calling it… |
| IntegrationTests-010 | Low | Correctness & logic bugs | `src/MxGateway.IntegrationTests/WorkerLiveMxAccessSmokeTests.cs:366-369` | `WaitForFirstMessageAsync` accepts only a `timeout` and never observes a `CancellationToken`. There is no per-test cancellation propagation, so if the gateway/worker hangs without writing an event the test relies solely on the 15s `WaitAsy… |
| Tests-007 | Low | Code organization & conventions | `src/MxGateway.Tests/Gateway/Grpc/MxAccessGatewayServiceTests.cs:682`, `src/MxGateway.Tests/Gateway/Grpc/GalaxyRepositoryGrpcServiceTests.cs:324`, `src/MxGateway.Tests/Gateway/GatewayEndToEndFakeWorkerSmokeTests.cs:460`, `src/MxGateway.Tests/Security/Authorization/GatewayGrpcAuthorizationInterceptorTests.cs:233` | A near-identical `TestServerCallContext` implementation is copy-pasted into at least four test files (and `AllowAllConstraintEnforcer` / `TestServerStreamWriter` / `RecordingStreamWriter` into several). Duplication risks the copies driftin… |
| Tests-008 | Low | mxaccessgw conventions | `src/MxGateway.Tests/Gateway/Sessions/WorkerAlarmRpcDispatcherTests.cs:1-9`, `src/MxGateway.Tests/Gateway/Sessions/NotWiredAlarmRpcDispatcherTests.cs:1-3`, `src/MxGateway.Tests/Gateway/Sessions/SessionManagerAlarmAutoSubscribeTests.cs:1` | The alarm test files diverge from the project's C# style and the rest of the suite: snake_case test method names instead of the PascalCase `Method_Condition_Result` pattern; redundant explicit `using System;`/`System.Threading;` imports de… |
| Tests-009 | Low | Documentation & comments | `src/MxGateway.Tests/Gateway/Sessions/SessionManagerTests.cs:36-37,99,365` | Several XML `<summary>` comments are copy-paste mismatches: the comment above `OpenSessionAsync_SetsInitialDefaultLease` describes correlation-ID generation; the comment above `GatewaySessionSubscribeBulkAsync_ForwardsOneBulkCommand…` desc… |
| Tests-010 | Low | Security | `src/MxGateway.Tests/Gateway/Dashboard/DashboardAuthorizationHandlerTests.cs:26-36` | The anonymous-localhost bypass is tested only for the success case (`allowAnonymousLocalhost: true` + loopback succeeds) and the remote-unauthenticated denial. There is no test for the security-critical negatives: anonymous + loopback when… |
| Tests-011 | Low | Correctness & logic bugs | `src/MxGateway.Tests/Gateway/GatewayEndToEndFakeWorkerSmokeTests.cs:233-301` | `GatewayEndToEndFakeWorkerSmokeTests` correctly stores and awaits `launcher.WorkerTask`, but `SessionWorkerClientFactoryFakeWorkerTests` uses `_ = RunWorkerAsync(...)` with no stored task (lines 152, 184, 220). An unhandled exception in th… |
| Tests-012 | Low | Concurrency & thread safety | `src/MxGateway.Tests/Gateway/Workers/Fakes/FakeWorkerHarness.cs:62`, `src/MxGateway.Tests/Gateway/Workers/WorkerClientTests.cs:472` | Pipe names are uniquified per test with a GUID (good), but xUnit runs test classes in parallel by default and there is no `xunit.runner.json` or collection configuration. Tests that build a full `WebApplication` bind ephemeral ports (`--ur… |
| Worker.Tests-008 | Low | Documentation & comments | `src/MxGateway.Worker.Tests/Conversion/VariantConverterTests.cs:175-182` | `Redactor_WithCredentialBearingValueFields_RedactsBeforeLogging` lives in `VariantConverterTests` but asserts on `WorkerLogRedactor.RedactValue`, which has nothing to do with `VariantConverter`. It is also a near-duplicate of coverage in `… |
| Worker.Tests-009 | Low | Code organization & conventions | `src/MxGateway.Worker.Tests/MxAccess/AlarmCommandHandlerTests.cs`, `AlarmDispatcherTests.cs`, `AlarmCommandExecutorTests.cs`, `AlarmRecordTransitionMapperTests.cs`, `WnWrapAlarmConsumerXmlTests.cs` | The alarm-related test files use `snake_case` method names while the rest of the project uses the `Method_State_Result` PascalCase convention. `docs/style-guides/CSharpStyleGuide.md` and the surrounding code establish PascalCase as the pro… |
| Worker.Tests-010 | Low | Correctness & logic bugs | `src/MxGateway.Worker.Tests/MxAccess/MxAccessStaSessionTests.cs:230-258` | `StartAsync_WithoutAlarmCommandHandlerFactory_SubscribeAlarmsReturnsInvalidRequest` asserts `Assert.Contains("alarm", reply.DiagnosticMessage, StringComparison.OrdinalIgnoreCase)`. The XML doc claims it verifies the diagnostic says "alarm… |
| Worker.Tests-011 | Low | Documentation & comments | `src/MxGateway.Worker.Tests/Sta/StaCommandDispatcherTests.cs:92-112` | `DispatchAsync_WhenCanceledAfterExecutionStarts_StillReturnsLateReply` is named and documented as if it proves cancellation arrived after execution began. The test does `Started.Wait(...)` then `cancellation.Cancel()`, which proves executi… |
| Worker.Tests-012 | Low | Testing coverage | `src/MxGateway.Worker.Tests/Ipc/WorkerFrameProtocolTests.cs` | `docs/WorkerFrameProtocol.md` states the reader "rejects zero-length payloads and payloads larger than the configured maximum (default 16 MiB) before allocating the payload buffer." `WorkerFrameProtocolTests` covers malformed-length, wrong… |
| Worker.Tests-013 | Low | Concurrency & thread safety | `src/MxGateway.Worker.Tests/Ipc/WorkerPipeSessionTests.cs:539-546` | `ThrowIfCompletedAsync` does an unconditional `await Task.Delay(TimeSpan.FromMilliseconds(100))` then checks `task.IsCompleted`. This adds a fixed 100 ms to the test and only catches a `RunAsync` that fails within that arbitrary window; a… |
| Worker.Tests-014 | Low | Code organization & conventions | `src/MxGateway.Worker.Tests/Ipc/WorkerPipeClientTests.cs:194`, `WorkerPipeSessionTests.cs:622`, `Sta/StaCommandDispatcherTests.cs:348`, `MxAccess/MxAccessStaSessionTests.cs:334`, `MxAccess/MxAccessCommandExecutorTests.cs:1124` | `FakeRuntimeSession`, `NoopComApartmentInitializer`, `NoopEventSink`/`NullEventSink`, and the `CreateFrame`/`WriteUInt32LittleEndian` helpers are re-implemented independently in multiple test files. The two `FakeRuntimeSession` implementat… |
| Worker.Tests-015 | Low | Testing coverage | `src/MxGateway.Worker.Tests/MxAccess/MxAccessEventQueueTests.cs` | `MxAccessEventQueueTests` covers monotonic sequencing, drain, capacity overflow, and first-fault-wins, but does not cover `Drain` with `maxEvents: 0` (drain-all) — a branch `FakeRuntimeSession.DrainEvents` even special-cases — nor draining… |
## Closed findings ## Closed findings
@@ -142,12 +107,32 @@ Findings with status `Resolved`, `Won't Fix`, or `Deferred`.
| Client.Java-010 | Low | Resolved | Documentation & comments | `clients/java/mxgateway-client/src/main/java/com/dohertylan/mxgateway/client/MxGatewayClient.java:269-272`, `clients/java/README.md:76` | | Client.Java-010 | Low | Resolved | Documentation & comments | `clients/java/mxgateway-client/src/main/java/com/dohertylan/mxgateway/client/MxGatewayClient.java:269-272`, `clients/java/README.md:76` |
| Client.Java-011 | Low | Resolved | Performance & resource management | `clients/java/mxgateway-client/src/main/java/com/dohertylan/mxgateway/client/MxEventStream.java:37-63` | | Client.Java-011 | Low | Resolved | Performance & resource management | `clients/java/mxgateway-client/src/main/java/com/dohertylan/mxgateway/client/MxEventStream.java:37-63` |
| Client.Java-012 | Low | Resolved | Correctness & logic bugs | `clients/java/mxgateway-cli/src/main/java/com/dohertylan/mxgateway/cli/MxGatewayCli.java:667-674` | | Client.Java-012 | Low | Resolved | Correctness & logic bugs | `clients/java/mxgateway-cli/src/main/java/com/dohertylan/mxgateway/cli/MxGatewayCli.java:667-674` |
| Client.Python-001 | Low | Resolved | Documentation & comments | `clients/python/pyproject.toml:8,25`, `clients/python/src/mxgateway_cli/commands.py:25` |
| Client.Python-002 | Low | Resolved | Code organization & conventions | `clients/python/src/mxgateway/__init__.py:27` |
| Client.Python-004 | Low | Resolved | Correctness & logic bugs | `clients/python/src/mxgateway_cli/commands.py:386,402-404` |
| Client.Python-006 | Low | Resolved | Concurrency & thread safety | `clients/python/src/mxgateway/client.py:74-82`, `clients/python/src/mxgateway/galaxy.py:85-93`, `clients/python/src/mxgateway/session.py:38-55` |
| Client.Python-007 | Low | Resolved | Error handling & resilience | `clients/python/src/mxgateway/client.py:204-213` |
| Client.Python-008 | Low | Resolved | Correctness & logic bugs | `clients/python/src/mxgateway/values.py:62-67,83-88` |
| Client.Python-010 | Low | Resolved | Code organization & conventions | `clients/python/src/mxgateway/session.py:404`, `clients/python/src/mxgateway_cli/commands.py:422-425` |
| Client.Python-011 | Low | Resolved | Error handling & resilience | `clients/python/src/mxgateway/errors.py:122-148` |
| Client.Python-012 | Low | Won't Fix | mxaccessgw conventions | `clients/python/src/mxgateway/client.py:84-108`, `clients/python/src/mxgateway/session.py:57-77` |
| Client.Rust-004 | Low | Resolved | Documentation & comments | `clients/rust/src/version.rs:7` | | Client.Rust-004 | Low | Resolved | Documentation & comments | `clients/rust/src/version.rs:7` |
| Client.Rust-007 | Low | Resolved | Design-document adherence | `clients/rust/RustClientDesign.md:14-55` | | Client.Rust-007 | Low | Resolved | Design-document adherence | `clients/rust/RustClientDesign.md:14-55` |
| Client.Rust-008 | Low | Resolved | Performance & resource management | `clients/rust/src/value.rs:161-261` | | Client.Rust-008 | Low | Resolved | Performance & resource management | `clients/rust/src/value.rs:161-261` |
| Client.Rust-009 | Low | Resolved | Testing coverage | `clients/rust/tests/client_behavior.rs`, `clients/rust/src/galaxy.rs` | | Client.Rust-009 | Low | Resolved | Testing coverage | `clients/rust/tests/client_behavior.rs`, `clients/rust/src/galaxy.rs` |
| Client.Rust-010 | Low | Resolved | Error handling & resilience | `clients/rust/src/client.rs:255-268`, `clients/rust/src/galaxy.rs:204-216` | | Client.Rust-010 | Low | Resolved | Error handling & resilience | `clients/rust/src/client.rs:255-268`, `clients/rust/src/galaxy.rs:204-216` |
| Client.Rust-011 | Low | Resolved | mxaccessgw conventions | `clients/rust/src/session.rs:469` | | Client.Rust-011 | Low | Resolved | mxaccessgw conventions | `clients/rust/src/session.rs:469` |
| Contracts-001 | Low | Resolved | Design-document adherence | `docs/Grpc.md:13` (and `:3`, `:32`, `:39`) |
| Contracts-003 | Low | Won't Fix | Code organization & conventions | `src/MxGateway.Contracts/MxGateway.Contracts.csproj:10` |
| Contracts-004 | Low | Resolved | Documentation & comments | `src/MxGateway.Contracts/GatewayContractInfo.cs:3-6` |
| Contracts-005 | Low | Resolved | mxaccessgw conventions | `src/MxGateway.Contracts/Protos/mxaccess_gateway.proto`, `src/MxGateway.Contracts/Protos/mxaccess_worker.proto` |
| Contracts-006 | Low | Resolved | Correctness & logic bugs | `src/MxGateway.Contracts/Protos/mxaccess_gateway.proto:647` |
| Contracts-007 | Low | Resolved | Testing coverage | `src/MxGateway.Tests/Contracts/ProtobufContractRoundTripTests.cs` |
| Contracts-008 | Low | Resolved | Design-document adherence | `src/MxGateway.Contracts/Protos/mxaccess_gateway.proto:451-459`, `:627-636` |
| IntegrationTests-007 | Low | Resolved | Concurrency & thread safety | `src/MxGateway.IntegrationTests/WorkerLiveMxAccessSmokeTests.cs:20`, `src/MxGateway.IntegrationTests/Galaxy/GalaxyRepositoryLiveTests.cs:5`, `src/MxGateway.IntegrationTests/DashboardLdapLiveTests.cs:9` |
| IntegrationTests-008 | Low | Resolved | Code organization & conventions | `src/MxGateway.IntegrationTests/LiveLdapFactAttribute.cs`, `src/MxGateway.IntegrationTests/Galaxy/LiveGalaxyRepositoryFactAttribute.cs`, `src/MxGateway.IntegrationTests/LiveMxAccessFactAttribute.cs` |
| IntegrationTests-009 | Low | Resolved | Documentation & comments | `src/MxGateway.IntegrationTests/WorkerLiveMxAccessSmokeTests.cs:372-375` |
| IntegrationTests-010 | Low | Resolved | Correctness & logic bugs | `src/MxGateway.IntegrationTests/WorkerLiveMxAccessSmokeTests.cs:366-369` |
| Server-007 | Low | Resolved | Performance & resource management | `src/MxGateway.Server/Galaxy/GalaxyHierarchyProjector.cs:55-70` | | Server-007 | Low | Resolved | Performance & resource management | `src/MxGateway.Server/Galaxy/GalaxyHierarchyProjector.cs:55-70` |
| Server-008 | Low | Resolved | Performance & resource management | `src/MxGateway.Server/Grpc/GalaxyRepositoryGrpcService.cs:111-134,160-189` | | Server-008 | Low | Resolved | Performance & resource management | `src/MxGateway.Server/Grpc/GalaxyRepositoryGrpcService.cs:111-134,160-189` |
| Server-009 | Low | Resolved | Error handling & resilience | `src/MxGateway.Server/Security/Authentication/AuthSqliteConnectionFactory.cs:15-32` | | Server-009 | Low | Resolved | Error handling & resilience | `src/MxGateway.Server/Security/Authentication/AuthSqliteConnectionFactory.cs:15-32` |
@@ -156,6 +141,12 @@ Findings with status `Resolved`, `Won't Fix`, or `Deferred`.
| Server-012 | Low | Resolved | Documentation & comments | `CLAUDE.md` (Authentication section and `apikey create` example) | | Server-012 | Low | Resolved | Documentation & comments | `CLAUDE.md` (Authentication section and `apikey create` example) |
| Server-013 | Low | Resolved | Testing coverage | `src/MxGateway.Tests/Gateway/Dashboard/DashboardAuthorizationHandlerTests.cs`, `src/MxGateway.Tests/Gateway/GatewayApplicationTests.cs` | | Server-013 | Low | Resolved | Testing coverage | `src/MxGateway.Tests/Gateway/Dashboard/DashboardAuthorizationHandlerTests.cs`, `src/MxGateway.Tests/Gateway/GatewayApplicationTests.cs` |
| Server-014 | Low | Resolved | Documentation & comments | `src/MxGateway.Server/Grpc/MxAccessGatewayService.cs:162-171,191-198,206-214,229-237` | | Server-014 | Low | Resolved | Documentation & comments | `src/MxGateway.Server/Grpc/MxAccessGatewayService.cs:162-171,191-198,206-214,229-237` |
| Tests-007 | Low | Resolved | Code organization & conventions | `src/MxGateway.Tests/Gateway/Grpc/MxAccessGatewayServiceTests.cs:682`, `src/MxGateway.Tests/Gateway/Grpc/GalaxyRepositoryGrpcServiceTests.cs:324`, `src/MxGateway.Tests/Gateway/GatewayEndToEndFakeWorkerSmokeTests.cs:460`, `src/MxGateway.Tests/Security/Authorization/GatewayGrpcAuthorizationInterceptorTests.cs:233` |
| Tests-008 | Low | Resolved | mxaccessgw conventions | `src/MxGateway.Tests/Gateway/Sessions/WorkerAlarmRpcDispatcherTests.cs:1-9`, `src/MxGateway.Tests/Gateway/Sessions/NotWiredAlarmRpcDispatcherTests.cs:1-3`, `src/MxGateway.Tests/Gateway/Sessions/SessionManagerAlarmAutoSubscribeTests.cs:1` |
| Tests-009 | Low | Resolved | Documentation & comments | `src/MxGateway.Tests/Gateway/Sessions/SessionManagerTests.cs:36-37,99,365` |
| Tests-010 | Low | Resolved | Security | `src/MxGateway.Tests/Gateway/Dashboard/DashboardAuthorizationHandlerTests.cs:26-36` |
| Tests-011 | Low | Resolved | Correctness & logic bugs | `src/MxGateway.Tests/Gateway/GatewayEndToEndFakeWorkerSmokeTests.cs:233-301` |
| Tests-012 | Low | Resolved | Concurrency & thread safety | `src/MxGateway.Tests/Gateway/Workers/Fakes/FakeWorkerHarness.cs:62`, `src/MxGateway.Tests/Gateway/Workers/WorkerClientTests.cs:472` |
| Worker-009 | Low | Resolved | Performance & resource management | `src/MxGateway.Worker/Ipc/WorkerFrameReader.cs:31,49`, `src/MxGateway.Worker/Ipc/WorkerFrameWriter.cs:57-58` | | Worker-009 | Low | Resolved | Performance & resource management | `src/MxGateway.Worker/Ipc/WorkerFrameReader.cs:31,49`, `src/MxGateway.Worker/Ipc/WorkerFrameWriter.cs:57-58` |
| Worker-010 | Low | Resolved | Correctness & logic bugs | `src/MxGateway.Worker/Conversion/VariantConverter.cs:204-226` | | Worker-010 | Low | Resolved | Correctness & logic bugs | `src/MxGateway.Worker/Conversion/VariantConverter.cs:204-226` |
| Worker-011 | Low | Resolved | Correctness & logic bugs | `src/MxGateway.Worker/Ipc/WorkerPipeClient.cs:169-171` | | Worker-011 | Low | Resolved | Correctness & logic bugs | `src/MxGateway.Worker/Ipc/WorkerPipeClient.cs:169-171` |
@@ -163,3 +154,11 @@ Findings with status `Resolved`, `Won't Fix`, or `Deferred`.
| Worker-013 | Low | Resolved | Testing coverage | `src/MxGateway.Worker/Sta/StaMessagePump.cs` | | Worker-013 | Low | Resolved | Testing coverage | `src/MxGateway.Worker/Sta/StaMessagePump.cs` |
| Worker-014 | Low | Resolved | Code organization & conventions | `src/MxGateway.Worker/MxAccess/AlarmCommandHandler.cs:33`, `:202` | | Worker-014 | Low | Resolved | Code organization & conventions | `src/MxGateway.Worker/MxAccess/AlarmCommandHandler.cs:33`, `:202` |
| Worker-015 | Low | Resolved | Correctness & logic bugs | `src/MxGateway.Worker/MxAccess/MxAccessEventQueue.cs:115-145` | | Worker-015 | Low | Resolved | Correctness & logic bugs | `src/MxGateway.Worker/MxAccess/MxAccessEventQueue.cs:115-145` |
| Worker.Tests-008 | Low | Resolved | Documentation & comments | `src/MxGateway.Worker.Tests/Conversion/VariantConverterTests.cs:175-182` |
| Worker.Tests-009 | Low | Resolved | Code organization & conventions | `src/MxGateway.Worker.Tests/MxAccess/AlarmCommandHandlerTests.cs`, `AlarmDispatcherTests.cs`, `AlarmCommandExecutorTests.cs`, `AlarmRecordTransitionMapperTests.cs`, `WnWrapAlarmConsumerXmlTests.cs` |
| Worker.Tests-010 | Low | Resolved | Correctness & logic bugs | `src/MxGateway.Worker.Tests/MxAccess/MxAccessStaSessionTests.cs:230-258` |
| Worker.Tests-011 | Low | Resolved | Documentation & comments | `src/MxGateway.Worker.Tests/Sta/StaCommandDispatcherTests.cs:92-112` |
| Worker.Tests-012 | Low | Resolved | Testing coverage | `src/MxGateway.Worker.Tests/Ipc/WorkerFrameProtocolTests.cs` |
| Worker.Tests-013 | Low | Resolved | Concurrency & thread safety | `src/MxGateway.Worker.Tests/Ipc/WorkerPipeSessionTests.cs:539-546` |
| Worker.Tests-014 | Low | Resolved | Code organization & conventions | `src/MxGateway.Worker.Tests/Ipc/WorkerPipeClientTests.cs:194`, `WorkerPipeSessionTests.cs:622`, `Sta/StaCommandDispatcherTests.cs:348`, `MxAccess/MxAccessStaSessionTests.cs:334`, `MxAccess/MxAccessCommandExecutorTests.cs:1124` |
| Worker.Tests-015 | Low | Resolved | Testing coverage | `src/MxGateway.Worker.Tests/MxAccess/MxAccessEventQueueTests.cs` |
+15 -13
View File
@@ -7,7 +7,7 @@
| Review date | 2026-05-18 | | Review date | 2026-05-18 |
| Commit reviewed | `6c64030` | | Commit reviewed | `6c64030` |
| Status | Reviewed | | Status | Reviewed |
| Open findings | 6 | | Open findings | 0 |
## Checklist coverage ## Checklist coverage
@@ -127,13 +127,13 @@
| Severity | Low | | Severity | Low |
| Category | Code organization & conventions | | Category | Code organization & conventions |
| Location | `src/MxGateway.Tests/Gateway/Grpc/MxAccessGatewayServiceTests.cs:682`, `src/MxGateway.Tests/Gateway/Grpc/GalaxyRepositoryGrpcServiceTests.cs:324`, `src/MxGateway.Tests/Gateway/GatewayEndToEndFakeWorkerSmokeTests.cs:460`, `src/MxGateway.Tests/Security/Authorization/GatewayGrpcAuthorizationInterceptorTests.cs:233` | | Location | `src/MxGateway.Tests/Gateway/Grpc/MxAccessGatewayServiceTests.cs:682`, `src/MxGateway.Tests/Gateway/Grpc/GalaxyRepositoryGrpcServiceTests.cs:324`, `src/MxGateway.Tests/Gateway/GatewayEndToEndFakeWorkerSmokeTests.cs:460`, `src/MxGateway.Tests/Security/Authorization/GatewayGrpcAuthorizationInterceptorTests.cs:233` |
| Status | Open | | Status | Resolved |
**Description:** A near-identical `TestServerCallContext` implementation is copy-pasted into at least four test files (and `AllowAllConstraintEnforcer` / `TestServerStreamWriter` / `RecordingStreamWriter` into several). Duplication risks the copies drifting and bloats each file. **Description:** A near-identical `TestServerCallContext` implementation is copy-pasted into at least four test files (and `AllowAllConstraintEnforcer` / `TestServerStreamWriter` / `RecordingStreamWriter` into several). Duplication risks the copies drifting and bloats each file.
**Recommendation:** Extract a shared `TestServerCallContext`, `RecordingServerStreamWriter<T>`, and `AllowAllConstraintEnforcer` into a common test-support folder/namespace. **Recommendation:** Extract a shared `TestServerCallContext`, `RecordingServerStreamWriter<T>`, and `AllowAllConstraintEnforcer` into a common test-support folder/namespace.
**Resolution:** _(open)_ **Resolution:** Resolved 2026-05-18: confirmed five duplicated copies (the brief's four plus a fifth in `Galaxy/GalaxyFilterInputSafetyTests.cs`). Added a shared `MxGateway.Tests.TestSupport` namespace under `src/MxGateway.Tests/TestSupport/`: `TestServerCallContext.cs` (single class with an optional `Metadata? requestHeaders` constructor parameter that subsumes both the no-arg and headers-bearing variants), `RecordingServerStreamWriter.cs` (thread-safe writer with `Messages` and `WaitForFirstMessageAsync`, replacing `TestServerStreamWriter`/`RecordingStreamWriter`/`RecordingServerStreamWriter`), and `AllowAllConstraintEnforcer.cs`. Deleted all five `TestServerCallContext` copies, both `AllowAllConstraintEnforcer` copies, and the three stream-writer copies; updated the five test files to `using MxGateway.Tests.TestSupport;` and renamed `.Items` call sites to `.Messages`. Removed the now-unused `Grpc.Core` using from `GatewayEndToEndFakeWorkerSmokeTests.cs`. Build clean (0 warnings) and suite green.
### Tests-008 ### Tests-008
@@ -142,13 +142,15 @@
| Severity | Low | | Severity | Low |
| Category | mxaccessgw conventions | | Category | mxaccessgw conventions |
| Location | `src/MxGateway.Tests/Gateway/Sessions/WorkerAlarmRpcDispatcherTests.cs:1-9`, `src/MxGateway.Tests/Gateway/Sessions/NotWiredAlarmRpcDispatcherTests.cs:1-3`, `src/MxGateway.Tests/Gateway/Sessions/SessionManagerAlarmAutoSubscribeTests.cs:1` | | Location | `src/MxGateway.Tests/Gateway/Sessions/WorkerAlarmRpcDispatcherTests.cs:1-9`, `src/MxGateway.Tests/Gateway/Sessions/NotWiredAlarmRpcDispatcherTests.cs:1-3`, `src/MxGateway.Tests/Gateway/Sessions/SessionManagerAlarmAutoSubscribeTests.cs:1` |
| Status | Open | | Status | Resolved |
**Description:** The alarm test files diverge from the project's C# style and the rest of the suite: snake_case test method names instead of the PascalCase `Method_Condition_Result` pattern; redundant explicit `using System;`/`System.Threading;` imports despite implicit global usings; and explicit-type `new` instead of target-typed `new()` used elsewhere. There is also a typo in fixture data (`"wnwrap subscribe failed"`). **Description:** The alarm test files diverge from the project's C# style and the rest of the suite: snake_case test method names instead of the PascalCase `Method_Condition_Result` pattern; redundant explicit `using System;`/`System.Threading;` imports despite implicit global usings; and explicit-type `new` instead of target-typed `new()` used elsewhere. There is also a typo in fixture data (`"wnwrap subscribe failed"`).
**Recommendation:** Rename the alarm tests to the house `Method_Condition_Result` convention, drop redundant `System.*` usings, align `new` usage, and fix the `wnwrap` typo. **Recommendation:** Rename the alarm tests to the house `Method_Condition_Result` convention, drop redundant `System.*` usings, align `new` usage, and fix the `wnwrap` typo.
**Resolution:** _(open)_ **Re-triage note:** Two of the finding's claims are incorrect. (1) `"wnwrap subscribe failed"` is **not a typo**`WnWrap` is the real name of the worker's `WnWrapAlarmConsumer` MXAccess component (`src/MxGateway.Worker/MxAccess/WnWrapAlarmConsumer.cs`); the fixture string deliberately references it, so it was left unchanged. (2) `SessionManagerAlarmAutoSubscribeTests.cs` already uses PascalCase `Method_Condition_Result` names and target-typed `new()`, and its lone `using System.Runtime.CompilerServices;` is **required** for `[EnumeratorCancellation]` (not a global using) — it is not redundant. That file needed no change. The genuine style drift was confined to `WorkerAlarmRpcDispatcherTests.cs` and `NotWiredAlarmRpcDispatcherTests.cs`.
**Resolution:** Resolved 2026-05-18: renamed all ten `WorkerAlarmRpcDispatcherTests` methods and both `NotWiredAlarmRpcDispatcherTests` methods from snake_case to the house `Method_Condition_Result` PascalCase convention; dropped the redundant `System`/`System.Collections.Generic`/`System.Linq`/`System.Threading`/`System.Threading.Tasks` usings from `WorkerAlarmRpcDispatcherTests.cs` and `System.Threading`/`System.Threading.Tasks` from `NotWiredAlarmRpcDispatcherTests.cs` (all are implicit global usings), keeping the required `System.Runtime.CompilerServices`; converted explicit-type `new SessionRegistry()`/`new WorkerAlarmRpcDispatcher(...)`/`new FakeAlarmWorkerClient`/`new List<...>()`/`new GatewaySession(...)` to target-typed `new()`; and replaced the fully-qualified `System.StringComparison` with `StringComparison`. See the re-triage note for the two claims not actioned. Suite green.
### Tests-009 ### Tests-009
@@ -157,13 +159,13 @@
| Severity | Low | | Severity | Low |
| Category | Documentation & comments | | Category | Documentation & comments |
| Location | `src/MxGateway.Tests/Gateway/Sessions/SessionManagerTests.cs:36-37,99,365` | | Location | `src/MxGateway.Tests/Gateway/Sessions/SessionManagerTests.cs:36-37,99,365` |
| Status | Open | | Status | Resolved |
**Description:** Several XML `<summary>` comments are copy-paste mismatches: the comment above `OpenSessionAsync_SetsInitialDefaultLease` describes correlation-ID generation; the comment above `GatewaySessionSubscribeBulkAsync_ForwardsOneBulkCommand…` describes lease refresh; the comment above `CloseExpiredLeasesAsync_DoesNotCloseActiveEventSubscriber` describes shutdown closing all sessions. Misleading test docs hinder triage. **Description:** Several XML `<summary>` comments are copy-paste mismatches: the comment above `OpenSessionAsync_SetsInitialDefaultLease` describes correlation-ID generation; the comment above `GatewaySessionSubscribeBulkAsync_ForwardsOneBulkCommand…` describes lease refresh; the comment above `CloseExpiredLeasesAsync_DoesNotCloseActiveEventSubscriber` describes shutdown closing all sessions. Misleading test docs hinder triage.
**Recommendation:** Correct the `<summary>` text to match each test's actual behavior, or remove the redundant comments since the test names already describe the behavior. **Recommendation:** Correct the `<summary>` text to match each test's actual behavior, or remove the redundant comments since the test names already describe the behavior.
**Resolution:** _(open)_ **Resolution:** Resolved 2026-05-18: confirmed three copy-paste `<summary>` mismatches. The mislabelled comments were the summaries of the *following* tests left attached to the wrong method (the test below each then had no summary). Corrected all three: `OpenSessionAsync_SetsInitialDefaultLease` now describes setting the initial lease expiry; the comment above `InvokeAsync_WhenSessionReady_RefreshesLease` (the finding mis-cited the method name as `GatewaySessionSubscribeBulkAsync_…`) now describes lease refresh on invoke; and `CloseExpiredLeasesAsync_DoesNotCloseActiveEventSubscriber` now describes the expired-lease sweep leaving an active-event-subscriber session open. No behavior change.
### Tests-010 ### Tests-010
@@ -172,13 +174,13 @@
| Severity | Low | | Severity | Low |
| Category | Security | | Category | Security |
| Location | `src/MxGateway.Tests/Gateway/Dashboard/DashboardAuthorizationHandlerTests.cs:26-36` | | Location | `src/MxGateway.Tests/Gateway/Dashboard/DashboardAuthorizationHandlerTests.cs:26-36` |
| Status | Open | | Status | Resolved |
**Description:** The anonymous-localhost bypass is tested only for the success case (`allowAnonymousLocalhost: true` + loopback succeeds) and the remote-unauthenticated denial. There is no test for the security-critical negatives: anonymous + loopback when `AllowAnonymousLocalhost` is `false` must be denied, and anonymous + non-loopback when the flag is `true` must still be denied (the bypass is scoped strictly to loopback). Those are the misconfiguration cases that would expose the dashboard. **Description:** The anonymous-localhost bypass is tested only for the success case (`allowAnonymousLocalhost: true` + loopback succeeds) and the remote-unauthenticated denial. There is no test for the security-critical negatives: anonymous + loopback when `AllowAnonymousLocalhost` is `false` must be denied, and anonymous + non-loopback when the flag is `true` must still be denied (the bypass is scoped strictly to loopback). Those are the misconfiguration cases that would expose the dashboard.
**Recommendation:** Add tests: anonymous + loopback + `allowAnonymousLocalhost: false` → not succeeded; anonymous + non-loopback + `allowAnonymousLocalhost: true` → not succeeded. **Recommendation:** Add tests: anonymous + loopback + `allowAnonymousLocalhost: false` → not succeeded; anonymous + non-loopback + `allowAnonymousLocalhost: true` → not succeeded.
**Resolution:** _(open)_ **Resolution:** Resolved 2026-05-18: confirmed the coverage gap and confirmed `DashboardAuthorizationHandler` already gates the bypass correctly on `AllowAnonymousLocalhost && IsLoopbackRequest()` (no product bug). Added two `DashboardAuthorizationHandlerTests`: `HandleAsync_AnonymousLocalhostDisallowed_DoesNotSucceed` (anonymous + loopback + `allowAnonymousLocalhost: false` → not succeeded) and `HandleAsync_AnonymousLocalhostAllowedFromRemoteAddress_DoesNotSucceed` (anonymous + non-loopback + `allowAnonymousLocalhost: true` → not succeeded, proving the bypass stays scoped to loopback). Both pass.
### Tests-011 ### Tests-011
@@ -187,13 +189,13 @@
| Severity | Low | | Severity | Low |
| Category | Correctness & logic bugs | | Category | Correctness & logic bugs |
| Location | `src/MxGateway.Tests/Gateway/GatewayEndToEndFakeWorkerSmokeTests.cs:233-301` | | Location | `src/MxGateway.Tests/Gateway/GatewayEndToEndFakeWorkerSmokeTests.cs:233-301` |
| Status | Open | | Status | Resolved |
**Description:** `GatewayEndToEndFakeWorkerSmokeTests` correctly stores and awaits `launcher.WorkerTask`, but `SessionWorkerClientFactoryFakeWorkerTests` uses `_ = RunWorkerAsync(...)` with no stored task (lines 152, 184, 220). An unhandled exception in the scripted worker becomes an unobserved `TaskException` that can surface as a process-level failure in an unrelated later test rather than failing the owning test. **Description:** `GatewayEndToEndFakeWorkerSmokeTests` correctly stores and awaits `launcher.WorkerTask`, but `SessionWorkerClientFactoryFakeWorkerTests` uses `_ = RunWorkerAsync(...)` with no stored task (lines 152, 184, 220). An unhandled exception in the scripted worker becomes an unobserved `TaskException` that can surface as a process-level failure in an unrelated later test rather than failing the owning test.
**Recommendation:** Store the worker task and either await it during disposal or attach a continuation that fails the test on fault, mirroring `GatewayEndToEndFakeWorkerSmokeTests`. **Recommendation:** Store the worker task and either await it during disposal or attach a continuation that fails the test on fault, mirroring `GatewayEndToEndFakeWorkerSmokeTests`.
**Resolution:** _(open)_ **Resolution:** Resolved 2026-05-18: confirmed all three scripted launchers in `SessionWorkerClientFactoryFakeWorkerTests` discarded the worker task. Added an `IWorkerTaskLauncher` interface (each launcher now stores its scripted task in a `WorkerTask` property and exposes `ObserveWorkerTaskAsync`); the test class now implements `IAsyncDisposable`, tracks every launcher it creates via a `Track` helper, and in `DisposeAsync` awaits each `WorkerTask` (within `TestTimeout`) so a scripted-worker fault fails the owning test instead of leaking as an unobserved `TaskScheduler.UnobservedTaskException`. `OperationCanceledException` and `IOException` — the expected outcomes of the worker client tearing the pipe down — are swallowed; anything else rethrows. `NeverReadyWorkerProcessLauncher` (which parks on an infinite `Task.Delay`) was given its own `CancellationTokenSource` so disposal can cancel and observe the parked task. Suite green.
### Tests-012 ### Tests-012
@@ -202,10 +204,10 @@
| Severity | Low | | Severity | Low |
| Category | Concurrency & thread safety | | Category | Concurrency & thread safety |
| Location | `src/MxGateway.Tests/Gateway/Workers/Fakes/FakeWorkerHarness.cs:62`, `src/MxGateway.Tests/Gateway/Workers/WorkerClientTests.cs:472` | | Location | `src/MxGateway.Tests/Gateway/Workers/Fakes/FakeWorkerHarness.cs:62`, `src/MxGateway.Tests/Gateway/Workers/WorkerClientTests.cs:472` |
| Status | Open | | Status | Resolved |
**Description:** Pipe names are uniquified per test with a GUID (good), but xUnit runs test classes in parallel by default and there is no `xunit.runner.json` or collection configuration. Tests that build a full `WebApplication` bind ephemeral ports (`--urls=http://127.0.0.1:0`, fine) but spin up DI containers and hosted services concurrently. Currently safe, but a future test binding a fixed port would silently collide. **Description:** Pipe names are uniquified per test with a GUID (good), but xUnit runs test classes in parallel by default and there is no `xunit.runner.json` or collection configuration. Tests that build a full `WebApplication` bind ephemeral ports (`--urls=http://127.0.0.1:0`, fine) but spin up DI containers and hosted services concurrently. Currently safe, but a future test binding a fixed port would silently collide.
**Recommendation:** Add an `xunit.runner.json` or a collection grouping the `WebApplication`-building tests, and keep the `:0` ephemeral-port convention explicit so future tests do not introduce a fixed-port collision. **Recommendation:** Add an `xunit.runner.json` or a collection grouping the `WebApplication`-building tests, and keep the `:0` ephemeral-port convention explicit so future tests do not introduce a fixed-port collision.
**Resolution:** _(open)_ **Resolution:** Resolved 2026-05-18: added `src/MxGateway.Tests/xunit.runner.json` making the parallelism policy explicit (`parallelizeTestCollections: true`, `maxParallelThreads: -1`, `parallelizeAssembly: false`, `longRunningTestSeconds: 30`) and wired it into `MxGateway.Tests.csproj` as `<None Update="xunit.runner.json" CopyToOutputDirectory="PreserveNewest" />` so the runner picks it up (confirmed present in `bin/Debug/net10.0/`). Added a comment at the only `WebApplication`-building call site (`GatewayApplicationTests.cs`, `--urls=http://127.0.0.1:0`) documenting that the ephemeral-port (`:0`) convention is mandatory because test collections run in parallel. No fixed-port binding exists today; this is a preventative guardrail as the finding recommends.
+17 -17
View File
@@ -7,7 +7,7 @@
| Review date | 2026-05-18 | | Review date | 2026-05-18 |
| Commit reviewed | `6c64030` | | Commit reviewed | `6c64030` |
| Status | Reviewed | | Status | Reviewed |
| Open findings | 8 | | Open findings | 0 |
## Checklist coverage ## Checklist coverage
@@ -138,13 +138,13 @@
| Severity | Low | | Severity | Low |
| Category | Documentation & comments | | Category | Documentation & comments |
| Location | `src/MxGateway.Worker.Tests/Conversion/VariantConverterTests.cs:175-182` | | Location | `src/MxGateway.Worker.Tests/Conversion/VariantConverterTests.cs:175-182` |
| Status | Open | | Status | Resolved |
**Description:** `Redactor_WithCredentialBearingValueFields_RedactsBeforeLogging` lives in `VariantConverterTests` but asserts on `WorkerLogRedactor.RedactValue`, which has nothing to do with `VariantConverter`. It is also a near-duplicate of coverage in `WorkerLogRedactorTests`. Placing redaction coverage inside the variant-converter class is misleading. **Description:** `Redactor_WithCredentialBearingValueFields_RedactsBeforeLogging` lives in `VariantConverterTests` but asserts on `WorkerLogRedactor.RedactValue`, which has nothing to do with `VariantConverter`. It is also a near-duplicate of coverage in `WorkerLogRedactorTests`. Placing redaction coverage inside the variant-converter class is misleading.
**Recommendation:** Move this test into `Bootstrap/WorkerLogRedactorTests.cs` (which already exists and tests `RedactFields`). **Recommendation:** Move this test into `Bootstrap/WorkerLogRedactorTests.cs` (which already exists and tests `RedactFields`).
**Resolution:** _(open)_ **Resolution:** 2026-05-18 — The misplaced redaction test was removed from `VariantConverterTests.cs` and re-added to `Bootstrap/WorkerLogRedactorTests.cs` as `RedactValue_WithCredentialBearingFieldNames_ReturnsRedactedValue` — alongside the existing `RedactFields` coverage, where redaction tests belong. Confirmed root cause: the old test asserted only on `WorkerLogRedactor.RedactValue` and never touched `VariantConverter`. The now-orphaned `using MxGateway.Worker.Bootstrap;` was removed from `VariantConverterTests.cs` (`TreatWarningsAsErrors`). The new home is `RedactValue` per-field coverage; `WorkerLogRedactorTests.RedactFields_...` already covers the dictionary path, so the two are complementary rather than duplicates.
### Worker.Tests-009 ### Worker.Tests-009
@@ -153,13 +153,13 @@
| Severity | Low | | Severity | Low |
| Category | Code organization & conventions | | Category | Code organization & conventions |
| Location | `src/MxGateway.Worker.Tests/MxAccess/AlarmCommandHandlerTests.cs`, `AlarmDispatcherTests.cs`, `AlarmCommandExecutorTests.cs`, `AlarmRecordTransitionMapperTests.cs`, `WnWrapAlarmConsumerXmlTests.cs` | | Location | `src/MxGateway.Worker.Tests/MxAccess/AlarmCommandHandlerTests.cs`, `AlarmDispatcherTests.cs`, `AlarmCommandExecutorTests.cs`, `AlarmRecordTransitionMapperTests.cs`, `WnWrapAlarmConsumerXmlTests.cs` |
| Status | Open | | Status | Resolved |
**Description:** The alarm-related test files use `snake_case` method names while the rest of the project uses the `Method_State_Result` PascalCase convention. `docs/style-guides/CSharpStyleGuide.md` and the surrounding code establish PascalCase as the project convention; the alarm files diverge. **Description:** The alarm-related test files use `snake_case` method names while the rest of the project uses the `Method_State_Result` PascalCase convention. `docs/style-guides/CSharpStyleGuide.md` and the surrounding code establish PascalCase as the project convention; the alarm files diverge.
**Recommendation:** Rename alarm-test methods to the `Method_Scenario_Expectation` PascalCase form for one consistent convention. **Recommendation:** Rename alarm-test methods to the `Method_Scenario_Expectation` PascalCase form for one consistent convention.
**Resolution:** _(open)_ **Resolution:** 2026-05-18 — Renamed every `[Fact]`/`[Theory]` method in the five alarm test files from `snake_case` to the project's `Method_Scenario_Expectation` PascalCase form (46 test methods total: 10 in `AlarmCommandHandlerTests`, 8 in `AlarmDispatcherTests`, 12 in `AlarmCommandExecutorTests`, 8 in `AlarmRecordTransitionMapperTests`, 9 in `WnWrapAlarmConsumerXmlTests` minus the existing PascalCase probe methods). Only test methods were renamed — `snake_case` is not present; the method names that *look* like helpers (`Subscribe`, `PollOnce`, `Dispose` on the fake doubles) are interface implementations of `IAlarmCommandHandler`/`IAlarmTransitionConsumer`/`IDisposable` and were correctly left unchanged. The suite stays green; xUnit discovers tests by attribute, not name, so the renames are behaviour-neutral.
### Worker.Tests-010 ### Worker.Tests-010
@@ -168,13 +168,13 @@
| Severity | Low | | Severity | Low |
| Category | Correctness & logic bugs | | Category | Correctness & logic bugs |
| Location | `src/MxGateway.Worker.Tests/MxAccess/MxAccessStaSessionTests.cs:230-258` | | Location | `src/MxGateway.Worker.Tests/MxAccess/MxAccessStaSessionTests.cs:230-258` |
| Status | Open | | Status | Resolved |
**Description:** `StartAsync_WithoutAlarmCommandHandlerFactory_SubscribeAlarmsReturnsInvalidRequest` asserts `Assert.Contains("alarm", reply.DiagnosticMessage, StringComparison.OrdinalIgnoreCase)`. The XML doc claims it verifies the diagnostic says "alarm consumer not configured", but the assertion only checks the substring "alarm" — which would also match an unrelated message like "invalid alarm GUID". The assertion is weaker than the documented intent. **Description:** `StartAsync_WithoutAlarmCommandHandlerFactory_SubscribeAlarmsReturnsInvalidRequest` asserts `Assert.Contains("alarm", reply.DiagnosticMessage, StringComparison.OrdinalIgnoreCase)`. The XML doc claims it verifies the diagnostic says "alarm consumer not configured", but the assertion only checks the substring "alarm" — which would also match an unrelated message like "invalid alarm GUID". The assertion is weaker than the documented intent.
**Recommendation:** Assert the full diagnostic phrase so the test fails if the diagnostic regresses to a misleading message. **Recommendation:** Assert the full diagnostic phrase so the test fails if the diagnostic regresses to a misleading message.
**Resolution:** _(open)_ **Resolution:** 2026-05-18 — The weak `Assert.Contains("alarm", ...)` was replaced with an exact `Assert.Equal` against the diagnostic the executor actually emits. Re-triage: the test's XML doc claimed the phrase was "alarm consumer not configured", but `MxAccessCommandExecutor.ExecuteSubscribeAlarms` (verified in `src/MxGateway.Worker/MxAccess/MxAccessCommandExecutor.cs:310-315`) produces "SubscribeAlarms requires an alarm command handler; the worker was constructed without one." — the doc was wrong, so both the assertion and the XML doc were corrected to the real phrase. The test now fails if the diagnostic regresses to any other message.
### Worker.Tests-011 ### Worker.Tests-011
@@ -183,13 +183,13 @@
| Severity | Low | | Severity | Low |
| Category | Documentation & comments | | Category | Documentation & comments |
| Location | `src/MxGateway.Worker.Tests/Sta/StaCommandDispatcherTests.cs:92-112` | | Location | `src/MxGateway.Worker.Tests/Sta/StaCommandDispatcherTests.cs:92-112` |
| Status | Open | | Status | Resolved |
**Description:** `DispatchAsync_WhenCanceledAfterExecutionStarts_StillReturnsLateReply` is named and documented as if it proves cancellation arrived after execution began. The test does `Started.Wait(...)` then `cancellation.Cancel()`, which proves execution started, but because the executor is already running on the STA the cancellation is inherently a no-op — the test cannot distinguish "cancel was observed and ignored" from "cancel was never checked". The name overstates what is proven. **Description:** `DispatchAsync_WhenCanceledAfterExecutionStarts_StillReturnsLateReply` is named and documented as if it proves cancellation arrived after execution began. The test does `Started.Wait(...)` then `cancellation.Cancel()`, which proves execution started, but because the executor is already running on the STA the cancellation is inherently a no-op — the test cannot distinguish "cancel was observed and ignored" from "cancel was never checked". The name overstates what is proven.
**Recommendation:** Either tighten the test (assert the dispatcher's cancel path was reached and declined) or rename/comment it to "cancellation cannot abort an in-flight STA command", matching `gateway.md`'s stated behavior. **Recommendation:** Either tighten the test (assert the dispatcher's cancel path was reached and declined) or rename/comment it to "cancellation cannot abort an in-flight STA command", matching `gateway.md`'s stated behavior.
**Resolution:** _(open)_ **Resolution:** 2026-05-18 — Took the rename/re-document option. The test is renamed `DispatchAsync_WhenCanceledWhileExecuting_DoesNotAbortInFlightCommand` and its XML doc rewritten to state exactly what it proves — an in-flight STA command is *not* aborted by cancellation — and to state explicitly that the test cannot and does not distinguish "cancel observed and ignored" from "cancel never checked". The doc now cites `gateway.md`'s wording ("cannot safely abort an in-flight COM call on the STA"). The test body is unchanged: it already asserts the command runs to completion and returns its normal `Ok` reply, which is the genuine behaviour. No runtime behaviour changed.
### Worker.Tests-012 ### Worker.Tests-012
@@ -198,13 +198,13 @@
| Severity | Low | | Severity | Low |
| Category | Testing coverage | | Category | Testing coverage |
| Location | `src/MxGateway.Worker.Tests/Ipc/WorkerFrameProtocolTests.cs` | | Location | `src/MxGateway.Worker.Tests/Ipc/WorkerFrameProtocolTests.cs` |
| Status | Open | | Status | Resolved |
**Description:** `docs/WorkerFrameProtocol.md` states the reader "rejects zero-length payloads and payloads larger than the configured maximum (default 16 MiB) before allocating the payload buffer." `WorkerFrameProtocolTests` covers malformed-length, wrong protocol version, wrong session, and malformed payload, but has no test for the zero-length-payload rejection or the oversized-frame rejection — both explicit security-relevant input-validation paths. **Description:** `docs/WorkerFrameProtocol.md` states the reader "rejects zero-length payloads and payloads larger than the configured maximum (default 16 MiB) before allocating the payload buffer." `WorkerFrameProtocolTests` covers malformed-length, wrong protocol version, wrong session, and malformed payload, but has no test for the zero-length-payload rejection or the oversized-frame rejection — both explicit security-relevant input-validation paths.
**Recommendation:** Add tests feeding a frame with `payload_length == 0` and one with `payload_length` above the configured maximum, asserting the corresponding `WorkerFrameProtocolErrorCode`. **Recommendation:** Add tests feeding a frame with `payload_length == 0` and one with `payload_length` above the configured maximum, asserting the corresponding `WorkerFrameProtocolErrorCode`.
**Resolution:** _(open)_ **Resolution:** 2026-05-18 — Re-triage of the zero-length half: the finding's "no test for the zero-length-payload rejection" is partly inaccurate. The pre-existing `ReadAsync_WithMalformedLength_ThrowsMalformedLength` fed a four-zero-byte stream — which is exactly a frame declaring `payload_length == 0` — so the zero-length path *was* already covered, just under a misleading name (the length prefix itself is well-formed; only the declared length is zero). That test was renamed `ReadAsync_WithZeroLengthPayload_ThrowsMalformedLength` with an XML doc explaining the four-zero-byte construction, rather than adding a duplicate. The oversized half was a genuine gap: a new `ReadAsync_WithPayloadAboveConfiguredMaximum_ThrowsMessageTooLarge` constructs `WorkerFrameProtocolOptions` with a 64-byte maximum, feeds a length prefix of 65, and asserts `WorkerFrameProtocolErrorCode.MessageTooLarge` — verified against `WorkerFrameReader.ReadAsync`, both checks fire before the payload buffer is rented. The small configured maximum keeps the test from allocating a multi-megabyte buffer.
### Worker.Tests-013 ### Worker.Tests-013
@@ -213,13 +213,13 @@
| Severity | Low | | Severity | Low |
| Category | Concurrency & thread safety | | Category | Concurrency & thread safety |
| Location | `src/MxGateway.Worker.Tests/Ipc/WorkerPipeSessionTests.cs:539-546` | | Location | `src/MxGateway.Worker.Tests/Ipc/WorkerPipeSessionTests.cs:539-546` |
| Status | Open | | Status | Resolved |
**Description:** `ThrowIfCompletedAsync` does an unconditional `await Task.Delay(TimeSpan.FromMilliseconds(100))` then checks `task.IsCompleted`. This adds a fixed 100 ms to the test and only catches a `RunAsync` that fails within that arbitrary window; a session that faults after 100 ms slips past undetected. **Description:** `ThrowIfCompletedAsync` does an unconditional `await Task.Delay(TimeSpan.FromMilliseconds(100))` then checks `task.IsCompleted`. This adds a fixed 100 ms to the test and only catches a `RunAsync` that fails within that arbitrary window; a session that faults after 100 ms slips past undetected.
**Recommendation:** Replace with a deterministic race: `await Task.WhenAny(runTask, <first-expected-frame-read>)` and assert the run task did not win. **Recommendation:** Replace with a deterministic race: `await Task.WhenAny(runTask, <first-expected-frame-read>)` and assert the run task did not win.
**Resolution:** _(open)_ **Resolution:** 2026-05-18 — `ThrowIfCompletedAsync` was deleted (it had a single call site, in `RunAsync_SendsHeartbeatPayloadFromRuntimeSnapshot`). That test now races `runTask` against the first-heartbeat `ReadUntilAsync` with `Task.WhenAny`; if `runTask` wins it is awaited to surface the underlying fault and the test fails via `Assert.Fail`. The fixed 100 ms delay is gone — the check is now deterministic: a `RunAsync` faulting at *any* time before the first heartbeat is caught, and a healthy run completes as soon as the heartbeat arrives instead of always paying 100 ms.
### Worker.Tests-014 ### Worker.Tests-014
@@ -228,13 +228,13 @@
| Severity | Low | | Severity | Low |
| Category | Code organization & conventions | | Category | Code organization & conventions |
| Location | `src/MxGateway.Worker.Tests/Ipc/WorkerPipeClientTests.cs:194`, `WorkerPipeSessionTests.cs:622`, `Sta/StaCommandDispatcherTests.cs:348`, `MxAccess/MxAccessStaSessionTests.cs:334`, `MxAccess/MxAccessCommandExecutorTests.cs:1124` | | Location | `src/MxGateway.Worker.Tests/Ipc/WorkerPipeClientTests.cs:194`, `WorkerPipeSessionTests.cs:622`, `Sta/StaCommandDispatcherTests.cs:348`, `MxAccess/MxAccessStaSessionTests.cs:334`, `MxAccess/MxAccessCommandExecutorTests.cs:1124` |
| Status | Open | | Status | Resolved |
**Description:** `FakeRuntimeSession`, `NoopComApartmentInitializer`, `NoopEventSink`/`NullEventSink`, and the `CreateFrame`/`WriteUInt32LittleEndian` helpers are re-implemented independently in multiple test files. The two `FakeRuntimeSession` implementations have already diverged (one supports `BlockDispatch`/event enqueue, one does not), and `NoopComApartmentInitializer` is defined four times. **Description:** `FakeRuntimeSession`, `NoopComApartmentInitializer`, `NoopEventSink`/`NullEventSink`, and the `CreateFrame`/`WriteUInt32LittleEndian` helpers are re-implemented independently in multiple test files. The two `FakeRuntimeSession` implementations have already diverged (one supports `BlockDispatch`/event enqueue, one does not), and `NoopComApartmentInitializer` is defined four times.
**Recommendation:** Extract shared test doubles (`NoopComApartmentInitializer`, frame helpers, a single configurable `FakeRuntimeSession`) into a `TestSupport` folder/namespace consumed by all test classes. **Recommendation:** Extract shared test doubles (`NoopComApartmentInitializer`, frame helpers, a single configurable `FakeRuntimeSession`) into a `TestSupport` folder/namespace consumed by all test classes.
**Resolution:** _(open)_ **Resolution:** 2026-05-18 — Added a `src/MxGateway.Worker.Tests/TestSupport/` folder (namespace `MxGateway.Worker.Tests.TestSupport`) with four shared doubles: `NoopComApartmentInitializer`, `NoopEventSink`, `WorkerFrameTestHelpers` (`CreateFrame`/`WriteUInt32LittleEndian`), and a single configurable `FakeRuntimeSession`. The consolidated `FakeRuntimeSession` is the richer of the two divergent copies (it supports `BlockDispatch`, event enqueue, shutdown-timeout, and throw-after-release); the minimal `WorkerPipeClientTests` caller simply leaves the options unset. The per-file copies were deleted from `WorkerPipeClientTests`, `WorkerPipeSessionTests`, `StaCommandDispatcherTests`, `MxAccessStaSessionTests`, `MxAccessCommandExecutorTests`, and `WorkerFrameProtocolTests`, and the orphaned `NullEventSink` in `AlarmCommandExecutorTests` was replaced with the shared `NoopEventSink`. Re-triage: the finding says `NoopComApartmentInitializer` "is defined four times" — it was defined **three** times (`StaCommandDispatcherTests`, `MxAccessStaSessionTests`, `MxAccessCommandExecutorTests`); the fourth alarm-area `IStaComApartmentInitializer` implementation is `StaRuntimeTests.RecordingComApartmentInitializer`, which is a *recording* double (asserts init/uninit ordering), not a no-op, so it was deliberately left in place rather than folded into the shared no-op. Unused `using` directives left behind by the removals were stripped (`TreatWarningsAsErrors`).
### Worker.Tests-015 ### Worker.Tests-015
@@ -243,10 +243,10 @@
| Severity | Low | | Severity | Low |
| Category | Testing coverage | | Category | Testing coverage |
| Location | `src/MxGateway.Worker.Tests/MxAccess/MxAccessEventQueueTests.cs` | | Location | `src/MxGateway.Worker.Tests/MxAccess/MxAccessEventQueueTests.cs` |
| Status | Open | | Status | Resolved |
**Description:** `MxAccessEventQueueTests` covers monotonic sequencing, drain, capacity overflow, and first-fault-wins, but does not cover `Drain` with `maxEvents: 0` (drain-all) — a branch `FakeRuntimeSession.DrainEvents` even special-cases — nor draining an empty queue, nor enqueue after a manual `RecordFault`. These are minor branches but the overflow/fault interaction is the worker's backpressure contract. **Description:** `MxAccessEventQueueTests` covers monotonic sequencing, drain, capacity overflow, and first-fault-wins, but does not cover `Drain` with `maxEvents: 0` (drain-all) — a branch `FakeRuntimeSession.DrainEvents` even special-cases — nor draining an empty queue, nor enqueue after a manual `RecordFault`. These are minor branches but the overflow/fault interaction is the worker's backpressure contract.
**Recommendation:** Add a `Drain(0)` drain-all test and an empty-queue drain test. **Recommendation:** Add a `Drain(0)` drain-all test and an empty-queue drain test.
**Resolution:** _(open)_ **Resolution:** 2026-05-18 — Added three tests to `MxAccessEventQueueTests`. `Drain_WithZeroMaxEvents_DrainsAllEvents` covers the `maxEvents == 0` drain-all branch in `MxAccessEventQueue.Drain` (verified at `src/MxGateway.Worker/MxAccess/MxAccessEventQueue.cs:174`) — three events enqueued, `Drain(0)` returns all three in order and empties the queue. `Drain_WhenQueueIsEmpty_ReturnsEmptyList` covers the `drainCount == 0` early-return branch for both `Drain(0)` and `Drain(5)` on an empty queue. `Enqueue_AfterRecordFault_ThrowsInvalidOperationException` covers the backpressure contract gap the finding flagged — after a manual `RecordFault`, `Enqueue` throws `InvalidOperationException` ("outbound event queue is faulted") and the event is not queued.
+22
View File
@@ -776,6 +776,28 @@ case, to distinguish the two acks. `WorkerAlarmRpcDispatcher` reads only
the top-level `hresult`/`protocol_status`, so it handles both arms the top-level `hresult`/`protocol_status`, so it handles both arms
without unpacking the payload. without unpacking the payload.
**Worker `native_status` → public `AcknowledgeAlarmReply` mapping.** The
worker carries the ack outcome as a single `int32`
(`AcknowledgeAlarmReplyPayload.native_status`, the `AlarmAckByName` /
`AlarmAckByGUID` return code; `0` = success), also mirrored into the
worker `MxCommandReply.hresult`. The public `AcknowledgeAlarmReply` has
two outcome-shaped fields, but only one is populated:
- `AcknowledgeAlarmReply.hresult``WorkerAlarmRpcDispatcher` copies the
worker's `MxCommandReply.hresult` (the native return code) into this
field. **This is the authoritative ack-outcome field**; `0` means the
ack succeeded. It is absent only when the worker reply omitted the
value, which is a protocol violation surfaced in `protocol_status`.
- `AcknowledgeAlarmReply.status` (`MxStatusProxy`) — the worker by-name /
by-GUID ack path produces only the `int32` return code, never a
populated `MXSTATUS_PROXY` struct, so `WorkerAlarmRpcDispatcher` leaves
this field **unset on every reply**. It is reserved for a future
structured view of the ack outcome. Clients must not depend on it.
Client authors should therefore branch on `protocol_status` first (for
transport/session-level failures) and then on `hresult` (`0` = ack
accepted by MXAccess) — never on `status`.
### 5. STA / threading — production fix needed ### 5. STA / threading — production fix needed
The wnwrap COM is `ThreadingModel=Apartment`. The consumer's The wnwrap COM is `ThreadingModel=Apartment`. The consumer's
+12 -2
View File
@@ -10,7 +10,7 @@ The layer is composed of four collaborators:
| Type | Lifetime | Role | | Type | Lifetime | Role |
|------|----------|------| |------|----------|------|
| `MxAccessGatewayService` | scoped (gRPC) | Implements the four `MxAccessGateway` RPCs, performs exception mapping. | | `MxAccessGatewayService` | scoped (gRPC) | Implements the six `MxAccessGateway` RPCs, performs exception mapping. |
| `MxAccessGrpcRequestValidator` | singleton | Rejects malformed requests before any session work runs. | | `MxAccessGrpcRequestValidator` | singleton | Rejects malformed requests before any session work runs. |
| `MxAccessGrpcMapper` | singleton | Converts public proto types to internal `WorkerCommand`/`WorkerEvent` types and back. | | `MxAccessGrpcMapper` | singleton | Converts public proto types to internal `WorkerCommand`/`WorkerEvent` types and back. |
| `IEventStreamService` (`EventStreamService`) | singleton | Owns the event stream pipeline, including bounded queue and backpressure handling. | | `IEventStreamService` (`EventStreamService`) | singleton | Owns the event stream pipeline, including bounded queue and backpressure handling. |
@@ -29,7 +29,7 @@ A second gRPC service, `GalaxyRepositoryGrpcService`, is mapped alongside it. It
## RPC Handlers ## RPC Handlers
`MxAccessGatewayService` derives from the generated `MxAccessGateway.MxAccessGatewayBase` and implements every RPC declared in `mxaccess_gateway.proto`. The proto contract itself is documented in [Contracts](./Contracts.md); this section covers only what the server-side handler does on top of that contract. `MxAccessGatewayService` derives from the generated `MxAccessGateway.MxAccessGatewayBase` and implements every RPC declared in `mxaccess_gateway.proto` — six in total: `OpenSession`, `CloseSession`, `Invoke`, `StreamEvents`, `AcknowledgeAlarm`, and `QueryActiveAlarms`. The proto contract itself is documented in [Contracts](./Contracts.md); this section covers only what the server-side handler does on top of that contract.
Public gRPC send and receive message sizes are configured from Public gRPC send and receive message sizes are configured from
`MxGateway:Protocol:MaxGrpcMessageBytes` (default 16 MiB). Official clients use `MxGateway:Protocol:MaxGrpcMessageBytes` (default 16 MiB). Official clients use
@@ -86,6 +86,14 @@ Carrying the enqueue timestamp into the worker layer is what lets queue-wait tim
`StreamEvents` is a server-streaming RPC. The handler delegates the full pipeline to `IEventStreamService` and just forwards each `MxEvent` onto the response stream. Keeping the channel and producer/consumer machinery out of the handler means cancellation, exception mapping, and metric bookkeeping live in one place. `StreamEvents` is a server-streaming RPC. The handler delegates the full pipeline to `IEventStreamService` and just forwards each `MxEvent` onto the response stream. Keeping the channel and producer/consumer machinery out of the handler means cancellation, exception mapping, and metric bookkeeping live in one place.
### `AcknowledgeAlarm`
`AcknowledgeAlarm` is a unary RPC that acknowledges a single alarm. The handler validates `session_id` and `alarm_full_reference` inline (it does not run through `MxAccessGrpcRequestValidator`, because the alarm surface routes through `IAlarmRpcDispatcher` rather than the generic `Invoke` path), resolves the session, then delegates to the registered `IAlarmRpcDispatcher`. The production `WorkerAlarmRpcDispatcher` routes the ack over the worker IPC by GUID (`AcknowledgeAlarmCommand`) when the reference parses as a canonical GUID, or by `Provider!Group.Tag` reference (`AcknowledgeAlarmByNameCommand`) otherwise. The handler-level RPC behaviour and the alarm contract itself are documented in [Alarm Client Discovery](./AlarmClientDiscovery.md).
### `QueryActiveAlarms`
`QueryActiveAlarms` is a server-streaming RPC that returns an `ActiveAlarmSnapshot` per currently active alarm. The handler validates `session_id` inline, resolves the session, and delegates to `IAlarmRpcDispatcher`; `WorkerAlarmRpcDispatcher` issues a `QueryActiveAlarmsCommand` over the worker IPC and streams each snapshot from the worker reply.
## Validation Rules ## Validation Rules
`MxAccessGrpcRequestValidator` rejects requests with `StatusCode.InvalidArgument` before any session work happens. The rules are intentionally narrow — anything that requires session state (for example, "session does not exist") is left for `ISessionManager` so the validator can stay synchronous and side-effect free. `MxAccessGrpcRequestValidator` rejects requests with `StatusCode.InvalidArgument` before any session work happens. The rules are intentionally narrow — anything that requires session state (for example, "session does not exist") is left for `ISessionManager` so the validator can stay synchronous and side-effect free.
@@ -96,6 +104,8 @@ Carrying the enqueue timestamp into the worker layer is what lets queue-wait tim
| `CloseSession` | `session_id` must be non-empty. | `InvalidArgument` | | `CloseSession` | `session_id` must be non-empty. | `InvalidArgument` |
| `StreamEvents` | `session_id` must be non-empty. | `InvalidArgument` | | `StreamEvents` | `session_id` must be non-empty. | `InvalidArgument` |
| `Invoke` | `session_id` non-empty, `command` present, `kind` not `Unspecified`, payload oneof must match `kind`. | `InvalidArgument` | | `Invoke` | `session_id` non-empty, `command` present, `kind` not `Unspecified`, payload oneof must match `kind`. | `InvalidArgument` |
| `AcknowledgeAlarm` | `session_id` and `alarm_full_reference` must be non-empty. Validated inline in the handler, not by `MxAccessGrpcRequestValidator`. | `InvalidArgument` |
| `QueryActiveAlarms` | `session_id` must be non-empty. Validated inline in the handler, not by `MxAccessGrpcRequestValidator`. | `InvalidArgument` |
The payload-vs-kind check matters because the `MxCommand.payload` oneof is non-discriminated on the wire — a misaligned client could send `kind = Write` with a `Register` payload and silently confuse the worker. The validator turns that into a clear client error: The payload-vs-kind check matters because the `MxCommand.payload` oneof is non-discriminated on the wire — a misaligned client could send `kind = Write` with a `Register` payload and silently confuse the worker. The validator turns that into a clear client error:
@@ -1,8 +1,10 @@
namespace MxGateway.Contracts; namespace MxGateway.Contracts;
/// <summary> /// <summary>
/// Exposes version metadata shared by gateway components before generated /// Holds the protocol version constants shared by gateway components.
/// protobuf contracts are introduced. /// <see cref="GatewayProtocolVersion"/> is advertised to clients in
/// <c>OpenSessionReply</c>; <see cref="WorkerProtocolVersion"/> is used to
/// validate <c>WorkerEnvelope</c> protocol framing on the gateway↔worker pipe.
/// </summary> /// </summary>
public static class GatewayContractInfo public static class GatewayContractInfo
{ {
@@ -21418,7 +21418,12 @@ namespace MxGateway.Contracts.Proto {
private int hresult_; private int hresult_;
/// <summary> /// <summary>
/// HRESULT captured from MXAccess if the ack failed at the COM layer. /// Native ack return code echoed from the worker. The worker carries the
/// ack outcome as a single int32 (AcknowledgeAlarmReplyPayload.native_status,
/// = AlarmAckByName / AlarmAckByGUID return code; 0 = success); the gateway's
/// WorkerAlarmRpcDispatcher copies that value here. This is the authoritative
/// ack-outcome field for the public RPC. Absent only when the worker reply
/// omitted the value entirely (a protocol violation).
/// </summary> /// </summary>
[global::System.Diagnostics.DebuggerNonUserCodeAttribute] [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
[global::System.CodeDom.Compiler.GeneratedCode("protoc", null)] [global::System.CodeDom.Compiler.GeneratedCode("protoc", null)]
@@ -21446,7 +21451,11 @@ namespace MxGateway.Contracts.Proto {
public const int StatusFieldNumber = 5; public const int StatusFieldNumber = 5;
private global::MxGateway.Contracts.Proto.MxStatusProxy status_; private global::MxGateway.Contracts.Proto.MxStatusProxy status_;
/// <summary> /// <summary>
/// Native MxAccess status describing the outcome of the ack. /// Reserved for a structured MxStatusProxy view of the ack outcome. The
/// worker by-name/by-GUID ack path produces only the int32 return code
/// (see `hresult`), so the current gateway leaves this field UNSET on every
/// reply. Clients must read `hresult` (and `protocol_status`) for the ack
/// result and must not depend on `status` being populated.
/// </summary> /// </summary>
[global::System.Diagnostics.DebuggerNonUserCodeAttribute] [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
[global::System.CodeDom.Compiler.GeneratedCode("protoc", null)] [global::System.CodeDom.Compiler.GeneratedCode("protoc", null)]
@@ -22078,6 +22087,17 @@ namespace MxGateway.Contracts.Proto {
/// <summary>Field number for the "success" field.</summary> /// <summary>Field number for the "success" field.</summary>
public const int SuccessFieldNumber = 1; public const int SuccessFieldNumber = 1;
private int success_; private int success_;
/// <summary>
/// Mirrors the `success` member of the MXAccess MXSTATUS_PROXY struct
/// (a 16-bit signed value in the COM struct, widened to int32 on the
/// wire). Despite the name it is NOT a boolean — it is the raw numeric
/// indicator the worker reads off the COM struct without reinterpretation.
/// It is carried verbatim for diagnostics; the authoritative success/
/// failure of the operation is `category` (MX_STATUS_CATEGORY_OK marks
/// success), with `detail`, `diagnostic_text`, `raw_category`, and
/// `raw_detected_by` describing any non-OK outcome. Clients should branch
/// on `category`, not on a specific `success` value.
/// </summary>
[global::System.Diagnostics.DebuggerNonUserCodeAttribute] [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
[global::System.CodeDom.Compiler.GeneratedCode("protoc", null)] [global::System.CodeDom.Compiler.GeneratedCode("protoc", null)]
public int Success { public int Success {
@@ -7,6 +7,13 @@ option csharp_namespace = "MxGateway.Contracts.Proto.Galaxy";
import "google/protobuf/timestamp.proto"; import "google/protobuf/timestamp.proto";
import "google/protobuf/wrappers.proto"; import "google/protobuf/wrappers.proto";
// Wire-compatibility policy (ProtobufStyleGuide): this contract evolves
// additively only. Never renumber or repurpose an existing field number or
// enum value. When a field or enum value is removed, add a `reserved` range
// (and `reserved` name) covering it in the same change so a future editor
// cannot accidentally reuse the retired tag. There are no `reserved`
// declarations today because no field or enum value has ever been removed.
// Read-only browse over the AVEVA System Platform Galaxy Repository (ZB SQL // Read-only browse over the AVEVA System Platform Galaxy Repository (ZB SQL
// database). Lets clients enumerate the deployed object hierarchy and each // database). Lets clients enumerate the deployed object hierarchy and each
// object's dynamic attributes so they know what tag references to subscribe // object's dynamic attributes so they know what tag references to subscribe
@@ -7,6 +7,13 @@ option csharp_namespace = "MxGateway.Contracts.Proto";
import "google/protobuf/duration.proto"; import "google/protobuf/duration.proto";
import "google/protobuf/timestamp.proto"; import "google/protobuf/timestamp.proto";
// Wire-compatibility policy (ProtobufStyleGuide): this contract evolves
// additively only. Never renumber or repurpose an existing field number or
// enum value. When a field or enum value is removed, add a `reserved` range
// (and `reserved` name) covering it in the same change so a future editor
// cannot accidentally reuse the retired tag. There are no `reserved`
// declarations today because no field or enum value has ever been removed.
// Public client API for MXAccess sessions hosted by the gateway. // Public client API for MXAccess sessions hosted by the gateway.
service MxAccessGateway { service MxAccessGateway {
rpc OpenSession(OpenSessionRequest) returns (OpenSessionReply); rpc OpenSession(OpenSessionRequest) returns (OpenSessionReply);
@@ -641,9 +648,18 @@ message AcknowledgeAlarmReply {
string session_id = 1; string session_id = 1;
string correlation_id = 2; string correlation_id = 2;
ProtocolStatus protocol_status = 3; ProtocolStatus protocol_status = 3;
// HRESULT captured from MXAccess if the ack failed at the COM layer. // Native ack return code echoed from the worker. The worker carries the
// ack outcome as a single int32 (AcknowledgeAlarmReplyPayload.native_status,
// = AlarmAckByName / AlarmAckByGUID return code; 0 = success); the gateway's
// WorkerAlarmRpcDispatcher copies that value here. This is the authoritative
// ack-outcome field for the public RPC. Absent only when the worker reply
// omitted the value entirely (a protocol violation).
optional int32 hresult = 4; optional int32 hresult = 4;
// Native MxAccess status describing the outcome of the ack. // Reserved for a structured MxStatusProxy view of the ack outcome. The
// worker by-name/by-GUID ack path produces only the int32 return code
// (see `hresult`), so the current gateway leaves this field UNSET on every
// reply. Clients must read `hresult` (and `protocol_status`) for the ack
// result and must not depend on `status` being populated.
MxStatusProxy status = 5; MxStatusProxy status = 5;
string diagnostic_message = 6; string diagnostic_message = 6;
} }
@@ -657,6 +673,15 @@ message QueryActiveAlarmsRequest {
} }
message MxStatusProxy { message MxStatusProxy {
// Mirrors the `success` member of the MXAccess MXSTATUS_PROXY struct
// (a 16-bit signed value in the COM struct, widened to int32 on the
// wire). Despite the name it is NOT a boolean it is the raw numeric
// indicator the worker reads off the COM struct without reinterpretation.
// It is carried verbatim for diagnostics; the authoritative success/
// failure of the operation is `category` (MX_STATUS_CATEGORY_OK marks
// success), with `detail`, `diagnostic_text`, `raw_category`, and
// `raw_detected_by` describing any non-OK outcome. Clients should branch
// on `category`, not on a specific `success` value.
int32 success = 1; int32 success = 1;
MxStatusCategory category = 2; MxStatusCategory category = 2;
MxStatusSource detected_by = 3; MxStatusSource detected_by = 3;
@@ -8,6 +8,13 @@ import "google/protobuf/duration.proto";
import "google/protobuf/timestamp.proto"; import "google/protobuf/timestamp.proto";
import "mxaccess_gateway.proto"; import "mxaccess_gateway.proto";
// Wire-compatibility policy (ProtobufStyleGuide): this contract evolves
// additively only. Never renumber or repurpose an existing field number or
// enum value. When a field or enum value is removed, add a `reserved` range
// (and `reserved` name) covering it in the same change so a future editor
// cannot accidentally reuse the retired tag. There are no `reserved`
// declarations today because no field or enum value has ever been removed.
// Gateway-to-worker IPC envelope. Named-pipe framing prepends a little-endian // Gateway-to-worker IPC envelope. Named-pipe framing prepends a little-endian
// uint32 payload length to this protobuf payload. // uint32 payload length to this protobuf payload.
message WorkerEnvelope { message WorkerEnvelope {
@@ -6,6 +6,7 @@ using MxGateway.Server.Dashboard;
namespace MxGateway.IntegrationTests; namespace MxGateway.IntegrationTests;
[Collection(LiveResourcesCollection.Name)]
public sealed class DashboardLdapLiveTests public sealed class DashboardLdapLiveTests
{ {
[LiveLdapFact] [LiveLdapFact]
@@ -2,6 +2,7 @@ using MxGateway.Server.Galaxy;
namespace MxGateway.IntegrationTests.Galaxy; namespace MxGateway.IntegrationTests.Galaxy;
[Collection(LiveResourcesCollection.Name)]
public sealed class GalaxyRepositoryLiveTests public sealed class GalaxyRepositoryLiveTests
{ {
/// <summary>Verifies that the Galaxy Repository can establish a live connection to the ZB database.</summary> /// <summary>Verifies that the Galaxy Repository can establish a live connection to the ZB database.</summary>
@@ -18,11 +18,7 @@ public sealed class LiveGalaxyRepositoryFactAttribute : FactAttribute
} }
/// <summary>Gets a value indicating whether live Galaxy Repository tests are enabled.</summary> /// <summary>Gets a value indicating whether live Galaxy Repository tests are enabled.</summary>
public static bool Enabled => public static bool Enabled => IntegrationTestEnvironment.IsEnabled(EnableVariableName);
string.Equals(
Environment.GetEnvironmentVariable(EnableVariableName),
"1",
StringComparison.Ordinal);
/// <summary>Gets the Galaxy Repository connection string from environment or default.</summary> /// <summary>Gets the Galaxy Repository connection string from environment or default.</summary>
public static string ConnectionString => public static string ConnectionString =>
@@ -9,9 +9,18 @@ public static class IntegrationTestEnvironment
public const string LiveMxAccessEventTimeoutSecondsVariableName = "MXGATEWAY_LIVE_MXACCESS_EVENT_TIMEOUT_SECONDS"; public const string LiveMxAccessEventTimeoutSecondsVariableName = "MXGATEWAY_LIVE_MXACCESS_EVENT_TIMEOUT_SECONDS";
/// <summary>Gets whether live MXAccess tests are enabled.</summary> /// <summary>Gets whether live MXAccess tests are enabled.</summary>
public static bool LiveMxAccessTestsEnabled => public static bool LiveMxAccessTestsEnabled => IsEnabled(LiveMxAccessVariableName);
/// <summary>
/// Gets whether an opt-in live-test suite is enabled, by comparing the named
/// environment variable to <c>1</c>. Shared by every <c>Live*FactAttribute</c>
/// so the opt-in check has a single implementation.
/// </summary>
/// <param name="variableName">The environment variable that gates the suite.</param>
/// <returns><see langword="true"/> when the variable is exactly <c>1</c>.</returns>
public static bool IsEnabled(string variableName) =>
string.Equals( string.Equals(
Environment.GetEnvironmentVariable(LiveMxAccessVariableName), Environment.GetEnvironmentVariable(variableName),
"1", "1",
StringComparison.Ordinal); StringComparison.Ordinal);
@@ -12,9 +12,5 @@ public sealed class LiveLdapFactAttribute : FactAttribute
} }
} }
public static bool Enabled => public static bool Enabled => IntegrationTestEnvironment.IsEnabled(EnableVariableName);
string.Equals(
Environment.GetEnvironmentVariable(EnableVariableName),
"1",
StringComparison.Ordinal);
} }
@@ -0,0 +1,16 @@
namespace MxGateway.IntegrationTests;
/// <summary>
/// xUnit collection that serializes every live integration-test class. The live
/// suites contend for genuinely shared singletons — one MXAccess COM provider,
/// one <c>ZB</c> SQL database, and one GLAuth instance with a per-IP failure
/// lockout — so they must not run in parallel with one another. Placing each
/// live class in this collection disables xUnit's default cross-class
/// parallelism for them while leaving non-live tests free to parallelize.
/// </summary>
[CollectionDefinition(Name, DisableParallelization = true)]
public sealed class LiveResourcesCollection
{
/// <summary>The collection name applied via <c>[Collection]</c> on live test classes.</summary>
public const string Name = "Live external resources";
}
@@ -17,6 +17,7 @@ using Xunit.Abstractions;
namespace MxGateway.IntegrationTests; namespace MxGateway.IntegrationTests;
[Collection(LiveResourcesCollection.Name)]
public sealed class WorkerLiveMxAccessSmokeTests(ITestOutputHelper output) public sealed class WorkerLiveMxAccessSmokeTests(ITestOutputHelper output)
{ {
private static readonly TimeSpan CommandTimeout = TimeSpan.FromSeconds(15); private static readonly TimeSpan CommandTimeout = TimeSpan.FromSeconds(15);
@@ -40,6 +41,7 @@ public sealed class WorkerLiveMxAccessSmokeTests(ITestOutputHelper output)
string? sessionId = null; string? sessionId = null;
RecordingServerStreamWriter<MxEvent>? eventWriter = null; RecordingServerStreamWriter<MxEvent>? eventWriter = null;
Task? streamTask = null; Task? streamTask = null;
using CancellationTokenSource streamCancellation = new();
try try
{ {
@@ -61,7 +63,7 @@ public sealed class WorkerLiveMxAccessSmokeTests(ITestOutputHelper output)
streamTask = fixture.Service.StreamEvents( streamTask = fixture.Service.StreamEvents(
new StreamEventsRequest { SessionId = sessionId }, new StreamEventsRequest { SessionId = sessionId },
eventWriter, eventWriter,
new TestServerCallContext()); new TestServerCallContext(streamCancellation.Token));
MxCommandReply registerReply = await fixture.Service.Invoke( MxCommandReply registerReply = await fixture.Service.Invoke(
CreateRegisterRequest(sessionId), CreateRegisterRequest(sessionId),
@@ -94,7 +96,8 @@ public sealed class WorkerLiveMxAccessSmokeTests(ITestOutputHelper output)
MxEvent dataChange = await eventWriter MxEvent dataChange = await eventWriter
.WaitForMessageAsync( .WaitForMessageAsync(
candidate => candidate.Family == MxEventFamily.OnDataChange, candidate => candidate.Family == MxEventFamily.OnDataChange,
IntegrationTestEnvironment.LiveMxAccessEventTimeout) IntegrationTestEnvironment.LiveMxAccessEventTimeout,
streamCancellation.Token)
.ConfigureAwait(false); .ConfigureAwait(false);
LogEvent(dataChange); LogEvent(dataChange);
@@ -560,12 +563,20 @@ public sealed class WorkerLiveMxAccessSmokeTests(ITestOutputHelper output)
/// </summary> /// </summary>
/// <param name="predicate">Filter the awaited message must satisfy.</param> /// <param name="predicate">Filter the awaited message must satisfy.</param>
/// <param name="timeout">The maximum total time to wait.</param> /// <param name="timeout">The maximum total time to wait.</param>
/// <param name="cancellationToken">
/// Token observed alongside the timeout so a per-test cancellation (for example the
/// gRPC call context's token) aborts the wait promptly instead of hanging until the
/// timeout elapses.
/// </param>
/// <returns>The first message that satisfies the predicate.</returns> /// <returns>The first message that satisfies the predicate.</returns>
public async Task<T> WaitForMessageAsync( public async Task<T> WaitForMessageAsync(
Func<T, bool> predicate, Func<T, bool> predicate,
TimeSpan timeout) TimeSpan timeout,
CancellationToken cancellationToken = default)
{ {
using CancellationTokenSource timeoutCancellation = new(timeout); using CancellationTokenSource timeoutCancellation = new(timeout);
using CancellationTokenSource linkedCancellation =
CancellationTokenSource.CreateLinkedTokenSource(timeoutCancellation.Token, cancellationToken);
int scanned = 0; int scanned = 0;
while (true) while (true)
@@ -586,7 +597,7 @@ public sealed class WorkerLiveMxAccessSmokeTests(ITestOutputHelper output)
try try
{ {
await messageArrived.WaitAsync(timeoutCancellation.Token).ConfigureAwait(false); await messageArrived.WaitAsync(linkedCancellation.Token).ConfigureAwait(false);
} }
catch (OperationCanceledException) when (timeoutCancellation.IsCancellationRequested) catch (OperationCanceledException) when (timeoutCancellation.IsCancellationRequested)
{ {
@@ -598,7 +609,9 @@ public sealed class WorkerLiveMxAccessSmokeTests(ITestOutputHelper output)
} }
/// <summary> /// <summary>
/// Mock server call context for testing gRPC calls. /// Minimal <see cref="ServerCallContext"/> stub for invoking the gRPC service
/// in-process. It is a hand-written fake with no verification behavior — it
/// only supplies the context values the service reads during a call.
/// </summary> /// </summary>
private sealed class TestServerCallContext(CancellationToken cancellationToken = default) : ServerCallContext private sealed class TestServerCallContext(CancellationToken cancellationToken = default) : ServerCallContext
{ {
@@ -2,6 +2,7 @@ using Google.Protobuf;
using Google.Protobuf.WellKnownTypes; using Google.Protobuf.WellKnownTypes;
using MxGateway.Contracts; using MxGateway.Contracts;
using MxGateway.Contracts.Proto; using MxGateway.Contracts.Proto;
using MxGateway.Contracts.Proto.Galaxy;
namespace MxGateway.Tests.Contracts; namespace MxGateway.Tests.Contracts;
@@ -439,4 +440,334 @@ public sealed class ProtobufContractRoundTripTests
Assert.Equal(withoutFilter, QueryActiveAlarmsRequest.Parser.ParseFrom(withoutFilter.ToByteArray())); Assert.Equal(withoutFilter, QueryActiveAlarmsRequest.Parser.ParseFrom(withoutFilter.ToByteArray()));
Assert.Equal(withFilter, QueryActiveAlarmsRequest.Parser.ParseFrom(withFilter.ToByteArray())); Assert.Equal(withFilter, QueryActiveAlarmsRequest.Parser.ParseFrom(withFilter.ToByteArray()));
} }
/// <summary>Verifies that an MxValue carrying a raw_value bytes payload round-trips.</summary>
[Fact]
public void MxValue_RoundTripsRawValueBytesPayload()
{
var original = new MxValue
{
DataType = MxDataType.Unknown,
VariantType = "VT_UNKNOWN",
RawDataType = 99,
RawDiagnostic = "uninterpreted COM variant",
RawValue = ByteString.CopyFrom(0x01, 0x02, 0xFE, 0xFF),
};
var parsed = MxValue.Parser.ParseFrom(original.ToByteArray());
Assert.Equal(original, parsed);
Assert.Equal(MxValue.KindOneofCase.RawValue, parsed.KindCase);
Assert.Equal(new byte[] { 0x01, 0x02, 0xFE, 0xFF }, parsed.RawValue.ToByteArray());
}
/// <summary>Verifies that an MxArray carrying a RawArray of byte blobs round-trips.</summary>
[Fact]
public void MxArray_RoundTripsRawArrayPayload()
{
var original = new MxArray
{
ElementDataType = MxDataType.Unknown,
VariantType = "VT_ARRAY|VT_UNKNOWN",
RawElementDataType = 99,
RawDiagnostic = "uninterpreted SAFEARRAY",
Dimensions = { 2 },
RawValues = new RawArray
{
Values =
{
ByteString.CopyFrom(0xAA, 0xBB),
ByteString.CopyFrom(0xCC, 0xDD),
},
},
};
var parsed = MxArray.Parser.ParseFrom(original.ToByteArray());
Assert.Equal(original, parsed);
Assert.Equal(MxArray.ValuesOneofCase.RawValues, parsed.ValuesCase);
Assert.Equal(2, parsed.RawValues.Values.Count);
}
/// <summary>Verifies that a BulkSubscribeReply with per-item SubscribeResults round-trips.</summary>
[Fact]
public void BulkSubscribeReply_RoundTripsSubscribeResults()
{
var original = new BulkSubscribeReply
{
Results =
{
new SubscribeResult
{
ServerHandle = 10,
TagAddress = "Provider!Tank01.Level",
ItemHandle = 21,
WasSuccessful = true,
},
new SubscribeResult
{
ServerHandle = 10,
TagAddress = "Provider!Bad.Tag",
WasSuccessful = false,
ErrorMessage = "item not found",
},
},
};
var parsed = BulkSubscribeReply.Parser.ParseFrom(original.ToByteArray());
Assert.Equal(original, parsed);
Assert.Equal(2, parsed.Results.Count);
Assert.True(parsed.Results[0].WasSuccessful);
Assert.False(parsed.Results[1].WasSuccessful);
}
/// <summary>Verifies that a bulk-subscribe command and its BulkSubscribeReply payload round-trip.</summary>
[Fact]
public void MxCommandReply_RoundTripsBulkSubscribePayload()
{
var original = new MxCommandReply
{
SessionId = "session-1",
CorrelationId = "gateway-correlation-bulk",
Kind = MxCommandKind.SubscribeBulk,
ProtocolStatus = new ProtocolStatus { Code = ProtocolStatusCode.Ok },
Hresult = 0,
SubscribeBulk = new BulkSubscribeReply
{
Results =
{
new SubscribeResult
{
ServerHandle = 5,
TagAddress = "Provider!Tank01.Level",
ItemHandle = 7,
WasSuccessful = true,
},
},
},
};
var parsed = MxCommandReply.Parser.ParseFrom(original.ToByteArray());
Assert.Equal(original, parsed);
Assert.Equal(MxCommandReply.PayloadOneofCase.SubscribeBulk, parsed.PayloadCase);
Assert.Single(parsed.SubscribeBulk.Results);
}
/// <summary>Verifies that a WorkerEnvelope carrying a WorkerFault body round-trips.</summary>
[Fact]
public void WorkerEnvelope_RoundTripsWorkerFaultBody()
{
var original = new WorkerEnvelope
{
ProtocolVersion = GatewayContractInfo.WorkerProtocolVersion,
SessionId = "session-1",
Sequence = 11,
CorrelationId = "gateway-correlation-fault",
WorkerFault = new WorkerFault
{
Category = WorkerFaultCategory.MxaccessCommandFailed,
CommandMethod = "Register",
Hresult = unchecked((int)0x80004005),
ExceptionType = "System.Runtime.InteropServices.COMException",
DiagnosticMessage = "MXAccess COM call failed.",
ProtocolStatus = new ProtocolStatus { Code = ProtocolStatusCode.ProtocolViolation },
},
};
var parsed = WorkerEnvelope.Parser.ParseFrom(original.ToByteArray());
Assert.Equal(original, parsed);
Assert.Equal(WorkerEnvelope.BodyOneofCase.WorkerFault, parsed.BodyCase);
Assert.True(parsed.WorkerFault.HasHresult);
}
/// <summary>Verifies that a WorkerEnvelope carrying a WorkerHeartbeat body round-trips.</summary>
[Fact]
public void WorkerEnvelope_RoundTripsWorkerHeartbeatBody()
{
var activity = Timestamp.FromDateTime(new DateTime(2026, 5, 18, 9, 0, 0, DateTimeKind.Utc));
var original = new WorkerEnvelope
{
ProtocolVersion = GatewayContractInfo.WorkerProtocolVersion,
SessionId = "session-1",
Sequence = 12,
CorrelationId = "gateway-correlation-heartbeat",
WorkerHeartbeat = new WorkerHeartbeat
{
WorkerProcessId = 4242,
State = WorkerState.Ready,
LastStaActivityTimestamp = activity,
PendingCommandCount = 3,
OutboundEventQueueDepth = 7,
LastEventSequence = 1234,
CurrentCommandCorrelationId = "in-flight-1",
},
};
var parsed = WorkerEnvelope.Parser.ParseFrom(original.ToByteArray());
Assert.Equal(original, parsed);
Assert.Equal(WorkerEnvelope.BodyOneofCase.WorkerHeartbeat, parsed.BodyCase);
Assert.Equal(WorkerState.Ready, parsed.WorkerHeartbeat.State);
}
/// <summary>Verifies that the Galaxy Repository service descriptor exposes its browse RPCs.</summary>
[Fact]
public void GalaxyRepositoryDescriptor_ContainsBrowseServiceMethods()
{
var service = Assert.Single(
GalaxyRepositoryReflection.Descriptor.Services,
descriptor => descriptor.Name == "GalaxyRepository");
Assert.Contains(service.Methods, method => method.Name == "TestConnection");
Assert.Contains(service.Methods, method => method.Name == "GetLastDeployTime");
Assert.Contains(service.Methods, method => method.Name == "DiscoverHierarchy");
Assert.Contains(service.Methods, method => method.Name == "WatchDeployEvents");
}
/// <summary>
/// Verifies that a DiscoverHierarchyRequest round-trips through every
/// <c>root</c> oneof arm and its proto wrapper-typed <c>max_depth</c> field.
/// </summary>
[Theory]
[InlineData(0)]
[InlineData(1)]
[InlineData(2)]
public void DiscoverHierarchyRequest_RoundTripsRootOneofAndWrapperFields(int rootArm)
{
var original = new DiscoverHierarchyRequest
{
PageSize = 100,
PageToken = "page-2",
MaxDepth = 5,
CategoryIds = { 3, 9 },
TemplateChainContains = { "Analog", "Pump" },
TagNameGlob = "Tank*",
IncludeAttributes = true,
AlarmBearingOnly = true,
HistorizedOnly = false,
};
switch (rootArm)
{
case 0:
original.RootGobjectId = 4711;
break;
case 1:
original.RootTagName = "Tank01";
break;
default:
original.RootContainedPath = "Area1.Tank01";
break;
}
var parsed = DiscoverHierarchyRequest.Parser.ParseFrom(original.ToByteArray());
Assert.Equal(original, parsed);
Assert.Equal(original.RootCase, parsed.RootCase);
Assert.NotEqual(DiscoverHierarchyRequest.RootOneofCase.None, parsed.RootCase);
Assert.NotNull(parsed.MaxDepth);
Assert.Equal(5, parsed.MaxDepth!.Value);
Assert.True(parsed.HasIncludeAttributes);
Assert.True(parsed.IncludeAttributes);
}
/// <summary>
/// Verifies that a DiscoverHierarchyReply round-trips with nested
/// GalaxyObject and GalaxyAttribute graphs.
/// </summary>
[Fact]
public void DiscoverHierarchyReply_RoundTripsObjectAndAttributeGraph()
{
var original = new DiscoverHierarchyReply
{
NextPageToken = "page-3",
TotalObjectCount = 2,
Objects =
{
new GalaxyObject
{
GobjectId = 4711,
TagName = "Tank01",
ContainedName = "Tank01",
BrowseName = "Tank 01",
ParentGobjectId = 12,
IsArea = false,
CategoryId = 3,
HostedByGobjectId = 8,
TemplateChain = { "$AnalogDevice", "$Tank" },
Attributes =
{
new GalaxyAttribute
{
AttributeName = "Level",
FullTagReference = "Galaxy!Tank01.Level",
MxDataType = 3,
DataTypeName = "Float",
IsArray = false,
ArrayDimension = 0,
ArrayDimensionPresent = false,
MxAttributeCategory = 1,
SecurityClassification = 0,
IsHistorized = true,
IsAlarm = true,
},
},
},
new GalaxyObject
{
GobjectId = 12,
TagName = "Area1",
IsArea = true,
},
},
};
var parsed = DiscoverHierarchyReply.Parser.ParseFrom(original.ToByteArray());
Assert.Equal(original, parsed);
Assert.Equal(2, parsed.Objects.Count);
Assert.Single(parsed.Objects[0].Attributes);
Assert.True(parsed.Objects[0].Attributes[0].IsAlarm);
}
/// <summary>Verifies that a DeployEvent round-trips with its timestamp and counters.</summary>
[Fact]
public void DeployEvent_RoundTripsTimestampAndCounters()
{
var observed = Timestamp.FromDateTime(new DateTime(2026, 5, 18, 8, 30, 0, DateTimeKind.Utc));
var deploy = Timestamp.FromDateTime(new DateTime(2026, 5, 18, 8, 0, 0, DateTimeKind.Utc));
var original = new DeployEvent
{
Sequence = 17,
ObservedAt = observed,
TimeOfLastDeploy = deploy,
TimeOfLastDeployPresent = true,
ObjectCount = 240,
AttributeCount = 3600,
};
var parsed = DeployEvent.Parser.ParseFrom(original.ToByteArray());
Assert.Equal(original, parsed);
Assert.True(parsed.TimeOfLastDeployPresent);
Assert.Equal(deploy, parsed.TimeOfLastDeploy);
}
/// <summary>Verifies that GetLastDeployTimeReply and TestConnectionReply round-trip.</summary>
[Fact]
public void GalaxyConnectionReplies_RoundTrip()
{
var deploy = Timestamp.FromDateTime(new DateTime(2026, 5, 18, 8, 0, 0, DateTimeKind.Utc));
var lastDeploy = new GetLastDeployTimeReply
{
Present = true,
TimeOfLastDeploy = deploy,
};
var testConnection = new TestConnectionReply { Ok = true };
Assert.Equal(lastDeploy, GetLastDeployTimeReply.Parser.ParseFrom(lastDeploy.ToByteArray()));
Assert.Equal(testConnection, TestConnectionReply.Parser.ParseFrom(testConnection.ToByteArray()));
}
} }
@@ -6,6 +6,7 @@ using MxGateway.Server.Dashboard;
using MxGateway.Server.Galaxy; using MxGateway.Server.Galaxy;
using MxGateway.Server.Grpc; using MxGateway.Server.Grpc;
using MxGateway.Server.Security.Authorization; using MxGateway.Server.Security.Authorization;
using MxGateway.Tests.TestSupport;
namespace MxGateway.Tests.Galaxy; namespace MxGateway.Tests.Galaxy;
@@ -302,51 +303,4 @@ public sealed class GalaxyFilterInputSafetyTests
public Task WaitForFirstLoadAsync(CancellationToken cancellationToken) => Task.CompletedTask; public Task WaitForFirstLoadAsync(CancellationToken cancellationToken) => Task.CompletedTask;
} }
private sealed class TestServerCallContext(CancellationToken cancellationToken = default) : ServerCallContext
{
private readonly Metadata requestHeaders = [];
private readonly Metadata responseTrailers = [];
private readonly Dictionary<object, object> userState = [];
private Status status;
private WriteOptions? writeOptions;
protected override string MethodCore => "/galaxy_repository.v1.GalaxyRepository/DiscoverHierarchy";
protected override string HostCore => "localhost";
protected override string PeerCore => "ipv4:127.0.0.1:5000";
protected override DateTime DeadlineCore => DateTime.UtcNow.AddMinutes(1);
protected override Metadata RequestHeadersCore => requestHeaders;
protected override CancellationToken CancellationTokenCore => cancellationToken;
protected override Metadata ResponseTrailersCore => responseTrailers;
protected override Status StatusCore
{
get => status;
set => status = value;
}
protected override WriteOptions? WriteOptionsCore
{
get => writeOptions;
set => writeOptions = value;
}
protected override AuthContext AuthContextCore { get; } = new(
string.Empty,
new Dictionary<string, List<AuthProperty>>(StringComparer.Ordinal));
protected override IDictionary<object, object> UserStateCore => userState;
protected override Task WriteResponseHeadersAsyncCore(Metadata responseHeaders) => Task.CompletedTask;
protected override ContextPropagationToken CreatePropagationTokenCore(ContextPropagationOptions? options)
{
throw new NotSupportedException();
}
}
} }
@@ -22,13 +22,23 @@ public sealed class GalaxyHierarchyRefreshServiceTests
using CancellationTokenSource cts = new(); using CancellationTokenSource cts = new();
await service.StartAsync(cts.Token); await service.StartAsync(cts.Token);
// Wait until the first RefreshAsync has actually been attempted (and
// thrown) before cancelling, so cancellation cannot race ahead of the
// first-load path under test — this is what made the test flaky under
// parallel load.
await cache.FirstRefreshAttempted.WaitAsync(TimeSpan.FromSeconds(10));
await cts.CancelAsync(); await cts.CancelAsync();
// The background loop must have stopped cleanly: ExecuteTask completes // The background loop must have stopped cleanly: ExecuteTask reaches a
// (RanToCompletion or Canceled) rather than faulting on the first refresh. // terminal state that is not Faulted (RanToCompletion or Canceled)
// rather than faulting on the first refresh. WhenAny is used so a
// Canceled task does not rethrow before the IsFaulted assertion.
Task? executeTask = service.ExecuteTask; Task? executeTask = service.ExecuteTask;
Assert.NotNull(executeTask); Assert.NotNull(executeTask);
await executeTask; Task completed = await Task.WhenAny(executeTask, Task.Delay(TimeSpan.FromSeconds(10)));
Assert.Same(executeTask, completed);
Assert.False(executeTask.IsFaulted); Assert.False(executeTask.IsFaulted);
Assert.Equal(1, cache.RefreshCallCount); Assert.Equal(1, cache.RefreshCallCount);
@@ -49,13 +59,20 @@ public sealed class GalaxyHierarchyRefreshServiceTests
private sealed class ThrowingCache(Exception toThrow) : IGalaxyHierarchyCache private sealed class ThrowingCache(Exception toThrow) : IGalaxyHierarchyCache
{ {
private readonly TaskCompletionSource firstRefreshAttempted =
new(TaskCreationOptions.RunContinuationsAsynchronously);
public int RefreshCallCount { get; private set; } public int RefreshCallCount { get; private set; }
/// <summary>Completes once <see cref="RefreshAsync"/> has been invoked at least once.</summary>
public Task FirstRefreshAttempted => firstRefreshAttempted.Task;
public GalaxyHierarchyCacheEntry Current => GalaxyHierarchyCacheEntry.Empty; public GalaxyHierarchyCacheEntry Current => GalaxyHierarchyCacheEntry.Empty;
public Task RefreshAsync(CancellationToken cancellationToken) public Task RefreshAsync(CancellationToken cancellationToken)
{ {
RefreshCallCount++; RefreshCallCount++;
firstRefreshAttempted.TrySetResult();
throw toThrow; throw toThrow;
} }
@@ -35,6 +35,36 @@ public sealed class DashboardAuthorizationHandlerTests
Assert.True(context.HasSucceeded); Assert.True(context.HasSucceeded);
} }
/// <summary>
/// Verifies that the anonymous-localhost bypass is denied when <c>AllowAnonymousLocalhost</c>
/// is off, even on a loopback connection — the misconfiguration must not expose the dashboard.
/// </summary>
[Fact]
public async Task HandleAsync_AnonymousLocalhostDisallowed_DoesNotSucceed()
{
AuthorizationHandlerContext context = await AuthorizeAsync(
new ClaimsPrincipal(new ClaimsIdentity()),
IPAddress.Loopback,
allowAnonymousLocalhost: false);
Assert.False(context.HasSucceeded);
}
/// <summary>
/// Verifies that the anonymous-localhost bypass stays scoped to loopback: an anonymous
/// request from a non-loopback address is denied even when <c>AllowAnonymousLocalhost</c> is on.
/// </summary>
[Fact]
public async Task HandleAsync_AnonymousLocalhostAllowedFromRemoteAddress_DoesNotSucceed()
{
AuthorizationHandlerContext context = await AuthorizeAsync(
new ClaimsPrincipal(new ClaimsIdentity()),
IPAddress.Parse("10.0.0.5"),
allowAnonymousLocalhost: true);
Assert.False(context.HasSucceeded);
}
/// <summary>Verifies that authenticated users without admin scope fail authorization.</summary> /// <summary>Verifies that authenticated users without admin scope fail authorization.</summary>
[Fact] [Fact]
public async Task HandleAsync_AuthenticatedWithoutAdminScope_DoesNotSucceed() public async Task HandleAsync_AuthenticatedWithoutAdminScope_DoesNotSucceed()
@@ -147,6 +147,8 @@ public sealed class GatewayApplicationTests
string value, string value,
string expectedFailure) string expectedFailure)
{ {
// Bind an ephemeral port (:0) — xUnit runs test collections in parallel, so any
// WebApplication-building test must avoid a fixed port to prevent a bind collision.
await using WebApplication app = GatewayApplication.Build( await using WebApplication app = GatewayApplication.Build(
[$"--{key}={value}", "--urls=http://127.0.0.1:0"]); [$"--{key}={value}", "--urls=http://127.0.0.1:0"]);
@@ -1,6 +1,5 @@
using System.Collections.Concurrent; using System.Collections.Concurrent;
using Google.Protobuf.WellKnownTypes; using Google.Protobuf.WellKnownTypes;
using Grpc.Core;
using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Logging.Abstractions;
using Microsoft.Extensions.Options; using Microsoft.Extensions.Options;
using MxGateway.Contracts; using MxGateway.Contracts;
@@ -13,6 +12,7 @@ using MxGateway.Server.Security.Authorization;
using MxGateway.Server.Sessions; using MxGateway.Server.Sessions;
using MxGateway.Server.Workers; using MxGateway.Server.Workers;
using MxGateway.Tests.Gateway.Workers.Fakes; using MxGateway.Tests.Gateway.Workers.Fakes;
using MxGateway.Tests.TestSupport;
namespace MxGateway.Tests.Gateway; namespace MxGateway.Tests.Gateway;
@@ -405,159 +405,4 @@ public sealed class GatewayEndToEndFakeWorkerSmokeTests
} }
} }
private sealed class RecordingServerStreamWriter<T> : IServerStreamWriter<T>
{
private readonly object _syncRoot = new();
private readonly TaskCompletionSource<T> _firstMessage = new(TaskCreationOptions.RunContinuationsAsynchronously);
private readonly List<T> _messages = [];
/// <summary>
/// Gets the recorded messages written to this stream.
/// </summary>
public IReadOnlyList<T> Messages
{
get
{
lock (_syncRoot)
{
return _messages.ToArray();
}
}
}
/// <summary>
/// Gets or sets options for writing messages to the stream.
/// </summary>
public WriteOptions? WriteOptions { get; set; }
/// <summary>
/// Writes a message to the stream asynchronously.
/// </summary>
/// <param name="message">The message to write.</param>
/// <returns>Completed task.</returns>
public Task WriteAsync(T message)
{
lock (_syncRoot)
{
_messages.Add(message);
}
_firstMessage.TrySetResult(message);
return Task.CompletedTask;
}
/// <summary>
/// Waits for the first message to be written within the specified timeout.
/// </summary>
/// <param name="timeout">Maximum time to wait for the first message.</param>
/// <returns>The first message written to this stream.</returns>
public async Task<T> WaitForFirstMessageAsync(TimeSpan timeout)
{
return await _firstMessage.Task.WaitAsync(timeout).ConfigureAwait(false);
}
}
private sealed class TestServerCallContext(CancellationToken cancellationToken = default) : ServerCallContext
{
private readonly Metadata _requestHeaders = [];
private readonly Metadata _responseTrailers = [];
private readonly Dictionary<object, object> _userState = [];
private Status _status;
private WriteOptions? _writeOptions;
/// <inheritdoc />
protected override string MethodCore => "/mxaccess_gateway.v1.MxAccessGateway/Test";
/// <inheritdoc />
protected override string HostCore => "localhost";
/// <inheritdoc />
protected override string PeerCore => "ipv4:127.0.0.1:5000";
/// <inheritdoc />
protected override DateTime DeadlineCore => DateTime.UtcNow.AddMinutes(1);
/// <inheritdoc />
protected override Metadata RequestHeadersCore => _requestHeaders;
/// <inheritdoc />
protected override CancellationToken CancellationTokenCore => cancellationToken;
/// <inheritdoc />
protected override Metadata ResponseTrailersCore => _responseTrailers;
/// <inheritdoc />
protected override Status StatusCore
{
get => _status;
set => _status = value;
}
/// <inheritdoc />
protected override WriteOptions? WriteOptionsCore
{
get => _writeOptions;
set => _writeOptions = value;
}
/// <inheritdoc />
protected override AuthContext AuthContextCore { get; } = new(
string.Empty,
new Dictionary<string, List<AuthProperty>>(StringComparer.Ordinal));
/// <inheritdoc />
protected override IDictionary<object, object> UserStateCore => _userState;
/// <summary>
/// Writes response headers asynchronously.
/// </summary>
/// <param name="responseHeaders">Headers to write.</param>
/// <returns>Completed task.</returns>
/// <inheritdoc />
protected override Task WriteResponseHeadersAsyncCore(Metadata responseHeaders)
{
return Task.CompletedTask;
}
/// <summary>
/// Creates a context propagation token with the specified options.
/// </summary>
/// <param name="options">Propagation options.</param>
/// <returns>Propagation token.</returns>
/// <inheritdoc />
protected override ContextPropagationToken CreatePropagationTokenCore(
ContextPropagationOptions? options)
{
throw new NotSupportedException();
}
}
private sealed class AllowAllConstraintEnforcer : IConstraintEnforcer
{
public Task<ConstraintFailure?> CheckReadTagAsync(
ApiKeyIdentity? identity,
string tagAddress,
CancellationToken cancellationToken) => Task.FromResult<ConstraintFailure?>(null);
public Task<ConstraintFailure?> CheckReadHandleAsync(
ApiKeyIdentity? identity,
GatewaySession session,
int serverHandle,
int itemHandle,
CancellationToken cancellationToken) => Task.FromResult<ConstraintFailure?>(null);
public Task<ConstraintFailure?> CheckWriteHandleAsync(
ApiKeyIdentity? identity,
GatewaySession session,
int serverHandle,
int itemHandle,
CancellationToken cancellationToken) => Task.FromResult<ConstraintFailure?>(null);
public Task RecordDenialAsync(
ApiKeyIdentity? identity,
string commandKind,
string target,
ConstraintFailure failure,
CancellationToken cancellationToken) => Task.CompletedTask;
}
} }
@@ -5,6 +5,7 @@ using MxGateway.Server.Dashboard;
using MxGateway.Server.Galaxy; using MxGateway.Server.Galaxy;
using MxGateway.Server.Grpc; using MxGateway.Server.Grpc;
using MxGateway.Server.Security.Authorization; using MxGateway.Server.Security.Authorization;
using MxGateway.Tests.TestSupport;
namespace MxGateway.Tests.Gateway.Grpc; namespace MxGateway.Tests.Gateway.Grpc;
@@ -321,51 +322,4 @@ public sealed class GalaxyRepositoryGrpcServiceTests
public Task WaitForFirstLoadAsync(CancellationToken cancellationToken) => Task.CompletedTask; public Task WaitForFirstLoadAsync(CancellationToken cancellationToken) => Task.CompletedTask;
} }
private sealed class TestServerCallContext(CancellationToken cancellationToken = default) : ServerCallContext
{
private readonly Metadata requestHeaders = [];
private readonly Metadata responseTrailers = [];
private readonly Dictionary<object, object> userState = [];
private Status status;
private WriteOptions? writeOptions;
protected override string MethodCore => "/galaxy_repository.v1.GalaxyRepository/DiscoverHierarchy";
protected override string HostCore => "localhost";
protected override string PeerCore => "ipv4:127.0.0.1:5000";
protected override DateTime DeadlineCore => DateTime.UtcNow.AddMinutes(1);
protected override Metadata RequestHeadersCore => requestHeaders;
protected override CancellationToken CancellationTokenCore => cancellationToken;
protected override Metadata ResponseTrailersCore => responseTrailers;
protected override Status StatusCore
{
get => status;
set => status = value;
}
protected override WriteOptions? WriteOptionsCore
{
get => writeOptions;
set => writeOptions = value;
}
protected override AuthContext AuthContextCore { get; } = new(
string.Empty,
new Dictionary<string, List<AuthProperty>>(StringComparer.Ordinal));
protected override IDictionary<object, object> UserStateCore => userState;
protected override Task WriteResponseHeadersAsyncCore(Metadata responseHeaders) => Task.CompletedTask;
protected override ContextPropagationToken CreatePropagationTokenCore(ContextPropagationOptions? options)
{
throw new NotSupportedException();
}
}
} }
@@ -11,6 +11,7 @@ using MxGateway.Server.Security.Authentication;
using MxGateway.Server.Security.Authorization; using MxGateway.Server.Security.Authorization;
using MxGateway.Server.Sessions; using MxGateway.Server.Sessions;
using MxGateway.Server.Workers; using MxGateway.Server.Workers;
using MxGateway.Tests.TestSupport;
namespace MxGateway.Tests.Gateway.Grpc; namespace MxGateway.Tests.Gateway.Grpc;
@@ -132,7 +133,7 @@ public sealed class MxAccessGatewayServiceTests
SessionId = "session-missing", SessionId = "session-missing",
AlarmFilterPrefix = "Tank01.", AlarmFilterPrefix = "Tank01.",
}, },
new RecordingStreamWriter<ActiveAlarmSnapshot>(), new RecordingServerStreamWriter<ActiveAlarmSnapshot>(),
new TestServerCallContext())); new TestServerCallContext()));
Assert.Equal(StatusCode.NotFound, exception.StatusCode); Assert.Equal(StatusCode.NotFound, exception.StatusCode);
@@ -225,7 +226,7 @@ public sealed class MxAccessGatewayServiceTests
sessionManager.Events.Add(CreateWorkerEvent("session-1", workerSequence: 1)); sessionManager.Events.Add(CreateWorkerEvent("session-1", workerSequence: 1));
sessionManager.Events.Add(CreateWorkerEvent("session-1", workerSequence: 2)); sessionManager.Events.Add(CreateWorkerEvent("session-1", workerSequence: 2));
MxAccessGatewayService service = CreateService(sessionManager); MxAccessGatewayService service = CreateService(sessionManager);
TestServerStreamWriter<MxEvent> writer = new(); RecordingServerStreamWriter<MxEvent> writer = new();
await service.StreamEvents( await service.StreamEvents(
new StreamEventsRequest new StreamEventsRequest
@@ -276,7 +277,7 @@ public sealed class MxAccessGatewayServiceTests
FakeSessionManager sessionManager = new(); FakeSessionManager sessionManager = new();
sessionManager.Events.Add(CreateWorkerEvent("session-1", workerSequence: 2)); sessionManager.Events.Add(CreateWorkerEvent("session-1", workerSequence: 2));
MxAccessGatewayService service = CreateService(sessionManager, metrics: metrics); MxAccessGatewayService service = CreateService(sessionManager, metrics: metrics);
TestServerStreamWriter<MxEvent> writer = new(); RecordingServerStreamWriter<MxEvent> writer = new();
await service.StreamEvents( await service.StreamEvents(
new StreamEventsRequest { SessionId = "session-1" }, new StreamEventsRequest { SessionId = "session-1" },
@@ -375,7 +376,7 @@ public sealed class MxAccessGatewayServiceTests
RpcException exception = await Assert.ThrowsAsync<RpcException>( RpcException exception = await Assert.ThrowsAsync<RpcException>(
async () => await service.QueryActiveAlarms( async () => await service.QueryActiveAlarms(
new QueryActiveAlarmsRequest(), new QueryActiveAlarmsRequest(),
new RecordingStreamWriter<ActiveAlarmSnapshot>(), new RecordingServerStreamWriter<ActiveAlarmSnapshot>(),
new TestServerCallContext())); new TestServerCallContext()));
Assert.Equal(StatusCode.InvalidArgument, exception.StatusCode); Assert.Equal(StatusCode.InvalidArgument, exception.StatusCode);
@@ -386,7 +387,7 @@ public sealed class MxAccessGatewayServiceTests
public async Task QueryActiveAlarms_WithValidRequest_StreamsZeroSnapshots() public async Task QueryActiveAlarms_WithValidRequest_StreamsZeroSnapshots()
{ {
MxAccessGatewayService service = CreateService(new FakeSessionManager()); MxAccessGatewayService service = CreateService(new FakeSessionManager());
RecordingStreamWriter<ActiveAlarmSnapshot> sink = new(); RecordingServerStreamWriter<ActiveAlarmSnapshot> sink = new();
await service.QueryActiveAlarms( await service.QueryActiveAlarms(
new QueryActiveAlarmsRequest new QueryActiveAlarmsRequest
@@ -397,7 +398,7 @@ public sealed class MxAccessGatewayServiceTests
sink, sink,
new TestServerCallContext()); new TestServerCallContext());
Assert.Empty(sink.Items); Assert.Empty(sink.Messages);
} }
/// <summary>Verifies OpenSession advertises the alarm RPC capability strings.</summary> /// <summary>Verifies OpenSession advertises the alarm RPC capability strings.</summary>
@@ -664,35 +665,6 @@ public sealed class MxAccessGatewayServiceTests
} }
} }
private sealed class AllowAllConstraintEnforcer : IConstraintEnforcer
{
public Task<ConstraintFailure?> CheckReadTagAsync(
ApiKeyIdentity? identity,
string tagAddress,
CancellationToken cancellationToken) => Task.FromResult<ConstraintFailure?>(null);
public Task<ConstraintFailure?> CheckReadHandleAsync(
ApiKeyIdentity? identity,
GatewaySession session,
int serverHandle,
int itemHandle,
CancellationToken cancellationToken) => Task.FromResult<ConstraintFailure?>(null);
public Task<ConstraintFailure?> CheckWriteHandleAsync(
ApiKeyIdentity? identity,
GatewaySession session,
int serverHandle,
int itemHandle,
CancellationToken cancellationToken) => Task.FromResult<ConstraintFailure?>(null);
public Task RecordDenialAsync(
ApiKeyIdentity? identity,
string commandKind,
string target,
ConstraintFailure failure,
CancellationToken cancellationToken) => Task.CompletedTask;
}
private sealed class FakeWorkerClient(int processId) : IWorkerClient private sealed class FakeWorkerClient(int processId) : IWorkerClient
{ {
/// <inheritdoc /> /// <inheritdoc />
@@ -750,97 +722,4 @@ public sealed class MxAccessGatewayServiceTests
} }
} }
private sealed class TestServerStreamWriter<T> : IServerStreamWriter<T>
{
/// <inheritdoc />
public List<T> Messages { get; } = [];
/// <inheritdoc />
public WriteOptions? WriteOptions { get; set; }
/// <inheritdoc />
public Task WriteAsync(T message)
{
Messages.Add(message);
return Task.CompletedTask;
}
}
private sealed class RecordingStreamWriter<T> : IServerStreamWriter<T>
{
public List<T> Items { get; } = new();
public WriteOptions? WriteOptions { get; set; }
public Task WriteAsync(T message)
{
Items.Add(message);
return Task.CompletedTask;
}
}
private sealed class TestServerCallContext(CancellationToken cancellationToken = default) : ServerCallContext
{
private readonly Metadata requestHeaders = [];
private readonly Metadata responseTrailers = [];
private readonly Dictionary<object, object> userState = [];
private Status status;
private WriteOptions? writeOptions;
/// <inheritdoc />
protected override string MethodCore => "/mxaccess_gateway.v1.MxAccessGateway/Test";
/// <inheritdoc />
protected override string HostCore => "localhost";
/// <inheritdoc />
protected override string PeerCore => "ipv4:127.0.0.1:5000";
/// <inheritdoc />
protected override DateTime DeadlineCore => DateTime.UtcNow.AddMinutes(1);
/// <inheritdoc />
protected override Metadata RequestHeadersCore => requestHeaders;
/// <inheritdoc />
protected override CancellationToken CancellationTokenCore => cancellationToken;
/// <inheritdoc />
protected override Metadata ResponseTrailersCore => responseTrailers;
/// <inheritdoc />
protected override Status StatusCore
{
get => status;
set => status = value;
}
/// <inheritdoc />
protected override WriteOptions? WriteOptionsCore
{
get => writeOptions;
set => writeOptions = value;
}
/// <inheritdoc />
protected override AuthContext AuthContextCore { get; } = new(
string.Empty,
new Dictionary<string, List<AuthProperty>>(StringComparer.Ordinal));
/// <inheritdoc />
protected override IDictionary<object, object> UserStateCore => userState;
/// <inheritdoc />
protected override Task WriteResponseHeadersAsyncCore(Metadata responseHeaders)
{
return Task.CompletedTask;
}
/// <inheritdoc />
protected override ContextPropagationToken CreatePropagationTokenCore(
ContextPropagationOptions? options)
{
throw new NotSupportedException();
}
}
} }
@@ -1,5 +1,3 @@
using System.Threading;
using System.Threading.Tasks;
using MxGateway.Contracts.Proto; using MxGateway.Contracts.Proto;
using MxGateway.Server.Sessions; using MxGateway.Server.Sessions;
@@ -15,7 +13,7 @@ namespace MxGateway.Tests.Gateway.Sessions;
public sealed class NotWiredAlarmRpcDispatcherTests public sealed class NotWiredAlarmRpcDispatcherTests
{ {
[Fact] [Fact]
public async Task AcknowledgeAsync_returns_ok_with_worker_pending_diagnostic() public async Task AcknowledgeAsync_WhenNotWired_ReturnsOkWithWorkerPendingDiagnostic()
{ {
IAlarmRpcDispatcher dispatcher = new NotWiredAlarmRpcDispatcher(); IAlarmRpcDispatcher dispatcher = new NotWiredAlarmRpcDispatcher();
@@ -33,11 +31,11 @@ public sealed class NotWiredAlarmRpcDispatcherTests
Assert.Equal(ProtocolStatusCode.Ok, reply.ProtocolStatus.Code); Assert.Equal(ProtocolStatusCode.Ok, reply.ProtocolStatus.Code);
Assert.Equal("session-1", reply.SessionId); Assert.Equal("session-1", reply.SessionId);
Assert.Equal("corr-1", reply.CorrelationId); Assert.Equal("corr-1", reply.CorrelationId);
Assert.Contains("worker", reply.DiagnosticMessage, System.StringComparison.OrdinalIgnoreCase); Assert.Contains("worker", reply.DiagnosticMessage, StringComparison.OrdinalIgnoreCase);
} }
[Fact] [Fact]
public async Task QueryActiveAlarmsAsync_yields_no_snapshots() public async Task QueryActiveAlarmsAsync_WhenNotWired_YieldsNoSnapshots()
{ {
IAlarmRpcDispatcher dispatcher = new NotWiredAlarmRpcDispatcher(); IAlarmRpcDispatcher dispatcher = new NotWiredAlarmRpcDispatcher();
@@ -33,7 +33,7 @@ public sealed class SessionManagerTests
Assert.Equal(1, metrics.GetSnapshot().SessionsOpened); Assert.Equal(1, metrics.GetSnapshot().SessionsOpened);
} }
/// <summary>Verifies that opening a session generates a correlation ID from the client name and session ID.</summary> /// <summary>Verifies that opening a session sets the initial lease expiry from the configured default lease.</summary>
[Fact] [Fact]
public async Task OpenSessionAsync_SetsInitialDefaultLease() public async Task OpenSessionAsync_SetsInitialDefaultLease()
{ {
@@ -96,7 +96,7 @@ public sealed class SessionManagerTests
Assert.Equal(MxCommandKind.Ping, reply.Reply.Kind); Assert.Equal(MxCommandKind.Ping, reply.Reply.Kind);
} }
/// <summary>Verifies that bulk subscribe forwards the command and returns subscription results.</summary> /// <summary>Verifies that invoking a command on a ready session refreshes its lease expiry.</summary>
[Fact] [Fact]
public async Task InvokeAsync_WhenSessionReady_RefreshesLease() public async Task InvokeAsync_WhenSessionReady_RefreshesLease()
{ {
@@ -362,7 +362,7 @@ public sealed class SessionManagerTests
Assert.Equal(0, activeClient.ShutdownCount); Assert.Equal(0, activeClient.ShutdownCount);
} }
/// <summary>Verifies that shutdown closes all registered sessions.</summary> /// <summary>Verifies that an expired-lease sweep leaves a session with an active event subscriber open.</summary>
[Fact] [Fact]
public async Task CloseExpiredLeasesAsync_DoesNotCloseActiveEventSubscriber() public async Task CloseExpiredLeasesAsync_DoesNotCloseActiveEventSubscriber()
{ {
@@ -10,15 +10,29 @@ using MxGateway.Tests.Gateway.Workers.Fakes;
namespace MxGateway.Tests.Gateway.Sessions; namespace MxGateway.Tests.Gateway.Sessions;
public sealed class SessionWorkerClientFactoryFakeWorkerTests public sealed class SessionWorkerClientFactoryFakeWorkerTests : IAsyncDisposable
{ {
private static readonly TimeSpan TestTimeout = TimeSpan.FromSeconds(5); private static readonly TimeSpan TestTimeout = TimeSpan.FromSeconds(5);
private readonly List<IWorkerTaskLauncher> _launchers = [];
/// <summary>
/// Awaits every scripted worker task so an unhandled exception fails the owning test
/// instead of surfacing later as an unobserved <see cref="TaskScheduler.UnobservedTaskException"/>.
/// </summary>
public async ValueTask DisposeAsync()
{
foreach (IWorkerTaskLauncher launcher in _launchers)
{
await launcher.ObserveWorkerTaskAsync(TestTimeout);
}
}
/// <summary>Verifies that the factory creates a ready worker client with a scripted fake worker process.</summary> /// <summary>Verifies that the factory creates a ready worker client with a scripted fake worker process.</summary>
[Fact] [Fact]
public async Task CreateAsync_WithScriptedFakeWorker_ReturnsReadyClient() public async Task CreateAsync_WithScriptedFakeWorker_ReturnsReadyClient()
{ {
ScriptedFakeWorkerProcessLauncher launcher = new(); ScriptedFakeWorkerProcessLauncher launcher = Track(new ScriptedFakeWorkerProcessLauncher());
using GatewayMetrics metrics = new(); using GatewayMetrics metrics = new();
SessionWorkerClientFactory factory = new( SessionWorkerClientFactory factory = new(
launcher, launcher,
@@ -51,7 +65,7 @@ public sealed class SessionWorkerClientFactoryFakeWorkerTests
[Fact] [Fact]
public async Task CreateAsync_WhenFakeWorkerStartupFails_ThrowsWorkerClientException() public async Task CreateAsync_WhenFakeWorkerStartupFails_ThrowsWorkerClientException()
{ {
FailingStartupWorkerProcessLauncher launcher = new(); FailingStartupWorkerProcessLauncher launcher = Track(new FailingStartupWorkerProcessLauncher());
using GatewayMetrics metrics = new(); using GatewayMetrics metrics = new();
SessionWorkerClientFactory factory = new( SessionWorkerClientFactory factory = new(
launcher, launcher,
@@ -71,7 +85,7 @@ public sealed class SessionWorkerClientFactoryFakeWorkerTests
[Fact] [Fact]
public async Task CreateAsync_WhenFakeWorkerNeverSendsReady_TimesOutAndKillsWorker() public async Task CreateAsync_WhenFakeWorkerNeverSendsReady_TimesOutAndKillsWorker()
{ {
NeverReadyWorkerProcessLauncher launcher = new(); NeverReadyWorkerProcessLauncher launcher = Track(new NeverReadyWorkerProcessLauncher());
using GatewayMetrics metrics = new(); using GatewayMetrics metrics = new();
SessionWorkerClientFactory factory = new( SessionWorkerClientFactory factory = new(
launcher, launcher,
@@ -134,8 +148,50 @@ public sealed class SessionWorkerClientFactoryFakeWorkerTests
}; };
} }
private T Track<T>(T launcher)
where T : IWorkerTaskLauncher
{
_launchers.Add(launcher);
return launcher;
}
/// <summary>
/// A fake worker launcher that runs a scripted worker on a background task and exposes
/// that task so the owning test observes it rather than leaking an unobserved fault.
/// </summary>
private interface IWorkerTaskLauncher : IWorkerProcessLauncher
{
/// <summary>
/// Awaits the scripted worker task within the timeout, swallowing only the pipe
/// teardown faults expected when the worker client kills or disposes the worker.
/// </summary>
/// <param name="timeout">Maximum time to wait for the worker task.</param>
Task ObserveWorkerTaskAsync(TimeSpan timeout);
}
/// <summary>
/// Awaits a scripted worker task, treating cancellation and pipe-disconnect I/O faults as
/// the expected outcome of the worker client tearing the worker down, and rethrowing anything else.
/// </summary>
private static async Task ObserveWorkerTaskAsync(Task workerTask, TimeSpan timeout)
{
try
{
await workerTask.WaitAsync(timeout).ConfigureAwait(false);
}
catch (OperationCanceledException)
{
// Expected: the worker client cancelled the scripted worker during teardown.
}
catch (IOException)
{
// Expected: the gateway pipe was closed when the worker client disposed.
}
}
/// <summary>Fake worker launcher that connects a scripted fake worker harness.</summary> /// <summary>Fake worker launcher that connects a scripted fake worker harness.</summary>
private sealed class ScriptedFakeWorkerProcessLauncher : IWorkerProcessLauncher private sealed class ScriptedFakeWorkerProcessLauncher : IWorkerTaskLauncher
{ {
/// <summary>The fake process ID used by the scripted launcher.</summary> /// <summary>The fake process ID used by the scripted launcher.</summary>
public const int ProcessId = 2468; public const int ProcessId = 2468;
@@ -144,16 +200,23 @@ public sealed class SessionWorkerClientFactoryFakeWorkerTests
/// <summary>Gets the connected fake worker harness.</summary> /// <summary>Gets the connected fake worker harness.</summary>
public FakeWorkerHarness? Harness { get; private set; } public FakeWorkerHarness? Harness { get; private set; }
/// <summary>Gets the scripted worker task.</summary>
public Task WorkerTask { get; private set; } = Task.CompletedTask;
/// <inheritdoc /> /// <inheritdoc />
public Task<WorkerProcessHandle> LaunchAsync( public Task<WorkerProcessHandle> LaunchAsync(
WorkerProcessLaunchRequest request, WorkerProcessLaunchRequest request,
CancellationToken cancellationToken = default) CancellationToken cancellationToken = default)
{ {
_ = RunWorkerAsync(request, cancellationToken); WorkerTask = RunWorkerAsync(request, cancellationToken);
return Task.FromResult(CreateHandle(_process)); return Task.FromResult(CreateHandle(_process));
} }
/// <inheritdoc />
public Task ObserveWorkerTaskAsync(TimeSpan timeout) =>
SessionWorkerClientFactoryFakeWorkerTests.ObserveWorkerTaskAsync(WorkerTask, timeout);
private async Task RunWorkerAsync( private async Task RunWorkerAsync(
WorkerProcessLaunchRequest request, WorkerProcessLaunchRequest request,
CancellationToken cancellationToken) CancellationToken cancellationToken)
@@ -169,21 +232,28 @@ public sealed class SessionWorkerClientFactoryFakeWorkerTests
} }
/// <summary>Fake worker launcher that fails during startup with protocol version mismatch.</summary> /// <summary>Fake worker launcher that fails during startup with protocol version mismatch.</summary>
private sealed class FailingStartupWorkerProcessLauncher : IWorkerProcessLauncher private sealed class FailingStartupWorkerProcessLauncher : IWorkerTaskLauncher
{ {
/// <summary>Gets the fake worker process.</summary> /// <summary>Gets the fake worker process.</summary>
public FakeWorkerProcess Process { get; } = new(processId: 3579); public FakeWorkerProcess Process { get; } = new(processId: 3579);
/// <summary>Gets the scripted worker task.</summary>
public Task WorkerTask { get; private set; } = Task.CompletedTask;
/// <inheritdoc /> /// <inheritdoc />
public Task<WorkerProcessHandle> LaunchAsync( public Task<WorkerProcessHandle> LaunchAsync(
WorkerProcessLaunchRequest request, WorkerProcessLaunchRequest request,
CancellationToken cancellationToken = default) CancellationToken cancellationToken = default)
{ {
_ = RunWorkerAsync(request, cancellationToken); WorkerTask = RunWorkerAsync(request, cancellationToken);
return Task.FromResult(CreateHandle(Process)); return Task.FromResult(CreateHandle(Process));
} }
/// <inheritdoc />
public Task ObserveWorkerTaskAsync(TimeSpan timeout) =>
SessionWorkerClientFactoryFakeWorkerTests.ObserveWorkerTaskAsync(WorkerTask, timeout);
private async Task RunWorkerAsync( private async Task RunWorkerAsync(
WorkerProcessLaunchRequest request, WorkerProcessLaunchRequest request,
CancellationToken cancellationToken) CancellationToken cancellationToken)
@@ -203,37 +273,52 @@ public sealed class SessionWorkerClientFactoryFakeWorkerTests
} }
/// <summary>Fake worker launcher that never completes startup, simulating a hung worker.</summary> /// <summary>Fake worker launcher that never completes startup, simulating a hung worker.</summary>
private sealed class NeverReadyWorkerProcessLauncher : IWorkerProcessLauncher private sealed class NeverReadyWorkerProcessLauncher : IWorkerTaskLauncher
{ {
private readonly CancellationTokenSource _stop = new();
/// <summary>Gets the fake worker process.</summary> /// <summary>Gets the fake worker process.</summary>
public FakeWorkerProcess Process { get; } = new(processId: 4680); public FakeWorkerProcess Process { get; } = new(processId: 4680);
/// <summary>Gets the scripted worker task.</summary>
public Task WorkerTask { get; private set; } = Task.CompletedTask;
/// <inheritdoc /> /// <inheritdoc />
public Task<WorkerProcessHandle> LaunchAsync( public Task<WorkerProcessHandle> LaunchAsync(
WorkerProcessLaunchRequest request, WorkerProcessLaunchRequest request,
CancellationToken cancellationToken = default) CancellationToken cancellationToken = default)
{ {
_ = RunWorkerAsync(request, cancellationToken); WorkerTask = RunWorkerAsync(request);
return Task.FromResult(CreateHandle(Process)); return Task.FromResult(CreateHandle(Process));
} }
private async Task RunWorkerAsync( /// <inheritdoc />
WorkerProcessLaunchRequest request, public async Task ObserveWorkerTaskAsync(TimeSpan timeout)
CancellationToken cancellationToken) {
// The scripted worker parks on an infinite delay; cancel it so disposal observes
// the task instead of leaking it as an unobserved fault.
await _stop.CancelAsync().ConfigureAwait(false);
await SessionWorkerClientFactoryFakeWorkerTests
.ObserveWorkerTaskAsync(WorkerTask, timeout)
.ConfigureAwait(false);
_stop.Dispose();
}
private async Task RunWorkerAsync(WorkerProcessLaunchRequest request)
{ {
await using FakeWorkerHarness harness = await FakeWorkerHarness.ConnectToGatewayPipeAsync( await using FakeWorkerHarness harness = await FakeWorkerHarness.ConnectToGatewayPipeAsync(
request.SessionId, request.SessionId,
request.Nonce, request.Nonce,
request.PipeName, request.PipeName,
request.ProtocolVersion, request.ProtocolVersion,
cancellationToken: cancellationToken).ConfigureAwait(false); cancellationToken: _stop.Token).ConfigureAwait(false);
_ = await harness.ReadGatewayEnvelopeAsync(cancellationToken).ConfigureAwait(false); _ = await harness.ReadGatewayEnvelopeAsync(_stop.Token).ConfigureAwait(false);
await harness.SendWorkerHelloAsync( await harness.SendWorkerHelloAsync(
workerProcessId: Process.Id, workerProcessId: Process.Id,
workerProtocolVersion: request.ProtocolVersion, workerProtocolVersion: request.ProtocolVersion,
cancellationToken: cancellationToken).ConfigureAwait(false); cancellationToken: _stop.Token).ConfigureAwait(false);
await Task.Delay(Timeout.InfiniteTimeSpan, cancellationToken).ConfigureAwait(false); await Task.Delay(Timeout.InfiniteTimeSpan, _stop.Token).ConfigureAwait(false);
} }
} }
@@ -1,9 +1,4 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Runtime.CompilerServices; using System.Runtime.CompilerServices;
using System.Threading;
using System.Threading.Tasks;
using MxGateway.Contracts.Proto; using MxGateway.Contracts.Proto;
using MxGateway.Server.Sessions; using MxGateway.Server.Sessions;
using MxGateway.Server.Workers; using MxGateway.Server.Workers;
@@ -19,10 +14,10 @@ namespace MxGateway.Tests.Gateway.Sessions;
public sealed class WorkerAlarmRpcDispatcherTests public sealed class WorkerAlarmRpcDispatcherTests
{ {
[Fact] [Fact]
public async Task AcknowledgeAsync_returns_session_not_found_when_session_missing() public async Task AcknowledgeAsync_WhenSessionMissing_ReturnsSessionNotFound()
{ {
SessionRegistry registry = new SessionRegistry(); SessionRegistry registry = new();
WorkerAlarmRpcDispatcher dispatcher = new WorkerAlarmRpcDispatcher(registry); WorkerAlarmRpcDispatcher dispatcher = new(registry);
AcknowledgeAlarmReply reply = await dispatcher.AcknowledgeAsync( AcknowledgeAlarmReply reply = await dispatcher.AcknowledgeAsync(
new AcknowledgeAlarmRequest new AcknowledgeAlarmRequest
@@ -37,11 +32,11 @@ public sealed class WorkerAlarmRpcDispatcherTests
} }
[Fact] [Fact]
public async Task AcknowledgeAsync_forwards_guid_and_returns_native_status() public async Task AcknowledgeAsync_WithGuidReference_ForwardsGuidAndReturnsNativeStatus()
{ {
SessionRegistry registry = new SessionRegistry(); SessionRegistry registry = new();
Guid alarmGuid = Guid.NewGuid(); Guid alarmGuid = Guid.NewGuid();
FakeAlarmWorkerClient worker = new FakeAlarmWorkerClient FakeAlarmWorkerClient worker = new()
{ {
ReplyFactory = command => ReplyFactory = command =>
{ {
@@ -63,7 +58,7 @@ public sealed class WorkerAlarmRpcDispatcherTests
session.MarkReady(); session.MarkReady();
registry.TryAdd(session); registry.TryAdd(session);
WorkerAlarmRpcDispatcher dispatcher = new WorkerAlarmRpcDispatcher(registry); WorkerAlarmRpcDispatcher dispatcher = new(registry);
AcknowledgeAlarmReply reply = await dispatcher.AcknowledgeAsync( AcknowledgeAlarmReply reply = await dispatcher.AcknowledgeAsync(
new AcknowledgeAlarmRequest new AcknowledgeAlarmRequest
@@ -84,10 +79,10 @@ public sealed class WorkerAlarmRpcDispatcherTests
} }
[Fact] [Fact]
public async Task AcknowledgeAsync_propagates_worker_diagnostic_on_failure() public async Task AcknowledgeAsync_WhenWorkerFails_PropagatesWorkerDiagnostic()
{ {
SessionRegistry registry = new SessionRegistry(); SessionRegistry registry = new();
FakeAlarmWorkerClient worker = new FakeAlarmWorkerClient FakeAlarmWorkerClient worker = new()
{ {
ReplyFactory = _ => new MxCommandReply ReplyFactory = _ => new MxCommandReply
{ {
@@ -106,7 +101,7 @@ public sealed class WorkerAlarmRpcDispatcherTests
session.MarkReady(); session.MarkReady();
registry.TryAdd(session); registry.TryAdd(session);
WorkerAlarmRpcDispatcher dispatcher = new WorkerAlarmRpcDispatcher(registry); WorkerAlarmRpcDispatcher dispatcher = new(registry);
AcknowledgeAlarmReply reply = await dispatcher.AcknowledgeAsync( AcknowledgeAlarmReply reply = await dispatcher.AcknowledgeAsync(
new AcknowledgeAlarmRequest new AcknowledgeAlarmRequest
@@ -125,7 +120,7 @@ public sealed class WorkerAlarmRpcDispatcherTests
[InlineData("Galaxy!TestArea.TestMachine_001.TestAlarm001", "Galaxy", "TestArea", "TestMachine_001.TestAlarm001")] [InlineData("Galaxy!TestArea.TestMachine_001.TestAlarm001", "Galaxy", "TestArea", "TestMachine_001.TestAlarm001")]
[InlineData("Galaxy!Area.Tag", "Galaxy", "Area", "Tag")] [InlineData("Galaxy!Area.Tag", "Galaxy", "Area", "Tag")]
[InlineData("Provider!Group.Tag.With.Dots", "Provider", "Group", "Tag.With.Dots")] [InlineData("Provider!Group.Tag.With.Dots", "Provider", "Group", "Tag.With.Dots")]
public void TryParseAlarmReference_decomposes_provider_group_tag( public void TryParseAlarmReference_WithProviderGroupTag_DecomposesParts(
string reference, string expectedProvider, string expectedGroup, string expectedName) string reference, string expectedProvider, string expectedGroup, string expectedName)
{ {
Assert.True(WorkerAlarmRpcDispatcher.TryParseAlarmReference( Assert.True(WorkerAlarmRpcDispatcher.TryParseAlarmReference(
@@ -145,18 +140,18 @@ public sealed class WorkerAlarmRpcDispatcherTests
[InlineData("Galaxy!Group")] // missing dot [InlineData("Galaxy!Group")] // missing dot
[InlineData("Galaxy!.Tag")] // empty group [InlineData("Galaxy!.Tag")] // empty group
[InlineData("Galaxy!Group.")] // empty tag [InlineData("Galaxy!Group.")] // empty tag
public void TryParseAlarmReference_rejects_malformed_references(string? reference) public void TryParseAlarmReference_WithMalformedReference_ReturnsFalse(string? reference)
{ {
Assert.False(WorkerAlarmRpcDispatcher.TryParseAlarmReference( Assert.False(WorkerAlarmRpcDispatcher.TryParseAlarmReference(
reference, out _, out _, out _)); reference, out _, out _, out _));
} }
[Fact] [Fact]
public async Task AcknowledgeAsync_routes_provider_group_tag_via_AckByName() public async Task AcknowledgeAsync_WithProviderGroupTagReference_RoutesViaAckByName()
{ {
SessionRegistry registry = new SessionRegistry(); SessionRegistry registry = new();
AcknowledgeAlarmByNameCommand? observed = null; AcknowledgeAlarmByNameCommand? observed = null;
FakeAlarmWorkerClient worker = new FakeAlarmWorkerClient FakeAlarmWorkerClient worker = new()
{ {
ReplyFactory = command => ReplyFactory = command =>
{ {
@@ -176,7 +171,7 @@ public sealed class WorkerAlarmRpcDispatcherTests
session.MarkReady(); session.MarkReady();
registry.TryAdd(session); registry.TryAdd(session);
WorkerAlarmRpcDispatcher dispatcher = new WorkerAlarmRpcDispatcher(registry); WorkerAlarmRpcDispatcher dispatcher = new(registry);
AcknowledgeAlarmReply reply = await dispatcher.AcknowledgeAsync( AcknowledgeAlarmReply reply = await dispatcher.AcknowledgeAsync(
new AcknowledgeAlarmRequest new AcknowledgeAlarmRequest
@@ -199,16 +194,16 @@ public sealed class WorkerAlarmRpcDispatcherTests
} }
[Fact] [Fact]
public async Task AcknowledgeAsync_returns_invalid_request_for_unparseable_reference() public async Task AcknowledgeAsync_WithUnparseableReference_ReturnsInvalidRequest()
{ {
SessionRegistry registry = new SessionRegistry(); SessionRegistry registry = new();
FakeAlarmWorkerClient worker = new FakeAlarmWorkerClient(); FakeAlarmWorkerClient worker = new();
GatewaySession session = NewSession("s1"); GatewaySession session = NewSession("s1");
session.AttachWorkerClient(worker); session.AttachWorkerClient(worker);
session.MarkReady(); session.MarkReady();
registry.TryAdd(session); registry.TryAdd(session);
WorkerAlarmRpcDispatcher dispatcher = new WorkerAlarmRpcDispatcher(registry); WorkerAlarmRpcDispatcher dispatcher = new(registry);
AcknowledgeAlarmReply reply = await dispatcher.AcknowledgeAsync( AcknowledgeAlarmReply reply = await dispatcher.AcknowledgeAsync(
new AcknowledgeAlarmRequest new AcknowledgeAlarmRequest
@@ -223,10 +218,10 @@ public sealed class WorkerAlarmRpcDispatcherTests
} }
[Fact] [Fact]
public async Task QueryActiveAlarmsAsync_yields_each_snapshot_from_payload() public async Task QueryActiveAlarmsAsync_WithPayloadSnapshots_YieldsEachSnapshot()
{ {
SessionRegistry registry = new SessionRegistry(); SessionRegistry registry = new();
FakeAlarmWorkerClient worker = new FakeAlarmWorkerClient FakeAlarmWorkerClient worker = new()
{ {
ReplyFactory = command => ReplyFactory = command =>
{ {
@@ -257,9 +252,9 @@ public sealed class WorkerAlarmRpcDispatcherTests
session.MarkReady(); session.MarkReady();
registry.TryAdd(session); registry.TryAdd(session);
WorkerAlarmRpcDispatcher dispatcher = new WorkerAlarmRpcDispatcher(registry); WorkerAlarmRpcDispatcher dispatcher = new(registry);
List<ActiveAlarmSnapshot> collected = new List<ActiveAlarmSnapshot>(); List<ActiveAlarmSnapshot> collected = new();
await foreach (ActiveAlarmSnapshot snap in dispatcher.QueryActiveAlarmsAsync( await foreach (ActiveAlarmSnapshot snap in dispatcher.QueryActiveAlarmsAsync(
new QueryActiveAlarmsRequest { SessionId = "s1" }, new QueryActiveAlarmsRequest { SessionId = "s1" },
CancellationToken.None)) CancellationToken.None))
@@ -273,12 +268,12 @@ public sealed class WorkerAlarmRpcDispatcherTests
} }
[Fact] [Fact]
public async Task QueryActiveAlarmsAsync_yields_empty_when_session_missing() public async Task QueryActiveAlarmsAsync_WhenSessionMissing_YieldsEmpty()
{ {
SessionRegistry registry = new SessionRegistry(); SessionRegistry registry = new();
WorkerAlarmRpcDispatcher dispatcher = new WorkerAlarmRpcDispatcher(registry); WorkerAlarmRpcDispatcher dispatcher = new(registry);
List<ActiveAlarmSnapshot> collected = new List<ActiveAlarmSnapshot>(); List<ActiveAlarmSnapshot> collected = new();
await foreach (ActiveAlarmSnapshot snap in dispatcher.QueryActiveAlarmsAsync( await foreach (ActiveAlarmSnapshot snap in dispatcher.QueryActiveAlarmsAsync(
new QueryActiveAlarmsRequest { SessionId = "missing" }, new QueryActiveAlarmsRequest { SessionId = "missing" },
CancellationToken.None)) CancellationToken.None))
@@ -290,10 +285,10 @@ public sealed class WorkerAlarmRpcDispatcherTests
} }
[Fact] [Fact]
public async Task QueryActiveAlarmsAsync_yields_empty_on_worker_failure() public async Task QueryActiveAlarmsAsync_WhenWorkerFails_YieldsEmpty()
{ {
SessionRegistry registry = new SessionRegistry(); SessionRegistry registry = new();
FakeAlarmWorkerClient worker = new FakeAlarmWorkerClient FakeAlarmWorkerClient worker = new()
{ {
ReplyFactory = _ => new MxCommandReply ReplyFactory = _ => new MxCommandReply
{ {
@@ -310,9 +305,9 @@ public sealed class WorkerAlarmRpcDispatcherTests
session.MarkReady(); session.MarkReady();
registry.TryAdd(session); registry.TryAdd(session);
WorkerAlarmRpcDispatcher dispatcher = new WorkerAlarmRpcDispatcher(registry); WorkerAlarmRpcDispatcher dispatcher = new(registry);
List<ActiveAlarmSnapshot> collected = new List<ActiveAlarmSnapshot>(); List<ActiveAlarmSnapshot> collected = new();
await foreach (ActiveAlarmSnapshot snap in dispatcher.QueryActiveAlarmsAsync( await foreach (ActiveAlarmSnapshot snap in dispatcher.QueryActiveAlarmsAsync(
new QueryActiveAlarmsRequest { SessionId = "s1" }, new QueryActiveAlarmsRequest { SessionId = "s1" },
CancellationToken.None)) CancellationToken.None))
@@ -325,7 +320,7 @@ public sealed class WorkerAlarmRpcDispatcherTests
private static GatewaySession NewSession(string sessionId) private static GatewaySession NewSession(string sessionId)
{ {
return new GatewaySession( return new(
sessionId, sessionId,
"mxaccess", "mxaccess",
$"mxaccess-gateway-1-{sessionId}", $"mxaccess-gateway-1-{sessionId}",
@@ -16,6 +16,13 @@
<Using Include="Xunit" /> <Using Include="Xunit" />
</ItemGroup> </ItemGroup>
<ItemGroup>
<!-- Makes the xUnit parallelism policy explicit (Tests-012): test collections run in
parallel, so WebApplication-building tests must keep binding ephemeral ports
(http://127.0.0.1:0) to avoid a future fixed-port collision. -->
<None Update="xunit.runner.json" CopyToOutputDirectory="PreserveNewest" />
</ItemGroup>
<ItemGroup> <ItemGroup>
<ProjectReference Include="..\MxGateway.Contracts\MxGateway.Contracts.csproj" /> <ProjectReference Include="..\MxGateway.Contracts\MxGateway.Contracts.csproj" />
<ProjectReference Include="..\MxGateway.Server\MxGateway.Server.csproj" /> <ProjectReference Include="..\MxGateway.Server\MxGateway.Server.csproj" />
@@ -10,6 +10,7 @@ using MxGateway.Server.Metrics;
using MxGateway.Server.Security.Authentication; using MxGateway.Server.Security.Authentication;
using MxGateway.Server.Security.Authorization; using MxGateway.Server.Security.Authorization;
using MxGateway.Server.Sessions; using MxGateway.Server.Sessions;
using MxGateway.Tests.TestSupport;
namespace MxGateway.Tests.Security.Authorization; namespace MxGateway.Tests.Security.Authorization;
@@ -107,7 +108,7 @@ public sealed class GatewayGrpcAuthorizationInterceptorTests
RpcException exception = await Assert.ThrowsAsync<RpcException>( RpcException exception = await Assert.ThrowsAsync<RpcException>(
() => interceptor.ServerStreamingServerHandler( () => interceptor.ServerStreamingServerHandler(
new StreamEventsRequest(), new StreamEventsRequest(),
new TestServerStreamWriter<MxEvent>(), new RecordingServerStreamWriter<MxEvent>(),
ContextWithAuthorization("Bearer mxgw_operator01_secret"), ContextWithAuthorization("Bearer mxgw_operator01_secret"),
(_, _, _) => Task.CompletedTask)); (_, _, _) => Task.CompletedTask));
@@ -123,7 +124,7 @@ public sealed class GatewayGrpcAuthorizationInterceptorTests
GatewayGrpcAuthorizationInterceptor interceptor = CreateInterceptor( GatewayGrpcAuthorizationInterceptor interceptor = CreateInterceptor(
new FakeApiKeyVerifier(SuccessWithScopes(GatewayScopes.EventsRead)), new FakeApiKeyVerifier(SuccessWithScopes(GatewayScopes.EventsRead)),
identityAccessor); identityAccessor);
TestServerStreamWriter<MxEvent> streamWriter = new(); RecordingServerStreamWriter<MxEvent> streamWriter = new();
await interceptor.ServerStreamingServerHandler( await interceptor.ServerStreamingServerHandler(
new StreamEventsRequest(), new StreamEventsRequest(),
@@ -396,40 +397,6 @@ public sealed class GatewayGrpcAuthorizationInterceptorTests
} }
} }
/// <summary>Constraint enforcer that permits every operation for composition tests.</summary>
private sealed class AllowAllConstraintEnforcer : IConstraintEnforcer
{
/// <inheritdoc />
public Task<ConstraintFailure?> CheckReadTagAsync(
ApiKeyIdentity? identity,
string tagAddress,
CancellationToken cancellationToken) => Task.FromResult<ConstraintFailure?>(null);
/// <inheritdoc />
public Task<ConstraintFailure?> CheckReadHandleAsync(
ApiKeyIdentity? identity,
GatewaySession session,
int serverHandle,
int itemHandle,
CancellationToken cancellationToken) => Task.FromResult<ConstraintFailure?>(null);
/// <inheritdoc />
public Task<ConstraintFailure?> CheckWriteHandleAsync(
ApiKeyIdentity? identity,
GatewaySession session,
int serverHandle,
int itemHandle,
CancellationToken cancellationToken) => Task.FromResult<ConstraintFailure?>(null);
/// <inheritdoc />
public Task RecordDenialAsync(
ApiKeyIdentity? identity,
string commandKind,
string target,
ConstraintFailure failure,
CancellationToken cancellationToken) => Task.CompletedTask;
}
private sealed class FakeApiKeyVerifier(ApiKeyVerificationResult result) : IApiKeyVerifier private sealed class FakeApiKeyVerifier(ApiKeyVerificationResult result) : IApiKeyVerifier
{ {
/// <summary>Gets whether the verifier was called.</summary> /// <summary>Gets whether the verifier was called.</summary>
@@ -453,88 +420,4 @@ public sealed class GatewayGrpcAuthorizationInterceptorTests
} }
} }
private sealed class TestServerStreamWriter<T> : IServerStreamWriter<T>
{
/// <summary>Gets messages written to the stream.</summary>
public List<T> Messages { get; } = [];
/// <summary>Gets or sets write options for the stream.</summary>
public WriteOptions? WriteOptions { get; set; }
/// <summary>Writes a message to the stream.</summary>
/// <param name="message">The message to write.</param>
/// <returns>Task representing the write operation.</returns>
public Task WriteAsync(T message)
{
Messages.Add(message);
return Task.CompletedTask;
}
}
private sealed class TestServerCallContext(
Metadata requestHeaders,
CancellationToken cancellationToken = default) : ServerCallContext
{
private readonly Metadata responseTrailers = [];
private readonly Dictionary<object, object> userState = [];
private Status status;
private WriteOptions? writeOptions;
/// <inheritdoc />
protected override string MethodCore => "/mxaccess_gateway.v1.MxAccessGateway/Test";
/// <inheritdoc />
protected override string HostCore => "localhost";
/// <inheritdoc />
protected override string PeerCore => "ipv4:127.0.0.1:5000";
/// <inheritdoc />
protected override DateTime DeadlineCore => DateTime.UtcNow.AddMinutes(1);
/// <inheritdoc />
protected override Metadata RequestHeadersCore => requestHeaders;
/// <inheritdoc />
protected override CancellationToken CancellationTokenCore => cancellationToken;
/// <inheritdoc />
protected override Metadata ResponseTrailersCore => responseTrailers;
/// <inheritdoc />
protected override Status StatusCore
{
get => status;
set => status = value;
}
/// <inheritdoc />
protected override WriteOptions? WriteOptionsCore
{
get => writeOptions;
set => writeOptions = value;
}
/// <inheritdoc />
protected override AuthContext AuthContextCore { get; } = new(
string.Empty,
new Dictionary<string, List<AuthProperty>>(StringComparer.Ordinal));
/// <inheritdoc />
protected override IDictionary<object, object> UserStateCore => userState;
/// <inheritdoc />
protected override Task WriteResponseHeadersAsyncCore(Metadata responseHeaders)
{
return Task.CompletedTask;
}
/// <inheritdoc />
protected override ContextPropagationToken CreatePropagationTokenCore(
ContextPropagationOptions? options)
{
throw new NotSupportedException();
}
}
} }
@@ -0,0 +1,42 @@
using MxGateway.Server.Security.Authentication;
using MxGateway.Server.Security.Authorization;
using MxGateway.Server.Sessions;
namespace MxGateway.Tests.TestSupport;
/// <summary>
/// <see cref="IConstraintEnforcer"/> that permits every operation, for tests that
/// exercise gRPC service or interceptor behaviour without constraint policy.
/// </summary>
public sealed class AllowAllConstraintEnforcer : IConstraintEnforcer
{
/// <inheritdoc />
public Task<ConstraintFailure?> CheckReadTagAsync(
ApiKeyIdentity? identity,
string tagAddress,
CancellationToken cancellationToken) => Task.FromResult<ConstraintFailure?>(null);
/// <inheritdoc />
public Task<ConstraintFailure?> CheckReadHandleAsync(
ApiKeyIdentity? identity,
GatewaySession session,
int serverHandle,
int itemHandle,
CancellationToken cancellationToken) => Task.FromResult<ConstraintFailure?>(null);
/// <inheritdoc />
public Task<ConstraintFailure?> CheckWriteHandleAsync(
ApiKeyIdentity? identity,
GatewaySession session,
int serverHandle,
int itemHandle,
CancellationToken cancellationToken) => Task.FromResult<ConstraintFailure?>(null);
/// <inheritdoc />
public Task RecordDenialAsync(
ApiKeyIdentity? identity,
string commandKind,
string target,
ConstraintFailure failure,
CancellationToken cancellationToken) => Task.CompletedTask;
}
@@ -0,0 +1,50 @@
using Grpc.Core;
namespace MxGateway.Tests.TestSupport;
/// <summary>
/// Thread-safe <see cref="IServerStreamWriter{T}"/> that records every written message
/// and lets a test await the first message with a timeout.
/// </summary>
/// <typeparam name="T">The streamed message type.</typeparam>
public sealed class RecordingServerStreamWriter<T> : IServerStreamWriter<T>
{
private readonly object _syncRoot = new();
private readonly TaskCompletionSource<T> _firstMessage = new(TaskCreationOptions.RunContinuationsAsynchronously);
private readonly List<T> _messages = [];
/// <summary>Gets the messages written to this stream, in order.</summary>
public IReadOnlyList<T> Messages
{
get
{
lock (_syncRoot)
{
return _messages.ToArray();
}
}
}
/// <summary>Gets or sets options for writing messages to the stream.</summary>
public WriteOptions? WriteOptions { get; set; }
/// <summary>Records the supplied message.</summary>
/// <param name="message">The message to record.</param>
/// <returns>A completed task.</returns>
public Task WriteAsync(T message)
{
lock (_syncRoot)
{
_messages.Add(message);
}
_firstMessage.TrySetResult(message);
return Task.CompletedTask;
}
/// <summary>Waits for the first message to be written within the specified timeout.</summary>
/// <param name="timeout">Maximum time to wait for the first message.</param>
/// <returns>The first message written to this stream.</returns>
public async Task<T> WaitForFirstMessageAsync(TimeSpan timeout) =>
await _firstMessage.Task.WaitAsync(timeout).ConfigureAwait(false);
}
@@ -0,0 +1,76 @@
using Grpc.Core;
namespace MxGateway.Tests.TestSupport;
/// <summary>
/// Minimal in-memory <see cref="ServerCallContext"/> for exercising gRPC service
/// implementations directly in unit tests, without a real gRPC transport.
/// </summary>
public sealed class TestServerCallContext : ServerCallContext
{
private readonly Metadata _requestHeaders;
private readonly Metadata _responseTrailers = [];
private readonly Dictionary<object, object> _userState = [];
private readonly CancellationToken _cancellationToken;
private Status _status;
private WriteOptions? _writeOptions;
/// <summary>Initializes the context with the supplied request headers and cancellation token.</summary>
/// <param name="requestHeaders">Request headers visible to the service; defaults to empty.</param>
/// <param name="cancellationToken">Cancellation token surfaced to the service.</param>
public TestServerCallContext(Metadata? requestHeaders = null, CancellationToken cancellationToken = default)
{
_requestHeaders = requestHeaders ?? [];
_cancellationToken = cancellationToken;
}
/// <inheritdoc />
protected override string MethodCore => "/mxaccess_gateway.v1.MxAccessGateway/Test";
/// <inheritdoc />
protected override string HostCore => "localhost";
/// <inheritdoc />
protected override string PeerCore => "ipv4:127.0.0.1:5000";
/// <inheritdoc />
protected override DateTime DeadlineCore => DateTime.UtcNow.AddMinutes(1);
/// <inheritdoc />
protected override Metadata RequestHeadersCore => _requestHeaders;
/// <inheritdoc />
protected override CancellationToken CancellationTokenCore => _cancellationToken;
/// <inheritdoc />
protected override Metadata ResponseTrailersCore => _responseTrailers;
/// <inheritdoc />
protected override Status StatusCore
{
get => _status;
set => _status = value;
}
/// <inheritdoc />
protected override WriteOptions? WriteOptionsCore
{
get => _writeOptions;
set => _writeOptions = value;
}
/// <inheritdoc />
protected override AuthContext AuthContextCore { get; } = new(
string.Empty,
new Dictionary<string, List<AuthProperty>>(StringComparer.Ordinal));
/// <inheritdoc />
protected override IDictionary<object, object> UserStateCore => _userState;
/// <inheritdoc />
protected override Task WriteResponseHeadersAsyncCore(Metadata responseHeaders) => Task.CompletedTask;
/// <inheritdoc />
protected override ContextPropagationToken CreatePropagationTokenCore(ContextPropagationOptions? options) =>
throw new NotSupportedException();
}
+8
View File
@@ -0,0 +1,8 @@
{
"$schema": "https://xunit.net/schema/current/xunit.runner.schema.json",
"appDomain": "denied",
"parallelizeAssembly": false,
"parallelizeTestCollections": true,
"maxParallelThreads": -1,
"longRunningTestSeconds": 30
}
@@ -32,4 +32,16 @@ public sealed class WorkerLogRedactorTests
Assert.Equal("[redacted]", redacted["api_key"]); Assert.Equal("[redacted]", redacted["api_key"]);
Assert.Equal("session-1", redacted["session_id"]); Assert.Equal("session-1", redacted["session_id"]);
} }
/// <summary>
/// Verifies <see cref="WorkerLogRedactor.RedactValue"/> redacts individual
/// credential-bearing fields before they reach a log sink.
/// </summary>
[Fact]
public void RedactValue_WithCredentialBearingFieldNames_ReturnsRedactedValue()
{
Assert.Equal(WorkerLogRedactor.RedactedValue, WorkerLogRedactor.RedactValue("credential_value", "secret"));
Assert.Equal(WorkerLogRedactor.RedactedValue, WorkerLogRedactor.RedactValue("password_value", "secret"));
Assert.Equal(WorkerLogRedactor.RedactedValue, WorkerLogRedactor.RedactValue("secured_write_token", "secret"));
}
} }
@@ -1,7 +1,6 @@
using System; using System;
using Google.Protobuf; using Google.Protobuf;
using MxGateway.Contracts.Proto; using MxGateway.Contracts.Proto;
using MxGateway.Worker.Bootstrap;
using MxGateway.Worker.Conversion; using MxGateway.Worker.Conversion;
using ProtobufTimestamp = Google.Protobuf.WellKnownTypes.Timestamp; using ProtobufTimestamp = Google.Protobuf.WellKnownTypes.Timestamp;
@@ -192,15 +191,6 @@ public sealed class VariantConverterTests
Assert.Contains(typeof(UnsupportedVariant).FullName!, converted.ArrayValue.RawDiagnostic); Assert.Contains(typeof(UnsupportedVariant).FullName!, converted.ArrayValue.RawDiagnostic);
} }
/// <summary>Verifies that credential-bearing fields are redacted before logging.</summary>
[Fact]
public void Redactor_WithCredentialBearingValueFields_RedactsBeforeLogging()
{
Assert.Equal(WorkerLogRedactor.RedactedValue, WorkerLogRedactor.RedactValue("credential_value", "secret"));
Assert.Equal(WorkerLogRedactor.RedactedValue, WorkerLogRedactor.RedactValue("password_value", "secret"));
Assert.Equal(WorkerLogRedactor.RedactedValue, WorkerLogRedactor.RedactValue("secured_write_token", "secret"));
}
/// <summary>Fake unsupported variant type for testing unknown type handling.</summary> /// <summary>Fake unsupported variant type for testing unknown type handling.</summary>
private sealed class UnsupportedVariant private sealed class UnsupportedVariant
{ {
@@ -1,10 +1,10 @@
using System.IO; using System.IO;
using System.Linq; using System.Linq;
using System.Threading.Tasks; using System.Threading.Tasks;
using Google.Protobuf;
using MxGateway.Contracts; using MxGateway.Contracts;
using MxGateway.Contracts.Proto; using MxGateway.Contracts.Proto;
using MxGateway.Worker.Ipc; using MxGateway.Worker.Ipc;
using MxGateway.Worker.Tests.TestSupport;
namespace MxGateway.Worker.Tests.Ipc; namespace MxGateway.Worker.Tests.Ipc;
@@ -38,7 +38,7 @@ public sealed class WorkerFrameProtocolTests
WorkerFrameProtocolOptions options = CreateOptions(); WorkerFrameProtocolOptions options = CreateOptions();
WorkerEnvelope envelope = CreateGatewayHelloEnvelope(); WorkerEnvelope envelope = CreateGatewayHelloEnvelope();
envelope.ProtocolVersion++; envelope.ProtocolVersion++;
using MemoryStream stream = new(CreateFrame(envelope)); using MemoryStream stream = new(WorkerFrameTestHelpers.CreateFrame(envelope));
WorkerFrameReader reader = new(stream, options); WorkerFrameReader reader = new(stream, options);
WorkerFrameProtocolException exception = WorkerFrameProtocolException exception =
@@ -55,7 +55,7 @@ public sealed class WorkerFrameProtocolTests
WorkerFrameProtocolOptions options = CreateOptions(); WorkerFrameProtocolOptions options = CreateOptions();
WorkerEnvelope envelope = CreateGatewayHelloEnvelope(); WorkerEnvelope envelope = CreateGatewayHelloEnvelope();
envelope.SessionId = "different-session"; envelope.SessionId = "different-session";
using MemoryStream stream = new(CreateFrame(envelope)); using MemoryStream stream = new(WorkerFrameTestHelpers.CreateFrame(envelope));
WorkerFrameReader reader = new(stream, options); WorkerFrameReader reader = new(stream, options);
WorkerFrameProtocolException exception = WorkerFrameProtocolException exception =
@@ -65,9 +65,15 @@ public sealed class WorkerFrameProtocolTests
Assert.Equal(WorkerFrameProtocolErrorCode.SessionMismatch, exception.ErrorCode); Assert.Equal(WorkerFrameProtocolErrorCode.SessionMismatch, exception.ErrorCode);
} }
/// <summary>Verifies that malformed length throws error.</summary> /// <summary>
/// Verifies that a frame whose length prefix is zero is rejected before the
/// payload buffer is allocated. <c>docs/WorkerFrameProtocol.md</c> states the
/// reader rejects zero-length payloads as a malformed-length error. The
/// length prefix is the leading four bytes of the stream, so a four-zero-byte
/// stream is exactly a frame declaring a zero-length payload.
/// </summary>
[Fact] [Fact]
public async Task ReadAsync_WithMalformedLength_ThrowsMalformedLength() public async Task ReadAsync_WithZeroLengthPayload_ThrowsMalformedLength()
{ {
WorkerFrameProtocolOptions options = CreateOptions(); WorkerFrameProtocolOptions options = CreateOptions();
using MemoryStream stream = new(new byte[sizeof(uint)]); using MemoryStream stream = new(new byte[sizeof(uint)]);
@@ -80,12 +86,40 @@ public sealed class WorkerFrameProtocolTests
Assert.Equal(WorkerFrameProtocolErrorCode.MalformedLength, exception.ErrorCode); Assert.Equal(WorkerFrameProtocolErrorCode.MalformedLength, exception.ErrorCode);
} }
/// <summary>
/// Verifies that a frame whose length prefix exceeds the configured maximum
/// is rejected before the payload buffer is allocated. <c>docs/WorkerFrameProtocol.md</c>
/// states the reader rejects oversized payloads as a message-too-large error.
/// A small maximum is configured so the rejection is asserted without
/// allocating a multi-megabyte buffer.
/// </summary>
[Fact]
public async Task ReadAsync_WithPayloadAboveConfiguredMaximum_ThrowsMessageTooLarge()
{
const int maxMessageBytes = 64;
WorkerFrameProtocolOptions options = new(
SessionId,
GatewayContractInfo.WorkerProtocolVersion,
Nonce,
maxMessageBytes);
byte[] frame = new byte[sizeof(uint)];
WorkerFrameTestHelpers.WriteUInt32LittleEndian(frame, maxMessageBytes + 1);
using MemoryStream stream = new(frame);
WorkerFrameReader reader = new(stream, options);
WorkerFrameProtocolException exception =
await Assert.ThrowsAsync<WorkerFrameProtocolException>(
async () => await reader.ReadAsync());
Assert.Equal(WorkerFrameProtocolErrorCode.MessageTooLarge, exception.ErrorCode);
}
/// <summary>Verifies that malformed payload throws invalid envelope error.</summary> /// <summary>Verifies that malformed payload throws invalid envelope error.</summary>
[Fact] [Fact]
public async Task ReadAsync_WithMalformedPayload_ThrowsInvalidEnvelope() public async Task ReadAsync_WithMalformedPayload_ThrowsInvalidEnvelope()
{ {
WorkerFrameProtocolOptions options = CreateOptions(); WorkerFrameProtocolOptions options = CreateOptions();
using MemoryStream stream = new(CreateFrame(new byte[] { 0x80 })); using MemoryStream stream = new(WorkerFrameTestHelpers.CreateFrame(new byte[] { 0x80 }));
WorkerFrameReader reader = new(stream, options); WorkerFrameReader reader = new(stream, options);
WorkerFrameProtocolException exception = WorkerFrameProtocolException exception =
@@ -175,27 +209,4 @@ public sealed class WorkerFrameProtocolTests
}; };
} }
private static byte[] CreateFrame(IMessage message)
{
return CreateFrame(message.ToByteArray());
}
private static byte[] CreateFrame(byte[] payload)
{
byte[] frame = new byte[sizeof(uint) + payload.Length];
WriteUInt32LittleEndian(frame, (uint)payload.Length);
payload.CopyTo(frame, sizeof(uint));
return frame;
}
private static void WriteUInt32LittleEndian(
byte[] buffer,
uint value)
{
buffer[0] = (byte)value;
buffer[1] = (byte)(value >> 8);
buffer[2] = (byte)(value >> 16);
buffer[3] = (byte)(value >> 24);
}
} }
@@ -1,5 +1,4 @@
using System; using System;
using System.Collections.Generic;
using System.IO; using System.IO;
using System.IO.Pipes; using System.IO.Pipes;
using System.Threading; using System.Threading;
@@ -9,8 +8,7 @@ using MxGateway.Contracts;
using MxGateway.Contracts.Proto; using MxGateway.Contracts.Proto;
using MxGateway.Worker.Bootstrap; using MxGateway.Worker.Bootstrap;
using MxGateway.Worker.Ipc; using MxGateway.Worker.Ipc;
using MxGateway.Worker.MxAccess; using MxGateway.Worker.Tests.TestSupport;
using MxGateway.Worker.Sta;
namespace MxGateway.Worker.Tests.Ipc; namespace MxGateway.Worker.Tests.Ipc;
@@ -213,100 +211,4 @@ public sealed class WorkerPipeClientTests
}, },
}; };
} }
private sealed class FakeRuntimeSession : IWorkerRuntimeSession
{
/// <summary>Starts the worker session.</summary>
/// <param name="sessionId">Session ID.</param>
/// <param name="workerProcessId">Worker process ID.</param>
/// <param name="cancellationToken">Cancellation token.</param>
/// <returns>Worker ready response.</returns>
public Task<WorkerReady> StartAsync(
string sessionId,
int workerProcessId,
CancellationToken cancellationToken = default)
{
return Task.FromResult(new WorkerReady
{
WorkerProcessId = workerProcessId,
MxaccessProgid = MxGateway.Worker.MxAccess.MxAccessInteropInfo.ProgId,
MxaccessClsid = MxGateway.Worker.MxAccess.MxAccessInteropInfo.Clsid,
ReadyTimestamp = Timestamp.FromDateTimeOffset(DateTimeOffset.UtcNow),
});
}
/// <summary>Dispatches a command to STA thread.</summary>
/// <param name="command">The command.</param>
/// <returns>Command reply.</returns>
public Task<MxCommandReply> DispatchAsync(StaCommand command)
{
return Task.FromResult(new MxCommandReply
{
SessionId = command.SessionId,
CorrelationId = command.CorrelationId,
Kind = command.Kind,
ProtocolStatus = new ProtocolStatus
{
Code = ProtocolStatusCode.Ok,
Message = "OK",
},
});
}
/// <summary>Captures current runtime heartbeat snapshot.</summary>
/// <returns>Heartbeat snapshot.</returns>
public WorkerRuntimeHeartbeatSnapshot CaptureHeartbeat()
{
return new WorkerRuntimeHeartbeatSnapshot(
DateTimeOffset.UtcNow,
pendingCommandCount: 0,
outboundEventQueueDepth: 0,
lastEventSequence: 0,
currentCommandCorrelationId: string.Empty);
}
/// <summary>Drains queued events.</summary>
/// <param name="maxEvents">Maximum events to drain.</param>
/// <returns>Drained events.</returns>
public IReadOnlyList<WorkerEvent> DrainEvents(uint maxEvents)
{
return Array.Empty<WorkerEvent>();
}
/// <summary>Drains pending fault if any.</summary>
/// <returns>Fault or null.</returns>
public WorkerFault? DrainFault()
{
return null;
}
/// <summary>Cancels a command by correlation ID.</summary>
/// <param name="correlationId">Command correlation ID.</param>
/// <returns>True if cancelled.</returns>
public bool CancelCommand(string correlationId)
{
return false;
}
/// <summary>Requests graceful shutdown.</summary>
public void RequestShutdown()
{
}
/// <summary>Shuts down gracefully within timeout.</summary>
/// <param name="timeout">Shutdown timeout.</param>
/// <param name="cancellationToken">Cancellation token.</param>
/// <returns>Shutdown result.</returns>
public Task<MxAccessShutdownResult> ShutdownGracefullyAsync(
TimeSpan timeout,
CancellationToken cancellationToken = default)
{
return Task.FromResult(new MxAccessShutdownResult(Array.Empty<MxAccessShutdownFailure>()));
}
/// <summary>Disposes resources.</summary>
public void Dispose()
{
}
}
} }
@@ -11,6 +11,7 @@ using MxGateway.Contracts.Proto;
using MxGateway.Worker.Ipc; using MxGateway.Worker.Ipc;
using MxGateway.Worker.MxAccess; using MxGateway.Worker.MxAccess;
using MxGateway.Worker.Sta; using MxGateway.Worker.Sta;
using MxGateway.Worker.Tests.TestSupport;
namespace MxGateway.Worker.Tests.Ipc; namespace MxGateway.Worker.Tests.Ipc;
@@ -110,7 +111,7 @@ public sealed class WorkerPipeSessionTests
public async Task CompleteStartupHandshakeAsync_WithMalformedFrame_WritesWorkerFault() public async Task CompleteStartupHandshakeAsync_WithMalformedFrame_WritesWorkerFault()
{ {
WorkerFrameProtocolOptions options = CreateOptions(); WorkerFrameProtocolOptions options = CreateOptions();
using MemoryStream inbound = new(CreateFrame(new byte[] { 0x80 })); using MemoryStream inbound = new(WorkerFrameTestHelpers.CreateFrame(new byte[] { 0x80 }));
using MemoryStream outbound = new(); using MemoryStream outbound = new();
WorkerPipeSession session = CreateSession(inbound, outbound, options); WorkerPipeSession session = CreateSession(inbound, outbound, options);
bool initialized = false; bool initialized = false;
@@ -181,12 +182,24 @@ public sealed class WorkerPipeSessionTests
Task runTask = session.RunAsync(cancellation.Token); Task runTask = session.RunAsync(cancellation.Token);
await CompleteGatewayHandshakeAsync(pipePair, cancellation.Token); await CompleteGatewayHandshakeAsync(pipePair, cancellation.Token);
await ThrowIfCompletedAsync(runTask);
WorkerEnvelope heartbeat = await ReadUntilAsync( // Deterministic race: read the first heartbeat while watching runTask.
// A faulted RunAsync would complete the run task first; if it wins the
// race the test fails immediately with the underlying fault instead of
// waiting out an arbitrary fixed delay.
Task<WorkerEnvelope> heartbeatTask = ReadUntilAsync(
pipePair.GatewayReader, pipePair.GatewayReader,
WorkerEnvelope.BodyOneofCase.WorkerHeartbeat, WorkerEnvelope.BodyOneofCase.WorkerHeartbeat,
cancellation.Token); cancellation.Token);
Task winner = await Task.WhenAny(runTask, heartbeatTask);
if (winner == runTask)
{
// Surface the RunAsync fault (or assert it did not exit early).
await runTask;
Assert.Fail("RunAsync completed before the first heartbeat was received.");
}
WorkerEnvelope heartbeat = await heartbeatTask;
Assert.Equal(WorkerState.ExecutingCommand, heartbeat.WorkerHeartbeat.State); Assert.Equal(WorkerState.ExecutingCommand, heartbeat.WorkerHeartbeat.State);
Assert.Equal(1234, heartbeat.WorkerHeartbeat.WorkerProcessId); Assert.Equal(1234, heartbeat.WorkerHeartbeat.WorkerProcessId);
@@ -761,15 +774,6 @@ public sealed class WorkerPipeSessionTests
await runTask.ConfigureAwait(false); await runTask.ConfigureAwait(false);
} }
private static async Task ThrowIfCompletedAsync(Task task)
{
await Task.Delay(TimeSpan.FromMilliseconds(100));
if (task.IsCompleted)
{
await task;
}
}
/// <summary>Reads frames until one matching the expected body type is found.</summary> /// <summary>Reads frames until one matching the expected body type is found.</summary>
/// <param name="reader">Frame reader.</param> /// <param name="reader">Frame reader.</param>
/// <param name="expectedBody">Expected body case.</param> /// <param name="expectedBody">Expected body case.</param>
@@ -825,25 +829,6 @@ public sealed class WorkerPipeSessionTests
return envelopes.ToArray(); return envelopes.ToArray();
} }
private static byte[] CreateFrame(byte[] payload)
{
byte[] frame = new byte[sizeof(uint) + payload.Length];
WriteUInt32LittleEndian(frame, (uint)payload.Length);
payload.CopyTo(frame, sizeof(uint));
return frame;
}
private static void WriteUInt32LittleEndian(
byte[] buffer,
uint value)
{
buffer[0] = (byte)value;
buffer[1] = (byte)(value >> 8);
buffer[2] = (byte)(value >> 16);
buffer[3] = (byte)(value >> 24);
}
private sealed class RecordingWorkerLogger : MxGateway.Worker.Bootstrap.IWorkerLogger private sealed class RecordingWorkerLogger : MxGateway.Worker.Bootstrap.IWorkerLogger
{ {
private readonly object gate = new(); private readonly object gate = new();
@@ -907,204 +892,6 @@ public sealed class WorkerPipeSessionTests
} }
} }
private sealed class FakeRuntimeSession : IWorkerRuntimeSession
{
private readonly ManualResetEventSlim releaseDispatch = new(false);
private readonly object gate = new();
private readonly Queue<WorkerEvent> events = new();
private WorkerRuntimeHeartbeatSnapshot snapshot = new(
DateTimeOffset.UtcNow,
pendingCommandCount: 0,
outboundEventQueueDepth: 0,
lastEventSequence: 0,
currentCommandCorrelationId: string.Empty);
/// <summary>Gets the event signaled when dispatch begins.</summary>
public ManualResetEventSlim DispatchStarted { get; } = new(false);
/// <summary>Blocks dispatch execution until explicitly released.</summary>
public bool BlockDispatch { get; set; }
/// <summary>Gets or sets whether to throw an exception after dispatch is released.</summary>
public bool ThrowAfterDispatchReleased { get; set; }
/// <summary>Gets or sets whether ShutdownGracefullyAsync throws a TimeoutException.</summary>
public bool ThrowTimeoutOnShutdown { get; set; }
/// <summary>Gets a value indicating whether Dispose was called.</summary>
public bool Disposed { get; private set; }
/// <summary>Starts the worker session with the given session ID and process ID.</summary>
/// <param name="sessionId">The session identifier.</param>
/// <param name="workerProcessId">The worker process ID.</param>
/// <param name="cancellationToken">Cancellation token.</param>
/// <returns>Worker ready response.</returns>
public Task<WorkerReady> StartAsync(
string sessionId,
int workerProcessId,
CancellationToken cancellationToken = default)
{
return Task.FromResult(new WorkerReady
{
WorkerProcessId = workerProcessId,
MxaccessProgid = MxGateway.Worker.MxAccess.MxAccessInteropInfo.ProgId,
MxaccessClsid = MxGateway.Worker.MxAccess.MxAccessInteropInfo.Clsid,
ReadyTimestamp = Timestamp.FromDateTimeOffset(DateTimeOffset.UtcNow),
});
}
/// <summary>Dispatches a command to the STA thread.</summary>
/// <param name="command">The command to dispatch.</param>
/// <returns>The command reply.</returns>
public Task<MxCommandReply> DispatchAsync(StaCommand command)
{
return Task.Run(
() =>
{
SetSnapshot(new WorkerRuntimeHeartbeatSnapshot(
DateTimeOffset.UtcNow,
pendingCommandCount: 0,
outboundEventQueueDepth: 0,
lastEventSequence: 0,
command.CorrelationId));
DispatchStarted.Set();
if (BlockDispatch)
{
releaseDispatch.Wait(TimeSpan.FromSeconds(5));
}
SetSnapshot(new WorkerRuntimeHeartbeatSnapshot(
DateTimeOffset.UtcNow,
pendingCommandCount: 0,
outboundEventQueueDepth: 0,
lastEventSequence: 0,
currentCommandCorrelationId: string.Empty));
if (ThrowAfterDispatchReleased)
{
throw new InvalidOperationException("Command failed after shutdown started.");
}
return new MxCommandReply
{
SessionId = command.SessionId,
CorrelationId = command.CorrelationId,
Kind = command.Kind,
ProtocolStatus = new ProtocolStatus
{
Code = ProtocolStatusCode.Ok,
Message = "OK",
},
};
});
}
/// <summary>Captures current heartbeat snapshot.</summary>
/// <returns>Current runtime heartbeat snapshot.</returns>
public WorkerRuntimeHeartbeatSnapshot CaptureHeartbeat()
{
lock (gate)
{
return snapshot;
}
}
/// <summary>Drains queued events up to the specified limit.</summary>
/// <param name="maxEvents">Maximum events to drain; 0 drains all.</param>
/// <returns>The drained events.</returns>
public IReadOnlyList<WorkerEvent> DrainEvents(uint maxEvents)
{
lock (gate)
{
int drainCount = maxEvents == 0
? events.Count
: Math.Min(events.Count, checked((int)Math.Min(maxEvents, int.MaxValue)));
List<WorkerEvent> drained = new(drainCount);
for (int index = 0; index < drainCount; index++)
{
drained.Add(events.Dequeue());
}
return drained;
}
}
/// <summary>Drains a pending fault if any.</summary>
/// <returns>Pending fault or null.</returns>
public WorkerFault? DrainFault()
{
return null;
}
/// <summary>Cancels command by correlation ID.</summary>
/// <param name="correlationId">The command correlation ID.</param>
/// <returns>True if cancelled; false otherwise.</returns>
public bool CancelCommand(string correlationId)
{
return false;
}
/// <summary>Requests graceful shutdown.</summary>
public void RequestShutdown()
{
releaseDispatch.Set();
}
/// <summary>Shuts down gracefully within the specified timeout.</summary>
/// <param name="timeout">Shutdown timeout period.</param>
/// <param name="cancellationToken">Cancellation token.</param>
/// <returns>Shutdown result.</returns>
public Task<MxAccessShutdownResult> ShutdownGracefullyAsync(
TimeSpan timeout,
CancellationToken cancellationToken = default)
{
releaseDispatch.Set();
if (ThrowTimeoutOnShutdown)
{
return Task.FromException<MxAccessShutdownResult>(
new TimeoutException("Simulated graceful shutdown timeout."));
}
return Task.FromResult(new MxAccessShutdownResult(Array.Empty<MxAccessShutdownFailure>()));
}
/// <summary>Releases a blocked dispatch.</summary>
public void ReleaseDispatch()
{
releaseDispatch.Set();
}
/// <summary>Sets the current heartbeat snapshot.</summary>
/// <param name="value">The snapshot to set.</param>
public void SetSnapshot(WorkerRuntimeHeartbeatSnapshot value)
{
lock (gate)
{
snapshot = value;
}
}
/// <summary>Enqueues a worker event to be drained.</summary>
/// <param name="workerEvent">The event to enqueue.</param>
public void EnqueueEvent(WorkerEvent workerEvent)
{
lock (gate)
{
events.Enqueue(workerEvent);
}
}
/// <summary>Disposes resources.</summary>
public void Dispose()
{
Disposed = true;
releaseDispatch.Set();
releaseDispatch.Dispose();
DispatchStarted.Dispose();
}
}
private sealed class PipePair : IDisposable private sealed class PipePair : IDisposable
{ {
private readonly NamedPipeServerStream gatewayStream; private readonly NamedPipeServerStream gatewayStream;
@@ -3,6 +3,7 @@ using System.Collections.Generic;
using MxGateway.Contracts.Proto; using MxGateway.Contracts.Proto;
using MxGateway.Worker.MxAccess; using MxGateway.Worker.MxAccess;
using MxGateway.Worker.Sta; using MxGateway.Worker.Sta;
using MxGateway.Worker.Tests.TestSupport;
namespace MxGateway.Worker.Tests.MxAccess; namespace MxGateway.Worker.Tests.MxAccess;
@@ -22,7 +23,7 @@ public sealed class AlarmCommandExecutorTests
private const string CorrelationId = "C"; private const string CorrelationId = "C";
[Fact] [Fact]
public void SubscribeAlarms_routes_to_handler_and_returns_ok() public void SubscribeAlarms_WithHandler_RoutesToHandlerAndReturnsOk()
{ {
FakeAlarmHandler handler = new FakeAlarmHandler(); FakeAlarmHandler handler = new FakeAlarmHandler();
MxAccessCommandExecutor executor = NewExecutor(handler); MxAccessCommandExecutor executor = NewExecutor(handler);
@@ -46,7 +47,7 @@ public sealed class AlarmCommandExecutorTests
} }
[Fact] [Fact]
public void SubscribeAlarms_without_handler_returns_invalid_request() public void SubscribeAlarms_WithoutHandler_ReturnsInvalidRequest()
{ {
MxAccessCommandExecutor executor = NewExecutor(alarmHandler: null); MxAccessCommandExecutor executor = NewExecutor(alarmHandler: null);
@@ -67,7 +68,7 @@ public sealed class AlarmCommandExecutorTests
} }
[Fact] [Fact]
public void SubscribeAlarms_with_empty_expression_returns_invalid_request() public void SubscribeAlarms_WithEmptyExpression_ReturnsInvalidRequest()
{ {
MxAccessCommandExecutor executor = NewExecutor(new FakeAlarmHandler()); MxAccessCommandExecutor executor = NewExecutor(new FakeAlarmHandler());
@@ -88,7 +89,7 @@ public sealed class AlarmCommandExecutorTests
} }
[Fact] [Fact]
public void AcknowledgeAlarm_routes_native_status_into_hresult_and_payload() public void AcknowledgeAlarm_WithHandler_RoutesNativeStatusIntoHresultAndPayload()
{ {
FakeAlarmHandler handler = new FakeAlarmHandler { AcknowledgeReturn = 0 }; FakeAlarmHandler handler = new FakeAlarmHandler { AcknowledgeReturn = 0 };
MxAccessCommandExecutor executor = NewExecutor(handler); MxAccessCommandExecutor executor = NewExecutor(handler);
@@ -121,7 +122,7 @@ public sealed class AlarmCommandExecutorTests
} }
[Fact] [Fact]
public void AcknowledgeAlarm_with_invalid_guid_returns_invalid_request() public void AcknowledgeAlarm_WithInvalidGuid_ReturnsInvalidRequest()
{ {
MxAccessCommandExecutor executor = NewExecutor(new FakeAlarmHandler()); MxAccessCommandExecutor executor = NewExecutor(new FakeAlarmHandler());
@@ -142,7 +143,7 @@ public sealed class AlarmCommandExecutorTests
} }
[Fact] [Fact]
public void AcknowledgeAlarm_with_nonzero_native_status_carries_diagnostic() public void AcknowledgeAlarm_WithNonzeroNativeStatus_CarriesDiagnostic()
{ {
FakeAlarmHandler handler = new FakeAlarmHandler { AcknowledgeReturn = -123 }; FakeAlarmHandler handler = new FakeAlarmHandler { AcknowledgeReturn = -123 };
MxAccessCommandExecutor executor = NewExecutor(handler); MxAccessCommandExecutor executor = NewExecutor(handler);
@@ -165,7 +166,7 @@ public sealed class AlarmCommandExecutorTests
} }
[Fact] [Fact]
public void AcknowledgeAlarmByName_routes_tuple_to_handler() public void AcknowledgeAlarmByName_WithHandler_RoutesTupleToHandler()
{ {
FakeAlarmHandler handler = new FakeAlarmHandler { AcknowledgeReturn = 0 }; FakeAlarmHandler handler = new FakeAlarmHandler { AcknowledgeReturn = 0 };
MxAccessCommandExecutor executor = NewExecutor(handler); MxAccessCommandExecutor executor = NewExecutor(handler);
@@ -198,7 +199,7 @@ public sealed class AlarmCommandExecutorTests
} }
[Fact] [Fact]
public void AcknowledgeAlarmByName_with_empty_name_returns_invalid_request() public void AcknowledgeAlarmByName_WithEmptyName_ReturnsInvalidRequest()
{ {
MxAccessCommandExecutor executor = NewExecutor(new FakeAlarmHandler()); MxAccessCommandExecutor executor = NewExecutor(new FakeAlarmHandler());
@@ -221,7 +222,7 @@ public sealed class AlarmCommandExecutorTests
} }
[Fact] [Fact]
public void QueryActiveAlarms_returns_payload_with_snapshots() public void QueryActiveAlarms_WithHandler_ReturnsPayloadWithSnapshots()
{ {
FakeAlarmHandler handler = new FakeAlarmHandler FakeAlarmHandler handler = new FakeAlarmHandler
{ {
@@ -253,7 +254,7 @@ public sealed class AlarmCommandExecutorTests
} }
[Fact] [Fact]
public void UnsubscribeAlarms_routes_to_handler() public void UnsubscribeAlarms_WithHandler_RoutesToHandler()
{ {
FakeAlarmHandler handler = new FakeAlarmHandler(); FakeAlarmHandler handler = new FakeAlarmHandler();
MxAccessCommandExecutor executor = NewExecutor(handler); MxAccessCommandExecutor executor = NewExecutor(handler);
@@ -273,7 +274,7 @@ public sealed class AlarmCommandExecutorTests
} }
[Fact] [Fact]
public void UnsubscribeAlarms_without_handler_is_ok_noop() public void UnsubscribeAlarms_WithoutHandler_IsOkNoop()
{ {
MxAccessCommandExecutor executor = NewExecutor(alarmHandler: null); MxAccessCommandExecutor executor = NewExecutor(alarmHandler: null);
@@ -291,7 +292,7 @@ public sealed class AlarmCommandExecutorTests
} }
[Fact] [Fact]
public void Acknowledge_handler_throw_returns_mxaccess_failure() public void AcknowledgeAlarm_WhenHandlerThrows_ReturnsMxaccessFailure()
{ {
FakeAlarmHandler handler = new FakeAlarmHandler { AcknowledgeThrow = true }; FakeAlarmHandler handler = new FakeAlarmHandler { AcknowledgeThrow = true };
MxAccessCommandExecutor executor = NewExecutor(handler); MxAccessCommandExecutor executor = NewExecutor(handler);
@@ -357,7 +358,7 @@ public sealed class AlarmCommandExecutorTests
{ {
new object(), new object(),
new NullMxAccessServer(), new NullMxAccessServer(),
new NullEventSink(), new NoopEventSink(),
new MxAccessHandleRegistry(), new MxAccessHandleRegistry(),
System.Environment.CurrentManagedThreadId, System.Environment.CurrentManagedThreadId,
}); });
@@ -386,12 +387,6 @@ public sealed class AlarmCommandExecutorTests
public int ArchestrAUserToId(string userName) => 0; public int ArchestrAUserToId(string userName) => 0;
} }
private sealed class NullEventSink : IMxAccessEventSink
{
public void Attach(object mxAccessComObject, string sessionId) { }
public void Detach() { }
}
private sealed class FakeAlarmHandler : IAlarmCommandHandler private sealed class FakeAlarmHandler : IAlarmCommandHandler
{ {
public string? LastSubscription { get; private set; } public string? LastSubscription { get; private set; }
@@ -13,7 +13,7 @@ namespace MxGateway.Worker.Tests.MxAccess;
public sealed class AlarmCommandHandlerTests public sealed class AlarmCommandHandlerTests
{ {
[Fact] [Fact]
public void Subscribe_creates_consumer_and_calls_subscribe() public void Subscribe_WhenNotYetSubscribed_CreatesConsumerAndCallsSubscribe()
{ {
FakeConsumer consumer = new FakeConsumer(); FakeConsumer consumer = new FakeConsumer();
AlarmCommandHandler handler = new AlarmCommandHandler( AlarmCommandHandler handler = new AlarmCommandHandler(
@@ -27,7 +27,7 @@ public sealed class AlarmCommandHandlerTests
} }
[Fact] [Fact]
public void Second_subscribe_without_unsubscribe_throws() public void Subscribe_WhenAlreadySubscribed_Throws()
{ {
FakeConsumer consumer = new FakeConsumer(); FakeConsumer consumer = new FakeConsumer();
AlarmCommandHandler handler = new AlarmCommandHandler( AlarmCommandHandler handler = new AlarmCommandHandler(
@@ -40,7 +40,7 @@ public sealed class AlarmCommandHandlerTests
} }
[Fact] [Fact]
public void Subscribe_disposes_consumer_when_underlying_subscribe_throws() public void Subscribe_WhenUnderlyingSubscribeThrows_DisposesConsumer()
{ {
FakeConsumer consumer = new FakeConsumer { ThrowOnSubscribe = true }; FakeConsumer consumer = new FakeConsumer { ThrowOnSubscribe = true };
AlarmCommandHandler handler = new AlarmCommandHandler( AlarmCommandHandler handler = new AlarmCommandHandler(
@@ -54,7 +54,7 @@ public sealed class AlarmCommandHandlerTests
} }
[Fact] [Fact]
public void Unsubscribe_disposes_consumer_and_clears_state() public void Unsubscribe_WhenSubscribed_DisposesConsumerAndClearsState()
{ {
FakeConsumer consumer = new FakeConsumer(); FakeConsumer consumer = new FakeConsumer();
AlarmCommandHandler handler = new AlarmCommandHandler( AlarmCommandHandler handler = new AlarmCommandHandler(
@@ -69,7 +69,7 @@ public sealed class AlarmCommandHandlerTests
} }
[Fact] [Fact]
public void Unsubscribe_without_prior_subscribe_is_noop() public void Unsubscribe_WithoutPriorSubscribe_IsNoop()
{ {
AlarmCommandHandler handler = new AlarmCommandHandler( AlarmCommandHandler handler = new AlarmCommandHandler(
new MxAccessEventQueue(), new MxAccessEventQueue(),
@@ -79,7 +79,7 @@ public sealed class AlarmCommandHandlerTests
} }
[Fact] [Fact]
public void Acknowledge_forwards_to_consumer_with_full_operator_identity() public void Acknowledge_WhenSubscribed_ForwardsToConsumerWithFullOperatorIdentity()
{ {
FakeConsumer consumer = new FakeConsumer { AcknowledgeReturn = 0 }; FakeConsumer consumer = new FakeConsumer { AcknowledgeReturn = 0 };
AlarmCommandHandler handler = new AlarmCommandHandler( AlarmCommandHandler handler = new AlarmCommandHandler(
@@ -96,7 +96,7 @@ public sealed class AlarmCommandHandlerTests
} }
[Fact] [Fact]
public void Acknowledge_before_subscribe_throws_invalid_op() public void Acknowledge_BeforeSubscribe_ThrowsInvalidOperation()
{ {
AlarmCommandHandler handler = new AlarmCommandHandler( AlarmCommandHandler handler = new AlarmCommandHandler(
new MxAccessEventQueue(), new MxAccessEventQueue(),
@@ -107,7 +107,7 @@ public sealed class AlarmCommandHandlerTests
} }
[Fact] [Fact]
public void QueryActive_returns_mapped_proto_snapshots() public void QueryActive_WhenConsumerHasAlarms_ReturnsMappedProtoSnapshots()
{ {
FakeConsumer consumer = new FakeConsumer FakeConsumer consumer = new FakeConsumer
{ {
@@ -138,7 +138,7 @@ public sealed class AlarmCommandHandlerTests
} }
[Fact] [Fact]
public void QueryActive_filters_by_prefix() public void QueryActive_WithPrefix_FiltersByPrefix()
{ {
FakeConsumer consumer = new FakeConsumer FakeConsumer consumer = new FakeConsumer
{ {
@@ -160,7 +160,7 @@ public sealed class AlarmCommandHandlerTests
} }
[Fact] [Fact]
public void Dispose_unsubscribes_and_disposes_consumer() public void Dispose_WhenSubscribed_UnsubscribesAndDisposesConsumer()
{ {
FakeConsumer consumer = new FakeConsumer(); FakeConsumer consumer = new FakeConsumer();
AlarmCommandHandler handler = new AlarmCommandHandler( AlarmCommandHandler handler = new AlarmCommandHandler(
@@ -18,7 +18,7 @@ public sealed class AlarmDispatcherTests
private const string SessionId = "session-001"; private const string SessionId = "session-001";
[Fact] [Fact]
public void TransitionEvent_lands_in_queue_with_mapped_fields() public void OnTransition_WhenAlarmTransitionRaised_LandsInQueueWithMappedFields()
{ {
FakeAlarmConsumer consumer = new FakeAlarmConsumer(); FakeAlarmConsumer consumer = new FakeAlarmConsumer();
MxAccessEventQueue queue = new MxAccessEventQueue(); MxAccessEventQueue queue = new MxAccessEventQueue();
@@ -64,7 +64,7 @@ public sealed class AlarmDispatcherTests
} }
[Fact] [Fact]
public void Consecutive_unchanged_state_does_not_emit_a_transition() public void OnTransition_WithConsecutiveUnchangedState_DoesNotEmitTransition()
{ {
// Mapper.MapTransition returns Unspecified when the state didn't // Mapper.MapTransition returns Unspecified when the state didn't
// change; the dispatcher should drop the event before queueing. // change; the dispatcher should drop the event before queueing.
@@ -94,7 +94,7 @@ public sealed class AlarmDispatcherTests
[InlineData(MxAlarmStateKind.UnackAlm, MxAlarmStateKind.AckAlm, AlarmTransitionKind.Acknowledge)] [InlineData(MxAlarmStateKind.UnackAlm, MxAlarmStateKind.AckAlm, AlarmTransitionKind.Acknowledge)]
[InlineData(MxAlarmStateKind.UnackAlm, MxAlarmStateKind.UnackRtn, AlarmTransitionKind.Clear)] [InlineData(MxAlarmStateKind.UnackAlm, MxAlarmStateKind.UnackRtn, AlarmTransitionKind.Clear)]
[InlineData(MxAlarmStateKind.UnackRtn, MxAlarmStateKind.UnackAlm, AlarmTransitionKind.Raise)] [InlineData(MxAlarmStateKind.UnackRtn, MxAlarmStateKind.UnackAlm, AlarmTransitionKind.Raise)]
public void Transition_kind_follows_state_table( public void MapTransition_ForEachStatePair_FollowsStateTable(
MxAlarmStateKind previous, MxAlarmStateKind previous,
MxAlarmStateKind current, MxAlarmStateKind current,
AlarmTransitionKind expected) AlarmTransitionKind expected)
@@ -123,7 +123,7 @@ public sealed class AlarmDispatcherTests
} }
[Fact] [Fact]
public void Subscribe_forwards_to_consumer() public void Subscribe_WhenInvoked_ForwardsToConsumer()
{ {
FakeAlarmConsumer consumer = new FakeAlarmConsumer(); FakeAlarmConsumer consumer = new FakeAlarmConsumer();
using AlarmDispatcher dispatcher = new AlarmDispatcher( using AlarmDispatcher dispatcher = new AlarmDispatcher(
@@ -136,7 +136,7 @@ public sealed class AlarmDispatcherTests
} }
[Fact] [Fact]
public void Acknowledge_forwards_to_consumer_with_full_operator_identity() public void Acknowledge_WhenInvoked_ForwardsToConsumerWithFullOperatorIdentity()
{ {
FakeAlarmConsumer consumer = new FakeAlarmConsumer(); FakeAlarmConsumer consumer = new FakeAlarmConsumer();
consumer.AcknowledgeReturn = 0; consumer.AcknowledgeReturn = 0;
@@ -159,7 +159,7 @@ public sealed class AlarmDispatcherTests
} }
[Fact] [Fact]
public void AcknowledgeByName_forwards_to_consumer_with_full_tuple() public void AcknowledgeByName_WhenInvoked_ForwardsToConsumerWithFullTuple()
{ {
FakeAlarmConsumer consumer = new FakeAlarmConsumer { AcknowledgeReturn = 0 }; FakeAlarmConsumer consumer = new FakeAlarmConsumer { AcknowledgeReturn = 0 };
using AlarmDispatcher dispatcher = new AlarmDispatcher( using AlarmDispatcher dispatcher = new AlarmDispatcher(
@@ -185,7 +185,7 @@ public sealed class AlarmDispatcherTests
} }
[Fact] [Fact]
public void SnapshotActiveAlarms_maps_records_to_protos() public void SnapshotActiveAlarms_WhenConsumerHasRecords_MapsRecordsToProtos()
{ {
FakeAlarmConsumer consumer = new FakeAlarmConsumer(); FakeAlarmConsumer consumer = new FakeAlarmConsumer();
DateTime ts = new DateTime(2026, 5, 1, 17, 26, 14, 709, DateTimeKind.Utc); DateTime ts = new DateTime(2026, 5, 1, 17, 26, 14, 709, DateTimeKind.Utc);
@@ -233,7 +233,7 @@ public sealed class AlarmDispatcherTests
} }
[Fact] [Fact]
public void Dispose_unsubscribes_handler_and_disposes_consumer() public void Dispose_WhenSubscribed_UnsubscribesHandlerAndDisposesConsumer()
{ {
FakeAlarmConsumer consumer = new FakeAlarmConsumer(); FakeAlarmConsumer consumer = new FakeAlarmConsumer();
MxAccessEventQueue queue = new MxAccessEventQueue(); MxAccessEventQueue queue = new MxAccessEventQueue();
@@ -15,7 +15,7 @@ namespace MxGateway.Worker.Tests.MxAccess;
public sealed class AlarmRecordTransitionMapperTests public sealed class AlarmRecordTransitionMapperTests
{ {
[Fact] [Fact]
public void ComposeFullReference_uses_provider_bang_group_dot_name_format() public void ComposeFullReference_WithProviderAndGroup_UsesProviderBangGroupDotNameFormat()
{ {
string reference = AlarmRecordTransitionMapper.ComposeFullReference( string reference = AlarmRecordTransitionMapper.ComposeFullReference(
providerName: "GalaxyAlarmProvider", providerName: "GalaxyAlarmProvider",
@@ -25,7 +25,7 @@ public sealed class AlarmRecordTransitionMapperTests
} }
[Fact] [Fact]
public void ComposeFullReference_drops_provider_when_empty() public void ComposeFullReference_WithEmptyProvider_DropsProvider()
{ {
string reference = AlarmRecordTransitionMapper.ComposeFullReference( string reference = AlarmRecordTransitionMapper.ComposeFullReference(
providerName: null, groupName: "Tank01", alarmName: "Level.HiHi"); providerName: null, groupName: "Tank01", alarmName: "Level.HiHi");
@@ -33,7 +33,7 @@ public sealed class AlarmRecordTransitionMapperTests
} }
[Fact] [Fact]
public void ComposeFullReference_drops_group_when_empty() public void ComposeFullReference_WithEmptyGroup_DropsGroup()
{ {
string reference = AlarmRecordTransitionMapper.ComposeFullReference( string reference = AlarmRecordTransitionMapper.ComposeFullReference(
providerName: "GalaxyAlarmProvider", groupName: null, alarmName: "GlobalAlarm"); providerName: "GalaxyAlarmProvider", groupName: null, alarmName: "GlobalAlarm");
@@ -41,7 +41,7 @@ public sealed class AlarmRecordTransitionMapperTests
} }
[Fact] [Fact]
public void ComposeFullReference_returns_alarm_name_when_provider_and_group_empty() public void ComposeFullReference_WithEmptyProviderAndGroup_ReturnsAlarmName()
{ {
string reference = AlarmRecordTransitionMapper.ComposeFullReference( string reference = AlarmRecordTransitionMapper.ComposeFullReference(
providerName: null, groupName: null, alarmName: "Bare"); providerName: null, groupName: null, alarmName: "Bare");
@@ -58,7 +58,7 @@ public sealed class AlarmRecordTransitionMapperTests
[InlineData("UNKNOWN", MxAlarmStateKind.Unspecified)] [InlineData("UNKNOWN", MxAlarmStateKind.Unspecified)]
[InlineData("", MxAlarmStateKind.Unspecified)] [InlineData("", MxAlarmStateKind.Unspecified)]
[InlineData(null, MxAlarmStateKind.Unspecified)] [InlineData(null, MxAlarmStateKind.Unspecified)]
public void ParseStateKind_decodes_state_strings(string? input, MxAlarmStateKind expected) public void ParseStateKind_ForEachStateString_DecodesStateKind(string? input, MxAlarmStateKind expected)
{ {
Assert.Equal(expected, AlarmRecordTransitionMapper.ParseStateKind(input)); Assert.Equal(expected, AlarmRecordTransitionMapper.ParseStateKind(input));
} }
@@ -83,7 +83,7 @@ public sealed class AlarmRecordTransitionMapperTests
[InlineData(MxAlarmStateKind.UnackAlm, MxAlarmStateKind.UnackAlm, AlarmTransitionKind.Unspecified)] [InlineData(MxAlarmStateKind.UnackAlm, MxAlarmStateKind.UnackAlm, AlarmTransitionKind.Unspecified)]
// Current=Unspecified → Unspecified. // Current=Unspecified → Unspecified.
[InlineData(MxAlarmStateKind.UnackAlm, MxAlarmStateKind.Unspecified, AlarmTransitionKind.Unspecified)] [InlineData(MxAlarmStateKind.UnackAlm, MxAlarmStateKind.Unspecified, AlarmTransitionKind.Unspecified)]
public void MapTransition_decides_proto_kind( public void MapTransition_ForEachStatePair_DecidesProtoKind(
MxAlarmStateKind previous, MxAlarmStateKind previous,
MxAlarmStateKind current, MxAlarmStateKind current,
AlarmTransitionKind expected) AlarmTransitionKind expected)
@@ -92,7 +92,7 @@ public sealed class AlarmRecordTransitionMapperTests
} }
[Fact] [Fact]
public void ParseTransitionTimestampUtc_assembles_utc_from_xml_fields() public void ParseTransitionTimestampUtc_WithValidXmlFields_AssemblesUtc()
{ {
// Captured payload from probe (2026-05-01): EDT producer, GMTOFFSET=240, DSTADJUST=0. // Captured payload from probe (2026-05-01): EDT producer, GMTOFFSET=240, DSTADJUST=0.
// Local 13:26:14.709 + 240 minutes (4h) = 17:26:14.709 UTC. // Local 13:26:14.709 + 240 minutes (4h) = 17:26:14.709 UTC.
@@ -110,7 +110,7 @@ public sealed class AlarmRecordTransitionMapperTests
} }
[Fact] [Fact]
public void ParseTransitionTimestampUtc_returns_min_value_on_unparseable_inputs() public void ParseTransitionTimestampUtc_WithUnparseableInputs_ReturnsMinValue()
{ {
Assert.Equal(DateTime.MinValue, Assert.Equal(DateTime.MinValue,
AlarmRecordTransitionMapper.ParseTransitionTimestampUtc(null, null, 0, 0)); AlarmRecordTransitionMapper.ParseTransitionTimestampUtc(null, null, 0, 0));
@@ -6,6 +6,7 @@ using System.Threading.Tasks;
using MxGateway.Contracts.Proto; using MxGateway.Contracts.Proto;
using MxGateway.Worker.MxAccess; using MxGateway.Worker.MxAccess;
using MxGateway.Worker.Sta; using MxGateway.Worker.Sta;
using MxGateway.Worker.Tests.TestSupport;
namespace MxGateway.Worker.Tests.MxAccess; namespace MxGateway.Worker.Tests.MxAccess;
@@ -1102,35 +1103,4 @@ public sealed class MxAccessCommandExecutorTests
} }
} }
/// <summary>No-operation event sink for testing.</summary>
private sealed class NoopEventSink : IMxAccessEventSink
{
/// <summary>Attaches to a MXAccess COM object (no-op in test).</summary>
/// <param name="mxAccessComObject">The MXAccess COM object to attach to.</param>
/// <param name="sessionId">Identifier of the session.</param>
public void Attach(
object mxAccessComObject,
string sessionId)
{
}
/// <summary>Detaches from the MXAccess COM object (no-op in test).</summary>
public void Detach()
{
}
}
/// <summary>No-operation STA apartment initializer for testing.</summary>
private sealed class NoopComApartmentInitializer : IStaComApartmentInitializer
{
/// <summary>Initializes the STA apartment (no-op in test).</summary>
public void Initialize()
{
}
/// <summary>Uninitializes the STA apartment (no-op in test).</summary>
public void Uninitialize()
{
}
}
} }
@@ -46,6 +46,53 @@ public sealed class MxAccessEventQueueTests
Assert.Equal(1, queue.Count); Assert.Equal(1, queue.Count);
} }
/// <summary>Verifies that Drain with maxEvents 0 drains every queued event.</summary>
[Fact]
public void Drain_WithZeroMaxEvents_DrainsAllEvents()
{
MxAccessEventQueue queue = new(capacity: 4);
queue.Enqueue(CreateEvent(MxEventFamily.OnDataChange, itemHandle: 10));
queue.Enqueue(CreateEvent(MxEventFamily.OnDataChange, itemHandle: 11));
queue.Enqueue(CreateEvent(MxEventFamily.OnDataChange, itemHandle: 12));
IReadOnlyList<WorkerEvent> drained = queue.Drain(maxEvents: 0);
Assert.Equal(3, drained.Count);
Assert.Equal(new[] { 10, 11, 12 }, new[]
{
drained[0].Event.ItemHandle,
drained[1].Event.ItemHandle,
drained[2].Event.ItemHandle,
});
Assert.Equal(0, queue.Count);
}
/// <summary>Verifies that draining an empty queue returns an empty list.</summary>
[Fact]
public void Drain_WhenQueueIsEmpty_ReturnsEmptyList()
{
MxAccessEventQueue queue = new(capacity: 4);
Assert.Empty(queue.Drain(maxEvents: 0));
Assert.Empty(queue.Drain(maxEvents: 5));
Assert.Equal(0, queue.Count);
}
/// <summary>Verifies that Enqueue is rejected after a fault is recorded manually.</summary>
[Fact]
public void Enqueue_AfterRecordFault_ThrowsInvalidOperationException()
{
MxAccessEventQueue queue = new(capacity: 4);
queue.RecordFault(new WorkerFault
{
Category = WorkerFaultCategory.MxaccessEventConversionFailed,
});
Assert.Throws<InvalidOperationException>(
() => queue.Enqueue(CreateEvent(MxEventFamily.OnDataChange, itemHandle: 10)));
Assert.Equal(0, queue.Count);
}
/// <summary>Verifies that Enqueue records an overflow fault and rejects new events when capacity is exceeded.</summary> /// <summary>Verifies that Enqueue records an overflow fault and rejects new events when capacity is exceeded.</summary>
[Fact] [Fact]
public void Enqueue_WhenCapacityIsExceeded_RecordsOverflowFaultAndRejectsNewEvents() public void Enqueue_WhenCapacityIsExceeded_RecordsOverflowFaultAndRejectsNewEvents()
@@ -6,6 +6,7 @@ using System.Threading.Tasks;
using MxGateway.Contracts.Proto; using MxGateway.Contracts.Proto;
using MxGateway.Worker.MxAccess; using MxGateway.Worker.MxAccess;
using MxGateway.Worker.Sta; using MxGateway.Worker.Sta;
using MxGateway.Worker.Tests.TestSupport;
namespace MxGateway.Worker.Tests.MxAccess; namespace MxGateway.Worker.Tests.MxAccess;
@@ -223,10 +224,12 @@ public sealed class MxAccessStaSessionTests
} }
/// <summary> /// <summary>
/// Gap 1: Verifies that when MxAccessStaSession is created with the default /// Gap 1: Verifies that when MxAccessStaSession is created without an alarm
/// parameterless constructor (no alarm factory), SubscribeAlarms returns /// command handler factory, SubscribeAlarms returns InvalidRequest with the
/// InvalidRequest with "alarm consumer not configured" diagnostic. /// exact "SubscribeAlarms requires an alarm command handler; the worker was
/// This validates the baseline before the fix. /// constructed without one." diagnostic. The full phrase is asserted so the
/// test fails if the diagnostic regresses to a misleading message that still
/// happens to contain the word "alarm".
/// </summary> /// </summary>
[Fact] [Fact]
public async Task StartAsync_WithoutAlarmCommandHandlerFactory_SubscribeAlarmsReturnsInvalidRequest() public async Task StartAsync_WithoutAlarmCommandHandlerFactory_SubscribeAlarmsReturnsInvalidRequest()
@@ -254,7 +257,9 @@ public sealed class MxAccessStaSessionTests
MxCommandReply reply = await session.DispatchAsync(subscribeCommand); MxCommandReply reply = await session.DispatchAsync(subscribeCommand);
Assert.Equal(ProtocolStatusCode.InvalidRequest, reply.ProtocolStatus.Code); Assert.Equal(ProtocolStatusCode.InvalidRequest, reply.ProtocolStatus.Code);
Assert.Contains("alarm", reply.DiagnosticMessage, StringComparison.OrdinalIgnoreCase); Assert.Equal(
"SubscribeAlarms requires an alarm command handler; the worker was constructed without one.",
reply.DiagnosticMessage);
} }
/// <summary> /// <summary>
@@ -411,26 +416,6 @@ public sealed class MxAccessStaSessionTests
MxAccessStaSession.AssertOnAlarmConsumerThread(expectedThreadId: null, actualThreadId: 123); MxAccessStaSession.AssertOnAlarmConsumerThread(expectedThreadId: null, actualThreadId: 123);
} }
/// <summary>
/// Noop STA COM apartment initializer for testing.
/// </summary>
private sealed class NoopComApartmentInitializer : IStaComApartmentInitializer
{
/// <summary>
/// Initializes the COM apartment (no-op).
/// </summary>
public void Initialize()
{
}
/// <summary>
/// Uninitializes the COM apartment (no-op).
/// </summary>
public void Uninitialize()
{
}
}
/// <summary> /// <summary>
/// Fake alarm command handler that records calls and tracks poll thread. /// Fake alarm command handler that records calls and tracks poll thread.
/// </summary> /// </summary>
@@ -35,21 +35,21 @@ public sealed class WnWrapAlarmConsumerXmlTests
"<?xml version=\"1.0\"?><ALARM_RECORDS COUNT=\"0\"></ALARM_RECORDS>"; "<?xml version=\"1.0\"?><ALARM_RECORDS COUNT=\"0\"></ALARM_RECORDS>";
[Fact] [Fact]
public void ParseSnapshotXml_returns_empty_dictionary_for_empty_payload() public void ParseSnapshotXml_WithEmptyPayload_ReturnsEmptyDictionary()
{ {
var records = WnWrapAlarmConsumer.ParseSnapshotXml(EmptyXml); var records = WnWrapAlarmConsumer.ParseSnapshotXml(EmptyXml);
Assert.Empty(records); Assert.Empty(records);
} }
[Fact] [Fact]
public void ParseSnapshotXml_returns_empty_dictionary_for_null_or_whitespace() public void ParseSnapshotXml_WithNullOrWhitespace_ReturnsEmptyDictionary()
{ {
Assert.Empty(WnWrapAlarmConsumer.ParseSnapshotXml("")); Assert.Empty(WnWrapAlarmConsumer.ParseSnapshotXml(""));
Assert.Empty(WnWrapAlarmConsumer.ParseSnapshotXml(" ")); Assert.Empty(WnWrapAlarmConsumer.ParseSnapshotXml(" "));
} }
[Fact] [Fact]
public void ParseSnapshotXml_decodes_single_active_alarm_record() public void ParseSnapshotXml_WithSingleActiveAlarm_DecodesRecord()
{ {
var records = WnWrapAlarmConsumer.ParseSnapshotXml(SingleAlarmActiveXml); var records = WnWrapAlarmConsumer.ParseSnapshotXml(SingleAlarmActiveXml);
@@ -74,7 +74,7 @@ public sealed class WnWrapAlarmConsumerXmlTests
} }
[Fact] [Fact]
public void ParseSnapshotXml_silently_drops_records_with_invalid_guids() public void ParseSnapshotXml_WithInvalidGuids_SilentlyDropsRecords()
{ {
string xml = SingleAlarmActiveXml.Replace( string xml = SingleAlarmActiveXml.Replace(
"<GUID>BCC4705395424D65BDAABCDEA6A32A73</GUID>", "<GUID>BCC4705395424D65BDAABCDEA6A32A73</GUID>",
@@ -85,7 +85,7 @@ public sealed class WnWrapAlarmConsumerXmlTests
[Theory] [Theory]
[InlineData("BCC4705395424D65BDAABCDEA6A32A73", "BCC47053-9542-4D65-BDAA-BCDEA6A32A73")] [InlineData("BCC4705395424D65BDAABCDEA6A32A73", "BCC47053-9542-4D65-BDAA-BCDEA6A32A73")]
[InlineData("00000000000000000000000000000000", "00000000-0000-0000-0000-000000000000")] [InlineData("00000000000000000000000000000000", "00000000-0000-0000-0000-000000000000")]
public void TryParseHexGuid_handles_dashless_32_char_hex(string hex, string expected) public void TryParseHexGuid_WithDashless32CharHex_Parses(string hex, string expected)
{ {
Assert.True(WnWrapAlarmConsumer.TryParseHexGuid(hex, out Guid guid)); Assert.True(WnWrapAlarmConsumer.TryParseHexGuid(hex, out Guid guid));
Assert.Equal(new Guid(expected), guid); Assert.Equal(new Guid(expected), guid);
@@ -93,7 +93,7 @@ public sealed class WnWrapAlarmConsumerXmlTests
[Theory] [Theory]
[InlineData("BCC47053-9542-4D65-BDAA-BCDEA6A32A73")] [InlineData("BCC47053-9542-4D65-BDAA-BCDEA6A32A73")]
public void TryParseHexGuid_accepts_canonical_dashed_form(string canonical) public void TryParseHexGuid_WithCanonicalDashedForm_Accepts(string canonical)
{ {
Assert.True(WnWrapAlarmConsumer.TryParseHexGuid(canonical, out Guid guid)); Assert.True(WnWrapAlarmConsumer.TryParseHexGuid(canonical, out Guid guid));
Assert.Equal(new Guid(canonical), guid); Assert.Equal(new Guid(canonical), guid);
@@ -106,7 +106,7 @@ public sealed class WnWrapAlarmConsumerXmlTests
[InlineData("nope")] [InlineData("nope")]
[InlineData("0123456789ABCDEF")] // too short [InlineData("0123456789ABCDEF")] // too short
[InlineData("BCC4705395424D65BDAABCDEA6A32A73XX")] // too long [InlineData("BCC4705395424D65BDAABCDEA6A32A73XX")] // too long
public void TryParseHexGuid_rejects_invalid_input(string? hex) public void TryParseHexGuid_WithInvalidInput_Rejects(string? hex)
{ {
Assert.False(WnWrapAlarmConsumer.TryParseHexGuid(hex, out Guid guid)); Assert.False(WnWrapAlarmConsumer.TryParseHexGuid(hex, out Guid guid));
Assert.Equal(Guid.Empty, guid); Assert.Equal(Guid.Empty, guid);
@@ -120,7 +120,7 @@ public sealed class WnWrapAlarmConsumerXmlTests
/// callback must not exist on the type. /// callback must not exist on the type.
/// </summary> /// </summary>
[Fact] [Fact]
public void WnWrapAlarmConsumer_has_no_internal_timer_field() public void WnWrapAlarmConsumer_ByReflection_HasNoInternalTimerField()
{ {
FieldInfo[] fields = typeof(WnWrapAlarmConsumer) FieldInfo[] fields = typeof(WnWrapAlarmConsumer)
.GetFields(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic); .GetFields(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic);
@@ -138,7 +138,7 @@ public sealed class WnWrapAlarmConsumerXmlTests
/// footgun structurally unreachable. /// footgun structurally unreachable.
/// </summary> /// </summary>
[Fact] [Fact]
public void WnWrapAlarmConsumer_exposes_no_poll_interval_constructor_parameter() public void WnWrapAlarmConsumer_ByReflection_ExposesNoPollIntervalConstructorParameter()
{ {
foreach (ConstructorInfo constructor in typeof(WnWrapAlarmConsumer) foreach (ConstructorInfo constructor in typeof(WnWrapAlarmConsumer)
.GetConstructors(BindingFlags.Instance | BindingFlags.Public)) .GetConstructors(BindingFlags.Instance | BindingFlags.Public))
@@ -29,9 +29,16 @@ public sealed class WorkerProjectReferenceTests
Assert.Equal("x86", ElementValue(project, "PlatformTarget")); Assert.Equal("x86", ElementValue(project, "PlatformTarget"));
} }
/// <summary>Verifies that MXAccess interop reference exists only in the worker project.</summary> /// <summary>
/// Verifies that the MXAccess COM interop is referenced only by the
/// worker project and its test project — never by the gateway server
/// or the contracts project. The gateway must never load MXAccess COM
/// directly (see <c>gateway.md</c>); the worker test project
/// legitimately references the interop so it can exercise the
/// COM-facing worker code (e.g. <c>WnWrapAlarmConsumer</c>).
/// </summary>
[Fact] [Fact]
public void MxAccessInteropReference_ExistsOnlyInWorkerProject() public void MxAccessInteropReference_ExistsOnlyInWorkerAndWorkerTestProjects()
{ {
DirectoryInfo repositoryRoot = FindRepositoryRoot(); DirectoryInfo repositoryRoot = FindRepositoryRoot();
string[] projectFiles = Directory.GetFiles(repositoryRoot.FullName, "*.csproj", SearchOption.AllDirectories) string[] projectFiles = Directory.GetFiles(repositoryRoot.FullName, "*.csproj", SearchOption.AllDirectories)
@@ -42,9 +49,12 @@ public sealed class WorkerProjectReferenceTests
IReadOnlyList<string> projectsWithMxAccessReference = projectFiles IReadOnlyList<string> projectsWithMxAccessReference = projectFiles
.Where(ProjectReferencesMxAccess) .Where(ProjectReferencesMxAccess)
.Select(path => Path.GetFileNameWithoutExtension(path)) .Select(path => Path.GetFileNameWithoutExtension(path))
.OrderBy(name => name, StringComparer.Ordinal)
.ToArray(); .ToArray();
Assert.Equal(["MxGateway.Worker"], projectsWithMxAccessReference); Assert.Equal(
["MxGateway.Worker", "MxGateway.Worker.Tests"],
projectsWithMxAccessReference);
} }
private static bool ProjectReferencesMxAccess(string projectPath) private static bool ProjectReferencesMxAccess(string projectPath)
@@ -6,6 +6,7 @@ using System.Threading;
using System.Threading.Tasks; using System.Threading.Tasks;
using MxGateway.Contracts.Proto; using MxGateway.Contracts.Proto;
using MxGateway.Worker.Sta; using MxGateway.Worker.Sta;
using MxGateway.Worker.Tests.TestSupport;
namespace MxGateway.Worker.Tests.Sta; namespace MxGateway.Worker.Tests.Sta;
@@ -87,10 +88,16 @@ public sealed class StaCommandDispatcherTests
} }
/// <summary> /// <summary>
/// Verifies cancellation after execution starts still returns the reply once execution completes. /// Verifies cancellation cannot abort a command already executing on the STA:
/// once the executor has started, cancelling the token is a no-op and the
/// command still runs to completion and returns its normal reply. This
/// matches <c>gateway.md</c>: cancellation "cannot safely abort an in-flight
/// COM call on the STA". The test does not — and cannot — distinguish "cancel
/// observed and ignored" from "cancel never checked"; it only proves the
/// in-flight command is not aborted.
/// </summary> /// </summary>
[Fact] [Fact]
public async Task DispatchAsync_WhenCanceledAfterExecutionStarts_StillReturnsLateReply() public async Task DispatchAsync_WhenCanceledWhileExecuting_DoesNotAbortInFlightCommand()
{ {
using StaRuntime runtime = CreateRuntime(); using StaRuntime runtime = CreateRuntime();
runtime.Start(); runtime.Start();
@@ -341,20 +348,4 @@ public sealed class StaCommandDispatcherTests
throw exception; throw exception;
} }
} }
/// <summary>
/// No-op COM apartment initializer for testing.
/// </summary>
private sealed class NoopComApartmentInitializer : IStaComApartmentInitializer
{
/// <inheritdoc />
public void Initialize()
{
}
/// <inheritdoc />
public void Uninitialize()
{
}
}
} }
@@ -0,0 +1,217 @@
using System;
using System.Collections.Generic;
using System.Threading;
using System.Threading.Tasks;
using Google.Protobuf.WellKnownTypes;
using MxGateway.Contracts.Proto;
using MxGateway.Worker.Ipc;
using MxGateway.Worker.MxAccess;
using MxGateway.Worker.Sta;
namespace MxGateway.Worker.Tests.TestSupport;
/// <summary>
/// Single configurable <see cref="IWorkerRuntimeSession"/> test double shared by
/// the IPC tests. Replaces the two independent (and previously diverged)
/// <c>FakeRuntimeSession</c> copies in WorkerPipeSessionTests and
/// WorkerPipeClientTests: one supported dispatch blocking and event enqueue, the
/// other did not. This consolidated double supports every configuration both
/// call sites needed, so a minimal caller simply leaves the options unset.
/// </summary>
internal sealed class FakeRuntimeSession : IWorkerRuntimeSession
{
private readonly ManualResetEventSlim releaseDispatch = new(false);
private readonly object gate = new();
private readonly Queue<WorkerEvent> events = new();
private WorkerRuntimeHeartbeatSnapshot snapshot = new(
DateTimeOffset.UtcNow,
pendingCommandCount: 0,
outboundEventQueueDepth: 0,
lastEventSequence: 0,
currentCommandCorrelationId: string.Empty);
/// <summary>Gets the event signaled when dispatch begins.</summary>
public ManualResetEventSlim DispatchStarted { get; } = new(false);
/// <summary>Blocks dispatch execution until explicitly released.</summary>
public bool BlockDispatch { get; set; }
/// <summary>Gets or sets whether to throw an exception after dispatch is released.</summary>
public bool ThrowAfterDispatchReleased { get; set; }
/// <summary>Gets or sets whether ShutdownGracefullyAsync throws a TimeoutException.</summary>
public bool ThrowTimeoutOnShutdown { get; set; }
/// <summary>Gets a value indicating whether Dispose was called.</summary>
public bool Disposed { get; private set; }
/// <summary>Starts the worker session with the given session ID and process ID.</summary>
/// <param name="sessionId">The session identifier.</param>
/// <param name="workerProcessId">The worker process ID.</param>
/// <param name="cancellationToken">Cancellation token.</param>
/// <returns>Worker ready response.</returns>
public Task<WorkerReady> StartAsync(
string sessionId,
int workerProcessId,
CancellationToken cancellationToken = default)
{
return Task.FromResult(new WorkerReady
{
WorkerProcessId = workerProcessId,
MxaccessProgid = MxAccessInteropInfo.ProgId,
MxaccessClsid = MxAccessInteropInfo.Clsid,
ReadyTimestamp = Timestamp.FromDateTimeOffset(DateTimeOffset.UtcNow),
});
}
/// <summary>Dispatches a command to the STA thread.</summary>
/// <param name="command">The command to dispatch.</param>
/// <returns>The command reply.</returns>
public Task<MxCommandReply> DispatchAsync(StaCommand command)
{
return Task.Run(
() =>
{
SetSnapshot(new WorkerRuntimeHeartbeatSnapshot(
DateTimeOffset.UtcNow,
pendingCommandCount: 0,
outboundEventQueueDepth: 0,
lastEventSequence: 0,
command.CorrelationId));
DispatchStarted.Set();
if (BlockDispatch)
{
releaseDispatch.Wait(TimeSpan.FromSeconds(5));
}
SetSnapshot(new WorkerRuntimeHeartbeatSnapshot(
DateTimeOffset.UtcNow,
pendingCommandCount: 0,
outboundEventQueueDepth: 0,
lastEventSequence: 0,
currentCommandCorrelationId: string.Empty));
if (ThrowAfterDispatchReleased)
{
throw new InvalidOperationException("Command failed after shutdown started.");
}
return new MxCommandReply
{
SessionId = command.SessionId,
CorrelationId = command.CorrelationId,
Kind = command.Kind,
ProtocolStatus = new ProtocolStatus
{
Code = ProtocolStatusCode.Ok,
Message = "OK",
},
};
});
}
/// <summary>Captures current heartbeat snapshot.</summary>
/// <returns>Current runtime heartbeat snapshot.</returns>
public WorkerRuntimeHeartbeatSnapshot CaptureHeartbeat()
{
lock (gate)
{
return snapshot;
}
}
/// <summary>Drains queued events up to the specified limit.</summary>
/// <param name="maxEvents">Maximum events to drain; 0 drains all.</param>
/// <returns>The drained events.</returns>
public IReadOnlyList<WorkerEvent> DrainEvents(uint maxEvents)
{
lock (gate)
{
int drainCount = maxEvents == 0
? events.Count
: Math.Min(events.Count, checked((int)Math.Min(maxEvents, int.MaxValue)));
List<WorkerEvent> drained = new(drainCount);
for (int index = 0; index < drainCount; index++)
{
drained.Add(events.Dequeue());
}
return drained;
}
}
/// <summary>Drains a pending fault if any.</summary>
/// <returns>Pending fault or null.</returns>
public WorkerFault? DrainFault()
{
return null;
}
/// <summary>Cancels command by correlation ID.</summary>
/// <param name="correlationId">The command correlation ID.</param>
/// <returns>True if cancelled; false otherwise.</returns>
public bool CancelCommand(string correlationId)
{
return false;
}
/// <summary>Requests graceful shutdown.</summary>
public void RequestShutdown()
{
releaseDispatch.Set();
}
/// <summary>Shuts down gracefully within the specified timeout.</summary>
/// <param name="timeout">Shutdown timeout period.</param>
/// <param name="cancellationToken">Cancellation token.</param>
/// <returns>Shutdown result.</returns>
public Task<MxAccessShutdownResult> ShutdownGracefullyAsync(
TimeSpan timeout,
CancellationToken cancellationToken = default)
{
releaseDispatch.Set();
if (ThrowTimeoutOnShutdown)
{
return Task.FromException<MxAccessShutdownResult>(
new TimeoutException("Simulated graceful shutdown timeout."));
}
return Task.FromResult(new MxAccessShutdownResult(Array.Empty<MxAccessShutdownFailure>()));
}
/// <summary>Releases a blocked dispatch.</summary>
public void ReleaseDispatch()
{
releaseDispatch.Set();
}
/// <summary>Sets the current heartbeat snapshot.</summary>
/// <param name="value">The snapshot to set.</param>
public void SetSnapshot(WorkerRuntimeHeartbeatSnapshot value)
{
lock (gate)
{
snapshot = value;
}
}
/// <summary>Enqueues a worker event to be drained.</summary>
/// <param name="workerEvent">The event to enqueue.</param>
public void EnqueueEvent(WorkerEvent workerEvent)
{
lock (gate)
{
events.Enqueue(workerEvent);
}
}
/// <summary>Disposes resources.</summary>
public void Dispose()
{
Disposed = true;
releaseDispatch.Set();
releaseDispatch.Dispose();
DispatchStarted.Dispose();
}
}
@@ -0,0 +1,22 @@
using MxGateway.Worker.Sta;
namespace MxGateway.Worker.Tests.TestSupport;
/// <summary>
/// Shared no-operation <see cref="IStaComApartmentInitializer"/> for tests that
/// construct an <see cref="StaRuntime"/> without a real COM apartment. Replaces
/// the per-file copies that were previously defined independently in
/// StaCommandDispatcherTests, MxAccessStaSessionTests, and MxAccessCommandExecutorTests.
/// </summary>
internal sealed class NoopComApartmentInitializer : IStaComApartmentInitializer
{
/// <inheritdoc />
public void Initialize()
{
}
/// <inheritdoc />
public void Uninitialize()
{
}
}
@@ -0,0 +1,23 @@
using MxGateway.Worker.MxAccess;
namespace MxGateway.Worker.Tests.TestSupport;
/// <summary>
/// Shared no-operation <see cref="IMxAccessEventSink"/> for tests that construct
/// an <see cref="MxAccessStaSession"/> but do not exercise the event sink.
/// Replaces the per-file <c>NoopEventSink</c>/<c>NullEventSink</c> copies that
/// were previously defined independently in MxAccessCommandExecutorTests and
/// AlarmCommandExecutorTests.
/// </summary>
internal sealed class NoopEventSink : IMxAccessEventSink
{
/// <inheritdoc />
public void Attach(object mxAccessComObject, string sessionId)
{
}
/// <inheritdoc />
public void Detach()
{
}
}
@@ -0,0 +1,43 @@
using Google.Protobuf;
namespace MxGateway.Worker.Tests.TestSupport;
/// <summary>
/// Shared helpers for building raw length-prefixed worker frames in tests.
/// Replaces the per-file <c>CreateFrame</c>/<c>WriteUInt32LittleEndian</c> copies
/// that were previously defined independently in WorkerFrameProtocolTests and
/// WorkerPipeSessionTests.
/// </summary>
internal static class WorkerFrameTestHelpers
{
/// <summary>Builds a length-prefixed frame from a protobuf message.</summary>
/// <param name="message">Message to serialize into the frame payload.</param>
public static byte[] CreateFrame(IMessage message)
{
return CreateFrame(message.ToByteArray());
}
/// <summary>Builds a length-prefixed frame from a raw payload.</summary>
/// <param name="payload">Payload bytes to wrap in a frame.</param>
public static byte[] CreateFrame(byte[] payload)
{
byte[] frame = new byte[sizeof(uint) + payload.Length];
WriteUInt32LittleEndian(frame, (uint)payload.Length);
payload.CopyTo(frame, sizeof(uint));
return frame;
}
/// <summary>Writes a little-endian unsigned 32-bit integer to the buffer head.</summary>
/// <param name="buffer">Buffer to write into; must have at least four bytes.</param>
/// <param name="value">Value to encode.</param>
public static void WriteUInt32LittleEndian(
byte[] buffer,
uint value)
{
buffer[0] = (byte)value;
buffer[1] = (byte)(value >> 8);
buffer[2] = (byte)(value >> 16);
buffer[3] = (byte)(value >> 24);
}
}