-
Notifications
You must be signed in to change notification settings - Fork 47
refactor(scripts): standardize PowerShell entry point guard pattern #477
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor(scripts): standardize PowerShell entry point guard pattern #477
Conversation
…ing.ps1 - add [CmdletBinding()] to 8 functions missing it - extract ~70 lines of inline main logic into Invoke-DependencyPinningAnalysis - restructure guard to canonical if-wraps-try form with Write-Error + Write-CIAnnotation - remove else clause that rejected dot-sourcing - add #region/#endregion Functions and Main Execution markers 🔧 - Generated by Copilot
- normalize 5 Pattern A scripts to canonical if-wraps-try form - migrate 6 Pattern B scripts from HVE_SKIP_MAIN to dot-source guard - add guards and extract Invoke-* orchestrators for 4 unguarded scripts - update 10 Pester test files for new guard consumption pattern - document guard convention in scripts/README.md and scripts/linting/README.md 🔧 - Generated by Copilot
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #477 +/- ##
==========================================
+ Coverage 76.22% 83.43% +7.21%
==========================================
Files 20 20
Lines 3503 3507 +4
==========================================
+ Hits 2670 2926 +256
+ Misses 833 581 -252
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Standardizes PowerShell script entry-point behavior across the repo by replacing the $env:HVE_SKIP_MAIN/$script:SkipMain mechanism with a canonical dot-source guard (if ($MyInvocation.InvocationName -ne '.') { try { ... } catch { ... } }) and by extracting orchestrator functions to make scripts easier to test.
Changes:
- Refactored multiple production scripts to expose
Invoke-*orchestrator functions and run them only under the dot-source guard. - Updated Pester tests to dot-source scripts (for function access) and call orchestrators directly instead of manipulating
HVE_SKIP_MAIN. - Documented the canonical guard pattern in
scripts/README.mdand updated linting docs examples.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/security/Update-ActionSHAPinning.ps1 | Extracts Invoke-ActionSHAPinningUpdate and converts main block to canonical guard. |
| scripts/security/Test-SHAStaleness.ps1 | Extracts Invoke-SHAStalenessCheck, updates failure semantics to throw under guard. |
| scripts/security/Test-DependencyPinning.ps1 | Extracts Invoke-DependencyPinningAnalysis and adds [CmdletBinding()] to multiple functions. |
| scripts/security/Test-ActionVersionConsistency.ps1 | Extracts Invoke-ActionVersionConsistencyCheck and normalizes guard behavior. |
| scripts/linting/Validate-MarkdownFrontmatter.ps1 | Normalizes guard/try ordering and updates error handling in main execution. |
| scripts/linting/Test-CopyrightHeaders.ps1 | Extracts Invoke-CopyrightHeaderCheck for testability and converts to guard pattern. |
| scripts/linting/Markdown-Link-Check.ps1 | Extracts Invoke-MarkdownLinkCheck and converts main execution to guard + throw model. |
| scripts/linting/Link-Lang-Check.ps1 | Adds shebang and introduces Invoke-LinkLanguageCheck orchestrator with canonical guard. |
| scripts/linting/Invoke-YamlLint.ps1 | Extracts Invoke-YamlLintCore and adjusts flow to return/throw instead of exiting mid-function. |
| scripts/linting/Invoke-PSScriptAnalyzer.ps1 | Extracts Invoke-PSScriptAnalyzerCore and converts to return/throw + guarded entry point. |
| scripts/linting/Invoke-LinkLanguageCheck.ps1 | Updates wrapper entry point to canonical guard (dot-source friendly). |
| scripts/lib/Get-VerifiedDownload.ps1 | Swaps guard/try ordering to canonical pattern. |
| scripts/extension/Prepare-Extension.ps1 | Swaps guard/try ordering and normalizes error message string. |
| scripts/extension/Package-Extension.ps1 | Swaps guard/try ordering and normalizes error message string. |
| .github/skills/video-to-gif/scripts/convert.ps1 | Extracts Invoke-VideoConversion and adds canonical guard for dot-sourcing. |
| scripts/tests/security/Update-ActionSHAPinning.Tests.ps1 | Removes HVE_SKIP_MAIN manipulation; relies on dot-sourcing for isolation. |
| scripts/tests/security/Test-SHAStaleness.Tests.ps1 | Removes HVE_SKIP_MAIN usage; updates test description to dot-source pattern. |
| scripts/tests/security/Test-DependencyPinning.Tests.ps1 | Removes dot-sourcing error assertion (behavior changed under new guard model). |
| scripts/tests/security/Test-ActionVersionConsistency.Tests.ps1 | Removes HVE_SKIP_MAIN usage and updates subprocess helpers accordingly. |
| scripts/tests/linting/Test-CopyrightHeaders.Tests.ps1 | Dot-sources script and invokes extracted orchestrator for testing. |
| scripts/tests/linting/Markdown-Link-Check.Tests.ps1 | Removes HVE_SKIP_MAIN toggling; continues integration testing via direct invocation. |
| scripts/tests/linting/Link-Lang-Check.Tests.ps1 | Removes HVE_SKIP_MAIN toggling; continues integration testing via direct invocation. |
| scripts/tests/linting/Invoke-YamlLint.Tests.ps1 | Dot-sources script and tests Invoke-YamlLintCore directly. |
| scripts/tests/linting/Invoke-PSScriptAnalyzer.Tests.ps1 | Dot-sources script and tests Invoke-PSScriptAnalyzerCore directly. |
| scripts/tests/linting/Invoke-LinkLanguageCheck.Tests.ps1 | Removes HVE_SKIP_MAIN manipulation; relies on dot-sourcing. |
| scripts/linting/README.md | Updates example to match new orchestrator + guard pattern and throw-based failures. |
| scripts/README.md | Documents canonical entry-point guard pattern for future scripts. |
…ror calls - add -ErrorAction Continue to 13 Write-Error calls across 12 script files - move Set-StrictMode from file scope to function body in Update-ActionSHAPinning.ps1 - add null-coalescing git fallback in Test-CopyrightHeaders.ps1 Path parameter - update scripts/README.md guard template with -ErrorAction Continue and Write-CIAnnotation 🔧 - Generated by Copilot
- add tests for Invoke-MarkdownLinkCheck, Test-SHAStaleness, Update-ActionSHAPinning - add SecurityClasses tests and npm-violations test fixture - fix Update-WorkflowFile to return PSCustomObject instead of hashtable 🧪 - Generated by Copilot
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 31 out of 31 changed files in this pull request and generated 2 comments.
- restore GITHUB_OUTPUT in Test-SHAStaleness test AfterEach to prevent env leak - replace raw >> with guarded Out-File pattern in pester-tests workflow - move StaleDependencies init to script scope for correct cross-function access - add -ErrorAction Continue to Write-Error in catch blocks and README example - replace null-coalescing operator with if/else for PS 5.1 compatibility 🔧 - Generated by Copilot
|
@chaosdinosaur - there's a solid chance this PR is going to cause some re-work on #482 ... my apologies in advance |
katriendg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huge PR, I have used the great PR Reviewer to help identify any inconsistencies. Leaving two comments and one general remark (maybe in a subsequent issue) - see pester-tests.yml comment.
- restructure guard pattern in Generate-PrReference.ps1 - correct test name typo in Update-ActionSHAPinning.Tests.ps1 - guard GITHUB_ENV writes in pester-tests.yml 🔧 - Generated by Copilot
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 32 out of 32 changed files in this pull request and generated 3 comments.
- restore GITHUB_TOKEN in AfterAll to prevent env var leakage across tests - rename misleading test description to match actual assertion behavior - add FRONTMATTER_VALIDATION_FAILED assertion with try/finally cleanup 🔧 - Generated by Copilot
- Set-CIEnv writes to GITHUB_ENV file, not the current process - mock Set-CIEnv and use Should -Invoke with ParameterFilter 🔧 - Generated by Copilot
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 32 out of 32 changed files in this pull request and generated 1 comment.
Pull Request
Description
Standardized all PowerShell entry point guards across the repository to use a single canonical pattern:
if ($MyInvocation.InvocationName -ne '.')wrappingtry/catch. Removed the legacy$script:SkipMain/HVE_SKIP_MAINenvironment variable mechanism and extracted orchestrator functions from scripts that previously embedded logic directly in their main blocks.Key changes:
if ($MyInvocation.InvocationName -ne '.') { try { ... } catch { ... } }) across 16 production scripts$script:SkipMainand$env:HVE_SKIP_MAINfrom 6 scripts that used the environment variable mechanismInvoke-VideoConversion,Invoke-PSScriptAnalyzerCore,Invoke-YamlLintCore,Invoke-LinkLanguageCheck,Invoke-MarkdownLinkCheck,Invoke-CopyrightHeaderCheck,Invoke-ActionVersionConsistencyCheck,Invoke-DependencyPinningAnalysis) for direct testability[CmdletBinding()]to functions missing the attributeHVE_SKIP_MAINenv var manipulationscripts/README.mdandscripts/linting/README.mdRelated Issue(s)
Closes #325
Closes #327
Type of Change
Select all that apply:
Code & Documentation:
Infrastructure & Configuration:
AI Artifacts:
prompt-builderagent and addressed all feedback.github/instructions/*.instructions.md).github/prompts/*.prompt.md).github/agents/*.agent.md).github/skills/*/SKILL.md)Other:
.ps1,.sh,.py)Testing
npm run test:ps)npm run lint:md,npm run lint:ps,npm run lint:frontmatter,npm run lint:md-linksHVE_SKIP_MAINenv varChecklist
Required Checks
Required Automated Checks
The following validation commands must pass before merging:
npm run lint:mdnpm run spell-checknpm run lint:frontmatternpm run lint:md-linksnpm run lint:psSecurity Considerations
Additional Notes
$env:HVE_SKIP_MAIN = '1'to suppress script main block execution. The new pattern uses$MyInvocation.InvocationName -ne '.'which requires dot-sourcing for function isolation.Test-DependencyPinning.ps1was normalized in a separate preparatory commit (9252b08) before the main standardization commit (2a18d9a).🔧 - Generated by Copilot