review(wave-b): close SA {{equip}} authoring gap (M1) + harden parity tests (L3)

MEDIUM-1: the editor<->authoring<->deploy invariant now holds on the ScriptedAlarm surface too. CreateScriptedAlarmAsync/UpdateScriptedAlarmAsync validate {{equip}}/<RefName> in BOTH the predicate script and the message template (via ExtractEquipReferenceNamesFromText), matching what DraftValidator already enforces at deploy. Refactored ValidateEquipTokenAsync into shared LoadEquipReferenceNamesAsync + CheckEquipReferencesResolve helpers. LOW-1: LoadEquipReferenceNamesAsync uses IsNullOrEmpty (defense-in-depth) to match EquipmentReferenceMap.Build; empty override is already normalized->null at the write path. LOW-3: parity test hardened with two edge cases — unresolved-ref-left-intact (both seams leave {{equip}}/X identical) and folder+tag-group RawPath ancestry. Tests: VirtualTagEquipTokenValidationTests 9/9 (+3 SA), parity 4/4 (+2). AdminUI 659/0. Claude-Session: https://claude.ai/code/session_01LVneM3eh1UtJxEisFXgmox
2026-07-16 07:18:59 -04:00
parent 29bb4f176e
commit 96af69e3d2
3 changed files with 174 additions and 2 deletions
@@ -1241,8 +1241,37 @@ public sealed class UnsTreeService(
            .Select(s => s.SourceCode).FirstOrDefaultAsync(ct);
        var used = EquipmentScriptPaths.ExtractEquipReferenceNames(src);
        if (used.Count == 0) return null;
+        var effectiveNames = await LoadEquipReferenceNamesAsync(db, equipmentId, ct);
+        return CheckEquipReferencesResolve(used, effectiveNames, equipmentId);
+    }

