From 2546710604412e6f6d7fcef200b61002a645607c Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 6 May 2026 04:28:38 -0400 Subject: [PATCH] design/followups: add F35-F44 for M6 implementation plan 10 new followups decompose M6 (compatibility shim + production hardening) into parallel-safe sub-streams: - F35: mxaccess-compat LMXProxyServer-shaped facade (18 methods over Session/AsbSession) - F36: Session::subscribe_buffered NMX path per R2 single-sample - F37: ASB subscribe_buffered capability gate - F38: counting-allocator cargo bench harness for R12 target - F39: zero-copy codec pass (depends on F38) - F40: optional metrics feature - F41: cargo public-api baseline (depends on F35/F36/F37/F39/F40) - F42: cargo doc cleanup pass - F43: cargo publish --dry-run all crates (depends on F41) - F44: decode buffered batch + suspend captures (077, 079-082, 094) for R2/R5 evidence Parallelization: Wave 1 = F35/F36/F37/F38/F40/F42/F44 (different crates / different concerns); Wave 2 = F39 (needs F38's bench); Wave 3 = F41 (needs API stable); Wave 4 = F43 (release). Co-Authored-By: Claude Opus 4.7 (1M context) --- design/followups.md | 171 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 171 insertions(+) diff --git a/design/followups.md b/design/followups.md index c9b33f7..0c81790 100644 --- a/design/followups.md +++ b/design/followups.md @@ -6,6 +6,177 @@ 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`). + +### F36 — `Session::subscribe_buffered` (NMX) per R2 single-sample-per-event answer +**Severity:** P1 — blocks M6 DoD bullet 2 (`subscribe_buffered` (NMX feature) — guarded by `BufferedOptions`; no synthesis if provider returns single-sample batches). +**Source:** `design/60-roadmap.md:97` + `design/70-risks-and-open-questions.md` R2 (single-sample-per-event verified against `wwtools/mxaccesscli/docs/api-notes.md:97-100,138-140,154-157`). + +**Scope.** Add a `subscribe_buffered(reference, BufferedOptions { update_interval_ms })` method to `mxaccess::Session` that returns `Stream>` — same item shape as plain `subscribe`, just with a per-session-cached cadence knob. Internal wiring: +1. Translate `update_interval_ms` to LMX's `SetBufferedUpdateInterval` semantics (rounded to nearest 100ms per `MxNativeCompatibilityServer.SetBufferedUpdateInterval:638`). +2. Use the existing `Session::subscribe` machinery; the buffered cadence is a server-side delivery rate knob, not a payload-shape change. +3. Surface as a separate Session method (not just an option on `subscribe`) so the API discoverably documents the cadence semantics. + +**Definition of done:** +1. `Session::subscribe_buffered` returns `Stream>` and internally drives the LMX `SetBufferedUpdateInterval` + `AddBufferedItem` call sequence per the captures `079-frida-add-buffered-advise-testint` and `082-frida-add-buffered-plain-advise-testint`. +2. New example `examples/subscribe-buffered.rs` exercises a 1-second cadence against the live AVEVA install. Per R2, no multi-sample synthesis. +3. Integration test asserts `Stream::Item == DataChange` (no `DataChangeBatch`); compile-time check. +4. Doc on `subscribe_buffered` cites R2's verification source and explicitly says "single-sample, cadence knob — not multi-sample payload." + +**Resolves when:** `cargo run -p mxaccess --example subscribe-buffered` runs against AVEVA and the live captures `079`/`082` byte-round-trip via the new code path. + +### F37 — ASB `subscribe_buffered` capability gate +**Severity:** P3 — small wiring item; M5 already documented `Error::Unsupported(SubscribeBuffered)` as the intended ASB shape (`design/60-roadmap.md:88`). +**Source:** `design/60-roadmap.md:88` + F18 closeout block. + +**Scope.** Wire `AsbSession::subscribe_buffered` (and the equivalent on the `mxaccess::Session` ASB-transport variant if the API converges) to return `Error::Unsupported(Capability::SubscribeBuffered)` synchronously. ASB does not have a buffered-cadence equivalent — the per-monitored-item `SampleInterval` already plays that role. + +**Definition of done:** +1. Calling `subscribe_buffered` against an ASB-backed session returns `Error::Unsupported(Capability::SubscribeBuffered)` without touching the wire. +2. Unit test asserts the error variant + capability discriminant. +3. Doc on the method explains the ASB asymmetry (per-monitored-item `SampleInterval` is the buffered-cadence analogue, callers should use `MinimalMonitoredItem.sample_interval` instead). + +**Resolves when:** F36 has the NMX path landed and the same call signature on ASB returns the typed `Unsupported` error. + +### F38 — Counting-allocator `cargo bench` harness +**Severity:** P1 — blocks M6 DoD bullet 3 (per-write allocation target measurable) and F39 (zero-copy pass needs the harness to verify). +**Source:** `design/60-roadmap.md:102` + `design/70-risks-and-open-questions.md` R12. + +**Scope.** Add a `cargo bench` setup under `crates/mxaccess-codec/benches/` (and possibly `mxaccess/benches/`) that wraps the global allocator with a counting allocator (e.g. `dhat` or a hand-rolled `GlobalAlloc` impl) and reports per-operation allocation counts for the proven matrix: +- write encode (Int32 / Float / Double / String / Bool) +- subscribe decode (DataUpdate single-record) +- ASB ConnectedRequest envelope encode (RegisterItems + AddMonitoredItems) + +Print baseline numbers in a CSV/markdown table committed to `design/` so future PRs can diff against it. + +**Definition of done:** +1. `cargo bench -p mxaccess-codec` runs on Windows + measures alloc counts for the proven matrix. +2. Baseline numbers committed to `design/M6-bench-baseline.md` (or similar). +3. F39 can use this harness to verify the < 5 allocs/write target. + +**Resolves when:** baseline numbers are committed and the harness is reproducible. + +### F39 — Zero-copy codec pass (per R12) +**Severity:** P1 — M6 DoD bullet 3. +**Source:** `design/70-risks-and-open-questions.md` R12 + `design/60-roadmap.md:97-102`. +**Depends on:** F38 (bench harness must exist to verify). + +**Scope.** Convert the codec's encode path to `BytesMut::with_capacity(MAX_FRAME)` and the decode path to `bytes::Bytes` zero-copy slices where possible. Target < 5 allocations per write at steady state. Subscription stream should not allocate per-message (all per-frame buffers reused via a per-connection pool). + +**Definition of done:** +1. `cargo bench -p mxaccess-codec` reports < 5 allocs per write encode for the proven matrix. +2. Live `cargo run -p mxaccess --example subscribe -- --tag TestChildObject.TestInt` under churn (10+ Hz updates) shows zero per-message allocations in the steady-state region of the bench output. +3. No correctness regressions in the round-trip fixture suite. + +**Resolves when:** the bench numbers hit the targets and the workspace test suite stays green. + +### 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`. +**Depends on:** F35, F36, F37, F39, F40 (the public surface must be stable before snapshotting). + +**Scope.** Run `cargo public-api` on each crate; commit the resulting baseline to `design/public-api/{crate}.txt`. Add a CI step that diffs against the baseline and fails if the public surface changes without a corresponding baseline update. + +**Definition of done:** +1. `cargo public-api -p mxaccess` runs clean + baseline committed. +2. Same for `mxaccess-codec`, `mxaccess-compat`, `mxaccess-asb`, `mxaccess-asb-nettcp`, `mxaccess-galaxy`, `mxaccess-rpc`, `mxaccess-callback`, `mxaccess-nmx`. +3. CI diff step in `.github/workflows/rust.yml` (or equivalent). + +**Resolves when:** baseline files exist and CI catches drift. + +### F42 — `cargo doc` cleanup pass +**Severity:** P2 — M6 DoD bullet 5. +**Source:** `design/60-roadmap.md:99`. + +**Scope.** Run `cargo doc --workspace --no-deps` and fix every rustdoc warning (broken intra-doc links, missing top-level crate doc, missing examples on public items). Add `#![warn(missing_docs)]` lints to crate roots after the cleanup so future regressions are caught. + +**Definition of done:** +1. `cargo doc --workspace --no-deps -- -D warnings` passes clean. +2. Each crate has a top-level doc comment summarising its role. +3. Public API items have at least a one-line doc comment. + +**Resolves when:** the doc build is warning-clean and the lint enforces it going forward. + +### F43 — Release prep: `cargo publish --dry-run` all crates +**Severity:** P1 — M6 DoD bullet 6. +**Source:** `design/60-roadmap.md:100`. +**Depends on:** F41 (public-api baseline). + +**Scope.** Run `cargo publish --dry-run -p {crate}` for every workspace crate. Resolve any missing `description`, `keywords`, `categories`, `readme` metadata fields. Decide a version-bump strategy (likely 0.1.0 across the board for V1 release). + +**Definition of done:** +1. `cargo publish --dry-run` passes for every crate. +2. Workspace `Cargo.toml` + per-crate metadata complete. +3. Release notes draft in `CHANGELOG.md` for V1. + +**Resolves when:** dry-runs are green and the release notes are written. + +### 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. + ### F3 — Cross-domain NTLM Type1/2/3 fixture **Severity:** P2 **Status:** Permanently out-of-scope on the current dev host (no second AD domain). Resolution requires external infrastructure not available here.