feat(powershell): PowerShell static-analysis module - #1
Conversation
First standards module (Phase 1). Ports and decouples medley's PowerShell linting into modules/powershell/: - PSScriptAnalyzerSettings.psd1 — OTBS ruleset, PS 7.4/7.6, repo-agnostic - Invoke-Pssa.ps1 — runner with per-file pwsh subprocess isolation (works around the analyzer's Linux cache race); self-skips when the analyzer is absent so local hooks don't hard-fail (CI is authoritative) - good/bad fixtures + pssa.test.sh on the shell harness - powershell.yml CI: ensure PSScriptAnalyzer 1.25.0, self-lint, fixtures test Also ignore .claude/worktrees/ (PR worktree workflow). Verified locally: runner self-lints clean, good fixture clean, bad fixture flags 9 findings; harness 2/2; shellcheck -x + actionlint clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b84b0d1148
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # Single-quoted here-string: variables expand in the child from its | ||
| # inherited environment, not here. | ||
| $output = pwsh -NoProfile -NonInteractive -Command @' | ||
| Import-Module PSScriptAnalyzer -RequiredVersion $env:PSSA_VERSION -ErrorAction Stop |
There was a problem hiding this comment.
When a developer has only a newer PSScriptAnalyzer installed (for example 1.26.0), the parent gate accepts it because it checks >= $AnalyzerVersion, but this child import still asks for exactly 1.25.0; the Import-Module docs define -RequiredVersion as exact, while -MinimumVersion is the minimum acceptable version. That makes the runner fail every subprocess instead of supporting the README/parameter contract of PSScriptAnalyzer ≥ 1.25.0.
Useful? React with 👍 / 👎.
| # https://github.com/PoshCode/PowerShellPracticeAndStyle | ||
|
|
||
| @{ | ||
| Severity = @('Error', 'Warning') |
There was a problem hiding this comment.
With this top-level severity filter, enabled Information rules are discarded before the runner sees them: the settings docs describe Severity=@('Error','Warning') as excluding other severities, and the rule table marks PSAvoidUsingDoubleQuotesForConstantString, PSUseCorrectCasing, and PSProvideCommentHelp as Information. A file that only violates those configured standards will be reported clean, so either include Information or don't enable those rules.
Useful? React with 👍 / 👎.
The self-lint passed multiple files as `-Path f1 f2`; PowerShell bound only f1 to -Path and f2 positionally to -Settings, so it treated a .ps1 as the settings file (CI failure). Make -Path the sole positional parameter and add -ExcludePath so the runner self-enumerates (single `-Path . -ExcludePath ...`) instead of fragile multi-file argument passing. The .git directory is always skipped. Verified: runner + good fixture clean, bad fixture flags 9, and the CI invocation is clean across 2 files. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a8db44a457
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| if (-not (Test-Path -LiteralPath $Settings)) { | ||
| Write-Error "Settings file not found: $Settings" |
There was a problem hiding this comment.
Because this script sets $ErrorActionPreference = 'Stop', Write-Error is terminating here, so the following exit 2 is never reached; a missing settings file (and the same pattern for missing paths or crashed child processes) exits as PowerShell's default error code 1, which is indistinguishable from lint findings despite the documented exit-code contract. Use a non-terminating write to stderr or exit before throwing so callers can reliably detect configuration failures.
Useful? React with 👍 / 👎.
| if (Test-Path -LiteralPath $entry -PathType Leaf) { | ||
| $resolved = (Resolve-Path -LiteralPath $entry).Path | ||
| if (-not (Test-PathExcluded -FullPath $resolved -Patterns $ExcludePath)) { | ||
| $files.Add($resolved) |
There was a problem hiding this comment.
When a hook passes a mixed list of staged files as the README suggests, a non-PowerShell leaf like README.md passes this branch and is added to $files; only directory traversal applies the .ps1/.psm1 filter. The Invoke-ScriptAnalyzer -Path docs only say non-script files are ignored when the input is a directory, so handing arbitrary leaf files to the child can produce false hook failures on non-PowerShell changes. Filter leaf inputs by extension before adding them.
Useful? React with 👍 / 👎.
- Severity: include Information so the enabled Information-severity rules (PSUseCorrectCasing, PSProvideCommentHelp, PSAvoidUsingDoubleQuotesForConstantString) actually gate - child import: -MinimumVersion (not -RequiredVersion) to match the parent >= gate, so a newer analyzer still imports - config errors: non-terminating Write-Error so the documented exit-2 contract holds under $ErrorActionPreference='Stop' - leaf inputs: filter by .ps1/.psm1 so a hook's mixed file list does not analyze non-PowerShell files - rename Invoke-Pssa.ps1 -> Invoke-PSScriptAnalyzer.ps1 (ecosystem keeps "PS" uppercase per .NET capitalization + PoshCode); PascalCase fixtures (Clean.ps1 / Violations.ps1) for consistency Verified: self/good clean; bad flags 10 (incl. the double-quote rule); nonexistent-settings exits 2; a .md leaf is skipped; harness + shellcheck -x + actionlint clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 277e668219
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # (PSUseCorrectCasing, PSProvideCommentHelp, | ||
| # PSAvoidUsingDoubleQuotesForConstantString) actually gate — a top-level | ||
| # Severity filter discards any severity not listed here. | ||
| Severity = @('Error', 'Warning', 'Information') |
There was a problem hiding this comment.
With this explicit Severity allow-list, syntax errors are filtered out because PSScriptAnalyzer emits parser failures as diagnostics with severity ParseError; the Microsoft docs state, “To suppress ParseErrors, don't include it as a value in the Severity parameter.” In a repo that relies on this runner as its CI gate, a malformed .ps1/.psm1 can therefore produce no findings and exit 0, so add ParseError here or remove the filter.
Useful? React with 👍 / 👎.
1 participant
What
The first
standardsmodule (Phase 1): opinionated PowerShell static analysis, ported and decoupled from the medley sandbox.modules/powershell/PSScriptAnalyzerSettings.psd1— OTBS ruleset, targets PS 7.4/7.6, cross-platform, rules justified inlinemodules/powershell/Invoke-Pssa.ps1— runner with per-filepwshsubprocess isolation (avoids the PSScriptAnalyzer cache/runspace race seen on Linux); self-skips when the analyzer is absentfixtures/powershell/{good,bad}+modules/powershell/pssa.test.sh(shell harness).github/workflows/powershell.yml— ensures PSScriptAnalyzer 1.25.0, self-lints the repo's PowerShell (excluding the intentionally-bad fixtures), runs the fixtures test.gitignore: ignore.claude/worktrees/Verification (local — pwsh 7.6.3 + PSScriptAnalyzer 1.25.0)
harness/shell/run-tests.sh: 2 passedshellcheck -xandactionlintcleanMarkdown is the next module slice.
🤖 Generated with Claude Code