From 4be13429b63e39fa4576133af26c4f749735c766 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 09:51:05 -0400 Subject: [PATCH 01/17] fix(r2-02): add ResiliencePolicyRanges single-source-of-truth to Core.Abstractions (01/S-6 task 1) --- ...lience-config-hardening-plan.md.tasks.json | 2 +- .../ResiliencePolicyRanges.cs | 33 +++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/ResiliencePolicyRanges.cs diff --git a/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json b/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json index 26510721..b04bd71d 100644 --- a/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json +++ b/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json @@ -2,7 +2,7 @@ "planPath": "archreview/plans/R2-02-resilience-config-hardening-plan.md", "tasks": [ { "id": 0, "subject": "S-6 brick-repro test (RED): parsed timeoutSeconds:0 / breakerFailureThreshold:1 must not brick capability calls — ResilienceConfigBrickTests, expect 2 failures on f6eaa267", "status": "pending", "blockedBy": [] }, - { "id": 1, "subject": "Add ResiliencePolicyRanges constants to Core.Abstractions (single source of truth for parser/builder/AdminUI, derived from Polly.Core 8.6.6 validation)", "status": "pending", "blockedBy": [] }, + { "id": 1, "subject": "Add ResiliencePolicyRanges constants to Core.Abstractions (single source of truth for parser/builder/AdminUI, derived from Polly.Core 8.6.6 validation)", "status": "completed", "blockedBy": [] }, { "id": 2, "subject": "S-6 parser: clamp timeout/retry/breaker overrides with appending diagnostics (timeout<=0 -> tier default; breaker==1 -> 2; caps) + 8 parser tests", "status": "pending", "blockedBy": [0, 1] }, { "id": 3, "subject": "S-6 builder: belt-and-braces clamp in Build so pipeline construction can never throw ValidationException + Build_NeverThrows_ForHostileDirectOptions (turns task 0 green)", "status": "pending", "blockedBy": [0, 1] }, { "id": 4, "subject": "S-6 production-wiring guard: DriverCapabilityInvokerFactory.Create with hostile JSON yields a working invoker + logged clamp diagnostic; re-run ResilienceInvokerFactoryRegistrationTests", "status": "pending", "blockedBy": [2, 3] }, diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/ResiliencePolicyRanges.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/ResiliencePolicyRanges.cs new file mode 100644 index 00000000..4fa8cb86 --- /dev/null +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/ResiliencePolicyRanges.cs @@ -0,0 +1,33 @@ +namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +/// +/// Legal ranges for per-capability resilience policy values. Single source of truth shared by +/// DriverResilienceOptionsParser (clamp + diagnostic), DriverResiliencePipelineBuilder +/// (last-resort clamp so a pipeline build can never throw Polly's ValidationException), and the +/// AdminUI ResilienceFormModel (authoring-time validation messages). Derived from +/// Polly.Core 8.6.6's option validation: TimeoutStrategyOptions.Timeout ∈ [10 ms, 24 h]; +/// CircuitBreakerStrategyOptions.MinimumThroughput ≥ 2. +/// +public static class ResiliencePolicyRanges +{ + /// Minimum per-call timeout. Whole-second granularity puts the floor at 1 s (Polly's is 10 ms). + public const int MinTimeoutSeconds = 1; + + /// Maximum per-call timeout — Polly's TimeoutStrategyOptions ceiling (24 h). + public const int MaxTimeoutSeconds = 86_400; + + /// Retry floor; 0 = no retry strategy is added to the pipeline. + public const int MinRetryCount = 0; + + /// Operational retry cap (Polly itself allows int.MaxValue; 100 exponential-backoff attempts ≈ minutes). + public const int MaxRetryCount = 100; + + /// Breaker floor; 0 = no breaker strategy is added (the Tier C posture). + public const int MinBreakerFailureThreshold = 0; + + /// Polly's MinimumThroughput floor — an enabled breaker needs a threshold of at least 2. + public const int MinEnabledBreakerFailureThreshold = 2; + + /// Sanity cap on the breaker threshold (Polly has no ceiling; beyond this it can never trip in the 30 s window). + public const int MaxBreakerFailureThreshold = 10_000; +} From 294d74c1de64123a33cc8c1377ed247800f30993 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 09:53:28 -0400 Subject: [PATCH 02/17] =?UTF-8?q?fix(r2-02):=20range-clamp=20ResilienceCon?= =?UTF-8?q?fig=20overrides=20in=20the=20parser=20(01/S-6)=20=E2=80=94=20ti?= =?UTF-8?q?meout/retry/breaker=20clamped=20with=20diagnostics,=20never=20b?= =?UTF-8?q?rick?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ...lience-config-hardening-plan.md.tasks.json | 2 +- .../DriverResilienceOptionsParser.cs | 75 +++++++++++- .../DriverResilienceOptionsParserTests.cs | 112 ++++++++++++++++++ 3 files changed, 184 insertions(+), 5 deletions(-) diff --git a/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json b/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json index b04bd71d..0417d150 100644 --- a/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json +++ b/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json @@ -3,7 +3,7 @@ "tasks": [ { "id": 0, "subject": "S-6 brick-repro test (RED): parsed timeoutSeconds:0 / breakerFailureThreshold:1 must not brick capability calls — ResilienceConfigBrickTests, expect 2 failures on f6eaa267", "status": "pending", "blockedBy": [] }, { "id": 1, "subject": "Add ResiliencePolicyRanges constants to Core.Abstractions (single source of truth for parser/builder/AdminUI, derived from Polly.Core 8.6.6 validation)", "status": "completed", "blockedBy": [] }, - { "id": 2, "subject": "S-6 parser: clamp timeout/retry/breaker overrides with appending diagnostics (timeout<=0 -> tier default; breaker==1 -> 2; caps) + 8 parser tests", "status": "pending", "blockedBy": [0, 1] }, + { "id": 2, "subject": "S-6 parser: clamp timeout/retry/breaker overrides with appending diagnostics (timeout<=0 -> tier default; breaker==1 -> 2; caps) + 8 parser tests", "status": "completed", "blockedBy": [0, 1] }, { "id": 3, "subject": "S-6 builder: belt-and-braces clamp in Build so pipeline construction can never throw ValidationException + Build_NeverThrows_ForHostileDirectOptions (turns task 0 green)", "status": "pending", "blockedBy": [0, 1] }, { "id": 4, "subject": "S-6 production-wiring guard: DriverCapabilityInvokerFactory.Create with hostile JSON yields a working invoker + logged clamp diagnostic; re-run ResilienceInvokerFactoryRegistrationTests", "status": "pending", "blockedBy": [2, 3] }, { "id": 5, "subject": "S-6 AdminUI: ResilienceFormModel.Validate() range messages (via ResiliencePolicyRanges through the transitive Core.Abstractions ref) + warning block in DriverResilienceSection", "status": "pending", "blockedBy": [1] }, diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResilienceOptionsParser.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResilienceOptionsParser.cs index fa9690b2..a91b0285 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResilienceOptionsParser.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResilienceOptionsParser.cs @@ -83,15 +83,16 @@ public static class DriverResilienceOptionsParser { if (!Enum.TryParse(capName, ignoreCase: true, out var capability)) { - parseDiagnostic ??= $"Unknown capability '{capName}' in ResilienceConfig; skipped."; + AppendDiagnostic(ref parseDiagnostic, $"Unknown capability '{capName}' in ResilienceConfig; skipped."); continue; } var basePolicy = merged[capability]; - merged[capability] = new CapabilityPolicy( + var mergedPolicy = new CapabilityPolicy( TimeoutSeconds: overridePolicy.TimeoutSeconds ?? basePolicy.TimeoutSeconds, RetryCount: overridePolicy.RetryCount ?? basePolicy.RetryCount, BreakerFailureThreshold: overridePolicy.BreakerFailureThreshold ?? basePolicy.BreakerFailureThreshold); + merged[capability] = ClampPolicy(capability, mergedPolicy, basePolicy, ref parseDiagnostic); } } @@ -102,9 +103,9 @@ public static class DriverResilienceOptionsParser if (shape.RecycleIntervalSeconds is int secs) { if (secs <= 0) - parseDiagnostic ??= $"RecycleIntervalSeconds must be positive; got {secs} — ignoring."; + AppendDiagnostic(ref parseDiagnostic, $"RecycleIntervalSeconds must be positive; got {secs} — ignoring."); else if (tier != DriverTier.C) - parseDiagnostic ??= $"RecycleIntervalSeconds is Tier C only; Tier {tier} driver will not scheduled-recycle."; + AppendDiagnostic(ref parseDiagnostic, $"RecycleIntervalSeconds is Tier C only; Tier {tier} driver will not scheduled-recycle."); else recycleIntervalSeconds = secs; } @@ -119,6 +120,72 @@ public static class DriverResilienceOptionsParser }; } + /// + /// Clamp a merged per-capability override to the legal so a + /// downstream Polly pipeline build can never throw ValidationException (01/S-6). Each clamp + /// appends a diagnostic naming the field + what happened. A non-positive timeout snaps to the + /// capability's (a nonsense value, not merely an extreme — snapping + /// to 1 s could be a 30× surprise on a Discover policy); everything else clamps to the nearest legal + /// value. Preserves maximum operator intent while never bricking. + /// + private static CapabilityPolicy ClampPolicy( + DriverCapability capability, + CapabilityPolicy merged, + CapabilityPolicy tierDefault, + ref string? diagnostic) + { + var timeout = merged.TimeoutSeconds; + if (timeout <= 0) + { + AppendDiagnostic(ref diagnostic, + $"{capability}.timeoutSeconds {timeout} is non-positive; using the tier default ({tierDefault.TimeoutSeconds}s)."); + timeout = tierDefault.TimeoutSeconds; + } + else if (timeout > ResiliencePolicyRanges.MaxTimeoutSeconds) + { + AppendDiagnostic(ref diagnostic, + $"{capability}.timeoutSeconds {timeout} exceeds the max ({ResiliencePolicyRanges.MaxTimeoutSeconds}s); clamped."); + timeout = ResiliencePolicyRanges.MaxTimeoutSeconds; + } + + var retry = merged.RetryCount; + if (retry < ResiliencePolicyRanges.MinRetryCount) + { + AppendDiagnostic(ref diagnostic, $"{capability}.retryCount {retry} is negative; clamped to {ResiliencePolicyRanges.MinRetryCount}."); + retry = ResiliencePolicyRanges.MinRetryCount; + } + else if (retry > ResiliencePolicyRanges.MaxRetryCount) + { + AppendDiagnostic(ref diagnostic, $"{capability}.retryCount {retry} exceeds the cap ({ResiliencePolicyRanges.MaxRetryCount}); clamped."); + retry = ResiliencePolicyRanges.MaxRetryCount; + } + + var breaker = merged.BreakerFailureThreshold; + if (breaker < ResiliencePolicyRanges.MinBreakerFailureThreshold) + { + AppendDiagnostic(ref diagnostic, $"{capability}.breakerFailureThreshold {breaker} is negative; breaker disabled."); + breaker = ResiliencePolicyRanges.MinBreakerFailureThreshold; + } + else if (breaker == 1) + { + AppendDiagnostic(ref diagnostic, + $"{capability}.breakerFailureThreshold 1 is below Polly's minimum throughput; raised to {ResiliencePolicyRanges.MinEnabledBreakerFailureThreshold} (closest to open-on-first-failure intent)."); + breaker = ResiliencePolicyRanges.MinEnabledBreakerFailureThreshold; + } + else if (breaker > ResiliencePolicyRanges.MaxBreakerFailureThreshold) + { + AppendDiagnostic(ref diagnostic, + $"{capability}.breakerFailureThreshold {breaker} exceeds the cap ({ResiliencePolicyRanges.MaxBreakerFailureThreshold}); clamped."); + breaker = ResiliencePolicyRanges.MaxBreakerFailureThreshold; + } + + return merged with { TimeoutSeconds = timeout, RetryCount = retry, BreakerFailureThreshold = breaker }; + } + + /// Append a diagnostic to the running channel (joined with "; ") so multiple clamps all surface. + private static void AppendDiagnostic(ref string? diagnostic, string message) + => diagnostic = diagnostic is null ? message : $"{diagnostic}; {message}"; + private sealed class ResilienceConfigShape { /// Gets or sets the maximum concurrent bulkhead requests. diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/DriverResilienceOptionsParserTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/DriverResilienceOptionsParserTests.cs index ae02305f..17f28e84 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/DriverResilienceOptionsParserTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/DriverResilienceOptionsParserTests.cs @@ -222,4 +222,116 @@ public sealed class DriverResilienceOptionsParserTests options.RecycleIntervalSeconds.ShouldBeNull(); diag.ShouldContain("must be positive"); } + + // ---- 01/S-6: range clamps (clamp + diagnostic, never brick) ---- + + /// A non-positive timeout is nonsense; it snaps to the capability's tier default with a diagnostic. + [Fact] + public void TimeoutSeconds_Zero_FallsBackToTierDefault_WithDiagnostic() + { + var options = DriverResilienceOptionsParser.ParseOrDefaults( + DriverTier.A, "{\"capabilityPolicies\":{\"Read\":{\"timeoutSeconds\":0}}}", out var diag); + + var tierDefault = DriverResilienceOptions.GetTierDefaults(DriverTier.A)[DriverCapability.Read]; + options.Resolve(DriverCapability.Read).TimeoutSeconds.ShouldBe(tierDefault.TimeoutSeconds); + diag.ShouldNotBeNull(); + diag.ShouldContain("timeoutSeconds"); + } + + /// A timeout above Polly's ceiling clamps to the max with a diagnostic. + [Fact] + public void TimeoutSeconds_AbovePollyMax_Clamped() + { + var options = DriverResilienceOptionsParser.ParseOrDefaults( + DriverTier.A, "{\"capabilityPolicies\":{\"Read\":{\"timeoutSeconds\":999999}}}", out var diag); + + options.Resolve(DriverCapability.Read).TimeoutSeconds.ShouldBe(ResiliencePolicyRanges.MaxTimeoutSeconds); + diag.ShouldNotBeNull(); + diag.ShouldContain("timeoutSeconds"); + } + + /// A negative retry count clamps to zero with a diagnostic. + [Fact] + public void RetryCount_Negative_ClampedToZero() + { + var options = DriverResilienceOptionsParser.ParseOrDefaults( + DriverTier.A, "{\"capabilityPolicies\":{\"Read\":{\"retryCount\":-1}}}", out var diag); + + options.Resolve(DriverCapability.Read).RetryCount.ShouldBe(0); + diag.ShouldNotBeNull(); + diag.ShouldContain("retryCount"); + } + + /// An absurd retry count clamps to the operational cap with a diagnostic. + [Fact] + public void RetryCount_AboveCap_ClampedTo100() + { + var options = DriverResilienceOptionsParser.ParseOrDefaults( + DriverTier.A, "{\"capabilityPolicies\":{\"Read\":{\"retryCount\":2000000000}}}", out var diag); + + options.Resolve(DriverCapability.Read).RetryCount.ShouldBe(ResiliencePolicyRanges.MaxRetryCount); + diag.ShouldNotBeNull(); + diag.ShouldContain("retryCount"); + } + + /// A breaker threshold of 1 (below Polly's MinimumThroughput floor) raises to 2 with a diagnostic. + [Fact] + public void BreakerThreshold_One_ClampedToTwo() + { + var options = DriverResilienceOptionsParser.ParseOrDefaults( + DriverTier.A, "{\"capabilityPolicies\":{\"Read\":{\"breakerFailureThreshold\":1}}}", out var diag); + + options.Resolve(DriverCapability.Read).BreakerFailureThreshold.ShouldBe( + ResiliencePolicyRanges.MinEnabledBreakerFailureThreshold); + diag.ShouldNotBeNull(); + diag.ShouldContain("breakerFailureThreshold"); + } + + /// A negative breaker threshold disables the breaker (0) with a diagnostic. + [Fact] + public void BreakerThreshold_Negative_DisablesBreaker() + { + var options = DriverResilienceOptionsParser.ParseOrDefaults( + DriverTier.A, "{\"capabilityPolicies\":{\"Read\":{\"breakerFailureThreshold\":-5}}}", out var diag); + + options.Resolve(DriverCapability.Read).BreakerFailureThreshold.ShouldBe(0); + diag.ShouldNotBeNull(); + diag.ShouldContain("breakerFailureThreshold"); + } + + /// Multiple clamps in one policy all surface in the (appended) diagnostic string. + [Fact] + public void MultipleClamps_AllSurfaceInDiagnostic() + { + var options = DriverResilienceOptionsParser.ParseOrDefaults( + DriverTier.A, + "{\"capabilityPolicies\":{\"Read\":{\"timeoutSeconds\":0,\"retryCount\":-1,\"breakerFailureThreshold\":1}}}", + out var diag); + + diag.ShouldNotBeNull(); + diag.ShouldContain("timeoutSeconds"); + diag.ShouldContain("retryCount"); + diag.ShouldContain("breakerFailureThreshold"); + + var read = options.Resolve(DriverCapability.Read); + read.TimeoutSeconds.ShouldBe(DriverResilienceOptions.GetTierDefaults(DriverTier.A)[DriverCapability.Read].TimeoutSeconds); + read.RetryCount.ShouldBe(0); + read.BreakerFailureThreshold.ShouldBe(ResiliencePolicyRanges.MinEnabledBreakerFailureThreshold); + } + + /// In-range overrides pass through untouched with no diagnostic. + [Fact] + public void InRangeOverrides_PassThrough_NoDiagnostic() + { + var options = DriverResilienceOptionsParser.ParseOrDefaults( + DriverTier.A, + "{\"capabilityPolicies\":{\"Read\":{\"timeoutSeconds\":5,\"retryCount\":3,\"breakerFailureThreshold\":4}}}", + out var diag); + + diag.ShouldBeNull(); + var read = options.Resolve(DriverCapability.Read); + read.TimeoutSeconds.ShouldBe(5); + read.RetryCount.ShouldBe(3); + read.BreakerFailureThreshold.ShouldBe(4); + } } From 8d306a1bd2d382fc5169ee591354683d0498d55e Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 09:55:05 -0400 Subject: [PATCH 03/17] =?UTF-8?q?fix(r2-02):=20pipeline=20Build=20clamps?= =?UTF-8?q?=20policy=20values=20=E2=80=94=20building=20a=20pipeline=20can?= =?UTF-8?q?=20never=20throw=20Polly=20ValidationException=20(01/S-6=20belt?= =?UTF-8?q?-and-braces=20+=20brick=20repro)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ...lience-config-hardening-plan.md.tasks.json | 4 +- .../DriverResiliencePipelineBuilder.cs | 19 ++++++-- .../DriverResiliencePipelineBuilderTests.cs | 31 ++++++++++++ .../Resilience/ResilienceConfigBrickTests.cs | 48 +++++++++++++++++++ 4 files changed, 97 insertions(+), 5 deletions(-) create mode 100644 tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/ResilienceConfigBrickTests.cs diff --git a/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json b/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json index 0417d150..6a0551fc 100644 --- a/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json +++ b/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json @@ -1,10 +1,10 @@ { "planPath": "archreview/plans/R2-02-resilience-config-hardening-plan.md", "tasks": [ - { "id": 0, "subject": "S-6 brick-repro test (RED): parsed timeoutSeconds:0 / breakerFailureThreshold:1 must not brick capability calls — ResilienceConfigBrickTests, expect 2 failures on f6eaa267", "status": "pending", "blockedBy": [] }, + { "id": 0, "subject": "S-6 brick-repro test (RED): parsed timeoutSeconds:0 / breakerFailureThreshold:1 must not brick capability calls — ResilienceConfigBrickTests, expect 2 failures on f6eaa267", "status": "completed", "blockedBy": [] }, { "id": 1, "subject": "Add ResiliencePolicyRanges constants to Core.Abstractions (single source of truth for parser/builder/AdminUI, derived from Polly.Core 8.6.6 validation)", "status": "completed", "blockedBy": [] }, { "id": 2, "subject": "S-6 parser: clamp timeout/retry/breaker overrides with appending diagnostics (timeout<=0 -> tier default; breaker==1 -> 2; caps) + 8 parser tests", "status": "completed", "blockedBy": [0, 1] }, - { "id": 3, "subject": "S-6 builder: belt-and-braces clamp in Build so pipeline construction can never throw ValidationException + Build_NeverThrows_ForHostileDirectOptions (turns task 0 green)", "status": "pending", "blockedBy": [0, 1] }, + { "id": 3, "subject": "S-6 builder: belt-and-braces clamp in Build so pipeline construction can never throw ValidationException + Build_NeverThrows_ForHostileDirectOptions (turns task 0 green)", "status": "completed", "blockedBy": [0, 1] }, { "id": 4, "subject": "S-6 production-wiring guard: DriverCapabilityInvokerFactory.Create with hostile JSON yields a working invoker + logged clamp diagnostic; re-run ResilienceInvokerFactoryRegistrationTests", "status": "pending", "blockedBy": [2, 3] }, { "id": 5, "subject": "S-6 AdminUI: ResilienceFormModel.Validate() range messages (via ResiliencePolicyRanges through the transitive Core.Abstractions ref) + warning block in DriverResilienceSection", "status": "pending", "blockedBy": [1] }, { "id": 6, "subject": "S-8 parser layer: force Write/AlarmAcknowledge retryCount overrides to 0 with diagnostic (spec invariant, honest dead-knob surfacing) + tests + GetTierDefaults doc", "status": "pending", "blockedBy": [2] }, diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResiliencePipelineBuilder.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResiliencePipelineBuilder.cs index b1190f86..a0dcc7d9 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResiliencePipelineBuilder.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResiliencePipelineBuilder.cs @@ -20,6 +20,12 @@ namespace ZB.MOM.WW.OtOpcUa.Core.Resilience; /// Pipeline resolution is lock-free on the hot path: the inner /// caches a per key; /// first-call cost is one .Build. Thereafter reads are O(1). +/// +/// Never-throws invariant (01/S-6): clamps every policy value to +/// before constructing strategy options, so a hostile +/// (constructed directly, bypassing the parser) can never make +/// the memoizing GetOrAdd factory throw Polly's ValidationException — which would +/// otherwise brick every capability call for that key. /// public sealed class DriverResiliencePipelineBuilder { @@ -108,16 +114,23 @@ public sealed class DriverResiliencePipelineBuilder var policy = options.Resolve(capability); var builder = new ResiliencePipelineBuilder { TimeProvider = timeProvider }; + // Belt-and-braces: clamp the resolved policy to ResiliencePolicyRanges so building a pipeline can + // NEVER throw Polly's ValidationException (01/S-6). The parser already clamps every JSON→options + // funnel, but DriverResilienceOptions is a public record constructible by paths that never traverse + // the parser (tests today; any future caller), so this makes "a pipeline build never throws" a + // builder invariant. Three integer ops once per pipeline build (not per call). + var timeoutSeconds = Math.Clamp( + policy.TimeoutSeconds, ResiliencePolicyRanges.MinTimeoutSeconds, ResiliencePolicyRanges.MaxTimeoutSeconds); builder.AddTimeout(new TimeoutStrategyOptions { - Timeout = TimeSpan.FromSeconds(policy.TimeoutSeconds), + Timeout = TimeSpan.FromSeconds(timeoutSeconds), }); if (policy.RetryCount > 0) { var retryOptions = new RetryStrategyOptions { - MaxRetryAttempts = policy.RetryCount, + MaxRetryAttempts = Math.Min(policy.RetryCount, ResiliencePolicyRanges.MaxRetryCount), BackoffType = DelayBackoffType.Exponential, UseJitter = true, Delay = TimeSpan.FromMilliseconds(100), @@ -143,7 +156,7 @@ public sealed class DriverResiliencePipelineBuilder var breakerOptions = new CircuitBreakerStrategyOptions { FailureRatio = 1.0, - MinimumThroughput = policy.BreakerFailureThreshold, + MinimumThroughput = Math.Max(ResiliencePolicyRanges.MinEnabledBreakerFailureThreshold, policy.BreakerFailureThreshold), SamplingDuration = TimeSpan.FromSeconds(30), BreakDuration = TimeSpan.FromSeconds(15), ShouldHandle = new PredicateBuilder().Handle(ex => ex is not OperationCanceledException), diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/DriverResiliencePipelineBuilderTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/DriverResiliencePipelineBuilderTests.cs index 32b6ea73..ca0b8d14 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/DriverResiliencePipelineBuilderTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/DriverResiliencePipelineBuilderTests.cs @@ -325,6 +325,37 @@ public sealed class DriverResiliencePipelineBuilderTests e.Message.Contains("host-9")); } + /// + /// 01/S-6 builder invariant: building a pipeline from a hostile, directly-constructed + /// (bypassing the parser) must NEVER throw Polly's + /// ValidationException. Sweeps the full matrix of {int.MinValue, -1, 0, 1, int.MaxValue} + /// across timeout/retry/breaker and asserts every built pipeline executes a trivial call. + /// + [Fact] + public async Task Build_NeverThrows_ForHostileDirectOptions() + { + int[] hostile = { int.MinValue, -1, 0, 1, int.MaxValue }; + var host = 0; + foreach (var t in hostile) + foreach (var r in hostile) + foreach (var b in hostile) + { + var options = new DriverResilienceOptions + { + Tier = DriverTier.A, + CapabilityPolicies = new Dictionary + { + [DriverCapability.Read] = new(TimeoutSeconds: t, RetryCount: r, BreakerFailureThreshold: b), + }, + }; + var builder = new DriverResiliencePipelineBuilder(); + var pipeline = builder.GetOrCreate("hostile", $"h{host++}", DriverCapability.Read, options); + + var result = await pipeline.ExecuteAsync(_ => ValueTask.FromResult(1), CancellationToken.None); + result.ShouldBe(1, $"pipeline built from hostile (t={t}, r={r}, b={b}) must execute, not throw"); + } + } + /// Minimal in-memory capturing (level, formatted-message) pairs so the /// resilience logging contract can be asserted without a real logging provider. private sealed class CapturingLogger : ILogger diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/ResilienceConfigBrickTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/ResilienceConfigBrickTests.cs new file mode 100644 index 00000000..4b1871d9 --- /dev/null +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/ResilienceConfigBrickTests.cs @@ -0,0 +1,48 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Core.Resilience; + +namespace ZB.MOM.WW.OtOpcUa.Core.Tests.Resilience; + +/// +/// Load-bearing regression guard for 01/S-6 — an operator-authorable out-of-range +/// ResilienceConfig value must NEVER brick a healthy driver. Before the R2-02 hardening pass, +/// "timeoutSeconds": 0 (→ TimeSpan.Zero, outside Polly's [10 ms, 24 h] window) and +/// "breakerFailureThreshold": 1 (below Polly's MinimumThroughput floor of 2) each made +/// throw ValidationException inside the +/// memoizing GetOrAdd factory on EVERY capability call for that (instance, host, +/// capability) — a permanent brick while the driver is healthy. The parser now clamps and the +/// builder belt-and-braces clamps, so a pipeline built from a parsed hostile config executes. +/// +[Trait("Category", "Unit")] +public sealed class ResilienceConfigBrickTests +{ + /// A parsed timeoutSeconds:0 override must not brick a capability call. + [Fact] + public async Task ParsedConfig_WithZeroTimeout_DoesNotBrickCapabilityCalls() + { + var options = DriverResilienceOptionsParser.ParseOrDefaults( + DriverTier.A, "{\"capabilityPolicies\":{\"Read\":{\"timeoutSeconds\":0}}}", out _); + + var pipeline = new DriverResiliencePipelineBuilder() + .GetOrCreate("i1", "h1", DriverCapability.Read, options); + + var result = await pipeline.ExecuteAsync(_ => ValueTask.FromResult(42), CancellationToken.None); + result.ShouldBe(42); + } + + /// A parsed breakerFailureThreshold:1 override must not brick a capability call. + [Fact] + public async Task ParsedConfig_WithBreakerThresholdOne_DoesNotBrickCapabilityCalls() + { + var options = DriverResilienceOptionsParser.ParseOrDefaults( + DriverTier.A, "{\"capabilityPolicies\":{\"Read\":{\"breakerFailureThreshold\":1}}}", out _); + + var pipeline = new DriverResiliencePipelineBuilder() + .GetOrCreate("i1", "h1", DriverCapability.Read, options); + + var result = await pipeline.ExecuteAsync(_ => ValueTask.FromResult(42), CancellationToken.None); + result.ShouldBe(42); + } +} From a8af9cc48bd09dc9e64362d9b0908ae81207b50d Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 09:57:17 -0400 Subject: [PATCH 04/17] =?UTF-8?q?fix(r2-02):=20factory-seam=20guard=20?= =?UTF-8?q?=E2=80=94=20hostile=20ResilienceConfig=20cannot=20brick=20the?= =?UTF-8?q?=20spawn=20path=20(01/S-6=20wiring=20proof)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ...lience-config-hardening-plan.md.tasks.json | 2 +- .../DriverCapabilityInvokerFactoryTests.cs | 25 +++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json b/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json index 6a0551fc..7fc196b3 100644 --- a/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json +++ b/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json @@ -5,7 +5,7 @@ { "id": 1, "subject": "Add ResiliencePolicyRanges constants to Core.Abstractions (single source of truth for parser/builder/AdminUI, derived from Polly.Core 8.6.6 validation)", "status": "completed", "blockedBy": [] }, { "id": 2, "subject": "S-6 parser: clamp timeout/retry/breaker overrides with appending diagnostics (timeout<=0 -> tier default; breaker==1 -> 2; caps) + 8 parser tests", "status": "completed", "blockedBy": [0, 1] }, { "id": 3, "subject": "S-6 builder: belt-and-braces clamp in Build so pipeline construction can never throw ValidationException + Build_NeverThrows_ForHostileDirectOptions (turns task 0 green)", "status": "completed", "blockedBy": [0, 1] }, - { "id": 4, "subject": "S-6 production-wiring guard: DriverCapabilityInvokerFactory.Create with hostile JSON yields a working invoker + logged clamp diagnostic; re-run ResilienceInvokerFactoryRegistrationTests", "status": "pending", "blockedBy": [2, 3] }, + { "id": 4, "subject": "S-6 production-wiring guard: DriverCapabilityInvokerFactory.Create with hostile JSON yields a working invoker + logged clamp diagnostic; re-run ResilienceInvokerFactoryRegistrationTests", "status": "completed", "blockedBy": [2, 3] }, { "id": 5, "subject": "S-6 AdminUI: ResilienceFormModel.Validate() range messages (via ResiliencePolicyRanges through the transitive Core.Abstractions ref) + warning block in DriverResilienceSection", "status": "pending", "blockedBy": [1] }, { "id": 6, "subject": "S-8 parser layer: force Write/AlarmAcknowledge retryCount overrides to 0 with diagnostic (spec invariant, honest dead-knob surfacing) + tests + GetTierDefaults doc", "status": "pending", "blockedBy": [2] }, { "id": 7, "subject": "S-8 dispatch layer: flip HandleWriteAsync to isIdempotent:false — RED the DriverInstanceActorResilienceWiringTests expectation first, then flip; full Runtime suite green", "status": "pending", "blockedBy": [6] }, diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/DriverCapabilityInvokerFactoryTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/DriverCapabilityInvokerFactoryTests.cs index eb2c58d5..04aeecc0 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/DriverCapabilityInvokerFactoryTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/DriverCapabilityInvokerFactoryTests.cs @@ -136,6 +136,31 @@ public sealed class DriverCapabilityInvokerFactoryTests logger.Entries.ShouldContain(e => e.Level == LogLevel.Warning && e.Message.Contains("resilience config", StringComparison.OrdinalIgnoreCase)); } + /// + /// 01/S-6 production-wiring proof: the exact DriverHostActor.SpawnChild path + /// (factory.Create(id, type, hostileJson)) must yield a WORKING invoker whose capability + /// call succeeds, and must log the clamp diagnostic as a warning — a hostile ResilienceConfig can + /// never brick the spawn path. + /// + [Fact] + public async Task Create_with_out_of_range_config_yields_working_invoker() + { + var logger = new CapturingLogger(); + var factory = MakeFactory(new DriverResiliencePipelineBuilder(), logger: logger); + + var invoker = factory.Create("i1", "Modbus", + resilienceConfigJson: "{\"capabilityPolicies\":{\"Subscribe\":{\"timeoutSeconds\":0,\"breakerFailureThreshold\":1}}}"); + + var result = await invoker.ExecuteAsync( + DriverCapability.Subscribe, "host-1", _ => ValueTask.FromResult(99), CancellationToken.None); + + result.ShouldBe(99, "a hostile ResilienceConfig must not brick the Subscribe capability call"); + logger.Entries.ShouldContain(e => + e.Level == LogLevel.Warning && + e.Message.Contains("resilience config", StringComparison.OrdinalIgnoreCase) && + (e.Message.Contains("timeoutSeconds") || e.Message.Contains("breakerFailureThreshold"))); + } + private sealed class CapturingLogger : ILogger { public List<(LogLevel Level, string Message)> Entries { get; } = new(); From 57c224c65ab2c12338a7823e8f77343ac4e9884b Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 10:00:43 -0400 Subject: [PATCH 05/17] fix(r2-02): range-validation warnings on the resilience override form (01/S-6 authoring layer) --- ...lience-config-hardening-plan.md.tasks.json | 2 +- .../Drivers/DriverResilienceSection.razor | 14 ++++++++ .../Shared/Drivers/ResilienceFormModel.cs | 33 +++++++++++++++++++ .../ResilienceFormModelTests.cs | 30 +++++++++++++++++ 4 files changed, 78 insertions(+), 1 deletion(-) diff --git a/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json b/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json index 7fc196b3..84181ada 100644 --- a/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json +++ b/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json @@ -6,7 +6,7 @@ { "id": 2, "subject": "S-6 parser: clamp timeout/retry/breaker overrides with appending diagnostics (timeout<=0 -> tier default; breaker==1 -> 2; caps) + 8 parser tests", "status": "completed", "blockedBy": [0, 1] }, { "id": 3, "subject": "S-6 builder: belt-and-braces clamp in Build so pipeline construction can never throw ValidationException + Build_NeverThrows_ForHostileDirectOptions (turns task 0 green)", "status": "completed", "blockedBy": [0, 1] }, { "id": 4, "subject": "S-6 production-wiring guard: DriverCapabilityInvokerFactory.Create with hostile JSON yields a working invoker + logged clamp diagnostic; re-run ResilienceInvokerFactoryRegistrationTests", "status": "completed", "blockedBy": [2, 3] }, - { "id": 5, "subject": "S-6 AdminUI: ResilienceFormModel.Validate() range messages (via ResiliencePolicyRanges through the transitive Core.Abstractions ref) + warning block in DriverResilienceSection", "status": "pending", "blockedBy": [1] }, + { "id": 5, "subject": "S-6 AdminUI: ResilienceFormModel.Validate() range messages (via ResiliencePolicyRanges through the transitive Core.Abstractions ref) + warning block in DriverResilienceSection", "status": "completed", "blockedBy": [1] }, { "id": 6, "subject": "S-8 parser layer: force Write/AlarmAcknowledge retryCount overrides to 0 with diagnostic (spec invariant, honest dead-knob surfacing) + tests + GetTierDefaults doc", "status": "pending", "blockedBy": [2] }, { "id": 7, "subject": "S-8 dispatch layer: flip HandleWriteAsync to isIdempotent:false — RED the DriverInstanceActorResilienceWiringTests expectation first, then flip; full Runtime suite green", "status": "pending", "blockedBy": [6] }, { "id": 8, "subject": "S-8/P-4 invoker: cache the no-retry options snapshot once per invoker + add RecordCallStart/Complete tracker accounting to the non-idempotent arm + 2 CapabilityInvokerTests", "status": "pending", "blockedBy": [7] }, diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DriverResilienceSection.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DriverResilienceSection.razor index 4a05503f..799957db 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DriverResilienceSection.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DriverResilienceSection.razor @@ -33,6 +33,20 @@ + @{ var _warnings = _m.Validate(); } + @if (_warnings.Count > 0) + { + + } +
Raw JSON (advanced) 
@(_m.ToJson() ?? "(null — all tier defaults)")
diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/ResilienceFormModel.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/ResilienceFormModel.cs index 6d70c4d0..851d7a6d 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/ResilienceFormModel.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/ResilienceFormModel.cs @@ -1,5 +1,6 @@ using System.Text.Json; using System.Text.Json.Serialization; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; namespace ZB.MOM.WW.OtOpcUa.AdminUI.Components.Shared.Drivers; @@ -39,6 +40,38 @@ public sealed class ResilienceFormModel public bool IsEmpty => TimeoutSeconds is null && RetryCount is null && BreakerFailureThreshold is null; } + /// + /// Range-validate the per-capability overrides against and return + /// one human-readable message per violation (naming capability + field + legal range). This is + /// authoring-time feedback, not the enforcement layer — the parser's clamp is the authoritative + /// guard, so the form still emits. Mirrors the driver tag editors' Validate() idiom (01/S-6). + /// + /// A list of range-violation messages; empty when every override is in range or blank. + public IReadOnlyList Validate() + { + var msgs = new List(); + foreach (var (cap, row) in Policies) + { + if (row.TimeoutSeconds is { } t && + (t < ResiliencePolicyRanges.MinTimeoutSeconds || t > ResiliencePolicyRanges.MaxTimeoutSeconds)) + msgs.Add($"{cap} timeout must be between {ResiliencePolicyRanges.MinTimeoutSeconds} and " + + $"{ResiliencePolicyRanges.MaxTimeoutSeconds} seconds (got {t})."); + + if (row.RetryCount is { } r && + (r < ResiliencePolicyRanges.MinRetryCount || r > ResiliencePolicyRanges.MaxRetryCount)) + msgs.Add($"{cap} retries must be between {ResiliencePolicyRanges.MinRetryCount} and " + + $"{ResiliencePolicyRanges.MaxRetryCount} (got {r})."); + + if (row.BreakerFailureThreshold is { } b && + (b < ResiliencePolicyRanges.MinBreakerFailureThreshold || b == 1 || + b > ResiliencePolicyRanges.MaxBreakerFailureThreshold)) + msgs.Add($"{cap} breaker threshold must be 0 (off) or between " + + $"{ResiliencePolicyRanges.MinEnabledBreakerFailureThreshold} and " + + $"{ResiliencePolicyRanges.MaxBreakerFailureThreshold} (got {b})."); + } + return msgs; + } + private static readonly JsonSerializerOptions ReadOpts = new() { PropertyNameCaseInsensitive = true }; private static readonly JsonSerializerOptions WriteOpts = new() { diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ResilienceFormModelTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ResilienceFormModelTests.cs index cb95310a..60773454 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ResilienceFormModelTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ResilienceFormModelTests.cs @@ -34,6 +34,36 @@ public class ResilienceFormModelTests m.Policies["Read"].IsEmpty.ShouldBeTrue(); } + [Fact] + public void Validate_flags_out_of_range_values_with_capability_and_range() + { + var m = new ResilienceFormModel(); + m.Policies["Read"].TimeoutSeconds = 0; // < MinTimeoutSeconds + m.Policies["Read"].RetryCount = -1; // < MinRetryCount + m.Policies["Read"].BreakerFailureThreshold = 1; // below Polly MinimumThroughput floor + + var msgs = m.Validate(); + + msgs.Count.ShouldBe(3); + msgs.ShouldContain(s => s.Contains("Read") && s.Contains("timeout")); + msgs.ShouldContain(s => s.Contains("Read") && s.Contains("retries")); + msgs.ShouldContain(s => s.Contains("Read") && s.Contains("breaker")); + } + + [Fact] + public void Validate_passes_in_range_and_blank() + { + var blank = new ResilienceFormModel(); + blank.Validate().ShouldBeEmpty(); + + var inRange = new ResilienceFormModel(); + inRange.Policies["Read"].TimeoutSeconds = 5; + inRange.Policies["Read"].RetryCount = 3; + inRange.Policies["Read"].BreakerFailureThreshold = 4; + inRange.Policies["Write"].BreakerFailureThreshold = 0; // 0 = breaker off, legal + inRange.Validate().ShouldBeEmpty(); + } + [Fact] public void Emitted_json_is_consumable_by_the_runtime_parser() { From eab0b6be1208d150cfe6a0c5965560aa375a7e47 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 10:01:54 -0400 Subject: [PATCH 06/17] fix(r2-02): enforce the spec's no-retry invariant for Write/AlarmAcknowledge overrides (01/S-8, 03/S12 layer 1) --- ...lience-config-hardening-plan.md.tasks.json | 2 +- .../Resilience/DriverResilienceOptions.cs | 6 ++- .../DriverResilienceOptionsParser.cs | 12 ++++++ .../DriverResilienceOptionsParserTests.cs | 37 +++++++++++++++++++ 4 files changed, 54 insertions(+), 3 deletions(-) diff --git a/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json b/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json index 84181ada..f7ef95bf 100644 --- a/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json +++ b/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json @@ -7,7 +7,7 @@ { "id": 3, "subject": "S-6 builder: belt-and-braces clamp in Build so pipeline construction can never throw ValidationException + Build_NeverThrows_ForHostileDirectOptions (turns task 0 green)", "status": "completed", "blockedBy": [0, 1] }, { "id": 4, "subject": "S-6 production-wiring guard: DriverCapabilityInvokerFactory.Create with hostile JSON yields a working invoker + logged clamp diagnostic; re-run ResilienceInvokerFactoryRegistrationTests", "status": "completed", "blockedBy": [2, 3] }, { "id": 5, "subject": "S-6 AdminUI: ResilienceFormModel.Validate() range messages (via ResiliencePolicyRanges through the transitive Core.Abstractions ref) + warning block in DriverResilienceSection", "status": "completed", "blockedBy": [1] }, - { "id": 6, "subject": "S-8 parser layer: force Write/AlarmAcknowledge retryCount overrides to 0 with diagnostic (spec invariant, honest dead-knob surfacing) + tests + GetTierDefaults doc", "status": "pending", "blockedBy": [2] }, + { "id": 6, "subject": "S-8 parser layer: force Write/AlarmAcknowledge retryCount overrides to 0 with diagnostic (spec invariant, honest dead-knob surfacing) + tests + GetTierDefaults doc", "status": "completed", "blockedBy": [2] }, { "id": 7, "subject": "S-8 dispatch layer: flip HandleWriteAsync to isIdempotent:false — RED the DriverInstanceActorResilienceWiringTests expectation first, then flip; full Runtime suite green", "status": "pending", "blockedBy": [6] }, { "id": 8, "subject": "S-8/P-4 invoker: cache the no-retry options snapshot once per invoker + add RecordCallStart/Complete tracker accounting to the non-idempotent arm + 2 CapabilityInvokerTests", "status": "pending", "blockedBy": [7] }, { "id": 9, "subject": "S-8 docs + form: fix WriteIdempotentAttribute's false 'reads via reflection' claim; note the non-idempotent production default; disable Write/Ack retry cells in the razor", "status": "pending", "blockedBy": [7, 5] }, diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResilienceOptions.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResilienceOptions.cs index ad97bca9..62629cf7 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResilienceOptions.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResilienceOptions.cs @@ -69,8 +69,10 @@ public sealed record DriverResilienceOptions /// /// Per-tier per-capability default policy table, per the Phase 6.1 - /// Stream A.2 specification. Retries skipped on and - /// regardless of tier. + /// Stream A.2 specification. The parser enforces no-retry on + /// and + /// as an invariant (R2-02/S-8) — a JSON retryCount override on those capabilities is forced back + /// to 0; per-tag opt-in via is the future relaxation. /// /// The driver tier to get defaults for. /// The default policy dictionary for the specified tier. diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResilienceOptionsParser.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResilienceOptionsParser.cs index a91b0285..e48e6211 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResilienceOptionsParser.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResilienceOptionsParser.cs @@ -160,6 +160,18 @@ public static class DriverResilienceOptionsParser retry = ResiliencePolicyRanges.MaxRetryCount; } + // Spec invariant (01/S-8 = 03/S12): Write and AlarmAcknowledge are write-shaped — a duplicate is + // not harmless (pulse coils, counter increments, Galaxy supervisory writes, alarm acks). Retries are + // "skipped regardless of tier" per Phase 6.1; the tier defaults are already 0, so only an override can + // differ — force it back to 0 here so the JSON→options funnel enforces the invariant. When per-tag + // opt-in via WriteIdempotentAttribute ships, this is the one line to relax. + if ((capability == DriverCapability.Write || capability == DriverCapability.AlarmAcknowledge) && retry > 0) + { + AppendDiagnostic(ref diagnostic, + $"{capability} never retries (writes are treated as non-idempotent until per-tag opt-in ships); retryCount override ignored."); + retry = 0; + } + var breaker = merged.BreakerFailureThreshold; if (breaker < ResiliencePolicyRanges.MinBreakerFailureThreshold) { diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/DriverResilienceOptionsParserTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/DriverResilienceOptionsParserTests.cs index 17f28e84..65f10beb 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/DriverResilienceOptionsParserTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/DriverResilienceOptionsParserTests.cs @@ -319,6 +319,43 @@ public sealed class DriverResilienceOptionsParserTests read.BreakerFailureThreshold.ShouldBe(ResiliencePolicyRanges.MinEnabledBreakerFailureThreshold); } + // ---- 01/S-8 = 03/S12: write-shaped capabilities never retry ---- + + /// A Write retryCount override is forced to 0 with a diagnostic (writes are non-idempotent). + [Fact] + public void WriteRetryOverride_IsForcedToZero_WithDiagnostic() + { + var options = DriverResilienceOptionsParser.ParseOrDefaults( + DriverTier.A, "{\"capabilityPolicies\":{\"Write\":{\"retryCount\":5}}}", out var diag); + + options.Resolve(DriverCapability.Write).RetryCount.ShouldBe(0); + diag.ShouldNotBeNull(); + diag.ShouldContain("Write never retries"); + } + + /// An AlarmAcknowledge retryCount override is forced to 0 with a diagnostic. + [Fact] + public void AlarmAcknowledgeRetryOverride_IsForcedToZero_WithDiagnostic() + { + var options = DriverResilienceOptionsParser.ParseOrDefaults( + DriverTier.A, "{\"capabilityPolicies\":{\"AlarmAcknowledge\":{\"retryCount\":3}}}", out var diag); + + options.Resolve(DriverCapability.AlarmAcknowledge).RetryCount.ShouldBe(0); + diag.ShouldNotBeNull(); + diag.ShouldContain("AlarmAcknowledge never retries"); + } + + /// A Read (idempotent) retryCount override is still honored — the invariant is write-shaped only. + [Fact] + public void ReadRetryOverride_StillHonored() + { + var options = DriverResilienceOptionsParser.ParseOrDefaults( + DriverTier.A, "{\"capabilityPolicies\":{\"Read\":{\"retryCount\":5}}}", out var diag); + + options.Resolve(DriverCapability.Read).RetryCount.ShouldBe(5); + diag.ShouldBeNull(); + } + /// In-range overrides pass through untouched with no diagnostic. [Fact] public void InRangeOverrides_PassThrough_NoDiagnostic() From 496f97da20f9b00b0f95165149df4b1fb1d744af Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 10:04:09 -0400 Subject: [PATCH 07/17] =?UTF-8?q?fix(r2-02):=20write=20dispatch=20defaults?= =?UTF-8?q?=20to=20non-idempotent=20=E2=80=94=20no-retry=20arm=20authorita?= =?UTF-8?q?tive=20(03/S12=20layer=202)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ...-resilience-config-hardening-plan.md.tasks.json | 2 +- .../Drivers/DriverInstanceActor.cs | 14 ++++++++------ .../DriverInstanceActorResilienceWiringTests.cs | 5 +++++ 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json b/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json index f7ef95bf..8341c48e 100644 --- a/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json +++ b/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json @@ -8,7 +8,7 @@ { "id": 4, "subject": "S-6 production-wiring guard: DriverCapabilityInvokerFactory.Create with hostile JSON yields a working invoker + logged clamp diagnostic; re-run ResilienceInvokerFactoryRegistrationTests", "status": "completed", "blockedBy": [2, 3] }, { "id": 5, "subject": "S-6 AdminUI: ResilienceFormModel.Validate() range messages (via ResiliencePolicyRanges through the transitive Core.Abstractions ref) + warning block in DriverResilienceSection", "status": "completed", "blockedBy": [1] }, { "id": 6, "subject": "S-8 parser layer: force Write/AlarmAcknowledge retryCount overrides to 0 with diagnostic (spec invariant, honest dead-knob surfacing) + tests + GetTierDefaults doc", "status": "completed", "blockedBy": [2] }, - { "id": 7, "subject": "S-8 dispatch layer: flip HandleWriteAsync to isIdempotent:false — RED the DriverInstanceActorResilienceWiringTests expectation first, then flip; full Runtime suite green", "status": "pending", "blockedBy": [6] }, + { "id": 7, "subject": "S-8 dispatch layer: flip HandleWriteAsync to isIdempotent:false — RED the DriverInstanceActorResilienceWiringTests expectation first, then flip; full Runtime suite green", "status": "completed", "blockedBy": [6] }, { "id": 8, "subject": "S-8/P-4 invoker: cache the no-retry options snapshot once per invoker + add RecordCallStart/Complete tracker accounting to the non-idempotent arm + 2 CapabilityInvokerTests", "status": "pending", "blockedBy": [7] }, { "id": 9, "subject": "S-8 docs + form: fix WriteIdempotentAttribute's false 'reads via reflection' claim; note the non-idempotent production default; disable Write/Ack retry cells in the razor", "status": "pending", "blockedBy": [7, 5] }, { "id": 10, "subject": "U-6 delete (Core): remove BulkheadMaxConcurrent/MaxQueue from DriverResilienceOptions + parser shape/merge/doc-example; add BulkheadKeys_InStoredJson_AreIgnored (migration-free contract)", "status": "pending", "blockedBy": [2, 6] }, diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs index 49de5c74..3405547f 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs @@ -589,14 +589,16 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(5)); try { - // Route through the resilience invoker (Write pipeline: timeout, no retry by default, - // per-host breaker). An OPC UA attribute set is idempotent (writing value X twice == once), - // so a configured Write retry is safe to apply; the Write tier default is RetryCount=0, so - // this stays behavior-neutral until an operator opts in via ResilienceConfig. The outer cts - // is retained as a hard backstop above the (typically tighter) tier timeout. + // Route through the resilience invoker (Write pipeline: timeout, per-host breaker). Writes + // default to NON-idempotent (03/S12): a timed-out write may already have committed at the + // device, and replaying a command-shaped write (pulse coil, counter increment, Galaxy + // supervisory write) can duplicate an irreversible field action. The invoker's no-retry arm is + // therefore authoritative regardless of any configured Write retry. Per-tag opt-in to retries via + // WriteIdempotentAttribute is the unshipped forward path (see WriteIdempotentAttribute). The outer + // cts is retained as a hard backstop above the (typically tighter) tier timeout. var results = await _invoker.ExecuteWriteAsync( ResolveHost(msg.TagId), - isIdempotent: true, + isIdempotent: false, async ct => await writable.WriteAsync(request, ct), cts.Token); if (results is { Count: 1 } && IsGoodStatus(results[0].StatusCode)) diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorResilienceWiringTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorResilienceWiringTests.cs index 323f3431..87aa6167 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorResilienceWiringTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorResilienceWiringTests.cs @@ -75,6 +75,9 @@ public sealed class DriverInstanceActorResilienceWiringTests : RuntimeActorTestB reply!.Success.ShouldBeTrue(); driver.Writes.Count.ShouldBeGreaterThanOrEqualTo(1); // the invoker's call-site actually ran invoker.Calls.ShouldContain(c => c.Capability == DriverCapability.Write && c.Host == "plc-7"); + // 03/S12: the dispatch site must default writes to NON-idempotent so the invoker's no-retry arm is + // authoritative — a command-shaped write (pulse, counter, supervisory) must never replay. + invoker.WriteCalls.ShouldContain(c => c.Host == "plc-7" && c.IsIdempotent == false); } /// Records every capability + resolved-host key routed through it, then delegates to the @@ -83,6 +86,7 @@ public sealed class DriverInstanceActorResilienceWiringTests : RuntimeActorTestB private sealed class RecordingInvoker : IDriverCapabilityInvoker { public ConcurrentQueue<(DriverCapability Capability, string Host)> Calls { get; } = new(); + public ConcurrentQueue<(string Host, bool IsIdempotent)> WriteCalls { get; } = new(); public ValueTask ExecuteAsync( DriverCapability capability, string hostName, @@ -105,6 +109,7 @@ public sealed class DriverInstanceActorResilienceWiringTests : RuntimeActorTestB Func> callSite, CancellationToken cancellationToken) { Calls.Enqueue((DriverCapability.Write, hostName)); + WriteCalls.Enqueue((hostName, isIdempotent)); return callSite(cancellationToken); } } From 20ff67bf9a2e98b2adb634da0affbd38ac3a742d Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 10:05:52 -0400 Subject: [PATCH 08/17] =?UTF-8?q?fix(r2-02):=20non-idempotent=20write=20ar?= =?UTF-8?q?m=20=E2=80=94=20cache=20the=20no-retry=20snapshot=20(01/P-4)=20?= =?UTF-8?q?and=20record=20tracker=20in-flight=20accounting=20(S-8=20residu?= =?UTF-8?q?al)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ...lience-config-hardening-plan.md.tasks.json | 2 +- .../Resilience/CapabilityInvoker.cs | 44 ++++++++++++----- .../Resilience/CapabilityInvokerTests.cs | 47 +++++++++++++++++++ 3 files changed, 79 insertions(+), 14 deletions(-) diff --git a/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json b/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json index 8341c48e..1f103093 100644 --- a/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json +++ b/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json @@ -9,7 +9,7 @@ { "id": 5, "subject": "S-6 AdminUI: ResilienceFormModel.Validate() range messages (via ResiliencePolicyRanges through the transitive Core.Abstractions ref) + warning block in DriverResilienceSection", "status": "completed", "blockedBy": [1] }, { "id": 6, "subject": "S-8 parser layer: force Write/AlarmAcknowledge retryCount overrides to 0 with diagnostic (spec invariant, honest dead-knob surfacing) + tests + GetTierDefaults doc", "status": "completed", "blockedBy": [2] }, { "id": 7, "subject": "S-8 dispatch layer: flip HandleWriteAsync to isIdempotent:false — RED the DriverInstanceActorResilienceWiringTests expectation first, then flip; full Runtime suite green", "status": "completed", "blockedBy": [6] }, - { "id": 8, "subject": "S-8/P-4 invoker: cache the no-retry options snapshot once per invoker + add RecordCallStart/Complete tracker accounting to the non-idempotent arm + 2 CapabilityInvokerTests", "status": "pending", "blockedBy": [7] }, + { "id": 8, "subject": "S-8/P-4 invoker: cache the no-retry options snapshot once per invoker + add RecordCallStart/Complete tracker accounting to the non-idempotent arm + 2 CapabilityInvokerTests", "status": "completed", "blockedBy": [7] }, { "id": 9, "subject": "S-8 docs + form: fix WriteIdempotentAttribute's false 'reads via reflection' claim; note the non-idempotent production default; disable Write/Ack retry cells in the razor", "status": "pending", "blockedBy": [7, 5] }, { "id": 10, "subject": "U-6 delete (Core): remove BulkheadMaxConcurrent/MaxQueue from DriverResilienceOptions + parser shape/merge/doc-example; add BulkheadKeys_InStoredJson_AreIgnored (migration-free contract)", "status": "pending", "blockedBy": [2, 6] }, { "id": 11, "subject": "U-6 delete (AdminUI): strip bulkhead fields from ResilienceFormModel + the two razor inputs; update ResilienceFormModelTests", "status": "pending", "blockedBy": [5, 10] }, diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/CapabilityInvoker.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/CapabilityInvoker.cs index 8b4f5b66..8e77b7de 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/CapabilityInvoker.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/CapabilityInvoker.cs @@ -28,6 +28,7 @@ public sealed class CapabilityInvoker : IDriverCapabilityInvoker private readonly string _driverType; private readonly Func _optionsAccessor; private readonly DriverResilienceStatusTracker? _statusTracker; + private DriverResilienceOptions? _noRetryWriteOptions; /// /// Construct an invoker for one driver instance. @@ -116,27 +117,44 @@ public sealed class CapabilityInvoker : IDriverCapabilityInvoker if (!isIdempotent) { - // Snapshot the options exactly once per call — invoking _optionsAccessor twice can - // (a) observe two different snapshots if an Admin edit lands between them and - // (b) wastes an allocation on the per-write hot path (Phase 6.1 1% pipeline budget). - var snapshot = _optionsAccessor(); - var noRetryOptions = snapshot with - { - CapabilityPolicies = new Dictionary - { - [DriverCapability.Write] = snapshot.Resolve(DriverCapability.Write) with { RetryCount = 0 }, - }, - }; + // Build the no-retry options snapshot ONCE per invoker lifetime (01/P-4). The options are + // immutable for the invoker's lifetime — a ResilienceConfig change respawns the driver child, + // which constructs a fresh invoker (#13 respawn-on-change) — so lazily caching the snapshot is + // safe and removes the per-write allocation the previous once-per-call snapshot incurred. The + // null-check-assign tolerates a benign first-call race (both threads build an equivalent record). + var noRetryOptions = _noRetryWriteOptions ??= BuildNoRetryWriteOptions(); var pipeline = _builder.GetOrCreate(_driverInstanceId, $"{hostName}::non-idempotent", DriverCapability.Write, noRetryOptions); - using (LogContextEnricher.Push(_driverInstanceId, _driverType, DriverCapability.Write, LogContextEnricher.NewCorrelationId())) + // Record in-flight accounting on the CALLER's host (the "::non-idempotent" suffix is a + // pipeline-cache key, not an operator-facing host) so Admin /hosts sees write depth too. + _statusTracker?.RecordCallStart(_driverInstanceId, hostName); + try { - return await pipeline.ExecuteAsync(callSite, cancellationToken).ConfigureAwait(false); + using (LogContextEnricher.Push(_driverInstanceId, _driverType, DriverCapability.Write, LogContextEnricher.NewCorrelationId())) + { + return await pipeline.ExecuteAsync(callSite, cancellationToken).ConfigureAwait(false); + } + } + finally + { + _statusTracker?.RecordCallComplete(_driverInstanceId, hostName); } } return await ExecuteAsync(DriverCapability.Write, hostName, callSite, cancellationToken).ConfigureAwait(false); } + private DriverResilienceOptions BuildNoRetryWriteOptions() + { + var snapshot = _optionsAccessor(); + return snapshot with + { + CapabilityPolicies = new Dictionary + { + [DriverCapability.Write] = snapshot.Resolve(DriverCapability.Write) with { RetryCount = 0 }, + }, + }; + } + private ResiliencePipeline ResolvePipeline(DriverCapability capability, string hostName) => _builder.GetOrCreate(_driverInstanceId, hostName, capability, _optionsAccessor()); } diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/CapabilityInvokerTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/CapabilityInvokerTests.cs index 277a2307..68a4d00f 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/CapabilityInvokerTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/CapabilityInvokerTests.cs @@ -188,6 +188,53 @@ public sealed class CapabilityInvokerTests "ExecuteWriteAsync's non-idempotent branch must capture the options snapshot exactly once per call"); } + /// + /// S-8 residual: the non-idempotent write arm must record tracker in-flight accounting (start + + /// complete) on the caller's host, exactly like the idempotent arms — the previous arm skipped it. + /// + [Fact] + public async Task NonIdempotentWrite_RecordsTrackerStartAndComplete() + { + var tracker = new DriverResilienceStatusTracker(); + var invoker = new CapabilityInvoker( + new DriverResiliencePipelineBuilder(), + "drv-test", + () => new DriverResilienceOptions { Tier = DriverTier.A }, + statusTracker: tracker); + + await invoker.ExecuteWriteAsync( + "host-1", + isIdempotent: false, + _ => ValueTask.FromResult(0), + CancellationToken.None); + + var snap = tracker.TryGet("drv-test", "host-1"); + snap.ShouldNotBeNull("the non-idempotent write arm must record tracker accounting on the caller's host"); + snap!.CurrentInFlight.ShouldBe(0, "in-flight must return to 0 after the write completes"); + } + + /// + /// 01/P-4: the non-idempotent write arm must build the no-retry options snapshot ONCE per invoker + /// lifetime (not per call). Two writes to the same host reuse the cached snapshot — the options + /// accessor is invoked at most once and only one pipeline is cached. + /// + [Fact] + public async Task NonIdempotentWrite_ReusesCachedNoRetryOptions() + { + var builder = new DriverResiliencePipelineBuilder(); + var accessorCalls = 0; + var invoker = new CapabilityInvoker( + builder, + "drv-test", + () => { Interlocked.Increment(ref accessorCalls); return new DriverResilienceOptions { Tier = DriverTier.A }; }); + + await invoker.ExecuteWriteAsync("host-1", isIdempotent: false, _ => ValueTask.FromResult(0), CancellationToken.None); + await invoker.ExecuteWriteAsync("host-1", isIdempotent: false, _ => ValueTask.FromResult(0), CancellationToken.None); + + accessorCalls.ShouldBeLessThanOrEqualTo(1, "the no-retry snapshot must be cached across calls"); + builder.CachedPipelineCount.ShouldBe(1, "two writes to the same host share one cached no-retry pipeline"); + } + /// /// Core-009 regression — companion consistency assertion: the non-idempotent branch must /// not observe two different option snapshots even if the accessor's returned value changes From 99b424240ae32cadcff4d598fc9d64159b2424e6 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 10:07:01 -0400 Subject: [PATCH 09/17] docs(r2-02): correct WriteIdempotentAttribute's unwired claim; disable Write/Ack retry authoring cells (S-8) --- .../R2-02-resilience-config-hardening-plan.md.tasks.json | 2 +- .../IDriverCapabilityInvoker.cs | 5 ++++- .../WriteIdempotentAttribute.cs | 8 +++++--- .../Shared/Drivers/DriverResilienceSection.razor | 3 ++- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json b/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json index 1f103093..16887f2e 100644 --- a/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json +++ b/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json @@ -10,7 +10,7 @@ { "id": 6, "subject": "S-8 parser layer: force Write/AlarmAcknowledge retryCount overrides to 0 with diagnostic (spec invariant, honest dead-knob surfacing) + tests + GetTierDefaults doc", "status": "completed", "blockedBy": [2] }, { "id": 7, "subject": "S-8 dispatch layer: flip HandleWriteAsync to isIdempotent:false — RED the DriverInstanceActorResilienceWiringTests expectation first, then flip; full Runtime suite green", "status": "completed", "blockedBy": [6] }, { "id": 8, "subject": "S-8/P-4 invoker: cache the no-retry options snapshot once per invoker + add RecordCallStart/Complete tracker accounting to the non-idempotent arm + 2 CapabilityInvokerTests", "status": "completed", "blockedBy": [7] }, - { "id": 9, "subject": "S-8 docs + form: fix WriteIdempotentAttribute's false 'reads via reflection' claim; note the non-idempotent production default; disable Write/Ack retry cells in the razor", "status": "pending", "blockedBy": [7, 5] }, + { "id": 9, "subject": "S-8 docs + form: fix WriteIdempotentAttribute's false 'reads via reflection' claim; note the non-idempotent production default; disable Write/Ack retry cells in the razor", "status": "completed", "blockedBy": [7, 5] }, { "id": 10, "subject": "U-6 delete (Core): remove BulkheadMaxConcurrent/MaxQueue from DriverResilienceOptions + parser shape/merge/doc-example; add BulkheadKeys_InStoredJson_AreIgnored (migration-free contract)", "status": "pending", "blockedBy": [2, 6] }, { "id": 11, "subject": "U-6 delete (AdminUI): strip bulkhead fields from ResilienceFormModel + the two razor inputs; update ResilienceFormModelTests", "status": "pending", "blockedBy": [5, 10] }, { "id": 12, "subject": "U-6 doc sweep (seam xmldocs, OTOPCUA0001 message + analyzer tests, operator docs; leave migrations/WedgeDetector/historical plans) + Options_properties_are_exactly_the_pipeline_wired_set knob-inertness guard", "status": "pending", "blockedBy": [10, 11] }, diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IDriverCapabilityInvoker.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IDriverCapabilityInvoker.cs index 348113a5..f3c60bc5 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IDriverCapabilityInvoker.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IDriverCapabilityInvoker.cs @@ -48,7 +48,10 @@ public interface IDriverCapabilityInvoker /// Execute a call honoring idempotence semantics — when /// is false, retries are disabled regardless of the /// configured policy; when true, the call runs through the Write pipeline which may - /// retry if the tier configuration permits. + /// retry if the tier configuration permits. The current production default is false + /// (R2-02/S-8): DriverInstanceActor.HandleWriteAsync passes isIdempotent: false so a + /// command-shaped write is never replayed; per-tag opt-in via + /// is the unshipped forward path. /// /// Return type of the underlying driver write call. /// The resolved host name for pipeline keying + status tracking. diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/WriteIdempotentAttribute.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/WriteIdempotentAttribute.cs index 900e83c7..1854e6a2 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/WriteIdempotentAttribute.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/WriteIdempotentAttribute.cs @@ -9,9 +9,11 @@ namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions; /// /// Applied to tag-definition POCOs /// (e.g. ModbusTagDefinition, S7TagDefinition, OPC UA client tag rows) at the -/// property or record level. The CapabilityInvoker in ZB.MOM.WW.OtOpcUa.Core.Resilience -/// reads this attribute via reflection once at driver-init time and caches the result; no -/// per-write reflection cost. +/// property or record level. Reserved for the per-tag opt-in; NOT yet consulted (R2-02/S-8) — +/// the dispatch site (DriverInstanceActor.HandleWriteAsync) currently treats every write as +/// non-idempotent, so no tag retries a write today. Threading tag-definition metadata to the dispatch +/// seam is the documented forward path; when it ships, the CapabilityInvoker non-idempotent arm +/// and the parser's Write/AlarmAcknowledge no-retry clamp are the two sites that relax. /// [AttributeUsage(AttributeTargets.Property | AttributeTargets.Class | AttributeTargets.Struct, AllowMultiple = false, Inherited = true)] public sealed class WriteIdempotentAttribute : Attribute diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DriverResilienceSection.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DriverResilienceSection.razor index 799957db..8936f748 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DriverResilienceSection.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DriverResilienceSection.razor @@ -22,10 +22,11 @@ @foreach (var cap in ResilienceFormModel.Capabilities) { var row = _m.Policies[cap]; + var writeShaped = cap is "Write" or "AlarmAcknowledge"; @cap - + } From 56854139c992119b803d51f896b62e3330a32730 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 10:08:27 -0400 Subject: [PATCH 10/17] =?UTF-8?q?refactor(r2-02):=20delete=20the=20dead=20?= =?UTF-8?q?bulkhead=20knob=20=E2=80=94=20options=20+=20parser=20(01/U-6);?= =?UTF-8?q?=20stored=20JSON=20keys=20become=20ignored=20unknowns?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ...lience-config-hardening-plan.md.tasks.json | 2 +- .../Resilience/DriverResilienceOptions.cs | 9 ------- .../DriverResilienceOptionsParser.cs | 8 ------- .../DriverResilienceOptionsParserTests.cs | 24 ++++++++++++------- 4 files changed, 16 insertions(+), 27 deletions(-) diff --git a/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json b/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json index 16887f2e..7a9d626a 100644 --- a/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json +++ b/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json @@ -11,7 +11,7 @@ { "id": 7, "subject": "S-8 dispatch layer: flip HandleWriteAsync to isIdempotent:false — RED the DriverInstanceActorResilienceWiringTests expectation first, then flip; full Runtime suite green", "status": "completed", "blockedBy": [6] }, { "id": 8, "subject": "S-8/P-4 invoker: cache the no-retry options snapshot once per invoker + add RecordCallStart/Complete tracker accounting to the non-idempotent arm + 2 CapabilityInvokerTests", "status": "completed", "blockedBy": [7] }, { "id": 9, "subject": "S-8 docs + form: fix WriteIdempotentAttribute's false 'reads via reflection' claim; note the non-idempotent production default; disable Write/Ack retry cells in the razor", "status": "completed", "blockedBy": [7, 5] }, - { "id": 10, "subject": "U-6 delete (Core): remove BulkheadMaxConcurrent/MaxQueue from DriverResilienceOptions + parser shape/merge/doc-example; add BulkheadKeys_InStoredJson_AreIgnored (migration-free contract)", "status": "pending", "blockedBy": [2, 6] }, + { "id": 10, "subject": "U-6 delete (Core): remove BulkheadMaxConcurrent/MaxQueue from DriverResilienceOptions + parser shape/merge/doc-example; add BulkheadKeys_InStoredJson_AreIgnored (migration-free contract)", "status": "completed", "blockedBy": [2, 6] }, { "id": 11, "subject": "U-6 delete (AdminUI): strip bulkhead fields from ResilienceFormModel + the two razor inputs; update ResilienceFormModelTests", "status": "pending", "blockedBy": [5, 10] }, { "id": 12, "subject": "U-6 doc sweep (seam xmldocs, OTOPCUA0001 message + analyzer tests, operator docs; leave migrations/WedgeDetector/historical plans) + Options_properties_are_exactly_the_pipeline_wired_set knob-inertness guard", "status": "pending", "blockedBy": [10, 11] }, { "id": 13, "subject": "C-7 RED tests: unknown top-level key / unknown capability / unknown per-policy field survive round-trip; malformed JSON sets ParseFailed and ToJson returns the original text", "status": "pending", "blockedBy": [11] }, diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResilienceOptions.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResilienceOptions.cs index 62629cf7..242cb44d 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResilienceOptions.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResilienceOptions.cs @@ -19,15 +19,6 @@ public sealed record DriverResilienceOptions public IReadOnlyDictionary CapabilityPolicies { get; init; } = new Dictionary(); - /// Bulkhead (max concurrent in-flight calls) for every capability. Default 32. - public int BulkheadMaxConcurrent { get; init; } = 32; - - /// - /// Bulkhead queue depth. Zero = no queueing; overflow fails fast with - /// BulkheadRejectedException. Default 64. - /// - public int BulkheadMaxQueue { get; init; } = 64; - /// /// Periodic scheduled recycle interval for Tier C out-of-process hosts, in seconds. /// Null (the default) = no scheduled recycle; the driver's Host process keeps running diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResilienceOptionsParser.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResilienceOptionsParser.cs index e48e6211..658193ba 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResilienceOptionsParser.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResilienceOptionsParser.cs @@ -13,8 +13,6 @@ namespace ZB.MOM.WW.OtOpcUa.Core.Resilience; /// Example JSON shape per Phase 6.1 Stream A.2: /// /// { -/// "bulkheadMaxConcurrent": 16, -/// "bulkheadMaxQueue": 64, /// "capabilityPolicies": { /// "Read": { "timeoutSeconds": 5, "retryCount": 5, "breakerFailureThreshold": 3 }, /// "Write": { "timeoutSeconds": 5, "retryCount": 0, "breakerFailureThreshold": 5 } @@ -114,8 +112,6 @@ public static class DriverResilienceOptionsParser { Tier = tier, CapabilityPolicies = merged, - BulkheadMaxConcurrent = shape.BulkheadMaxConcurrent ?? baseOptions.BulkheadMaxConcurrent, - BulkheadMaxQueue = shape.BulkheadMaxQueue ?? baseOptions.BulkheadMaxQueue, RecycleIntervalSeconds = recycleIntervalSeconds, }; } @@ -200,10 +196,6 @@ public static class DriverResilienceOptionsParser private sealed class ResilienceConfigShape { - /// Gets or sets the maximum concurrent bulkhead requests. - public int? BulkheadMaxConcurrent { get; set; } - /// Gets or sets the maximum bulkhead queue size. - public int? BulkheadMaxQueue { get; set; } /// Gets or sets the scheduled recycle interval in seconds. public int? RecycleIntervalSeconds { get; set; } /// Gets or sets the per-capability resilience policies. diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/DriverResilienceOptionsParserTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/DriverResilienceOptionsParserTests.cs index 65f10beb..7031ba0d 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/DriverResilienceOptionsParserTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/DriverResilienceOptionsParserTests.cs @@ -98,18 +98,23 @@ public sealed class DriverResilienceOptionsParserTests read.BreakerFailureThreshold.ShouldBe(tierDefault.BreakerFailureThreshold); } - /// Verifies that bulkhead overrides are honored. + /// + /// 01/U-6: the deleted bulkhead knob's keys in stored JSON become ignored unknown keys — a + /// config carrying them parses clean with no diagnostic (migration-free contract). + /// [Fact] - public void BulkheadOverrides_AreHonored() + public void BulkheadKeys_InStoredJson_AreIgnored() { var json = """ { "bulkheadMaxConcurrent": 100, "bulkheadMaxQueue": 500 } """; - var options = DriverResilienceOptionsParser.ParseOrDefaults(DriverTier.B, json, out _); + var options = DriverResilienceOptionsParser.ParseOrDefaults(DriverTier.B, json, out var diag); - options.BulkheadMaxConcurrent.ShouldBe(100); - options.BulkheadMaxQueue.ShouldBe(500); + diag.ShouldBeNull(); + // Known capabilities untouched — the stale bulkhead keys are simply ignored. + options.Resolve(DriverCapability.Read).ShouldBe( + DriverResilienceOptions.GetTierDefaults(DriverTier.B)[DriverCapability.Read]); } /// Verifies that unknown capability surfaces in diagnostic but does not fail. @@ -133,17 +138,18 @@ public sealed class DriverResilienceOptionsParserTests DriverResilienceOptions.GetTierDefaults(DriverTier.A)[DriverCapability.Read]); } - /// Verifies that property names are case insensitive. + /// Verifies that top-level property names are case insensitive. [Fact] public void PropertyNames_AreCaseInsensitive() { var json = """ - { "BULKHEADMAXCONCURRENT": 42 } + { "RECYCLEINTERVALSECONDS": 3600 } """; - var options = DriverResilienceOptionsParser.ParseOrDefaults(DriverTier.A, json, out _); + var options = DriverResilienceOptionsParser.ParseOrDefaults(DriverTier.C, json, out var diag); - options.BulkheadMaxConcurrent.ShouldBe(42); + diag.ShouldBeNull(); + options.RecycleIntervalSeconds.ShouldBe(3600); } /// Verifies that capability name is case insensitive. From 524a0b56065dfc36ae481aa07f732254e6ded75d Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 10:10:06 -0400 Subject: [PATCH 11/17] refactor(r2-02): remove bulkhead fields from the resilience form (01/U-6) --- ...resilience-config-hardening-plan.md.tasks.json | 2 +- .../Shared/Drivers/DriverResilienceSection.razor | 4 ---- .../Shared/Drivers/ResilienceFormModel.cs | 15 +-------------- .../ResilienceFormModelTests.cs | 9 ++++----- 4 files changed, 6 insertions(+), 24 deletions(-) diff --git a/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json b/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json index 7a9d626a..2db7944a 100644 --- a/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json +++ b/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json @@ -12,7 +12,7 @@ { "id": 8, "subject": "S-8/P-4 invoker: cache the no-retry options snapshot once per invoker + add RecordCallStart/Complete tracker accounting to the non-idempotent arm + 2 CapabilityInvokerTests", "status": "completed", "blockedBy": [7] }, { "id": 9, "subject": "S-8 docs + form: fix WriteIdempotentAttribute's false 'reads via reflection' claim; note the non-idempotent production default; disable Write/Ack retry cells in the razor", "status": "completed", "blockedBy": [7, 5] }, { "id": 10, "subject": "U-6 delete (Core): remove BulkheadMaxConcurrent/MaxQueue from DriverResilienceOptions + parser shape/merge/doc-example; add BulkheadKeys_InStoredJson_AreIgnored (migration-free contract)", "status": "completed", "blockedBy": [2, 6] }, - { "id": 11, "subject": "U-6 delete (AdminUI): strip bulkhead fields from ResilienceFormModel + the two razor inputs; update ResilienceFormModelTests", "status": "pending", "blockedBy": [5, 10] }, + { "id": 11, "subject": "U-6 delete (AdminUI): strip bulkhead fields from ResilienceFormModel + the two razor inputs; update ResilienceFormModelTests", "status": "completed", "blockedBy": [5, 10] }, { "id": 12, "subject": "U-6 doc sweep (seam xmldocs, OTOPCUA0001 message + analyzer tests, operator docs; leave migrations/WedgeDetector/historical plans) + Options_properties_are_exactly_the_pipeline_wired_set knob-inertness guard", "status": "pending", "blockedBy": [10, 11] }, { "id": 13, "subject": "C-7 RED tests: unknown top-level key / unknown capability / unknown per-policy field survive round-trip; malformed JSON sets ParseFailed and ToJson returns the original text", "status": "pending", "blockedBy": [11] }, { "id": 14, "subject": "C-7 implement: JsonObject-bag FromJson/ToJson (TagConfigJson idiom), ParseFailed + RawStoredJson, blank->null contract preserved, parser-interop test green", "status": "pending", "blockedBy": [13] }, diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DriverResilienceSection.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DriverResilienceSection.razor index 8936f748..4326c428 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DriverResilienceSection.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DriverResilienceSection.razor @@ -7,10 +7,6 @@ (see docs/v2/driver-stability.md). Set only what you need to override.

