diff --git a/archreview/plans/R3-02-stale-good-family-plan.md b/archreview/plans/R3-02-stale-good-family-plan.md index 7cdc46d9..54fd096e 100644 --- a/archreview/plans/R3-02-stale-good-family-plan.md +++ b/archreview/plans/R3-02-stale-good-family-plan.md @@ -816,3 +816,35 @@ test could only pass once the production change landed. `docs/ScriptEditor.md` (T16), `docs/drivers/Calculation.md` (T14), `docs/drivers/Mqtt.md` (T5). - `CLAUDE.md`: one bullet under the comms-loss watchdog section — value nodes go Bad, nothing Good on reconnect (+ the audit meter), and the **(value, status)** dedup rule any future consumer must repeat. + +### Merge of master (R3-03 + R3-05 + R3-01) — before T11 + +`git merge master` at `d79d51dd`. Two conflicts, both **additive-vs-additive** on parameters R3-03 and +this plan each appended last: + +- `DriverHostActor` — `reprovisionBackoff` (R3-03) and `commsRecoveryAuditDelay` (T9) both claimed the + final ctor/Props slot, plus adjacent field blocks. Resolved by keeping BOTH in master-then-mine order + across all three positional sites (Props signature, `Props.Create` call, ctor), per the file's own + "new parameters go LAST in all three places" warning. +- `V3ArtifactSeeder` — R3-03 had independently added the same `enabled` + `driverFactory` seams T10 + needed (`driversEnabled`). **Master's naming won**; mine was deleted and the T10 call site moved to + `enabled: true` + named `commsRecoveryAuditDelay:`. `Spawn`/`SpawnHostAndApply` carry all three + optional knobs. +- One non-conflicting mis-merge caught by eye: git interleaved the new + `DriverCommsLatchedRefs` counter *between* `PrimaryGateDenied`'s xmldoc and its declaration in + `OtOpcUaTelemetry`. Re-ordered. +- Post-merge: solution builds clean, `Runtime.Tests` **670 passed / 3 skipped**. + +### T11 — `DriverInstanceActor.PostStop` announces comms loss + +- RED: `--filter "…Stopping_a_connected_child_announces…"` ⇒ **FAILED**, `Expected a message of type + …ConnectivityChanged, but received {…}` — stopping announced nothing at all. +- GREEN: `DriverInstanceActor*` 67/67; full `Runtime.Tests` **672 passed / 3 skipped**. +- **Deviation (narrowing, deliberate):** the Tell is **gated on the child having reached `Connected`** + (`_connectedSinceUtc != default`, the field R3-03 added for the watchdog grace) rather than + unconditional as the plan sketched. A never-connected child's nodes are still + `BadWaitingForInitialData`, which is more precise than "communication lost"; overwriting it would claim + an outage that never happened. Pinned by a control test + (`Stopping_a_never_connected_child_announces_nothing`). +- With R3-03 merged the recovery leg holds: `PostStop` → host latches Bad → R3-03's `SpawnChild` always + sends the desired set → the fresh child subscribes → the driver's initial push clears the latch. diff --git a/archreview/plans/R3-02-stale-good-family-plan.md.tasks.json b/archreview/plans/R3-02-stale-good-family-plan.md.tasks.json index 2e05fd07..36700d7f 100644 --- a/archreview/plans/R3-02-stale-good-family-plan.md.tasks.json +++ b/archreview/plans/R3-02-stale-good-family-plan.md.tasks.json @@ -79,7 +79,7 @@ { "id": "T11", "subject": "DriverInstanceActor.PostStop Tells ConnectivityChanged(false) so operator restart / respawn latches nodes Bad (land after R3-03 merges)", - "status": "pending", + "status": "done", "blockedBy": [ "T8" ] 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 cdcde90c..7fcc26b3 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs @@ -1420,6 +1420,21 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers /// protected override void PostStop() { + // Archreview 03/S15 — announce comms loss on the way out. Every OTHER route into Reconnecting + // (watchdog trip, failed initial connect, Force Reconnect) announces it from the Reconnecting + // entry; stopping did not, so an operator's Restart and a supervision respawn tore this child down + // silently and its value nodes went on serving their last value at Good across the gap. The fresh + // child's re-subscribe + initial push (03/S16) is what clears them again; on a deploy that REMOVES + // the driver the host has already rebuilt its ref index without it, so the Tell finds nothing. + // + // Gated on having actually reached Connected: a child that never connected leaves its nodes at + // BadWaitingForInitialData, which says something more precise than "communication lost", and + // overwriting it would claim an outage that never happened. + if (_connectedSinceUtc != default) + { + Context.Parent.Tell(new ConnectivityChanged(_driverInstanceId, Connected: false)); + } + DetachSubscription(); // MUST happen: the IDriver instance can outlive this actor (the host respawns a child around the // same driver object), so a missing unsubscribe accumulates a handler per respawn holding a dead Self. diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorTests.cs index 6f1c4958..9dd9eb0c 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorTests.cs @@ -374,4 +374,54 @@ public sealed class DriverInstanceActorTests : RuntimeActorTestBase AwaitCondition(() => driver.SubscribeCount >= 1, TimeSpan.FromSeconds(3)); } + /// + /// Archreview 03/S15 (T11) — a child that is stopped announces comms loss on its way + /// out, so the host latches its value nodes BadNoCommunication. + /// Every other route into Reconnecting announces it (watchdog trip, failed initial + /// connect, Force Reconnect all run the Reconnecting entry). Stopping did not: an operator's + /// Restart and a supervision respawn tore the child down silently, so its nodes kept serving their + /// last value at Good across the gap. The fresh child's re-subscribe + initial push (03/S16) is what + /// clears them again. + /// + [Fact] + public void Stopping_a_connected_child_announces_comms_loss_to_its_parent() + { + var parent = CreateTestProbe(); + var driver = new StubDriver(); + var actor = parent.ChildActorOf(DriverInstanceActor.Props(driver)); + + actor.Tell(new DriverInstanceActor.InitializeRequested("{}")); + parent.ExpectMsg(PresenceBudget).Connected.ShouldBeTrue(); + + parent.Watch(actor); + parent.Send(actor, PoisonPill.Instance); + + parent.ExpectMsg(PresenceBudget).Connected.ShouldBeFalse(); + parent.ExpectTerminated(actor, PresenceBudget); + } + + /// + /// Control. A child that never reached Connected announces nothing when stopped — its + /// nodes are still BadWaitingForInitialData, which is a more precise statement than + /// "communication lost", and overwriting it would claim an outage that never happened. + /// + [Fact] + public void Stopping_a_never_connected_child_announces_nothing() + { + var parent = CreateTestProbe(); + var driver = new StubDriver { InitializeShouldThrow = true }; + var actor = parent.ChildActorOf(DriverInstanceActor.Props( + driver, reconnectInterval: TimeSpan.FromSeconds(30))); + + actor.Tell(new DriverInstanceActor.InitializeRequested("{}")); + // The failed initial connect announces Connected=false from the Reconnecting entry; consume it, so + // what follows is only what STOPPING produced. + parent.ExpectMsg(PresenceBudget).Connected.ShouldBeFalse(); + + parent.Watch(actor); + parent.Send(actor, PoisonPill.Instance); + + parent.ExpectTerminated(actor, PresenceBudget); + parent.ExpectNoMsg(TimeSpan.FromMilliseconds(300)); + } }