feat(host): warn when Cluster:Roles and OTOPCUA_ROLES disagree; boot banner names the winning source

archreview/plans/R3-04-one-role-authority-plan.md.tasks.json

@@ -43,7 +43,7 @@
     {
       "id": "T6",
       "subject": "Warning when Cluster:Roles and OTOPCUA_ROLES disagree; boot banner names the winning source",
-      "status": "pending",
+      "status": "completed",
       "blockedBy": [
         "T3"
       ]

src/Core/ZB.MOM.WW.OtOpcUa.Cluster/EffectiveRoles.cs

@@ -38,10 +38,11 @@ public enum RoleSource
 /// </remarks>
 public sealed class EffectiveRoles
 {
-    private EffectiveRoles(IReadOnlyList<string> gossiped, RoleSource source)
+    private EffectiveRoles(IReadOnlyList<string> gossiped, RoleSource source, string? disagreementWarning)
     {
         Gossiped = gossiped;
         Source = source;
+        DisagreementWarning = disagreementWarning;
         ClusterRole = RoleParser.ClusterRoleFromRoles(gossiped);
         ClusterId = ClusterRole is null ? null : RoleParser.ClusterIdFromRole(ClusterRole);
     }
@@ -79,6 +80,13 @@ public sealed class EffectiveRoles
     /// for a legacy / single-mesh node.</summary>
     public string? ClusterId { get; }
 
+    /// <summary>Non-null when BOTH <c>Cluster:Roles</c> and <c>OTOPCUA_ROLES</c> were set and their
+    /// normalized role sets differ — the <c>Cluster:Roles</c>-wins case an operator almost certainly
+    /// did not intend (choice 5). Observability only: <see cref="Gossiped"/> still resolves from
+    /// <c>Cluster:Roles</c> exactly as usual. Returned rather than logged from this static factory —
+    /// the caller (<c>Program.cs</c>) logs it once Serilog is configured.</summary>
+    public string? DisagreementWarning { get; }
+
     /// <summary>The one factory: resolves the effective roles off <paramref name="configuration"/>'s
     /// <c>Cluster:Roles</c> and the process's <c>OTOPCUA_ROLES</c> environment variable.</summary>
     /// <param name="configuration">The application configuration.</param>
@@ -101,20 +109,42 @@ public sealed class EffectiveRoles
     /// <exception cref="ArgumentException">Either source names an unknown role.</exception>
     public static EffectiveRoles Resolve(IReadOnlyList<string>? configured, string? envRoles)
     {
-        if (configured is { Count: > 0 })
+        var configuredNormalized = configured is { Count: > 0 }
-        {
+            ? NormalizeAndValidateConfigured(configured)
-            return new EffectiveRoles(NormalizeAndValidateConfigured(configured), RoleSource.Configured);
+            : null;
-        }
 
-        if (!string.IsNullOrWhiteSpace(envRoles))
+        // RoleParser.Parse already trims, lower-cases (fully — including a cluster suffix, which is
-        {
+        // intentionally different from the configured path; see ClusterIdFromRole's remarks), dedupes
-            // RoleParser.Parse already trims, lower-cases (fully — including a cluster suffix, which
+        // and validates.
-            // is intentionally different from the configured path; see ClusterIdFromRole's remarks),
+        var envNormalized = !string.IsNullOrWhiteSpace(envRoles)
-            // dedupes and validates.
+            ? RoleParser.Parse(envRoles)
-            return new EffectiveRoles(RoleParser.Parse(envRoles), RoleSource.Environment);
+            : null;
-        }
 
-        return new EffectiveRoles(Array.Empty<string>(), RoleSource.None);
+        var warning = BuildDisagreementWarning(configuredNormalized, envNormalized);
+
+        if (configuredNormalized is not null)
+            return new EffectiveRoles(configuredNormalized, RoleSource.Configured, warning);
+
+        if (envNormalized is not null)
+            return new EffectiveRoles(envNormalized, RoleSource.Environment, warning);
+
+        return new EffectiveRoles(Array.Empty<string>(), RoleSource.None, null);
+    }
+
+    /// <summary>Choice 5: a Warning (not an error) when both sources are set and their normalized
+    /// role sets differ — this cannot fire when only one source is set, so it is always <see
+    /// langword="null"/> in that case regardless of the other source's absence.</summary>
+    private static string? BuildDisagreementWarning(string[]? configuredNormalized, string[]? envNormalized)
+    {
+        if (configuredNormalized is null || envNormalized is null) return null;
+
+        var configuredSet = new HashSet<string>(configuredNormalized, StringComparer.OrdinalIgnoreCase);
+        var envSet = new HashSet<string>(envNormalized, StringComparer.OrdinalIgnoreCase);
+        if (configuredSet.SetEquals(envSet)) return null;
+
+        return $"{ConfiguredRolesKey} ([{string.Join(",", configuredNormalized)}]) and "
+            + $"{RoleParser.RolesEnvVar} ([{string.Join(",", envNormalized)}]) disagree; "
+            + $"{ConfiguredRolesKey} wins outright.";
     }
 
     /// <summary>Normalizes and validates the <c>Cluster:Roles</c> entries: a plain role is trimmed and

src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs

@@ -796,6 +796,13 @@ Log.Information(
     "OtOpcUa.Host starting with roles=[{Roles}] (admin={HasAdmin}, driver={HasDriver}) source={Source}",
     string.Join(",", roles), hasAdmin, hasDriver, effectiveRoles.Source);
 
+// R3-04 choice 5: Cluster:Roles and OTOPCUA_ROLES set to different normalized role sets is almost
+// certainly not what the operator intended (Cluster:Roles still wins outright — this is
+// observability, not a behaviour change). Logged here rather than inside the static factory because
+// Serilog is not configured yet when EffectiveRoles.Resolve runs at the top of this file.
+if (effectiveRoles.DisagreementWarning is not null)
+    Log.Warning("{Warning}", effectiveRoles.DisagreementWarning);
+
 // #561 — startup growth self-check: the banner above went through the live pipeline; verify each
 // configured file sink physically grew. A sink whose writes silently fail (the wonder-app-vd03
 // 0-byte-log incident) is reported at boot — through SelfLog + stderr, which don't depend on the

tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/EffectiveRolesTests.cs

@@ -210,4 +210,45 @@ public sealed class EffectiveRolesTests
             roles.Gossiped.ShouldBe(new[] { "admin", "driver" });
         }
     }
