docs(r2-06): record S-11/U-7/C-7 remediation in STATUS.md + 06 report rows (06/S-11,U-7,C-7)

This commit is contained in:
Joseph Doherty
2026-07-13 10:07:00 -04:00
parent 4640ecb19b
commit fc38cee00e
3 changed files with 14 additions and 2 deletions
+7 -1
View File
@@ -43,7 +43,7 @@
| U-5 (Low) | Outbox serializer has no version/format byte | **STILL OPEN**`HistorizationOutboxEntrySerializer.cs:14-16` unchanged. |
| U-6 | Test-coverage thin spots (advise-failure status, out-of-order `RemoveAsync`, S-3 hang, pump filter) | **STILL OPEN** — zero test changes in the three gateway test projects since `9cad9ed0`. |
**New findings this pass:** S-11 (Medium), U-7 (Medium), C-7 (Low) — see body.
**New findings this pass:** S-11 (Medium), U-7 (Medium), C-7 (Low) — see body. **All three REMEDIATED on `r2/06-serverhistorian-failfast` (R2-06, 2026-07-13); the U-7 live EnsureTags/retype leg is operator-gated pending a VPN run.**
---
@@ -138,6 +138,8 @@ The read synthesis (`GalaxyDriver.cs:738-755`) fills pending snapshots with `Bad
#### S-11 (Medium, NEW) — An enabled-but-misconfigured `ServerHistorian` section crash-loops the Host with a raw `UriFormatException`; one corner gets no warning at all
> **REMEDIATED — `r2/06-serverhistorian-failfast` (R2-06, 2026-07-13; live leg operator-gated).** New fail-fast `ServerHistorianOptionsValidator` (Host, wired via `AddValidatedOptions`) consumer-gates on `ServerHistorian:Enabled` **OR** `AlarmHistorian:Enabled` (closing the zero-warning corner) and fails startup with a named, aggregated `OptionsValidationException` on an empty / non-absolute / non-http(s) `Endpoint`. Adapter defense-in-depth replaces the bare `new Uri(...)` with `Uri.TryCreate`→named `InvalidOperationException`. `Validate()` gains malformed-endpoint warning parity; an alarm-only deployment logs a startup warning. Also directly answers C-4's "give the warnings style a fail-fast tier".
`HistorianGatewayClientAdapter.Create` eagerly does `new Uri(options.Endpoint)` (`HistorianGatewayClientAdapter.cs:45`). `ServerHistorianOptions.Validate()` produces exactly the right warning for an empty Endpoint (`ServerHistorianOptions.cs:78-79`) — but it is **warnings-only**: registration proceeds (`Runtime/ServiceCollectionExtensions.cs:135-139`, `Host/Program.cs:123-127`) and the first DI resolution throws an unhandled `UriFormatException`, crashing the Host at bring-up. This is not theoretical — it made docker-dev unbootable until the config-side fix `7233e2ba` (whose commit message documents the exact failure). The code path is untouched.
The sharper corner: **`AddAlarmHistorian` gates only on `AlarmHistorian:Enabled`** (`Runtime/ServiceCollectionExtensions.cs:87`) but sources its connection from the `ServerHistorian` section (`Program.cs:127`). With `AlarmHistorian:Enabled=true` and `ServerHistorian:Enabled=false` (Endpoint legitimately empty), `Validate()` early-returns on `!Enabled` (`ServerHistorianOptions.cs:77`) — so the operator gets **zero warning**, then the same `UriFormatException` out of `CreateAlarmWriter` when the sink singleton is built. Same exposure for the continuous-historization writer (`Program.cs:187`, though that one is co-gated on `ServerHistorian.Enabled`, so only a *malformed* non-empty Endpoint reaches it).
@@ -212,6 +214,8 @@ Galaxy: throw-on-construct for required fields; DataAnnotations attributes on th
#### C-7 (Low, NEW) — The live-value writer populates a documented-dead `Quality` field
> **REMEDIATED — `r2/06-serverhistorian-failfast` (R2-06, 2026-07-13).** Decision: **keep populating** the field (the "stop populating so it's visible" alternative was rejected — proto3 sends `0` = Bad when unset, planting a future bug the instant a gateway version honors the field, whereas the recorder's `192` is correct). Visibility achieved with doc comments instead: `GatewayHistorianValueWriter` `<remarks>` + inline note at the mapping, `ContinuousHistorizationRecorder.GoodQuality` guard note, and a `docs/Historian.md` sentence — all citing 0.2.0 Contracts C-002.
`GatewayHistorianValueWriter` maps `HistorizationValue.Quality` onto `HistorianLiveValue.Quality` (`GatewayHistorianValueWriter.cs:63`), but the 0.2.0 contract now documents that field (and `QualityDetail`) as **silently discarded on the `WriteLiveValues` SQL path** — the gateway's SQL login is column-permission-denied on the server-managed Quality column, so the value is accepted on the wire and dropped. Harmless today because the recorder hardcodes `GoodQuality = 192` for every captured value (`ContinuousHistorizationRecorder.cs:45,256` — the outbox round-trips it, `:419`), so nothing meaningful is lost. The hazard is forward-looking: if the recorder is ever extended to capture *real* node quality (a natural next step), it will appear to work and silently persist nothing — quality will remain server-stamped. Add a comment at the mapping site citing the 0.2.0 contract note (Contracts C-002), or stop populating the field so the dead input is visible.
### 4. Underdeveloped Areas
@@ -237,6 +241,8 @@ The `Category=LiveIntegration` suite is complete and skip-clean: `GatewayLiveFix
#### U-7 (Medium, NEW) — The 0.2.0 bump's impact analysis missed the `DataType` presence change that alters Boolean tag provisioning
> **REMEDIATED (code + docs) — `r2/06-serverhistorian-failfast` (R2-06, 2026-07-13); LIVE LEG OPERATOR-GATED.** Pinned the 0.2.0 Boolean→Int1 explicit-presence semantics at two levels (`GatewayTagProvisionerTests.Boolean_definition_carries_explicit_Int1_presence` asserts `DataType==Int1` **and** `HasDataType` — the 0.2.0 presence witness; a package downgrade breaks the compile, a dropped-presence contract change breaks the assert). Added a written upsert/retype migration note to `docs/Historian.md` (three retype hypotheses + `failed=N` tally observability). Added the env-gated live Boolean EnsureTags leg + a **retype probe** (`HISTGW_BOOL_SANDBOX_TAG`) that records the deployed historian's in-place-retype policy. **Pending:** the live VPN run on a 0.2.0 gateway (feeds the observed retype answer back into the doc note; also discharges U-2's full documented run).
The bump commit (`f6eaa267`) gates only on the M3 `opc_quality` change ("checked and CLEAR" — correct, verified: OtOpcUa never writes `HistorianHistoricalValue`). But 0.2.0 also made **`HistorianTagDefinition.DataType` proto3-optional** (contracts review M-2), and that one lands squarely on `GatewayTagProvisioner`:
- The provisioner always sets `DataType` explicitly (`GatewayTagProvisioner.cs:57`), and `Boolean → HistorianDataType.Int1` (`HistorianTypeMapper.cs:28`) — where **Int1 is wire value 0**.
@@ -79,7 +79,7 @@
{
"id": "T11",
"subject": "Bookkeeping: update archreview/plans/STATUS.md + 06-gateway-integrations.md prior-finding rows for S-11/U-7/C-7 when the work lands",
"status": "pending",
"status": "completed",
"blockedBy": [
"T6",
"T9",
+6
View File
@@ -150,3 +150,9 @@ surface), 03/S2/S3 block-bridging, 03/P1 surgical adds (guard prerequisite in pl
per-plan task counts, effort, and suggested execution order: [`00-INDEX.md`](00-INDEX.md) §"Round 2
plans". 193 bite-sized TDD tasks total, ~1824 dev-days end-to-end; R2-01/02/03 (the re-review's
new-and-sharp items) are ~2 days combined.
### Round-2 remediation landed
| Plan | Findings | Branch | Commits | Status |
|---|---|---|---|---|
| **R2-06** | 06/S-11 (rank 7), 06/U-7 (rank 8), 06/C-7 | `r2/06-serverhistorian-failfast` (off `1676c8f4`) | `3a049a13``4640ecb1` (10 task commits) | **Code-complete, offline-verified.** S-11: fail-fast `ServerHistorianOptionsValidator` (consumer-gated on `ServerHistorian:Enabled` OR `AlarmHistorian:Enabled`, fails empty/non-http(s) Endpoint at startup) + `AddValidatedOptions` wiring + adapter defense-in-depth (`TryCreate`→named `InvalidOperationException`, no more raw `UriFormatException` crash-loop) + `Validate()` warning parity + alarm-only-mode startup warning. U-7: pinned Boolean→Int1 explicit-presence (`HasDataType`) + `HISTGW_BOOL_SANDBOX_TAG` live Boolean EnsureTags leg + retype probe (skip-clean offline) + upsert/retype migration note (`docs/Historian.md`). C-7: keep-populating decision + dead-`Quality` doc comments (0.2.0 C-002). **Live leg operator-gated (T12) — pending a VPN run on a 0.2.0 gateway** (records the observed in-place retype policy back into `docs/Historian.md`; also discharges 06/U-2's full documented run). |