From 9668600c38b2fe2db4afcc9b3ddcbba64f7d77b1 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 09:49:53 -0400 Subject: [PATCH] docs(inbound): correct bus-consumer claims in tracker + invalidation-contract doc + CLAUDE.md (plan R2-06 T2) --- CLAUDE.md | 2 +- archreview/plans/00-MASTER-TRACKER.md | 4 ++-- ...7-08-script-artifact-invalidation-contract.md | 16 +++++++--------- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index b8ce6646..4b32d017 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -150,7 +150,7 @@ Other peers in the `scadaproj` family (see `scadaproj/CLAUDE.md` for details): ` - Last-write-wins for concurrent template editing (no optimistic concurrency on templates). - Optimistic concurrency on deployment status records. - Naming collisions in composed feature modules are design-time errors. -- Transport (#24, M8): bundles now move site/instance config (`Site`s, site-scoped `DataConnection`s, `Instance`s + override children + `Area` by name), not just central-only config — but still never touch site runtime nodes (imported instances land `NotDeployed`). A `BundleNameMap` name-mapping subsystem (Commons: `SiteMapping`/`ConnectionMapping`/`MappingAction`) reconciles environment-specific identifiers (sites by `SiteIdentifier`, connections by site-qualified `{SiteIdentifier}/{Name}`, instances by `UniqueName`): auto-match → operator override via the import-wizard Map step / CLI `--map-site`/`--map-connection`/`--create-missing-*` → blocker rows for the unresolved. D3 "carry full config (encrypted secrets)": Site addresses travel; `DataConnection` config rides the encrypted `SecretsBlock` (presence-only in diffs). Per-line Myers diff for code fields via the pure `LineDiffer` (`ArtifactDiff` embeds a size-capped structured `lineDiff`; input capped at `MaxInputLines`=4000 → summary-only, so a crafted bundle can't OOM the active node); `ImportResult.StaleInstanceIds` is real via the `IStaleInstanceProbe` seam (Commons; implemented in DeploymentManager). `schemaVersion` 1.0→1.1 (additive); `bundleFormatVersion` stays 1; no new EF tables/columns. **Arch-review 05 (plan 05):** template child fidelity completed (round-trip-guard-verified: `LockedInDerived`, native-alarm-sources, script cadence/timeout, real `AreaName`, ES retry config + methods-on-Add, notification recipients, folder parent edges); import runs the **script trust gate** (template/shared/ApiMethod bodies **and** Expression-trigger bodies) as a hard pre-runtime gate + a name-resolution severity split (template=advisory `ConflictKind.Warning`+`ImportResult.Warnings`, ApiMethod=hard error); `ArtifactDiff` resolves folder/base/composition names (unchanged structured templates classify Identical again); `MaxConcurrentImportSessions`=8 session cap. Rename does NOT rewrite call sites (deploy gate on the target is the real check). Plan-06 handoff: `ScriptArtifactsChanged`/`IScriptArtifactChangeBus` seam + Host in-process bus + Transport post-commit publisher shipped (consumers = plan 06). +- Transport (#24, M8): bundles now move site/instance config (`Site`s, site-scoped `DataConnection`s, `Instance`s + override children + `Area` by name), not just central-only config — but still never touch site runtime nodes (imported instances land `NotDeployed`). A `BundleNameMap` name-mapping subsystem (Commons: `SiteMapping`/`ConnectionMapping`/`MappingAction`) reconciles environment-specific identifiers (sites by `SiteIdentifier`, connections by site-qualified `{SiteIdentifier}/{Name}`, instances by `UniqueName`): auto-match → operator override via the import-wizard Map step / CLI `--map-site`/`--map-connection`/`--create-missing-*` → blocker rows for the unresolved. D3 "carry full config (encrypted secrets)": Site addresses travel; `DataConnection` config rides the encrypted `SecretsBlock` (presence-only in diffs). Per-line Myers diff for code fields via the pure `LineDiffer` (`ArtifactDiff` embeds a size-capped structured `lineDiff`; input capped at `MaxInputLines`=4000 → summary-only, so a crafted bundle can't OOM the active node); `ImportResult.StaleInstanceIds` is real via the `IStaleInstanceProbe` seam (Commons; implemented in DeploymentManager). `schemaVersion` 1.0→1.1 (additive); `bundleFormatVersion` stays 1; no new EF tables/columns. **Arch-review 05 (plan 05):** template child fidelity completed (round-trip-guard-verified: `LockedInDerived`, native-alarm-sources, script cadence/timeout, real `AreaName`, ES retry config + methods-on-Add, notification recipients, folder parent edges); import runs the **script trust gate** (template/shared/ApiMethod bodies **and** Expression-trigger bodies) as a hard pre-runtime gate + a name-resolution severity split (template=advisory `ConflictKind.Warning`+`ImportResult.Warnings`, ApiMethod=hard error); `ArtifactDiff` resolves folder/base/composition names (unchanged structured templates classify Identical again); `MaxConcurrentImportSessions`=8 session cap. Rename does NOT rewrite call sites (deploy gate on the target is the real check). Plan-06 handoff: `ScriptArtifactsChanged`/`IScriptArtifactChangeBus` seam + Host in-process bus + Transport post-commit publisher shipped (ApiMethod consumer = InboundAPI `ScriptArtifactChangeSubscriber`, wired in plan R2-06; SharedScript/Template publish with no consumer — nothing at central caches those kinds). ### Store-and-Forward - Fixed retry interval, no max buffer size. Only transient failures buffered. diff --git a/archreview/plans/00-MASTER-TRACKER.md b/archreview/plans/00-MASTER-TRACKER.md index 02669a0e..e0f90bcb 100644 --- a/archreview/plans/00-MASTER-TRACKER.md +++ b/archreview/plans/00-MASTER-TRACKER.md @@ -84,7 +84,7 @@ Status values: ⬜ Not started · 🟨 In progress · ✅ Complete · ⏸ Blocke **Wave 3 — PLAN-05 COMPLETE (2026-07-10):** all 25 tasks delivered on worktree branch `worktree-archreview-wave3-plan05` and **merged to `main` `ec23db35` via fast-forward** (main was still at PLAN-04's `dbd3ed2f`, zero divergence; worktree removed, branch deleted). Templates/Deployment/Transport hardening across seven report-and-wait waves: import correctness spine (BundleImporter consumes `BaseTemplateName`/composition/inheritance edges + import-time acyclicity, `MinTimeBetweenRuns`/`ExecutionTimeoutSeconds`/`LockedInDerived`/native-alarm-sources/real `AreaName` carried through — T1–T7), a reflection round-trip equivalence guard that surfaced and fixed 5 silent-data-loss holes (ES retry+methods-on-Add, notification recipients, folder parent edges — T8), flattener/collision-detector repeated-composition fixes (T9/T10), revision-hash + DiffService native-source coverage (T11/T12), native-source lock enforcement (T13), the `IScriptArtifactChangeBus` invalidation contract + Host in-process bus + Transport post-commit publisher (T14, plan-06 handoff), compile-verdict cache + read-only-path compile skip + post-success audit isolation + central NotDeployed-delete (T15–T18), and the closing security/quality chain: **import blocker severity split** (template-script findings advisory, ApiMethod hard — T19), **the script trust gate on bundle import** (5th `ScriptTrustValidator` call site, over script bodies *and* Expression-trigger bodies — T20, given a code-reviewer security pass that caught + fixed 2 Important gaps: expression-trigger coverage and per-origin local-declaration scoping), ScriptTrustPolicy deny-list hardening (T21), LineDiffer input cap (T22), ArtifactDiff placeholder-identity resolution (T23), the low-severity batch (session cap / full error lists / leaf-fallback warning — T24), and the design-doc + CLAUDE.md sweep (T25). **Terminal verification:** full slnx build 0 warn/0 err; the entire `dotnet test` sweep green (~5,900 unit + integration tests across every suite) plus a direct `CLI.Tests` run (341). Only residue: the same environmental 8 `CentralUI.PlaywrightTests` Deployment E2E failures (`SQLite Error 14` on `instance deploy` — needs a live docker cluster; PLAN-05 touched zero files in that project's deployment-DB path). Initiative total: **128 of 192** (sum of the Done column). -**Wave 4 — PLAN-06 COMPLETE (2026-07-10):** all 24 tasks delivered and **merged to `main` `3e964ac2` via fast-forward** (main was still at PLAN-05's `9b0cf44c`, zero divergence; worktree removed, branch deleted). Edge Integrations hardening across six report-and-wait waves of file-disjoint parallel subagents: the **InboundAPI** spine — revision-checked compiled-handler cache (heals failover / direct-SQL / known-bad staleness, DB row now authoritative — T1/T2), `InvalidateMethod` seam consuming PLAN-05's `IScriptArtifactChangeBus` contract (T3), deferred DI-scope disposal + orphan-execution telemetry (T4), machine-readable error `code`s incl. `SITE_UNREACHABLE` + 415-on-non-JSON (T5), `IOptionsMonitor` hot-reload (T6), parallel startup compile (T7); the **ESG** per-system `TimeoutSeconds` full vertical slice (entity→EF migration→artifact→site SQLite→client honor→mgmt→CLI→UI→doc — T8–T12) plus `{param}` path-template substitution (T13), bounded outbound response buffering (T14), structured JSON `AuthConfiguration` with legacy-colon fallback (T15), cancellation-aware `IsTransient` (T16); the **Notification** batch — SMS first-transient short-circuit (T17), wired the dead `SmsConfiguration.MaxRetries/RetryDelay` per-type retry policy (T18), configurable OAuth2 authority/scope (entity→EF migration→token service→mgmt/CLI/UI surfaces — T19/T20), deterministic lowest-Id SMTP selection + atomic OAuth2 token cache (T21), Twilio `AccountSid` format validation (T22); and the docs (DelmiaNotifier at-least-once semantics — T23; dual-key API-key rotation procedure — T24). Two EF migrations added (`AddExternalSystemTimeoutSeconds` T8, `AddSmtpOAuth2AuthorityScope` T19, serialized on the shared snapshot). **Terminal verification:** full slnx build 0 warn/0 err; every unit + integration suite green (InboundAPI 253, ESG 132, NotificationOutbox 132, NotificationService 57, ManagementService 245, CLI 346, CentralUI 893, SiteRuntime 494, Commons 597, ConfigDB 338, Host 260, + all others). Only residue: the same environmental 8 `CentralUI.PlaywrightTests` Deployment E2E failures (`SQLite Error 14` on `instance deploy` — needs a live docker cluster; PLAN-06 touched zero files in that project's deployment-DB path). Execution note: `isolation:"worktree"` subagents proved non-isolating here (shared index → commit races); the working model was pre-created dedicated `git worktree`s per task + cherry-pick onto an integration branch + per-wave combined build/impacted-suite barrier. Initiative total: **152 of 192** (sum of the Done column). +**Wave 4 — PLAN-06 COMPLETE (2026-07-10):** all 24 tasks delivered and **merged to `main` `3e964ac2` via fast-forward** (main was still at PLAN-05's `9b0cf44c`, zero divergence; worktree removed, branch deleted). Edge Integrations hardening across six report-and-wait waves of file-disjoint parallel subagents: the **InboundAPI** spine — revision-checked compiled-handler cache (heals failover / direct-SQL / known-bad staleness, DB row now authoritative — T1/T2), `InvalidateMethod` seam (T3; the bus *subscriber* was NOT wired here — arch-review 06 round-2 N1 — and shipped later via PLAN-R2-06 T1), deferred DI-scope disposal + orphan-execution telemetry (T4), machine-readable error `code`s incl. `SITE_UNREACHABLE` + 415-on-non-JSON (T5), `IOptionsMonitor` hot-reload (T6), parallel startup compile (T7); the **ESG** per-system `TimeoutSeconds` full vertical slice (entity→EF migration→artifact→site SQLite→client honor→mgmt→CLI→UI→doc — T8–T12) plus `{param}` path-template substitution (T13), bounded outbound response buffering (T14), structured JSON `AuthConfiguration` with legacy-colon fallback (T15), cancellation-aware `IsTransient` (T16); the **Notification** batch — SMS first-transient short-circuit (T17), wired the dead `SmsConfiguration.MaxRetries/RetryDelay` per-type retry policy (T18), configurable OAuth2 authority/scope (entity→EF migration→token service→mgmt/CLI/UI surfaces — T19/T20), deterministic lowest-Id SMTP selection + atomic OAuth2 token cache (T21), Twilio `AccountSid` format validation (T22); and the docs (DelmiaNotifier at-least-once semantics — T23; dual-key API-key rotation procedure — T24). Two EF migrations added (`AddExternalSystemTimeoutSeconds` T8, `AddSmtpOAuth2AuthorityScope` T19, serialized on the shared snapshot). **Terminal verification:** full slnx build 0 warn/0 err; every unit + integration suite green (InboundAPI 253, ESG 132, NotificationOutbox 132, NotificationService 57, ManagementService 245, CLI 346, CentralUI 893, SiteRuntime 494, Commons 597, ConfigDB 338, Host 260, + all others). Only residue: the same environmental 8 `CentralUI.PlaywrightTests` Deployment E2E failures (`SQLite Error 14` on `instance deploy` — needs a live docker cluster; PLAN-06 touched zero files in that project's deployment-DB path). Execution note: `isolation:"worktree"` subagents proved non-isolating here (shared index → commit races); the working model was pre-created dedicated `git worktree`s per task + cherry-pick onto an integration branch + per-wave combined build/impacted-suite barrier. Initiative total: **152 of 192** (sum of the Done column). **Wave 5 — PLAN-07 COMPLETE (2026-07-10):** all 32 active tasks (T4–T32; T1–T3 landed in P0; **T33 `skipped-duplicate`** — CLI.Tests already in the slnx via PLAN-08 T1) delivered on worktree branch `plan07` and **merged to `main` via fast-forward** (main was still at PLAN-06's `4fb754f9`, zero divergence; all worktrees removed, branch deleted). UI/Management/Security hardening across six report-and-wait waves of file-disjoint parallel subagents against the dominant **ManagementActor single-writer lane** (15 tasks serialized in one integration worktree; the file-disjoint remainder ran in dedicated per-task worktrees, cherry-picked in with a per-wave combined build + impacted-suite barrier): **C1** auth-disabled artifact killed + `DisableLoginGuard` fail-fast (T1–T3, P0); **C2** `DeployToSiteAsync` single-site deploy + fleet-wide site-scope guard (T4/T5); **C3** connection/external-system/DB-connection secret elision via `ConfigSecretScrubber` + sentinel-preserving update (T6–T9); **UA1/C6/C5** any-of role gates + `ListSecuredWrites` gating + area-gate reconciliation (Designer|Deployer) + role-casing canonicalization (T10–T12); **UA2** the frozen `GetRequiredRoles` authorization matrix over all **138** registered commands + dispatch-coverage guard (T13/T24); **S2/P3** real server-side secured-write TTL + expiry-at-approve/reject + opportunistic list sweep + list paging + UI history pager & submission-age dialog (T14–T18); **S1** 5-min transport/deploy Ask timeout + shipped config (T19/T20); **C4** additive `SiteIdentifier` on browse/verify/cert commands + actor dispatch (Designer browse/verify, Admin cert-trust) + dispatch-coverage + CLI parity (T21–T25); **P1/UA5** `LoginThrottle` per-username+IP LDAP-bind lockout wired into every bind surface (T26/T27); **P2** additive Skip/Take paging on template/instance lists (T28); **S3** `PollGate` reentrancy guard on Health/AlarmSummary timers (T29); **S4** transport single-flight + export base64-copy elimination (T30); **S5/C7/C8/P4/UA6** debug-stream fault logging + event-log-filter doc fix + AlarmSummary memoization/a11y + the stale CLAUDE.md `SourceNode` note corrected (C7/C8 both verified true — T31/T32). Two message-contract additions (`AuditKind.SecuredWriteExpire`, secured-write list paging params) landed additively; no EF migrations needed. **Terminal verification:** full slnx build 0 warn/0 err; 28 of 30 test assemblies green (ManagementService 448 incl. the frozen matrix + dispatch-coverage, CentralUI 898, CLI 359, Security 175, Commons 609, Communication 263, Transport 138, ConfigDB 341, SiteRuntime 494, Host 260, + all others), after fixing one stale enum-count lock-in test (`AuditEnumTests` 15→16 for T15's additive `SecuredWriteExpire` — the per-task filtered runs never hit that Commons guard, so the full sweep caught it, exactly as in PLAN-04). Only residue: the same environmental 8 `CentralUI.PlaywrightTests` Deployment E2E failures (`SQLite Error 14` on `instance deploy` — needs a live docker cluster; PLAN-07 touched zero files in that project's deployment-DB path). Initiative total: **181 of 192** (sum of the Done column; PLAN-07's max is 32/33 with T33 a documented skip). @@ -146,7 +146,7 @@ Run up to **five plan-lane executor sessions concurrently** (one each for PLAN-0 - **PLAN-04 Task 13 (additive `RequestedBy` message change) before any PLAN-07 sender refactors** touching the same messages. **Soft seams (either order works, coordinate):** -- **PLAN-05 Task 14 (`IScriptArtifactChangeBus`) ↔ PLAN-06 Task 3 (`InboundScriptExecutor.InvalidateMethod`).** PLAN-06 Task 1's per-request revision check is the designed correctness fallback, so there is no hard sequencing; when both land, wire the bus to the invalidate seam. +- **PLAN-05 Task 14 (`IScriptArtifactChangeBus`) ↔ PLAN-06 Task 3 (`InboundScriptExecutor.InvalidateMethod`).** PLAN-06 Task 1's per-request revision check is the designed correctness fallback, so there is no hard sequencing; when both land, wire the bus to the invalidate seam. **Closed 2026-07-13 (PLAN-R2-06 T1):** `ScriptArtifactChangeSubscriber` (InboundAPI hosted service) subscribes and invalidates on `ApiMethod` notifications. - **PLAN-03's `DeployCompileValidator` should adopt PLAN-05's compile-cache abstraction** once it exists (later refactor, not a blocker). - **PLAN-03 Task 8's stuck-thread watchdog pattern is reusable by PLAN-06 Task 4** (inbound orphan counter). diff --git a/docs/plans/2026-07-08-script-artifact-invalidation-contract.md b/docs/plans/2026-07-08-script-artifact-invalidation-contract.md index 81bce194..8dce8c69 100644 --- a/docs/plans/2026-07-08-script-artifact-invalidation-contract.md +++ b/docs/plans/2026-07-08-script-artifact-invalidation-contract.md @@ -1,6 +1,6 @@ # Script-Artifact Invalidation Contract (`ScriptArtifactsChanged`) -**Status:** Seam + first producer shipped in PLAN-05 (arch-review 05, Task 14). Consumer + additional producers land in **plan 06**. +**Status:** Seam + first producer shipped in PLAN-05 (T14). **Consumer (a) shipped in PLAN-R2-06 T1** (`ScriptArtifactChangeSubscriber`). (b) Management producers remain unwired by design — delete calls `InvalidateMethod` directly; edits self-heal via the revision check. (c) is satisfied by the per-request revision check (PLAN-06 T1). **Problem.** When a script-bearing artifact's definition changes at runtime — an Inbound API method's script, a shared script, or a template's scripts — any node that has already **compiled and cached** a handler/delegate for it keeps serving the stale version until a process restart. This is the root of two recorded project-memory gotchas (a DB-direct edit to a broken inbound method keeps returning "Script compilation failed" from `_knownBadMethods`; a non-compiling update leaves the old delegate serving). A bundle import is a third mutation path with the same hazard. @@ -45,14 +45,12 @@ public interface IScriptArtifactChangeBus - `InProcessScriptArtifactChangeBus` in the Host, registered as a central-role singleton. - The **Transport bundle importer as the first producer**: `BundleImporter.ApplyAsync` collects the Overwrite/Rename resolutions for `ApiMethod` / `SharedScript` / `Template` and publishes one `ScriptArtifactsChanged` per kind **after** `tx.CommitAsync`. The importer's dependency on the bus is optional (`IScriptArtifactChangeBus?`) so Transport-only hosts (and unit tests) run without it. -## Handoff to plan 06 +## Handoff to plan 06 — dispositions -Plan 06 implements the consumer side and the remaining producers: - -| Item | Responsibility | +| Item | Disposition | |---|---| -| **(a) Inbound API consumer** | `InboundScriptExecutor` subscribes to the bus and, on an `ApiMethod` notification, evicts the named entries from `_scriptHandlers` **and** `_knownBadMethods` — closing the "DB-direct edit keeps returning compile error" and "non-compiling update keeps old handler" gotchas. | -| **(b) Management producers** | Publish `ScriptArtifactsChanged` from the `ManagementActor` `UpdateApiMethod` / shared-script / template-script edit paths (`Source = "Management"`), so a UI/CLI edit invalidates the same caches a bundle import does. | -| **(c) Cross-node / failover freshness** | A revision-keyed handler cache (compile keyed by code hash — see PLAN-05 Task 15's `ScriptCompileVerdictCache`) so a node that missed the in-process notification still self-heals on next use, and a newly-active failover node never serves a stale compile. | +| **(a) Inbound API consumer** | **Shipped (PLAN-R2-06 T1).** `ScriptArtifactChangeSubscriber` (an InboundAPI `IHostedService` registered by `AddInboundAPI`) subscribes to the bus and, on an `ApiMethod` notification, calls `InboundScriptExecutor.InvalidateMethod` per name — evicting the named entries from `_scriptHandlers` **and** `_knownBadMethods`, closing the "DB-direct edit keeps returning compile error" and "non-compiling update keeps old handler" gotchas. | +| **(b) Management producers** | **Unwired by design.** The `ManagementActor` delete path calls `InboundScriptExecutor.InvalidateMethod` directly; the update/edit paths self-heal via the per-request revision check, so no notification is needed for correctness. Left unwired deliberately rather than publishing `Source = "Management"` notifications. | +| **(c) Cross-node / failover freshness** | **Satisfied by the per-request revision check (PLAN-06 T1).** The compiled handler is revision-checked against the freshly-fetched `ApiMethod.Script` on every request, so a node that missed the in-process notification still self-heals on next use, and a newly-active failover node never serves a stale compile. | -Until (a)–(c) land, plan 05's publisher is a no-op in practice (no subscribers), but the seam is stable and the producer is correct, so plan 06 is purely additive. +Consumer (a) is now wired, so plan 05's `ApiMethod` publisher reaches a real subscriber. `SharedScript`/`Template` notifications still publish with **no consumer** — that is safe: nothing at central caches compiled artifacts of those kinds, so there is nothing to invalidate. The seam is stable and the producer is correct.