From d7eaee6c6f87f9138bf3ae3f0ab9c012a84e57b3 Mon Sep 17 00:00:00 2001 From: laurentiu021 Date: Fri, 4 Sep 2026 10:22:44 +0300 Subject: [PATCH] fix: report an unreadable disk as unread rather than degrading HealthScoreService marked the disk component unavailable only for an EMPTY list. A drive with no SMART data still produces a report (HealthPercent returns null), so the list is non-empty, ComputeDiskScore returns the deliberate unknown 80, and ClassifySmartHealth reads 80 with unavailable=false through its >= 60 branch as "Disk health degrading" -- a claim about failing hardware on a machine where nothing was measured. Both the classifier's own remarks and UnknownComponentScore_StaysBelowEveryGreenBranch already ruled that outcome out in prose while this path delivered it. Fixed with disks.All(d => d.HealthPercent is null), and the decision extracted into a pure internal UnavailableComponents so it can be asserted without querying WMI. All, not Any: with one drive failing at 20% and one unreadable, Any would replace "Disk health critical" with "could not be read" and hide the failing drive. The mutation proof also showed Any breaks the empty-list case, since [].Any() is false while [].All() is true. Closes #2099 --- CHANGELOG.md | 15 ++++++ .../HealthScoreServiceTests.cs | 50 +++++++++++++++++++ .../SysManager/Services/HealthScoreService.cs | 42 +++++++++++++--- SysManager/SysManager/SysManager.csproj | 6 +-- 4 files changed, 103 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fe4abc5a..2ad4948a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,21 @@ That paragraph is not decoration: the release workflow copies each entry verbati the GitHub release body and the announcement discussion, so it is the first thing a prospective user reads. CI fails a pull request whose newest entry is missing it. +## [1.76.11] - 2026-09-04 + +If Windows cannot report your drive's health — common on desktop SATA and NVMe drives, and always the case in +a virtual machine — the Landing tab said "Disk health degrading" and showed a warning. Nothing was degrading. +It now says the health could not be read, which is what actually happened. + +### Fixed +- **A drive whose health cannot be read is no longer reported as degrading.** SysManager scores an unmeasured + drive cautiously on purpose, so it can never be called healthy without evidence. But that cautious score was + then read as a verdict, and the warning band starts just below it — so a machine that reported no drive + health at all was told its disk was on the way out. The tab now distinguishes "could not read this" from "we + read it and it looks worse than it should", and only the second one warns. A machine with one readable drive + and one unreadable one still reports the readable drive's verdict, so a genuinely failing disk is never + hidden behind a "could not read" message. + ## [1.76.10] - 2026-09-04 The "can be freed" and "in Recycle Bin" figures on the Cleanup tab could stop counting partway through a diff --git a/SysManager/SysManager.Tests/HealthScoreServiceTests.cs b/SysManager/SysManager.Tests/HealthScoreServiceTests.cs index 3924c5f5..fa02b4b0 100644 --- a/SysManager/SysManager.Tests/HealthScoreServiceTests.cs +++ b/SysManager/SysManager.Tests/HealthScoreServiceTests.cs @@ -298,6 +298,56 @@ public void ComputeDiskScore_DiskWithNoDataAtAll_ScoresTheUnknownValue() Assert.Equal(80, HealthScoreService.ComputeDiskScore(disks)); } + + // ---------- UnavailableComponents ---------- + + [Fact] + public void UnavailableComponents_DrivesPresentButNoneReadable_MarksTheDiskUnavailable() + { + // The score alone cannot carry this. Two drives with no SMART data score the deliberate unknown 80, + // and ClassifySmartHealth reads 80 with unavailable=false through its `>= 60` branch as "Disk health + // degrading" — a claim about failing hardware on a machine where nothing was measured. The test + // directly below UnknownComponentScore_StaysBelowEveryGreenBranch asserts that intent in prose + // ("must not read as degrading either — nothing was measured") while this path delivered exactly that. + var disks = new List + { + new() { FriendlyName = "Samsung SSD", HealthStatus = "" }, + new() { FriendlyName = "WDC HDD", HealthStatus = "" } + }; + Assert.All(disks, d => Assert.Null(d.HealthPercent)); // the premise, not an assumption + + var unavailable = HealthScoreService.UnavailableComponents(disks, null); + + Assert.Contains(HealthScoreService.DiskComponent, unavailable); + } + + [Fact] + public void UnavailableComponents_OneReadableDriveAmongUnreadable_KeepsTheDiskAvailable() + { + // The negative half, and the reason the rule is All rather than Any: marking the component + // unavailable here would replace "Disk health critical" with "could not be read" and hide a drive + // Windows has already flagged as failing. + var disks = new List + { + new() { FriendlyName = "Failing drive", HealthStatus = "Unhealthy" }, + new() { FriendlyName = "Unreadable drive", HealthStatus = "" } + }; + + var unavailable = HealthScoreService.UnavailableComponents(disks, null); + + Assert.DoesNotContain(HealthScoreService.DiskComponent, unavailable); + Assert.Equal(20, HealthScoreService.ComputeDiskScore(disks)); // and the failing verdict survives + } + + [Fact] + public void UnavailableComponents_NoDrivesAtAll_StillMarksTheDiskUnavailable() + { + Assert.Contains(HealthScoreService.DiskComponent, + HealthScoreService.UnavailableComponents([], null)); + Assert.Contains(HealthScoreService.DiskComponent, + HealthScoreService.UnavailableComponents(null, null)); + } + [Fact] public void UnknownComponentScore_StaysBelowEveryGreenBranch() { diff --git a/SysManager/SysManager/Services/HealthScoreService.cs b/SysManager/SysManager/Services/HealthScoreService.cs index f307815e..28bb8673 100644 --- a/SysManager/SysManager/Services/HealthScoreService.cs +++ b/SysManager/SysManager/Services/HealthScoreService.cs @@ -93,13 +93,7 @@ public async Task ComputeAsync(CancellationToken ct = default // Recorded so a consumer can say "could not read this" instead of reading a verdict out of a // fallback number. The scores above already refuse to claim health; this is what makes the reason // visible. - List unavailable = []; - if (disks is null || disks.Count == 0) unavailable.Add(DiskComponent); - if (snapshot is null) - { - unavailable.Add(MemoryComponent); - unavailable.Add(UptimeComponent); - } + var unavailable = UnavailableComponents(disks, snapshot); return new HealthScoreResult { @@ -114,6 +108,40 @@ public async Task ComputeAsync(CancellationToken ct = default }; } + /// + /// Which components produced no usable evidence, so a consumer can say "could not read this" instead of + /// reading a verdict out of a fallback number. The scores already refuse to claim health; this is what + /// makes the reason visible. + /// + /// + /// Pure and internal for the same reason is: the decision is worth + /// asserting, and asserting it through would mean querying WMI. + /// Drives present but none readable is the same absence of evidence as no drives at all, and it is + /// the common case — plenty of consumer SATA and NVMe disks expose nothing through + /// MSFT_StorageReliabilityCounter, and a VM exposes nothing whatever. Testing only for an empty + /// list left that machine scored at the deliberate unknown 80, which + /// DashboardViewModel.ClassifySmartHealth then reads through its >= 60 branch as "Disk + /// health degrading" — the outcome that method's own remarks rule out, because nothing is degrading when + /// nothing was measured. + /// Deliberately All, not Any. With one readable drive at 30% and one unreadable, + /// Any would mark the component unavailable and replace a critical-disk warning with "could not be + /// read", hiding a failing drive. A mixed read keeps the worst measured verdict. All also covers + /// the empty list, which is why that case is no longer spelled out. + /// + internal static List UnavailableComponents( + IReadOnlyList? disks, SystemSnapshot? snapshot) + { + List unavailable = []; + if (disks is null || disks.All(d => d.HealthPercent is null)) unavailable.Add(DiskComponent); + if (snapshot is null) + { + unavailable.Add(MemoryComponent); + unavailable.Add(UptimeComponent); + } + + return unavailable; + } + /// Component names used in . internal const string DiskComponent = "Disk"; internal const string MemoryComponent = "Memory"; diff --git a/SysManager/SysManager/SysManager.csproj b/SysManager/SysManager/SysManager.csproj index 7f4f4b60..b6a45282 100644 --- a/SysManager/SysManager/SysManager.csproj +++ b/SysManager/SysManager/SysManager.csproj @@ -10,9 +10,9 @@ SysManager true NU1603;NU1701 - 1.76.10 - 1.76.10.0 - 1.76.10.0 + 1.76.11 + 1.76.11.0 + 1.76.11.0 SysManager SysManager — Windows system monitoring toolkit by laurentiu021. Network, updates, health, logs, safe deep cleanup. https://github.com/laurentiu021/SystemManager