From adb5e75ec36e0bb9d29d765dba7ffd8af15b1bff Mon Sep 17 00:00:00 2001
From: Joseph Doherty <dohejw01@gmail.com>
Date: Sat, 16 May 2026 22:32:30 -0400
Subject: [PATCH] =?UTF-8?q?fix(template-engine):=20resolve=20TemplateEngin?=
 =?UTF-8?q?e-011,013,014=20=E2=80=94=20remove=20dead=20converter,=20duplic?=
 =?UTF-8?q?ate-id-safe=20cycle=20detection,=20unified=20deletion=20logic;?=
 =?UTF-8?q?=20TemplateEngine-012=20deferred?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 code-reviews/TemplateEngine/findings.md       | 79 ++++++++++++++++---
 .../CollisionDetector.cs                      |  5 +-
 src/ScadaLink.TemplateEngine/CycleDetector.cs | 46 +++++++----
 .../Flattening/RevisionHashService.cs         | 32 ++++----
 .../TemplateResolver.cs                       |  5 +-
 .../TemplateService.cs                        | 63 +++------------
 .../CycleDetectorTests.cs                     | 73 +++++++++++++++++
 .../Flattening/RevisionHashServiceTests.cs    | 32 ++++++++
 .../TemplateServiceTests.cs                   | 37 ++++++++-
 9 files changed, 274 insertions(+), 98 deletions(-)

diff --git a/code-reviews/TemplateEngine/findings.md b/code-reviews/TemplateEngine/findings.md
index a0dec5a..f2de185 100644
--- a/code-reviews/TemplateEngine/findings.md
+++ b/code-reviews/TemplateEngine/findings.md
@@ -8,7 +8,7 @@
 | Last reviewed | 2026-05-16 |
 | Reviewer | claude-agent |
 | Commit reviewed | `9c60592` |
-| Open findings | 4 |
+| Open findings | 0 |
 
 ## Summary
 
@@ -474,7 +474,7 @@ behaviour change; no regression test (documentation-only).
 |--|--|
 | Severity | Low |
 | Category | Documentation & comments |
-| Status | Open |
+| Status | Resolved |
 | Location | `src/ScadaLink.TemplateEngine/Flattening/RevisionHashService.cs:136` |
 
 **Description**
@@ -497,7 +497,18 @@ ordering of the `Hashable*` records (ideally enforced by a test).
 
 **Resolution**
 
-_Unresolved._
+Resolved 2026-05-16 (commit `pending commit`): deleted the dead
+`SortedPropertiesConverterFactory` (and removed it from `CanonicalJsonOptions`),
+and replaced the misleading "sorted keys / consistent ordering" comments with an
+explicit DETERMINISM CONTRACT note on the `RevisionHashService` class summary —
+`System.Text.Json` serializes properties in CLR declaration order and does not
+sort, so stable hashes rely on the private `Hashable*` records declaring their
+properties alphabetically (collections are explicitly sorted by `CanonicalName`).
+That manual ordering is now guarded by a regression test:
+`RevisionHashServiceTests.HashableRecords_PropertiesDeclaredAlphabetically`
+(reflects over the nested `Hashable*` types and asserts ordinal-alphabetical
+property declaration order), so adding a property out of order now fails the build's
+test gate instead of silently changing every revision hash.
 
 ### TemplateEngine-012 — `DataType` enum naming diverges from the design doc
 
@@ -505,7 +516,7 @@ _Unresolved._
 |--|--|
 | Severity | Low |
 | Category | Design-document adherence |
-| Status | Open |
+| Status | Deferred |
 | Location | `src/ScadaLink.TemplateEngine/Validation/SemanticValidator.cs:18` |
 
 **Description**
@@ -523,9 +534,30 @@ are numeric.
 Update `docs/requirements/Component-TemplateEngine.md` to list the actual enum members,
 or rename the enum to match the doc if "Integer" is the intended canonical name.
 
