fix(ablegacy): opaque replies are neither answers nor faults; round-2 minors

Review r1 N1 (Important): the I1 fix made IsDeviceRefusal default-open and mapped libplctag's ERR_REMOTE_ERR to an answer. libplctag folds EVERY non-zero PCCC STS and CIP status into that code, including a routing hop's "unconnected send timed out" / "connection lost" / "remote node missing", and native BadReply covers truncated / mismatched / unmapped replies - so a powered-off PLC behind a bridge read as reachable: Healthy, LastSuccessfulRead advancing, the comms-loss watchdog unable to fire. Three classes now, each pinned both ways: - no reply (Timeout/BadConnection/Abort/Pending/unknown): comms fault; - answered - an ALLOW-LIST again (NotFound/NotAllowed/OutOfBounds/TooLarge/ TooSmall/TypeMismatch/DeviceStateConflict): proves reachability; - OPAQUE (BadReply/Partial; ERR_REMOTE_ERR/ERR_BAD_STATUS now translate to the existing BadReply, OPC UA BadCommunicationError as before r0): per-tag Bad, handle kept, coalescing fallback, never fail-fast, never an answer. Option (a): a device whose pass/call produced only opaque outcomes is Degraded with LastSuccessfulRead not advanced, so the watchdog reaches a dead routed PLC. The probe and index reads classify the same way. Minors: - N2: a mismatched array (type vs data file) READS BadTypeMismatch instead of decoding the file at the wrong width as Good. - N3: a swept indirect resolution is never reconnected under a forgotten gate; that one call answers neutrally. - N4: an index read answered Unsupported is neutral, not a transport fault. - N5: ArrayFileMismatch checks indirect addresses (the file letter is static). - Test nit: the evicted-while-waiting test signals gate arrival via a seam instead of sleeping. Live: 51/51 on ab_server (slc500/micrologix/plc5 x both transports), incl. an opaque-only libplctag device staying Degraded with LastSuccessfulRead frozen. Refs #738 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
2026-10-22 15:45:04 -04:00
parent 8a5b4434a4
commit e3cb4f0c7b
8 changed files with 434 additions and 75 deletions
@@ -101,9 +101,46 @@ public sealed class AbLegacyReadCoalescingIntegrationTests(AbLegacyServerFixture
            r[0].StatusCode.ShouldBe(AbLegacyStatusMapper.Good, $"poll {poll}: neighbour A");
            r[1].StatusCode.ShouldBe(AbLegacyStatusMapper.Good, $"poll {poll}: neighbour B");
            r[2].StatusCode.ShouldNotBe(AbLegacyStatusMapper.Good, $"poll {poll}: the past-the-end tag");
-            r[2].StatusCode.ShouldNotBe(AbLegacyStatusMapper.BadCommunicationError,
-                $"poll {poll}: the PLC answered — not a comms fault (status 0x{r[2].StatusCode:X8})");
        }
+        // The refusal (libplctag: an opaque ERR_REMOTE_ERR; native: a mapped STS) is never a comms fault: the
+        // answering neighbours keep the driver Healthy and the device is never failed fast.
        drv.GetHealth().State.ShouldBe(DriverState.Healthy);
    }
