From 48b4ca9c72121534a7ddf59fef6e4ac573c67b97 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 25 Sep 2026 15:30:17 +0000 Subject: [PATCH] Stop the shared ci.yml template filtering paths on pull_request The canonical ci.yml in docs/shared-ci.md carried paths-ignore on both push and pull_request. The pull_request half is the defect #5 is about: a filtered trigger reports no check at all rather than a neutral one, so any ruleset requiring "Build, Test & Release" blocks a docs-only pull request permanently, including pull requests editing DESCRIPTION.md and TAGS.md. This is why #5's sweep did not stick. cf13319 removed the filter across 41 repositories on 2026-08-23; a4cec36 put it back three days later as a side effect of adopting the unified workflow, and 53bf50b re-broadcast it on 2026-09-14 as a file described as byte-identical in every repository. Measured against current main: all 28 repositories sampled still carry paths-ignore under pull_request, including the ones the issue counts as already swept. Sweeping the repositories without fixing the template they are regenerated from would be reverted a third time, so the template is the place to change. The push filter stays. Release gating runs off KtsuBuild's should_release rather than the event type, so a docs-only push to main would otherwise cut a version. That asymmetry is now stated in the document and asserted by a test, since symmetry is the obvious tidy-up and is what broke it twice. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012PwjCMZsvWpvHQhPu5kvBX --- docs/shared-ci.md | 21 +++- scripts/tests/shared-ci-template.tests.ps1 | 132 +++++++++++++++++++++ 2 files changed, 151 insertions(+), 2 deletions(-) create mode 100644 scripts/tests/shared-ci-template.tests.ps1 diff --git a/docs/shared-ci.md b/docs/shared-ci.md index 5bd44c0..9eff08a 100644 --- a/docs/shared-ci.md +++ b/docs/shared-ci.md @@ -77,8 +77,6 @@ on: paths-ignore: ["**.md", ".github/ISSUE_TEMPLATE/**", ".github/pull_request_template.md"] pull_request: - paths-ignore: - ["**.md", ".github/ISSUE_TEMPLATE/**", ".github/pull_request_template.md"] schedule: - cron: "0 23 * * *" # Daily at 11 PM UTC workflow_dispatch: @@ -120,6 +118,25 @@ Triggers, path filters and concurrency live here because a reusable workflow can triggers, and a concurrency group inside one would contend with the caller waiting on it — `github.workflow` resolves to the caller on both sides, so the group string would collide. +### Why only `push` filters paths + +The asymmetry is deliberate and is the one part of this block that must not be "tidied up" +into symmetry. `scripts/tests/shared-ci-template.tests.ps1` asserts both halves of it. + +`push` keeps its `paths-ignore` because release gating runs off KtsuBuild's `should_release` +rather than the event type, so a docs-only push to `main` would otherwise cut a version. + +`pull_request` must not have one. A filtered `pull_request` trigger does not report a +neutral check — it reports *nothing*, so any ruleset requiring **Build, Test & Release** +blocks a docs-only pull request permanently, with no check to wait on and nothing to +override. That also catches pull requests editing `DESCRIPTION.md` and `TAGS.md`, which the +Terraform workspace derives repository metadata from. + +This has already been reverted twice by rollouts that regenerated the trigger block from a +symmetric template — `cf13319` removed the filter across 41 repositories on 2026-08-23, and +`a4cec36` put it back three days later as a side effect of adopting the unified workflow. +See ktsu-dev/.github#5. + ## The `release` tag Callers reference `@release`, a tag that is moved rather than a version that has to be diff --git a/scripts/tests/shared-ci-template.tests.ps1 b/scripts/tests/shared-ci-template.tests.ps1 new file mode 100644 index 0000000..2c5e0bd --- /dev/null +++ b/scripts/tests/shared-ci-template.tests.ps1 @@ -0,0 +1,132 @@ +<# +.SYNOPSIS + Checks the canonical ci.yml in docs/shared-ci.md against the regression it has already had. + +.DESCRIPTION + docs/shared-ci.md carries the `ci.yml` every repository holds byte-identical. Its trigger + block has a deliberate asymmetry -- `push` filters paths, `pull_request` does not -- and + that asymmetry has been silently undone twice by rollouts that regenerated the block from + a symmetric template: + + cf13319 (2026-08-23) removed paths-ignore from pull_request across 41 repositories + a4cec36 (2026-08-26) put it back, as a side effect of adopting the unified workflow + + A filtered `pull_request` trigger reports no check at all rather than a neutral one, so a + ruleset requiring "Build, Test & Release" blocks every docs-only pull request permanently. + See ktsu-dev/.github#5. + + Run with pwsh: ./scripts/tests/shared-ci-template.tests.ps1 +#> +[CmdletBinding()] +param() + +Set-StrictMode -Version Latest +$ErrorActionPreference = 'Stop' + +$script:Doc = Join-Path $PSScriptRoot '..' '..' 'docs' 'shared-ci.md' | Resolve-Path +$script:Failures = 0 +$script:Count = 0 + +function Test-Case { + param([string]$Name, [scriptblock]$Assertion) + + $script:Count++ + $problem = & $Assertion + if ($problem) { + $script:Failures++ + Write-Host "FAIL $Name" -ForegroundColor Red + Write-Host " $problem" -ForegroundColor Red + return + } + + Write-Host "ok $Name" -ForegroundColor Green +} + +# The block is located by the sentence that introduces it rather than by being the first +# fenced block in the file, so that adding an example earlier in the document does not +# silently point these assertions at the wrong YAML. +function Get-CallerWorkflow { + $lines = Get-Content -LiteralPath $script:Doc + $anchor = $lines.IndexOf(($lines | Where-Object { $_ -match '^Each repository holds this at .*ci\.yml' } | Select-Object -First 1)) + if ($anchor -lt 0) { + throw "The sentence introducing the canonical ci.yml is no longer in $script:Doc, so this test cannot find the block it guards." + } + + $start = -1 + for ($i = $anchor; $i -lt $lines.Count; $i++) { + if ($lines[$i] -eq '```yaml') { $start = $i + 1; break } + } + if ($start -lt 0) { throw 'No fenced yaml block follows the introducing sentence.' } + + $body = @() + for ($i = $start; $i -lt $lines.Count; $i++) { + if ($lines[$i] -eq '```') { return , $body } + $body += $lines[$i] + } + throw 'The fenced yaml block is not closed.' +} + +# Returns the lines nested under a given trigger inside the top-level `on:` mapping. +function Get-TriggerBody { + param([string[]]$Workflow, [string]$Trigger) + + $inOn = $false + $body = @() + $collecting = $false + + foreach ($line in $Workflow) { + if ($line -match '^on:\s*$') { $inOn = $true; continue } + if (-not $inOn) { continue } + + # Any column-zero content ends the `on:` mapping. + if ($line -match '^\S') { break } + + if ($line -match "^ $([regex]::Escape($Trigger)):\s*$") { $collecting = $true; continue } + + # A sibling trigger at the same indent ends this one. + if ($collecting -and $line -match '^ \S') { break } + + if ($collecting -and $line.Trim()) { $body += $line } + } + + return , $body +} + +$workflow = Get-CallerWorkflow + +# The headline requirement, and the half that has regressed twice. A pull_request trigger +# carrying paths-ignore makes a docs-only PR unmergeable under a ruleset that requires the +# build check, because a skipped trigger reports nothing for the ruleset to wait on. +Test-Case -Name 'pull_request does not filter paths' -Assertion { + $body = Get-TriggerBody -Workflow $workflow -Trigger 'pull_request' + if ($body | Where-Object { $_ -match 'paths-ignore' }) { + return "pull_request carries paths-ignore:`n $($body -join "`n ")" + } +} + +# The other half. Removing this one is the obvious over-correction, and it is wrong: release +# gating runs off KtsuBuild's should_release rather than the event type, so a docs-only push +# to main would cut a version. +Test-Case -Name 'push still filters paths' -Assertion { + $body = Get-TriggerBody -Workflow $workflow -Trigger 'push' + if (-not ($body | Where-Object { $_ -match 'paths-ignore' })) { + return "push has lost its paths-ignore:`n $($body -join "`n ")" + } +} + +# Both triggers must still exist. A rewrite that drops `pull_request` altogether would pass +# the first assertion for the wrong reason. +Test-Case -Name 'both triggers are still declared' -Assertion { + $missing = @('push', 'pull_request') | Where-Object { + -not ($workflow | Where-Object { $_ -match "^ $_`:" }) + } + if ($missing) { return "missing trigger(s): $($missing -join ', ')" } +} + +Write-Host '' +if ($script:Failures -gt 0) { + Write-Host "$script:Failures of $script:Count case(s) failed." -ForegroundColor Red + exit 1 +} + +Write-Host "All $script:Count case(s) passed." -ForegroundColor Green