+**Re-triage**
+
+Verified against the source: the `DataType` enum is declared in
+`src/ScadaLink.Commons/Types/Enums/DataType.cs` (`Boolean, Int32, Float, Double,
+String, DateTime, Binary`) — **not** in the TemplateEngine module — and is consumed
+across modules (`TemplateAttribute` entity, management command contracts). The only
+in-module file the finding cites, `SemanticValidator.cs:18`, is confirmed **correct**:
+`NumericDataTypes` already hard-codes the real enum names. Both remediation options
+in the recommendation therefore land **outside** this module's resolution boundary
+(`src/ScadaLink.TemplateEngine/**`): renaming the enum touches `ScadaLink.Commons`
+(and every consumer of `DataType`), and the alternative — updating the design doc —
+touches `docs/requirements/Component-TemplateEngine.md`. There is no in-module code
+defect to fix. Re-triaged from Open to Deferred: the fix is a one-line design-doc
+correction (list the actual seven enum members instead of "Boolean, Integer, Float,
+String") that must be made by an agent owning the docs / Commons scope.
+
 **Resolution**
 
-_Unresolved._
+Deferred 2026-05-16 (no commit): no in-module fix possible — see Re-triage. The
+TemplateEngine code is correct as-is. FLAGGED for the docs owner: correct the
+Attribute data-type list in `docs/requirements/Component-TemplateEngine.md` to match
+`ScadaLink.Commons` `DataType` (`Boolean, Int32, Float, Double, String, DateTime,
+Binary`). Renaming the enum is not recommended (cross-module churn for no behavioural
+gain); the doc is the authoritative thing to fix.
 
 ### TemplateEngine-013 — `ToDictionary(t => t.Id)` throws on duplicate IDs; cycle detectors overload Id 0 as a sentinel
 
@@ -533,7 +565,7 @@ _Unresolved._
 |--|--|
 | Severity | Low |
 | Category | Correctness & logic bugs |
-| Status | Open |
+| Status | Resolved |
 | Location | `src/ScadaLink.TemplateEngine/CycleDetector.cs:30`, `src/ScadaLink.TemplateEngine/CycleDetector.cs:38` |
 
 **Description**
@@ -556,7 +588,23 @@ special.
 
 **Resolution**
 
-_Unresolved._
+Resolved 2026-05-16 (commit `pending commit`): added `CycleDetector.BuildLookup`,
+a duplicate-tolerant Id-keyed lookup (`Dictionary` + `TryAdd`, first occurrence wins)
+that replaces every `allTemplates.ToDictionary(t => t.Id)` in the static helpers —
+`CycleDetector` (all three methods), `TemplateResolver.ResolveAllMembers`, and
+`CollisionDetector.DetectCollisions` — so a list containing two templates with the
+same `Id` (e.g. not-yet-saved templates carrying `Id 0`) no longer throws an
+unhandled `ArgumentException`. Separately, the `0`-as-"no-parent" sentinel was
+removed: `DetectInheritanceCycle` now walks the parent chain via the `int?`
+`ParentTemplateId` (`HasValue` gates continuation), and `DetectCrossGraphCycle`
+gates the proposed parent/composed edges on `HasValue` rather than `!= 0`, so a
+template with a real `Id` of 0 is treated like any other node and cycles through it
+are detected. Regression tests:
+`CycleDetectorTests.DetectInheritanceCycle_DuplicateIdsInList_DoesNotThrow`,
+`DetectCompositionCycle_DuplicateIdsInList_DoesNotThrow`,
+`DetectCrossGraphCycle_DuplicateIdsInList_DoesNotThrow`,
+`DetectInheritanceCycle_RealIdZero_StillDetectsCycle`,
+`DetectInheritanceCycle_ParentChainThroughIdZero_DetectsCycle`.
 
 ### TemplateEngine-014 — Template-deletion constraint logic is duplicated and divergent
 
@@ -564,7 +612,7 @@ _Unresolved._
 |--|--|
 | Severity | Low |
 | Category | Code organization & conventions |
-| Status | Open |
+| Status | Resolved |
 | Location | `src/ScadaLink.TemplateEngine/TemplateService.cs:109`, `src/ScadaLink.TemplateEngine/Services/TemplateDeletionService.cs:27` |
 
 **Description**
@@ -586,4 +634,17 @@ vice versa) so the constraint logic lives in exactly one place.
 
 **Resolution**
 
