merge: #549 — authoring never accepts a TagConfig enum the runtime rejects (6 models, one shared authority)

src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/TagConfigJson.cs

@@ -48,7 +48,39 @@ public static class TagConfigJson
             value = default;
             return JsonEnumRead.Absent;
         }
-        if (Enum.TryParse(e.GetString(), ignoreCase: true, out value))
+        return ClassifyEnumText(e.GetString(), out value);
+    }
+
+    /// <summary>
+    ///     The text-level heart of <see cref="TryReadEnum{TEnum}"/>, and with it the single authority on
+    ///     which enum spellings a TagConfig may carry: classifies an already-extracted enum STRING.
+    ///     <para>
+    ///         It is public because the authoring side reads the same blob through a different door — the
+    ///         AdminUI's typed tag-config editors hold a mutable <c>JsonObject</c> key bag, not a
+    ///         <see cref="JsonElement"/> — and an authoring gate that RESTATED the accept/reject boundary
+    ///         would drift from the runtime the moment either side moved, in the one direction that hurts:
+    ///         a value the modal accepts and the driver then rejects (Gitea #549). Both doors reach this
+    ///         method.
+    ///     </para>
+    ///     <para>
+    ///         <see langword="null"/> is <see cref="JsonEnumRead.Absent"/>, matching the JSON reader's
+    ///         treatment of an absent or present-but-non-string property — so a caller that extracts "the
+    ///         string value, or null" gets the runtime's leniency for free.
+    ///     </para>
+    /// </summary>
+    /// <typeparam name="TEnum">The enum type to parse.</typeparam>
+    /// <param name="text">The authored value's text, or <see langword="null"/> when there is none.</param>
+    /// <param name="value">The parsed enum on <see cref="JsonEnumRead.Valid"/>; otherwise <c>default</c>.</param>
+    /// <returns>The read outcome.</returns>
+    public static JsonEnumRead ClassifyEnumText<TEnum>(string? text, out TEnum value)
+        where TEnum : struct, Enum
+    {
+        if (text is null)
+        {
+            value = default;
+            return JsonEnumRead.Absent;
+        }
+        if (Enum.TryParse(text, ignoreCase: true, out value))
             return JsonEnumRead.Valid;
         value = default;
         return JsonEnumRead.Invalid;
@@ -120,10 +152,26 @@ public static class TagConfigJson
     /// <returns>The warning text, or <see langword="null"/>.</returns>
     public static string? DescribeInvalidEnum<TEnum>(JsonElement o, string name)
         where TEnum : struct, Enum
+        => TryReadEnum<TEnum>(o, name, out _) != JsonEnumRead.Invalid
+            ? null
+            : DescribeInvalidEnumText<TEnum>(name, o.TryGetProperty(name, out var e) ? e.GetString() : null);
+
+    /// <summary>
+    ///     The text-level counterpart of <see cref="DescribeInvalidEnum{TEnum}(JsonElement, string)"/>,
+    ///     over the same <see cref="ClassifyEnumText{TEnum}"/> verdict: a human-readable warning for a
+    ///     present-but-invalid enum value, or <see langword="null"/> when it is absent
+    ///     (<paramref name="text"/> <see langword="null"/>) or valid. The authoring gate uses this so its
+    ///     wording, and not only its verdict, comes from the same place as the deploy gate's.
+    /// </summary>
+    /// <typeparam name="TEnum">The enum type expected.</typeparam>
+    /// <param name="name">The property name to describe.</param>
+    /// <param name="text">The authored value's text, or <see langword="null"/> when there is none.</param>
+    /// <returns>The warning text, or <see langword="null"/>.</returns>
+    public static string? DescribeInvalidEnumText<TEnum>(string name, string? text)
+        where TEnum : struct, Enum
     {
-        if (TryReadEnum<TEnum>(o, name, out _) != JsonEnumRead.Invalid) return null;
-        var raw = o.TryGetProperty(name, out var e) ? e.GetString() : null;
+        if (ClassifyEnumText<TEnum>(text, out _) != JsonEnumRead.Invalid) return null;
         var valid = string.Join(", ", Enum.GetNames<TEnum>());
-        return $"value '{raw}' for '{name}' is not a valid {typeof(TEnum).Name}; valid: {valid}";
+        return $"value '{text}' for '{name}' is not a valid {typeof(TEnum).Name}; valid: {valid}";
     }
 }

