lmxopcua/docs/v2/implementation/pr-4-body.md

PR 4 — Phase 2 follow-up: close the 4 open MXAccess findings

Source: phase-2-pr4-findings (branched from phase-2-stream-d) Target: v2

Summary

Closes the 4 high/medium open findings carried forward in exit-gate-phase-2-final.md:

• High 1 — ReadAsync subscription-leak on cancel. One-shot read now wraps the subscribe→first-OnDataChange→unsubscribe pattern in a try/finally so the per-tag callback is always detached, and if the read installed the underlying MXAccess subscription itself (no other caller had it), it tears it down on the way out.
• High 2 — No reconnect loop on the MXAccess COM connection. New MxAccessClientOptions { AutoReconnect, MonitorInterval, StaleThreshold } + a background MonitorLoopAsync that watches a stale-activity threshold + probes the proxy via a no-op COM call, then reconnects-with-replay (re-Register, re-AddItem every active subscription) when the proxy is dead. Liveness signal: every OnDataChange callback bumps _lastObservedActivityUtc. Defaults match v1 monitor cadence (5s poll, 60s stale). ReconnectCount exposed for diagnostics; ConnectionStateChanged event for downstream consumers (the supervisor on the Proxy side already surfaces this through its HeartbeatMonitor, but the Host-side event lets local logging/metrics hook in).
• Medium 3 — MxAccessGalaxyBackend.SubscribeAsync doesn't push OnDataChange frames back to the Proxy. New IGalaxyBackend.OnDataChange / OnAlarmEvent / OnHostStatusChanged events that the new GalaxyFrameHandler.AttachConnection subscribes per-connection and forwards as outbound OnDataChangeNotification / AlarmEvent / RuntimeStatusChange frames through the connection's FrameWriter. MxAccessGalaxyBackend fans out per-tag value changes to every SubscriptionId that's listening to that tag (multiple Proxy subs may share a Galaxy attribute — single COM subscription, multi-fan-out on the wire). Stub + DbBacked backends declare the events with #pragma warning disable CS0067 (treat-warnings-as-errors would otherwise fail on never-raised events that exist only to satisfy the interface).
• Medium 4 — WriteValuesAsync doesn't await OnWriteComplete. New WriteAsync(...) overload returns bool after awaiting the OnWriteComplete callback via the v1-style TaskCompletionSource-keyed-by-item-handle pattern in _pendingWrites. MxAccessGalaxyBackend.WriteValuesAsync now reports per-tag Bad_InternalError when the runtime rejected the write, instead of false-positive Good.

Pipe server change

IFrameHandler gains AttachConnection(FrameWriter writer): IDisposable so the handler can register backend event sinks on each accepted connection and detach them at disconnect. The PipeServer.RunOneConnectionAsync calls it after the Hello handshake and disposes it in the finally of the per-connection scope. StubFrameHandler returns IFrameHandler.NoopAttachment.Instance (net48 doesn't support default interface methods, so the empty-attach lives as a public nested class).

Tests

dotnet test ZB.MOM.WW.OtOpcUa.slnx: 460 pass / 7 skip (E2E on admin shell) / 1 pre-existing baseline failure. No regressions. The Driver.Galaxy.Host unit tests + 5 live ZB smoke + 3 live MXAccess COM smoke all pass unchanged.

Test plan for reviewers

• dotnet build clean
• dotnet test shows 460/7-skip/1-baseline
• Spot-check MxAccessClient.MonitorLoopAsync against v1's MxAccessClient.Monitor partial (src/ZB.MOM.WW.OtOpcUa.Host/MxAccess/MxAccessClient.Monitor.cs) — same polling cadence, same probe-then-reconnect-with-replay shape
• Read GalaxyFrameHandler.ConnectionSink.Dispose and confirm event handlers are detached on connection close (no leaked invocation list refs)
• WriteValuesAsync returning Bad_InternalError on a runtime-rejected write is the correct shape — confirm against the v1 MxAccessClient.ReadWrite.cs pattern

What's NOT in this PR

• Wonderware Historian SDK plugin port (Task B.1.h) — separate PR, larger scope.
• Alarm subsystem wire-up (MxAccessGalaxyBackend.SubscribeAlarmsAsync is still a no-op). OnAlarmEvent is declared on the backend interface and pushed by the frame handler when raised; MxAccessGalaxyBackend just doesn't raise it yet (waits for the alarm-tracking port from v1's AlarmObjectFilter + Galaxy alarm primitives).
• Host-status push (OnHostStatusChanged) — declared on the interface and pushed by the frame handler; MxAccessGalaxyBackend doesn't raise it (the Galaxy.Host's HostConnectivityProbe from v1 needs porting too, scoped under the Historian PR).

Adversarial review

Quick pass over the PR 4 deltas. No new findings beyond:

• Low 1 — MonitorLoopAsync's $Heartbeat probe item-handle is leaked (AddItem succeeds, never RemoveItem'd). Cosmetic — the probe item is internal to the COM connection, dies with Unregister at disconnect/recycle. Worth a follow-up to call RemoveItem after the probe succeeds.
• Low 2 — Replay loop in MonitorLoopAsync swallows per-subscription failures. If Galaxy permanently rejects a previously-valid reference (rare but possible after a re-deploy), the user gets silent data loss for that one subscription. The stub-handler- unaware operator wouldn't notice. Worth surfacing as a ConnectionStateChanged(false) → ConnectionStateChanged(true) payload that includes the replay-failures list.

Both are low-priority follow-ups, not PR 4 blockers.