Repository navigation
Skip redundant tests when staging a release - #121
Conversation
The publish job ran the full suite for both profiles, adding roughly 45 minutes to every release to re-test a tree CI had already tested. A release tag points at a merge commit on main, and CI runs on every push to main, so that commit already has its own green run for both profiles. Staging now builds with -DskipTests. To keep the guarantee that the released tree was actually tested, a new step first asserts the tagged commit has successful Build & Test runs for both Spark 3.5 and Spark 4.1, and fails before anything is uploaded if it does not. This also closes a gap the old design left open: a tag placed on a commit that never went through CI used to be released as long as the tests happened to pass on the runner. -DskipTests suppresses only Scala test execution. All thirteen governance exec bindings, including the version-consistency and packaged-jar contents checks, still run during staging — verified by running the test phase with the flag set. The two changes are load-bearing together, so release-pipeline-tests.sh now asserts both the flag and the provenance gate are present. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
The new CI gate can incorrectly fail when multiple check runs with the same name exist for a commit (e.g., after reruns), because the conclusion parsing may become multi-line and not equal to success.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR speeds up the release publishing workflow by avoiding redundant re-execution of the full test suite during staging, while preserving safety by gating releases on already-successful CI runs for the tagged commit.
Changes:
- Add a “Verify the release commit passed CI” gate in
publish.yml(requireschecks: read) to ensure the tagged commit has green CI for both Spark profiles before staging. - Stage both Spark profiles with Maven
-DskipTeststo skip redundant test execution. - Update release pipeline contract tests and contributor docs to document/enforce the paired gate + skip-tests design.
File summaries
| File | Description |
|---|---|
| docs/contributors/releasing.html | Documents the new CI-provenance gate and the rationale for skipping tests during staging. |
| dev/scripts/release-pipeline-tests.sh | Adds contract assertions to ensure both the CI gate and -DskipTests remain present together. |
| .github/workflows/publish.yml | Adds check-runs verification for the release commit and applies -DskipTests to both staging deploy steps. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| sha=$(git rev-parse HEAD) | ||
| checks=$( | ||
| gh api "repos/$GITHUB_REPOSITORY/commits/$sha/check-runs?per_page=100" \ | ||
| --jq '.check_runs[] | "\(.name)\t\(.conclusion)"' | ||
| ) | ||
| for name in "Build & Test (Spark 3.5)" "Build & Test (Spark 4.1)"; do | ||
| conclusion=$(printf '%s\n' "$checks" | awk -F'\t' -v n="$name" '$1 == n { print $2 }') | ||
| if [[ "$conclusion" != "success" ]]; then | ||
| echo "$name did not succeed for $sha (got: ${conclusion:-no run})." >&2 | ||
| echo "Staging skips tests, so the tagged commit must already have a green CI run." >&2 | ||
| exit 1 | ||
| fi | ||
| done |
There was a problem hiding this comment.
Good catch — this was a real bug, and my assumption behind the original code was wrong.
I had believed the check-runs API's default filter=latest collapsed duplicates by name. It does not. Commit
7f0176ce on main already carries two Analyze (actions) runs, and total_count is 7 both with and without
filter=all.
Running the original logic against that commit returns $'success\nsuccess' — two lines — so it would have rejected
the commit. A re-run of either Build & Test job would therefore have blocked an otherwise valid release.
Fixed in 6182bff by sorting the matching runs by completed_at and taking the last, so exactly one conclusion is
compared. I used sort-and-take-last rather than "any run succeeded" deliberately, so the gate stays correct in the
other direction too: if a re-run fails after an earlier success, the latest conclusion is the failure and the release
is rejected.
Verified against the live API plus synthetic cases: duplicate names now yield a single success; a re-run whose
latest attempt failed yields failure; a still-running check yields incomplete; a commit with no runs yields
no run. Only the first case passes the gate.
The gate matched check runs by name and compared the result directly to "success". A commit can carry several check runs sharing one name — a re-run adds another rather than replacing the first, and the check-runs API does not collapse them. Commit 7f0176c already carries two "Analyze (actions)" runs, where the previous logic yields $'success\nsuccess' and rejects the commit. That would have blocked a release whose only irregularity was a re-run job. The lookup now sorts the matching runs by completed_at and takes the last, so exactly one conclusion is compared. Sorting rather than searching for any success also keeps the gate honest in the other direction: when a re-run fails, the latest conclusion is the failure. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🔵 Needs a closer look
-DskipTests also skips Maven-bound governance scripts via exec-maven-plugin’s <skip>${skipTests}</skip> wiring, which weakens the release guards unless the workflow runs those checks explicitly.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
.github/workflows/publish.yml:110
-DskipTestswill also cause Maven-bound governance scripts to be skipped here becauseexec-maven-pluginis configured with<skip>${skipTests}</skip>inpom.xml. That undermines the release guards described in the PR (e.g., version/reference checks) even though Scala tests are skipped. Consider running the governance scripts explicitly before staging (the repo already hasdev/scripts/run-governance-checks.sh, which CI uses for this exact purpose).
dev/scripts/release-pipeline-tests.sh:65- This contract test asserts
-DskipTestsand the CI-provenance gate are present, but it should also assert that governance checks still run somewhere in the workflow. With the currentpom.xmlwiring (exec-maven-pluginskips whenskipTestsis set), omitting an explicit governance step would silently drop several release guards.
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Lite
Code Coverage — Spark 3.5
|
-DskipTests does not only suppress the Scala suites. The governance
scripts in dev/scripts are bound to the test phase through
exec-maven-plugin, which carries a plugin-level
<skip>${skipTests}</skip>. Execution-level configuration merges with
that block rather than replacing it, so all fourteen test-phase
bindings inherit the skip. Only clean-api-docs-output and
test-package-contents opt out with <skip>false</skip>.
Adding -DskipTests to the staging steps therefore disabled
readme-has-version.sh, the version-consistency check that exists
specifically to catch a mismatched release, along with the rest of the
governance suite. Verified by replacing readme-has-version.sh with a
script that prints a sentinel and exits 1: `mvn test -DskipTests`
succeeded and never printed the sentinel.
Run dev/scripts/run-governance-checks.sh as an explicit step before
staging. The script derives its list from pom.xml, so a newly
registered check cannot silently go unrun. The same sentinel makes the
runner fail, confirming the guards are genuinely restored.
Correct the releasing guide, which claimed the guards still ran.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
You're right, and I'd claimed the opposite in the PR description without checking. Fixed in a20a07f. Verified it properly this time: I replaced The cause is the plugin-level The fix runs |
There was a problem hiding this comment.
🔵 Needs a closer look
The new CI provenance gate can incorrectly pass when a job is re-run and the latest attempt is still in progress because it sorts only by completed_at (ignoring started_at), potentially allowing staging to proceed without a final CI result.
Review details
Suppressed comments (1)
.github/workflows/publish.yml:91
- The provenance gate can incorrectly pass when a check was re-run and the newest attempt is still in progress: in-progress runs typically have completed_at=null, so sorting only by completed_at selects the most recently completed run (possibly a prior success) and ignores the in-progress attempt. That allows staging to proceed while CI is still running (or before the latest attempt’s conclusion is known). Sort by started_at as a fallback so an in-progress re-run is treated as the latest attempt and yields "incomplete".
| sort_by(.completed_at // "")
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Lite
A re-run adds a second check run under the same name instead of replacing the first, and an unfinished attempt reports completed_at and conclusion as null. Ordering by completed_at therefore sorted the running attempt to the front and selected the previous, already finished one, so a release triggered while CI was re-running could be matched against a stale success. Order by started_at, which is always populated and increases with each attempt, and treat any attempt that has not reached status completed as incomplete rather than reading its empty conclusion. Verified against live data: while a re-run was in flight both profiles resolved to incomplete, and the 0.1.10-beta release commit still resolved to success. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Right again — fixed in e607496. My earlier "in-progress" test case only covered a commit where the sole attempt was unfinished, which resolves correctly by accident. It never covered a finished attempt sitting alongside an unfinished re-run, which is the case you describe. Confirmed the premise on live data rather than assuming it: I re-ran this PR's CI and queried The gate now sorts by Re-tested across single success, no runs, only a differently-named check, a re-run that fixed a failure, a re-run that broke a success, in progress, queued, cancelled, and an in-progress re-run following a fix. Also validated against real payloads: with the re-run in flight both profiles resolved to |
There was a problem hiding this comment.
🟡 Changes recommended
The provenance gate can incorrectly pass when the newest CI attempt is queued due to sorting only by started_at.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Lite
| printf '%s' "$checks" | | ||
| jq -r --arg name "$name" ' | ||
| [.check_runs[] | select(.name == $name)] | ||
| | sort_by(.started_at // "") |
|
|
||
| <p>Before staging, the workflow checks that the tagged commit already has successful <code>Build & Test (Spark 3.5)</code> and <code>Build & Test (Spark 4.1)</code> runs. Because CI runs on every push to <code>main</code>, the merge commit a release tag points at has its own green run. Staging then builds with <code>-DskipTests</code>, since re-running the same suite on the same tree adds roughly 45 minutes and cannot discover anything new. If the tag points at a commit CI never tested, the release stops before anything is uploaded.</p> | ||
|
|
||
| <p>Re-running a job adds another check run under the same name rather than replacing the old one, so the gate takes the attempt with the newest <code>started_at</code> and requires it to have finished. An unfinished attempt reports both <code>completed_at</code> and <code>conclusion</code> as <code>null</code>, so a release started while CI is still re-running is refused rather than being matched against an earlier green attempt.</p> |
Reading a single attempt makes the gate depend on how a pending check run fills in its timestamps: if an unfinished attempt sorted ahead of a finished one, the finished attempt would be read instead and a stale success could admit a release while CI was still running. Refuse outright when any attempt for a required check has not reached status completed, and only then read the conclusion of the newest attempt by started_at. The safety property no longer rests on any timestamp being populated on a pending run. Exercised across eleven cases including a queued attempt with a null started_at, an in-progress re-run following a green run, a re-run that fixed a failure and one that broke a success. Against live data both profiles resolve to incomplete while a re-run is in flight, and the 0.1.10-beta release commit still resolves to success. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
I checked the premise this time rather than assuming, and on GitHub Actions it doesn't hold: So a queued attempt sorts last, not first, and the previous logic already returned That said, I've made the change anyway (371d24f), because your underlying point stands even though the specific mechanism doesn't: the gate shouldn't depend on a pending run populating a particular timestamp at all. It now refuses the release if any attempt for a required check hasn't reached Retested across eleven cases, including a queued attempt with I've corrected the docs paragraph to describe the actual rule rather than the selection key, and |
There was a problem hiding this comment.
🟡 Changes recommended
The new release provenance gate can accept commits where CI “Build & Test” jobs succeeded even though the actual build/test steps were skipped, weakening the safety guarantee behind skipping tests during staging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
| sha=$(git rev-parse HEAD) | ||
| checks=$(gh api "repos/$GITHUB_REPOSITORY/commits/$sha/check-runs?per_page=100") | ||
| for name in "Build & Test (Spark 3.5)" "Build & Test (Spark 4.1)"; do | ||
| conclusion=$( | ||
| printf '%s' "$checks" | | ||
| jq -r --arg name "$name" ' | ||
| [.check_runs[] | select(.name == $name)] | ||
| | if length == 0 then "no run" | ||
| elif any(.[]; .status != "completed") then "incomplete" | ||
| else (sort_by(.started_at // "") | last | .conclusion // "incomplete") | ||
| end | ||
| ' |
CI classifies each change and guards its build steps with `if: needs.changes.outputs.source == 'true'`, so a documentation-only commit reports Build & Test as successful within seconds having compiled nothing. Commit 1474a03 is such a case: both jobs are green, in 23 and 7 seconds, and neither built anything. Checking the job conclusion alone therefore let the gate accept a commit whose artifacts were never built or tested, which is precisely what the gate exists to prevent, and Maven Central coordinates cannot be replaced once published. Read the conclusion of the build step inside each job and require it to have run. Verified by extracting the gate from the workflow and running it unmodified: fdecbb2, the 0.1.10-beta release commit, passes, while the documentation-only 1474a03 is refused. The step names are matched literally, so release-pipeline-tests.sh now checks that each one still exists in ci.yml and fails on a rename. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Confirmed and fixed in 26175cf — this was a real hole, and a serious one given Central coordinates can't be replaced.
Both green, neither compiled anything. Tagging that commit would have passed the gate and published artifacts no test ever touched — the exact scenario the gate was added to prevent. I went with your option (b). Option (a) would make the check conclusion A check run's id is the Actions job id, so the gate now fetches
Verified by extracting the gate step out of the workflow YAML and running it unmodified, so there's no risk I tested a paraphrase: Since the gate now names CI steps literally, |
There was a problem hiding this comment.
🟡 Changes recommended
The new step calls the Actions Jobs API but the workflow permissions don’t currently grant actions: read, which can cause the provenance gate to fail even for valid releases.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
The gate reads the job behind each check run to confirm the build step ran. That endpoint sits under /actions/, and declaring permissions explicitly sets every unlisted scope to none, so the workflow needs actions: read alongside checks: read. Without it the gate would fail on releases that should have been allowed. Local verification did not catch this because it ran with a personal token rather than the workflow token. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Good catch — fixed in 8743242 by adding You're right that the explicit Worth noting why my verification missed it: I tested the gate by extracting it from the workflow and running it locally, which used my personal token and therefore had scopes the workflow token won't have. Testing the logic locally can't validate the permission surface.
Incidentally, the workflow-jobs API reference also documents |
There was a problem hiding this comment.
🟡 Changes recommended
The provenance gate appears to use a Checks API check-run ID as if it were an Actions job ID when calling the Actions Jobs API, which can break the gate and block valid releases.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
| [.check_runs[] | select(.name == $name)] | sort_by(.started_at // "") | last | .id | ||
| ' | ||
| ) | ||
| step_conclusion=$( | ||
| gh api "repos/$GITHUB_REPOSITORY/actions/jobs/$job" | |
The gate looks up the job behind a check run using the check run's own id. That holds for check runs produced by Actions, and GitHub asserts the link in both directions, but the gate should not depend on it silently. Check the job's name matches the check run's before reading its steps, so a mismatch stops the release instead of judging some other job. Verified by injecting a valid id for the other profile's job, which the gate now rejects. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
I checked this one rather than taking it on trust, and the two ids are in fact the same for check runs produced by Actions — GitHub links them in both directions:
The check run's A 404 would also have been the safe direction here — the step runs under That said, your second concern is the one worth acting on, so I've done that in 3265636: reading the wrong job's steps would be a silent wrong answer rather than a loud failure. The gate now confirms the job's So the gate no longer depends on the id relationship holding — if it ever stops holding, the release stops rather than being judged against the wrong job. I'd rather have that than the extra request needed to list a run's jobs and match by name, which would depend on the same name matching anyway. |
There was a problem hiding this comment.
🟡 Changes recommended
The new provenance gate treats a check-run id as a GitHub Actions job id when calling actions/jobs/{id}, which can break job-step verification and incorrectly fail (or mis-evaluate) releases.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
.github/workflows/publish.yml:115
- The gate assumes the check-run
.idfromcommits/{sha}/check-runsis a GitHub Actions job id and callsactions/jobs/{id}with it. In the REST API these are different identifiers, so this can 404 or fetch the wrong job, breaking releases or mis-evaluating step conclusions. Derive the job id from the check run’sdetails_url(or via check_suite → workflow run → jobs) before callingactions/jobs/{job_id}.
job=$(
printf '%s' "$checks" |
jq -r --arg name "$name" '
[.check_runs[] | select(.name == $name)] | sort_by(.started_at // "") | last | .id
'
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
| assert_contains .github/workflows/publish.yml "actions/jobs/" | ||
| # The job behind a check run is looked up by the check run's own id. Confirm the job's name before trusting its | ||
| # steps, so a mismatch fails the release instead of judging some other job. | ||
| assert_contains .github/workflows/publish.yml 'if [[ "$job_name" != "$name" ]]' |
The check run's details_url names the job that produced it, so parse the id from there rather than relying on the check run's own id doubling as a job id. The two do match for check runs produced by Actions, but reading it from the URL says so directly instead of leaving a reader to wonder. Fall back to the check run id if the URL is not in the expected shape. Either route is validated by the job name check, so a wrong id stops the release rather than being judged against another job. Verified against real commits and against a details_url that is null, malformed and well formed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Done in 54d42d3 — the gate now takes the job id from To be clear about what was actually broken: the two ids do match for check runs produced by Actions, and I verified that in both directions before changing anything — It's still the better way to write it. Reading the id out of Exercised against a Re-ran the gate end to end from the workflow YAML: |
There was a problem hiding this comment.
🟡 Changes recommended
The new CI-provenance gate is central to release safety and needs a couple of reliability/error-reporting fixes (pagination and clearer failure handling) before it can be trusted for releases.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
.github/workflows/publish.yml:86
- The check-runs fetch is limited to the first 100 results. If a commit ever accumulates >100 check runs (e.g., additional tooling over time), the required "Build & Test" check runs might not be present in this first page and the release would be rejected with "no run" even though CI succeeded. Fetch all pages (and optionally set filter=all explicitly) before running jq.
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
| # A check run produced by Actions carries the id of the job behind it, and the job points back at the same | ||
| # check run. Confirm the job's name matches before reading its steps, so the gate fails rather than | ||
| # judging the wrong job if that ever stops holding. | ||
| job_json=$(gh api "repos/$GITHUB_REPOSITORY/actions/jobs/$job") |
The gate read only the first page of check runs. A commit carrying more than fit on one page could have a required Build & Test run on a later page, where the gate would read it as missing and stop a release that should have gone ahead. Request every page and merge them before matching. Verified by forcing four pages with per_page=2: the verdicts are unchanged, with fdecbb2 passing and the documentation-only 1474a03 refused either way. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Fixed in 12efb9f — the gate now requests every page and merges them before matching. You're right about the direction of the failure: a required run on a later page would read as This repo is nowhere near the limit today (
I left |
There was a problem hiding this comment.
🟡 Changes recommended
The new jq aggregation for check runs can yield check_runs: null on empty paginated results, causing the gate to error out unexpectedly instead of cleanly reporting “no run.”
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
| # read as missing and stop a release that should have gone ahead. | ||
| checks=$( | ||
| gh api --paginate --slurp "repos/$GITHUB_REPOSITORY/commits/$sha/check-runs?per_page=100" | | ||
| jq '{check_runs: (map(.check_runs) | add)}' |
Merging pages with add returns null if there are no pages at all, and iterating that would abort the step with a jq error rather than reporting that the commit has no run. Default to an empty list. The API returns a page even when a commit has no check runs, so this is a guard rather than a fix for observed behaviour. Confirmed the guarded expression is unchanged for a single empty page and for several populated pages, and that the gate reports "no run" cleanly for a commit with no check runs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Added the guard in 9e16ccc. Worth separating the two halves of this, though. The jq semantics are exactly as you say — But it isn't reachable through this endpoint.
I've added |
There was a problem hiding this comment.
🔵 Needs a closer look
It modifies the release/publishing pipeline with non-trivial GitHub API gating logic, so it warrants final human verification despite only minor review notes.
Review details
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
.github/workflows/publish.yml:129
- The comment claims the Actions check run “carries the id of the job behind it”, but this step explicitly avoids assuming the check run id is a job id (it parses the job id from details_url and only falls back to the check run id). This inconsistency is misleading for future maintenance.
dev/scripts/release-pipeline-tests.sh:96
- This comment says the workflow looks up the Actions job “by the check run's own id”, but publish.yml now prefers parsing the job id from the check run’s details_url (only falling back to the check run id). Keeping the comment aligned with the implementation will prevent confusion if the gate needs changes later.
assert_contains .github/workflows/publish.yml "actions/jobs/"
# The job behind a check run is looked up by the check run's own id. Confirm the job's name before trusting its
# steps, so a mismatch fails the release instead of judging some other job.
assert_contains .github/workflows/publish.yml 'if [[ "$job_name" != "$name" ]]'
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Lite
Both comments still said the job is looked up by the check run's own id, which stopped being true when the id moved to details_url. Describe the lookup that is there, so the fallback and the name check read as deliberate rather than redundant. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Both correct — fixed in 2d18388. They were accurate when written and went stale when the lookup moved to
and I also grepped the three changed files for the other phrasings of that claim to be sure none survived elsewhere. |
There was a problem hiding this comment.
🟢 Approval recommended
The workflow changes maintain release safety via an explicit CI provenance gate and governance checks while achieving the intended reduction in staging time.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Lite
Problem
publish.ymlstages both Spark profiles with./mvnw clean deploy, which re-runs the full test suite. That is roughly 45 of the ~60 minutes a release takes, and it cannot discover anything new: a release tag points at a commit already built and tested by CI onmain. For0.1.10-betathe tested tree and the released tree were byte-identical (405dc48).Change
Skip tests during staging. Both staging steps now pass
-DskipTests.Gate on CI provenance instead. Skipping tests is only sound if the tagged commit really was tested, so a new step runs before staging and fails the release unless the tagged commit has successful
Build & Test (Spark 3.5)andBuild & Test (Spark 4.1)check runs.ci.ymltriggers onpush: [main], so the merge commit a tag points at has its own run. The gate also reads the conclusion of the build step inside each job: CI guards its build steps withif: needs.changes.outputs.source == 'true', so a documentation-only commit reports both jobs green within seconds having compiled nothing, and a job conclusion alone would not show the artifacts were built. If a tag points at a commit that was never built or tested, the release stops before anything is uploaded — which matters because Maven Central coordinates are immutable. The gate reads every page of check runs, so a required run that lands beyond the first page cannot read as missing and stop a release that should have gone ahead.Run the governance checks explicitly.
-DskipTestsdoes not only suppress the Scala suites. The scripts indev/scriptsare bound to thetestphase viaexec-maven-plugin, which carries a plugin-level<skip>${skipTests}</skip>; execution-level configuration merges with that block rather than replacing it, so all fourteen bindings inherit the skip — includingreadme-has-version.sh, the version-consistency check that exists specifically to catch a mismatched release. The workflow now runsdev/scripts/run-governance-checks.shbefore staging. That script derives its list frompom.xml, so a newly registered check cannot silently go unrun.Two executions opt out with
<skip>false</skip>and still run during staging:clean-api-docs-outputandtest-package-contents, the latter validating the shaded jar's contents. Scalafix and scalastyle are separate plugins and are unaffected.Verification
The skip was real. Replaced
readme-has-version.shwith a script printing a sentinel and exiting 1.mvn test -Pspark35 -DskipTestssucceeded and never printed the sentinel — the script did not run. Maven prints the execution header regardless, which is what made this easy to miss.The fix is real. The same sentinel makes
run-governance-checks.shfail. Clean tree: all 15 checks pass.The conclusion lookup handles re-runs.
commits/{sha}/check-runsdoes not deduplicate by name — commit7f0176cecarries twoAnalyze (actions)runs, andtotal_countis 7 with and withoutfilter=all. Reading a single attempt is therefore not enough: an unfinished attempt reportscompleted_atandconclusionasnull(confirmed on a live re-run), so ordering by completion time sorted it to the front and selected the previous, already finished attempt — a release started while CI was re-running could be matched against a stale success.The gate now refuses outright if any attempt for a required check has not reached
status == "completed", and only then reads the conclusion of the newest attempt bystarted_at. Failing closed means the safety property does not depend on how a pending run populates its timestamps.Exercised across eleven cases: a single success, no runs at all, only a differently-named check, a re-run that fixed a failure, a re-run that broke a success, an in-progress attempt alone, an in-progress re-run following a green run, a queued attempt with
started_atset, a queued attempt withstarted_atnull, a cancelled attempt, and two greens where the newest must win. Confirmed against live API data too — with a re-run in flight both profiles resolved toincomplete, while the0.1.10-betarelease commit still resolved tosuccess.The step check distinguishes a real build from a skipped one. Extracted the gate from the workflow YAML and ran it unmodified against real commits:
fdecbb2(the0.1.10-betarelease commit) passes, while the documentation-only1474a03— both jobs green in 23s and 7s — is refused withits "Build and test with coverage" step was skipped. The job is located via the check run'sdetails_url, and its name is checked before its steps are read, so a mis-identified job stops the release instead of being judged in place of the right one.Pagination does not change any verdict. This repository is far from the page limit (
total_countis 7 on the commits checked), so the paging was forced rather than waited for: running the same gate withper_page=2splits those runs across four pages and produces identical results, withfdecbb2passing and1474a03refused either way. Merging pages defaults to an empty list, so a commit with no check runs reportsno runrather than failing on a null — confirmed againstcfea07e, which genuinely has none.dev/scripts/release-pipeline-tests.shties the pieces together so they cannot drift apart:-DskipTests, the gate step, both check names, the paginated fetch, the fail-closed condition, the job lookup and job-name check, the governance step, and bothchecks: readandactions: read. Because the gate names CI steps literally, it also verifies each name still exists inci.ymland fails on a rename.Notes
checks: readandactions: readwere added to the workflow permissions: the gate reads check runs and then the job behind each one, and declaring permissions explicitly sets every unlisted scope tonone.docs/contributors/releasing.htmldocuments the tradeoff and the skip semantics.