Consolidate UI version parse/normalize into shared VersionText (fixes latent two-part-version throw)

CHANGELOG.md

@@ -73,6 +73,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **Dashboard and Lite: the server-list version display no longer risks a crash on a two-part version string** ([#1510]) — six spots across the two apps' server-list / landing / health views hand-rolled `Version.TryParse(v, out p) ? new Version(p.Major, p.Minor, p.Build).ToString() : v`, which carries a latent throw: `Version.TryParse("3.1")` succeeds with `Build == -1`, and `new Version(3, 1, -1)` throws `ArgumentOutOfRangeException`. They are consolidated into one shared `PerformanceMonitor.Ui.VersionText` (`Parse` clamps `Build` up from -1 and strips SemVer `+build`/`-prerelease` suffixes; `Normalize` gives the three-part display form) — the UI-layer twin of the installer's `ScriptProvider.TryParseVersionCore` hardening from [#1498], now pinned by regression tests in the CI-wired `Lite.Tests`. Behavior-preserving for every realistic input (a real version or a status string); only the two-part throw changes.
 - **Clean install is Azure SQL Managed Instance-safe, and a failed drop no longer leaves the database bricked** ([#1498]) — the clean-install teardown ran `ALTER DATABASE ... SET SINGLE_USER WITH ROLLBACK IMMEDIATE` before `DROP DATABASE` to evict other connections. `SINGLE_USER` is non-modifiable on Azure SQL Managed Instance (`SERVERPROPERTY('EngineEdition') = 8`), so the whole teardown batch died on that line there; it is now gated to box SQL Server (MI drops directly, relying on there being no other sessions, which an installer teardown normally guarantees). And because `SINGLE_USER` and `DROP` are two separate autocommitted statements, a `DROP` that failed (another session racing the single-user slot) or hit the client command-timeout/cancel *after* `SINGLE_USER` committed left the database online but stranded in `SINGLE_USER` — semi-bricked. The teardown now restores `MULTI_USER` best-effort on a fresh connection before re-raising the original error (a client-attention timeout is invisible to a server-side `TRY/CATCH`, so this has to be C#-side); the Dashboard's server-drop path warns if that restore itself fails. Applies to `InstallationService.CleanInstallAsync` and the Dashboard's "remove server" drop; the SQL constructs were verified against Microsoft Learn for both box SQL Server and Managed Instance.
 - **CLI installer: a password beginning with `--` gets a clear message instead of a misleading "password required"** ([#1498]) — a value like `--h7x` passed as the SQL-auth password is indistinguishable from a flag, so the positional-argument filter dropped it and the run then reported *"Password is required"* — for a password that *was* supplied. The error now adds a targeted note (only when a `--`-token that is not a recognized flag actually appeared) explaining that such a password must go through `PM_SQL_PASSWORD`. The parsing is unchanged — deliberately, since accepting `--`-prefixed values positionally is what would make a mistyped flag become a password.
 - **Dashboard: a failed upgrade migration no longer stamps the database as successfully upgraded** ([#1498]) — `AddServerDialog` logged *"Upgrade failures detected. Continuing with full installation to ensure consistency..."* when an `upgrades/*` script failed, ran the full install over the partially-upgraded database anyway, and then wrote an `installation_history` row using `InstallationResult.Success` — which is `FilesFailed == 0` for the **install files only** and excludes upgrade failures entirely. That row was permanent damage: version detection reads the most recent `installation_status = 'SUCCESS'` row, and upgrade discovery only offers hops newer than the detected version, so the failed migration was stamped SUCCESS at the target version, the server then reported as current with the `ALTER`s never applied, and the hop was **never offered again** — silent schema drift recoverable only by a clean install. `ExecuteAllUpgradesAsync` already documented the contract ("*the caller aborts the whole install when totalFailureCount > 0*") and the CLI installer honored it; only the Dashboard's single-server path did not. The dialog now aborts on upgrade failure without running the install or writing a history row — leaving no `SUCCESS` row is exactly what makes the hop get re-offered, and upgrade scripts are written with guarded `ALTER`s so a re-run after fixing the error resumes cleanly — and folds the upgrade counts into the history write, matching the CLI. Relatedly, `ScriptProvider.FilterUpgrades` now throws on a version that is present but has no parseable numeric core instead of silently returning "no upgrades needed" (an empty result must never be the answer to "I could not read the version you gave me" — that path is what let a status string like `"Unreachable"` skip every migration while reporting zero failures), and `ExecuteAllUpgradesAsync` reports that as a failure so callers abort through the existing contract rather than hitting an unhandled exception. The **CLI**'s abort was itself nested inside `if (upgradeCount > 0)`, so a discovery failure (which reports zero hops) slipped past it — the CLI printed *"No pending upgrades found."* and ran the whole install; the abort now sits outside that block. Two latent version-parsing bugs the strict parse would have detonated are closed at the same choke point: `Version.TryParse("3.1")` succeeds with `Build == -1`, which the `Version` ctor rejects (now clamped), and `GetAppVersion` strips a `+metadata` suffix but not `-prerelease` — so a `3.2.0-rc1` `<InformationalVersion>` would have hard-aborted every upgrade. `FilterUpgrades` now normalizes SemVer suffixes at the choke point rather than relying on the seven call sites that each hand-strip them, mirroring `SingleInstanceDecision.ParseProductVersion`. The two decisions that can corrupt the ledger — "is installing safe?" (`InstallGuard`) and "are a repair's file failures the expected kind?" (`RepairOutcome`) — are extracted into `Installer.Core` and shared by both surfaces rather than hand-copied, and pinned by regression cases in `Installer.Tests`, which is CI-wired (`Dashboard.Tests` is not, so tests placed there could never fail a PR). One caveat worth recording: the ledger-lost probe is SQL, so its only test lives in `VersionDetectionTests`, which CI excludes because it needs a real SQL Server — that query was verified by hand against SQL Server 2022, but it cannot fail a PR. Found while reviewing [#963].
@@ -293,6 +294,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 [#1484]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1484
 [#1488]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1488
 [#1486]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1486
+[#1510]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1510
 [#1507]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1507
 [#1505]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1505
 [#1504]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1504

Dashboard/Controls/LandingPage.xaml.cs

@@ -12,6 +12,7 @@
 using System.Linq;
 using System.Reflection;
 using System.Threading.Tasks;
+using PerformanceMonitor.Ui;
 using System.Windows;
 using System.Windows.Controls;
 using System.Windows.Threading;
@@ -279,18 +280,13 @@ private async void ServerHealthCard_CheckVersionRequested(object? sender, Server
                 string appVersion = Assembly.GetExecutingAssembly()
                     .GetCustomAttribute<AssemblyInformationalVersionAttribute>()?.InformationalVersion
                     ?? Assembly.GetExecutingAssembly().GetName().Version?.ToString() ?? "0.0.0";
-                int plusIndex = appVersion.IndexOf('+');
-                if (plusIndex >= 0) appVersion = appVersion[..plusIndex];
 
-                static string Normalize(string v) =>
-                    Version.TryParse(v, out var p) ? new Version(p.Major, p.Minor, p.Build).ToString() : v;
+                string normalizedInstalled = VersionText.Normalize(installedVersion);
+                string normalizedApp = VersionText.Normalize(appVersion);
 
-                string normalizedInstalled = Normalize(installedVersion);
-                string normalizedApp = Normalize(appVersion);
-
-                if (Version.TryParse(normalizedInstalled, out var installed) &&
-                    Version.TryParse(normalizedApp, out var app) &&
-                    installed < app)
+                Version? installed = VersionText.Parse(installedVersion);
+                Version? app = VersionText.Parse(appVersion);
+                if (installed != null && app != null && installed < app)
                 {
                     var result = MessageBox.Show(
                         $"'{server.DisplayNameWithIntent}' has v{normalizedInstalled} installed.\n\nv{normalizedApp} is available. Open the server editor to upgrade?",

Dashboard/MainWindow.xaml.cs

@@ -1172,18 +1172,13 @@ private async void CheckServerVersion_Click(object sender, RoutedEventArgs e)
                 string appVersion = Assembly.GetExecutingAssembly()
                     .GetCustomAttribute<AssemblyInformationalVersionAttribute>()?.InformationalVersion
                     ?? Assembly.GetExecutingAssembly().GetName().Version?.ToString() ?? "0.0.0";
-                int plusIndex = appVersion.IndexOf('+');
-                if (plusIndex >= 0) appVersion = appVersion[..plusIndex];
 
-                static string Normalize(string v) =>
-                    Version.TryParse(v, out var p) ? new Version(p.Major, p.Minor, p.Build).ToString() : v;
+                string normalizedInstalled = VersionText.Normalize(installedVersion);
+                string normalizedApp = VersionText.Normalize(appVersion);
 
-                string normalizedInstalled = Normalize(installedVersion);
-                string normalizedApp = Normalize(appVersion);
-
-                if (Version.TryParse(normalizedInstalled, out var installed) &&
-                    Version.TryParse(normalizedApp, out var app) &&
-                    installed < app)
+                Version? installed = VersionText.Parse(installedVersion);
+                Version? app = VersionText.Parse(appVersion);
+                if (installed != null && app != null && installed < app)
                 {
                     var result = MessageBox.Show(
                         $"'{server.DisplayNameWithIntent}' has v{normalizedInstalled} installed.\n\nv{normalizedApp} is available. Open the server editor to upgrade?",

Dashboard/ManageServersWindow.xaml.cs

@@ -63,14 +63,7 @@ private async Task LoadInstalledVersionsAsync()
             await Task.WhenAll(tasks);
         }
 
-        private static string NormalizeVersion(string version)
-        {
-            int plusIndex = version.IndexOf('+');
-            string trimmed = plusIndex >= 0 ? version[..plusIndex] : version;
-            return Version.TryParse(trimmed, out var v)
-                ? new Version(v.Major, v.Minor, v.Build).ToString()
-                : trimmed;
-        }
+        private static string NormalizeVersion(string version) => VersionText.Normalize(version);
 
         private void ServersDataGrid_MouseDoubleClick(object sender, MouseButtonEventArgs e)
         {
@@ -260,24 +253,13 @@ private async void CheckForUpdates_Click(object sender, RoutedEventArgs e)
                 string appVersion = Assembly.GetExecutingAssembly()
                     .GetCustomAttribute<AssemblyInformationalVersionAttribute>()?.InformationalVersion
                     ?? Assembly.GetExecutingAssembly().GetName().Version?.ToString() ?? "0.0.0";
-                /* Strip git hash suffix if present (e.g., "2.5.0+abc123" → "2.5.0") */
-                int plusIndex = appVersion.IndexOf('+');
-                if (plusIndex >= 0) appVersion = appVersion[..plusIndex];
-
-                /* Normalize both to 3-part for comparison */
-                string Normalize(string v)
-                {
-                    if (Version.TryParse(v, out var parsed))
-                        return new Version(parsed.Major, parsed.Minor, parsed.Build).ToString();
-                    return v;
-                }
 
-                string normalizedInstalled = Normalize(installedVersion);
-                string normalizedApp = Normalize(appVersion);
+                string normalizedInstalled = VersionText.Normalize(installedVersion);
+                string normalizedApp = VersionText.Normalize(appVersion);
 
-                if (Version.TryParse(normalizedInstalled, out var installed) &&
-                    Version.TryParse(normalizedApp, out var app) &&
-                    installed < app)
+                Version? installed = VersionText.Parse(installedVersion);
+                Version? app = VersionText.Parse(appVersion);
+                if (installed != null && app != null && installed < app)
                 {
                     var result = MessageBox.Show(
                         $"'{server.DisplayNameWithIntent}' has v{normalizedInstalled} installed.\n\nv{normalizedApp} is available. Open the server editor to upgrade?",

Dashboard/Models/ServerHealthStatus.cs

@@ -10,6 +10,7 @@
 using System.Collections.Generic;
 using System.ComponentModel;
 using System.Runtime.CompilerServices;
+using PerformanceMonitor.Ui;
 
 namespace PerformanceMonitorDashboard.Models
 {
@@ -568,12 +569,7 @@ public string ConnectionStatusText
             }
         }
 
-        private static string NormalizeVersion(string version)
-        {
-            if (Version.TryParse(version, out var parsed))
-                return new Version(parsed.Major, parsed.Minor, parsed.Build).ToString();
-            return version;
-        }
+        private static string NormalizeVersion(string version) => VersionText.Normalize(version);
 
         // Overall health - worst severity across all metrics
         public HealthSeverity OverallSeverity

Dashboard/Models/ServerListItem.cs

@@ -9,6 +9,7 @@
 using System.ComponentModel;
 using System.Runtime.CompilerServices;
 using PerformanceMonitor.Common;
+using PerformanceMonitor.Ui;
 
 namespace PerformanceMonitorDashboard.Models
 {
@@ -130,9 +131,7 @@ public string MonitorVersionDisplay
             {
                 if (string.IsNullOrEmpty(_status.InstalledMonitorVersion))
                     return string.Empty;
-                if (System.Version.TryParse(_status.InstalledMonitorVersion, out var v))
-                    return $"Monitor v{new System.Version(v.Major, v.Minor, v.Build)}";
-                return $"Monitor v{_status.InstalledMonitorVersion}";
+                return $"Monitor v{VersionText.Normalize(_status.InstalledMonitorVersion)}";
             }
         }
 

Lite.Tests/VersionTextTests.cs

@@ -0,0 +1,98 @@
+using System;
+using PerformanceMonitor.Ui;
+using Xunit;
+
+namespace PerformanceMonitorLite.Tests;
+
+/// <summary>
+/// Pins <see cref="VersionText"/>, the shared version parse/normalize the Dashboard and Lite server-list
+/// views use. The load-bearing case is the two-part-version clamp: the hand-rolled snippets this replaced
+/// did <c>new Version(p.Major, p.Minor, p.Build)</c> after <c>Version.TryParse</c>, which THROWS on a
+/// two-part version like "3.1" (TryParse leaves Build == -1, and the Version ctor rejects a negative build).
+/// The first test is that regression, as an assertion that it no longer throws.
+/// </summary>
+public class VersionTextTests
+{
+    [Fact]
+    public void TwoPartVersion_ClampsBuildToZero_InsteadOfThrowing()
+    {
+        // The whole point: this used to throw ArgumentOutOfRangeException at every call site.
+        var ex = Record.Exception(() => VersionText.Normalize("3.1"));
+        Assert.Null(ex);
+
+        Assert.Equal("3.1.0", VersionText.Normalize("3.1"));
+        Assert.Equal(new Version(3, 1, 0), VersionText.Parse("3.1"));
+    }
+
+    [Theory]
+    [InlineData("3.1.0", "3.1.0")]
+    [InlineData("3.1.0.0", "3.1.0")]       // four-part collapses to three
+    [InlineData("3.1", "3.1.0")]           // two-part clamps
+    [InlineData("3.2.0+abc123", "3.2.0")]  // +build metadata stripped
+    [InlineData("3.2.0-rc1", "3.2.0")]     // -prerelease stripped
+    [InlineData("  3.1.0  ", "3.1.0")]     // trimmed
+    public void Normalize_ProducesThreePartDisplay(string raw, string expected)
+    {
+        Assert.Equal(expected, VersionText.Normalize(raw));
+    }
+
+    [Theory]
+    [InlineData(null)]
+    [InlineData("")]
+    [InlineData("   ")]
+    public void Normalize_NullOrBlank_IsEmptyString(string? raw)
+    {
+        Assert.Equal(string.Empty, VersionText.Normalize(raw));
+    }
+
+    [Theory]
+    [InlineData("Unreachable")]
+    [InlineData("not-a-version")]
+    [InlineData("v3")]
+    public void Normalize_Unparseable_PassesTheTrimmedOriginalThrough(string raw)
+    {
+        // Better to show a weird value than to lose it — the views fall back to the raw string.
+        Assert.Equal(raw.Trim(), VersionText.Normalize(raw));
+    }
+
+    [Theory]
+    [InlineData(null)]
+    [InlineData("")]
+    [InlineData("Unreachable")]
+    [InlineData("v3")]
+    public void Parse_NullBlankOrGarbage_IsNull(string? raw)
+    {
+        Assert.Null(VersionText.Parse(raw));
+    }
+
+    [Theory]
+    [InlineData("3.0.0", "3.1.0")]
+    [InlineData("3.1", "3.1.1")]           // the two-part clamp on the left still compares correctly
+    [InlineData("2.9.0", "3.0.0")]
+    [InlineData("3.1.0", "3.2.0")]
+    public void Parse_OrdersOlderBeforeNewer(string older, string newer)
+    {
+        var a = VersionText.Parse(older);
+        var b = VersionText.Parse(newer);
+        Assert.NotNull(a);
+        Assert.NotNull(b);
+        Assert.True(a < b, $"{older} should parse older than {newer}");
+    }
+
+    [Fact]
+    public void Parse_PrereleaseAndReleaseOfSameCore_CompareEqual()
+    {
+        // "3.0.0-rc1" strips to "3.0.0", so it is NOT treated as older than "3.0.0". This is a known
+        // limitation of stripping the suffix (fine here: these views compare RELEASED product versions).
+        Assert.Equal(VersionText.Parse("3.0.0"), VersionText.Parse("3.0.0-rc1"));
+    }
+
+    [Fact]
+    public void Parse_DropsTheFourthPart_MatchingTheOriginalThreePartNormalize()
+    {
+        // The hand-rolled sites did new Version(p.Major, p.Minor, p.Build) -- three parts, Revision dropped.
+        // Preserve that: a four-part version collapses to its three-part core, so "3.1.0.7" == "3.1.0".
+        Assert.Equal(VersionText.Parse("3.1.0"), VersionText.Parse("3.1.0.7"));
+        Assert.Equal("3.1.0", VersionText.Normalize("3.1.0.7"));
+    }
+}

Lite/Windows/ManageServersWindow.xaml.cs

@@ -52,11 +52,7 @@ private static string GetAppVersion()
             ?? Assembly.GetExecutingAssembly().GetName().Version?.ToString()
             ?? "0.0.0";
 
-        int plusIndex = raw.IndexOf('+');
-        string trimmed = plusIndex >= 0 ? raw[..plusIndex] : raw;
-        return System.Version.TryParse(trimmed, out var v)
-            ? new System.Version(v.Major, v.Minor, v.Build).ToString()
-            : trimmed;
+        return VersionText.Normalize(raw);
     }
 
     private void AddButton_Click(object sender, RoutedEventArgs e)