refactor(mqtt): accept a blank jsonPath; guide with a seeded "$" instead

Review follow-up. Dropped the hard Validate rejection of a blank jsonPath under
a Json payload: the runtime defaults it to the document root, which is the real
"publisher puts a bare JSON scalar on the topic" case, so rejecting it made the
editor refuse what the driver accepts — inverting "authoring surface accepts <=>
publish accepts" — and blocked whole CSV-import batches, since this validator
also gates RawManualTagEntryModal's review grid.

Replaced with guidance: an UNAUTHORED tag (no topic AND no jsonPath) seeds the
field with "$", so a fresh tag emits an explicit "jsonPath":"$" while an
existing path-less tag keeps the key ABSENT and the driver defaults it. The
wildcard-topic rule stays strict — a wildcard has no legitimate single-Tag
interpretation.

Also: omit a blank "topic" rather than writing "topic":"", and record why the
_lastConfigJson guard diverges from the Modbus template — the landmine is a
DERIVED, NON-PERSISTED UI FIELD INFERRED FROM BLOB CONTENT (Mode), which Task
24's Sparkplug field group will be tempted to add more of. Named the host-side
dependency (RawTagModal only mutates through this callback) that keeps the
staleness trade benign today.

Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW

src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/TagEditors/MqttTagConfigEditor.razor

@@ -133,8 +133,22 @@
         apply();
         _validationError = _m.Validate();
         var json = _m.ToJson();
-        // Keep the guard in OnParametersSet in step with what we just emitted, so the host echoing the
-        // new JSON back as a parameter cannot re-parse and clobber an in-progress edit.
+
+        // Keep the OnParametersSet guard in step with what we just emitted, so the host echoing the new
+        // JSON straight back as a parameter cannot re-parse and clobber an in-progress edit.
+        //
+        // WHY THIS DIVERGES FROM THE MODBUS TEMPLATE (which does not do this): the general landmine is a
+        // DERIVED, NON-PERSISTED UI FIELD INFERRED FROM BLOB CONTENT. Mode is exactly that — it is
+        // re-inferred by FromJson from the Sparkplug keys and never serialised, so a plain re-parse of
+        // our own output would silently reset the operator's shape selection to Plain on the very next
+        // render. Modbus has no such field, which is why it needs no guard. Task 24's Sparkplug field
+        // group will be tempted to add more of them; each one needs this guard to hold.
+        //
+        // TRADE (does not manifest today): the editor will not pick up a change the HOST makes to the
+        // JSON while it is open. That is safe only because RawTagModal/RawManualTagEntryModal mutate
+        // _form.TagConfig exclusively through this callback. A host-side feature that rewrites the blob
+        // out-of-band — a "reformat JSON" action, a bulk address re-pick — would reintroduce staleness
+        // here and must re-key the guard (e.g. compare against a host-supplied revision, not the text).
         _lastConfigJson = json;
         await ConfigJsonChanged.InvokeAsync(json);
     }

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

@@ -38,6 +38,13 @@ public sealed class MqttTagConfigModel
     private const string QosKey = "qos";
     private const string RetainSeedKey = "retainSeed";
 
+    /// <summary>
+    ///     The JSONPath the driver applies when the blob omits <c>jsonPath</c> — the document root.
+    ///     Seeded into an unauthored tag's field so the "no extraction needed" case is one click away
+    ///     rather than something the operator has to know.
+    /// </summary>
+    private const string RootJsonPath = "$";
+
     /// <summary>The Sparkplug B descriptor keys — authored by Task 24, preserved (never dropped) in P1.</summary>
     private static readonly string[] SparkplugKeys = ["groupId", "edgeNodeId", "deviceId", "metricName"];
 
@@ -57,8 +64,11 @@ public sealed class MqttTagConfigModel
     public MqttPayloadFormat PayloadFormat { get; set; } = MqttPayloadFormat.Json;
 
     /// <summary>
-    ///     JSONPath selecting the value inside a <see cref="MqttPayloadFormat.Json"/> payload. Blank ⇒
-    ///     the key is omitted and the driver applies the document root (<c>$</c>).
+    ///     JSONPath selecting the value inside a <see cref="MqttPayloadFormat.Json"/> payload. Blank is
+    ///     legal and NOT a validation failure — the key is omitted and the driver applies the document
+    ///     root (<c>$</c>), which is the real "publisher puts a bare JSON scalar on the topic" case.
+    ///     An <b>unauthored</b> tag is seeded with <c>$</c> by <see cref="FromJson"/> so that common case
+    ///     is visible and one click away; see <see cref="RootJsonPath"/>.
     /// </summary>
     public string JsonPath { get; set; } = "";
 
@@ -84,17 +94,29 @@ public sealed class MqttTagConfigModel
     ///     original key (so fields this editor doesn't expose — history intent, array/alarm objects, and
     ///     the Sparkplug descriptor keys — survive a load→save).
     /// </summary>
