feat: Dashboard Quick Tune-Up wizard#264
Conversation
- TuneUpService: orchestrates temp cleanup, Recycle Bin (with confirm), broken shortcut scan, disk SMART check, uptime/RAM monitoring - DashboardViewModel: RunTuneUp command with progress, cancel, dismiss - DashboardView: green Quick Tune-Up button + step progress + summary card - TuneUpResult model with warning aggregation and color-coded verdicts - IntGreaterThanZeroConverter for conditional XAML visibility - OperationLockService integration prevents concurrent disk operations - Non-destructive, no admin required, no registry edits - 35 new unit tests (TuneUpService + DashboardViewModel + converter) - README, ARCHITECTURE, CHANGELOG updated Closes #261
📝 WalkthroughWalkthroughThis PR introduces a complete "Quick Tune-Up" wizard feature on the Dashboard that orchestrates safe system cleanup and health checks in one click. A new ChangesQuick Tune-Up Wizard Feature
Sequence Diagram(s)The Quick Tune-Up feature introduces sequential orchestration across multiple services and a multi-step command flow with progress tracking: sequenceDiagram
participant User
participant View as DashboardView
participant ViewModel as DashboardViewModel
participant TuneUp as TuneUpService
participant Shortcuts as ShortcutCleaner
participant Disk as DiskHealth
participant Info as SystemInfo
User->>View: Click Quick Tune-Up
View->>ViewModel: RunTuneUpAsync
ViewModel->>ViewModel: Confirm Recycle Bin emptying
ViewModel->>ViewModel: Acquire operation lock
ViewModel->>ViewModel: IsTuneUpRunning = true
ViewModel->>TuneUp: RunAsync(emptyRecycleBin, progress)
TuneUp->>TuneUp: Clean TEMP files
TuneUp->>ViewModel: Progress(1, "TEMP cleaned")
TuneUp->>TuneUp: Empty Recycle Bin (optional)
TuneUp->>ViewModel: Progress(2, "Recycle Bin processed")
TuneUp->>Shortcuts: GetBrokenShortcutCount
TuneUp->>ViewModel: Progress(3, "Shortcuts scanned")
TuneUp->>Disk: GetDiskHealthSummaries
TuneUp->>ViewModel: Progress(4, "Disk SMART checked")
TuneUp->>Info: Get Uptime and RAM usage
TuneUp->>ViewModel: Progress(5, "System info captured")
TuneUp->>ViewModel: Return TuneUpResult
ViewModel->>ViewModel: TuneUpResult = result
ViewModel->>ViewModel: HasTuneUpResult = true
ViewModel->>ViewModel: IsTuneUpRunning = false
View->>User: Display results summary card
User->>View: Dismiss results
View->>ViewModel: DismissTuneUpResultCommand
ViewModel->>ViewModel: TuneUpResult = null
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@SysManager/SysManager/Services/TuneUpService.cs`:
- Around line 221-227: The call to NativeMethods.SHEmptyRecycleBin currently
discards its HRESULT so failures are treated as success and the Win32Exception
catch won't run; update the TuneUpService method(s) that call
NativeMethods.SHEmptyRecycleBin to capture its integer return value, check it
for success (HRESULT == 0 / S_OK) and if non-zero log the HRESULT (include the
numeric code and descriptive text) and return false; do not rely on exceptions
or SetLastError—either change the P/Invoke signature to return the HRESULT or
treat the existing return value accordingly, and apply the same check-and-log
pattern to the second occurrence noted (lines 233–234) so both calls correctly
detect and handle failures.
In `@SysManager/SysManager/ViewModels/DashboardViewModel.cs`:
- Around line 174-179: Dispose currently disposes _tuneUpCts without cancelling
it, which can leave an in-flight tune-up running and cause post-dispose
callbacks; in Dispose(bool disposing) call _tuneUpCts?.Cancel() before
_tuneUpCts?.Dispose(), then set _tuneUpCts = null to avoid further use, ensuring
you only touch the _tuneUpCts CancellationTokenSource and not swallow
exceptions—use the existing _tuneUpCts symbol in the Dispose override.
In `@SysManager/SysManager/Views/DashboardView.xaml`:
- Around line 166-167: The TextBlock currently forces a hardcoded green
Foreground ("#22C55E") while binding the verdict text; change the Foreground to
bind to a verdict-based brush instead (e.g., Foreground="{Binding VerdictColor}"
or Foreground="{Binding Verdict, Converter={StaticResource
VerdictToBrushConverter}}") so the color reflects warning/critical/ok states;
update both occurrences that reference Verdict (the TextBlock with
Text="{Binding Verdict}" and the similar block at lines 211-212) and ensure the
VM exposes a VerdictColor/Brush property or register a VerdictToBrushConverter
resource used by the binding.
- Around line 142-143: The TextBlock instances showing "— open Shortcut Cleaner
to review" should be converted into actionable controls (e.g., Hyperlink inside
a TextBlock or a Button) that invoke the app's navigation command to open the
Shortcut Cleaner tab; replace each TextBlock occurrence (the one shown and the
similar ones at the other noted locations) with a clickable control bound to the
existing navigation command or method (e.g., NavigateToTabCommand or
OpenTab/ShowShortcutCleaner method) and pass the ShortcutCleaner tab identifier
as the command parameter so clicking the link navigates to the correct tab.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b6600801-98a5-4beb-9e35-654285cfb564
📒 Files selected for processing (12)
ARCHITECTURE.mdCHANGELOG.mdREADME.mdSysManager/SysManager.Tests/DashboardViewModelTests.csSysManager/SysManager.Tests/IntGreaterThanZeroConverterTests.csSysManager/SysManager.Tests/TuneUpServiceTests.csSysManager/SysManager/App.xamlSysManager/SysManager/Helpers/IntGreaterThanZeroConverter.csSysManager/SysManager/Models/TuneUpResult.csSysManager/SysManager/Services/TuneUpService.csSysManager/SysManager/ViewModels/DashboardViewModel.csSysManager/SysManager/Views/DashboardView.xaml
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build & unit tests
- GitHub Check: Analyze (csharp)
🔇 Additional comments (4)
SysManager/SysManager/Models/TuneUpResult.cs (1)
41-66: Solid aggregation logic for tune-up summary metrics.
WarningCount,OverallVerdict, andOverallColorHexare clear and map cleanly to dashboard presentation states.SysManager/SysManager/ViewModels/DashboardViewModel.cs (1)
96-157: Tune-up command flow is well-structured.The lock acquisition, progress reporting, command state updates, and cleanup in
finallyare all implemented cleanly.SysManager/SysManager.Tests/DashboardViewModelTests.cs (1)
136-211: Good coverage expansion for tune-up ViewModel state.The new tests for default values, command exposure, dismissal behavior, notifications, and disposal meaningfully increase confidence in the new Dashboard flow.
SysManager/SysManager.Tests/TuneUpServiceTests.cs (1)
31-280: Nice validation of TuneUpResult computed behavior and thresholds.These tests provide solid guardrails for warning aggregation and UI-facing verdict/color outputs.
| NativeMethods.SHEmptyRecycleBin(IntPtr.Zero, null, 0x00000007); | ||
| return true; | ||
| } | ||
| catch (System.ComponentModel.Win32Exception ex) | ||
| { | ||
| Log.Warning("TuneUp empty recycle bin failed: {Error}", ex.Message); | ||
| return false; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
SHEmptyRecycleBin return value semantics (HRESULT) and recommended C# P/Invoke error handling pattern for failures.
💡 Result:
SHEmptyRecycleBin return value semantics (HRESULT) - SHEmptyRecycleBinA/SHEmptyRecycleBin returns an HRESULT. If it succeeds, it returns S_OK. Otherwise, it returns an HRESULT error code. [1][2] Recommended C# P/Invoke error handling pattern for failures 1) Declare the P/Invoke to return an int/HRESULT, and treat any value!= S_OK as failure. Example: [DllImport("shell32.dll", CharSet = CharSet.Unicode, ExactSpelling = true)] static extern int SHEmptyRecycleBin(IntPtr hwnd, string pszRootPath, uint dwFlags); const int S_OK = 0; uint flags = 0; // SHERB_* values as needed int hr = SHEmptyRecycleBin(IntPtr.Zero, /pszRootPath/ null, flags); if (hr!= S_OK) { // Don’t rely on GetLastWin32Error here; use the HRESULT from the return value. throw new System.Runtime.InteropServices.COMException("SHEmptyRecycleBin failed", hr); } This matches the documented contract that failures are indicated by the HRESULT return value. [1][2] 2) Avoid using Marshal.GetLastWin32Error for SHEmptyRecycleBin failure reporting. - The Microsoft guidance for this API is based on the HRESULT return value (S_OK vs an HRESULT error code). [1][2] - Using GetLastWin32Error is not the right mechanism for interpreting SHEmptyRecycleBin’s failures, and an issue report notes that SHEmptyRecycleBin uses HRESULT to indicate error status and does not indicate that it sets last error. [3] 3) Optional: special-case “already empty” only if you empirically observe it in your environment. - Community notes report E_UNEXPECTED (0x8000FFFF) when the recycle bin is already empty, but this is not stated in the official docs and should be treated as non-guaranteed behavior. [4] Sources [1] Microsoft Learn: SHEmptyRecycleBinA function (shellapi.h) return value semantics (S_OK / HRESULT error code). [2] Same Microsoft Learn page, reiterated. [3] PowerShell/PowerShell issue discussing misuse of GetLastWin32Error vs HRESULT for this function. [4] pinvoke.net note about returning 0x8000FFFF (E_UNEXPECTED) when already empty.
Citations:
- 1: https://learn.microsoft.com/en-us/windows/win32/api/shellapi/nf-shellapi-shemptyrecyclebina?redirectedfrom=MSDN
- 2: https://learn.microsoft.com/en-us/windows/win32/api/shellapi/nf-shellapi-shemptyrecyclebina
- 3: Misuse of
SHEmptyRecycleBininClear-RecycleBincmdlet PowerShell/PowerShell#6743 - 4: https://pinvoke.net/print.aspx/shell32.SHEmptyRecycleBin
🏁 Script executed:
#!/bin/bash
# Find the NativeMethods class or file containing SHEmptyRecycleBin declaration
rg -i "SHEmptyRecycleBin" --type cs -B 2 -A 2Repository: laurentiu021/SystemManager
Length of output: 1070
🏁 Script executed:
#!/bin/bash
# Examine the TuneUpService.cs file around the mentioned lines
cd SysManager/SysManager/Services || exit 1
wc -l TuneUpService.csRepository: laurentiu021/SystemManager
Length of output: 91
🏁 Script executed:
#!/bin/bash
# Look at the specific lines in TuneUpService.cs
cd SysManager/SysManager/Services || exit 1
sed -n '210,240p' TuneUpService.csRepository: laurentiu021/SystemManager
Length of output: 1017
Handle SHEmptyRecycleBin HRESULT return value; current code always reports success even on failure.
Line 221 discards the HRESULT return value, and the Win32Exception catch block will not execute because the P/Invoke has no SetLastError=true attribute. This causes failures to be silently treated as success.
Suggested fix
private static bool EmptyRecycleBin()
{
- try
- {
- // SHEmptyRecycleBin with SHERB_NOCONFIRMATION | SHERB_NOPROGRESSUI | SHERB_NOSOUND
- NativeMethods.SHEmptyRecycleBin(IntPtr.Zero, null, 0x00000007);
- return true;
- }
- catch (System.ComponentModel.Win32Exception ex)
- {
- Log.Warning("TuneUp empty recycle bin failed: {Error}", ex.Message);
- return false;
- }
+ const uint flags = 0x00000007; // NOCONFIRMATION | NOPROGRESSUI | NOSOUND
+ int hr = NativeMethods.SHEmptyRecycleBin(IntPtr.Zero, null, flags);
+ if (hr != 0)
+ {
+ Log.Warning("TuneUp empty recycle bin failed with HRESULT 0x{HResult:X8}", hr);
+ return false;
+ }
+ return true;
}Also applies to: 233-234
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@SysManager/SysManager/Services/TuneUpService.cs` around lines 221 - 227, The
call to NativeMethods.SHEmptyRecycleBin currently discards its HRESULT so
failures are treated as success and the Win32Exception catch won't run; update
the TuneUpService method(s) that call NativeMethods.SHEmptyRecycleBin to capture
its integer return value, check it for success (HRESULT == 0 / S_OK) and if
non-zero log the HRESULT (include the numeric code and descriptive text) and
return false; do not rely on exceptions or SetLastError—either change the
P/Invoke signature to return the HRESULT or treat the existing return value
accordingly, and apply the same check-and-log pattern to the second occurrence
noted (lines 233–234) so both calls correctly detect and handle failures.
| protected override void Dispose(bool disposing) | ||
| { | ||
| if (disposing) | ||
| { | ||
| _tuneUpCts?.Dispose(); | ||
| } |
There was a problem hiding this comment.
Cancel in-flight tune-up before disposing token source.
On Line 178, disposing _tuneUpCts without cancellation can leave a running tune-up operation alive during ViewModel teardown. Cancel first, then dispose/reset to avoid post-dispose updates.
Suggested fix
protected override void Dispose(bool disposing)
{
if (disposing)
{
+ _tuneUpCts?.Cancel();
_tuneUpCts?.Dispose();
+ _tuneUpCts = null;
}
base.Dispose(disposing);
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@SysManager/SysManager/ViewModels/DashboardViewModel.cs` around lines 174 -
179, Dispose currently disposes _tuneUpCts without cancelling it, which can
leave an in-flight tune-up running and cause post-dispose callbacks; in
Dispose(bool disposing) call _tuneUpCts?.Cancel() before _tuneUpCts?.Dispose(),
then set _tuneUpCts = null to avoid further use, ensuring you only touch the
_tuneUpCts CancellationTokenSource and not swallow exceptions—use the existing
_tuneUpCts symbol in the Dispose override.
| <TextBlock Text=" — open Shortcut Cleaner to review" Foreground="#F59E0B" | ||
| VerticalAlignment="Center"> |
There was a problem hiding this comment.
Recommendation messages are not actionable links.
The summary currently shows plain text guidance only. The linked objective calls for actionable recommendations with links to relevant tabs, so these should be clickable controls (e.g., Hyperlink/Button + navigation command).
Also applies to: 186-188, 204-206
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@SysManager/SysManager/Views/DashboardView.xaml` around lines 142 - 143, The
TextBlock instances showing "— open Shortcut Cleaner to review" should be
converted into actionable controls (e.g., Hyperlink inside a TextBlock or a
Button) that invoke the app's navigation command to open the Shortcut Cleaner
tab; replace each TextBlock occurrence (the one shown and the similar ones at
the other noted locations) with a clickable control bound to the existing
navigation command or method (e.g., NavigateToTabCommand or
OpenTab/ShowShortcutCleaner method) and pass the ShortcutCleaner tab identifier
as the command parameter so clicking the link navigates to the correct tab.
| <TextBlock Text="{Binding Verdict}" FontWeight="SemiBold" | ||
| VerticalAlignment="Center" Foreground="#22C55E"/> |
There was a problem hiding this comment.
Use bound verdict colors instead of hardcoded green.
Line 166 and Line 211 force green even when verdicts are warning/critical, which can mislead users about disk/system health.
Suggested fix
-<TextBlock Text="{Binding Verdict}" FontWeight="SemiBold"
- VerticalAlignment="Center" Foreground="#22C55E"/>
+<TextBlock Text="{Binding Verdict}" FontWeight="SemiBold"
+ VerticalAlignment="Center"
+ Foreground="{Binding ColorHex, Converter={StaticResource HexBrush}}"/>
-<TextBlock Text="{Binding TuneUpResult.OverallVerdict}" FontWeight="SemiBold" FontSize="13"
- Foreground="#22C55E"/>
+<TextBlock Text="{Binding TuneUpResult.OverallVerdict}" FontWeight="SemiBold" FontSize="13"
+ Foreground="{Binding TuneUpResult.OverallColorHex, Converter={StaticResource HexBrush}}"/>Also applies to: 211-212
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@SysManager/SysManager/Views/DashboardView.xaml` around lines 166 - 167, The
TextBlock currently forces a hardcoded green Foreground ("#22C55E") while
binding the verdict text; change the Foreground to bind to a verdict-based brush
instead (e.g., Foreground="{Binding VerdictColor}" or Foreground="{Binding
Verdict, Converter={StaticResource VerdictToBrushConverter}}") so the color
reflects warning/critical/ok states; update both occurrences that reference
Verdict (the TextBlock with Text="{Binding Verdict}" and the similar block at
lines 211-212) and ensure the VM exposes a VerdictColor/Brush property or
register a VerdictToBrushConverter resource used by the binding.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| var tempPaths = new[] | ||
| { | ||
| Environment.GetEnvironmentVariable("TEMP") ?? "", | ||
| Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.Windows), "Temp") |
| if (!Directory.EnumerateFileSystemEntries(sub).Any()) | ||
| Directory.Delete(sub); | ||
| } | ||
| catch (IOException) { } |
| Directory.Delete(sub); | ||
| } | ||
| catch (IOException) { } | ||
| catch (UnauthorizedAccessException) { } |
| try | ||
| { | ||
| // SHEmptyRecycleBin with SHERB_NOCONFIRMATION | SHERB_NOPROGRESSUI | SHERB_NOSOUND | ||
| NativeMethods.SHEmptyRecycleBin(IntPtr.Zero, null, 0x00000007); |
| "Do you also want to empty the Recycle Bin?", | ||
| "Quick Tune-Up"); | ||
|
|
||
| var opLock = OperationLockService.Instance.TryAcquire( |
## Summary SFC and DISM auto-relaunched the application with admin privileges without asking the user first. This is a security concern — the user must always be in control of privilege elevation. ## Changes ### CleanupViewModel.cs - **RunSfcAsync**: Replaced silent auto-elevation with a Yes/No MessageBox confirmation dialog. If the user declines, the operation is cancelled with a clear status message. - **RunDismAsync**: Same pattern — explicit consent required before elevation. ## Security impact - Users are no longer surprised by UAC prompts or unexpected app restarts - Declining elevation gracefully cancels the operation instead of leaving the app in an undefined state ## Testing - Build: 0 errors - When not elevated + user clicks Yes: app restarts with admin (existing behavior, now with consent) - When not elevated + user clicks No: operation cancelled, status message shown - When already elevated: SFC/DISM runs normally (no dialog shown) Closes #264 Co-authored-by: laurentiu021 <laurentiu021@users.noreply.github.com>
- TuneUpService: orchestrates temp cleanup, Recycle Bin (with confirm), broken shortcut scan, disk SMART check, uptime/RAM monitoring - DashboardViewModel: RunTuneUp command with progress, cancel, dismiss - DashboardView: green Quick Tune-Up button + step progress + summary card - TuneUpResult model with warning aggregation and color-coded verdicts - IntGreaterThanZeroConverter for conditional XAML visibility - OperationLockService integration prevents concurrent disk operations - Non-destructive, no admin required, no registry edits - 35 new unit tests (TuneUpService + DashboardViewModel + converter) - README, ARCHITECTURE, CHANGELOG updated Closes #261 Co-authored-by: laurentiu021 <laurentiu021@users.noreply.github.com>
3 participants
Summary
One-Click PC Tune-Up wizard on the Dashboard tab — a single green button that runs safe, non-destructive maintenance:
Displays step-by-step progress during execution and a dismissible summary card with freed space, disk verdicts, and actionable recommendations.
What it does NOT do
Files added
Files modified
Tests (35 new)
Docs
Closes #261
Summary by CodeRabbit
Release Notes
New Features
Tests
Documentation