Fix: address all code review findings

[SamErde]

Summary

This PR addresses all findings from the repository code review. Changes span bug fixes, safety improvements, test coverage, and CI improvements.

---

Bug Fixes

Clear-OldExchangeLog

• Fixed wrong callee — was calling nonexistent Clear-OldIisLogFiles; corrected to Clear-OldIISLog
• Fixed date filter — removed unreliable string coercion; now compares $_.LastWriteTime -le $LastWriteDate directly
• Replaced .Delete() with Remove-Item -LiteralPath so WhatIf/Confirm are honoured
• Added -WhatIf:$WhatIfPreference passthrough to Clear-OldIISLog call
• Added [ValidateRange(1,[int16]::MaxValue)] on -Days
• Added -File to Get-ChildItem to exclude directories

Clear-CurrentUserTemp

• Fixed Out-Null bug: $EmptyDirectories = … | Out-Null silently assigned $null; wrapped pipeline in @(…)
• Added ShouldProcess guard around directory removal
• Added WhatIf infinite-loop guard: do/until loop now breaks if WhatIf skipped all removals

Get-StaleUserProfile

• Fixed missing $ prefix: !(StaleUserProfiles) → -not $StaleUserProfiles

---

Safety / ShouldProcess

Remove-OldFiles (private helper)

• Added [Parameter(Mandatory)], [ValidateNotNullOrEmpty()], and [ValidateScript({Test-Path …})] on -Path
• Added [ValidateRange(1,[int16]::MaxValue)] on -Days
• Changed file age filter from CreationTime → LastWriteTime (creation time is unreliable on Windows when files are copied)
• Wrapped Remove-Item in per-file $PSCmdlet.ShouldProcess loop using -LiteralPath
• Fixed SuppressMessageAttribute — was missing required second (empty string) argument

Clear-OldIISLog

• Added SupportsShouldProcess, ConfirmImpact = 'Medium' to CmdletBinding
• Removed hardcoded -WhatIf on every Remove-OldFiles call (was always forcing WhatIf mode)
• Wrapped each Remove-OldFiles call in $PSCmdlet.ShouldProcess

---

Help / Metadata

Start-Cleaning

• Corrected -Dedication help string (was documenting the removed -NoLogo parameter)

docs/Clear-OldIISLog.md

• Added -WhatIf and -Confirm parameter documentation

TheCleaners.psd1

• Removed WebAdministration from ExternalModuleDependencies — the code already guards it as optional at runtime; declaring it as a hard dependency caused Import-Module failures on systems without IIS

---

Tests

• Promoted from SkipUnit/ to Unit/: ExportedFunctions.Tests.ps1 and TheCleaners-Module.Tests.ps1 now run as part of the normal build
• ExportedFunctions.Tests.ps1: Rewrote to Pester v5 -ForEach pattern; removed AfterAll { Remove-Module } block that unloaded the module before platyPS help generation ran
• New CleanupBehavior.Tests.ps1: Regression tests for the fixed logic — age filter, WhatIf behaviour, empty-dir removal, Exchange log cleanup, IIS log delegation

---

CI

• Switched runner to windows-latest (module uses Windows-only CIM, registry, and IIS APIs)
• Added permissions: contents: read (least-privilege)
• Removed verbose module/environment debug step
• Removed stefanzweifel/git-auto-commit-action auto-commit step (generated artifacts should not be committed back to PR branches)

---

Validation

Full build passes: 18 tasks, 0 errors, 0 warnings, 16 unit tests passing

[codacy-production]

Up to standards ✅

🟢 Issues 2 minor

Results:
2 new issues

Category	Results
CodeStyle	2 minor

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

[Copilot]

Pull request overview

This PR addresses prior repository code review findings by improving safety/ShouldProcess behavior in cleanup cmdlets, correcting logic bugs, promoting/adding unit tests, and updating CI to run on Windows.

Changes:

