Move Lite's data out of the installer's install root (#1832)

.github/ISSUE_TEMPLATE/bug_report.yml

@@ -85,7 +85,7 @@ body:
     id: errors
     attributes:
       label: Error Messages / Log Output
-      description: Paste any error messages or relevant log entries. Dashboard logs are in %LOCALAPPDATA%\PerformanceMonitor\Logs\; Lite logs are in %LOCALAPPDATA%\PerformanceMonitorLite\Logs\.
+      description: Paste any error messages or relevant log entries. Dashboard logs are in %LOCALAPPDATA%\PerformanceMonitor\Logs\; Lite logs are in %LOCALAPPDATA%\PerformanceMonitorLite-Data\logs\ (on versions before the data-root move, %LOCALAPPDATA%\PerformanceMonitorLite\logs\).
       render: text
     validations:
       required: false

CHANGELOG.md

@@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Important
+
+- **Lite's data now lives OUTSIDE the install directory - and on any version older than this one, upgrading with `Setup.exe` destroys it** ([#1832]) - Lite kept everything in `%LOCALAPPDATA%\PerformanceMonitorLite`, which is also the installer's own install root. Re-running `Setup.exe` over an existing install renames that folder aside and deletes it, so an installer-based upgrade took the DuckDB store, the Parquet archive, the logs, and `settings.json` with it, in one move, before any code from the new build ran. **In-app updates were always safe** - Help > About replaces the `current\` subfolder in place and never touches the data sitting beside it - which is precisely why the loss looked random instead of reproducible. Two things always survived and still do: the monitored-server list (`%ProgramData%\PerformanceMonitorLite\config\servers.json`, machine-wide) and every password and webhook URL in Windows Credential Manager.
+
+  **The one-time caveat: reaching THIS version via `Setup.exe` still loses your data.** The installer deletes the old directory before the new build starts, so no fix shipping inside the new build can prevent it - the code that would migrate your data has not run yet when the data is deleted. Upgrade **in place** instead and nothing is lost: **Help > About** downloads and applies the update, or extract the portable ZIP over your existing copy. From this version forward `Setup.exe` is safe, because the data is no longer where the installer writes.
+
+  Data now lives in the sibling `%LOCALAPPDATA%\PerformanceMonitorLite-Data`, and the first start MOVES `config\`, `archive\`, `logs\`, `monitor.duckdb` (plus its WAL) and `alert_state.json` there from the old location automatically - a same-volume rename, so a multi-GB store transfers instantly. Nothing already in the new location is overwritten, nothing anywhere is deleted, and a `DATA-MOVED.txt` signpost is left behind pointing at the new path. The move is per-artifact rather than all-or-nothing, so a run interrupted by a locked store file finishes itself on the next start rather than stranding the store in a folder the next installer run would delete. The updater's own `Update.exe`, `current\` and `packages\` share that old folder and are deliberately left where they are.
+
 ### Added
 
 - **A blocking alert that fires on total blocked WAIT TIME, not just how many sessions are blocked** ([#1839]) - the existing gate counts blocked-process events in a rolling window, which cannot tell one session blocked for an hour from one blocked for a second: the count is 1 either way. `blockingWaitSecondsThreshold` (Lite: Settings > Alerts, "Total blocked wait"; Darling: `darling.json` + the Viewer's Settings window) sums `wait_time_ms` across the rows of the LATEST blocking snapshot and fires when that total reaches the threshold. **Defaults to 0 = off in both SKUs**, so nothing starts alerting because you took an update. It is a SNAPSHOT sum rather than a windowed one on purpose - the alert answers "how much aggregate blocked wait is happening right now", and summing a window would accumulate blocking that has already ended and could never clear while the window still held it. Unlike the count gate's rolling-window edge trigger it is LEVEL-triggered, the shape the High CPU check already uses: it re-fires every cooldown while the wait stays above the threshold and resolves with "Blocking Wait Cleared" when it drops below, so standing blocking keeps announcing itself instead of going quiet after one alert. It reports under its own "Blocking Wait Time" metric rather than reusing "Blocking Detected", so mute rules, cooldowns, and history rows for the two gates never tangle - but it carries the same blocked-process incident content, built from the rows the sweep already fetched for the count gate rather than a second query. A STALE snapshot neither fires it nor holds it active ([#1812]'s rule, at the blocking collector's cadence): a stopped collector leaves a "latest" snapshot that would otherwise read as NOW, and a level-triggered alert on frozen rows would re-fire forever - staleness resolves rather than latching, the opposite of the running-jobs treatment, because for a level condition the worse failure is staying latched on data of unknown age. The value travels as a real number ([#1830]), never parsed back out of the display text. Both gates stay under the one `blockingEnabled` toggle. Darling stores migrate to **schema v40** (one additive `NOT NULL DEFAULT 0` column) - automatic, no operator action. Two adjacent corrections ride along: DuckDB widens `SUM(BIGINT)` to HUGEINT, which the .NET driver hands back as a `BigInteger` that `Convert.ToInt64` cannot convert, so the read is cast in SQL on both stores (caught by the new round-trip test, and it would have thrown at alert-check time); and Lite's MCP `get_alert_settings` had been reporting the blocking COUNT under the key `threshold_seconds` - a name copied from the deprecated Dashboard, whose blocking threshold genuinely is seconds - which is now `threshold_count` beside the new `wait_threshold_seconds`.
@@ -17,6 +25,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **Opening the Settings window after a data loss no longer deletes the webhook URLs that survived it** ([#1832]) - `App.LoadAlertSettings` read the Teams, Slack, and generic webhook secrets out of Credential Manager at the TAIL of its `settings.json` parse, behind an `if (!File.Exists(path)) return;` early exit. With `settings.json` gone - exactly the state the install-directory wipe above produced - the load returned before those four reads ever happened, so the Settings window's webhook boxes rendered EMPTY while the credentials were still sitting in the store. Clicking Save then wrote those blanks back through `SaveWebhookUrl`, which DELETES the credential when handed a blank: the secrets that had survived the data loss were destroyed by looking at the settings window afterwards. The credential reads now run FIRST and unconditionally, before the early exit, so the boxes populate from Credential Manager whether or not `settings.json` exists and Save round-trips what it displayed. The Save path is otherwise unchanged - blank still means delete, which is how you clear a webhook on purpose. A settings-file-absent test pins that all four keys are read.
+
+- **A tray notification when an update is available, not just a title-bar suffix** ([#1832]) - the startup update check appended "Update vX available (Help > About)" to the window title and did nothing else, and a title bar nobody reads is how installs both stayed on old builds and got "upgraded" by re-downloading `Setup.exe` - the one path that deleted their data. The check now also raises one tray balloon per launch pointing at Help > About and saying that route updates in place and keeps your data. Same check, same cadence, one more surface; the update UX is otherwise untouched.
+
 - **Top Procedures collects on Azure SQL Database** ([#1833]) - `procedure_stats` was one of two database-scoped collectors that never opted into the Azure per-database connection path: it ran once on the server entry's own connection - whose catalog defaults to `master` when the entry's Database field is blank - and its Azure variant's `WHERE s.database_id = DB_ID()` then matched only master's procedures. Zero user rows, logged `SUCCESS`, empty Top Procedures grid that looked healthy, while Top Queries (which does run per database) kept working right next to it - the asymmetry that fingerprinted the bug. The collector now declares `RunsPerDatabase` on Azure like `query_stats` and its five database-scoped siblings, and the per-database connection makes `DB_ID()` mean each user database in turn - exactly what that predicate was written for. Both Lite and Darling get it (shared definition). The other collector with the same gap is Query Store, which needs a per-database rework on Azure rather than a one-line override - tracked as [#1836], with the invisible-zero-rows Collection Health gap that hid both defects tracked as [#1837].
 
 - **Chart x-axis timestamps now honor the Local/Server/UTC time toggle** ([#1831]) - Lite plots chart X values in server time and converts for display at render, but the ONE render surface doing the axis labels - the single shared `LabelFormatter` all three apps reach - never consulted the display mode, so every chart's bottom axis showed server time no matter what the toggle said, while grids, slicers, tooltips, and the crosshair all converted around it. Refresh and reconnect could not help: they re-plotted the same unconverted labels. The formatter now converts each label through the `UiTimeContext` hook Lite wired at startup for exactly this purpose and never used here; the Darling Viewer pre-converts its plotted X and deliberately leaves the hook at identity, so the change is a no-op there by construction - the double-conversion trap a Viewer-style port into Lite would have hit is documented at the site and pinned by test. Three companions ride along, all the same defect in different clothes: the display-mode toggle now re-plots the charts immediately (it refreshed nine grids and six slicers and zero charts, so even the fixed formatter only showed on the next data cycle); the Queries heatmap's hand-built axis labels convert like that chart's own tooltip always did; and the three history windows switch to the shared formatter, gaining both the conversion and the date-change labels. The deprecated Dashboard's startup hack that force-pinned the dropdown to ServerTime ("charts always render in server time") is retired - and its saved preference, which was written on every change and read never, is finally restored at startup. New tests pin the formatter's conversion, the Viewer's identity no-op, and the display-date boundary behavior - nothing anywhere tested the formatter before.
@@ -1975,3 +1987,4 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 [#1837]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1837
 [#1830]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1830
 [#1839]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1839
+[#1832]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1832

Lite.Tests/AlertSettingsCredentialLoadTests.cs

@@ -0,0 +1,168 @@
+/*
+ * 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.Collections.Generic;
+using System.IO;
+using PerformanceMonitorLite;
+using Xunit;
+
+namespace PerformanceMonitorLite.Tests;
+
+/// <summary>
+/// #1832 second defect: webhook secrets live in Windows Credential Manager, never in settings.json, but
+/// <see cref="App.LoadAlertSettings()"/> used to read them at the TAIL of its settings.json parse - behind
+/// the "no settings.json, nothing to do" early exit. So on an install whose settings.json had been
+/// destroyed (which is exactly what re-running Setup.exe over the old data root produced), the Settings
+/// window's webhook boxes rendered EMPTY while the credentials were still in the store, and Save wrote the
+/// blanks back through SaveWebhookUrl - which DELETES on blank. Looking at the settings window finished off
+/// the secrets that had survived the data loss.
+///
+/// The reads must therefore happen regardless of settings.json. That is a wiring property: both halves
+/// work fine in isolation and the bug lives entirely in the order they run in, so only a test that removes
+/// settings.json and watches which credential keys get requested can see it.
+///
+/// Shares a collection with <see cref="AppAlertSettingsTests"/> because both write the App webhook statics
+/// and xUnit runs separate classes in parallel.
+/// </summary>
+[Collection("app-webhook-statics")]
+public class AlertSettingsCredentialLoadTests
+{
+    private static readonly string[] ExpectedKeys =
+        { "TeamsWebhook", "SlackWebhook", "GenericWebhook", "GenericWebhookHeaders" };
+
+    private static string NewTempDir(string tag)
+    {
+        var dir = Path.Combine(Path.GetTempPath(), $"pmlite_{tag}_{Guid.NewGuid():N}");
+        Directory.CreateDirectory(dir);
+        return dir;
+    }
+
+    /// <summary>
+    /// The credential WRITER for every case that must not write. The real one edits Windows Credential
+    /// Manager for the user running the suite, so a case that unexpectedly reaches the #1506 legacy
+    /// migration would overwrite a live webhook URL rather than fail a test.
+    /// </summary>
+    private static void FailOnWrite(string key, string value) =>
+        Assert.Fail($"LoadAlertSettings wrote credential '{key}' when nothing should have been saved.");
+
+    /// <summary>
+    /// The regression itself: no settings.json, and all four secrets are still read and still land on the
+    /// statics the Settings window binds its boxes to.
+    /// </summary>
+    [Fact]
+    public void LoadAlertSettings_ReadsWebhookCredentials_WhenSettingsJsonIsMissing()
+    {
+        var configDir = NewTempDir("nosettings");
+        try
+        {
+            Assert.False(File.Exists(Path.Combine(configDir, "settings.json")));
+
+            var requested = new List<string>();
+            App.LoadAlertSettings(configDir, key => { requested.Add(key); return "stored:" + key; }, FailOnWrite);
+
+            foreach (var key in ExpectedKeys)
+            {
+                Assert.Contains(key, requested);
+            }
+
+            /* SettingsWindow populates its boxes straight off these, so a blank here IS the blank box that
+               Save then wrote back as a delete. */
+            Assert.Equal("stored:TeamsWebhook", App.TeamsWebhookUrl);
+            Assert.Equal("stored:SlackWebhook", App.SlackWebhookUrl);
+            Assert.Equal("stored:GenericWebhook", App.GenericWebhookUrl);
+            Assert.Equal("stored:GenericWebhookHeaders", App.GenericWebhookHeadersJson);
+        }
+        finally
+        {
+            Directory.Delete(configDir, true);
+        }
+    }
+
+    /// <summary>
+    /// A settings.json that exists but is corrupt takes the catch, which is the same trap one layer in.
+    /// </summary>
+    [Fact]
+    public void LoadAlertSettings_ReadsWebhookCredentials_WhenSettingsJsonIsCorrupt()
+    {
+        var configDir = NewTempDir("corrupt");
+        try
+        {
+            File.WriteAllText(Path.Combine(configDir, "settings.json"), "{ this is not json");
+
+            var requested = new List<string>();
+            App.LoadAlertSettings(configDir, key => { requested.Add(key); return "stored:" + key; }, FailOnWrite);
+
+            foreach (var key in ExpectedKeys)
+            {
+                Assert.Contains(key, requested);
+            }
+
+            Assert.Equal("stored:TeamsWebhook", App.TeamsWebhookUrl);
+        }
+        finally
+        {
+            Directory.Delete(configDir, true);
+        }
+    }
+
+    /// <summary>
+    /// The path that always worked keeps working: a real settings.json still gets parsed, and the secrets
+    /// still come from the credential store rather than the file.
+    /// </summary>
+    [Fact]
+    public void LoadAlertSettings_StillParsesSettings_AndStillPrefersTheCredentialStore()
+    {
+        var configDir = NewTempDir("present");
+        try
+        {
+            File.WriteAllText(
+                Path.Combine(configDir, "settings.json"),
+                "{\"teams_webhook_enabled\":true,\"teams_proxy_address\":\"proxy.sentinel.test\"}");
+
+            App.TeamsWebhookEnabled = false;
+            App.LoadAlertSettings(configDir, key => "stored:" + key, FailOnWrite);
+
+            Assert.True(App.TeamsWebhookEnabled);
+            Assert.Equal("proxy.sentinel.test", App.TeamsProxyAddress);
+            Assert.Equal("stored:TeamsWebhook", App.TeamsWebhookUrl);
+        }
+        finally
+        {
+            Directory.Delete(configDir, true);
+        }
+    }
+
+    /// <summary>
+    /// The #1506 legacy migration is unchanged by the hoist: a plaintext URL still in settings.json is
+    /// written to the credential store AND becomes the live value, which is what the old tail read-back
+    /// produced. The injected reader returns a different value, so a regression to "the stored value wins"
+    /// is visible rather than coincidentally equal.
+    /// </summary>
+    [Fact]
+    public void LoadAlertSettings_LegacyPlaintextUrl_StillOverridesTheStoredValue()
+    {
+        var configDir = NewTempDir("legacy");
+        try
+        {
+            File.WriteAllText(
+                Path.Combine(configDir, "settings.json"),
+                "{\"teams_webhook_url\":\"https://legacy.sentinel.test/hook\"}");
+
+            var written = new List<(string Key, string Value)>();
+            App.LoadAlertSettings(configDir, key => "stored:" + key, (key, value) => written.Add((key, value)));
+
+            Assert.Equal(("TeamsWebhook", "https://legacy.sentinel.test/hook"), Assert.Single(written));
+            Assert.Equal("https://legacy.sentinel.test/hook", App.TeamsWebhookUrl);
+        }
+        finally
+        {
+            Directory.Delete(configDir, true);
+        }
+    }
+}

Lite.Tests/AppAlertSettingsTests.cs

@@ -26,7 +26,12 @@ namespace PerformanceMonitorLite.Tests;
 /// in parallel — touching the shared analysis statics here could flake that class.
 /// The pass-through contract is type-uniform (every member is `=> App.X`), so the
 /// SMTP/webhook coverage proves the invariant for the analysis members too.
+///
+/// Shares a collection with <see cref="AlertSettingsCredentialLoadTests"/> for the same reason: that class
+/// drives App.LoadAlertSettings, which writes the four webhook statics this one asserts on, and xUnit runs
+/// separate classes in parallel.
 /// </summary>
+[Collection("app-webhook-statics")]
 public class AppAlertSettingsTests
 {
     /// <summary>