From bc1f341e8aec9f7fed2346a02969a8c11317df55 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 16:58:37 -0400 Subject: [PATCH] fix(mtconnect): Unsupported latch inside the checked section; live-index reads do not clear a write degradation (review r1 N-4, N-5) - N-4: the /sample Unsupported latch is set inside TryDegradeStream's generation-checked critical section (latchUnsupported), so a superseded pump can no longer latch it on the session that replaced it. - N-5: a read served from the live pump index clears only the read degradation - it is stream evidence, like a chunk, which by the I-3 ruling does not clear an unanswered write. The 'cannot stick' wording in the XML doc and docs/drivers/MTConnect.md now states the latch-until-next-answered-/current-read- or-write behaviour, incl. the subscription-only consequence. Tested (chunk + live-index read keep Degraded; an answered write clears it). Refs #743 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT --- docs/drivers/MTConnect.md | 8 +++-- .../MTConnectDriver.cs | 30 ++++++++++------- .../MTConnectBatchedWriteTests.cs | 33 +++++++++++++++++++ 3 files changed, 57 insertions(+), 14 deletions(-) diff --git a/docs/drivers/MTConnect.md b/docs/drivers/MTConnect.md index ad0b7984..9999b3e4 100644 --- a/docs/drivers/MTConnect.md +++ b/docs/drivers/MTConnect.md @@ -437,9 +437,11 @@ refused — advances `LastSuccessfulRead`, with the state following the read / ` (a write cannot clear either of those paths' degradation). A write that got **no** answer (transport failure, deadline) sets **Degraded** with the stamp preserved and a type-only `LastError` (review 2026-10-21 I-3, coordinator ruling). One success writer is shared: that -degradation is cleared by the next answered **write** or the next good **read** (a `/current` -answer or a live-index read), so it cannot stick on a driver still talking to the Agent; a -`/sample` chunk alone does not clear it. +degradation is **latched until the next answered write or the next answered `/current` read**. +Neither a `/sample` chunk nor a read served from the live pump index clears it (both are stream +evidence, not an answer to a request), so on a **subscription-only** driver one timed-out write +keeps the driver Degraded until the next write — the comms-loss watchdog does not trip on it, +because the stream keeps advancing `LastSuccessfulRead`. **One request per device (review 2026-10-21 T-6).** The slots that survive the refusals above go out as one `POST {agent}/{device}?id1=v1&id2=v2…` per device (the Agent's PUT surface takes diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs index 853caf36..816b877b 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs @@ -371,8 +371,9 @@ public sealed partial class MTConnectDriver /// /// Whether the last Agent write got no reply (review I-3 / coordinator ruling: an unanswered write degrades). - /// Cleared by the next answered read OR write ( / ), - /// so it cannot latch on a driver that is still talking to the Agent. + /// Latched until the next answered /current read or answered write ( / + /// ). Neither a /sample chunk nor a live-index read clears it, so on + /// a subscription-only driver one timed-out write keeps the driver Degraded until the next write (r1 N-5). /// private volatile bool _writeDegraded; @@ -855,9 +856,13 @@ public sealed partial class MTConnectDriver // The read path is answering (from a stream that is delivering), so a degradation a failed /current left // behind is no longer true of it. The stamp stays the pump's — this read heard nothing from the Agent itself. - if (_readDegraded || _writeDegraded) + // Clears only the READ degradation (r1 N-5): an index read is the stream's evidence, not an Agent answer to a + // request — the same evidence a /sample chunk carries, which by the I-3 ruling does not clear an unanswered + // write either. + if (_readDegraded) { - RecordReadAnswered(ReadHealth().LastSuccessfulRead ?? DateTime.UtcNow); + _readDegraded = false; + PublishHealthy(ReadHealth().LastSuccessfulRead ?? DateTime.UtcNow); } return results; @@ -2255,12 +2260,9 @@ public sealed partial class MTConnectDriver // forever, so retrying would be an unbounded hot loop against an endpoint // that will never stream until an operator changes something. Latch it, say // so loudly, and stop. A re-initialize clears the latch. - lock (_subscriptionLock) - { - _streamUnsupported = true; - } - - if (!TryDegradeStream(generation, outcome.Error!.Message)) + // r1 N-4: the latch is set inside the same generation-checked section as the degrade, so a + // superseded pump can never latch Unsupported on the session that replaced it. + if (!TryDegradeStream(generation, outcome.Error!.Message, latchUnsupported: true)) { return; } @@ -2707,7 +2709,8 @@ public sealed partial class MTConnectDriver /// The pump's session generation. /// The failure (redacted text only). /// when the pump's session has been replaced (nothing written). - private bool TryDegradeStream(long generation, string message) + /// Also latch the "endpoint cannot stream" verdict (r1 N-4). + private bool TryDegradeStream(long generation, string message, bool latchUnsupported = false) { // The mirror of TryMarkStreamDelivered: a superseded pump's failure is not the new session's. lock (_subscriptionLock) @@ -2717,6 +2720,11 @@ public sealed partial class MTConnectDriver return false; } + if (latchUnsupported) + { + _streamUnsupported = true; + } + _streamDegraded = true; Degrade(message); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectBatchedWriteTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectBatchedWriteTests.cs index cb9c96c3..c230abbb 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectBatchedWriteTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectBatchedWriteTests.cs @@ -165,4 +165,37 @@ public sealed class MTConnectBatchedWriteTests driver.GetHealth().State.ShouldBe(DriverState.Degraded); } + + /// + /// r1 N-5: stream evidence does not clear an unanswered write — neither a delivered /sample chunk nor a read + /// served from the live pump index. The next answered write does (the positive control). + /// + [Fact] + public async Task Stream_evidence_does_not_clear_an_unanswered_write_but_an_answered_write_does() + { + var (driver, client) = await WritableAsync(); + try + { + await driver.SubscribeAsync([PosRaw], TimeSpan.FromMilliseconds(50), Ct); + await client.PumpAsync(MTConnectPumpGenerationChunks.Pos(108, 113, "1.5")).WaitAsync(TimeSpan.FromSeconds(30), Ct); + client.WriteFailure = new HttpRequestException("connection refused"); + await driver.WriteAsync([new WriteRequest(PosRaw, 9.5)], Ct); + driver.GetHealth().State.ShouldBe(DriverState.Degraded); + var currentCalls = client.CurrentCallCount; + + await client.PumpAsync(MTConnectPumpGenerationChunks.Pos(113, 118, "2.5")).WaitAsync(TimeSpan.FromSeconds(30), Ct); + (await driver.ReadAsync([PosRaw], Ct)).Single().StatusCode.ShouldBe(Good); + client.CurrentCallCount.ShouldBe(currentCalls, "the read was served from the live index"); + + driver.GetHealth().State.ShouldBe(DriverState.Degraded, "stream evidence is not an answer to the write"); + + client.WriteFailure = null; + await driver.WriteAsync([new WriteRequest(PosRaw, 3.5)], Ct); + driver.GetHealth().State.ShouldBe(DriverState.Healthy); + } + finally + { + await driver.ShutdownAsync(CancellationToken.None); + } + } }