diff --git a/docs/plans/2026-10-09-pluggable-historian-design.md b/docs/plans/2026-10-09-pluggable-historian-design.md index cf13922f..c3f33661 100644 --- a/docs/plans/2026-10-09-pluggable-historian-design.md +++ b/docs/plans/2026-10-09-pluggable-historian-design.md @@ -49,7 +49,7 @@ It lives in **Runtime** because the seams it returns span `Core.Abstractions` an `CreateUpdater` returns `null`; `SupportedKinds = {Double, Int64, Boolean}`. - `SqlServerHistorianProvider` (new project `Driver.Historian.SqlServer`); `SupportedKinds` = all kinds. - The Host owns a name→provider table (`HistorianBackendRegistry`). `Program.cs`'s four hard-wired - `GatewayHistorian.Create…` call sites and the inline `new GatewayHistorianValueWriter(...)` become calls + `GatewayHistorian.Create…` call sites (three, plus the inline `new GatewayHistorianValueWriter(...)`) become calls on the selected provider. The existing gates (`AddServerHistorian`, `AddHistorianProvisioning`, `AddAlarmHistorian`, the outbox registration, the recorder spawn) are unchanged — they get their objects from the provider. @@ -77,7 +77,7 @@ It lives in **Runtime** because the seams it returns span `Core.Abstractions` an ``` - Unknown `Backend` ⇒ host start fails (`OptionsValidationException` listing the valid names). -- `ServerHistorianOptionsValidator` requires `Endpoint` only when `Backend == Gateway`. +- `ServerHistorianOptionsValidator` runs its three gateway checks (Endpoint, ApiKey, UseTls⇔scheme) only when `Backend == Gateway`; backend-specific options are validated there too, consumer-gated (`ServerHistorian:Enabled || AlarmHistorian:Enabled`), never at registry resolution. - New `SqlServerHistorianOptions` validation: `ConnectionString` required, `Schema` a valid SQL identifier (`^[A-Za-z_][A-Za-z0-9_]{0,127}$` — it is spliced into DDL/DML), `CommandTimeout > 0`. - Validators remain driver-block-only (admin-only nodes never build historian singletons). @@ -330,3 +330,19 @@ delete-raw. Prints one line per item with its status code name. Docs in `docs/Cl `docs/Client.CLI.md`, `CLAUDE.md` Historian section, and the scadaproj `sister-project-history/otopcua.md` entry (ScadaBridge impact: none unless it issues HistoryUpdate; HistoryRead semantics unchanged on the gateway backend apart from honest Bad quality on Bad samples). + +## 9. Pre-execution review amendments (Fable, 2026-10-09) + +These were verified against the code and override anything above that conflicts. + +1. **Outbox recovery.** `FasterLogHistorizationOutbox`'s constructor (`RecoverState`) deserializes every record too, so the "unreadable ⇒ drop + count, never throw" rule applies there as well. An unreadable record is indexed under a synthetic id so FIFO truncation can pass it. If the v2 magic matches but structural validation fails, the legacy exact-length test runs before the entry is declared unreadable. `IHistorizationOutbox` gains `UnreadableCount` (default 0). +2. **Validation** is as in §2 (all three gateway checks gated on `Backend`; consumer-gated backend validation). `HistorianUpdaterHolder.None` is always registered on driver nodes; the real updater only when `ServerHistorian:Enabled`. +3. **Timestamps** are bound as `datetime2(7)` everywhere: `SqlDbType.DateTime2` scalar parameters and `SqlDataRecord` TVPs, never `DataTable`, which would round to 3.33 ms `datetime`. +4. **Raw** `maxValuesPerNode == 0` means unbounded. **Events** fetch `maxEvents + 1` and return a non-null continuation point when truncated, as the interface requires. +5. **Concurrency.** Tag creation and sample `MERGE`s use `WITH (HOLDLOCK)`, and `2627/2601` is treated as "already present". Both pair nodes provision on every deploy. +6. **`Event`** has a nonclustered PK on `EventId` and is clustered on `(EventTimeUtc, EventId)`. The EventId hashes length-prefixed fields and includes `EquipmentPath`. +7. **Processed reads** are capped at 100 000 buckets per request. +8. **HistoryUpdate admission.** The `HistoryWrite` AccessLevel bit decides whether the SDK base admits a node to HistoryUpdate at all, so the updater must be wired before any node materialises. `OtOpcUaServerHostedService.StartAsync` assigns it (via the new `OtOpcUaSdkServer.SetHistorianDataUpdater`) before `SetSink`. Nodes materialised earlier keep their AccessLevel (no retro-update). +9. **HistoryUpdate results.** Successful arms set `errors[i] = Good`. A `SourceTimestamp == DateTime.MinValue` is `BadInvalidTimestamp`. +10. **Comms-loss is not historized.** Watchdog `BadNoCommunication` stamps are node-only and never reach the mux, so they are not historized. A driver's own Bad publish is, usually with a null value, so it is stored as `Empty` kind with the driver's Bad code (Modbus `0x80050000`). +11. **Parallel safety.** `SqlServerTagCatalog` belongs to the skeleton task (T8), so the parallel SQL tasks share one. diff --git a/docs/plans/2026-10-09-pluggable-historian-plan.md b/docs/plans/2026-10-09-pluggable-historian-plan.md index 9a4e395e..67b5a881 100644 --- a/docs/plans/2026-10-09-pluggable-historian-plan.md +++ b/docs/plans/2026-10-09-pluggable-historian-plan.md @@ -42,9 +42,9 @@ | Role | Model | Why | |---|---|---| | Plan review (before execution) | **fable** | user-requested | -| Implementer, mechanical/standard tasks (T1, T2, T4, T5, T7, T8, T9, T11, T12, T13, T14) | **sonnet** | multi-file but fully specified | -| Implementer, judgment-heavy tasks (T3 recorder actor, T6 node-manager HistoryUpdate + auth, T10 reads/aggregates) | **opus** | actor concurrency, auth semantics, SQL/Part 13 semantics | -| Per-task reviewer | **sonnet** for sonnet tasks; **opus** for T3/T6/T10 | scale to risk | +| Implementer, mechanical/standard tasks (T1, T4, T5, T7, T8, T9, T11, T12, T13, T14) | **sonnet** | multi-file but fully specified | +| Implementer, judgment-heavy tasks (T2 outbox crash boundary, T3 recorder actor, T6 node-manager HistoryUpdate + auth, T10 reads/aggregates) | **opus** | actor concurrency, auth semantics, SQL/Part 13 semantics | +| Per-task reviewer | **sonnet** for sonnet tasks; **opus** for T2/T3/T6/T10 | scale to risk | | Fix rounds 4–5 escalation | one tier up (sonnet→opus, opus→fable) | per SDD skill | | Final whole-branch review | **fable** | cross-task integration risk | | Live gate (T15) | controller (this session) | needs rig + judgement; never delegated | @@ -55,7 +55,7 @@ Always pass `model` explicitly on every Agent dispatch. ``` Wave 1 (serial): T1 contracts + compile ripple [sonnet] -Wave 2 (parallel): T2 outbox v2 ‖ T3 recorder ‖ T4 gateway provider ‖ T6 server HistoryUpdate ‖ T7 CLI ‖ T8 SQL skeleton+schema +Wave 2 (parallel): T2 outbox v2 [opus] ‖ T3 recorder ‖ T4 gateway provider ‖ T6 server HistoryUpdate ‖ T7 CLI ‖ T8 SQL skeleton+schema Wave 3 (parallel): T5 registry+Host ‖ T9 SQL writes ‖ T10 SQL reads ‖ T11 SQL updater ‖ T13 over-the-wire test Wave 4 (serial): T12 SQL provider + Host registration Wave 5 (serial): T14 docs → final fable review → T15 live gate (controller) @@ -72,6 +72,11 @@ Dependencies: T2,T3,T4,T6,T7,T8 ← T1. T5 ← T4. T9,T10,T11 ← T8. T13 ← T6 |---|---| | `ZB.MOM.WW.OtOpcUa.slnx` | T8 (adds both SqlServer projects) | | `src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs` | T5 (wave 3), then T12 (wave 4) | + | `.../Host/Configuration/ServerHistorianOptionsValidator.cs` | T5 (wave 3), then T12 (wave 4) | + | `.../Runtime/Historian/ContinuousHistorizationMetrics.cs` | T5 | + | `.../OpcUaServer/OtOpcUaSdkServer.cs`, `.../Host/OpcUa/OtOpcUaServerHostedService.cs` | T6 | + | `tests/Server/.../Runtime.Tests/Historian/ContinuousHistorizationRecorderTests.cs` | T1 (ripple) then T3 | + | `Core.Abstractions/Historian/IHistorizationOutbox.cs` | T1 | | `tests/Core/.../Core.Abstractions.Tests/*.csproj` | T8 | | `src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs` | T6 only | | `.../Runtime/Historian/ContinuousHistorizationRecorder.cs` | T1 (ripple) then T3 only | @@ -102,12 +107,17 @@ The rig `sql` container must be up: `docker ps --filter name=otopcua-dev-sql-1`. **Goal:** Introduce `HistorianValue`, `HistorianValueKind`, `HistoryUpdateMode`, `IHistorianDataUpdater`, `IHistorianBackendProvider`, change `HistorizationValue`/`HistorizationOutboxEntry` to typed+status, and keep the whole solution compiling with **no behaviour change**. +> **Review amendments (Fable, 2026-10-09) — binding, override anything below that conflicts:** +> - Add `long UnreadableCount => 0;` as a default interface member on `IHistorizationOutbox` (Core.Abstractions) — T2 overrides it, T5 binds the gauge. (#6) +> - The record call sites to update are explicit: `grep -rn "HistorizationOutboxEntry(\|HistorizationValue(" src tests --include='*.cs'` (≈17 sites in 7 files incl. `GatewayLiveIntegrationTests.cs`, `HistorizationOutboxEntrySerializerTests.cs`, `GatewayHistorianValueWriterTests.cs`, `ContinuousHistorizationRecorderTests.cs`). Every one of them is in T1's Files. Do not stop at an unlisted one. (#3) + **Files:** - Create: `src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/Historian/HistorianValue.cs` - Create: `src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/Historian/IHistorianDataUpdater.cs` - Create: `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Historian/IHistorianBackendProvider.cs` - Modify: `src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/Historian/IHistorianValueWriter.cs` (`HistorizationValue`) - Modify: `src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/Historian/HistorizationOutboxEntry.cs` +- Modify: `src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/Historian/IHistorizationOutbox.cs` (default member `UnreadableCount`) - Modify (ripple only): `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Historian/ContinuousHistorizationRecorder.cs` (lines ~548 entry construction, ~692 drain mapping), `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/Recorder/GatewayHistorianValueWriter.cs`, `.../Recorder/HistorizationOutboxEntrySerializer.cs`, and every test that constructs these records (find with `grep -rn "HistorizationOutboxEntry(\|HistorizationValue(" src tests --include='*.cs'`). - Test: `tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/Historian/HistorianValueTests.cs` @@ -234,6 +244,13 @@ public sealed record HistorizationOutboxEntry(Guid Id, string Tag, HistorianValu **Goal:** Serialize every `HistorianValueKind` + `uint StatusCode`; read pre-upgrade entries; drop (and count) unreadable entries instead of throwing. +> **Review amendments (Fable, 2026-10-09) — binding, override anything below that conflicts:** +> - **Model: opus** (FIFO invariant on the crash boundary). (#1) +> - `FasterLogHistorizationOutbox.RecoverState()` (ctor) ALSO deserializes every record. Use `TryDeserialize` in **both** `RecoverState` and `PeekBatchAsync`. In `RecoverState`, index an unreadable record in `_live/_index` under a synthetic `Guid.NewGuid()` with its address so FIFO truncation can pass it; in Peek skip it (and make sure an unreadable entry at the head is removable so it can't pin the head — e.g. Peek auto-removes leading unreadable entries). `UnreadableCount` (overrides T1's default member) increments in both. (#1) +> - Required test: write a raw garbage record straight into the FasterLog directory (open `FasterLog` directly), **restart** the outbox, prove the ctor succeeds, the following good entry drains, and the head advances past the garbage. (#1) +> - Legacy/v2 ambiguity: when the 4-byte magic matches but v2 structural validation fails, fall back to the legacy exact-length test before declaring unreadable. (#18) +> - Drop the meter bullet: T5 binds the gauge via `IHistorizationOutbox.UnreadableCount`. (#6) + **Files:** - Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/Recorder/HistorizationOutboxEntrySerializer.cs` - Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/Recorder/FasterLogHistorizationOutbox.cs` (only the deserialize call site in Peek: skip+count unreadable entries; expose `long UnreadableCount`) @@ -251,7 +268,7 @@ public sealed record HistorizationOutboxEntry(Guid Id, string Tag, HistorianValu **Steps:** (1) capture the legacy hex from the current serializer via a throwaway test print, paste into the new test; (2) write failing v2 tests; (3) implement `Serialize` (v2 always) + `TryDeserialize(ReadOnlySpan, out HistorizationOutboxEntry?)` (v2 by magic, else legacy by exact length, else false) and keep a throwing `Deserialize` wrapper only if other callers need it; (4) wire skip+count in the outbox peek; (5) green; commit `feat(historian): outbox format v2 carries typed values + status; legacy entries still read`. ```json:metadata -{"files":["src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/Recorder/HistorizationOutboxEntrySerializer.cs","src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/Recorder/FasterLogHistorizationOutbox.cs"],"acceptanceCriteria":["v2 layout","pinned legacy bytes read","corrupt entry skipped and counted, never pins head"],"modelTier":"standard","model":"sonnet","wave":2} +{"files":["src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/Recorder/HistorizationOutboxEntrySerializer.cs","src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/Recorder/FasterLogHistorizationOutbox.cs"],"acceptanceCriteria":["v2 layout","pinned legacy bytes read","corrupt entry skipped and counted, never pins head"],"modelTier":"frontier","model":"opus","wave":2} ``` --- @@ -260,6 +277,11 @@ public sealed record HistorizationOutboxEntry(Guid Id, string Tag, HistorianValu **Goal:** The recorder records `HistorianValue` + `msg.StatusCode` and drops kinds the backend does not support. +> **Review amendments (Fable, 2026-10-09) — binding, override anything below that conflicts:** +> - Add `tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Historian/ContinuousHistorizationRecorderTests.cs` to Files — lines ~337 and ~1149 assert `DroppedNonNumeric` and must follow the rename. (#3) +> - The recorder remarks ("carrying only (TagId, Value, TimestampUtc) and no quality") are stale: `DependencyMuxActor.cs:114,150` passes `item.StatusCode`. Rewrite them. (verified) +> - Comms-loss watchdog `BadNoCommunication` stamps are node-only (`FanCommsLossToValueNodes` never reaches the mux) and are NOT historized — say so in the remarks; only a driver's own Bad publish (usually `null` value ⇒ `Empty`) is. (#14) + **Files:** - Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Historian/ContinuousHistorizationRecorder.cs` - Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ServiceCollectionExtensions.cs` **only** the recorder `Props` construction (~line 485–520) to pass `supportedKinds` — resolve `IHistorianBackendProvider` optionally; absent ⇒ `{Double, Int64, Boolean}` (today's behaviour). @@ -311,6 +333,14 @@ public sealed record HistorizationOutboxEntry(Guid Id, string Tag, HistorianValu **Goal:** `ServerHistorian:Backend` selects a provider; Program.cs builds every historian seam through it. +> **Review amendments (Fable, 2026-10-09) — binding, override anything below that conflicts:** +> - Gate **all three** gateway checks in `ServerHistorianOptionsValidator` (Endpoint, ApiKey, UseTls⇔scheme) on `Backend == Gateway`, and the same in `ServerHistorianOptions.Validate()` warnings. Tests: `Backend=SqlServer, Enabled=true, no Endpoint/ApiKey ⇒ passes`; control `Backend=Gateway, no ApiKey ⇒ fails` (unchanged). (#2) +> - Unknown `Backend` ⇒ always fail (no I/O). Backend-specific option validation (SqlServer, added by T12) runs inside `ServerHistorianOptionsValidator` and only when consumed (`ServerHistorian:Enabled || AlarmHistorian:Enabled`) — the existing consumer-gated posture. `HistorianBackendRegistry.Resolve` must not validate. (#11) +> - Register `HistorianUpdaterHolder.None` **unconditionally** in the driver block; register the real `new HistorianUpdaterHolder(provider.CreateUpdater(sp))` only when `ServerHistorian:Enabled` (HistoryUpdate gated like HistoryRead; last registration wins). (#11) +> - Add `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Historian/ContinuousHistorizationMetrics.cs` to Files: add observable gauge `otopcua.historian.outbox.unreadable` from `IHistorizationOutbox.UnreadableCount` in `BindOutbox`. (#6) +> - Program.cs is top-level statements and not unit-testable: the "DI graph unchanged" AC is tested as `HistorianBackendRegistry.Resolve(gatewayOpts, cfg).CreateDataSource(sp) is GatewayHistorianDataSource` (and likewise for the writer/alarm writer/provisioner). Do not try to host-start Program. (#22) +> - Spec wording fix: there are three `GatewayHistorian.Create…` call sites (Program.cs ~241/262/273) plus the inline writer (~313). (#16) + **Files:** - Create: `src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/HistorianBackendRegistry.cs` - Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Historian/ServerHistorianOptions.cs` (add `Backend` string, default `"Gateway"`) @@ -328,7 +358,7 @@ public sealed record HistorizationOutboxEntry(Guid Id, string Tag, HistorianValu **Verify:** `dotnet build ZB.MOM.WW.OtOpcUa.slnx && dotnet test tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests --filter "FullyQualifiedName~Historian"` → green. ```json:metadata -{"files":["src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/HistorianBackendRegistry.cs","src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Historian/ServerHistorianOptions.cs","src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/ServerHistorianOptionsValidator.cs","src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs"],"acceptanceCriteria":["unknown backend fails boot","Gateway default resolves identical types","updater holder registered"],"modelTier":"standard","model":"sonnet","wave":3} +{"files":["src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/HistorianBackendRegistry.cs","src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Historian/ContinuousHistorizationMetrics.cs","src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Historian/ServerHistorianOptions.cs","src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/ServerHistorianOptionsValidator.cs","src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs"],"acceptanceCriteria":["unknown backend fails boot","Gateway default resolves identical types","updater holder registered"],"modelTier":"standard","model":"sonnet","wave":3} ``` --- @@ -337,6 +367,14 @@ public sealed record HistorizationOutboxEntry(Guid Id, string Tag, HistorianValu **Goal:** Clients can HistoryUpdate historized nodes when the backend supplies an updater, gated by ACL (flag on) or the new `HistoryUpdate` role (flag off). +> **Review amendments (Fable, 2026-10-09) — binding, override anything below that conflicts:** +> - SDK pinned `OPCFoundation.NetStandard.Opc.Ua.* 1.5.378.106`. Verified: public `HistoryUpdate(OperationContext, Type detailsType, …)` runs under `Lock`, pre-seeds `errors[ii] = BadHistoryOperationUnsupported`, and admits a variable **only if `(AccessLevel & AccessLevels.HistoryWrite) != 0`**; it releases the lock and dispatches to protected `HistoryUpdateData` / `HistoryDeleteRawModified` / `HistoryDeleteAtTime(ServerSystemContext, IList<…Details>, IList, IList, List, IDictionary)`. The HistoryWrite bit is therefore **dispatch-load-bearing**. (#4) +> - Add `src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaSdkServer.cs` to Files: `SetHistorianDataUpdater(IHistorianDataUpdater?)` mirroring `SetHistorianDataSource` (line ~122). In `OtOpcUaServerHostedService.StartAsync` assign it **immediately after the `NodeManager is null` guard (~line 164), before `_deferredSink.SetSink` (~171)**; clear it in `StopAsync` next to the data-source clear (~373). Hosted service is registered before `AddAkka`, so this precedes every deploy apply. (#4, #5) +> - On success each arm sets `errors[handle.Index] = ServiceResult.Good` (the base pre-seeds Unsupported; every HistoryRead success path here does the same, e.g. OtOpcUaNodeManager.cs ~5133). Per-node `HistoryUpdateResult.StatusCode` stays Good when per-item results are mixed; only a whole-node failure (deny / unsupported / not historized) sets the node code. (#13, #21) +> - `UpdateDataDetails.UpdateValues[i].SourceTimestamp` is non-nullable: `== DateTime.MinValue` ⇒ `BadInvalidTimestamp` for that item. (#21) +> - At least one test MUST drive the public `nm.HistoryUpdate(OperationContext, typeof(UpdateDataDetails), …)` entry point and assert the updater is called — it is the test that fails when the AccessLevel bit is missing. Tests that call the protected arm directly do not count toward the ALLOW criterion. (#4) +> - AclEdit: just add `NodePermissions.HistoryUpdate` to the `PermissionBits` array (~161–166); the page renders enum names, there is no label table. (#23) + **Files:** - Modify: `src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs` - Modify: `src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Security/OpcUaDataPlaneRoles.cs` (`HistoryUpdate = "HistoryUpdate"`) @@ -362,7 +400,7 @@ public sealed record HistorizationOutboxEntry(Guid Id, string Tag, HistorianValu **Verify:** `dotnet test tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests --filter "FullyQualifiedName~HistoryUpdate" && dotnet test tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests --filter "FullyQualifiedName~ApiKeyInterimRoleMapper"` → green. ```json:metadata -{"files":["src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs","src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Security/OpcUaDataPlaneRoles.cs","src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/ApiKeyInterimRoleMapper.cs","src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/OtOpcUaServerHostedService.cs","src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Historian/HistorianUpdaterHolder.cs","src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Clusters/AclEdit.razor","src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Enums/NodePermissions.cs"],"acceptanceCriteria":["SDK signatures verified","ALLOW test","WriteOperate does not confer HistoryUpdate","HistoryWrite access bit"],"modelTier":"frontier","model":"opus","wave":2} +{"files":["src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs","src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaSdkServer.cs","src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Security/OpcUaDataPlaneRoles.cs","src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/ApiKeyInterimRoleMapper.cs","src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/OtOpcUaServerHostedService.cs","src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Historian/HistorianUpdaterHolder.cs","src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Clusters/AclEdit.razor","src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Enums/NodePermissions.cs"],"acceptanceCriteria":["SDK signatures verified","ALLOW test","WriteOperate does not confer HistoryUpdate","HistoryWrite access bit"],"modelTier":"frontier","model":"opus","wave":2} ``` Note: T5 references `HistorianUpdaterHolder` — T5 runs in wave 3 after T6 merges, so the type exists. @@ -395,6 +433,14 @@ Note: T5 references `HistorianUpdaterHolder` — T5 runs in wave 3 after T6 merg **Goal:** New projects, embedded DDL, idempotent locked bootstrap, options + validation, shared connection/mapping helpers, and the env-gated integration fixture. +> **Review amendments (Fable, 2026-10-09) — binding, override anything below that conflicts:** +> - **Move `SqlServerTagCatalog.cs` here from T9** (get-or-create + read-only lookup), used by T9/T10/T11/T12. Create path: `MERGE {s}.Tag WITH (HOLDLOCK) AS t …`; on `2627/2601` re-select the TagId. Integration test: two concurrent get-or-create on the same new name ⇒ both return the same id, one row. (#10, #12) +> - Timestamps: helper `AddDateTime2(SqlCommand, name, DateTime)` (`SqlDbType.DateTime2`, scale 7) for every scalar param; TVPs are `IEnumerable` with `SqlMetaData(…, SqlDbType.DateTime2, 0, 7)` — **never a `DataTable`** (infers 3.33 ms `datetime`). Mapper tests use non-millisecond-aligned ticks (`…T00:00:00.1234567Z`). (#7) +> - DDL: `CREATE SCHEMA` must be alone in its batch ⇒ `IF SCHEMA_ID(N'$(Schema)') IS NULL EXEC(N'CREATE SCHEMA [$(Schema)]')`; substitute `$(Schema)` **before** splitting on `GO`. (#17) +> - `Event`: `EventId nvarchar(64) NOT NULL CONSTRAINT PK_Event PRIMARY KEY NONCLUSTERED`, plus `CREATE CLUSTERED INDEX CX_Event_Time ON {s}.Event (EventTimeUtc, EventId)` (drop the separate `IX_Event_Time`). Keep `IX_Event_Source_Time`. (#19) +> - Private helper types introduced by wave-3 tasks inside this project must be prefixed with their task (`T9…`, `T10…`, `T11…`) to avoid merge collisions. (#12) +> - Verified: `StatusCodes.*` names in `SqlHistorianStatusCodes` all exist in the pinned SDK, and `AggregateBits` Calculated=0x1 / Interpolated=0x2 with InfoType 0x400 are correct; keep the SDK-oracle unit test. (#26) + **Files:** - Create: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.SqlServer/ZB.MOM.WW.OtOpcUa.Driver.Historian.SqlServer.csproj` (refs: Core.Abstractions, Core.AlarmHistorian, Runtime; ``, `Microsoft.Extensions.Logging.Abstractions`; ``; `InternalsVisibleTo` both test projects) - Create: `.../SqlServerHistorianOptions.cs` (+ `Validate()` returning errors per spec §2), `.../SqlHistorianStatusCodes.cs`, `.../SqlHistorianConnection.cs` (open connection, schema-qualify helper `Q(string obj)` ⇒ `[{schema}].[{obj}]`, transient-error classifier: 1205, -2, 40613, 40197, 40501, 49918, 4060, 233, 10053, 10054, 10060, 11001, SqlException with `IsTransient`), `.../SampleRowMapper.cs` (HistorianValue ⇄ columns per spec §4 + `DataTable` builders for `SampleRow` and `TimeRow` TVPs), `.../SqlServerSchemaBootstrapper.cs`, `.../Schema/001_initial.sql` @@ -414,7 +460,7 @@ Note: T5 references `HistorianUpdaterHolder` — T5 runs in wave 3 after T6 merg **Verify:** `dotnet build ZB.MOM.WW.OtOpcUa.slnx && dotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.SqlServer.Tests && dotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.SqlServer.IntegrationTests && dotnet test tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests --filter "FullyQualifiedName~StatusCodeParity"` → green (integration with the env var set; also run once WITHOUT it and confirm all skip). ```json:metadata -{"files":["src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.SqlServer/","tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.SqlServer.Tests/","tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.SqlServer.IntegrationTests/","ZB.MOM.WW.OtOpcUa.slnx","docs/historian/sqlserver/001_initial.sql"],"acceptanceCriteria":["idempotent DDL","applock bootstrap","parity-guarded status consts","env-gated throwaway DB fixture"],"modelTier":"standard","model":"sonnet","wave":2} +{"files":["src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.SqlServer/SqlServerTagCatalog.cs","src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.SqlServer/","tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.SqlServer.Tests/","tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.SqlServer.IntegrationTests/","ZB.MOM.WW.OtOpcUa.slnx","docs/historian/sqlserver/001_initial.sql"],"acceptanceCriteria":["idempotent DDL","applock bootstrap","parity-guarded status consts","env-gated throwaway DB fixture"],"modelTier":"standard","model":"sonnet","wave":2} ``` --- @@ -423,8 +469,14 @@ Note: T5 references `HistorianUpdaterHolder` — T5 runs in wave 3 after T6 merg **Goal:** Live values, tag provisioning and alarm events persist idempotently. +> **Review amendments (Fable, 2026-10-09) — binding, override anything below that conflicts:** +> - `SqlServerTagCatalog` now comes from T8 — do NOT create it; use it. (#12) +> - Insert-if-absent: catch `2627/2601` and treat as success (the row is durably present), so a concurrent writer never books a #538 failure. (#25) +> - Provisioning `MERGE … WITH (HOLDLOCK)`; integration test with two concurrent provisioners of the same new tags ⇒ both report Ensured, no Failed. (#10) +> - EventId: SHA-256 over **length-prefixed** UTF-8 fields `EquipmentPath, AlarmId, EventKind, TimestampUtc.Ticks, User, Comment, Message` (null ⇒ length −1). Unit test that `"a|b"+"c"` and `"a"+"b|c"` hash differently. (#20) +> - All timestamp round-trips use non-ms-aligned ticks and assert tick equality. (#7) + **Files (create only):** -- `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.SqlServer/SqlServerTagCatalog.cs` (TagId cache + `GetOrCreateAsync(name, kindHint)` via `MERGE … OUTPUT inserted.TagId`) - `.../SqlServerHistorianValueWriter.cs`, `.../SqlServerTagProvisioner.cs`, `.../SqlServerAlarmHistorianWriter.cs` - Tests: `tests/.../SqlServer.IntegrationTests/WriteTests.cs`, `ProvisioningTests.cs`, `AlarmWriterTests.cs`; unit `tests/.../SqlServer.Tests/AlarmEventIdTests.cs` @@ -438,7 +490,7 @@ Note: T5 references `HistorianUpdaterHolder` — T5 runs in wave 3 after T6 merg **Verify:** `dotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.SqlServer.IntegrationTests --filter "FullyQualifiedName~Write|FullyQualifiedName~Provisioning|FullyQualifiedName~AlarmWriter"` → green. ```json:metadata -{"files":["src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.SqlServer/SqlServerTagCatalog.cs","src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.SqlServer/SqlServerHistorianValueWriter.cs","src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.SqlServer/SqlServerTagProvisioner.cs","src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.SqlServer/SqlServerAlarmHistorianWriter.cs"],"acceptanceCriteria":["idempotent redelivery","never throws","deterministic EventId"],"modelTier":"standard","model":"sonnet","wave":3} +{"files":["src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.SqlServer/SqlServerHistorianValueWriter.cs","src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.SqlServer/SqlServerTagProvisioner.cs","src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.SqlServer/SqlServerAlarmHistorianWriter.cs"],"acceptanceCriteria":["idempotent redelivery","never throws","deterministic EventId"],"modelTier":"standard","model":"sonnet","wave":3} ``` --- @@ -447,6 +499,12 @@ Note: T5 references `HistorianUpdaterHolder` — T5 runs in wave 3 after T6 merg **Goal:** `SqlServerHistorianDataSource : IHistorianDataSource` per spec §4 "Reads". +> **Review amendments (Fable, 2026-10-09) — binding, override anything below that conflicts:** +> - `maxValuesPerNode == 0` is the node manager's unbounded sentinel ⇒ `TOP(2147483647)`; test it. (#8) +> - Events: `TOP(@maxEvents + 1)` (maxEvents 0 ⇒ unbounded); when the extra row is present drop it and return a non-null opaque ContinuationPoint (e.g. UTF-8 of `EventTimeUtc.Ticks|EventId`) — the interface says MUST. Test `maxEvents=2` over 3 rows. (#9) +> - Processed: cap the bucket count at 100 000; above it throw `ArgumentOutOfRangeException` and document that the node manager surfaces it as a per-node Bad. Check what the node manager's processed path does with a thrown non-aggregate exception and match it. (#24) +> - Tag lookup uses T8's `SqlServerTagCatalog` read-only lookup. All timestamp tests non-ms-aligned. (#7, #12) + **Files (create only):** `.../SqlServerHistorianDataSource.cs`, `.../SqlAggregateQueries.cs` (one method per aggregate family returning the SQL text), tests `tests/.../SqlServer.IntegrationTests/RawReadTests.cs`, `AtTimeReadTests.cs`, `EventReadTests.cs`, `ProcessedReadTests.cs`. **Acceptance Criteria:** @@ -471,13 +529,18 @@ Note: T5 references `HistorianUpdaterHolder` — T5 runs in wave 3 after T6 merg **Goal:** `SqlServerHistorianDataUpdater : IHistorianDataUpdater` per spec §4 "Updates". +> **Review amendments (Fable, 2026-10-09) — binding, override anything below that conflicts:** +> - Use T8's `SqlServerTagCatalog` for create-on-Insert/Update (remove the "private MERGE" fallback). Sample `MERGE … WITH (HOLDLOCK)`. (#10, #12) +> - "Null timestamp" means `DateTime.MinValue` (the node manager maps it to `BadInvalidTimestamp` before calling, but the updater must still treat `MinValue` the same defensively). (#21) +> - Non-ms-aligned ticks in every Replace/DeleteAtTime test. (#7) + **Files (create only):** `.../SqlServerHistorianDataUpdater.cs`; tests `tests/.../SqlServer.IntegrationTests/UpdateTests.cs`. **Acceptance Criteria:** - [ ] One transaction per call; `MERGE Sample AS t USING @rows AS r ON t.TagId=@tagId AND t.SourceTimeUtc=r.SourceTimeUtc` with `OUTPUT $action, r.SourceTimeUtc`; Insert uses `WHEN NOT MATCHED THEN INSERT` only (matched ⇒ BadEntryExists), Replace `WHEN MATCHED THEN UPDATE` only (not matched ⇒ BadNoEntryExists), Update both. Results mapped back to the caller's original order (including within-request duplicate timestamps: second occurrence ⇒ `BadEntryExists` for Insert, last-wins for Replace/Update — document). - [ ] Null timestamp ⇒ `BadInvalidTimestamp` for that item, others proceed. - [ ] Delete raw `[start,end)` ⇒ `Good` with rows deleted, `BadNoData` when 0; DeleteAtTime per time. -- [ ] Unknown tag: Insert/Update create it (via a private catalog lookup — do NOT edit T9's `SqlServerTagCatalog`; if T9 is not merged yet, implement a private `MERGE` in this file); Replace/Delete ⇒ BadNoEntryExists/BadNoData. +- [ ] Unknown tag: Insert/Update create it via T8's `SqlServerTagCatalog`; Replace/Delete ⇒ BadNoEntryExists/BadNoData. - [ ] Integration test per row of the spec's result table + the duplicate/null cases; replaced value is visible via direct SQL with the new status. **Verify:** `dotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.SqlServer.IntegrationTests --filter "FullyQualifiedName~Update"` → green. @@ -492,6 +555,10 @@ Note: T5 references `HistorianUpdaterHolder` — T5 runs in wave 3 after T6 merg **Goal:** `Backend=SqlServer` works end to end in the Host. +> **Review amendments (Fable, 2026-10-09) — binding, override anything below that conflicts:** +> - SqlServer option validation lives in `ServerHistorianOptionsValidator`, consumer-gated (see T5 amendment) — add it there (this task owns that file in wave 4), not in `Resolve`. (#11) +> - Add `src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/ServerHistorianOptionsValidator.cs` to Files. One shared `SqlServerSchemaBootstrapper` + `SqlServerTagCatalog` per provider. (#11, #12) + **Files:** - Create: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.SqlServer/SqlServerHistorianProvider.cs` (one shared bootstrapper + tag catalog instance across the five seams; `SupportedKinds` = all six kinds) - Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/HistorianBackendRegistry.cs` (add `"SqlServer"`, binding `ServerHistorian:SqlServer` to `SqlServerHistorianOptions` and failing on its `Validate()` errors), `src/Server/ZB.MOM.WW.OtOpcUa.Host/ZB.MOM.WW.OtOpcUa.Host.csproj` (ProjectReference), `Program.cs` only if T5 left a seam that needs it @@ -514,6 +581,9 @@ Note: T5 references `HistorianUpdaterHolder` — T5 runs in wave 3 after T6 merg **Goal:** Prove HistoryUpdate works through a real SDK client session (the encoding/dispatch the node-manager unit tests mock around). +> **Review amendments (Fable, 2026-10-09) — binding, override anything below that conflicts:** +> - Install the fake updater (`Nm.HistorianDataUpdater = fake`, same slot the rig uses for `Nm.Authorizer`, cf. `AclKeyExpiryEnforcementTests.cs:141-215`) **before** materialising the historized node. Add a control test that materialises first, then installs the updater, and proves HistoryUpdate returns `BadHistoryOperationUnsupported` (documents the no-retro-update rule). Assert the per-node `StatusCode` as well as `OperationResults`. (#4, #13, #15) + **Files (create only):** `tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/HistoryUpdateOverTheWireTests.cs` (use the existing integration harness that `ConditionMethodEventTests` uses; inject an in-memory fake `IHistorianDataUpdater` + `IHistorianDataSource` sharing one dictionary). **Acceptance Criteria:** @@ -561,7 +631,7 @@ Note: T5 references `HistorianUpdaterHolder` — T5 runs in wave 3 after T6 merg 3. `Client.CLI historyread` raw + processed (Average, 10 s) on the raw NodeId AND its UNS NodeId — same rows. 4. `Client.CLI historyupdate` insert (new timestamp) ⇒ `GoodEntryInserted`; repeat ⇒ `BadEntryExists`; replace ⇒ `GoodEntryReplaced`; delete-at-time ⇒ `Good`; each confirmed by `historyread` and by direct SQL. Use an API key whose principal has a Cluster-scoped `HistoryUpdate` grant; a key with only WriteOperate ⇒ `BadUserAccessDenied`. 5. Stop the Primary container; the Secondary resumes recording into the same table; no duplicate `(TagId, SourceTimeUtc)` (guaranteed by PK — also check count growth rate stays 1×); restart. -6. Stop the driver's device (or blackhole) ⇒ a Bad sample stored with Bad status (SQL Server backend only). +6. Stop the driver's device (or blackhole) **while the driver is still publishing** (before #527 trips Reconnecting) ⇒ a row with `ValueKind = 0 (Empty)` and the driver's Bad code (Modbus: `BadCommunicationError 0x80050000`). Watchdog `BadNoCommunication` stamps are node-only and are NOT historized — do not expect them. 7. Record evidence in `docs/plans/2026-10-09-pluggable-historian-live-gate.md`; restore the rig config (Backend off) unless the user wants it kept. ```json:metadata