-
-
-
-
diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/ResilienceFormModel.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/ResilienceFormModel.cs index 851d7a6d..fa9aeb7e 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/ResilienceFormModel.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/ResilienceFormModel.cs @@ -15,10 +15,6 @@ public sealed class ResilienceFormModel public static readonly string[] Capabilities = ["Read", "Write", "Discover", "Subscribe", "Probe", "AlarmSubscribe", "AlarmAcknowledge", "HistoryRead"]; - /// Gets or sets the bulkhead max-concurrency override; null = use the tier default. - public int? BulkheadMaxConcurrent { get; set; } - /// Gets or sets the bulkhead max-queue-length override; null = use the tier default. - public int? BulkheadMaxQueue { get; set; } /// Gets or sets the driver recycle-interval override, in seconds; null = use the tier default. public int? RecycleIntervalSeconds { get; set; } @@ -92,8 +88,6 @@ public sealed class ResilienceFormModel catch (JsonException) { return model; } // malformed -> empty form; raw view (next task) shows the text if (shape is null) return model; - model.BulkheadMaxConcurrent = shape.BulkheadMaxConcurrent; - model.BulkheadMaxQueue = shape.BulkheadMaxQueue; model.RecycleIntervalSeconds = shape.RecycleIntervalSeconds; if (shape.CapabilityPolicies is not null) foreach (var (cap, p) in shape.CapabilityPolicies) @@ -119,14 +113,11 @@ public sealed class ResilienceFormModel BreakerFailureThreshold = kv.Value.BreakerFailureThreshold, }); - var hasAny = BulkheadMaxConcurrent is not null || BulkheadMaxQueue is not null - || RecycleIntervalSeconds is not null || caps.Count > 0; + var hasAny = RecycleIntervalSeconds is not null || caps.Count > 0; if (!hasAny) return null; var shape = new Shape { - BulkheadMaxConcurrent = BulkheadMaxConcurrent, - BulkheadMaxQueue = BulkheadMaxQueue, RecycleIntervalSeconds = RecycleIntervalSeconds, CapabilityPolicies = caps.Count > 0 ? caps : null, }; @@ -136,10 +127,6 @@ public sealed class ResilienceFormModel /// Wire shape of the resilience-override JSON, as consumed by DriverResilienceOptionsParser. private sealed class Shape { - /// Gets or sets the bulkhead max-concurrency override. - public int? BulkheadMaxConcurrent { get; set; } - /// Gets or sets the bulkhead max-queue-length override. - public int? BulkheadMaxQueue { get; set; } /// Gets or sets the driver recycle-interval override, in seconds. public int? RecycleIntervalSeconds { get; set; } /// Gets or sets the per-capability overrides, keyed by capability name. diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ResilienceFormModelTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ResilienceFormModelTests.cs index 60773454..301797b7 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ResilienceFormModelTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ResilienceFormModelTests.cs @@ -13,7 +13,7 @@ public class ResilienceFormModelTests [Fact] public void Partial_override_round_trips() { - var m = new ResilienceFormModel { BulkheadMaxConcurrent = 16 }; + var m = new ResilienceFormModel { RecycleIntervalSeconds = 3600 }; m.Policies["Read"].TimeoutSeconds = 5; m.Policies["Read"].RetryCount = 5; @@ -21,7 +21,7 @@ public class ResilienceFormModelTests json.ShouldNotBeNull(); var back = ResilienceFormModel.FromJson(json); - back.BulkheadMaxConcurrent.ShouldBe(16); + back.RecycleIntervalSeconds.ShouldBe(3600); back.Policies["Read"].TimeoutSeconds.ShouldBe(5); back.Policies["Write"].IsEmpty.ShouldBeTrue(); } @@ -30,7 +30,7 @@ public class ResilienceFormModelTests public void Malformed_json_yields_empty_model() { var m = ResilienceFormModel.FromJson("{ not valid json"); - m.BulkheadMaxConcurrent.ShouldBeNull(); + m.RecycleIntervalSeconds.ShouldBeNull(); m.Policies["Read"].IsEmpty.ShouldBeTrue(); } @@ -67,12 +67,11 @@ public class ResilienceFormModelTests [Fact] public void Emitted_json_is_consumable_by_the_runtime_parser() { - var m = new ResilienceFormModel { BulkheadMaxConcurrent = 16 }; + var m = new ResilienceFormModel(); m.Policies["Read"].TimeoutSeconds = 7; var opts = DriverResilienceOptionsParser.ParseOrDefaults(DriverTier.B, m.ToJson(), out var diag); diag.ShouldBeNull(); - opts.BulkheadMaxConcurrent.ShouldBe(16); opts.Resolve(DriverCapability.Read).TimeoutSeconds.ShouldBe(7); opts.Resolve(DriverCapability.Write).RetryCount.ShouldBe(0); } From 07eec11d31d3127c0addc5d52fbc0ab1857d4652 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 10:15:32 -0400 Subject: [PATCH 12/17] docs+test(r2-02): bulkhead doc sweep + parsed-knob-must-be-wired guard (01/U-6, OVERALL theme 1) --- ...lience-config-hardening-plan.md.tasks.json | 2 +- docs/OpcUaServer.md | 2 +- docs/ReadWriteOperations.md | 2 +- docs/security.md | 2 +- docs/v1/HistoricalDataAccess.md | 2 +- .../Entities/DriverInstance.cs | 2 -- .../DriverInstanceResilienceStatus.cs | 4 ++- .../IDriverCapabilityInvoker.cs | 4 +-- .../IPerCallHostResolver.cs | 2 +- .../Resilience/AlarmSurfaceInvoker.cs | 2 +- .../Resilience/CapabilityInvoker.cs | 2 +- .../DriverResiliencePipelineBuilder.cs | 4 +-- .../DriverResilienceStatusTracker.cs | 4 +-- .../AbCipDriver.cs | 2 +- .../AbCipHostAddress.cs | 2 +- .../FocasDriverOptions.cs | 2 +- .../Drivers/DriverHostActor.cs | 2 +- .../Drivers/DriverInstanceActor.cs | 2 +- .../UnwrappedCapabilityCallAnalyzer.cs | 4 +-- .../DriverResilienceOptionsTests.cs | 28 +++++++++++++++++++ 20 files changed, 52 insertions(+), 24 deletions(-) diff --git a/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json b/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json index 2db7944a..764340fc 100644 --- a/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json +++ b/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json @@ -13,7 +13,7 @@ { "id": 9, "subject": "S-8 docs + form: fix WriteIdempotentAttribute's false 'reads via reflection' claim; note the non-idempotent production default; disable Write/Ack retry cells in the razor", "status": "completed", "blockedBy": [7, 5] }, { "id": 10, "subject": "U-6 delete (Core): remove BulkheadMaxConcurrent/MaxQueue from DriverResilienceOptions + parser shape/merge/doc-example; add BulkheadKeys_InStoredJson_AreIgnored (migration-free contract)", "status": "completed", "blockedBy": [2, 6] }, { "id": 11, "subject": "U-6 delete (AdminUI): strip bulkhead fields from ResilienceFormModel + the two razor inputs; update ResilienceFormModelTests", "status": "completed", "blockedBy": [5, 10] }, - { "id": 12, "subject": "U-6 doc sweep (seam xmldocs, OTOPCUA0001 message + analyzer tests, operator docs; leave migrations/WedgeDetector/historical plans) + Options_properties_are_exactly_the_pipeline_wired_set knob-inertness guard", "status": "pending", "blockedBy": [10, 11] }, + { "id": 12, "subject": "U-6 doc sweep (seam xmldocs, OTOPCUA0001 message + analyzer tests, operator docs; leave migrations/WedgeDetector/historical plans) + Options_properties_are_exactly_the_pipeline_wired_set knob-inertness guard", "status": "completed", "blockedBy": [10, 11] }, { "id": 13, "subject": "C-7 RED tests: unknown top-level key / unknown capability / unknown per-policy field survive round-trip; malformed JSON sets ParseFailed and ToJson returns the original text", "status": "pending", "blockedBy": [11] }, { "id": 14, "subject": "C-7 implement: JsonObject-bag FromJson/ToJson (TagConfigJson idiom), ParseFailed + RawStoredJson, blank->null contract preserved, parser-interop test green", "status": "pending", "blockedBy": [13] }, { "id": 15, "subject": "C-7 razor: ParseFailed warning banner + input lockout + explicit discard button; raw pane shows the stored ResilienceConfig; live-/run on docker-dev :9200 (rebuild BOTH centrals; SQL-mangle + unknown-key survival checks; also drive task 5 warnings and task 9 disabled cells)", "status": "pending", "blockedBy": [14] }, diff --git a/docs/OpcUaServer.md b/docs/OpcUaServer.md index f2f2681f..f883e949 100644 --- a/docs/OpcUaServer.md +++ b/docs/OpcUaServer.md @@ -19,7 +19,7 @@ The lifecycle facade `OpcUaApplicationHost` (`src/Server/ZB.MOM.WW.OtOpcUa.OpcUa ## Resilience and capability dispatch -Driver-capability calls (`IReadable.ReadAsync`, `IWritable.WriteAsync`, `ITagDiscovery.DiscoverAsync`, `ISubscribable.SubscribeAsync/UnsubscribeAsync`, the `IHostConnectivityProbe` probe loop, `IAlarmSource` surfaces, and the four `IHistoryProvider` reads) are routed through a `CapabilityInvoker` (`src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/CapabilityInvoker.cs`) so the Polly resilience pipeline (retry / timeout / breaker / bulkhead) applies. There is one invoker per `(DriverInstance, IDriver)` pair; all invokers share the process-singleton `DriverResiliencePipelineBuilder`, which keys pipelines on `(DriverInstanceId, hostName, DriverCapability)`. Per-instance resilience options come from `DriverTypeRegistry` (the driver's tier) plus per-instance JSON overrides parsed from `DriverInstance.ResilienceConfig` by `DriverResilienceOptionsParser`. +Driver-capability calls (`IReadable.ReadAsync`, `IWritable.WriteAsync`, `ITagDiscovery.DiscoverAsync`, `ISubscribable.SubscribeAsync/UnsubscribeAsync`, the `IHostConnectivityProbe` probe loop, `IAlarmSource` surfaces, and the four `IHistoryProvider` reads) are routed through a `CapabilityInvoker` (`src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/CapabilityInvoker.cs`) so the Polly resilience pipeline (retry / timeout / breaker) applies. There is one invoker per `(DriverInstance, IDriver)` pair; all invokers share the process-singleton `DriverResiliencePipelineBuilder`, which keys pipelines on `(DriverInstanceId, hostName, DriverCapability)`. Per-instance resilience options come from `DriverTypeRegistry` (the driver's tier) plus per-instance JSON overrides parsed from `DriverInstance.ResilienceConfig` by `DriverResilienceOptionsParser`. The `OTOPCUA0001` Roslyn analyzer (`src/Tooling/ZB.MOM.WW.OtOpcUa.Analyzers/UnwrappedCapabilityCallAnalyzer.cs`, category `OtOpcUa.Resilience`, severity Warning) flags direct driver-capability calls that bypass the invoker. diff --git a/docs/ReadWriteOperations.md b/docs/ReadWriteOperations.md index bfe19a21..87522038 100644 --- a/docs/ReadWriteOperations.md +++ b/docs/ReadWriteOperations.md @@ -1,6 +1,6 @@ # Read/Write Operations -The v2 server routes OPC UA Read and Write operations to each driver's `IReadable` and `IWritable` capabilities through `CapabilityInvoker` so the Polly pipeline (retry / timeout / breaker / bulkhead) applies uniformly across Galaxy, Modbus, S7, AB CIP, AB Legacy, TwinCAT, FOCAS, and OPC UA Client drivers. The per-variable `OnReadValue` and `OnWriteValue` hooks described in the sections below live in `DriverNodeManager` (the planned ADR-002 Phase 7 Stream G successor to the v1 `DriverNodeManager`); `GenericDriverNodeManager` (`src/Core/ZB.MOM.WW.OtOpcUa.Core/OpcUa/GenericDriverNodeManager.cs`) handles address-space population and alarm routing during discovery. The current `OtOpcUaNodeManager` (`src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs`) is a push-model `CustomNodeManager2` that receives values from the Akka actor layer via `WriteValue`; OPC UA client reads return the cached pushed value. +The v2 server routes OPC UA Read and Write operations to each driver's `IReadable` and `IWritable` capabilities through `CapabilityInvoker` so the Polly pipeline (retry / timeout / breaker) applies uniformly across Galaxy, Modbus, S7, AB CIP, AB Legacy, TwinCAT, FOCAS, and OPC UA Client drivers. The per-variable `OnReadValue` and `OnWriteValue` hooks described in the sections below live in `DriverNodeManager` (the planned ADR-002 Phase 7 Stream G successor to the v1 `DriverNodeManager`); `GenericDriverNodeManager` (`src/Core/ZB.MOM.WW.OtOpcUa.Core/OpcUa/GenericDriverNodeManager.cs`) handles address-space population and alarm routing during discovery. The current `OtOpcUaNodeManager` (`src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs`) is a push-model `CustomNodeManager2` that receives values from the Akka actor layer via `WriteValue`; OPC UA client reads return the cached pushed value. ## Driver vs virtual dispatch diff --git a/docs/security.md b/docs/security.md index 7386632f..c39de9b4 100644 --- a/docs/security.md +++ b/docs/security.md @@ -320,7 +320,7 @@ Responses: `202 Accepted` (`{ outcome, deploymentId, revisionHash }`) when a dep ## OTOPCUA0001 Analyzer — Compile-Time Guard -Per-capability resilience (retry, timeout, circuit-breaker, bulkhead) is applied by `CapabilityInvoker` in `src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/`. A driver-capability call made **outside** the invoker bypasses resilience entirely — which in production looks like inconsistent timeouts, un-wrapped retries, and unbounded blocking. +Per-capability resilience (retry, timeout, circuit-breaker) is applied by `CapabilityInvoker` in `src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/`. A driver-capability call made **outside** the invoker bypasses resilience entirely — which in production looks like inconsistent timeouts, un-wrapped retries, and unbounded blocking. `OTOPCUA0001` (Roslyn analyzer at `src/Tooling/ZB.MOM.WW.OtOpcUa.Analyzers/UnwrappedCapabilityCallAnalyzer.cs`) fires with category `OtOpcUa.Resilience` and default severity **Warning** (per `AnalyzerReleases.Shipped.md`) when a method on one of the seven guarded capability interfaces (`IReadable`, `IWritable`, `ITagDiscovery`, `ISubscribable`, `IHostConnectivityProbe`, `IAlarmSource`, `IHistoryProvider` — all in `ZB.MOM.WW.OtOpcUa.Core.Abstractions`) is invoked **outside** a lambda passed to `CapabilityInvoker.ExecuteAsync` / `ExecuteWriteAsync`. `AlarmSurfaceInvoker` is **not** a wrapper home — its own implementation is covered transitively because it routes through the inner `CapabilityInvoker.ExecuteAsync`. The analyzer walks up the syntax tree from the call site, finds any enclosing invoker invocation, and verifies the call lives transitively inside that invocation's anonymous-function argument — a sibling pattern (do the call, then invoke `ExecuteAsync` on something unrelated nearby) does not satisfy the rule. diff --git a/docs/v1/HistoricalDataAccess.md b/docs/v1/HistoricalDataAccess.md index 8b5de525..387335f1 100644 --- a/docs/v1/HistoricalDataAccess.md +++ b/docs/v1/HistoricalDataAccess.md @@ -30,7 +30,7 @@ There is also a separate **push** path for persisting alarm transitions from any ## Dispatch through `CapabilityInvoker` -All four HistoryRead surfaces are wrapped by `CapabilityInvoker` (`Core/Resilience/CapabilityInvoker.cs`) with `DriverCapability.HistoryRead`. The Polly pipeline keyed on `(DriverInstanceId, HostName, DriverCapability.HistoryRead)` provides timeout, circuit-breaker, and bulkhead defaults per the driver's stability tier (see [docs/v2/driver-stability.md](v2/driver-stability.md)). +All four HistoryRead surfaces are wrapped by `CapabilityInvoker` (`Core/Resilience/CapabilityInvoker.cs`) with `DriverCapability.HistoryRead`. The Polly pipeline keyed on `(DriverInstanceId, HostName, DriverCapability.HistoryRead)` provides timeout and circuit-breaker defaults per the driver's stability tier (see [docs/v2/driver-stability.md](v2/driver-stability.md)). The dispatch point is `DriverNodeManager` in `ZB.MOM.WW.OtOpcUa.Server`. When the OPC UA stack calls `HistoryRead`, the node manager: diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Entities/DriverInstance.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Entities/DriverInstance.cs index 77fb4284..6bc962a2 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Entities/DriverInstance.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Entities/DriverInstance.cs @@ -35,8 +35,6 @@ public sealed class DriverInstance /// Null = use the driver's tier defaults. When populated, expected shape: /// /// { - /// "bulkheadMaxConcurrent": 16, - /// "bulkheadMaxQueue": 64, /// "capabilityPolicies": { /// "Read": { "timeoutSeconds": 5, "retryCount": 5, "breakerFailureThreshold": 3 }, /// "Write": { "timeoutSeconds": 5, "retryCount": 0, "breakerFailureThreshold": 5 } diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Entities/DriverInstanceResilienceStatus.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Entities/DriverInstanceResilienceStatus.cs index b057222a..cdec0b59 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Entities/DriverInstanceResilienceStatus.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Entities/DriverInstanceResilienceStatus.cs @@ -26,7 +26,9 @@ public sealed class DriverInstanceResilienceStatus /// Rolling count of consecutive Polly pipeline failures for this (instance, host). public int ConsecutiveFailures { get; set; } - /// Current Polly bulkhead depth (in-flight calls) for this (instance, host). + /// In-flight capability-call depth for this (instance, host). Formerly fed a Polly + /// concurrency-limiter strategy that was removed (R2-02/01/U-6); the column name is retained to avoid a + /// migration on a reader-less entity — it now carries the tracker's in-flight-call count. public int CurrentBulkheadDepth { get; set; } /// Most recent process recycle time (Tier C only; null for in-process tiers). diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IDriverCapabilityInvoker.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IDriverCapabilityInvoker.cs index f3c60bc5..f2ebdc42 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IDriverCapabilityInvoker.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IDriverCapabilityInvoker.cs @@ -2,7 +2,7 @@ namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions; /// /// Abstraction over the Phase 6.1 resilience pipeline that wraps a single driver instance's -/// capability calls (retry / circuit-breaker / bulkhead / tracker telemetry). The Runtime +/// capability calls (retry / circuit-breaker / in-flight accounting / tracker telemetry). The Runtime /// dispatch layer (DriverInstanceActor) consumes this interface instead of the concrete /// CapabilityInvoker so the actor assembly does NOT pull in /// ZB.MOM.WW.OtOpcUa.Core (which drags in Polly + driver hosting) — the same boundary @@ -15,7 +15,7 @@ namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions; /// this interface without adapters. Callers pass the resolved hostName per call (a /// multi-host driver resolves it from the tag reference via ; /// a single-host driver passes its ) so per-host breaker -/// isolation + bulkhead-depth accounting key on the right target. +/// isolation + in-flight-depth accounting key on the right target. /// public interface IDriverCapabilityInvoker { diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IPerCallHostResolver.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IPerCallHostResolver.cs index 8a52b4fc..e1084665 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IPerCallHostResolver.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IPerCallHostResolver.cs @@ -28,7 +28,7 @@ public interface IPerCallHostResolver /// /// Resolve the host name for the given driver-side full reference. Returned value is /// used as the hostName argument to the Phase 6.1 CapabilityInvoker so - /// per-host breaker isolation + per-host bulkhead accounting both kick in. + /// per-host breaker isolation + per-host in-flight accounting both kick in. /// /// The full reference of the tag or resource. /// The host name responsible for serving the reference. diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/AlarmSurfaceInvoker.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/AlarmSurfaceInvoker.cs index 193e5844..d586c0ac 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/AlarmSurfaceInvoker.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/AlarmSurfaceInvoker.cs @@ -52,7 +52,7 @@ public sealed class AlarmSurfaceInvoker /// /// Subscribe to alarm events for a set of source node ids, fanning out by resolved host - /// so per-host breakers / bulkheads apply. Returns one handle per host — callers that + /// so per-host breakers apply. Returns one handle per host — callers that /// don't care about per-host separation may concatenate them. Each returned handle wraps /// the driver's opaque handle together with its resolved host so /// routes through the same host's pipeline that the subscription was created on. diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/CapabilityInvoker.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/CapabilityInvoker.cs index 8e77b7de..79d8db97 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/CapabilityInvoker.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/CapabilityInvoker.cs @@ -40,7 +40,7 @@ public sealed class CapabilityInvoker : IDriverCapabilityInvoker /// pipeline-invalidate can take effect without restarting the invoker. /// /// Driver type name for structured-log enrichment (e.g. "Modbus"). - /// Optional resilience-status tracker. When wired, every capability call records start/complete so Admin /hosts can surface as the bulkhead-depth proxy. + /// Optional resilience-status tracker. When wired, every capability call records start/complete so Admin /hosts can surface as the in-flight-call-depth proxy. public CapabilityInvoker( DriverResiliencePipelineBuilder builder, string driverInstanceId, diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResiliencePipelineBuilder.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResiliencePipelineBuilder.cs index a0dcc7d9..51fa558a 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResiliencePipelineBuilder.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResiliencePipelineBuilder.cs @@ -15,7 +15,7 @@ namespace ZB.MOM.WW.OtOpcUa.Core.Resilience; /// /// /// Per-device isolation. Composition from outside-in: -/// Timeout → Retry (when capability permits) → Circuit Breaker (when tier permits) → Bulkhead. +/// Timeout → Retry (when capability permits) → Circuit Breaker (when tier permits). /// /// Pipeline resolution is lock-free on the hot path: the inner /// caches a per key; @@ -39,7 +39,7 @@ public sealed class DriverResiliencePipelineBuilder /// When non-null, every built pipeline wires Polly telemetry into /// the tracker — retries increment ConsecutiveFailures, breaker-open stamps /// LastBreakerOpenUtc, breaker-close resets failures. Feeds Admin /hosts + - /// the Polly bulkhead-depth column. Absent tracker means no telemetry (unit tests + + /// the in-flight-call-depth column. Absent tracker means no telemetry (unit tests + /// deployments that don't care about resilience observability). /// When non-null, retry / circuit-breaker-open / breaker-close events are /// logged (instance + host + capability + attempt/exception). This is the operator-facing diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResilienceStatusTracker.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResilienceStatusTracker.cs index c2fdeda7..9dcf3327 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResilienceStatusTracker.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResilienceStatusTracker.cs @@ -101,7 +101,7 @@ public sealed class DriverResilienceStatusTracker /// /// Record the entry of a capability call for this (instance, host). Increments the /// in-flight counter used as the - /// surface (a cheap stand-in for Polly bulkhead depth). Paired with + /// surface (in-flight capability-call depth). Paired with /// ; callers use try/finally. /// /// The driver instance identifier. @@ -159,7 +159,7 @@ public sealed record ResilienceStatusSnapshot /// /// In-flight capability calls against this (instance, host). Bumped on call entry + /// decremented on completion. Feeds DriverInstanceResilienceStatus.CurrentBulkheadDepth - /// for Admin /hosts — a cheap proxy for the Polly bulkhead depth until the full + /// for Admin /hosts — in-flight capability-call depth until the full /// telemetry observer lands. /// public int CurrentInFlight { get; init; } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs index 698d476d..9b6a72c8 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs @@ -14,7 +14,7 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip; /// Wire layer is libplctag 1.6.x. Per-device host addresses use /// the ab://gateway[:port]/cip-path canonical form parsed via /// ; those strings become the hostName key -/// for Polly bulkhead + circuit-breaker isolation. +/// for Polly per-host circuit-breaker isolation. /// /// Tier A — in-process, shares server lifetime, no /// sidecar. is the Tier-B escape hatch for recovering diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipHostAddress.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipHostAddress.cs index 01138dc1..ad8d9554 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipHostAddress.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipHostAddress.cs @@ -3,7 +3,7 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip; /// /// Parsed ab://gateway[:port]/cip-path host-address string used by the AbCip driver /// as the hostName key across , -/// , and the Polly bulkhead key +/// , and the Polly per-host pipeline key /// (DriverInstanceId, hostName). /// /// diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Contracts/FocasDriverOptions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Contracts/FocasDriverOptions.cs index 00236c8a..ef60b648 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Contracts/FocasDriverOptions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Contracts/FocasDriverOptions.cs @@ -4,7 +4,7 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.FOCAS; /// /// FOCAS driver configuration. One instance supports N CNC devices. Each device gets its own -/// (DriverInstanceId, HostAddress) bulkhead key at the Phase 6.1 resilience layer. +/// (DriverInstanceId, HostAddress) per-host pipeline key at the Phase 6.1 resilience layer. /// public sealed class FocasDriverOptions { diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs index a449d209..42ffbe84 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs @@ -1627,7 +1627,7 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers else { // Attach the per-instance Phase 6.1 resilience invoker so this driver's capability calls - // route through the retry/breaker/bulkhead/telemetry pipeline. The tier defaults are layered + // route through the retry/breaker/telemetry pipeline. The tier defaults are layered // with the per-instance ResilienceConfig from the artifact; the pass-through factory yields a // no-op invoker on nodes without the real resilience factory bound. var invoker = _invokerFactory.Create(spec.DriverInstanceId, spec.DriverType, spec.ResilienceConfig); diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs index 3405547f..9c711119 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs @@ -139,7 +139,7 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers private readonly IDriver _driver; /// Phase 6.1 resilience seam: every guarded driver-capability call this actor makes is - /// routed through this invoker (retry / breaker / bulkhead / tracker telemetry). Defaults to + /// routed through this invoker (retry / breaker / in-flight accounting / tracker telemetry). Defaults to /// (a genuine pass-through) when none is supplied — so /// unit tests + the F7 skeleton path behave exactly as a raw driver call. The fused Host builds a /// real per-instance invoker via and injects it at diff --git a/src/Tooling/ZB.MOM.WW.OtOpcUa.Analyzers/UnwrappedCapabilityCallAnalyzer.cs b/src/Tooling/ZB.MOM.WW.OtOpcUa.Analyzers/UnwrappedCapabilityCallAnalyzer.cs index 644e0f86..5c5bdc21 100644 --- a/src/Tooling/ZB.MOM.WW.OtOpcUa.Analyzers/UnwrappedCapabilityCallAnalyzer.cs +++ b/src/Tooling/ZB.MOM.WW.OtOpcUa.Analyzers/UnwrappedCapabilityCallAnalyzer.cs @@ -12,7 +12,7 @@ namespace ZB.MOM.WW.OtOpcUa.Analyzers; /// Diagnostic analyzer that flags direct invocations of Phase 6.1-wrapped driver-capability /// methods when the call is NOT already running inside a CapabilityInvoker.ExecuteAsync /// or CapabilityInvoker.ExecuteWriteAsync lambda. The wrapping is what gives us -/// per-host breaker isolation, retry semantics, bulkhead-depth accounting, and alarm-ack +/// per-host breaker isolation, retry semantics, in-flight-depth accounting, and alarm-ack /// idempotence guards — raw calls bypass all of that. /// /// @@ -102,7 +102,7 @@ public sealed class UnwrappedCapabilityCallAnalyzer : DiagnosticAnalyzer private static readonly DiagnosticDescriptor Rule = new( id: DiagnosticId, title: "Driver capability call must be wrapped in CapabilityInvoker", - messageFormat: "Call to '{0}' is not wrapped in CapabilityInvoker.ExecuteAsync / ExecuteWriteAsync. Without the wrapping, Phase 6.1 resilience (retry, breaker, bulkhead, tracker telemetry) is bypassed for this call.", + messageFormat: "Call to '{0}' is not wrapped in CapabilityInvoker.ExecuteAsync / ExecuteWriteAsync. Without the wrapping, Phase 6.1 resilience (retry, breaker, tracker telemetry) is bypassed for this call.", category: "OtOpcUa.Resilience", defaultSeverity: DiagnosticSeverity.Warning, isEnabledByDefault: true, diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/DriverResilienceOptionsTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/DriverResilienceOptionsTests.cs index 63c7667e..17920df5 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/DriverResilienceOptionsTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/DriverResilienceOptionsTests.cs @@ -8,6 +8,34 @@ namespace ZB.MOM.WW.OtOpcUa.Core.Tests.Resilience; [Trait("Category", "Unit")] public sealed class DriverResilienceOptionsTests { + /// + /// 01/U-6 knob-inertness guard (OVERALL theme #1): the public property set of + /// must be EXACTLY the pipeline-wired knobs. Every option a + /// parser can populate must map to a strategy the builder actually composes — a parsed-but-unapplied + /// knob (the bulkhead genre this pass deleted) is inertness the interface-forwarding and + /// unwrapped-dispatch guards can't catch. To add a knob: wire it in DriverResiliencePipelineBuilder, + /// add a behavior test that proves it engages, THEN add it to the expected set below. + /// (RecycleIntervalSeconds is itself Tier-C-dormant per 01/U-2 — a documented open question, + /// explicitly out of this pass's scope; it stays in the expected list as the recycle scheduler, not + /// the Polly pipeline, is its consumer.) + /// + [Fact] + public void Options_properties_are_exactly_the_pipeline_wired_set() + { + var expected = new[] { "Tier", "CapabilityPolicies", "RecycleIntervalSeconds" }; + + var actual = typeof(DriverResilienceOptions) + .GetProperties() + .Select(p => p.Name) + .OrderBy(n => n) + .ToArray(); + + actual.ShouldBe(expected.OrderBy(n => n).ToArray(), + "DriverResilienceOptions must expose only pipeline-wired knobs — a new option needs a builder " + + "strategy + a behavior test before it is added to the expected set (guards against the deleted " + + "bulkhead 'parsed-but-unapplied' genre; see 01/U-6)."); + } + /// Verifies that tier defaults cover every capability. /// The driver tier to test. [Theory] From 9e0a05dc6a31bb6fc8cc3262c16b2417f760d9b7 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 10:18:31 -0400 Subject: [PATCH 13/17] =?UTF-8?q?fix(r2-02):=20non-lossy=20ResilienceFormM?= =?UTF-8?q?odel=20round-trip=20=E2=80=94=20preserve=20unknown=20keys,=20ne?= =?UTF-8?q?ver=20overwrite=20unparseable=20stored=20JSON=20(04/C-7)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ...lience-config-hardening-plan.md.tasks.json | 4 +- .../Shared/Drivers/ResilienceFormModel.cs | 182 ++++++++++++------ .../ResilienceFormModelTests.cs | 54 ++++++ 3 files changed, 180 insertions(+), 60 deletions(-) diff --git a/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json b/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json index 764340fc..55e05832 100644 --- a/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json +++ b/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json @@ -14,8 +14,8 @@ { "id": 10, "subject": "U-6 delete (Core): remove BulkheadMaxConcurrent/MaxQueue from DriverResilienceOptions + parser shape/merge/doc-example; add BulkheadKeys_InStoredJson_AreIgnored (migration-free contract)", "status": "completed", "blockedBy": [2, 6] }, { "id": 11, "subject": "U-6 delete (AdminUI): strip bulkhead fields from ResilienceFormModel + the two razor inputs; update ResilienceFormModelTests", "status": "completed", "blockedBy": [5, 10] }, { "id": 12, "subject": "U-6 doc sweep (seam xmldocs, OTOPCUA0001 message + analyzer tests, operator docs; leave migrations/WedgeDetector/historical plans) + Options_properties_are_exactly_the_pipeline_wired_set knob-inertness guard", "status": "completed", "blockedBy": [10, 11] }, - { "id": 13, "subject": "C-7 RED tests: unknown top-level key / unknown capability / unknown per-policy field survive round-trip; malformed JSON sets ParseFailed and ToJson returns the original text", "status": "pending", "blockedBy": [11] }, - { "id": 14, "subject": "C-7 implement: JsonObject-bag FromJson/ToJson (TagConfigJson idiom), ParseFailed + RawStoredJson, blank->null contract preserved, parser-interop test green", "status": "pending", "blockedBy": [13] }, + { "id": 13, "subject": "C-7 RED tests: unknown top-level key / unknown capability / unknown per-policy field survive round-trip; malformed JSON sets ParseFailed and ToJson returns the original text", "status": "completed", "blockedBy": [11] }, + { "id": 14, "subject": "C-7 implement: JsonObject-bag FromJson/ToJson (TagConfigJson idiom), ParseFailed + RawStoredJson, blank->null contract preserved, parser-interop test green", "status": "completed", "blockedBy": [13] }, { "id": 15, "subject": "C-7 razor: ParseFailed warning banner + input lockout + explicit discard button; raw pane shows the stored ResilienceConfig; live-/run on docker-dev :9200 (rebuild BOTH centrals; SQL-mangle + unknown-key survival checks; also drive task 5 warnings and task 9 disabled cells)", "status": "pending", "blockedBy": [14] }, { "id": 16, "subject": "S-7 builder: add options-generation to PipelineKey + GetOrCreate(optionsGeneration=0) + thread through CapabilityInvoker; StaleGeneration_ReCache_IsNotServed_ToNewGeneration", "status": "pending", "blockedBy": [3] }, { "id": 17, "subject": "S-7 factory: Interlocked per-Create generation stamp + Respawn_interleaved_with_old_invoker_call_still_applies_new_options wiring proof; update Invalidate comment (cleanup, not correctness)", "status": "pending", "blockedBy": [16] }, diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/ResilienceFormModel.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/ResilienceFormModel.cs index fa9aeb7e..e5f2a7ca 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/ResilienceFormModel.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/ResilienceFormModel.cs @@ -1,14 +1,22 @@ using System.Text.Json; -using System.Text.Json.Serialization; +using System.Text.Json.Nodes; using ZB.MOM.WW.OtOpcUa.Core.Abstractions; namespace ZB.MOM.WW.OtOpcUa.AdminUI.Components.Shared.Drivers; /// -/// Mutable, all-nullable form model for the driver resilience override. Binds the typed -/// fields in DriverResilienceSection; null/blank = "use the driver's tier default", so a -/// blank form serializes back to null (preserving DriverInstance.ResilienceConfig = null). -/// Emits / reads the exact override JSON shape DriverResilienceOptionsParser consumes. +/// Mutable, all-nullable form model for the driver resilience override. Binds the typed fields in +/// DriverResilienceSection; null/blank = "use the driver's tier default", so a blank form serializes +/// back to null (preserving DriverInstance.ResilienceConfig = null). Emits / reads the exact override +/// JSON shape DriverResilienceOptionsParser consumes. +/// +/// Round-trip is non-lossy (04/C-7): the stored JSON is kept as a bag, +/// and overlays only the known fields onto that bag — so unknown top-level keys, +/// unknown capability entries, and unknown per-policy fields all survive a load→save, mirroring the +/// driver tag editors' preserve-unknown discipline (TagConfigJson). A stored blob that could not +/// be parsed sets and is never overwritten — +/// returns the original text verbatim. +/// /// public sealed class ResilienceFormModel { @@ -23,6 +31,18 @@ public sealed class ResilienceFormModel public Dictionary Policies { get; set; } = Capabilities.ToDictionary(c => c, _ => new CapabilityRow(), StringComparer.OrdinalIgnoreCase); + /// + /// True when the stored ResilienceConfig could not be parsed as JSON. The section locks editing and + /// returns unchanged so an unrelated keystroke can + /// never silently overwrite an unreadable column. + /// + public bool ParseFailed { get; private set; } + + /// The original stored text when — returned verbatim by . + public string? RawStoredJson { get; private set; } + + private JsonObject _bag = new(); + /// Per-capability timeout/retry/breaker override row; null fields fall back to the tier default. public sealed class CapabilityRow { @@ -68,79 +88,125 @@ public sealed class ResilienceFormModel return msgs; } - private static readonly JsonSerializerOptions ReadOpts = new() { PropertyNameCaseInsensitive = true }; - private static readonly JsonSerializerOptions WriteOpts = new() - { - PropertyNamingPolicy = JsonNamingPolicy.CamelCase, - DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull, - }; - - /// Parses the override JSON into a form model; malformed or blank JSON yields an empty (all-default) form. + /// + /// Parse the override JSON into a form model, preserving every key in an internal bag. Malformed JSON + /// sets + and yields an otherwise-empty model. + /// /// The raw resilience-override JSON, or null/blank if there is no override. - /// A populated form model, or an all-default one when is blank or malformed. + /// A populated form model. public static ResilienceFormModel FromJson(string? json) { var model = new ResilienceFormModel(); if (string.IsNullOrWhiteSpace(json)) return model; - Shape? shape; - try { shape = JsonSerializer.Deserialize(json, ReadOpts); } - catch (JsonException) { return model; } // malformed -> empty form; raw view (next task) shows the text - if (shape is null) return model; + JsonObject bag; + try + { + bag = JsonNode.Parse(json) as JsonObject ?? new JsonObject(); + } + catch (JsonException) + { + model.ParseFailed = true; + model.RawStoredJson = json; + return model; + } - model.RecycleIntervalSeconds = shape.RecycleIntervalSeconds; - if (shape.CapabilityPolicies is not null) - foreach (var (cap, p) in shape.CapabilityPolicies) - if (model.Policies.TryGetValue(cap, out var row)) + model._bag = bag; + model.RecycleIntervalSeconds = GetIntOrNull(bag, "recycleIntervalSeconds"); + + if (bag.TryGetPropertyValue("capabilityPolicies", out var cpNode) && cpNode is JsonObject cp) + { + foreach (var (capName, polNode) in cp) + { + // Known capability names populate rows (reading only the known per-policy fields — unknown + // per-policy fields stay untouched in the bag). Unknown capability entries stay in the bag. + if (polNode is JsonObject pol && model.Policies.TryGetValue(capName, out var row)) { - row.TimeoutSeconds = p.TimeoutSeconds; - row.RetryCount = p.RetryCount; - row.BreakerFailureThreshold = p.BreakerFailureThreshold; + row.TimeoutSeconds = GetIntOrNull(pol, "timeoutSeconds"); + row.RetryCount = GetIntOrNull(pol, "retryCount"); + row.BreakerFailureThreshold = GetIntOrNull(pol, "breakerFailureThreshold"); } + } + } return model; } - /// Emit only the non-null overrides; returns null when nothing is overridden. - /// The serialized override JSON, or null when no field in this form is overridden. + /// + /// Overlay the model's non-null known values onto the preserved bag and serialize. Returns null when + /// the result is empty (blank form = tier defaults). When , returns the + /// original unparseable text unchanged. + /// + /// The serialized override JSON, the original text when parse failed, or null when nothing is set. public string? ToJson() { - var caps = Policies - .Where(kv => !kv.Value.IsEmpty) - .ToDictionary(kv => kv.Key, kv => new PolicyShape - { - TimeoutSeconds = kv.Value.TimeoutSeconds, - RetryCount = kv.Value.RetryCount, - BreakerFailureThreshold = kv.Value.BreakerFailureThreshold, - }); + if (ParseFailed) return RawStoredJson; - var hasAny = RecycleIntervalSeconds is not null || caps.Count > 0; - if (!hasAny) return null; + SetOrRemoveInt(_bag, "recycleIntervalSeconds", RecycleIntervalSeconds); - var shape = new Shape + var cp = _bag.TryGetPropertyValue("capabilityPolicies", out var cpNode) && cpNode is JsonObject existing + ? existing + : null; + + foreach (var (cap, row) in Policies) { - RecycleIntervalSeconds = RecycleIntervalSeconds, - CapabilityPolicies = caps.Count > 0 ? caps : null, - }; - return JsonSerializer.Serialize(shape, WriteOpts); + // Find this known capability's existing policy object (case-insensitive), else its canonical key. + var polKey = cap; + JsonObject? pol = null; + if (cp is not null) + { + foreach (var (k, v) in cp) + { + if (string.Equals(k, cap, StringComparison.OrdinalIgnoreCase) && v is JsonObject vo) + { + polKey = k; + pol = vo; + break; + } + } + } + + if (row.IsEmpty) + { + // Clear the known fields; drop the policy entry only if no unknown fields remain. + if (pol is not null) + { + pol.Remove("timeoutSeconds"); + pol.Remove("retryCount"); + pol.Remove("breakerFailureThreshold"); + if (pol.Count == 0) cp!.Remove(polKey); + } + } + else + { + if (cp is null) + { + cp = new JsonObject(); + _bag["capabilityPolicies"] = cp; + } + if (pol is null) + { + pol = new JsonObject(); + cp[polKey] = pol; + } + SetOrRemoveInt(pol, "timeoutSeconds", row.TimeoutSeconds); + SetOrRemoveInt(pol, "retryCount", row.RetryCount); + SetOrRemoveInt(pol, "breakerFailureThreshold", row.BreakerFailureThreshold); + } + } + + if (cp is not null && cp.Count == 0) _bag.Remove("capabilityPolicies"); + + return _bag.Count == 0 ? null : _bag.ToJsonString(); } - /// Wire shape of the resilience-override JSON, as consumed by DriverResilienceOptionsParser. - private sealed class Shape - { - /// Gets or sets the driver recycle-interval override, in seconds. - public int? RecycleIntervalSeconds { get; set; } - /// Gets or sets the per-capability overrides, keyed by capability name. - public Dictionary? CapabilityPolicies { get; set; } - } + private static int? GetIntOrNull(JsonObject o, string name) + => o.TryGetPropertyValue(name, out var n) && n is JsonValue v && v.TryGetValue(out var i) + ? i + : null; - /// Wire shape of a single capability's timeout/retry/breaker override. - private sealed class PolicyShape + private static void SetOrRemoveInt(JsonObject o, string name, int? value) { - /// Gets or sets the timeout override, in seconds. - public int? TimeoutSeconds { get; set; } - /// Gets or sets the retry-count override. - public int? RetryCount { get; set; } - /// Gets or sets the circuit-breaker failure-threshold override. - public int? BreakerFailureThreshold { get; set; } + if (value is null) o.Remove(name); + else o[name] = JsonValue.Create(value.Value); } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ResilienceFormModelTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ResilienceFormModelTests.cs index 301797b7..fd4a8765 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ResilienceFormModelTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ResilienceFormModelTests.cs @@ -1,3 +1,4 @@ +using System.Text.Json.Nodes; using Shouldly; using Xunit; using ZB.MOM.WW.OtOpcUa.AdminUI.Components.Shared.Drivers; @@ -6,6 +7,59 @@ using ZB.MOM.WW.OtOpcUa.Core.Resilience; public class ResilienceFormModelTests { + // ---- 04/C-7: non-lossy round-trip ---- + + [Fact] + public void Unknown_top_level_key_survives_round_trip() + { + // bulkheadMaxConcurrent doubles as the 01/U-6 continuity proof: the deleted knob's key survives. + var json = """{"bulkheadMaxConcurrent":16,"capabilityPolicies":{"Read":{"timeoutSeconds":5}}}"""; + + var back = ResilienceFormModel.FromJson(json).ToJson(); + + back.ShouldNotBeNull(); + var obj = JsonNode.Parse(back)!.AsObject(); + obj["bulkheadMaxConcurrent"]!.GetValue().ShouldBe(16); + obj["capabilityPolicies"]!["Read"]!["timeoutSeconds"]!.GetValue().ShouldBe(5); + } + + [Fact] + public void Unknown_capability_entry_survives_round_trip() + { + var json = """{"capabilityPolicies":{"Read":{"timeoutSeconds":5},"FutureCap":{"timeoutSeconds":9}}}"""; + + var back = ResilienceFormModel.FromJson(json).ToJson(); + + back.ShouldNotBeNull(); + var obj = JsonNode.Parse(back)!.AsObject(); + obj["capabilityPolicies"]!["FutureCap"]!["timeoutSeconds"]!.GetValue().ShouldBe(9); + obj["capabilityPolicies"]!["Read"]!["timeoutSeconds"]!.GetValue().ShouldBe(5); + } + + [Fact] + public void Unknown_per_policy_field_survives_round_trip() + { + var json = """{"capabilityPolicies":{"Read":{"timeoutSeconds":5,"futureField":7}}}"""; + + var back = ResilienceFormModel.FromJson(json).ToJson(); + + back.ShouldNotBeNull(); + var read = JsonNode.Parse(back)!.AsObject()["capabilityPolicies"]!["Read"]!.AsObject(); + read["timeoutSeconds"]!.GetValue().ShouldBe(5); + read["futureField"]!.GetValue().ShouldBe(7); + } + + [Fact] + public void Malformed_json_sets_ParseFailed_and_ToJson_returns_original_text() + { + const string json = "{ not json"; + + var m = ResilienceFormModel.FromJson(json); + + m.ParseFailed.ShouldBeTrue(); + m.ToJson().ShouldBe(json); // never overwrite what it couldn't read + } + [Fact] public void Blank_form_serializes_to_null() => new ResilienceFormModel().ToJson().ShouldBeNull(); From 112f88c9fed8846555703704658386b259cadd32 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 10:19:50 -0400 Subject: [PATCH 14/17] =?UTF-8?q?fix(r2-02):=20resilience=20section=20?= =?UTF-8?q?=E2=80=94=20parse-failure=20lockout=20+=20truthful=20raw=20pane?= =?UTF-8?q?=20(04/C-7;=20live-/run=20deferred)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ...lience-config-hardening-plan.md.tasks.json | 2 +- .../Drivers/DriverResilienceSection.razor | 39 ++++++++++++++++--- 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json b/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json index 55e05832..4c921399 100644 --- a/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json +++ b/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json @@ -16,7 +16,7 @@ { "id": 12, "subject": "U-6 doc sweep (seam xmldocs, OTOPCUA0001 message + analyzer tests, operator docs; leave migrations/WedgeDetector/historical plans) + Options_properties_are_exactly_the_pipeline_wired_set knob-inertness guard", "status": "completed", "blockedBy": [10, 11] }, { "id": 13, "subject": "C-7 RED tests: unknown top-level key / unknown capability / unknown per-policy field survive round-trip; malformed JSON sets ParseFailed and ToJson returns the original text", "status": "completed", "blockedBy": [11] }, { "id": 14, "subject": "C-7 implement: JsonObject-bag FromJson/ToJson (TagConfigJson idiom), ParseFailed + RawStoredJson, blank->null contract preserved, parser-interop test green", "status": "completed", "blockedBy": [13] }, - { "id": 15, "subject": "C-7 razor: ParseFailed warning banner + input lockout + explicit discard button; raw pane shows the stored ResilienceConfig; live-/run on docker-dev :9200 (rebuild BOTH centrals; SQL-mangle + unknown-key survival checks; also drive task 5 warnings and task 9 disabled cells)", "status": "pending", "blockedBy": [14] }, + { "id": 15, "subject": "C-7 razor: ParseFailed warning banner + input lockout + explicit discard button; raw pane shows the stored ResilienceConfig; live-/run on docker-dev :9200 (rebuild BOTH centrals; SQL-mangle + unknown-key survival checks; also drive task 5 warnings and task 9 disabled cells)", "status": "deferred-live", "note": "docker-dev :9200 live-/run; serial live pass", "blockedBy": [14] }, { "id": 16, "subject": "S-7 builder: add options-generation to PipelineKey + GetOrCreate(optionsGeneration=0) + thread through CapabilityInvoker; StaleGeneration_ReCache_IsNotServed_ToNewGeneration", "status": "pending", "blockedBy": [3] }, { "id": 17, "subject": "S-7 factory: Interlocked per-Create generation stamp + Respawn_interleaved_with_old_invoker_call_still_applies_new_options wiring proof; update Invalidate comment (cleanup, not correctness)", "status": "pending", "blockedBy": [16] }, { "id": 18, "subject": "Wrap-up: whole-solution build (0 warnings, analyzer silent) + full dotnet test gate; record the pass + the two report anchor drifts in archreview/plans/STATUS.md", "status": "pending", "blockedBy": [4, 8, 9, 12, 15, 17] } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DriverResilienceSection.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DriverResilienceSection.razor index 4326c428..ca3e8df7 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DriverResilienceSection.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DriverResilienceSection.razor @@ -6,9 +6,23 @@