-_Unresolved._
+Resolved 2026-05-16 (commit `pending commit`): `TemplateService.DeleteTemplateAsync`
+no longer re-implements the deletion-constraint rules — it now delegates the
+constraint check and the delete to `TemplateDeletionService.DeleteTemplateAsync`
+(the surviving single implementation, which already accumulates every blocking
+reason rather than returning on the first failing category). `TemplateService`
+retains only its audit-logging side effect: after a successful delete it writes the
+`Delete` audit entry and calls `SaveChangesAsync` (the deletion service is unaware of
+auditing and persists the delete itself, so the audit entry needs its own save).
+The constraint logic now lives in exactly one place, so a future rule change cannot
+drift between two implementations. Behavioural change: `DeleteTemplateAsync` now
+reports all blocking reasons and uses `TemplateDeletionService`'s phrasing — the
+affected `TemplateServiceTests` delete tests were updated to the unified messages,
+and a regression test `DeleteTemplate_MultipleConstraints_ReportsAllNotJustFirst`
+verifies all three constraint categories are surfaced together.
diff --git a/src/ScadaLink.TemplateEngine/CollisionDetector.cs b/src/ScadaLink.TemplateEngine/CollisionDetector.cs
index d6bfb7c..6cf910e 100644
--- a/src/ScadaLink.TemplateEngine/CollisionDetector.cs
+++ b/src/ScadaLink.TemplateEngine/CollisionDetector.cs
@@ -27,7 +27,10 @@ public static class CollisionDetector
         Template template,
         IReadOnlyList<Template> allTemplates)
     {
-        var lookup = allTemplates.ToDictionary(t => t.Id);
+        // Duplicate-tolerant lookup (see CycleDetector.BuildLookup): a plain
+        // ToDictionary(t => t.Id) throws if two templates share an Id (e.g.
+        // not-yet-saved templates carrying Id 0).
+        var lookup = CycleDetector.BuildLookup(allTemplates);
         var allMembers = new List<ResolvedMember>();
 
         // Collect direct (top-level) members
diff --git a/src/ScadaLink.TemplateEngine/CycleDetector.cs b/src/ScadaLink.TemplateEngine/CycleDetector.cs
index 8f351cd..92b3011 100644
--- a/src/ScadaLink.TemplateEngine/CycleDetector.cs
+++ b/src/ScadaLink.TemplateEngine/CycleDetector.cs
@@ -9,6 +9,21 @@ namespace ScadaLink.TemplateEngine;
 /// </summary>
 public static class CycleDetector
 {
+    /// <summary>
+    /// Builds an Id-keyed lookup that tolerates duplicate Ids in the input list
+    /// (e.g. multiple not-yet-saved templates all carrying Id 0). On a duplicate
+    /// the first occurrence wins — graph walks only need one representative node
+    /// per Id, and a real cycle through any duplicate would still be reachable.
+    /// A plain <c>ToDictionary(t =&gt; t.Id)</c> would instead throw ArgumentException.
+    /// </summary>
+    internal static Dictionary<int, Template> BuildLookup(IReadOnlyList<Template> allTemplates)
+    {
+        var lookup = new Dictionary<int, Template>();
+        foreach (var t in allTemplates)
+            lookup.TryAdd(t.Id, t);
+        return lookup;
+    }
+
     /// <summary>
     /// Checks whether setting <paramref name="parentId"/> as the parent of template
     /// <paramref name="templateId"/> would introduce an inheritance cycle.
@@ -27,28 +42,30 @@ public static class CycleDetector
 
         // Walk the inheritance chain from the proposed parent upward.
         // If we arrive back at templateId, there is a cycle.
-        var lookup = allTemplates.ToDictionary(t => t.Id);
+        var lookup = BuildLookup(allTemplates);
         var visited = new HashSet<int> { templateId };
         var chain = new List<string>();
 
         var templateName = lookup.TryGetValue(templateId, out var tmpl) ? tmpl.Name : templateId.ToString();
         chain.Add(templateName);
 
-        var currentId = parentId;
-        while (currentId != 0)
+        // ParentTemplateId is int? — a missing value (not 0) means "no parent",
+        // so a template with a real Id of 0 is walked like any other node.
+        int? currentId = parentId;
+        while (currentId.HasValue)
         {
-            if (!lookup.TryGetValue(currentId, out var current))
+            if (!lookup.TryGetValue(currentId.Value, out var current))
                 break;
 
             chain.Add(current.Name);
 
-            if (visited.Contains(currentId))
+            if (visited.Contains(currentId.Value))
             {
                 return $"Inheritance cycle detected: {string.Join(" -> ", chain)}.";
             }
 
-            visited.Add(currentId);
-            currentId = current.ParentTemplateId ?? 0;
+            visited.Add(currentId.Value);
+            currentId = current.ParentTemplateId;
         }
 
         return null;
@@ -70,7 +87,7 @@ public static class CycleDetector
             return $"Template '{selfName}' cannot compose itself.";
         }
 
-        var lookup = allTemplates.ToDictionary(t => t.Id);
+        var lookup = BuildLookup(allTemplates);
 
         // BFS/DFS from composedTemplateId through all its compositions.
         // If we reach templateId, that's a cycle.
@@ -115,7 +132,7 @@ public static class CycleDetector
         int? proposedComposedTemplateId,
         IReadOnlyList<Template> allTemplates)
     {
-        var lookup = allTemplates.ToDictionary(t => t.Id);
+        var lookup = BuildLookup(allTemplates);
 
         // Build adjacency: for each template, collect all reachable templates
         // via inheritance (parent) and composition edges.
@@ -124,11 +141,12 @@ public static class CycleDetector
         var visited = new HashSet<int>();
         var queue = new Queue<int>();
 
-        // Seed with proposed targets
-        if (proposedParentId.HasValue && proposedParentId.Value != 0)
+        // Seed with proposed targets. A null proposed id means "no edge"; a value
+        // of 0 is a legitimate Id, so only HasValue gates enqueuing.
+        if (proposedParentId.HasValue)
             queue.Enqueue(proposedParentId.Value);
 
-        if (proposedComposedTemplateId.HasValue && proposedComposedTemplateId.Value != 0)
+        if (proposedComposedTemplateId.HasValue)
             queue.Enqueue(proposedComposedTemplateId.Value);
 
         while (queue.Count > 0)
@@ -146,8 +164,8 @@ public static class CycleDetector
             if (!lookup.TryGetValue(currentId, out var current))
                 continue;
 
-            // Follow inheritance edge
-            if (current.ParentTemplateId.HasValue && current.ParentTemplateId.Value != 0)
+            // Follow inheritance edge (int? — missing value means no parent)
+            if (current.ParentTemplateId.HasValue)
                 queue.Enqueue(current.ParentTemplateId.Value);
 
             // Follow composition edges
diff --git a/src/ScadaLink.TemplateEngine/Flattening/RevisionHashService.cs b/src/ScadaLink.TemplateEngine/Flattening/RevisionHashService.cs
index 0143b85..3e37116 100644
--- a/src/ScadaLink.TemplateEngine/Flattening/RevisionHashService.cs
+++ b/src/ScadaLink.TemplateEngine/Flattening/RevisionHashService.cs
@@ -8,19 +8,24 @@ namespace ScadaLink.TemplateEngine.Flattening;
 
 /// <summary>
 /// Produces a deterministic SHA-256 hash of a FlattenedConfiguration.
-/// Same content always produces the same hash across runs by using
-/// canonical JSON serialization with sorted keys.
+/// Same content always produces the same hash across runs.
+/// <para>
+/// DETERMINISM CONTRACT: System.Text.Json serializes properties in CLR
+/// declaration order, which it does NOT sort. Stable output therefore relies
+/// on the private <c>Hashable*</c> records below declaring their properties
+/// in alphabetical order. A contributor adding a property out of alphabetical
+/// order would silently change every revision hash; the ordering is guarded
+/// by <c>RevisionHashServiceTests.HashableRecords_PropertiesDeclaredAlphabetically</c>.
+/// Collections are explicitly sorted by <c>CanonicalName</c> before hashing.
+/// </para>
 /// </summary>
 public class RevisionHashService
 {
     private static readonly JsonSerializerOptions CanonicalJsonOptions = new()
     {
-        // Sort properties alphabetically for determinism
         PropertyNamingPolicy = JsonNamingPolicy.CamelCase,
         DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull,
-        WriteIndented = false,
-        // Ensure consistent ordering
-        Converters = { new SortedPropertiesConverterFactory() }
+        WriteIndented = false
     };
 
     /// <summary>
@@ -84,7 +89,9 @@ public class RevisionHashService
         return $"sha256:{Convert.ToHexStringLower(hashBytes)}";
     }
 
-    // Internal types for deterministic serialization (sorted property names via camelCase + alphabetical)
+    // Internal types for deterministic serialization. Properties MUST be declared
+    // in alphabetical order — System.Text.Json emits them in declaration order and
+    // does not sort. See the DETERMINISM CONTRACT note on the class summary.
     private sealed record HashableConfiguration
     {
         public List<HashableAlarm> Alarms { get; init; } = [];
@@ -128,14 +135,3 @@ public class RevisionHashService
         public string? TriggerType { get; init; }
     }
 }
