Tracking IDs are review bookkeeping that rots in shipped source; the rationale they tagged is kept. Comment-only change (CommentChecker 458 -> 16 false positives).
The remediation reviews approved every task but left a tail of small notes.
This lands the gateway-side half of them.
Hardening (behavior changes, all narrow):
- BuildFilteredWriteBulkCommand's unreachable default case failed OPEN: a
fifth bulk-write kind added upstream without a filter case here would have
shipped the DENIED entries to the worker while reporting them denied to the
caller. It now throws UnreachableException.
- SqliteCanonicalAuditStore.ListRecentAsync no longer throws on a row it
cannot date. The retention sweep deliberately preserves such rows (SQLite's
datetime() yields NULL, so the DELETE never matches), which guaranteed the
dashboard's recent-audit view would meet one eventually and lose the whole
page to it. The row is now reported at DateTimeOffset.MinValue with every
other column intact, behind an optional logger.
- The audit drain loop's finally now completes the channel writer alongside
detaching the drain, so a producer that raced past the attached check takes
the write-through branch instead of stranding its event in a buffer nobody
reads until shutdown. TryComplete is idempotent, so StopAsync is unaffected.
Tests:
- MapCommandReply ownership (Assert.Same on the inner reply), mirroring the
existing MapEvent ownership test.
- Redactor key-id length boundary at exactly 64 and 65 characters, pinning
which way it fails. Nothing validates key-id length at creation, so
docs/Diagnostics.md's "which no issued key id does" is now stated as the
heuristic it is.
- ApiKeyFailureLimiter.Reset with a PartitionResolution whose partition was
evicted between the Check and the Reset: inert, and clears nobody else's
block.
- Constraint-cache concurrency stress: the cap is enforced by the inserting
thread, so overshoot must be transient and proportional to the in-flight
inserters, and the cache must settle at or under the cap.
- ListRecentAsync against a raw-SQL undateable row.
Comment/doc accuracy:
- EventsHubViewerRegistry.ReleaseConnection records that it relies on
SignalR's default sequential per-connection dispatch
(MaximumParallelInvocationsPerClient = 1).
- A PERF(followup) note on Invoke's double session resolve and why removing it
needs a SessionManager overload.
- SessionEventDistributor: the volatile-field comment named the pump as the
lock-free reader, but the pump's single capture point is inside _replayLock;
the genuinely lock-free reader is SubscriberCount. OnSubscriberOverflow's
"cannot be observed here" now excepts the DisposeAsync abandon path. The
churn test names its ConcurrentDictionary bucket-order assumption and that a
violation surfaces as a read timeout, not a silent pass.
- The two "restores the sequential drain's behavior" claims (SessionManager,
docs/Sessions.md) were wrong: the sequential drain leaked too, because
KillWorkerAsync's entry ThrowIfCancellationRequested aborted the whole loop
on the first session for zero kills. Reworded to "fixes a leak the
sequential drain also had", with the sweep-bound/shutdown-unbound
ParallelOptions asymmetry explained.
- ISessionManager.ShutdownAsync's token doc: it degrades the drain to a kill
sweep rather than cancelling it, with the bounded overrun stated.
SessionShutdownHostedService.StopAsync records that its cancellation-logging
branch is now unreachable.
Invoke deep-cloned the whole MxCommandRequest — including its command
payload, potentially a large bulk-write graph — only to overwrite the
cloned command with commandToInvoke and discard it. MapCommand then did
the one clone actually needed. Net cost: a full wasted command deep-clone
per Invoke, worst for exactly the bulk writes that are largest.
Adds a MapCommand(MxCommand) overload (MapCommand reads nothing else off
the request) and has Invoke pass commandToInvoke directly; the request
overload delegates so other callers are untouched.
The remaining clone inside MapCommand stays and is now documented as
required rather than incidental: commandToInvoke may be the gRPC-owned
request.Command, and the caller reads it again after dispatch via
TrackCommandReply, so ownership transfer (à la GWC-07) is not safe here.
That clone is what keeps WorkerClient.CreateCommandEnvelope's no-aliasing
invariant true.
Tests: MxAccessGrpcMapperTests.MapCommandFromCommandClonesPayload
(mutating the input leaves the mapped command untouched; both overloads
produce equal results under a fixed TimeProvider).
Behavior-preserving allocation cuts on the per-event/per-command path:
- GWC-06: StreamEvents send-timing uses Stopwatch.GetTimestamp() +
GetElapsedTime() instead of a per-event Stopwatch allocation (same measured
span).
- GWC-07/IPC-05 (event): MapEvent transfers ownership of the inner MxEvent
instead of .Clone()-ing it. Safe: WorkerEvent is parsed fresh per pipe frame
with the distributor pump as its single consumer (GWC-01), MapEvent runs once
before fan-out, and every downstream consumer (subscribers, replay ring)
only READS the event (WorkerSequence is stamped upstream; verified no
post-mapping mutation). Comment documents the invariant + restore-clone
caveat if a second consumer is added.
- IPC-05 (command): CreateCommandEnvelope no longer re-clones; MapCommand
already isolated the graph from the caller-owned gRPC message.
- GWC-15: grpc_stream_queue.depth converts from a per-event push counter to an
ObservableGauge summing registered channel sources at scrape time only
(name/semantics unchanged); removes all per-event .Count/lock work.
Kept every load-bearing isolation clone (MapCommand, Invoke, bulk filters,
MapCommandReply). Server build clean (0 warnings); EventStream/Metrics/
Distributor/Mapper tests 62/62 (incl. formerly-flaky queue-depth tests, now
green under the lazy gauge, + 2 new MapEvent ownership tests). Docs: Metrics.md,
Grpc.md.
Claude-Session: https://claude.ai/code/session_01DMXXvNuPekkkrTEyPNxEkW
Apply the ZB.MOM.WW. prefix to all gateway-side projects, folders,
.csproj/.sln contents, C# namespaces, using directives, generated proto
C# (csharp_namespace + checked-in generated files), InternalsVisibleTo
attributes, project-name string literals (LoadProject, .sln lookups,
worker exe paths, staticwebassets manifest), and the install/script/doc
references that point at any of the above. Migrate the solution from
.sln to .slnx via `dotnet sln migrate` and delete the old file.
External-runtime identifiers are intentionally NOT prefixed so external
configuration keeps working:
- GatewayMetrics.cs MeterName ("MxGateway.Server")
- DashboardAuthenticationDefaults Scheme/Policy ("MxGateway.Dashboard")
- GatewayRequestLoggingMiddleware logger category ("MxGateway.Request")
- StaRuntime thread name ("MxGateway.Worker.STA")
- appsettings.json root section "MxGateway" + env-var prefix
MxGateway__... and secret-name MxGateway:ApiKeyPepper
- C:\ProgramData\MxGateway\ data dir paths
Also fixes two tests that were not rename-related but became visible
while validating the rename:
- WorkerLiveMxAccessSmokeTests.ShutDownAsync: cancellation that the
gateway service correctly maps to RpcException(Cancelled) per gRPC
convention was being misclassified as a stream fault. Added a sibling
catch on RpcException with StatusCode.Cancelled.
- IntegrationTestEnvironment.ResolveRepositoryRoot: extracted IsRepositoryRoot
and made it accept either a .git marker OR a .sln/.slnx next to src/
so the worker-exe walker works in non-git working copies.
clients/proto/proto-inputs.json's protoRoot updated to point at
src/ZB.MOM.WW.MxGateway.Contracts/Protos.
Verified by `dotnet build` and a full `dotnet test` of the .slnx with
MXGATEWAY_RUN_LIVE_{MXACCESS,LDAP,GALAXY}_TESTS=1:
Tests: 472/472 pass
Worker.Tests: 280/280 pass (4 dev-rig [Fact(Skip=...)] skipped)
IntegrationTests: 18/18 pass
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Joseph Doherty