Make the darling-pg nightly job green: bootstrap role + one grant gap + four latent test fixes - #1551
Merged
Merged
Conversation
The job initialized its test cluster as user postgres, but the V8 schema-split migration runs CREATE SCHEMA ... AUTHORIZATION darling (PgSchemaGenerator.OwnerRole), so every fresh-store migration - and with it the entire gated live-PostgreSQL suite - failed with 42704 role "darling" does not exist. Nobody saw it for two weeks: the job was added 2026-07-02, V8 landed 2026-07-04, and scheduled nightlies execute main's workflow file (which predates the job), so its first real execution was today's manual dispatch. initdb -U darling matches the managed bundled store's owner shape (DarlingManagedPostgres also bootstraps its superuser as darling); createdb and DARLING_TEST_PG follow. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
With the darling role fix in place, the gated live-PostgreSQL suite executed for the first time and surfaced one real grant gap and four latent test defects (none of these tests had ever run - they are gated on DARLING_TEST_PG, and the CI job that sets it never fired): - DarlingManagedRoles: V18 added alert_delivery_mode_override to config_monitored_servers without adding it to the fail-closed non-secret grant list, so a read-only viewer seat was silently denied the column. The live ratchet caught it as designed. Real product fix; the derived SQL pins follow automatically. - ScratchPostgres (new): the two SeedAndRead AgainstScratchPostgres tests assumed an isolated store but ran against the shared CI database, where SeedIfEmptyAsync correctly no-ops after an earlier test seeds (17 vs 5 = reading DarlingAlertingTests' seed). Each now mints and force-drops its own scratch database. - DarlingMcpHealthParserToolsTests: the planted INSERT named a collection_id column that never existed on system_health_events (its prefix id column is system_health_event_id). - DarlingManagedPostgresTests: derive expected max_worker_processes from TimescaleSupport.HypertableCount (catalog + collection_log), matching BuildWorkerSizingConfAppend, not HypertableTables.Count. - TimescaleSupportTests: V23 is a guarded upgrade fast-path that skips when the extension does not exist at migration time (true in the shared CI database and on any fresh managed store); call the authoritative EnsureCollectionLogHypertableAsync runtime path the service runs on every start, then assert the hypertable. Ungated suite: 2148 passed / 0 failed locally. The gated suite gets its verdict from the validation dispatch of this branch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
erikdarlingdata
changed the title
Two more from the darling-pg campaign: - DarlingSecuritySplitLiveTests.V8_MovesTables...: the "fresh connection inherits the database default search_path" assertion drew from the Npgsql pool, which can hand back a physical connection opened BEFORE the best-effort ALTER DATABASE ... SET search_path landed (its session default predates it) - 42P01 on the bare SELECT, order-dependent under parallel xunit (passed run 2, failed run 3, same code). Pooling=false makes the connection genuinely new, which is the thing the comment claims to prove. - nightly.yml: the darling-pg job now checks out the DISPATCHED ref on workflow_dispatch (dev on schedule, as before). Run 3 proved the gap: it ran my branch's workflow against dev's test code, so branch fixes to gated tests were untestable before merge. The artifact-publishing build job stays pinned to dev either way, so a branch dispatch can never ship branch binaries. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Validation run 3 (the first to test this branch's own code) got the
gated suite down to ONE failure: the same bootstrap E2E, one
assertion past the conf check my earlier fix corrected. The
current_setting('max_worker_processes') liveness assertion was
hard-pinned to "40" from the 27-hypertable era; the server correctly
runs with 49 today. Derive it from the same HypertableCount formula
BuildWorkerSizingConfAppend writes, so it proves the setting is LIVE
without ever going stale again.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 17, 2026
argpna
pushed a commit
to argpna/PerformanceMonitor
that referenced
this pull request
Jul 26, 2026
…rikdarlingdata#1639) The fail-closed viewer column ACL (erikdarlingdata#1262) is authored once in C# (DarlingManagedRoles.ViewerRestrictedConfigTables) and applied two ways. Managed mode GENERATES the REVOKE-then-GRANT-columns carve from that list on every service start, so it is correct by construction. BYO mode has no service to generate anything: the operator runs Darling/tools/provision-roles.sql by hand, and its three GRANT SELECT (...) lists are a hand copy of the same columns. Nothing pinned the two together and they drifted: config_monitored_servers MISSING alert_delivery_mode_override (erikdarlingdata#1551) config_notification MISSING generic_body_template (erikdarlingdata#1506) config_notification MISSING generic_proxy (erikdarlingdata#1506) config_command in sync Because the carve is fail-CLOSED, an unlisted column is invisible, so on a BYO store a read-only viewer seat took a bare SQLSTATE 42501 on the two projections written specifically FOR it: MonitoredServersSelectSql (the Manage Servers list + sidebar reconcile) and NotificationSelectNoSecretSql (the Settings prefill, where one throw blanks every later section). Managed deployments were never affected. Only the three drifted columns are added - no grant is widened. The real defect is the unguarded hand mirror, so add an UNGATED ProvisionRolesAclDriftTests: it parses the SHIPPED .sql (copied beside the test binary by the csproj, so there is no stale second copy and no hardcoded repo path), extracts every GRANT SELECT (columns) ON schema.table TO role, and asserts set-equality against the C# list in both directions, plus the paired REVOKE and its ordering, that no secret column is ever granted, and that the script still provisions admin+viewer only (BYO deliberately has no mcp role). A meta-test mutates the real file three ways and asserts the same comparison reports each, so a parser that silently matched nothing cannot pass as "no drift". Darling.Tests: 2958 passed, 0 failed, 147 gated-live skipped (DARLING_TEST_PG cleared). Verified by hand that removing either restored column reintroduces a failing test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
pull Bot
pushed a commit
to ehtick/PerformanceMonitor
that referenced
this pull request
Jul 29, 2026
…hat executes Scheduled workflows always execute the DEFAULT branch's copy of the workflow file, while nightly artifacts deliberately build from dev's tree. That skew is exactly how the 2026-07-26 scheduled nightly failed (run 30194606068): main's stale copy still read Dashboard/Dashboard.csproj, a path erikdarlingdata#1612 moved to deprecated/ on dev, so 'Set nightly version' died on a file missing from the tree it had just checked out. Same trap as before (erikdarlingdata#1550/erikdarlingdata#1551) - syncing the file again would fix tonight and re-break at the next pre-release layout drift. Structural fix instead: on schedule the workflow does nothing but re-dispatch itself onto the dev ref (GITHUB_TOKEN can create workflow_dispatch runs; the Actions recursion guard exempts workflow_dispatch, and the dispatched run cannot loop back because it arrives as workflow_dispatch, not schedule). A workflow_dispatch run executes the DISPATCHED ref's copy of the file - dev's, current by definition - so after a one-time sync of this file to main, main's copy has exactly one tree-independent job that must keep working, and nightly logic changes take effect the night they merge to dev. Semantics preserved: manual dispatches (from_schedule defaults false) always build, exactly as before; the scheduled path keeps the 24h new-commit check via from_schedule=true; the artifact-publishing build job stays pinned to ref dev so a branch dispatch still cannot ship branch binaries. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The nightly
darling-pgjob had never actually executed — scheduled nightlies run the workflow file frommain, which predates the job — so its first real run (today's dispatch validating #1550) failed wall-to-wall. This PR makes it green, in dependency order:1. Bootstrap role (workflow)
The job initdb'd its throwaway cluster as
postgres, but the V8 schema-split migration runsCREATE SCHEMA ... AUTHORIZATION darling(PgSchemaGenerator.OwnerRole) — every fresh-store migration died with42704: role "darling" does not exist. The cluster now initdbs its superuser asdarling(the managed bundled store's own owner shape);createdbandDARLING_TEST_PGfollow.2. Real product fix: viewer-role grant gap (
DarlingManagedRoles)With migrations unblocked, the suite ran and the fail-closed column-ACL ratchet fired exactly as designed: V18 added
alert_delivery_mode_overridetoconfig_monitored_serverswithout adding it to the non-secret grant list, so a read-onlyviewerseat (including a remote network viewer, whose pg_hba default role isviewer) was silently denied that column. Now granted. All SQL pins derive from the list, so they follow automatically.3. Four latent test defects (never-run tests, gated on
DARLING_TEST_PG)ScratchPostgres(new helper) — the twoSeedAndRead ... AgainstScratchPostgresround-trips assumed an isolated store but ran against the one shared CI database, whereSeedIfEmptyAsynccorrectly no-ops after any earlier test seeds (the observed17 vs 5was a read ofDarlingAlertingTests' seed). Each now mints its own database and force-drops it on dispose.DarlingMcpHealthParserToolsTests— planted INSERT named acollection_idcolumn that never existed onsystem_health_events(its prefix id column issystem_health_event_id).DarlingManagedPostgresTests— expectedmax_worker_processesderived fromHypertableTables.Count; the product correctly sizes fromTimescaleSupport.HypertableCount(catalog + 1 forcollection_log).TimescaleSupportTests— assertedcollection_logbecomes a hypertable via the V23 migration alone, but V23 is a deliberately guarded upgrade fast-path that skips when the extension doesn't exist at migration time (true in the shared CI database and on any fresh managed store). The test now callsEnsureCollectionLogHypertableAsync— the authoritative runtime path the service runs on every start — then asserts.Testing
Darling.Tests: 2148 passed / 0 failed locally (Release).darling-pgjob green (its first-ever pass).🤖 Generated with Claude Code