Prune manual-deploy rollback backups, and report them once (#2525)

CHANGELOG.md

@@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 - **Ten viewer surfaces the browser and MCP could not reach now have read endpoints** ([#2484]) - `/api/read/*` is machine-derived from the MCP tool catalog, so a read with no tool was absent from BOTH surfaces: the WPF viewer on a Windows desktop was the only way to that data, and an agent asked about it got nothing. Every one of them was a missing ENDPOINT rather than missing collection - the store already held the rows. Now served on both SKUs: `get_collection_log` (the raw per-run log under `get_collection_health`'s seven-day rollup, which is what you reach for when the rollup reads HEALTHY and collection still looks wrong), `get_current_waits_trend`, `get_blocking_stats` (severity, where the existing trends only counted incidents - ten one-second blocks and one ten-minute block are the same count and a different problem), `get_health_parser_significant_waits` (the ninth member of a family whose other eight already had one), the three Performance Trends siblings, `get_query_store_regressions`, `get_query_heatmap`, `get_lock_wait_trend` and `get_daily_summary_range` (without which the web calendar could only ever show today). One correction came out of it: the viewer's execution-count trend was a DUPLICATE of a number `get_query_duration_trend` already returned rather than a missing sibling, so it did not become an eleventh read - what it got instead was `executions_per_second`, because that count shipped truncated to a `long` and a server running 0.4 executions a second reported **zero** and read as idle.
+- **A supported in-place upgrade script, because the step that had no script was the one accumulating 5.48 GB** ([#2525]) - `install-darling.ps1` registers a service and `uninstall-darling.ps1` removes one; laying a new build over an old one was a procedure that lived in people's heads and in ad-hoc SSM scripts, and the part of it nobody remembered by hand was deleting the backups from the last twenty deploys. A dogfood box was found carrying **46 `_rollback_manual_*` directories, 5.48 GB, the oldest three weeks old** - and the [#2185] install-location report, doing exactly its job, named every one of them on every start. **46 warnings for something our own procedure created is a guard that has stopped guarding by being too loud**: a real layout problem - a stray DLL, a half-extracted upgrade - arrives as warning 47 in a list of 46 identical ones, which is the same failure as a pin that never bites wearing the opposite clothes. So both halves changed. `upgrade-darling.ps1` ships in the zip and owns retention **at deploy time**, keeping the newest `-KeepRollbacks` (3 - enough for "the release, the one before, and the one before that"; the fourth can only roll back to a version nobody wants) and pruning the rest AFTER taking the new backup, so the tree never passes through a moment with fewer rollback points than retention promises. It verifies the source zip's SHA256, refuses to copy while anything is still running out of the install tree - **naming what, never killing it**, because the bundled PostgreSQL lives under `pg-runtime` and a blanket kill takes the store down - refuses to run from the install directory itself (the copy would overwrite the script PowerShell is reading), confirms `darling.json` is byte-identical afterwards, and is safe to re-run at every step: a backup taken in the last `-BackupWindowMinutes` is REUSED rather than replaced, so a re-run after a failed extract cannot overwrite the good pre-upgrade copy with a copy of a half-upgraded tree. And the service now RECOGNISES the convention rather than deleting anything - it never deletes what it did not create - reporting the whole set on **one** line with a count, a total, the oldest, and `upgrade-darling.ps1 -PruneOnly`, informational within retention and a warning past it. That half matters even where retention is running, because the boxes carrying the backlog got it before any script pruned anything and no upgrade removes it. The naming convention is a single shared constant with the script's own predicate executed against the service's in CI, because two spellings of one convention is how this happened in the first place - and the drift would be silent, since each of those 46 warnings was true. Two defects were found by running the script's functions against planted trees rather than reading them: a backup dated in the FUTURE (clock skew) read as "recent" and would have suppressed the deploy's backup entirely, and a failure while composing a LOG LINE was caught by the delete's handler and reported as a failed delete - two removed and two failures, for the same two directories. Nothing here could be tested on real Windows from the machine that wrote it; the pins run in CI.
 - **The web dashboard gets a per-query drill-down, which is what makes `get_query_trend` reachable there at all** ([#2520]) - the read answers "is *this* query getting worse", the question you ask second and the one that decides whether you act, and it keys on a **required** `query_hash` plus a **required** `database_name`. Every panel on the per-server page fetched with nothing but a server and a window, so there was no `query_hash` anywhere on the surface to send and `/api/read/get_query_trend` could not be called from a browser - the ONLY read in the catalog whose absence was missing UI rather than a stated product boundary (the other nineteen each have one: a dedicated endpoint already serves the data, a write path the web deliberately lacks, a payload shape the table renderer cannot draw, or a desktop capability a web imitation would be worse than). The Queries tab's Top Queries table now carries a picker, in the shape the Wait Stats tab established: **the options are the rows of the table directly above it**, and an option's VALUE is that row's index into the array the table rendered - so the query trended is the same array element the reader is looking at, not a name that matched. That is deliberately not a "list every query" read: `get_wait_types` is unread on the Wait Stats tab for exactly this reason, because a full distinct set would offer queries absent from the table and make the two disagree. The chart carries avg CPU and avg elapsed only - both are milliseconds, so one y-domain holds them honestly, and both survive the hourly rollup [#2353] falls back to past the raw tier's four days; executions, DOP and the plan hash go in a grid of the per-collection snapshots below it, where the read's NULL reads as the blank it is instead of flattening a chart to a zero nobody measured. When the rollup answers, its `aggregate_note` and the truncated-window disclosure are RENDERED rather than dropped - the page's range reaches 30 days, so without them those columns simply go blank and a blank column reads as "nothing to see" rather than "not measured at this resolution". Rows carrying no `query_hash` (history predating the column) are shown in the table and not offered in the picker, because a request the read answers with a 400 is worse than an absence. A new pin asserts the category rather than the instance: every REQUIRED parameter of every read the page fetches is one the page actually sends - the existing pin only asked whether the keys sent were bound, which a read fetched with NO keys passes vacuously.
 - **The analysis family can be anchored at a past window too, and `analyze_server` refuses to persist when it is** ([#2506]) - [#2495] anchored 57 Darling reads and 50 Lite reads and deliberately left four out, and they are the four an incident investigation reaches for first: `analyze_server`, `get_analysis_facts`, `compare_analysis` and `get_analysis_findings`. Their window is not built in the tool; it is built inside the shared analysis engine, which the tools hand a bare `hours_back`. **`as_of` now reaches the engine** - same parameter name, semantics and description text as the other 107, from the same shared constant - so `analyze_server` and `get_analysis_facts` collect and score facts over the anchored window, `compare_analysis` hangs **both** of its windows off the anchor (`baseline_hours_back` has always been measured from the comparison window's end), and the anomaly detector's hour-of-day x day-of-week baseline moves with the window - which matters more than the window itself, because a run pinned to now compares Tuesday 03:00 against whatever baseline bucket today happens to be in. `get_analysis_findings` is the one whose window is on **analysis time**, so anchoring it asks what a scheduled pass was *saying* then rather than re-analyzing that window now; its read also gained an **upper** bound, without which an anchor could only ever move the window's start earlier and every anchored read would still have returned everything up to now.
 - **`analyze_server` accepts the anchor and does not persist an anchored run** ([#2506]) - it is the one tool in the family that WRITES, and an anchored run persisted normally would be worse than no anchor at all. A finding row's identity, for every consumer there is, is its `analysis_time` - the moment the pass ran, not the window it looked at: the viewers' Recommendations tab reads `MAX(analysis_time)` and calls the result the server's current state, and `get_analysis_findings` filters on `analysis_time` and then collapses on `(story_path_hash, incident_id)` to produce occurrences / first_seen / last_seen / peak_severity. So a backdated pass stamped now would become "what is wrong with this server" for every human looking at the viewer and would inflate the very occurrence stats an operator uses to judge whether a live incident is getting worse - caused, invisibly, by somebody else's exploratory read. Recording the window on the row does not fix either: `time_range_start` / `time_range_end` are **already** persisted and already returned, and no consumer filters on them. So an anchored pass is exploratory by definition - its findings come back in full, and neither the row nor the completion event that feeds notification is emitted. The rule is **derived** in the engine (`AnalysisContext.PersistFindings` is false exactly when the window was anchored) rather than a flag a caller sets, because there is no legitimate caller for "anchored AND persist" and so there should be no way to express it. The result says which it was, in `persisted` / `persistence_note`.
@@ -2871,6 +2872,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 [#2512]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2512
 [#2515]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2515
 [#2520]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2520
+[#2525]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2525
 [#2480]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2480
 [#2489]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2489
 [#2481]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2481