lmxopcua/archreview/01-core-composition.md

# Architecture Review — Core Composition Pipeline and Core Libraries

**Date:** 2026-08-19
**Commit:** `12b0035a` (master, clean tree)
**Note:** This is the **second update** of this report: round 1 ran 2026-07-08 at `9cad9ed0`, round 2
2026-07-12 at `f6eaa267`. The window since `f6eaa267` is the largest this domain has ever absorbed —
207 commits touching Core + OpcUaServer alone (~23k insertions / ~27k deletions across 213 files),
including the Round-2 remediation plans R2-02/R2-07/R2-10/R2-11 landing in this domain, the v3
dual-namespace address space (Batch 4), the **2026-08-15 surgical-deploys** rework
(`AddressSpaceChangeClassifier` whitelist + delta materialisation), the gzip-compressed deployment
artifact, the per-cluster mesh program's Cluster-project additions (bootstrap guard, seed-order and
transport validators), the ClusterClient→gRPC migration's Commons DTO layer, and the #520 API-key/ACL
workstream's `Core/Authorization` rework. The #522 deletion of the inert Tier-C recycle machinery and
the LiteDB `LocalCache` deletion (`2bae1b4c`) also removed two long-standing subjects of this report.

**Reviewer scope:**

- `src/Core/ZB.MOM.WW.OtOpcUa.Core` (driver hosting, resilience, stability, authorization trie)
- `src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions` (driver capability seams, `PollGroupEngine`, historian seams, `ResiliencePolicyRanges`, `TagConfigJson`)
- `src/Core/ZB.MOM.WW.OtOpcUa.Commons` (message contracts, deferred sinks, `V3NodeIds`/`RawPaths` identity authorities, `TagConfigIntent`, CSV, mesh DTOs)
- `src/Core/ZB.MOM.WW.OtOpcUa.Configuration` (EF config persistence, draft validation — the LiteDB LocalCache subsystem is deleted)
- `src/Core/ZB.MOM.WW.OtOpcUa.Cluster` (Akka bootstrap, bootstrap guard, seed-order/transport/telemetry validators, ServiceLevel)
- The composition pipeline in `src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/` — `AddressSpaceComposer` / `AddressSpacePlanner` (`AddressSpacePlan.cs`) / `AddressSpaceChangeClassifier` / `AddressSpaceApplier` — plus its Runtime consumers `OpcUaPublishActor` and `DeploymentArtifact`, and `ControlPlane/AdminOperations/ConfigComposer` (artifact + RevisionHash).

---

## Prior-finding status (f6eaa267 → 12b0035a)

Every prior finding was re-verified against current code (not STATUS.md or plan-doc claims).