+    /// <remarks>
+    ///     An <b>unauthored</b> Plain tag — no <c>topic</c> AND no <c>jsonPath</c>, i.e. a brand-new tag
+    ///     rather than an existing one the operator deliberately left path-less — has its
+    ///     <see cref="JsonPath"/> seeded to <see cref="RootJsonPath"/>. The condition is deliberately
+    ///     narrow: an existing tag that carries a topic and no <c>jsonPath</c> keeps the key ABSENT
+    ///     through a load→save, so this can never rewrite already-deployed blobs.
+    /// </remarks>
     /// <param name="json">The raw TagConfig JSON string, or <c>null</c> for a new/empty config.</param>
     /// <returns>The populated <see cref="MqttTagConfigModel"/>.</returns>
     public static MqttTagConfigModel FromJson(string? json)
     {
         var o = TagConfigJson.ParseOrNew(json);
+        var topic = TagConfigJson.GetString(o, TopicKey) ?? "";
+        var jsonPath = TagConfigJson.GetString(o, JsonPathKey) ?? "";
+        var mode = InferMode(o);
+        if (mode == MqttMode.Plain && topic.Length == 0 && jsonPath.Length == 0) { jsonPath = RootJsonPath; }
+
         return new MqttTagConfigModel
         {
-            Mode = InferMode(o),
-            Topic = TagConfigJson.GetString(o, TopicKey) ?? "",
+            Mode = mode,
+            Topic = topic,
             PayloadFormat = TagConfigJson.GetEnum(o, PayloadFormatKey, MqttPayloadFormat.Json),
-            JsonPath = TagConfigJson.GetString(o, JsonPathKey) ?? "",
+            JsonPath = jsonPath,
             DataType = TagConfigJson.GetEnum(o, DataTypeKey, DriverDataType.String),
             Qos = GetIntNullable(o, QosKey),
             RetainSeed = TagConfigJson.GetBoolNullable(o, RetainSeedKey),
@@ -111,7 +133,9 @@ public sealed class MqttTagConfigModel
     /// <returns>The serialised TagConfig JSON string.</returns>
     public string ToJson()
     {
-        TagConfigJson.Set(_bag, TopicKey, Topic.Trim());
+        // Blank ⇒ omit, like every other optional here: toggling the shape dropdown on an untouched tag
+        // must not leave a stray "topic":"" behind.
+        TagConfigJson.Set(_bag, TopicKey, string.IsNullOrWhiteSpace(Topic) ? null : Topic.Trim());
         TagConfigJson.Set(_bag, PayloadFormatKey, PayloadFormat);
         TagConfigJson.Set(_bag, DataTypeKey, DataType);
         TagConfigJson.Set(_bag, JsonPathKey, string.IsNullOrWhiteSpace(JsonPath) ? null : JsonPath.Trim());
@@ -125,14 +149,26 @@ public sealed class MqttTagConfigModel
     ///     Returns the first error, or <c>null</c> when the config is valid.
     /// </summary>
     /// <remarks>
-    ///     Two rules are deliberately <b>stricter</b> than the runtime parser, because this is an
-    ///     authoring surface and the cheapest place to stop the mistake: a wildcard topic (the runtime
-    ///     accepts it and <c>Inspect</c> only warns at deploy) and a <c>Json</c> payload with no
-    ///     <c>jsonPath</c> (the runtime defaults to the document root). Everything else — required
-    ///     topic, strict enums, QoS 0–2 — matches <c>MqttTagDefinitionFactory</c> exactly. The strict
-    ///     enum/QoS checks read the ORIGINAL key bag, not the defaulted typed fields, so a blob that
-    ///     never went through this editor (e.g. a CSV import) cannot pass validation while the driver
-    ///     would reject it.
+    ///     <para>
+    ///     Exactly ONE rule is deliberately <b>stricter</b> than the runtime parser: a wildcard topic,
+    ///     which the runtime accepts and <c>Inspect</c> only warns about at deploy. It earns the
+    ///     strictness because a wildcard has no legitimate single-Tag interpretation — one Tag holds one
+    ///     value, and <c>+</c>/<c>#</c> would feed it from many source topics. Everything else —
+    ///     required topic, strict enums, QoS 0–2 — matches <c>MqttTagDefinitionFactory</c> exactly.
+    ///     </para>
+    ///     <para>
+    ///     A blank <c>jsonPath</c> under a <c>Json</c> payload is explicitly <b>accepted</b>, matching
+    ///     the runtime's document-root default. Rejecting it would make this editor refuse a config the
+    ///     driver handles happily — inverting the "authoring surface accepts ⇔ publish accepts"
+    ///     principle — and, because this validator also gates the CSV-import review grid
+    ///     (<c>RawManualTagEntryModal</c>), would block whole import batches over a sane default. The
+    ///     operator is guided by the seeded <c>$</c> from <see cref="FromJson"/> instead of a blocker.
+    ///     </para>
+    ///     <para>
+    ///     The strict enum/QoS checks read the ORIGINAL key bag, not the defaulted typed fields, so a
+    ///     blob that never went through this editor (e.g. a CSV import) cannot pass validation while the
+    ///     driver would reject it.
+    ///     </para>
     /// </remarks>
     /// <returns>An error message describing the validation failure, or <c>null</c> when valid.</returns>
     public string? Validate()
@@ -153,11 +189,8 @@ public sealed class MqttTagConfigModel
                  + "or the tag would be fed by every matching topic.";
         }
 
-        if (PayloadFormat == MqttPayloadFormat.Json && string.IsNullOrWhiteSpace(JsonPath))
-        {
-            return "A jsonPath is required when the payload format is Json (use \"$\" for the whole document).";
-        }
-
+        // NB no jsonPath rule — see the remarks above. A blank path is the driver's document-root
+        // default, not an authoring error.
         return null;
     }
 

tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/MqttTagConfigModelTests.cs

@@ -30,7 +30,9 @@ public sealed class MqttTagConfigModelTests
         m.Mode.ShouldBe(MqttMode.Plain);
         m.Topic.ShouldBe("");
         m.PayloadFormat.ShouldBe(MqttPayloadFormat.Json);
-        m.JsonPath.ShouldBe("");
+        // An UNAUTHORED tag seeds the document-root path so the common "no extraction needed" case is
+        // visible and one click away — it is a guide, not a requirement (blank is accepted, see below).
+        m.JsonPath.ShouldBe("$");
         m.DataType.ShouldBe(DriverDataType.String);
         m.Qos.ShouldBeNull();
         m.RetainSeed.ShouldBeNull();
@@ -107,6 +109,15 @@ public sealed class MqttTagConfigModelTests
         json.ShouldNotContain("jsonPath");
     }
 