src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Contracts/MTConnectTagDefinitionFactory.cs

@@ -154,6 +154,28 @@ public static class MTConnectTagDefinitionFactory
         return warnings;
     }
 
+    /// <summary>
+    ///     <b>The single authority on which coercion-type spellings an MTConnect tag may carry</b>, consulted
+    ///     by the runtime + deploy gate (through <see cref="TryFromTagConfig"/> / <see cref="Inspect"/>) and
+    ///     by the AdminUI's authoring model (<c>MTConnectTagConfigModel.Validate</c>) alike.
+    ///     <para>
+    ///         It is public for exactly that second caller. The authoring model used to read the type with a
+    ///         lenient enum parse that fell back to <c>String</c> on ANY unrecognised value, so a misspelled
+    ///         name ("Strng") from a hand-authored blob, a CSV import or the API was accepted at authoring —
+    ///         and silently rewritten — while this parse rejected it and the tag died at the driver
+    ///         (Gitea #549). A second copy of the rule in the AdminUI would have reopened that gap the
+    ///         moment either side moved, so there is one rule and both sides ask it.
+    ///     </para>
+    ///     <para>
+    ///         A blank/absent value is ACCEPTED: it declares no override, and <see cref="TryFromTagConfig"/>
+    ///         then falls back to the Agent's <c>/probe</c> declaration rather than rejecting the tag.
+    ///     </para>
+    /// </summary>
+    /// <param name="raw">The authored type text, or <see langword="null"/>/blank when none was authored.</param>
+    /// <returns><see langword="true"/> when the driver would accept this authored type.</returns>
+    public static bool IsAcceptedDataTypeName(string? raw)
+        => string.IsNullOrWhiteSpace(raw) || TryParseDataType(raw.Trim(), out _);
+
     /// <summary>
     ///     Parses an authored coercion type by NAME. A digits-only value is rejected outright, BEFORE the
     ///     parse: <see cref="Enum.TryParse{TEnum}(string, bool, out TEnum)"/> accepts <c>"8"</c> as readily

src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/AbCipTagConfigModel.cs

@@ -52,7 +52,18 @@ public sealed class AbCipTagConfigModel
     }
 
     /// <summary>Validation hook; returns an error message or null when the model is valid.</summary>
+    /// <remarks>
+    ///     The <c>dataType</c> check reads the ORIGINAL key bag rather than the leniently-defaulted
+    ///     <see cref="DataType"/>, because <c>AbCipTagDefinitionFactory</c> reads it with
+    ///     <c>TryReadEnumStrict</c> — a present-but-invalid string rejects the whole tag at the driver
+    ///     (<c>BadNodeIdUnknown</c>). Without this the blob would be accepted here, silently rewritten to
+    ///     <c>DInt</c> on save, and — on the CSV-import and API paths, which save it verbatim — deployed
+    ///     into a tag the driver skips (Gitea #549).
+    /// </remarks>
     /// <returns>An error message describing the validation failure, or <c>null</c> when valid.</returns>
     public string? Validate()
-        => string.IsNullOrWhiteSpace(TagPath) ? "Tag path is required." : null;
+    {
+        if (TagConfigJson.DescribeInvalidEnum<AbCipDataType>(_bag, "dataType") is { } dtError) { return dtError; }
+        return string.IsNullOrWhiteSpace(TagPath) ? "Tag path is required." : null;
+    }
 }

src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/AbLegacyTagConfigModel.cs

