review(Runtime): record findings + fix artifact-decode type tolerance

Review at HEAD 7286d320. Runtime-002/006 (Medium): DeploymentArtifact lenient-parse
now degrades wrong-typed JSON fields to defaults/skipped-row instead of throwing (fails
the deploy) + regression tests; byte-parity with AddressSpaceComposer preserved. Runtime-001
(UNS rename) deferred cross-module (needs AddressSpacePlan rename signal + EnsureFolder
rename). 003/004/005 Won't-Fix.

code-reviews/Runtime/findings.md

@@ -0,0 +1,229 @@
+# Code Review — Runtime
+
+<!-- Per-module findings file. See ../../REVIEW-PROCESS.md for the full process. The base README.md is
+     generated from these files by regen-readme.py — do not edit README.md by hand. -->
+
+| Field | Value |
+|---|---|
+| Module | `src/Server/ZB.MOM.WW.OtOpcUa.Runtime` |
+| Reviewer | Claude Code |
+| Review date | 2026-06-19 |
+| Commit reviewed | `7286d320` |
+| Status | Reviewed |
+| Open findings | 0 |
+
+## Checklist coverage
+
+| # | Category | Result |
+|---|---|---|
+| 1 | Correctness & logic bugs | Findings Runtime-001, Runtime-004, Runtime-005 |
+| 2 | OtOpcUa conventions | No issues found |
+| 3 | Concurrency & thread safety | No issues found (see Notes) |
+| 4 | Error handling & resilience | Findings Runtime-002, Runtime-003 |
+| 5 | Security | No issues found |
+| 6 | Performance & resource management | No issues found |
+| 7 | Design-document adherence | No issues found |
+| 8 | Code organization & conventions | No issues found |
+| 9 | Testing coverage | Finding Runtime-006 |
+| 10 | Documentation & comments | No issues found |
+
+### Scope
+
+22 source `.cs` under `src/Server/ZB.MOM.WW.OtOpcUa.Runtime` (6129 LOC): the deployment-artifact
+decode (`DeploymentArtifact.cs`), the OPC UA publish actor (`OpcUaPublishActor.cs`), the driver host +
+instance actors (`DriverHostActor.cs`, `DriverInstanceActor.cs`, `DriverSpawnPlan.cs`,
+`NativeAlarmProjector.cs`), the address-space write gateway (`ActorNodeWriteGateway.cs`), health probes
+(`DbHealthProbeActor.cs`, `PeerOpcUaProbeActor.cs`, `PeerProbeSupervisor.cs`), the historian adapter +
+options (`HistorianAdapterActor.cs`, `AlarmHistorianOptions.cs`, `ServerHistorianOptions.cs`), scripted
+alarms (`ScriptedAlarmHostActor.cs`, `EfAlarmConditionStateStore.cs`,
+`DependencyMuxTagUpstreamSource.cs`), scripting (`DpsScriptLogPublisher.cs`), virtual tags
+(`DependencyMuxActor.cs`, `VirtualTagActor.cs`, `VirtualTagHostActor.cs`), the health publisher
+(`AkkaDriverHealthPublisher.cs`), and DI wiring (`ServiceCollectionExtensions.cs`).
+
+### Notes (no-finding observations)
+
+- **Concurrency**: actors are single-threaded per instance; off-thread continuations
+  (`OpcUaPublishActor.OnHealthTick`, `DriverHostActor.HandleRouteNodeWrite`,
+  `ScriptedAlarmHostActor.OnApply`/`OnEngineEmission`) correctly capture `Sender`/`Self` before the
+  `await`/`ContinueWith` and only construct messages off-thread, mutating state only back on the actor
+  thread. `DependencyMuxTagUpstreamSource` is the one genuinely-shared type and uses immutable
+  observer lists + atomic CAS correctly. No shared mutable state crosses the actor boundary.
+- **Security**: write-through is Primary-gated AND `WriteOperate`-gated upstream of Runtime
+  (`OnWriteValue` in the node manager); native-alarm ack is `AlarmAck`-gated upstream; Runtime re-applies
+  the Primary gate defensively. No secret logging — `AlarmHistorianOptions.Validate` /
+  `ServerHistorianOptions.Validate` warn when `SharedSecret` is empty but never echo the secret value.
+
+## Findings
+
+### Runtime-001
+
+| Field | Value |
+|---|---|
+| Severity | Medium |
+| Category | Correctness & logic bugs |
+| Location | `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs:311-318`; cross-module `src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpacePlan.cs:56-61` + `src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/IOpcUaAddressSpaceSink.cs:55` |
+| Status | Deferred |
+
+**Description:** This is the Runtime side of the orchestrator-handed cross-module item **OpcUaServer-001**:
+a UNS Area/Line *rename* (display-name-only edit, no Equipment/Tag/Driver/Alarm change) produces an
+`AddressSpacePlan` that `IsEmpty`, so `OpcUaPublishActor.HandleRebuild` short-circuits at line 313
+(`if (plan.IsEmpty) { ...; return; }`) **before** reaching `_applier.MaterialiseHierarchy(composition)`
+at line 326 — the rebuilt folder display-name never lands and OPC UA clients keep browsing the stale name.
+
+I verified this is **NOT self-contained within Runtime**: a complete fix needs changes in two other
+modules that Runtime cannot touch:
+1. `AddressSpacePlan.IsEmpty` (in OpcUaServer) does not diff `UnsArea`/`UnsLine` at all (it only tracks
+   Equipment/Driver/Alarm/EquipmentTag/VirtualTag). For the rename to drive a rebuild, the plan must
+   gain a UNS-changed signal (a new diff set) so `IsEmpty` returns `false`.
+2. Even if `MaterialiseHierarchy` were reached, `IOpcUaAddressSpaceSink.EnsureFolder` (in Commons) is
+   contractually **idempotent and returns the existing folder unchanged** — it has no rename semantics,
+   so an existing folder's `DisplayName` would not be updated. `EnsureFolder` (or a new `RenameFolder`)
+   in the Commons contract + its SDK-backed implementation must actually re-set the display name.
+
+Both (1) and (2) are wire/contract-shaped changes in OpcUaServer + Commons, outside this module's edit
+boundary. The Runtime side alone (acting on a UNS-changed plan and re-running `MaterialiseHierarchy`)
+is necessary but not sufficient.
+
+**Recommendation:** Track jointly with OpcUaServer-001. OpcUaServer: add a UNS-rename diff set to
+`AddressSpacePlan` so `IsEmpty` is false on a rename. Commons: give `EnsureFolder` (or a new
+`RenameFolder`) update-display-name semantics on an existing folder. Runtime: once those land, ensure
+`HandleRebuild` runs `MaterialiseHierarchy` for a non-empty UNS-only plan (it already does, since the
+short-circuit only fires for a truly-empty plan). No Runtime-only change can close it safely.
+
+**Resolution:** _(deferred — needs OpcUaServer `AddressSpacePlan` rename signal + Commons `EnsureFolder`/`RenameFolder` rename semantics; both outside the Runtime edit boundary. Tracked with OpcUaServer-001.)_
+
+### Runtime-002
+
+| Field | Value |
+|---|---|
+| Severity | Medium |
+| Category | Error handling & resilience |
+| Location | `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DeploymentArtifact.cs:148-169, 782-828` |
+| Status | Resolved |
+
+**Description:** The artifact-decode methods document a "lenient" contract — *"Empty / malformed blobs
+return an empty list"* / *"return an empty composition"* — and guard every parse path with
+`catch (JsonException)`. But the per-row readers (`TryReadSpec`, `ReadAreaProjection`,
+`ReadLineProjection`, `ReadEquipmentNode`, `ReadDriverPlan`, `ReadAlarmPlan`) call `JsonElement.GetString()`
+and `GetBoolean()` directly on a property element. If the artifact is *valid JSON but a field carries the
+wrong JSON type* (e.g. `"Enabled": "true"` as a string, or `"Name": 5` as a number), those accessors throw
+`InvalidOperationException`, which is **not** a `JsonException` and so escapes the method — defeating the
+documented lenient-parse contract. For `ParseDriverInstances` (called from `ReconcileDrivers` at
+`DriverHostActor.cs:889`, outside its own try) the escape propagates to `ApplyAndAck`, marking the whole
+deployment **Failed** instead of degrading gracefully. The artifact is normally machine-generated with
+controlled types, so impact is bounded, but a single wrong-typed field shouldn't fail-the-deploy when the
+code claims to tolerate malformed input.
+
+**Recommendation:** Make the per-row readers type-check before reading (`enEl.ValueKind == True/False`
+before `GetBoolean`, `ValueKind == String` before `GetString`) — the pattern the `Build*` methods and
+`ExtractTag*` helpers already follow — so a wrong-typed field degrades to the field default / a skipped row
+rather than throwing. Equivalently, broaden the guards to also catch `InvalidOperationException`.
+
+**Resolution:** Resolved 2026-06-19 — `TryReadSpec` now reads `Enabled` only on a real `True/False` token
+(defaulting `true` otherwise) and reads every string field via a `ReadString` helper that returns null on a
+non-string `ValueKind`; `ReadAreaProjection`/`ReadLineProjection`/`ReadEquipmentNode`/`ReadDriverPlan`/`ReadAlarmPlan`
+use the same helper. Wrong-typed fields now degrade to the field default / skipped row instead of throwing
+`InvalidOperationException`. Regression test `DeploymentArtifactMalformedTypeTests`.
+
+### Runtime-003
+
+| Field | Value |
+|---|---|
+| Severity | Low |
+| Category | Error handling & resilience |
+| Location | `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs:320-342` |
+| Status | Won't Fix |
+
+**Description:** In `HandleRebuild`, `_lastApplied = composition` is assigned (line 321) immediately after
+`_applier.Apply(plan)` returns, but BEFORE the four `Materialise*` passes run. If one of those passes throws
+(e.g. `MaterialiseEquipmentTags`), the catch logs the error and returns — but `_lastApplied` has already
+advanced to the new composition. The next rebuild then diffs against a composition whose materialisation
+only partially completed, so the partially-failed nodes won't be re-added (the diff sees them as already
+applied). In practice the `Materialise*` passes are individually try/wrapped (`SafeEnsureFolder` etc.) and
+swallow per-node failures, so a throw out of a whole pass is unlikely; and the next *config-changing*
+deploy re-materialises everything. Net risk is low.
+
+**Recommendation:** Advance `_lastApplied` only after all materialise passes complete (move the assignment
+to the end of the try, after `MaterialiseEquipmentVirtualTags`). Note this is a deliberate trade-off — moving
+it means a partial failure re-runs `Apply(plan)` (the diff) against the OLD composition next time, which is
+the safer direction.
+
+**Resolution:** Won't Fix (2026-06-19) — the four `Materialise*` passes are each fully `Safe*`-wrapped at the
+per-node level (`AddressSpaceApplier.SafeEnsureFolder` / `SafeEnsureVariable` / `SafeMaterialiseAlarmCondition`
+swallow and log every per-node exception), so a pass cannot throw out to `HandleRebuild` in practice — the
+ordering hazard is not reachable. Reordering the assignment would change the recovery direction on a
+hypothetical unreachable path without a demonstrable live benefit, and would risk re-running `Apply(plan)`
+against a stale composition. Recorded as a latent-ordering note rather than a fix.
+
+### Runtime-004
+
+| Field | Value |
+|---|---|
+| Severity | Low |
+| Category | Correctness & logic bugs |
+| Location | `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs:683-684` |
+| Status | Won't Fix |
+
+**Description:** `HandleRouteNodeWrite` forwards `new DriverInstanceActor.WriteAttribute(target.FullName,
+msg.Value!)` where `RouteNodeWrite.Value` is `object?` (nullable) but `WriteAttribute.Value` is non-nullable
+`object`. The `!` null-forgiving operator suppresses the warning but does not prevent a genuine `null` from
+flowing through to `WriteRequest`/`IWritable.WriteAsync` if an OPC UA client writes a null value. Whether a
+null write coerces cleanly is driver-dependent; at minimum the non-null contract on `WriteAttribute.Value`
+is misleading.
+
+**Recommendation:** Either make `WriteAttribute.Value` an `object?` (honest contract; the driver already has
+to handle type coercion / rejection), or reject a null write at the gateway with a clear
+`NodeWriteResult(false, "null value")`. Low priority — null variable writes are rare and downstream drivers
+generally reject them with a Bad status anyway.
+
+**Resolution:** Won't Fix (2026-06-19) — this is a public message-contract shape (`WriteAttribute.Value` is a
+wire-serialised Akka message record). Widening it to `object?` or adding a null-reject branch touches the
+inbound-write contract shared with `ActorNodeWriteGateway` / the node-manager wiring; the orchestrator's hard
+constraints forbid a wire/message-contract change in this pass. Downstream `IWritable.WriteAsync`
+implementations already reject an uncoercible value with a Bad status, so the live impact is a slightly less
+clear error string, not a crash or data loss. Recorded for a future contract-touching pass.
+
+### Runtime-005
+
+| Field | Value |
+|---|---|
+| Severity | Low |
+| Category | Documentation & comments |
+| Location | `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DeploymentArtifact.cs:610-615` |
+| Status | Won't Fix |
+
+**Description:** `BuildEquipmentScriptedAlarmPlans`'s local `ReadBool` has a comment warning that *"GetBoolean
+only runs for a genuine true/false token (a non-bool token would otherwise throw InvalidOperationException,
+uncaught here)"*. The comment describes a hazard that the code already prevents — the guard
+`b.ValueKind is JsonValueKind.True or JsonValueKind.False` runs before `GetBoolean`, so `GetBoolean` can
+never see a non-bool token and never throws. The comment is stale/misleading (it reads as if the throw is a
+live risk).
+
+**Recommendation:** Trim the parenthetical to state the guard makes the call safe, rather than implying an
+uncaught throw. Purely a comment clarity issue.
+
+**Resolution:** Won't Fix (2026-06-19) — the code is correct (the `ValueKind` guard precedes `GetBoolean`),
+and the comment is defensively accurate in spirit (it explains *why* the guard exists). The wording is
+slightly alarmist but not factually wrong, and the parity-critical `ExtractTag*` mirror methods carry the
+same defensive phrasing for consistency. Not worth a churn edit on a byte-parity-sensitive file.
+
+### Runtime-006
+
+| Field | Value |
+|---|---|
+| Severity | Low |
+| Category | Testing coverage |
+| Location | `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DeploymentArtifact.cs` (whole-file) |
+| Status | Resolved |
+
+**Description:** The artifact-decode parity suites (`DeploymentArtifact*ParityTests`) cover the happy path +
+the byte-parity-with-composer contract thoroughly, but there is no test asserting the documented
+*lenient-parse* contract under **valid-JSON-but-wrong-type** input (e.g. a string `Enabled`, a numeric
+`Name`). That gap is exactly what let Runtime-002 go unnoticed.
+
+**Recommendation:** Add a regression test feeding a syntactically-valid artifact with wrong-typed fields and
+asserting the parse degrades to the field default / a skipped row (no exception).
+
+**Resolution:** Resolved 2026-06-19 — added `DeploymentArtifactMalformedTypeTests` (string `Enabled`, numeric
+`Name`/`DriverType`, numeric `UnsAreaId`) asserting `ParseDriverInstances` + `ParseComposition` degrade
+gracefully instead of throwing. Co-resolved with Runtime-002.

