From e2d7e357930f1297ffea1c235b1340a216f9f190 Mon Sep 17 00:00:00 2001 From: Sam Erde <20478745+SamErde@users.noreply.github.com> Date: Mon, 1 Jun 2026 09:27:24 -0400 Subject: [PATCH 1/5] ci: make Build Module + Upstream-Compatibility gates always-report for required checks Prereq for adding required status checks. Both workflows were path-filtered, so a required check from them would stay Pending (and block) on PRs that don't trigger the workflow (docs/CI-only). Remove the pull_request path filters and gate the heavy jobs internally: - A lightweight changes/pr-changes job (gh pr view --json files) detects whether build/tooling-relevant paths changed. - The build matrix / pr-smoke job runs when relevant (real result) and is skipped otherwise (a skipped job satisfies a required check, so non-bundle PRs are never blocked). - Build Module is reused by Release via workflow_call: the build always builds when inputs.ref is set (release) or on workflow_dispatch, and errs toward building on any ambiguity, so the release path is never accidentally skipped. Chose this over a parallel status-shim workflow, which can mask a failing build on a mixed-path PR (same context reported twice). Co-Authored-By: Claude Opus 4.8 --- .github/workflows/Build Module.yml | 51 +++++++++++++++++--- .github/workflows/Upstream-Compatibility.yml | 43 ++++++++++++++--- 2 files changed, 80 insertions(+), 14 deletions(-) diff --git a/.github/workflows/Build Module.yml b/.github/workflows/Build Module.yml index 75f5de3..22843b1 100644 --- a/.github/workflows/Build Module.yml +++ b/.github/workflows/Build Module.yml @@ -18,13 +18,12 @@ on: required: false workflow_dispatch: pull_request: - paths: - - "src/**" - - "build/**" - - "tests/**" - - "tools/**" - - ".github/ci-scripts/**" - - ".github/workflows/Build Module.yml" + # No path filter: this workflow always triggers on PRs to main so its "Build and test module" + # checks always REPORT (a required status check from a path-filtered workflow that never triggers + # stays Pending and blocks the PR forever). The expensive matrix build is gated below by the + # `changes` job — on a PR that doesn't touch build-relevant paths the build job is skipped, and a + # skipped job satisfies the required check, so docs-/CI-only PRs are never blocked. + branches: [main] #push: # branches: # - main @@ -42,9 +41,47 @@ concurrency: jobs: + changes: + name: Detect build-relevant changes + # Only meaningful for a directly-triggered PR. workflow_call (release) passes inputs.ref and must + # always build; workflow_dispatch must always build. Both skip this job (handled in build's `if`). + if: github.event_name == 'pull_request' && inputs.ref == '' + runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: read + outputs: + relevant: ${{ steps.detect.outputs.relevant }} + steps: + - name: Detect whether build-relevant paths changed + id: detect + shell: pwsh + env: + GH_TOKEN: ${{ github.token }} + run: | + $ErrorActionPreference = 'Stop' + $Files = gh pr view ${{ github.event.pull_request.number }} --repo ${{ github.repository }} --json files --jq '.files[].path' + $Patterns = @('^src/', '^build/', '^tests/', '^tools/', '^\.github/ci-scripts/', '^\.github/workflows/Build Module\.yml$') + $Relevant = $false + foreach ($File in $Files) { + foreach ($Pattern in $Patterns) { + if ($File -match $Pattern) { $Relevant = $true; break } + } + if ($Relevant) { break } + } + "relevant=$($Relevant.ToString().ToLowerInvariant())" | Out-File -FilePath $env:GITHUB_OUTPUT -Append -Encoding utf8 + Write-Host "Build-relevant change detected: $Relevant" + build: name: Build and test module (${{ matrix.os }}) + needs: changes + # Always build for workflow_call (release; inputs.ref set) and workflow_dispatch. For a direct PR, + # build only when build-relevant paths changed; otherwise skip (a skipped job still satisfies the + # required "Build and test module" check). always() stops the skip of `changes` in the + # call/dispatch paths from cascading into a skip of build; the `!= 'false'` guard errs toward + # building on any ambiguity, so the release path is never accidentally skipped. + if: ${{ always() && (inputs.ref != '' || github.event_name == 'workflow_dispatch' || needs.changes.outputs.relevant != 'false') }} runs-on: ${{ matrix.os }} strategy: fail-fast: false diff --git a/.github/workflows/Upstream-Compatibility.yml b/.github/workflows/Upstream-Compatibility.yml index ddc7026..a9bebea 100644 --- a/.github/workflows/Upstream-Compatibility.yml +++ b/.github/workflows/Upstream-Compatibility.yml @@ -11,12 +11,10 @@ on: schedule: - cron: "34 8 * * 1" pull_request: - paths: - - "build/dependency-policy.json" - - "src/DLLPickle.Build/**" - - "tests/**" - - "tools/**" - - ".github/workflows/Upstream-Compatibility.yml" + # No path filter: always trigger on PRs to main so the "Validate upstream compatibility tooling" + # required check always reports. The job is gated below by `pr-changes` — skipped (== passing + # required check) on PRs that don't touch the tooling/policy paths. + branches: [main] permissions: contents: read @@ -26,9 +24,40 @@ concurrency: cancel-in-progress: false jobs: + pr-changes: + name: Detect tooling/policy changes + if: github.event_name == 'pull_request' + runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: read + outputs: + relevant: ${{ steps.detect.outputs.relevant }} + steps: + - name: Detect whether upstream-compatibility paths changed + id: detect + shell: pwsh + env: + GH_TOKEN: ${{ github.token }} + run: | + $ErrorActionPreference = 'Stop' + $Files = gh pr view ${{ github.event.pull_request.number }} --repo ${{ github.repository }} --json files --jq '.files[].path' + $Patterns = @('^build/', '^src/DLLPickle\.Build/', '^tests/', '^tools/', '^\.github/workflows/Upstream-Compatibility\.yml$') + $Relevant = $false + foreach ($File in $Files) { + foreach ($Pattern in $Patterns) { + if ($File -match $Pattern) { $Relevant = $true; break } + } + if ($Relevant) { break } + } + "relevant=$($Relevant.ToString().ToLowerInvariant())" | Out-File -FilePath $env:GITHUB_OUTPUT -Append -Encoding utf8 + Write-Host "Upstream-compatibility-relevant change detected: $Relevant" + pr-smoke-validation: name: Validate upstream compatibility tooling - if: github.event_name == 'pull_request' + needs: pr-changes + # Runs on PRs that touch the tooling/policy paths; skipped (== passing required check) otherwise. + if: github.event_name == 'pull_request' && needs.pr-changes.outputs.relevant == 'true' runs-on: windows-2025 permissions: contents: read From b68781517a6c370c0332f7bcf7bd00c6b97b28ee Mon Sep 17 00:00:00 2001 From: Sam Erde <20478745+SamErde@users.noreply.github.com> Date: Mon, 1 Jun 2026 09:43:40 -0400 Subject: [PATCH 2/5] ci: harden required-check gates per Codex/Copilot review on #228 - P1 (Codex): a matrix job skipped via job-level if never creates its per-OS check contexts (if is evaluated before matrix expansion), so requiring "Build and test module (os)" would leave non-bundle PRs stuck waiting. Add an always-running aggregate "Build gate" job as the stable required-check target (passes when the matrix build succeeded or was skipped). - P2 (Codex): gh pr view --json files caps at 100 files; a later src/ change on a large PR could be missed. Switch both detectors to the paginated REST files endpoint (gh api .../files --paginate). - Copilot + Codex: make detection fail-safe. The detect step now defaults to relevant=true on any error, and Upstream-Compatibility's pr-smoke job uses always() + relevant != 'false' so a detector failure runs the validation rather than skipping a required check on a relevant PR. Co-Authored-By: Claude Opus 4.8 --- .github/workflows/Build Module.yml | 49 ++++++++++++++++---- .github/workflows/Upstream-Compatibility.yml | 32 +++++++++---- 2 files changed, 63 insertions(+), 18 deletions(-) diff --git a/.github/workflows/Build Module.yml b/.github/workflows/Build Module.yml index 22843b1..7aba37a 100644 --- a/.github/workflows/Build Module.yml +++ b/.github/workflows/Build Module.yml @@ -59,15 +59,24 @@ jobs: env: GH_TOKEN: ${{ github.token }} run: | - $ErrorActionPreference = 'Stop' - $Files = gh pr view ${{ github.event.pull_request.number }} --repo ${{ github.repository }} --json files --jq '.files[].path' - $Patterns = @('^src/', '^build/', '^tests/', '^tools/', '^\.github/ci-scripts/', '^\.github/workflows/Build Module\.yml$') - $Relevant = $false - foreach ($File in $Files) { - foreach ($Pattern in $Patterns) { - if ($File -match $Pattern) { $Relevant = $true; break } + # Fail-safe: default to relevant=true so any error enumerating changed files runs the build + # rather than silently skipping it. Use the PAGINATED REST files endpoint -- gh pr view + # --json files caps at 100 files and could miss a later src/ change on a large PR. + $Relevant = $true + try { + $ErrorActionPreference = 'Stop' + $Files = gh api "repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/files" --paginate --jq '.[].filename' + $Patterns = @('^src/', '^build/', '^tests/', '^tools/', '^\.github/ci-scripts/', '^\.github/workflows/Build Module\.yml$') + $Relevant = $false + foreach ($File in $Files) { + foreach ($Pattern in $Patterns) { + if ($File -match $Pattern) { $Relevant = $true; break } + } + if ($Relevant) { break } } - if ($Relevant) { break } + } catch { + Write-Host "::warning::Changed-file detection failed; defaulting to relevant=true (fail-safe). $_" + $Relevant = $true } "relevant=$($Relevant.ToString().ToLowerInvariant())" | Out-File -FilePath $env:GITHUB_OUTPUT -Append -Encoding utf8 Write-Host "Build-relevant change detected: $Relevant" @@ -215,3 +224,27 @@ jobs: name: module-build path: ./module/DLLPickle if-no-files-found: warn + + build-gate: + name: Build gate + # Aggregate required-check target. GitHub evaluates a matrix job's `if` BEFORE expanding the + # matrix, so skipping the `build` job does NOT create the per-OS "Build and test module (...)" + # check contexts -- requiring those directly would leave non-bundle PRs stuck "Expected - waiting + # for status". This single, always-running job is the stable check to require instead: it passes + # when the matrix build succeeded OR was skipped (no build-relevant changes) and fails when the + # build failed or was cancelled. + needs: build + if: ${{ always() }} + runs-on: ubuntu-latest + steps: + - name: Aggregate build result + shell: pwsh + run: | + $Result = '${{ needs.build.result }}' + Write-Host "build job result: $Result" + if ($Result -eq 'success' -or $Result -eq 'skipped') { + Write-Host 'Build gate passed.' + exit 0 + } + Write-Host "::error::Build gate failed (build result: $Result)." + exit 1 diff --git a/.github/workflows/Upstream-Compatibility.yml b/.github/workflows/Upstream-Compatibility.yml index a9bebea..12a8c4c 100644 --- a/.github/workflows/Upstream-Compatibility.yml +++ b/.github/workflows/Upstream-Compatibility.yml @@ -40,15 +40,24 @@ jobs: env: GH_TOKEN: ${{ github.token }} run: | - $ErrorActionPreference = 'Stop' - $Files = gh pr view ${{ github.event.pull_request.number }} --repo ${{ github.repository }} --json files --jq '.files[].path' - $Patterns = @('^build/', '^src/DLLPickle\.Build/', '^tests/', '^tools/', '^\.github/workflows/Upstream-Compatibility\.yml$') - $Relevant = $false - foreach ($File in $Files) { - foreach ($Pattern in $Patterns) { - if ($File -match $Pattern) { $Relevant = $true; break } + # Fail-safe: default relevant=true so any error enumerating changed files runs the + # validation rather than silently skipping a required check. Paginated REST files endpoint + # (gh pr view --json files caps at 100 files and could miss a later build/ or tools/ change). + $Relevant = $true + try { + $ErrorActionPreference = 'Stop' + $Files = gh api "repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/files" --paginate --jq '.[].filename' + $Patterns = @('^build/', '^src/DLLPickle\.Build/', '^tests/', '^tools/', '^\.github/workflows/Upstream-Compatibility\.yml$') + $Relevant = $false + foreach ($File in $Files) { + foreach ($Pattern in $Patterns) { + if ($File -match $Pattern) { $Relevant = $true; break } + } + if ($Relevant) { break } } - if ($Relevant) { break } + } catch { + Write-Host "::warning::Changed-file detection failed; defaulting to relevant=true (fail-safe). $_" + $Relevant = $true } "relevant=$($Relevant.ToString().ToLowerInvariant())" | Out-File -FilePath $env:GITHUB_OUTPUT -Append -Encoding utf8 Write-Host "Upstream-compatibility-relevant change detected: $Relevant" @@ -56,8 +65,11 @@ jobs: pr-smoke-validation: name: Validate upstream compatibility tooling needs: pr-changes - # Runs on PRs that touch the tooling/policy paths; skipped (== passing required check) otherwise. - if: github.event_name == 'pull_request' && needs.pr-changes.outputs.relevant == 'true' + # Runs on PRs that touch the tooling/policy paths; skipped (== passing required check) only when + # detection conclusively says false. always() + `!= 'false'` makes it FAIL-SAFE: if pr-changes + # fails or its output is unknown, the validation still runs rather than being skipped (which would + # otherwise satisfy the required check without actually validating a relevant PR). + if: ${{ always() && github.event_name == 'pull_request' && needs.pr-changes.outputs.relevant != 'false' }} runs-on: windows-2025 permissions: contents: read From 573ebd54a29cd451c58de899ccc5fb071cd62a61 Mon Sep 17 00:00:00 2001 From: Sam Erde <20478745+SamErde@users.noreply.github.com> Date: Mon, 1 Jun 2026 09:52:05 -0400 Subject: [PATCH 3/5] ci: make change-detection fail-safe on native gh failure + handle renames Second Codex pass on #228: - Check $LASTEXITCODE after the gh api call: pwsh does not reliably throw on a native non-zero exit, so a transient/permission failure that emitted no output could otherwise write relevant=false and skip a required check. Now it throws into the catch (relevant=true, fail-safe). - Project .previous_filename alongside .filename so a rename that moves a tracked file OUT of a matched tree is still detected as relevant. Co-Authored-By: Claude Opus 4.8 --- .github/workflows/Build Module.yml | 6 +++++- .github/workflows/Upstream-Compatibility.yml | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/.github/workflows/Build Module.yml b/.github/workflows/Build Module.yml index 7aba37a..8e6bf33 100644 --- a/.github/workflows/Build Module.yml +++ b/.github/workflows/Build Module.yml @@ -65,7 +65,11 @@ jobs: $Relevant = $true try { $ErrorActionPreference = 'Stop' - $Files = gh api "repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/files" --paginate --jq '.[].filename' + # Project previous_filename too, so a rename moving a tracked file OUT of a matched tree + # (reported as the new path in .filename, old path in .previous_filename) is still seen. + $Files = gh api "repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/files" --paginate --jq '.[] | .filename, (.previous_filename // empty)' + # Native non-zero exits don't reliably throw in pwsh, so check explicitly -> fail-safe. + if ($LASTEXITCODE -ne 0) { throw "gh api exited $LASTEXITCODE while listing PR files" } $Patterns = @('^src/', '^build/', '^tests/', '^tools/', '^\.github/ci-scripts/', '^\.github/workflows/Build Module\.yml$') $Relevant = $false foreach ($File in $Files) { diff --git a/.github/workflows/Upstream-Compatibility.yml b/.github/workflows/Upstream-Compatibility.yml index 12a8c4c..0fd301f 100644 --- a/.github/workflows/Upstream-Compatibility.yml +++ b/.github/workflows/Upstream-Compatibility.yml @@ -46,7 +46,11 @@ jobs: $Relevant = $true try { $ErrorActionPreference = 'Stop' - $Files = gh api "repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/files" --paginate --jq '.[].filename' + # Project previous_filename too, so a rename moving a tracked file OUT of a matched tree + # (reported as the new path in .filename, old path in .previous_filename) is still seen. + $Files = gh api "repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/files" --paginate --jq '.[] | .filename, (.previous_filename // empty)' + # Native non-zero exits don't reliably throw in pwsh, so check explicitly -> fail-safe. + if ($LASTEXITCODE -ne 0) { throw "gh api exited $LASTEXITCODE while listing PR files" } $Patterns = @('^build/', '^src/DLLPickle\.Build/', '^tests/', '^tools/', '^\.github/workflows/Upstream-Compatibility\.yml$') $Relevant = $false foreach ($File in $Files) { From 1a8ce011e1403a34595cd1084d73b57202534f06 Mon Sep 17 00:00:00 2001 From: Sam Erde <20478745+SamErde@users.noreply.github.com> Date: Mon, 1 Jun 2026 10:02:11 -0400 Subject: [PATCH 4/5] ci: extract change-detection into a shared ci-script (dedupe merge-gate logic) Copilot review on #228: the PR-file detection was duplicated in Build Module and Upstream-Compatibility, and it is merge-gate-critical (pagination, rename handling, fail-safe) so the copies could drift. Extract the now-final logic into .github/ci-scripts/Get-DLLPickleChangedPathRelevance.ps1 (parameterized by the regex pattern set) and call it from both detection jobs (each now checks out the repo). A scoped, justified PSAvoidUsingWriteHost suppression documents that the script writes Actions log/annotation lines to stdout by design. Co-Authored-By: Claude Opus 4.8 --- .../Get-DLLPickleChangedPathRelevance.ps1 | 67 +++++++++++++++++++ .github/workflows/Build Module.yml | 31 ++------- .github/workflows/Upstream-Compatibility.yml | 31 ++------- 3 files changed, 79 insertions(+), 50 deletions(-) create mode 100644 .github/ci-scripts/Get-DLLPickleChangedPathRelevance.ps1 diff --git a/.github/ci-scripts/Get-DLLPickleChangedPathRelevance.ps1 b/.github/ci-scripts/Get-DLLPickleChangedPathRelevance.ps1 new file mode 100644 index 0000000..46a5c47 --- /dev/null +++ b/.github/ci-scripts/Get-DLLPickleChangedPathRelevance.ps1 @@ -0,0 +1,67 @@ +<# +.SYNOPSIS + Reports whether a pull request changed any path relevant to a CI gate, writing + "relevant=true|false" to $env:GITHUB_OUTPUT. +.DESCRIPTION + Single source of the merge-gate change-detection used by the always-triggered gate workflows + (Build Module, Upstream-Compatibility) so the required-check skip logic cannot drift between them. + + Enumerates the PR's changed files via the PAGINATED REST endpoint (gh pr view --json files caps at + 100 files) and includes previous_filename so a rename that moves a tracked file OUT of a matched + tree is still detected. Matching is case-sensitive regex against each path. + + Fail-safe: any error enumerating files - including a native gh non-zero exit, which pwsh does not + reliably surface as a terminating error - defaults to relevant=true, so a required-check gate runs + its validation rather than silently skipping on a transient/permission failure. +.PARAMETER Repository + The owner/repo slug (e.g. SamErde/DLLPickle). +.PARAMETER PullNumber + The pull request number. +.PARAMETER Pattern + One or more regex patterns matched against each changed (and previous) file path. +#> +[Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSAvoidUsingWriteHost', '', Justification = 'CI script: emits GitHub Actions log lines and ::warning:: annotations to stdout by design.')] +[CmdletBinding()] +param( + [Parameter(Mandatory)] + [ValidateNotNullOrEmpty()] + [string]$Repository, + + [Parameter(Mandatory)] + [int]$PullNumber, + + [Parameter(Mandatory)] + [ValidateNotNullOrEmpty()] + [string[]]$Pattern +) + +$Relevant = $true +try { + $ErrorActionPreference = 'Stop' + $Files = gh api "repos/$Repository/pulls/$PullNumber/files" --paginate --jq '.[] | .filename, (.previous_filename // empty)' + if ($LASTEXITCODE -ne 0) { + throw "gh api exited $LASTEXITCODE while listing PR files" + } + + $Relevant = $false + foreach ($File in $Files) { + foreach ($Expression in $Pattern) { + if ($File -match $Expression) { + $Relevant = $true + break + } + } + if ($Relevant) { + break + } + } +} catch { + Write-Host "::warning::Changed-file detection failed; defaulting to relevant=true (fail-safe). $_" + $Relevant = $true +} + +$RelevantValue = $Relevant.ToString().ToLowerInvariant() +if ($env:GITHUB_OUTPUT) { + "relevant=$RelevantValue" | Out-File -FilePath $env:GITHUB_OUTPUT -Append -Encoding utf8 +} +Write-Host "Change relevance: $RelevantValue" diff --git a/.github/workflows/Build Module.yml b/.github/workflows/Build Module.yml index 8e6bf33..f28e77c 100644 --- a/.github/workflows/Build Module.yml +++ b/.github/workflows/Build Module.yml @@ -53,37 +53,18 @@ jobs: outputs: relevant: ${{ steps.detect.outputs.relevant }} steps: + - name: Check out repository + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - name: Detect whether build-relevant paths changed id: detect shell: pwsh env: GH_TOKEN: ${{ github.token }} run: | - # Fail-safe: default to relevant=true so any error enumerating changed files runs the build - # rather than silently skipping it. Use the PAGINATED REST files endpoint -- gh pr view - # --json files caps at 100 files and could miss a later src/ change on a large PR. - $Relevant = $true - try { - $ErrorActionPreference = 'Stop' - # Project previous_filename too, so a rename moving a tracked file OUT of a matched tree - # (reported as the new path in .filename, old path in .previous_filename) is still seen. - $Files = gh api "repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/files" --paginate --jq '.[] | .filename, (.previous_filename // empty)' - # Native non-zero exits don't reliably throw in pwsh, so check explicitly -> fail-safe. - if ($LASTEXITCODE -ne 0) { throw "gh api exited $LASTEXITCODE while listing PR files" } - $Patterns = @('^src/', '^build/', '^tests/', '^tools/', '^\.github/ci-scripts/', '^\.github/workflows/Build Module\.yml$') - $Relevant = $false - foreach ($File in $Files) { - foreach ($Pattern in $Patterns) { - if ($File -match $Pattern) { $Relevant = $true; break } - } - if ($Relevant) { break } - } - } catch { - Write-Host "::warning::Changed-file detection failed; defaulting to relevant=true (fail-safe). $_" - $Relevant = $true - } - "relevant=$($Relevant.ToString().ToLowerInvariant())" | Out-File -FilePath $env:GITHUB_OUTPUT -Append -Encoding utf8 - Write-Host "Build-relevant change detected: $Relevant" + ./.github/ci-scripts/Get-DLLPickleChangedPathRelevance.ps1 ` + -Repository '${{ github.repository }}' ` + -PullNumber ${{ github.event.pull_request.number }} ` + -Pattern '^src/', '^build/', '^tests/', '^tools/', '^\.github/ci-scripts/', '^\.github/workflows/Build Module\.yml$' build: diff --git a/.github/workflows/Upstream-Compatibility.yml b/.github/workflows/Upstream-Compatibility.yml index 0fd301f..bf069ca 100644 --- a/.github/workflows/Upstream-Compatibility.yml +++ b/.github/workflows/Upstream-Compatibility.yml @@ -34,37 +34,18 @@ jobs: outputs: relevant: ${{ steps.detect.outputs.relevant }} steps: + - name: Check out repository + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - name: Detect whether upstream-compatibility paths changed id: detect shell: pwsh env: GH_TOKEN: ${{ github.token }} run: | - # Fail-safe: default relevant=true so any error enumerating changed files runs the - # validation rather than silently skipping a required check. Paginated REST files endpoint - # (gh pr view --json files caps at 100 files and could miss a later build/ or tools/ change). - $Relevant = $true - try { - $ErrorActionPreference = 'Stop' - # Project previous_filename too, so a rename moving a tracked file OUT of a matched tree - # (reported as the new path in .filename, old path in .previous_filename) is still seen. - $Files = gh api "repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/files" --paginate --jq '.[] | .filename, (.previous_filename // empty)' - # Native non-zero exits don't reliably throw in pwsh, so check explicitly -> fail-safe. - if ($LASTEXITCODE -ne 0) { throw "gh api exited $LASTEXITCODE while listing PR files" } - $Patterns = @('^build/', '^src/DLLPickle\.Build/', '^tests/', '^tools/', '^\.github/workflows/Upstream-Compatibility\.yml$') - $Relevant = $false - foreach ($File in $Files) { - foreach ($Pattern in $Patterns) { - if ($File -match $Pattern) { $Relevant = $true; break } - } - if ($Relevant) { break } - } - } catch { - Write-Host "::warning::Changed-file detection failed; defaulting to relevant=true (fail-safe). $_" - $Relevant = $true - } - "relevant=$($Relevant.ToString().ToLowerInvariant())" | Out-File -FilePath $env:GITHUB_OUTPUT -Append -Encoding utf8 - Write-Host "Upstream-compatibility-relevant change detected: $Relevant" + ./.github/ci-scripts/Get-DLLPickleChangedPathRelevance.ps1 ` + -Repository '${{ github.repository }}' ` + -PullNumber ${{ github.event.pull_request.number }} ` + -Pattern '^build/', '^src/DLLPickle\.Build/', '^tests/', '^tools/', '^\.github/workflows/Upstream-Compatibility\.yml$' pr-smoke-validation: name: Validate upstream compatibility tooling From a922be638ba48cdc0f7d412e306bc58c04abbf7b Mon Sep 17 00:00:00 2001 From: Sam Erde <20478745+SamErde@users.noreply.github.com> Date: Mon, 1 Jun 2026 10:12:27 -0400 Subject: [PATCH 5/5] ci: keep change-detection inline (revert shared-script extraction) Codex flagged a P1 on the extracted .github/ci-scripts detector: pull_request runs the PR's checked-out copy, so a PR could modify the shared gate script to emit relevant=false while changing src/, skipping the build while Build gate passes. Reverting to inline detection keeps the merge-gate logic in the workflow file, which benefits from GitHub's stricter workflow-edit review (and a related P2: the shared-script path wasn't in the upstream relevance set). The small duplication Copilot flagged is accepted as the security tradeoff; a note in each detector documents that it is inline on purpose. This reverts commit 1a8ce01 and adds the explanatory note. Co-Authored-By: Claude Opus 4.8 --- .../Get-DLLPickleChangedPathRelevance.ps1 | 67 ------------------- .github/workflows/Build Module.yml | 34 ++++++++-- .github/workflows/Upstream-Compatibility.yml | 34 ++++++++-- 3 files changed, 56 insertions(+), 79 deletions(-) delete mode 100644 .github/ci-scripts/Get-DLLPickleChangedPathRelevance.ps1 diff --git a/.github/ci-scripts/Get-DLLPickleChangedPathRelevance.ps1 b/.github/ci-scripts/Get-DLLPickleChangedPathRelevance.ps1 deleted file mode 100644 index 46a5c47..0000000 --- a/.github/ci-scripts/Get-DLLPickleChangedPathRelevance.ps1 +++ /dev/null @@ -1,67 +0,0 @@ -<# -.SYNOPSIS - Reports whether a pull request changed any path relevant to a CI gate, writing - "relevant=true|false" to $env:GITHUB_OUTPUT. -.DESCRIPTION - Single source of the merge-gate change-detection used by the always-triggered gate workflows - (Build Module, Upstream-Compatibility) so the required-check skip logic cannot drift between them. - - Enumerates the PR's changed files via the PAGINATED REST endpoint (gh pr view --json files caps at - 100 files) and includes previous_filename so a rename that moves a tracked file OUT of a matched - tree is still detected. Matching is case-sensitive regex against each path. - - Fail-safe: any error enumerating files - including a native gh non-zero exit, which pwsh does not - reliably surface as a terminating error - defaults to relevant=true, so a required-check gate runs - its validation rather than silently skipping on a transient/permission failure. -.PARAMETER Repository - The owner/repo slug (e.g. SamErde/DLLPickle). -.PARAMETER PullNumber - The pull request number. -.PARAMETER Pattern - One or more regex patterns matched against each changed (and previous) file path. -#> -[Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSAvoidUsingWriteHost', '', Justification = 'CI script: emits GitHub Actions log lines and ::warning:: annotations to stdout by design.')] -[CmdletBinding()] -param( - [Parameter(Mandatory)] - [ValidateNotNullOrEmpty()] - [string]$Repository, - - [Parameter(Mandatory)] - [int]$PullNumber, - - [Parameter(Mandatory)] - [ValidateNotNullOrEmpty()] - [string[]]$Pattern -) - -$Relevant = $true -try { - $ErrorActionPreference = 'Stop' - $Files = gh api "repos/$Repository/pulls/$PullNumber/files" --paginate --jq '.[] | .filename, (.previous_filename // empty)' - if ($LASTEXITCODE -ne 0) { - throw "gh api exited $LASTEXITCODE while listing PR files" - } - - $Relevant = $false - foreach ($File in $Files) { - foreach ($Expression in $Pattern) { - if ($File -match $Expression) { - $Relevant = $true - break - } - } - if ($Relevant) { - break - } - } -} catch { - Write-Host "::warning::Changed-file detection failed; defaulting to relevant=true (fail-safe). $_" - $Relevant = $true -} - -$RelevantValue = $Relevant.ToString().ToLowerInvariant() -if ($env:GITHUB_OUTPUT) { - "relevant=$RelevantValue" | Out-File -FilePath $env:GITHUB_OUTPUT -Append -Encoding utf8 -} -Write-Host "Change relevance: $RelevantValue" diff --git a/.github/workflows/Build Module.yml b/.github/workflows/Build Module.yml index f28e77c..98b5c95 100644 --- a/.github/workflows/Build Module.yml +++ b/.github/workflows/Build Module.yml @@ -53,18 +53,40 @@ jobs: outputs: relevant: ${{ steps.detect.outputs.relevant }} steps: - - name: Check out repository - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - name: Detect whether build-relevant paths changed id: detect shell: pwsh env: GH_TOKEN: ${{ github.token }} run: | - ./.github/ci-scripts/Get-DLLPickleChangedPathRelevance.ps1 ` - -Repository '${{ github.repository }}' ` - -PullNumber ${{ github.event.pull_request.number }} ` - -Pattern '^src/', '^build/', '^tests/', '^tools/', '^\.github/ci-scripts/', '^\.github/workflows/Build Module\.yml$' + # Detection is intentionally INLINE (not a shared .github/ci-scripts script): keeping the + # merge-gate logic in the workflow file benefits from GitHub's stricter workflow-edit review, + # so a PR can't relocate it to a less-scrutinized script and tamper the skip decision (#228). + # Fail-safe: default to relevant=true so any error enumerating changed files runs the build + # rather than silently skipping it. Use the PAGINATED REST files endpoint -- gh pr view + # --json files caps at 100 files and could miss a later src/ change on a large PR. + $Relevant = $true + try { + $ErrorActionPreference = 'Stop' + # Project previous_filename too, so a rename moving a tracked file OUT of a matched tree + # (reported as the new path in .filename, old path in .previous_filename) is still seen. + $Files = gh api "repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/files" --paginate --jq '.[] | .filename, (.previous_filename // empty)' + # Native non-zero exits don't reliably throw in pwsh, so check explicitly -> fail-safe. + if ($LASTEXITCODE -ne 0) { throw "gh api exited $LASTEXITCODE while listing PR files" } + $Patterns = @('^src/', '^build/', '^tests/', '^tools/', '^\.github/ci-scripts/', '^\.github/workflows/Build Module\.yml$') + $Relevant = $false + foreach ($File in $Files) { + foreach ($Pattern in $Patterns) { + if ($File -match $Pattern) { $Relevant = $true; break } + } + if ($Relevant) { break } + } + } catch { + Write-Host "::warning::Changed-file detection failed; defaulting to relevant=true (fail-safe). $_" + $Relevant = $true + } + "relevant=$($Relevant.ToString().ToLowerInvariant())" | Out-File -FilePath $env:GITHUB_OUTPUT -Append -Encoding utf8 + Write-Host "Build-relevant change detected: $Relevant" build: diff --git a/.github/workflows/Upstream-Compatibility.yml b/.github/workflows/Upstream-Compatibility.yml index bf069ca..02ebebf 100644 --- a/.github/workflows/Upstream-Compatibility.yml +++ b/.github/workflows/Upstream-Compatibility.yml @@ -34,18 +34,40 @@ jobs: outputs: relevant: ${{ steps.detect.outputs.relevant }} steps: - - name: Check out repository - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - name: Detect whether upstream-compatibility paths changed id: detect shell: pwsh env: GH_TOKEN: ${{ github.token }} run: | - ./.github/ci-scripts/Get-DLLPickleChangedPathRelevance.ps1 ` - -Repository '${{ github.repository }}' ` - -PullNumber ${{ github.event.pull_request.number }} ` - -Pattern '^build/', '^src/DLLPickle\.Build/', '^tests/', '^tools/', '^\.github/workflows/Upstream-Compatibility\.yml$' + # Detection is intentionally INLINE (not a shared .github/ci-scripts script): keeping the + # merge-gate logic in the workflow file benefits from GitHub's stricter workflow-edit review, + # so a PR can't relocate it to a less-scrutinized script and tamper the skip decision (#228). + # Fail-safe: default relevant=true so any error enumerating changed files runs the + # validation rather than silently skipping a required check. Paginated REST files endpoint + # (gh pr view --json files caps at 100 files and could miss a later build/ or tools/ change). + $Relevant = $true + try { + $ErrorActionPreference = 'Stop' + # Project previous_filename too, so a rename moving a tracked file OUT of a matched tree + # (reported as the new path in .filename, old path in .previous_filename) is still seen. + $Files = gh api "repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/files" --paginate --jq '.[] | .filename, (.previous_filename // empty)' + # Native non-zero exits don't reliably throw in pwsh, so check explicitly -> fail-safe. + if ($LASTEXITCODE -ne 0) { throw "gh api exited $LASTEXITCODE while listing PR files" } + $Patterns = @('^build/', '^src/DLLPickle\.Build/', '^tests/', '^tools/', '^\.github/workflows/Upstream-Compatibility\.yml$') + $Relevant = $false + foreach ($File in $Files) { + foreach ($Pattern in $Patterns) { + if ($File -match $Pattern) { $Relevant = $true; break } + } + if ($Relevant) { break } + } + } catch { + Write-Host "::warning::Changed-file detection failed; defaulting to relevant=true (fail-safe). $_" + $Relevant = $true + } + "relevant=$($Relevant.ToString().ToLowerInvariant())" | Out-File -FilePath $env:GITHUB_OUTPUT -Append -Encoding utf8 + Write-Host "Upstream-compatibility-relevant change detected: $Relevant" pr-smoke-validation: name: Validate upstream compatibility tooling