@@ -55,6 +55,14 @@ public sealed class AbLegacyTagConfigModel
     /// <returns>An error message describing the validation failure, or <c>null</c> when valid.</returns>
     public string? Validate()
     {
+        // Strict enum, read from the ORIGINAL key bag: AbLegacyTagDefinitionFactory reads "dataType" with
+        // TryReadEnumStrict, so a present-but-invalid string rejects the whole tag at the driver
+        // (BadNodeIdUnknown) while the lenient load above would default it to Int. Accepting it here would
+        // let the CSV-import and API paths — which save the blob verbatim — deploy an unreadable tag
+        // (Gitea #549).
+        if (TagConfigJson.DescribeInvalidEnum<AbLegacyDataType>(_bag, "dataType") is { } dtError)
+            return dtError;
+
         if (string.IsNullOrWhiteSpace(Address))
             return "Address is required.";
 

src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/FocasTagConfigModel.cs

@@ -59,6 +59,13 @@ public sealed class FocasTagConfigModel
     /// </remarks>
     public string? Validate()
     {
+        // Strict enum, read from the ORIGINAL key bag: FocasTagDefinitionFactory reads "dataType" with
+        // TryReadEnumStrict, so a present-but-invalid string rejects the whole tag at the driver
+        // (BadNodeIdUnknown) while the lenient load above defaults it to Int32. Accepting it here would
+        // let the CSV-import and API paths — which save the blob verbatim — deploy an unreadable tag,
+        // the same shape as the #540 address gap this method already closes (Gitea #549).
+        if (TagConfigJson.DescribeInvalidEnum<FocasDataType>(_bag, "dataType") is { } dtError) return dtError;
+
         if (string.IsNullOrWhiteSpace(Address)) return "Address is required.";
         return FocasAddress.TryParse(Address) is null ? DescribeUnparseableAddress(Address) : null;
     }

src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/MTConnectTagConfigModel.cs

@@ -1,5 +1,6 @@
 using System.Text.Json.Nodes;
 using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
+using ZB.MOM.WW.OtOpcUa.Driver.MTConnect;
 
 namespace ZB.MOM.WW.OtOpcUa.AdminUI.Uns.TagEditors;
 
@@ -57,12 +58,17 @@ public sealed class MTConnectTagConfigModel
     /// consumed by the driver or the runtime.</summary>
     public string? MtComponent { get; set; }
 
-    // The as-loaded raw "dataType" JSON value, kept only to detect the ParseEnum numeric-text trap in
-    // Validate() below — Enum.TryParse (which TagConfigJson.GetEnum uses) silently accepts a quoted
-    // number ("8" -> Float64) exactly as readily as a name, so a config that was ever hand-authored or
-    // migrated with a numeric dataType would otherwise load into a *valid-looking* enum value with no
-    // signal that it isn't a name. ToJson() always re-writes DataType as a name, so this can only ever
-    // be non-null on the FIRST load of an already-poisoned config, never after a save through this model.
+    // The as-loaded raw "dataType" JSON token, kept so Validate() below can put it to the DRIVER's own
+    // accept/reject rule (MTConnectTagDefinitionFactory.IsAcceptedDataTypeName) rather than to the
+    // lenient load. TagConfigJson.GetEnum falls back to String on ANY unrecognised value, so without this
+    // a misspelled name ("Strng") or a quoted number ("8" -> Float64, the ParseEnum numeric-text trap)
+    // loads as a *valid-looking* enum value with no signal that it is not what the operator wrote — while
+    // the strict runtime rejects the tag outright (Gitea #549). Captured as the raw TOKEN, not via
+    // GetString: the runtime deserialises this key into a `string?` DTO field, so a NON-string token
+    // (`"dataType": 8`) throws JsonException there and rejects the tag too, where GetString would report
+    // it as simply absent. ToJson() always re-writes dataType as a name, so this can only ever be
+    // poisoned on the FIRST load of a blob authored elsewhere (hand-edited, CSV import, API), never after
+    // a save through this model.
     private string? _rawDataType;
 
     private JsonObject _bag = new();
@@ -96,7 +102,7 @@ public sealed class MTConnectTagConfigModel
             Units = TagConfigJson.GetString(o, "units"),
             MtDevice = TagConfigJson.GetString(o, "mtDevice"),
             MtComponent = TagConfigJson.GetString(o, "mtComponent"),
-            _rawDataType = TagConfigJson.GetString(o, "dataType"),
+            _rawDataType = ReadRawDataType(o),
             _bag = o,
         };
     }
