diff --git a/.github/workflows/verify-closing-keywords.yml b/.github/workflows/verify-closing-keywords.yml new file mode 100644 index 0000000..cdf63d7 --- /dev/null +++ b/.github/workflows/verify-closing-keywords.yml @@ -0,0 +1,81 @@ +name: Verify Closing Keywords + +# Fails a pull request whose body or commit messages list issue references after ONE +# closing keyword -- `Fixes #100, #101` closes #100 only. GitHub reads closing keywords AT +# MERGE TIME, so editing a merged PR's body fixes nothing; this is the only point at which +# the mistake can be caught. tools/Test-PfbClosingKeywords.ps1 holds the rule and its scope +# notes. To quote the wrong form on purpose, put it in backticks. +# +# It does NOT read the PR title or comments, and it does not check that a referenced issue +# exists or is the right one -- only that every reference meant to close has its own keyword. +# +# ubuntu-latest because a PR body can be 65,536 characters and Windows caps an environment +# variable at 32,767. The body reaches the script as an environment VALUE, never as text +# built into a command line. +# +# COMMITS: those on the PR head that are not merges and not already on the base branch. +# `--not origin/` as well as the merge-base, because pull_request.base.sha can be +# stale: after the branch is updated from main, merge-base(stale base, head) is the stale +# base, and main's own commits since then would otherwise be read as the PR's. + +on: + pull_request: + types: [opened, edited, synchronize, reopened] + +permissions: + contents: read + +concurrency: + group: verify-closing-keywords-${{ github.ref }} + cancel-in-progress: true + +jobs: + check: + name: Each issue has its own closing keyword + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - name: Check out the repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + fetch-depth: 0 + + - name: Check the PR body and commit messages + shell: pwsh + env: + PR_BODY: ${{ github.event.pull_request.body }} + BASE_SHA: ${{ github.event.pull_request.base.sha }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + BASE_REF: ${{ github.event.pull_request.base.ref }} + run: | + $ErrorActionPreference = 'Stop' + $check = './tools/Test-PfbClosingKeywords.ps1' + $failures = 0 + # Workflow-command values must escape %, CR and LF, or text from the PR could + # shape the annotation. + function Format-Annotation { param([string]$s) ($s -replace '%', '%25' -replace "`r", '%0D' -replace "`n", '%0A') } + + foreach ($f in @(& $check -Text $env:PR_BODY)) { + $failures++ + Write-Host ("::error::PR body, line {0}: '{1}' closes only its first reference. Write: {2}" -f $f.Line, (Format-Annotation $f.Fragment), (Format-Annotation $f.Corrected)) + } + + $mergeBase = ([string](git merge-base $env:BASE_SHA $env:HEAD_SHA)).Trim() + if ($LASTEXITCODE -ne 0 -or -not $mergeBase) { throw 'git merge-base failed' } + $exclude = @($mergeBase) + git rev-parse --verify --quiet "origin/$($env:BASE_REF)" | Out-Null + if ($LASTEXITCODE -eq 0) { $exclude += "origin/$($env:BASE_REF)" } + else { Write-Host "::warning::origin/$($env:BASE_REF) is not in the checkout; reading commits from the merge-base only." } + + $commits = @(git rev-list --no-merges $env:HEAD_SHA --not @exclude) + if ($LASTEXITCODE -ne 0) { throw 'git rev-list failed' } + foreach ($sha in $commits) { + $message = (git log -1 --format=%B $sha) -join "`n" + foreach ($f in @(& $check -Text $message)) { + $failures++ + Write-Host ("::error::Commit {0}, line {1}: '{2}' closes only its first reference. Write: {3}" -f $sha.Substring(0, 10), $f.Line, (Format-Annotation $f.Fragment), (Format-Annotation $f.Corrected)) + } + } + + Write-Host "Checked the PR body and $($commits.Count) commit message(s): $failures finding(s)." + if ($failures -gt 0) { exit 1 } diff --git a/.github/workflows/verify-wire-exemption.yml b/.github/workflows/verify-wire-exemption.yml new file mode 100644 index 0000000..b3f5a2d --- /dev/null +++ b/.github/workflows/verify-wire-exemption.yml @@ -0,0 +1,74 @@ +name: Verify Wire Exemption + +# INFORMATIONAL ONLY. Classifies this pull request's diff with +# tools/Test-PfbWireExemption.ps1 -- can it change a request the module sends or a response +# it parses? -- and writes the verdict to the job summary. +# +# NOTHING MAY GATE ON THIS JOB. A pull_request run executes the workflow file from the PR's +# own merge commit, so a PR that edits this file controls what it prints. The job therefore +# never fails on the verdict: NotExempt still exits 0, Undecided is a warning. It fails only +# when it malfunctions. A gate that needs the verdict recomputes it outside the PR's control +# (see "TRUSTING THE VERDICT" in the script's help). +# +# It runs the BASE revision's copy of the classifier, never the PR's. The classifier only +# reads git blobs and tokenises them; it never executes code from the diff. +# +# On the PR that introduces the classifier the base has no copy yet, so the job says so and +# skips. + +on: + pull_request: + types: [opened, synchronize, reopened] + +permissions: + contents: read + +concurrency: + group: verify-wire-exemption-${{ github.ref }} + cancel-in-progress: true + +jobs: + classify: + name: Classify the diff (informational) + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - name: Check out the repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + fetch-depth: 0 + + - name: Classify with the base revision's classifier + shell: pwsh + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + $ErrorActionPreference = 'Stop' + git cat-file -e "$($env:BASE_SHA):tools/Test-PfbWireExemption.ps1" 2>$null + if ($LASTEXITCODE -ne 0) { + Write-Host '::notice::The base revision has no tools/Test-PfbWireExemption.ps1 (for example, the PR that adds it), so there is no trusted copy to run. Skipped.' + exit 0 + } + $copy = Join-Path $env:RUNNER_TEMP 'Test-PfbWireExemption.ps1' + git show "$($env:BASE_SHA):tools/Test-PfbWireExemption.ps1" > $copy + if ($LASTEXITCODE -ne 0) { throw "git show failed with exit $LASTEXITCODE" } + + $verdict = @(& $copy -RepoPath $env:GITHUB_WORKSPACE -BaseRef $env:BASE_SHA -HeadRef $env:HEAD_SHA)[-1] + $code = $LASTEXITCODE + if ($null -eq $verdict -or -not $verdict.PSObject.Properties['Decision']) { throw "The classifier returned no verdict (exit $code)." } + + $lines = @('## Wire exemption (informational)', '', "Decision: **$($verdict.Decision)** (exit $code)", '') + if ($verdict.Decision -eq 'Undecided' -and $verdict.Reason) { $lines += "Reason: $($verdict.Reason)"; $lines += '' } + if (@($verdict.Files).Count -gt 0) { + $lines += '| File | Verdict | First executable line |' + $lines += '|---|---|---|' + foreach ($f in $verdict.Files) { $lines += ('| {0} | {1} | {2} |' -f ($f.Path -replace '\|', '\|'), $f.Verdict, $f.FirstExecutableLine) } + $lines += '' + } + if ($verdict.Basis) { $lines += "Basis: $($verdict.Basis)"; $lines += '' } + $lines += 'This job is informational. Nothing gates on its result.' + [System.IO.File]::AppendAllText($env:GITHUB_STEP_SUMMARY, ($lines -join "`n") + "`n") + + if ($verdict.Decision -eq 'Undecided') { Write-Host '::warning::The wire-exemption classifier could not decide (treat as not exempt). The reason is in the job summary.' } + exit 0 diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..3c40721 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,38 @@ +# Start here + +The tool-neutral entry point for anyone -- person or agent -- changing this repository. The +detailed rules live in three files: + +- [`CLAUDE.md`](CLAUDE.md) -- what the module is, when the derived reports must be + regenerated, and what to run before pushing. +- [`Tests/CLAUDE.md`](Tests/CLAUDE.md) -- writing and running the Pester tests, the + two-edition rule, and the coverage baseline in `Tests/coverage-baseline.psd1`. +- [`.github/workflows/CLAUDE.md`](.github/workflows/CLAUDE.md) -- authoring the GitHub + Actions workflows. + +## What CI checks on a pull request + +| Workflow | What it checks | Fails the PR? | +|---|---|---| +| `cross-platform-tests.yml` | Pester on Windows PowerShell 5.1 and on PowerShell 7 (Windows, Linux, macOS), the coverage baseline, and PSScriptAnalyzer | Yes | +| `verify-derived-artifacts.yml` | Every committed artifact in `Data/` and `Reports/` matches a regeneration from the branch (runs when an input changes) | Yes | +| `verify-workflows.yml` | actionlint over `.github/workflows/` (runs when `.github/` changes) | Yes | +| `verify-closing-keywords.yml` | Every issue the PR body or a commit references after a closing keyword has its own keyword | Yes | +| `verify-wire-exemption.yml` | Whether the diff can change a request the module sends or a response it parses | No -- informational | + +"Fails the PR" means the check goes red. None is a required status check; merging is the +maintainer's decision. + +Scheduled, not on pull requests: `update-api-capability-map.yml`, +`verify-agent-ready-briefs.yml` and `report-action-pins.yml`. + +## Scripts you can run locally + +Under PowerShell 7 (`pwsh`); only the module itself has to run on Windows PowerShell 5.1. + +- `scripts/Assert-PfbDerivedArtifacts.ps1` -- the derived-artifact check CI runs. +- `tools/Test-PfbWireExemption.ps1 -BaseRef origin/main` -- whether your branch can change + what goes on the wire. Exit 0 exempt, 1 not, 2 undecided. Prints a basis line for the PR + body when exempt. +- `tools/Test-PfbClosingKeywords.ps1 -Text ` -- the closing-keyword check, with + the corrected form for each finding. diff --git a/Tests/Test-PfbClosingKeywords.Tests.ps1 b/Tests/Test-PfbClosingKeywords.Tests.ps1 new file mode 100644 index 0000000..8c6c686 --- /dev/null +++ b/Tests/Test-PfbClosingKeywords.Tests.ps1 @@ -0,0 +1,145 @@ +#Requires -Modules @{ ModuleName = 'Pester'; ModuleVersion = '5.0' } +<# +.SYNOPSIS + tools/Test-PfbClosingKeywords.ps1: every reference a closing keyword does NOT close is found, + and nothing GitHub would not close from is flagged. +.DESCRIPTION + UNGATED on edition: the script is 5.1-safe and runs on both legs. Listed in + RequiredDescribes for both editions. +#> + +BeforeAll { + $script:repoRoot = Split-Path -Parent $PSScriptRoot + $script:check = Join-Path (Join-Path $script:repoRoot 'tools') 'Test-PfbClosingKeywords.ps1' + # The unary comma is load-bearing: without it a single finding is unrolled on return, + # and on Windows PowerShell 5.1 a lone pscustomobject has no .Count (it reads $null). + function Get-TestFinding { param([AllowNull()][string]$Text) return , @(& $script:check -Text $Text) } +} + +Describe 'Test-PfbClosingKeywords: the PR #108 incident' { + It 'flags the incident text and hands back the corrected form' { + $f = Get-TestFinding 'Fixes #100, #101, #103, #87, and #80.' + $f.Count | Should -Be 1 + $f[0].Line | Should -Be 1 + $f[0].Keyword | Should -BeExactly 'Fixes' + $f[0].Fragment | Should -BeExactly 'Fixes #100, #101, #103, #87, and #80' + $f[0].Corrected | Should -BeExactly 'Fixes #100, fixes #101, fixes #103, fixes #87, fixes #80' + @($f[0].Missed) -join ',' | Should -BeExactly '#101,#103,#87,#80' + } + It 'passes the corrected form' { + (Get-TestFinding 'Fixes #100, fixes #101, fixes #103, fixes #87, fixes #80.').Count | Should -Be 0 + } + It 'reports the right line in a CRLF body (GitHub stores web-edited bodies with CRLF)' { + $f = Get-TestFinding "## Summary`r`n`r`nFixes #100, #101, #103, #87, and #80.`r`n" + $f.Count | Should -Be 1 + $f[0].Line | Should -Be 3 + } +} + +Describe 'Test-PfbClosingKeywords: what is flagged' { + It 'flags ' -ForEach @( + @{ Text = 'Fixes #100, #101'; Corrected = 'Fixes #100, fixes #101' } + @{ Text = 'fixes #87, #88'; Corrected = 'fixes #87, fixes #88' } + @{ Text = 'Closes #1 and dmann000/fb-powershell#2'; Corrected = 'Closes #1, closes dmann000/fb-powershell#2' } + @{ Text = 'Resolves #5, #6'; Corrected = 'Resolves #5, resolves #6' } + @{ Text = 'Fixes: #1, #2'; Corrected = 'Fixes #1, fixes #2' } + @{ Text = 'Fixes https://github.com/dmann000/fb-powershell/issues/1, #2'; Corrected = 'Fixes https://github.com/dmann000/fb-powershell/issues/1, fixes #2' } + @{ Text = 'Fixes #1; #2 / #3 & #4 + #5 plus #6'; Corrected = 'Fixes #1, fixes #2, fixes #3, fixes #4, fixes #5, fixes #6' } + @{ Text = "Fixes #1,$([char]0x00A0)#2"; Corrected = 'Fixes #1, fixes #2' } + ) { + $f = Get-TestFinding $Text + $f.Count | Should -Be 1 + $f[0].Corrected | Should -BeExactly $Corrected + } + It 'knows all nine keywords in any case: ' -ForEach @( + 'close', 'closes', 'closed', 'fix', 'fixes', 'fixed', 'resolve', 'resolves', 'resolved', + 'CLOSES', 'Fixed', 'ReSoLvEd' | ForEach-Object { @{ Kw = $_ } } + ) { + (Get-TestFinding "$Kw #1, #2").Count | Should -Be 1 + } + It 'reports two chains in one text, each on its own line' { + $f = Get-TestFinding "Fixes #1, #2`nand later`nCloses #3, #4" + @($f | ForEach-Object Line) -join ',' | Should -BeExactly '1,3' + } + It 'joins pipeline input into ONE text, so a fence split across lines is still a fence' { + @('```', 'Fixes #1, #2', '```' | & $script:check).Count | Should -Be 0 + @('intro', 'Fixes #1, #2' | & $script:check)[0].Line | Should -Be 2 + } +} + +Describe 'Test-PfbClosingKeywords: what is not flagged' { + It 'passes ' -ForEach @( + @{ Why = 'a single reference'; Text = 'Fixes #63' } + @{ Why = 'prose between references'; Text = 'Fixes #100. Related: #101, #102' } + @{ Why = 'a conventional-commit subject'; Text = 'fix(admin): stop the dead query keys (#99, #100)' } + @{ Why = 'a code span'; Text = 'The old body read `Fixes #100, #101` and closed only #100.' } + @{ Why = 'a ``` fence'; Text = "Fixes #63.`n`n``````n Fixes #1, #2`n``````n" } + @{ Why = 'a ~~~ fence'; Text = "~~~`nFixes #1, #2`n~~~" } + @{ Why = 'an HTML comment (the PR template''s own example)'; Text = "" } + @{ Why = 'a keyword AFTER the list'; Text = 'See #1, #2 -- this fixes them' } + @{ Why = 'a keyword at the end of one line and references on the next'; Text = "this is fixed`n#1, #2" } + @{ Why = 'a keyword that is only the tail of a longer word'; Text = 'hotfixes #1, #2' } + @{ Why = 'an empty text'; Text = '' } + ) { + (Get-TestFinding $Text).Count | Should -Be 0 + } + It 'accepts $null without throwing' { + { Get-TestFinding $null } | Should -Not -Throw + (Get-TestFinding $null).Count | Should -Be 0 + } + It 'handles a 65,536-character body quickly (no catastrophic backtracking)' { + $body = ('x ' * 32760) + 'Fixes #1, #2' + $body.Length | Should -BeGreaterOrEqual 65000 + $elapsed = Measure-Command { $script:big = Get-TestFinding $body } + $script:big.Count | Should -Be 1 + $elapsed.TotalSeconds | Should -BeLessThan 5 + } +} + +Describe 'Test-PfbClosingKeywords: letters are ASCII, as in the JavaScript original' { + # The local hook is JavaScript, whose /i flag (without /u) folds only within ASCII for these + # letters. .NET IgnoreCase does not: on pwsh 7 it treats the Kelvin sign (U+212A) as a K, so + # [^A-Za-z0-9_] stops matching it and the keyword after it is missed -- while Windows + # PowerShell 5.1 agrees with JavaScript. The script spells case out in explicit classes so + # both editions and the hook give one answer. + It 'still sees a keyword right after a Kelvin sign' { + (Get-TestFinding "$([char]0x212A)fixes #1, #2").Count | Should -Be 1 + } + It 'does not read a long s (U+017F) as an s' { + (Get-TestFinding "clo$([char]0x017F)es #1, #2").Count | Should -Be 0 + (Get-TestFinding "Fixes #1 plu$([char]0x017F) #2").Count | Should -Be 0 + } +} + +Describe 'verify-closing-keywords.yml' { + BeforeAll { + $script:ck = [System.IO.File]::ReadAllText((Join-Path $script:repoRoot '.github/workflows/verify-closing-keywords.yml')) + $script:ckRun = @([regex]::Matches($script:ck, '(?m)^([ \t]+)(?:- )?run: \|[ \t]*\r?\n((?:(?:\1[ \t]+\S[^\r\n]*|[ \t]*)(?:\r?\n|$))+)') | ForEach-Object { $_.Groups[2].Value }) + } + It 'runs on opened, edited, synchronize and reopened, on ubuntu (a body can exceed the Windows env-var cap)' { + $script:ck | Should -Match '(?m)^ types: \[opened, edited, synchronize, reopened\]\s*$' + $script:ck | Should -Match '(?m)^ runs-on: ubuntu-latest\s*$' + } + It 'reads contents only, with no secret' { + $permissions = [regex]::Match($script:ck, '(?ms)^permissions:[ \t]*\r?\n(.*?)(?=^\S)').Groups[1].Value + @([regex]::Matches($permissions, '(?m)^ ([a-z-]+: \S+)') | ForEach-Object { $_.Groups[1].Value }) -join ',' | Should -BeExactly 'contents: read' + $script:ck | Should -Not -Match 'secrets\.' + } + It 'passes the body as an env VALUE and interpolates nothing into a run block' { + $script:ck | Should -Match ([regex]::Escape('PR_BODY: ${{ github.event.pull_request.body }}')) + $script:ck | Should -Match ([regex]::Escape('-Text $env:PR_BODY')) + $script:ckRun.Count | Should -BeGreaterThan 0 + @($script:ckRun | Where-Object { $_.Contains('${{') }).Count | Should -Be 0 + } + It 'reads commits from a full-depth checkout, without merges, excluding what is already on the base branch' { + $script:ck | Should -Match '(?m)^\s+fetch-depth: 0\s*$' + $script:ck | Should -Match ([regex]::Escape('git merge-base $env:BASE_SHA $env:HEAD_SHA')) + $script:ck | Should -Match 'git rev-list --no-merges \$env:HEAD_SHA --not' + $script:ck | Should -Match ([regex]::Escape('"origin/$($env:BASE_REF)"')) + } + It 'annotates each finding with the bare ::error:: form and fails the job' { + $script:ck | Should -Match '::error::' + $script:ck | Should -Not -Match '::error file=' + $script:ck | Should -Match '(?m)^\s+if \(\$failures -gt 0\) \{ exit 1 \}' + } +} diff --git a/Tests/Test-PfbWireExemption.Tests.ps1 b/Tests/Test-PfbWireExemption.Tests.ps1 new file mode 100644 index 0000000..ae5274a --- /dev/null +++ b/Tests/Test-PfbWireExemption.Tests.ps1 @@ -0,0 +1,284 @@ +#Requires -Modules @{ ModuleName = 'Pester'; ModuleVersion = '5.0' } +<# +.SYNOPSIS + tools/Test-PfbWireExemption.ps1 against scratch git repositories, one mutation each. +.DESCRIPTION + A green verdict on a real branch proves nothing on its own: a classifier that always said + "exempt" would produce it too. Each case builds a throwaway repo, makes ONE specific + change on a feature branch, and asserts the verdict -- including the cases a regex-based + check gets wrong (#Requires, a comment on a code line, '<#' or '#' inside a here-string). + + Setup that must NOT be part of the diff is committed on main BEFORE the feature branch is + cut (the -Base scriptblock). A harness that committed setup on the feature branch would + make the here-string case NOT EXEMPT for the wrong reason. + + EDITION-GATED: -Skip:($PSVersionTable.PSVersion.Major -lt 7) on every Describe; the 5.1 + skip count is pinned in Tests/coverage-baseline.psd1. +#> + +BeforeAll { + $script:repoRoot = Split-Path -Parent $PSScriptRoot + $script:checker = Join-Path (Join-Path $script:repoRoot 'tools') 'Test-PfbWireExemption.ps1' + $script:utf8 = New-Object System.Text.UTF8Encoding $false + $script:cmdlet = 'Public/Things/Get-PfbThing.ps1' + $script:cmdletSource = (@' +<# +.SYNOPSIS + Fake cmdlet. +.DESCRIPTION + Original description. +#> +function Get-PfbThing { + [CmdletBinding()] + param([string]$Name) + + $uri = "/api/2.0/things" + $body = @{ name = $Name } + Invoke-PfbApiRequest -Uri $uri -Body $body # trailing comment here +} +'@) -replace "`r`n", "`n" + + function Invoke-TestGit { + param([string]$Repo, [string[]]$Arguments) + $out = & git -C $Repo @Arguments 2>&1 + if ($LASTEXITCODE -ne 0) { throw "git $($Arguments -join ' ') failed: $out" } + $out + } + function Write-TestFile { + param([string]$Repo, [string]$RelativePath, [string]$Content) + $full = Join-Path $Repo $RelativePath + $dir = Split-Path -Parent $full + if (-not (Test-Path -LiteralPath $dir)) { $null = New-Item -ItemType Directory -Path $dir -Force } + [System.IO.File]::WriteAllText($full, $Content, $script:utf8) + } + # Throws on a miss. A Replace() that silently finds nothing would turn an "exempt" case + # into a pass on an unchanged file. + function Edit-TestFile { + param([string]$Repo, [string]$RelativePath, [string]$Old, [string]$New) + $full = Join-Path $Repo $RelativePath + $text = [System.IO.File]::ReadAllText($full) + if (-not $text.Contains($Old)) { throw "Edit-TestFile: '$Old' not found in $RelativePath" } + [System.IO.File]::WriteAllText($full, $text.Replace($Old, $New), $script:utf8) + } + function New-TestRepo { + [Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseShouldProcessForStateChangingFunctions', '', Justification = 'Test helper building a scratch git repo under $TestDrive; nothing to confirm.')] + param([scriptblock]$Base) + $repo = Join-Path $TestDrive ([guid]::NewGuid().ToString('N')) + $null = New-Item -ItemType Directory -Path $repo + Invoke-TestGit $repo @('init', '-q', '-b', 'main') | Out-Null + foreach ($pair in @( + @('user.email', 't@example.invalid'), @('user.name', 'test'), + @('core.autocrlf', 'false'), @('commit.gpgsign', 'false'), + @('core.hooksPath', (Join-Path $repo '.no-hooks')))) { + Invoke-TestGit $repo @('config', $pair[0], $pair[1]) | Out-Null + } + Write-TestFile $repo $script:cmdlet $script:cmdletSource + Write-TestFile $repo 'Tests/Some.Tests.ps1' "Describe 'x' { It 'y' { 1 | Should -Be 1 } }`n" + Write-TestFile $repo 'PureStorageFlashBladePowerShell.psd1' "@{ ModuleVersion = '1.0.0' }`n" + Write-TestFile $repo 'PureStorageFlashBladePowerShell.psm1' "# module loader`n. (Join-Path `$PSScriptRoot 'x.ps1')`n" + if ($Base) { & $Base $repo } + Invoke-TestGit $repo @('add', '-A') | Out-Null + Invoke-TestGit $repo @('commit', '-q', '-m', 'base') | Out-Null + Invoke-TestGit $repo @('checkout', '-q', '-b', 'feature') | Out-Null + return $repo + } + function Invoke-TestCase { + param([scriptblock]$Mutate, [scriptblock]$Base, [hashtable]$Extra = @{}) + $repo = New-TestRepo -Base $Base + & $Mutate $repo + Invoke-TestGit $repo @('add', '-A') | Out-Null + Invoke-TestGit $repo @('commit', '-q', '-m', 'change') | Out-Null + $params = @{ RepoPath = $repo; BaseRef = 'main' } + foreach ($k in $Extra.Keys) { $params[$k] = $Extra[$k] } + $out = @(& $script:checker @params 6>$null) + [pscustomobject]@{ Output = $out; ExitCode = $LASTEXITCODE; Repo = $repo } + } +} + +# -ForEach data is evaluated at DISCOVERY, before any BeforeAll runs, so scriptblocks the +# data table references by variable must be defined here. The blocks' BODIES run later, +# inside an It, where the BeforeAll helpers and $script:cmdlet exist. They are $script: +# variables, as elsewhere in this repo's BeforeDiscovery blocks: a plain local that is only +# read from another block is "assigned but never used" to PSUseDeclaredVarsMoreThanAssignments, +# which the analyze job gates at zero. +BeforeDiscovery { + $script:hereStringLt = { param($r) Edit-TestFile $r $script:cmdlet "Invoke-PfbApiRequest -Uri `$uri -Body `$body # trailing comment here`n}" ("Invoke-PfbApiRequest -Uri `$uri -Body `$body # trailing comment here`n}`nfunction Get-PfbDoc {`n `$t = @`"`n<# this is data, not a comment #>`n`"@`n `$t`n}") } + $script:hereStringHash = { param($r) Edit-TestFile $r $script:cmdlet "Invoke-PfbApiRequest -Uri `$uri -Body `$body # trailing comment here`n}" ("Invoke-PfbApiRequest -Uri `$uri -Body `$body # trailing comment here`n}`nfunction Get-PfbNote {`n `$t = @`"`n# not a comment, just data`n`"@`n `$t`n}") } +} + +Describe 'Test-PfbWireExemption exit codes (negative-control pairs)' -Skip:($PSVersionTable.PSVersion.Major -lt 7) { + It ' -> exit ' -ForEach @( + # --- help blocks, deletions, scope, and code-line comments ------------------------ + @{ Name = 'comment-only edit in a help block'; Code = 0; Base = $null; Mutate = { param($r) Edit-TestFile $r $script:cmdlet 'Original description.' 'Rewritten description.' } } + @{ Name = 'whole help block added above the function'; Code = 0; Base = $null; Mutate = { param($r) Edit-TestFile $r $script:cmdlet 'function Get-PfbThing {' "<#`n.NOTES`n Added block.`n#>`nfunction Get-PfbThing {" } } + @{ Name = 'Tests/ only (nothing in scope)'; Code = 0; Base = $null; Mutate = { param($r) Write-TestFile $r 'Tests/Some.Tests.ps1' "Describe 'x' { It 'y' { 1 | Should -Be 1 } }`n# more`n" } } + @{ Name = 'a help line deleted (deletion-only, inert)'; Code = 0; Base = $null; Mutate = { param($r) Edit-TestFile $r $script:cmdlet ".DESCRIPTION`n Original description.`n" '' } } + @{ Name = 'executable line changed (uri)'; Code = 1; Base = $null; Mutate = { param($r) Edit-TestFile $r $script:cmdlet '/api/2.0/things' '/api/2.1/things' } } + @{ Name = 'executable line removed (deletion-only, base-side check)'; Code = 1; Base = $null; Mutate = { param($r) Edit-TestFile $r $script:cmdlet " `$body = @{ name = `$Name }`n" '' } } + @{ Name = 'comment edited on a line that also holds code'; Code = 1; Base = $null; Mutate = { param($r) Edit-TestFile $r $script:cmdlet '# trailing comment here' '# reworded comment' } } + @{ Name = '#Requires added (a Comment token that changes load behaviour)'; Code = 1; Base = $null; Mutate = { param($r) Edit-TestFile $r $script:cmdlet '<#' "#Requires -Version 7.0`n<#" } } + @{ Name = 'new cmdlet file added to Public/'; Code = 1; Base = $null; Mutate = { param($r) Write-TestFile $r 'Public/Things/Get-PfbOther.ps1' "function Get-PfbOther { 'x' }`n" } } + @{ Name = 'cmdlet file deleted from Public/'; Code = 1; Base = $null; Mutate = { param($r) Remove-Item (Join-Path $r $script:cmdlet) } } + @{ Name = 'manifest changed'; Code = 1; Base = $null; Mutate = { param($r) Edit-TestFile $r 'PureStorageFlashBladePowerShell.psd1' '1.0.0' '1.0.1' } } + @{ Name = 'one executable line alongside a comment-only edit'; Code = 1; Base = $null; Mutate = { param($r) Edit-TestFile $r $script:cmdlet 'Original description.' 'Reworded.'; Edit-TestFile $r $script:cmdlet '/api/2.0/' '/api/2.9/' } } + @{ Name = "'<#' inside an edited here-string (setup on main)"; Code = 1; Base = $script:hereStringLt; Mutate = { param($r) Edit-TestFile $r $script:cmdlet 'this is data' 'this is payload' } } + # --- rename, #Requires edited, '#' in a here-string, root module ------------------- + @{ Name = "'#' inside an edited here-string (setup on main)"; Code = 1; Base = $script:hereStringHash; Mutate = { param($r) Edit-TestFile $r $script:cmdlet 'just data' 'just payload' } } + @{ Name = '#Requires edited (setup on main)'; Code = 1; Base = { param($r) Edit-TestFile $r $script:cmdlet '<#' "#Requires -Version 5.1`n<#" }; Mutate = { param($r) Edit-TestFile $r $script:cmdlet '#Requires -Version 5.1' '#Requires -Version 7.0' } } + @{ Name = 'cmdlet file renamed in Public/'; Code = 1; Base = $null; Mutate = { param($r) Invoke-TestGit $r @('mv', $script:cmdlet, 'Public/Things/Get-PfbThing2.ps1') | Out-Null } } + @{ Name = 'cmdlet file moved out of Public/ (rename out of scope)'; Code = 1; Base = $null; Mutate = { param($r) New-Item -ItemType Directory -Force (Join-Path $r 'tools') | Out-Null; Invoke-TestGit $r @('mv', $script:cmdlet, 'tools/Get-PfbThing.ps1') | Out-Null } } + @{ Name = 'comment-only edit in the root module (in scope, inert)'; Code = 0; Base = $null; Mutate = { param($r) Edit-TestFile $r 'PureStorageFlashBladePowerShell.psm1' '# module loader' '# module loader, reworded' } } + ) { + $case = Invoke-TestCase -Mutate $Mutate -Base $Base + $case.ExitCode | Should -Be $Code -Because ($case.Output | Out-String) + } + + It 'an unreachable base ref cannot be decided -> exit 2' { + $case = Invoke-TestCase -Mutate { param($r) Edit-TestFile $r $script:cmdlet '/api/2.0/' '/api/2.1/' } -Extra @{ BaseRef = 'no-such-ref' } + $case.ExitCode | Should -Be 2 + } +} + +Describe 'Test-PfbWireExemption verdict object' -Skip:($PSVersionTable.PSVersion.Major -lt 7) { + It 'emits exactly one object, whose Decision agrees with the exit code ()' -ForEach @( + @{ Decision = 'Exempt'; Code = 0; Mutate = { param($r) Edit-TestFile $r $script:cmdlet 'Original description.' 'Rewritten.' }; Extra = @{} } + @{ Decision = 'NotExempt'; Code = 1; Mutate = { param($r) Edit-TestFile $r $script:cmdlet '/api/2.0/' '/api/2.1/' }; Extra = @{} } + @{ Decision = 'Undecided'; Code = 2; Mutate = { param($r) Edit-TestFile $r $script:cmdlet '/api/2.0/' '/api/2.1/' }; Extra = @{ BaseRef = 'no-such-ref' } } + ) { + $case = Invoke-TestCase -Mutate $Mutate -Extra $Extra + $case.Output.Count | Should -Be 1 -Because 'human-readable text goes to Write-Host, never the success stream' + $case.Output[0].Decision | Should -BeExactly $Decision + $case.ExitCode | Should -Be $Code + } + It 'carries a paste-ready Basis only when exempt' { + (Invoke-TestCase -Mutate { param($r) Edit-TestFile $r $script:cmdlet 'Original description.' 'Rewritten.' }).Output[0].Basis | Should -Match '\S' + (Invoke-TestCase -Mutate { param($r) Edit-TestFile $r $script:cmdlet '/api/2.0/' '/api/2.1/' }).Output[0].Basis | Should -BeNullOrEmpty + } + It 'names the file, its verdict and the first executable line changed' { + $v = (Invoke-TestCase -Mutate { param($r) Edit-TestFile $r $script:cmdlet '/api/2.0/' '/api/2.1/' }).Output[0] + @($v.Files).Count | Should -Be 1 + $v.Files[0].Path | Should -BeExactly $script:cmdlet + $v.Files[0].Verdict | Should -BeExactly 'Executable' + $v.Files[0].FirstExecutableLine | Should -Be 11 + } + It 'an unreachable -HeadRef cannot be decided -> Undecided, exit 2' { + $case = Invoke-TestCase -Mutate { param($r) Edit-TestFile $r $script:cmdlet '/api/2.0/' '/api/2.1/' } -Extra @{ HeadRef = 'no-such-ref' } + $case.Output[0].Decision | Should -BeExactly 'Undecided' + $case.ExitCode | Should -Be 2 + } + It 'classifies -HeadRef, not whatever is checked out (control pair)' { + $case = Invoke-TestCase -Mutate { param($r) Edit-TestFile $r $script:cmdlet '/api/2.0/' '/api/2.1/' } + Invoke-TestGit $case.Repo @('checkout', '-q', 'main') | Out-Null + $atHead = @(& $script:checker -RepoPath $case.Repo -BaseRef main 6>$null) + $atHead[0].Decision | Should -BeExactly 'Exempt' -Because 'HEAD is main, so the diff is empty' + $atFeature = @(& $script:checker -RepoPath $case.Repo -BaseRef main -HeadRef feature 6>$null) + $atFeature[0].Decision | Should -BeExactly 'NotExempt' + } + It 'nothing in scope -> Exempt with its own Basis, no Files and no Reason' { + $case = Invoke-TestCase -Mutate { param($r) Write-TestFile $r 'Tests/Some.Tests.ps1' "Describe 'x' { It 'y' { 1 | Should -Be 1 } }`n# more`n" } + $case.Output.Count | Should -Be 1 + $case.Output[0].Decision | Should -BeExactly 'Exempt' + $case.Output[0].Basis | Should -Match 'entirely untouched' + @($case.Output[0].Files).Count | Should -Be 0 + $case.Output[0].Reason | Should -BeNullOrEmpty + } + It 'an inert file record has no executable line and says comment-only' { + $v = (Invoke-TestCase -Mutate { param($r) Edit-TestFile $r $script:cmdlet 'Original description.' 'Rewritten.' }).Output[0] + @($v.Files).Count | Should -Be 1 + $v.Files[0].Verdict | Should -BeExactly 'Inert' + $v.Files[0].FirstExecutableLine | Should -BeNullOrEmpty + $v.Files[0].Reason | Should -BeExactly 'comment-only' + } + It 'a deletion-only change reports the base-side line as FirstExecutableLine' { + $v = (Invoke-TestCase -Mutate { param($r) Edit-TestFile $r $script:cmdlet " `$body = @{ name = `$Name }`n" '' }).Output[0] + $v.Decision | Should -BeExactly 'NotExempt' + $v.Files[0].Verdict | Should -BeExactly 'Executable' + $v.Files[0].FirstExecutableLine | Should -Be 12 + } + # A failure AFTER revision resolution: the head blob is corrupted, so rev-parse, + # merge-base and --name-status (which compare object ids only) still succeed and the + # per-file 'git diff -U0' is what fails. Without the catch the error escapes the script. + It 'a git failure after revision resolution -> one Undecided object naming it, exit 2' { + $case = Invoke-TestCase -Mutate { param($r) Edit-TestFile $r $script:cmdlet '/api/2.0/' '/api/2.1/' } + $case.Output[0].Decision | Should -BeExactly 'NotExempt' -Because 'control: the same repo classifies cleanly before the corruption' + + $sha = ([string](Invoke-TestGit $case.Repo @('rev-parse', "feature:$($script:cmdlet)"))).Trim() + $object = Join-Path (Join-Path (Join-Path (Join-Path $case.Repo '.git') 'objects') $sha.Substring(0, 2)) $sha.Substring(2) + Test-Path -LiteralPath $object | Should -BeTrue -Because 'a fresh repo stores the blob loose' + [System.IO.File]::SetAttributes($object, [System.IO.FileAttributes]::Normal) + [System.IO.File]::WriteAllBytes($object, [byte[]](1..20)) + + $out = @(& $script:checker -RepoPath $case.Repo -BaseRef main 6>$null) + $code = $LASTEXITCODE + $out.Count | Should -Be 1 + $out[0].Decision | Should -BeExactly 'Undecided' + $out[0].Reason | Should -Match 'could not classify the diff' + $out[0].Reason | Should -Match $sha -Because 'the reason names the failure, here the unreadable object' + $out[0].Reason | Should -Not -Match '(?': an elided 'C:\...\repo' must fail too. + It 'carries no absolute Windows path' { + $script:src | Should -Not -Match '(? 26 for issue #141 Task 4: two new Describes (the five-bucket partition # reconciliation and the unknown-Surface refusal) exercise # tools/Build-PfbFieldCmdletMap.ps1 itself, which carries `#Requires -Version 7.0`, @@ -381,6 +401,12 @@ 'verify-workflows.yml (actionlint)' 'report-action-pins.yml (weekly, report-only)' 'GitHub issue forms and PR template' + # PR gates: the closing-keyword checker and its workflow, ungated text checks, every leg. + 'Test-PfbClosingKeywords: the PR #108 incident' + 'Test-PfbClosingKeywords: what is flagged' + 'Test-PfbClosingKeywords: what is not flagged' + 'Test-PfbClosingKeywords: letters are ASCII, as in the JavaScript original' + 'verify-closing-keywords.yml' ) } } diff --git a/tools/Test-PfbClosingKeywords.ps1 b/tools/Test-PfbClosingKeywords.ps1 new file mode 100644 index 0000000..543beba --- /dev/null +++ b/tools/Test-PfbClosingKeywords.ps1 @@ -0,0 +1,146 @@ +<# +.SYNOPSIS + Finds issue references that a closing keyword does NOT close, in a PR body or a commit message. +.DESCRIPTION + GitHub closes an issue on merge only when a closing keyword immediately precedes THAT + issue's own reference. In a list, only the first reference closes: + + Fixes #100, #101, #103 closes #100 only + Fixes #100, fixes #101 closes both + + A pull request here once opened with `Fixes #100, #101, #103, #87, and #80`, and on merge + closed #100 and nothing else. The mistake is silent, and it cannot be fixed afterwards: + GitHub reads closing keywords AT MERGE TIME, so editing a merged PR's body closes nothing. + That is why a finding is meant to stop a submission rather than warn about it: the PR + looks right, the merge succeeds, and the issues simply stay open until someone notices + the backlog. A warning that can be scrolled past is the wrong shape for a mistake whose + whole problem is that nobody notices it. + + This script is the single implementation of the rule. .github/workflows/verify-closing-keywords.yml + runs it on every pull request's body and commits, and local pre-submit tooling may call + it too. It returns the corrected form for each finding. + + KEYWORDS: close, closes, closed, fix, fixes, fixed, resolve, resolves, resolved -- any + case, optionally followed by a colon (Fixes: #1). REFERENCES: #N, owner/repo#N, and + https://github.com/owner/repo/issues/N (or /pull/N). + + OUT OF SCOPE, each for a reason: + * Titles, issue bodies and comments. A keyword there closes nothing, so the mistake + cannot happen there. Only a PR body and the commits that land on the default branch + are checked. + * Code spans, ``` and ~~~ fences, and HTML comments (). GitHub neither links + nor closes from them, so they are blanked before matching. This is the escape hatch: + to quote the wrong form on purpose, put it in backticks. The PR template keeps its + example inside an HTML comment for the same reason. + * A chain continues only across list punctuation -- , ; / & + "and" "plus" and + whitespace. `Fixes #100. Related: #101` is not a finding. + * A conventional-commit subject such as `fix(admin): ... (#99, #100)` has no reference + right after the keyword, so there is no closing anchor. + * A keyword and its reference must share a line; `...is fixed` at the end of one line + does not adopt `#1, #2` on the next. +.PARAMETER Text + The text to check. Pipeline input is joined with newlines into ONE text, so + `Get-Content body.md | ./tools/Test-PfbClosingKeywords.ps1` checks the file as a whole. +.OUTPUTS + One object per finding: Line (1-based line of the keyword), Keyword (as written), + Fragment (what was written, whitespace collapsed), Corrected (what to write instead), + Missed (the references that would stay open). No output means no finding. +.EXAMPLE + ./tools/Test-PfbClosingKeywords.ps1 -Text 'Fixes #1, #2' +#> +[CmdletBinding()] +param( + [Parameter(ValueFromPipeline = $true)][AllowNull()][AllowEmptyString()][string]$Text +) + +begin { + Set-StrictMode -Version Latest + $ErrorActionPreference = 'Stop' + $parts = New-Object System.Collections.Generic.List[string] + + # JavaScript's regex classes, spelled out. .NET's \b, \d and \s are Unicode-aware and its + # $ also matches before a final newline; any of those would make this disagree with the + # local hook that falls back to the JavaScript original. + # + # Case is spelled out too, and no regex here uses IgnoreCase. JavaScript's /i (without /u) + # folds these letters only within ASCII. .NET IgnoreCase does not, and not even the same + # way on both editions: on pwsh 7 it counts the Kelvin sign (U+212A) as a K, so + # [^A-Za-z0-9_] stops matching it and a keyword right after one is missed, while Windows + # PowerShell 5.1 agrees with JavaScript. Explicit [Xx] classes give all three one answer. + $word = '[A-Za-z0-9_]' + $jsBoundary = "(?:(?<=$word)(?!$word)|(?', $script:Blank) + return [regex]::Replace($t, '`+[^\n`]*`+', $script:Blank) + } + + function Get-PfbClosingChain { + param([string]$Source) + $t = ConvertTo-PfbInertBlankText $Source + $refs = @(foreach ($m in $script:ReRef.Matches($t)) { [pscustomobject]@{ Start = $m.Index; End = $m.Index + $m.Length; Text = $m.Value } }) + $chains = New-Object System.Collections.Generic.List[object] + $cur = $null + for ($i = 0; $i -lt $refs.Count; $i++) { + $windowStart = [Math]::Max(0, $refs[$i].Start - 24) + $kw = $script:ReKeywordTail.Match($t.Substring($windowStart, $refs[$i].Start - $windowStart)) + if ($kw.Success) { + $kwText = $kw.Groups[1].Value + $offset = 1 + if ($kw.Value.StartsWith($kwText, [System.StringComparison]::Ordinal)) { $offset = 0 } + $cur = [pscustomobject]@{ Keyword = $kwText; KwStart = $windowStart + $kw.Index + $offset; Anchor = $refs[$i]; Bare = (New-Object System.Collections.Generic.List[object]) } + $chains.Add($cur) + continue + } + if ($null -ne $cur -and $i -gt 0 -and $script:ReSeparatorOnly.IsMatch($t.Substring($refs[$i - 1].End, $refs[$i].Start - $refs[$i - 1].End))) { + $cur.Bare.Add($refs[$i]) + continue + } + $cur = $null + } + foreach ($c in $chains) { + if ($c.Bare.Count -eq 0) { continue } + $last = $c.Bare[$c.Bare.Count - 1] + $fragment = $script:ReSpaceRun.Replace($t.Substring($c.KwStart, $last.End - $c.KwStart), ' ').Trim() + $lower = $c.Keyword.ToLowerInvariant() + $corrected = @("$($c.Keyword) $($c.Anchor.Text)") + @($c.Bare | ForEach-Object { "$lower $($_.Text)" }) + [pscustomobject]@{ + Line = [regex]::Matches($t.Substring(0, $c.KwStart), "`n").Count + 1 + Keyword = $c.Keyword + Fragment = $fragment + Corrected = $corrected -join ', ' + Missed = @($c.Bare | ForEach-Object { $_.Text }) + } + } + } +} + +process { + if ($null -ne $Text) { $parts.Add($Text) } +} + +end { + if ($parts.Count -eq 0) { return } + Get-PfbClosingChain ($parts -join "`n") +} diff --git a/tools/Test-PfbWireExemption.ps1 b/tools/Test-PfbWireExemption.ps1 new file mode 100644 index 0000000..6a0d27c --- /dev/null +++ b/tools/Test-PfbWireExemption.ps1 @@ -0,0 +1,360 @@ +#Requires -Version 7.0 + +<# +.SYNOPSIS + Decides whether a diff can change what this module sends to, or parses from, a FlashBlade. +.DESCRIPTION + Changes that can alter a request the module sends or a response it parses are verified + against a real FlashBlade before they merge, because mocked tests encode our belief about + the REST API rather than the API itself. This script decides, mechanically, whether a + diff is exempt from that: it is exempt when no EXECUTABLE line changed in any of + + Public/ Private/ PureStorageFlashBladePowerShell.psd1 PureStorageFlashBladePowerShell.psm1 + + "Executable" is decided by the PowerShell tokeniser, not by eye and not by regex. A + changed line is inert only when every token overlapping it is a comment. That is + deliberately STRICTER than "reaches the wire": an executable change that provably cannot + reach the wire (adding a Write-Verbose, renaming a local) still counts, because erring + toward a live check is the recoverable direction. + + Why the tokeniser and not a regex: a '#' inside a here-string, and a string literal + containing '<#', both defeat text matching. The tokeniser resolves them. + + #Requires tokenises as a Comment yet still populates ScriptRequirements, so it changes + what the module loads under. It is forced to executable here. + + A comment reworded on a line that also holds code counts as executable: git is + line-granular, so that edit cannot be told apart from a code edit. + + Both sides of the diff are parsed. Deleted lines do not exist at -HeadRef, so they are + classified against the merge-base revision of the file. An added, deleted or renamed + in-scope file is never inert. + + The script only runs `git` and tokenises blobs. It never executes code from the diff, so + it is safe to run against an untrusted pull request. + + TRUSTING THE VERDICT. A pull_request CI run executes the workflow file from the PR's own + merge commit, so a PR that edits that workflow controls what the CI job prints. Nothing + may gate on the CI job's conclusion or its summary. A gate that needs this verdict + recomputes it outside the PR's control: + + git fetch origin + git show origin/main:tools/Test-PfbWireExemption.ps1 > /Test-PfbWireExemption.ps1 + /Test-PfbWireExemption.ps1 -RepoPath -BaseRef
-HeadRef + + It uses the main TIP (not pull_request.base.sha, which can be stale), passes both SHAs + explicitly, and branches on the returned Decision. It never reads a check conclusion or a + job summary. +.PARAMETER BaseRef + Revision to diff against. Default 'origin/main'. The comparison is three-dot (from the + merge-base of -BaseRef and -HeadRef). +.PARAMETER HeadRef + Revision to classify. Default 'HEAD'. Need not be checked out. +.PARAMETER RepoPath + Repository or worktree to inspect. Defaults to the current directory. +.OUTPUTS + Exactly one object: Decision ('Exempt' | 'NotExempt' | 'Undecided'); Files, one record + per in-scope file (Path, Verdict 'Inert' | 'Executable', FirstExecutableLine, Reason); + Basis, a line ready to paste into the PR body, or $null unless exempt; Reason, one line + naming what went wrong (absolute paths replaced by placeholders), or $null unless + undecided. Everything human-readable goes to Write-Host. + + Exit code 0 = exempt, 1 = not exempt, 2 = could not decide (treat as not exempt). +.EXAMPLE + ./tools/Test-PfbWireExemption.ps1 -BaseRef origin/main +.EXAMPLE + $v = ./tools/Test-PfbWireExemption.ps1 -BaseRef $baseSha -HeadRef $headSha 6>$null + if ($v.Decision -ne 'Exempt') { 'live verification required' } +#> +[CmdletBinding()] +param( + [string]$BaseRef = 'origin/main', + [string]$HeadRef = 'HEAD', + [string]$RepoPath = '.' +) + +Set-StrictMode -Version Latest +$ErrorActionPreference = 'Stop' + +# The in-scope locations: the module source and its manifest (see .DESCRIPTION). +$script:ScopePrefixes = @('Public/', 'Private/') +$script:ScopeExact = @( + 'PureStorageFlashBladePowerShell.psd1', + 'PureStorageFlashBladePowerShell.psm1' +) + +function Test-InScope { + param([string]$Path) + foreach ($p in $script:ScopePrefixes) { if ($Path.StartsWith($p)) { return $true } } + return ($script:ScopeExact -contains $Path) +} + +function Invoke-Git { + param([string[]]$Arguments, [switch]$AllowFailure) + $out = & git -C $RepoPath @Arguments 2>&1 + if ($LASTEXITCODE -ne 0 -and -not $AllowFailure) { + throw "git $($Arguments -join ' ') failed: $out" + } + return $out +} + +# Lines carrying any token that is not a comment. Everything else -- blank lines, +# whole-line comments, help blocks -- is inert. +# +# A line with a trailing comment ('$x = 1 # note') counts as EXECUTABLE, because +# git is line-granular: we cannot tell a comment-only edit on that line from a +# code edit, so we must assume the latter. +function Get-ExecutableLine { + param([string]$Source) + + $tokens = $null + $errors = $null + [void][System.Management.Automation.Language.Parser]::ParseInput($Source, [ref]$tokens, [ref]$errors) + + if ($errors -and $errors.Count -gt 0) { + throw "source does not parse: $($errors[0].Message)" + } + + $executable = New-Object 'System.Collections.Generic.HashSet[int]' + foreach ($t in $tokens) { + $kind = $t.Kind.ToString() + if ($kind -eq 'NewLine' -or $kind -eq 'EndOfInput' -or $kind -eq 'LineContinuation') { continue } + + $isInert = $false + if ($kind -eq 'Comment') { + $isInert = $true + # #Requires is a Comment token that changes load behaviour. Not inert. + if ($t.Text -match '^\s*#\s*requires\b') { $isInert = $false } + } + if ($isInert) { continue } + + for ($n = $t.Extent.StartLineNumber; $n -le $t.Extent.EndLineNumber; $n++) { + [void]$executable.Add($n) + } + } + # Comma operator is load-bearing: a bare 'return $executable' unrolls the set + # into the pipeline, and a single-element set collapses to a bare [int] whose + # .Contains() then throws. Cost an hour once. + return ,$executable +} + +# Changed line numbers per side, from a -U0 diff of one file. +function Get-ChangedLine { + param([string]$Path, [string]$Base, [string]$Head) + + $added = New-Object 'System.Collections.Generic.List[int]' + $removed = New-Object 'System.Collections.Generic.List[int]' + + $diff = Invoke-Git @('diff', '-U0', '--no-color', "$Base..$Head", '--', $Path) + foreach ($line in $diff) { + if ($line -notmatch '^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@') { continue } + + $oldStart = [int]$Matches[1] + $oldCount = 1 + if ($Matches[2]) { $oldCount = [int]$Matches[2] } + $newStart = [int]$Matches[3] + $newCount = 1 + if ($Matches[4]) { $newCount = [int]$Matches[4] } + + for ($n = 0; $n -lt $oldCount; $n++) { $removed.Add($oldStart + $n) } + for ($n = 0; $n -lt $newCount; $n++) { $added.Add($newStart + $n) } + } + return @{ Added = $added; Removed = $removed } +} + +function Get-Blob { + param([string]$Rev, [string]$Path) + $content = Invoke-Git @('show', "${Rev}:${Path}") -AllowFailure + if ($LASTEXITCODE -ne 0) { return $null } + return ($content -join "`n") +} + +# The ONE object this script puts on the success stream. Everything human-readable goes +# through Write-Host, so a caller can do `$v = & ./tools/Test-PfbWireExemption.ps1 ...` and +# branch on $v.Decision without filtering text out of it. +function New-PfbWireVerdict { + [Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseShouldProcessForStateChangingFunctions', '', Justification = 'Constructs an in-memory object; changes no state.')] + param([string]$Decision, [object[]]$Files = @(), [string]$Basis = '', [string]$Reason = '') + $b = $null + if ($Basis) { $b = $Basis } + $r = $null + if ($Reason) { $r = $Reason } + [pscustomobject]@{ Decision = $Decision; Files = @($Files); Basis = $b; Reason = $r } +} + +# A failure message fit for a verdict object that may be posted publicly: one line, with +# the repository path and any other absolute path replaced by a placeholder. git's own +# errors can name the object store by absolute path. +function Get-PfbFailureText { + param([string]$Message) + $text = ($Message -replace '\s+', ' ').Trim() + $root = $null + try { $root = (Resolve-Path -LiteralPath $RepoPath -ErrorAction Stop).ProviderPath } catch { $root = $null } + if ($root) { + foreach ($form in @($root, $root.Replace('\', '/'))) { + $text = $text -replace ('(?i)' + [regex]::Escape($form.TrimEnd('\', '/'))), '' + } + } + $text = $text -replace '(?i)(?' + $text = $text -replace '(?-])/(?:[^\s''"()/]+/)+[^\s''"()]*', '' + return $text +} + +# --- resolve revisions ------------------------------------------------------- + +try { + $head = ([string](Invoke-Git @('rev-parse', '--verify', "$HeadRef^{commit}"))).Trim() + $mergeBase = ([string](Invoke-Git @('merge-base', $BaseRef, $head))).Trim() +} catch { + $why = Get-PfbFailureText "could not resolve revisions: $_" + Write-Host $why -ForegroundColor Red + New-PfbWireVerdict -Decision 'Undecided' -Reason $why + exit 2 +} + +Write-Host "" +Write-Host "base $BaseRef @ $($mergeBase.Substring(0,10))" +Write-Host "head $HeadRef @ $($head.Substring(0,10))" +Write-Host "" + +# Everything from here to the report runs inside one try. An unplanned failure (git refusing +# a diff, an unreadable object) is a malfunction, not a verdict: it must reach the caller as +# Undecided/exit 2, never as an uncaught error that exits 1 and reads as NotExempt. +try { + # --- collect in-scope changes -------------------------------------------- + + # --no-renames: a rename is reported as a delete of the old path plus an add of the new + # one, so a file moved OUT of scope still counts as an in-scope deletion. With rename + # detection on, only the new path would be seen and the move would read as exempt. + $nameStatus = Invoke-Git @('diff', '--name-status', '--no-renames', '--no-color', "$mergeBase..$head") + + $inScope = @() + foreach ($line in $nameStatus) { + if ([string]::IsNullOrWhiteSpace($line)) { continue } + $parts = $line -split "`t" + $status = $parts[0] + $path = $parts[-1] + if (-not (Test-InScope $path)) { continue } + $inScope += [pscustomobject]@{ Status = $status; Path = $path } + } + + if ($inScope.Count -eq 0) { + Write-Host "No in-scope file changed." -ForegroundColor Green + Write-Host "" + Write-Host "VERDICT: EXEMPT" -ForegroundColor Green + Write-Host "" + # States what the diff does not touch; names no tooling, so it reads correctly in any PR body. + Write-Host "Basis for the PR body:" + Write-Host " The diff leaves the module source and the manifest entirely untouched," + Write-Host " so nothing here can alter a request the module sends or a response it" + Write-Host " parses." + New-PfbWireVerdict -Decision 'Exempt' -Basis 'The diff leaves the module source and the manifest entirely untouched, so nothing here can alter a request the module sends or a response it parses.' + exit 0 + } + + # --- classify ------------------------------------------------------------ + + $verdicts = @() + $failed = $false + + foreach ($file in $inScope) { + $path = $file.Path + $reasons = @() + # First executable changed line: head side if any, else base side. $null when the file + # is inert or was rejected as a whole (added or deleted; a rename arrives as both). + $firstLine = $null + + # An added or deleted file changes the exported surface. Never inert. + if ($file.Status -eq 'A') { $reasons += 'file added' } + elseif ($file.Status -eq 'D') { $reasons += 'file deleted' } + else { + $changed = Get-ChangedLine -Path $path -Base $mergeBase -Head $head + + if ($changed.Added.Count -gt 0) { + $headSource = Get-Blob -Rev $head -Path $path + if ($null -eq $headSource) { + $reasons += 'could not read head blob' + } else { + try { + $exec = Get-ExecutableLine -Source $headSource + $hits = @($changed.Added | Where-Object { $exec.Contains($_) } | Sort-Object -Unique) + if ($hits.Count -gt 0) { $firstLine = [int]$hits[0] } + if ($hits.Count -gt 0) { + $shown = ($hits | Select-Object -First 6) -join ', ' + $suffix = '' + if ($hits.Count -gt 6) { $suffix = " (+$($hits.Count - 6) more)" } + $reasons += "executable line(s) added: $shown$suffix" + } + } catch { $reasons += "head side: $_" } + } + } + + if ($changed.Removed.Count -gt 0) { + $baseSource = Get-Blob -Rev $mergeBase -Path $path + if ($null -eq $baseSource) { + $reasons += 'could not read base blob' + } else { + try { + $exec = Get-ExecutableLine -Source $baseSource + $hits = @($changed.Removed | Where-Object { $exec.Contains($_) } | Sort-Object -Unique) + if ($hits.Count -gt 0 -and $null -eq $firstLine) { $firstLine = [int]$hits[0] } + if ($hits.Count -gt 0) { + $shown = ($hits | Select-Object -First 6) -join ', ' + $suffix = '' + if ($hits.Count -gt 6) { $suffix = " (+$($hits.Count - 6) more)" } + $reasons += "executable line(s) removed: $shown$suffix" + } + } catch { $reasons += "base side: $_" } + } + } + } + + $isInert = ($reasons.Count -eq 0) + if (-not $isInert) { $failed = $true } + + $verdicts += [pscustomobject]@{ + Path = $path + Verdict = $(if ($isInert) { 'Inert' } else { 'Executable' }) + FirstExecutableLine = $firstLine + Reason = $(if ($isInert) { 'comment-only' } else { $reasons -join '; ' }) + } + } +} catch { + $why = Get-PfbFailureText "could not classify the diff: $_" + Write-Host $why -ForegroundColor Red + New-PfbWireVerdict -Decision 'Undecided' -Reason $why + exit 2 +} + +# --- report ------------------------------------------------------------------ + +Write-Host "In-scope files changed: $($inScope.Count)" +Write-Host "" +foreach ($v in $verdicts) { + if ($v.Verdict -eq 'Inert') { + Write-Host (" inert {0}" -f $v.Path) -ForegroundColor Green + } else { + Write-Host (" EXECUTABLE {0}" -f $v.Path) -ForegroundColor Red + Write-Host (" {0}" -f $v.Reason) -ForegroundColor Red + } +} +Write-Host "" + +if ($failed) { + Write-Host "VERDICT: NOT EXEMPT -- live verification required" -ForegroundColor Red + Write-Host "" + Write-Host "One executable line anywhere in scope disqualifies the whole PR." + Write-Host "There is no partial exemption." + New-PfbWireVerdict -Decision 'NotExempt' -Files $verdicts + exit 1 +} + +Write-Host "VERDICT: EXEMPT" -ForegroundColor Green +Write-Host "" +# The basis line states the substance -- what did and did not change -- so it can be pasted into a PR body as-is. +Write-Host "Basis for the PR body:" +Write-Host " Every in-scope change is inside a comment -- no executable line changed in" +Write-Host " the module source or the manifest, confirmed by comparing the parsed token" +Write-Host " stream on both sides of the diff." +New-PfbWireVerdict -Decision 'Exempt' -Files $verdicts -Basis 'Every in-scope change is inside a comment -- no executable line changed in the module source or the manifest, confirmed by comparing the parsed token stream on both sides of the diff.' +exit 0