-
-/// <summary>
-/// A JSON converter factory that ensures properties are serialized in alphabetical order
-/// for deterministic output. Works with record types.
-/// </summary>
-internal class SortedPropertiesConverterFactory : JsonConverterFactory
-{
-    public override bool CanConvert(Type typeToConvert) => false;
-
-    public override JsonConverter? CreateConverter(Type typeToConvert, JsonSerializerOptions options) => null;
-}
diff --git a/src/ScadaLink.TemplateEngine/TemplateResolver.cs b/src/ScadaLink.TemplateEngine/TemplateResolver.cs
index ae9ec29..7eee29f 100644
--- a/src/ScadaLink.TemplateEngine/TemplateResolver.cs
+++ b/src/ScadaLink.TemplateEngine/TemplateResolver.cs
@@ -39,7 +39,10 @@ public static class TemplateResolver
         int templateId,
         IReadOnlyList<Template> allTemplates)
     {
-        var lookup = allTemplates.ToDictionary(t => t.Id);
+        // Duplicate-tolerant lookup (see CycleDetector.BuildLookup): a plain
+        // ToDictionary(t => t.Id) throws if two templates share an Id (e.g.
+        // not-yet-saved templates carrying Id 0).
+        var lookup = CycleDetector.BuildLookup(allTemplates);
         if (!lookup.TryGetValue(templateId, out var template))
             return Array.Empty<ResolvedTemplateMember>();
 
diff --git a/src/ScadaLink.TemplateEngine/TemplateService.cs b/src/ScadaLink.TemplateEngine/TemplateService.cs
index e954b5d..11d621e 100644
--- a/src/ScadaLink.TemplateEngine/TemplateService.cs
+++ b/src/ScadaLink.TemplateEngine/TemplateService.cs
@@ -112,59 +112,18 @@ public class TemplateService
         if (template == null)
             return Result<bool>.Failure($"Template with ID {templateId} not found.");
 
-        // Derived templates are owned by their composition row and must be removed
-        // by deleting the composition (which cascades) — block direct deletion.
-        if (template.IsDerived)
-            return Result<bool>.Failure(
-                $"Cannot delete template '{template.Name}': it is a derived template. " +
-                "Remove the owning composition on its parent template instead.");
+        // Deletion-constraint logic (instances / child / derived / composing
+        // templates) lives in exactly one place — TemplateDeletionService — so a
+        // future rule change cannot drift between two implementations
+        // (TemplateEngine-014). TemplateService owns only the audit-logging side
+        // effect, which the deletion service is unaware of.
+        var deletionService = new Services.TemplateDeletionService(_repository);
+        var deleteResult = await deletionService.DeleteTemplateAsync(templateId, cancellationToken);
+        if (deleteResult.IsFailure)
+            return deleteResult;
 
-        // Check for instances referencing this template
-        var instances = await _repository.GetInstancesByTemplateIdAsync(templateId, cancellationToken);
-        if (instances.Count > 0)
-            return Result<bool>.Failure(
-                $"Cannot delete template '{template.Name}': it is referenced by {instances.Count} instance(s).");
-
-        // Check for child templates inheriting from this template.
-        // Split derived vs. regular children — the message and remediation differ.
-        var allTemplates = await _repository.GetAllTemplatesAsync(cancellationToken);
-        var inheritors = allTemplates.Where(t => t.ParentTemplateId == templateId).ToList();
-        var derivatives = inheritors.Where(t => t.IsDerived).ToList();
-        var regularChildren = inheritors.Where(t => !t.IsDerived).ToList();
-
-        if (regularChildren.Count > 0)
-            return Result<bool>.Failure(
-                $"Cannot delete template '{template.Name}': it is inherited by {regularChildren.Count} child template(s): " +
-                string.Join(", ", regularChildren.Select(c => $"'{c.Name}'")));
-
-        if (derivatives.Count > 0)
-        {
-            // Name each derivative by its owning parent template + composition slot.
-            var ownerCompIds = derivatives.Select(d => d.OwnerCompositionId).Where(id => id.HasValue).Select(id => id!.Value).ToHashSet();
-            var ownerLookup = allTemplates
-                .SelectMany(t => t.Compositions.Select(c => new { Owner = t, Composition = c }))
-                .Where(x => ownerCompIds.Contains(x.Composition.Id))
-                .ToDictionary(x => x.Composition.Id, x => $"'{x.Owner.Name}' (as '{x.Composition.InstanceName}')");
-
-            var details = derivatives
-                .Select(d => d.OwnerCompositionId.HasValue && ownerLookup.TryGetValue(d.OwnerCompositionId.Value, out var label)
-                    ? label
-                    : $"'{d.Name}'");
-            return Result<bool>.Failure(
-                $"Cannot delete template '{template.Name}': it is the base of {derivatives.Count} derived template(s) used in: " +
-                string.Join(", ", details) + ". Remove those compositions first.");
-        }
-
-        // Check for templates composing this template
-        var composedBy = allTemplates
-            .Where(t => t.Compositions.Any(c => c.ComposedTemplateId == templateId))
-            .ToList();
-        if (composedBy.Count > 0)
-            return Result<bool>.Failure(
-                $"Cannot delete template '{template.Name}': it is composed by {composedBy.Count} template(s): " +
-                string.Join(", ", composedBy.Select(c => $"'{c.Name}'")));
-
-        await _repository.DeleteTemplateAsync(templateId, cancellationToken);
+        // TemplateDeletionService already persisted the delete; the audit entry
+        // is added to the change tracker here and needs its own SaveChangesAsync.
         await _auditService.LogAsync(user, "Delete", "Template", templateId.ToString(), template.Name, null, cancellationToken);
         await _repository.SaveChangesAsync(cancellationToken);
 
diff --git a/tests/ScadaLink.TemplateEngine.Tests/CycleDetectorTests.cs b/tests/ScadaLink.TemplateEngine.Tests/CycleDetectorTests.cs
index 4032010..c49d701 100644
--- a/tests/ScadaLink.TemplateEngine.Tests/CycleDetectorTests.cs
+++ b/tests/ScadaLink.TemplateEngine.Tests/CycleDetectorTests.cs
@@ -153,4 +153,77 @@ public class CycleDetectorTests
 
         Assert.Null(result);
     }
