dohertj2/scadaproj
1
0
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(health): AkkaActiveNodeGate.ForOptions shares the probe's RolePreference scope; bump 0.3.1 (scadaproj#48)

Browse Source 
Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
This commit is contained in:
Joseph Doherty
2026-11-09 14:55:45 -05:00
parent 3ef43ca926
commit ab3fa9ca83
7 changed files with 90 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ZB.MOM.WW.Health/CLAUDE.md
+1 -1
View File
@@ -69,7 +69,7 @@ divergence actually occurred, so the assertions cannot pass for the wrong reason
## Status
Built at **0.3.0** and published to the Gitea NuGet feed. **Adopted** by all four apps — adoption is tracked in the component backlog:
Built at **0.3.1** (UNPUBLISHED; adds `AkkaActiveNodeGate.ForOptions` + `ClusterActiveNode.SelfIsActiveForPreference` so a gate can share the probe's `RolePreference` scope — Health-007, scadaproj#48); **0.3.0** is published to the Gitea NuGet feed. **Adopted** by all four apps — adoption is tracked in the component backlog:
- `~/Desktop/scadaproj/components/health/GAPS.md` — adoption order, effort, and risk
ZB.MOM.WW.Health/Directory.Build.props
+1 -1
View File
@@ -5,7 +5,7 @@
<Nullable>enable</Nullable>
<ImplicitUsings>enable</ImplicitUsings>
<LangVersion>latest</LangVersion>
<Version>0.3.0</Version>
<Version>0.3.1</Version>
<ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
<!-- Emit XML docs so the public API summaries ship inside the packed nupkgs (IntelliSense for
consumers). CS1591 (missing doc on a public member) is suppressed so undocumented test /
ZB.MOM.WW.Health/src/ZB.MOM.WW.Health.Akka/AkkaActiveNodeGate.cs
+28 -2
View File
@@ -14,7 +14,8 @@ namespace ZB.MOM.WW.Health.Akka;
/// Shares its rule with <see cref="ActiveNodeHealthCheck"/> via
/// <see cref="ClusterActiveNode"/>, so an endpoint gated by this type and the
/// <c>/health/active</c> probe an orchestrator routes by can never disagree about which node
/// is in charge.
/// is in charge — provided both are given the same scope: the same single role, both
/// unscoped, or (via <see cref="ForOptions"/>) the same <c>RolePreference</c> list.
/// </para>
/// <para>
/// <b>Behaviour change in 0.3.0:</b> through 0.2.1 this gate selected by
@@ -33,6 +34,7 @@ public sealed class AkkaActiveNodeGate : IActiveNodeGate
{
private readonly IServiceProvider _serviceProvider;
private readonly string? _role;
private readonly IReadOnlyList<string>? _rolePreference;
/// <summary>Initializes a new <see cref="AkkaActiveNodeGate"/>.</summary>
/// <param name="serviceProvider">
@@ -49,6 +51,27 @@ public sealed class AkkaActiveNodeGate : IActiveNodeGate
_role = role;
}
private AkkaActiveNodeGate(IServiceProvider serviceProvider, IReadOnlyList<string> rolePreference)
{
_serviceProvider = serviceProvider ?? throw new ArgumentNullException(nameof(serviceProvider));
_rolePreference = rolePreference;
}
/// <summary>
/// Creates a gate scoped by the same ordered <see cref="ActiveNodeHealthCheckOptions.RolePreference"/>
/// as <see cref="ActiveNodeHealthCheck"/>, so the gate and <c>/health/active</c> agree on every
/// node of a mixed-role mesh (a fused node answers for the first preferred role it carries).
/// A node carrying none of the preferred roles is never active.
/// </summary>
/// <param name="serviceProvider">The application service provider (see the primary constructor).</param>
/// <param name="options">The options the active-node probe is registered with.</param>
/// <returns>The gate.</returns>
public static AkkaActiveNodeGate ForOptions(IServiceProvider serviceProvider, ActiveNodeHealthCheckOptions options)
{
ArgumentNullException.ThrowIfNull(options);
return new AkkaActiveNodeGate(serviceProvider, options.RolePreference);
}
/// <inheritdoc />
public bool IsActiveNode
{
@@ -60,7 +83,10 @@ public sealed class AkkaActiveNodeGate : IActiveNodeGate
try
{
return ClusterActiveNode.SelfIsActive(Cluster.Get(system), _role);
var cluster = Cluster.Get(system);
return _rolePreference is not null
? ClusterActiveNode.SelfIsActiveForPreference(cluster, _rolePreference)
: ClusterActiveNode.SelfIsActive(cluster, _role);
}
catch (Exception)
{
ZB.MOM.WW.Health/src/ZB.MOM.WW.Health.Akka/ClusterActiveNode.cs
+24
View File
@@ -98,6 +98,30 @@ public static class ClusterActiveNode
return oldest is not null && oldest.UniqueAddress.Equals(self.UniqueAddress);
}
/// <summary>
/// Whether the local node is the active node under an ordered role preference — the exact scope
/// <see cref="ActiveNodeHealthCheck"/> answers for: the first preferred role the node carries
/// (see <see cref="ResolveActiveRole"/>), then <see cref="SelfIsActive(Cluster, string?)"/> in
/// that role.
/// </summary>
/// <param name="cluster">The Akka cluster to evaluate.</param>
/// <param name="rolePreference">
/// Ordered role preference. Empty means unscoped (every Up member competes).
/// </param>
/// <returns>
/// <c>false</c> when the preference is non-empty and the node carries none of its roles (it owns
/// no active work); otherwise the result of the role-scoped <c>SelfIsActive</c>.
/// </returns>
public static bool SelfIsActiveForPreference(Cluster cluster, IReadOnlyList<string> rolePreference)
{
ArgumentNullException.ThrowIfNull(cluster);
ArgumentNullException.ThrowIfNull(rolePreference);
var role = ResolveActiveRole(cluster.SelfMember.Roles, rolePreference);
if (rolePreference.Count > 0 && role is null) return false;
return SelfIsActive(cluster, role);
}
/// <summary>
/// The first role in <paramref name="rolePreference"/> that <paramref name="selfRoles"/> carries
/// — the role whose oldest Up member owns this node's active work.
ZB.MOM.WW.Health/tests/ZB.MOM.WW.Health.Akka.Tests/ClusterActiveNodeTests.cs
+28
View File
@@ -220,6 +220,34 @@ public sealed class ClusterActiveNodeTests : IAsyncLifetime
Assert.False(new AkkaActiveNodeGate(ProviderFor(_youngest!), "admin").IsActiveNode);
}
[Fact]
public async Task Gate_ForOptions_AgreesWithTheProbeOnEveryNode_ForAMultiRolePreference()
{
// Health-007: the string-role ctor cannot express ["admin","driver"], so a gate and the
// probe could disagree on a mixed mesh. ForOptions shares the probe's scope definition.
var options = new ActiveNodeHealthCheckOptions { RolePreference = ["admin", "driver"] };
foreach (var system in new[] { _oldest!, _youngest! })
{
var probeActive = (await CheckAsync(system, options)).Status == HealthStatus.Healthy;
Assert.Equal(probeActive, AkkaActiveNodeGate.ForOptions(ProviderFor(system), options).IsActiveNode);
}
Assert.True(AkkaActiveNodeGate.ForOptions(ProviderFor(_oldest!), options).IsActiveNode);
Assert.False(AkkaActiveNodeGate.ForOptions(ProviderFor(_youngest!), options).IsActiveNode);
}
[Fact]
public void Gate_ForOptions_NodeCarryingNoPreferredRole_IsNeverActive_AndEmptyPreferenceIsUnscoped()
{
var none = new ActiveNodeHealthCheckOptions { RolePreference = ["no-such-role"] };
Assert.False(AkkaActiveNodeGate.ForOptions(ProviderFor(_oldest!), none).IsActiveNode);
var unscoped = new ActiveNodeHealthCheckOptions();
Assert.True(AkkaActiveNodeGate.ForOptions(ProviderFor(_oldest!), unscoped).IsActiveNode);
Assert.False(AkkaActiveNodeGate.ForOptions(ProviderFor(_youngest!), unscoped).IsActiveNode);
}
private static IServiceProvider ProviderFor(ActorSystem system) =>
new ServiceCollection().AddSingleton(system).BuildServiceProvider();
code-reviews/Health/findings.md
+3 -3
View File
@@ -10,7 +10,7 @@
| Last reviewed | 2026-11-09 (re-review; baseline 2026-06-01 at `5f75cd4`) |
| Reviewer | Claude (2026-11-09 re-review, scadaproj#15; baseline: Claude automated) |
| Commit reviewed | `91fb2b4` |
| Open findings | 1 |
| Open findings | 0 |
## Summary
@@ -304,7 +304,7 @@ Resolved in `544a6dd`, 2026-06-01 — set `<GenerateDocumentationFile>true</Gene
| Severity | Low |
| Priority | P3 |
| Category | Public API surface & compatibility |
| Status | Open |
| Status | Resolved |
| Location | `ZB.MOM.WW.Health/src/ZB.MOM.WW.Health.Akka/AkkaActiveNodeGate.cs:46` (ctor `string? role`), `:14-17` (doc claim), vs `ZB.MOM.WW.Health/src/ZB.MOM.WW.Health.Akka/ActiveNodeHealthCheck.cs:114-115` |
**Description**
@@ -339,4 +339,4 @@ both are configured with the same single role or both unscoped.
**Resolution**
_Unresolved._
Resolved 2026-11-09 in the commit tagged `(scadaproj#48)` (Health `0.3.1`, unpublished) — added `AkkaActiveNodeGate.ForOptions(sp, ActiveNodeHealthCheckOptions)` and a shared `ClusterActiveNode.SelfIsActiveForPreference(cluster, rolePreference)` (resolve the first preferred role the node carries, then `SelfIsActive`; a node with none of the roles is never active), so the gate and `/health/active` share one scope definition. Chose a static factory over a second public constructor so an existing `new AkkaActiveNodeGate(sp, null)` call is not made ambiguous. Class remarks qualified. Tests `Gate_ForOptions_AgreesWithTheProbeOnEveryNode_ForAMultiRolePreference` and `..._NodeCarryingNoPreferredRole_IsNeverActive_AndEmptyPreferenceIsUnscoped` (new API, so they cannot compile without the fix).
code-reviews/README.md
+5 -6
View File
@@ -40,7 +40,7 @@ code-reviews/
## Summary
10 of 10 libraries reviewed. 35 pending findings across all libraries.
10 of 10 libraries reviewed. 34 pending findings across all libraries.
Coverage: Secrets, LocalDb, GalaxyRepository and Overview first reviewed, and Auth, Health,
Theme, Audit, Configuration and Telemetry re-reviewed, on 2026-11-09 (#15). Each finding also
@@ -51,8 +51,8 @@ carries a P1/P2/P3 priority (P1 = Critical/High, P2 = Medium, P3 = Low).
| Critical | 0 |
| High | 0 |
| Medium | 11 |
| Low | 24 |
| **Total** | **35** |
| Low | 23 |
| **Total** | **34** |
## Library Status
@@ -62,7 +62,7 @@ carries a P1/P2/P3 priority (P1 = Critical/High, P2 = Medium, P3 = Low).
| [Auth](Auth/findings.md) | 2026-11-09 | `91fb2b4` | 0/0/1/3 | 4 | 10 |
| [Configuration](Configuration/findings.md) | 2026-11-09 | `91fb2b4` | 0/0/0/0 | 0 | 4 |
| [GalaxyRepository](GalaxyRepository/findings.md) | 2026-11-09 | `91fb2b4` | 0/0/0/0 | 0 | 8 |
| [Health](Health/findings.md) | 2026-11-09 | `91fb2b4` | 0/0/0/1 | 1 | 7 |
| [Health](Health/findings.md) | 2026-11-09 | `91fb2b4` | 0/0/0/0 | 0 | 7 |
| [LocalDb](LocalDb/findings.md) | 2026-11-09 | `91fb2b4` | 0/0/4/3 | 7 | 7 |
| [Overview](Overview/findings.md) | 2026-11-09 | `91fb2b4` | 0/0/0/0 | 0 | 5 |
| [Secrets](Secrets/findings.md) | 2026-11-09 | `91fb2b4` | 0/0/6/13 | 19 | 19 |
@@ -100,14 +100,13 @@ _None open._
| Secrets-013 | [Secrets](Secrets/findings.md) | UI add/rotate/delete: if the audit write throws after the store write succeeded, the change is reported and audited as a failure and the resolver cache is not evicted |
| Secrets-017 | [Secrets](Secrets/findings.md) | `secret set`/`rotate` take the plaintext on argv; the host builder re-parses argv as configuration and can crash with the value in the error |
### Low (24)
### Low (23)
| ID | Library | Title |
|----|---------|-------|
| Auth-008 | [Auth](Auth/findings.md) | SQL Server store silently truncates over-length values and matches key ids case-insensitively, diverging from SQLite |
| Auth-009 | [Auth](Auth/findings.md) | Canonical username is the first value of a possibly multi-valued attribute, not the value the user matched |
| Auth-010 | [Auth](Auth/findings.md) | `ApiKeyVerifier` lets store exceptions escape despite its "only exception path is cancellation" contract |
| Health-007 | [Health](Health/findings.md) | `AkkaActiveNodeGate` cannot express the probe's `RolePreference`, so gate and `/health/active` can disagree despite the documented guarantee |
| LocalDb-005 | [LocalDb](LocalDb/findings.md) | Auto-assigned rowid / AUTOINCREMENT keys collide across nodes and LWW silently drops one row |
| LocalDb-006 | [LocalDb](LocalDb/findings.md) | Column names containing `'` are captured but break the apply and snapshot SQL |
| LocalDb-007 | [LocalDb](LocalDb/findings.md) | README is stale on version, adoption, test count, `Synchronous` values and baseline scope |