diff --git a/design/followups.md b/design/followups.md index 2a4a50b..a4a0527 100644 --- a/design/followups.md +++ b/design/followups.md @@ -6,20 +6,6 @@ move to `## Resolved` with a date + commit hash. ## Open -### F45 — Recovery replay should re-issue `RegisterReference` for buffered subscriptions -**Severity:** P2 — F36 buffered subscriptions survive across `recover_connection` only via `AdviseSupervisory` replay, which loses the `.property(buffer)` registration. -**Source:** `crates/mxaccess/src/session.rs::recover_connection_core` (the loop iterates `subscriptions` and replays via `advise_supervisory`). -**Depends on:** F36 (closed by the same iteration as this followup is filed). - -**Scope.** `Session::subscribe_buffered` records its `Subscription` in the same `SessionInner::subscriptions` registry as plain `subscribe` does, so the registry-walking recovery loop replays them via `AdviseSupervisory` rather than `RegisterReference` with `.property(buffer)`. The metadata stored in `SubscriptionEntry` is the original (un-suffixed) tag's `GalaxyTagMetadata`; the buffered name suffix is lost on replay. The server may continue to deliver values under the existing `.property(buffer)` registration on the engine side because the OBJREF / engine id pair survives the rebuild — but if the server tears the buffered registration down on disconnect, recovery will silently downgrade buffered → plain. - -**Definition of done:** -1. `SubscriptionEntry` gains a discriminator (`enum SubscriptionMode { Plain, Buffered { rounded_interval_ms: u32 } }`) so recovery can branch on the original advise shape. -2. The buffered branch in `recover_connection_core` rebuilds the original `NmxReferenceRegistrationMessage` (with `.property(buffer)` suffix + the saved correlation id + `subscribe = true`) and dispatches `register_reference` against the rebuilt transport. -3. Live regression: `cargo run -p mxaccess --example subscribe-buffered` against AVEVA, then force a recovery via `Session::recover_connection`, and confirm subsequent `OnBufferedDataChange`-rate updates continue at the same cadence. - -**Resolves when:** the recovery path treats buffered subscriptions identically to how the original advise was issued. - ### 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. @@ -31,6 +17,12 @@ move to `## Resolved` with a date + commit hash. ## Resolved +### F47 — `Session::unsubscribe` should skip `UnAdvise` for buffered subscriptions +**Resolved:** 2026-05-06 (commit `1a1830f`). `Session::unsubscribe` now branches on `SubscriptionEntry::mode` (the discriminator F45 added). For `SubscriptionMode::Buffered { ... }`, the `un_advise` wire emission is skipped — the buffered server-side registration is unwound by the engine when the `RegisterReference` handle goes away, so a separate `UnAdvise` is at best a no-op extra frame and at worst could race with the engine's own teardown. Mirrors the .NET reference's `if (!subscription.IsBuffered)` guard at `MxNativeSession.cs:361-381`. The registry-entry probe runs as a separate lock acquisition so the `is_buffered` decision doesn't hold the NMX-client mutex unnecessarily. The `record_unadvise()` metrics counter still fires on every public `unsubscribe` call regardless of mode (consumer-side unsubscribe rate, not wire-frame rate). New unit test `unsubscribe_skips_un_advise_for_buffered_subscription` issues a plain subscribe (recorded as 1 RPC), mutates the registry entry to `SubscriptionMode::Buffered`, calls unsubscribe, and asserts the recorded RPC count stays at 1 (no UnAdvise emitted). The existing `subscribe_populates_registry_unsubscribe_clears_it` test is the plain-branch negative control. Workspace 794 → 795 tests; clippy + rustdoc clean. + +### F45 — Recovery replay should re-issue `RegisterReference` for buffered subscriptions +**Resolved:** 2026-05-06 (commit `9b57cf8`). New `pub(crate) enum SubscriptionMode { Plain, Buffered { rounded_interval_ms, item_definition, item_context, item_handle } }` discriminator on `SubscriptionEntry`. `Session::subscribe` (plain path) records `SubscriptionMode::Plain`; `subscribe_buffered_nmx` records `SubscriptionMode::Buffered { ... }` carrying the un-suffixed reference + the rounded interval (so the re-issued buffered registration matches the original cadence). `recover_connection_core` matches on `entry.mode`: plain branch unchanged; buffered branch re-applies `.property(buffer)` via `to_buffered_item_definition` (idempotent), rebuilds the original `NmxReferenceRegistrationMessage` with the saved correlation id + `subscribe = true`, and dispatches `register_reference` (kind=ItemControl, inner command `0x10`) against the replacement transport. Mirrors `MxNativeSession.ReAdviseSubscription` (`MxNativeSession.cs:538-569`). New unit test `recover_connection_replays_buffered_subscription_via_register_reference` synthesises a buffered registry entry, installs a `RebuildFactory` pointing at a recording NMX server, drives `recover_connection`, then asserts the recorded `TransferData` carries inner command `0x10` (NOT `0x1f`) with the `.property(buffer)`-suffixed item_definition + the saved correlation id + subscribe=true. Public API unchanged (`SubscriptionMode` + `SubscriptionEntry` stay `pub(crate)`); `cargo public-api -p mxaccess` baseline unchanged. Workspace 793 → 794 tests; clippy + rustdoc clean. Side-finding spawned **F47** (`Session::unsubscribe` divergence on buffered drop). + ### F46 — Capture `LmxProxy.dll!CLMXProxyServer.Suspend`/`.Activate` wire emission **Resolved:** 2026-05-06 (commit `808fea1`). `analysis/frida/mx-nmx-trace.js` extended with `Interceptor.attach` hooks on `LmxProxy.dll!CLMXProxyServer.Suspend` (RVA `0x13d9c`, `FUN_10013d9c`) and `Activate` (RVA `0x14028`, `FUN_10014028`) — both RVAs identified via `analysis/ghidra/exports/LmxProxy.dll.string-refs.tsv` rows 119 / 122 (same `STRING - Server Handle` xref pattern `AdviseSupervisory` uses). Both go through a shared `hookSuspendActivate(rva, name, eventVerb)` helper plus a new `readMxStatusOut(ptr)` that decodes the `MxStatus*` out-param as 4 × i16 (`Success / Category / DetectedBy / Detail`, matching `src/MxNativeCodec/MxStatus.cs`). Hooks emit `mx.suspend.begin/end` and `mx.activate.begin/end` events for grep-ability. **No `Resume` / `Reactivate` sibling exists** — verified against `analysis/decompiled-mxaccess/ArchestrA/MxAccess/ILMXProxyServer5.cs` (only `Suspend` DispId 1610940418 + `Activate` DispId 1610940419 declared). Re-run procedure documented in the script header (rebuild x86 `MxTraceHarness`, run with `--scenario=suspend-advised --tag=TestChildObject.ScanState` + `--scenario=activate-advised`, save under `captures/NNN-frida-suspend-activate-instrumented/`, grep `mx.suspend.*` / `mx.activate.*` and correlate with `nmx.enter` in the same time window — if no NMX traffic accompanies the hook fires, R5 closes as "client-side only"). R5 in `design/70-risks-and-open-questions.md` updated to point at F46 as the next-step. Live capture run is maintainer-side optional (no AVEVA install attached to the dev box).