lmxopcua/archreview/06-gateway-integrations.md

Architecture Review 06 — Gateway Integrations (Galaxy Driver + Historian Gateway Driver)

• Date: 2026-07-12
• Commit: f6eaa267 (master, clean tree)
• Updates: the 2026-07-08 review at 9cad9ed0. Since then the arch-review remediation branches merged to master (none targeted this domain's code), plus two in-domain deltas: the docker-dev ServerHistorian stale-keys fix (7233e2ba) and the ZB.MOM.WW.HistorianGateway Client + Contracts 0.1.0 → 0.2.0 bump (f6eaa267). Both src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy* and src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway (and their test projects) are byte-identical to 9cad9ed0 — all prior file:line citations remain exact.
• Scope:
  • src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy (+ .Contracts, .Browser)
  • src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway
  • Test coverage: tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests, .Galaxy.Browser.Tests, .Historian.Gateway.Tests
  • Peripheral (read for context, not reviewed in depth): Runtime/Historian/* (recorder, options), AddressSpaceApplier provisioning/subscription hooks, OtOpcUaNodeManager tie-cluster paging, Host Program.cs historian wiring, docker-dev/docker-compose.yml.
• Dimensions: Stability, Performance, Conventions, Underdeveloped Areas.

---

Prior-finding status (9cad9ed0 → f6eaa267)

ID	One-liner	Status @ f6eaa267
S-1 (High)	Galaxy write success is optimistic; committed-write failure can never surface	STILL OPEN — GatewayGalaxyDataWriter.cs:281-302 unchanged (empty-statuses ⇒ Good; advise-failure path :234-243 still proceeds). STATUS.md lists "06/S-1 (Galaxy fail-closed)" under suggested next, not done.
S-2 (Med)	EventPump saturation drops the newest events (inverted staleness bias)	STILL OPEN — EventPump.cs:128-140 unchanged.
S-3 (Med)	ReadViaSubscribeOnceAsync can wait forever on a non-cancellable token	STILL OPEN — GalaxyDriver.cs:738-755 unchanged.
S-4 (Med)	Transport-failure detection is EventPump-only; subscription-less driver never degrades	STILL OPEN — ReportTransportFailure still has the single pump-fault caller (GalaxyDriver.cs:929-949).
S-5 (Med)	Outbox RemoveAsync truncates the FIFO prefix; out-of-order acks silently drop	STILL OPEN — FasterLogHistorizationOutbox.cs:158-182 unchanged.
S-6 (Med)	RefreshConnectionStateAsync has no production caller; snapshot flags dormant	STILL OPEN — verified by grep: only GatewayHealthSnapshotTests (3 call sites) invokes it; no hosted-service was added.
S-7 (Low)	Alarm feed reconnect has no backoff (fixed 5 s)	STILL OPEN — GatewayGalaxyAlarmFeed.cs:102-148 unchanged.
S-8 (Low)	GalaxyMxSession state unsynchronized across reopen (accepted, emergent safety)	STILL OPEN — unchanged; still un-documented invariant.
S-9 / S-10 (Positive)	Store-and-forward discipline; TLS/API-key posture	Unchanged — and S-10 strengthened for free by the 0.2.0 client (PEM-bundle trust anchors; see bump assessment).
P-1 (Med)	Galaxy Read = 3 round-trips + publish wait	STILL OPEN — gateway-constrained, unchanged.
P-2 (Med)	Alarm drain is one unary SendEvent per event	STILL OPEN — 0.2.0 ships no batched SendEvents; client surface for events unchanged.
P-3 (Low)	Four channels to the historian sidecar (deliberate)	Unchanged — trade-off still argued in-source.
P-4 (Positive)	Fan-in/fan-out indexed, bounded, metered	Unchanged.
P-5 (Low)	HistoryRead buffers whole replies; maxEvents <= 0 unbounded	STILL OPEN — 0.2.0 ReadEventsAsync signature is identical (no wire-side cap param); adapter still drops maxEvents (HistorianGatewayClientAdapter.cs:76-89).
P-6 (Low)	Outbox peek holds the state lock across disk I/O	STILL OPEN — unchanged, still negligible at 64-entry batches.
C-1 / C-2 (Positive)	Secret handling; seam quality	Unchanged.
C-3 (Med)	Historian seam leaks wire types by design; keep contained	STILL OPEN — IHistorianGatewayClient.cs doc unchanged; no guard note added. The 0.2.0 bump is a live demonstration of the risk: contract-semantics changes now flow straight into driver behavior (see U-7).
C-4 (Low)	Options-validation styles diverge	STILL OPEN — unchanged; and the warnings-only style's sharp edge materialized live (see S-11).
C-5 (Low)	Retired Wonderware project directories still on disk	STILL OPEN — all five dirs (src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware{,.Client,.Client.Contracts} + 2 test dirs) still present, still absent from the slnx.
C-6 (Low)	EventPump silently drops unknown event families with no metric	STILL OPEN — EventPump.cs:192-207 unchanged.
U-1 (High, doc-drift)	CLAUDE.md KNOWN LIMITATION 2 stale (ref feed wired but doc said empty-set)	FIXED — the docs branch (9fadead6, merged to master via b67bd9e8) rewrote LIMITATION 2 to "value-capture wired; live end-to-end verification still pending", matching the code. The remaining live verification half was never part of the drift finding — it lives on under U-2.
U-2 (Med)	Live-validation gate built but only partially run	STILL OPEN — no live run since; the arch-review #12 rig session ran with the historian disabled (that's what surfaced 7233e2ba). The 0.2.0 bump adds fresh reason to re-run it (see U-7).
U-3 (Med)	WriteSecured / VerifiedWrite user identity stubbed at zero	STILL OPEN — GatewayGalaxyDataWriter.cs:263-264 unchanged.
U-4 (Med)	Dormant-paths inventory (refresh, batched replay, footprint, client-side event cap/filter)	STILL OPEN — all four items unchanged; 0.2.0 fixes none of the gateway gaps (no wire-side maxEvents, no batched replay/SendEvents).
U-5 (Low)	Outbox serializer has no version/format byte	STILL OPEN — HistorizationOutboxEntrySerializer.cs:14-16 unchanged.
U-6	Test-coverage thin spots (advise-failure status, out-of-order RemoveAsync, S-3 hang, pump filter)	STILL OPEN — zero test changes in the three gateway test projects since 9cad9ed0.

New findings this pass: S-11 (Medium), U-7 (Medium), C-7 (Low) — see body.

---

What changed since 2026-07-08 (in-domain delta assessment)

The HistorianGateway 0.1.0 → 0.2.0 bump (f6eaa267)

One-line change to Directory.Packages.props; the consumed contract/client surface changed in five ways (verified by diffing the cached 0.1.0 vs 0.2.0 package XML docs + member lists):

1. HistorianTagDefinition.DataType is now proto3-optional (HasDataType/ClearDataType added). Old semantics: wire-0 (INT1) was indistinguishable from unset ⇒ the gateway mapped it to the SDK default Float. New semantics: presence is explicit, so a set DataType — including wire-0 Int1 — is honored. This directly changes GatewayTagProvisioner behavior for Boolean tags (it maps Boolean → HistorianDataType.Int1, HistorianTypeMapper.cs:28, and always sets the field, GatewayTagProvisioner.cs:57): under 0.1.0 a Boolean tag was silently provisioned as Float; under 0.2.0 it provisions as Int1 (correct). The bump commit's impact analysis covered only the opc_quality change — this one was not assessed (see U-7).
2. HistorianHistoricalValue.OpcQuality is now proto3-optional. The commit's gate check is correct and verified: OtOpcUa never constructs HistorianHistoricalValue (no AddHistoricalValues caller anywhere in src/tests); all OpcQuality uses are the read-side HistorianSample/HistorianAggregateSample, unaffected.
3. HistorianLiveValue.Quality/QualityDetail documented as dead inputs on WriteLiveValues — the gateway's SQL live path silently discards them (server-managed quality; the SQL login is column-permission-denied). GatewayHistorianValueWriter.cs:63 populates Quality anyway (see C-7).
4. Client ToUtcTimestamp behavior fix: a DateTimeKind.Unspecified timestamp is now interpreted as already-UTC instead of being shifted by the host's UTC offset. Absorbed improvement — OtOpcUa's own conversion sites already SpecifyKind(..., Utc) defensively (GatewayHistorianDataSource.cs:269, AlarmEventMapper.cs:20, GatewayHistorianValueWriter.cs:70), and UA-stack DateTimes are Utc-kinded, so no behavior change is expected here; the client-side times passed to ReadRaw/ReadAggregate are now safe even if an Unspecified kind ever leaks through.
5. Client hardening/docs: LoadTrustAnchors now honors a full PEM bundle (root + intermediates) for CaCertificatePath pinning (strengthens S-10); the RawRead/RawWrite/RawAdmin escape hatches are documented as bypassing bearer-header injection, typed-exception mapping, and the retry pipeline (the adapter uses only typed methods — unaffected); per-RPC scope requirements (historian:read / historian:tags:write) are now documented, matching CLAUDE.md's prerequisites; streaming reads documented as not retried (transient failures surface — consistent with how GatewayHistorianDataSource maps them to health failures); a new IHistorianGatewayClientTransport seam exists client-side (unused by OtOpcUa; IHistorianGatewayClient remains the repo's seam).

Wire compatibility is preserved (field numbers unchanged; the proto3-optional changes are wire-compatible). No compile-surface break: the solution builds and the gateway-driver suite passed per the bump commit. The residual risk is behavioral, not structural — captured as U-7.

The docker-dev ServerHistorian stale-keys fix (7233e2ba)

Config-only: docker-dev compose had ServerHistorian__Enabled=true with the retired Wonderware Host/Port/SharedSecret keys, so post-cutover code read an empty Endpoint, and HistorianGatewayClientAdapter.Create threw an unhandled UriFormatException at every bring-up — docker-dev was unbootable against current code. The fix rewrites both centrals to the gateway-shape keys, disabled by default, enable via OTOPCUA_HISTORIAN_ENABLED + OTOPCUA_HISTORIAN_ENDPOINT + env ServerHistorian__ApiKey; central-1 verified booting clean. Correct fix for the rig — but it fixed only the config, not the code path that turns a misconfigured section into a crash-loop with a raw UriFormatException. That code gap is new finding S-11.

---

Architecture Overview

Galaxy driver data flow (mxaccessgw)

GalaxyDriver (GalaxyDriver.cs) is a standard Tier-A in-process Equipment-kind driver registered under type name GalaxyMxGateway. All Galaxy access flows over gRPC to the external mxaccessgw gateway (sibling repo), consumed via the Gitea-feed packages ZB.MOM.WW.MxGateway.Client / .Contracts (MxCommand / MxEvent protos — unchanged this period). The driver holds three logical gRPC clients:

1. Worker session — GalaxyMxSession wraps MxGatewayClient.OpenSessionAsync + MXAccess Register. All data-plane traffic rides it:
   • Subscribe — GatewayGalaxySubscriber.SubscribeBulkAsync (session-level SetBufferedUpdateInterval applied first, cached last-applied value), events consumed by the shared EventPump off the bidirectional StreamEvents RPC, fanned out through SubscriptionRegistry's handle→subscription reverse map onto OnDataChange.
   • Read — MxAccess has no one-shot read; ReadViaSubscribeOnceAsync synthesises Read as SubscribeBulk → first-event wait per handle → UnsubscribeBulk.
   • Write — GatewayGalaxyDataWriter: lazy AddItem (or borrow a live handle from the subscription registry), AdviseSupervisory once per handle (required for commit under WriteUserId=0), then raw Write or WriteSecured routed on the discovery-captured per-tag SecurityClassification.
2. Session-less client (_ownedMxClient) — the always-on central alarm monitor: GatewayGalaxyAlarmFeed (StreamAlarms: active-alarm snapshot → snapshot_complete → live transitions, self-reconnecting) and GatewayGalaxyAlarmAcknowledger (unary AcknowledgeAlarm).
3. Repository client (GalaxyRepositoryClient) — hierarchy browse for DiscoverAsync (via IGalaxyHierarchySource → GalaxyDiscoverer) and the DeployWatcher deploy-event stream that raises IRediscoverable.OnRediscoveryNeeded.

Recovery is owned by ReconnectSupervisor (Healthy → TransportLost → Reopening → Replaying → Healthy, capped exponential backoff, never gives up): the EventPump stream fault feeds ReportTransportFailure; reopen routes through GalaxyMxSession.RecreateAsync (dispose stale session + client, rebuild) and invalidates the writer's handle/advise caches; replay recreates the EventPump then re-issues SubscribeBulk per tracked subscription and Rebinds the registry with the fresh handles. Host connectivity is surfaced through HostStatusAggregator (transport state from the supervisor + per-platform ScanState probes via PerPlatformProbeWatcher).

Since 9cad9ed0, Galaxy capability dispatch from DriverInstanceActor is additionally wrapped by the arch-review #10 IDriverCapabilityInvoker seam (tier-A resilience pipeline) — a Runtime-side change outside this domain's code, noted here because Galaxy calls now flow through it.

Historian gateway data flow (HistorianGateway sidecar)

Driver.Historian.Gateway is the sole historian backend, consuming the ZB.MOM.WW.HistorianGateway.Client package 0.2.0 (historian_gateway.v1) behind the proto-typed IHistorianGatewayClient seam (HistorianGatewayClientAdapter is a pure pass-through over the typed client methods; channels are lazy — no I/O at construction, except the eager new Uri(Endpoint) at HistorianGatewayClientAdapter.cs:45, see S-11). Four independent paths, each with its own gRPC channel to the same sidecar (documented deliberate trade-off, GatewayHistorianServiceCollectionExtensions.cs:57-64):

1. Read — GatewayHistorianDataSource (IHistorianDataSource): OPC UA HistoryRead Raw/Processed/AtTime/Events → ReadRaw / ReadAggregate / ReadAtTime / ReadEvents; at-time replies re-aligned one-snapshot-per-requested-timestamp; health counters under a single lock. Tie-cluster paging (MaxTieClusterOverfetch) lives server-side in OtOpcUaNodeManager (~line 2200), bound from ServerHistorianOptions.
2. Alarm history write — GatewayAlarmHistorianWriter (SendEvent per event) behind the durable SqliteStoreAndForwardSink; maps every outcome (ack, typed client exceptions, raw RpcException) to exactly one Ack / RetryPlease / PermanentFail per event and never throws.
3. Continuous historization — ContinuousHistorizationRecorder (Runtime actor) taps the dependency-mux value fan-out, appends to the crash-safe FasterLogHistorizationOutbox (PerEntry fsync or Periodic commit; bounded drop-oldest capacity; startup recovery scan), drains through GatewayHistorianValueWriter (WriteLiveValues, non-throwing bool).
4. Tag provisioning — GatewayTagProvisioner (EnsureTags), dispatched fire-and-forget from AddressSpaceApplier.Apply with a tally log; non-historizable data types skipped; can never block or fail a deploy.

---

Findings

1. Stability

S-1 (High) — Galaxy write success is optimistic; a committed-write failure can never surface

GatewayGalaxyDataWriter.TranslateReply (GatewayGalaxyDataWriter.cs:281-302) honours the protocol status and the first MXAccess status row, defaulting to Good when the statuses array is empty — and the gateway's write execution is effectively fire-and-forget past dispatch (the reply reflects command acceptance, not the eventual COM-side commit). Two concrete consequences:

• The supervisory-advise failure path (GatewayGalaxyDataWriter.cs:234-243) logs a warning, forgets the handle, and lets the write proceed anyway — but the file's own comment (:163-165) states a raw Write without supervisory advise "doesn't throw (reply looks OK) but the value never reaches the galaxy". That is a silent write loss returning Good to the OPC UA client.
• The server's write-outcome self-correction (#5, reverts the node on a failed device write) can structurally never trigger for Galaxy, so a lost write leaves a phantom-Good node value indefinitely.

Blast radius: operator writes (WriteOperate-gated) to Galaxy attributes that silently don't commit, with no Bad status, no health degradation, no metric. Not scheduled in the remediation pass; STATUS.md carries it under "suggested next" as 06/S-1 (Galaxy fail-closed). Recommendation: (a) return Uncertain (not proceed-to-Good) when supervisory advise fails; (b) pursue a gateway-side WriteComplete correlation (the gw backlog's OnWriteComplete event family already exists in the proto — EventPump.cs:200-206 filters it out) so the reply/statuses row carries the real commit outcome; (c) add a galaxy.writes.unconfirmed counter in the interim.

S-2 (Medium) — EventPump saturation drops the newest events (inverted staleness bias)

EventPump.RunAsync (EventPump.cs:128-140) uses TryWrite against a bounded channel: when the fan-out consumer stalls, the just-arrived event is dropped and 50 000 stale queued events are preserved. For last-value-wins OPC UA telemetry this is the wrong bias — after a stall clears, subscribers replay old values and the freshest one may be the dropped one. The drop is at least metered (galaxy.events.dropped). Recommendation: per-item-handle conflation (keep newest per handle) or drop-oldest ring semantics; either preserves the no-backpressure requirement while keeping recent data.

S-3 (Medium) — ReadViaSubscribeOnceAsync can wait forever on a non-cancellable token

The read synthesis (GalaxyDriver.cs:738-755) fills pending snapshots with BadTimeout only via cancellationToken.Register. If a caller passes CancellationToken.None (or a token that never fires) and a successfully subscribed tag never publishes an initial event (gateway hiccup between SubscribeBulk and first push), the awaits at :753 never complete and the read hangs, holding the subscription open. Recommendation: race each pending TCS against an internal deadline derived from DefaultCallTimeoutSeconds / PublishingIntervalMs regardless of the caller's token.

S-4 (Medium) — Transport-failure detection is EventPump-only; a subscription-less driver never degrades

ReconnectSupervisor.ReportTransportFailure has exactly one production caller: the EventPump stream-fault callback (GalaxyDriver.cs:929-949). The pump only starts on the first subscribe/read. A driver doing only writes (or idle after discovery) whose gateway restarts will keep GetHealth() == Healthy; each write then fails per-request with BadCommunicationError, but no reopen/replay/Degraded transition ever runs, and the stale session persists until something subscribes. Failed unary RPCs (SubscribeBulk, Write, AcknowledgeAlarm) do not feed the supervisor either. Recommendation: report classified transport exceptions from the write/subscribe paths into the supervisor (idempotent by design, so this is cheap).

S-5 (Medium) — FasterLog outbox RemoveAsync truncates the FIFO prefix; out-of-order acks silently drop unacked entries

FasterLogHistorizationOutbox.RemoveAsync (FasterLogHistorizationOutbox.cs:158-182) removes the target plus every older live entry and truncates the log to the target's successor. The contract "recorder acks in FIFO order" is enforced only by comment. If the drain ever acks a mid-batch id first (e.g. per-tag partial write success in a future recorder change), all older unacked values are durably discarded without incrementing DroppedCount. Recommendation: count and warn when the prefix removal drops non-target entries, or make the API batch-oriented (RemoveThroughAsync) so the semantics are explicit at the call site.

S-6 (Medium) — Historian health snapshot's connection flags are dormant in production

GatewayHistorianDataSource.RefreshConnectionStateAsync (GatewayHistorianDataSource.cs:219-244) is documented as "intended to be driven by a periodic health hosted-service", but no production caller exists — re-verified this pass: only GatewayHealthSnapshotTests invokes it. ProcessConnectionOpen / EventConnectionOpen in GetHealthSnapshot are therefore permanently false in a deployed host, which any dashboard consuming the snapshot will read as "historian down" (or, if ignored, the flags are dead weight). Same shape as the memory's "register-AND-pass-into-consumer" trap that bit GatewayTagProvisioner in PR #423. Recommendation: add the periodic refresh hosted-service (or fold a refresh into the existing health probe cadence), or remove the flags from the snapshot.

S-7 (Low) — Alarm feed reconnect has no backoff

GatewayGalaxyAlarmFeed.RunAsync (GatewayGalaxyAlarmFeed.cs:102-148) re-opens on a fixed 5 s delay forever. DeployWatcher (capped exponential + jitter, DeployWatcher.cs:212-233) and ReconnectSupervisor both do this properly. A dead gateway gets hammered every 5 s per driver instance. Cosmetic at this scale, but inconsistent; align on capped exponential.

S-8 (Low) — GalaxyMxSession state is unsynchronized across reopen

_session / _connected (GalaxyMxSession.cs:28-32) are plain fields; RecreateAsync tears down while concurrent writers/subscribers may hold or fetch Session. In practice the supervisor's single-flight recovery plus the per-call try/catch (mapped to BadCommunicationError) contain it, and the caches are invalidated post-reopen — but the safety is emergent, not designed. Acceptable; document the invariant on Session.

S-11 (Medium, NEW) — An enabled-but-misconfigured ServerHistorian section crash-loops the Host with a raw UriFormatException; one corner gets no warning at all

HistorianGatewayClientAdapter.Create eagerly does new Uri(options.Endpoint) (HistorianGatewayClientAdapter.cs:45). ServerHistorianOptions.Validate() produces exactly the right warning for an empty Endpoint (ServerHistorianOptions.cs:78-79) — but it is warnings-only: registration proceeds (Runtime/ServiceCollectionExtensions.cs:135-139, Host/Program.cs:123-127) and the first DI resolution throws an unhandled UriFormatException, crashing the Host at bring-up. This is not theoretical — it made docker-dev unbootable until the config-side fix 7233e2ba (whose commit message documents the exact failure). The code path is untouched.

The sharper corner: AddAlarmHistorian gates only on AlarmHistorian:Enabled (Runtime/ServiceCollectionExtensions.cs:87) but sources its connection from the ServerHistorian section (Program.cs:127). With AlarmHistorian:Enabled=true and ServerHistorian:Enabled=false (Endpoint legitimately empty), Validate() early-returns on !Enabled (ServerHistorianOptions.cs:77) — so the operator gets zero warning, then the same UriFormatException out of CreateAlarmWriter when the sink singleton is built. Same exposure for the continuous-historization writer (Program.cs:187, though that one is co-gated on ServerHistorian.Enabled, so only a malformed non-empty Endpoint reaches it).

A hard fail at startup on misconfiguration is defensible — but it should be a deliberate, named failure, not an incidental UriFormatException from deep inside a DI factory. Recommendation: (a) validate Endpoint parses as an absolute URI in Validate(); (b) when the section is Enabled (or consumed by an enabled AlarmHistorian) and Endpoint is empty/invalid, fail fast with an InvalidOperationException naming the configuration key — or force-disable and keep the Null defaults; (c) make AddAlarmHistorian also run ServerHistorianOptions.Validate()-equivalent checks on the connection section it actually depends on.

S-9 (Positive) — Store-and-forward + outbox crash-safety discipline is strong

• GatewayAlarmHistorianWriter (GatewayAlarmHistorianWriter.cs:66-118) short-circuits remaining batch entries to RetryPlease on shutdown, classifies auth failures as retryable (an auth blip never dead-letters), and defaults unknowns to PermanentFail so poison events cannot loop.
• FasterLogHistorizationOutbox PerEntry mode fsyncs before append returns; recovery rebuilds the index from BeginAddress; the capacity-overflow-after-crash convergence case is analysed in-source (FasterLogHistorizationOutbox.cs:200-204).
• Partial-open recovery in GalaxyMxSession.ConnectAsync (GalaxyMxSession.cs:95-101) tears down half-open state so retry rebuilds cleanly.

S-10 (Positive) — TLS / API-key posture

Both integrations support TLS with CA pinning (CaCertificatePath), the historian's AllowUntrustedServerCertificate inversion is explicitly documented at the mapping site (HistorianGatewayClientAdapter.cs:49-52), and keys are never logged. The 0.2.0 client strengthens this for free: CaCertificatePath now honours a full PEM bundle (root + intermediates) as trust anchors, and the raw-client escape hatches are documented as bypassing bearer injection (the adapter uses only typed methods, so no exposure). See C-1 for the resolver.

2. Performance

P-1 (Medium) — Galaxy Read costs three gateway round-trips plus a publish wait per OPC UA Read

ReadViaSubscribeOnceAsync = SubscribeBulk + first-event wait (up to PublishingIntervalMs) + UnsubscribeBulk, with server-side AddItem/RemoveItem churn per read (GalaxyDriver.cs:644-780). This is forced by MxAccess (no one-shot read RPC) and is correctly batched per request, but a client polling via Read instead of subscribing multiplies gateway/COM load ~3×. Recommendation: document "subscribe, don't poll" as the supported pattern; consider a short-lived read-through cache keyed on full reference if polling clients appear.

P-2 (Medium) — Alarm-history drain is one unary SendEvent per event

GatewayAlarmHistorianWriter.WriteBatchAsync (GatewayAlarmHistorianWriter.cs:66-80) serializes the batch — deliberate poison-event isolation, but an alarm storm of N events costs N sequential RPC round-trips out of the SQLite drain worker. The 0.2.0 client still has no batched SendEvents with per-event status rows; when the gateway grows one, adopt it. Until then this is an accepted, well-documented ceiling.

P-3 (Low) — Four channels to the historian sidecar; two-plus to mxaccessgw

Each historian path owns its channel (read / alarm-write / provisioner / value-writer) and the Galaxy driver holds a session client, a session-less client, and a repository client. All are lazy, HTTP/2-multiplexed, and the trade-off (clean independent dispose ownership over channel sharing) is argued in-source (GatewayHistorianServiceCollectionExtensions.cs:57-64). No action; noted so nobody "optimizes" it into a shared-singleton dispose bug.

P-4 (Positive) — Fan-in/fan-out paths are indexed, bounded, and metered

• SubscriptionRegistry maintains a handle→subscriptions reverse map with a per-entry handle→fullRef index, so ResolveSubscribers is O(subscribers), not O(bindings) (SubscriptionRegistry.cs:97-111), and TryResolveItemHandle lets the writer skip AddItem for subscribed tags with a stale-entry cross-check (:128-142).
• Subscribe/replay correlation uses a one-pass result index (GalaxyDriver.BuildResultIndex, fixing a former O(n²) on the 50k-tag path).
• EventPump decouples network read from dispatch via a bounded channel (default 50 000, configurable) with received/dispatched/dropped counters.

P-5 (Low) — HistoryRead buffers whole replies; maxEvents <= 0 is unbounded

ReadRawAsync / ReadProcessedAsync / ReadEventsAsync (GatewayHistorianDataSource.cs:59-182) drain the stream into a list before mapping. Raw reads are capped by maxValuesPerNode, but an events read with maxEvents <= 0 collects without limit (the gateway's RuntimeDb:EventReadMaxRows is the only cap, and it's a remote deployment knob). Re-checked against 0.2.0: ReadEventsAsync's client signature is unchanged — still no wire-side cap, so the adapter still cannot forward maxEvents (HistorianGatewayClientAdapter.cs:76-89). The tie-cluster paging bound (MaxTieClusterOverfetch, validated > 0 in ServerHistorianOptions.Validate, enforced at OtOpcUaNodeManager.cs:2200-2214) is well-designed. Recommendation: clamp events reads to a server-side default cap when the caller passes 0.

P-6 (Low) — Outbox peek holds the state lock across disk I/O

PeekBatchAsync scans FasterLog from the logical head under _state (FasterLogHistorizationOutbox.cs:140-155), blocking a concurrent AppendAsync's index update for the duration of a (mostly page-cached) disk scan. At the recorder's 64-entry default batch this is negligible; revisit only if drain batches grow.

3. Conventions

C-1 (Positive) — Secret handling is a model for the other drivers

GalaxySecretRef (Galaxy.Contracts/GalaxySecretRef.cs) resolves env: / file: / dev: / literal with fail-fast on unset env vars and a startup warning on unprefixed cleartext; it lives in Contracts so the runtime driver and the AdminUI browser share one implementation (GalaxyDriverBrowser.cs:125). The historian side takes ServerHistorian__ApiKey via env with a "never commit" doc contract and an empty-key Validate() warning; the docker-dev rewrite (7233e2ba) follows the same discipline (${ServerHistorian__ApiKey:-} pass-through, key never committed). No key value is ever logged on either path.

C-2 (Positive) — Seam quality and driver-family consistency

• IHistorianGatewayClient is a clean single seam: every consumer (GatewayHistorianDataSource, GatewayAlarmHistorianWriter, GatewayTagProvisioner, GatewayHistorianValueWriter) depends only on it; FakeHistorianGatewayClient backs the offline suite; the adapter is a zero-translation pass-through. (The 0.2.0 client's new IHistorianGatewayClientTransport seam is package-internal plumbing — correctly not adopted; the repo seam stays IHistorianGatewayClient.)
• The Galaxy capability seams (IGalaxyHierarchySource, IGalaxyDataReader/Writer, IGalaxySubscriber, IGalaxyAlarmFeed/Acknowledger) plus the internal test ctor mirror the protocol-driver pattern; tracing decorators (Traced*) wrap the production seams without an OpenTelemetry dependency.
• GalaxyDriverFactoryExtensions.CreateInstance throws precise, instance-named errors on missing required fields (GalaxyDriverFactoryExtensions.cs:56-75), and GalaxyDriverProbe uses JsonStringEnumConverter (the FB-9/FB-10 enum-serialization lesson applied).

C-3 (Medium) — The historian seam leaks wire types by design; keep it contained

IHistorianGatewayClient signatures use HistorianGateway.Contracts.Grpc types (HistorianSample, RetrievalMode, WriteAck, ...) — documented as deliberate (IHistorianGatewayClient.cs:5-11). This is fine while the driver is the only consumer, but the pure Mapping/ layer is the real anti-corruption boundary: any future consumer must sit above GatewayHistorianDataSource, never on the seam. The 0.2.0 bump demonstrated the flip side concretely: a semantics change in a contract type (HistorianTagDefinition.DataType presence, U-7) flowed straight into driver behavior with no compile signal. Worth a one-line guard note in the interface doc, plus a bump-checklist habit of diffing the contract XML (this pass's method) rather than relying on green builds.

C-4 (Low) — Options-validation styles diverge

Galaxy: throw-on-construct for required fields; DataAnnotations attributes on the records are decorative (nothing runs Validator). Historian: Validate() returns operator warnings and never throws (registration logs them). Both are defensible, but the repo now has two idioms for "driver-adjacent options validation" — and the warnings-only historian style's sharp edge materialized live this period (the docker-dev crash, see S-11): the operator got the correct warning immediately followed by an opaque crash. Pick one idiom for new sections, and give the warnings style a fail-fast tier for configurations that will provably crash anyway.

C-5 (Low) — Retired Wonderware project directories still on disk

src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware{,.Client,.Client.Contracts} (and their test dirs) remain in the tree but are absent from ZB.MOM.WW.OtOpcUa.slnx (0 references) — re-verified this pass. Dead directories invite grep noise and accidental resurrection; delete them (git history preserves the code).

C-6 (Low) — EventPump silently drops unknown event families with no metric

Dispatch (EventPump.cs:192-207) returns without counting on any non-OnDataChange family. The rationale comment is good, but a gateway version emitting OnBufferedDataChange (or a new family) would silently discard data with zero observability — the alarm feed counts its decode drops (AlarmTransitionsDecodingFailures); the pump should count filtered/unknown families the same way.

C-7 (Low, NEW) — The live-value writer populates a documented-dead Quality field

GatewayHistorianValueWriter maps HistorizationValue.Quality onto HistorianLiveValue.Quality (GatewayHistorianValueWriter.cs:63), but the 0.2.0 contract now documents that field (and QualityDetail) as silently discarded on the WriteLiveValues SQL path — the gateway's SQL login is column-permission-denied on the server-managed Quality column, so the value is accepted on the wire and dropped. Harmless today because the recorder hardcodes GoodQuality = 192 for every captured value (ContinuousHistorizationRecorder.cs:45,256 — the outbox round-trips it, :419), so nothing meaningful is lost. The hazard is forward-looking: if the recorder is ever extended to capture real node quality (a natural next step), it will appear to work and silently persist nothing — quality will remain server-stamped. Add a comment at the mapping site citing the 0.2.0 contract note (Contracts C-002), or stop populating the field so the dead input is visible.

4. Underdeveloped Areas

U-2 (Medium) — KNOWN LIMITATION 1: the live-validation gate is built but still not fully run

The Category=LiveIntegration suite is complete and skip-clean: GatewayLiveFixture env-gates on HISTGW_GATEWAY_ENDPOINT/APIKEY (+ per-test HISTGW_TEST_TAG, HISTGW_WRITE_SANDBOX_TAG, HISTGW_ALARM_SOURCE) with a bounded 3 s TCP probe so a down VPN skips instead of hanging (Live/GatewayLiveFixture.cs). Four live tests cover read, EnsureTags+write, alarm SendEvent→ReadEvents round-trip, and a SendEvent contract check. The alarm leg has been run live at least once (merge 245316d8); nothing further has run since the prior review — the arch-review #12 rig session ran with the historian disabled. The 0.2.0 bump raises the stakes: the suite has never executed against a 0.2.0 gateway with the 0.2.0 client, and the DataType presence change (U-7) is precisely the kind of semantics shift only the live EnsureTags leg can validate. A full documented run of the suite is the remaining work — the infrastructure is not the gap.

U-3 (Medium) — WriteSecured / VerifiedWrite user identity is stubbed at zero

InvokeWriteSecuredAsync hardcodes CurrentUserId = 0, VerifierUserId = 0 (GatewayGalaxyDataWriter.cs:263-264). ArchestrA secured/verified writes exist precisely to attribute and dual-authorize the operation; user 0 will be rejected or unattributed by any galaxy that actually classifies tags SecuredWrite/VerifiedWrite. There is no mapping from the OPC UA session identity (the LDAP-authenticated principal is available server-side) to an ArchestrA user id. Until that lands, writes to secured-classification tags are effectively unsupported — document it, or fail fast with a clear status instead of sending user 0.

U-4 (Medium) — Dormant paths inventory

• GatewayHistorianDataSource.RefreshConnectionStateAsync — no production caller (see S-6).
• Replay still fans out per-subscription SubscribeBulk; the gateway's batched ReplaySubscriptionsCommand remains a "PR 6.x can swap this" note (GalaxyDriver.cs:314-316).
• GalaxyDriver.FlushOptionalCachesAsync is a no-op and GetMemoryFootprint is a constants-based estimate (GalaxyDriver.cs:561-575) — fine, but the server's cache-flush heuristic gets synthetic data from this driver.
• HistorianGatewayClientAdapter.ReadEventsAsync never forwards maxEvents on the wire (client-side cap only, documented at IHistorianGatewayClient.cs:58-63) and the source filter is re-applied client-side defensively (GatewayHistorianDataSource.cs:152-157) because the gateway-side filter "may not be present" — re-checked against 0.2.0: neither gap is addressed (the ReadEventsAsync client signature is unchanged). Both remain polite workarounds for gateway gaps that should be tracked against the sidecar repo.

U-5 (Low) — Outbox serializer has no version/format byte

HistorizationOutboxEntrySerializer writes a fixed positional layout with no version prefix (HistorizationOutboxEntrySerializer.cs:14-16). Any future field addition breaks recovery of pre-existing on-disk entries with an undiagnosable deserialization failure mid-RecoverState. One reserved byte now is free; a migration later is not.

U-7 (Medium, NEW) — The 0.2.0 bump's impact analysis missed the DataType presence change that alters Boolean tag provisioning

The bump commit (f6eaa267) gates only on the M3 opc_quality change ("checked and CLEAR" — correct, verified: OtOpcUa never writes HistorianHistoricalValue). But 0.2.0 also made HistorianTagDefinition.DataType proto3-optional (contracts review M-2), and that one lands squarely on GatewayTagProvisioner:

• The provisioner always sets DataType explicitly (GatewayTagProvisioner.cs:57), and Boolean → HistorianDataType.Int1 (HistorianTypeMapper.cs:28) — where Int1 is wire value 0.
• Under 0.1.0 the gateway could not distinguish wire-0 from unset and mapped it to the SDK default Float — i.e. every Boolean tag EnsureTags'd to date was silently provisioned as a Float historian tag.
• Under 0.2.0 presence is explicit, so the same call now provisions Int1 — the intended behavior, arriving as a silent semantic change with zero code diff on our side.

Two consequences need closing out: (1) any Boolean tags provisioned under 0.1.0 exist in the historian as Float, and what the gateway's EnsureTags does when the existing tag's type mismatches the (now-honored) requested type is unverified — per-tag failure outcomes would surface only in the provisioner's tally log (failed=N); (2) the live EnsureTags gate (U-2) has never run under 0.2.0 semantics. Recommendation: re-run the Category=LiveIntegration EnsureTags leg including a Boolean tag; check historian-side types for any previously provisioned Boolean tags; and adopt "diff the contract XML between package versions" (this review's method — both versions sit in the local NuGet cache) as the standing bump-gate checklist, since green builds cannot catch presence-semantics changes.

U-6 — Test coverage assessment

Zero test changes in the three gateway test projects since 9cad9ed0; the assessment stands verbatim:

• Galaxy (~250 test methods across 34 files): strong unit coverage of the hard parts — reconnect orchestration (GalaxyMxSessionReconnectTests, ReconnectSupervisorTests), pump fault/bounded-channel behaviour, registry rebind/handle-resolve, writer caches, alarm feed decode, value encode/decode, status mapping, probe, factory. Three skip-gated live smokes (GatewayGalaxyLiveReopenAndWriteTests — proven 2/2 against 10.100.0.48:5120 per project memory — and GatewayGalaxyAlarmFeedLiveTests) gated on MXGW_ENDPOINT + GALAXY_MXGW_API_KEY.
• Historian.Gateway (~15 files): mappers fully covered, writer/provisioner/data-source outcome classification covered via FakeHistorianGatewayClient, outbox recovery/capacity covered, plus the 4-test live suite.
• Thin spots: no test drives GalaxyDriver write-through the supervisory-advise failure branch asserting the returned status (S-1's blast radius is untested); no test exercises out-of-order RemoveAsync on the outbox (S-5); nothing covers the read-hang case in S-3; EventPump unknown-family filtering has no assertion (C-6). New this pass: nothing asserts the misconfigured-Enabled crash shape (S-11), and no test pins the Boolean→Int1 provisioning expectation against a 0.2.0-semantics fake (U-7).

---

Maturity Ratings

Dimension	Rating (1-5)	Prior	Justification
Stability	4	4	Layered recovery (supervisor, self-reconnecting feeds, backoff, partial-open teardown) and disciplined never-throw write sinks are unchanged; still docked for the Galaxy silent-write-loss blast radius (S-1 — explicitly deferred by the remediation pass), EventPump-only fault detection (S-4), the dormant health refresh (S-6), and now the proven-live misconfiguration crash-loop (S-11, fixed only config-side).
Performance	4	4	No in-domain code change; bounded channels with drop metering, O(1) fan-out indexes, handle borrowing, and a bounded tie-cluster over-fetch stand; still docked for the 3-round-trip read synthesis (P-1) and serial alarm sends (P-2), both documented and still gateway-constrained in 0.2.0.
Conventions	4	4	Exemplary seams, shared secret resolver, consistent driver-family shape and tracing decorators; still docked for the dual options-validation idiom (whose sharp edge now has a live incident attached), the on-disk retired Wonderware dirs, the unmetered pump filter, and the new dead-Quality-input mapping (C-7).
Underdeveloped areas	3	3	The U-1 doc drift is fixed (the one prior finding closed this period), but the live gates net widened: the historian live suite has still never fully run, and the 0.2.0 bump added unverified semantics on the provisioning path (U-7) that only the live EnsureTags leg can retire. WriteSecured identity remains stubbed; the gateway-gap workarounds remain.

---

Cross-Cutting Themes

1. "Wired but never invoked" is this codebase's recurring failure mode — the provisioner (PR #423), the F10b sink forwarding, and RefreshConnectionStateAsync (S-6, still open). Any new capability interface or hook needs an explicit production-caller check plus a live /run, not just unit tests. (The arch-review pass built the reflection forwarding guard for the sink family; the health-refresh instance in this domain remains unguarded.)
2. Optimistic-Good on fire-and-forget writes — Galaxy structurally cannot report a failed commit (S-1, deferred by the remediation pass); reviewers of the node-write router / write-outcome self-correction should not assume driver parity here.
3. Green builds cannot gate contract-semantics changes — the 0.2.0 bump was wire-compatible and compile-clean yet silently changed Boolean tag provisioning (U-7) and dead-lettered the live-value Quality field (C-7). The bump gate for gateway packages should include a contract-XML diff (both versions are always in the local NuGet cache) and a live-suite run, not just dotnet test.
4. Warnings-only validation defers the crash, it doesn't prevent it — the docker-dev incident (S-11) logged the exact right warning milliseconds before an opaque UriFormatException crash-loop. Configurations that will provably fail should fail fast, by name.
5. Gateway-gap workarounds accrue client-side — defensive source filters, client-side event caps, missing batched replay/SendEvents; 0.2.0 closed none of them. These belong on the sister repos' backlogs with links from the in-source comments.