+
+    /// <summary>
+    ///     Review 07 r1 N1, on the wire: libplctag reports the past-the-end refusal as the OPAQUE ERR_REMOTE_ERR —
+    ///     the same code a bridge returns for a powered-off PLC behind it — so a device whose ONLY traffic is that
+    ///     reply must not look reachable: Degraded, <c>LastSuccessfulRead</c> frozen (the comms-loss watchdog's
+    ///     evidence). The native transport maps the STS itself and is only checked to stay non-Good.
+    /// </summary>
+    /// <param name="profile">The simulator profile.</param>
+    /// <param name="transport">The transport stack the driver runs on.</param>
+    [AbLegacyTheory]
+    [MemberData(nameof(TransportMatrix.Profiles), MemberType = typeof(TransportMatrix))]
+    public async Task An_opaque_only_device_never_proves_reachability(AbLegacyServerProfile profile, AbLegacyTransport transport)
+    {
+        if (sim.SkipReasonFor(profile) is { } skip) Assert.Skip(skip);
+        var ct = TestContext.Current.CancellationToken;
+        var uri = $"ab://{sim.Host}:{sim.PortFor(profile)}/{sim.CipPath}";
+        await using var drv = new AbLegacyDriver(new AbLegacyDriverOptions
+        {
+            Devices = [new AbLegacyDeviceOptions(uri, profile.Family)],
+            RawTags = AbLegacyRawTags.Entries(new AbLegacyTagDefinition("PastEnd", uri, "N7:12", AbLegacyDataType.Int)),
+            Timeout = TimeSpan.FromSeconds(5),
+            Probe = new AbLegacyProbeOptions { Enabled = false },
+            Transport = transport,
+        }, driverInstanceId: $"ablegacy-opaque-{profile.Family}-{transport}");
+        await drv.InitializeAsync("{}", ct);
+        var initLastRead = drv.GetHealth().LastSuccessfulRead;
+        await Task.Delay(20, ct);
+
+        for (var poll = 0; poll < 2; poll++)
+            (await drv.ReadAsync(["PastEnd"], ct))[0].StatusCode.ShouldNotBe(AbLegacyStatusMapper.Good);
+
+        if (transport == AbLegacyTransport.Libplctag)
+        {
+            drv.GetHealth().State.ShouldBe(DriverState.Degraded);
+            drv.GetHealth().LastSuccessfulRead.ShouldBe(initLastRead);
+        }
+    }
 }
@@ -7,18 +7,24 @@ using ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport;
 namespace ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests;

 /// <summary>
-///     Review 07 r0 I1: on the default libplctag transport a PLC's own refusal (a non-zero PCCC STS — e.g. an element
-///     past the end of the file) arrives as <c>ERR_REMOTE_ERR</c>, which used to translate to <c>BadConnection</c>: a
-///     no-reply comms fault. A coalesced block holding one such member then failed as a group every poll, and two
-///     refused tags in a row tripped the device's fail-fast window. Statuses the device RETURNED are now answers;
-///     only no-reply statuses are comms faults.
+///     Pins BOTH sides of the reply-classification line (review 07 r0 I1 + r1 N1).
+///     <list type="bullet">
+///         <item><b>No reply</b> (Timeout / BadConnection / Abort / Pending / unknown) — comms fault: evict, Degraded,
+///         counts toward fail-fast.</item>
+///         <item><b>Answered</b> (an allow-list: NotFound / NotAllowed / OutOfBounds / TooLarge / TooSmall / TypeMismatch /
+///         DeviceStateConflict) — proves reachability: per-tag Bad, Healthy, <c>LastSuccessfulRead</c> advances.</item>
+///         <item><b>Opaque</b> (BadReply / Partial — incl. libplctag's <c>ERR_REMOTE_ERR</c>, which folds a PLC refusal
+///         and a routing hop's "end device gone" together) — neither: per-tag Bad, never fail-fast, never blacks out
+///         coalesced neighbours, but NEVER advances <c>LastSuccessfulRead</c>; an opaque-only device degrades so the
+///         comms-loss watchdog can reach a dead PLC behind a bridge.</item>
+///     </list>
 /// </summary>
 [Trait("Category", "Unit")]
 public sealed class AbLegacyAnsweredStatusTests
 {
    private const string Host = "ab://10.0.0.5/1,0";

-    /// <summary>What the libplctag runtime's GetStatus() reports for a PLC refusal (through the real translation).</summary>
+    /// <summary>What the libplctag runtime's GetStatus() reports for ERR_REMOTE_ERR (through the real translation).</summary>
    private static readonly int RemoteErr = (int)LibplctagLegacyStatusTranslation.ToAbStatus(Status.ErrorRemoteErr);

    private static AbLegacyDriver NewDriver(FakeAbLegacyTagFactory factory, bool coalesce, params AbLegacyTagDefinition[] tags) =>
@@ -30,37 +36,75 @@ public sealed class AbLegacyAnsweredStatusTests
            CoalesceReads = coalesce,
        }, "drv-i1", factory);

