fix: Retry the analyzer in the Analyze task - #148
Merged
Conversation
PSScriptAnalyzer executes its script rules in parallel against a process-wide, unsynchronised singleton, so concurrent rules can observe half-updated PowerShell session state and crash. The failure has no relationship to the code being analyzed, which is why re-running the job always succeeded. Retry the analyzer call up to three times, rethrowing on the last attempt. The retry is unconditional: the observed crash signatures are symptoms of an upstream race rather than a fixed contract, and matching on them would let a new variant bypass the mitigation. Pass -ErrorAction Stop explicitly rather than relying on build.ps1 setting ErrorActionPreference process-wide, so the try/catch does not depend on an ambient caller preference. Validated over 45 runs of the Analyze task against a reproduction of the race: zero build failures, with the crash occurring and being absorbed. Refs #136 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y2tE5nDWYPfmyWP2yPeWXT
There was a problem hiding this comment.
Pull request overview
Adds resilience to the repository build’s Analyze psake task by retrying Invoke-ScriptAnalyzer to mitigate intermittent upstream PSScriptAnalyzer crashes, while also making failure semantics explicit via -ErrorAction Stop.
Changes:
- Wrap
Invoke-ScriptAnalyzerin a bounded (3-attempt) retry loop with a short delay between attempts. - Pass
-ErrorAction Stopvia a splatted parameter hashtable to avoid relying on ambient$ErrorActionPreference. - Add inline documentation referencing the tracked flake (issue #136).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The retry warning named only the attempt number, so a crash absorbed before the final attempt left no record of what actually failed. That undercuts the reason the retry is unconditional: a new crash variant would be retried away silently instead of being visible in the log. Include the error id and message in the warning. The id is the useful discriminator - RULE_ERROR marks the upstream rule crash, while anything else, such as PathNotFound, indicates a genuine analyzer failure that happens to be getting retried. Also copy $_ into a variable at the top of the catch block, per the error handling convention in instructions/powershell.instructions.md. Raised in review on #148. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y2tE5nDWYPfmyWP2yPeWXT
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
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
Invoke-ScriptAnalyzercall in theAnalyzetask up to three times, rethrowing on the last attempt, so the intermittent PSScriptAnalyzer crash tracked in CI: Intermittent PSScriptAnalyzer crash in the Analyze task #136 no longer fails the build-ErrorAction Stopexplicitly rather than relying onbuild.ps1setting$ErrorActionPreferenceprocess-wide, so thetry/catchdoes not depend on an ambient caller preferenceThe crash is a thread-safety defect inside PSScriptAnalyzer, which runs its script rules in parallel against a process-wide unsynchronised singleton. It has no relationship to the code being analyzed, which is why re-running the job always succeeded. Full root cause analysis, reproduction, and upstream links are in this comment on #136.
The retry is deliberately unconditional rather than matched against the known crash signatures. Those signatures are symptoms of an upstream race rather than a fixed contract — four distinct exception types appeared during the analysis — so matching on them would let a new variant bypass the mitigation. The cost is that a genuine analyzer failure takes three attempts before failing, which is a few seconds on a path already headed for red.
Test Plan
./build.ps1 -Task Testpasses (422 passed, 0 failed, 15 skipped)Analyzetask, zero build failures, with the crash occurring and being absorbedubuntu-latest/windows-latest/macOS-latestmatrixBreaking Changes
None. This changes only this repository's own build, not the shipped module.
Notes
Test-PSBuildScriptAnalysisin the shipped module is exposed to the same race but behaves differently, since it callsInvoke-ScriptAnalyzerwithout-ErrorAction Stopand so does not fail. Tracked separately in Test-PSBuildScriptAnalysis does not fail on the PSScriptAnalyzer rule crash from #136 #147 and deliberately out of scope here.CHANGELOG.mdentry, per the changelog scope rule ininstructions/repository-specific.instructions.md: the changelog covers user-facing changes to the shipped module, and this touches only this repository's own build script.🤖 Generated with Claude Code
https://claude.ai/code/session_01Y2tE5nDWYPfmyWP2yPeWXT
Generated by Claude Code