Repository navigation
ci: report full CI failures and gate release merge groups on external suites - #1105
Conversation
… suites Daily and manual full runs are not merge gates, so their failures could go unnoticed. Report full validation keeps one issue per long-lived branch open while full CI fails and closes it once a full run passes. Releases could also merge after only change-based checks. A merge group that changes the release manifest now runs full validation, and Rust Test & NAPI Build requires the external suite comparisons to pass for it.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 5475f30540
ℹ️ 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".
| const base = `${release}~1`; | ||
| const baseSha = execFileSync("git", ["rev-parse", base], { |
There was a problem hiding this comment.
The lint job checks out the repository without overriding actions/checkout's documented default fetch-depth: 1 (action definition), so ${release}~1 is unavailable there. Whenever repository script tests run—including for this commit and every full-validation run—this git rev-parse exits with status 128 and fails the entire Lint & Format job before the classifier behavior is tested. Construct the history in a temporary fixture or explicitly fetch sufficient history in that job.
Useful? React with 👍 / 👎.
Syncing master into develop carries the release manifest change too, which made the develop merge group run and wait for the external comparisons although it tags nothing. Pass the merge group base ref to the classifier and only mark master merge groups as releases.
ci-changes.mjs could skip product validation for a Release Please pull request, but ci.yml never set RELEASE_PLEASE_PR, and the release diff also bumps Cargo.toml and Cargo.lock. The release PR therefore ran every job on each master push (71 runs and about 3,600 job-minutes in two weeks). Set the flag for the repository's own release branch and accept its Cargo version bumps. The master merge group that cuts the release now runs full validation, so nothing merges without it.
CI checks out a single commit, so the release test could not find the parent of the latest release manifest change. Create the two commits it needs in a temporary repository instead.
1 participant
Summary
Full CI now runs daily and on demand, while PRs and merge groups run
change-based checks. This closes two gaps in that model:
Full CI failures reach someone. A new
Report full validationjob runs atthe end of scheduled and dispatched full runs on
masteranddevelop. Afailed or cancelled job opens "Full CI is failing on " (or comments on
it while it stays open); the next passing full run closes it. The logic lives
in
scripts/report-full-ci.mjs.A release cannot merge after only change-based checks. Only the release
PR changes
.release-please-manifest.json.ci-changes.mjsnow emitsrelease=truefor amastermerge group whose diff changes it (or whosediff cannot be determined). Syncing master into
developcarries the samemanifest change but tags nothing, so
developmerge groups are neverreleases; the classifier receives
MERGE_GROUP_BASE_REFfor this. That enables every path, runs the external Verilator/Icaruscomparisons, and makes the existing required
Rust Test & NAPI Buildcheckrequire them to pass. No ruleset change is needed.
The release PR stops re-running everything.
ci-changes.mjsalreadyhad a Release Please skip, but
ci.ymlnever setRELEASE_PLEASE_PR, and therelease diff also bumps
Cargo.toml/Cargo.lock. The release PR ran everyjob on each master push (71 runs, ~3,600 job-minutes in two weeks). The flag
is now set for the repository's own release branch and the Cargo version
files count as release files. This is safe because the release's master
merge group runs full validation.
Daily and manual full runs do not gate on the external comparisons; their
failures go to the issue above. The release PR's own
pull_requestruns remainchange-based.
CONTRIBUTING.mddocuments both.Validation
node --test scripts/*.test.mjs: 130 pass, also withGITHUB_EVENT_NAME=merge_groupandMERGE_GROUP_BASE_REF=refs/heads/masterin the environment as in a master merge group job.
New tests cover the issue reporter (open/comment/close/duplicates), release
detection against a real release-manifest commit (a release on master, not on
develop), unknown merge group diffs,
the gate truth table with the release requirement, and that the report job
waits on every other
ci.ymljob.ci-changes.mjswithRELEASE_PLEASE_PR=trueon the diff of theopen release PR chore(master): release 0.11.0 #1007 selects no product jobs.
actionlint .github/workflows/ci.yml: no new findings (only the existingubuntu-26.04-armrunner label warning).GitHub Actions. The first daily run after merge and the next weekly release
will exercise them.
timeout on the
masteranddeveloprulesets was raised from 60 to 180minutes, since the external suites allow 90 minutes plus runner wait.