-    [Fact]
-    public void Remote_error_translates_to_an_answered_status_not_a_comms_fault()
+    // ---- the classification table ----
+
+    [Theory]
+    [InlineData(AbStatus.NotFound)]
+    [InlineData(AbStatus.NotAllowed)]
+    [InlineData(AbStatus.OutOfBounds)]
+    [InlineData(AbStatus.TooLarge)]
+    [InlineData(AbStatus.TooSmall)]
+    [InlineData(AbStatus.TypeMismatch)]
+    [InlineData(AbStatus.DeviceStateConflict)]
+    public void The_answered_class_is_an_allow_list(AbStatus status)
    {
-        var s = LibplctagLegacyStatusTranslation.ToAbStatus(Status.ErrorRemoteErr);
-        AbLegacyDriver.IsNoReply(s).ShouldBeFalse();
-        AbLegacyDriver.IsDeviceRefusal(s).ShouldBeTrue();
-        AbLegacyDriver.IsDeviceRefusal(LibplctagLegacyStatusTranslation.ToAbStatus(Status.ErrorBadStatus)).ShouldBeTrue();
-        // Positive control: real no-reply statuses still are.
-        AbLegacyDriver.IsNoReply(LibplctagLegacyStatusTranslation.ToAbStatus(Status.ErrorTimeout)).ShouldBeTrue();
-        AbLegacyDriver.IsNoReply(LibplctagLegacyStatusTranslation.ToAbStatus(Status.ErrorBadConnection)).ShouldBeTrue();
+        AbLegacyDriver.IsDeviceRefusal(status).ShouldBeTrue();
+        AbLegacyDriver.IsOpaque(status).ShouldBeFalse();
+        AbLegacyDriver.IsNoReply(status).ShouldBeFalse();
    }

    [Theory]
-    [InlineData(AbStatus.BadReply)]     // native: an STS the transport does not map
+    [InlineData(AbStatus.BadReply)]
    [InlineData(AbStatus.Partial)]
-    [InlineData(AbStatus.NotFound)]
-    [InlineData(AbStatus.DeviceStateConflict)]
-    public void Statuses_the_device_returned_are_answers(AbStatus status) =>
-        AbLegacyDriver.IsDeviceRefusal(status).ShouldBeTrue();
+    public void Unclassifiable_replies_are_opaque_not_answers(AbStatus status)
+    {
+        AbLegacyDriver.IsOpaque(status).ShouldBeTrue();
+        AbLegacyDriver.IsDeviceRefusal(status).ShouldBeFalse();
+        AbLegacyDriver.IsNoReply(status).ShouldBeFalse();
+    }

    [Theory]
    [InlineData(AbStatus.Timeout)]
    [InlineData(AbStatus.BadConnection)]
    [InlineData(AbStatus.Abort)]
+    [InlineData(AbStatus.Pending)]
    [InlineData((AbStatus)(-99))] // a code this build does not know: conservatively no reply
    public void Only_no_reply_statuses_are_comms_faults(AbStatus status)
    {
        AbLegacyDriver.IsNoReply(status).ShouldBeTrue();
        AbLegacyDriver.IsDeviceRefusal(status).ShouldBeFalse();
+        AbLegacyDriver.IsOpaque(status).ShouldBeFalse();
    }