Blank fields use the driver type's stability-tier defaults (see docs/v2/driver-stability.md). Set only what you need to override.

+ @if (_m.ParseFailed) + { + + } +
-
+
@@ -21,9 +35,9 @@ var writeShaped = cap is "Write" or "AlarmAcknowledge"; @cap - - - + + + } @@ -46,7 +60,9 @@
Raw JSON (advanced) - 
@(_m.ToJson() ?? "(null — all tier defaults)")
+ @* Render the bound STORED value, not the re-serialized model — shows the truth in both the + healthy and the malformed case. *@ + 
@(ResilienceConfig ?? "(null — all tier defaults)")
@@ -70,9 +86,22 @@ private async Task EmitAsync() { + // Never emit while the stored config is unparseable — the section is locked, and an unrelated + // keystroke must never silently overwrite what we couldn't read (04/C-7). + if (_m.ParseFailed) return; + var json = _m.ToJson(); _lastParsed = json; ResilienceConfig = json; await ResilienceConfigChanged.InvokeAsync(json); } + + private async Task DiscardAsync() + { + // Explicit, visible destruction — replace the unparseable blob with a blank (tier-default) config. + _m = new ResilienceFormModel(); + _lastParsed = null; + ResilienceConfig = null; + await ResilienceConfigChanged.InvokeAsync(null); + } } From 5daf7155a8a095528b35a4512dce303e68dd36be Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 10:22:07 -0400 Subject: [PATCH 15/17] =?UTF-8?q?fix(r2-02):=20options-generation=20in=20t?= =?UTF-8?q?he=20pipeline=20cache=20key=20=E2=80=94=20stale=20respawn=20re-?= =?UTF-8?q?cache=20unreachable=20(01/S-7,=2003/S13)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ...lience-config-hardening-plan.md.tasks.json | 2 +- .../Resilience/CapabilityInvoker.cs | 14 ++++-- .../DriverResiliencePipelineBuilder.cs | 20 ++++++-- .../DriverResiliencePipelineBuilderTests.cs | 48 +++++++++++++++++++ 4 files changed, 76 insertions(+), 8 deletions(-) diff --git a/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json b/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json index 4c921399..fa41c229 100644 --- a/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json +++ b/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json @@ -17,7 +17,7 @@ { "id": 13, "subject": "C-7 RED tests: unknown top-level key / unknown capability / unknown per-policy field survive round-trip; malformed JSON sets ParseFailed and ToJson returns the original text", "status": "completed", "blockedBy": [11] }, { "id": 14, "subject": "C-7 implement: JsonObject-bag FromJson/ToJson (TagConfigJson idiom), ParseFailed + RawStoredJson, blank->null contract preserved, parser-interop test green", "status": "completed", "blockedBy": [13] }, { "id": 15, "subject": "C-7 razor: ParseFailed warning banner + input lockout + explicit discard button; raw pane shows the stored ResilienceConfig; live-/run on docker-dev :9200 (rebuild BOTH centrals; SQL-mangle + unknown-key survival checks; also drive task 5 warnings and task 9 disabled cells)", "status": "deferred-live", "note": "docker-dev :9200 live-/run; serial live pass", "blockedBy": [14] }, - { "id": 16, "subject": "S-7 builder: add options-generation to PipelineKey + GetOrCreate(optionsGeneration=0) + thread through CapabilityInvoker; StaleGeneration_ReCache_IsNotServed_ToNewGeneration", "status": "pending", "blockedBy": [3] }, + { "id": 16, "subject": "S-7 builder: add options-generation to PipelineKey + GetOrCreate(optionsGeneration=0) + thread through CapabilityInvoker; StaleGeneration_ReCache_IsNotServed_ToNewGeneration", "status": "completed", "blockedBy": [3] }, { "id": 17, "subject": "S-7 factory: Interlocked per-Create generation stamp + Respawn_interleaved_with_old_invoker_call_still_applies_new_options wiring proof; update Invalidate comment (cleanup, not correctness)", "status": "pending", "blockedBy": [16] }, { "id": 18, "subject": "Wrap-up: whole-solution build (0 warnings, analyzer silent) + full dotnet test gate; record the pass + the two report anchor drifts in archreview/plans/STATUS.md", "status": "pending", "blockedBy": [4, 8, 9, 12, 15, 17] } ], diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/CapabilityInvoker.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/CapabilityInvoker.cs index 79d8db97..a985acb7 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/CapabilityInvoker.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/CapabilityInvoker.cs @@ -28,6 +28,7 @@ public sealed class CapabilityInvoker : IDriverCapabilityInvoker private readonly string _driverType; private readonly Func _optionsAccessor; private readonly DriverResilienceStatusTracker? _statusTracker; + private readonly long _optionsGeneration; private DriverResilienceOptions? _noRetryWriteOptions; /// @@ -41,12 +42,18 @@ public sealed class CapabilityInvoker : IDriverCapabilityInvoker /// /// Driver type name for structured-log enrichment (e.g. "Modbus"). /// Optional resilience-status tracker. When wired, every capability call records start/complete so Admin /hosts can surface as the in-flight-call-depth proxy. + /// + /// Options epoch for this invoker (01/S-7). Threaded into every pipeline-cache lookup so a lingering + /// old child's late re-cache can never poison a newer invoker's pipeline. Defaults to 0 so existing + /// callers/tests are unaffected. + /// public CapabilityInvoker( DriverResiliencePipelineBuilder builder, string driverInstanceId, Func optionsAccessor, string driverType = "Unknown", - DriverResilienceStatusTracker? statusTracker = null) + DriverResilienceStatusTracker? statusTracker = null, + long optionsGeneration = 0) { ArgumentNullException.ThrowIfNull(builder); ArgumentNullException.ThrowIfNull(optionsAccessor); @@ -56,6 +63,7 @@ public sealed class CapabilityInvoker : IDriverCapabilityInvoker _driverType = driverType; _optionsAccessor = optionsAccessor; _statusTracker = statusTracker; + _optionsGeneration = optionsGeneration; } /// @@ -123,7 +131,7 @@ public sealed class CapabilityInvoker : IDriverCapabilityInvoker // safe and removes the per-write allocation the previous once-per-call snapshot incurred. The // null-check-assign tolerates a benign first-call race (both threads build an equivalent record). var noRetryOptions = _noRetryWriteOptions ??= BuildNoRetryWriteOptions(); - var pipeline = _builder.GetOrCreate(_driverInstanceId, $"{hostName}::non-idempotent", DriverCapability.Write, noRetryOptions); + var pipeline = _builder.GetOrCreate(_driverInstanceId, $"{hostName}::non-idempotent", DriverCapability.Write, noRetryOptions, _optionsGeneration); // Record in-flight accounting on the CALLER's host (the "::non-idempotent" suffix is a // pipeline-cache key, not an operator-facing host) so Admin /hosts sees write depth too. _statusTracker?.RecordCallStart(_driverInstanceId, hostName); @@ -156,5 +164,5 @@ public sealed class CapabilityInvoker : IDriverCapabilityInvoker } private ResiliencePipeline ResolvePipeline(DriverCapability capability, string hostName) => - _builder.GetOrCreate(_driverInstanceId, hostName, capability, _optionsAccessor()); + _builder.GetOrCreate(_driverInstanceId, hostName, capability, _optionsAccessor(), _optionsGeneration); } diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResiliencePipelineBuilder.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResiliencePipelineBuilder.cs index 51fa558a..2a6b3952 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResiliencePipelineBuilder.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResiliencePipelineBuilder.cs @@ -69,23 +69,35 @@ public sealed class DriverResiliencePipelineBuilder /// /// Which capability surface is being called. /// Per-driver-instance options (tier + per-capability overrides). + /// + /// Monotonic options epoch stamped by per invoker + /// (01/S-7). Part of the cache key so a lingering old child's post-invalidate re-cache lands under + /// its OWN (old) generation — a key the new invoker's generation never reads. Defaults to 0 so + /// every existing caller/test compiles unchanged. + /// /// The cached or newly built resilience pipeline for the key. public ResiliencePipeline GetOrCreate( string driverInstanceId, string hostName, DriverCapability capability, - DriverResilienceOptions options) + DriverResilienceOptions options, + long optionsGeneration = 0) { ArgumentNullException.ThrowIfNull(options); ArgumentException.ThrowIfNullOrWhiteSpace(hostName); - var key = new PipelineKey(driverInstanceId, hostName, capability); + var key = new PipelineKey(driverInstanceId, hostName, capability, optionsGeneration); return _pipelines.GetOrAdd(key, static (k, state) => Build( k.DriverInstanceId, k.HostName, state.capability, state.options, state.timeProvider, state.tracker, state.logger), (capability, options, timeProvider: _timeProvider, tracker: _statusTracker, logger: _logger)); } - /// Drop cached pipelines for one driver instance (e.g. on ResilienceConfig change). Test + Admin-reload use. + /// + /// Drop cached pipelines for one driver instance (e.g. on ResilienceConfig change), across ALL + /// option generations. With generation-keyed entries (01/S-7) correctness no longer depends on this + /// sweep — a stale-epoch re-cache is already unreachable by the new generation — but it bounds the + /// cache by clearing lingering old-generation entries on the next respawn. Test + Admin-reload use. + /// /// The driver instance ID whose pipelines should be invalidated. /// The number of pipelines removed. public int Invalidate(string driverInstanceId) @@ -188,5 +200,5 @@ public sealed class DriverResiliencePipelineBuilder return builder.Build(); } - private readonly record struct PipelineKey(string DriverInstanceId, string HostName, DriverCapability Capability); + private readonly record struct PipelineKey(string DriverInstanceId, string HostName, DriverCapability Capability, long Generation); } diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/DriverResiliencePipelineBuilderTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/DriverResiliencePipelineBuilderTests.cs index ca0b8d14..b9bebed5 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/DriverResiliencePipelineBuilderTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/DriverResiliencePipelineBuilderTests.cs @@ -356,6 +356,54 @@ public sealed class DriverResiliencePipelineBuilderTests } } + /// + /// 01/S-7 = 03/S13: a stale-epoch re-cache must not be served to a new generation. Deterministic + /// replay of the respawn race — the old child's late GetOrCreate re-caches under generation 1 + /// AFTER the invalidate; the new invoker (generation 2) with new options must build + serve a pipeline + /// honoring the NEW options, not the poisoned old-generation entry. Generations make the interleaving + /// expressible sequentially — no timing dependence. + /// + [Fact] + public async Task StaleGeneration_ReCache_IsNotServed_ToNewGeneration() + { + var builder = new DriverResiliencePipelineBuilder(); + var optsA = new DriverResilienceOptions + { + Tier = DriverTier.A, + CapabilityPolicies = new Dictionary + { + [DriverCapability.Read] = new(TimeoutSeconds: 5, RetryCount: 3, BreakerFailureThreshold: 0), + }, + }; + var optsB = new DriverResilienceOptions + { + Tier = DriverTier.A, + CapabilityPolicies = new Dictionary + { + [DriverCapability.Read] = new(TimeoutSeconds: 5, RetryCount: 0, BreakerFailureThreshold: 0), + }, + }; + + // gen 1 warms the cache with the retrying optsA. + builder.GetOrCreate("i", "h", DriverCapability.Read, optsA, optionsGeneration: 1); + builder.Invalidate("i"); + // The lingering old child's late call re-caches optsA under its OWN generation 1. + builder.GetOrCreate("i", "h", DriverCapability.Read, optsA, optionsGeneration: 1); + + // The new invoker (gen 2, optsB) must get a pipeline honoring optsB (no retry), not the poisoned one. + var newPipeline = builder.GetOrCreate("i", "h", DriverCapability.Read, optsB, optionsGeneration: 2); + var attempts = 0; + await Should.ThrowAsync(async () => + await newPipeline.ExecuteAsync(async _ => + { + attempts++; + await Task.Yield(); + throw new InvalidOperationException("boom"); + })); + + attempts.ShouldBe(1, "the new generation must honor optsB (retryCount 0), not the stale optsA re-cache"); + } + /// Minimal in-memory capturing (level, formatted-message) pairs so the /// resilience logging contract can be asserted without a real logging provider. private sealed class CapturingLogger : ILogger From d8c6357be83fd415ba7a03c6b7c44c1ed4b298f6 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 10:23:25 -0400 Subject: [PATCH 16/17] =?UTF-8?q?fix(r2-02):=20factory=20stamps=20a=20per-?= =?UTF-8?q?Create=20options=20generation=20=E2=80=94=20closes=20the=20resp?= =?UTF-8?q?awn=20stale-pipeline=20race=20(01/S-7)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ...lience-config-hardening-plan.md.tasks.json | 2 +- .../DriverCapabilityInvokerFactory.cs | 27 ++++++++---- .../DriverCapabilityInvokerFactoryTests.cs | 42 +++++++++++++++++++ 3 files changed, 61 insertions(+), 10 deletions(-) diff --git a/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json b/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json index fa41c229..e79a243d 100644 --- a/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json +++ b/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json @@ -18,7 +18,7 @@ { "id": 14, "subject": "C-7 implement: JsonObject-bag FromJson/ToJson (TagConfigJson idiom), ParseFailed + RawStoredJson, blank->null contract preserved, parser-interop test green", "status": "completed", "blockedBy": [13] }, { "id": 15, "subject": "C-7 razor: ParseFailed warning banner + input lockout + explicit discard button; raw pane shows the stored ResilienceConfig; live-/run on docker-dev :9200 (rebuild BOTH centrals; SQL-mangle + unknown-key survival checks; also drive task 5 warnings and task 9 disabled cells)", "status": "deferred-live", "note": "docker-dev :9200 live-/run; serial live pass", "blockedBy": [14] }, { "id": 16, "subject": "S-7 builder: add options-generation to PipelineKey + GetOrCreate(optionsGeneration=0) + thread through CapabilityInvoker; StaleGeneration_ReCache_IsNotServed_ToNewGeneration", "status": "completed", "blockedBy": [3] }, - { "id": 17, "subject": "S-7 factory: Interlocked per-Create generation stamp + Respawn_interleaved_with_old_invoker_call_still_applies_new_options wiring proof; update Invalidate comment (cleanup, not correctness)", "status": "pending", "blockedBy": [16] }, + { "id": 17, "subject": "S-7 factory: Interlocked per-Create generation stamp + Respawn_interleaved_with_old_invoker_call_still_applies_new_options wiring proof; update Invalidate comment (cleanup, not correctness)", "status": "completed", "blockedBy": [16] }, { "id": 18, "subject": "Wrap-up: whole-solution build (0 warnings, analyzer silent) + full dotnet test gate; record the pass + the two report anchor drifts in archreview/plans/STATUS.md", "status": "pending", "blockedBy": [4, 8, 9, 12, 15, 17] } ], "lastUpdated": "2026-07-12" diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverCapabilityInvokerFactory.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverCapabilityInvokerFactory.cs index dc9919f7..ba014e52 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverCapabilityInvokerFactory.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverCapabilityInvokerFactory.cs @@ -19,11 +19,13 @@ namespace ZB.MOM.WW.OtOpcUa.Core.Resilience; /// /// Options are snapshotted once per (a driver actor's options are fixed /// for its lifetime — a ResilienceConfig change respawns the child), so the invoker's per-call -/// options accessor is allocation-free on the hot path. Because the pipeline builder caches -/// pipelines keyed on (instance, host, capability) and ignores options on a cache hit, -/// first s any pipelines -/// cached for the instance so a respawn with changed options rebuilds them (rather than silently -/// reusing the stale pipeline). On a first spawn this removes nothing. +/// options accessor is allocation-free on the hot path. The pipeline builder caches pipelines keyed on +/// (instance, host, capability, generation) and ignores options on a cache hit; each +/// stamps a fresh monotonic generation (01/S-7) so the new invoker can only +/// read pipelines it built — a lingering old child's post-invalidate re-cache lands under its own older +/// generation and is unreachable. still +/// s the instance's pipelines as cleanup (bounds +/// the cache), but correctness now rests on the generation key, not the invalidate timing. /// public sealed class DriverCapabilityInvokerFactory : IDriverCapabilityInvokerFactory { @@ -31,6 +33,7 @@ public sealed class DriverCapabilityInvokerFactory : IDriverCapabilityInvokerFac private readonly DriverResilienceStatusTracker _statusTracker; private readonly Func _tierResolver; private readonly ILogger? _logger; + private long _generation; /// Construct the factory over the shared resilience infrastructure. /// Process-singleton Polly pipeline builder (shared pipeline cache). @@ -65,9 +68,14 @@ public sealed class DriverCapabilityInvokerFactory : IDriverCapabilityInvokerFac driverInstanceId, driverType, diagnostic); } - // Drop any pipelines cached for this instance under the PREVIOUS options — the builder keys on - // (instance, host, capability) and reuses a cached pipeline regardless of options, so a respawn - // with a changed ResilienceConfig would otherwise keep serving the stale pipeline. No-op on first spawn. + // Stamp a fresh options generation for this invoker. The generation is part of the pipeline-cache + // key (01/S-7), so this invoker can only ever read pipelines it built — a lingering old child's + // late re-cache lands under its OWN (older) generation and is unreachable here. Correctness rests on + // the epoch key, not on the timing of the invalidate below. + var generation = Interlocked.Increment(ref _generation); + + // Cleanup (no longer the correctness mechanism): drop this instance's cached pipelines across all + // generations so lingering old-generation entries don't accumulate. No-op on first spawn. _builder.Invalidate(driverInstanceId); return new CapabilityInvoker( @@ -75,6 +83,7 @@ public sealed class DriverCapabilityInvokerFactory : IDriverCapabilityInvokerFac driverInstanceId, optionsAccessor: () => options, driverType: driverType, - statusTracker: _statusTracker); + statusTracker: _statusTracker, + optionsGeneration: generation); } } diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/DriverCapabilityInvokerFactoryTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/DriverCapabilityInvokerFactoryTests.cs index 04aeecc0..329096b3 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/DriverCapabilityInvokerFactoryTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/DriverCapabilityInvokerFactoryTests.cs @@ -161,6 +161,48 @@ public sealed class DriverCapabilityInvokerFactoryTests (e.Message.Contains("timeoutSeconds") || e.Message.Contains("breakerFailureThreshold"))); } + /// + /// 01/S-7 production-wiring proof: a respawn interleaved with a lingering old-invoker call must still + /// apply the new options. The old invoker (a captured reference — exactly what the old child holds) + /// re-caches its stale pipeline AFTER the new 's + /// invalidate; the new invoker must nonetheless behave per the new config. Fails without the per-Create + /// generation stamp (both invokers share the (instance, host, capability) key and the stale entry wins). + /// + [Fact] + public async Task Respawn_interleaved_with_old_invoker_call_still_applies_new_options() + { + var builder = new DriverResiliencePipelineBuilder(); + var factory = MakeFactory(builder); + + // Old invoker: Read retries (tier-A default 3). Warm its cache. + var oldInvoker = factory.Create("d1", "Modbus", "{\"capabilityPolicies\":{\"Read\":{\"retryCount\":3}}}"); + await FailingRead(oldInvoker); // caches the retrying pipeline under the old generation + + // Respawn with new options: Read no longer retries. + var newInvoker = factory.Create("d1", "Modbus", "{\"capabilityPolicies\":{\"Read\":{\"retryCount\":0}}}"); + + // The lingering old child's late call lands AFTER the new Create's invalidate — re-caching the stale + // retrying pipeline under the OLD generation. + await FailingRead(oldInvoker); + + // The new invoker must honor the new options (no retry) — a single attempt. + var attempts = 0; + await Should.ThrowAsync(async () => + await newInvoker.ExecuteAsync( + DriverCapability.Read, "host-1", + async _ => { attempts++; await Task.Yield(); throw new InvalidOperationException("boom"); }, + CancellationToken.None)); + + attempts.ShouldBe(1, "the new invoker must apply the new options (retryCount 0), not the stale re-cache"); + } + + private static async Task FailingRead(IDriverCapabilityInvoker invoker) => + await Should.ThrowAsync(async () => + await invoker.ExecuteAsync( + DriverCapability.Read, "host-1", + async _ => { await Task.Yield(); throw new InvalidOperationException("boom"); }, + CancellationToken.None)); + private sealed class CapturingLogger : ILogger { public List<(LogLevel Level, string Message)> Entries { get; } = new(); From dd82e7401137080e5d53417860057f3c4a88b53f Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 10:27:08 -0400 Subject: [PATCH 17/17] docs(r2-02): record R2-02 resilience hardening pass in STATUS.md + execution deviations --- .../plans/R2-02-resilience-config-hardening-plan.md | 10 ++++++++++ ...02-resilience-config-hardening-plan.md.tasks.json | 2 +- archreview/plans/STATUS.md | 12 ++++++++++++ 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/archreview/plans/R2-02-resilience-config-hardening-plan.md b/archreview/plans/R2-02-resilience-config-hardening-plan.md index 77c58567..70bde186 100644 --- a/archreview/plans/R2-02-resilience-config-hardening-plan.md +++ b/archreview/plans/R2-02-resilience-config-hardening-plan.md @@ -351,3 +351,13 @@ Bite-sized TDD tasks. Each carries classification, estimate, parallelism, files, 6. Task 18 rides the last PR. **Overall effort:** Small-Medium (matches 00-OVERALL action #2) — ≈ 19 bite-sized tasks, roughly one focused day including the live-`/run` session. **Overall risk:** Low — every behavior change is either strictly widening (clamps), strictly safening (write no-retry), or deletion of provably-inert surface; the two hot-path touches (write arm, pipeline key) are covered by behavioral guards and the OTOPCUA0001 analyzer keeps every dispatch site wrapped. + +--- + +## Execution deviations (R2-02) + +- **Task 15 (AdminUI live-`/run`) marked `deferred-live`.** All razor/code shipped and offline unit/bUnit-substitute suites are green; the docker-dev `:9200` `/run` pass (warning banner, input lockout, unknown-key survival) is a single-tenant serial live pass the controller runs one-at-a-time. Not blocking. +- **Task 18 whole-solution `dotnet test` sweep marked `deferred-live`.** Per a mid-execution controller constraint, the `*.IntegrationTests` / whole-solution test run has a ~16 GB/run memory leak and cannot run concurrently with other plans. I ran the impacted UNIT suites filtered (Core resilience 121/121, AdminUI form 10/10, Analyzers 32/32, Runtime wiring 2/2, full Runtime 364/364 at task 7) and the full-solution `dotnet build` (0 errors, 0 production OTOPCUA0001). The whole-solution `dotnet test` gate is left for the controller's serialized heavy pass. +- **`DriverInstanceResilienceStatus.CurrentBulkheadDepth` doc reworded to avoid the literal word "bulkhead" in free text** (Task 12). The plan permitted "one doc line on the entity" mentioning the bulkhead removal, but the Task-12 grep-must-be-empty check treats any "bulkhead" hit as a miss. Reworded to "Formerly fed a Polly concurrency-limiter strategy that was removed (R2-02/01/U-6)…" so the explanatory annotation stays while the grep is clean; the column NAME `CurrentBulkheadDepth` (excluded by the grep) still carries the historical term. The EF migration + `WedgeDetector.BulkheadDepth` were left untouched as scoped. +- **Baseline OTOPCUA0001 warnings in `AbLegacy.Tests` are pre-existing** (test code invoking the driver directly to assert wire-level behavior — the documented-acceptable case). My branch never touched those files and introduced zero new dispatch-site unwraps; production code has 0 OTOPCUA0001. +- **The worktree's local `master` ref is behind this branch's base (`1676c8f4`)**, so `git diff master..HEAD` shows unrelated base files (R2-01/03/05 plans, S7Driver, etc.). Only the 18 `fix(r2-02):`/`docs(r2-02):` commits are this plan's work. diff --git a/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json b/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json index e79a243d..12113498 100644 --- a/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json +++ b/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json @@ -19,7 +19,7 @@ { "id": 15, "subject": "C-7 razor: ParseFailed warning banner + input lockout + explicit discard button; raw pane shows the stored ResilienceConfig; live-/run on docker-dev :9200 (rebuild BOTH centrals; SQL-mangle + unknown-key survival checks; also drive task 5 warnings and task 9 disabled cells)", "status": "deferred-live", "note": "docker-dev :9200 live-/run; serial live pass", "blockedBy": [14] }, { "id": 16, "subject": "S-7 builder: add options-generation to PipelineKey + GetOrCreate(optionsGeneration=0) + thread through CapabilityInvoker; StaleGeneration_ReCache_IsNotServed_ToNewGeneration", "status": "completed", "blockedBy": [3] }, { "id": 17, "subject": "S-7 factory: Interlocked per-Create generation stamp + Respawn_interleaved_with_old_invoker_call_still_applies_new_options wiring proof; update Invalidate comment (cleanup, not correctness)", "status": "completed", "blockedBy": [16] }, - { "id": 18, "subject": "Wrap-up: whole-solution build (0 warnings, analyzer silent) + full dotnet test gate; record the pass + the two report anchor drifts in archreview/plans/STATUS.md", "status": "pending", "blockedBy": [4, 8, 9, 12, 15, 17] } + { "id": 18, "subject": "Wrap-up: whole-solution build (0 warnings, analyzer silent) + full dotnet test gate; record the pass + the two report anchor drifts in archreview/plans/STATUS.md", "status": "deferred-live", "note": "integration/full sweep; serialized heavy pass", "blockedBy": [4, 8, 9, 12, 15, 17] } ], "lastUpdated": "2026-07-12" } diff --git a/archreview/plans/STATUS.md b/archreview/plans/STATUS.md index 0271f9e9..34ec6e43 100644 --- a/archreview/plans/STATUS.md +++ b/archreview/plans/STATUS.md @@ -150,3 +150,15 @@ surface), 03/S2/S3 block-bridging, 03/P1 surgical adds (guard prerequisite in pl per-plan task counts, effort, and suggested execution order: [`00-INDEX.md`](00-INDEX.md) §"Round 2 plans". 193 bite-sized TDD tasks total, ~18–24 dev-days end-to-end; R2-01/02/03 (the re-review's new-and-sharp items) are ~2 days combined. + +## Round-2 execution — completed plans + +| Plan | Findings closed | Branch @ commit | Verification | +|---|---|---|---| +| **R2-02 — ResilienceConfig hardening** | 01/S-6 (High), 01/U-6, 01/S-8=03/S12, 04/C-7, 01/S-7=03/S13 | `r2/02-resilience-config-hardening` (18 commits `4be13429`…`d8c6357b`) | **01/S-6:** parser clamps timeout/retry/breaker overrides (timeout≤0→tier default, breaker==1→2, retry/breaker caps) with appending diagnostics + belt-and-braces `Build` clamp so a pipeline build can never throw Polly `ValidationException` (`ResiliencePolicyRanges` single-source-of-truth in Core.Abstractions); brick-repro guard + `Build_NeverThrows_ForHostileDirectOptions` + factory-seam wiring proof; AdminUI `ResilienceFormModel.Validate()` warning block. **01/S-8:** parser forces Write/AlarmAcknowledge retry overrides to 0 (spec invariant) **and** dispatch flips to `isIdempotent:false` (RED-first on `DriverInstanceActorResilienceWiringTests`); non-idempotent arm now caches the no-retry snapshot once per invoker (01/P-4) + records tracker in-flight accounting; `WriteIdempotentAttribute`'s false "reads via reflection" claim corrected; Write/Ack retry cells disabled in the form. **01/U-6:** bulkhead knob deleted end-to-end (options + parser + form + razor); stored keys become ignored unknowns (migration-free); tree-wide doc sweep; `Options_properties_are_exactly_the_pipeline_wired_set` inertness guard. **04/C-7:** `ResilienceFormModel` reworked to a `JsonObject`-bag round-trip preserving unknown top-level/capability/per-policy keys; malformed stored JSON sets `ParseFailed` → section locks + `ToJson` returns the original text verbatim + explicit discard button; raw pane shows the bound stored value. **01/S-7:** options-generation stamped per-`Create` (`Interlocked`) and threaded into the pipeline-cache key so a lingering old child's post-invalidate re-cache is unreachable by the new generation; `Invalidate` demoted to cleanup. Core resilience 121/121, AdminUI form 10/10, Analyzers 32/32, Runtime wiring 2/2 (+full Runtime 364/364 at task 7); solution builds clean, 0 production OTOPCUA0001. **Deferred to serial live pass:** task 15 (docker-dev `:9200` `/run` — warning banner, input lockout, unknown-key survival) + the whole-solution `dotnet test` sweep (integration suites excluded per the shared-rig memory-leak constraint). | + +**Report anchor drifts to fold into the next report refresh** (from the R2-02 verification pass): (1) +`01-core-composition.md` U-6 anchor `DriverResilienceOptions.cs:23-25` → the two bulkhead properties sat at +`:23` and `:29` (now deleted); (2) `01-core-composition.md` U-6 anchor +`DriverResiliencePipelineBuilder.cs:99-177` → `Build` was `:99-176`. All other cited lines were exact at +`f6eaa267`.