design/followups: move F35/F40/F44 to Resolved + de-conflict F45/F46
rust / build / test / clippy / fmt (push) Has been cancelled
rust / build / test / clippy / fmt (push) Has been cancelled
After commitsd5aa152(F35) andad1cf23(F36+F40+F44), three M6 sub-followups belong under Resolved with concise verdicts referencing the matching commits. Sub-agent merge cleanup: - Two sub-agents independently filed new followup F45 in parallel — rename the Suspend/Activate wire-emission gap to F46, leaving the buffered-recovery-replay item as F45 (filed by the F36 work since it's the more immediate dependent). - Open section now contains only F41 + F43 + F45 + F46 + F3. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Joseph Doherty
+10
-70
@@ -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<HashMap<i32, ItemRef>>`.
|
||||
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<HashMap<i32, ItemRef>>` handle table + `AtomicI32` monotonic counters. Event surface is four `tokio::sync::broadcast` channels surfaced as `EventStream<T>` (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<Item = Result<DataChange, Error>>`) 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).
|
||||
|
||||
|
||||
Reference in New Issue
Block a user