fix(adminui): /hosts eviction keys on authored + DEPLOYED drivers (review D-b r1 I1)

Staleness is not a liveness proof: DriverInstanceActor publishes on a 30 s tick and dedups on a health fingerprint, so a running Faulted, idle or tagless driver deleted in /raw (undeployed) went stale and was evicted. DeployedDriverSet reads, per node, the latest and latest-Applied NodeDeploymentState deployments plus the latest Sealed one, parses each artifact's DriverInstances once (cached by deployment id, pruned to the live set, only the blob column of an uncached row is read), and /hosts unions it with the authored set. The staleness clause stays as a second guard. An unreadable artifact or a failed read means 'unknown' and skips eviction. The Hosts.razor comment claiming '~12 health publishes' is corrected. N-new-2: the CSV download's Content-Disposition carries an RFC 5987 filename* via ContentDispositionHeaderValue.SetHttpFileName. Refs #745 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
2026-10-22 18:30:23 -04:00
parent 0f1f8ad8fa
commit 2e64af26ef
6 changed files with 326 additions and 14 deletions
@@ -19,6 +19,7 @@
@inject IDriverStatusSnapshotStore DriverStore
@inject IDriverResilienceStatusStore ResilienceStore
@inject IDbContextFactory<OtOpcUaConfigDbContext> DbFactory
+@inject DeployedDriverSet Deployed
@inject Microsoft.Extensions.Logging.ILogger<Hosts> Logger
@implements IAsyncDisposable