+    [Fact]
+    public void ToJson_omits_a_blank_topic()
+    {
+        // Toggling the shape dropdown on an untouched tag must not leave a stray "topic":"" behind.
+        var json = new MqttTagConfigModel { PayloadFormat = MqttPayloadFormat.Raw }.ToJson();
+
+        json.ShouldNotContain("topic");
+    }
+
     [Fact]
     public void FromJson_then_ToJson_preserves_unknown_keys()
     {
@@ -171,13 +182,38 @@ public sealed class MqttTagConfigModelTests
         => MqttTagConfigModel.FromJson($$"""{"topic":"{{topic}}","payloadFormat":"Raw","dataType":"String"}""")
             .Validate().ShouldNotBeNullOrWhiteSpace();
 
+    // A blank jsonPath under a Json payload is ACCEPTED — the runtime defaults it to the document root
+    // ("$"), which is the real "publisher puts a bare JSON scalar on the topic" case. Rejecting it would
+    // make the editor refuse what the driver accepts, and would block CSV-import batches (this validator
+    // also gates RawManualTagEntryModal's review grid) over a sane default.
     [Fact]
-    public void Validate_JsonWithoutPath_Fails()
+    public void Validate_JsonWithoutPath_IsAccepted()
         => MqttTagConfigModel.FromJson("""{"topic":"a/b","payloadFormat":"Json","dataType":"Float64"}""")
-            .Validate().ShouldNotBeNullOrWhiteSpace();
+            .Validate().ShouldBeNull();
+
+    // …and the key stays ABSENT through a load→save, so the driver applies its own default rather than
+    // this editor freezing "$" into an already-deployed blob.
+    [Fact]
+    public void ToJson_leaves_an_existing_blank_jsonPath_absent()
+    {
+        var json = MqttTagConfigModel
+            .FromJson("""{"topic":"a/b","payloadFormat":"Json","dataType":"Float64"}""")
+            .ToJson();
+
+        json.ShouldNotContain("jsonPath");
+    }
+
+    // The seed applies ONLY to an unauthored tag (no topic AND no jsonPath) — a brand-new tag emits an
+    // explicit "$" once the operator touches anything, which is the guided half of the deal.
+    [Fact]
+    public void ToJson_seeds_the_root_jsonPath_on_a_fresh_tag()
+    {
+        var m = MqttTagConfigModel.FromJson(null);
+        m.Topic = "a/b";
+
+        m.ToJson().ShouldContain("\"jsonPath\":\"$\"");
+    }
 
-    // Falsifiability control for the test above: the SAME blob with a jsonPath must validate clean, so
-    // the failure above is provably the missing jsonPath and not the topic or the dataType.
     [Fact]
     public void Validate_JsonWithPath_ReturnsNull()
         => MqttTagConfigModel.FromJson("""{"topic":"a/b","payloadFormat":"Json","jsonPath":"$","dataType":"Float64"}""")