fix: functionality fixes — health analyzer, depth sort, history race, multi-disk

CHANGELOG.md

@@ -6,6 +6,22 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
 
+## [0.48.25] - 2026-05-15
+
+### Fixed
+- **HealthAnalyzer** — no longer claims "DNS is clean" when DNS IS bad; when
+  both DNS and game server show trouble, correctly returns Mixed verdict
+  instead of GameServer (FUNC-M2).
+- **TuneUpService** — empty directory removal now sorts by path depth (separator
+  count) instead of string length, ensuring deepest directories are deleted
+  first regardless of path name length (FUNC-M3).
+- **SpeedTestHistoryService** — `SaveAsync` and `ClearAsync` now serialize via
+  `SemaphoreSlim` to prevent concurrent load-modify-save races that could lose
+  history entries (FUNC-M4).
+- **FixedDriveService** — multi-disk enrichment now maps drive letters to
+  physical disks via `MSFT_Partition.DiskNumber`, correctly annotating media
+  type and bus type on systems with multiple drives (FUNC-M5).
+
 ## [0.48.24] - 2026-05-15
 
 ### Fixed

SysManager/SysManager.Tests/HealthAnalyzerEdgeCaseTests.cs

@@ -133,7 +133,7 @@ public void Analyze_StreamBadOnly_ReturnsStreamingService()
     }
 
     [Fact]
-    public void Analyze_DnsAndGameBad_ReturnsGameServer()
+    public void Analyze_DnsAndGameBad_ReturnsMixed()
     {
         var metrics = new[]
         {
@@ -142,9 +142,9 @@ public void Analyze_DnsAndGameBad_ReturnsGameServer()
             new TargetMetric("game", TargetRole.GameServer, 80.0, JitterWarnMs + 5, 0, 10),
         };
         var result = HealthAnalyzer.Analyze(metrics);
-        // When dns AND game are bad, it doesn't match "dnsBad && !gameBad && !streamBad"
-        // so falls through to gameBad check
-        Assert.Equal(HealthVerdict.GameServer, result.Verdict);
+        // FUNC-M2: When dns AND game are bad, return Mixed — not GameServer,
+        // because saying "DNS is clean" would be incorrect.
+        Assert.Equal(HealthVerdict.Mixed, result.Verdict);
     }
 
     [Fact]

SysManager/SysManager.Tests/HealthAnalyzerTests.cs

@@ -183,11 +183,9 @@ public void DnsAndGameBad_ReturnsMixed()
             M("dns", TargetRole.PublicDns, loss: 5),
             M("game", TargetRole.GameServer, loss: 5),
         });
-        // dns bad + game bad → not pure ISP, not pure GameServer → Mixed
-        // Actually per the code: dnsBad && !gameBad → ISP. But here gameBad is true too.
-        // The code checks: if (dnsBad && !gameBad && !streamBad) → ISP. Fails.
-        // Then: if (gameBad) → GameServer. This wins.
-        Assert.Equal(HealthVerdict.GameServer, d.Verdict);
+        // FUNC-M2: dns bad + game bad → Mixed (not GameServer, because DNS
+        // is NOT clean — claiming "it's the game server, not you" would be wrong).
+        Assert.Equal(HealthVerdict.Mixed, d.Verdict);
     }
 
     [Fact]

SysManager/SysManager/Services/FixedDriveService.cs

@@ -69,13 +69,41 @@ public static IReadOnlyList<FixedDrive> Enumerate()
                 }
             }
 
-            // We can't easily map DeviceId -> drive letter without another join,
-            // so if we have exactly one disk we annotate everything with it.
-            if (media.Count == 1)
+            // FUNC-M5: Map drive letters to physical disks via MSFT_Partition.
+            // This works for multi-disk systems (not just single-disk).
+            var letterToDisk = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
+            try
             {
-                var (m, b) = media.Values.First();
-                for (var i = 0; i < drives.Count; i++)
+                using var partSearch = new ManagementObjectSearcher(scope,
+                    new ObjectQuery("SELECT DriveLetter, DiskNumber FROM MSFT_Partition WHERE DriveLetter IS NOT NULL"));
+                using var partCollection = partSearch.Get();
+                foreach (ManagementObject mo in partCollection)
+                {
+                    using (mo)
+                    {
+                        var letter = mo["DriveLetter"]?.ToString();
+                        var diskNum = mo["DiskNumber"]?.ToString();
+                        if (!string.IsNullOrEmpty(letter) && !string.IsNullOrEmpty(diskNum))
+                            letterToDisk[letter + ":"] = diskNum;
+                    }
+                }
+            }
+            catch (ManagementException) { /* partition query not supported — fall through */ }
+
+            for (var i = 0; i < drives.Count; i++)
+            {
+                var driveLetter = drives[i].Letter.TrimEnd('\\', '/');
+                if (letterToDisk.TryGetValue(driveLetter, out var diskId) &&
+                    media.TryGetValue(diskId, out var info))
+                {
+                    drives[i] = drives[i] with { MediaType = info.Media, BusType = info.Bus };
+                }
+                else if (media.Count == 1)
+                {
+                    // Fallback: single disk — annotate all drives with it.
+                    var (m, b) = media.Values.First();
                     drives[i] = drives[i] with { MediaType = m, BusType = b };
+                }
             }
         }
         catch (ManagementException)