+    [Fact]
+    public void Every_non_Ok_status_lands_in_at_most_one_class_and_Unsupported_in_none()
+    {
+        foreach (var s in Enum.GetValues<AbStatus>().Where(s => s != AbStatus.Ok))
+        {
+            var classes = new[] { AbLegacyDriver.IsNoReply(s), AbLegacyDriver.IsDeviceRefusal(s), AbLegacyDriver.IsOpaque(s) }
+                .Count(c => c);
+            if (s == AbStatus.Unsupported) classes.ShouldBe(0, "Unsupported is neutral");
+            else classes.ShouldBe(1, s.ToString());
+        }
+    }
+
+    [Fact]
+    public void libplctag_remote_errors_translate_to_opaque_not_to_an_answer_or_a_fault()
+    {
+        foreach (var st in new[] { Status.ErrorRemoteErr, Status.ErrorBadStatus })
+        {
+            var s = LibplctagLegacyStatusTranslation.ToAbStatus(st);
+            AbLegacyDriver.IsOpaque(s).ShouldBeTrue(st.ToString());
+            AbLegacyDriver.IsDeviceRefusal(s).ShouldBeFalse(st.ToString());
+            AbLegacyDriver.IsNoReply(s).ShouldBeFalse(st.ToString());
+        }
+        // Positive controls on the other two sides.
+        AbLegacyDriver.IsNoReply(LibplctagLegacyStatusTranslation.ToAbStatus(Status.ErrorTimeout)).ShouldBeTrue();
+        AbLegacyDriver.IsDeviceRefusal(LibplctagLegacyStatusTranslation.ToAbStatus(Status.ErrorNotFound)).ShouldBeTrue();
+    }
+
+    // ---- behaviour: the I1 side (an opaque member never blacks out its neighbours, never trips fail-fast) ----
+
    [Fact]
    public async Task A_remote_error_member_never_blacks_out_its_coalesced_neighbours()
    {
@@ -82,15 +126,15 @@ public sealed class AbLegacyAnsweredStatusTests
            var r = await drv.ReadAsync(["A", "B", "C"], CancellationToken.None);
            r[0].StatusCode.ShouldBe(AbLegacyStatusMapper.Good, $"poll {poll}");
            r[1].StatusCode.ShouldBe(AbLegacyStatusMapper.Good, $"poll {poll}");
-            r[2].StatusCode.ShouldBe(AbLegacyStatusMapper.BadDeviceFailure, $"poll {poll}");
+            r[2].StatusCode.ShouldNotBe(AbLegacyStatusMapper.Good, $"poll {poll}");
        }
-        drv.GetHealth().State.ShouldBe(DriverState.Healthy);
+        drv.GetHealth().State.ShouldBe(DriverState.Healthy); // the neighbours answered
        drv.GetDeviceState(Host)!.IsThrottled(DateTime.UtcNow).ShouldBeFalse();
        await drv.ShutdownAsync(CancellationToken.None);
    }

    [Fact]