| ID | One-line description | Verified status |
|---|---|---|
| S-1 | Applier swallows every sink failure; deploy reported applied even when broken | **SUBSTANTIALLY CLOSED.** `AddressSpaceApplyOutcome` now carries `RebuildFailed` + `FailedNodes` (`AddressSpaceApplier.cs:1297–1303`); every `Safe*` helper returns bool and callers tally (`:96–99, 176, 312, 338`); the five `Materialise*` passes return failed-node counts (`:779, 828, 939, 998, 1145`); `OpcUaPublishActor` sums both channels, logs at Error, and fires the `otopcua.opcua.apply.failed` meter (`OpcUaPublishActor.cs:503–509, 567–575`); a degraded **non-rebuild** apply does not advance `_lastApplied`, so the next deploy re-diffs and repairs (`:534–544`) — replacing the accidental full-pass self-healing the delta path removed. `AddressSpaceApplierFailureSurfaceTests` exists. **Residue re-filed as S-9:** the deploy ACK is still sent before, and independent of, the apply outcome. |
| S-2 | `PollGroupEngine.Unsubscribe` blocks up to 5 s per subscription | **CLOSED in production.** `UnsubscribeAsync` added (`PollGroupEngine.cs:191–196`, shared `TryTake` `:203–212`, async drain `:230–239`); all six poll drivers call it (Modbus:1354, FOCAS:851, TwinCAT:596, AbLegacy:502, S7:2102, Sql:583). The sync `Unsubscribe` is retained for compatibility and still blocks (`:225`, `DrainTimeout` `:242`) but has no production caller. |
| S-3 | No disposed guard: `Subscribe`/`RegisterAsync` race `DisposeAsync` | **STILL OPEN** — no `_disposed` in `PollGroupEngine.cs` (`Subscribe` `:156–167` vs `DisposeAsync` `:438–457`) or `Core/Hosting/DriverHost.cs` (`:58–63` vs `:92–106`). |
| S-4 | `ScheduledRecycleScheduler` catch-up recycle storm | **CLOSED BY DELETION** — `cbb88229` (#522) deleted the class; `Core/Stability/` now holds only `WedgeDetector.cs`. |
| S-5 | `GenericDriverNodeManager` re-walk teardown unsynchronized | **OPEN-BUT-MOOT** — class still present, still zero production references; folded into U-1. |
| S-6 | Valid-JSON out-of-range ResilienceConfig permanently bricks every guarded capability call | **CLOSED** (R2-02). `ClampPolicy` clamps timeout/retry/breaker with diagnostics (`DriverResilienceOptionsParser.cs:110–191`); `ResiliencePolicyRanges` in Core.Abstractions (`ResiliencePolicyRanges.cs:11`); belt-and-braces `Math.Clamp` in the builder (`DriverResiliencePipelineBuilder.cs:136`). All numerics handed to Polly are covered. |
| S-7 | Respawn `Invalidate` race re-caches a stale-options pipeline | **CLOSED** (R2-02). `PipelineKey` carries `Generation` (`DriverResiliencePipelineBuilder.cs:208`, keyed `:86–91`); factory `Interlocked.Increment` per `Create` (`DriverCapabilityInvokerFactory.cs:75, 91`). |
| S-8 | Dispatch hardcodes `isIdempotent: true`; non-idempotent safeguard unreachable | **CLOSED** (R2-02). The single production write dispatch passes `isIdempotent: false` with rationale (`DriverInstanceActor.cs:800–813`); parser forces Write/AlarmAck retry→0 (`DriverResilienceOptionsParser.cs:154–158`) and Initialize retry→0 (`:165–169`, from `dac7e188`). |
| P-1 | `Compose` parses each tag's TagConfig JSON four times | **CLOSED** (R2-11). `TagConfigIntent.Parse` once per raw tag (`AddressSpaceComposer.cs:769`); minor residue: each UNS reference re-parses its backing tag's blob (`:842`), so a tag referenced by N equipment parses N+1 times — trivial next to the old 4×-per-seam. |
| P-2 | Per-operation allocations on the authorization hot path | **OPEN, MITIGATED.** `PermissionTrie.CollectMatches` still allocates per walk (`PermissionTrie.cs:41, 44`), but stage-6 hot seams (read/browse/subscribe) hit `SessionAclDecisionCache` first (`OpcUaServer/Security/SessionAclDecisionCache.cs`), so the walk runs on cache miss only; write/alarm/HistoryRead still walk per request. Narrowed to Low. |
| P-3 | `PollGroupEngine`: task-per-subscription, no coalescing | **STILL OPEN** — `Task.Run` per `Subscribe` (`PollGroupEngine.cs:165`). The engine gained fixed-rate pacing + overrun metering this window (reviewed below — see S-11). |
| P-4 | Per-write allocation in the non-idempotent write path | **CLOSED** — the no-retry snapshot is cached once per invoker (`CapabilityInvoker.cs:32, 138, 161`). |
| C-1 | Byte-parity TagConfig parsing replicated in four projects | **CLOSED** (R2-11). `TagConfigIntent`/`DeviceConfigIntent` in `Commons/Types/`; `grep "MUST parse identically" src/` = 0; composer (`:769, :842`), artifact decode, and `DraftValidator` (`:488`) all consume the single authority; `TagConfigCorpusParityTests` guards compose≡decode. |
| C-2 | Core depends on Configuration; EF entities are the domain model | **STILL OPEN (accepted-risk)** — `Core.csproj:16`. |
| C-3 | `DraftValidator` re-derives NodeId scheme + hard-codes `"GalaxyMxGateway"` | **CLOSED BY REWORK.** The v3 validator consumes the shared authorities (`V3NodeIds` at `DraftValidator.cs:67, 190–191, 269–273`; `RawPaths.ValidateSegment` `:460`); `GalaxyMxGateway` no longer appears in Configuration. |
| C-4 | Composer violates purity contract with `Trace.TraceWarning` | **STILL OPEN** — `AddressSpaceComposer.cs:627` (now with an acknowledging comment `:618`; `AddressSpaceComposerPurityTests` pins determinism, not no-logging). |
| C-5 | Inconsistent duplicate-key defensiveness in `Compose` | **STILL OPEN, softened** — defensive `GroupBy…First()` at `:816, :874–877, :959–968, :995` coexists with throw-on-dupe `ToDictionary` at `:587, :689` (both keyed by PK, so unreachable in practice). |
| C-6 | Vestigial `Akka` package reference in Commons | **STILL OPEN** — `Commons.csproj:9`; zero `using Akka` in Commons source (the new mesh DTO mapper did not need it either). |
| C-7 | Stale scaffold-era XML docs on load-bearing classes | **PARTIALLY FIXED, new staleness added.** The F14-scaffold narrative is gone from the applier, but the rewritten doc contradicts itself: the PureRemove bullet describes the shipped in-place teardown **and** says "Maps to a full rebuild until R2-07 Phase 2 ships" (`AddressSpaceApplier.cs:26–28`), AddRemoveMix likewise (`:28–29`) — both phases shipped a year of commits ago. `DriverHost.cs:7–9` still promises "Phase 2 via named-pipe RPC". New #522 residue: `DriverTier.cs:18` and `DriverFactoryBootstrap.cs:179` describe the deleted recycle machinery in the present tense. |
| C-8 | Deferred-sink forwarding guard is namespace-filtered | **STILL OPEN** — `DeferredSinkForwardingReflectionTests.cs:44` and `:106` still filter `i.Namespace == typeof(IOpcUaAddressSpaceSink).Namespace`. The guard *did* do its job this window (the surgical remove members and batched `WriteValues` are all forwarded), but the blind spot stands. |
| U-1 | Dead code retained: `GenericDriverNodeManager`, `EquipmentNodeWalker`, `TryParseRelayBody` | **STILL OPEN** — all three present (`Core/OpcUa/`; `EquipmentScriptPaths.cs:228`), references are doc-comments and tests only. #522 proved the codebase can delete dormant machinery; this trio survived the purge. |
| U-2 | Tier C recycle machinery has no `IDriverSupervisor` implementation | **CLOSED BY DELETION** — #522 (`cbb88229`) removed `MemoryRecycle`, `ScheduledRecycleScheduler`, `IDriverSupervisor`, `MemoryTracking`, and the `RecycleIntervalSeconds` knob end-to-end; only tombstone comments remain (see C-7 residue). This resolves the "accreting surface for an execution path that does not exist" trajectory the last two reviews flagged. |
| U-3 | Continuous-historization restart convergence unverified | **CLOSED.** Recorder ref-set convergence unit-tested (`ContinuousHistorizationRecorderTests.cs:27–100`); applier feed tested (`AddressSpaceApplierProvisioningTests`); live end-to-end gate ran 2026-08-01/04 and #491 closed (with the `ConfigureAwait(false)`-destroys-`Self` defect found and fixed, plus #537/#538 — outside this domain's files but the gate discharged this finding's residue). |
| U-4 | Additive-only ACL model (no Deny) | **STILL OPEN (by design)** — `PermissionTrie.cs:13–15`. Stakes rose materially with #520 (ACLs now gate the whole data plane), so the limitation is now security-relevant, not latent. |
| U-5 | `ClusterRoleInfo` untested; Cluster coverage thin | **CLOSED** — `ClusterRoleInfoTests.cs` exists; the Cluster.Tests suite grew to 16 files covering the bootstrap guard/coordinator, seed validators, and transport/telemetry validators. |
| U-6 | Bulkhead advertised, authored, parsed — never applied | **CLOSED** (R2-02) — knob deleted end-to-end; `DriverResilienceOptionsTests.cs:15, 40` is the "options properties are exactly the pipeline-wired set" inertness guard. Cosmetic residue: `WedgeDetector.cs:65–76` still names a `BulkheadDepth` parameter. |

Net: **16 of 26 prior findings verifiably closed** (five by the R2-02 resilience hardening, two by
R2-11's parse consolidation, two by the #522 deletion, and — the headline — S-1's failure surface
finally built and consumed). The remediation posture inverted from the last cycle: this window
targeted this domain's backlog directly, and the closures are real code with real tests, not claims.
What remains open is dominated by accepted-risk items (C-2, U-4) and hygiene (C-4/5/6/7/8, U-1, S-3).

---

## Architecture Overview

### Composition pipeline (deploy → address space) — now four stages

1. **`AddressSpaceComposer.Compose`** (`OpcUaServer/AddressSpaceComposer.cs`) — pure static projection
   from EF entities into an `AddressSpaceComposition`. v3 reshaped it around the dual namespace: it
   emits the **Raw subtree** (`RawContainerNode` Folder→Driver→Device→TagGroup + `RawTagPlan` keyed by
   RawPath, `:708–794`) and the **UNS projection** (areas/lines/equipment + `UnsReferenceVariable`
   mirror records, `:803–860`), all ordinally sorted. Tag intent parses once per tag through the
   Commons `TagConfigIntent` authority (`:769`). The critical 2026-08-15 design decision lives in the
   record shape: `UnsReferenceVariable` carries `Backing{IsHistorized,HistorianTagname,IsArray,ArrayLength}`
   copied from the backing tag's intent (`:838–855`), so a raw-side historian/shape edit makes the
   reference record unequal and the planner emits the **coincident** delta that repairs the mirror —
   staleness prevention encoded in data, not in classifier logic.

2. **`AddressSpacePlanner.Compute`** (`AddressSpacePlan.cs:160–252`) — still a pure diff. New diff sets:
   raw containers/tags keyed by RawPath (a rename = remove+add, the immutable-NodeId rule) and UNS
   references keyed by the **stable row id, not NodeId** (`:216–219`) — the keying choice that makes a
   backing-tag rename a surgical re-point instead of a teardown.

3. **`AddressSpaceChangeClassifier.Classify`** (`AddressSpaceChangeClassifier.cs:48–125`) — the pure
   policy layer added by R2-07 and rewritten by surgical-deploys. First-match-wins; **default-closed
   Rebuild** for any node-affecting change, with three `with`-equality whitelists
   (`VtagDeltaIsNodeIrrelevant` `:133`, `TagDeltaIsSurgicalEligible` `:156`,
   `RawTagDeltaIsSurgicalEligible` `:174`) whose record-equality construction automatically closes
   over any future field. The WP4 alarm-notifier guard (alarm-bearing raw delta ⇒ Rebuild) is
   preserved and its narrowing (referencing-set-only deltas are node-inert for value tags) is
   documented in place with the blast-radius history (`:61–92`).

4. **`AddressSpaceApplier.Apply`** (`AddressSpaceApplier.cs:85–339`) — routes by kind: in-place
   surgical updates (folder renames, equipment/raw tag attributes, identity-preserving UNS-reference
   repairs `:275–298`), scoped removes with terminal Bad/`RemovedConditionState` writes
   (`ApplyPureRemove` `:360–446`), and the one-way **rebuild ratchet** on any surgical `false`/throw
   (`:312`). A degraded outcome is now first-class (`AddressSpaceApplyOutcome.RebuildFailed`/
   `FailedNodes`). The five `Materialise*` passes gained **delta-subset overloads** (surgical-deploys):
   a non-rebuild apply materialises the plan's delta, not the composition — adding one tag is O(change)
   instead of ~50k sink calls on the pinned publish thread — while `MaterialiseHierarchy` deliberately
   stays full-composition (ancestors must exist; `:767–774`). `OpcUaPublishActor` holds the other half
   of the contract: a degraded non-rebuild apply does **not** advance `_lastApplied`
   (`OpcUaPublishActor.cs:511–544`), so the next deploy re-diffs and repairs — the designed replacement
   for the accidental self-healing the full passes used to provide.

### Artifact chain (`ConfigComposer` → `DeploymentArtifact`)

`ConfigComposer.Pack` (`ControlPlane/AdminOperations/ConfigComposer.cs:99–109`) gzips the
deterministic snapshot and hashes the **compressed** bytes in one step, so blob and `RevisionHash`
can never be derived from different byte sequences — the invariant the FetchAndCache verify path and
LocalDb cache rest on. It is directly pinned by
`ConfigComposerTests.Blob_is_gzip_and_the_revision_hash_is_over_the_compressed_bytes`, whose comment
correctly observes that hashing the uncompressed JSON would fail every fleet fetch while every
hash-shape test stayed green. `DeploymentArtifact.Open` (`Runtime/Drivers/DeploymentArtifact.cs:117`)
sniffs the gzip magic (`:164`), inflates under a 512 MiB guard (`MaxDecompressedBytes` `:133`,
enforced `:177–181`), and surfaces every decompression failure as `JsonException` so it lands in the
existing malformed-JSON catches. `ParsedDeploymentArtifact` parses the blob **once** per apply and
serves all four consumers (driver specs, cluster scope, data-plane authority, composition) — a real
hot-path improvement over the old parse-per-question statics. The lenient static wrappers survive for
the boot/restore paths, which is where the one new stability finding lives (S-10).

### Cluster project — bootstrap guard and fail-fast validators

The Cluster project roughly tripled: `ClusterBootstrapGuard`/`ClusterBootstrapCoordinator` (the
simultaneous-cold-start founder election), `AkkaClusterOptionsValidator` (conditional self-first seed
ordering), `SplitTopologyTransportValidator`, `MeshTransportOptionsValidator` (the deleted
`ClusterClient` mode now **fails loudly**, never falls back — `MeshTransportOptionsValidator.cs:45–55`),
and `TelemetryOptions`/`ConfigSource` validators. The engineering quality is high — the guard makes
its one-shot join decision *before* `JoinSeedNodes` from an explicit TCP reachability signal
(deliberately avoiding the deleted `SelfFormAfter`'s mid-handshake re-form), `BuildDowningHocon`
throws on an unknown strategy, and every new class has a test file including real 2-node in-process
clusters. The findings here (C-9, S-12) are consistency gaps between the validators, not design
flaws.

---

## Findings

### 1. Stability

#### S-9 — The deploy ACK is decoupled from the address-space apply outcome: a degraded apply still seals the deployment green — **Medium** *(new; the residue of S-1)*

`DriverHostActor.HandleDeploy` upserts `NodeDeploymentState = Applied` and sends
`ApplyAckOutcome.Applied` **before** it Tells `RebuildAddressSpace` to the publish actor
(`Runtime/Drivers/DriverHostActor.cs:1931–1944`); the address-space work then runs asynchronously on
the publish actor's pinned thread. Everything S-1 asked for now exists inside that async leg — the
failure tally, the Error log, the `otopcua.opcua.apply.failed` meter, the no-advance re-diff repair
(`OpcUaPublishActor.cs:503–575`) — but none of it can reach the ack that was already sent. A
deployment whose address-space apply degrades (or whose rebuild throws outright,
`OpcUaPublishActor.cs:582–585`) seals **Applied** fleet-wide; the coordinator, `/fleet`, and the
audit trail all record success, and only the node-local log/meter disagree.

This is a smaller residue than the original S-1 (the failure is now *visible* and *self-repairing*,
where before it was silent and permanent), and making the ack await the apply would change the deploy
protocol's timing semantics — but the honest fix need not block: a follow-up degraded-ack message (or
a `NodeDeploymentState` status amendment posted by the publish actor after the apply completes) would
let central distinguish "artifact accepted" from "address space converged".

#### S-10 — The malformed-artifact leg of the #485 defect class is still open on the lenient boot/restore parse paths — and gzip added a new malformed shape — **Medium** *(new)*

#485 established the rule "no bytes is no answer, not an empty configuration", and the zero-length
guards are faithfully in place (`DriverHostActor.cs:2200–2204, 2487–2493`;
`OpcUaPublishActor.cs:452–463`). But the **non-empty-malformed** case takes a different route on the
boot-from-cache and bootstrap-restore paths: `ReconcileDriversFromBlob` calls the lenient
`DeploymentArtifact.ParseDriverInstances(blob, node)` (`DriverHostActor.cs:2206`), which swallows
`JsonException` to an **empty spec list** (`DeploymentArtifact.cs:263–266`), and
`ApplyDriverSpecs` then plans every running child for `StopChild` (`:2225–2235`) — the exact
"silently takes this node's entire field I/O down" consequence the #485 comment eleven lines above
describes. `PushDesiredSubscriptionsFromArtifact` has the same shape: the lenient `ParseComposition`
(`DriverHostActor.cs:2498`; swallow at `DeploymentArtifact.cs:307–310`) yields an empty composition →
an empty desired set → "drops the live subscription handle rather than re-subscribing" (its own
guard comment, `:2483–2486`). The surrounding `try/catch` there is dead code — the wrapper never
throws.

The apply paths are safe (they use `Open`, which throws, and the publish actor deliberately documents
why — `OpcUaPublishActor.cs:465–474`). The exposure is boot/restore with corrupt-but-nonempty bytes.
That window used to be negligible; the gzip change widens it: a truncated or bit-flipped gzip stream
is a new malformed shape that is *guaranteed* to surface as the swallowed `JsonException`
(`DeploymentArtifact.cs:196`). Mitigations that keep this Medium rather than High: the LocalDb cache
verifies SHA-256 on read, and SQL varbinary truncation is rare. The doc comment
(`DriverHostActor.cs:2190–2195`) shows the leniency is *known* ("the lenient 'malformed ⇒ no specs'
parse those paths have always had") — but #485's own rationale applies verbatim to it: nothing
legitimate produces unparseable bytes, so the honest reading is "no answer", i.e. warn and keep the
running children/subscriptions, exactly as the zero-length branch already does.

#### S-11 — `PollGroupEngine` batch path advances the change baseline before raising, so a throwing sink permanently loses that cycle's changes — **Low** *(new)*

The new per-cycle batch callback (`adebad71`) updates `LastValues` per tag **before** the batch raise
(`PollGroupEngine.cs:406` vs `:416`); an `onBatch` (or mid-loop `onChange`) throw is absorbed and
counted as a generic poll failure, but the diff baseline has already advanced, so the lost changes are
never re-raised. The pacing math itself is correct (fixed-rate with a `Task.Yield` tight-loop guard,
`:286–313`) and the overrun meter's slow-success/slow-failure conflation is documented (`:340–349`).

#### S-12 — Two role authorities can wire a node inconsistently: Akka gossips `driver` while the Host wires no driver stack — **Medium** *(new)*

`Program.cs:61–63` derives `hasAdmin`/`hasDriver` — the switches for the *entire* conditional host
wiring — solely from `OTOPCUA_ROLES`, while the Akka cluster roles resolve configured-first via
`RoleParser.ResolveEffectiveRoles` (`RoleParser.cs:115–125`; PostConfigure in
`Cluster/ServiceCollectionExtensions.cs`). A node configured only through `Cluster:Roles` (a shape
#532's own remediation made first-class on the Akka side) joins the mesh advertising `driver`,
becomes an expected-ack member of every deployment, and times them out forever because no driver
stack was registered. #532 fixed the env-only→no-Akka-roles direction; this is the surviving inverse.
The fix is one line — resolve `Program.cs`'s roles through the same `ResolveEffectiveRoles` — plus a
guard test asserting the two views agree.

#### S-3 — `PollGroupEngine.Subscribe` / `DriverHost.RegisterAsync` still race `DisposeAsync` — **Low** *(unchanged; third consecutive review)*

Unchanged from both prior reviews (`PollGroupEngine.cs:156–167` vs `:438–457`;
`DriverHost.cs:58–63` vs `:92–106`). Still a shutdown-window race, still cheap to close with a
`_disposed` flag checked before insert. Since S-2's fix added new teardown surface
(`UnsubscribeAsync`), closing this while the file is warm would be natural.

**Positive observations.** This window's stability engineering in this domain is the strongest of the
three review rounds. The surgical-deploys work is a model of default-closed design: the classifier's
`with`-equality whitelists close over future fields; every surgical failure ratchets one-way to a
full rebuild; the degraded-apply no-advance rule (`OpcUaPublishActor.cs:511–544`) *explicitly
documents the accidental self-healing it replaces and the re-remove/ratchet cost of the replacement* —
honesty about failure-path economics that most codebases never write down. The `Open`-vs-lenient
split on the apply path (`:465–474`), the #485 guards, the fire-and-forget provisioning hook's
faulted-task observation (`AddressSpaceApplier.cs:624–649`), and the R2-02 resilience clamps
(`ClampPolicy` with per-field diagnostics) are all correct. The Cluster bootstrap guard makes its
join decision from an explicit reachability signal *before* the handshake — the precise lesson of the
deleted `SelfFormAfter`, applied.

### 2. Performance

#### P-2 — Authorization-walk allocations: mitigated by the decision cache, unresolved at the trie — **Low** *(downgraded from Medium)*

`PermissionTrie.CollectMatches` still allocates a `HashSet` + `List` per walk
(`PermissionTrie.cs:41–44`), but the stage-6 hot seams now consult the per-(session, node, op, realm)
`SessionAclDecisionCache` first, and `UserAuthorizationState` is built only on a miss. Write, alarm
commands, and HistoryRead still walk per request — acceptable rates. Residual work: hoist the
principal set per session generation.

#### P-3 — `PollGroupEngine`: one task per subscription, no cross-subscription coalescing — **Medium** *(unchanged)*

Still `Task.Run` per `Subscribe` (`PollGroupEngine.cs:165`). The new pacing/overrun/batch work
improved per-loop behavior without touching the scaling shape: hundreds of subscriptions per driver
instance still mean hundreds of independent loops and duplicate protocol reads on overlapping tag
sets.

#### P-5 — UNS-reference composition re-parses the backing tag's TagConfig per reference — **Low** *(new; the residue of P-1)*

`BuildUnsReferenceVariables` calls `TagConfigIntent.Parse(tag.TagConfig)` per reference row
(`AddressSpaceComposer.cs:842`) after `BuildRawSubtree` already parsed the same blob (`:769`). N
references to one tag = N+1 parses. A per-compose `Dictionary<TagId, TagConfigIntent>` closes it.
Compose-time only; cosmetic at current fleet sizes.

**Positive observations.** This window delivered real throughput work in-scope: non-rebuild applies
materialise the **delta** (O(change) vs O(address space) — the 50k-sink-calls-for-one-tag case is
gone, `OpcUaPublishActor.cs:594–603`), the artifact is parsed once per apply
(`ParsedDeploymentArtifact`), batched `WriteValues` lands under one node-manager lock with a typed
partial-failure contract (`IOpcUaAddressSpaceSink.cs:193–205`), struct realm keys replaced formatted
NodeId strings (`4f177bc3`), and the artifact itself is now gzip-compressed (~10–20× smaller through
SQL, the cache, and the fetch stream) with an explicit "hash covers the compressed bytes" invariant
test. The ACL hot path gained a lookup-before-scope-build ordering and per-key cache shards.

### 3. Conventions

#### C-9 — Role normalization is inconsistent across the four Cluster fail-fast validators: three of them are case-sensitive over verbatim `Cluster:Roles` — **Medium** *(new)*

`SplitTopologyTransportValidator` normalizes roles (trim + lower) and its comment names the exact
hazard: "the `Cluster:Roles` bind path is verbatim (no lowercasing)… so 'Driver' / 'Admin' cannot
slip past the ordinal checks" (`SplitTopologyTransportValidator.cs:69–78`). Its three siblings do the
opposite: `MeshTransportOptionsValidator.cs:76–79`, `TelemetryOptions.cs:124–126`, and
`ConfigSourceOptionsValidator.cs:101–104` all run case-sensitive `Array.IndexOf(roles,
RoleParser.Driver)` over `ResolveEffectiveRoles`, which returns configured roles **verbatim**
(`RoleParser.cs:115–118` — only the env fallback is lowercased/validated by `Parse`). A node
configured `Cluster:Roles: ["Driver"]` silently skips every driver-scoped boot-time rule
(GrpcPsk-required, CentralGrpcEndpoints-required, FetchAndCache-required) and fails later at runtime
with precisely the absence-class failures these validators exist to fail fast on. Nothing validates
unknown/miscased plain role names on the configured path at all. Fix: normalize (and
`Allowed`-validate) inside `ResolveEffectiveRoles` itself so every consumer inherits it — one
authority, not four re-derivations; this is the same rule the authoring-vs-deploy-gate lesson already
established elsewhere in this codebase.

#### C-7 — Stale/contradictory class docs — **Low** *(carried; contents updated)*

Current inventory: (a) `AddressSpaceApplier.cs:26–29` — the class doc simultaneously describes the
shipped PureRemove/AddRemoveMix behavior and says each "maps to a full rebuild until Phase 2/3
ships"; (b) `DriverHost.cs:7–9` — the named-pipe "Phase 2" promise, now three reviews old, describing
an architecture superseded by the out-of-repo gateways; (c) #522 tombstones written in the present
tense: `DriverTier.cs:18` ("Process-kill protections (MemoryRecycle, ScheduledRecycleScheduler)
are…") and `DriverFactoryBootstrap.cs:179`; (d) `WedgeDetector.cs:65–76`'s `BulkheadDepth` parameter
name outliving the bulkhead deletion. Individually trivial; collectively they erode the one thing
this codebase's documentation is otherwise excellent at — being trustworthy.

#### C-8 — The deferred-sink forwarding guard's interface detection is still namespace-filtered — **Low** *(unchanged)*

`DeferredSinkForwardingReflectionTests.cs:44, 106`. Unchanged verdict and unchanged recommendation
(exclude a known-interface allowlist instead of filtering by namespace). Worth noting the guard
earned its keep this window: every new sink member (surgical removes, `WireAlarmNotifiers`, batched
`WriteValues`) is forwarded, with the batch overload explicitly overridden so the DIM per-item
fallback cannot silently discard the one-lock-per-batch win (`DeferredAddressSpaceSink.cs:30–33`).

#### C-10 — `CsvParser` does not strip a UTF-8 BOM — **Low** *(new)*

A U+FEFF-prefixed document makes the first header column `"Name"`, and `ParseWithHeader`
(`Commons/Csv/CsvParser.cs:259–292`) silently fails to key that column. CSV exported from Excel — the
overwhelmingly likely source for the raw-tree import this parser serves — is BOM-prefixed by default
when saved as "CSV UTF-8". One `TrimStart('')` on the first field of the first record.

#### C-11 — Applier's terminal-write failure tally is inconsistent across the remove pass — **Low** *(new)*

The UNS-reference remove-previous counts a failed terminal Bad write into `FailedNodes`
(`AddressSpaceApplier.cs:300`), and the condition-state writes count (`:102, :107, :374, :430`), but
the plain-variable terminal writes discard the bool (`:380, :389, :419, :435`). Either posture is
defensible (a failed terminal write before a successful remove is arguably benign); holding both at
once means the counter's meaning depends on node kind. Pick one.

#### C-12 — `ConfigComposer` snapshots whole EF entities: a future sensitive column ships fleet-wide automatically — **Low** *(new)*

`SnapshotAndFlattenAsync` serializes entire entity rows for 17 tables (`ConfigComposer.cs:39–64`).
The `ApiKeys` inclusion was handled with care (peppered HMAC only, pepper deliberately out of the
artifact — `:54–59`), which proves the risk is understood — but the protection is per-column
vigilance at authoring time, not a mechanism. Any future secret-bearing column on any snapshotted
entity reaches every driver node's LocalDb cache and the gRPC fetch stream by default. A
convention-level guard (a test asserting no property name matching `*Secret*`/`*Password*`/`*Key*`
outside an allowlist appears in the serialized shape) would make the posture mechanical.

**Positive observations.** The convention-enforcement trajectory continues upward: `TagConfigIntent`
killed the four-copy byte-parity contract *and* the corpus parity test makes the remaining two seams
(entity-side compose, JSON-side decode) mechanically checked; `V3NodeIds` + `RawPaths` are genuine
single identity authorities (17 consumer files, validation at every authoring seam), and the
`DraftValidator` rework consumed them instead of re-deriving (closing C-3); the mesh DTO mapper
throws on unset oneofs/UNSPECIFIED enums with the anti-silent-drop rationale documented
(`MeshTransportDtoMapper.cs:210, 242, 342, 369`); and `ISkippedTagReporter` encodes "null = no
answer, never a zero report" in its return type with a stage/commit ledger
(`ISkippedTagReporter.cs:47–55, 128–174`) — the #485 lesson generalized into a contract shape.

### 4. Underdeveloped Areas

#### U-1 — Dead code retained in Core: `GenericDriverNodeManager`, `EquipmentNodeWalker`, `TryParseRelayBody` — **Medium** *(unchanged; third consecutive review)*

All three survive with zero production references (`Core/OpcUa/GenericDriverNodeManager.cs`,
`Core/OpcUa/EquipmentNodeWalker.cs` + `IdentificationFolderBuilder`,
`EquipmentScriptPaths.cs:228`). The counter-argument died this window: #522 demonstrated the team
will delete dormant machinery wholesale when it decides to, and the v3 rework retired
`EquipmentNodeIds` cleanly — this trio is now the *only* dormant surface left in the domain. Delete
it (git preserves it), or move the walker into the test project.

#### U-7 — `RawPathResolver` returns a truncated path as success when the parent chain cycles — **Low** *(new)*

`BuildFolderChain`/`AppendGroupChain` (`Commons/Types/RawPathResolver.cs:100–113, 117–133`) exit the
depth-256 loop with `current != null` and return the truncated 256-segment chain as a *valid*
RawPath, where the class doc promises a broken chain "degrades to 'this tag has no RawPath'
(dropped)". Requires DB/artifact corruption to reach (the authoring paths prevent cycles), but a
wrong NodeId identity is strictly worse than a dropped tag — this is the optimistic-success shape in
the codebase's single identity authority. Return null when the loop exhausts with `current != null`.

#### U-8 — Surgically-added RAW nodes are materialised but never announced — the announce path is Uns-only — **Medium** *(new; known and documented, still open)*

`AnnounceAddedNodes` hardcodes `AddressSpaceRealm.Uns` (`AddressSpaceApplier.cs:572–579`) and
`ComputeAddAnnouncements` collects only UNS parents (`:515–536`); the publish actor's own comment
states the consequence precisely: before surgical deploys a raw add forced a full rebuild (clients
had to re-browse anyway), so the missing announcement was neutral — now a surgically-added raw
container/tag is served but **genuinely invisible** to model-aware clients until they happen to
re-browse ("a real follow-up, not an accepted limitation", `OpcUaPublishActor.cs:555–563`). The delta
work changed this from cosmetic to a functional gap in the feature's own headline scenario (add one
tag without disturbing anyone). Needs a realm-aware `RaiseNodesAddedModelChange`.

#### U-9 — Bootstrap-guard operability edges: founder election assumes identical host spelling; the peer-first hang warning is one-shot and log-only — **Low** *(new)*

(a) `ClusterBootstrapGuard.Analyze` orders `host:port` strings lexicographically from each node's own
seed list (`ClusterBootstrapGuard.cs:112–115`); if one pair node's config spells hosts as DNS names
and the other's as IPs, both can elect themselves founder — recreating the split the guard exists to
close, undetectably. A boot-time log of the computed ordering (or a validator asserting the pair's
lists are element-wise equal-modulo-self) would surface it. (b) The documented residual hang (founder
dies inside the probe→join window) warns exactly once (`ClusterBootstrapCoordinator.cs:142–158`) and
never reaches a health surface or ServiceLevel — a node can sit unjoined for hours emitting nothing
after the first line.

#### U-4 — Additive-only ACL model (no Deny) — **Low, rising** *(unchanged code, changed stakes)*

`PermissionTrie.cs:13–15` unchanged — but since #520 the trie now gates the *entire* data plane
(read/browse/subscribe/write/alarms/HistoryRead), not just two role gates. "A broad grant cannot be
carved back at a sub-scope" is now an operational security constraint operators will meet, not a
latent design note. Keep it prominent in `docs/security.md`; revisit when the first
carve-out request arrives.

---

## Maturity Ratings

| Dimension | 2026-07-08 | 2026-07-12 | 2026-08-19 | Justification |
|---|---|---|---|---|
| Stability | 4 | 4 | **4.5** | The domain's headline open item (S-1) finally got its failure surface — built, consumed, metered, tested, and paired with a designed repair path (degraded-apply no-advance + rebuild ratchet) whose trade-offs are documented in place. S-2/S-4/S-6/S-7/S-8 all closed with real code. The surgical-deploys pipeline is default-closed at every layer. What keeps it off 5: the ack is still decoupled from the apply outcome (S-9), the malformed-artifact leg of #485 survives on the boot paths with gzip widening it (S-10), and S-3 is now three reviews old. |
| Performance | 3 | 3 | **4** | Two of four prior findings closed (P-1, P-4) and the window shipped genuine throughput work in-scope: O(change) delta applies, once-per-apply artifact parse, batched WriteValues, struct realm keys, gzip artifacts, ACL decision caching. P-3's task-per-subscription shape and the trie-walk allocations (now miss-only) are the remaining ceilings. |
| Conventions | 4 | 4 | **4** | The systemic byte-parity duplication — the domain's worst conventions debt for three rounds — is gone (C-1/C-3 closed via `TagConfigIntent` + shared identity authorities, with a corpus parity test). But the window also *produced* a fresh instance of the same disease (C-9: one normalization rule stated in one validator, violated by its three siblings), the applier's rewritten doc contradicts itself, and C-4/C-5/C-6/C-8 survived a third review unchanged. High and holding, not rising. |
| Underdeveloped areas | 3 | 3 | **4** | The authored-but-inert backlog was cleared the right way: U-2 and U-6 deleted outright (#522/R2-02) rather than propped up, U-3 and U-5 closed with tests, and the forwarding guard caught real new members this window. Remaining: the U-1 trio (now the only dormant surface left, and inexcusable post-#522), the Uns-only announce gap that surgical deploys promoted from cosmetic to functional (U-8), and small optimistic-success edges in new code (U-7, U-9). |