Stop the compression detector reading TimescaleDB's never-ran sentinel as year 1 (#1760)

CHANGELOG.md

@@ -88,6 +88,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - **The directories cluttering a field instance's install folder were invisible to the report built to find them** ([#1785], extends [#1770]) - [#1775]'s report verified SILENT on the production instance: it scans siblings of the DATA directory under `%ProgramData%`, and the seven directories are under the INSTALL directory - a different parent entirely. They would have stayed invisible even in the right place, because that report identifies a store copy structurally by its `PG_VERSION` file and the arithmetic rules clusters out (a ~286 GB data directory on a ~500 GB volume with ~175 GB free - seven copies do not fit), so they are small hand-made snapshots with no store semantics at all. A second report now covers the install directory's top level: any directory the product's layout does not account for is named with its size once per start, and **never deleted** - an absolutism that matters more here than for store copies, because this classifies by ELIMINATION against a known layout rather than by any positive test, so it will sometimes be wrong by construction and reporting is the only verdict that stays harmless when it is. It carries no store semantics either: it can only say the product did not put a directory there, so that is all it says. The known layout is enumerated from the shipped artifacts and measured (`viewer` from the packaging step, `runtimes` and the culture directories from an actual publish, `wwwroot` from the csproj, `pg-runtime` and `pg-runtime-prev` from the runtime paths); locale directories are matched STRUCTURALLY - nothing but `*.resources.dll` - rather than by culture name, because that set is decided by which cultures our dependencies localize into and a hardcoded list would have the product reporting its own shipped directory as foreign. Its own 5-second whole-report budget rather than one shared with the store report, so one report's cost can never degrade another report's content; an exhausted budget degrades a directory's SIZE to "at least" and never drops the directory itself.
 - **Four test classes shared the live Postgres store without serializing against it, and three more were blamed for it by a substring match** ([#1783], closes [#1776]) - classes that read the shared `DARLING_TEST_PG` database without `[Collection("live-postgres")]` ran in parallel with the sixty-odd that have it, and with each other. Three consecutive full-suite runs against one long-lived database failed on a DIFFERENT unrelated class each time, which is the expensive shape of flake: it looks exactly like the change under test broke something it never touched. **CI cannot see it** - it creates a throwaway cluster per run - so the whole cost lands on local development and surfaces as someone else's problem in someone else's pull request, which is why it survived. The classification was verified against the source rather than taken as given, and it moved in both directions. **The most aggressive offender was not on the list**: `DarlingComposeTests` connects to the shared database and runs `PgMigrations.MigrateAsync` - DDL - against it. It does NOT simply take the attribute, because the class holds 127 tests of which 126 are pure and serializing all of them to protect one is the slowdown the issue itself warns against; the live test is split into its own class instead, the shape more than forty files here already use, which also makes the parent's existing claim that "the live round-trip is exercised elsewhere" true rather than aspirational. **Three of the six named were substring artifacts**: `DarlingManagedPostgresTests`, `DarlingStoreUpgradeTests` and `DarlingPgRuntimeVersionPinTests` never read `DARLING_TEST_PG` at all - they read `DARLING_TEST_PGRUNTIME` (and `_OLD` / `_NEWZIP`), different variables that merely share the prefix - so "do not serialize them" was the right answer reached for the wrong reason, and each now records which reason applies so the next sweep does not re-derive the false positive. Two further classes were missed and are genuinely exempt: they reach the variable only to CREATE and DROP their own database through `ScratchPostgres`. **The "transaction-rolled-back" comments those classes carried read like an exemption and are not one** - rolling back protects the DATA, not the CONCURRENCY, since an uncommitted write still holds its row locks and still fires the `config_version` bump trigger; that is now recorded on each, because the comment otherwise invites exactly the wrong conclusion. A new meta-test requires every class reading the shared store to either carry the attribute or record an `#1776 own-store` exemption - a DECISION, not the attribute, since a class with its own database should not serialize - and it matches the QUOTED literal rather than a substring, which is what keeps the three runtime classes and bare prose mentions out; matching the way the original sweep did would leave it permanently red on three innocent classes. It fails rather than skips when it cannot find the source tree, following the doc-comment pin, because a guard that silently skips is a guard that silently stops guarding. Test infrastructure only; no production code changes
 - **The config backups handed a second readable copy of every secret to any interactively-logged-on user** ([#1786], closes [#1769]) - a `darling.json.bak-*` is a byte-for-byte copy of the config: every monitored server's `encryptedPassword`, the MCP bearer token, the web access token, all DPAPI **LocalMachine** blobs with an entropy constant that ships in an open-source repo, so READ access IS the secret. Both the service's CLI verbs and `install-darling.ps1` granted those backups `NT AUTHORITY\INTERACTIVE` read, mirroring the live config. **Nothing reads a backup**: the only references to the `.bak-` name in non-test code are the two lines that construct it, and restoring one is a hand operation that already requires elevation because writing `darling.json` does - INTERACTIVE only ever had Read there, never Write. The grant bought nothing. Both paths now drop it, so the installer and the service cannot disagree, and the installer's summary states which file gets which posture instead of claiming one rule for both. **The live `darling.json` deliberately keeps its INTERACTIVE grant**, and the decision about tightening it moves to [#1792] rather than closing with this: removing it would break the shipped Viewer. `ViewerSettings.TryLoad` reads the config directly, the Viewer manifest is `asInvoker`, and under UAC an interactive administrator's default token carries `BUILTIN\Administrators` as SE_GROUP_USE_FOR_DENY_ONLY - so a DACL of exactly SYSTEM + Administrators + the service identity denies it outright, and the window shows "darling.json could not be read" with no data at all. That is the DEFAULT shipped layout, where the Viewer sits in `viewer\` under the service root and probes the parent for the service's config. The security rationale still holds - the `admin` role's SELECT carves out the secret columns, so the config file really is the only interactive-readable path to the SQL passwords and tokens - but closing it needs the Viewer given a non-secret path to the `postgres` section it actually wants, which is a design decision rather than a one-line ACL edit. Pinned two ways, because the failure modes differ: an exactly-three-identities assertion that NAMES an unintended principal (presence/absence checks pass happily while a fourth rides along, which is the reported defect's exact shape), and an end-to-end assertion through the real `WriteWithBackupAsync` path, because what was wrong was a CALL SITE passing the wrong argument and a test of the ACL shape alone cannot see that
+- **The compression self-heal read TimescaleDB's never-ran sentinel as a run that started in year 1** ([#1781], closes [#1760]) - filed as a nightly test flake, and it was a real defect in the detector behind the compression self-heal (#1585/#1586). `timescaledb_information.job_stats.last_run_started_at` is **-infinity, not NULL**, for a job that has never run - measured, along with the fact that a freshly added policy has no `bgw_job_stat` row at all and it is `alter_job` that materialises the row carrying the sentinel. Npgsql maps `-infinity` to `DateTime.MinValue`, so the stuck-Running arm computed an elapsed of roughly 739,000 days, cleared every bound, and reported a perfectly healthy job as "stuck in the Running state for 388,000,000 minutes". **That was reachable because `job_status` and `last_run_started_at` come from independent sources in TimescaleDB's own view**: `job_status` is `CASE WHEN pg_stat_activity.state = 'active' THEN 'Running'` joined on `application_name`, while `last_run_started_at` is `bgw_job_stat.last_start`. A job's FIRST run therefore reads `Running` while its start time is still the sentinel, so the window is structural rather than hypothetical - and every job on a newly provisioned store is in exactly that state. The consequence in the field was the self-heal re-arming a job that was running fine; it is self-limiting, since the sentinel clears once the first run finishes, but a "compression job stuck" alert on a fresh or freshly upgraded store may have been false, and that implausible minute count is the signature. The detection query becomes a const (the `RearmJobSql` idiom already in that file) and `NULLIF`s the sentinel, so BOTH `-infinity` tests run in SQL rather than through Npgsql's infinity mapping - the discipline the code already claimed for `next_start`; the pure predicate rejects `DateTime.MinValue` as a second line of defence. **The test's settle-wait had been hardened once for this exact class and failed anyway, and the reason is worth recording**: it polled `next_start` only - one of the two arms the detector evaluates - and the value it polled was the one the test's own `alter_job` had written one statement earlier. Instrumented, it returned after **POLLS=1, DELAYS=0**: it never waited at all, so no timeout increase could ever have helped, and the alternative hypothesis (a slow runner letting the wait time out quietly) is refuted outright because that path asserts loudly with a different message than the one observed. The wait now polls `ReadStuckCompressionJobsAsync` itself, so there is ONE predicate rather than two copies that drift and settled-according-to-the-wait IS settled-according-to-the-assertion. The live leg also gained the assertion it always claimed to make: the detector is failure-isolated, so a broken query returns an EMPTY list and `DoesNotContain` passed just as happily against SQL that never compiled - it now runs the production const directly, where an error throws, and requires the job to be present. Verified by mutation against a live PostgreSQL 18 + TimescaleDB 2.28.1 store: removing the `NULLIF` turned the **live** test red, which is what proves the sentinel is really `-infinity` and the guard is not dead code
 - **One stuck rollback copy froze the ageing-out of every other one** ([#1775], closes [#1770]) - reported from a production field instance carrying seven multi-GB store copies on a volume with roughly 175 GB free against a 286 GB data directory. The retained-copy sweep's failure handling sat OUTSIDE its loop, so a single copy that could not be measured or deleted - a file a not-yet-exited postmaster or an antivirus scan still holds, an ACL the service account lost - abandoned the sweep for every other copy too, and kept abandoning it for as long as the condition lasted. Their retention counters stopped advancing with it, so nothing aged out at all. Each copy now takes its own turn inside its own handler: a failure costs that one directory, names it in the log, and is retried next start. **The seven directories in the report were not ours** - `_rollback_manual_` appears nowhere in the product, and `DarlingStoreUpgrade.cs`, which owns the whole retain-two-starts contract, was created by [#1718]'s work days after they were made. They were made by hand, so they are now REPORTED rather than deleted: any sibling of the data directory holding a `PG_VERSION` file that this service did not create is named every start with its size and a running total, because a copy someone made by hand is a decision to reverse deliberately, not something a service should silently undo - but tens of gigabytes going unmentioned is how a volume reaches that state unnoticed. Identification is structural (`PG_VERSION`), never a name pattern, and the delete stays scoped to the names the upgrade itself produces, re-checked against the prefix because `Directory.GetDirectories` also matches Windows 8.3 short names. A half-built `<datadir>-upgrade-NN` cluster left by an interrupted upgrade gets its own line rather than being blamed on someone else - it is one of ours, deleted only best-effort by the upgrade's failure path - and it is still not removed automatically, because the commit point is two directory moves and a process that died between them leaves the UPGRADED cluster under that name.
 - **The restart delta seed read the whole store to find rows it was about to throw away** ([#1774], closes [#1772]) - reported from a production field instance, where the collector runner logged *"Failed to seed delta calculator from Postgres store... Exception while reading from stream"* on every start, including the one right after an upgrade. The four baseline seed queries carried no time bound on either half, so the inner `MAX(collection_time)` aggregated every chunk of the hypertable and the outer row-value probe scanned them all again - on a 276 GB store, on the default 30-second command timeout. The seed catches its own failures, so what the field actually lost was invisible: restart continuity, silently degraded to first-cycle-zero deltas on every service start. **The bound costs nothing that was being used.** All 36 delta call sites pass `maxGapSeconds: 300`, and the gap policy discards any baseline older than that and returns 0 - the same value an unseeded key returns - so every row found outside a five-minute window was work whose result was already being thrown away. The read is now bounded to a 15-minute window on **both** halves of each query (either one left open reads the whole table), with the cutoff carried as a reused `$1` that both Npgsql and DuckDB resolve to a single parameter, keeping the two apps' queries byte-identical. No timeout was raised. Lite carries the same bound. `memory_grant_stats` also gains `collection_time` in its SELECT list: it was the one family seeding a null timestamp, which disarms the gap policy for two monotonic counters where a stale baseline reads as a fabricated spike. Pinned by mutation on both sides, including against a live PostgreSQL 18.4 + TimescaleDB store, where dropping just the outer bound puts a two-day-old chunk back in the plan.
 - **The service tried to open its own firewall on every start, could never succeed, and a fresh networked install got no rule at all** ([#1773], closes [#1771]) - a production field instance logged both halves of the firewall self-heal failing for BOTH endpoints on EVERY service start: `Remove-NetFirewallRule : Access is denied` and `New-NetFirewallRule : Access is denied`, for 5152 (MCP) and 5153 (web). It was harmless *there* only because the rules already existed from an earlier manual setup. The real defect is what happens without that accident: on a genuinely fresh networked install nothing ever creates the rule, so remote MCP and dashboard clients are firewalled off with only a warning in a log file to say why - and **networked is the normal deployment mode for this product, not the exception**. The cause is structural rather than a bug in the reconcile: the service runs as the `NT SERVICE\PerformanceMonitor Darling` virtual account, which cannot modify Windows Firewall **by design**, and granting it that privilege is the wrong fix - a monitoring service should not hold it.
@@ -1804,6 +1805,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 [#1762]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1762
 [#1757]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1757
 [#1759]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1759
+[#1760]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1760
 [#1769]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1769
 [#1776]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1776
 [#1767]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1767
@@ -1817,6 +1819,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 [#1785]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1785
 [#1777]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1777
 [#1780]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1780
+[#1781]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1781
 [#1783]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1783
 [#1786]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1786
 [#1792]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1792