-    public async Task Consecutive_refused_tags_do_not_trip_fail_fast_on_an_answering_device()
+    public async Task Consecutive_opaque_replies_do_not_trip_fail_fast_on_an_answering_device()
    {
        var factory = new FakeAbLegacyTagFactory
        {
@@ -106,10 +150,82 @@ public sealed class AbLegacyAnsweredStatusTests
        for (var poll = 0; poll < 3; poll++)
        {
            var r = await drv.ReadAsync(["R1", "R2", "R3", "Good"], CancellationToken.None);
-            r[3].StatusCode.ShouldBe(AbLegacyStatusMapper.Good, $"poll {poll}: an answering device is never failed fast");
+            r[3].StatusCode.ShouldBe(AbLegacyStatusMapper.Good, $"poll {poll}: never failed fast");
        }
        factory.Tags["N7:9"].ReadCount.ShouldBe(3);
-        factory.Tags["N7:0"].InitializeCount.ShouldBe(1); // refused handles are kept, not churned
+        factory.Tags["N7:0"].InitializeCount.ShouldBe(1); // opaque handles are kept, not churned
+        drv.GetHealth().State.ShouldBe(DriverState.Healthy);
+        await drv.ShutdownAsync(CancellationToken.None);
+    }
+
+    // ---- behaviour: the N1 side (an opaque reply never proves reachability) ----
+
+    [Fact]
+    public async Task Remote_error_only_reads_never_advance_LastSuccessfulRead_and_degrade()
+    {
+        // A PLC behind a bridge is powered off: every request comes back fast with ERR_REMOTE_ERR.
+        var factory = new FakeAbLegacyTagFactory { Customise = p => new FakeAbLegacyTag(p) { Value = 1 } };
+        var drv = NewDriver(factory, coalesce: false,
+            new AbLegacyTagDefinition("X", Host, "N7:0", AbLegacyDataType.Int),
+            new AbLegacyTagDefinition("Y", Host, "N7:5", AbLegacyDataType.Int));
+        await drv.InitializeAsync("{}", CancellationToken.None);
+        (await drv.ReadAsync(["X", "Y"], CancellationToken.None)).ShouldAllBe(s => s.StatusCode == AbLegacyStatusMapper.Good);
+        var healthyAt = drv.GetHealth().LastSuccessfulRead; // positive control: Good reads DID advance it
+        healthyAt.ShouldNotBeNull();
+        foreach (var t in factory.Tags.Values) t.Status = RemoteErr;
+        await Task.Delay(5);
+
+        for (var poll = 0; poll < 3; poll++)
+            (await drv.ReadAsync(["X", "Y"], CancellationToken.None)).ShouldAllBe(s => s.StatusCode == AbLegacyStatusMapper.BadCommunicationError);
+
+        var h = drv.GetHealth();
+        h.LastSuccessfulRead.ShouldBe(healthyAt); // frozen: the comms-loss watchdog can reach this device
+        h.State.ShouldBe(DriverState.Degraded);
+        drv.GetDeviceState(Host)!.IsThrottled(DateTime.UtcNow).ShouldBeFalse(); // but no fail-fast window
+        factory.Tags["N7:0"].ReadCount.ShouldBe(4); // every poll still asked
+        await drv.ShutdownAsync(CancellationToken.None);
+    }
+
+    private sealed class ThrowingTag(AbLegacyTagCreateParams p, AbStatus status) : FakeAbLegacyTag(p)
+    {
+        public override Task ReadAsync(CancellationToken ct) => throw new AbTransportException(status, $"native {status}");
+    }
+
+    [Theory]
+    [InlineData(AbStatus.BadReply)]
+    [InlineData(AbStatus.Partial)]
+    public async Task A_native_unclassifiable_reply_is_not_counted_as_an_answer(AbStatus status)
+    {
+        var factory = new FakeAbLegacyTagFactory { Customise = p => new FakeAbLegacyTag(p) { Value = 1 } };
+        var drv = NewDriver(factory, coalesce: false, new AbLegacyTagDefinition("X", Host, "N7:0", AbLegacyDataType.Int));
+        await drv.InitializeAsync("{}", CancellationToken.None);
+        (await drv.ReadAsync(["X"], CancellationToken.None))[0].StatusCode.ShouldBe(AbLegacyStatusMapper.Good);
+        var healthyAt = drv.GetHealth().LastSuccessfulRead;
+        drv.GetDeviceState(Host)!.Runtimes.TryRemove("X", out _);
+        factory.Customise = p => new ThrowingTag(p, status);
+        await Task.Delay(5);
+
+        var r = await drv.ReadAsync(["X"], CancellationToken.None);
+
+        r[0].StatusCode.ShouldBe(AbLegacyStatusMapper.MapStatus(status));
+        drv.GetHealth().LastSuccessfulRead.ShouldBe(healthyAt);
+        drv.GetHealth().State.ShouldBe(DriverState.Degraded);
+        await drv.ShutdownAsync(CancellationToken.None);
+    }
+
+    [Fact]
+    public async Task Positive_control_a_real_device_refusal_alone_still_proves_reachability()
+    {
+        var factory = new FakeAbLegacyTagFactory { Customise = p => new FakeAbLegacyTag(p) { Status = (int)AbStatus.NotFound } };
+        var drv = NewDriver(factory, coalesce: false, new AbLegacyTagDefinition("X", Host, "N7:0", AbLegacyDataType.Int));
+        await drv.InitializeAsync("{}", CancellationToken.None);
+        var before = drv.GetHealth().LastSuccessfulRead;
+        await Task.Delay(5);
+
+        (await drv.ReadAsync(["X"], CancellationToken.None))[0].StatusCode.ShouldBe(AbLegacyStatusMapper.BadNodeIdUnknown);
+
+        drv.GetHealth().State.ShouldBe(DriverState.Healthy);
+        drv.GetHealth().LastSuccessfulRead!.Value.ShouldBeGreaterThan(before!.Value);
        await drv.ShutdownAsync(CancellationToken.None);
    }
 }
@@ -123,6 +123,21 @@ public sealed class AbLegacyArrayWriteTests
            .ShouldContain(w => w.Contains("element width")); // and the deploy gate names it
    }