@@ -329,7 +330,10 @@ else
@code {
    private const int RefreshIntervalSeconds = 5;

-    // An unconfigured driver's snapshot must be this old before eviction (≈ 12 health publishes).
+    // Second guard only: a snapshot of a driver neither authored nor deployed must also be this old before it
+    // is evicted. NOT a liveness proof — DriverInstanceActor publishes on a 30 s tick and dedups on a health
+    // fingerprint, so a running Faulted/idle/tagless driver can go hours without publishing; the deployed
+    // set (DeployedDriverSet) is what keeps such a driver's row.
    private static readonly TimeSpan EvictUnconfiguredAfter = TimeSpan.FromSeconds(60);

    private List<MemberRow>? _rows;
@@ -447,19 +451,26 @@ else
                .Select(d => new HostsDriverInstanceInfo(d.DriverInstanceId, d.ClusterId, d.Name, d.DriverType, d.Enabled))
                .ToListAsync(_cts.Token);

-            // Evict snapshots of driver instances that are no longer configured (deleted, or re-created
-            // under a new id) AND have stopped publishing. The stores have no other removal path, so
-            // without this a removed driver stays on this page as a frozen row until the admin node
-            // restarts. DriverInstances is the AUTHORED table: a driver deleted in /raw keeps running (and
-            // publishing every few seconds) until the next deploy, so staleness is what proves it stopped.
+            // Evict snapshots of driver instances that are neither AUTHORED nor DEPLOYED, and stale. The
+            // stores have no other removal path, so without this a removed driver stays on this page as a
+            // frozen row until the admin node restarts. DriverInstances is the authored (draft) table: a
+            // driver deleted in /raw keeps running until the next deploy, and a quiet one (Faulted, idle,
+            // tagless) may not publish at all meanwhile, so the deployed set — not staleness — is what keeps
+            // it (review D-b r1, I1). Unknown deployed set ⇒ no eviction this sweep.
            // Only a SUCCESSFUL load reaches here; the failure path below blanks _instances and must
            // never sweep the stores.
-            var configured = _instances
-                .Select(i => i.DriverInstanceId)
-                .ToHashSet(StringComparer.OrdinalIgnoreCase);
-            var staleBefore = DateTime.UtcNow - EvictUnconfiguredAfter;
-            var evicted = DriverStore.EvictUnconfiguredStale(configured, staleBefore);
-            ResilienceStore.EvictUnconfiguredStale(configured, staleBefore);
+            var evicted = 0;
+            var deployed = await Deployed.LoadAsync(_cts.Token);
+            if (deployed is not null)
+            {
+                var configured = _instances
+                    .Select(i => i.DriverInstanceId)
+                    .ToHashSet(StringComparer.OrdinalIgnoreCase);
+                configured.UnionWith(deployed);
+                var staleBefore = DateTime.UtcNow - EvictUnconfiguredAfter;
+                evicted = DriverStore.EvictUnconfiguredStale(configured, staleBefore);
+                ResilienceStore.EvictUnconfiguredStale(configured, staleBefore);
+            }

            return evicted > 0
                || !_nodes.SequenceEqual(previousNodes)
@@ -85,6 +85,8 @@ public static class EndpointRouteBuilderExtensions
        services.AddScoped<Acls.IAclScopeService, Acls.AclScopeService>();
        // CSV export enumeration (read-only; does not extend the closed IRawTreeService prelude).
        services.AddScoped<RawTagCsvExportReader>();
+        // /hosts eviction: what the fleet is running (cached per deployment id), unioned with the authored set.
+        services.AddSingleton<Hosts.DeployedDriverSet>();
        services.AddSingleton<IDriverBrowser, OpcUaClientDriverBrowser>();
        services.AddSingleton<IDriverBrowser, GalaxyDriverBrowser>();
        // Bespoke Sql schema browser (server → schema → table → column). Constructible bare — every
@@ -0,0 +1,118 @@
+using System.Collections.Concurrent;
+using Microsoft.EntityFrameworkCore;
+using Microsoft.Extensions.Logging;
+using ZB.MOM.WW.OtOpcUa.Configuration;
+using ZB.MOM.WW.OtOpcUa.Configuration.Enums;
+using ZB.MOM.WW.OtOpcUa.Runtime.Drivers;
+
+namespace ZB.MOM.WW.OtOpcUa.AdminUI.Hosts;
+
+/// <summary>
+///     The driver instance ids the fleet is (or may still be) RUNNING, read from deployment state rather than
+///     from the authored <c>DriverInstances</c> table (review D-b r1, I1). <c>/hosts</c> unions this with the
+///     authored set before evicting snapshots, so a driver deleted in <c>/raw</c> but not yet deployed keeps
+///     its row however quiet it is — <c>DriverInstanceActor</c> dedups its health publishes on a fingerprint,
+///     so a Faulted, idle or tagless driver that is still running may not publish for hours, and staleness
+///     alone cannot prove it stopped.
+/// </summary>
+/// <remarks>
+///     <para>"Running" deployments are, per node, its latest <c>NodeDeploymentState</c> row (whatever its
+///     status — an Applying/Failed apply may already be running part of it) and its latest Applied row (what
+///     it ran before a newer one failed), plus the latest Sealed deployment. The union over-approximates on
+///     purpose: keeping a ghost row a little longer is harmless, hiding a running driver is the defect.</para>
+///     <para>Each artifact is parsed once and cached by deployment id — artifacts are immutable — and only
+///     the one <c>ArtifactBlob</c> column of an uncached id is ever read. The cache is pruned to the current
+///     id set every call, so it stays the size of the fleet's live deployments.</para>
+///     <para>Fail-safe: any id whose artifact cannot be read (missing, retention-emptied, query failure)
+///     makes <see cref="LoadAsync"/> return null, and the caller then skips eviction for that sweep.</para>
+/// </remarks>
+public sealed class DeployedDriverSet
+{
+    private readonly IDbContextFactory<OtOpcUaConfigDbContext> _dbFactory;
+    private readonly Func<byte[], IEnumerable<string>> _parseDriverIds;
+    private readonly ILogger<DeployedDriverSet>? _logger;
+    private readonly ConcurrentDictionary<Guid, IReadOnlySet<string>> _byDeployment = new();
+
+    /// <summary>Creates the set.</summary>
+    /// <param name="dbFactory">The config DB factory.</param>
+    /// <param name="logger">Optional logger.</param>
+    /// <param name="parseDriverIds">Artifact → driver instance ids; defaults to <see cref="DeploymentArtifact.ParseDriverInstances(ReadOnlySpan{byte})"/>.</param>
+    public DeployedDriverSet(
+        IDbContextFactory<OtOpcUaConfigDbContext> dbFactory,
+        ILogger<DeployedDriverSet>? logger = null,
+        Func<byte[], IEnumerable<string>>? parseDriverIds = null)
+    {
+        _dbFactory = dbFactory;
+        _logger = logger;
+        _parseDriverIds = parseDriverIds
+            ?? (blob => DeploymentArtifact.ParseDriverInstances(blob).Select(s => s.DriverInstanceId));
+    }
+
+    /// <summary>The number of cached artifacts (for tests).</summary>
+    internal int CachedDeployments => _byDeployment.Count;
+
+    /// <summary>Loads the deployed driver instance ids, or null when they cannot be determined this time.</summary>
+    /// <param name="ct">Cancellation token; a cancellation propagates.</param>
+    /// <returns>The ids (ordinal-ignore-case), or null ⇒ do not evict.</returns>
+    public async Task<IReadOnlySet<string>?> LoadAsync(CancellationToken ct = default)
+    {
+        try
+        {
+            await using var db = await _dbFactory.CreateDbContextAsync(ct);
+
+            var latestPerNode = await db.NodeDeploymentStates.AsNoTracking()
+                .GroupBy(n => n.NodeId)
+                .Select(g => g.OrderByDescending(x => x.StartedAtUtc).Select(x => x.DeploymentId).First())
+                .ToListAsync(ct);
+            var latestAppliedPerNode = await db.NodeDeploymentStates.AsNoTracking()
+                .Where(n => n.Status == NodeDeploymentStatus.Applied)
+                .GroupBy(n => n.NodeId)
+                .Select(g => g.OrderByDescending(x => x.StartedAtUtc).Select(x => x.DeploymentId).First())
+                .ToListAsync(ct);
+            var latestSealed = await db.Deployments.AsNoTracking()
+                .Where(d => d.Status == DeploymentStatus.Sealed)
+                .OrderByDescending(d => d.CreatedAtUtc)
+                .Select(d => (Guid?)d.DeploymentId)
+                .FirstOrDefaultAsync(ct);
+
+            var ids = latestPerNode.Concat(latestAppliedPerNode).ToHashSet();
+            if (latestSealed is { } s) ids.Add(s);
+
+            var result = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
+            foreach (var id in ids)
+            {
+                if (!_byDeployment.TryGetValue(id, out var drivers))
+                {
+                    // Only the blob column of this one row — never the whole entity, never every row.
+                    var blob = await db.Deployments.AsNoTracking()
+                        .Where(d => d.DeploymentId == id)
+                        .Select(d => d.ArtifactBlob)
+                        .FirstOrDefaultAsync(ct);
+                    if (blob is null || blob.Length == 0)
+                    {
+                        // Unknown, not empty: a running node may still hold drivers this artifact named.
+                        _logger?.LogDebug("/hosts: artifact of deployment {DeploymentId} is unavailable; skipping eviction", id);
+                        return null;
+                    }
+
+                    drivers = _parseDriverIds(blob).ToHashSet(StringComparer.OrdinalIgnoreCase);
+                    _byDeployment[id] = drivers;
+                }
+
+                result.UnionWith(drivers);
+            }
+
+            foreach (var cached in _byDeployment.Keys)
+            {
+                if (!ids.Contains(cached)) _byDeployment.TryRemove(cached, out _);
+            }
+
+            return result;
+        }
+        catch (Exception ex) when (ex is not OperationCanceledException)
+        {
+            _logger?.LogWarning(ex, "/hosts: could not read the deployed driver set; skipping eviction this sweep.");
+            return null;
+        }
+    }
+}
@@ -105,6 +105,20 @@ public static class RawTagCsvExport
        return query.Count == 0 ? DownloadPath : DownloadPath + "?" + string.Join("&", query);
    }

