From c699a05d3083eafeee326e346f16d88628fbead1 Mon Sep 17 00:00:00 2001 From: laurentiu021 Date: Fri, 4 Sep 2026 10:06:34 +0300 Subject: [PATCH] fix: measure Cleanup's size estimates the way the cleanup actually works CleanupPreScanService sized both temp folders and the Recycle Bin with SearchOption.AllDirectories, which is wrong three ways: 1. It throws UnauthorizedAccessException out of MoveNext(), and the catch has to sit outside the foreach -- so one protected subfolder ended the whole walk and the headline reported whatever had been summed up to that point, silently low. 2. It follows reparse points. Deleting a junction puts one IN the Recycle Bin, so a single deleted junction could add every file it pointed at to the bin total. 3. It counted the extraction roots that the temp sweep has skipped since #2094, promising space Clean TEMP would correctly refuse to free. Both walks now go through TuneUpService.EnumerateFilesSkippingReparsePoints with the same two exclusions, which already has tests for the symlink, reparse-root and exclusion cases. The guard for that call shape read a hardcoded two-file list, so it could not see this caller at all -- the same weakness that let a third temp sweeper hide in a view-model for months. It now finds callers by looking for them. Its declaration filter matched the bare type name, which a call passing CancellationToken.None contains too, so both new calls would have been skipped as declarations; it now requires the type followed by a parameter name. Vacuity floor re-measured 5 -> 7. ICleanupPreScanService's contract offered "Unable to scan" as a temp label the method cannot produce; corrected to describe what it really returns. Closes #2098 --- CHANGELOG.md | 19 ++++++ .../SysManager.Tests/ArchitectureTests.cs | 61 ++++++++++++++----- .../Services/CleanupPreScanService.cs | 52 +++++++++------- .../Services/ICleanupPreScanService.cs | 10 ++- SysManager/SysManager/SysManager.csproj | 6 +- 5 files changed, 106 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f01f5199..fe4abc5a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,25 @@ 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.10] - 2026-09-04 + +The "can be freed" and "in Recycle Bin" figures on the Cleanup tab could stop counting partway through a +folder, and could count files that live outside the folder they claim to describe. Both are now measured the +same way the cleanup itself works, so the number matches what pressing the button would actually free. + +### Fixed +- **The Cleanup tab's size estimates stop under-reporting.** Both figures walked their folders with a method + that gives up entirely the moment it meets one folder Windows will not let it open — so on most machines the + total stopped early and reported less than was really there, with nothing on screen to say so. They now use + the same walker the cleanup uses, which skips what it cannot read and keeps going. +- **The Recycle Bin figure no longer counts files that are not in the bin.** The old walk followed folder + shortcuts, and deleting a folder shortcut puts one *in* the Recycle Bin — so a single deleted shortcut could + add every file it pointed at, live data included, to the "in Recycle Bin" total. The new walker never follows + them. +- **"Can be freed" no longer promises space Clean TEMP will leave alone.** Since 1.76.8 the temp cleanup skips + the folder running programs unpack themselves into. The estimate did not, so it counted bytes the cleanup + would correctly refuse to delete. It now applies the same two exclusions. + ## [1.76.9] - 2026-09-04 In Volume Control, sending an app to a particular speaker or headset looked like it had not worked: the "Choose diff --git a/SysManager/SysManager.Tests/ArchitectureTests.cs b/SysManager/SysManager.Tests/ArchitectureTests.cs index fc2ca81d..929de622 100644 --- a/SysManager/SysManager.Tests/ArchitectureTests.cs +++ b/SysManager/SysManager.Tests/ArchitectureTests.cs @@ -5448,7 +5448,11 @@ public void TheTimerResolutionQuery_BindsItsOutParametersCoarsestFinestCurrent() public void EveryTempTreeWalkerCall_PassesBothExtractionExclusions() { var appDir = FindAppProjectDir(); - var servicesDir = Path.Combine(appDir, "Services"); + + // The two files allowed to sweep %TEMP% themselves. This list bounds the STRAY check below only — + // the walker-argument check further down finds its own files by looking for callers, because a + // hardcoded list there could only ever check what it was already looking at. CleanupPreScanService + // began calling the walker and went unchecked for exactly that reason. string[] sweepers = ["TuneUpService.cs", "DeepCleanupService.cs"]; var callsChecked = 0; @@ -5484,20 +5488,33 @@ public void EveryTempTreeWalkerCall_PassesBothExtractionExclusions() + $"TuneUpService.CleanTempFilesAsync instead of writing a third sweeper:\n " + string.Join("\n ", strays)); - foreach (var file in sweepers) - { - // Comments stripped: both files explain this rule in prose right beside the calls, so a - // guard that read comments would pass on code that had dropped the argument. - var code = WithoutComments(File.ReadAllText(Path.Combine(servicesDir, file))); + // The files to check: the two sanctioned sweepers, PLUS any other file that calls one of the + // walkers. Only TuneUpService's are reachable from outside (DeepCleanupService's are private), so an + // outside caller is always a `...SkippingReparsePoints` call — that is what makes discovery cheap and + // exact. Comments stripped before matching: the sweepers explain this rule in prose right beside the + // calls, so a guard that read comments would pass on code that had dropped the argument. + var callers = Directory.GetFiles(appDir, "*.cs", SearchOption.AllDirectories) + .Where(f => !f.Contains($"{Path.DirectorySeparatorChar}obj{Path.DirectorySeparatorChar}", + StringComparison.Ordinal)) + .Select(f => (Path: f, Code: WithoutComments(File.ReadAllText(f)))) + .Where(f => sweepers.Contains(Path.GetFileName(f.Path)) + || f.Code.Contains("SkippingReparsePoints(", StringComparison.Ordinal)) + .ToList(); + foreach (var (path, code) in callers) + { foreach (var call in TempWalkerCall().Matches(code).Cast()) { var args = call.Groups["args"].Value; - // Skip the declarations: their parameter list names the type, a call site never does. - if (args.Contains("CancellationToken", StringComparison.Ordinal)) continue; + // Skip the declarations. Matching the bare type name was wrong: a call site passing + // `CancellationToken.None` contains it too, so both of CleanupPreScanService's calls were + // skipped as if they were declarations — the guard read them and checked nothing. A + // declaration is the type followed by a parameter NAME; a call is followed by a dot. + if (DeclaredCancellationParameter().IsMatch(args)) continue; callsChecked++; + var where = $"{Path.GetFileName(path)}: {call.Value}"; // BOTH, not either. OwnExtractionDirectory is one LEAF of BundleExtractionRoot, so on // its own it spared this app's unpacked native libraries and left every other @@ -5505,25 +5522,39 @@ public void EveryTempTreeWalkerCall_PassesBothExtractionExclusions() // OwnExtractionDirectory's own documentation describes, inflicted on someone else. The // leaf stays because for a non-single-file build BaseDirectory is the output folder, // which no extraction root contains. - Assert.Contains("SystemPaths.BundleExtractionRoot", args, StringComparison.Ordinal); - Assert.Contains("SystemPaths.OwnExtractionDirectory", args, StringComparison.Ordinal); + Assert.True(args.Contains("SystemPaths.BundleExtractionRoot", StringComparison.Ordinal) + && args.Contains("SystemPaths.OwnExtractionDirectory", StringComparison.Ordinal), + $"this walker call passes neither or only one extraction exclusion — {where}"); } } - // Vacuity floor: five call sites exist today across the two services. A collapse means the - // pattern stopped matching, and this guard would then pass without reading a single call. - Assert.True(callsChecked >= 5, - $"only {callsChecked} walker calls were matched — the pattern no longer matches the call " - + "shape, so this guard proves nothing. Re-derive it before trusting a pass."); + // Vacuity floor, re-measured: seven call sites across three files (five in the two sweepers, two in + // CleanupPreScanService). A collapse means the pattern or the declaration filter stopped matching, + // and this guard would then pass without reading a single call. + Assert.True(callsChecked >= 7, + $"only {callsChecked} walker calls were matched across {callers.Count} file(s) — the pattern no " + + "longer matches the call shape, so this guard proves nothing. Re-derive it before trusting a " + + "pass."); } /// /// Matches a call to one of the temp-tree walkers and captures its argument list. Bounded to /// argument lists without nested parentheses, which every current call site and declaration is. /// + /// + /// Deliberately loose on the name. DeepCleanupService's walkers are private and named plainly + /// (EnumerateFiles, EnumerateDirectoriesDepthFirst), so matching only + /// ...SkippingReparsePoints silently dropped its three call sites — the vacuity floor is what + /// caught that. The Directory. lookbehind is what keeps the loose form from matching the BCL call + /// of the same name. + /// [GeneratedRegex(@"(?[^()]*)\)")] private static partial Regex TempWalkerCall(); + /// A CancellationToken PARAMETER, i.e. the type followed by a name — a declaration. + [GeneratedRegex(@"\bCancellationToken\s+\w")] + private static partial Regex DeclaredCancellationParameter(); + /// /// A style that replaces a keyboard-operable control's template must still provide a focus visual. diff --git a/SysManager/SysManager/Services/CleanupPreScanService.cs b/SysManager/SysManager/Services/CleanupPreScanService.cs index 75b3afc7..47aa677d 100644 --- a/SysManager/SysManager/Services/CleanupPreScanService.cs +++ b/SysManager/SysManager/Services/CleanupPreScanService.cs @@ -21,29 +21,28 @@ public Task MeasureAsync() => // profile with no override. As statics these two were called for real, walking the entire temp tree and // every per-SID Recycle Bin folder, and that guard sat there for twenty minutes. They return a size LABEL // rather than a path, so it was not even reporting anything — pure collateral cost. + // Both walks go through TuneUpService's walker rather than SearchOption.AllDirectories, which was wrong + // three ways at once. AllDirectories throws UnauthorizedAccessException out of MoveNext(), and the catch + // has to sit outside the foreach — so one protected subfolder ended the whole walk and the headline + // reported whatever had been summed up to that point, silently low. It also follows junctions, so bytes + // living outside the tree counted as freeable; in the Recycle Bin that is the common case, because + // deleting a junction puts a reparse point IN the bin. And the temp figure has to exclude what the sweep + // refuses to delete, or it promises space Clean TEMP will correctly leave alone. private string MeasureTemp() { - long bytes = 0; var paths = new[] { Environment.GetEnvironmentVariable("TEMP") ?? "", Path.Join(Environment.GetFolderPath(Environment.SpecialFolder.Windows), "Temp"), }; - foreach (var path in paths.Where(p => !string.IsNullOrEmpty(p) && Directory.Exists(p))) - { - try - { - foreach (var file in Directory.EnumerateFiles(path, "*", SearchOption.AllDirectories)) - { - try { bytes += new FileInfo(file).Length; } - catch (IOException) { /* skip inaccessible file */ } - catch (UnauthorizedAccessException) { /* skip protected file */ } - } - } - catch (IOException) { /* skip inaccessible directory */ } - catch (UnauthorizedAccessException) { /* skip protected directory */ } - } + // The same two exclusions CleanTempFilesAsync passes, for the same reason: this number describes + // what that method would free, and it skips both extraction roots. + var bytes = paths + .Where(p => !string.IsNullOrEmpty(p) && Directory.Exists(p)) + .Sum(path => SumFileLengths(TuneUpService.EnumerateFilesSkippingReparsePoints( + path, CancellationToken.None, + SystemPaths.BundleExtractionRoot, SystemPaths.OwnExtractionDirectory))); return Describe(bytes, "can be freed"); } @@ -60,12 +59,9 @@ private string MeasureRecycleBin() foreach (var path in RecycleBinHelper.CurrentUserBinPaths()) { if (!Directory.Exists(path)) continue; - foreach (var file in Directory.EnumerateFiles(path, "*", SearchOption.AllDirectories)) - { - try { bytes += new FileInfo(file).Length; } - catch (IOException) { /* skip inaccessible file */ } - catch (UnauthorizedAccessException) { /* skip protected file */ } - } + bytes += SumFileLengths(TuneUpService.EnumerateFilesSkippingReparsePoints( + path, CancellationToken.None, + SystemPaths.BundleExtractionRoot, SystemPaths.OwnExtractionDirectory)); } return Describe(bytes, "in Recycle Bin"); @@ -74,6 +70,20 @@ private string MeasureRecycleBin() catch (UnauthorizedAccessException) { return "Unable to scan"; } } + /// Total size of the given files, skipping any that cannot be read. + private static long SumFileLengths(IEnumerable files) + { + long bytes = 0; + foreach (var file in files) + { + try { bytes += new FileInfo(file).Length; } + catch (IOException) { /* skip inaccessible file */ } + catch (UnauthorizedAccessException) { /* skip protected file */ } + } + + return bytes; + } + private static string Describe(long bytes, string suffix) => bytes > 0 ? string.Create(CultureInfo.InvariantCulture, $"{bytes / 1024.0 / 1024.0:F1} MB {suffix}") diff --git a/SysManager/SysManager/Services/ICleanupPreScanService.cs b/SysManager/SysManager/Services/ICleanupPreScanService.cs index 3c6ea337..5a0303c6 100644 --- a/SysManager/SysManager/Services/ICleanupPreScanService.cs +++ b/SysManager/SysManager/Services/ICleanupPreScanService.cs @@ -6,10 +6,14 @@ namespace SysManager.Services; /// The two headline numbers Quick Cleanup shows before the user asks for anything. /// -/// A finished, user-facing phrase for the temp folders — "412.7 MB can be freed", "Empty", or -/// "Unable to scan". +/// A finished, user-facing phrase for the temp folders — "412.7 MB can be freed" or "Empty". Never +/// "Unable to scan": the temp walk skips what it cannot read rather than giving up, so it always produces a +/// figure. A folder it could not open at all therefore reads as "Empty". +/// +/// +/// The same for the current user's Recycle Bin, which can also be "Unable to scan" — enumerating the per-SID +/// folders across every fixed drive can fail outright, and reporting "Empty" for that would be a claim. /// -/// The same for the current user's Recycle Bin. public sealed record CleanupPreScan(string TempLabel, string RecycleBinLabel); /// diff --git a/SysManager/SysManager/SysManager.csproj b/SysManager/SysManager/SysManager.csproj index 8adb4fed..7f4f4b60 100644 --- a/SysManager/SysManager/SysManager.csproj +++ b/SysManager/SysManager/SysManager.csproj @@ -10,9 +10,9 @@ SysManager true NU1603;NU1701 - 1.76.9 - 1.76.9.0 - 1.76.9.0 + 1.76.10 + 1.76.10.0 + 1.76.10.0 SysManager SysManager — Windows system monitoring toolkit by laurentiu021. Network, updates, health, logs, safe deep cleanup. https://github.com/laurentiu021/SystemManager