SysManager/SysManager/Services/HealthAnalyzer.cs

@@ -90,7 +90,7 @@ public static HealthDiagnostic Analyze(IEnumerable<TargetMetric> metrics)
             return diag;
         }
 
-        if (gameBad)
+        if (gameBad && !dnsBad)
         {
             diag.Verdict = HealthVerdict.GameServer;
             diag.Headline = "It's the game server, not you";
@@ -99,6 +99,15 @@ public static HealthDiagnostic Analyze(IEnumerable<TargetMetric> metrics)
             return diag;
         }
 
+        if (gameBad && dnsBad)
+        {
+            diag.Verdict = HealthVerdict.Mixed;
+            diag.Headline = "Multiple layers affected";
+            diag.Detail = "Both DNS and game server are showing trouble — likely an ISP or routing issue affecting multiple endpoints.";
+            diag.ColorHex = "#FF6B6B";
+            return diag;
+        }
+
         if (streamBad && !dnsBad)
         {
             diag.Verdict = HealthVerdict.StreamingService;

SysManager/SysManager/Services/SpeedTestHistoryService.cs

@@ -28,10 +28,19 @@ public sealed class SpeedTestHistoryService
         PropertyNamingPolicy = JsonNamingPolicy.CamelCase
     };
 
+    // FUNC-M4: Serialize all file operations to prevent concurrent SaveAsync
+    // calls from racing (load-modify-save is not atomic). A SemaphoreSlim(1,1)
+    // acts as an async-compatible mutex.
+    private readonly SemaphoreSlim _fileLock = new(1, 1);
+
     /// <summary>
     /// Loads all saved results from disk. Returns empty list on any error.
     /// </summary>
     public async Task<List<SpeedTestResult>> LoadAsync(CancellationToken ct = default)
+        => await LoadCoreAsync(ct).ConfigureAwait(false);
+
+    /// <summary>Internal load without locking — called from within locked sections.</summary>
+    private async Task<List<SpeedTestResult>> LoadCoreAsync(CancellationToken ct)
     {
         try
         {
@@ -68,9 +77,10 @@ public async Task<List<SpeedTestResult>> LoadAsync(CancellationToken ct = defaul
     /// </summary>
     public async Task SaveAsync(SpeedTestResult result, CancellationToken ct = default)
     {
+        await _fileLock.WaitAsync(ct).ConfigureAwait(false);
         try
         {
-            var all = await LoadAsync(ct).ConfigureAwait(false);
+            var all = await LoadCoreAsync(ct).ConfigureAwait(false);
             all.Add(result);
 
             // Trim per engine: keep only the most recent MaxPerEngine entries.
@@ -104,13 +114,18 @@ public async Task SaveAsync(SpeedTestResult result, CancellationToken ct = defau
         {
             Log.Warning(ex, "Access denied saving speed test history");
         }
+        finally
+        {
+            _fileLock.Release();
+        }
     }
 
     /// <summary>
     /// Clears history for a specific engine, or all history if engine is null.
     /// </summary>
     public async Task ClearAsync(string? engine = null, CancellationToken ct = default)
     {
+        await _fileLock.WaitAsync(ct).ConfigureAwait(false);
         try
         {
             if (engine == null)
@@ -120,7 +135,7 @@ public async Task ClearAsync(string? engine = null, CancellationToken ct = defau
                 return;
             }
 
-            var all = await LoadAsync(ct).ConfigureAwait(false);
+            var all = await LoadCoreAsync(ct).ConfigureAwait(false);
             var filtered = all.Where(r => !string.Equals(r.Engine, engine, StringComparison.OrdinalIgnoreCase)).ToList();
 
             if (filtered.Count == 0)
@@ -151,6 +166,10 @@ public async Task ClearAsync(string? engine = null, CancellationToken ct = defau
         {
             Log.Warning(ex, "Access denied clearing speed test history");
         }
+        finally
+        {
+            _fileLock.Release();
+        }
     }
 
     /// <summary>JSON-serializable DTO for history entries.</summary>

SysManager/SysManager/Services/TuneUpService.cs

@@ -182,8 +182,11 @@ private static (long BytesFreed, int FilesDeleted, int Errors) CleanTempFiles(Ca
                 }
 
                 // Try to remove empty subdirectories
+                // FUNC-M3: Sort by directory depth (separator count) descending,
+                // not string length. A path like "C:\a\b\c" is deeper than
+                // "C:\longname" despite being shorter — we must delete deepest first.
                 foreach (var sub in Directory.EnumerateDirectories(dir, "*", SearchOption.AllDirectories)
-                             .OrderByDescending(d => d.Length))
+                             .OrderByDescending(d => d.Count(c => c == Path.DirectorySeparatorChar || c == Path.AltDirectorySeparatorChar)))
                 {
                     ct.ThrowIfCancellationRequested();
                     try