docs(plan): perf follow-ups — 15-task plan off the remainder's follow-up register

2026-08-17 03:41:40 -04:00
parent e40a038527
commit f06cbe16d9
2 changed files with 636 additions and 0 deletions
@@ -0,0 +1,615 @@
+# Perf Follow-ups Implementation Plan
+
+> **For Claude:** REQUIRED SUB-SKILL: Use superpowers-extended-cc:subagent-driven-development
+> to implement this plan task-by-task (per the standing preference: Opus implementers,
+> classification-driven review chains).
+
+**Goal:** Close the actionable items in the perf-roadmap-remainder follow-up register
+(`docs/plans/2026-08-15-perf-roadmap-remainder.md` §"Follow-up register") and run the two
+release-condition live gates, on branch `perf/followups` off master merge `e40a0385`.
+
+**Architecture:** No new subsystems. Each task is a surgical close of a recorded follow-up:
+a metrics ride-along on the existing shared `OtOpcUaTelemetry.Meter`, four defect fixes in
+already-reviewed code (drain cancellation, ACL eviction race, revalidate memo key,
+`TrackBackgroundTask` ordering), two SQL-Server-backed test gaps, one parser streaming
+rewrite, an S7 multi-var read extension over the existing `IS7Plc` seam, and two live gates
+(s7_1500 fixture + local docker-dev rig).
+
+**Tech Stack:** .NET 10 / C#, xUnit + Shouldly, System.Diagnostics.Metrics, EF Core
+(SqlServer + InMemory), S7netplus 0.20.0, FasterLog, docker-dev rig (OrbStack).
+
+**Out of scope (recorded, deliberately not here):**
+- **HistorianGateway multi-tag batch `WriteLiveValues` RPC** — cross-repo
+  (`~/Desktop/HistorianGateway` proto + client-package release + pin bump here). Stays in
+  `docs/Historian.md` as the recorded ceiling on `DrainConcurrency`.
+- **S7 `SByte`** — a plan error, already corrected; not a gap.
+- `UpsertNodeDeploymentState` / `TryRecoverFromStale` synchronous writes — commented in
+  `DriverHostActor.cs:2930-2932` / `:2979-2982` as deliberate; not on the hot path.
+- Host-side mux interest gating — recorded as not-worth-it after the batch envelope.
+- Historian end-to-end live gate against the real AVEVA Historian — needs the
+  `wonder-sql-vd03` VPN (user-driven); recipe stays
+  `docs/plans/2026-08-01-continuous-historization-live-gate.md`.
+- Pushing master — user decision, unchanged.
+
+**Execution rules (unchanged from the remainder plan):** shared checkout — explicit-pathspec
+`git add`/`git commit -- <files>` only (never a bare commit); never checkout/switch/reset/
+stash/clean; retry on `index.lock` and CS2012/MSB3030 build races; reviewers judge commits
+via `git show`, never the working tree; falsification mutations reverted via Edit; TDD
+red-first with honest "pin, not red-first" reporting where the behaviour predates the test.
+
+---
+
+### Task 1: Per-driver poll-overrun meter + PollGroupEngine doc notes
+
+**Classification:** standard
+**Estimated implement time:** ~5 min
+**Parallelizable with:** Tasks 2, 3, 4, 5, 6, 7, 8, 10, 11
+
+**Files:**
+- Modify: `src/Core/ZB.MOM.WW.OtOpcUa.Commons/Observability/OtOpcUaTelemetry.cs` (new counter)
+- Modify: `src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/PollGroupEngine.cs` (doc notes only)
+- Modify (one-line each, at the `new PollGroupEngine(` call sites): `ModbusDriver.cs:157`,
+  `FocasDriver.cs:117`, `TwinCATDriver.cs:76`, `AbLegacyDriver.cs:74`, `S7Driver.cs:65`,
+  `SqlDriver.cs:226`, `AbCipDriver.cs:157`
+- Test: `tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/` (PollGroupEngine tests file) +
+  one per-driver wiring pin is NOT required (see below)
+
+**Spec.** The seven poll drivers' pacing change currently ships with no production consumer
+of `PollGroupEngine.OverrunCount` (the accepted-not-fixed item). Close it the way the
+engine's own doc note (`PollGroupEngine.cs:47-50`) prescribes: the driver wires its own
+meter to the existing `onOverrun` callback (ctor param, line 105).
+
+1. Add to `OtOpcUaTelemetry` (following the existing `CreateCounter` field pattern, e.g.
+   `otopcua.driver.lifecycle` at line 42):
+   `public static readonly Counter<long> PollOverruns = Meter.CreateCounter<long>("otopcua.driver.poll.overrun", unit: "{overrun}", description: "Poll cycles that ran longer than the poll interval (slow success and slow failure both count — see PollGroupEngine.NoteOverrun).");`
+2. In each of the seven drivers, pass
+   `onOverrun: _ => OtOpcUaTelemetry.PollOverruns.Add(1, new KeyValuePair<string, object?>("driver.type", "<type>"))`
+   where `<type>` is the driver's `DriverTypeNames` constant (use the constant, not a
+   literal — the `TwinCat`/`Focas` drift was a real bug once). If the constructor scope has
+   a cheap device/instance identifier already in hand, add a second `device` tag; do NOT
+   plumb new state to get one.
+   Note: Commons is referenced from driver projects already via the messages namespace — if
+   any of the seven driver csproj lacks the `Commons` reference, add the ProjectReference;
+   report it rather than inventing a per-driver Meter (Galaxy's own-meter pattern is the
+   outlier, not the template).
+3. Doc notes in `PollGroupEngine.cs` (comment-only):
+   - Update lines 47-50: the follow-up is now delivered driver-side via
+     `OtOpcUaTelemetry.PollOverruns`; the engine still deliberately owns no Meter.
+   - `NoteOverrun` (lines 315-322): add the caveat that it conflates a slow success with a
+     slow failure — both increment; disambiguation would need the loop's outcome, and no
+     consumer needs it yet.
+   - `backoffCap` ctor param (line 104): doc its units (a `TimeSpan` cap on the exponential
+     failure backoff) and its interaction with `minInterval` (backoff replaces, not adds to,
+     the interval on the failure path — state whichever the loop at 261-263 actually does;
+     read it, don't guess).
+
+**Tests.** One unit test on the engine pinning that an overrun invokes the `onOverrun`
+callback exactly as many times as `OverrunCount` increments (may already exist from Task 11
+of the remainder — if so, extend or cite it; do not duplicate). The seven driver wirings are
+one-line closures over a static counter — no per-driver test; a wiring test would just
+restate the line.
+
+**Step order:** test (or cite existing) → counter → 7 wirings → doc notes → build the
+solution → commit.
+
+**Commit:** `feat(drivers): per-driver poll-overrun meter on the shared OtOpcUa Meter; NoteOverrun + backoffCap doc notes`
+
+---
+
+### Task 2: S7 span-option validation guard
+
+**Classification:** small
+**Estimated implement time:** ~3 min
+**Parallelizable with:** Tasks 1, 3, 4, 5, 6, 7, 8, 10, 11
+
+**Files:**
+- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7DriverFactoryExtensions.cs` (`ParseOptions`, line 46 area)
+- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Contracts/S7DriverOptions.cs` (doc remark only, if needed)
+- Test: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/` (the factory/options parse test file)
+
+**Spec.** `SpanReadMaxSpanBytes` (`S7DriverOptions.cs:128`, default 220) has no validation:
+a nonsensical value is accepted at config time and only fails (or silently misbehaves) at
+read time. There is no `IValidateOptions` machinery for S7 (confirmed) — `ParseOptions` in
+`S7DriverFactoryExtensions.cs` is the single choke point, so validate there:
+
+- `SpanReadMaxSpanBytes < 1` → throw `ArgumentException`/config-error consistent with how
+  `ParseOptions` reports other bad config (read the method; match its existing error style).
+- `SpanReadMaxGapBytes < -1` → same (−1 = disabled is legal, per `S7Driver.cs:571`'s
+  `>= 0` enable check; 0 is legal "no gap tolerated").
+- Values **above** the negotiated PDU size cannot be checked here — the negotiated size is
+  opaque inside S7.Net (confirmed: no API exposes it) and S7.Net transparently chunks
+  oversized reads. Do NOT invent a fake upper bound; instead add one doc line to the
+  `SpanReadMaxSpanBytes` remark in `S7DriverOptions.cs` stating that over-PDU values are
+  legal but degrade to multi-request chunking inside S7.Net.
+
+**Tests.** Red-first: `SpanReadMaxSpanBytes = 0` rejected; `SpanReadMaxGapBytes = -2`
+rejected; `-1`/`0`/defaults accepted.
+
+**Commit:** `fix(s7): validate span-read options at parse time; document the over-PDU degradation`
+
+---
+
+### Task 3: Historian drain — shutdown cancellation is not a per-tag failure
+
+**Classification:** high-risk (concurrency, #538 quarantine semantics)
+**Estimated implement time:** ~5 min
+**Parallelizable with:** Tasks 1, 2, 4, 5, 6, 7, 8, 10, 11
+
+**Files:**
+- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Historian/ContinuousHistorizationRecorder.cs:703-712`
+- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Historian/ContinuousHistorizationRecorderTests.cs`
+
+**Spec.** In `DrainOnceAsync`, the per-tag parallel lambda wraps
+`_writer.WriteLiveValuesAsync(g.Key, g.Values, ct)` in a **bare `catch`** (`:707-712`) that
+converts a shutdown `OperationCanceledException` into `ok = false`. When sibling tags in the
+same pass already succeeded, the #538 logic (`:775-784`) then counts the cancelled tag as a
+consecutive per-tag failure — a shutdown can push a healthy tag toward abandonment. The
+outer `catch (OperationCanceledException) when (ct.IsCancellationRequested)` (`:812-816`)
+already treats whole-pass cancellation as non-fault but is never reached because the inner
+catch eats it first.
+
+Fix: in the per-tag lambda, rethrow genuine token cancellation ahead of the bare catch:
+
+```csharp
+catch (OperationCanceledException) when (ct.IsCancellationRequested)
+{
+    throw;
+}
+catch
+{
+    ok = false;
+}
+```
+
+The rethrow propagates through the awaited per-tag task to the pass's `await` and lands in
+the existing outer catch → `DrainResult(true, 0, 0)`, no ack, no failure counts. Verify the
+`SemaphoreSlim window` release still runs on this path (it is in a `finally` — confirm, and
+keep it that way). Do NOT touch the #538 discriminator ("did another tag write this pass"),
+the all-or-nothing ack, or the falsification control's `succeeded == 0` keying.
+
+**Tests.** Red-first: a writer fake where tag A completes successfully, then the token is
+cancelled and tag B's write throws `OperationCanceledException(ct)`. Assert: the pass does
+not ack (outbox not truncated), and — the load-bearing assertion — tag B accumulates **no**
+failure count (observe via the same seam the existing #538 tests
+`A_permanently_failing_tag_is_abandoned...` (`:408`) and
+`A_backend_that_fails_every_tag_abandons_nothing` (`:456`) use: run `MaxWriteAttempts`
+shutdown-cancelled passes interleaved with sibling successes, then show the tag is NOT
+abandoned). Also keep a control: a non-cancellation throw from B with A succeeded still
+counts (pins that the fix is scoped to genuine `ct` cancellation — an OCE thrown while
+`ct.IsCancellationRequested == false` must still count as a failure, which the `when` filter
+gives for free; test that too).
+
+**#491 hazard note:** this file is the `ConfigureAwait(false)`-destroys-`Self` site. The fix
+touches only the catch clauses inside an existing async lambda — do not add `ConfigureAwait`
+changes, do not touch `Self` usage.
+
+**Commit:** `fix(historian): a shutdown cancellation mid-drain is a pass abort, not a per-tag failure`
+
+---
+
+### Task 4: Direct-apply artifact read — explicit command timeout
+
+**Classification:** standard
+**Estimated implement time:** ~4 min
+**Parallelizable with:** Tasks 1, 2, 3, 5, 6, 7, 8, 10, 11
+
+**Files:**
+- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs:2082-2107` (`LoadArtifactBlob`)
+- Test: the DriverHostActor direct-apply test file (locate the Task 26 tests around `BeginDirectApply`/`ArtifactForApply` in `tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/`)
+
+**Spec.** The Task 26 Important: `LoadArtifactBlob` (EF read of `Deployment.ArtifactBlob`,
+runs in `Task.Run` off the mailbox) carries no timeout or cancellation. The code relies on
+an implicit bound and does not assert it. Confirmed by exploration: nothing sets a command
+timeout anywhere on the ConfigDb path (`ServiceCollectionExtensions.cs:30` has no
+`CommandTimeout`, no connection string in the repo carries `Command Timeout=`), so the only
+bound is SqlClient's default 30 s command timeout.
+
+Fix — make the bound explicit and local (do NOT change the global pooled-factory options;
+that would silently re-time every ConfigDb consumer, including migrations and long admin
+queries):
+
+1. In `LoadArtifactBlob`, after `CreateDbContext()`:
+   `db.Database.SetCommandTimeout(TimeSpan.FromSeconds(30));`
+   with a comment: this read runs off the mailbox on a pool thread with no CancellationToken
+   (the actor cannot be wedged, but a hung SQL read would pin a pool thread); the explicit
+   timeout replaces reliance on SqlClient's default so a future connection-string or
+   provider-default change cannot unbound it. A timeout surfaces as the existing
+   catch-to-null path (`:2093-2106`) → the apply fails, revision does not advance — the
+   #485-family "no answer" semantics, already correct.
+2. Do not thread a `CancellationToken` — `FirstOrDefault` would need to become async and the
+   only caller-side token is actor lifetime; the timeout gives the same bound without
+   touching the actor seam. State this in the comment.
+
+**Tests.** The behaviour delta is not black-box observable without a hung SQL server, so:
+pin with a source-level test only if the repo already has the source-scan pin pattern for
+this file; otherwise a unit test that `LoadArtifactBlob`'s failure path (throwing
+DbContext) still returns null and fails the apply (may already exist — cite it), plus the
+comment. Say honestly in the report which it was.
+
+**Commit:** `fix(runtime): explicit 30s command timeout on the Direct-apply artifact read`
+
+---
+
+### Task 5: SessionAclDecisionCache — eviction race on `_globalCount` + stress test
+
+**Classification:** high-risk (lock-free concurrency)
+**Estimated implement time:** ~5 min
+**Parallelizable with:** Tasks 1, 2, 3, 4, 6, 7, 8, 10, 11
+
+**Files:**
+- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Security/SessionAclDecisionCache.cs:178-182`
+- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/SessionAclDecisionCacheTests.cs`
+
+**Spec.** The per-shard eviction branch (`Set()`, lines 178-182) runs
+`shard.Entries.Clear()` then `Interlocked.Add(ref _globalCount, -Interlocked.Exchange(ref shard.ApproxCount, 0))`
+as two non-atomic steps. A concurrent `Set` on the same shard can insert into the
+just-cleared dictionary and increment both counts between them; the evictor's `Exchange`
+then subtracts a count that includes the racer's fresh insert → `_globalCount` under-counts
+(negative-only drift; trims early, never late — correctness of decisions is unaffected,
+which is why this was deferred).
+
+Fix by making the eviction atomic with respect to concurrent inserts: **replace the shard
+object** rather than clearing the shared dictionary — the same pattern `ClearAll()` already
+uses (it drops shard objects wholesale). Read the shard-storage shape first (how shards are
+held per session key) and swap a fresh `Shard` in with a CAS/indexer replace, then subtract
+the *displaced* shard's `ApproxCount` from `_globalCount`. A racer that inserted into the
+displaced shard loses its entry (acceptable — it re-caches on next miss) but its global
+increment is subtracted with the shard it landed in, so the count cannot over-subtract.
+Preserve: the outer-bound gate at `:171`, generation-rollover `ClearAll()` at `:238`/`:250`,
+and the "increment only on genuine `TryAdd`" invariant at `:186-189`.
+
+**Tests.** Red-first is impractical for a race; do it as: (1) a deterministic interleaving
+test if the seams allow (inject at the eviction boundary), else honestly a stress test —
+N writer threads hammering one session's shard past `MaxEntriesPerSession` while others
+insert, barrier-started, several rounds; assert at quiescence `_globalCount` equals the
+actual live entry total (expose via an internal count-audit helper or reflection over
+shards) and is never negative. Run the stress test with the fix reverted (Edit-revert, then
+restore) and report whether it goes red — if the race won't reproduce under test load, say
+so honestly; the deterministic invariant audit is then the pin.
+
+**Commit:** `fix(acl): shard eviction swaps the shard object so _globalCount cannot over-subtract; concurrency stress test`
+
+---
+
+### Task 6: Revalidate memo — reject empty node identifiers
+
+**Classification:** small
+**Estimated implement time:** ~3 min
+**Parallelizable with:** Tasks 1, 2, 3, 4, 5, 7, 8, 10, 11
+
+**Files:**
+- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs:3319-3321` (`IsSubscriptionStillPermitted`)
+- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerStage6EnforcementTests.cs`
+
+**Spec.** The pass-local revalidation memo key (`:3327`) tolerates an empty string
+identifier: the guard at `:3319-3321` rejects only a *non-string* identifier, so all items
+on empty-`s=` nodes for one (session, realm) collapse onto one memoized verdict.
+`DataPlaneAuthorizer.Evaluate` already rejects exactly this at
+`DataPlaneAuthorizer.cs:253` (`identifier.Length == 0`). Mirror it: extend the guard with
+`|| identifier.Length == 0` so empty identifiers fall through to the un-memoized
+`IsAllowedCached` call at `:3323-3324` (fail-safe: un-memoized means each item gets its own
+authorizer verdict, and the authorizer's own guard then handles the empty id).
+
+**Tests.** Red-first in `NodeManagerStage6EnforcementTests` (it already drives
+`RevalidateMonitoredItems` directly at `:484`/`:515`): two monitored items on nodes with
+empty string identifiers, different underlying permit outcomes by whatever seam the tests
+use — assert they do NOT share one memoized verdict after the fix. If distinct outcomes for
+two empty-id nodes aren't constructible through the fake, pin instead that empty-id items
+bypass the memo (e.g. authorizer-call count equals item count for empty ids, vs. 1 for a
+memoized duplicate pair) — state which shape was used.
+
+**Commit:** `fix(acl): revalidation memo rejects empty node identifiers, mirroring the authorizer's guard`
+
+---
+
+### Task 7: SQL-Server-backed tests for the drift probe + rename script scan
+
+**Classification:** standard
+**Estimated implement time:** ~5 min
+**Parallelizable with:** Tasks 1, 2, 3, 4, 5, 6, 8, 10, 11
+
+**Files:**
+- Create: `tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/AdminUiSqlServerFixture.cs`
+- Create: `tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Deployments/DeploymentDriftProbeSqlServerTests.cs`
+- Create: `tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/RawTreeServiceRenameWarningSqlServerTests.cs`
+- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawTreeService.cs:1451` (one comment line)
+- Modify (if needed): `tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ZB.MOM.WW.OtOpcUa.AdminUI.Tests.csproj` (`Microsoft.Data.SqlClient` if not already transitive)
+
+**Spec.** Two register items say the same thing: EF InMemory does not exercise the SQL
+Server provider's translation, so (a) the Task 17 `GroupBy(_ => 1)` watermark aggregate
+(`DeploymentDriftProbe.ReadWatermarkAsync`, `DeploymentDriftProbe.cs:122-135`, over
+**`ConfigEdits`** — not deployments; the folder name misleads) and (b) the Task 21
+expression-built `SourceCode.Contains` OR-chain (`RawTreeService.AnyPathContained`,
+`:1488-1513`) are unverified where they matter.
+
+1. **Fixture**: copy `SchemaComplianceFixture`'s shape
+   (`tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/SchemaComplianceFixture.cs`): env
+   `OTOPCUA_CONFIG_TEST_SERVER` (default `10.100.0.35,14330`) +
+   `OTOPCUA_CONFIG_TEST_SA_PASSWORD` (default `OtOpcUaDev_2026!`), unique per-run DB name,
+   `Database.Migrate()`, drop-on-dispose, collection fixture. **One deliberate deviation**:
+   probe reachability first (short TCP/`SqlConnection` attempt) and expose a `SkipReason`;
+   tests call `Assert.Skip` when unreachable (the `Snap7ServerFixture.cs:41-92` pattern) —
+   AdminUI.Tests must stay green offline, unlike Configuration.Tests which fails hard.
+   Document the deviation in the fixture doc comment.
+2. **Drift probe test**: seed `ConfigEdits` rows with known timestamps, run the real probe
+   against the SQL-backed context factory, assert `LastEditUtc`/`EditCount` — proving the
+   `GroupBy(_ => 1)` aggregate translates and returns the right watermark (a translation
+   failure throws; wrong translation returns wrong values — both caught).
+3. **Rename scan test**: seed a `Script` whose `SourceCode` contains a tag path in
+   **different case** than the rename target. On SQL Server's default case-insensitive
+   collation the scan must find it (the "collation widening" the InMemory provider cannot
+   show — InMemory `string.Contains` is case-sensitive). Assert the warning names the
+   script. Also one same-case control. Exercise through the real
+   `BuildScriptReferenceWarningAsync` (`RawTreeService.cs:1437-1470`) with > 1 batch if
+   cheap (201 paths) to also cross the chunk boundary against the real provider.
+4. **Comment**: one line at the batching loop (`RawTreeService.cs:1451`): worst case is
+   ~250 sequential round-trips for a rename touching the 50k-path scale (200-term chunks);
+   deliberate — bounded memory beats one giant OR-chain.
+
+**Commit:** `test(adminui): SQL-Server-backed pins for the drift watermark aggregate and the rename scan's collation widening`
+
+---
+
+### Task 8: ScriptedAlarmEngine — TrackBackgroundTask ordering + volatile `_disposed`
+
+**Classification:** high-risk (concurrency, disposal)
+**Estimated implement time:** ~5 min
+**Parallelizable with:** Tasks 1, 2, 3, 4, 5, 6, 7, 10, 11
+
+**Files:**
+- Modify: `src/Core/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms/ScriptedAlarmEngine.cs` (`TrackBackgroundTask` `:996-1006`, `_disposed` `:172`, call sites `:559`, `:989`)
+- Test: `tests/Core/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms.Tests/ScriptedAlarmEngineTests.cs`
+
+**Spec.** Two recorded gaps:
+
+1. **Registration-ordering**: callers evaluate the async-method argument (which starts the
+   work) *before* `TrackBackgroundTask` adds it to `_inFlight`
+   (`TrackBackgroundTask(DrainDirtyAsync(...))` at `:559`). `Dispose` can snapshot
+   `_inFlight` (`:1090-1094`) in that window and drain past a genuinely in-flight task.
+   Fix by removing the window, not shrinking it: change the call shape to register-first —
+   e.g. give `TrackBackgroundTask` a `Func<Task>` overload that (under `_inFlightLock`)
+   installs a placeholder/`TaskCompletionSource` (or checks `_disposed` and declines to
+   start), then invokes the factory and links completion — so a task that runs is always
+   visible to `Dispose`'s snapshot, and after `_disposed` no new background work starts.
+   Preserve exactly: the drain-latch handover semantics
+   (`Sustained_synchronous_drain_failures_hand_over_without_deepening_the_stack`, `:1627` —
+   the handover MUST keep spawning via `Task.Run` on a fresh stack, commit `c8a70143`), the
+   `ExecuteSynchronously` self-removal, and `Dispose`'s synchronous `WhenAll` drain. If the
+   handover call site (`TrackBackgroundTask(Task.Run(() => DrainDirtyAsync(ct)))`) needs
+   reshaping to the factory form, keep the `Task.Run` inside.
+2. **`_disposed`**: make it `volatile` (`:172`) — it is written in `Dispose` (`:1078`) and
+   read unsynchronized on other threads (`:235, 544, 603, 662, 682, 987, 1027`).
+
+**Tests.** Red-first for the ordering fix where constructible: a factory whose task
+completes synchronously/immediately while `Dispose` races — deterministically, pin the new
+invariant instead: (a) after `Dispose`, `TrackBackgroundTask`'s factory form does not invoke
+the factory (no new work post-disposal); (b) a task started via the factory form is awaited
+by `Dispose` even when it completes before the continuation attaches. Every existing engine
+test (`:817`, `:1458`, `:1521`, `:1563`, `:1627`) must stay green — they are the spec for
+what must not change.
+
+**Commit:** `fix(alarms): background tasks register before they start; volatile _disposed`
+
+---
+
+### Task 9: Test-reliability hardening (durability second-reader, #491 controls)
+
+**Classification:** small
+**Estimated implement time:** ~4 min
+**Parallelizable with:** Tasks 4, 5, 6, 7, 8, 10, 11 (NOT Task 3 — same recorder test file)
+**Blocked by:** Task 3
+
+**Files:**
+- Modify: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/Recorder/FasterLogHistorizationOutboxTests.cs:172-197` (+ helper `ReadCommitted` `:203+`)
+- Modify: `tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Historian/ContinuousHistorizationRecorderTests.cs:359-387`, `:797-831` (doc comments only)
+
+**Spec.**
+1. `AppendBatch_group_commit_is_durable_before_dispose` depends on filesystem flush
+   visibility to a second `FasterLog` reader, which macOS does not guarantee on the
+   Windows/Linux timescale. Prefer a robust wait: retry the second-reader scan (re-open or
+   re-read up to `CommittedUntilAddress`) with a bounded poll (e.g. up to 5 s) before
+   asserting — the property under test is "durable before dispose", which a bounded wait
+   still proves (the writer is not disposed during the poll). Only if FasterLog's reader
+   genuinely cannot observe later flushes on re-scan, fall back to an OS-conditional
+   `Assert.Skip` on macOS with a comment naming the platform limitation. Report which.
+2. The two #491 yielding-fake tests: ensure each carries a comment stating the
+   falsification control is nondeterministic under load (ThreadStatic actor cell) — a single
+   green run of the intended-red mutation is not proof the regression is caught. The `:790-793`
+   comment covers part of this; make both tests carry the load-nondeterminism warning.
+
+**Commit:** `test(historian): bounded-wait durability read; honest nondeterminism notes on the #491 controls`
+
+---
+
+### Task 10: MTConnect `Build` streams through XmlReader
+
+**Classification:** standard
+**Estimated implement time:** ~5 min
+**Parallelizable with:** Tasks 1, 2, 3, 4, 5, 6, 7, 8, 11
+
+**Files:**
+- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectStreamsParser.cs` (`Build` `:221`, the three `Parse` overloads `:102/:138/:155`)
+- Test: the existing MTConnect parser test file(s) under `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/`
+
+**Spec.** Task 27 removed the per-chunk string copies; `Build(XDocument)` still materialises
+the whole document tree per sample chunk. Rewrite the parse path to stream via `XmlReader`
+(`XmlReader.Create(stream, settings)` with DTD prohibited, async off — match the sync call
+shape) extracting exactly what `Build` extracts today, and route all three `Parse` overloads
+through it (the string overload via a `StringReader`). Requirements:
+
+- **Byte-for-byte behavioural parity** with the current `Build` on every existing test —
+  the current suite is the spec. Namespace handling: MTConnect documents version their
+  namespace URI; whatever name/namespace matching `Build` does today (local-name-based or
+  namespace-qualified) must be preserved exactly — read `Build` first and mirror it.
+- Keep the `AsStream` zero-copy path (`:216-219`) feeding the reader.
+- If, mid-implementation, `Build`'s extraction turns out to lean on document-order queries
+  that make a single-pass reader genuinely intractable (e.g. backward references), STOP and
+  report rather than shipping a half-streamed hybrid — that finding would itself close the
+  register item as "not worth it".
+
+**Tests.** The existing suite must pass unchanged (parity). Add one test only if a gap is
+found in existing coverage of odd shapes (empty streams element, condition entries, unknown
+elements skipped) — cite, don't duplicate.
+
+**Commit:** `perf(mtconnect): stream sample-chunk parsing through XmlReader; no per-chunk document tree`
+
+---
+
+### Task 11: IS7Plc multi-variable read seam
+
+**Classification:** small
+**Estimated implement time:** ~3 min
+**Parallelizable with:** Tasks 1, 2, 3, 4, 5, 6, 7, 8, 10
+
+**Files:**
+- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/IS7Plc.cs` (interface `:18-66`, `S7PlcAdapter` `:125-149`)
+
+**Spec.** S7.Net 0.20.0 ships `Plc.ReadMultipleVarsAsync(List<DataItem>, CancellationToken)`
+(confirmed in the shipped DLL); `IS7Plc` does not expose it. Add
+`Task<IReadOnlyList<DataItem>> ReadMultipleVarsAsync(List<DataItem> items, CancellationToken ct)`
+to the interface and implement in `S7PlcAdapter` delegating to the library, wrapped in
+`S7OperationDeadline.RunAsync` exactly like the existing members (`:126-149` pattern —
+mirror it, including the deadline/exception shape). Return the same `DataItem` list S7.Net
+populates. No driver-side use yet (Task 12). Update any test fakes implementing `IS7Plc`
+(they will fail to compile otherwise — implement as `throw new NotSupportedException` in
+fakes that don't care, or a delegate hook in the shared fake if one exists).
+
+**Tests.** Compile-level only for the adapter (it is a one-line delegation under the same
+deadline wrapper the other members already have pinned); the behaviour tests come with Task 12's fake-driven driver tests. State this honestly.
+
+**Commit:** `feat(s7): expose ReadMultipleVarsAsync through the IS7Plc seam`
+
+---
+
+### Task 12: S7 multi-var reads for span leftovers
+
+**Classification:** high-risk (wire behaviour, partial-failure semantics)
+**Estimated implement time:** ~6 min (accepted over budget — do not split further; the fallback semantics need one head)
+**Blocked by:** Tasks 1, 11 (both touch `S7Driver.cs` / `IS7Plc.cs`)
+
+**Files:**
+- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs` (the `perTag` leftover loop, `:602-614` area)
+- Test: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/` (extend `S7SpanReadParityTests.cs` or sibling)
+
+**Spec.** The Task 13 planner reads one PDU per span; leftovers (`plan.LeftoverIndexes`:
+Timer/Counter tags, unparseable addresses, `PrefersPerTagRead` singles) still go one
+round-trip each. Batch the *batchable* leftovers through the new
+`IS7Plc.ReadMultipleVarsAsync`:
+
+- **Partition** leftovers into DataItem-expressible vs not. Timer/Counter: S7.Net's
+  `DataItem`/`VarType` does support Timer/Counter — verify against the library types and
+  include them only if the driver's existing per-tag path for them maps cleanly onto a
+  `DataItem`; if the mapping is not obviously identical, leave Timer/Counter on the per-tag
+  path and say so (correctness beats the round-trip). Unparseable addresses stay per-tag by
+  definition.
+- **Chunk** conservatively: a multi-var request's item headers consume PDU space
+  (~12 bytes/item request-side against the common 240-byte PDU) and S7.Net does NOT chunk
+  multi-var reads for you. Use a `const int MultiVarReadMaxItems = 12` with a comment
+  deriving the number; no new config key (YAGNI — the planner options weren't config either
+  until they were needed).
+- **Fallback, per chunk**: if `ReadMultipleVarsAsync` throws for a chunk, fall back to the
+  existing per-tag `ReadOneAsync` path for that chunk's tags — a multi-var failure must
+  never make tags Bad that a per-tag read would have served. Only per-tag failure produces
+  the Bad snapshot (reuse `RecordReadFailure` semantics). Per-item errors inside a
+  successful multi-var response (S7.Net surfaces item error codes) likewise fall back
+  per-tag for the failed items only, if the library exposes them; read the library's
+  `DataItem` result shape and handle what it actually gives you.
+- Results must sort back into batch order exactly as today (`:613`).
+- Health semantics unchanged: the same success/failure accounting the per-tag loop feeds.
+
+**Tests.** Fake `IS7Plc` driven, red-first: (1) N batchable leftovers issue
+`⌈N/12⌉` multi-var calls, zero per-tag calls; (2) parity — values decoded from multi-var
+equal the per-tag path's (mirror `S7SpanReadParityTests` structure); (3) a throwing
+multi-var chunk falls back to per-tag and serves Good; (4) per-tag-only tags (Timer/Counter
+if excluded, unparseable) untouched; (5) falsification: break the chunk size (mutate to
+1000) or the fallback and show the corresponding test goes red, then revert via Edit.
+
+**Commit:** `perf(s7): span leftovers batch through multi-var read PDUs with per-chunk fallback`
+
+---
+
+### Task 13: S7 live gate — span planner + multi-var against the s7_1500 fixture
+
+**Classification:** standard (integration test + live run)
+**Estimated implement time:** ~6 min including the live run
+**Blocked by:** Tasks 2, 12
+
+**Files:**
+- Create: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests/S7_1500/S7_1500SpanReadTests.cs`
+
+**Spec.** The register's ⚠️: the 16/220 span defaults are analytically chosen and never
+live-gated. Add fixture-backed tests in the established pattern
+(`[Collection(Snap7ServerCollection.Name)]`, `[Trait("Category","Integration")]`,
+`[Trait("Device","S7_1500")]`, `Assert.Skip(sim.SkipReason)` when unreachable — see
+`S7_1500SmokeTests.cs:16-24`, fixture `Snap7ServerFixture.cs:40-92`, endpoint env
+`S7_SIM_ENDPOINT` default `10.100.0.35:1102`):
+
+1. Seed (via whatever helper the existing smoke tests use to seed DB bytes) a tag layout
+   that genuinely exercises the planner: several tags in one DB within the 16-byte gap
+   (one span), a tag beyond the gap (second span or leftover), and ≥ 2 leftover-class tags
+   (to exercise the Task 12 multi-var path against the real wire).
+2. Read the batch with span reads enabled (defaults) and again with spans disabled
+   (`SpanReadMaxGapBytes = -1`); assert value-for-value parity and all-Good quality.
+3. **Run it live** against the fixture: probe `10.100.0.35:1102` first; if down, bring the
+   stack up over SSH (`ssh dohertj2@10.100.0.35 'cd /opt/otopcua-s7 && docker compose --profile s7_1500 up -d'`
+   — the Mac path per CLAUDE.md; `lmxopcua-fix` does not exist here). Then
+   `dotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests --filter "FullyQualifiedName~S7_1500SpanRead"`.
+   The gate PASSES only on a real run (skipped-because-unreachable is not a pass — report
+   honestly). Record the result in the test file header comment and the task report.
+
+**Commit:** `test(s7): live-gate span reads + multi-var leftovers against the s7_1500 fixture`
+
+---
+
+### Task 14: docker-dev live gate (regression of the merged remainder)
+
+**Classification:** standard (no code — live verification)
+**Estimated implement time:** ~10 min wall (mostly rig startup)
+**Blocked by:** Tasks 1-12 (gates the branch's code state)
+
+**Files:** none (evidence appended to this plan's Outcomes)
+
+**Spec.** The remainder branch merged with **no live gate** (accepted at the time; recorded
+as a release condition). Run the bounded regression gate on the local docker-dev rig
+(OrbStack — the rig runs locally on this Mac; only fixtures live on 10.100.0.35):
+
+1. `docker compose -f docker-dev/docker-compose.yml up -d --build` (builds the current
+   branch image); wait for `cluster-seed` "done." (`docker-dev/README.md:71-77`).
+2. Verify all nodes healthy; AdminUI at `http://localhost:9200` (login disabled — drive it
+   directly).
+3. Deploy: headless `POST http://localhost:9200/api/deployments` with
+   `X-Api-Key: docker-dev-deploy-key`; confirm the deployment seals green (all nodes acked).
+4. Data plane: Client.CLI `browse` + `subscribe` against `opc.tcp://localhost:4840`;
+   confirm values stream Good. Make a config edit that produces a surgical apply, deploy,
+   and confirm the open subscription survives (the 2026-08-15 gate's step 3, reduced).
+5. `/alerts` and `/hosts` pages load and stream (exercises the Fleet/Hosts poll-guard and
+   telemetry changes).
+6. Tear down: `docker compose -f docker-dev/docker-compose.yml down -v`.
+
+Out of gate scope (say so in the evidence): historian end-to-end (needs VPN/real gateway),
+FetchAndCache flip, load-scale throughput. Any failure: STOP, report, do not "fix the rig"
+around a product defect.
+
+**Commit:** none (evidence lands in Task 15's docs commit).
+
+---
+
+### Task 15: Close-out — full sweep + Outcomes
+
+**Classification:** small
+**Estimated implement time:** ~5 min plus sweep wall time
+**Blocked by:** all prior tasks
+
+**Files:**
+- Modify: this plan (append `## Outcomes`: per-task commit SHAs, deviations, the two live-gate evidence blocks)
+- Modify: `docs/plans/2026-08-17-perf-followups.md.tasks.json` (final ledger)
+- Modify: `docs/plans/2026-08-15-perf-roadmap-remainder.md` (tick the closed register items — a one-line "closed by <sha>, 2026-08-17" annotation per item; do NOT rewrite history)
+- Modify (if any behaviour docs drifted): `docs/Historian.md`, `docs/ScriptedAlarms.md`, `CLAUDE.md` (only the lines these tasks invalidate — e.g. the "no production consumer of the overrun counter" caveat)
+
+**Spec.** (1) `dotnet build ZB.MOM.WW.OtOpcUa.slnx` — 0 errors. (2) Full unit sweep across
+all test projects (the remainder plan's §Task 28 procedure; DB suites against
+`10.100.0.35,14330` if reachable, recorded either way; the two empty Wonderware test dirs
+are glob noise, not failures). (3) Outcomes + register annotations + ledger. (4) Memory
+update is the orchestrator's job, not this task's.
+
+**Commit:** `docs(plan): perf follow-ups close-out — outcomes, register annotations, sweep record`
@@ -0,0 +1,21 @@
+{
+  "planPath": "docs/plans/2026-08-17-perf-followups.md",
+  "tasks": [
+    {"id": 1,  "subject": "Task 1: Per-driver poll-overrun meter + PollGroupEngine doc notes", "status": "pending"},
+    {"id": 2,  "subject": "Task 2: S7 span-option validation guard", "status": "pending"},
+    {"id": 3,  "subject": "Task 3: Historian drain — shutdown cancellation is not a per-tag failure", "status": "pending"},
+    {"id": 4,  "subject": "Task 4: Direct-apply artifact read — explicit command timeout", "status": "pending"},
+    {"id": 5,  "subject": "Task 5: SessionAclDecisionCache eviction race + stress test", "status": "pending"},
+    {"id": 6,  "subject": "Task 6: Revalidate memo — reject empty node identifiers", "status": "pending"},
+    {"id": 7,  "subject": "Task 7: SQL-Server-backed tests for drift probe + rename scan", "status": "pending"},
+    {"id": 8,  "subject": "Task 8: ScriptedAlarmEngine TrackBackgroundTask ordering + volatile _disposed", "status": "pending"},
+    {"id": 9,  "subject": "Task 9: Test-reliability hardening (durability second-reader, #491 controls)", "status": "pending", "blockedBy": [3]},
+    {"id": 10, "subject": "Task 10: MTConnect Build streams through XmlReader", "status": "pending"},
+    {"id": 11, "subject": "Task 11: IS7Plc multi-variable read seam", "status": "pending"},
+    {"id": 12, "subject": "Task 12: S7 multi-var reads for span leftovers", "status": "pending", "blockedBy": [1, 11]},
+    {"id": 13, "subject": "Task 13: S7 live gate — span planner + multi-var vs s7_1500 fixture", "status": "pending", "blockedBy": [2, 12]},
+    {"id": 14, "subject": "Task 14: docker-dev live gate (regression of the merged remainder)", "status": "pending", "blockedBy": [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]},
+    {"id": 15, "subject": "Task 15: Close-out — full sweep + Outcomes", "status": "pending", "blockedBy": [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14]}
+  ],
+  "lastUpdated": "2026-08-17T08:00:00Z"
+}