@@ -124,6 +130,15 @@ public sealed class MTConnectTagConfigModel
     }
 
     /// <summary>Validation hook; returns an error message or null when the model is valid.</summary>
+    /// <remarks>
+    ///     The <c>dataType</c> rule is not stated here — it is asked of
+    ///     <see cref="MTConnectTagDefinitionFactory.IsAcceptedDataTypeName"/>, the same predicate the driver
+    ///     and the deploy gate use. One authority, so this gate can never end up weaker than the runtime
+    ///     again (Gitea #549). Note that the previous, digits-only check is NOT retained as a tailored
+    ///     special case: the runtime draws no distinction between a bare number and a misspelled name —
+    ///     both fail that one predicate and both draw the same <c>Inspect</c> warning — so a second message
+    ///     here would be a second statement of a rule this model does not own.
+    /// </remarks>
     /// <returns>An error message describing the validation failure, or <c>null</c> when the model is valid.</returns>
     public string? Validate()
     {
@@ -132,16 +147,26 @@ public sealed class MTConnectTagConfigModel
             return "A DataItem id (fullName) is required.";
         }
 
-        // See the _rawDataType remarks: a quoted number parses as a "valid" enum today but is never
-        // something this editor itself would have written, so surface it instead of silently accepting it.
-        if (_rawDataType is { Length: > 0 } raw && raw.All(char.IsAsciiDigit))
+        // See the _rawDataType remarks: the value the blob CARRIES is judged, not the leniently-defaulted
+        // DataType above, because the CSV-import and API paths save the blob verbatim.
+        if (!MTConnectTagDefinitionFactory.IsAcceptedDataTypeName(_rawDataType))
         {
-            return $"dataType \"{raw}\" is a numeric value, not a named type — re-select a data type.";
+            return $"dataType \"{_rawDataType}\" is not a valid DriverDataType name — re-select a data type. "
+                + $"Valid: {string.Join(", ", Enum.GetNames<DriverDataType>())}.";
         }
 
         return null;
     }
 
+    // The as-loaded "dataType" TOKEN as text (see the _rawDataType remarks): a JSON string yields its
+    // value, any other present token yields its JSON text so Validate() judges what was actually written,
+    // and absent / JSON null yields null — the one shape the runtime treats as "no type declared".
+    private static string? ReadRawDataType(JsonObject o)
+    {
+        if (!o.TryGetPropertyValue("dataType", out var node) || node is null) { return null; }
+        return node is JsonValue v && v.TryGetValue<string>(out var s) ? s : node.ToJsonString();
+    }
+
     // Normalises a blank/whitespace-only string to null so TagConfigJson.Set omits the key rather than
     // persisting an empty string.
     private static string? Blank(string? value) => string.IsNullOrWhiteSpace(value) ? null : value;

src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/ModbusTagConfigModel.cs

@@ -68,6 +68,18 @@ public sealed class ModbusTagConfigModel
     }
 
     /// <summary>Validation hook; returns an error message or null when the model is valid.</summary>