+    /// <summary>Review 07 r1 N5: the file letter of an indirect address is static, so the deploy gate checks it too.</summary>
+    [Theory]
+    [InlineData("N7:[N7:0]", "Float", true)]
+    [InlineData("F[N7:0]:2", "Int", true)]
+    [InlineData("N7:[N7:0]", "Int", false)]
+    [InlineData("F[N7:0]:2", "Float", false)]
+    public void An_indirect_array_type_file_mismatch_is_reported_at_the_gate(string address, string dataType, bool mismatch)
+    {
+        var json = $$"""{"address":"{{address}}","dataType":"{{dataType}}","isArray":true,"arrayLength":2}""";
+        Enum.TryParse<AbLegacyDataType>(dataType, out var type).ShouldBeTrue();
+
+        (AbLegacyTagDefinitionFactory.ArrayFileMismatch(type, address) is not null).ShouldBe(mismatch);
+        AbLegacyTagDefinitionFactory.Inspect(json).Any(w => w.Contains("element width")).ShouldBe(mismatch);
+    }
+
    [Fact]
    public async Task Positive_control_a_Float_array_on_an_F_file_is_written()
    {
@@ -243,8 +243,12 @@ public sealed class AbLegacyHealthEvidenceTests

        var gate = state.GetRuntimeLock("X");
        await gate.WaitAsync();
-        var read = drv.ReadAsync(["X"], CancellationToken.None); // fetches `first`, then parks on the gate
-        await Task.Delay(30);
+        // Signalled at exactly "handle fetched, about to wait on the gate" — no sleep, so the eviction below always
+        // lands between the reader's fetch and its use (review 07 r1 test nit: the sleep could pass vacuously).
+        var atGate = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
+        state.BeforeOperationGate = key => { if (key == "X") atGate.TrySetResult(); };
+        var read = drv.ReadAsync(["X"], CancellationToken.None);
+        await atGate.Task.WaitAsync(TimeSpan.FromSeconds(5));
        state.Runtimes.TryRemove("X", out var evicted).ShouldBeTrue();
        evicted!.Dispose();
        gate.Release();
@@ -255,4 +259,92 @@ public sealed class AbLegacyHealthEvidenceTests
        drv.GetHealth().State.ShouldBe(DriverState.Healthy);
        await drv.ShutdownAsync(CancellationToken.None);
    }