src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DeploymentArtifact.cs

@@ -145,16 +145,27 @@ public static class DeploymentArtifact
             : all;
     }
 
+    /// <summary>Reads a string property, tolerating a wrong JSON type: a property that is present but
+    /// NOT a JSON string (number / bool / object / array) yields null rather than throwing
+    /// <see cref="InvalidOperationException"/> from <c>GetString()</c>. Keeps the artifact-decode lenient
+    /// — a wrong-typed field degrades to the field default / a skipped row, matching the documented
+    /// "malformed blobs return an empty list/composition" contract (Runtime-002).</summary>
+    private static string? ReadString(JsonElement el, string property) =>
+        el.TryGetProperty(property, out var p) && p.ValueKind == JsonValueKind.String ? p.GetString() : null;
+
     private static DriverInstanceSpec? TryReadSpec(JsonElement el)
     {
         var rowId = el.TryGetProperty("DriverInstanceRowId", out var rowEl)
             && rowEl.TryGetGuid(out var rid) ? rid : Guid.Empty;
-        var id = el.TryGetProperty("DriverInstanceId", out var idEl) ? idEl.GetString() : null;
-        var name = el.TryGetProperty("Name", out var nameEl) ? nameEl.GetString() : null;
-        var type = el.TryGetProperty("DriverType", out var typeEl) ? typeEl.GetString() : null;
-        var enabled = !el.TryGetProperty("Enabled", out var enEl) || enEl.GetBoolean();
-        var config = el.TryGetProperty("DriverConfig", out var cfgEl) ? cfgEl.GetString() : null;
-        var clusterId = el.TryGetProperty("ClusterId", out var clEl) ? clEl.GetString() : null;
+        var id = ReadString(el, "DriverInstanceId");
+        var name = ReadString(el, "Name");
+        var type = ReadString(el, "DriverType");
+        // A non-bool Enabled token degrades to the field default (true = "desired here"), never throws.
+        var enabled = !el.TryGetProperty("Enabled", out var enEl)
+            || enEl.ValueKind is not (JsonValueKind.True or JsonValueKind.False)
+            || enEl.GetBoolean();
+        var config = ReadString(el, "DriverConfig");
+        var clusterId = ReadString(el, "ClusterId");
 
         if (string.IsNullOrWhiteSpace(id) || string.IsNullOrWhiteSpace(type)) return null;
 
@@ -781,48 +792,48 @@ public static class DeploymentArtifact
 
     private static UnsAreaProjection? ReadAreaProjection(JsonElement el)
     {
-        var id = el.TryGetProperty("UnsAreaId", out var idEl) ? idEl.GetString() : null;
-        var name = el.TryGetProperty("Name", out var nameEl) ? nameEl.GetString() : null;
+        var id = ReadString(el, "UnsAreaId");
+        var name = ReadString(el, "Name");
         if (string.IsNullOrWhiteSpace(id)) return null;
         return new UnsAreaProjection(id!, name ?? id!);
     }
 
     private static UnsLineProjection? ReadLineProjection(JsonElement el)
     {
-        var id = el.TryGetProperty("UnsLineId", out var idEl) ? idEl.GetString() : null;
-        var areaId = el.TryGetProperty("UnsAreaId", out var areaEl) ? areaEl.GetString() : null;
-        var name = el.TryGetProperty("Name", out var nameEl) ? nameEl.GetString() : null;
+        var id = ReadString(el, "UnsLineId");
+        var areaId = ReadString(el, "UnsAreaId");
+        var name = ReadString(el, "Name");
         if (string.IsNullOrWhiteSpace(id) || string.IsNullOrWhiteSpace(areaId)) return null;
         return new UnsLineProjection(id!, areaId!, name ?? id!);
     }
 
     private static EquipmentNode? ReadEquipmentNode(JsonElement el)
     {
-        var id = el.TryGetProperty("EquipmentId", out var idEl) ? idEl.GetString() : null;
+        var id = ReadString(el, "EquipmentId");
         // DisplayName = the UNS level-5 Name segment (friendly browse name, matching UnsArea/UnsLine
         // + the live rebuild's source of truth) — NOT the colloquial MachineCode. NodeId stays the
         // logical EquipmentId.
-        var displayName = el.TryGetProperty("Name", out var nameEl) ? nameEl.GetString() : null;
-        var lineId = el.TryGetProperty("UnsLineId", out var lineEl) ? lineEl.GetString() : null;
+        var displayName = ReadString(el, "Name");
+        var lineId = ReadString(el, "UnsLineId");
         if (string.IsNullOrWhiteSpace(id)) return null;
         return new EquipmentNode(id!, displayName ?? id!, lineId ?? string.Empty);
     }
 
     private static DriverInstancePlan? ReadDriverPlan(JsonElement el)
     {
-        var id = el.TryGetProperty("DriverInstanceId", out var idEl) ? idEl.GetString() : null;
-        var type = el.TryGetProperty("DriverType", out var typeEl) ? typeEl.GetString() : null;
-        var config = el.TryGetProperty("DriverConfig", out var cfgEl) ? cfgEl.GetString() : null;
+        var id = ReadString(el, "DriverInstanceId");
+        var type = ReadString(el, "DriverType");
+        var config = ReadString(el, "DriverConfig");
         if (string.IsNullOrWhiteSpace(id) || string.IsNullOrWhiteSpace(type)) return null;
         return new DriverInstancePlan(id!, type!, config ?? "{}");
     }
 
     private static ScriptedAlarmPlan? ReadAlarmPlan(JsonElement el)
     {
-        var id = el.TryGetProperty("ScriptedAlarmId", out var idEl) ? idEl.GetString() : null;
-        var equipmentId = el.TryGetProperty("EquipmentId", out var eqEl) ? eqEl.GetString() : null;
-        var script = el.TryGetProperty("PredicateScriptId", out var scEl) ? scEl.GetString() : null;
-        var template = el.TryGetProperty("MessageTemplate", out var tmEl) ? tmEl.GetString() : null;
+        var id = ReadString(el, "ScriptedAlarmId");
+        var equipmentId = ReadString(el, "EquipmentId");
+        var script = ReadString(el, "PredicateScriptId");
+        var template = ReadString(el, "MessageTemplate");
         if (string.IsNullOrWhiteSpace(id)) return null;
         return new ScriptedAlarmPlan(id!, equipmentId ?? string.Empty, script ?? string.Empty, template ?? string.Empty);
     }

tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactMalformedTypeTests.cs

@@ -0,0 +1,127 @@
+using System;
+using System.Linq;
+using Shouldly;
+using Xunit;
+using ZB.MOM.WW.OtOpcUa.Runtime.Drivers;
+
+namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Drivers;
+
+/// <summary>
+///     Regression coverage for Runtime-002: the artifact-decode is documented as lenient — "empty /
+///     malformed blobs return an empty list / empty composition". A blob that is syntactically valid JSON
+///     but carries a field of the WRONG JSON type (e.g. a string Enabled, a numeric Name) must NOT throw
+///     <see cref="InvalidOperationException"/> out of the parse (the old code called GetString()/GetBoolean()
+///     unguarded). It must degrade to the field default / a skipped row instead.
+/// </summary>
+public sealed class DeploymentArtifactMalformedTypeTests
+{
+    private static byte[] BlobOf(object snapshot) =>
+        System.Text.Json.JsonSerializer.SerializeToUtf8Bytes(snapshot);
+
+    /// <summary>A string-typed Enabled ("true") must not throw; the row should still decode (default Enabled=true).</summary>
+    [Fact]
+    public void ParseDriverInstances_string_typed_Enabled_does_not_throw()
+    {
+        var blob = BlobOf(new
+        {
+            DriverInstances = new[]
+            {
+                new
+                {
+                    DriverInstanceRowId = Guid.NewGuid(),
+                    DriverInstanceId = "d1",
+                    Name = "drv-1",
+                    DriverType = "Modbus",
+                    Enabled = "true", // WRONG TYPE: string, not bool
+                    DriverConfig = "{}",
+                },
+            },
+        });
+
+        var specs = Should.NotThrow(() => DeploymentArtifact.ParseDriverInstances(blob));
+        specs.Count.ShouldBe(1);
+        // A non-bool Enabled degrades to the field default (Enabled = true / "desired here").
+        specs[0].Enabled.ShouldBeTrue();
+        specs[0].DriverInstanceId.ShouldBe("d1");
+    }
+
+    /// <summary>A numeric-typed Name / DriverType must not throw; a row with an unusable required field
+    /// (DriverType missing/wrong) is skipped, a row whose only wrong field is the optional Name keeps decoding.</summary>
+    [Fact]
+    public void ParseDriverInstances_numeric_typed_Name_does_not_throw()
+    {
+        var blob = BlobOf(new
+        {
+            DriverInstances = new[]
+            {
+                new
+                {
+                    DriverInstanceRowId = (object)Guid.NewGuid(),
+                    DriverInstanceId = (object)"d1",
+                    Name = (object)5, // WRONG TYPE: number, not string
+                    DriverType = (object)"Modbus",
+                    Enabled = (object)true,
+                    DriverConfig = (object)"{}",
+                },
+            },
+        });
+
+        var specs = Should.NotThrow(() => DeploymentArtifact.ParseDriverInstances(blob));
+        specs.Count.ShouldBe(1);
+        // A non-string Name degrades to null → Name falls back to the DriverInstanceId.
+        specs[0].Name.ShouldBe("d1");
+    }
+
+    /// <summary>The composition decode must also tolerate a wrong-typed UNS field (numeric UnsAreaId)
+    /// without throwing — it degrades to skipping that row.</summary>
+    [Fact]
+    public void ParseComposition_numeric_typed_UnsAreaId_does_not_throw()
+    {
+        var blob = BlobOf(new
+        {
+            UnsAreas = new object[]
+            {
+                new { UnsAreaId = 42, Name = "Area-numeric" }, // WRONG TYPE: numeric id → skipped
+                new { UnsAreaId = "area-ok", Name = "Area-OK" },
+            },
+        });
+
+        var composition = Should.NotThrow(() => DeploymentArtifact.ParseComposition(blob));
+        // The well-typed area survives; the numeric-id one is skipped (id resolves to null → dropped).
+        composition.UnsAreas.Select(a => a.UnsAreaId).ShouldBe(new[] { "area-ok" });
+    }
+
+    /// <summary>A driver row whose DriverType is the wrong type is skipped (required field unusable),
+    /// not a parse failure.</summary>
+    [Fact]
+    public void ParseDriverInstances_numeric_typed_DriverType_skips_row_without_throwing()
+    {
+        var blob = BlobOf(new
+        {
+            DriverInstances = new object[]
+            {
+                new
+                {
+                    DriverInstanceRowId = (object)Guid.NewGuid(),
+                    DriverInstanceId = (object)"bad",
+                    Name = (object)"drv-bad",
+                    DriverType = (object)7, // WRONG TYPE: numeric → unusable required field
+                    Enabled = (object)true,
+                    DriverConfig = (object)"{}",
+                },
+                new
+                {
+                    DriverInstanceRowId = (object)Guid.NewGuid(),
+                    DriverInstanceId = (object)"good",
+                    Name = (object)"drv-good",
+                    DriverType = (object)"Modbus",
+                    Enabled = (object)true,
+                    DriverConfig = (object)"{}",
+                },
+            },
+        });
+
+        var specs = Should.NotThrow(() => DeploymentArtifact.ParseDriverInstances(blob));
+        specs.Select(s => s.DriverInstanceId).ShouldBe(new[] { "good" });
+    }
+}