Add on-demand orphaned installer patch cleanup with safety quarantine#44
Open
AlrightLad wants to merge 5 commits intoDTC-Inc:mainfrom
Open
Add on-demand orphaned installer patch cleanup with safety quarantine#44AlrightLad wants to merge 5 commits intoDTC-Inc:mainfrom
AlrightLad wants to merge 5 commits intoDTC-Inc:mainfrom
Conversation
Adds Cleanup-InstallerPatches.ps1 — an on-demand NinjaRMM remediation script that removes orphaned .msi/.msp files from C:\Windows\Installer identified by the same registry-based orphan detection used in the monitor script. Designed to be triggered manually by a technician or automatically via NinjaRMM condition when the monitor reports Warning or Critical status. Cleanup operates in five phases: Phase 0 — Pre-flight: Checks C: drive free space. If disk is below 10% free, automatically switches from quarantine mode to direct deletion since quarantining would consume additional disk space on an already-full drive. Phase 1 — DISM Component Cleanup: Runs DISM /Online /Cleanup-Image /StartComponentCleanup to reclaim space from superseded Windows components. Deliberately avoids /ResetBase which would prevent future update uninstalls — too destructive for unattended automation. Skippable with -SkipDISM. Phase 2 — Orphaned Installer Cleanup: Runs the shared orphan detection function against the Windows Installer registry database. In default mode, orphaned files are moved to C:\DTC\InstallerCleanup\Quarantine\<date>\ for safe recovery. With -Force, files are deleted directly. If orphan detection fails for any reason, the entire cleanup aborts with exit code 1 — the script will never delete files it cannot confirm are orphaned. Phase 3 — $PatchCache$ Cleanup: Clears the $PatchCache$ subfolder contents when larger than 1 GB. This folder contains redundant copies of patch data that Windows can re-download if needed. Phase 4 — Quarantine Maintenance: Purges quarantine folders older than -QuarantineDays (default 30 days), reclaiming the space once the safety retention window has passed. Operating modes: Default (quarantine): Moves orphaned files to C:\DTC\InstallerCleanup\Quarantine\ — fully reversible by moving files back -Force: Deletes orphaned files directly — use when disk is critically full or after confirming quarantine contents are safe -WhatIf: Dry run that reports exactly what would be cleaned with zero filesystem changes — recommended as a first pass on any new machine Auto-Force: Engages automatically when C: drive free space drops below 10%, logged prominently as a warning Post-cleanup actions: Re-runs orphan detection to update NinjaRMM custom fields with current state Writes cleanup summary to Windows Application Event Log (Event ID 1001, source DTC-InstallerMonitor) Detailed per-file log written to C:\DTC\InstallerCleanup\Logs\ with timestamps, action taken, file sizes, and success/failure status Safety guarantees: Orphan detection failure aborts the entire cleanup — no blind deletion Locked files are caught and skipped gracefully (logged, continues to next file) All thresholds defined as variables at script top for easy adjustment Self-contained with no external dependencies (PowerShell 5.1 only) Designed to run as SYSTEM on Windows 10, 11, Server 2019, and Server 2022
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds a standalone PowerShell script that identifies orphaned .msi/.msp files in C:\Windows\Installer via registry queries, then quarantines or deletes them with phased execution, optional DISM/PatchCache$ cleanup, quarantine lifecycle maintenance, detailed logging, and NinjaRMM/Event Log reporting. (50 words) Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Script as DTC_Cleanup_Script
participant Registry as Windows_Registry
participant FileSystem as File_System
participant Quarantine as Quarantine_Folder
participant NinjaRMM as NinjaRMM_Service
participant EventLog as Windows_EventLog
User->>Script: Execute (WhatIf/Force/SkipDISM/QuarantineDays)
Script->>Script: Pre-flight checks (disk thresholds, params)
Script->>FileSystem: Enumerate C:\Windows\Installer files
Script->>Registry: Query product & patch references
Registry-->>Script: Referenced file list
Script->>Script: Classify files (referenced vs orphaned)
alt WhatIf
Script-->>User: Report planned actions
else Quarantine mode
Script->>Quarantine: Move orphaned files to dated folder
Quarantine-->>Script: Confirm move / log entry
else Delete mode
Script->>FileSystem: Delete orphaned files
FileSystem-->>Script: Confirm deletion / log entry
end
Script->>FileSystem: Optional PatchCache$ cleanup
Script->>Script: Purge quarantines older than QuarantineDays
Script->>NinjaRMM: Update custom fields / request scan refresh
Script->>EventLog: Write summary event
Script-->>User: Print final summary (counts, recovered space, logfile)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
rmm-ninja/Cleanup-InstallerPatches (2)
416-421: Deadelseif ($WhatIf)branch —$WhatIfis always$falseinsideif (-not $WhatIf).The entire Phase 5 block (line 376) is guarded by
if (-not $WhatIf), so theelseif ($WhatIf)in the event message here-string on line 418 is unreachable. The mode string will always evaluate to either"Direct Delete"or"Quarantine".♻️ Proposed cleanup
-Mode: $(if ($Force) {"Direct Delete"} elseif ($WhatIf) {"WhatIf (no changes)"} else {"Quarantine"}) +Mode: $(if ($Force) {"Direct Delete"} else {"Quarantine"})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rmm-ninja/Cleanup-InstallerPatches` around lines 416 - 421, The here-string assigned to $eventMessage contains an unreachable elseif ($WhatIf) branch because the Phase 5 block is already guarded by if (-not $WhatIf); remove the dead elseif and simplify the mode expression to only check $Force vs fallback (e.g., if ($Force) {"Direct Delete"} else {"Quarantine"}) or compute a $mode variable before the here-string and use that; update references to $Force, $WhatIf, and $eventMessage/Phase 5 accordingly.
232-234: DISM exit code is not checked — failures are only visible in raw[DISM]output lines.A non-zero
$LASTEXITCODE(e.g.,0x800F081F,87) after DISM is silently ignored. The cleanup summary and NinjaRMM fields never reflect a DISM failure.♻️ Proposed fix — check
$LASTEXITCODE$dismResult = & DISM /Online /Cleanup-Image /StartComponentCleanup 2>&1 $dismResult | ForEach-Object { Write-Output "[DISM] $_" } +if ($LASTEXITCODE -ne 0) { + Write-Warning "[PHASE 1] DISM exited with code $LASTEXITCODE — component cleanup may be incomplete." +} Write-Output ""🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rmm-ninja/Cleanup-InstallerPatches` around lines 232 - 234, The DISM invocation stores output in $dismResult but never checks $LASTEXITCODE, so failures (non-zero exit codes like 0x800F081F or 87) are only visible in the raw "[DISM] ..." lines; update the logic after running DISM (the block using $dismResult and the ForEach-Object/Write-Output lines) to inspect $LASTEXITCODE, treat any non-zero value as a failure, and propagate that state into the cleanup summary and NinjaRMM fields (e.g., set failure status/message variables or call the existing report/update function used for other steps) so the overall result reflects DISM errors instead of silently succeeding.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rmm-ninja/Cleanup-InstallerPatches`:
- Line 37: The script allows zero or negative values for $QuarantineDays which
causes (Get-Date).AddDays(-$QuarantineDays) to delete everything; add input
validation: attach a [ValidateRange(1,9999)] (or appropriate max) to the
parameter declaration for [int]$QuarantineDays or add an explicit guard early
(e.g., if ($QuarantineDays -lt 1) { throw "QuarantineDays must be >= 1" })
before computing the cutoff, and apply the same fix to the other occurrences of
the cutoff logic referenced near the second instance (occurrences around the
code that uses (Get-Date).AddDays(-$QuarantineDays)).
- Line 260: The quarantine path currently uses only date (variable
$quarantinePath) causing multiple runs the same day to share one folder and
letting Move-Item -Force silently overwrite files; change the path construction
to append a time component (e.g., Get-Date -Format 'yyyy-MM-dd_HHmmss' to match
$logFile) or, alternately, detect destination name collisions before calling
Move-Item and rename files (e.g., append a unique suffix or incrementing
counter) so Move-Item is never allowed to overwrite an existing quarantined
file; update all places that build $quarantinePath (including the other
occurrences noted around the Move-Item calls) to use the new uniquifying scheme.
- Around line 203-212: Replace the deprecated Get-WmiObject call with
Get-CimInstance to query Win32_LogicalDisk for DeviceID 'C:' and add a
null-guard and error handling before dereferencing $cDrive: wrap the query in
try/catch (or check for $null) and if $cDrive is $null,
Write-Warning/Write-Output a clear message and set safe defaults for
$freePercent and $freeSizeGB (e.g. 0 or -1) or abort gracefully so the script
doesn't call methods on a null; then continue using the existing
$AutoForceThresholdPct and $Force logic (only toggle $Force when $freePercent is
a valid number) to avoid the null-valued expression crash.
- Around line 1-31: The file is missing the .ps1 extension and uses PascalCase
which breaks NinjaRMM detection and repo naming conventions; rename the script
file from Cleanup-InstallerPatches to cleanup-installer-patches.ps1 and update
any references (task definitions, docs, or automation entries) that point to the
old filename so the condition-triggered runner and on-demand invocation can find
and execute the script.
---
Nitpick comments:
In `@rmm-ninja/Cleanup-InstallerPatches`:
- Around line 416-421: The here-string assigned to $eventMessage contains an
unreachable elseif ($WhatIf) branch because the Phase 5 block is already guarded
by if (-not $WhatIf); remove the dead elseif and simplify the mode expression to
only check $Force vs fallback (e.g., if ($Force) {"Direct Delete"} else
{"Quarantine"}) or compute a $mode variable before the here-string and use that;
update references to $Force, $WhatIf, and $eventMessage/Phase 5 accordingly.
- Around line 232-234: The DISM invocation stores output in $dismResult but
never checks $LASTEXITCODE, so failures (non-zero exit codes like 0x800F081F or
87) are only visible in the raw "[DISM] ..." lines; update the logic after
running DISM (the block using $dismResult and the ForEach-Object/Write-Output
lines) to inspect $LASTEXITCODE, treat any non-zero value as a failure, and
propagate that state into the cleanup summary and NinjaRMM fields (e.g., set
failure status/message variables or call the existing report/update function
used for other steps) so the overall result reflects DISM errors instead of
silently succeeding.
Comment on lines
+1
to
+31
| <# | ||
| .SYNOPSIS | ||
| DTC Installer Patch Cleanup — On-demand removal of orphaned .msi/.msp files | ||
| from C:\Windows\Installer with safety quarantine. | ||
| .DESCRIPTION | ||
| Detects and removes orphaned installer cache files from C:\Windows\Installer. | ||
| Supports quarantine mode (default), direct deletion (-Force), and dry-run (-WhatIf). | ||
| Includes DISM component cleanup, $PatchCache$ cleanup, and quarantine expiry management. | ||
|
|
||
| Safety: If orphan detection fails for ANY reason, the entire cleanup is aborted. | ||
|
|
||
| Deployment: NinjaRMM on-demand script (tech triggers manually) or condition-triggered. | ||
| Runs as: SYSTEM | ||
|
|
||
| Reference: HALO Ticket 1125653 — 128 GB orphaned patches, 96.1% disk utilization. | ||
| .PARAMETER WhatIf | ||
| Report what WOULD be cleaned — make ZERO filesystem changes. | ||
| .PARAMETER SkipDISM | ||
| Skip the DISM component cleanup phase. | ||
| .PARAMETER Force | ||
| Delete orphaned files directly instead of quarantining. Auto-enabled when disk < 10% free. | ||
| .PARAMETER QuarantineDays | ||
| Number of days to keep quarantined files before auto-purge. Default: 30. | ||
| .NOTES | ||
| Author: DTC Engineering | ||
| Version: 1.0.0 | ||
| Requires: PowerShell 5.1, Windows 10/11/Server 2019/2022 | ||
| Dependencies: None (self-contained) | ||
| #> | ||
|
|
||
| #Requires -Version 5.1 |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for the file without .ps1 extension and list all scripts in rmm-ninja/
echo "=== Files matching 'Cleanup-InstallerPatches' (any extension) ==="
fd "Cleanup-InstallerPatches" rmm-ninja/ 2>/dev/null || echo "No matches found"
echo ""
echo "=== All .ps1 files in rmm-ninja/ ==="
fd --type f --extension ps1 rmm-ninja/ 2>/dev/null || echo "No .ps1 files found"
echo ""
echo "=== All files in rmm-ninja/ directory (top level and first subdirs) ==="
find rmm-ninja/ -maxdepth 2 -type f 2>/dev/null | head -30Repository: DTC-Inc/msp-script-library
Length of output: 402
Rename file to cleanup-installer-patches.ps1 — missing extension and incorrect naming convention.
The file is stored as Cleanup-InstallerPatches without .ps1 extension and uses PascalCase naming. PowerShell script filenames must use lowercase-with-hyphens format (e.g., veeam-add-backup-repo.ps1). NinjaRMM's automation engine identifies scripts by extension; without it, the script will not be detected or executed by the condition-triggered runner or on-demand technician invocation. Additionally, the PascalCase naming violates the repository's PowerShell script naming convention.
Rename to cleanup-installer-patches.ps1.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rmm-ninja/Cleanup-InstallerPatches` around lines 1 - 31, The file is missing
the .ps1 extension and uses PascalCase which breaks NinjaRMM detection and repo
naming conventions; rename the script file from Cleanup-InstallerPatches to
cleanup-installer-patches.ps1 and update any references (task definitions, docs,
or automation entries) that point to the old filename so the condition-triggered
runner and on-demand invocation can find and execute the script.
Addresses all 6 findings from the CodeRabbit review on PR DTC-Inc#44, plus proactive fixes for patterns shared with the monitor script that would trigger findings on a second pass. Deprecated Get-WmiObject replaced with Get-CimInstance (crash fix): Get-WmiObject has been deprecated since PowerShell 3.0 in favor of Get-CimInstance and is absent entirely on PowerShell 7+. More critically, if the WMI/CIM query returns $null (possible on non-standard disk configurations, WMI corruption, or running inside certain container environments), the very next line calls .FreeSpace on a null object — a terminating error that aborts the script before any cleanup work begins. Added a null guard: if the CIM query fails, free-space check and auto-Force logic are skipped with a warning, defaulting to safe values (100% free, -1 GB) so the script continues into the cleanup phases rather than dying on pre-flight. QuarantineDays zero/negative purges entire quarantine (data loss fix): (Get-Date).AddDays(-$QuarantineDays) with a zero or negative value sets the cutoff date to the present or future, which matches every existing quarantine folder and silently destroys the entire recovery safety net in Phase 4. Added [ValidateRange(1, 365)] to the parameter declaration so PowerShell rejects invalid values at invocation time with a clear error message rather than allowing silent destructive behavior. The 365-day upper bound prevents accidental permanent retention that would defeat the purpose of quarantine expiry. Same-day quarantine path collision (data loss fix): The quarantine folder used date-only format (yyyy-MM-dd), so multiple cleanup runs on the same day shared one folder. Move-Item -Force silently overwrites a previously quarantined file with the same name — meaning a second run would destroy the first run's quarantined files with no error logged, eliminating the recovery path. Changed the format to yyyy-MM-dd_HHmmss so each run gets its own unique folder. This also makes forensic review easier since each quarantine folder maps to a specific cleanup execution. DISM exit code not checked (silent failure fix): The script captured DISM's stdout/stderr output and logged it line-by-line, but never checked $LASTEXITCODE. Non-zero DISM exit codes (e.g., 0x800F081F missing source, 87 invalid parameter, or timeout errors on slow disks) were silently swallowed — the cleanup would report success despite DISM component cleanup failing. Added an explicit check: if $LASTEXITCODE is non-zero, a warning is emitted with the exit code so technicians reviewing the NinjaRMM script log can see that DISM didn't complete cleanly. Dead elseif ($WhatIf) branch removed (dead code cleanup): The Phase 5 event log message included elseif ($WhatIf) {"WhatIf (no changes)"} in the Mode string, but the entire Phase 5 block is guarded by if (-not $WhatIf). The WhatIf branch was unreachable — the Mode string could only ever evaluate to "Direct Delete" or "Quarantine". Removed the dead branch to avoid confusion during future maintenance where someone might assume WhatIf event logging is implemented when it isn't. Unprotected EventLog SourceExists() call (crash fix): Same issue as the monitor script — [System.Diagnostics.EventLog]::SourceExists() throws a terminating SecurityException if the caller lacks permission to enumerate registered event-log sources. Wrapped in try/catch so the cleanup completes and reports results even if event source registration fails. This is especially important in the cleanup script since the actual disk recovery work has already been done by this point — crashing during reporting would leave the technician with no summary of what was cleaned. Post-cleanup threshold comparisons fixed to -ge (consistency fix): The Phase 5 post-cleanup status determination used -gt while the spec defines thresholds as >= values. This created an inconsistency where the cleanup script's post-run status could differ from the monitor script's status for the same folder size, causing confusing NinjaRMM field values. Changed to -ge to match both the spec and the monitor script. Null coercion on Measure-Object .Sum throughout (defensive arithmetic): Applied -as [long] coercion with explicit zero fallback to every Measure-Object .Sum call outside the shared function: $baselineSize in Phase 0, $patchCacheSize in Phase 3, and quarantine folder size measurement in Phase 4. Also applied the same fix to the shared Get-OrphanedInstallerFiles function (identical to the monitor script fix) so the function is safe on machines with an empty C:\Windows\Installer folder.
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
rmm-ninja/Cleanup-InstallerPatches (4)
148-165: Array+=in loop is O(n²) — use[System.Collections.Generic.List[PSCustomObject]].
$referenced += ...and$orphaned += ...inside theforeachloop re-allocates the entire array on every iteration. On systems with thousands of installer files (the target use case per the ticket — 128 GB of patches), this becomes a noticeable bottleneck.Suggested fix
# --- STEP 3: Compare and classify --- - $referenced = @() - $orphaned = @() + $referenced = [System.Collections.Generic.List[PSCustomObject]]::new() + $orphaned = [System.Collections.Generic.List[PSCustomObject]]::new() foreach ($file in $allFiles) { if ($referencedFiles.Contains($file.FullName)) { - $referenced += [PSCustomObject]@{ + $referenced.Add([PSCustomObject]@{ FullPath = $file.FullName SizeBytes = $file.Length - } + }) } else { - $orphaned += [PSCustomObject]@{ + $orphaned.Add([PSCustomObject]@{ FullPath = $file.FullName SizeBytes = $file.Length LastWriteTime = $file.LastWriteTime - } + }) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rmm-ninja/Cleanup-InstallerPatches` around lines 148 - 165, The loop is repeatedly reallocating arrays by using $referenced += ... and $orphaned += ..., causing O(n^2) behavior; replace the array accumulators with strongly-typed lists (e.g. [System.Collections.Generic.List[PSCustomObject]]), initialize them before the foreach, use the .Add(...) method inside the foreach that iterates $allFiles, and if an array is required later convert the lists back with .ToArray() or cast to [PSCustomObject[]]; update references to $referenced and $orphaned accordingly (e.g. when building the PSCustomObject entries and when consuming the collections).
44-47: Configuration variables use PascalCase instead of camelCase.Per repository conventions, PowerShell variables should use camelCase.
Suggested rename
-$WarningThresholdGB = 20 -$CriticalThresholdGB = 50 -$AutoForceThresholdPct = 10 -$PatchCacheThresholdGB = 1 +$warningThresholdGB = 20 +$criticalThresholdGB = 50 +$autoForceThresholdPct = 10 +$patchCacheThresholdGB = 1Based on learnings: "Use camelCase for PowerShell variables and functions."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rmm-ninja/Cleanup-InstallerPatches` around lines 44 - 47, The variables use PascalCase ($WarningThresholdGB, $CriticalThresholdGB, $AutoForceThresholdPct, $PatchCacheThresholdGB); rename them to camelCase (e.g., $warningThresholdGB, $criticalThresholdGB, $autoForceThresholdPct, $patchCacheThresholdGB) and update every reference (assignments, checks, functions, exported settings) to these symbols to preserve behavior and repo conventions; ensure any documentation, help text, or tests that reference the old names are updated too.
338-346:$totalRecoveredBytesoverstated if$PatchCache$files are locked or fail to delete.Line 342 uses
-ErrorAction SilentlyContinueonRemove-Item, silently skipping locked files. Line 343 then unconditionally adds the full pre-deletion$patchCacheSizeto$totalRecoveredBytes. The same pattern applies to Phase 2 quarantine failures — thecatchblock doesn't decrement the counter, but at least it logs the failure. Here, there's no visibility into partial deletion at all.Consider measuring the folder again after deletion, or accumulating only successful deletes:
Get-ChildItem $patchCachePath -Recurse -Force -ErrorAction SilentlyContinue | Remove-Item -Recurse -Force -ErrorAction SilentlyContinue - $totalRecoveredBytes += $patchCacheSize + $postCacheSize = (Get-ChildItem $patchCachePath -Recurse -Force -ErrorAction SilentlyContinue | + Measure-Object Length -Sum).Sum -as [long] + if (-not $postCacheSize) { $postCacheSize = [long]0 } + $totalRecoveredBytes += ($patchCacheSize - $postCacheSize)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rmm-ninja/Cleanup-InstallerPatches` around lines 338 - 346, The script currently adds the full $patchCacheSize to $totalRecoveredBytes even if Remove-Item skipped locked files; change this to compute actual reclaimed bytes after the delete attempt: capture the pre-delete size ($patchCacheSize or Measure-Object on $patchCachePath), perform deletion (prefer iterating files under $patchCachePath and removing each so failures are visible), then re-measure remaining size with Get-ChildItem $patchCachePath -Recurse | Measure-Object -Sum Length and compute recovered = preDeleteSize - postDeleteSize; add recovered to $totalRecoveredBytes and log the actual recovered bytes to $logFile (use the same log entry location instead of unconditionally logging $patchCacheSize).
1-39: Missing RMM three-part structure and variable declaration block.This script is deployed via NinjaRMM but does not follow the required three-part structure: (1) RMM variable declaration comment block, (2) input handling with
$RMMdetection, (3) script logic. There is no## PLEASE COMMENT YOUR VARIABLES DIRECTLY BELOW HERE IF YOU'RE RUNNING FROM A RMMheader, and no$RMMdetection to differentiate interactive vs. RMM execution paths. The$Force,$SkipDISM,$QuarantineDaysparameters would need to be settable as RMM variables for condition-triggered runs.Suggested structure addition after line 31
`#Requires` -Version 5.1 +## PLEASE COMMENT YOUR VARIABLES DIRECTLY BELOW HERE IF YOU'RE RUNNING FROM A RMM +## $WhatIf +## $SkipDISM +## $Force +## $QuarantineDays + param( [switch]$WhatIf, [switch]$SkipDISM, [switch]$Force, [ValidateRange(1, 365)] [int]$QuarantineDays = 30 )Based on learnings: "All PowerShell scripts must follow the three-part structure from script-template-powershell.ps1 (RMM variable declaration, input handling, script logic)" and "Top-of-file RMM variable declaration: include a comment block headed with '## PLEASE COMMENT YOUR VARIABLES DIRECTLY BELOW HERE IF YOU'RE RUNNING FROM A RMM' and list each required variable as comments."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rmm-ninja/Cleanup-InstallerPatches` around lines 1 - 39, The script lacks the required NinjaRMM three-part structure: add a top-of-file RMM variable declaration comment block headed with "## PLEASE COMMENT YOUR VARIABLES DIRECTLY BELOW HERE IF YOU'RE RUNNING FROM A RMM" listing the RMM-mappable variables (WhatIf, SkipDISM, Force, QuarantineDays), then implement an input-handling section that detects the $RMM sentinel and, when present, reads/assigns RMM inputs into the script variables (populate $Force, $SkipDISM, $QuarantineDays, and optionally $WhatIf) before executing the existing script logic; ensure this detection uses a clear $RMM check and sets types/ValidateRange for QuarantineDays to match the param block so both interactive and RMM-triggered runs behave the same.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rmm-ninja/Cleanup-InstallerPatches`:
- Around line 289-321: The loop currently increments $totalRecoveredBytes when
Move-Item in the non-Force (quarantine) branch succeeds even though files remain
on the same volume; update the logic so $totalRecoveredBytes only reflects
actual disk-space-recovered (i.e., when files are deleted), and introduce a
separate counter (e.g., $installerFolderFreedBytes) for space freed from the
Installer folder by Move-Item/quarantine; adjust the branches around Move-Item
and Remove-Item (referencing Move-Item, Remove-Item, $totalRecoveredBytes,
$quarantinePath, $Force, $WhatIf) to increment the correct counter and ensure
the final summary uses the appropriate field names and messaging (Disk space
recovered vs. Installer-folder space freed).
- Around line 196-222: The initial "Mode:" Write-Output is printed before the
auto-Force logic, so when $freePercent falls below $AutoForceThresholdPct the
script may switch $Force to $true but the startup log still shows "Quarantine";
update the script so the mode line reflects the final decision by moving the
mode announcement (the Write-Output that checks $WhatIf/$Force) to after the
auto-Force block that sets $Force based on $freePercent, or alternatively keep
the original line but add a corrected Write-Output immediately after the
auto-Force check that logs the updated mode and notes that auto-Force was
applied when $Force is changed by the $AutoForceThresholdPct check.
---
Nitpick comments:
In `@rmm-ninja/Cleanup-InstallerPatches`:
- Around line 148-165: The loop is repeatedly reallocating arrays by using
$referenced += ... and $orphaned += ..., causing O(n^2) behavior; replace the
array accumulators with strongly-typed lists (e.g.
[System.Collections.Generic.List[PSCustomObject]]), initialize them before the
foreach, use the .Add(...) method inside the foreach that iterates $allFiles,
and if an array is required later convert the lists back with .ToArray() or cast
to [PSCustomObject[]]; update references to $referenced and $orphaned
accordingly (e.g. when building the PSCustomObject entries and when consuming
the collections).
- Around line 44-47: The variables use PascalCase ($WarningThresholdGB,
$CriticalThresholdGB, $AutoForceThresholdPct, $PatchCacheThresholdGB); rename
them to camelCase (e.g., $warningThresholdGB, $criticalThresholdGB,
$autoForceThresholdPct, $patchCacheThresholdGB) and update every reference
(assignments, checks, functions, exported settings) to these symbols to preserve
behavior and repo conventions; ensure any documentation, help text, or tests
that reference the old names are updated too.
- Around line 338-346: The script currently adds the full $patchCacheSize to
$totalRecoveredBytes even if Remove-Item skipped locked files; change this to
compute actual reclaimed bytes after the delete attempt: capture the pre-delete
size ($patchCacheSize or Measure-Object on $patchCachePath), perform deletion
(prefer iterating files under $patchCachePath and removing each so failures are
visible), then re-measure remaining size with Get-ChildItem $patchCachePath
-Recurse | Measure-Object -Sum Length and compute recovered = preDeleteSize -
postDeleteSize; add recovered to $totalRecoveredBytes and log the actual
recovered bytes to $logFile (use the same log entry location instead of
unconditionally logging $patchCacheSize).
- Around line 1-39: The script lacks the required NinjaRMM three-part structure:
add a top-of-file RMM variable declaration comment block headed with "## PLEASE
COMMENT YOUR VARIABLES DIRECTLY BELOW HERE IF YOU'RE RUNNING FROM A RMM" listing
the RMM-mappable variables (WhatIf, SkipDISM, Force, QuarantineDays), then
implement an input-handling section that detects the $RMM sentinel and, when
present, reads/assigns RMM inputs into the script variables (populate $Force,
$SkipDISM, $QuarantineDays, and optionally $WhatIf) before executing the
existing script logic; ensure this detection uses a clear $RMM check and sets
types/ValidateRange for QuarantineDays to match the param block so both
interactive and RMM-triggered runs behave the same.
Cleanup script changes: Mode announcement moved after auto-Force decision (misleading log fix): The mode line was printed before the free-space check that can auto-enable Force mode. If disk was below 10% free and -Force wasn't explicitly set, the startup log would say "Quarantine" but the script would actually operate in Force mode. Operators reviewing NinjaRMM script activity would see contradictory information. Moved the Write-Output to after the auto-Force block. Recovery metrics split into two counters (misleading metric fix): $totalRecoveredBytes conflated "installer folder freed" with "disk space recovered." In quarantine mode, Move-Item moves files from C:\Windows\Installer to C:\DTC\InstallerCleanup\Quarantine — both on C:. No disk space is freed (data stays on the same volume, only the directory entry changes). Yet the summary reported this as "Space recovered." Now tracks: $installerFolderFreedBytes: bytes removed from C:\Windows\Installer (Move or Delete — what shrinks the folder for monitor threshold purposes) $diskSpaceRecoveredBytes: bytes actually freed from disk (Delete only + Phase 4 quarantine purge) NinjaRMM installerCleanupRecoveredGB now reports actual disk recovery. Console summary shows both metrics with a note explaining that quarantined files will free disk space when quarantine expires. PatchCache$ recovery re-measured after deletion (inflated metric fix): Previously added the full pre-deletion $patchCacheSize to recovered bytes even though Remove-Item -ErrorAction SilentlyContinue silently skips locked files. Now re-measures the folder after deletion and computes $actualCacheRecovered = $preSize - $postSize. Log entry shows "X of Y bytes" so technicians can see if locked files prevented full cleanup. Quarantine purge recovery verified after deletion (inflated metric fix): Same pattern as PatchCache$ — Phase 4 now checks Test-Path after Remove-Item. If the folder is fully deleted, counts the full size. If partially remaining (e.g., locked files), re-measures and counts only the actual difference. PatchCache threshold -gt → -ge (consistency fix): All other threshold comparisons in both scripts use -ge (greater-or-equal). PatchCache$ used -gt (strictly greater), meaning a folder exactly at the 1 GB threshold would be skipped. Changed to -ge for consistent boundary behavior. Post-cleanup threshold uses unfiltered folder total: Phase 5 post-cleanup re-scan now uses InstallerFolderTotalBytes for the threshold comparison, matching the monitor script's behavior so the post-cleanup status in NinjaRMM is consistent with what the weekly monitor would report.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
rmm-ninja/Cleanup-InstallerPatches (3)
44-47: Configuration variable names use PascalCase; repo convention requires camelCase.
$WarningThresholdGB,$CriticalThresholdGB,$AutoForceThresholdPct, and$PatchCacheThresholdGBshould be$warningThresholdGB,$criticalThresholdGB,$autoForceThresholdPct, and$patchCacheThresholdGB. Cascading renames also apply to every downstream reference.Based on learnings: "Use camelCase for PowerShell variables and functions."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rmm-ninja/Cleanup-InstallerPatches` around lines 44 - 47, The variables use PascalCase but repo convention requires camelCase; rename $WarningThresholdGB, $CriticalThresholdGB, $AutoForceThresholdPct, and $PatchCacheThresholdGB to $warningThresholdGB, $criticalThresholdGB, $autoForceThresholdPct, and $patchCacheThresholdGB respectively, and update every downstream reference (functions, conditionals, assignments, and any exported/config usages) to the new names so all callers (e.g., any code that reads these thresholds) compile and behave the same; ensure you update comments and tests that reference the old symbols and run a quick grep/IDE rename to catch all occurrences.
383-384: Quarantine expiry relies onCreationTime, which can be reset on copy/restore.
$_.CreationTimeis an NTFS metadata attribute that can be silently reset to the current time if theC:\DTCtree is ever copied, restored from backup, or moved across volumes. Folder names already encodeyyyy-MM-dd_HHmmss; parsing the name for the cutoff comparison is more reliable and self-documenting.♻️ Proposed alternative — parse date from folder name
- $expiredFolders = Get-ChildItem $quarantineRoot -Directory | - Where-Object { $_.CreationTime -lt $cutoffDate } + $expiredFolders = Get-ChildItem $quarantineRoot -Directory | Where-Object { + $folderDate = $null + if ([datetime]::TryParseExact( + ($_.Name -replace '_\d{6}$', ''), # strip HHmmss suffix + 'yyyy-MM-dd', + [System.Globalization.CultureInfo]::InvariantCulture, + [System.Globalization.DateTimeStyles]::None, + [ref]$folderDate)) { + $folderDate -lt $cutoffDate + } else { + $_.CreationTime -lt $cutoffDate # fallback for non-standard names + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rmm-ninja/Cleanup-InstallerPatches` around lines 383 - 384, The current quarantine expiry filter uses filesystem CreationTime which can be reset; update the logic that builds $expiredFolders (the pipeline starting with Get-ChildItem $quarantineRoot | Where-Object ...) to instead parse the timestamp from each folder's name (folders follow yyyy-MM-dd_HHmmss), validate the name with a regex, convert using [DateTime]::ParseExact or TryParseExact, and compare that parsed DateTime to $cutoffDate so only folders with a parsed timestamp older than $cutoffDate are selected; keep the filter limited to directories and skip names that don’t match the pattern to avoid false positives.
33-50: Add the required RMM variable declaration comment block.The repo template mandates a top-of-file block headed
## PLEASE COMMENT YOUR VARIABLES DIRECTLY BELOW HERE IF YOU'RE RUNNING FROM A RMMlisting each configurable variable as a comment. This serves as in-platform documentation for technicians configuring the script in NinjaRMM's script library. The block and the$RMMdetection pattern are both absent.♻️ Suggested addition (top of file, before `param`)
+## PLEASE COMMENT YOUR VARIABLES DIRECTLY BELOW HERE IF YOU'RE RUNNING FROM A RMM +## $Force # Set to $true to delete orphaned files directly instead of quarantining +## $WhatIf # Set to $true for a dry-run (no filesystem changes) +## $SkipDISM # Set to $true to skip Phase 1 DISM component cleanup +## $QuarantineDays # Days to retain quarantined files before auto-purge (default: 30, range: 1-365) + `#Requires` -Version 5.1Based on learnings: "Top-of-file RMM variable declaration: include a comment block headed with '## PLEASE COMMENT YOUR VARIABLES DIRECTLY BELOW HERE IF YOU'RE RUNNING FROM A RMM' and list each required variable as comments."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rmm-ninja/Cleanup-InstallerPatches` around lines 33 - 50, Add the missing top-of-file RMM variable declaration comment block before the existing param(...) by inserting a header comment "## PLEASE COMMENT YOUR VARIABLES DIRECTLY BELOW HERE IF YOU'RE RUNNING FROM A RMM" followed by commented lines for each configurable variable present in the script (e.g., $WarningThresholdGB, $CriticalThresholdGB, $AutoForceThresholdPct, $PatchCacheThresholdGB, $QuarantineDays, $WhatIf, $SkipDISM, $Force) and add the $RMM detection pattern/variable check (referencing $RMM) so the script documents and conditionally supports running from NinjaRMM; ensure the comments mirror the variable names exactly and appear immediately above the param block.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rmm-ninja/Cleanup-InstallerPatches`:
- Around line 249-256: The synchronous call to DISM via "& DISM /Online
/Cleanup-Image /StartComponentCleanup" (captured in $dismResult and checked with
$LASTEXITCODE) can exceed the RMM execution window; replace this direct call
with launching DISM as a background job (e.g., Start-Job) and use a bounded wait
(Wait-Job -Timeout) so the script continues to Phases 2–4 if the timeout is
reached, then capture/pipe the job output into the same logging path, check/kill
the job on timeout, log a clear warning including the job timeout and any
partial results, and set appropriate status fields; if you cannot implement a
background job, instead add a .NOTES runtime requirement (e.g., >=120 minutes)
and emit a prominent runtime warning before invoking DISM so callers can
increase the NinjaRMM script timeout.
- Around line 289-294: The quarantine directory creation (New-Item for
$quarantinePath) is not verified and errors are suppressed, so subsequent
Move-Item calls will fail and produce only per-file "[QUARANTINE-FAILED]" logs;
update the block that runs when -not $WhatIf to check the result/error from
New-Item for $quarantinePath (or use New-Item with -ErrorAction Stop inside a
try/catch) and abort the script (or set a visible error state) if creation fails
when -not $Force, ensuring the code paths that later call Move-Item detect and
do not proceed without a valid quarantine directory; reference $quarantinePath,
New-Item, Move-Item, WhatIf and Force to locate the changes.
---
Duplicate comments:
In `@rmm-ninja/Cleanup-InstallerPatches`:
- Around line 1-31: The script filename must be renamed so NinjaRMM can detect
it; change the repository file named "Cleanup-InstallerPatches" to
"cleanup-installer-patches.ps1" (kebab-case with the .ps1 extension) and ensure
any external references or deployment manifests that call this script use the
new name; do not modify the script body (e.g., the existing .SYNOPSIS/.PARAMETER
blocks or the `#Requires` -Version 5.1 line).
---
Nitpick comments:
In `@rmm-ninja/Cleanup-InstallerPatches`:
- Around line 44-47: The variables use PascalCase but repo convention requires
camelCase; rename $WarningThresholdGB, $CriticalThresholdGB,
$AutoForceThresholdPct, and $PatchCacheThresholdGB to $warningThresholdGB,
$criticalThresholdGB, $autoForceThresholdPct, and $patchCacheThresholdGB
respectively, and update every downstream reference (functions, conditionals,
assignments, and any exported/config usages) to the new names so all callers
(e.g., any code that reads these thresholds) compile and behave the same; ensure
you update comments and tests that reference the old symbols and run a quick
grep/IDE rename to catch all occurrences.
- Around line 383-384: The current quarantine expiry filter uses filesystem
CreationTime which can be reset; update the logic that builds $expiredFolders
(the pipeline starting with Get-ChildItem $quarantineRoot | Where-Object ...) to
instead parse the timestamp from each folder's name (folders follow
yyyy-MM-dd_HHmmss), validate the name with a regex, convert using
[DateTime]::ParseExact or TryParseExact, and compare that parsed DateTime to
$cutoffDate so only folders with a parsed timestamp older than $cutoffDate are
selected; keep the filter limited to directories and skip names that don’t match
the pattern to avoid false positives.
- Around line 33-50: Add the missing top-of-file RMM variable declaration
comment block before the existing param(...) by inserting a header comment "##
PLEASE COMMENT YOUR VARIABLES DIRECTLY BELOW HERE IF YOU'RE RUNNING FROM A RMM"
followed by commented lines for each configurable variable present in the script
(e.g., $WarningThresholdGB, $CriticalThresholdGB, $AutoForceThresholdPct,
$PatchCacheThresholdGB, $QuarantineDays, $WhatIf, $SkipDISM, $Force) and add the
$RMM detection pattern/variable check (referencing $RMM) so the script documents
and conditionally supports running from NinjaRMM; ensure the comments mirror the
variable names exactly and appear immediately above the param block.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
rmm-ninja/Cleanup-InstallerPatches (2)
44-48: Configuration variable names use PascalCase instead of camelCase.
$WarningThresholdGB,$CriticalThresholdGB,$AutoForceThresholdPct,$PatchCacheThresholdGB, and$DismTimeoutMinutesshould be camelCase ($warningThresholdGB,$criticalThresholdGB, etc.). The same applies toGet-OrphanedInstallerFileson line 53, though that name follows the standard PowerShellVerb-Nounconvention which directly conflicts with the camelCase guideline — worth a team decision on which takes precedence for function names.Based on learnings: "Use camelCase for PowerShell variables and functions."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rmm-ninja/Cleanup-InstallerPatches` around lines 44 - 48, Rename the configuration variables from PascalCase to camelCase (e.g., $WarningThresholdGB -> $warningThresholdGB, $CriticalThresholdGB -> $criticalThresholdGB, $AutoForceThresholdPct -> $autoForceThresholdPct, $PatchCacheThresholdGB -> $patchCacheThresholdGB, $DismTimeoutMinutes -> $dismTimeoutMinutes) and update every reference/usages across the file so names remain consistent; for the function Get-OrphanedInstallerFiles, decide the team convention (keep PowerShell Verb-Noun or rename to camelCase like getOrphanedInstallerFiles) and then rename the function and all its callers accordingly to avoid mismatches.
301-302:$logPathis hardcoded; it should follow the$LogPathconvention set in the RMM/interactive input-handling section.
C:\DTC\InstallerCleanup\Logsis not%WINDIR%\logs, which is where technicians are expected to look after execution and where the repo's RMM simulation guidance directs verification. Once the three-part structure (above) is added,$logPathshould derive from the$LogPathvariable established during input handling:-$logPath = "C:\DTC\InstallerCleanup\Logs" +$logPath = $LogPath # set to $env:WINDIR\logs or $RMMScriptPath\logs during input handlingBased on learnings: "set $LogPath to %WINDIR%\logs in interactive, and $RMMScriptPath\logs (fallback to %WINDIR%\logs) in RMM" and "Verify transcripts/logs in %WINDIR%\logs after execution."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rmm-ninja/Cleanup-InstallerPatches` around lines 301 - 302, Replace the hardcoded $logPath with a path derived from the existing $LogPath variable (respecting the three-part structure introduced in input handling): if $LogPath is set use it, otherwise fall back to "$Env:Windir\logs"; then append the InstallerCleanup/Logs subfolders (so $logPath is Join-Path $LogPath "InstallerCleanup" "Logs" or the fallback) and ensure the directory is created before computing $logFile (which should keep the existing timestamped filename logic). Use the variables $logPath, $LogPath, and $logFile to locate and implement this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rmm-ninja/Cleanup-InstallerPatches`:
- Around line 33-48: The script is missing the required RMM three-part scaffold;
add a top-of-file RMM variable declaration comment block (the "## PLEASE COMMENT
YOUR VARIABLES..." section) declaring defaults for $RMM, $RMMScriptPath,
$LogPath, $Description and exposing the existing params ($WhatIf, $SkipDISM,
$Force, $QuarantineDays) for RMM override, then implement an if ($RMM -eq 1)
branch that reads inputs from those RMM variables (setting $LogPath = Join-Path
$RMMScriptPath 'logs' with a fallback to $env:windir\logs) and an else
interactive path that prompts with Read-Host/validation loops to populate
$QuarantineDays, $Force, $SkipDISM and capture $Description, ensuring the same
variable names used by the rest of the script (e.g., $WarningThresholdGB,
$PatchCacheThresholdGB, $DismTimeoutMinutes) remain untouched; finally ensure
$Description is always set and present before proceeding.
---
Duplicate comments:
In `@rmm-ninja/Cleanup-InstallerPatches`:
- Around line 304-310: The quarantine directory creation is unguarded: New-Item
-Path $quarantinePath is piped to Out-Null with no -ErrorAction Stop or
existence check, so later Move-Item calls silently fail; update the block that
creates quarantinePath (check the WhatIf/Force branch where New-Item is called)
to explicitly test for existence (Test-Path $quarantinePath) and attempt
creation with New-Item -ErrorAction Stop, capture/handle any exceptions, and if
creation fails log an explicit error and set a failure flag or throw so Phase 2
does not silently no-op when Move-Item operations would later fail.
- Around line 1-31: The file is misnamed and missing the .ps1 extension; rename
the script from "Cleanup-InstallerPatches" to the recommended lowercase,
hyphenated "cleanup-installer-patches.ps1" and update any external
references/callers to that new name (search for usages of
Cleanup-InstallerPatches); ensure internal metadata/comments (like .SYNOPSIS or
.PARAMETER descriptions) remain unchanged and that any packaging/deployment
manifests or NinjaRMM job definitions referencing the old name are updated
accordingly.
---
Nitpick comments:
In `@rmm-ninja/Cleanup-InstallerPatches`:
- Around line 44-48: Rename the configuration variables from PascalCase to
camelCase (e.g., $WarningThresholdGB -> $warningThresholdGB,
$CriticalThresholdGB -> $criticalThresholdGB, $AutoForceThresholdPct ->
$autoForceThresholdPct, $PatchCacheThresholdGB -> $patchCacheThresholdGB,
$DismTimeoutMinutes -> $dismTimeoutMinutes) and update every reference/usages
across the file so names remain consistent; for the function
Get-OrphanedInstallerFiles, decide the team convention (keep PowerShell
Verb-Noun or rename to camelCase like getOrphanedInstallerFiles) and then rename
the function and all its callers accordingly to avoid mismatches.
- Around line 301-302: Replace the hardcoded $logPath with a path derived from
the existing $LogPath variable (respecting the three-part structure introduced
in input handling): if $LogPath is set use it, otherwise fall back to
"$Env:Windir\logs"; then append the InstallerCleanup/Logs subfolders (so
$logPath is Join-Path $LogPath "InstallerCleanup" "Logs" or the fallback) and
ensure the directory is created before computing $logFile (which should keep the
existing timestamped filename logic). Use the variables $logPath, $LogPath, and
$logFile to locate and implement this change.
Comment on lines
+33
to
+48
| param( | ||
| [switch]$WhatIf, | ||
| [switch]$SkipDISM, | ||
| [switch]$Force, | ||
| [ValidateRange(1, 365)] | ||
| [int]$QuarantineDays = 30 | ||
| ) | ||
|
|
||
| # ============================================================================ | ||
| # CONFIGURATION — Adjust thresholds here | ||
| # ============================================================================ | ||
| $WarningThresholdGB = 20 # Total Installer folder size >= this = "Warning" | ||
| $CriticalThresholdGB = 50 # Total Installer folder size >= this = "Critical" | ||
| $AutoForceThresholdPct = 10 # Free disk % below which Force mode auto-enables | ||
| $PatchCacheThresholdGB = 1 # $PatchCache$ size above which cleanup is triggered | ||
| $DismTimeoutMinutes = 60 # Max minutes to wait for DISM before continuing |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Missing required RMM three-part structure: variable declaration block, $RMM detection, interactive path, and $Description.
The script omits the three required structural sections mandated by the repo template:
- No top-of-file RMM variable block — the
## PLEASE COMMENT YOUR VARIABLES DIRECTLY BELOW HERE IF YOU'RE RUNNING FROM A RMMcomment block is absent. - No
$RMMdetection — there is noif ($RMM -eq 1)branch to distinguish interactive from RMM execution. - No interactive input path — no
Read-Host/validation loop for technician-driven runs. - No
$Descriptioncapture — always required per the template.
♻️ Suggested structural skeleton
+## PLEASE COMMENT YOUR VARIABLES DIRECTLY BELOW HERE IF YOU'RE RUNNING FROM A RMM
+## $SkipDISM — set to 1 to skip DISM phase
+## $Force — set to 1 to delete directly instead of quarantining
+## $QuarantineDays — days to retain quarantined files (default: 30)
+
param(
[switch]$WhatIf,
[switch]$SkipDISM,
[switch]$Force,
[ValidateRange(1, 365)]
[int]$QuarantineDays = 30
)
+
+# ============================================================================
+# INPUT HANDLING
+# ============================================================================
+if ($RMM -eq 1) {
+ # RMM: variables pre-set above
+ $Description = "RMM-triggered installer patch cleanup"
+ $LogPath = if ($RMMScriptPath) { Join-Path $RMMScriptPath "logs" } else { "$env:WINDIR\logs" }
+} else {
+ # Interactive: prompt technician
+ $Description = Read-Host "Enter a description for this cleanup run"
+ $LogPath = "$env:WINDIR\logs"
+ # Optionally prompt for $Force / $SkipDISM here
+}Based on learnings: "All PowerShell scripts must follow the three-part structure from script-template-powershell.ps1 (RMM variable declaration, input handling, script logic)" and "Input handling must detect $RMM (1 for RMM, undefined for interactive); interactive uses Read-Host with validation loop; RMM uses pre-set variables; always capture $Description; set $LogPath to %WINDIR%\logs in interactive, and $RMMScriptPath\logs (fallback to %WINDIR%\logs) in RMM."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rmm-ninja/Cleanup-InstallerPatches` around lines 33 - 48, The script is
missing the required RMM three-part scaffold; add a top-of-file RMM variable
declaration comment block (the "## PLEASE COMMENT YOUR VARIABLES..." section)
declaring defaults for $RMM, $RMMScriptPath, $LogPath, $Description and exposing
the existing params ($WhatIf, $SkipDISM, $Force, $QuarantineDays) for RMM
override, then implement an if ($RMM -eq 1) branch that reads inputs from those
RMM variables (setting $LogPath = Join-Path $RMMScriptPath 'logs' with a
fallback to $env:windir\logs) and an else interactive path that prompts with
Read-Host/validation loops to populate $QuarantineDays, $Force, $SkipDISM and
capture $Description, ensuring the same variable names used by the rest of the
script (e.g., $WarningThresholdGB, $PatchCacheThresholdGB, $DismTimeoutMinutes)
remain untouched; finally ensure $Description is always set and present before
proceeding.
Refactor registry error handling and improve comments for clarity.
Contributor
|
@
I do not understand why this script exists or was written still. Please elaborate. What is the demand for this exactly? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds Cleanup-InstallerPatches.ps1 — an on-demand NinjaRMM remediation script that removes orphaned .msi/.msp files from C:\Windows\Installer identified by the same registry-based orphan detection used in the monitor script. Designed to be triggered manually by a technician or automatically via NinjaRMM condition when the monitor reports Warning or Critical status.
Cleanup operates in five phases:
Phase 0 — Pre-flight: Checks C: drive free space. If disk is below 10% free, automatically switches from quarantine mode to direct deletion since quarantining would consume additional disk space on an already-full drive. Phase 1 — DISM Component Cleanup: Runs DISM /Online /Cleanup-Image /StartComponentCleanup to reclaim space from superseded Windows components. Deliberately avoids /ResetBase which would prevent future update uninstalls — too destructive for unattended automation. Skippable with -SkipDISM. Phase 2 — Orphaned Installer Cleanup: Runs the shared orphan detection function against the Windows Installer registry database. In default mode, orphaned files are moved to C:\DTC\InstallerCleanup\Quarantine<date>\ for safe recovery. With -Force, files are deleted directly. If orphan detection fails for any reason, the entire cleanup aborts with exit code 1 — the script will never delete files it cannot confirm are orphaned. Phase 3 —$PatchCache$ Cleanup: Clears the $PatchCache$ subfolder contents when larger than 1 GB. This folder contains redundant copies of patch data that Windows can re-download if needed. Phase 4 — Quarantine Maintenance: Purges quarantine folders older than -QuarantineDays (default 30 days), reclaiming the space once the safety retention window has passed. Operating modes:
Default (quarantine): Moves orphaned files to C:\DTC\InstallerCleanup\Quarantine\ — fully reversible by moving files back -Force: Deletes orphaned files directly — use when disk is critically full or after confirming quarantine contents are safe -WhatIf: Dry run that reports exactly what would be cleaned with zero filesystem changes — recommended as a first pass on any new machine Auto-Force: Engages automatically when C: drive free space drops below 10%, logged prominently as a warning Post-cleanup actions:
Re-runs orphan detection to update NinjaRMM custom fields with current state Writes cleanup summary to Windows Application Event Log (Event ID 1001, source DTC-InstallerMonitor) Detailed per-file log written to C:\DTC\InstallerCleanup\Logs\ with timestamps, action taken, file sizes, and success/failure status Safety guarantees:
Orphan detection failure aborts the entire cleanup — no blind deletion Locked files are caught and skipped gracefully (logged, continues to next file) All thresholds defined as variables at script top for easy adjustment Self-contained with no external dependencies (PowerShell 5.1 only) Designed to run as SYSTEM on Windows 10, 11, Server 2019, and Server 2022
Summary by CodeRabbit
New Features
Bug Fixes / Reliability