+
+    /// <summary>
+    ///     Review 07 r1 N3: a reader parked on a resolved INDIRECT key's gate while the index sweep evicted that key and
+    ///     forgot its gate used to RECONNECT it under the lock — resurrecting a stale resolution under a gate nobody
+    ///     else would get. It now answers that one call neutrally and leaves the key gone.
+    /// </summary>
+    [Fact]
+    public async Task A_swept_indirect_resolution_is_never_resurrected_under_a_forgotten_gate()
+    {
+        var creates = new System.Collections.Concurrent.ConcurrentQueue<string>();
+        var f = new SwitchableFactory
+        {
+            Make = p =>
+            {
+                creates.Enqueue(p.TagName);
+                return new FakeAbLegacyTag(p) { Value = p.TagName == "N7:0" ? (short)5 : (short)42 };
+            },
+        };
+        var drv = NewDriver(f, new AbLegacyTagDefinition("Ind", Host, "N7:[N7:0]", AbLegacyDataType.Int));
+        await drv.InitializeAsync("{}", CancellationToken.None);
+        (await drv.ReadAsync(["Ind"], CancellationToken.None))[0].Value.ShouldBe(42); // positive control
+        var state = drv.GetDeviceState(Host)!;
+        const string key = "Ind=>N7:5";
+        state.Runtimes.ContainsKey(key).ShouldBeTrue();
+
+        var gate = state.GetRuntimeLock(key);
+        await gate.WaitAsync();
+        var atGate = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
+        state.BeforeOperationGate = k => { if (k == key) atGate.TrySetResult(); };
+        var read = drv.ReadAsync(["Ind"], CancellationToken.None);
+        await atGate.Task.WaitAsync(TimeSpan.FromSeconds(5));
+        // What the sweep does when the index moves: evict the resolution, then forget its gate.
+        state.Runtimes.TryRemove(key, out var swept).ShouldBeTrue();
+        swept!.Dispose();
+        state.ForgetGates(key);
+        gate.Release();
+        var r = await read;
+
+        r[0].StatusCode.ShouldBe(AbLegacyStatusMapper.BadCommunicationError);
+        state.Runtimes.ContainsKey(key).ShouldBeFalse();  // not resurrected
+        creates.Count(n => n == "N7:5").ShouldBe(1);       // no reconnect of the stale resolution
+        drv.GetHealth().State.ShouldBe(DriverState.Healthy); // neutral: no health evidence either way
+        await drv.ShutdownAsync(CancellationToken.None);
+    }
+
+    /// <summary>Review 07 r1 N4: an index read answered Unsupported is neutral, not a transport fault.</summary>
+    [Fact]
+    public async Task An_index_read_answered_Unsupported_is_neutral()
+    {
+        var f = new SwitchableFactory
+        {
+            Make = p => p.TagName == "N7:0"
+                ? new FakeAbLegacyTag(p) { Status = (int)AbStatus.Unsupported }
+                : new FakeAbLegacyTag(p) { Value = 1 },
+        };
+        var drv = NewDriver(f,
+            new AbLegacyTagDefinition("Ind", Host, "N7:[N7:0]", AbLegacyDataType.Int),
+            new AbLegacyTagDefinition("Plain", Host, "N9:0", AbLegacyDataType.Int));
+        await drv.InitializeAsync("{}", CancellationToken.None);
+
+        var r = await drv.ReadAsync(["Ind", "Plain"], CancellationToken.None);
+
+        r[0].StatusCode.ShouldBe(AbLegacyStatusMapper.BadNotSupported);
+        r[1].StatusCode.ShouldBe(AbLegacyStatusMapper.Good); // positive control
+        drv.GetHealth().State.ShouldBe(DriverState.Healthy);
+        drv.GetDeviceState(Host)!.ParentRuntimes.ContainsKey("N7:0").ShouldBeTrue(); // handle kept, not evicted
+        await drv.ShutdownAsync(CancellationToken.None);
+    }
+
+    /// <summary>Review 07 r1 N2: a persisted mismatched array READS BadTypeMismatch instead of garbage as Good.</summary>
+    [Fact]
+    public async Task A_mismatched_array_reads_BadTypeMismatch_not_garbage()
+    {
+        var f = new SwitchableFactory { Make = p => new FakeAbLegacyTag(p) { ArrayValue = new[] { 1f, 2f } } };
+        var drv = NewDriver(f,
+            new AbLegacyTagDefinition("Bad", Host, "F8:0", AbLegacyDataType.Int, ArrayLength: 2),
+            new AbLegacyTagDefinition("Good", Host, "F9:0", AbLegacyDataType.Float, ArrayLength: 2));
+        await drv.InitializeAsync("{}", CancellationToken.None);
+
+        var r = await drv.ReadAsync(["Bad", "Good"], CancellationToken.None);
+
+        r[0].StatusCode.ShouldBe(AbLegacyStatusMapper.BadTypeMismatch);
+        f.Tags.ContainsKey("F8:0").ShouldBeFalse(); // nothing read at the wrong width
+        r[1].StatusCode.ShouldBe(AbLegacyStatusMapper.Good); // positive control
+        r[1].Value.ShouldBe(new[] { 1f, 2f });
+        drv.GetHealth().State.ShouldBe(DriverState.Healthy);
+        await drv.ShutdownAsync(CancellationToken.None);
+    }
 }
@@ -89,9 +89,6 @@ public sealed class AbLegacyHostAndStatusTests
        Status.ErrorNotAllowed => AbLegacyStatusMapper.BadNotWritable,
        Status.ErrorOutOfBounds or Status.ErrorTooLarge or Status.ErrorTooSmall => AbLegacyStatusMapper.BadOutOfRange,
        Status.ErrorUnsupported or Status.ErrorNotImplemented => AbLegacyStatusMapper.BadNotSupported,
-        // DELIBERATE change (review 07 r0 I1): a PLC's own refusal (PCCC STS / CIP status → ERR_REMOTE_ERR /
-        // ERR_BAD_STATUS) is "device answered", not a comms error.
-        Status.ErrorRemoteErr or Status.ErrorBadStatus => AbLegacyStatusMapper.BadDeviceFailure,
        _ => AbLegacyStatusMapper.BadCommunicationError,
    };