Add VixTemp folder cleanup script for dental imaging workstations#40
Add VixTemp folder cleanup script for dental imaging workstations#40AlrightLad wants to merge 2 commits into
Conversation
Automated cleanup script for C:\vixtemp folder used by Vixwin/Dentimax/Curve dental imaging sensors. Prevents sensor failures when folder exceeds 250MB. ## Features - Archives oldest files when folder exceeds 250MB threshold - Targets 200MB post-cleanup (50MB buffer) - Maintains 5 rotating archive versions for image recovery - Server detection - only runs on workstations - Concurrent execution prevention via lock file - Ninja RMM alerting via Windows Event Log (Event IDs 1000-1002) - Dual logging (transcript + daily logs) for troubleshooting - Disk space pre-check with 150% safety buffer - Gracefully skips locked files ## Deployment - Designed for Ninja RMM scheduled task (every 4 hours) - Runs as SYSTEM - Self-determining: safe to deploy broadly (exits silently on servers or machines without C:\vixtemp) ## Paths - Source: C:\vixtemp - Archives: C:\vixtemp_archives - Logs: C:\ProgramData\VixTempCleanup\Logs ## Testing - Tested on Windows 10/11 workstations - Validated with 32-bit PowerShell (uses Compress-Archive for compatibility)
|
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. 📝 WalkthroughWalkthroughA new PowerShell script that monitors C:\vixtemp and automatically archives oldest files when folder size exceeds a configured threshold. Features include lock-based concurrency control, configurable retention policies, logging with optional event log alerts, and workstation-only operation. Changes
Sequence DiagramsequenceDiagram
participant User
participant Script
participant Monitor as Folder Monitor
participant Lock as Lock Manager
participant Disk as Disk Space Check
participant Archive as Archive Creator
participant Cleanup as File Cleanup
participant Log as Logger & Alerts
User->>Script: Execute script
Script->>Script: Initialize directories
Script->>Monitor: Check if server OS
alt Server detected
Script->>Log: Exit early
else Workstation
Script->>Lock: Acquire lock file
alt Lock obtained
Script->>Monitor: Check folder size
alt Size exceeds threshold
Script->>Monitor: Identify oldest files
Script->>Disk: Verify sufficient disk space
alt Space available
Script->>Archive: Create ZIP archive
Script->>Cleanup: Delete archived files
Script->>Cleanup: Prune empty directories
Script->>Cleanup: Rotate old archives
Script->>Log: Log success
else Insufficient space
Script->>Log: Log failure & alert
end
else Size under threshold
Script->>Log: Log as skipped
end
Script->>Lock: Release lock file
else Lock conflict
Script->>Log: Exit with lock error
end
end
Script->>Log: Close transcript
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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@rmm-misc/VixWinTempFolderCleanup`:
- Line 11: The .NOTES version string and the $ScriptVersion variable are out of
sync (one is 1.1.0, the other 1.1.2); update either the .NOTES heading or the
$ScriptVersion constant so they match (preferably set the .NOTES to the value in
$ScriptVersion or vice versa) and ensure both the human-readable .NOTES block
and the $ScriptVersion symbol are synchronized wherever they appear (including
any duplicate occurrences).
- Line 586: The log line uses $spaceCheck.FreeMB but incorrectly labels it "GB
free"; update the Write-Log call so units match: either log
"$($spaceCheck.FreeMB) MB free" or compute GB by dividing FreeMB by 1024 (e.g.
[$spaceCheck.FreeMB / 1024]) and log that value with "GB free"; ensure you also
adjust the RequiredMB label consistently (variables: Write-Log,
$spaceCheck.FreeMB, $spaceCheck.RequiredMB).
- Around line 208-210: The Get-UnixTimestamp function uses culture-sensitive
parsing ([double]::Parse on Get-Date -UFormat %s) which can break on non-US
locales; replace its body to use .NET's culture-independent Unix timestamp APIs
(e.g., call [DateTimeOffset]::UtcNow.ToUnixTimeSeconds() or
ToUnixTimeMilliseconds()/1000 if subsecond precision is needed) and return that
integer, ensuring you update the Get-UnixTimestamp function to directly return
the result from DateTimeOffset instead of parsing a formatted string.
🧹 Nitpick comments (1)
rmm-misc/VixWinTempFolderCleanup (1)
264-275: Potential file handle leak ifClose()throws.The current pattern
OpenRead().Close()doesn't guarantee disposal ifClose()fails. Consider using atry/finallyblock withDispose().♻️ Proposed fix for safer resource handling
try { # Test if file is accessible - $null = [System.IO.File]::OpenRead($file.FullName).Close() + $stream = [System.IO.File]::OpenRead($file.FullName) + $stream.Dispose() $filePaths += $file.FullName $archivedFiles += $file $originalSizeBytes += $file.Length }
Fixes issues identified in code review: - Fix version mismatch: .NOTES and $ScriptVersion now both 1.1.3 - Fix locale-sensitive timestamp parsing: replaced [double]::Parse(Get-Date -UFormat %s) with [DateTimeOffset]::UtcNow.ToUnixTimeSeconds() to avoid failures in non-US locales that use comma decimal separators - Fix unit label in disk space log: changed "GB free" to "MB free" to match variable name - Fix potential resource leak: file accessibility check now uses proper try/finally with explicit Dispose() to prevent handle leaks if closure fails Bumps version to 1.1.3
Gumbees
left a comment
There was a problem hiding this comment.
Summary
Adds a 653-line VixTemp folder cleanup script for dental imaging workstations (Vixwin/Dentimax/Curve). Archives oldest files when C:\vixtemp exceeds 250MB, maintains 5 rotating archives, server-detection guard, lock-file concurrency control, Ninja RMM alerting via Event Log, dual logging.
What works
- Genuinely defensive design: workstation-only guard (
Test-IsServercheckingProductType), lock-file with stale-lock detection (5min TTL), disk-space pre-check with 150% safety buffer, gracefully skips locked files. - Compress-Archive with rotation (
vixtemp_archive_<unix>_<friendly>.zip, keep last 5) gives recovery without unbounded growth. - Overdue-cleanup alerting (24h since last success) catches the silent-failure mode where the script runs but exits early every time.
- Explicit exit codes (0/1/2/3/4) are RMM-friendly.
- Atomic operations: archive first, then delete only what was successfully archived ... never deletes files it couldn't archive.
- DISABLE_CLEANUP override file is a nice operator escape hatch.
- Follows the template properly. RMM variable block at top,
$RMMdual-mode,Start-Transcript,$Description,$LogPathwith fallback. This is the template-compliance gold standard for the current PR backlog. - File correctly placed in
rmm-misc/with.ps1extension. FilenameVixWinTempFolderCleanup... actually wait, the diff showsrmm-misc/VixWinTempFolderCleanupwith no.ps1extension. See blockers.
Concerns
BLOCKER: Filename has no .ps1 extension. The diff adds rmm-misc/VixWinTempFolderCleanup ... no extension. PowerShell will not execute this as a script. Rename to rmm-misc/vixwin-temp-folder-cleanup.ps1 or similar.
Filename casing. PascalCase VixWinTempFolderCleanup should be kebab-case per PR #51.
Folder placement. This is a Vixwin/Dentimax/Curve specific cleanup. app-vixwin/ (which PR #35 creates) or app-dentrix/ would be more discoverable than rmm-misc/. Cross-coordinate with PR #35 which creates app-vixwin/VixTempCleanup.ps1 ... these two PRs are doing similar things in different folders.
Overlaps with PR #35. PR #35 adds app-vixwin/VixTempCleanup.ps1. This PR adds rmm-misc/VixWinTempFolderCleanup. Same author, same problem space, different folder, different filename. Pick one ... two scripts maintained in parallel is the worst outcome.
Suggestions
- The 150% disk-space buffer is conservative ... probably fine but document why 1.5x specifically (vs 1.2x or 2x). Magic number deserves a comment.
- Consider exposing
$ThresholdMBand$TargetMBas RMM variables for per-site tuning ... 250MB is the documented Vixwin sensor failure threshold but some sites may want lower headroom.
Verdict
REQUEST CHANGES ... .ps1 extension blocker, and the overlap with PR #35 needs reconciliation before either lands. The script body itself is among the most defensively-coded in the backlog ... once the file is renamed and the duplicate is resolved, easy approve.
Automated cleanup script for C:\vixtemp folder used by Vixwin/Dentimax/Curve dental imaging sensors. Prevents sensor failures when folder exceeds 250MB.
Features
Deployment
Paths
Testing
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.