Lite: prove the Agent-status SQL, not just the decision it feeds

CHANGELOG.md

@@ -57,6 +57,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **Lite: prove the Agent-status SQL, not just the decision it feeds** ([#1730]) - [#1725]'s tests all run against constructed `AgentStatusRow` values, which is the right shape for pinning what a row MEANS but leaves the query that produces those rows unproven. It is not trivial SQL: `ever_seen_running` is a window aggregate that has to see the whole retained partition while the surrounding query collapses to the newest row per server, and the two ways that goes wrong both return a plausible boolean and both pass every model-level test. If it collapsed to the newest row, a server that ran Agent yesterday and stopped today would read "never ran Agent" and a **genuine outage would render neutral** - the exact failure the fix exists to prevent, arrived at from the other direction. If the partition leaked, one real server's history would make every Agent-less container read as a server that runs Agent, putting the red "Stopped" straight back where [#1725] removed it. Six real-DuckDB round-trips now cover both, plus the newest-row-wins tiebreak, `collection_time` driving staleness, and the fresh case NOT being suppressed. Confirmed non-vacuous by mutation: dropping `PARTITION BY server_id` from the shipped query fails two of them. Also pins that the stale window stays longer than the collector's own cadence, read from `CollectorScheduleDefaults` so it follows a retune - a live trap rather than a hypothetical, since the shared `ServerHealthThresholds.StaleThreshold` is two minutes derived from the FASTEST collector's one-minute cadence, looks like the obvious thing to unify this with, and would render a perfectly healthy Agent as "unknown (stale)" most of the time because `agent_status` collects every five. Tests only; no production code, and the shipped query passes as written.
 - **Credential-file ACL failures now name the owner, and the DPAPI credential files verify the result instead of assuming it** ([#1727]) - hardening #1721. When the service cannot re-ACL a file it protects, the error said "fix the file permissions by hand" and left the operator to work out which permissions, held by whom, and why. It now names the file's OWNER and the ordinary-user group that can read it, and states the part that actually resolves it: `SetAccessControl` needs WRITE_DAC, which comes with ownership or FullControl, so a service account holding only inherited Modify on a file owned by someone else can NEVER succeed - restarting will not clear it, and only granting FullControl or ownership will. That was the exact live condition on a field box, where the same error repeated every start for a day. The bigger half: the post-harden verification that `darling.json` already had - attempt, then CHECK whether ordinary users can still read the bytes, and log Critical if they can - is now applied to the machine-scoped DPAPI **credential** files too (the managed-Postgres owner/admin/viewer/MCP credentials and the least-privilege role credentials), which previously only logged the attempt. For LocalMachine DPAPI, read access to the file IS the secret, so "we tried to harden it" and "the secret is protected" are different claims and only one of them is worth logging.
 - **Lite: the Job History Agent indicator stops calling absence of signal a problem** ([#1725]) - the display-tier companion to the Agent-alert capability gate, closing the same misleading-by-platform class on the surface a user actually looks at. The header rendered a red **"Agent: Stopped"** in exactly two situations where nothing is wrong. A server where SQL Agent has NEVER been observed running - a container built without it, Express, a Linux-minimal image - is not stopped; nothing stopped, the target simply does not run Agent, and Lite collects in-process so nothing here depends on it. That now reads a neutral **"not present"**. And a STALE snapshot is not evidence Agent is down: a server nobody has collected from in days was rendering its last-known state in red forever. That now reads **"unknown (stale)"**, using the same 30-minute refusal window the headless service's alert path already applies, pinned equal by test so the two surfaces cannot drift into disagreeing about whether a reading is old enough to judge. Red is now reserved for the one case that means something: a FRESH reading of a genuinely stopped Agent on a server that runs one. The fleet roll-up counts only the servers the question applies to, so "2/2 running" no longer becomes "2/5 running, 3 stopped" because three containers were counted as failures. The backing read gained `collection_time` and an ever-seen-running flag to make any of this decidable - it previously returned neither, so the header had no way to tell the three cases apart. Also corrects the collector-schedule description shown in Lite's own UI, which told users `agent_status` "drives the Job History tab header and the Agent Not Running alert"; Lite raises no such alert, and now the text says so.
 - **Darling Web: a capped time-series panel kept the OLDEST slice of the window and said nothing** ([#1724]) - closes #991's sibling #1687, found live on DARLING01 while building the #1606 demo view. A grouped panel whose buckets x groups product exceeds the 10,000-row hard cap got `ORDER BY bucket LIMIT 10000`, which keeps the EARLIEST rows: a 24-hour wait_stats panel grouped by `wait_type` at minute grain rendered its first ~87 minutes as though that were the whole window, and because the truncated domain never crossed midnight the x-axis showed no dates, so nothing looked wrong. Same silent-truncation class as the retention-routing gaps (#1661/#1664/#1665) - the numbers are right and the WINDOW is wrong, which a monitoring product must say out loud. **Two halves, both needed.** The cap now applies `ORDER BY bucket DESC` inside a subquery and re-sorts the survivors ascending for the renderer, so a truncated chart shows the RECENT end - what a monitoring chart is for - instead of the start of the window. And when a time-series panel comes back sitting exactly on the cap it attaches a notice through the existing #1665 `notice` plumbing ("row cap reached - showing the most recent 10,000 buckets of the window, not the whole of it. Coarsen the time bucket or narrow the group-by..."), rendered by the `noticeStrip` already on every web surface and riding verbatim through MCP. Exactly-at-cap is the only signal available, because group cardinality is a property of the data rather than the spec - so a panel that legitimately produces exactly 10,000 rows is reported as capped, a false positive taken deliberately over the reverse. Retention and row-cap notices are JOINED rather than one winning, since the two truncate a panel from the same end and a long grouped panel on a retention-active store hits both. Ranked and Scalar panels are untouched: a Ranked LIMIT is the caller's own topN, so reaching it is the request being honored, not truncation. The issue's optional third item (a write-time hint) is deliberately not implemented - `MaxBuckets` (5,000) is already below the row cap, so buckets alone can never trip it and any pre-flight warning would have to guess at group cardinality it cannot know.
@@ -1715,6 +1716,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 [#1710]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1710
 [#1726]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1726
 [#1725]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1725
+[#1730]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1730
 [#1727]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1727
 [#1690]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1690
 [#1693]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1693

Lite.Tests/AgentStatusReaderSqlTests.cs

@@ -0,0 +1,214 @@
+/*
+ * Copyright (c) 2026 Erik Darling, Darling Data LLC
+ *
+ * This file is part of the SQL Server Performance Monitor Lite.
+ *
+ * Licensed under the MIT License. See LICENSE file in the project root for full license information.
+ */
+
+using System;
+using System.Threading.Tasks;
+using DuckDB.NET.Data;
+using PerformanceMonitor.Collectors;
+using PerformanceMonitorLite.Database;
+using PerformanceMonitorLite.Services;
+using Xunit;
+
+namespace PerformanceMonitorLite.Tests;
+
+/// <summary>
+/// Real-DuckDB round-trips for the two columns #1725 added to
+/// <c>LocalDataService.GetAgentStatusAsync</c>: <c>collection_time</c> and the derived
+/// <c>ever_seen_running</c>.
+///
+/// <para><c>AgentStatusHeaderHonestyTests</c> pins the DECISION — what a row means once you have it — against
+/// constructed rows, which is the right shape for that half. Nothing yet proved the SQL that produces those
+/// rows, and it is not trivial SQL: <c>ever_seen_running</c> is a window aggregate that must see the WHOLE
+/// retained partition while the surrounding query collapses to the newest row per server. The two ways that
+/// goes wrong both return a plausible boolean and both pass every model-level test — it silently reports the
+/// newest row's own <c>agent_running</c> (so a server that ran Agent yesterday and stopped today reads "never
+/// ran Agent", and a genuine outage renders neutral), or the partition leaks (so an Agent-less container
+/// inherits a real server's history and goes back to being called "Stopped"). Each case below fails against
+/// one of those.</para>
+/// </summary>
+public sealed class AgentStatusReaderSqlTests : IClassFixture<SharedDuckDbFixture>, IDisposable
+{
+    private readonly DuckDbInitializer _duckDb;
+    private DuckDBConnection? _seedConn;
+    private long _nextId = 1;
+
+    public AgentStatusReaderSqlTests(SharedDuckDbFixture fixture)
+    {
+        fixture.ResetData();
+        _duckDb = fixture.DuckDb;
+    }
+
+    public void Dispose() => _seedConn?.Dispose();
+
+    private async Task<DuckDBConnection> SeedConnectionAsync()
+    {
+        if (_seedConn is null)
+        {
+            _seedConn = _duckDb.CreateConnection();
+            await _seedConn.OpenAsync();
+        }
+        return _seedConn;
+    }
+
+    private static DateTime Truncate(DateTime t) =>
+        new(t.Year, t.Month, t.Day, t.Hour, t.Minute, t.Second, DateTimeKind.Unspecified);
+
+    private async Task SeedAsync(int serverId, string serverName, DateTime collectionTimeUtc, bool agentRunning)
+    {
+        using var readLock = _duckDb.AcquireReadLock();
+        var connection = await SeedConnectionAsync();
+        using var cmd = connection.CreateCommand();
+        cmd.CommandText = @"
+INSERT INTO agent_status
+    (collection_id, collection_time, server_id, server_name, agent_running, agent_status_desc, agent_startup_desc, next_scheduled_run)
+VALUES ($1, $2, $3, $4, $5, $6, $7, $8)";
+        cmd.Parameters.Add(new DuckDBParameter { Value = _nextId++ });
+        cmd.Parameters.Add(new DuckDBParameter { Value = Truncate(collectionTimeUtc) });
+        cmd.Parameters.Add(new DuckDBParameter { Value = serverId });
+        cmd.Parameters.Add(new DuckDBParameter { Value = serverName });
+        cmd.Parameters.Add(new DuckDBParameter { Value = agentRunning });
+        cmd.Parameters.Add(new DuckDBParameter { Value = agentRunning ? "Running" : "Stopped" });
+        cmd.Parameters.Add(new DuckDBParameter { Value = "Automatic" });
+        cmd.Parameters.Add(new DuckDBParameter { Value = DBNull.Value });
+        await cmd.ExecuteNonQueryAsync();
+    }
+
+    [Fact]
+    public async Task EverSeenRunning_ReadsTheHistory_NotJustTheNewestRow()
+    {
+        /* Agent ran, then stopped — the case the whole gate turns on. The newest row says false; the retained
+           history says this server genuinely runs Agent, so it must still render as a real problem. A window
+           aggregate that collapsed to the newest row would call this "not present" and paint it neutral,
+           silently swallowing a genuine outage. */
+        var service = new LocalDataService(_duckDb);
+
+        await SeedAsync(5101, "REAL", DateTime.UtcNow.AddHours(-4), agentRunning: true);
+        await SeedAsync(5101, "REAL", DateTime.UtcNow.AddMinutes(-3), agentRunning: false);
+
+        var row = Assert.Single(await service.GetAgentStatusAsync(5101));
+
+        Assert.False(row.AgentRunning);
+        Assert.True(row.EverSeenRunning);
+        Assert.False(row.IsStale);
+        Assert.True(row.IsAgentProblem);
+        Assert.Equal("Stopped", row.StatusDisplay);
+    }
+
+    [Fact]
+    public async Task EverSeenRunning_IsFalse_WhenNoRetainedRowEverRan()
+    {
+        /* A container built without Agent, Express, Azure SQL DB: every retained row is false. */
+        var service = new LocalDataService(_duckDb);
+
+        await SeedAsync(5102, "CONTAINER", DateTime.UtcNow.AddHours(-4), agentRunning: false);
+        await SeedAsync(5102, "CONTAINER", DateTime.UtcNow.AddHours(-1), agentRunning: false);
+        await SeedAsync(5102, "CONTAINER", DateTime.UtcNow.AddMinutes(-3), agentRunning: false);
+
+        var row = Assert.Single(await service.GetAgentStatusAsync(5102));
+
+        Assert.False(row.EverSeenRunning);
+        Assert.False(row.IsAgentProblem);
+        Assert.Equal("not present", row.StatusDisplay);
+    }
+
+    [Fact]
+    public async Task EverSeenRunning_IsPerServer_AndDoesNotLeakAcrossThePartition()
+    {
+        /* Without PARTITION BY server_id the aggregate spans every row in the table, and one real server's
+           history would make every Agent-less target look like it runs Agent — putting the red "Stopped"
+           straight back on the containers this was meant to stop nagging about. */
+        var service = new LocalDataService(_duckDb);
+
+        await SeedAsync(5103, "USES-AGENT", DateTime.UtcNow.AddHours(-2), agentRunning: true);
+        await SeedAsync(5103, "USES-AGENT", DateTime.UtcNow.AddMinutes(-2), agentRunning: false);
+        await SeedAsync(5104, "NO-AGENT", DateTime.UtcNow.AddHours(-2), agentRunning: false);
+        await SeedAsync(5104, "NO-AGENT", DateTime.UtcNow.AddMinutes(-2), agentRunning: false);
+
+        var rows = await service.GetAgentStatusAsync();
+
+        var usesAgent = Assert.Single(rows, r => r.ServerId == 5103);
+        var noAgent = Assert.Single(rows, r => r.ServerId == 5104);
+
+        Assert.True(usesAgent.EverSeenRunning);
+        Assert.True(usesAgent.IsAgentProblem);
+
+        Assert.False(noAgent.EverSeenRunning);
+        Assert.False(noAgent.IsAgentProblem);
+    }
+
+    [Fact]
+    public async Task CollectionTime_IsProjected_AndTheNewestRowWins()
+    {
+        var service = new LocalDataService(_duckDb);
+
+        await SeedAsync(5105, "LAGGED", DateTime.UtcNow.AddDays(-2), agentRunning: true);
+        await SeedAsync(5105, "LAGGED", DateTime.UtcNow.AddHours(-6), agentRunning: false);
+
+        var row = Assert.Single(await service.GetAgentStatusAsync(5105));
+
+        Assert.NotNull(row.CollectionTime);
+        Assert.True(row.IsStale);
+        Assert.False(row.IsAgentProblem);
+        Assert.Equal("unknown (stale)", row.StatusDisplay);
+    }
+
+    [Fact]
+    public async Task FreshReadingIsNotStale_SoTheGateOnlyBitesOnOldData()
+    {
+        /* The other side of the staleness gate: a current reading must NOT be suppressed. */
+        var service = new LocalDataService(_duckDb);
+
+        await SeedAsync(5106, "FRESH", DateTime.UtcNow.AddMinutes(-1), agentRunning: true);
+
+        var row = Assert.Single(await service.GetAgentStatusAsync(5106));
+
+        Assert.False(row.IsStale);
+        Assert.Equal("Running", row.StatusDisplay);
+    }
+
+    [Fact]
+    public async Task FleetRead_ReturnsNewestRowPerServer_WithBothColumnsPopulated()
+    {
+        var service = new LocalDataService(_duckDb);
+
+        await SeedAsync(5107, "SQL-A", DateTime.UtcNow.AddHours(-3), agentRunning: true);
+        await SeedAsync(5107, "SQL-A", DateTime.UtcNow.AddMinutes(-1), agentRunning: true);
+        await SeedAsync(5108, "SQL-B", DateTime.UtcNow.AddMinutes(-1), agentRunning: false);
+
+        var rows = await service.GetAgentStatusAsync();
+
+        var a = Assert.Single(rows, r => r.ServerId == 5107);
+        var b = Assert.Single(rows, r => r.ServerId == 5108);
+
+        Assert.True(a.AgentRunning);
+        Assert.True(a.EverSeenRunning);
+        Assert.NotNull(a.CollectionTime);
+        Assert.False(a.IsAgentProblem);
+
+        Assert.False(b.AgentRunning);
+        Assert.False(b.EverSeenRunning);
+        Assert.NotNull(b.CollectionTime);
+        Assert.False(b.IsAgentProblem);
+    }
+
+    [Fact]
+    public void StaleWindow_StaysLongerThanTheCollectorsOwnCadence()
+    {
+        /* A stale window at or under the collector's cadence would render a perfectly healthy Agent as
+           "unknown (stale)" most of the time. This is a live trap rather than a hypothetical: the shared
+           ServerHealthThresholds.StaleThreshold is two minutes, derived from the FASTEST collector's
+           one-minute cadence, and looks like the obvious thing to unify this with — but agent_status collects
+           every five, so adopting it would break the header. Pinned against the schedule default so the guard
+           follows the cadence if it is ever retuned. */
+        var cadence = TimeSpan.FromMinutes(CollectorScheduleDefaults.All["agent_status"].FrequencyMinutes);
+
+        Assert.True(
+            AgentStatusRow.StaleWindow > cadence,
+            $"StaleWindow ({AgentStatusRow.StaleWindow}) must exceed the agent_status cadence ({cadence}).");
+    }
+}