fix(r2-04): Galaxy fail-closed write when AdviseSupervisory fails - Bad status feeds #5 node revert (06/S-1)

2026-07-13 10:51:16 -04:00
parent a537d29f44
commit 610306734c
3 changed files with 69 additions and 7 deletions
@@ -7,7 +7,7 @@
    { "id": "T3", "subject": "Materialise* passes return swallowed-failure counts (void -> int, five methods)", "status": "completed", "blockedBy": ["T2"] },
    { "id": "T4", "subject": "OpcUaApplyFailed counter + OpcUaPublishActor Error/meter surfacing of degraded applies", "status": "completed", "blockedBy": ["T3"] },
    { "id": "T5", "subject": "Galaxy internal IMxWriteOps seam under GatewayGalaxyDataWriter (sealed SDK session untestable) (06/S-1)", "status": "completed", "blockedBy": [] },
-    { "id": "T6", "subject": "Galaxy fail-closed write on AdviseSupervisory failure -> BadCommunicationError, no raw write", "status": "pending", "blockedBy": ["T5"] },
+    { "id": "T6", "subject": "Galaxy fail-closed write on AdviseSupervisory failure -> BadCommunicationError, no raw write", "status": "completed", "blockedBy": ["T5"] },
    { "id": "T7", "subject": "Galaxy counters galaxy.writes.advise_failed + galaxy.writes.unconfirmed (empty-statuses Good metered)", "status": "pending", "blockedBy": ["T6"] },
    { "id": "T8", "subject": "Galaxy skip-gated live smoke: normal advised write still returns Good post-change", "status": "pending", "blockedBy": ["T6"] },
    { "id": "T9", "subject": "PrimaryGatePolicy pure class + truth-table tests + otopcua.redundancy.primary_gate_denied counter (03/S4)", "status": "pending", "blockedBy": [] },
@@ -178,8 +178,16 @@ public sealed class GatewayGalaxyDataWriter : IGalaxyDataWriter
                // A raw Write runs with NO user login (WriteUserId is typically 0), so MXAccess
                // only COMMITS the value when the item is advised in SUPERVISORY mode. Without it
                // the gateway's Write call doesn't throw (reply looks OK) but the value never
-                // reaches the galaxy. AdviseSupervisory once per handle, then Write.
-                await EnsureSupervisoryAdvisedAsync(ops, itemHandle, ct).ConfigureAwait(false);
+                // reaches the galaxy — a knowingly-lost write. FAIL-CLOSED (archreview 06/S-1): when
+                // AdviseSupervisory fails we do NOT issue the raw Write; we return Bad so the
+                // #5 write-outcome self-correction reverts the phantom-Good node instead of leaving it.
+                // The real fix is a gateway-side WriteComplete correlation (mxaccessgw backlog) so the
+                // unary reply's Statuses carry the actual COM commit outcome; until then advise-status +
+                // the galaxy.writes.* meters are the honest signal.
+                if (!await EnsureSupervisoryAdvisedAsync(ops, itemHandle, ct).ConfigureAwait(false))
+                {
+                    return new WriteResult(StatusCodeMap.BadCommunicationError);
+                }
                reply = await ops.WriteRawAsync(itemHandle, mxValue, _writeUserId, ct)
                    .ConfigureAwait(false);
            }
@@ -226,10 +234,16 @@ public sealed class GatewayGalaxyDataWriter : IGalaxyDataWriter
    ///     session reconnect via <see cref="InvalidateHandleCaches"/>, so a fresh session always
    ///     re-advises — the supervisory state never outlives the handle it was taken against.
    /// </summary>
