From 5f7a2acd2762cde12a0da4435bac80dd6aa00113 Mon Sep 17 00:00:00 2001
From: Joseph Doherty <dohejw01@gmail.com>
Date: Tue, 16 Jun 2026 22:30:11 -0400
Subject: [PATCH] fix(abcip): isArray:true without a positive arrayLength is
 scalar (review I-2 consistency)

---
 .../AbCipEquipmentTagParser.cs                | 12 ++++----
 .../AbCipArrayTests.cs                        | 28 +++++++++++++++++++
 2 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts/AbCipEquipmentTagParser.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts/AbCipEquipmentTagParser.cs
index 80e1746a..2a799fcb 100644
--- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts/AbCipEquipmentTagParser.cs
+++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts/AbCipEquipmentTagParser.cs
@@ -49,9 +49,11 @@ public static class AbCipEquipmentTagParser
 
     /// <summary>
     ///     Resolve the 1-D array shape from an <c>isArray</c> / <c>arrayLength</c> pair (the foundation
-    ///     contract carrier). The tag is an ARRAY ⟺ <c>isArray</c> is truthy AND <c>arrayLength</c>
-    ///     is a number <c>&gt;= 1</c> (a 1-element array is valid). Returns
-    ///     <c>(IsArray: false, ElementCount: 1)</c> for a scalar.
+    ///     contract carrier). The canonical rule: the tag is an ARRAY ⟺ <c>isArray</c> is truthy AND
+    ///     <c>arrayLength</c> is a number <c>&gt;= 1</c>. Any other combination (isArray absent/false,
+    ///     or isArray:true with arrayLength missing/invalid/&lt;1) is a SCALAR and returns
+    ///     <c>(IsArray: false, ElementCount: 1)</c>. This matches every other driver (Modbus, S7,
+    ///     TwinCAT, AbLegacy) which also return scalar for that degenerate input.
     /// </summary>
     private static (bool IsArray, int ElementCount) ReadArrayShape(JsonElement o)
     {
@@ -62,8 +64,8 @@ public static class AbCipEquipmentTagParser
             && len.TryGetInt32(out var n)
             && n >= 1)
             return (true, n);
-        // isArray:true but arrayLength missing/invalid — treat as a 1-element array (count 1).
-        return (true, 1);
+        // isArray:true but arrayLength missing/invalid/<1 — canonical rule: scalar (matches all other drivers).
+        return (false, 1);
     }
 
     private static TEnum ReadEnum<TEnum>(JsonElement o, string name, TEnum fallback) where TEnum : struct, Enum
diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipArrayTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipArrayTests.cs
index 3ea4fb0e..f8047cd9 100644
--- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipArrayTests.cs
+++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipArrayTests.cs
@@ -201,6 +201,34 @@ public sealed class AbCipArrayTests
         def.IsArray.ShouldBeFalse();
     }
 
+    /// <summary>
+    ///     Review finding I-2 — <c>isArray:true</c> with NO <c>arrayLength</c> is a DEGENERATE input
+    ///     that must parse as SCALAR (IsArray false, ElementCount 1), matching every other driver
+    ///     (Modbus, S7, TwinCAT, AbLegacy). The AdminUI validator blocks authoring this combination,
+    ///     but the parser contract must still be consistent cross-driver.
+    /// </summary>
+    [Fact]
+    public void Parser_isArray_true_without_arrayLength_parses_as_scalar()
+    {
+        var json = """{"tagPath":"Recipe","dataType":"DInt","isArray":true}""";
+        AbCipEquipmentTagParser.TryParse(json, out var def).ShouldBeTrue();
+        def!.IsArray.ShouldBeFalse();
+        def.ElementCount.ShouldBe(1);
+    }
+
+    /// <summary>
+    ///     Review finding I-2 — <c>isArray:true</c> with an invalid (zero) <c>arrayLength</c> also
+    ///     parses as SCALAR, consistent with the cross-driver rule.
+    /// </summary>
+    [Fact]
+    public void Parser_isArray_true_with_zero_arrayLength_parses_as_scalar()
+    {
+        var json = """{"tagPath":"Recipe","dataType":"DInt","isArray":true,"arrayLength":0}""";
+        AbCipEquipmentTagParser.TryParse(json, out var def).ShouldBeTrue();
+        def!.IsArray.ShouldBeFalse();
+        def.ElementCount.ShouldBe(1);
+    }
+
     /// <summary>
     ///     Review finding I-1 — a 1-element array (<c>isArray:true, arrayLength:1</c>) is a valid
     ///     1-element array, NOT a scalar: the parser sets <see cref="AbCipTagDefinition.IsArray"/>