Make the darling-pg nightly job green: bootstrap role + one grant gap + four latent test fixes

.github/workflows/nightly.yml

@@ -191,9 +191,13 @@ jobs:
     runs-on: windows-latest
 
     steps:
+      # Scheduled runs always test dev (schedules execute from the default branch, so ref_name
+      # would be main). A manual dispatch tests the DISPATCHED ref — the only way to validate a
+      # branch's gated-pg test changes before merge; the artifact-publishing build job stays
+      # pinned to dev either way, so a branch dispatch can never ship branch binaries.
       - uses: actions/checkout@v5
         with:
-          ref: dev
+          ref: ${{ github.event_name == 'workflow_dispatch' && github.ref_name || 'dev' }}
 
       - name: Setup .NET 10.0
         uses: actions/setup-dotnet@v5
@@ -250,14 +254,18 @@ jobs:
           $bin     = "$env:GITHUB_WORKSPACE\Darling\artifacts\pg-runtime\pgsql\bin"
           $dataDir = "$env:RUNNER_TEMP\darling-pgdata"
           $logFile = "$env:RUNNER_TEMP\darling-pg.log"
-          & "$bin\initdb.exe" -D $dataDir -U postgres -A trust --encoding=UTF8
+          # The bootstrap superuser is named "darling", not "postgres": the V8 schema-split
+          # migration runs CREATE SCHEMA ... AUTHORIZATION darling (PgSchemaGenerator.OwnerRole),
+          # so a cluster without that role fails every fresh-store migration. Matching managed
+          # mode's shape (DarlingManagedPostgres also initdbs its owner as "darling").
+          & "$bin\initdb.exe" -D $dataDir -U darling -A trust --encoding=UTF8
           if ($LASTEXITCODE -ne 0) { throw "initdb failed ($LASTEXITCODE)" }
           Add-Content -Path "$dataDir\postgresql.conf" -Value "shared_preload_libraries = 'timescaledb'"
           Add-Content -Path "$dataDir\postgresql.conf" -Value "port = 5541"
           Add-Content -Path "$dataDir\postgresql.conf" -Value "listen_addresses = '127.0.0.1'"
           & "$bin\pg_ctl.exe" -D $dataDir -l $logFile -w start
           if ($LASTEXITCODE -ne 0) { if (Test-Path $logFile) { Get-Content $logFile -Tail 50 }; throw "pg_ctl start failed ($LASTEXITCODE)" }
-          & "$bin\createdb.exe" -h 127.0.0.1 -p 5541 -U postgres darling
+          & "$bin\createdb.exe" -h 127.0.0.1 -p 5541 -U darling darling
           if ($LASTEXITCODE -ne 0) { throw "createdb failed ($LASTEXITCODE)" }
 
       # DARLING_TEST_PG lights up the [Collection("live-postgres")] classes; DARLING_TEST_PGRUNTIME
@@ -266,7 +274,7 @@ jobs:
       - name: Run Darling PG tests
         shell: pwsh
         env:
-          DARLING_TEST_PG: "Host=127.0.0.1;Port=5541;Username=postgres;Database=darling"
+          DARLING_TEST_PG: "Host=127.0.0.1;Port=5541;Username=darling;Database=darling"
           DARLING_TEST_PGRUNTIME: ${{ github.workspace }}\Darling\artifacts\pg-runtime
         run: dotnet test Darling/Darling.Tests/Darling.Tests.csproj -c Release --no-build --verbosity normal --logger "trx;LogFileName=darling-nightly.trx" --results-directory TestResults
 

CHANGELOG.md

