dohertj2/lmxopcua
1
0
Fork 0
Code Issues 8 Pull Requests Actions Packages Projects Releases Wiki Activity

docs(localdb): phase-1 recon findings

Browse Source
This commit is contained in:
Joseph Doherty
2026-07-20 10:17:40 -04:00
parent be87ddeb0b
commit 42f8550716
5 changed files with 1642 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/plans/2026-07-20-localdb-adoption-phase1.md
+706
View File
@@ -0,0 +1,706 @@
# OtOpcUa LocalDb Adoption — Phase 1 Implementation Plan
> **For Claude:** REQUIRED SUB-SKILL: Use superpowers-extended-cc:executing-plans to implement this plan task-by-task.
>
> **Execution model:** This plan is optimized for **Claude Opus** agents (`claude --model opus`).
> Dispatch implementer subagents on Opus for every task classified `high-risk`; `small`/`trivial`
> tasks may use Sonnet. Work on branch **`feat/localdb-phase1`** in this repo (`~/Desktop/OtOpcUa`,
> remote `lmxopcua`). Do NOT merge to `master` as part of this plan — stop at the DoD task and
> report.
>
> **Design authority:** `~/Desktop/scadaproj/docs/plans/2026-07-20-otopcua-localdb-design.md`.
> Read it before Task 0. The reference adoption is ScadaBridge (merged PR #23) — when in doubt,
> mirror `~/Desktop/ScadaBridge` (files named per task below).
**Goal:** Give every driver-role OtOpcUa node a consolidated `ZB.MOM.WW.LocalDb` database that caches
the deployed-configuration artifact (chunked), boots from that cache when central SQL Server is
unreachable, and optionally replicates it to the node's redundant pair peer over the library's gRPC
sync (default-OFF, fail-closed bearer auth).
**Architecture:** `AddZbLocalDb`/`AddZbLocalDbReplication` wired in a new `LocalDbRegistration`
(mirroring `SecretsRegistration`), DDL + `RegisterReplicated` in `LocalDbSetup.OnReady`, a dedicated
Kestrel h2c listener for `MapZbLocalDbSync` gated on `LocalDb:SyncListenPort`, a consumer-supplied
fail-closed bearer interceptor, and an `IDeploymentArtifactCache` seam written by `DriverHostActor`
after each successful apply and read as a boot fallback. The dormant LiteDB LocalCache subsystem is
deleted as superseded.
**Tech stack:** .NET 10, `ZB.MOM.WW.LocalDb`/`.Replication`/`.Contracts` **0.1.1** (Gitea feed),
`Grpc.AspNetCore` 2.76.0, Akka.NET (existing), xunit.v3 (Host.IntegrationTests) + xunit2 TestKit
(actor tests).
**Hard rules (from the ScadaBridge adoption — violating any of these is a defect):**
1. Order inside `OnReady` is load-bearing: **DDL → `RegisterReplicated` → writes**. Rows written
before registration are never captured or snapshotted — silently, forever.
2. No autoincrement PKs, no BLOB columns in replicated tables.
3. The library's passive sync endpoint verifies **no** ApiKey — the host interceptor is the only
auth, and it must be fail-closed (no key configured ⇒ deny everything).
4. `ILocalDb.CreateConnection()` returns an **already-open**, pragma-configured connection with the
`zb_hlc_next()` UDF. Never call `Open()` on it. There is **no in-memory mode** — tests use temp
files + `SqliteConnection.ClearAllPools()` before delete.
5. `Grpc.Core.Testing` does not exist on grpc-dotnet — hand-roll fake `ServerCallContext`s.
6. Explicit Kestrel `Listen*` calls make Kestrel **ignore `ASPNETCORE_URLS`** — when adding the sync
listener you must re-bind the existing HTTP port in the same block, and verify it on the rig.
7. Never run host `sqlite3` against a live bind-mounted WAL DB (copy the `db`/`-wal`/`-shm` triplet
with `cp` instead). `aspnet:10.0` containers have no `curl` — use a
`curlimages/curl` sidecar with `--network container:<name>`.
---
### Task 0: Preflight + recon (produces `docs/plans/2026-07-20-localdb-phase1-recon.md`)
**Classification:** standard
**Estimated implement time:** ~5 min
**Parallelizable with:** none (everything downstream consumes its findings)
**Files:**
- Create: `docs/plans/2026-07-20-localdb-phase1-recon.md`
- Read-only: `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs`,
`src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DeploymentArtifact.cs`,
`src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs`,
`src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/SecretsRegistration.cs`,
`Directory.Packages.props`, `docker-dev/docker-compose.yml`,
the observability registration (`AddOtOpcUaObservability` implementation),
`src/Core/ZB.MOM.WW.OtOpcUa.Configuration/LocalCache/*`
**Step 1: Verify the feed packages restore.**
Run:
```bash
cd ~/Desktop/OtOpcUa && dotnet package search ZB.MOM.WW.LocalDb --source dohertj2-gitea --format json | head -50
```
Expected: `ZB.MOM.WW.LocalDb`, `.Replication`, `.Contracts` at `0.1.1`. If absent → STOP, report.
**Step 2: Map and record, with `file:line` citations, each of:**
1. **Deploy fetch path:** where `DriverHostActor` loads `Deployment.ArtifactBlob` (the
`_dbFactory.CreateDbContext()` sites, ~L1472/1543), the artifact's runtime type
(`DeploymentArtifact` — how it's deserialized from the blob), the type/format of
`DeploymentId` and `RevisionHash`, and where a successful apply completes (the point after
which the cache write belongs).
2. **Cold-boot path:** what the actor does at startup before any `DispatchDeployment` arrives —
does it query central SQL for the current deployment? Where does the failure path land
(the `Stale` state, `DbHealthProbeActor` interaction, L1345 dispatch-ignore)? Identify the
exact seam where "central fetch failed at boot" is known — that is where the cache fallback goes.
3. **Cluster identity:** how a driver node knows its `ClusterId` (config key / `ClusterNode` row /
`IClusterRoleInfo`) — the cache is keyed by it.
4. **DriverHostActor construction:** how its dependencies are injected
(`WithOtOpcUaRuntimeActors`, `Props` wiring) so `IDeploymentArtifactCache` can be threaded in.
⚠ `Props.Create` builds an expression tree — adding a parameter silently rebinds
out-of-position named args at call sites; list every construction site.
5. **Telemetry allowlist:** does `AddOtOpcUaObservability` restrict meters
(`ZbTelemetryOptions.Meters` or equivalent)? If yes, record where the list lives.
6. **Kestrel/URLs:** confirm the Host binds via `ASPNETCORE_URLS=http://+:9000` with no
`ConfigureKestrel` calls; record any existing `builder.WebHost` usage.
7. **LiteDB LocalCache:** confirm (grep) that nothing outside
`src/Core/ZB.MOM.WW.OtOpcUa.Configuration/LocalCache/` and its tests references
`ILocalConfigCache`/`GenerationSealedCache`/`ResilientConfigReader`/`LiteDB`.
8. **docker-dev volumes:** whether nodes have a writable data volume; record what `LocalDb:Path`
should be per node and how env vars are passed (`Cluster__*` style double-underscore).
**Step 3: STOP conditions — halt the plan and report instead of improvising if:**
- the artifact apply path cannot be given a post-apply hook without restructuring the actor;
- `DeploymentId`/cluster identity are not stable strings/GUIDs;
- LiteDB LocalCache turns out to be referenced by live code.
**Step 4: Commit.**
```bash
git add docs/plans/2026-07-20-localdb-phase1-recon.md
git commit -m "docs(localdb): phase-1 recon findings"
```
---
### Task 1: Package references and pins
**Classification:** small
**Estimated implement time:** ~3 min
**Parallelizable with:** none (all code tasks build on it)
**Files:**
- Modify: `Directory.Packages.props`
- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Host/ZB.MOM.WW.OtOpcUa.Host.csproj`
- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ZB.MOM.WW.OtOpcUa.Runtime.csproj`
**Step 1:** Add to `Directory.Packages.props` (versions exact):
```xml
<PackageVersion Include="ZB.MOM.WW.LocalDb" Version="0.1.1" />
<PackageVersion Include="ZB.MOM.WW.LocalDb.Replication" Version="0.1.1" />
<PackageVersion Include="Grpc.AspNetCore" Version="2.76.0" />
```
Check the existing `Grpc.Net.Client`/`Grpc.Core.Api` are already ≥ 2.76.0 and
`Google.Protobuf` ≥ 3.34.1 (the LocalDb.Replication nuspec floors); raise if not. Confirm a
`SQLitePCLRaw` pin ≥ 2.1.12 exists for the transitive `Microsoft.Data.Sqlite` chain (the family
advisory GHSA-2m69-gcr7-jv3q); Core.AlarmHistorian already pins `bundle_e_sqlite3` 2.1.12 — add
`SQLitePCLRaw.lib.e_sqlite3` 2.1.12 as an explicit reference in the Host if the audit flags it.
**Step 2:** Host csproj: `ZB.MOM.WW.LocalDb`, `ZB.MOM.WW.LocalDb.Replication`, `Grpc.AspNetCore`.
Runtime csproj: `ZB.MOM.WW.LocalDb` only (it needs `ILocalDb`, not the sync engine).
**Step 3:** `dotnet build ZB.MOM.WW.OtOpcUa.slnx` → 0 warnings/errors (repo builds warnings-as-errors).
**Step 4: Commit.** `git commit -m "build(localdb): reference ZB.MOM.WW.LocalDb 0.1.1 + Grpc.AspNetCore"`
---
### Task 2: Schema + `LocalDbSetup.OnReady` + registration tests
**Classification:** standard
**Estimated implement time:** ~5 min
**Parallelizable with:** Task 3
**Files:**
- Create: `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Deployment/DeploymentCacheSchema.cs`
- Create: `src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/LocalDbSetup.cs`
- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/LocalDbSetupTests.cs` (create the test project
reference layout to match neighboring Host unit tests; if Host has no unit-test project, place in
the closest existing one and note the deviation)
**Step 1: Write the failing tests first** — using a real temp-file `ILocalDb` (build it via
`new ServiceCollection().AddZbLocalDb(config, LocalDbSetup.OnReady)` with `LocalDb:Path` pointed at
a temp file; dispose + `SqliteConnection.ClearAllPools()` in cleanup):
- `OnReady_RegistersExactlyTheTwoDeploymentTables` — `db.ReplicatedTables.Keys` ordinal-sorted
equals `["deployment_artifacts", "deployment_pointer"]` (assert BOTH directions: no fewer, no more).
- `DeploymentArtifacts_PkIsDeploymentIdPlusChunkIndex` and `DeploymentPointer_PkIsClusterId`
(pin `ReplicatedTable.PkColumns`).
- `RowsWrittenAfterOnReady_EnterTheOplog` — insert a row, assert `SELECT COUNT(*) FROM __localdb_oplog` ≥ 1
(pins the DDL→register ordering; this is the assertion that catches a silently-broken CDC).
**Step 2: Run tests, watch them fail** (types don't exist yet).
**Step 3: Implement.** `DeploymentCacheSchema` depends only on `Microsoft.Data.Sqlite` (the
ScadaBridge `*Schema.Apply` pattern — self-sufficient for direct construction in tests):
```csharp
namespace ZB.MOM.WW.OtOpcUa.Runtime.Deployment;
public static class DeploymentCacheSchema
{
public static void Apply(SqliteConnection connection)
{
using var cmd = connection.CreateCommand();
cmd.CommandText = """
CREATE TABLE IF NOT EXISTS deployment_artifacts (
deployment_id TEXT NOT NULL,
chunk_index INTEGER NOT NULL,
cluster_id TEXT NOT NULL,
revision_hash TEXT NOT NULL,
chunk_count INTEGER NOT NULL,
chunk_base64 TEXT NOT NULL,
cached_at_utc TEXT NOT NULL,
PRIMARY KEY (deployment_id, chunk_index)
);
CREATE TABLE IF NOT EXISTS deployment_pointer (
cluster_id TEXT NOT NULL PRIMARY KEY,
deployment_id TEXT NOT NULL,
revision_hash TEXT NOT NULL,
artifact_sha256 TEXT NOT NULL,
applied_at_utc TEXT NOT NULL
);
""";
cmd.ExecuteNonQuery();
}
}
```
`LocalDbSetup` (Host):
```csharp
namespace ZB.MOM.WW.OtOpcUa.Host.Configuration;
internal static class LocalDbSetup
{
// ORDER IS LOAD-BEARING: DDL, then RegisterReplicated, then (nothing else in Phase 1).
// Rows written before RegisterReplicated are invisible to the peer forever, silently.
public static void OnReady(ILocalDb db)
{
using var connection = db.CreateConnection(); // already open — do NOT call Open()
DeploymentCacheSchema.Apply(connection);
db.RegisterReplicated("deployment_artifacts");
db.RegisterReplicated("deployment_pointer");
}
}
```
**Step 4:** `dotnet test --filter "FullyQualifiedName~LocalDbSetupTests"` → PASS.
**Step 5: Commit.** `git commit -m "feat(localdb): deployment-cache schema + OnReady registration"`
---
### Task 3: Fail-closed sync auth interceptor + tests
**Classification:** standard
**Estimated implement time:** ~4 min
**Parallelizable with:** Task 2
**Files:**
- Create: `src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/LocalDbSyncAuthInterceptor.cs`
- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/LocalDbSyncAuthInterceptorTests.cs`
Port ScadaBridge's `src/ZB.MOM.WW.ScadaBridge.Host/LocalDbSyncAuthInterceptor.cs` (read it first;
keep behavior identical, adjust namespace only). Semantics to preserve exactly:
- `ServicePrefix = "/localdb_sync.v1.LocalDbSync/"`; any other method path passes through untouched.
- Expected token from `IOptions<ReplicationOptions>.Value.ApiKey`.
- **Fail-closed:** null/empty configured key ⇒ `RpcException(new Status(StatusCode.PermissionDenied, ...))`
for every sync call. Missing/mismatched bearer ⇒ same.
- `CryptographicOperations.FixedTimeEquals` over UTF-8 bytes; header `authorization`,
case-insensitive, `"Bearer "` prefix. Override all four server handler kinds.
**Tests** (hand-rolled fake `ServerCallContext` — `Grpc.Core.Testing` does not exist on grpc-dotnet;
copy ScadaBridge's `LocalDbSyncAuthInterceptorTests.cs` fake): non-sync method passes with no key;
sync + no configured key → PermissionDenied; wrong bearer → PermissionDenied; correct bearer → passes.
Write tests first (fail), implement, `dotnet test --filter "FullyQualifiedName~LocalDbSyncAuthInterceptor"`,
commit `feat(localdb): fail-closed bearer interceptor for the sync endpoint`.
---
### Task 4: `LocalDbRegistration` + Program.cs DI wiring + config defaults
**Classification:** high-risk (touches Program.cs / role gating)
**Estimated implement time:** ~5 min
**Parallelizable with:** none (Task 5 edits the same file)
**Files:**
- Create: `src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/LocalDbRegistration.cs`
- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs` (inside the `hasDriver` branch)
- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Host/appsettings.json`
**Step 1:** `LocalDbRegistration`, mirroring `SecretsRegistration`'s shape:
```csharp
namespace ZB.MOM.WW.OtOpcUa.Host.Configuration;
internal static class LocalDbRegistration
{
/// Driver-role nodes only. Storage is unconditional; replication stays inert until
/// LocalDb:Replication:PeerAddress (initiator) / LocalDb:SyncListenPort (listener) are set.
public static IServiceCollection AddOtOpcUaLocalDb(
this IServiceCollection services, IConfiguration configuration)
{
services.AddZbLocalDb(configuration, LocalDbSetup.OnReady);
services.AddZbLocalDbReplication(configuration);
return services;
}
public static int SyncListenPort(IConfiguration configuration) =>
configuration.GetValue<int>("LocalDb:SyncListenPort");
}
```
**Step 2:** Program.cs, in the `hasDriver` block (near `AddAlarmHistorian`):
`builder.Services.AddOtOpcUaLocalDb(builder.Configuration);` and
`builder.Services.AddGrpc(o => o.Interceptors.Add<LocalDbSyncAuthInterceptor>());`
(AddGrpc only under `hasDriver` — admin-only nodes expose no sync surface).
**Step 3:** appsettings.json:
```json
"LocalDb": {
"Path": "./data/otopcua-localdb.db",
"SyncListenPort": 0,
"Replication": {}
}
```
`LocalDb:Path` is `ValidateOnStart`-required once `AddZbLocalDb` runs — since registration is
driver-gated, admin-only configs need nothing. Check the role-overlay files
(`appsettings.driver.json` / `appsettings.admin-driver.json`) and any deploy templates for
conflicting `LocalDb` keys.
**Step 4:** Build + full Host unit tests. Verify an admin-only graph doesn't require the key: this
is pinned properly in Task 10's integration tests, but do a quick
`OTOPCUA_ROLES=admin dotnet run` smoke only if cheap; otherwise rely on Task 10.
**Step 5: Commit.** `feat(localdb): wire AddOtOpcUaLocalDb into the driver role`
---
### Task 5: Dedicated h2c sync listener + endpoint mapping (THE Kestrel task)
**Classification:** high-risk
**Estimated implement time:** ~5 min
**Parallelizable with:** none (Program.cs)
**Files:**
- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs`
**Why high-risk:** the Host today binds exclusively via `ASPNETCORE_URLS=http://+:9000`. Any
explicit `ConfigureKestrel(... Listen*)` makes Kestrel **ignore URLs entirely** (it logs
"Overriding address(es)"), which would silently kill the AdminUI/deploy API behind Traefik.
**Step 1:** Add, before `builder.Build()`:
```csharp
var syncPort = LocalDbRegistration.SyncListenPort(builder.Configuration);
if (hasDriver && syncPort > 0)
{
// Explicit Listen* replaces ASPNETCORE_URLS wholesale, so re-bind the primary HTTP
// endpoint here too. Parse it from the configured URLs rather than hard-coding 9000.
var urls = builder.Configuration["ASPNETCORE_URLS"] ?? builder.Configuration["urls"] ?? "http://+:9000";
var httpPort = new Uri(urls.Split(';')[0].Replace("+", "localhost").Replace("*", "localhost")).Port;
builder.WebHost.ConfigureKestrel(k =>
{
k.ListenAnyIP(httpPort); // HTTP/1.1 (existing surface)
k.ListenAnyIP(syncPort, o => o.Protocols = HttpProtocols.Http2); // h2c, prior-knowledge gRPC
});
}
```
When `syncPort == 0` (the default) nothing changes — URLs binding stays untouched. Cleartext
`Http1AndHttp2` cannot serve prior-knowledge h2c, hence the dedicated `Http2`-only listener.
**Step 2:** After the app pipeline's other `Map*` calls, driver-gated:
```csharp
if (hasDriver && syncPort > 0)
{
app.MapZbLocalDbSync();
}
```
(Mapping is harmless when unauthenticated — the interceptor fail-closes — but gating on the port
keeps admin-only and default-OFF graphs entirely free of the endpoint.)
**Step 3: Verify both surfaces.**
```bash
dotnet build ZB.MOM.WW.OtOpcUa.slnx
```
Then a local smoke with the port on:
`ASPNETCORE_URLS=http://+:9000 OTOPCUA_ROLES=driver LocalDb__SyncListenPort=9001 dotnet run --project src/Server/ZB.MOM.WW.OtOpcUa.Host ...`
— expect startup logs to show BOTH `:9000` and `:9001` bound, and `curl -s localhost:9000/healthz`
(or the mapped health route) still answering. If the app needs SQL to boot, defer the smoke to the
Task 12 rig check but say so explicitly in the task notes.
**Step 4: Commit.** `feat(localdb): dedicated h2c listener + MapZbLocalDbSync gated on LocalDb:SyncListenPort`
---
### Task 6: `IDeploymentArtifactCache` + chunked LocalDb implementation + tests
**Classification:** standard
**Estimated implement time:** ~5 min
**Parallelizable with:** Task 3, Task 5
**Files:**
- Create: `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Deployment/IDeploymentArtifactCache.cs`
- Create: `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Deployment/LocalDbDeploymentArtifactCache.cs`
- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Deployment/LocalDbDeploymentArtifactCacheTests.cs`
**Step 1: Failing tests** (real temp-file `ILocalDb` with `LocalDbSetup.OnReady` applied):
- round-trip: store a 300 KiB random artifact → `GetCurrentAsync` returns byte-identical payload
(forces ≥ 3 chunks at 128 KiB);
- retention: store 3 deployments for one cluster → only the newest 2 remain in
`deployment_artifacts`; pointer names the newest;
- integrity: corrupt one chunk row via SQL → `GetCurrentAsync` returns null (miss), never a
truncated artifact;
- missing pointer → null.
**Step 2: Implement:**
```csharp
public interface IDeploymentArtifactCache
{
Task StoreAsync(string clusterId, string deploymentId, string revisionHash,
byte[] artifact, CancellationToken ct = default);
Task<CachedDeploymentArtifact?> GetCurrentAsync(string clusterId, CancellationToken ct = default);
}
public sealed record CachedDeploymentArtifact(
string DeploymentId, string RevisionHash, byte[] Artifact, DateTimeOffset AppliedAtUtc);
```
`LocalDbDeploymentArtifactCache(ILocalDb db)`:
- `StoreAsync`: one `ILocalDbTransaction` — delete any existing chunks for this `deployment_id`
(idempotent re-store), insert chunks (raw 128 * 1024 bytes per chunk, `Convert.ToBase64String`),
upsert the pointer (`INSERT ... ON CONFLICT(cluster_id) DO UPDATE`), then prune: delete
`deployment_artifacts` rows whose `deployment_id` is not among the newest 2 `cached_at_utc` for
this `cluster_id`. SHA-256 over the raw artifact into the pointer. ISO-8601 UTC timestamps.
- `GetCurrentAsync`: read pointer; read chunks `ORDER BY chunk_index`; verify count ==
`chunk_count` and SHA-256 matches; return null on any mismatch (log a warning naming which check
failed).
- Use `db.ExecuteAsync`/`QueryAsync` with anonymous-object parameters (`@Name` markers). Remember
a `Dictionary<string,string>` parameter **throws** — use anonymous objects.
**Step 3:** run tests → PASS. **Step 4: Commit.**
`feat(localdb): chunked deployment-artifact cache over ILocalDb`
---
### Task 7: Cache write path in `DriverHostActor`
**Classification:** high-risk (actor model, `Props` expression-tree trap)
**Estimated implement time:** ~5 min
**Parallelizable with:** none
**Files:** (exact edit sites come from the Task 0 recon doc — cite it in the commit)
- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs`
- Modify: the actor-registration site (`WithOtOpcUaRuntimeActors` / DI extension) to provide
`IDeploymentArtifactCache`
- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs` or `LocalDbRegistration` — register
`IDeploymentArtifactCache` → `LocalDbDeploymentArtifactCache` singleton (driver branch)
- Test: the **xunit2** TestKit project used for existing DriverHostActor tests (recon names it)
**Steps:**
1. **Failing test:** after a successful artifact apply, the cache received `StoreAsync` with the
applied deployment's id/hash/bytes (inject a recording fake `IDeploymentArtifactCache`).
Also: a cache that throws on `StoreAsync` does NOT fail the apply (apply result unchanged, error
logged).
2. Thread the dependency through the actor's constructor. ⚠ `Props.Create` is an expression tree:
after adding the parameter, re-check **every** construction site for positional/named-arg
rebinding (the recon lists them) — do not rely on the compiler.
3. Invoke `StoreAsync` fire-and-forget (`PipeTo`-style or a guarded `Task.Run` per the actor's
existing async conventions — match whatever pattern the actor already uses for side-effect IO)
at the post-apply point identified in recon. Cluster id from the recon-identified source.
4. Run the actor test suite for this project. Commit:
`feat(localdb): DriverHostActor stores applied artifacts in the pair-local cache`
---
### Task 8: Boot-from-cache read path + running-from-cache signal
**Classification:** high-risk
**Estimated implement time:** ~5 min
**Parallelizable with:** none
**Files:**
- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs`
- Test: same xunit2 TestKit project as Task 7
**Steps:**
1. **Failing tests:**
- central fetch fails at startup AND cache has an artifact → the actor applies the cached
artifact (assert via whatever observable the apply already exposes) and logs/flags
running-from-cache;
- central fetch fails AND cache empty → today's behavior exactly (Stale, no apply);
- central fetch **succeeds** → cache is NOT consulted (fresh config wins; assert the fake
cache's `GetCurrentAsync` was never called on the happy path).
2. Implement at the recon-identified boot-failure seam. The signal: a log warning at minimum plus,
if the actor already publishes health/status (recon says how), a `RunningFromCache` marker on it.
Do NOT touch `DispatchDeployment` handling — a new deployment still requires central.
3. Run tests; commit `feat(localdb): boot from the pair-local artifact cache when central SQL is unreachable`.
---
### Task 9: Delete the dormant LiteDB LocalCache subsystem
**Classification:** small
**Estimated implement time:** ~3 min
**Parallelizable with:** Task 6, Task 10, Task 11
**Files:**
- Delete: `src/Core/ZB.MOM.WW.OtOpcUa.Configuration/LocalCache/` (entire directory:
`ILocalConfigCache`, `LiteDbConfigCache`, `GenerationSealedCache`, `ResilientConfigReader`,
`GenerationSnapshot`, `StaleConfigFlag`, exceptions)
- Delete: `tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/GenerationSealedCacheTests.cs`,
`ResilientConfigReaderTests.cs`
- Modify: `Directory.Packages.props` + the Configuration csproj — remove the `LiteDB` package if
nothing else references it (grep first)
**Steps:** Confirm the Task 0 recon's "nothing references it" finding still holds
(`grep -rn "ILocalConfigCache\|GenerationSealedCache\|ResilientConfigReader\|LiteDB" src/ tests/`),
delete, build the full solution, run the Configuration test project. DoD phrasing: **no references
from code** — leave any explanatory prose/comments that point readers at LocalDb instead. Add one
line to the recon doc noting LiteDB's removal. Commit:
`refactor(localdb): delete the dormant LiteDB LocalCache (superseded by ZB.MOM.WW.LocalDb)`
---
### Task 10: Health check + telemetry meter allowlist
**Classification:** small
**Estimated implement time:** ~4 min
**Parallelizable with:** Task 9, Task 11
**Files:**
- Create: `src/Server/ZB.MOM.WW.OtOpcUa.Host/Health/LocalDbReplicationHealthCheck.cs` (or the
directory `AddOtOpcUaHealth` uses — recon names it)
- Modify: the `AddOtOpcUaHealth` registration (driver branch)
- Modify: the observability meter list IF Task 0 found an allowlist
**Steps:**
1. Failing unit tests: replication unconfigured (no peer, no listener) → `Healthy` (default-OFF
must not degrade a plain node); peer configured + `ISyncStatus.Connected == false` → `Degraded`;
connected + backlog `null` → `Degraded` (unknown backlog is not healthy); connected + backlog
small → `Healthy`.
2. Implement against `ISyncStatus` + `IOptions<ReplicationOptions>` (peer-configured = non-empty
`PeerAddress` OR `SyncListenPort > 0` — pass the latter in via options/config).
3. **Meter allowlist:** if `AddOtOpcUaObservability` restricts meters, add
`"ZB.MOM.WW.LocalDb.Replication"` (use `LocalDbMetrics.MeterName`) — this is a silent allowlist;
the ScadaBridge live gate is the proof it bites. If there is no allowlist, record that in the
recon doc and skip.
4. Commit `feat(localdb): replication health check + meter export`.
---
### Task 11: DI-pin integration tests over the real Program.cs
**Classification:** standard
**Estimated implement time:** ~5 min
**Parallelizable with:** Task 9, Task 10
**Files:**
- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LocalDbWiringTests.cs` (xunit.v3,
`WebApplicationFactory<Program>` — the project already builds the real Program.cs; set
`OTOPCUA_ROLES` per test case the way neighboring tests do)
**Pins (each its own test):**
1. Driver graph: `ILocalDb` resolves, is a singleton, `ReplicatedTables` ordinal-sorted ==
`["deployment_artifacts", "deployment_pointer"]` — exact set, both directions load-bearing.
2. Driver graph: `IDeploymentArtifactCache` resolves to `LocalDbDeploymentArtifactCache`.
3. Default-OFF pin: `ISyncStatus` resolves with `Connected == false`, `PeerNodeId == null`.
4. Admin-only graph: `ILocalDb` is NOT registered (`GetService<ILocalDb>()` is null) and boot does
not demand `LocalDb:Path`.
5. Health: the LocalDb health check is registered in the driver graph.
Point `LocalDb:Path` at a per-test temp file via the factory's config overrides;
`ClearAllPools` + delete in cleanup. These tests exist because DI extensions without a
container-built test have shipped inert three times in this family (Secrets 0.2.0/0.2.2,
ScadaBridge#22). Commit `test(localdb): DI pins over the real host graph`.
---
### Task 12: Two-node convergence harness + tests
**Classification:** high-risk
**Estimated implement time:** ~5 min (harness) + ~4 min (scenarios) — split the commit if needed
**Parallelizable with:** none (depends on Tasks 2, 3, 5, 6)
**Files:**
- Create: `tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LocalDb/LocalDbPairHarness.cs`
- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LocalDb/LocalDbPairConvergenceTests.cs`
**Harness:** copy the pattern from
`~/Desktop/ScadaBridge/tests/ZB.MOM.WW.ScadaBridge.IntegrationTests/LocalDbSitePairHarness.cs` and
the library's `ConvergenceFixture` (`~/Desktop/scadaproj/ZB.MOM.WW.LocalDb/tests/.../Convergence/ConvergenceFixture.cs`):
- Two full stacks over a real loopback Kestrel h2c socket (port 0), **through the real
`LocalDbSyncAuthInterceptor`** with a shared test key; node A initiator, node B passive.
- Both initialized via the **production `LocalDbSetup.OnReady`** — a hand-written schema proves
only that the test agrees with itself.
- DBs owned by the harness, registered as pre-constructed singletons (so MS.DI won't dispose them
on host teardown); `KillTransportAsync`/`RestartTransportAsync`; tight `FlushInterval` (50 ms),
bounded backoff (2 s); temp files, `ClearAllPools` cleanup; a collection definition to serialize
these tests.
**Scenarios:**
1. `ArtifactStoredOnA_ConvergesToB_ByteIdentical` — store a multi-chunk artifact via
`LocalDbDeploymentArtifactCache` on A; B's cache returns byte-identical bytes; both nodes'
`__localdb_row_version` rows for the pointer carry the **same HLC and origin node id** (B holds
A's row, not a re-derived one).
2. `RetentionPruneOnA_TombstonesReachB` — third deployment on A prunes the first; B's chunk rows
for the pruned deployment disappear and `__localdb_row_version WHERE is_tombstone=1` rows exist
on B.
3. `WritesWhileTransportDown_SurviveRejoin` — kill transport, store on A, restart, converge.
4. `WrongApiKey_NeverConverges` — harness with mismatched keys: assert **no** convergence after a
bounded wait AND (positive control) that the same scenario with matching keys converges — an
absence assertion without a positive control passed vacuously in ScadaBridge.
**DoD within this task:** temporarily comment out the two `RegisterReplicated` calls and confirm
scenarios 1–3 go red (run locally, do not commit the red state); restore. Record the red/green
evidence in the task notes. Commit `test(localdb): 2-node convergence harness + scenarios`.
---
### Task 13: docker-dev rig configuration
**Classification:** small
**Estimated implement time:** ~4 min
**Parallelizable with:** Task 14
**Files:**
- Modify: `docker-dev/docker-compose.yml`
**Steps** (adjust to the recon's findings on volumes/env style):
1. Every driver node: a named volume (or existing data mount) backing `/app/data`, and
`LocalDb__Path=/app/data/otopcua-localdb.db`.
2. **site-a pair only** (mirror the ScadaBridge default-OFF posture — site-b stays unreplicated as
the pin):
- site-a-1: `LocalDb__SyncListenPort=9001`, `LocalDb__Replication__PeerAddress=http://site-a-2:9001`,
`LocalDb__Replication__ApiKey=dev-site-a-localdb-sync-key`, `LocalDb__Replication__MaxBatchSize=16`
- site-a-2: `LocalDb__SyncListenPort=9001`, same `ApiKey`, same `MaxBatchSize`, **no PeerAddress**
(passive; the stream is bidirectional).
The key must be byte-identical on both — the interceptor fail-closes on any mismatch and the
pair silently stops converging.
3. `MaxBatchSize=16` because batching is row-count-only against gRPC's 4 MB cap and artifact chunk
rows are ≈ 171 KB (16 × 171 KB ≈ 2.7 MB worst case).
4. `docker compose config` to validate; do NOT bring the rig up in this task (the live gate task
owns rig runs).
5. Commit `chore(localdb): rig config — consolidated path everywhere, replication on the site-a pair`.
---
### Task 14: Documentation
**Classification:** small
**Estimated implement time:** ~4 min
**Parallelizable with:** Task 13
**Files:**
- Create: `docs/operations/2026-07-20-localdb-pair-replication.md` (runbook: enable/disable,
ApiKey handling via `${secret:}`, stop/start-together rule, tombstone-retention resurrection
window, MaxBatchSize row-count-vs-4MB, never host-`sqlite3` a live WAL DB / cp-triplet recipe)
- Modify: `docs/Redundancy.md` (a short "pair-local config cache" section: what replicates, what
boot-from-cache does and does not cover)
- Modify: `CLAUDE.md` (this repo): LocalDb adoption row/paragraph — state Phase 1 scope, default-OFF,
rig-only enablement
- Modify: `docs/Configuration.md` if it documents config keys (add the `LocalDb` section)
Commit `docs(localdb): phase-1 runbook + redundancy/config docs`.
---
### Task 15: DoD sweep (offline)
**Classification:** standard
**Estimated implement time:** ~5 min
**Parallelizable with:** none (final offline task)
**Steps:**
1. `dotnet build ZB.MOM.WW.OtOpcUa.slnx` → **0 warnings** (warnings-as-errors).
2. `dotnet test ZB.MOM.WW.OtOpcUa.slnx` → full suite green; record counts. Baseline any
pre-existing failures BEFORE this branch (`git stash` → run → unstash) rather than assuming;
report deltas only.
3. Greps (phrase as "no references from code"; explanatory comments may remain):
`grep -rn "LiteDB\|ILocalConfigCache\|GenerationSealedCache" src/ tests/` → no code references.
4. Confirm the positive-control evidence from Task 12 is recorded.
5. Update `docs/plans/2026-07-20-localdb-adoption-phase1.md.tasks.json` statuses.
6. Commit `chore(localdb): phase-1 DoD sweep` and STOP. Report: branch name, commit list, test
counts, and that the live gate (Task 16) needs the rig + operator go-ahead.
---
### Task 16: Live gate on the docker-dev rig (run only with explicit user go-ahead)
**Classification:** high-risk (rig)
**Estimated implement time:** ~30 min wall-clock (mostly waiting)
**Parallelizable with:** none
**Preamble:** All DB inspection via `cp` of the `db`/`-wal`/`-shm` triplet out of the container
(`docker cp`) and querying the copy — never host `sqlite3` on the live file (it poisons the WAL
across virtiofs; root cause of the 2026-07-20 ScadaBridge incident). Metrics via
`docker run --rm --network container:<node> curlimages/curl:latest -s localhost:<port>/metrics`.
If an anomaly appears within seconds of one of your own measurements, suspect the measurement
first — restart both nodes and re-run untouched before blaming the library.
**Checks (all must PASS; record evidence in `docs/plans/2026-07-20-localdb-phase1-live-gate.md`):**
1. Rig up, site-a pair healthy, `/metrics` on both shows `localdb_` series (proves the meter
allowlist work).
2. Deploy a config through the normal deploy API → both site-a nodes' `deployment_pointer` +
`deployment_artifacts` rows are byte-identical with **identical HLC + origin node id** (one
node's row replicated, not two independent derivations — both nodes also locally store after
apply, so expect LWW-converged identical content either way; assert convergence, and note which
origin won).
3. **Boot-from-cache:** stop the SQL container; restart site-a-2; it comes up serving the cached
config with the running-from-cache signal in its log; start SQL again; node recovers fresh
behavior.
4. **Replicated-cache payoff:** wipe site-a-2's local DB file (container stopped), restart it with
SQL **up**, let replication repopulate the cache from site-a-1, then repeat check 3's SQL-down
restart — it boots from a cache it never wrote itself.
5. Transport kill (pause site-a-2 container): store/deploy on a-1, oplog depth rises; unpause;
drains to 0; converged.
6. Retention: deploy a 3rd config; pruned deployment's chunks gone on BOTH nodes with tombstone
rows present on both.
7. Both-nodes-together restart: clean rejoin, identical counts, zero
`disk I/O error`/`SQLITE_IOERR`/`corrupt` in either log.
8. site-b pair (default-OFF pin): LocalDb file exists and works locally, `ISyncStatus` health
Healthy, no sync connections, no listener on 9001.
Record PASS/FAIL per check with the actual evidence (row counts, md5s, log lines). Commit the gate
doc. Do not merge — report.
---
## Task persistence
Tasks file: `docs/plans/2026-07-20-localdb-adoption-phase1.md.tasks.json` (same directory).
Update statuses as tasks complete; any deviation from this plan gets a `deviation` note on the task
(the ScadaBridge tasks.json convention — the record of *why* is what makes the plan auditable).
docs/plans/2026-07-20-localdb-adoption-phase1.md.tasks.json
+25
View File
@@ -0,0 +1,25 @@
{
"planPath": "docs/plans/2026-07-20-localdb-adoption-phase1.md",
"tasks": [
{ "id": 0, "subject": "Task 0: Preflight + recon (findings doc, STOP conditions)", "status": "completed",
"notes": "Feed verified: LocalDb/.Contracts/.Replication all 0.1.1. All 3 STOP conditions clear. Findings: docs/plans/2026-07-20-localdb-phase1-recon.md. 8 deviations recorded (D-1..D-8); D-1/D-3/D-5/D-6 change downstream work materially.",
"deviation": "D-1 cache read cannot be keyed by ClusterId at the boot seam (unkeyed newest-pointer read instead); D-3 a third LiteDB test file must be deleted; D-5 WebApplicationFactory<Program> is deliberately unused in this repo (use TwoNodeClusterHarness); D-6 driver-only nodes have no ASPNETCORE_URLS so the Kestrel re-bind fallback is wrong. See recon doc §9." },
{ "id": 1, "subject": "Task 1: Package references and pins (LocalDb 0.1.1, Grpc.AspNetCore, SQLitePCLRaw)", "status": "pending", "blockedBy": [0] },
{ "id": 2, "subject": "Task 2: Schema + LocalDbSetup.OnReady + registration tests", "status": "pending", "blockedBy": [1] },
{ "id": 3, "subject": "Task 3: Fail-closed sync auth interceptor + tests", "status": "pending", "blockedBy": [1] },
{ "id": 4, "subject": "Task 4: LocalDbRegistration + Program.cs DI wiring + config defaults", "status": "pending", "blockedBy": [2, 3] },
{ "id": 5, "subject": "Task 5: Dedicated h2c sync listener + MapZbLocalDbSync (Kestrel URLs-override risk)", "status": "pending", "blockedBy": [4] },
{ "id": 6, "subject": "Task 6: IDeploymentArtifactCache + chunked LocalDb implementation + tests", "status": "pending", "blockedBy": [2] },
{ "id": 7, "subject": "Task 7: Cache write path in DriverHostActor (Props expression-tree trap)", "status": "pending", "blockedBy": [0, 6] },
{ "id": 8, "subject": "Task 8: Boot-from-cache read path + running-from-cache signal", "status": "pending", "blockedBy": [7] },
{ "id": 9, "subject": "Task 9: Delete the dormant LiteDB LocalCache subsystem", "status": "pending", "blockedBy": [0] },
{ "id": 10, "subject": "Task 10: Health check + telemetry meter allowlist", "status": "pending", "blockedBy": [4] },
{ "id": 11, "subject": "Task 11: DI-pin integration tests over the real Program.cs", "status": "pending", "blockedBy": [5, 10] },
{ "id": 12, "subject": "Task 12: Two-node convergence harness + scenarios (+ positive control)", "status": "pending", "blockedBy": [5, 6] },
{ "id": 13, "subject": "Task 13: docker-dev rig configuration (site-a pair on, site-b pin off)", "status": "pending", "blockedBy": [5] },
{ "id": 14, "subject": "Task 14: Documentation (runbook, Redundancy.md, CLAUDE.md)", "status": "pending", "blockedBy": [8] },
{ "id": 15, "subject": "Task 15: DoD sweep (offline) — STOP and report after this", "status": "pending", "blockedBy": [7, 8, 9, 11, 12, 13, 14] },
{ "id": 16, "subject": "Task 16: Live gate on the docker-dev rig (needs explicit user go-ahead)", "status": "pending", "blockedBy": [15] }
],
"lastUpdated": "2026-07-20T00:00:00Z"
}
docs/plans/2026-07-20-localdb-adoption-phase2.md
+293
View File
@@ -0,0 +1,293 @@
# OtOpcUa LocalDb Adoption — Phase 2 Implementation Plan (alarm-historian store-and-forward)
> **For Claude:** REQUIRED SUB-SKILL: Use superpowers-extended-cc:executing-plans to implement this plan task-by-task.
>
> **Execution model:** Optimized for **Claude Opus** agents (`claude --model opus`); dispatch
> `high-risk` tasks on Opus. Branch **`feat/localdb-phase2`** in `~/Desktop/OtOpcUa` (remote
> `lmxopcua`). **Prerequisite: Phase 1 (`docs/plans/2026-07-20-localdb-adoption-phase1.md`) is
> merged (or this branch is stacked on it) and its live gate passed.** Do not merge as part of this
> plan; stop at the DoD task.
>
> **Design authority:** `~/Desktop/scadaproj/docs/plans/2026-07-20-otopcua-localdb-design.md` §3.4,
> D8, D9. Reference implementation for "replace a bespoke store" phasing: ScadaBridge Phase 2
> (`~/Desktop/ScadaBridge/docs/plans/2026-07-19-localdb-adoption-phase2.md` + its live gate doc).
**Goal:** Move the alarm-historian store-and-forward buffer (today the standalone
`alarm-historian.db` owned by `SqliteStoreAndForwardSink`) into the consolidated LocalDb file as a
replicated table with a primary-gated drain, so a redundant pair no longer loses buffered alarm
history when a node dies — and delete the sink's bespoke file/connection management outright.
**Architecture:** `alarm_sf_events` (TEXT GUID PK) registered in `LocalDbSetup.OnReady`; the sink
rewired onto `ILocalDb` behind its unchanged public seam; drain gated on the delivered-snapshot
Primary role via `PrimaryGatePolicy` (at-least-once across failover, accepted and documented); a
one-time idempotent migrator from the legacy file, running **after** registration.
**Risk framing (from ScadaBridge):** Phase 2 replaces a *working* mechanism — a harder risk class
than Phase 1's "add where none existed." Cutover (delete + rewire) lands in **one commit**; there is
no dual-mechanism period, and the cutover is the test.
**Hard rules:** identical to Phase 1's list (OnReady ordering, no autoincrement/BLOB, fail-closed
auth already in place, no in-memory SQLite in tests, cp-triplet-only rig inspection), plus:
- **Legacy-copy column lists must INTERSECT** with what the legacy file actually has
(`pragma_table_info` probe) — a missing column throws and readers silently discard every row.
- Absence assertions need a positive control.
- DoD greps phrased as "no references from code" (explanatory comments may survive).
---
### Task 0: Recon (produces `docs/plans/2026-07-20-localdb-phase2-recon.md`)
**Classification:** standard
**Estimated implement time:** ~5 min
**Parallelizable with:** none
**Files:**
- Create: `docs/plans/2026-07-20-localdb-phase2-recon.md`
- Read-only: `src/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian/SqliteStoreAndForwardSink.cs` (and the
whole `Core.AlarmHistorian` project), `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Historian/AlarmHistorianOptions.cs`,
the drain worker (whatever forwards batches to `GatewayHistorian`/`SendEvent`),
`src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/PrimaryGatePolicy.cs`,
`src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs` (how `_localRole` +
driver-member-count reach the gate today)
**Record, with `file:line` citations:**
1. The sink's exact table schema: name, columns, PK type (**autoincrement?** — decides whether the
migrator needs deterministic `mig-{node}-{legacyId}` ids), indices, and any BLOB columns
(**STOP condition:** a BLOB payload column cannot be registered — it must become base64 TEXT in
the new schema, and the recon must size the largest realistic payload against the 171 KB-ish
chunk guidance; alarm events are small JSON, so expect this to be fine, but verify).
2. The public seam: the interface the drain worker and producers use (e.g. `IAlarmHistorianSink` /
enqueue+dequeue+markDelivered+deadLetter methods), so the rewire can keep it byte-compatible.
3. Semantics to preserve: `Capacity` (1,000,000) enforcement, `MaxAttempts` (10), dead-letter
retention (30 d), `BatchSize` (100), `DrainIntervalSeconds` (5) — where each lives.
4. The drain worker's lifecycle: hosted service or actor? Where a Primary-role check can be
injected, and how the delivered-snapshot role (`RedundancyStateChanged` cache) is accessible
from it (via `DriverHostActor`, a shared status service, or a message). If the role is only
available inside `DriverHostActor`, note the cleanest bridge (e.g. an `IRedundancyRoleView`
singleton the actor updates) — that becomes Task 4's shape.
5. Whether the sink is constructed per-node config path (`AlarmHistorian:DatabasePath`) anywhere
else (tests, tooling).
6. How `AlarmHistorian:Enabled=false` short-circuits (NullAlarmHistorianSink) — the rewire must
keep the disabled path allocating no LocalDb tables? No: tables are created unconditionally in
`OnReady` (cheap, empty); only the sink/drain stay Null. Note this in the doc.
Commit: `docs(localdb): phase-2 recon findings`.
---
### Task 1: `alarm_sf_events` schema + registration (+ tests)
**Classification:** standard
**Estimated implement time:** ~4 min
**Parallelizable with:** none (Task 2 depends on it)
**Files:**
- Create: `src/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian/AlarmSfSchema.cs` (depends only on
`Microsoft.Data.Sqlite`)
- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/LocalDbSetup.cs`
- Test: extend `LocalDbSetupTests`
Schema shape (adjust column names to the recon's findings — preserve today's semantics):
```sql
CREATE TABLE IF NOT EXISTS alarm_sf_events (
id TEXT NOT NULL PRIMARY KEY, -- app-minted GUID (never autoincrement)
payload_json TEXT NOT NULL,
enqueued_at_utc TEXT NOT NULL,
attempts INTEGER NOT NULL DEFAULT 0,
status TEXT NOT NULL DEFAULT 'pending', -- pending | delivered | dead
last_attempt_utc TEXT NULL,
dead_at_utc TEXT NULL
);
CREATE INDEX IF NOT EXISTS ix_alarm_sf_events_status ON alarm_sf_events(status, enqueued_at_utc);
```
`OnReady` order becomes: Phase-1 DDL → `AlarmSfSchema.Apply` → the two Phase-1
`RegisterReplicated` calls → `RegisterReplicated("alarm_sf_events")` → **migrator (Task 5) last**.
(All DDL may run before all registrations; the invariant is registration-before-writes.)
TDD: failing test first — exact replicated set becomes
`["alarm_sf_events", "deployment_artifacts", "deployment_pointer"]` (ordinal-sorted; update the
Phase-1 exact-set pins in the same commit — they are *supposed* to go red here, that's them
working). Oplog-capture test for an `alarm_sf_events` insert. Commit
`feat(localdb): alarm_sf_events replicated table`.
---
### Task 2: Rewire the sink onto `ILocalDb` + delete bespoke file management (the cutover commit, part 1 of 2 — see Task 3)
**Classification:** high-risk
**Estimated implement time:** ~5 min
**Parallelizable with:** none
**Files:**
- Modify: `src/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian/SqliteStoreAndForwardSink.cs` (or replace
with `LocalDbStoreAndForwardSink.cs` — keep the public seam identical either way)
- Modify: its registration (`AddAlarmHistorian`) to inject `ILocalDb`
- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Historian/AlarmHistorianOptions.cs` — remove
`DatabasePath` (a breaking config key removal: note it in the runbook/CHANGELOG task)
- Test: the sink's existing unit tests, rewired to a temp-file `ILocalDb` via `TestLocalDb`-style
helper (create `tests/.../TestSupport` helper if none exists — real DB, never a stub: a stubbed
bare `SqliteConnection` lacks `zb_hlc_next()` and fails closed on registered tables)
**Steps:**
1. Write/port failing tests for the seam's semantics: enqueue, drain batch of `BatchSize`,
`MaxAttempts` → dead-letter, capacity enforcement, dead-letter retention purge.
2. Implement over `ILocalDb.ExecuteAsync/QueryAsync` (anonymous-object params). Delete the private
connection/pragma/file-open code and any `PRAGMA journal_mode` calls (LocalDb owns pragmas).
GUIDs minted at enqueue (`Guid.NewGuid().ToString("N")`).
3. `Capacity` enforcement: count-based insert guard (preserve today's overflow behavior per recon).
4. Core.AlarmHistorian gains a package ref on core `ZB.MOM.WW.LocalDb` (interface only).
5. Build + project tests green. **Commit together with Task 3** if the drain gate can't compile
separately (the ScadaBridge tasks-14/15/16 circular-dependency landmine — check before assuming
they're independent commits).
---
### Task 3: Primary-gated drain
**Classification:** high-risk
**Estimated implement time:** ~5 min
**Parallelizable with:** none
**Files:** (exact shape from recon item 4)
- Modify: the drain worker
- Create (if recon says so): `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Redundancy/IRedundancyRoleView.cs`
— a singleton snapshot (`RedundancyRole? LocalRole`, `int DriverMemberCount`) updated by
`DriverHostActor` where it already caches `_localRole`
- Test: drain-worker tests + an actor test pinning that `DriverHostActor` publishes role changes to
the view
**Semantics:**
- Drain runs only when `PrimaryGatePolicy.ShouldServiceAsPrimary(localRole, driverMemberCount)` is
true — same policy, same boot-window posture (unknown role drains only when the node is alone).
- Delivered/dead-letter marks are row UPDATEs → they replicate, so the standby's copy tracks drain
progress and does not re-deliver already-marked rows after failover.
- **At-least-once across failover is accepted:** rows delivered on the old primary whose
`delivered` mark hadn't replicated yet will be re-sent by the new primary. Document in the
runbook (Task 7); do NOT build dedup.
- When replication is OFF (default), the gate still applies but `driverMemberCount` for a solo
node keeps today's behavior — verify with a test: single-node, role unknown → drains (no
regression for unpaired deployments).
TDD: failing tests — secondary role does not drain; primary drains; unknown+alone drains;
unknown+paired does not. Commit (with Task 2 if coupled):
`feat(localdb): alarm S&F on LocalDb with primary-gated drain (cutover)`.
---
### Task 4: One-time legacy migrator (`alarm-historian.db` → consolidated)
**Classification:** standard
**Estimated implement time:** ~5 min
**Parallelizable with:** Task 6
**Files:**
- Create: `src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/AlarmSfLegacyMigrator.cs`
- Modify: `LocalDbSetup.OnReady` — call it **last**, after all `RegisterReplicated` calls
- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/AlarmSfLegacyMigratorTests.cs`
Mirror `~/Desktop/ScadaBridge/src/ZB.MOM.WW.ScadaBridge.Host/SiteLocalDbLegacyMigrator.cs`:
- Source path from the pre-removal `AlarmHistorian:DatabasePath` default (`alarm-historian.db`) —
read the raw config key even though the option property is gone.
- `ShouldMigrate`: skip if file missing or `<file>.migrated` exists. `:memory:`/`file:` sources → no-op.
- **If the legacy PK is autoincrement (recon):** deterministic ids `mig-{NodeName}-{legacyId}`
(node name from the recon-identified config key) — rerunnable without duplicates under
`INSERT OR IGNORE`, and no cross-node collision. If already GUIDs, copy as-is.
- One transaction for the whole copy; `File.Move(path, path + ".migrated")` only after commit;
failure throws out of `OnReady` → boot fails, legacy untouched.
- **Column intersection** via `pragma_table_info` on the legacy table; required-PK guard.
- Both pair nodes migrate independently; their rows have distinct ids (node-prefixed), so the
merged buffer is the union — expected; note that the new primary will drain the standby's
migrated rows too.
TDD: failing tests — happy path row counts; idempotent re-run; failure leaves legacy file
untouched; migrated rows **enter the oplog** (assert `__localdb_oplog` — the registration-order
pin); older legacy file missing a column still migrates (intersection). Commit
`feat(localdb): one-time alarm-historian.db migrator`.
---
### Task 5: Convergence + failover scenarios in the pair harness
**Classification:** high-risk
**Estimated implement time:** ~5 min
**Parallelizable with:** Task 4
**Files:**
- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LocalDb/AlarmSfConvergenceTests.cs`
(reuses the Phase-1 `LocalDbPairHarness`)
**Scenarios:**
1. `AlarmBurstOnA_ConvergesToB_AndOplogDrains` — enqueue N events on A; identical rowset on B;
oplog → 0.
2. `DeliveredMarksReplicate` — mark rows delivered on A; B's copies show delivered (the
no-redeliver-after-failover property, asserted at the data layer).
3. `WritesWhileTransportDown_SurviveRejoin`.
4. Positive control: with `RegisterReplicated("alarm_sf_events")` commented out, scenarios 1–3 go
red (run locally, record, restore — do not commit red).
Commit `test(localdb): alarm S&F convergence scenarios`.
---
### Task 6: Rig config + docs
**Classification:** small
**Estimated implement time:** ~4 min
**Parallelizable with:** Task 4
**Files:**
- Modify: `docker-dev/docker-compose.yml` — enable `AlarmHistorian__Enabled=true` on the site-a
pair if not already (the gate needs a live buffer); remove any `AlarmHistorian__DatabasePath`
env vars (key deleted).
- Modify: `docs/operations/2026-07-20-localdb-pair-replication.md` — add: alarm S&F replication
semantics, at-least-once-across-failover statement, migrator behavior (`.migrated` sidecar),
`DatabasePath` key removal.
- Modify: `docs/AlarmHistorian.md` + `CLAUDE.md` — sink now lives in the consolidated LocalDb;
drain is primary-gated.
Commit `docs+chore(localdb): phase-2 rig config + docs`.
---
### Task 7: DoD sweep (offline)
**Classification:** standard
**Estimated implement time:** ~5 min
**Parallelizable with:** none
1. Full solution build → 0 warnings; full test suite green (deltas vs pre-branch baseline only).
2. Greps, phrased as "no references from **code**": the old bespoke connection management
(`AlarmHistorian:DatabasePath`, direct `new SqliteConnection` inside Core.AlarmHistorian except
via schema helpers/tests) — explanatory comments may remain.
3. Positive-control evidence from Task 5 recorded.
4. Exact-set replicated-tables pin = 3 tables, both directions.
5. Update `…phase2.md.tasks.json`; commit `chore(localdb): phase-2 DoD sweep`; STOP and report.
---
### Task 8: Live gate on the docker-dev rig (run only with explicit user go-ahead)
**Classification:** high-risk (rig)
**Estimated implement time:** ~30 min wall-clock
**Parallelizable with:** none
Same inspection rules as Phase 1's gate (cp-triplet, curl sidecar, observer-suspicion rule).
Record in `docs/plans/2026-07-20-localdb-phase2-live-gate.md`:
1. Migration ran: `.migrated` sidecar present on both site-a nodes; row counts match legacy.
2. Alarm burst (drive a real driver alarm or the historian-gateway-unreachable path) converges:
identical rowsets, oplog drains to 0, dead letters 0.
3. Only the primary drains: stop the historian gateway egress, buffer builds on BOTH nodes'
tables (replicated), but only the primary's drain worker logs attempts.
4. Failover: stop the primary; the standby (new primary) resumes draining the shared buffer;
count of double-delivered events observed and recorded (at-least-once evidence, not a failure).
5. Both-nodes-together restart clean; zero `disk I/O error`/`SQLITE_IOERR` in logs.
6. site-b (default-OFF pin): sink works locally, no sync traffic.
Commit the gate doc; report; do not merge.
---
## Task persistence
Tasks file: `docs/plans/2026-07-20-localdb-adoption-phase2.md.tasks.json`. Record deviations per
task — especially if Tasks 2/3 had to land as one commit (the expected outcome).
docs/plans/2026-07-20-localdb-adoption-phase2.md.tasks.json
+15
View File
@@ -0,0 +1,15 @@
{
"planPath": "docs/plans/2026-07-20-localdb-adoption-phase2.md",
"tasks": [
{ "id": 0, "subject": "Task 0: Recon — sink schema, seam, drain lifecycle, role-view bridge (STOP conditions)", "status": "pending" },
{ "id": 1, "subject": "Task 1: alarm_sf_events schema + registration (+ exact-set pin update)", "status": "pending", "blockedBy": [0] },
{ "id": 2, "subject": "Task 2: Rewire sink onto ILocalDb + delete bespoke file management (cutover 1/2)", "status": "pending", "blockedBy": [1] },
{ "id": 3, "subject": "Task 3: Primary-gated drain via PrimaryGatePolicy (cutover 2/2 — may co-commit with Task 2)", "status": "pending", "blockedBy": [2] },
{ "id": 4, "subject": "Task 4: One-time alarm-historian.db legacy migrator", "status": "pending", "blockedBy": [3] },
{ "id": 5, "subject": "Task 5: Convergence + failover scenarios in the pair harness (+ positive control)", "status": "pending", "blockedBy": [3] },
{ "id": 6, "subject": "Task 6: Rig config + docs", "status": "pending", "blockedBy": [3] },
{ "id": 7, "subject": "Task 7: DoD sweep (offline) — STOP and report after this", "status": "pending", "blockedBy": [4, 5, 6] },
{ "id": 8, "subject": "Task 8: Live gate on the docker-dev rig (needs explicit user go-ahead)", "status": "pending", "blockedBy": [7] }
],
"lastUpdated": "2026-07-20T00:00:00Z"
}
docs/plans/2026-07-20-localdb-phase1-recon.md
+603
View File
@@ -0,0 +1,603 @@
# LocalDb Phase 1 — Task 0 Recon Findings
**Date:** 2026-07-20
**Branch:** `feat/localdb-phase1` (based on `master`)
**Plan:** `docs/plans/2026-07-20-localdb-adoption-phase1.md`
**Design authority:** `~/Desktop/scadaproj/docs/plans/2026-07-20-otopcua-localdb-design.md`
All `file:line` citations are against the branch base commit.
---
## Step 1 — Feed verification: PASS
```
$ dotnet package search ZB.MOM.WW.LocalDb --source dohertj2-gitea --format json
ZB.MOM.WW.LocalDb 0.1.1
ZB.MOM.WW.LocalDb.Contracts 0.1.1
ZB.MOM.WW.LocalDb.Replication 0.1.1
```
All three present at **0.1.1**. No STOP. (Note: the local NuGet cache holds both `0.1.0` and `0.1.1`
— pin exactly `0.1.1` so a stale cache cannot win.)
---
## Step 3 — STOP conditions: NONE TRIGGERED
| STOP condition | Verdict |
|---|---|
| Artifact apply path cannot take a post-apply hook without restructuring the actor | **Clear** — `ApplyAndAck` has a clean insertion point (§2.2) |
| `DeploymentId` / cluster identity are not stable strings/GUIDs | **Clear** — both are stable strings, but cluster identity is **not resolvable at boot**; see D-1 |
| LiteDB LocalCache is referenced by live code | **Clear** — zero live-code references (§7) |
Three STOP conditions clear. **Eight deviations** from the plan's stated assumptions are recorded in
§9 — four of them (D-1, D-3, D-5, D-6) change the work materially.
---
## 1. Deploy fetch path
### 1.1 Blob-load sites — there are **five** `CreateDbContext()` calls, not two
| Line | Method | Purpose |
|---|---|---|
| `DriverHostActor.cs:513` | `Bootstrap()` | `NodeDeploymentStates` + `Deployments` at PreStart |
| `DriverHostActor.cs:1472` | `ReconcileDrivers(DeploymentId)` | **loads `ArtifactBlob`** |
| `DriverHostActor.cs:1543` | `PushDesiredSubscriptions(DeploymentId)` | **loads `ArtifactBlob` again** |
| `DriverHostActor.cs:2000` | `TryRecoverFromStale()` | latest `Sealed` deployment (id + hash only, no blob) |
| `DriverHostActor.cs:2029` | `UpsertNodeDeploymentState(...)` | writes `NodeDeploymentState` |
`DriverHostActor.cs:1472-1476`:
```csharp
using var db = _dbFactory.CreateDbContext();
blob = db.Deployments.AsNoTracking()
.Where(d => d.DeploymentId == deploymentId.Value)
.Select(d => d.ArtifactBlob)
.FirstOrDefault() ?? Array.Empty<byte>();
```
`:1543-1547` is byte-identical apart from the local. **The same blob is read twice per apply.**
### 1.2 Artifact runtime type — there is no instance type
`DeploymentArtifact` (`DeploymentArtifact.cs:48`) is a **static parser class** over
`ReadOnlySpan<byte>`. The blob is never materialised into an object graph.
- Storage: `byte[]` — `Deployment.cs:29` `public byte[] ArtifactBlob { get; init; } = Array.Empty<byte>();`
- Format: **UTF-8 JSON**, `JsonDocument.Parse` (`DeploymentArtifact.cs:66`, `:490`). Not MessagePack.
- Leniency contract: empty/malformed blob → empty result, **never throws** (`:62`, `:537-541`).
**Consequence for the cache:** we cache raw `byte[]`. No serializer coupling. Good.
### 1.3 Identity types
| Type | Definition | Shape |
|---|---|---|
| `DeploymentId` | `Commons/Types/DeploymentId.cs:3` — `readonly record struct DeploymentId(Guid Value)` | `ToString()` → `Value.ToString("N")`, 32 hex chars, **no hyphens** (`:10`) |
| `RevisionHash` | `Commons/Types/RevisionHash.cs:8` — `readonly record struct RevisionHash(string Value)` | SHA-256 hex, **lowercase 64 chars**, no `0x`; empty invalid (`:3-6`) |
Both are stable, filesystem/SQL-safe strings. Use `DeploymentId.ToString()` (the "N" form) as the
`deployment_id` TEXT column value.
---
## 2. Post-apply point
### 2.1 `ApplyAndAck` control flow — `DriverHostActor.cs:1413-1458`
```
1415 _applyingDeploymentId = deploymentId;
1416 Become(Applying);
1425 UpsertNodeDeploymentState(..., Applying, null)
1427 try {
1429 ReconcileDrivers(deploymentId); // blob fetch #1 — SWALLOWS DB errors
1430 _currentRevision = revision;
1431 UpsertNodeDeploymentState(..., Applied, null);
1432 SendAck(..., ApplyAckOutcome.Applied, null, correlation);
1436 _opcUaPublishActor?.Tell(RebuildAddressSpace(...));
1439 PushDesiredSubscriptions(deploymentId); // blob fetch #2 — SWALLOWS DB errors
1440 OtOpcUaTelemetry.DeploymentApplied.Add(1, "outcome"="ack");
1441 _log.Info("applied deployment ...");
1443 }
1444 catch (Exception ex) { ...Failed ACK... }
1452 finally { _applyingDeploymentId = null; Become(Steady); }
```
### 2.2 Where the cache write belongs
**Immediately after `DriverHostActor.cs:1439`**, before `:1440` — but **wrapped in its own
try/catch**. Reason: `:1432` already sent an `Applied` ACK to the coordinator. An unguarded cache
write that throws would fall into the `catch` at `:1444` and send a *second*, contradictory
`Failed` ACK for a deployment the fleet already believes is applied.
### 2.3 Paths that must NOT write to the cache
1. **`catch` at `:1444`** — any apply failure.
2. **Idempotent short-circuit at `:1402-1409`** — `HandleDispatchFromSteady` returns early with an
`Applied` ACK when `_currentRevision == msg.RevisionHash`. `ApplyAndAck` is never entered and no
blob is read.
3. **⚠ THE SILENT-DEGRADATION TRAP.** `ReconcileDrivers:1478-1483` and
`PushDesiredSubscriptions:1549-1553` catch DB failures, log a **warning**, and `return` **without
rethrowing**. `ApplyAndAck` therefore reaches `:1440` and declares success having applied **zero
drivers** when the blob load failed. Caching "whatever we read" would **persist an empty artifact
as a successful apply** — poisoning the boot-from-cache path with a config that starts no drivers.
**Mitigation (load-bearing):** the cache write must be gated on the blob having actually loaded
non-empty. Have `ReconcileDrivers` surface the loaded blob upward (a field or an out-param)
rather than re-reading it a third time, and skip the cache write when it is
`Array.Empty<byte>()`. This also removes the duplicate read noted in §1.1.
4. **`RestoreApplied` (`:1514-1528`)** — the bootstrap replay path. Restoring from central; caching
here is harmless and arguably desirable, but it is a distinct seam. **Phase 1 scope: skip it.**
5. **`TryRecoverFromStale` (`:1996-2022`)** — never reads the artifact at all (see §3.4).
---
## 3. Cold-boot path
### 3.1 PreStart — `DriverHostActor.cs:415-436`
Ctor already did `Become(Steady)` at `:411`. PreStart subscribes to topics, spawns the virtual-tag
and scripted-alarm hosts, then calls `Bootstrap()` at `:435`.
### 3.2 `Bootstrap()` — `DriverHostActor.cs:506-565` — **yes, it queries central SQL at boot**
Two queries inside one `try`: latest `NodeDeploymentState` for self (`:514-518`), then the matching
`Deployment` row (`:527-532`). Four outcomes: no prior deployments → Steady; `Applied` →
`RestoreApplied` (`:544`); `Applying` orphan → `ApplyAndAck` replay (`:549`); `Failed` → Steady.
### 3.3 **THE SEAM** — `DriverHostActor.cs:560-564`
```csharp
catch (Exception ex)
{
_log.Warning(ex, "DriverHost {Node}: ConfigDb unreachable on bootstrap; entering Stale", _localNode);
Become(Stale);
}
```
**This is the exact point where "central fetch failed at boot" is known.** The cache fallback goes
between `:562` and `:563`: attempt a local-cache load, and only `Become(Stale)` if the cache is also
a miss.
⚠ The catch also fires if the *second* query throws after the first succeeded, so `latest` is not in
scope-valid state from the catch. **The cache must be self-describing** — it must carry its own
`DeploymentId` + `RevisionHash`, because at this seam the actor has no idea what it should be
restoring. The planned `deployment_pointer` shape already satisfies this.
### 3.4 The `Stale` state — `DriverHostActor.cs:1343-1379`
`ReconnectInterval = 30s` (`:55`), retry timer started at `:1378`.
The dispatch-ignore at **`:1345-1348`** discards `DispatchDeployment` entirely — no ACK, no
buffering (contrast `Applying:599` which does `Self.Forward(msg)`). A Stale node looks like a
timeout to the coordinator. Phase 1 does **not** change this (per design §3.3: a *new* deployment
still requires central).
`RouteNodeWrite` fast-fails at `:1359-1360` with `"driver host stale (config DB unreachable)"`.
### 3.5 Pre-existing gap worth recording (not Phase 1 scope)
`TryRecoverFromStale` (`:1996-2022`) sets `_currentRevision` and marks `Applied` **without calling
`ReconcileDrivers` or `PushDesiredSubscriptions`** — documented at `:1692-1695` and `:1706`. A
Stale-recovered node reports Applied while running **zero drivers**, and
`HandleDispatchFromSteady:1402` then short-circuits on revision match. Boot-from-cache largely
neutralises this in practice, but the bookkeeping remains optimistic. **Logged as a follow-up, not
fixed here.**
### 3.6 `DbHealthProbeActor`
**No interaction with `DriverHostActor` whatsoever.** Spawned as a sibling
(`ServiceCollectionExtensions.cs:243-246`), consumed only by `OpcUaPublishActor`
(`OpcUaPublishActor.cs:105`, `:256`, `:528`, `:543-544`). `DriverHostActor` runs its own independent
30s `retry-db` timer. If running-from-cache should later influence OPC UA ServiceLevel,
`OpcUaPublishActor`'s DB-health seam is the precedent to mirror.
---
## 4. Cluster identity — **the one real design problem (see D-1)**
**There is no `ClusterId` field on `DriverHostActor` and no accessor for it.** The actor knows only
`_localNode` (`DriverHostActor.cs:61`, type `Commons.Types.NodeId`, aliased at `:28`).
`ClusterId` is resolved **per-artifact**, by `DeploymentArtifact.ResolveClusterScope(blob, nodeId)`
(`DeploymentArtifact.cs:485-516`) — it scans the artifact's `Nodes[]` array for a matching `NodeId`
and reads that element's `ClusterId`. Types: `ClusterScope` (`:46`), `ClusterFilterMode` (`:28`).
`IClusterRoleInfo` (`Commons/Interfaces/IClusterRoleInfo.cs:12`) exposes `LocalNode`, `LocalRoles`,
`HasRole`, `MembersWithRole`, `RoleLeader`, `RoleLeaderChanged` — **no `ClusterId`**.
`docker-dev/docker-compose.yml` has `Cluster__Hostname`/`Port`/`Roles`/`SeedNodes` — **no
`Cluster__ClusterId`**. The `ClusterNode` entity
(`Configuration/Entities/ClusterNode.cs:7,10`) carries both, but lives in central SQL — exactly
what is unreachable at the seam that needs it.
> **The bind:** the design keys the cache by `ClusterId` so a replicated pair shares one cache
> entry. But `ClusterId` is only discoverable *from a blob you already have* or *from central SQL*.
> At the §3.3 boot-failure seam we have neither.
**Resolution adopted (D-1):** keep `cluster_id` as the `deployment_pointer` PK — pair sharing is the
whole point of replication and must not be given up. Resolve it at each end as follows:
- **Write path** (`ApplyAndAck`, §2.2): call `DeploymentArtifact.ResolveClusterScope(blob, nodeId)`
on the blob just applied. `ClusterFilterMode.ScopeTo` → use its `ClusterId`. `None` (single-cluster
artifact) or `Suppress` → fall back to the literal `"__single"` sentinel, matching the actor's own
existing degenerate-case convention at `:1827-1830`.
- **Read path** (boot seam, §3.3): we cannot compute the key, so **do not key the read**. Select the
single `deployment_pointer` row, ordered by `applied_at_utc DESC`, `LIMIT 1`. A node belongs to
exactly one cluster and its pair peer replicates the same cluster's row, so the table holds one
row in every real topology. If more than one row is present (a node was re-homed between
clusters), take the newest and **log a warning naming both cluster ids** — an operator-visible
signal rather than a silent wrong-config boot.
- Optional escape hatch: honour a `Cluster:ClusterId` config key when present. **Deferred** — not
needed for the rig, and adding a required key contradicts the design's "storage ships
unconditionally" posture.
This preserves the replication payoff (design §3.3) and the schema exactly as specified, and
confines the change to the read query's `WHERE` clause.
---
## 5. `DriverHostActor` construction — the `Props` expression tree
### 5.1 The factory — `DriverHostActor.cs:326-346`
`static Props(...)` takes **16 parameters**, 14 optional, and forwards them **fully positionally**
into `Akka.Actor.Props.Create(() => new DriverHostActor(...))` at **`:343-346`**. Ctor at
`:375-412`, assignments `:393-408`.
> ⚠ Six `IActorRef?` parameters and three interface-typed parameters mean many mis-bindings are
> **type-compatible and therefore compile clean**. Callers use *named* arguments (safe); the
> internal forwarding at `:343-346` is *positional* (unsafe).
>
> **Rule for Task 7: append the new parameter LAST in the `Props` signature, LAST in the ctor
> signature, and LAST in the positional forwarding list at `:346`. Change nothing else.**
### 5.2 Construction sites — **27 total** (1 production + 26 test)
**Production (1):** `Runtime/ServiceCollectionExtensions.cs:332`, inside `WithOtOpcUaRuntimeActors`
(`:193`); registered as `DriverHostActorKey` (`:344`). All args named except the first three. Does
not pass `virtualTagHostOverride`, `scriptedAlarmHostOverride`, or `driverMemberCountProvider`.
`WithOtOpcUaRuntimeActors` callers: `Host/Program.cs:331`,
`Host.IntegrationTests/TwoNodeClusterHarness.cs:398`, `ServiceCollectionExtensionsTests.cs:42,144`.
**Tests (26)**, all in `tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/`:
| File | Lines |
|---|---|
| `Drivers/DriverHostActorTests.cs` | 28, 60, 81, 118, 143, 189 |
| `Drivers/DriverHostActorReconcileTests.cs` | 37, 59, 90, 119, 156, 193 |
| `Drivers/DriverHostActorDiscoveryTests.cs` | 385, 671, 748, 1023 |
| `Drivers/DriverHostActorWriteRoutingTests.cs` | 148, 208 |
| `Drivers/DriverHostActorPrimaryGateTests.cs` | 228, 247 |
| `Drivers/DriverHostActorVirtualTagTests.cs` | 44, 76 |
| `Drivers/DriverHostActorNativeAlarmTests.cs` | 350 |
| `Drivers/DriverHostActorNativeAlarmAckRoutingTests.cs` | 126 |
| `Drivers/DriverHostActorProbeResultDropTests.cs` | 66 |
| `Drivers/DriverHostActorHistoryWriterTests.cs` | 59 |
| `Drivers/DiscoveryInjectionEndToEndTests.cs` | 219 |
| `VirtualTags/DependencyMuxActorTests.cs` | 139 |
No site calls `Props.Create<DriverHostActor>()` or `new DriverHostActor(...)` directly — the only
`new DriverHostActor` in the tree is inside the factory at `:343`. Because the new parameter is
optional and appended last, **no test site should need editing.**
### 5.3 Status object for the running-from-cache signal
`NodeDiagnosticsSnapshot` (`Commons/Interfaces/NodeDiagnosticsSnapshot.cs:17-22`) — built in
`HandleGetDiagnostics` (`DriverHostActor.cs:1381-1398`), a positional record of
`(NodeId, RevisionHash? CurrentRevision, IReadOnlyList<DriverInstanceDiagnostics> Drivers, DateTime AsOfUtc)`.
`GetDiagnostics` is handled in **all three states** (`Steady:570`, `Applying:601`, `Stale:1349`), so
a flag here is observable regardless of state. **This is the natural home for `RunningFromCache`**
(append last to the record). Consumer: `IFleetDiagnosticsClient` → AdminUI.
`IDriverHealthPublisher` (`:73`, defaulted `:402`) is **not** a fit — the host never calls it; it
only passes it down to `DriverInstanceActor` children (`:1845`, `:1859`). Per-driver, not per-node.
### 5.4 Async / side-effect IO convention
The actor is **overwhelmingly synchronous-blocking on DB IO** — all five `CreateDbContext()` sites
use `using var db = ...` with synchronous LINQ on the actor thread. No `async`/`await` in the file,
no `Task.Run`.
The single async pattern is `ContinueWith(..., TaskScheduler.Default).PipeTo(replyTo)` in
`HandleRouteNodeWrite` (`:1188-1200`), with an explicit `Sender` capture before the continuation.
**Recommendation for Task 7:** a **synchronous** cache write at the §2.2 point is *consistent with
the file's existing style* and is correct there — the actor is mid-apply, holds `Applying`, and
every other DB call on that path already blocks. A fire-and-forget `PipeTo(Self)` would need a
handler registered in `Steady`, `Applying` **and** `Stale`, or it dead-letters after the
`Become(Steady)` at `:1456`. **Prefer synchronous + try/catch.** (This deviates from the plan's
"fire-and-forget"; see D-4.)
`IWithTimers` is already implemented (`:50`, `:282`); `"retry-db"` key in use at `:1378`/`:2009`.
---
## 6. Host composition root
### 6.1 Role flag — `Program.cs:47-50`
```csharp
var roles = RoleParser.Parse(Environment.GetEnvironmentVariable("OTOPCUA_ROLES"));
var hasAdmin = roles.Contains("admin");
var hasDriver = roles.Contains("driver");
```
Allowed roles: `admin`, `driver`, `dev` (`Cluster/RoleParser.cs:5-8`); unknown throws (`:25-27`).
### 6.2 `hasDriver` branch — `Program.cs:131-310`
Anchor for the new call: **`Program.cs:173-175`** `builder.Services.AddAlarmHistorian(...)`, inside
the config-gated cluster at `:154-195`. Must land **before** `AddAkka` at `:314`.
### 6.3 Pipeline — `Program.cs:368-404`
`:383` `MapStaticAssets` · `:385-396` `if (hasAdmin) { ... }` · **`:398` `MapOtOpcUaHealth()`** ·
`:399` `MapOtOpcUaMetrics()` · `:404` `RunAsync()`.
⚠ **There is no `hasDriver` block in the pipeline half** — only `hasAdmin` is branched on. A
driver-gated `app.Map*` is a new construct; it goes between `:396` and `:398`.
`Program.cs:406-410` re-exports `public partial class Program`.
### 6.4 Kestrel / URLs — **CONFIRMED: no Kestrel configuration exists**
- `Program.cs:57` `builder.WebHost.UseStaticWebAssets();` — the **only** `builder.WebHost` call.
- **Zero** `ConfigureKestrel` / `UseUrls` / `ListenAnyIP` / `UseKestrel` anywhere in `src/`.
- Binding is exclusively via `ASPNETCORE_URLS`: `docker-compose.yml:173` (central-1) and `:240`
(central-2) set `http://+:9000`; `launchSettings.json:7` sets `http://localhost:9000`.
Two traps for Task 5:
1. ⚠ **`scripts/install/Install-Services.ps1:186`** (`$hostEnv += "ASPNETCORE_URLS=..."`) is inside
`if ($hasAdmin)` at `:185`. **A driver-only Windows-service node gets no `ASPNETCORE_URLS` at
all** and binds the ASP.NET default. The plan's parse-URLs-then-rebind snippet would re-bind
`http://+:9000` there — a port that node never listened on. The `?? "http://+:9000"` fallback in
the plan's snippet is therefore **wrong for driver-only nodes**; see D-6.
2. ⚠ **`Host.IntegrationTests/TwoNodeClusterHarness.cs:331`** already calls
`builder.WebHost.UseKestrel(o => o.Listen(IPAddress.Parse(LoopbackHost), 0));`. Adding a
production `ConfigureKestrel` gives two competing explicit configurations in integration tests —
last-one-wins. The `syncPort > 0` gate keeps this dormant by default (harness sets no sync port),
but any harness that *does* set one will fight the port-0 binding.
**Health route for smoke tests:** `/healthz` — liveness, runs no checks, always 200 while the
process is up, `AllowAnonymous`. (`Health/HealthEndpoints.cs:48` → `MapZbHealth()`; routes from the
`ZB.MOM.WW.Health` package.) Also `/health/ready`, `/health/active`, and `/metrics` (`Program.cs:399`).
### 6.5 `SecretsRegistration` template — `Host/Configuration/SecretsRegistration.cs`
Shape for `LocalDbRegistration` to mirror: `public static class` in
`ZB.MOM.WW.OtOpcUa.Host.Configuration`; `public const string` section/key path constants; a
`public static bool IsXEnabled(IConfiguration)` predicate using
`GetValue(key, defaultValue: false)` (**default-deny**) that `Program.cs` can also call;
`AddOtOpcUaX(this IServiceCollection, IConfiguration)` returning `IServiceCollection`, both args
`ArgumentNullException.ThrowIfNull`-guarded; XML docs with `<remarks><para>` explaining *why* the
gate is default-deny.
Call-site precedent: `Program.cs:363` `AddOtOpcUaSecrets(...)`; predicate reused inside the `AddAkka`
lambda at `Program.cs:323`.
### 6.6 Telemetry meter allowlist — **EXISTS. Task 10 step 3 is REQUIRED, not conditional.**
`Host/Observability/ObservabilityExtensions.cs:25-38`:
```csharp
return services.AddZbTelemetry(o =>
{
o.ServiceName = "otopcua";
o.Meters = [OtOpcUaTelemetry.MeterName]; // <-- ObservabilityExtensions.cs:30
o.ActivitySources = [OtOpcUaTelemetry.ActivitySourceName];
```
`o.Meters` is a **strict allowlist** — `ZbTelemetryExtensions.cs:51-54` iterates it calling
`metrics.AddMeter(name)`; **no wildcard support**. Anything unlisted is silently not exported.
Single element today: `OtOpcUaTelemetry.MeterName` = `"ZB.MOM.WW.OtOpcUa"`
(`Commons/Observability/OtOpcUaTelemetry.cs:19`).
**Action:** add `LocalDbMetrics.MeterName` at `ObservabilityExtensions.cs:30`. This is exactly the
omission the ScadaBridge live gate caught.
### 6.7 Health registration — `Host/Health/HealthEndpoints.cs:19-41`
`AddOtOpcUaHealth()` **takes no parameters** and is called **unconditionally** at `Program.cs:365`,
outside both role blocks. Three `.AddTypeActivatedCheck<>` calls (`configdb`, `akka`,
`admin-leader`); **no registration-time role gating exists** — `ActiveNodeHealthCheck` does runtime
role scoping instead (returns `Healthy` when the node lacks the role).
⚠ **There are zero `IHealthCheck` implementations in this repo** — all three come from the
`ZB.MOM.WW.Health.*` packages (`Directory.Packages.props:124-126`, v0.1.0). A LocalDb check would be
the **first health-check class in the tree**; `Host/Health/` holds only `HealthEndpoints.cs` today.
**Consequence for Task 10:** driver-gating the new check requires changing
`AddOtOpcUaHealth()`'s signature (add `bool hasDriver` or `IConfiguration`) and its `Program.cs:365`
call site. **Preferred alternative:** follow the `ActiveNodeHealthCheck` precedent — register
unconditionally and have the check return `Healthy` when LocalDb is not registered. That keeps the
signature and matches the "default-OFF must not degrade a plain node" requirement for free. See D-7.
### 6.8 Config files
| File | Top-level sections |
|---|---|
| `appsettings.json` | `Serilog`, `Security`, `Secrets`, `ServerHistorian`, `ContinuousHistorization`, `AlarmHistorian`, `Deployment` |
| `appsettings.driver.json` / `.admin.json` / `.admin-driver.json` / `.Development.json` | `Serilog`, `Security` only |
Overlay selection: `Program.cs:67` sorts roles ordinally and joins with `-` → `admin,driver`
becomes `appsettings.admin-driver.json`, added `optional: true` at `:70`; env vars (`:77`) and CLI
args (`:78`) are re-appended after, so deployment overrides outrank the overlay.
**`LocalDb` key search: CONFIRMED ABSENT.** Case-insensitive `localdb` across all `*.json`,
`*.yml`, `*.ps1`, `*.csproj`, `*.props` matches only the two Phase-1/Phase-2 planning artifacts. No
conflicting keys in any overlay or in `Install-Services.ps1`.
**Placement convention:** each feature section opens with a `"_comment"` string then `"Enabled":
false`; per-field caveats use sibling `"_<Field>Comment"` keys (`appsettings.json:34`, `:45`). Place
`LocalDb` between `AlarmHistorian` (ends `:61`) and `Deployment` (`:62`).
---
## 7. LiteDB LocalCache — dormant, safe to delete
### 7.1 Files (6) under `src/Core/ZB.MOM.WW.OtOpcUa.Configuration/LocalCache/`
`ILocalConfigCache.cs`, `GenerationSnapshot.cs`, `StaleConfigFlag.cs`, `LiteDbConfigCache.cs`
(also declares `LocalConfigCacheCorruptException` at `:128`), `GenerationSealedCache.cs`,
`ResilientConfigReader.cs`. Single namespace `...Configuration.LocalCache`. The only two
`using LiteDB;` statements in `src/` are `LiteDbConfigCache.cs:1` and `GenerationSealedCache.cs:1`.
### 7.2 Reference classification — **no live-code references. NO STOP.**
- **(c) live code elsewhere: ZERO.** Also confirms full dormancy: `SealedBootstrap` (referenced by
`docs/v2/v2-release-readiness.md:46`) has **zero hits** in `src/` and `tests/` — it no longer exists.
- Two benign near-misses:
- `Configuration/Services/ILdapGroupRoleMappingService.cs:14` — the string `ResilientConfigReader`
inside an **XML doc comment**. Prose only. Needs a comment edit or the doc goes stale.
- `Configuration/ZB.MOM.WW.OtOpcUa.Configuration.csproj:29` — `<PackageReference Include="LiteDB"/>`,
the only one in the repo.
- Dozens of `LocalConfigCacheCorruptException` hits under `src/**/bin/**/*.xml` are **generated doc
build artifacts** (`GenerateDocumentationFile=true`), not source.
### 7.3 ⚠ Test-file drift — the plan's delete list is incomplete
Plan `…phase1.md:484-485` names only `GenerationSealedCacheTests.cs` and
`ResilientConfigReaderTests.cs`. Actual test files in
`tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/`:
| File | Status |
|---|---|
| `GenerationSealedCacheTests.cs` | in plan ✅ |
| `ResilientConfigReaderTests.cs` | in plan ✅ — note `StaleConfigFlagTests` is co-located here at `:323` |
| **`LiteDbConfigCacheTests.cs`** | **MISSING from the plan** — will fail to compile if the source is deleted (`:125` calls `new LiteDB.LiteDatabase(...)` directly) |
See D-3.
### 7.4 Package removal
- `PackageReference`: `Configuration.csproj:29` — only consumer.
- `PackageVersion`: `Directory.Packages.props:35` — `LiteDB` `5.0.21`.
Both safe to delete; no transitive consumer.
---
## 8. Test projects, rig, packages
### 8.1 Test project layout
| Question | Answer |
|---|---|
| `tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/` | **DOES NOT EXIST** — there is no Host *unit*-test project at all |
| Closest Host project | `Host.IntegrationTests` — despite the name it carries plenty of pure unit tests (`LdapOptionsValidatorTests`, `ServerHistorianOptionsValidatorTests`, `ResilienceInvokerFactoryRegistrationTests`, …) |
| `tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/` | **EXISTS** |
| DriverHostActor tests | `Runtime.Tests/Drivers/` — 11 `DriverHostActor*Tests.cs` files. **xunit v2** (`xunit` 2.9.3), references **`Akka.TestKit.Xunit2`** ✅ |
| `Host.IntegrationTests` xunit version | **xunit.v3** 1.1.0; SDK `Microsoft.NET.Sdk.Web` |
`Directory.Packages.props` documents that AdminUI.Tests / ControlPlane.Tests / **Runtime.Tests** are
the three xunit-v2 holdouts, held there by `Akka.TestKit.Xunit2` (verified 2026-07-13: no xunit.v3
Akka TestKit exists). This matches the design's "actor tests live in an xunit2 TestKit project".
### 8.2 ⚠ `WebApplicationFactory<Program>` is **deliberately not used** — Task 11 must be rewritten
The only mention of the type in the entire `tests/` tree is a comment explaining why it is avoided:
`Host.IntegrationTests/TwoNodeClusterHarness.cs:48`
```
/// <para>Why not <c>WebApplicationFactory&lt;Program&gt;</c>? Program.cs reads <c>OTOPCUA_ROLES</c> ...
```
The actual pattern builds the host directly — `TwoNodeClusterHarness.cs:329`:
```csharp
var builder = WebApplication.CreateBuilder(new WebApplicationOptions { Args = [] });
```
`Microsoft.AspNetCore.Mvc.Testing` is still referenced (csproj `:15`) but only for transitive
test-host plumbing. **Copy `TwoNodeClusterHarness`, not `WebApplicationFactory`.** See D-5.
`Host.IntegrationTests` also has its own `docker-compose.yml` (sql `14331`, ldap `3894`) gated by
`DockerFixtureAvailability.cs`.
### 8.3 docker-dev rig topology — plan's site-a/site-b assumption is **correct**
Single Akka mesh (`otopcua`), seeded by `central-1`. Tenant separation is by `ServerCluster.ClusterId`
rows (MAIN / SITE-A / SITE-B) inside **one shared ConfigDb** — not separate meshes.
| Service | Roles | Cluster |
|---|---|---|
| `central-1`, `central-2` | `admin,driver` | MAIN |
| `site-a-1`, `site-a-2` | `driver` | SITE-A |
| `site-b-1`, `site-b-2` | `driver` | SITE-B |
| `sql`, `migrator`, `cluster-seed`, `traefik` | — | infra |
**There is no admin-only service** — the "admin" nodes are fused `admin,driver`. All six host nodes
share the `&otopcua-host` YAML anchor.
⚠ **NO HOST NODE HAS A WRITABLE VOLUME.** The only `volumes:` in the file are `sql` →
`otopcua-mssql-data`, plus two read-only binds (`./seed:ro`, `./traefik-dynamic.yml:ro`). All six
nodes write to the container's ephemeral layer, destroyed on recreate. This is already an accepted
pattern (per-container `Secrets:SqlitePath=otopcua-secrets.db`, replicated over Akka rather than
persisted). **Task 13 must add six named volumes** — there is nothing to piggyback on. See D-8.
**Env style: confirmed double-underscore `Section__Key`**, with array indices and dictionary keys as
a third segment (`Cluster__SeedNodes__0`, `Security__Ldap__GroupToRole__ReadOnly`). Exceptions that
are plain SCREAMING_SNAKE (read directly, not config-bound): `OTOPCUA_ROLES`,
`ZB_SECRETS_MASTER_KEY`, `GALAXY_MXGW_API_KEY`, `OTOPCUA_CONFIG_CONNECTION`, `ASPNETCORE_URLS`.
**Ports.** Host-published: `14330`→sql, `4840`-`4845`→the six nodes' OPC UA, `9200`→traefik web,
`8089`→traefik dashboard. Container-internal on every node: `4053` (Akka remoting), `9000`
(Kestrel), `4840` (OPC UA). Also reserved on this machine: `14331`/`3894` (Host.IntegrationTests
compose), `80`/`8080` (sibling scadabridge/scadalink stack), `10.100.0.35:3893` (shared GLAuth).
**`9001` (the plan's choice) is free** container-internal and need not be published. ✅
### 8.4 Package versions — `Directory.Packages.props`
| Package | Version | Line |
|---|---|---|
| `Grpc.Core.Api` | 2.76.0 | 32 |
| `Grpc.Net.Client` | 2.76.0 | 33 |
| **`Grpc.AspNetCore`** | **NOT PRESENT** | — |
| `Google.Protobuf` | 3.34.1 | 31 |
| `Microsoft.Data.Sqlite` | 10.0.7 | ~55 |
| `SQLitePCLRaw.bundle_e_sqlite3` | 2.1.12 | ~108 |
| `LiteDB` | 5.0.21 | 35 |
Both Grpc floors and the Protobuf floor are already satisfied — **no bumps needed**, only the new
`Grpc.AspNetCore` entry (use 2.76.0 to match the train; watch for NU1605/NU1608 if it wants a
Protobuf newer than the pinned 3.34.1).
⚠ **`SQLitePCLRaw` is a surgical direct pin, not a transitive one.**
`CentralPackageTransitivePinningEnabled` is deliberately **off** (it breaks the Roslyn version
split). The 2.1.12 pin is promoted by a **direct `PackageReference` in `Core.AlarmHistorian`**
(`Core.AlarmHistorian.csproj:15` + `:21`) — today the only Sqlite consumer in `src/`.
> **Any new project that pulls `Microsoft.Data.Sqlite` MUST also take a direct
> `<PackageReference Include="SQLitePCLRaw.bundle_e_sqlite3"/>`,** or it silently resolves the
> vulnerable 2.1.11 native bundle. This applies to **Host** and **Runtime** (both gain `ZB.MOM.WW.LocalDb`,
> which depends on `Microsoft.Data.Sqlite`) and to every test project that builds a real `ILocalDb`.
> Copy the `Core.AlarmHistorian.csproj:15,21` pair exactly. The plan's Task 1 mentions
> `SQLitePCLRaw.lib.e_sqlite3` "if the audit flags it" — the correct package is
> **`bundle_e_sqlite3`**, and it is **required, not conditional**.
---
## 9. Deviations from the plan
Recorded per the tasks.json `deviation` convention. **D-1, D-3, D-5, D-6 change the work
materially.**
| # | Plan says | Reality | Resolution |
|---|---|---|---|
| **D-1** | Cache keyed by `ClusterId`; "cluster identity" is a recon item with a STOP condition | `ClusterId` is stable, but **not resolvable at the boot seam** — it lives inside the artifact or in the unreachable central DB (§4) | Keep `cluster_id` as PK (pair sharing is the point). **Write:** resolve via `DeploymentArtifact.ResolveClusterScope`, `"__single"` sentinel for the degenerate case. **Read:** unkeyed `ORDER BY applied_at_utc DESC LIMIT 1`, warn if >1 row. No STOP. |
| **D-2** | Cache write "fire-and-forget (`PipeTo`-style or guarded `Task.Run`)" | The actor has **no async DB IO at all**; a `PipeTo(Self)` needs handlers in all three states or it dead-letters after `Become(Steady)` (§5.4) | Synchronous write in its own try/catch at `:1439`. Matches the file's style; failure still cannot fail the apply. |
| **D-3** | Task 9 deletes 2 test files | **3** test files reference the subsystem — `LiteDbConfigCacheTests.cs` is missing from the list (§7.3) | Delete all three. Also fix the stale XML-doc mention at `ILdapGroupRoleMappingService.cs:14`. |
| **D-4** | Cache the artifact after a successful apply | `ReconcileDrivers`/`PushDesiredSubscriptions` **swallow** DB errors, so a "successful" apply can have loaded an **empty blob** and started zero drivers (§2.3) | Gate the write on a non-empty blob; surface the blob from `ReconcileDrivers` instead of a third read. **Load-bearing — without it the cache can be poisoned with an empty config.** |
| **D-5** | Task 11 uses `WebApplicationFactory<Program>` | Deliberately **not used** in this repo; `TwoNodeClusterHarness.cs:48` documents why (Program.cs reads `OTOPCUA_ROLES` from the environment) (§8.2) | Rewrite Task 11 against the `TwoNodeClusterHarness` direct-host-build pattern. |
| **D-6** | Task 5 re-binds `ASPNETCORE_URLS ?? "http://+:9000"` | `Install-Services.ps1:185-186` sets `ASPNETCORE_URLS` **only when `hasAdmin`** — the fallback would bind a driver-only node to a port it never served (§6.4) | Re-bind only when a URL is actually configured; when absent, add **only** the sync listener. Also note `TwoNodeClusterHarness.cs:331` already calls `UseKestrel(Listen(…, 0))`. |
| **D-7** | Task 10 driver-gates the health check | `AddOtOpcUaHealth()` takes no args and is called unconditionally at `Program.cs:365`; no registration-time gating exists anywhere (§6.7) | Register unconditionally; return `Healthy` when LocalDb is absent — the `ActiveNodeHealthCheck` precedent. Avoids a signature change and satisfies "default-OFF must not degrade a plain node". |
| **D-8** | Task 13 "existing data mount" | **No host node has any writable volume** (§8.3) | Add six named volumes. Note: `9001` is free container-internal. Meter allowlist (§6.6) is **required**, not conditional. |
### Follow-ups logged, not fixed here
1. **`TryRecoverFromStale` marks `Applied` without reconciling drivers** (§3.5) — pre-existing,
documented in-code at `:1692-1695`. Out of Phase 1 scope.
2. **Duplicate blob read per apply** (`:1472` and `:1543`, §1.1) — D-4's fix removes one of them as a
side effect.
3. **`Install-Services.ps1` gives driver-only nodes no `ASPNETCORE_URLS`** (§6.4) — pre-existing;
D-6 works around it rather than fixing it.