• Fix and harden cleanup cmdlets (Clear-OldExchangeLog, Clear-CurrentUserTemp, Clear-OldIISLog, Remove-OldFiles) with corrected filtering and ShouldProcess support.
• Promote/add Pester v5 unit tests and update build code coverage configuration.
• Update documentation and CI workflow (Windows runner, least-privilege permissions, remove auto-commit behavior).

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/TheCleaners/TheCleaners.psd1	Removes hard dependency on WebAdministration by clearing ExternalModuleDependencies.
src/TheCleaners/Public/Start-Cleaning.ps1	Updates comment-based help parameter documentation.
src/TheCleaners/Public/Get-StaleUserProfile.ps1	Fixes stale profile empty-check logic.
src/TheCleaners/Public/Clear-OldIISLog.ps1	Adds SupportsShouldProcess and wraps cleanup calls in ShouldProcess.
src/TheCleaners/Public/Clear-OldExchangeLog.ps1	Fixes IIS delegation call, date filtering, and removal method; adds validation.
src/TheCleaners/Public/Clear-CurrentUserTemp.ps1	Fixes empty-directory capture and adds ShouldProcess guard + WhatIf loop exit.
src/TheCleaners/Private/Remove-OldFiles.ps1	Adds parameter validation, uses LastWriteTime, and implements per-file ShouldProcess.
src/TheCleaners.build.ps1	Updates Pester code coverage paths to use module source path joins.
src/Tests/Unit/TheCleaners-Module.Tests.ps1	Refactors module manifest tests for Pester v5 structure.
src/Tests/Unit/ExportedFunctions.Tests.ps1	Promotes exported-functions/help coverage tests into Unit suite.
src/Tests/Unit/CleanupBehavior.Tests.ps1	Adds regression coverage for cleanup behaviors and WhatIf handling.
src/Tests/SkipUnit/ExportedFunctions.Tests.ps1	Removes the skipped version of exported-functions tests (now promoted).
docs/Start-Cleaning.md	Updates generated help text for -Dedication.
docs/Clear-OldIISLog.md	Documents -WhatIf/-Confirm and refreshes help formatting.
.github/workflows/Build Module.yml	Switches CI to windows-latest, adds read-only permissions, removes debug/auto-commit steps.

[SamErde]

@copilot apply changes based on the comments in this thread

[Copilot]

@copilot apply changes based on the comments in this thread

All four review findings have been addressed in commit a08eb46:

1. Clear-OldExchangeLog.ps1:32 — Changed [int] to [int16] so the parameter type matches the ValidateRange(1, [int16]::MaxValue) constraint.
2. Clear-CurrentUserTemp.ps1:91-95 — Wrapped Remove-Item in a try/catch inside the empty-directory loop; failures now emit a Write-Warning and allow cleanup to continue for remaining directories.
3. docs/Clear-OldIISLog.md:27 — Fixed the example command from Clear-OldIISLogFile to Clear-OldIISLog.
4. docs/Clear-OldIISLog.md:34-46 — Corrected the stated default from "30 days" to "60 days" to match the actual parameter default.

[Copilot]

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

src/TheCleaners/Public/Clear-OldIISLog.ps1:29

• The comment-based help is now out of sync with the function: it still says the default is 30 days and the example uses Clear-OldIISLogFile, but the parameter default is $Days = 60 and the function name is Clear-OldIISLog. Please update the help block so the documented default and example match the implementation.

    [CmdletBinding(SupportsShouldProcess, ConfirmImpact = 'Medium')]
    [Alias('Clean-IISLog')]
    #[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseSingularNouns')]
    param (
        [Parameter()]

[SamErde]

@copilot apply changes based on the comments in this thread

[Copilot]

@copilot apply changes based on the comments in this thread

All three findings addressed in commit 3262877:

1. Clear-OldExchangeLog.ps1:46-51 — Removed leading \ from DiagnosticLogsPath and MessageTrackingLogsPath Join-Path -ChildPath values so all paths stay under $ExchangeInstallPath rather than resolving from the drive root.
2. Remove-OldFiles.ps1:9-11 — Fixed the help example: removed the non-existent -Recurse parameter and corrected the typo "60 does" → "60 days".
3. ExportedFunctions.Tests.ps1:11 — Added -CommandType Function to Get-Command so the runtime query is scoped to functions only, matching $manifestExported which comes from ExportedFunctions.