+    /// <summary>
+    ///     The attachment <c>Content-Disposition</c> value. Emits both <c>filename</c> and the RFC 5987
+    ///     <c>filename*</c> (review r1 N-new-2): a non-ASCII device id such as <c>Prüfstand</c> keeps its
+    ///     name in every browser, which an RFC 2047 encoded word (System.Net.Mime) does not.
+    /// </summary>
+    /// <param name="fileName">The download file name.</param>
+    /// <returns>The header value.</returns>
+    public static string AttachmentHeader(string fileName)
+    {
+        var header = new Microsoft.Net.Http.Headers.ContentDispositionHeaderValue("attachment");
+        header.SetHttpFileName(fileName);
+        return header.ToString();
+    }
+
    /// <summary>The download file name for a device.</summary>
    /// <param name="deviceId">The device id.</param>
    /// <returns>A file-system-safe name.</returns>
@@ -131,8 +145,7 @@ public static class RawTagCsvExport

                // Attachment, so the browser saves rather than renders. Results.Text writes the string
                // through the response encoder, so no second whole-file byte[] copy is held (review r0 M8).
-                http.Response.Headers.ContentDisposition =
-                    new System.Net.Mime.ContentDisposition { FileName = export.FileName, DispositionType = "attachment" }.ToString();
+                http.Response.Headers.ContentDisposition = AttachmentHeader(export.FileName);
                return Results.Text(export.Csv, "text/csv; charset=utf-8", System.Text.Encoding.UTF8);
            })
            .RequireAuthorization(AdminUiPolicies.ConfigEditor)
