From a6c6c596205fed046bf291c32e7208aeafca972d Mon Sep 17 00:00:00 2001 From: Justin Emerson Date: Mon, 28 Sep 2026 11:18:10 +0000 Subject: [PATCH 01/10] test(wire-exemption): port the classifier's negative-control harness to Pester Adds the rename, #Requires-edited, '#'-in-here-string and root-module cases, and commits here-string setup on main so the '<#' case can fail for the right reason. Co-Authored-By: Claude Opus 5.5 (1M context) --- Tests/Test-PfbWireExemption.Tests.ps1 | 139 ++++++++++++++++++++++++++ 1 file changed, 139 insertions(+) create mode 100644 Tests/Test-PfbWireExemption.Tests.ps1 diff --git a/Tests/Test-PfbWireExemption.Tests.ps1 b/Tests/Test-PfbWireExemption.Tests.ps1 new file mode 100644 index 0000000..771b720 --- /dev/null +++ b/Tests/Test-PfbWireExemption.Tests.ps1 @@ -0,0 +1,139 @@ +#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). The original harness committed it on the feature branch, + which made its 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 (ported from the original negative-control harness, plus additions)' -Skip:($PSVersionTable.PSVersion.Major -lt 7) { + It ' -> exit ' -ForEach @( + # --- the 13 original cases -------------------------------------------------------- + @{ 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' } } + # --- additions: 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 = '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 + } +} From 2b3fee6c99ff3130d43f3601831586546fe405e7 Mon Sep 17 00:00:00 2001 From: Justin Emerson Date: Mon, 28 Sep 2026 11:18:10 +0000 Subject: [PATCH 02/10] feat(tools): add the wire-exemption classifier with -HeadRef and a verdict object Moved from local tooling. Adds -HeadRef, returns one Decision/Files/Basis object with exit codes unchanged, and documents how a gate must recompute the verdict from origin/main rather than trust a pull_request run. Co-Authored-By: Claude Opus 5.5 (1M context) --- Tests/Test-PfbWireExemption.Tests.ps1 | 63 +++++ tools/Test-PfbWireExemption.ps1 | 326 ++++++++++++++++++++++++++ 2 files changed, 389 insertions(+) create mode 100644 tools/Test-PfbWireExemption.ps1 diff --git a/Tests/Test-PfbWireExemption.Tests.ps1 b/Tests/Test-PfbWireExemption.Tests.ps1 index 771b720..a91800f 100644 --- a/Tests/Test-PfbWireExemption.Tests.ps1 +++ b/Tests/Test-PfbWireExemption.Tests.ps1 @@ -137,3 +137,66 @@ Describe 'Test-PfbWireExemption exit codes (ported from the original negative-co $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' + } +} + +Describe 'Test-PfbWireExemption is publishable as written' -Skip:($PSVersionTable.PSVersion.Major -lt 7) { + BeforeAll { $script:src = [System.IO.File]::ReadAllText($script:checker) } + It 'names no private rule and no local tooling' { + $script:src | Should -Not -Match '(?i)private (development )?rule' + $script:src | Should -Not -Match '(?i)does not belong|not belong in' + } + # Any drive root, not just 'X:\': an elided 'C:\...\worktrees' must fail too. + It 'carries no absolute Windows path' { + $script:src | Should -Not -Match '(? /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. 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 four in-scope locations, verbatim from the rule. +$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 = '') + $b = $null + if ($Basis) { $b = $Basis } + [pscustomobject]@{ Decision = $Decision; Files = @($Files); Basis = $b } +} + +# --- resolve revisions ------------------------------------------------------- + +try { + $head = ([string](Invoke-Git @('rev-parse', '--verify', "$HeadRef^{commit}"))).Trim() + $mergeBase = ([string](Invoke-Git @('merge-base', $BaseRef, $head))).Trim() +} catch { + Write-Host "Could not resolve revisions: $_" -ForegroundColor Red + New-PfbWireVerdict -Decision 'Undecided' + exit 2 +} + +Write-Host "" +Write-Host "base $BaseRef @ $($mergeBase.Substring(0,10))" +Write-Host "head $HeadRef @ $($head.Substring(0,10))" +Write-Host "" + +# --- collect in-scope changes ------------------------------------------------ + +$nameStatus = Invoke-Git @('diff', '--name-status', '--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, deleted, renamed). + $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' } + elseif ($file.Status -like 'R*') { $reasons += "file renamed ($($file.Status))" } + 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 '; ' }) + } +} + +# --- 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 From 3afb63885d64a67bc68c6ae7c0cc3ebfd62e1c35 Mon Sep 17 00:00:00 2001 From: Justin Emerson Date: Mon, 28 Sep 2026 11:28:14 +0000 Subject: [PATCH 03/10] fix(wire-exemption): report an unplanned git failure as Undecided, and tighten test coverage A git failure after revision resolution now yields one Undecided verdict with a path-free Reason and exit 2, instead of an uncaught error that exits 1 and reads as NotExempt. Adds tests for that path, the no-in-scope Exempt object, an inert file record and a base-side-only first executable line, and rewords comments that pointed at material outside the repo. Co-Authored-By: Claude Opus 5.5 (1M context) --- Tests/Test-PfbWireExemption.Tests.ps1 | 57 ++++++- tools/Test-PfbWireExemption.ps1 | 218 +++++++++++++++----------- 2 files changed, 176 insertions(+), 99 deletions(-) diff --git a/Tests/Test-PfbWireExemption.Tests.ps1 b/Tests/Test-PfbWireExemption.Tests.ps1 index a91800f..b73b2b6 100644 --- a/Tests/Test-PfbWireExemption.Tests.ps1 +++ b/Tests/Test-PfbWireExemption.Tests.ps1 @@ -9,8 +9,8 @@ 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). The original harness committed it on the feature branch, - which made its here-string case NOT EXEMPT for the wrong reason. + 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. @@ -106,9 +106,9 @@ BeforeDiscovery { $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 (ported from the original negative-control harness, plus additions)' -Skip:($PSVersionTable.PSVersion.Major -lt 7) { +Describe 'Test-PfbWireExemption exit codes (negative-control pairs)' -Skip:($PSVersionTable.PSVersion.Major -lt 7) { It ' -> exit ' -ForEach @( - # --- the 13 original cases -------------------------------------------------------- + # --- 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" } } @@ -122,7 +122,7 @@ Describe 'Test-PfbWireExemption exit codes (ported from the original negative-co @{ 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' } } - # --- additions: rename, #Requires edited, '#' in a here-string, root module ---------- + # --- 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 } } @@ -173,6 +173,51 @@ Describe 'Test-PfbWireExemption verdict object' -Skip:($PSVersionTable.PSVersion $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:\...\worktrees' must fail too. + # Any drive root, not just 'X:\': an elided 'C:\...\repo' must fail too. It 'carries no absolute Windows path' { $script:src | Should -Not -Match '(?' + } + } + $text = $text -replace '(?i)(?' + $text = $text -replace '(?-])/(?:[^\s''"()/]+/)+[^\s''"()]*', '' + return $text } # --- resolve revisions ------------------------------------------------------- @@ -185,8 +206,9 @@ try { $head = ([string](Invoke-Git @('rev-parse', '--verify', "$HeadRef^{commit}"))).Trim() $mergeBase = ([string](Invoke-Git @('merge-base', $BaseRef, $head))).Trim() } catch { - Write-Host "Could not resolve revisions: $_" -ForegroundColor Red - New-PfbWireVerdict -Decision 'Undecided' + $why = Get-PfbFailureText "could not resolve revisions: $_" + Write-Host $why -ForegroundColor Red + New-PfbWireVerdict -Decision 'Undecided' -Reason $why exit 2 } @@ -195,101 +217,111 @@ Write-Host "base $BaseRef @ $($mergeBase.Substring(0,10))" Write-Host "head $HeadRef @ $($head.Substring(0,10))" Write-Host "" -# --- collect in-scope changes ------------------------------------------------ - -$nameStatus = Invoke-Git @('diff', '--name-status', '--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 } -} +# 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 -------------------------------------------- + + $nameStatus = Invoke-Git @('diff', '--name-status', '--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 -} + 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, deleted, renamed). - $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' } - elseif ($file.Status -like 'R*') { $reasons += "file renamed ($($file.Status))" } - 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: $_" } + # --- 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, deleted, renamed). + $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' } + elseif ($file.Status -like 'R*') { $reasons += "file renamed ($($file.Status))" } + 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: $_" } + 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 } + $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 '; ' }) + $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 ------------------------------------------------------------------ From ce909ff41d068f277c4c707f616c155f13322d60 Mon Sep 17 00:00:00 2001 From: Justin Emerson Date: Mon, 28 Sep 2026 11:38:05 +0000 Subject: [PATCH 04/10] ci: report an informational wire-exemption verdict on every PR test(coverage): pin Test-PfbWireExemption.Tests.ps1 at 40 winps51 skips. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/verify-wire-exemption.yml | 74 +++++++++++++++++++++ Tests/Test-PfbWireExemption.Tests.ps1 | 36 ++++++++++ Tests/coverage-baseline.psd1 | 11 ++- 3 files changed, 120 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/verify-wire-exemption.yml diff --git a/.github/workflows/verify-wire-exemption.yml b/.github/workflows/verify-wire-exemption.yml new file mode 100644 index 0000000..8ac21f6 --- /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@v7 + 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 (this is 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/Tests/Test-PfbWireExemption.Tests.ps1 b/Tests/Test-PfbWireExemption.Tests.ps1 index b73b2b6..4c22fc5 100644 --- a/Tests/Test-PfbWireExemption.Tests.ps1 +++ b/Tests/Test-PfbWireExemption.Tests.ps1 @@ -245,3 +245,39 @@ Describe 'Test-PfbWireExemption is publishable as written' -Skip:($PSVersionTabl (Get-Help $script:checker).Synopsis | Should -Not -Match ([regex]::Escape('Test-PfbWireExemption.ps1')) } } + +Describe 'verify-wire-exemption.yml (informational only)' -Skip:($PSVersionTable.PSVersion.Major -lt 7) { + BeforeAll { + $script:wx = [System.IO.File]::ReadAllText((Join-Path $script:repoRoot '.github/workflows/verify-wire-exemption.yml')) + $script:wxRun = @([regex]::Matches($script:wx, '(?m)^([ \t]+)(?:- )?run: \|[ \t]*\r?\n((?:(?:\1[ \t]+\S[^\r\n]*|[ \t]*)(?:\r?\n|$))+)') | ForEach-Object { $_.Groups[2].Value }) + } + It 'runs on pull_request opened, synchronize and reopened' { + $script:wx | Should -Match '(?m)^ types: \[opened, synchronize, reopened\]\s*$' + } + It 'reads contents only, with no secret' { + $permissions = [regex]::Match($script:wx, '(?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:wx | Should -Not -Match 'secrets\.' + } + It 'checks out full history and runs the BASE revision''s copy of the classifier, never the PR''s' { + $script:wx | Should -Match '(?m)^\s+fetch-depth: 0\s*$' + $script:wx | Should -Match ([regex]::Escape('git show "$($env:BASE_SHA):tools/Test-PfbWireExemption.ps1"')) + $script:wx | Should -Match ([regex]::Escape('-BaseRef $env:BASE_SHA -HeadRef $env:HEAD_SHA')) + $script:wx | Should -Not -Match '\./tools/Test-PfbWireExemption\.ps1' + } + It 'passes the SHAs through env only, and interpolates nothing into a run block' { + $script:wx | Should -Match ([regex]::Escape('BASE_SHA: ${{ github.event.pull_request.base.sha }}')) + $script:wx | Should -Match ([regex]::Escape('HEAD_SHA: ${{ github.event.pull_request.head.sha }}')) + $script:wxRun.Count | Should -BeGreaterThan 0 + @($script:wxRun | Where-Object { $_.Contains('${{') }).Count | Should -Be 0 + } + It 'never fails on the verdict: NotExempt exits 0, Undecided warns, a missing base script is a notice' { + $script:wx | Should -Match '::warning::' + $script:wx | Should -Match '::notice::' + $script:wx | Should -Not -Match '(?m)^\s+continue-on-error:' + @($script:wxRun | Where-Object { $_ -match '(?m)^\s*exit 1\b' }).Count | Should -Be 0 + } + It 'writes an Undecided verdict''s Reason to the job summary' { + @($script:wxRun | Where-Object { $_ -match '(?m)^[^\r\n]*Decision -eq ''Undecided''[^\r\n]*\$lines \+= [^\r\n]*\$verdict\.Reason' }).Count | Should -Be 1 + } +} diff --git a/Tests/coverage-baseline.psd1 b/Tests/coverage-baseline.psd1 index 2657ca8..28d05e2 100644 --- a/Tests/coverage-baseline.psd1 +++ b/Tests/coverage-baseline.psd1 @@ -37,7 +37,7 @@ # the granularity RequiredDescribes can reach), no UNDECLARED file may skip, and a declared # file that stops running at all is a violation rather than a stale entry to delete. # - # Only files that actually skip appear here -- 23 of 217 on 5.1, 2 on pwsh 7 -- so this is + # Only files that actually skip appear here -- 24 of 218 on 5.1, 2 on pwsh 7 -- so this is # a short list, not a per-file census. Attribution is by leaf file name, which is sound # because Tests/ is flat and no two *.Tests.ps1 files share a leaf; the gate fails loudly # if that ever stops being true. @@ -248,6 +248,10 @@ # (tools/Get-PfbActionPinStatus.ps1): Get-PfbActionPinStatus.Tests.ps1 6 -- a new file, # gated wholesale because the script under test is `#Requires -Version 7.0`. # 503 + 6 = 509. Recomputed from the map, not incremented by hand: 23 entries, 509. + # One more entry was then ADDED for the wire-exemption classifier + # (tools/Test-PfbWireExemption.ps1): Test-PfbWireExemption.Tests.ps1 40, a new file + # gated wholesale for the same reason. 509 + 40 = 549. Recomputed from the map, not + # incremented by hand: 24 entries, 549. # # Recompute this total from the map itself rather than adjusting it by the delta in # hand -- an earlier revision of this note said "one entry has moved ... sum to 307", @@ -292,6 +296,11 @@ # because it reads through tools/lib/PfbGitHubRead.ps1. Gated wholesale; measured # on Windows PowerShell 5.1 for this file alone: 0 passed / 0 failed / 6 skipped. 'Get-PfbActionPinStatus.Tests.ps1' = 6 + # The wire-exemption classifier (tools/Test-PfbWireExemption.ps1, `#Requires -Version + # 7.0`). Gated wholesale: every Describe builds scratch git repos and calls the + # PS7-only script, or reads the workflow that runs it. Measured on Windows + # PowerShell 5.1 for this file alone: 0 passed / 0 failed / 40 skipped, container ok. + 'Test-PfbWireExemption.Tests.ps1' = 40 # 15 -> 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`, From f3c9081d2c895a8326a6d3a955fd6344a2d0b5ab Mon Sep 17 00:00:00 2001 From: Justin Emerson Date: Mon, 28 Sep 2026 11:27:27 +0000 Subject: [PATCH 05/10] feat(tools): find issue references a closing keyword does not close Single implementation of the per-issue closing-keyword rule, transliterated from the local hook with JavaScript's \b, \s and $ semantics spelled out so the two agree; parity checked on every case. Case is spelled out as explicit [Xx] classes rather than IgnoreCase: on pwsh 7 .NET IgnoreCase folds the Kelvin sign into K, so a keyword right after one was missed there while Windows PowerShell 5.1 and JavaScript both find it. Co-Authored-By: Claude Opus 5.5 (1M context) --- Tests/Test-PfbClosingKeywords.Tests.ps1 | 112 ++++++++++++++++++ tools/Test-PfbClosingKeywords.ps1 | 146 ++++++++++++++++++++++++ 2 files changed, 258 insertions(+) create mode 100644 Tests/Test-PfbClosingKeywords.Tests.ps1 create mode 100644 tools/Test-PfbClosingKeywords.ps1 diff --git a/Tests/Test-PfbClosingKeywords.Tests.ps1 b/Tests/Test-PfbClosingKeywords.Tests.ps1 new file mode 100644 index 0000000..ea9a943 --- /dev/null +++ b/Tests/Test-PfbClosingKeywords.Tests.ps1 @@ -0,0 +1,112 @@ +#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 + } +} 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") +} From a61e438f3fdacaf4abfd64bc15751768e4f86de3 Mon Sep 17 00:00:00 2001 From: Justin Emerson Date: Mon, 28 Sep 2026 11:36:22 +0000 Subject: [PATCH 06/10] ci: fail a PR whose body or commits leave issues unclosed by one keyword Co-Authored-By: Claude Opus 5.5 --- .github/workflows/verify-closing-keywords.yml | 81 +++++++++++++++++++ Tests/Test-PfbClosingKeywords.Tests.ps1 | 33 ++++++++ 2 files changed, 114 insertions(+) create mode 100644 .github/workflows/verify-closing-keywords.yml 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/Tests/Test-PfbClosingKeywords.Tests.ps1 b/Tests/Test-PfbClosingKeywords.Tests.ps1 index ea9a943..8c6c686 100644 --- a/Tests/Test-PfbClosingKeywords.Tests.ps1 +++ b/Tests/Test-PfbClosingKeywords.Tests.ps1 @@ -110,3 +110,36 @@ Describe 'Test-PfbClosingKeywords: letters are ASCII, as in the JavaScript origi (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 \}' + } +} From 141d31c3690fe6f894b1020fe0dc33e58574961f Mon Sep 17 00:00:00 2001 From: Justin Emerson Date: Mon, 28 Sep 2026 11:43:54 +0000 Subject: [PATCH 07/10] ci: pin the actions in workflows that arrived from main The wire-exemption workflow was written before main required every remote action to be SHA-pinned; pin its checkout to the same v7.0.1 commit the other workflows use. Also word its no-base-copy notice so it does not assume this is the PR that adds the classifier. Co-Authored-By: Claude Opus 5.5 (1M context) --- .github/workflows/verify-wire-exemption.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/verify-wire-exemption.yml b/.github/workflows/verify-wire-exemption.yml index 8ac21f6..b3f5a2d 100644 --- a/.github/workflows/verify-wire-exemption.yml +++ b/.github/workflows/verify-wire-exemption.yml @@ -34,7 +34,7 @@ jobs: timeout-minutes: 10 steps: - name: Check out the repository - uses: actions/checkout@v7 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: fetch-depth: 0 @@ -47,7 +47,7 @@ jobs: $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 (this is the PR that adds it), so there is no trusted copy to run. Skipped.' + 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' From fb04b24f3ba4052fb415e14fca3dfbbd14cbd421 Mon Sep 17 00:00:00 2001 From: Justin Emerson Date: Mon, 28 Sep 2026 11:44:42 +0000 Subject: [PATCH 08/10] docs: add AGENTS.md, a tool-neutral entry point listing the CI checks Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 AGENTS.md diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..bae8008 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,36 @@ +# 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 + +- `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. From 10bc58b79aef6e93df931e929299d5e6e4f68208 Mon Sep 17 00:00:00 2001 From: Justin Emerson Date: Mon, 28 Sep 2026 11:46:43 +0000 Subject: [PATCH 09/10] test(coverage): require the PR-gate rails The closing-keyword Describes and its workflow's Describe are ungated, so both editions require them; the wire-exemption Describes are PS7-gated, so only pwsh 7 does. The header count is recomputed from git ls-files: 24 of 219 files skip on 5.1. Co-Authored-By: Claude Opus 5.5 (1M context) --- Tests/coverage-baseline.psd1 | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/Tests/coverage-baseline.psd1 b/Tests/coverage-baseline.psd1 index 28d05e2..96e358e 100644 --- a/Tests/coverage-baseline.psd1 +++ b/Tests/coverage-baseline.psd1 @@ -37,7 +37,7 @@ # the granularity RequiredDescribes can reach), no UNDECLARED file may skip, and a declared # file that stops running at all is a violation rather than a stale entry to delete. # - # Only files that actually skip appear here -- 24 of 218 on 5.1, 2 on pwsh 7 -- so this is + # Only files that actually skip appear here -- 24 of 219 on 5.1, 2 on pwsh 7 -- so this is # a short list, not a per-file census. Attribution is by leaf file name, which is sound # because Tests/ is flat and no two *.Tests.ps1 files share a leaf; the gate fails loudly # if that ever stops being true. @@ -138,6 +138,17 @@ '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' + # The wire-exemption classifier and its workflow: PS7-gated, so pwsh 7 only. + 'Test-PfbWireExemption exit codes (negative-control pairs)' + 'Test-PfbWireExemption verdict object' + 'Test-PfbWireExemption is publishable as written' + 'verify-wire-exemption.yml (informational only)' ) } winps51 = @{ @@ -390,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' ) } } From 159c755405c8fbce4cd1a09f1b40aaa479f9c2e7 Mon Sep 17 00:00:00 2001 From: Justin Emerson Date: Mon, 28 Sep 2026 12:10:12 +0000 Subject: [PATCH 10/10] fix(wire-exemption): count a file moved out of scope as an in-scope deletion With git's rename detection on, --name-status reports only the new path of a rename, so a cmdlet moved from Public/ to an out-of-scope folder was never seen and the diff read as exempt. Pass --no-renames so a move arrives as a delete plus an add; the rename branch is now unreachable and is removed. A new case moves a cmdlet out of Public/ and expects exit 1 (it fails with the flag removed). test(coverage): Test-PfbWireExemption.Tests.ps1 now skips 41 on 5.1 (total 550). docs: AGENTS.md says the local scripts run under PowerShell 7. Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 2 ++ Tests/Test-PfbWireExemption.Tests.ps1 | 1 + Tests/coverage-baseline.psd1 | 10 +++++----- tools/Test-PfbWireExemption.ps1 | 8 +++++--- 4 files changed, 13 insertions(+), 8 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index bae8008..3c40721 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -28,6 +28,8 @@ Scheduled, not on pull requests: `update-api-capability-map.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 diff --git a/Tests/Test-PfbWireExemption.Tests.ps1 b/Tests/Test-PfbWireExemption.Tests.ps1 index 4c22fc5..ae5274a 100644 --- a/Tests/Test-PfbWireExemption.Tests.ps1 +++ b/Tests/Test-PfbWireExemption.Tests.ps1 @@ -126,6 +126,7 @@ Describe 'Test-PfbWireExemption exit codes (negative-control pairs)' -Skip:($PSV @{ 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 diff --git a/Tests/coverage-baseline.psd1 b/Tests/coverage-baseline.psd1 index 96e358e..1b99b85 100644 --- a/Tests/coverage-baseline.psd1 +++ b/Tests/coverage-baseline.psd1 @@ -260,9 +260,9 @@ # gated wholesale because the script under test is `#Requires -Version 7.0`. # 503 + 6 = 509. Recomputed from the map, not incremented by hand: 23 entries, 509. # One more entry was then ADDED for the wire-exemption classifier - # (tools/Test-PfbWireExemption.ps1): Test-PfbWireExemption.Tests.ps1 40, a new file - # gated wholesale for the same reason. 509 + 40 = 549. Recomputed from the map, not - # incremented by hand: 24 entries, 549. + # (tools/Test-PfbWireExemption.ps1): Test-PfbWireExemption.Tests.ps1 41, a new file + # gated wholesale for the same reason. 509 + 41 = 550. Recomputed from the map, not + # incremented by hand: 24 entries, 550. # # Recompute this total from the map itself rather than adjusting it by the delta in # hand -- an earlier revision of this note said "one entry has moved ... sum to 307", @@ -310,8 +310,8 @@ # The wire-exemption classifier (tools/Test-PfbWireExemption.ps1, `#Requires -Version # 7.0`). Gated wholesale: every Describe builds scratch git repos and calls the # PS7-only script, or reads the workflow that runs it. Measured on Windows - # PowerShell 5.1 for this file alone: 0 passed / 0 failed / 40 skipped, container ok. - 'Test-PfbWireExemption.Tests.ps1' = 40 + # PowerShell 5.1 for this file alone: 0 passed / 0 failed / 41 skipped, container ok. + 'Test-PfbWireExemption.Tests.ps1' = 41 # 15 -> 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`, diff --git a/tools/Test-PfbWireExemption.ps1 b/tools/Test-PfbWireExemption.ps1 index 4c3c0b5..6a0d27c 100644 --- a/tools/Test-PfbWireExemption.ps1 +++ b/tools/Test-PfbWireExemption.ps1 @@ -223,7 +223,10 @@ Write-Host "" try { # --- collect in-scope changes -------------------------------------------- - $nameStatus = Invoke-Git @('diff', '--name-status', '--no-color', "$mergeBase..$head") + # --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) { @@ -258,13 +261,12 @@ try { $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, deleted, renamed). + # 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' } - elseif ($file.Status -like 'R*') { $reasons += "file renamed ($($file.Status))" } else { $changed = Get-ChangedLine -Path $path -Base $mergeBase -Head $head