docs(plans): low-findings remediation plan — 21 review Lows + register row 103 as 5 phases / 15 model-matched tasks
This commit is contained in:
Joseph Doherty
@@ -0,0 +1,357 @@
|
||||
# Low-Findings Remediation Plan — 2026-08-19 Arch Review
|
||||
|
||||
Scope: the **21 Low** findings left open after the High wave (`c4d3c8b8..d18f344e`) and the
|
||||
Medium/Med-High wave (`d18f344e..9b67c7cb`, deployed to the rig), **plus register row 103**
|
||||
(the two TTL-bounded lesser instances of P6-6, deferred by the Medium wave to "next
|
||||
CentralUI hardening pass" — that pass is Phase 4 here). Baseline: `main @ 9b67c7cb`.
|
||||
|
||||
Structured as **five phases, one per subsystem section**. All 15 tasks are file-disjoint
|
||||
across the whole plan, so phases may run concurrently under the usual rolling dispatch —
|
||||
the phase grouping is thematic (one reviewer lens per phase at integration), not a
|
||||
dependency ordering. The one true ordering constraint: T5 (LocalDb 0.3.3) must publish
|
||||
before the pin bump lands on main.
|
||||
|
||||
**Model assignment rule** (same as the Medium wave): **Opus** for tasks whose defect is a
|
||||
lifecycle/ordering race, a state-machine gap, or a cross-repo library invariant — anywhere
|
||||
the fix can be subtly wrong while tests pass. **Sonnet** for pattern-mirroring, mechanical
|
||||
guards, index/migration work, docs, and test hygiene where the correct pattern already
|
||||
exists in-repo and the task is to copy it faithfully.
|
||||
|
||||
Execution model (unchanged): one dedicated **manual git worktree** per task (memory rule:
|
||||
`isolation:worktree` is broken here), model-matched subagent, **pathspec commits**,
|
||||
cherry-pick onto main, combined build + affected suites, `git diff` review, single merge
|
||||
session.
|
||||
|
||||
## Shared constraints (every task)
|
||||
|
||||
- **Proof of failure:** every new test must be shown to fail against the unfixed code
|
||||
(stash/unstash or targeted revert) before the fix lands. Where the defect is
|
||||
timing-dependent and won't reproduce at test sizes, do what T1 did in the Medium wave:
|
||||
inject a modeled delay to prove the mechanism, plus a timing-independent invariant test,
|
||||
and record the honesty note in the test's doc comment.
|
||||
- **No new sleep-then-assert** (register row 40): `AssertHoldsFor` / event-based waits;
|
||||
Watch/Stop/ExpectTerminated on any ticker ≤1s (rows 82/95).
|
||||
- **EF migrations:** build first, never `--no-build`; every migration ships the
|
||||
handwritten-SQL twin under `docs/plans/sql/` with parity (Medium-wave T7 precedent).
|
||||
- **Additive-only** message/DTO/proto evolution; no proto changes are expected in this
|
||||
plan at all.
|
||||
- **Register and CLAUDE.md edits happen at integration only**, never by task agents
|
||||
(file contention). Integration closes row 103 (T12) and applies the P7-6 doc decision.
|
||||
- Verifier confidence is quoted from the review: findings marked *plausible-needs-
|
||||
verification* (P3-4, P7-5) require the agent to **confirm the defect first** and report
|
||||
honestly if it does not reproduce — a guard test still lands, but the register note
|
||||
must say what was and wasn't proven.
|
||||
|
||||
## Decisions made in this plan (owner may override)
|
||||
|
||||
1. **P7-6 takes the behavior fix, not the docs fix:** bulk `DeploySiteAsync` gains an
|
||||
up-to-date pre-filter via `IStaleInstanceProbe` (skipped instances reported in the
|
||||
existing `SkippedInstances` list with reason "up-to-date"), plus an explicit `Force`
|
||||
flag (management command + CLI `deploy site --force`) that restores today's
|
||||
unconditional behavior. Rationale: an unconditional redeploy **wipes static overrides
|
||||
and native alarm state on unchanged instances** — that destructive side effect, not
|
||||
the wording, is the hazard; and CLAUDE.md already promises "out-of-date" semantics.
|
||||
2. **P4-6 takes the real fix, not the comment deletion:** run the `auto_vacuum`
|
||||
pragma before `LocalDbSchema.EnsureCreated` via a documented pre-create hook/ordering
|
||||
in `SiteLocalDbSetup`, with `VACUUM` fallback explicitly NOT attempted (too costly on
|
||||
a 1 GB live file); if ordering proves impossible without a LocalDb API change, fall
|
||||
back to deleting the misleading comment + register row, and say so.
|
||||
3. **P4-7 ships as LocalDb 0.3.3** (wire-compatible, no schema change), published to the
|
||||
Gitea feed and pinned here in the same integration session — same flow as 0.3.2.
|
||||
|
||||
---
|
||||
|
||||
## Phase 1 — Site Runtime / DCL / Communication (streams)
|
||||
|
||||
### T1 (Sonnet) — P1-4: projected single-row reads in DeploymentManagerActor
|
||||
|
||||
**Files:** `src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/DeploymentManagerActor.cs`
|
||||
(:1573, :2061, :1489), `Services/SiteStorageService.cs` (:84), its interface, tests.
|
||||
|
||||
`HandleEnsureInstanceStarted`, `HandleDeploymentStateQuery`, and `HandleEnable` each call
|
||||
`GetAllDeployedConfigsAsync` — materializing ALL configs **including ConfigJson blobs** —
|
||||
to `FirstOrDefault` one row. A volume-restore reconcile healing hundreds of instances does
|
||||
hundreds of full-table reads; the sweep tick needs only `(name, revision, is_enabled)`.
|
||||
**Fix:** add `GetDeployedConfigAsync(name)` (single row) and a metadata-only projection
|
||||
for the sweep; switch the three call sites; leave semantics identical (T1 of the Medium
|
||||
wave already threads `StoreGenerationAtRead` here — do not disturb the generation fencing;
|
||||
capture the generation at the same point the full-table read did). Tests: projection
|
||||
returns identical decisions to the old path; generation capture still fences (existing
|
||||
tests keep passing).
|
||||
|
||||
### T2 (Opus) — P2-5 + P2-6: DataConnectionActor probe seeding + Connecting-state gap
|
||||
|
||||
**File:** `src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Actors/DataConnectionActor.cs`
|
||||
(+ `Adapters/OpcUaDataConnection.cs` read-only for the `_disconnectFired` trace), tests.
|
||||
|
||||
- **P2-5:** initial subscribe (:984) and reconnect resubscribe (:2329-2344) call
|
||||
`SeedTagsAsync`; the resolution-probe path (:1975-1988) does not. OPC UA masks it via
|
||||
the initial notification; MxGateway advise can be `VT_EMPTY`, so a late-resolving
|
||||
static tag stays Uncertain forever. **Fix:** seed newly probe-resolved tags — same
|
||||
call the other two paths make.
|
||||
- **P2-6:** `Connecting` has no `AdapterDisconnected` case → Unhandled. A connect-then-
|
||||
immediate-drop can enqueue the disconnect before `ConnectResult(true)`; the adapter's
|
||||
once-only `_disconnectFired` guard (OpcUaDataConnection.cs:486) means no later
|
||||
`Disconnected` ever arrives, so a zero-subscription connection sits Connected on a dead
|
||||
adapter. **Fix:** stash (or handle) `AdapterDisconnected` in Connecting so it is
|
||||
replayed/processed after the state transition; respect the Medium-wave T3
|
||||
`AdapterGeneration` fencing (a stale generation's disconnect must still be dropped).
|
||||
Opus because the fix sits inside the Become/Stash machine next to fresh generation
|
||||
logic. Tests: ordered enqueue disconnect-before-ConnectResult ends Disconnected/
|
||||
reconnecting, not Connected; stale-generation disconnect still ignored.
|
||||
|
||||
### T3 (Opus) — P3-4 + P3-5: stream open-attempt deadline + duplicate-subscribe cap order
|
||||
|
||||
**Files:** `src/ZB.MOM.WW.ScadaBridge.Communication/Actors/SiteAlarmAggregatorActor.cs`
|
||||
(:800, :444), `Grpc/SiteStreamGrpcServer.cs` (:368-385), tests.
|
||||
|
||||
- **P3-4 (plausible-needs-verification):** if response headers take >10s
|
||||
(`ConnectedHeaderTimeout`) and an alarm-only site stays quiet,
|
||||
`GrpcAlarmStreamConnected` never fires; `_streamDown` stays true but `_openInFlight`
|
||||
stays pinned, so `OnReconcileTick`'s reopen is skipped until the 4h stream lifetime
|
||||
cycles. Mitigated by the server header flush — **verify first** whether the starvation
|
||||
is still reachable; regardless, add an **open-attempt deadline** that clears
|
||||
`_openInFlight` so reconcile can retry (self-healing beats reachability analysis).
|
||||
- **P3-5:** duplicate-subscribe cancels the incumbent stream (:368-379) **then** checks
|
||||
the cap (:385): at cap, a reconnect kills the healthy old stream and refuses the
|
||||
replacement (`ResourceExhausted`) → zero streams until retry. **Fix:** treat the
|
||||
duplicate case as a replacement (exclude it from the cap check) or check the cap before
|
||||
cancelling; must not regress the newest-wins semantics. Tests: at-cap reconnect ends
|
||||
with exactly one live stream; cap still refuses a genuinely new subscriber.
|
||||
|
||||
## Phase 2 — LocalDb / Site Event Log
|
||||
|
||||
### T4 (Sonnet) — P4-5 + P4-6: purge startup yield + auto_vacuum reality
|
||||
|
||||
**Files:** `src/ZB.MOM.WW.ScadaBridge.SiteEventLogging/EventLogPurgeService.cs`
|
||||
(:169, :208-212, :516-519), `SiteEventLogSchema.cs` (:42-46),
|
||||
`src/ZB.MOM.WW.ScadaBridge.Host/SiteLocalDbSetup.cs`, tests.
|
||||
|
||||
- **P4-5:** `ExecuteAsync` runs the initial `RunPurge()` synchronously before its first
|
||||
await, so `StartAsync` blocks every later hosted service (including `AkkaHostedService`)
|
||||
behind the purge — ~17s ceiling-limited, ~285s unpaced worst case; a throwing gate is
|
||||
also treated as active (:208-212). **Fix:** `await Task.Yield()` first (and make the
|
||||
gate's throw path fail closed with a logged fallback, not "active"). Test: hosted-service
|
||||
start returns promptly with a slow purge stubbed in.
|
||||
- **P4-6:** `auto_vacuum=INCREMENTAL` is only honored on a table-less database, and
|
||||
`LocalDbSchema.EnsureCreated` runs first — so the consolidated file is permanently
|
||||
`auto_vacuum=NONE` and the per-trim `PRAGMA incremental_vacuum` is a silent no-op while
|
||||
the adjacent comment claims otherwise (the standalone `auditlog.db` does it right,
|
||||
`SqliteAuditWriter.cs:154-157`). **Fix per plan decision 2:** issue the pragma before
|
||||
any table exists (pre-`EnsureCreated` ordering in `SiteLocalDbSetup`); if that requires
|
||||
a LocalDb API change, do NOT make one in this task — delete the trap comment, log a
|
||||
one-time Information note that vacuum is unavailable on legacy files, and report for a
|
||||
register row. Note either way: existing rig files already have tables, so the pragma
|
||||
only helps fresh files — document that in the code comment honestly. Test: fresh file
|
||||
reports `PRAGMA auto_vacuum = 2`.
|
||||
|
||||
### T5 (Opus) — P4-7: LwwApplier latent hazards → LocalDb 0.3.3 (cross-repo)
|
||||
|
||||
**Files (scadaproj):** `~/Desktop/scadaproj/ZB.MOM.WW.LocalDb/src/.../Internal/LwwApplier.cs`
|
||||
(:95-99, :273), `SqliteLocalDb.cs` (RegisterReplicated), `Directory.Build.props`
|
||||
(0.3.2 → 0.3.3), tests. **Files (here, at integration):** `Directory.Packages.props` pin.
|
||||
|
||||
- **Hazard 1:** `INSERT OR REPLACE`'s implicit delete fires FK `ON DELETE CASCADE` with
|
||||
capture suppressed (`foreign_keys=ON`) → silent divergence for any future FK'd
|
||||
replicated table (none of the ten today — verified by the review). **Fix:**
|
||||
`RegisterReplicated` gains an FK guard — refuse (clear exception) to register a table
|
||||
that is the parent of an FK with cascading delete from another table, and document the
|
||||
constraint in the LocalDb README.
|
||||
- **Hazard 2:** the tombstone apply path runs `DeleteAsync` + `WriteRowVersionAsync` as
|
||||
two statements under a poison catch — a failure between them leaves a deleted row with
|
||||
a stale live ledger (SnapshotStreamer warns+skips). **Fix:** make the pair atomic
|
||||
(single transaction), preserving the poison-catch semantics around the transaction.
|
||||
|
||||
Wire-compatible, no schema change, no ScadaBridge code change. Publish 0.3.3 via
|
||||
`build/push.sh` (env-held `GITEA_NUGET_SOURCE`/`GITEA_NUGET_KEY`), bump the three-package
|
||||
pin here, restore/build 0/0. Opus: replication-library invariants where a wrong fix
|
||||
diverges pairs silently. Tests: FK-parent registration throws; injected failure between
|
||||
delete and ledger write leaves both or neither. Integration also updates the umbrella
|
||||
`scadaproj/CLAUDE.md` index (0.3.3 entry) per the cross-repo propagation rule.
|
||||
|
||||
## Phase 3 — Central Outbox / Audit / KPI
|
||||
|
||||
### T6 (Sonnet) — P5-3 + P5-4: Notifications list index + operator-path status guard
|
||||
|
||||
**Files:** `src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase/Repositories/NotificationOutboxRepository.cs`
|
||||
(:192-203, :280-294), a new EF migration + `docs/plans/sql/` twin,
|
||||
`src/ZB.MOM.WW.ScadaBridge.NotificationOutbox/NotificationOutboxActor.cs`
|
||||
(:1088-1129, :1172-1213), tests.
|
||||
|
||||
- **P5-3:** the list page sorts `CreatedAt DESC, NotificationId DESC` but every index
|
||||
leads elsewhere → the default page is a 2× whole-table scan/sort (COUNT + page).
|
||||
**Fix:** add `IX_Notifications_Created_NotificationId (CreatedAt DESC, NotificationId
|
||||
DESC)` — exact mirror of the Medium wave's `IX_SiteCalls_Created_TrackedOperationId`
|
||||
(T7), including the VarChar-typed `FromSqlRaw` parameter check and the handwritten SQL.
|
||||
- **P5-4:** operator Retry/Discard is read-check-write keyed on `NotificationId` only;
|
||||
detached `PipeTo` tasks interleaving with the dispatcher can stamp a `Delivered` row
|
||||
`Discarded` with stale fields. **Fix:** add a `Status == Parked` predicate to the
|
||||
operator-path `ExecuteUpdate` (conditional update, rows-affected checked — the CAS
|
||||
shape T-P6-1 used); surface "already progressed" to the caller instead of silent
|
||||
overwrite. Tests: migration parity; Retry/Discard against a row that just went
|
||||
Delivered is a no-op with the honest reply.
|
||||
|
||||
### T7 (Sonnet) — P5-5: cross-validate the rate-fold interval triple
|
||||
|
||||
**Files:** `src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase/Repositories/KpiHistoryRepository.cs`
|
||||
(:169-172), the options classes' validators (`KpiHistoryOptions`, `SiteCallAuditOptions`,
|
||||
the outbox `DeliveredKpiWindow` owner), tests.
|
||||
|
||||
Rate classification silently assumes `DeliveredKpiWindow` (1 min) ==
|
||||
`SiteCallAuditOptions.KpiInterval` (1 min) == `KpiHistoryOptions.SampleInterval` (60s);
|
||||
widening `DeliveredKpiWindow` to 15 min makes rollups over-count ~15×. **Fix (pick the
|
||||
robust half):** compute the fold against `SampleInterval` directly where possible, and
|
||||
add eager cross-validation (ValidateOnStart, the §1.5 pattern) that fails boot with a
|
||||
message naming all three keys when they diverge — do not leave a validator-only fix if
|
||||
the computation can simply stop depending on the equality. Tests: divergent config fails
|
||||
validation with the three keys named; fold correct at a non-default SampleInterval.
|
||||
|
||||
### T8 (Sonnet) — P5-6: deterministic synthetic ReconciliationAbandoned rows
|
||||
|
||||
**File:** `src/ZB.MOM.WW.ScadaBridge.AuditLog/Central/SiteAuditReconciliationActor.cs`
|
||||
(:312-336, :362-364), tests.
|
||||
|
||||
After `MaxPermanentInsertAttempts` removal, the cursor held at `since` by a co-batch
|
||||
unresolved row re-pulls the abandoned row; its attempt counter restarts and a **new**
|
||||
synthetic `ReconciliationAbandoned` row is minted with a fresh `EventId` every 5-tick
|
||||
cycle. **Fix:** make the synthetic `EventId` deterministic — derived from the lost row's
|
||||
`EventId` (e.g. namespaced GUID) — so re-abandonment is idempotent under the existing
|
||||
insert-if-not-exists ingest; keep an in-memory abandoned-set only as a cheap short-circuit,
|
||||
never as the correctness mechanism (it dies with the singleton). Tests: same lost EventId
|
||||
abandoned twice yields one row; different lost rows yield distinct synthetic ids.
|
||||
|
||||
## Phase 4 — Central UI / Management / Deployment (the "CentralUI hardening pass")
|
||||
|
||||
### T9 (Sonnet) — P6-10: strict enum parsing in ManagementActor (row-88 class)
|
||||
|
||||
**File:** `src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs`
|
||||
(:2922, :2937, :2854, :2867 — alarm trigger type, ParseDataType, ParseElementDataType),
|
||||
tests.
|
||||
|
||||
Raw `Enum.Parse` accepts ordinals ("2") and undefined values ("99" → deploy-time bomb);
|
||||
a typo surfaces as a masked "internal error". **Fix:** apply the `ParseDeploymentStatus`
|
||||
guard pattern (:2745) to all three parse sites — names only, defined values only, honest
|
||||
error naming the field and the valid set. Mind the Medium-wave T8 nuance: `Enum.TryParse`
|
||||
accepts comma-joined name lists and bare ordinals; the guard must reject both (reuse or
|
||||
mirror `ParseEnumListStrict`'s comma-token rejection from Commons where a single value is
|
||||
expected). Tests: ordinal, undefined, comma-joined, and typo inputs all rejected with the
|
||||
field-naming error; every defined name accepted.
|
||||
|
||||
### T10 (Sonnet) — P6-11: request-generation guard on Deployments page
|
||||
|
||||
**File:** `src/ZB.MOM.WW.ScadaBridge.CentralUI/.../Deployments.razor` (:428, :500-552),
|
||||
bUnit tests.
|
||||
|
||||
Three fetch initiators (search, paging, timer) interleave at awaits and a stale flight
|
||||
overwrites a newer grid; `_currentPage = 1` is also written on the Timer thread (:428).
|
||||
**Fix:** a monotonic request generation captured at fetch start and checked at apply
|
||||
(last-write-wins by generation, not by completion order); marshal the timer's state
|
||||
writes through `InvokeAsync`. Tests: delayed stale response arriving after a newer one
|
||||
does not overwrite; timer refresh does not reset the page mid-search.
|
||||
|
||||
### T11 (Sonnet) — P6-12: dispose sandbox-run CTS in the two script forms
|
||||
|
||||
**Files:** `src/ZB.MOM.WW.ScadaBridge.CentralUI/.../ApiMethodForm.razor` (:233),
|
||||
`SharedScriptForm.razor`, bUnit tests.
|
||||
|
||||
Neither form implements `IDisposable`, so an abandoned sandbox run parks a thread on
|
||||
`GetAwaiter().GetResult()` until the script timeout and its continuation touches disposed
|
||||
state. `TemplateScriptDialog.razor:354-358` does it right. **Fix:** copy that pattern
|
||||
exactly — `IDisposable`, cancel+dispose `_runCts`, guard the continuation on disposal.
|
||||
Tests: disposing the form mid-run cancels the sandbox run; continuation after dispose is
|
||||
a no-op.
|
||||
|
||||
### T12 (Sonnet) — Register row 103: TransportImport slot strand + Topology timer
|
||||
|
||||
**Files:** `src/ZB.MOM.WW.ScadaBridge.CentralUI/.../TransportImport.razor.cs` (:244),
|
||||
`Topology.razor` (:201-214), bUnit tests.
|
||||
|
||||
The two TTL-bounded lesser instances of the P6-6 class, deferred by the Medium wave:
|
||||
`TryLoadAsync` abandoned mid-start strands 1 of 8 import session slots for 30 min
|
||||
(partially defeating row 43), and `Topology.razor`'s timer survives disposal in the same
|
||||
window shape. **Fix:** apply the Medium wave T10 pattern (`DebugStreamHub`'s take-once
|
||||
`_sessionLock`/`TakeSession()`) — post-await disposal re-check with exactly-once cleanup
|
||||
ownership; timer created under the same discipline. Closing row 103 happens at
|
||||
integration. Tests: dispose-during-start releases the import slot immediately; disposed
|
||||
Topology page leaves no live timer (Watch/ExpectTerminated-style assertion, not sleep).
|
||||
|
||||
### T13 (Opus) — P7-5 + P7-6: bulk-deploy DbContext poisoning + out-of-date pre-filter
|
||||
|
||||
**Files:** `src/ZB.MOM.WW.ScadaBridge.TemplateEngine/.../DeploymentService.cs`
|
||||
(:677-683, :804), `StaleInstanceProbe`/`IStaleInstanceProbe` call path, the bulk-deploy
|
||||
management command + `SiteDeploymentSummary` (additive), CLI `deploy site` (`--force`),
|
||||
tests.
|
||||
|
||||
- **P7-5 (plausible-needs-verification):** the three bulk phases share one scoped
|
||||
DbContext; a persistent `SaveChanges` failure on one tracked entity rethrows on every
|
||||
later save including `MarkDeploymentFailedAsync` cleanup — later instances fail and
|
||||
records stay `InProgress` (locks are released; reconcile self-heals next deploy).
|
||||
**Verify first** with a forced save failure; **fix:** detach the failed entries on
|
||||
cleanup-save failure so subsequent instances save cleanly.
|
||||
- **P7-6 (plan decision 1):** implement the up-to-date pre-filter using
|
||||
`IStaleInstanceProbe` (its memo timing was fixed by Medium-wave P7-4 —
|
||||
`FlattenSession.VisitedTemplateVersions` records at the right point, so the probe is
|
||||
trustworthy here); up-to-date instances land in the existing additive
|
||||
`SkippedInstances` list with reason "up-to-date"; `Force = true` (management command,
|
||||
additive; CLI `--force`) restores unconditional behavior. Must compose with the P7-2
|
||||
Disabled-skip: Disabled wins (never deployed even under `--force` at bulk scope —
|
||||
single-instance deploy remains the explicit re-enable path). Opus: deployment-semantics
|
||||
change composing three prior fixes. Tests: unchanged instance skipped with reason and
|
||||
its static overrides untouched; `--force` deploys it; Disabled skipped under both;
|
||||
poisoned entity no longer cascades.
|
||||
|
||||
## Phase 5 — Tests / Infra / Docs
|
||||
|
||||
### T14 (Sonnet) — P8-8 + P8-9: residual test-health instances
|
||||
|
||||
**Files:** `tests/.../DataConnectionLayer.Tests/Actors/DataConnectionActorBatchTests.cs`
|
||||
(:181-184, :206, :214), `SiteAlarmLiveCacheServiceTests.cs` (:453-454, :471-472),
|
||||
`StoreAndForwardServiceTests.cs` (:891-892), `KpiHistoryRecorderActorTests.cs`
|
||||
(:1043-1049), `tests/.../Host.Tests/ScadaBridgeTelemetryTests.cs` (:30).
|
||||
|
||||
- **P8-8:** the remaining row-40-class wall-clock sleeps outside Communication.Tests —
|
||||
apply the Medium-wave T17 treatment (`AssertHoldsFor` for absence assertions,
|
||||
event/condition waits for presence; no widened tolerances that mask real regressions).
|
||||
- **P8-9:** `SetQueueDepthProvider(() => 5)` mutates the process-global row-32 slot with
|
||||
no clear and no `[Collection]` — a cross-assembly landmine. **Fix:** compare-and-clear
|
||||
in `finally` (restore prior provider), plus a collection if any second reader appears.
|
||||
Proof-of-failure here = demonstrate the pre-fix flake mechanism (e.g. run the sleep
|
||||
assertion under load) or, where impractical, the honesty-note pattern.
|
||||
|
||||
### T15 (Sonnet) — P8-7 + P8-10: docs drift + env2 parity tail
|
||||
|
||||
**Files:** `docker/README.md` (:74, :155), `docker-env2/README.md` (:56),
|
||||
`docker-env2/docker-compose.yml` (:34 area), `docker-env2/teardown.sh` (:13, :163-171),
|
||||
`CLAUDE.md` (:15, at integration if contended), `infra/` compose comment.
|
||||
|
||||
- **P8-7:** both rig READMEs still list the retired `scadabridge-ldap` container; the
|
||||
infra compose retirement comment is true only of env2; CLAUDE.md names Traefik as an
|
||||
`infra/` service (it is per-rig, `docker/docker-compose.yml:411`) and points at
|
||||
`scadaproj/infra/glauth` where `docker/` actually uses `glauth-redundant`. **Fix:**
|
||||
correct all five spots to match the running rigs (LDAP = shared GLAuth
|
||||
`10.100.0.35:3893`; name the actual glauth config each rig uses).
|
||||
- **P8-10:** env2 maps no host port for the central gRPC control plane (docker/ maps
|
||||
9013/9014→8083; 9113/9114 are free — use them), and `teardown.sh` still claims data is
|
||||
"preserved in node directories" — untrue since the named-volume migration — with no
|
||||
volume-rm recipe. **Fix:** add the port mappings + rewrite teardown's data note with an
|
||||
explicit `docker volume rm` recipe (destructive step clearly labeled). Verify env2
|
||||
still boots: `bash docker-env2/deploy.sh` gate (it runs concurrently with docker/ —
|
||||
do not touch docker/ ports).
|
||||
|
||||
---
|
||||
|
||||
## Integration (single session, after all phases report)
|
||||
|
||||
1. Cherry-pick order: T5 (LocalDb 0.3.3 publish + pin) first or last-but-isolated;
|
||||
everything else in phase order. Interim build every ~6 picks.
|
||||
2. Combined `dotnet build ZB.MOM.WW.ScadaBridge.slnx` 0/0, then the full sweep
|
||||
(known flakes: `SiteConnectionUpGauge_*`, DCL027, IO-errno under load — rerun in
|
||||
isolation before calling regression).
|
||||
3. Register: close **row 103** (T12); add rows for anything a task honestly deferred
|
||||
(P4-6 fallback branch, P3-4 if starvation didn't reproduce); new row for the P4-7
|
||||
FK-guard constraint ("replicated tables must not be cascade-delete FK parents").
|
||||
4. CLAUDE.md: bulk-deploy bullet gains the out-of-date filter + `--force` sentence
|
||||
(P7-6); LocalDb bullet gains the 0.3.3 line; fix the :15 Traefik/glauth drift if T15
|
||||
left it for integration. Umbrella `scadaproj/CLAUDE.md`: 0.3.3 entry.
|
||||
5. `git diff` review, single descriptive merge-session commit set; push and rig redeploy
|
||||
only on explicit request.
|
||||
Reference in New Issue
Block a user