fix: make PowerShell 5.1/7.4 CI green (array returns, menu header, test mocks)#1
Merged
Conversation
added 2 commits
July 14, 2026 10:49
…il-fast) - Guard Get-Printer calls in Get-Printers/Get-PrinterShareStatus/ Get-PrintComplianceReport so they return an empty array (not $null) when the Print Spooler is unavailable (terminating exception on CI). - Guard Clear-Host/ReadKey in Write-MenuHeader/Wait-Menu for non-interactive hosts (invalid CursorPosition handle). - Fix Test-Administrator test (was passing -is as a parameter) and scope Assert-Elevated mocks with -ModuleName so they apply inside the module. - Set fail-fast: false on the analyze matrix so both PS versions report.
PowerShell enumerates a returned array, so functions returning @(...) yield a scalar when there are 0/1 elements. Wrap with ,@(...) so Get-Printers, Get-PrinterShareStatus, Get-PrinterSharingCompatibility and Get-PrintComplianceReport always return a real array regardless of printer count.
📝 WalkthroughWalkthroughChangesRuntime resilience and output consistency
CI matrix reliability
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
Modules/Utilities/PrinterToolkit.Utilities.psm1 (1)
126-126: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPreserve diagnostics while suppressing expected host failures.
Both empty catches intentionally provide CI resilience, but they hide unexpected failures and trigger PSScriptAnalyzer warnings. Add
Write-Verbosediagnostics or replace the blocks with explicit silent error handling.
Modules/Utilities/PrinterToolkit.Utilities.psm1#L126-L126: retain theClear-Hostfallback with a diagnostic.Modules/Utilities/PrinterToolkit.Utilities.psm1#L143-L143: retain theReadKeyfallback with a diagnostic.🤖 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 `@Modules/Utilities/PrinterToolkit.Utilities.psm1` at line 126, Replace the empty catch blocks in Clear-Host at Modules/Utilities/PrinterToolkit.Utilities.psm1:126-126 and ReadKey at Modules/Utilities/PrinterToolkit.Utilities.psm1:143-143 with explicit silent error handling that emits Write-Verbose diagnostics, while preserving both existing fallbacks and CI resilience.Source: Linters/SAST tools
Modules/Core/PrinterToolkit.Core.psm1 (1)
147-151: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeclare the concrete
System.Object[]output type.These implementations intentionally preserve arrays but declare only
[array]; PSScriptAnalyzer reports the mismatch. Update the corresponding declarations to[OutputType([object[]])].
Modules/Core/PrinterToolkit.Core.psm1#L147-L151: updateGet-Printers.Modules/Reporting/PrinterToolkit.Reporting.psm1#L254-L254: updateGet-PrintComplianceReport.Modules/Sharing/PrinterToolkit.Sharing.psm1#L43-L43: updateGet-PrinterShareStatus.Modules/Sharing/PrinterToolkit.Sharing.psm1#L128-L128: updateGet-SmbSharePermissions.Modules/Sharing/PrinterToolkit.Sharing.psm1#L237-L237: updateGet-PrinterSharingCompatibility.🤖 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 `@Modules/Core/PrinterToolkit.Core.psm1` around lines 147 - 151, Replace the [array] output declarations with [OutputType([object[]])] for Get-Printers in Modules/Core/PrinterToolkit.Core.psm1 (lines 147-151), Get-PrintComplianceReport in Modules/Reporting/PrinterToolkit.Reporting.psm1 (line 254), Get-PrinterShareStatus and Get-SmbSharePermissions in Modules/Sharing/PrinterToolkit.Sharing.psm1 (lines 43 and 128), and Get-PrinterSharingCompatibility in Modules/Sharing/PrinterToolkit.Sharing.psm1 (line 237); leave their array-preserving implementations unchanged.Source: Linters/SAST tools
🤖 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 `@Modules/Core/PrinterToolkit.Core.psm1`:
- Around line 147-151: Update the Get-Printer calls inside the try/catch
fallbacks to use -ErrorAction Stop so failures reach the empty-array catch path.
Apply this change at Modules/Core/PrinterToolkit.Core.psm1 lines 147-151,
Modules/Reporting/PrinterToolkit.Reporting.psm1 lines 219-223, and
Modules/Sharing/PrinterToolkit.Sharing.psm1 lines 19-23; preserve the existing
return behavior.
---
Nitpick comments:
In `@Modules/Core/PrinterToolkit.Core.psm1`:
- Around line 147-151: Replace the [array] output declarations with
[OutputType([object[]])] for Get-Printers in
Modules/Core/PrinterToolkit.Core.psm1 (lines 147-151), Get-PrintComplianceReport
in Modules/Reporting/PrinterToolkit.Reporting.psm1 (line 254),
Get-PrinterShareStatus and Get-SmbSharePermissions in
Modules/Sharing/PrinterToolkit.Sharing.psm1 (lines 43 and 128), and
Get-PrinterSharingCompatibility in Modules/Sharing/PrinterToolkit.Sharing.psm1
(line 237); leave their array-preserving implementations unchanged.
In `@Modules/Utilities/PrinterToolkit.Utilities.psm1`:
- Line 126: Replace the empty catch blocks in Clear-Host at
Modules/Utilities/PrinterToolkit.Utilities.psm1:126-126 and ReadKey at
Modules/Utilities/PrinterToolkit.Utilities.psm1:143-143 with explicit silent
error handling that emits Write-Verbose diagnostics, while preserving both
existing fallbacks and CI resilience.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 69a5344e-fe17-4ea4-8325-94b46129cf21
📒 Files selected for processing (6)
.github/workflows/ci.ymlModules/Core/PrinterToolkit.Core.psm1Modules/Reporting/PrinterToolkit.Reporting.psm1Modules/Sharing/PrinterToolkit.Sharing.psm1Modules/Utilities/PrinterToolkit.Utilities.psm1Tests/PrinterToolkit.Tests.ps1
Comment on lines
+147
to
+151
| try { | ||
| return ,@(Get-Printer -ErrorAction SilentlyContinue) | ||
| } catch { | ||
| return ,@() | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the referenced files and inspect the surrounding lines.
for f in Modules/Core/PrinterToolkit.Core.psm1 Modules/Reporting/PrinterToolkit.Reporting.psm1 Modules/Sharing/PrinterToolkit.Sharing.psm1 Modules/Utilities/PrinterToolkit.Utilities.psm1; do
echo "===== $f ====="
if [ -f "$f" ]; then
wc -l "$f"
sed -n '1,260p' "$f" | nl -ba | sed -n '130,170p;210,235p;1,170p'
else
echo "missing"
fi
echo
done
# Search for Get-Printer usage and ErrorAction handling.
echo "===== Get-Printer usages ====="
rg -n --hidden --glob '!**/.git/**' 'Get-Printer|ErrorAction Stop|ErrorAction SilentlyContinue|catch \{' ModulesRepository: 00AstroGit00/windows-printer-toolkit
Length of output: 306
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show exact snippets around the cited lines.
sed -n '140,155p' Modules/Core/PrinterToolkit.Core.psm1 | nl -ba
echo "-----"
sed -n '214,228p' Modules/Reporting/PrinterToolkit.Reporting.psm1 | nl -ba
echo "-----"
sed -n '14,28p' Modules/Sharing/PrinterToolkit.Sharing.psm1 | nl -ba
echo "-----"
sed -n '120,150p' Modules/Utilities/PrinterToolkit.Utilities.psm1 | nl -baRepository: 00AstroGit00/windows-printer-toolkit
Length of output: 214
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect any docs/tests mentioning the behavior of Get-Printer errors or array output types.
rg -n --hidden --glob '!**/.git/**' 'Get-Printer|OutputType\(\[array\]\)|SilentlyContinue|ErrorAction Stop|try/catch' README* Modules tests .Repository: 00AstroGit00/windows-printer-toolkit
Length of output: 50392
Use -ErrorAction Stop for the printer fallbacks
These try/catch blocks won’t catch non-terminating Get-Printer errors, so the empty-array fallback never runs. Switch the three calls to -ErrorAction Stop:
Modules/Core/PrinterToolkit.Core.psm1#L147-L151Modules/Reporting/PrinterToolkit.Reporting.psm1#L219-L223Modules/Sharing/PrinterToolkit.Sharing.psm1#L19-L23
🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)
[info] 148-148: The cmdlet 'Get-Printers' returns an object of type 'System.Object[]' but this type is not declared in the OutputType attribute.
(PSUseOutputTypeCorrectly)
[info] 150-150: The cmdlet 'Get-Printers' returns an object of type 'System.Object[]' but this type is not declared in the OutputType attribute.
(PSUseOutputTypeCorrectly)
📍 Affects 3 files
Modules/Core/PrinterToolkit.Core.psm1#L147-L151(this comment)Modules/Reporting/PrinterToolkit.Reporting.psm1#L219-L223Modules/Sharing/PrinterToolkit.Sharing.psm1#L19-L23
🤖 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 `@Modules/Core/PrinterToolkit.Core.psm1` around lines 147 - 151, Update the
Get-Printer calls inside the try/catch fallbacks to use -ErrorAction Stop so
failures reach the empty-array catch path. Apply this change at
Modules/Core/PrinterToolkit.Core.psm1 lines 147-151,
Modules/Reporting/PrinterToolkit.Reporting.psm1 lines 219-223, and
Modules/Sharing/PrinterToolkit.Sharing.psm1 lines 19-23; preserve the existing
return behavior.
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
1 participant
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.
Summary
Fixes all 7 failing CI tests and makes the GitHub Actions workflow pass on both PowerShell 5.1 and 7.4.
Root cause
Functions returned
return @(...), but PowerShell enumerates a returned array through the pipeline — so whenGet-Printeryields 0/1 items the result collapsed to$null/a scalar and$result -is [array]was$false. Fixed with thereturn ,@(...)comma-array idiom.Changes
Modules/Core/PrinterToolkit.Core.psm1—Get-PrintersModules/Sharing/PrinterToolkit.Sharing.psm1—Get-PrinterShareStatus,Get-PrinterSharingCompatibility,Get-SmbSharePermissionsModules/Reporting/PrinterToolkit.Reporting.psm1—Get-PrintComplianceReportModules/Utilities/PrinterToolkit.Utilities.psm1— guardClear-Host/ReadKeyinWrite-MenuHeader/Wait-Menuso they don't throw "CursorPosition: handle is invalid" in non-interactive CITests/PrinterToolkit.Tests.ps1— fixTest-Administratortest (was passing-isas a parameter) and scopeAssert-Elevatedmocks with-ModuleName.github/workflows/ci.yml—fail-fast: falseon the analyze matrixTest plan
gh workflow run ci.yml --ref fix/ci-ps51-tests→ success (Analyze 5.1 ✅, Analyze 7.4 ✅, Build ✅)🤖 Generated with OpenCode
Summary by CodeRabbit
Bug Fixes
Tests
Chores