+    /// <remarks>
+    ///     All three enums are read from the ORIGINAL key bag, not from the leniently-defaulted typed
+    ///     fields above: <c>ModbusTagDefinitionFactory</c> reads each with <c>TryReadEnumStrict</c>, so a
+    ///     present-but-invalid string rejects the whole tag at the driver (<c>BadNodeIdUnknown</c>) where
+    ///     the load silently substitutes <c>HoldingRegisters</c>/<c>Int16</c>/<c>BigEndian</c>. Accepting
+    ///     one here would let the CSV-import and API paths — which save the blob verbatim — deploy a tag
+    ///     the driver skips (Gitea #549). Also note a <b>wrong-but-valid</b> region is not this gate's
+    ///     business: only a spelling the runtime cannot read is rejected.
+    /// </remarks>
     /// <returns>An error message describing the validation failure, or <c>null</c> when the model is valid.</returns>
-    public string? Validate() => null;
+    public string? Validate()
+        => TagConfigJson.DescribeInvalidEnum<ModbusRegion>(_bag, "region")
+            ?? TagConfigJson.DescribeInvalidEnum<ModbusDataType>(_bag, "dataType")
+            ?? TagConfigJson.DescribeInvalidEnum<ModbusByteOrder>(_bag, "byteOrder");
 }

src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/S7TagConfigModel.cs

@@ -53,6 +53,16 @@ public sealed class S7TagConfigModel
 
     /// <summary>Validation hook; returns an error message or null when the model is valid.</summary>
     /// <returns>An error message describing the validation failure, or null when the model is valid.</returns>
+    /// <remarks>
+    ///     The <c>dataType</c> check reads the ORIGINAL key bag rather than the leniently-defaulted
+    ///     <see cref="DataType"/>: <c>S7TagDefinitionFactory</c> reads it with <c>TryReadEnumStrict</c>,
+    ///     so a present-but-invalid string rejects the whole tag at the driver (<c>BadNodeIdUnknown</c>)
+    ///     where the load silently substitutes <c>Int16</c>. Accepting it here would let the CSV-import
+    ///     and API paths — which save the blob verbatim — deploy an unreadable tag (Gitea #549).
+    /// </remarks>
     public string? Validate()
-        => string.IsNullOrWhiteSpace(Address) ? "Address is required." : null;
+    {
+        if (TagConfigJson.DescribeInvalidEnum<S7DataType>(_bag, "dataType") is { } dtError) { return dtError; }
+        return string.IsNullOrWhiteSpace(Address) ? "Address is required." : null;
+    }
 }

src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/TagConfigJson.cs

@@ -1,5 +1,6 @@
 using System.Text.Json;
 using System.Text.Json.Nodes;
+using RuntimeTagConfigJson = ZB.MOM.WW.OtOpcUa.Core.Abstractions.TagConfigJson;
 
 namespace ZB.MOM.WW.OtOpcUa.AdminUI.Uns.TagEditors;
 
@@ -65,6 +66,33 @@ public static class TagConfigJson
     public static TEnum GetEnum<TEnum>(JsonObject o, string name, TEnum fallback) where TEnum : struct, Enum
         => GetString(o, name) is { } s && Enum.TryParse<TEnum>(s, ignoreCase: true, out var v) ? v : fallback;
 