-    private async Task EnsureSupervisoryAdvisedAsync(
+    /// <summary>Ensure the item is supervisory-advised so a no-login raw Write can commit. Returns
+    /// <c>true</c> when the item is advised (already-advised this session, or the advise round-trip
+    /// succeeded), <c>false</c> when the advise failed — the caller then FAILS THE WRITE CLOSED
+    /// (archreview 06/S-1) rather than issuing a raw Write that cannot commit.</summary>
+    /// <returns><c>true</c> when advised; <c>false</c> when the advise failed (caller must not write).</returns>
+    private async Task<bool> EnsureSupervisoryAdvisedAsync(
        IMxWriteOps ops, int itemHandle, CancellationToken ct)
    {
-        if (!_supervisedHandles.TryAdd(itemHandle, 0)) return;
+        // Already advised this session (a prior successful advise on this handle) — nothing to do.
+        if (!_supervisedHandles.TryAdd(itemHandle, 0)) return true;

        var request = new MxCommandRequest
        {
@@ -249,13 +263,16 @@ public sealed class GatewayGalaxyDataWriter : IGalaxyDataWriter
        var reply = await ops.InvokeAsync(request, ct).ConfigureAwait(false);
        if (reply.ProtocolStatus is { } proto && proto.Code != ProtocolStatusCode.Ok)
        {
-            // Supervisory advise failed — forget it so the next write retries, and let the
-            // write proceed (it surfaces its own status via TranslateReply).
+            // Supervisory advise failed — forget it so the next write retries the advise, and report
+            // failure so the caller fails the write closed (the raw Write would not commit).
            _supervisedHandles.TryRemove(itemHandle, out _);
            _logger.LogWarning(
                "GalaxyDriver supervisory advise failed for item {ItemHandle}: {Code} {Message}",
                itemHandle, proto.Code, proto.Message);
+            return false;
        }
+
+        return true;
    }

    /// <summary>
@@ -36,6 +36,51 @@ public sealed class GatewayGalaxyDataWriterFailClosedTests
        ops.RawWriteHandles.ShouldBeEmpty();
    }

+    // ---------------- T6: fail-closed on advise failure ----------------
+
+    /// <summary>When AdviseSupervisory fails (non-OK protocol status), the raw Write must NOT be issued and
+    /// the writer returns Bad (which fires the #5 node revert). The handle must not linger in the supervised
+    /// cache so the next write retries the advise.</summary>
+    [Fact]
+    public async Task WriteOne_AdviseFails_ReturnsBadCommunicationError_AndNeverIssuesRawWrite()
+    {
+        var ops = new FakeMxWriteOps
+        {
+            InvokeResponder = _ => new MxCommandReply
+            {
+                ProtocolStatus = new ProtocolStatus { Code = ProtocolStatusCode.MxaccessFailure, Message = "advise boom" },
+            },
+        };
+        var writer = NewWriter();
+
+        var result = await writer.WriteOneForTestAsync(
+            ops, new WriteRequest("Tag.A", true), SecurityClassification.FreeAccess, CancellationToken.None);
+
+        result.StatusCode.ShouldBe(StatusCodeMap.BadCommunicationError);
+        ops.Invokes.Count(r => r.Command.Kind == MxCommandKind.AdviseSupervisory).ShouldBe(1);
+        ops.RawWriteHandles.ShouldBeEmpty();               // the knowingly-lost raw write was NOT issued
+        writer.CachedSupervisedHandleCount.ShouldBe(0);    // advise failure forgot the handle → next write retries
+    }
+
+    /// <summary>When advise succeeds, the raw Write proceeds and a clean reply returns Good (regression).</summary>
+    [Fact]
+    public async Task WriteOne_AdviseSucceeds_RawWriteProceeds()
+    {
+        var ops = new FakeMxWriteOps
+        {
+            InvokeResponder = _ => new MxCommandReply { ProtocolStatus = new ProtocolStatus { Code = ProtocolStatusCode.Ok } },
+        };
+        var writer = NewWriter();
+
+        var result = await writer.WriteOneForTestAsync(
+            ops, new WriteRequest("Tag.B", 42), SecurityClassification.FreeAccess, CancellationToken.None);
+
+        result.StatusCode.ShouldBe(StatusCodeMap.Good);
+        ops.Invokes.Count(r => r.Command.Kind == MxCommandKind.AdviseSupervisory).ShouldBe(1);
+        ops.RawWriteHandles.ShouldHaveSingleItem();        // the raw write was issued after the successful advise
+        writer.CachedSupervisedHandleCount.ShouldBe(1);
+    }
+
    // ---------------- fakes ----------------

    /// <summary>A fake <see cref="IMxWriteOps"/> that records every call and returns canned replies.</summary>