+
+    // ========================================================================
+    // TemplateEngine-013: robustness against duplicate Ids and Id 0
+    // ========================================================================
+
+    [Fact]
+    public void DetectInheritanceCycle_DuplicateIdsInList_DoesNotThrow()
+    {
+        // Two not-yet-saved templates both carry Id == 0. ToDictionary(t => t.Id)
+        // would throw ArgumentException; the detector must tolerate it.
+        var templateA = new Template("A") { Id = 0 };
+        var templateB = new Template("B") { Id = 0 };
+        var saved = new Template("Saved") { Id = 1 };
+        var all = new List<Template> { templateA, templateB, saved };
+
+        var ex = Record.Exception(() => CycleDetector.DetectInheritanceCycle(1, 0, all));
+
+        Assert.Null(ex);
+    }
+
+    [Fact]
+    public void DetectCompositionCycle_DuplicateIdsInList_DoesNotThrow()
+    {
+        var templateA = new Template("A") { Id = 0 };
+        var templateB = new Template("B") { Id = 0 };
+        var all = new List<Template> { templateA, templateB };
+
+        var ex = Record.Exception(() => CycleDetector.DetectCompositionCycle(1, 2, all));
+
+        Assert.Null(ex);
+    }
+
+    [Fact]
+    public void DetectCrossGraphCycle_DuplicateIdsInList_DoesNotThrow()
+    {
+        var templateA = new Template("A") { Id = 0 };
+        var templateB = new Template("B") { Id = 0 };
+        var all = new List<Template> { templateA, templateB };
+
+        var ex = Record.Exception(() => CycleDetector.DetectCrossGraphCycle(5, 1, 2, all));
+
+        Assert.Null(ex);
+    }
+
+    [Fact]
+    public void DetectInheritanceCycle_RealIdZero_StillDetectsCycle()
+    {
+        // A template legitimately stored with Id 0 (in-memory / test scenario):
+        // a self-inheritance attempt must still be detected, not skipped as
+        // "no parent" by a 0-as-sentinel overload.
+        var template = new Template("Zero") { Id = 0 };
+        var all = new List<Template> { template };
+
+        var result = CycleDetector.DetectInheritanceCycle(0, 0, all);
+
+        Assert.NotNull(result);
+        Assert.Contains("itself", result);
+    }
+
+    [Fact]
+    public void DetectInheritanceCycle_ParentChainThroughIdZero_DetectsCycle()
+    {
+        // Child(1) -> parent Zero(0) -> parent Child(1): a cycle running through
+        // a template whose real Id is 0 must be detected, not silently skipped.
+        var zero = new Template("Zero") { Id = 0, ParentTemplateId = 1 };
+        var child = new Template("Child") { Id = 1, ParentTemplateId = null };
+        var all = new List<Template> { zero, child };
+
+        var result = CycleDetector.DetectInheritanceCycle(1, 0, all);
+
+        Assert.NotNull(result);
+        Assert.Contains("cycle", result, StringComparison.OrdinalIgnoreCase);
+    }
 }