+    /// <summary>
+    ///     Describes a present-but-invalid enum field the way the RUNTIME parser judges it, or
+    ///     <c>null</c> when the field is absent, present-but-non-string, or valid.
+    ///     <para>
+    ///         <see cref="GetEnum{TEnum}"/> is deliberately lenient — it must be, because a model has to
+    ///         LOAD a poisoned blob to let an operator fix it — but leniency at the save gate is the
+    ///         Gitea #549 defect: every driver's runtime read is
+    ///         <c>TagConfigJson.TryReadEnumStrict</c>, which REJECTS the whole tag on a present-but-invalid
+    ///         string (R2-11 Phase C), so a typo the modal accepts and rewrites is a tag the driver would
+    ///         have skipped with <c>BadNodeIdUnknown</c> on the CSV-import and API paths, which save the
+    ///         blob verbatim. A model's <c>Validate()</c> therefore calls this on its ORIGINAL key bag.
+    ///     </para>
+    ///     <para>
+    ///         The verdict and the wording both come from
+    ///         <see cref="RuntimeTagConfigJson.DescribeInvalidEnumText{TEnum}"/> — the same authority the
+    ///         runtime parsers and the deploy gate consult — rather than being restated here. Passing
+    ///         <see cref="GetString"/>'s null-for-non-string result reproduces the runtime's
+    ///         <c>Absent</c> treatment of a non-string token exactly.
+    ///     </para>
+    /// </summary>
+    /// <typeparam name="TEnum">The enum type expected.</typeparam>
+    /// <param name="o">The model's preserved TagConfig key bag, as loaded.</param>
+    /// <param name="name">The property name to check.</param>
+    /// <returns>The error text, or <c>null</c> when the runtime would accept the field.</returns>
+    public static string? DescribeInvalidEnum<TEnum>(JsonObject o, string name) where TEnum : struct, Enum
+        => RuntimeTagConfigJson.DescribeInvalidEnumText<TEnum>(name, GetString(o, name));
+
     /// <summary>Sets a string/number/enum-name value (enums via ToString()). A null value REMOVES the key, so it is omitted from the serialised JSON.</summary>
     /// <param name="o">The JSON object to mutate.</param>
     /// <param name="name">The property name to set.</param>

src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/TwinCATTagConfigModel.cs

@@ -53,6 +53,17 @@ public sealed class TwinCATTagConfigModel
 
     /// <summary>Validation hook; returns an error message or null when the model is valid.</summary>
     /// <returns>An error message describing the validation failure, or null when the model is valid.</returns>
+    /// <remarks>
+    ///     The <c>dataType</c> check reads the ORIGINAL key bag rather than the leniently-defaulted
+    ///     <see cref="DataType"/>: <c>TwinCATTagDefinitionFactory</c> reads it with
+    ///     <c>TryReadEnumStrict</c>, so a present-but-invalid string rejects the whole tag at the driver
+    ///     (<c>BadNodeIdUnknown</c>) where the load silently substitutes <c>DInt</c>. Accepting it here
+    ///     would let the CSV-import and API paths — which save the blob verbatim — deploy an unreadable
+    ///     tag (Gitea #549).
+    /// </remarks>
     public string? Validate()
-        => string.IsNullOrWhiteSpace(SymbolPath) ? "Symbol path is required." : null;
+    {
+        if (TagConfigJson.DescribeInvalidEnum<TwinCATDataType>(_bag, "dataType") is { } dtError) { return dtError; }
+        return string.IsNullOrWhiteSpace(SymbolPath) ? "Symbol path is required." : null;
+    }
 }

src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ZB.MOM.WW.OtOpcUa.AdminUI.csproj

@@ -26,6 +26,12 @@
         <!-- #520: API-key minting + the reserved-principal rule live in Core.Authorization, because
              both ends need them — the OPC UA server resolves principals, the AdminUI authors keys. -->
         <ProjectReference Include="..\..\Core\ZB.MOM.WW.OtOpcUa.Core\ZB.MOM.WW.OtOpcUa.Core.csproj"/>
+        <!-- #549: the typed tag-config editors ask Core.Abstractions' TagConfigJson whether an authored
+             enum spelling is one the RUNTIME parsers accept, so authoring can never be weaker than the
+             driver. That type is load-bearing for the save gate — referenced directly rather than
+             consumed transitively through Core, so a future re-plumbing of Core cannot silently take it
+             away. -->
+        <ProjectReference Include="..\..\Core\ZB.MOM.WW.OtOpcUa.Core.Abstractions\ZB.MOM.WW.OtOpcUa.Core.Abstractions.csproj"/>
         <ProjectReference Include="..\..\Core\ZB.MOM.WW.OtOpcUa.Core.Scripting\ZB.MOM.WW.OtOpcUa.Core.Scripting.csproj"/>
         <ProjectReference Include="..\..\Core\ZB.MOM.WW.OtOpcUa.Core.Scripting.Abstractions\ZB.MOM.WW.OtOpcUa.Core.Scripting.Abstractions.csproj"/>
     </ItemGroup>