@@ -0,0 +1,157 @@
+using System.Text;
+using Microsoft.EntityFrameworkCore;
+using Shouldly;
+using Xunit;
+using ZB.MOM.WW.OtOpcUa.AdminUI.Hosts;
+using ZB.MOM.WW.OtOpcUa.AdminUI.Hubs;
+using ZB.MOM.WW.OtOpcUa.Commons.Messages.Drivers;
+using ZB.MOM.WW.OtOpcUa.Configuration;
+using ZB.MOM.WW.OtOpcUa.Configuration.Entities;
+using ZB.MOM.WW.OtOpcUa.Configuration.Enums;
+
+namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Hosts;
+
+/// <summary>
+///     Review D-b r1, I1: <c>/hosts</c> keys eviction on authored ∪ DEPLOYED drivers, because a running driver
+///     whose health is unchanged never re-publishes (fingerprint dedup) — staleness is not a liveness proof.
+/// </summary>
+[Trait("Category", "Unit")]
+public sealed class DeployedDriverSetTests
+{
+    private static readonly DateTime Now = new(2026, 10, 22, 12, 0, 0, DateTimeKind.Utc);
+
+    // Test artifacts are a plain comma list; the real parser is DeploymentArtifact.ParseDriverInstances.
+    private static IEnumerable<string> Parse(byte[] blob) =>
+        Encoding.UTF8.GetString(blob).Split(',', StringSplitOptions.RemoveEmptyEntries);
+
+    private static byte[] Artifact(params string[] driverIds) => Encoding.UTF8.GetBytes(string.Join(",", driverIds));
+
+    private sealed class Factory(string name) : IDbContextFactory<OtOpcUaConfigDbContext>
+    {
+        public int Created;
+        public OtOpcUaConfigDbContext CreateDbContext()
+        {
+            Created++;
+            return new(new DbContextOptionsBuilder<OtOpcUaConfigDbContext>().UseInMemoryDatabase(name).Options);
+        }
+
+        public Task<OtOpcUaConfigDbContext> CreateDbContextAsync(CancellationToken ct = default) => Task.FromResult(CreateDbContext());
+    }
+
+    private static Deployment Dep(Guid id, DeploymentStatus status, DateTime created, byte[] blob) =>
+        new() { DeploymentId = id, RevisionHash = id.ToString("N"), CreatedBy = "t", Status = status, CreatedAtUtc = created, ArtifactBlob = blob };
+
+    private static NodeDeploymentState Node(string node, Guid dep, NodeDeploymentStatus status, DateTime started) =>
+        new() { NodeId = node, DeploymentId = dep, Status = status, StartedAtUtc = started };
+
+    [Fact]
+    public async Task Quiet_running_driver_deleted_in_raw_survives_the_sweep()
+    {
+        // drv-quiet is running (in the deployed artifact), deleted in /raw (not authored), and published
+        // once 5 minutes ago — a Faulted driver with a steady error never re-publishes.
+        var factory = new Factory(Guid.NewGuid().ToString("N"));
+        var d1 = Guid.NewGuid();
+        await using (var db = factory.CreateDbContext())
+        {
+            db.Deployments.Add(Dep(d1, DeploymentStatus.Sealed, Now.AddHours(-1), Artifact("drv-quiet", "drv-kept")));
+            db.NodeDeploymentStates.Add(Node("n1", d1, NodeDeploymentStatus.Applied, Now.AddHours(-1)));
+            await db.SaveChangesAsync();
+        }
+
+        var deployed = await new DeployedDriverSet(factory, parseDriverIds: Parse).LoadAsync();
+        deployed.ShouldNotBeNull();
+
+        var store = new InMemoryDriverStatusSnapshotStore();
+        store.Upsert(new DriverHealthChanged("MAIN", "drv-quiet", "Faulted", null, "refused", 3, Now.AddMinutes(-5)));
+        store.Upsert(new DriverHealthChanged("MAIN", "drv-gone", "Faulted", null, "refused", 3, Now.AddMinutes(-5)));
+
+        var configured = new HashSet<string>(["drv-kept"], StringComparer.OrdinalIgnoreCase); // authored
+        configured.UnionWith(deployed!);
+        var removed = store.EvictUnconfiguredStale(configured, Now.AddSeconds(-60));
+
+        store.TryGet("drv-quiet", out _).ShouldBeTrue();
+        // Positive control: an undeployed, unconfigured, stale driver is still evicted.
+        removed.ShouldBe(1);
+        store.TryGet("drv-gone", out _).ShouldBeFalse();
+    }
+
+    [Fact]
+    public async Task Union_covers_each_nodes_latest_and_latest_applied_and_the_latest_sealed()
+    {
+        var factory = new Factory(Guid.NewGuid().ToString("N"));
+        Guid old = Guid.NewGuid(), failed = Guid.NewGuid(), sealedOther = Guid.NewGuid(), stale = Guid.NewGuid();
+        await using (var db = factory.CreateDbContext())
+        {
+            db.Deployments.Add(Dep(stale, DeploymentStatus.Sealed, Now.AddDays(-9), Artifact("drv-ancient")));
+            db.Deployments.Add(Dep(old, DeploymentStatus.Sealed, Now.AddHours(-3), Artifact("drv-old")));
+            db.Deployments.Add(Dep(failed, DeploymentStatus.PartiallyFailed, Now.AddHours(-1), Artifact("drv-new")));
+            db.Deployments.Add(Dep(sealedOther, DeploymentStatus.Sealed, Now.AddHours(-2), Artifact("drv-sealed")));
+            db.NodeDeploymentStates.Add(Node("n1", stale, NodeDeploymentStatus.Applied, Now.AddDays(-9)));
+            db.NodeDeploymentStates.Add(Node("n1", old, NodeDeploymentStatus.Applied, Now.AddHours(-3)));
+            db.NodeDeploymentStates.Add(Node("n1", failed, NodeDeploymentStatus.Failed, Now.AddHours(-1)));
+            await db.SaveChangesAsync();
+        }
+
+        var deployed = await new DeployedDriverSet(factory, parseDriverIds: Parse).LoadAsync();
+
+        // n1's latest (failed apply, may be partly running), its latest Applied (what it ran before), and the
+        // latest Sealed deployment — but not a superseded one nobody runs.
+        deployed!.ShouldBe(["drv-new", "drv-old", "drv-sealed"], ignoreOrder: true);
+        deployed!.ShouldNotContain("drv-ancient");
+    }
+
+    [Fact]
+    public async Task Artifacts_are_parsed_once_per_deployment_and_pruned()
+    {
+        var factory = new Factory(Guid.NewGuid().ToString("N"));
+        var d1 = Guid.NewGuid();
+        await using (var db = factory.CreateDbContext())
+        {
+            db.Deployments.Add(Dep(d1, DeploymentStatus.Sealed, Now, Artifact("drv-a")));
+            await db.SaveChangesAsync();
+        }
+
+        var parses = 0;
+        var set = new DeployedDriverSet(factory, parseDriverIds: b => { parses++; return Parse(b); });
+        await set.LoadAsync();
+        await set.LoadAsync();
+        parses.ShouldBe(1);
+        set.CachedDeployments.ShouldBe(1);
+
+        var d2 = Guid.NewGuid();
+        await using (var db = factory.CreateDbContext())
+        {
+            db.Deployments.Add(Dep(d2, DeploymentStatus.Sealed, Now.AddMinutes(1), Artifact("drv-b")));
+            await db.SaveChangesAsync();
+        }
+
+        (await set.LoadAsync())!.ShouldBe(["drv-b"]);
+        set.CachedDeployments.ShouldBe(1); // d1 is no longer live, so it is pruned
+        parses.ShouldBe(2);
+    }
+
+    [Fact]
+    public async Task An_unreadable_artifact_means_unknown_so_nothing_is_evicted()
+    {
+        var factory = new Factory(Guid.NewGuid().ToString("N"));
+        var d1 = Guid.NewGuid();
+        await using (var db = factory.CreateDbContext())
+        {
+            db.Deployments.Add(Dep(d1, DeploymentStatus.Sealed, Now, Array.Empty<byte>())); // retention-emptied
+            await db.SaveChangesAsync();
+        }
+
+        (await new DeployedDriverSet(factory, parseDriverIds: Parse).LoadAsync()).ShouldBeNull();
+    }
+
+    [Fact]
+    public void Hosts_skips_eviction_when_the_deployed_set_is_unknown()
+    {
+        var src = AdminUiSource.Read("Components", "Pages", "Hosts.razor");
+        src.ShouldContain("var deployed = await Deployed.LoadAsync(_cts.Token);");
+        src.ShouldContain("configured.UnionWith(deployed);");
+        src.IndexOf("if (deployed is not null)", StringComparison.Ordinal)
+            .ShouldBeLessThan(src.IndexOf("DriverStore.EvictUnconfiguredStale(", StringComparison.Ordinal));
+        src.ShouldNotContain("≈ 12 health publishes");
+    }
+}
@@ -77,6 +77,17 @@ public sealed class RawCsvRenderingTests
        Commons.Csv.CsvParser.Parse(preview.Csv).Count.ShouldBe(2);  // header + 1
    }

+    [Fact]
+    public void Attachment_header_carries_an_rfc5987_name_for_non_ascii_ids()
+    {
+        var header = RawTagCsvExport.AttachmentHeader(RawTagCsvExport.BuildFileName("Prüfstand"));
+
+        header.ShouldStartWith("attachment");
+        header.ShouldContain("filename*=UTF-8''tags-Pr%C3%BCfstand.csv");
+        // Positive control: an ASCII name is still a plain filename.
+        RawTagCsvExport.AttachmentHeader("tags-dev1.csv").ShouldContain("filename=tags-dev1.csv");
+    }
+
    [Fact]
    public void Download_url_escapes_its_query_values()
    {