diff --git a/tests/ScadaLink.TemplateEngine.Tests/Flattening/RevisionHashServiceTests.cs b/tests/ScadaLink.TemplateEngine.Tests/Flattening/RevisionHashServiceTests.cs
index 4da3523..5ab851e 100644
--- a/tests/ScadaLink.TemplateEngine.Tests/Flattening/RevisionHashServiceTests.cs
+++ b/tests/ScadaLink.TemplateEngine.Tests/Flattening/RevisionHashServiceTests.cs
@@ -99,6 +99,38 @@ public class RevisionHashServiceTests
         Assert.Equal(_sut.ComputeHash(config1), _sut.ComputeHash(config2));
     }
 
+    [Fact]
+    public void HashableRecords_PropertiesDeclaredAlphabetically()
+    {
+        // TemplateEngine-011: revision-hash determinism depends entirely on the
+        // private Hashable* records declaring their properties in alphabetical
+        // order (System.Text.Json emits properties in CLR declaration order and
+        // does not sort). This guards against a contributor silently changing
+        // every revision hash by adding a property out of order.
+        var nested = typeof(RevisionHashService)
+            .GetNestedTypes(System.Reflection.BindingFlags.NonPublic)
+            .Where(t => t.Name.StartsWith("Hashable"))
+            .ToList();
+
+        Assert.NotEmpty(nested);
+
+        foreach (var type in nested)
+        {
+            var propNames = type
+                .GetProperties(System.Reflection.BindingFlags.Public | System.Reflection.BindingFlags.Instance)
+                .Where(p => p.Name != "EqualityContract")
+                .Select(p => p.Name)
+                .ToList();
+
+            var sorted = propNames.OrderBy(n => n, StringComparer.Ordinal).ToList();
+
+            Assert.True(
+                propNames.SequenceEqual(sorted),
+                $"{type.Name} properties must be declared alphabetically. " +
+                $"Declared: [{string.Join(", ", propNames)}] Expected: [{string.Join(", ", sorted)}]");
+        }
+    }
+
     private static FlattenedConfiguration CreateConfig(string instanceName, string tempValue)
     {
         return new FlattenedConfiguration
diff --git a/tests/ScadaLink.TemplateEngine.Tests/TemplateServiceTests.cs b/tests/ScadaLink.TemplateEngine.Tests/TemplateServiceTests.cs
index 2bc5adf..c556050 100644
--- a/tests/ScadaLink.TemplateEngine.Tests/TemplateServiceTests.cs
+++ b/tests/ScadaLink.TemplateEngine.Tests/TemplateServiceTests.cs
@@ -122,11 +122,13 @@ public class TemplateServiceTests
         _repoMock.Setup(r => r.GetTemplateByIdAsync(1, It.IsAny<CancellationToken>())).ReturnsAsync(template);
         _repoMock.Setup(r => r.GetInstancesByTemplateIdAsync(1, It.IsAny<CancellationToken>()))
             .ReturnsAsync(new List<Instance> { new Instance("Pump1") { Id = 1, TemplateId = 1, SiteId = 1 } });
+        _repoMock.Setup(r => r.GetAllTemplatesAsync(It.IsAny<CancellationToken>()))
+            .ReturnsAsync(new List<Template> { template });
 
         var result = await _service.DeleteTemplateAsync(1, "admin");
 
         Assert.True(result.IsFailure);
-        Assert.Contains("referenced by", result.Error);
+        Assert.Contains("instance(s) reference it", result.Error);
     }
 
     [Fact]
