mirror of
https://github.com/eclipse-sailing-analytics/sailing-analytics.git
synced 2026-09-17 11:19:15 +00:00
bug53: make build-gate a posted commit status, not a skippable job
The previous build-gate was a JOB, and GitHub Actions auto-creates a check-run for every job whose conclusion is filled from the job's fate — including "skipped" when the job's if: is false. Branch protection / rulesets treat a skipped required check as SATISFIED (mergeable), so a compile-only fork PR reported build-gate=skipped=passed and became mergeable without tests (PRs #53/#54; #53 merged this way). No job can represent "absent": a skipped job still materializes a skipped check-run. The earlier attempt to make it exit 1 (red) worked but was ugly and, on a failed build, still mis-reported skipped in practice. A commit status (legacy Statuses API) is created only if we explicitly POST it, and a skipped STEP posts nothing (steps are not check-runs). The ruleset matches the required context "build-gate" by NAME across both the Checks and Statuses APIs, so a posted commit status with context=build-gate satisfies it exactly as the old check-run did — the ruleset needs no change. Change: remove the build-gate job; add job post-build-gate-status (named so its own auto check-run carries a context the ruleset ignores) whose single step POSTs a commit status context=build-gate: - state=success when the change was trivial (build skipped, no relevant changes) OR a full build ran WITH tests and succeeded; - state=failure when a full build ran WITH tests and failed/cancelled (loud red X for a committer's own broken build); - NOTHING for any compile-only run (fork PR, *-reviewed-for-compile, workflow_dispatch skip_tests), succeeded or failed — so the build-gate context stays absent and the PR is blocked at "Expected" with no red X. The status is posted against the PR HEAD sha on pull_request events (github.sha there is the ephemeral refs/pull/N/merge commit the ruleset does not evaluate) and github.sha otherwise. Merge qualification for fork code still comes only from a maintainer pushing a *-reviewed-for-build tag (full trusted build with tests), which posts the success status. compile-gate (advisory) is unchanged. Assisted-By: Claude Opus 4.8
This commit is contained in:
@@ -305,55 +305,83 @@ jobs:
|
||||
echo "Identified CI job: ${JOB}"
|
||||
curl -u "${{ vars.CI_JOB_USERNAME }}:${{ secrets.CI_JOB_PASSWORD}}" ${{ vars.CI_BASE_URL }}/job/${JOB}/build?token=${{ secrets.CI_JOB_TOKEN }}
|
||||
|
||||
# build-gate is the REQUIRED status check in branch protection. It must be
|
||||
# SATISFIED (pass) ONLY for a genuine full build (tests actually ran) or a
|
||||
# legitimate trivial-change skip; in every other case it must FAIL (red).
|
||||
# build-gate is the REQUIRED status check in the branch ruleset (context
|
||||
# "build-gate"). It must be SATISFIED only for a genuine full build (tests
|
||||
# actually ran and passed) or a legitimate trivial-change skip; every other
|
||||
# case — most importantly a compile-only fork PR — must leave the PR
|
||||
# un-mergeable WITHOUT a red X.
|
||||
#
|
||||
# CRITICAL — why this job runs unconditionally (if: always()) and never skips
|
||||
# itself: GitHub branch protection / rulesets treat a required check whose
|
||||
# conclusion is "skipped" as SATISFIED (the PR becomes mergeable). This is true
|
||||
# BOTH for a job that runs and reports "skipped" AND for a job skipped by its
|
||||
# own if: — a skipped job still MATERIALIZES a check run with conclusion
|
||||
# "skipped". There is NO "job never starts -> pending" state for a job that is
|
||||
# part of a triggered workflow: an if:-false job reports "skipped", which
|
||||
# counts as PASSED. (This exact mistake let fork PRs #53/#54 become mergeable
|
||||
# on a compile-only build: the old if: skipped build-gate on a successful
|
||||
# compile-only run, GitHub read that as passed, and the PR merged WITHOUT the
|
||||
# secret-bearing test build ever running.)
|
||||
# WHY THIS IS A POSTED COMMIT STATUS, NOT A JOB NAMED build-gate:
|
||||
# GitHub Actions auto-creates a check-run for every JOB, and fills its
|
||||
# conclusion from the job's fate — including "skipped" when the job's if: is
|
||||
# false. A rule set / branch protection treats a "skipped" required check as
|
||||
# SATISFIED (mergeable). So a job named build-gate can NEVER represent "absent"
|
||||
# — a skipped job still materializes a skipped=passed check-run. That is the
|
||||
# exact bug that let compile-only fork PRs #53/#54 merge without tests.
|
||||
#
|
||||
# Therefore the ONLY conclusion that keeps a PR un-mergeable is FAILURE. A
|
||||
# SUCCESSFUL compile-only run (build succeeded AND skip_tests true — a fork
|
||||
# pull_request, a *-reviewed-for-compile tag, or workflow_dispatch with
|
||||
# skip_tests=true) must NOT qualify a PR for merge, so this gate RUNS for that
|
||||
# case and EXITS 1 (red). It is accurate: compile-only validation has not been
|
||||
# met. Merge qualification for fork code comes only from a maintainer pushing a
|
||||
# *-reviewed-for-build tag (full trusted build with tests), which turns this
|
||||
# gate green. compile-gate (advisory) still goes green to show the compile
|
||||
# itself succeeded.
|
||||
# A COMMIT STATUS (legacy Statuses API) is different: it exists only if we
|
||||
# explicitly POST it. A skipped STEP posts nothing (steps are not check-runs).
|
||||
# The ruleset matches the required context "build-gate" by NAME across both
|
||||
# APIs, so a posted commit status with context=build-gate satisfies it exactly
|
||||
# as the old check-run did — no ruleset change needed.
|
||||
#
|
||||
# always() also ensures a FAILED build (a job in needs:) does not cascade-skip
|
||||
# this gate by GitHub's default rule; the gate runs and the else-branch exits 1
|
||||
# -> red. The trivial-change skip (build skipped, no relevant changes) is the
|
||||
# one legitimate green-without-tests path.
|
||||
build-gate:
|
||||
permissions: {}
|
||||
# Hence: this job is named post-build-gate-status (NOT build-gate) so its own
|
||||
# auto check-run carries a context the ruleset ignores. Its single step POSTs a
|
||||
# commit status context=build-gate ONLY when: the change was trivial (build
|
||||
# skipped, no relevant changes) OR a full build ran WITH tests and succeeded
|
||||
# -> state=success; OR a full build ran WITH tests and FAILED (build.result not
|
||||
# success/skipped, skip_tests==false) -> state=failure (red X, so a committer's
|
||||
# own broken build is loud). In every OTHER case — a compile-only run, whether
|
||||
# it succeeded OR failed (fork PR, *-reviewed-for-compile, workflow_dispatch
|
||||
# skip_tests) — it posts NOTHING, so no build-gate context exists for the head
|
||||
# SHA and the PR stays blocked at "Expected" with no red X. (A compile-only run
|
||||
# ran no tests, so there is nothing meaningful to fail on; it simply hasn't met
|
||||
# the gate.) Merge qualification for fork code comes only from a maintainer
|
||||
# pushing a *-reviewed-for-build tag (full trusted build with tests), which
|
||||
# posts success/failure. compile-gate (advisory) still shows compile pass/fail.
|
||||
#
|
||||
# The status is posted against the PR HEAD sha on pull_request events
|
||||
# (github.sha there is the ephemeral refs/pull/N/merge commit, which the
|
||||
# ruleset does not evaluate) and github.sha otherwise. needs:[changes, build]
|
||||
# + if: always() so the trivial-change path (build skipped) is still evaluated.
|
||||
post-build-gate-status:
|
||||
permissions:
|
||||
statuses: write
|
||||
needs: [changes, build]
|
||||
if: always()
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- run: |
|
||||
if [[ "${{ needs.build.result }}" == "success" && "${{ needs.build.outputs.skip_tests }}" != "true" ]]; then
|
||||
echo "Full build with tests passed."
|
||||
elif [[ "${{ needs.build.result }}" == "skipped" && "${{ needs.changes.outputs.should_run }}" == "false" ]]; then
|
||||
echo "No relevant changes — skipping is OK."
|
||||
elif [[ "${{ needs.build.result }}" == "success" && "${{ needs.build.outputs.skip_tests }}" == "true" ]]; then
|
||||
echo "Compile-only build succeeded but tests did NOT run — this does not"
|
||||
echo "satisfy the required check. Push a *-reviewed-for-build tag at the"
|
||||
echo "reviewed SHA to run the full build with tests."
|
||||
exit 1
|
||||
- name: Post build-gate commit status
|
||||
env:
|
||||
GH_TOKEN: ${{ github.token }}
|
||||
BUILD_RESULT: ${{ needs.build.result }}
|
||||
SKIP_TESTS: ${{ needs.build.outputs.skip_tests }}
|
||||
SHOULD_RUN: ${{ needs.changes.outputs.should_run }}
|
||||
GATE_SHA: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.sha }}
|
||||
RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
|
||||
run: |
|
||||
STATE=""
|
||||
if [[ "$BUILD_RESULT" == "success" && "$SKIP_TESTS" != "true" ]]; then
|
||||
STATE=success; REASON="Full build with tests passed."
|
||||
elif [[ "$BUILD_RESULT" == "skipped" && "$SHOULD_RUN" == "false" ]]; then
|
||||
STATE=success; REASON="No relevant changes — nothing to build."
|
||||
elif [[ "$BUILD_RESULT" != "success" && "$BUILD_RESULT" != "skipped" && "$SKIP_TESTS" != "true" ]]; then
|
||||
STATE=failure; REASON="Full build with tests failed."
|
||||
fi
|
||||
if [[ -n "$STATE" ]]; then
|
||||
echo "Posting build-gate=$STATE on $GATE_SHA: $REASON"
|
||||
gh api "repos/${GITHUB_REPOSITORY}/statuses/${GATE_SHA}" \
|
||||
-f state="$STATE" \
|
||||
-f context=build-gate \
|
||||
-f description="$REASON" \
|
||||
-f target_url="$RUN_URL"
|
||||
else
|
||||
echo "Build failed or was cancelled."
|
||||
exit 1
|
||||
echo "Not posting build-gate: build.result=$BUILD_RESULT skip_tests=$SKIP_TESTS should_run=$SHOULD_RUN"
|
||||
echo "The build-gate context stays absent, so the PR remains un-mergeable"
|
||||
echo "at 'Expected' without a red X — this covers a successful OR failed"
|
||||
echo "compile-only run (e.g. a fork PR, where no tests ran so there is"
|
||||
echo "nothing to fail on). A *-reviewed-for-build tag runs the full build"
|
||||
echo "with tests and posts success (or failure) to the build-gate context."
|
||||
fi
|
||||
|
||||
# compile-gate is an ADVISORY (non-required) check. It reflects only whether
|
||||
|
||||
Reference in New Issue
Block a user