+
+    // ---- DisagreementWarning (choice 5, T6) ----------------------------------------------------
+
+    [Fact]
+    public void Both_sources_set_and_agreeing_has_no_warning()
+    {
+        var roles = EffectiveRoles.Resolve(["driver"], "driver");
+
+        roles.DisagreementWarning.ShouldBeNull();
+    }
+
+    [Fact]
+    public void Only_one_source_set_has_no_warning()
+    {
+        EffectiveRoles.Resolve(["driver"], null).DisagreementWarning.ShouldBeNull();
+        EffectiveRoles.Resolve(null, "driver").DisagreementWarning.ShouldBeNull();
+        EffectiveRoles.Resolve(null, null).DisagreementWarning.ShouldBeNull();
+    }
+
+    [Fact]
+    public void Both_sources_set_and_disagreeing_produces_a_warning_naming_both_keys()
+    {
+        var roles = EffectiveRoles.Resolve(["admin"], "driver,cluster-SITE-A");
+
+        roles.DisagreementWarning.ShouldNotBeNull();
+        roles.DisagreementWarning.ShouldContain("Cluster:Roles");
+        roles.DisagreementWarning.ShouldContain(RoleParser.RolesEnvVar);
+        // Configured still wins outright — the warning is observability, not a behaviour change.
+        roles.Gossiped.ShouldBe(new[] { "admin" });
+        roles.Source.ShouldBe(RoleSource.Configured);
+    }
+
+    [Fact]
+    public void Both_sources_set_but_only_differing_by_case_do_not_warn()
+    {
+        // Same normalized set spelled differently in each source is NOT the "Cluster:Roles wins
+        // unintentionally" shape the warning exists for.
+        var roles = EffectiveRoles.Resolve(["Driver"], "driver");
+
+        roles.DisagreementWarning.ShouldBeNull();
+    }
 }