fix(cluster-infrastructure): resolve ClusterInfrastructure-005,007,008 — confirm config-section constant, XML docs, phase-status cleanup

code-reviews/ClusterInfrastructure/findings.md

@@ -8,7 +8,7 @@
 | Last reviewed | 2026-05-16 |
 | Reviewer | claude-agent |
 | Commit reviewed | `9c60592` |
-| Open findings | 3 |
+| Open findings | 0 |
 
 ## Summary
 
@@ -302,7 +302,7 @@ attributes cannot. Covered by `ClusterOptionsValidatorTests` (8 cases) and
 |--|--|
 | Severity | Low |
 | Category | Code organization & conventions |
-| Status | Open |
+| Status | Resolved |
 | Location | `src/ScadaLink.ClusterInfrastructure/ClusterOptions.cs:3` |
 
 **Description**
@@ -323,7 +323,17 @@ Add a `public const string SectionName = "Cluster";` (or the agreed name) to
 
 **Resolution**
 
-_Unresolved._
+Confirmed against the source: `ClusterOptions` previously exposed no section-name
+constant, leaving binding sites to hard-code the magic string.
+
+**Resolved** — fixing commit `commit pending`, date 2026-05-16. `ClusterOptions` now
+exposes `public const string SectionName = "ScadaLink:Cluster";` as the single source
+of truth for the `appsettings.json` section name, with an XML doc explaining its
+purpose. The chosen value matches the `ScadaLink:`-prefixed section convention used by
+peer option classes and referenced by `ClusterOptionsValidator` / the design doc.
+Covered by `ClusterOptionsTests.SectionName_IsTheExpectedAppSettingsSection`, which
+both pins the value and — by referencing the constant — guards against its removal
+(its deletion would break compilation).
 
 ### ClusterInfrastructure-006 — No tests for any cluster behaviour; only the options POCO is covered
 
@@ -385,7 +395,7 @@ design); `ClusterOptionsValidator` is the layer that now rejects `keep-majority`
 |--|--|
 | Severity | Low |
 | Category | Documentation & comments |
-| Status | Open |
+| Status | Resolved |
 | Location | `src/ScadaLink.ClusterInfrastructure/ClusterOptions.cs:3-11` |
 
 **Description**
@@ -407,7 +417,20 @@ the relevant design-doc sections as peer modules do.
 
 **Resolution**
 
-_Unresolved._
+Confirmed against the source: `ClusterOptions` previously had no XML doc comments on
+the class or any property.
+
+**Resolved** — fixing commit `commit pending`, date 2026-05-16. `ClusterOptions` now
+carries a class-level `<summary>` documenting the cluster configuration contract and
+the deliberate ownership split (node identity / remoting / gRPC in
+`Host.NodeOptions`, storage paths in the database options, cluster-formation settings
+here), plus `<summary>` comments on all seven properties — each stating units,
+defaults, and the design-doc constraints. Notably `MinNrOfMembers` is documented as
+required to be `1` (a value of `2` blocks the cluster singleton after failover) and
+`HeartbeatInterval` as required to be well below `FailureDetectionThreshold`, giving a
+future editor the in-code warnings the recommendation asks for. This is a
+documentation-only change, so no runtime regression test is meaningful; verified by
+inspection of `ClusterOptions.cs:3-74`. Module test suite green (17 passed).
 
 ### ClusterInfrastructure-008 — "Phase 0 skeleton" status is undocumented at the module level
 
@@ -415,7 +438,7 @@ _Unresolved._
 |--|--|
 | Severity | Low |
 | Category | Documentation & comments |
-| Status | Open |
+| Status | Resolved |
 | Location | `src/ScadaLink.ClusterInfrastructure/ServiceCollectionExtensions.cs:9`, `src/ScadaLink.ClusterInfrastructure/ServiceCollectionExtensions.cs:16` |
 
 **Description**
@@ -439,4 +462,31 @@ about which components are skeletons versus implemented.
 
 **Resolution**
 
-_Unresolved._
+Re-triaged in light of CI-001's resolution. The original defect was the mismatch
+between two misleading inline `// Phase 0: skeleton only` /
+`// Phase 0: placeholder` comments buried in private method bodies and a
+complete-looking design doc with no caveat. That premise has been overtaken by the
+CI-001/CI-002 work:
+
+- The "Phase 0 skeleton" comments no longer exist anywhere in
+  `src/ScadaLink.ClusterInfrastructure` (verified by `grep`). `ServiceCollectionExtensions`
+  now does real work (registers `ClusterOptionsValidator`) and `AddClusterInfrastructureActors`
+  throws explicitly — both with accurate XML docs explaining the ownership split.
+- The module is no longer an unimplemented skeleton. CI-001 established that the Akka
+  bootstrap legitimately lives in `ScadaLink.Host`, and this project's true scope —
+  the `ClusterOptions` configuration contract, its validator, and DI registration — is
+  fully implemented and tested.
+- The design doc `Component-ClusterInfrastructure.md` now opens with an
+  "Implementation Note — Code Placement" section (added by CI-001) that explicitly
+  states the component is a *design responsibility* realised across
+  `ScadaLink.ClusterInfrastructure` (configuration model) and `ScadaLink.Host`
+  (bootstrap/runtime wiring), and the README component table (row 13) was updated to
+  match. A reader of the design doc no longer assumes a single fully-built project.
+
+**Resolved** — fixing commit `commit pending`, date 2026-05-16. No further change
+required: the misleading skeleton comments are gone, the module's real status is
+documented at the design-doc level via the "Implementation Note — Code Placement"
+section, and the component table reflects the true placement. This is a
+documentation-only finding, so no runtime regression test is meaningful; verified by
+inspection of `ServiceCollectionExtensions.cs` and
+`docs/requirements/Component-ClusterInfrastructure.md:21-39`.

tests/ScadaLink.ClusterInfrastructure.Tests/ClusterOptionsTests.cs

@@ -35,6 +35,14 @@ public class ClusterOptionsTests
         Assert.Empty(options.SeedNodes);
     }
 
+    [Fact]
+    public void SectionName_IsTheExpectedAppSettingsSection()
+    {
+        // CI-005: ClusterOptions must expose a single-source-of-truth constant for
+        // its appsettings.json section so binding sites do not hard-code the string.
+        Assert.Equal("ScadaLink:Cluster", ClusterOptions.SectionName);
+    }
+
     [Fact]
     public void Properties_CanBeSetToCustomValues()
     {