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).