-        // The equipment's reference effective-name set: DisplayNameOverride else the backing raw tag's Name.
+    /// <summary>
+    /// Scripted-alarm variant of <see cref="ValidateEquipTokenAsync"/>: validates <c>{{equip}}/&lt;RefName&gt;</c>
+    /// tokens in BOTH the predicate script AND the message template against the equipment's reference set —
+    /// the authoring mirror of the deploy gate, which covers all three (VT scripts, SA predicates, SA templates).
+    /// Keeps the editor⇔authoring⇔deploy invariant tight on the SA surface.
+    /// </summary>
+    private static async Task<UnsMutationResult?> ValidateEquipTokenForAlarmAsync(
+        OtOpcUaConfigDbContext db, string equipmentId, string? predicateScriptId, string? messageTemplate, CancellationToken ct)
+    {
+        var used = new HashSet<string>(StringComparer.Ordinal);
+        if (!string.IsNullOrEmpty(predicateScriptId))
+        {
+            var src = await db.Scripts.Where(s => s.ScriptId == predicateScriptId)
+                .Select(s => s.SourceCode).FirstOrDefaultAsync(ct);
+            foreach (var u in EquipmentScriptPaths.ExtractEquipReferenceNames(src)) used.Add(u);
+        }
+        foreach (var u in EquipmentScriptPaths.ExtractEquipReferenceNamesFromText(messageTemplate)) used.Add(u);
+        if (used.Count == 0) return null;
+        var effectiveNames = await LoadEquipReferenceNamesAsync(db, equipmentId, ct);
+        return CheckEquipReferencesResolve(used, effectiveNames, equipmentId);
+    }
+
+    /// <summary>The equipment's reference effective-name set: <c>DisplayNameOverride</c> else the backing raw
+    /// tag's <c>Name</c> (empty override treated as absent, matching <c>EquipmentReferenceMap.Build</c>), ordinal.</summary>
+    private static async Task<HashSet<string>> LoadEquipReferenceNamesAsync(
+        OtOpcUaConfigDbContext db, string equipmentId, CancellationToken ct)
+    {
        var refs = await db.UnsTagReferences.AsNoTracking()
            .Where(r => r.EquipmentId == equipmentId)
            .Select(r => new { r.TagId, r.DisplayNameOverride })
@@ -1255,10 +1284,18 @@ public sealed class UnsTreeService(
        var effectiveNames = new HashSet<string>(StringComparer.Ordinal);
        foreach (var r in refs)
        {
-            var eff = r.DisplayNameOverride ?? (tagNameById.TryGetValue(r.TagId, out var n) ? n : null);
+            var eff = string.IsNullOrEmpty(r.DisplayNameOverride)
+                ? (tagNameById.TryGetValue(r.TagId, out var n) ? n : null)
+                : r.DisplayNameOverride;
            if (!string.IsNullOrEmpty(eff)) effectiveNames.Add(eff!);
        }
+        return effectiveNames;
+    }

+    /// <summary>Names the unresolved <c>{{equip}}/&lt;RefName&gt;</c> tokens (or null when all resolve).</summary>
+    private static UnsMutationResult? CheckEquipReferencesResolve(
+        IReadOnlyCollection<string> used, HashSet<string> effectiveNames, string equipmentId)
+    {
        var missing = used.Where(u => !effectiveNames.Contains(u)).ToList();
        if (missing.Count == 0) return null;
        return new UnsMutationResult(false,
@@ -1614,6 +1651,10 @@ public sealed class UnsTreeService(
        var nameGuard = await _guard.CheckAsync(equipmentId, input.Name, EffectiveNameSourceKind.ScriptedAlarm, null, ct);
        if (nameGuard is not null) return new UnsMutationResult(false, nameGuard);

+        // v3 Batch 3 (WP4): {{equip}}/<RefName> in the predicate script or message template must resolve.
+        var equipGuard = await ValidateEquipTokenForAlarmAsync(db, equipmentId, input.PredicateScriptId, input.MessageTemplate, ct);
+        if (equipGuard is not null) return equipGuard.Value;
+
        db.ScriptedAlarms.Add(new ScriptedAlarm
        {
            ScriptedAlarmId = input.ScriptedAlarmId,
@@ -1648,6 +1689,10 @@ public sealed class UnsTreeService(
        var nameGuard = await _guard.CheckAsync(entity.EquipmentId, input.Name, EffectiveNameSourceKind.ScriptedAlarm, scriptedAlarmId, ct);
        if (nameGuard is not null) return new UnsMutationResult(false, nameGuard);

+        // v3 Batch 3 (WP4): {{equip}}/<RefName> in the predicate script or message template must resolve.
+        var equipGuard = await ValidateEquipTokenForAlarmAsync(db, entity.EquipmentId, input.PredicateScriptId, input.MessageTemplate, ct);
+        if (equipGuard is not null) return equipGuard.Value;
+
        db.Entry(entity).Property(e => e.RowVersion).OriginalValue = rowVersion;
        entity.Name = input.Name;
        entity.AlarmType = input.AlarmType;
@@ -189,4 +189,55 @@ public sealed class VirtualTagEquipTokenValidationTests
        result.Ok.ShouldBeTrue();
        result.Error.ShouldBeNull();
    }
+
+    // ----- ScriptedAlarm (Wave-B review MEDIUM-1: the invariant must hold on the SA surface too) -----
+
+    private static ScriptedAlarmInput AlarmInput(string messageTemplate = "value out of range") =>
+        new("ALM-1", "HighTemp", AlarmType: "Rate", Severity: 500, MessageTemplate: messageTemplate,
+            PredicateScriptId: "SCRIPT-1", HistorizeToAveva: false, Retain: false, Enabled: true);
+
+    /// <summary>SA predicate uses {{equip}}/Speed with no matching reference → create rejected, error names the token.</summary>
+    [Fact]
+    public async Task Create_alarm_equip_ref_in_predicate_unresolved_rejected()
+    {
+        var (service, dbName) = Fresh();
+        Seed(dbName, EquipRefScript, referenceName: null);
+
+        var result = await service.CreateScriptedAlarmAsync("EQ-1", AlarmInput());
+
+        result.Ok.ShouldBeFalse();
+        result.Error.ShouldNotBeNull();
+        result.Error.ShouldContain("EQ-1");
+        result.Error.ShouldContain("{{equip}}/Speed");
+
+        using var db = UnsTreeTestDb.CreateNamed(dbName);
+        db.ScriptedAlarms.Any(a => a.ScriptedAlarmId == "ALM-1").ShouldBeFalse();
+    }
+
+    /// <summary>SA message template carries {{equip}}/Bad with no matching reference → create rejected (template scan).</summary>
+    [Fact]
+    public async Task Create_alarm_equip_ref_in_message_template_unresolved_rejected()
+    {
+        var (service, dbName) = Fresh();
+        Seed(dbName, PlainScript, referenceName: "Speed"); // predicate has no token; only the template does
+
+        var result = await service.CreateScriptedAlarmAsync("EQ-1", AlarmInput(messageTemplate: "bad {{equip}}/Missing here"));
+
+        result.Ok.ShouldBeFalse();
+        result.Error.ShouldNotBeNull();
+        result.Error.ShouldContain("{{equip}}/Missing");
+    }
+
+    /// <summary>SA predicate {{equip}}/Speed with a matching reference → create succeeds.</summary>
+    [Fact]
+    public async Task Create_alarm_equip_ref_that_resolves_succeeds()
+    {
+        var (service, dbName) = Fresh();
+        Seed(dbName, EquipRefScript, referenceName: "Speed");
+
+        var result = await service.CreateScriptedAlarmAsync("EQ-1", AlarmInput());
+
+        result.Ok.ShouldBeTrue();
+        result.Error.ShouldBeNull();
+    }
 }
@@ -122,4 +122,80 @@ public sealed class DeploymentArtifactEquipRefParityTests
        var dPlan = decoded.EquipmentScriptedAlarms.ShouldHaveSingleItem();
        dPlan.ShouldBe(cPlan);
    }
+
+    /// <summary>Wave-B review LOW-3 edge: an UNRESOLVED <c>{{equip}}/&lt;RefName&gt;</c> (no matching reference)
+    /// must be left literally intact — identically — on both seams (the deploy gate rejects it separately; the
+    /// compose seams must not diverge). Here the only reference is "MotorSpeed", so "Nope" cannot resolve.</summary>
+    [Fact]
+    public void VirtualTag_equip_ref_unresolved_left_intact_byte_parity()
+    {
+        const string unresolvedScript = "return ctx.GetTag(\"{{equip}}/Nope\").Value;";
+        var script = new Script { ScriptId = "s-1", Name = "n", SourceCode = unresolvedScript, SourceHash = "h" };
+        var vt = new VirtualTag { VirtualTagId = "vt-1", EquipmentId = "eq-1", Name = "Over", DataType = "Double", ScriptId = "s-1" };
+
+        var composed = AddressSpaceComposer.Compose(
+            Array.Empty<UnsArea>(), Array.Empty<UnsLine>(), Array.Empty<Equipment>(),
+            new[] { Driver() }, Array.Empty<ScriptedAlarm>(),
+            virtualTags: new[] { vt }, scripts: new[] { script },
+            unsTagReferences: new[] { Ref() }, tags: new[] { RawTag() },
+            rawFolders: Array.Empty<RawFolder>(), devices: new[] { Dev() }, tagGroups: Array.Empty<TagGroup>());
+
+        var blob = JsonSerializer.SerializeToUtf8Bytes(new
+        {
+            DriverInstances = new[] { new { DriverInstanceId = "drv-1", DriverType = "Modbus", DriverConfig = "{}", Name = "Modbus", RawFolderId = (string?)null } },
+            Devices = new[] { new { DeviceId = "dev-1", DriverInstanceId = "drv-1", Name = "Dev1", DeviceConfig = "{}" } },
+            Tags = new[] { new { TagId = "tag-1", DeviceId = "dev-1", Name = "Speed", DataType = "Float", TagConfig = "{}" } },
+            UnsTagReferences = new[] { new { UnsTagReferenceId = "ref-1", EquipmentId = "eq-1", TagId = "tag-1", DisplayNameOverride = "MotorSpeed" } },
+            Scripts = new[] { new { ScriptId = "s-1", SourceCode = unresolvedScript } },
+            VirtualTags = new[] { new { VirtualTagId = "vt-1", EquipmentId = "eq-1", Name = "Over", DataType = "Double", ScriptId = "s-1" } },
+        });
+
+        var decoded = DeploymentArtifact.ParseComposition(blob);
+
+        var cPlan = composed.EquipmentVirtualTags.ShouldHaveSingleItem();
+        cPlan.Expression.ShouldContain("{{equip}}/Nope"); // left intact — not substituted
+        var dPlan = decoded.EquipmentVirtualTags.ShouldHaveSingleItem();
+        dPlan.ShouldBe(cPlan);
+    }
+
+    /// <summary>Wave-B review LOW-3 edge: RawPath ancestry (folder + tag-group) must resolve identically on both
+    /// seams. Driver under folder "Cell1", tag under group "Fast" → RawPath "Cell1/Modbus/Dev1/Fast/Speed".</summary>
+    [Fact]
+    public void VirtualTag_equip_ref_resolves_through_folder_and_group_byte_parity()
+    {
+        const string deepRawPath = "Cell1/Modbus/Dev1/Fast/Speed";
+        var driver = new DriverInstance { DriverInstanceId = "drv-1", ClusterId = "c1", Name = "Modbus", DriverType = "Modbus", DriverConfig = "{}", RawFolderId = "fld-1" };
+        var folder = new RawFolder { RawFolderId = "fld-1", ClusterId = "c1", Name = "Cell1", ParentRawFolderId = null };
+        var group = new TagGroup { TagGroupId = "grp-1", DeviceId = "dev-1", Name = "Fast", ParentTagGroupId = null };
+        var tag = new Tag { TagId = "tag-1", DeviceId = "dev-1", TagGroupId = "grp-1", Name = "Speed", DataType = "Float", AccessLevel = TagAccessLevel.Read, TagConfig = "{}" };
+        var script = new Script { ScriptId = "s-1", Name = "n", SourceCode = VtScript, SourceHash = "h" };
+        var vt = new VirtualTag { VirtualTagId = "vt-1", EquipmentId = "eq-1", Name = "Over", DataType = "Boolean", ScriptId = "s-1" };
+
+        var composed = AddressSpaceComposer.Compose(
+            Array.Empty<UnsArea>(), Array.Empty<UnsLine>(), Array.Empty<Equipment>(),
+            new[] { driver }, Array.Empty<ScriptedAlarm>(),
+            virtualTags: new[] { vt }, scripts: new[] { script },
+            unsTagReferences: new[] { Ref() }, tags: new[] { tag },
+            rawFolders: new[] { folder }, devices: new[] { Dev() }, tagGroups: new[] { group });
+
+        var blob = JsonSerializer.SerializeToUtf8Bytes(new
+        {
+            RawFolders = new[] { new { RawFolderId = "fld-1", ParentRawFolderId = (string?)null, Name = "Cell1" } },
+            DriverInstances = new[] { new { DriverInstanceId = "drv-1", DriverType = "Modbus", DriverConfig = "{}", Name = "Modbus", RawFolderId = "fld-1" } },
+            Devices = new[] { new { DeviceId = "dev-1", DriverInstanceId = "drv-1", Name = "Dev1", DeviceConfig = "{}" } },
+            TagGroups = new[] { new { TagGroupId = "grp-1", ParentTagGroupId = (string?)null, Name = "Fast" } },
+            Tags = new[] { new { TagId = "tag-1", DeviceId = "dev-1", TagGroupId = "grp-1", Name = "Speed", DataType = "Float", TagConfig = "{}" } },
+            UnsTagReferences = new[] { new { UnsTagReferenceId = "ref-1", EquipmentId = "eq-1", TagId = "tag-1", DisplayNameOverride = "MotorSpeed" } },
+            Scripts = new[] { new { ScriptId = "s-1", SourceCode = VtScript } },
+            VirtualTags = new[] { new { VirtualTagId = "vt-1", EquipmentId = "eq-1", Name = "Over", DataType = "Boolean", ScriptId = "s-1" } },
+        });
+
+        var decoded = DeploymentArtifact.ParseComposition(blob);
+
+        var cPlan = composed.EquipmentVirtualTags.ShouldHaveSingleItem();
+        cPlan.Expression.ShouldContain($"ctx.GetTag(\"{deepRawPath}\")");
+        cPlan.DependencyRefs.ShouldBe(new[] { deepRawPath });
+        var dPlan = decoded.EquipmentVirtualTags.ShouldHaveSingleItem();
+        dPlan.ShouldBe(cPlan);
+    }
 }