@@ -15,6 +15,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - **Darling viewer: stop the hard crash when interacting with the system-tray icon (Hardcodet TrayToolTip issue #422)** ([#1538]) — hovering or clicking the tray icon could throw `ArgumentException: The root Visual of a VisualTarget cannot have a parent` from `Popup.CreateWindow` on the custom `TrayToolTip`, unhandled, killing the process (the tray icon just vanished). Lite has suppressed this since `562269f6` / `50257182` via an `IsTrayToolTipCrash` check in BOTH its Dispatcher (`e.Handled = true`) and AppDomain handlers, but the later port of Lite's tray code to the Darling viewer (`f25fe3a0`) copied the crash-prone custom `TrayToolTip` Border AND the comment claiming the handler suppresses the crash WITHOUT porting the actual suppression — a parity gap that left the viewer's `App.xaml.cs` handlers logging-and-dying. This ports Lite's exact `IsTrayToolTipCrash` detector + the Dispatcher (`e.Handled`) and AppDomain suppression into the viewer and corrects the stale comment, restoring Lite↔Darling parity with the proven handling.
 - **Charts: increase the shared top headroom from 5% to 15% so a series that plateaus at a hard ceiling no longer reads as clipped at the top** ([#1538]) — `ChartStyle.ComputeYLimitsWithLegendPadding` gave only 5%-of-range breathing room above the data max, so the CPU Scheduler pressure chart's Blocked-Tasks series (pinned at the scheduler count under HammerDB load) sat at ~95% of the plot height and looked cut off. Bumped the top padding to 15% (the data max now lands at ~87%); the pinned Y-limit math tests in `Lite.Tests` + `Dashboard.Tests` are updated. Applies to all charts via the shared `Ui.ChartStyle` (Lite / Darling / the deprecated Dashboard) — one consistent, more comfortable headroom, no per-app drift.
 
+- **The gated live-PostgreSQL test suite runs green for the first time — one real grant gap and five latent test defects** ([#1551]) — the nightly `darling-pg` job had never actually executed: it was added July 2, but scheduled nightly runs execute the workflow file from `main`, which predates the job entirely, so its first real run was a manual dispatch on July 17 — which failed wall-to-wall. Fixed, in dependency order: **(1) the throwaway cluster now initdbs its superuser as `darling`** (it used `postgres`, but the V8 schema-split migration runs `CREATE SCHEMA ... AUTHORIZATION darling`, so every fresh-store migration died with `role "darling" does not exist`); with migrations unblocked, the suite exposed **(2) a real viewer-role grant gap** — V18 added `alert_delivery_mode_override` to `config_monitored_servers` without adding it to `DarlingManagedRoles`' fail-closed non-secret column list, so a read-only `viewer` seat (including a remote network viewer, whose pg_hba default is `viewer`) was silently denied the per-server delivery-mode override column; the live ratchet test caught it exactly as designed, and the column is now granted. The remaining four were test defects that had never run: **(3)** the two `SeedAndRead ... AgainstScratchPostgres` round-trips assumed an isolated store but ran against the shared CI database, where `SeedIfEmptyAsync` correctly no-ops after any earlier test seeds — each now mints (and drops) its own scratch database; **(4)** the system_health planted-events test INSERTed a `collection_id` column that never existed on `system_health_events` (its id column is `system_health_event_id`); **(5)** the managed-bootstrap E2E derived its expected `max_worker_processes` from the catalog count while the product correctly sizes from `TimescaleSupport.HypertableCount` (catalog **plus** `collection_log`); and **(6)** the TimescaleDB E2E asserted `collection_log` becomes a hypertable via the V23 migration alone, but V23 is deliberately a guarded upgrade fast-path that skips when the extension does not exist yet — the test now exercises `EnsureCollectionLogHypertableAsync`, the authoritative runtime path the service calls on every start (which is also how a fresh managed store converges). No collector, store, or viewer behavior changes; the only product change is the viewer-role column grant.
+
 ### Added
 
 - **Nightly builds now include Darling** ([#1550]) — the nightly release previously carried only Dashboard, Lite, and the CLI Installer, so trying Darling on another machine meant cloning and building `dev` yourself. The nightly workflow now publishes the Darling service and viewer and packages them as `PerformanceMonitorDarling-<version>.zip` in the exact layout the stable release uses (service at the archive root with `darling.sample.json` alongside, viewer under `viewer\`, and the PostgreSQL/TimescaleDB `pg-runtime.zip` beside the service exe where first-run extraction expects it), with its SHA-256 in the existing `SHA256SUMS.txt`. The ~340MB pg-runtime fetch reuses the same content-hash cache key as the release workflow, so it downloads at most once per pin-set. Like every nightly artifact, the zip is unsigned — for signed builds use a stable release.
@@ -372,6 +374,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 [#1547]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1547
 [#1549]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1549
 [#1550]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1550
+[#1551]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1551
 [#1536]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1536
 [#1534]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1534
 [#1533]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1533

Darling/Darling.Tests/DarlingAlertTuningKnobsTests.cs

@@ -118,19 +118,23 @@ public void ApplyToConfig_SwapsNotifyConnectionChanges_ReflectedThroughTheSettin
     [Fact]
     public async Task SeedAndRead_RoundTripsLongRunningQueryReadShape_AgainstScratchPostgres()
     {
-        var connectionString = Environment.GetEnvironmentVariable("DARLING_TEST_PG");
-        Assert.SkipWhen(string.IsNullOrEmpty(connectionString),
-            "Set DARLING_TEST_PG to an ISOLATED scratch Postgres (it seeds the singleton config rows) to run the seed/read round-trip.");
+        var baseConnectionString = Environment.GetEnvironmentVariable("DARLING_TEST_PG");
+        Assert.SkipWhen(string.IsNullOrEmpty(baseConnectionString),
+            "Set DARLING_TEST_PG to a Postgres connection string to run the seed/read round-trip (the test mints its own scratch database).");
 
         var ct = TestContext.Current.CancellationToken;
 
-        await using (var connection = new NpgsqlConnection(connectionString))
+        /* SeedIfEmptyAsync no-ops on a store any earlier test already seeded, so this test needs
+           a database of its own — see ScratchPostgres. */
+        await using var scratch = await ScratchPostgres.CreateAsync(baseConnectionString!, ct);
+
+        await using (var connection = new NpgsqlConnection(scratch.ConnectionString))
         {
             await connection.OpenAsync(ct);
             await PgMigrations.MigrateAsync(connection, ct); // brings the scratch store to V20
         }
 
-        await using var dataSource = NpgsqlDataSource.Create(connectionString!);
+        await using var dataSource = NpgsqlDataSource.Create(scratch.ConnectionString);
         var provider = new StoreConfigProvider(dataSource);
 
         var config = new DarlingConfig();

Darling/Darling.Tests/DarlingDeliveryModeTests.cs

@@ -189,19 +189,23 @@ public async Task NullOverride_InheritsTheGlobalMode()
     [Fact]
     public async Task SeedAndRead_RoundTripsDeliveryMode_AndPerServerOverride_AgainstScratchPostgres()
     {
-        var connectionString = Environment.GetEnvironmentVariable("DARLING_TEST_PG");
-        Assert.SkipWhen(string.IsNullOrEmpty(connectionString),
-            "Set DARLING_TEST_PG to an ISOLATED scratch Postgres (it seeds the singleton config rows) to run the seed/read round-trip.");
+        var baseConnectionString = Environment.GetEnvironmentVariable("DARLING_TEST_PG");
+        Assert.SkipWhen(string.IsNullOrEmpty(baseConnectionString),
+            "Set DARLING_TEST_PG to a Postgres connection string to run the seed/read round-trip (the test mints its own scratch database).");
 
         var ct = TestContext.Current.CancellationToken;
 
-        await using (var connection = new NpgsqlConnection(connectionString))
+        /* SeedIfEmptyAsync no-ops on a store any earlier test already seeded, so this test needs
+           a database of its own — see ScratchPostgres. */
+        await using var scratch = await ScratchPostgres.CreateAsync(baseConnectionString!, ct);
+
+        await using (var connection = new NpgsqlConnection(scratch.ConnectionString))
         {
             await connection.OpenAsync(ct);
             await PgMigrations.MigrateAsync(connection, ct); // brings the scratch store to V18
         }
 
-        await using var dataSource = NpgsqlDataSource.Create(connectionString!);
+        await using var dataSource = NpgsqlDataSource.Create(scratch.ConnectionString);
         var provider = new StoreConfigProvider(dataSource);
 
         var config = new DarlingConfig();

Darling/Darling.Tests/DarlingManagedPostgresTests.cs

@@ -7,6 +7,7 @@
  */
 
 using System;
+using System.Globalization;
 using System.IO;
 using System.Net;
 using System.Net.Sockets;
@@ -392,7 +393,9 @@ public async Task Bootstrap_EndToEnd_FirstRunThenIdempotentSecondRun_Gated()
             var conf = File.ReadAllText(Path.Combine(dataDirectory, "postgresql.conf"));
             Assert.Contains("shared_preload_libraries = 'timescaledb'", conf, StringComparison.Ordinal);
             Assert.Contains("listen_addresses = '127.0.0.1'", conf, StringComparison.Ordinal);
-            Assert.Contains($"max_worker_processes = {3 + (TimescaleSupport.HypertableTables.Count + 2) + 8}", conf, StringComparison.Ordinal);
+            /* HypertableCount, not HypertableTables.Count: the product sizes workers from the TRUE
+               hypertable count (catalog + collection_log, the V23 non-catalog hypertable). */
+            Assert.Contains($"max_worker_processes = {3 + (TimescaleSupport.HypertableCount + 2) + 8}", conf, StringComparison.Ordinal);
 
             /* v3 memory sizing rode the SAME append path on first run, derived from THIS host's physical RAM
                (the exact MB depend on the runner, so pin the marker + that the settings are present). */
@@ -414,7 +417,10 @@ public async Task Bootstrap_EndToEnd_FirstRunThenIdempotentSecondRun_Gated()
                 Assert.True(await reader.ReadAsync(timeout.Token));
                 Assert.Equal("darling", reader.GetString(0));
                 Assert.Equal("darling", reader.GetString(1));
-                Assert.Equal("40", reader.GetString(2));
+                /* Derived, not hard-pinned (a "40" pin from the 27-hypertable era went stale when
+                   collectors were added): the same HypertableCount formula BuildWorkerSizingConfAppend
+                   writes into the conf, proven LIVE here. */
+                Assert.Equal((3 + (TimescaleSupport.HypertableCount + 2) + 8).ToString(CultureInfo.InvariantCulture), reader.GetString(2));
                 /* The v3 memory block is LIVE, not merely written: work_mem and shared_buffers hold our
                    derived values (>= the 16 MB work_mem floor / 25%-of-RAM shared_buffers on any real host),
                    never the stock 4 MB / 128 MB defaults. */

Darling/Darling.Tests/DarlingMcpHealthParserToolsTests.cs

@@ -321,7 +321,7 @@ public async Task HealthParserTools_ReadPlantedEvents_AgainstDevPostgres()
 
             async Task PlantEvent(string eventType, string fixture) =>
                 await DarlingMcpTestData.ExecAsync(connection, ct,
-                    @"INSERT INTO system_health_events (collection_id, collection_time, server_id, server_name, event_time, event_type, event_xml)
+                    @"INSERT INTO system_health_events (system_health_event_id, collection_time, server_id, server_name, event_time, event_type, event_xml)
 VALUES ($1,$2,$3,$4,$5,$6,$7)",
                     CollectionIdGenerator.Next(), t, ServerId, ServerName, t, eventType, LoadFixture(fixture));
 

Darling/Darling.Tests/DarlingSecuritySplitLiveTests.cs

@@ -66,8 +66,13 @@ public async Task V8_MovesTablesToCollectAndConfig_AndBareNamesResolve()
         Assert.Equal("config", await SchemaOfAsync(connection, "config_mute_rules", ct));
 
         /* The bare, unqualified name every SQL site uses resolves — proof the search_path works, so
-           no query file had to be re-qualified. A fresh connection inherits the database default. */
-        await using var fresh = new NpgsqlConnection(connectionString);
+           no query file had to be re-qualified. A fresh connection inherits the database default.
+           Pooling=false: under parallel xunit, the pool can hand back a physical connection opened
+           BEFORE this run's ALTER DATABASE ... SET search_path landed (its session default predates
+           it), which fails the bare SELECT with 42P01 — a pool artifact, not what this asserts. A
+           genuinely new connection is the thing the comment above claims to prove. */
+        var freshConnectionString = new NpgsqlConnectionStringBuilder(connectionString) { Pooling = false }.ConnectionString;
+        await using var fresh = new NpgsqlConnection(freshConnectionString);
         await fresh.OpenAsync(ct);
         using var bare = new NpgsqlCommand("SELECT count(*) FROM config_mute_rules", fresh);
         Assert.NotNull(await bare.ExecuteScalarAsync(ct));

Darling/Darling.Tests/ScratchPostgres.cs

@@ -0,0 +1,69 @@
+using System;
+using System.Threading;
+using System.Threading.Tasks;
+using Npgsql;
+
+namespace Darling.Tests;
+
+/// <summary>
+/// Mints an isolated scratch DATABASE on the DARLING_TEST_PG server for tests that seed the
+/// singleton config rows. <c>StoreConfigProvider.SeedIfEmptyAsync</c> deliberately no-ops on a
+/// store any earlier test already seeded, so running these tests against the one shared CI
+/// database made them order-dependent (caught live: the V20 round-trip read the V3 defaults an
+/// earlier test seeded). Each such test gets its own database instead — created here, dropped on
+/// dispose with <c>WITH (FORCE)</c> so a lingering connection can never wedge the drop.
+/// <c>Pooling=false</c> on the scratch connection string keeps in-process pooled connections from
+/// pinning the database in the first place.
+/// </summary>
+internal sealed class ScratchPostgres : IAsyncDisposable
+{
+    private readonly string _adminConnectionString;
+
+    public string DatabaseName { get; }
+
+    public string ConnectionString { get; }
+
+    private ScratchPostgres(string adminConnectionString, string databaseName, string connectionString)
+    {
+        _adminConnectionString = adminConnectionString;
+        DatabaseName = databaseName;
+        ConnectionString = connectionString;
+    }
+
+    public static async Task<ScratchPostgres> CreateAsync(string baseConnectionString, CancellationToken cancellationToken)
+    {
+        /* Hex-only generated name: safe to interpolate as a quoted identifier. */
+        var databaseName = "darling_scratch_" + Guid.NewGuid().ToString("N")[..12];
+
+        await using (var admin = new NpgsqlConnection(baseConnectionString))
+        {
+            await admin.OpenAsync(cancellationToken);
+            await using var create = new NpgsqlCommand($"CREATE DATABASE \"{databaseName}\"", admin);
+            await create.ExecuteNonQueryAsync(cancellationToken);
+        }
+
+        var builder = new NpgsqlConnectionStringBuilder(baseConnectionString)
+        {
+            Database = databaseName,
+            Pooling = false,
+        };
+
+        return new ScratchPostgres(baseConnectionString, databaseName, builder.ConnectionString);
+    }
+
+    public async ValueTask DisposeAsync()
+    {
+        try
+        {
+            await using var admin = new NpgsqlConnection(_adminConnectionString);
+            await admin.OpenAsync();
+            await using var drop = new NpgsqlCommand($"DROP DATABASE IF EXISTS \"{DatabaseName}\" WITH (FORCE)", admin);
+            await drop.ExecuteNonQueryAsync();
+        }
+        catch
+        {
+            /* Best-effort: a leaked scratch database on a throwaway test cluster is harmless,
+               and failing a passing test in its cleanup would invert the signal. */
+        }
+    }
+}