@@ -143,7 +145,7 @@ public class TemplateServiceTests
         var result = await _service.DeleteTemplateAsync(1, "admin");
 
         Assert.True(result.IsFailure);
-        Assert.Contains("inherited by", result.Error);
+        Assert.Contains("child template(s) inherit from it", result.Error);
     }
 
     [Fact]
@@ -162,7 +164,36 @@ public class TemplateServiceTests
         var result = await _service.DeleteTemplateAsync(1, "admin");
 
         Assert.True(result.IsFailure);
-        Assert.Contains("composed by", result.Error);
+        Assert.Contains("template(s) compose it", result.Error);
+    }
+
+    [Fact]
+    public async Task DeleteTemplate_MultipleConstraints_ReportsAllNotJustFirst()
+    {
+        // TemplateEngine-014: DeleteTemplateAsync delegates its constraint check
+        // to the single TemplateDeletionService implementation, which accumulates
+        // every blocking reason instead of returning on the first failing category.
+        var template = new Template("Busy") { Id = 1 };
+        var composer = new Template("Composer") { Id = 3 };
+        composer.Compositions.Add(new TemplateComposition("Module") { Id = 1, TemplateId = 3, ComposedTemplateId = 1 });
+
+        _repoMock.Setup(r => r.GetTemplateByIdAsync(1, It.IsAny<CancellationToken>())).ReturnsAsync(template);
+        _repoMock.Setup(r => r.GetInstancesByTemplateIdAsync(1, It.IsAny<CancellationToken>()))
+            .ReturnsAsync(new List<Instance> { new Instance("Inst1") { Id = 1, TemplateId = 1, SiteId = 1 } });
+        _repoMock.Setup(r => r.GetAllTemplatesAsync(It.IsAny<CancellationToken>()))
+            .ReturnsAsync(new List<Template>
+            {
+                template,
+                new Template("Child") { Id = 2, ParentTemplateId = 1 },
+                composer
+            });
+
+        var result = await _service.DeleteTemplateAsync(1, "admin");
+
+        Assert.True(result.IsFailure);
+        Assert.Contains("instance(s) reference it", result.Error);
+        Assert.Contains("child template(s) inherit from it", result.Error);
+        Assert.Contains("template(s) compose it", result.Error);
     }
 
     // ========================================================================