ci: always-report Build Module + Upstream-Compatibility gates (enable required checks)#228
Merged
Conversation
…r 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 <noreply@anthropic.com>
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Owner
Author
|
@codex review |
Contributor
Up to standards ✅🟢 Issues
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the CI gating strategy so two previously path-filtered workflows always trigger on PRs targeting main, enabling them to be configured as required status checks without leaving PRs stuck in Pending when the workflow wouldn’t have run.
Changes:
- Remove
pull_request.pathsfilters from Build Module and Upstream Compatibility workflows, and instead gate the expensive validation/build jobs internally. - Add lightweight PR file-change detection jobs (
changes/pr-changes) that decide whether the heavyweight job should run or be skipped. - Ensure the Build Module matrix job can still run for
workflow_call(release) andworkflow_dispatch, even when the PR-change detection job is skipped.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| .github/workflows/Upstream-Compatibility.yml | Always triggers on PRs to main; adds pr-changes job and gates the PR tooling validation job based on detected relevant paths. |
| .github/workflows/Build Module.yml | Always triggers on PRs to main; adds changes job and gates the matrix build based on detected relevant paths while still building for workflow_call/manual runs. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e2d7e35793
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- 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 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b68781517a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ames 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 <noreply@anthropic.com>
…te 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 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a8ce011e1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
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 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
SamErde
added a commit
that referenced
this pull request
Jun 1, 2026
* docs: require Build gate (not per-OS legs) in the §9 required-checks note Aligns the architecture note with #228: the gate workflows now always report (skip == passing check), so they can be required without blocking docs-/CI-only PRs. The matrix build's required-check target is the always-present aggregate "Build gate" (the per-OS "Build and test module (...)" contexts aren't created when the matrix job is skipped). Required set: Build gate, Validate upstream compatibility tooling, dependency-review. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs: reword Dependabot-Auto-Approve reference to match the workflow header Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
SamErde
added a commit
that referenced
this pull request
Jun 2, 2026
…tartup failure) (#232) fix(ci): grant pull-requests: read to the release build-and-test caller Release-and-Publish startup_failed on both pull_request:closed and workflow_dispatch after #228, so 2.2.0 never published. Root cause: #228 added a "changes" job to the reusable Build Module.yml that declares pull-requests: read. Release calls that workflow via uses:, and its build-and-test caller granted only contents: read + security-events: write. A reusable workflow's jobs may not request permissions the caller did not grant; GitHub validates this STATICALLY at startup (before any if: skip), so even though "changes" is skipped on the workflow_call path, the excess request fails the whole run before it starts. Grant pull-requests: read on the caller so static validation passes. The permission is unused on the release path (the "changes" job is skipped there) and is read-only and minimal. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Prerequisite for adding required status checks (point 1). Today the "Protect Main" ruleset has no required status checks, so
gh pr merge --auto(Dependabot) can complete before the build/drift checks pass. But simply marking the existing checks "required" would deadlock non-bundle PRs, because Build Module and Upstream-Compatibility are path-filtered — a required check from a workflow that never triggers staysPendingforever.This PR makes both gate workflows always trigger on PRs but short-circuit internally, so their checks always report:
changes/pr-changesjob (gh pr view --json files) detects whether build/tooling-relevant paths changed.workflow_call— the buildifalways builds wheninputs.refis set (release) or onworkflow_dispatch, and errs toward building on any ambiguity, so the release path is never accidentally skipped.Why not a parallel "status shim"?
A shim workflow that reports the same check name on the inverse paths has a safety hole: on a PR touching both bundle and non-bundle files, both the real check and the shim run, and a passing shim can mask a failing real build. The internal short-circuit reports exactly one result per PR.
Follow-up (separate step)
Once this is merged and the skip-path behavior is confirmed on a non-bundle PR, add a
required_status_checksrule to the ruleset:Build gate,Validate upstream compatibility tooling,dependency-review.🤖 Generated with Claude Code