From 2a74daf2285c4ec3449bf55baf65128c8aa90b1e Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 20 Apr 2026 16:08:01 -0400 Subject: [PATCH] =?UTF-8?q?ADR-002=20=E2=80=94=20driver-vs-virtual=20dispa?= =?UTF-8?q?tch:=20DriverNodeManager=20routes=20reads/writes/subscriptions?= =?UTF-8?q?=20across=20driver=20tags=20and=20virtual=20(scripted)=20tags?= =?UTF-8?q?=20via=20a=20single=20NodeManager=20with=20a=20NodeSource=20tag?= =?UTF-8?q?=20on=20NodeScopeResolver's=20output.=20Locks=20the=20architect?= =?UTF-8?q?ure=20decision=20Phase=207=20Stream=20G=20was=20going=20to=20ha?= =?UTF-8?q?ve=20to=20make=20anyway=20=E2=80=94=20documenting=20it=20up=20f?= =?UTF-8?q?ront=20so=20the=20stream=20implementation=20can=20reference=20t?= =?UTF-8?q?he=20chosen=20shape=20instead=20of=20rediscovering=20it.=20Opti?= =?UTF-8?q?on=20A=20(separate=20VirtualTagNodeManager=20sibling)=20rejecte?= =?UTF-8?q?d=20because=20shared=20Equipment=20folders=20owning=20both=20dr?= =?UTF-8?q?iver=20and=20virtual=20children=20would=20force=20two=20NodeMan?= =?UTF-8?q?agers=20to=20fight=20for=20ownership=20on=20every=20Equipment?= =?UTF-8?q?=20node=20=E2=80=94=20the=20common=20case,=20not=20the=20except?= =?UTF-8?q?ion=20=E2=80=94=20defeating=20the=20separation.=20Option=20C=20?= =?UTF-8?q?(virtual=20engine=20registers=20as=20a=20synthetic=20IDriver=20?= =?UTF-8?q?through=20DriverTypeRegistry)=20rejected=20because=20DriverInst?= =?UTF-8?q?ance=20shape=20is=20wrong=20for=20scripting=20config=20(no=20Dr?= =?UTF-8?q?iverType,=20no=20HostAddress,=20no=20connectivity=20probe,=20no?= =?UTF-8?q?=20NSSM=20wrapper),=20IDriver.InitializeAsync=20semantics=20don?= =?UTF-8?q?'t=20match=20script=20compilation,=20Polly=20resilience=20wrapp?= =?UTF-8?q?ers=20calibrated=20for=20network=20calls=20would=20either=20pas?= =?UTF-8?q?sthrough=20pointlessly=20or=20tune=20wrong,=20and=20Admin=20UI?= =?UTF-8?q?=20would=20need=20special-casing=20everywhere=20to=20hide=20fie?= =?UTF-8?q?lds=20that=20don't=20apply.=20Option=20B=20(single=20DriverNode?= =?UTF-8?q?Manager,=20NodeScopeResolver=20returns=20NodeSource=20enum=20al?= =?UTF-8?q?ongside=20ScopeId,=20dispatch=20branches=20on=20source)=20accep?= =?UTF-8?q?ted=20because=20it=20preserves=20one=20address-space=20tree=20w?= =?UTF-8?q?ith=20one=20walker,=20ACL=20binding=20works=20identically=20for?= =?UTF-8?q?=20both=20kinds,=20Phase=206.1=20resilience=20+=20Phase=206.2?= =?UTF-8?q?=20audit=20apply=20uniformly=20to=20the=20driver=20branch=20wit?= =?UTF-8?q?hout=20needing=20Roslyn=20analyzer=20exemptions,=20and=20adding?= =?UTF-8?q?=20future=20source=20kinds=20is=20a=20single-enum-case=20additi?= =?UTF-8?q?on.=20NodeScopeResolver.Resolve=20returns=20NodeScope(ScopeId,?= =?UTF-8?q?=20NodeSource,=20DriverInstanceId=3F,=20VirtualTagId=3F);=20Dri?= =?UTF-8?q?verNodeManager=20pattern-matches=20on=20scope.Source=20and=20ro?= =?UTF-8?q?utes=20to=20either=20the=20driver=20dictionary=20or=20IVirtualT?= =?UTF-8?q?agEngine.=20OPC=20UA=20client=20writes=20to=20a=20virtual=20nod?= =?UTF-8?q?e=20return=20BadUserAccessDenied=20before=20the=20dispatch=20br?= =?UTF-8?q?anch=20because=20Phase=207=20decision=20#6=20restricts=20virtua?= =?UTF-8?q?l-tag=20writes=20to=20scripts=20via=20ctx.SetVirtualTag.=20Disp?= =?UTF-8?q?atch=20test=20coverage=20specified=20for=20Stream=20G.4:=20mixe?= =?UTF-8?q?d=20Equipment=20folders=20browsing=20correctly,=20read=20routin?= =?UTF-8?q?g=20per=20source=20kind,=20subscription=20fan-out=20across=20bo?= =?UTF-8?q?th=20kinds,=20the=20BadUserAccessDenied=20guard=20on=20virtual?= =?UTF-8?q?=20writes,=20and=20script-driven=20writes=20firing=20subscripti?= =?UTF-8?q?on=20notifications.=20ADR-001's=20walker=20gains=20the=20Virtua?= =?UTF-8?q?lTag=20config-DB=20table=20as=20an=20additional=20input=20chann?= =?UTF-8?q?el=20alongside=20Tag;=20NodeScopeResolver's=20ScopeId=20return?= =?UTF-8?q?=20stays=20unchanged=20so=20Phase=206.2's=20ACL=20trie=20needs?= =?UTF-8?q?=20no=20modification.=20Consequences=20flagged:=20whether=20IVi?= =?UTF-8?q?rtualTagEngine=20lives=20in=20Core.Abstractions=20vs=20Phase=20?= =?UTF-8?q?7's=20Core.VirtualTags=20project,=20and=20whether=20future=20se?= =?UTF-8?q?rver-side=20methods=20on=20virtual=20nodes=20would=20route=20th?= =?UTF-8?q?rough=20this=20dispatch,=20both=20marked=20out-of-scope=20for?= =?UTF-8?q?=20ADR-002.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.7 (1M context) --- .../adr-002-driver-vs-virtual-dispatch.md | 136 ++++++++++++++++++ 1 file changed, 136 insertions(+) create mode 100644 docs/v2/implementation/adr-002-driver-vs-virtual-dispatch.md diff --git a/docs/v2/implementation/adr-002-driver-vs-virtual-dispatch.md b/docs/v2/implementation/adr-002-driver-vs-virtual-dispatch.md new file mode 100644 index 0000000..1978212 --- /dev/null +++ b/docs/v2/implementation/adr-002-driver-vs-virtual-dispatch.md @@ -0,0 +1,136 @@ +# ADR-002 — Driver-vs-virtual dispatch: how `DriverNodeManager` routes reads, writes, and subscriptions across driver tags and virtual (scripted) tags + +**Status:** Accepted 2026-04-20 — Option B (single NodeManager + NodeSource tag on the resolver output); Options A and C explicitly rejected. + +**Related phase:** [Phase 7 — Scripting Runtime + Scripted Alarms](phase-7-scripting-and-alarming.md) Stream G. + +**Related tasks:** #237 Phase 7 Stream G — Address-space integration. + +**Related ADRs:** [ADR-001 — Equipment node walker](adr-001-equipment-node-walker.md) (this ADR extends the walker + resolver it established). + +## Context + +Phase 7 introduces **virtual tags** — OPC UA variables whose values are computed by user-authored C# scripts against other tags (driver or virtual). Per design decision #2 in the Phase 7 plan, virtual tags **live in the Equipment tree alongside driver tags** (not a separate `/Virtual/...` namespace). An operator browsing `Enterprise/Site/Area/Line/Equipment/` sees a flat list of children that includes both driver-sourced variables (e.g. `SpeedSetpoint` coming from a Modbus tag) and virtual variables (e.g. `LineRate` computed from `SpeedSetpoint × 0.95`). + +From the operator's perspective there is no difference. From the server's perspective there is a big one: a read / write / subscribe on a driver node must dispatch to a driver's `IReadable` / `IWritable` / `ISubscribable` implementation; the same operation on a virtual node must dispatch to the `VirtualTagEngine`. The existing `DriverNodeManager` (shipped in Phase 1, extended by ADR-001) only knows about the driver case today. + +The question is how the dispatch should branch. Three options considered. + +## Options + +### Option A — A separate `VirtualTagNodeManager` sibling to `DriverNodeManager` + +Register a second `INodeManager` with the OPC UA stack dedicated to virtual-tag nodes. Each tag landed under an Equipment folder would be owned by whichever NodeManager materialized it; mixed folders would have children belonging to two different managers. + +**Pros:** +- Clean separation — virtual-tag code never touches driver code paths. +- Independent lifecycle: restart the virtual-tag engine without touching drivers. + +**Cons:** +- ADR-001's `EquipmentNodeWalker` was designed as a single walker producing a single tree under one NodeManager. Forking into two walkers (one per source) risks the UNS / Equipment folders existing twice (once per manager) with different child sets, and the OPC UA stack treating them as distinct nodes. +- Mixed equipment folders: when a Line has 3 driver tags + 2 virtual tags, a client browsing the Line folder expects to see 5 children. Two NodeManagers each claiming ownership of the same folder adds the browse-merge problem the stack doesn't do cleanly. +- ACL binding (Phase 6.2 trie): one scope per Equipment folder, resolved by `NodeScopeResolver`. Two NodeManagers means two resolution paths or shared resolution logic — cross-manager coupling that defeats the separation. +- Audit pathways (Phase 6.2 `IAuditLogger`) and resilience wrappers (Phase 6.1 `CapabilityInvoker`) are wired into the existing `DriverNodeManager`. Duplicating them into a second manager doubles the surface that the Roslyn analyzer from Phase 6.1 Stream A follow-up must keep honest. + +**Rejected** because the sharing of folders (Equipment nodes owning both kinds of children) is the common case, not the exception. Two NodeManagers would fight for ownership on every Equipment node. + +### Option B — Single `DriverNodeManager`, `NodeScopeResolver` returns a `NodeSource` tag, dispatch branches on source + +`NodeScopeResolver` (established in ADR-001) already joins nodes against the config DB to produce a `ScopeId` for ACL enforcement. Extend it to **also return a `NodeSource` enum** (`Driver` or `Virtual`). `DriverNodeManager` dispatch methods check the source and route: + +```csharp +internal sealed class DriverNodeManager : CustomNodeManager2 +{ + private readonly IReadOnlyDictionary _drivers; + private readonly IVirtualTagEngine _virtualTagEngine; + private readonly NodeScopeResolver _resolver; + + protected override async Task ReadValueAsync(NodeId nodeId, ...) + { + var scope = _resolver.Resolve(nodeId); + // ... ACL check via Phase 6.2 trie (unchanged) + return scope.Source switch + { + NodeSource.Driver => await _drivers[scope.DriverInstanceId].ReadAsync(...), + NodeSource.Virtual => await _virtualTagEngine.ReadAsync(scope.VirtualTagId, ...), + }; + } +} +``` + +**Pros:** +- Single address-space tree. `EquipmentNodeWalker` emits one folder per Equipment node and hangs both driver and virtual children under it. Browse / subscribe fan-out / ACL resolution all happen in one NodeManager with one mental model. +- ACL binding works identically for both kinds. A user with `ReadEquipment` on `Line1/Pump_7` can read every child, driver-sourced or virtual. +- Phase 6.1 resilience wrapping + Phase 6.2 audit logging apply uniformly. The `CapabilityInvoker` analyzer stays correct without new exemptions. +- Adding future source kinds (e.g. a "derived tag" that's neither a driver read nor a script evaluation) is a single-enum-case addition — no new NodeManager. + +**Cons:** +- `NodeScopeResolver` becomes slightly chunkier — it now carries dispatch metadata in addition to ACL scope. We own that complexity; the payoff is one tree, one lifecycle. +- A bug in the dispatch branch could leak a driver call into the virtual path or vice versa. Mitigated by an xUnit theory in Stream G.4 that mixes both kinds in one Equipment folder and asserts each routes correctly. + +**Accepted.** + +### Option C — Virtual tag engine registers as a synthetic `IDriver` + +Implement a `VirtualTagDriverAdapter` that wraps `VirtualTagEngine` and registers it alongside real drivers through the existing `DriverTypeRegistry`. Then `DriverNodeManager` dispatches everything through driver plumbing — virtual tags are just "a driver with no wire." + +**Pros:** +- Reuses every existing `IDriver` pathway without modification. +- Dispatch branch is trivial because there's no branch — everything routes through driver plumbing. + +**Cons:** +- `DriverInstance` is the wrong shape for virtual-tag config: no `DriverType`, no `HostAddress`, no connectivity probe, no lifecycle-initialization parameters, no NSSM wrapper. Forcing it to fit means adding null columns / sentinel values everywhere. +- `IDriver.InitializeAsync` / `IRediscoverable` semantics don't match a scripting engine — the engine doesn't "discover" tags against a wire, it compiles scripts against a config snapshot. +- The resilience Polly wrappers are calibrated for network-bound calls (timeout / retry / circuit breaker). Applying them to a script evaluation is either a pointless passthrough or wrong tuning. +- The Admin UI would need special-casing in every driver-config screen to hide fields that don't apply. The shape mismatch leaks everywhere. + +**Rejected** because the fit is worse than Option B's lightweight dispatch branch. The pretense of uniformity would cost more than the branch it avoids. + +## Decision + +**Option B is accepted.** + +`NodeScopeResolver.Resolve(nodeId)` returns a `NodeScope` record with: + +```csharp +public sealed record NodeScope( + string ScopeId, // ACL scope ID — unchanged from ADR-001 + NodeSource Source, // NEW: Driver or Virtual + string? DriverInstanceId, // populated when Source=Driver + string? VirtualTagId); // populated when Source=Virtual + +public enum NodeSource +{ + Driver, + Virtual, +} +``` + +`DriverNodeManager` holds a single reference to `IVirtualTagEngine` alongside its driver dictionary. Read / Write / Subscribe dispatch pattern-matches on `scope.Source` and routes accordingly. Writes to a virtual node from an OPC UA client return `BadUserAccessDenied` because per Phase 7 decision #6, virtual tags are writable **only** from scripts via `ctx.SetVirtualTag`. That check lives in `DriverNodeManager` before the dispatch branch — a dedicated ACL rule rather than a capability of the engine. + +Dispatch tests (Phase 7 Stream G.4) must cover at minimum: +- Mixed Equipment folder (driver + virtual children) browses with all children visible +- Read routes to the correct backend for each source kind +- Subscribe delivers changes from both kinds on the same subscription +- OPC UA client write to a virtual node returns `BadUserAccessDenied` without invoking the engine +- Script-driven write to a virtual node (via `ctx.SetVirtualTag`) updates the value + fires subscription notifications + +## Consequences + +- `EquipmentNodeWalker` (ADR-001) gains an extra input channel: the config DB's `VirtualTag` table alongside the existing `Tag` table. Walker emits both kinds of children under each Equipment folder with the `NodeSource` tag set per row. +- `NodeScopeResolver` gains a `NodeSource` return value. The change is additive (ADR-001's `ScopeId` field is unchanged), so Phase 6.2's ACL trie keeps working without modification. +- `DriverNodeManager` gains a dispatch branch but the shape of every `I*` call into drivers is unchanged. Phase 6.1's resilience wrapping applies identically to the driver branch; the virtual branch wraps separately (virtual tag evaluation errors map to `BadInternalError` per Phase 7 decision #11, not through the Polly pipeline). +- Adding a future source kind (e.g. an alias tag, a cross-cluster federation tag) is one enum case + one dispatch arm + the equivalent walker extension. The architecture is extensible without rewrite. + +## Not Decided (revisitable) + +- **Whether `IVirtualTagEngine` should live alongside `IDriver` in `Core.Abstractions` or stay in the Phase 7 project.** Plan currently keeps it in Phase 7's `Core.VirtualTags` project because it's not a driver capability. If Phase 7 Stream G discovers significant shared surface, promote later — not blocking. +- **Whether server-side method calls from OPC UA clients (e.g. a future "force-recompute-this-virtual-tag" admin method) should route through the same dispatch.** Out of scope — virtual tags have no method nodes today; scripted alarm method calls (`OneShotShelve` etc.) route through their own `ScriptedAlarmEngine` path per Phase 7 Stream C.6. + +## References + +- [Phase 7 — Scripting Runtime + Scripted Alarms](phase-7-scripting-and-alarming.md) Stream G +- [ADR-001 — Equipment node walker](adr-001-equipment-node-walker.md) +- [`docs/v2/plan.md`](../plan.md) decision #110 (Tag-to-Equipment binding) +- [`docs/v2/plan.md`](../plan.md) decision #120 (UNS hierarchy requirements) +- Phase 6.2 `NodeScopeResolver` ACL join