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); } }