From 2196c775f970e2d0a00bfa31e1df3975b2bd8b3d Mon Sep 17 00:00:00 2001 From: Sean Kirby Date: Thu, 2 Jul 2026 16:20:31 -0400 Subject: [PATCH 01/18] SPAR-348: Feat: add unit-tested check-run evaluators for CI gating Extract check_runs_incomplete_count and check_runs_failures into a sourced, network-free helper with a zero-dependency bash test harness. These evaluate the /commits/{sha}/check-runs payload so the CI-wait loop can gate on GitHub Actions check-runs, not just the legacy combined commit-status API. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01CUQaePmRdFCZU5tNqBdTps --- ci_checks.sh | 23 +++++++++++++++++++++++ test/ci_checks_test.sh | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+) create mode 100644 ci_checks.sh create mode 100644 test/ci_checks_test.sh diff --git a/ci_checks.sh b/ci_checks.sh new file mode 100644 index 0000000..a7b02e1 --- /dev/null +++ b/ci_checks.sh @@ -0,0 +1,23 @@ +#!/usr/bin/env bash +# Pure helpers for evaluating CI state on a commit. No network, no globals: +# every input is an argument, so these are unit-testable with fixtures. + +# Check-run conclusions we treat as passing. Anything else on a completed run +# (failure, timed_out, cancelled, action_required, stale, or null/unknown) +# blocks the merge. +CI_OK_CONCLUSIONS='["success","neutral","skipped"]' + +# Count check-runs not yet finished. $1 = /commits/{sha}/check-runs payload. +check_runs_incomplete_count() { + jq -r '[.check_runs[] | select(.status != "completed")] | length' <<<"$1" +} + +# "name: conclusion" for each completed check-run whose conclusion is not +# acceptable. Empty output = all completed runs passed. $1 = same payload. +check_runs_failures() { + jq -r --argjson ok "$CI_OK_CONCLUSIONS" \ + '.check_runs[] + | select(.status == "completed") + | select(.conclusion as $c | ($ok | index($c)) | not) + | "\(.name): \(.conclusion // "none")"' <<<"$1" +} diff --git a/test/ci_checks_test.sh b/test/ci_checks_test.sh new file mode 100644 index 0000000..61d9ffd --- /dev/null +++ b/test/ci_checks_test.sh @@ -0,0 +1,42 @@ +#!/usr/bin/env bash +# Zero-dependency tests for ci_checks.sh (needs bash + jq). Run: bash test/ci_checks_test.sh +set -uo pipefail +cd "$(dirname "$0")/.." || exit 1 +source ./ci_checks.sh + +fail=0 +assert_eq() { # $1=desc $2=expected $3=actual + if [[ "$2" == "$3" ]]; then echo "ok - $1" + else echo "FAIL - $1"; echo " expected: [$2]"; echo " actual: [$3]"; fail=1; fi +} + +ALL_GREEN='{"total_count":4,"check_runs":[ + {"name":"test","status":"completed","conclusion":"success"}, + {"name":"e2e","status":"completed","conclusion":"success"}, + {"name":"client_test","status":"completed","conclusion":"success"}, + {"name":"auto-merge","status":"completed","conclusion":"skipped"}]}' +RUNNING='{"total_count":2,"check_runs":[ + {"name":"test","status":"completed","conclusion":"success"}, + {"name":"e2e","status":"in_progress","conclusion":null}]}' +FAILED='{"total_count":2,"check_runs":[ + {"name":"test","status":"completed","conclusion":"success"}, + {"name":"e2e","status":"completed","conclusion":"failure"}]}' +CANCELLED='{"total_count":1,"check_runs":[ + {"name":"e2e","status":"completed","conclusion":"cancelled"}]}' +NULL_CONCL='{"total_count":1,"check_runs":[ + {"name":"weird","status":"completed","conclusion":null}]}' +NEUTRAL='{"total_count":1,"check_runs":[ + {"name":"advisory","status":"completed","conclusion":"neutral"}]}' +EMPTY='{"total_count":0,"check_runs":[]}' + +assert_eq "all green: 0 incomplete" "0" "$(check_runs_incomplete_count "$ALL_GREEN")" +assert_eq "all green: no failures" "" "$(check_runs_failures "$ALL_GREEN")" +assert_eq "running: 1 incomplete" "1" "$(check_runs_incomplete_count "$RUNNING")" +assert_eq "failed: e2e reported" "e2e: failure" "$(check_runs_failures "$FAILED")" +assert_eq "cancelled: e2e reported" "e2e: cancelled" "$(check_runs_failures "$CANCELLED")" +assert_eq "null conclusion is a failure" "weird: none" "$(check_runs_failures "$NULL_CONCL")" +assert_eq "neutral passes" "" "$(check_runs_failures "$NEUTRAL")" +assert_eq "empty: 0 incomplete" "0" "$(check_runs_incomplete_count "$EMPTY")" +assert_eq "empty: no failures" "" "$(check_runs_failures "$EMPTY")" + +exit $fail From 8422e523d3fc68154c726420630a591fed2e66f2 Mon Sep 17 00:00:00 2001 From: Sean Kirby Date: Thu, 2 Jul 2026 16:21:56 -0400 Subject: [PATCH 02/18] SPAR-348: Feat: wait for GitHub Actions check-runs before merging The CI-wait loop polled only the legacy combined commit-status API, which reflects Buildkite but is blind to GitHub Actions check-runs. In the spark monorepo that let /integrate merge a SensrTrxMES PR as soon as the Buildkite no-op went green, without waiting on the SFac suite (test/client_test/e2e). Add a second gate: after the legacy status resolves, wait for all check-runs on the rebased commit to complete, and fail unless every conclusion is success/neutral/skipped. Self-conditional (PackManager-only PRs have no SFac check-runs, so they are not gated) and race-safe (the pending-status guard plus an initial settle keep the loop alive until check-runs register). Bump banner to v2.0.0. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01CUQaePmRdFCZU5tNqBdTps --- Dockerfile | 1 + entrypoint.sh | 45 ++++++++++++++++++++++++++++++++++++--------- 2 files changed, 37 insertions(+), 9 deletions(-) diff --git a/Dockerfile b/Dockerfile index 639f355..e6ab333 100644 --- a/Dockerfile +++ b/Dockerfile @@ -11,5 +11,6 @@ LABEL "com.github.actions.color"="purple" RUN apk --no-cache add jq bash curl git git-lfs +ADD ci_checks.sh /ci_checks.sh ADD entrypoint.sh /entrypoint.sh ENTRYPOINT ["/entrypoint.sh"] diff --git a/entrypoint.sh b/entrypoint.sh index 3de88e5..3efa0ae 100755 --- a/entrypoint.sh +++ b/entrypoint.sh @@ -2,7 +2,7 @@ set -e -echo "Integrate Action v1.1.1" +echo "Integrate Action v2.0.0" # Workaround until new Actions support neutral strategy # See how it was before: https://developer.github.com/actions/creating-github-actions/accessing-the-runtime-environment/#exit-codes-and-statuses @@ -11,6 +11,9 @@ NEUTRAL_EXIT_CODE=0 # since https://github.blog/2022-04-12-git-security-vulnerability-announced/ git config --global --add safe.directory /github/workspace +# shellcheck source=ci_checks.sh +source /ci_checks.sh + # Skip if not a PR echo "Checking if issue is a pull request..." (jq -r ".issue.pull_request.url" "$GITHUB_EVENT_PATH") || exit $NEUTRAL_EXIT_CODE @@ -98,20 +101,44 @@ git push --force-with-lease HEAD_BRANCH_HEAD=$(git rev-parse HEAD) echo "(Potentially) Rebased commit hash of HEAD is: $HEAD_BRANCH_HEAD" -# Poll for CI status +# Give Buildkite and GitHub Actions a moment to register their status/check-runs +# on the freshly force-pushed commit before we trust an empty result set. +sleep 45 + +# Wait for BOTH CI systems to report on the rebased commit: +# - PackManager CI (Buildkite) -> legacy commit status (/status) +# - SFac CI (GitHub Actions) -> check-runs (/check-runs), invisible to /status while true; do - sleep 10 + status_json=$(curl -s -H "${AUTH_HEADER}" -H "${API_HEADER}" "${URI}/repos/$GITHUB_REPOSITORY/commits/$HEAD_BRANCH_HEAD/status") + STATUS_STATE=$(echo "$status_json" | jq -r ".state") + + if [[ "$STATUS_STATE" == "pending" ]]; then + echo "Polling for CI: legacy statuses still pending..." + sleep 10 + continue + fi - LAST_STATUS=$(curl -s -H "${AUTH_HEADER}" -H "${API_HEADER}" "${URI}/repos/$GITHUB_REPOSITORY/commits/$HEAD_BRANCH_HEAD/status" | jq -r ".state") + check_runs_json=$(curl -s -H "${AUTH_HEADER}" -H "${API_HEADER}" "${URI}/repos/$GITHUB_REPOSITORY/commits/$HEAD_BRANCH_HEAD/check-runs") + incomplete=$(check_runs_incomplete_count "$check_runs_json") - if [[ $LAST_STATUS != "pending" ]]; then - break + if [[ "$incomplete" -gt 0 ]]; then + echo "Polling for CI: $incomplete check-run(s) still running..." + sleep 10 + continue fi - echo "Polling for CI build completion..." + + break done -if [[ $LAST_STATUS != "success" ]]; then - echo "CI did not pass for branch $HEAD_BRANCH and HEAD commit $HEAD_BRANCH_HEAD. Cancelling integration." +if [[ "$STATUS_STATE" != "success" ]]; then + echo "CI did not pass (legacy status = $STATUS_STATE) for $HEAD_BRANCH @ $HEAD_BRANCH_HEAD. Cancelling integration." + exit 1 +fi + +failed_runs=$(check_runs_failures "$check_runs_json") +if [[ -n "$failed_runs" ]]; then + echo "CI did not pass. Failing check-runs for $HEAD_BRANCH @ $HEAD_BRANCH_HEAD:" + echo "$failed_runs" exit 1 fi From e7edc6de5963e49003469a11c84603a4b6841bcc Mon Sep 17 00:00:00 2001 From: Sean Kirby Date: Thu, 2 Jul 2026 16:22:36 -0400 Subject: [PATCH 03/18] SPAR-348: Ci: run check-run evaluator tests on push and PR Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01CUQaePmRdFCZU5tNqBdTps --- .github/workflows/test.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 .github/workflows/test.yml diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 0000000..337574f --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,12 @@ +name: Test +on: + push: + branches: [ master ] + pull_request: +jobs: + ci_checks: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Run ci_checks unit tests + run: bash test/ci_checks_test.sh From 71fae7fb615c556e63d9bf71a23360809c53e49c Mon Sep 17 00:00:00 2001 From: Sean Kirby Date: Thu, 2 Jul 2026 16:23:00 -0400 Subject: [PATCH 04/18] SPAR-348: Docs: document check-run gating and versioning Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01CUQaePmRdFCZU5tNqBdTps --- README.md | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 9907a70..6ad0915 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,10 @@ Fork of the `cirrus-actions/rebase` repo for integrating a PR. Supports two commands: -- `/integrate` -- Rebases, waits for CI, and merges the PR. +- `/integrate` -- Rebases, waits for CI, and merges the PR. "CI" means both the + legacy commit statuses (e.g. Buildkite) **and** all GitHub Actions check-runs + on the rebased commit; the merge proceeds only when every one has completed and + passed (a `success`, `neutral`, or `skipped` conclusion). - `/hotfix` -- Same as integrate, but appends `[skip tests]` to the merge commit message. # Example Usage @@ -45,3 +48,12 @@ Then on a PR, type `/integrate` or `/hotfix` into the comments section. Using `/ This will fail if the HEAD branch is not rebaseable on top of the BASE branch of the PR and the HEAD branch needs to be rebased. +# Versioning + +This action is released as git tags. Reference a tag for stable behavior, e.g. `nulogy/integrate-action@v2.0.0`. + +- `v2.0.0` -- waits for GitHub Actions check-runs in addition to legacy commit statuses before merging. Use this in repos whose CI runs (partly) on GitHub Actions, e.g. monorepos. +- `v1.1.1` -- legacy behavior: waits only on the combined commit-status API (check-runs are ignored). Pin this if you rely on the old behavior. + +`@master` tracks the latest release. + From d5300a89a0e83a2715ed9999c96ea46c738179e5 Mon Sep 17 00:00:00 2001 From: Sean Kirby Date: Thu, 2 Jul 2026 18:10:12 -0400 Subject: [PATCH 05/18] SPAR-348: Fix: drop the pre-loop settle, restore the 10s poll cadence The sleep 45 settle was inert for spark: the combined /status reads "pending" continuously from the force-push until Buildkite's build goes terminal (zero statuses => pending, then buildkite/packmanager=pending held for ~8-9 min), while SFac check-runs register within ~5-10s. So the first non-pending status read is minutes out whether the first poll is at +10s or +45s -- the settle could not move the merge earlier. Restore sleep 10 at the top of the loop (the original v1.1.x cadence); the pending guard, not a fixed sleep, is what holds the loop until check-runs are present. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01CUQaePmRdFCZU5tNqBdTps --- entrypoint.sh | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/entrypoint.sh b/entrypoint.sh index 3efa0ae..33b5070 100755 --- a/entrypoint.sh +++ b/entrypoint.sh @@ -101,29 +101,26 @@ git push --force-with-lease HEAD_BRANCH_HEAD=$(git rev-parse HEAD) echo "(Potentially) Rebased commit hash of HEAD is: $HEAD_BRANCH_HEAD" -# Give Buildkite and GitHub Actions a moment to register their status/check-runs -# on the freshly force-pushed commit before we trust an empty result set. -sleep 45 - -# Wait for BOTH CI systems to report on the rebased commit: +# Wait for BOTH CI systems to report on the rebased commit ($HEAD_BRANCH_HEAD): # - PackManager CI (Buildkite) -> legacy commit status (/status) # - SFac CI (GitHub Actions) -> check-runs (/check-runs), invisible to /status +# No pre-loop settle is needed: Buildkite posts a pending status within seconds +# of the force-push and holds it for minutes (until its build finishes), so this +# loop keeps waiting long after the Actions check-runs have registered. while true; do + sleep 10 + status_json=$(curl -s -H "${AUTH_HEADER}" -H "${API_HEADER}" "${URI}/repos/$GITHUB_REPOSITORY/commits/$HEAD_BRANCH_HEAD/status") STATUS_STATE=$(echo "$status_json" | jq -r ".state") - if [[ "$STATUS_STATE" == "pending" ]]; then echo "Polling for CI: legacy statuses still pending..." - sleep 10 continue fi check_runs_json=$(curl -s -H "${AUTH_HEADER}" -H "${API_HEADER}" "${URI}/repos/$GITHUB_REPOSITORY/commits/$HEAD_BRANCH_HEAD/check-runs") incomplete=$(check_runs_incomplete_count "$check_runs_json") - if [[ "$incomplete" -gt 0 ]]; then echo "Polling for CI: $incomplete check-run(s) still running..." - sleep 10 continue fi From e7f52465376e65bb4b2521e1cd28aa949ccee4cf Mon Sep 17 00:00:00 2001 From: Sean Kirby Date: Thu, 2 Jul 2026 19:06:02 -0400 Subject: [PATCH 06/18] SPAR-348: Feat: evaluate latest run per name; add required-checks + path helpers Harden the pure check-run helpers ahead of wiring them into the CI-wait loop: - Evaluate only the LATEST run per check name (max started_at), so a superseded or re-run check (e.g. concurrency-cancelled then re-created) no longer masks or fails the current run. - check_runs_payload_valid: distinguish a real check-runs body from a transient API error payload, so the caller can retry instead of aborting. - required_checks_pending / required_checks_failures: gate on a named set of expected checks (present + completed + acceptable), closing the hole where an empty check-run set was treated as "passed". - any_path_has_prefix: decide whether a PR is in scope for required checks. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01CUQaePmRdFCZU5tNqBdTps --- ci_checks.sh | 82 +++++++++++++++++++++++++++++++++++----- test/ci_checks_test.sh | 86 ++++++++++++++++++++++++++++++++++++------ 2 files changed, 147 insertions(+), 21 deletions(-) diff --git a/ci_checks.sh b/ci_checks.sh index a7b02e1..daf6f04 100644 --- a/ci_checks.sh +++ b/ci_checks.sh @@ -1,23 +1,85 @@ #!/usr/bin/env bash -# Pure helpers for evaluating CI state on a commit. No network, no globals: +# Pure helpers for evaluating CI state on a commit. No network and no globals: # every input is an argument, so these are unit-testable with fixtures. +# +# The "$1" argument is a GET /commits/{sha}/check-runs REST payload: +# {"total_count":N,"check_runs":[{name,status,conclusion,started_at},...]} +# GitHub can list SEVERAL runs for the same name (reruns, concurrency-cancelled +# then re-created). Every helper below evaluates only the LATEST run per name +# (max started_at) so a superseded run never masks or fails the current one. # Check-run conclusions we treat as passing. Anything else on a completed run # (failure, timed_out, cancelled, action_required, stale, or null/unknown) # blocks the merge. CI_OK_CONCLUSIONS='["success","neutral","skipped"]' -# Count check-runs not yet finished. $1 = /commits/{sha}/check-runs payload. +# True if $1 is a well-formed check-runs payload (has a .check_runs array), +# so callers can distinguish it from a transient API error body ({"message":...}) +# and keep polling instead of aborting. +check_runs_payload_valid() { + jq -e '(.check_runs | type) == "array"' <<<"$1" >/dev/null 2>&1 +} + +# Count latest-per-name check-runs that have not finished yet. check_runs_incomplete_count() { - jq -r '[.check_runs[] | select(.status != "completed")] | length' <<<"$1" + jq -r ' + [ .check_runs // [] | group_by(.name)[] | max_by(.started_at // "") ] + | map(select(.status != "completed")) | length + ' <<<"$1" } -# "name: conclusion" for each completed check-run whose conclusion is not -# acceptable. Empty output = all completed runs passed. $1 = same payload. +# "name: conclusion" for each latest-per-name completed check-run whose +# conclusion is not acceptable. Empty output = all completed runs passed. check_runs_failures() { - jq -r --argjson ok "$CI_OK_CONCLUSIONS" \ - '.check_runs[] - | select(.status == "completed") - | select(.conclusion as $c | ($ok | index($c)) | not) - | "\(.name): \(.conclusion // "none")"' <<<"$1" + jq -r --argjson ok "$CI_OK_CONCLUSIONS" ' + [ .check_runs // [] | group_by(.name)[] | max_by(.started_at // "") ][] + | select(.status == "completed") + | select(.conclusion as $c | ($ok | index($c)) | not) + | "\(.name): \(.conclusion // "none")" + ' <<<"$1" +} + +# Of the comma-separated required names in $2, print those NOT yet +# present-and-completed (absent from the commit, or latest run still running). +# Empty output = every required check has a completed latest run. Used to keep +# waiting until the expected checks actually show up (closes the race where an +# empty check-run set is mistaken for "all passed"). +required_checks_pending() { + jq -r --arg req "$2" ' + ( [ .check_runs // [] | group_by(.name)[] | max_by(.started_at // "") ] + | map({ (.name): . }) | add // {} ) as $byname + | ( $req | split(",") | map(select(length > 0)) )[] + | select( ($byname[.] // null) == null or $byname[.].status != "completed" ) + ' <<<"$1" +} + +# Of the comma-separated required names in $2, print "name: reason" for each that +# is absent ("missing") or whose latest completed conclusion is not acceptable. +# Empty output = every required check is present and passed. +required_checks_failures() { + jq -r --arg req "$2" --argjson ok "$CI_OK_CONCLUSIONS" ' + ( [ .check_runs // [] | group_by(.name)[] | max_by(.started_at // "") ] + | map({ (.name): . }) | add // {} ) as $byname + | ( $req | split(",") | map(select(length > 0)) )[] + | . as $name + | ($byname[$name] // null) as $run + | if $run == null then "\($name): missing" + elif ($ok | index($run.conclusion)) then empty + else "\($name): \($run.conclusion // "incomplete")" + end + ' <<<"$1" +} + +# True if any newline-separated path in $1 starts with any comma-separated +# prefix in $2. Matching is literal prefix (not glob), so "SensrTrxMES/" matches +# "SensrTrxMES/x" but not "SensrTrxMESX/x". +any_path_has_prefix() { + local paths="$1" prefixes_csv="$2" prefix p + while IFS= read -r prefix; do + [[ -z "$prefix" ]] && continue + while IFS= read -r p; do + [[ -n "$p" && "$p" == "$prefix"* ]] && return 0 + done <<<"$paths" + done < <(echo "$prefixes_csv" | tr ',' '\n') + return 1 } diff --git a/test/ci_checks_test.sh b/test/ci_checks_test.sh index 61d9ffd..9062080 100644 --- a/test/ci_checks_test.sh +++ b/test/ci_checks_test.sh @@ -9,26 +9,44 @@ assert_eq() { # $1=desc $2=expected $3=actual if [[ "$2" == "$3" ]]; then echo "ok - $1" else echo "FAIL - $1"; echo " expected: [$2]"; echo " actual: [$3]"; fail=1; fi } +assert_ok() { # $1=desc ; runs $2.. as a command, expects exit 0 + local desc="$1"; shift + if "$@" >/dev/null 2>&1; then echo "ok - $desc"; else echo "FAIL - $desc (expected exit 0)"; fail=1; fi +} +assert_notok() { # $1=desc ; runs $2.. expects non-zero exit + local desc="$1"; shift + if "$@" >/dev/null 2>&1; then echo "FAIL - $desc (expected non-zero exit)"; fail=1; else echo "ok - $desc"; fi +} ALL_GREEN='{"total_count":4,"check_runs":[ - {"name":"test","status":"completed","conclusion":"success"}, - {"name":"e2e","status":"completed","conclusion":"success"}, - {"name":"client_test","status":"completed","conclusion":"success"}, - {"name":"auto-merge","status":"completed","conclusion":"skipped"}]}' + {"name":"test","status":"completed","conclusion":"success","started_at":"2026-07-02T17:07:15Z"}, + {"name":"e2e","status":"completed","conclusion":"success","started_at":"2026-07-02T17:07:17Z"}, + {"name":"client_test","status":"completed","conclusion":"success","started_at":"2026-07-02T17:07:15Z"}, + {"name":"auto-merge","status":"completed","conclusion":"skipped","started_at":"2026-07-02T17:07:10Z"}]}' RUNNING='{"total_count":2,"check_runs":[ - {"name":"test","status":"completed","conclusion":"success"}, - {"name":"e2e","status":"in_progress","conclusion":null}]}' + {"name":"test","status":"completed","conclusion":"success","started_at":"2026-07-02T17:07:15Z"}, + {"name":"e2e","status":"in_progress","conclusion":null,"started_at":"2026-07-02T17:07:17Z"}]}' FAILED='{"total_count":2,"check_runs":[ - {"name":"test","status":"completed","conclusion":"success"}, - {"name":"e2e","status":"completed","conclusion":"failure"}]}' + {"name":"test","status":"completed","conclusion":"success","started_at":"2026-07-02T17:07:15Z"}, + {"name":"e2e","status":"completed","conclusion":"failure","started_at":"2026-07-02T17:07:17Z"}]}' CANCELLED='{"total_count":1,"check_runs":[ - {"name":"e2e","status":"completed","conclusion":"cancelled"}]}' + {"name":"e2e","status":"completed","conclusion":"cancelled","started_at":"2026-07-02T17:07:17Z"}]}' NULL_CONCL='{"total_count":1,"check_runs":[ - {"name":"weird","status":"completed","conclusion":null}]}' + {"name":"weird","status":"completed","conclusion":null,"started_at":"2026-07-02T17:07:17Z"}]}' NEUTRAL='{"total_count":1,"check_runs":[ - {"name":"advisory","status":"completed","conclusion":"neutral"}]}' + {"name":"advisory","status":"completed","conclusion":"neutral","started_at":"2026-07-02T17:07:17Z"}]}' EMPTY='{"total_count":0,"check_runs":[]}' +# A superseded run (older, cancelled) plus its rerun (newer, success) for the same name. +RERUN='{"total_count":2,"check_runs":[ + {"name":"e2e","status":"completed","conclusion":"cancelled","started_at":"2026-07-02T10:00:00Z"}, + {"name":"e2e","status":"completed","conclusion":"success","started_at":"2026-07-02T10:30:00Z"}]}' +# Same name, older success but newer run still running -> latest is incomplete. +RERUN_RUNNING='{"total_count":2,"check_runs":[ + {"name":"e2e","status":"completed","conclusion":"success","started_at":"2026-07-02T10:00:00Z"}, + {"name":"e2e","status":"in_progress","conclusion":null,"started_at":"2026-07-02T10:30:00Z"}]}' +ERROR_PAYLOAD='{"message":"Not Found","documentation_url":"https://docs.github.com/rest"}' +echo "# incomplete-count / failures (latest run per name)" assert_eq "all green: 0 incomplete" "0" "$(check_runs_incomplete_count "$ALL_GREEN")" assert_eq "all green: no failures" "" "$(check_runs_failures "$ALL_GREEN")" assert_eq "running: 1 incomplete" "1" "$(check_runs_incomplete_count "$RUNNING")" @@ -39,4 +57,50 @@ assert_eq "neutral passes" "" "$(check_runs_failures "$NEUTRAL")" assert_eq "empty: 0 incomplete" "0" "$(check_runs_incomplete_count "$EMPTY")" assert_eq "empty: no failures" "" "$(check_runs_failures "$EMPTY")" +echo "# reruns: only the latest run per name counts" +assert_eq "rerun success supersedes cancelled: no failures" "" "$(check_runs_failures "$RERUN")" +assert_eq "rerun success supersedes cancelled: 0 incomplete" "0" "$(check_runs_incomplete_count "$RERUN")" +assert_eq "rerun still running: 1 incomplete" "1" "$(check_runs_incomplete_count "$RERUN_RUNNING")" + +echo "# payload validity guard" +assert_ok "valid check-runs payload" check_runs_payload_valid "$ALL_GREEN" +assert_ok "empty check-runs payload valid" check_runs_payload_valid "$EMPTY" +assert_notok "error payload invalid" check_runs_payload_valid "$ERROR_PAYLOAD" +assert_notok "empty string invalid" check_runs_payload_valid "" + +echo "# required_checks_pending (names absent or latest run not completed)" +assert_eq "required all present+completed: none pending" "" \ + "$(required_checks_pending "$ALL_GREEN" "test,client_test,e2e")" +assert_eq "required e2e still running: e2e pending" "e2e" \ + "$(required_checks_pending "$RUNNING" "test,e2e")" +assert_eq "required missing name reported pending" "missingcheck" \ + "$(required_checks_pending "$ALL_GREEN" "missingcheck")" +assert_eq "required rerun still running: e2e pending" "e2e" \ + "$(required_checks_pending "$RERUN_RUNNING" "e2e")" +assert_eq "empty required list: nothing pending" "" \ + "$(required_checks_pending "$ALL_GREEN" "")" + +echo "# required_checks_failures (absent or bad-conclusion required names)" +assert_eq "required all green: no failures" "" \ + "$(required_checks_failures "$ALL_GREEN" "test,client_test,e2e")" +assert_eq "required missing: reported missing" "gone: missing" \ + "$(required_checks_failures "$ALL_GREEN" "gone")" +assert_eq "required failed: reported" "e2e: failure" \ + "$(required_checks_failures "$FAILED" "e2e")" +assert_eq "required cancelled (no rerun): reported" "e2e: cancelled" \ + "$(required_checks_failures "$CANCELLED" "e2e")" +assert_eq "required cancelled superseded by rerun success: no failure" "" \ + "$(required_checks_failures "$RERUN" "e2e")" +assert_eq "empty required list: no failures" "" \ + "$(required_checks_failures "$ALL_GREEN" "")" + +echo "# any_path_has_prefix" +FILES_MIXED=$'SensrTrxMES/app/x.js\nPackManager/db/schema.rb' +FILES_PM_ONLY=$'PackManager/db/schema.rb\n.github/workflows/integrate.yml' +assert_ok "matches SensrTrxMES/ prefix" any_path_has_prefix "$FILES_MIXED" "SensrTrxMES/" +assert_notok "PM-only does not match SensrTrxMES/" any_path_has_prefix "$FILES_PM_ONLY" "SensrTrxMES/" +assert_ok "matches one of several prefixes" any_path_has_prefix "$FILES_PM_ONLY" "SensrTrxMES/,PackManager/" +assert_notok "empty prefix list never matches" any_path_has_prefix "$FILES_MIXED" "" +assert_notok "prefix respects the trailing slash" any_path_has_prefix $'SensrTrxMESX/x.js' "SensrTrxMES/" + exit $fail From 2a5a655402837de483a8db4dbef8948b14b3b910 Mon Sep 17 00:00:00 2001 From: Sean Kirby Date: Thu, 2 Jul 2026 19:06:03 -0400 Subject: [PATCH 07/18] SPAR-348: Fix: timeout, error-payload retries, required checks, sha-guarded merge Wire the hardened helpers into the CI-wait loop and close the remaining holes found in review: - Wall-clock timeout (CI_WAIT_TIMEOUT_SECONDS, default 4h) so a never-terminal Buildkite status or a stuck check-run fails cleanly instead of spinning to the 6h job timeout. - Retry (don't abort) when /status or /check-runs returns a transient error payload; a garbage body no longer kills an otherwise-green integration. - REQUIRED_CHECK_RUNS (+ optional REQUIRED_CHECK_RUNS_PATHS scoping): require a named set of checks to be present and pass, instead of trusting whatever check-runs happen to be on the commit. Path scoping keeps PRs that don't touch the relevant product from waiting on checks that never run. - Merge with sha=$HEAD_BRANCH_HEAD so only the CI-validated commit can land, and drop the redundant post-CI rebase (it never re-fetched base, so it was a no-op that only risked merging an unvalidated commit if a fetch were ever added). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01CUQaePmRdFCZU5tNqBdTps --- entrypoint.sh | 133 ++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 108 insertions(+), 25 deletions(-) diff --git a/entrypoint.sh b/entrypoint.sh index 33b5070..4acd4b6 100755 --- a/entrypoint.sh +++ b/entrypoint.sh @@ -46,6 +46,21 @@ URI=https://api.github.com API_HEADER="Accept: application/vnd.github.v3+json" AUTH_HEADER="Authorization: token $GITHUB_TOKEN" +# CI-wait configuration (all optional; defaults preserve prior behavior). +# Max seconds to wait for CI before giving up (fail-closed). Must exceed the +# slowest check (spark's SFac e2e has run ~2.6h) yet stay under this job's own +# timeout (GitHub's default is 6h). +CI_WAIT_TIMEOUT_SECONDS="${CI_WAIT_TIMEOUT_SECONDS:-14400}" +# Comma-separated check-run names that MUST be present and pass before merging. +# When empty, the action instead gates on every check-run present on the commit +# (it cannot otherwise tell which checks are expected). +REQUIRED_CHECK_RUNS="${REQUIRED_CHECK_RUNS:-}" +# Comma-separated path prefixes. When set, REQUIRED_CHECK_RUNS is enforced only +# if the PR changes a file under one of these prefixes, so a PR that doesn't +# touch the relevant product isn't blocked waiting for checks that never run. +# When empty but REQUIRED_CHECK_RUNS is set, the required checks always apply. +REQUIRED_CHECK_RUNS_PATHS="${REQUIRED_CHECK_RUNS_PATHS:-}" + USER_URL=$(jq -r ".comment.user.url" "$GITHUB_EVENT_PATH") user_resp=$(curl -X GET -s -H "${API_HEADER}" -H "${AUTH_HEADER}" "${USER_URL}") @@ -101,50 +116,112 @@ git push --force-with-lease HEAD_BRANCH_HEAD=$(git rev-parse HEAD) echo "(Potentially) Rebased commit hash of HEAD is: $HEAD_BRANCH_HEAD" +# Does the PR touch any of the given comma-separated path prefixes? Paginates +# the PR file list and stops at the first match. +pr_touches_paths() { + local prefixes_csv="$1" page=1 resp count files + while : ; do + resp=$(curl -s -H "${AUTH_HEADER}" -H "${API_HEADER}" "${PR_URL}/files?per_page=100&page=$page") + if ! jq -e 'type == "array"' <<<"$resp" >/dev/null 2>&1; then + return 1 + fi + count=$(jq 'length' <<<"$resp") + if [[ "$count" -eq 0 ]]; then + return 1 + fi + files=$(jq -r '.[].filename' <<<"$resp") + if any_path_has_prefix "$files" "$prefixes_csv"; then + return 0 + fi + if [[ "$count" -lt 100 ]]; then + return 1 + fi + page=$((page + 1)) + done +} + +# Enforce REQUIRED_CHECK_RUNS on this PR only when in scope (see config above). +required_active="false" +if [[ -n "$REQUIRED_CHECK_RUNS" ]]; then + if [[ -z "$REQUIRED_CHECK_RUNS_PATHS" ]] || pr_touches_paths "$REQUIRED_CHECK_RUNS_PATHS"; then + required_active="true" + fi +fi +if [[ "$required_active" == "true" ]]; then + echo "Required check-runs enforced for this PR: $REQUIRED_CHECK_RUNS" +else + echo "No required check-runs in scope; gating on every check-run present on the commit." +fi + # Wait for BOTH CI systems to report on the rebased commit ($HEAD_BRANCH_HEAD): # - PackManager CI (Buildkite) -> legacy commit status (/status) # - SFac CI (GitHub Actions) -> check-runs (/check-runs), invisible to /status # No pre-loop settle is needed: Buildkite posts a pending status within seconds # of the force-push and holds it for minutes (until its build finishes), so this # loop keeps waiting long after the Actions check-runs have registered. +deadline=$(( $(date +%s) + CI_WAIT_TIMEOUT_SECONDS )) while true; do sleep 10 + if (( $(date +%s) > deadline )); then + echo "Timed out after ${CI_WAIT_TIMEOUT_SECONDS}s waiting for CI on $HEAD_BRANCH @ $HEAD_BRANCH_HEAD. Cancelling integration." + exit 1 + fi + status_json=$(curl -s -H "${AUTH_HEADER}" -H "${API_HEADER}" "${URI}/repos/$GITHUB_REPOSITORY/commits/$HEAD_BRANCH_HEAD/status") STATUS_STATE=$(echo "$status_json" | jq -r ".state") - if [[ "$STATUS_STATE" == "pending" ]]; then - echo "Polling for CI: legacy statuses still pending..." - continue - fi + case "$STATUS_STATE" in + success) ;; # legacy CI passed; check the check-runs next + failure|error) + echo "CI did not pass (legacy status = $STATUS_STATE) for $HEAD_BRANCH @ $HEAD_BRANCH_HEAD. Cancelling integration." + exit 1 ;; + *) # "pending", or "null" from a transient error body + echo "Polling for CI: legacy statuses not final yet ($STATUS_STATE)..." + continue ;; + esac check_runs_json=$(curl -s -H "${AUTH_HEADER}" -H "${API_HEADER}" "${URI}/repos/$GITHUB_REPOSITORY/commits/$HEAD_BRANCH_HEAD/check-runs") - incomplete=$(check_runs_incomplete_count "$check_runs_json") - if [[ "$incomplete" -gt 0 ]]; then - echo "Polling for CI: $incomplete check-run(s) still running..." + if ! check_runs_payload_valid "$check_runs_json"; then + echo "Polling for CI: check-runs endpoint returned no usable payload, retrying..." continue fi + if [[ "$required_active" == "true" ]]; then + pending=$(required_checks_pending "$check_runs_json" "$REQUIRED_CHECK_RUNS") + if [[ -n "$pending" ]]; then + echo "Polling for CI: waiting on required check-run(s): $(echo "$pending" | tr '\n' ' ')" + continue + fi + else + incomplete=$(check_runs_incomplete_count "$check_runs_json") + if [[ "$incomplete" -gt 0 ]]; then + echo "Polling for CI: $incomplete check-run(s) still running..." + continue + fi + fi + break done -if [[ "$STATUS_STATE" != "success" ]]; then - echo "CI did not pass (legacy status = $STATUS_STATE) for $HEAD_BRANCH @ $HEAD_BRANCH_HEAD. Cancelling integration." - exit 1 -fi - -failed_runs=$(check_runs_failures "$check_runs_json") -if [[ -n "$failed_runs" ]]; then - echo "CI did not pass. Failing check-runs for $HEAD_BRANCH @ $HEAD_BRANCH_HEAD:" - echo "$failed_runs" - exit 1 +# Reaching here means the legacy status is "success"; fail on any check-run problem. +if [[ "$required_active" == "true" ]]; then + required_failures=$(required_checks_failures "$check_runs_json" "$REQUIRED_CHECK_RUNS") + if [[ -n "$required_failures" ]]; then + echo "CI did not pass. Required check-run problem(s) for $HEAD_BRANCH @ $HEAD_BRANCH_HEAD:" + echo "$required_failures" + exit 1 + fi +else + failed_runs=$(check_runs_failures "$check_runs_json") + if [[ -n "$failed_runs" ]]; then + echo "CI did not pass. Failing check-runs for $HEAD_BRANCH @ $HEAD_BRANCH_HEAD:" + echo "$failed_runs" + exit 1 + fi fi -# Rebase -git checkout $HEAD_BRANCH -git rebase origin/$BASE_BRANCH -git push --force-with-lease - -# Hit the merge button +# Hit the merge button. Pass sha=$HEAD_BRANCH_HEAD so GitHub only merges if the +# branch head still matches the exact commit CI validated (a race push aborts). MERGE_COMMIT_TITLE="Merge branch '$HEAD_BRANCH' on behalf of $USER_FULL_NAME" if [[ "$ACTION_MODE" == "hotfix" ]]; then MERGE_COMMIT_TITLE="$MERGE_COMMIT_TITLE [skip tests]" @@ -170,11 +247,17 @@ if [[ $ADD_CHANGE_LOGS = "true" ]]; then JSON_STRING=$( jq -n \ --arg title "$MERGE_COMMIT_TITLE" \ --arg message "$MERGE_COMMIT_MESSAGE" \ - '{commit_title: $title, commit_message: $message}' ) + --arg sha "$HEAD_BRANCH_HEAD" \ + '{commit_title: $title, commit_message: $message, sha: $sha}' ) merge_resp=$(curl -X PUT -s -H "${AUTH_HEADER}" -H "${API_HEADER}" -d "$JSON_STRING" "${PR_URL}/merge") else - merge_resp=$(curl -X PUT -s -H "${AUTH_HEADER}" -H "${API_HEADER}" -d "{\"commit_title\":\"$MERGE_COMMIT_TITLE\"}" "${PR_URL}/merge") + JSON_STRING=$( jq -n \ + --arg title "$MERGE_COMMIT_TITLE" \ + --arg sha "$HEAD_BRANCH_HEAD" \ + '{commit_title: $title, sha: $sha}' ) + + merge_resp=$(curl -X PUT -s -H "${AUTH_HEADER}" -H "${API_HEADER}" -d "$JSON_STRING" "${PR_URL}/merge") fi if [[ $merge_resp != *"Pull Request successfully merged"* ]]; then From 7e7feb8f8203bd922a735bbc043525a2c853b532 Mon Sep 17 00:00:00 2001 From: Sean Kirby Date: Thu, 2 Jul 2026 19:06:57 -0400 Subject: [PATCH 08/18] SPAR-348: Docs: document CI_WAIT_TIMEOUT_SECONDS, REQUIRED_CHECK_RUNS[_PATHS], and the legacy-status requirement Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01CUQaePmRdFCZU5tNqBdTps --- README.md | 34 ++++++++++++++++++++++++++++++---- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 6ad0915..c6e6dd2 100644 --- a/README.md +++ b/README.md @@ -4,10 +4,12 @@ Fork of the `cirrus-actions/rebase` repo for integrating a PR. Supports two commands: -- `/integrate` -- Rebases, waits for CI, and merges the PR. "CI" means both the - legacy commit statuses (e.g. Buildkite) **and** all GitHub Actions check-runs - on the rebased commit; the merge proceeds only when every one has completed and - passed (a `success`, `neutral`, or `skipped` conclusion). +- `/integrate` -- Rebases, waits for CI, and merges the PR. "CI" means the legacy + commit statuses (e.g. Buildkite) **and** GitHub Actions check-runs on the + rebased commit. By default the merge proceeds only when the legacy status is + `success` and every check-run present has completed with a `success`, + `neutral`, or `skipped` conclusion. See [Configuration](#configuration) to + require a specific named set of checks instead. - `/hotfix` -- Same as integrate, but appends `[skip tests]` to the merge commit message. # Example Usage @@ -48,6 +50,30 @@ Then on a PR, type `/integrate` or `/hotfix` into the comments section. Using `/ This will fail if the HEAD branch is not rebaseable on top of the BASE branch of the PR and the HEAD branch needs to be rebased. +# Configuration + +All optional, passed via `env:` on the action step: + +| Variable | Default | Purpose | +|---|---|---| +| `GITHUB_TOKEN` | — | **Required.** Token allowed to merge into the PR's base branch. | +| `ADD_CHANGE_LOGS` | `false` | Collect `Change log:` PR comments into the merge commit message. | +| `CI_WAIT_TIMEOUT_SECONDS` | `14400` (4h) | Give up waiting for CI after this many seconds (fail, don't merge). Keep it above your slowest check and below the job's own timeout (GitHub's default is 6h). | +| `REQUIRED_CHECK_RUNS` | _(empty)_ | Comma-separated check-run names that must be **present and pass** before merging. When empty, the action gates on every check-run present on the commit. Set this to avoid trusting an empty/partial check-run set and to ignore unrelated/advisory checks. | +| `REQUIRED_CHECK_RUNS_PATHS` | _(empty)_ | Comma-separated path **prefixes**. When set, `REQUIRED_CHECK_RUNS` is enforced only if the PR changes a file under one of them — so a PR that doesn't touch the relevant product isn't blocked waiting for checks that never run. When empty, `REQUIRED_CHECK_RUNS` always applies. | + +Example (a monorepo whose SFac product's tests run as GitHub Actions): + +```yml + - uses: nulogy/integrate-action@v2.0.0 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_MERGING_TOKEN }} + REQUIRED_CHECK_RUNS: "test,client_test,e2e" + REQUIRED_CHECK_RUNS_PATHS: "SensrTrxMES/" +``` + +**Requirement:** the action waits on the legacy combined-status API and merges only when it reports `success`, so a repo must have **at least one legacy commit status** (e.g. Buildkite) that stays `pending` through its build. A pure-GitHub-Actions repo with no legacy statuses is not yet supported — the combined status reads `pending` indefinitely and the action will wait until it times out. + # Versioning This action is released as git tags. Reference a tag for stable behavior, e.g. `nulogy/integrate-action@v2.0.0`. From 03a0a15976b1bf99270eec94ac4408818204b963 Mon Sep 17 00:00:00 2001 From: Sean Kirby Date: Thu, 2 Jul 2026 19:32:57 -0400 Subject: [PATCH 09/18] SPAR-348: Fix: harden CI-wait against transient API failures (review findings) Address the adversarial review of the check-run gating: - HIGH: a transient non-JSON /status body or a curl transport failure aborted the whole integration under set -e (the retry path only covered JSON error bodies). Guard both curls with `|| { continue; }` + --max-time, and parse status with a `.state // "null"` fallback, so any transient blip retries instead of cancelling an in-flight, hours-long integration. - MEDIUM: pr_touches_paths now retries the /files fetch and fails CLOSED (treats an undeterminable scope as in-scope) instead of silently downgrading required-checks (Mode B) to the weaker all-present gate (Mode A). - MEDIUM: trim surrounding whitespace on REQUIRED_CHECK_RUNS entries so "test, e2e" doesn't wait to the timeout reporting present checks as missing. - LOW: pick the latest run per name by check-run id (then started_at), so a newer run with a null started_at can't be masked by an older one; ties are deterministic. - LOW: drop check-runs with a null name before keying by name, so a malformed element can't crash the required-checks gate under set -e. All confirmed via unit tests (latest-per-name/whitespace/null-name) and set -e repros under real bash. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01CUQaePmRdFCZU5tNqBdTps --- ci_checks.sh | 56 ++++++++++++++++++++++-------------------- entrypoint.sh | 35 ++++++++++++++++++++------ test/ci_checks_test.sh | 28 +++++++++++++++++++++ 3 files changed, 85 insertions(+), 34 deletions(-) diff --git a/ci_checks.sh b/ci_checks.sh index daf6f04..ed09693 100644 --- a/ci_checks.sh +++ b/ci_checks.sh @@ -3,18 +3,25 @@ # every input is an argument, so these are unit-testable with fixtures. # # The "$1" argument is a GET /commits/{sha}/check-runs REST payload: -# {"total_count":N,"check_runs":[{name,status,conclusion,started_at},...]} +# {"total_count":N,"check_runs":[{name,status,conclusion,started_at,id},...]} # GitHub can list SEVERAL runs for the same name (reruns, concurrency-cancelled -# then re-created). Every helper below evaluates only the LATEST run per name -# (max started_at) so a superseded run never masks or fails the current one. +# then re-created). Every helper evaluates only the LATEST run per name so a +# superseded run never masks or fails the current one. "Latest" = highest +# check-run id (monotonic at creation), then started_at as a tiebreak. # Check-run conclusions we treat as passing. Anything else on a completed run # (failure, timed_out, cancelled, action_required, stale, or null/unknown) # blocks the merge. CI_OK_CONCLUSIONS='["success","neutral","skipped"]' -# True if $1 is a well-formed check-runs payload (has a .check_runs array), -# so callers can distinguish it from a transient API error body ({"message":...}) +# jq prelude defining `latest_per_name`: check_runs reduced to the newest run per +# name, with unnamed runs dropped (a null name cannot be an object key). +_ci_jq_latest='def latest_per_name: + [ (.check_runs // []) | map(select(.name != null)) | group_by(.name)[] + | max_by([(.id // 0), (.started_at // "")]) ];' + +# True if $1 is a well-formed check-runs payload (has a .check_runs array), so +# callers can distinguish it from a transient API error body ({"message":...}) # and keep polling instead of aborting. check_runs_payload_valid() { jq -e '(.check_runs | type) == "array"' <<<"$1" >/dev/null 2>&1 @@ -22,45 +29,42 @@ check_runs_payload_valid() { # Count latest-per-name check-runs that have not finished yet. check_runs_incomplete_count() { - jq -r ' - [ .check_runs // [] | group_by(.name)[] | max_by(.started_at // "") ] - | map(select(.status != "completed")) | length + jq -r "$_ci_jq_latest"' + latest_per_name | map(select(.status != "completed")) | length ' <<<"$1" } # "name: conclusion" for each latest-per-name completed check-run whose # conclusion is not acceptable. Empty output = all completed runs passed. check_runs_failures() { - jq -r --argjson ok "$CI_OK_CONCLUSIONS" ' - [ .check_runs // [] | group_by(.name)[] | max_by(.started_at // "") ][] + jq -r --argjson ok "$CI_OK_CONCLUSIONS" "$_ci_jq_latest"' + latest_per_name[] | select(.status == "completed") | select(.conclusion as $c | ($ok | index($c)) | not) | "\(.name): \(.conclusion // "none")" ' <<<"$1" } -# Of the comma-separated required names in $2, print those NOT yet -# present-and-completed (absent from the commit, or latest run still running). -# Empty output = every required check has a completed latest run. Used to keep -# waiting until the expected checks actually show up (closes the race where an -# empty check-run set is mistaken for "all passed"). +# Of the comma-separated required names in $2 (surrounding whitespace trimmed), +# print those NOT yet present-and-completed (absent from the commit, or latest +# run still running). Empty output = every required check has a completed latest +# run. Used to keep waiting until the expected checks actually show up. required_checks_pending() { - jq -r --arg req "$2" ' - ( [ .check_runs // [] | group_by(.name)[] | max_by(.started_at // "") ] - | map({ (.name): . }) | add // {} ) as $byname - | ( $req | split(",") | map(select(length > 0)) )[] + jq -r --arg req "$2" "$_ci_jq_latest"' + (latest_per_name | map({ (.name): . }) | add // {}) as $byname + | ($req | split(",") | map(gsub("^\\s+|\\s+$"; "")) | map(select(length > 0)))[] | select( ($byname[.] // null) == null or $byname[.].status != "completed" ) ' <<<"$1" } -# Of the comma-separated required names in $2, print "name: reason" for each that -# is absent ("missing") or whose latest completed conclusion is not acceptable. -# Empty output = every required check is present and passed. +# Of the comma-separated required names in $2 (surrounding whitespace trimmed), +# print "name: reason" for each that is absent ("missing") or whose latest +# completed conclusion is not acceptable. Empty output = every required check is +# present and passed. required_checks_failures() { - jq -r --arg req "$2" --argjson ok "$CI_OK_CONCLUSIONS" ' - ( [ .check_runs // [] | group_by(.name)[] | max_by(.started_at // "") ] - | map({ (.name): . }) | add // {} ) as $byname - | ( $req | split(",") | map(select(length > 0)) )[] + jq -r --arg req "$2" --argjson ok "$CI_OK_CONCLUSIONS" "$_ci_jq_latest"' + (latest_per_name | map({ (.name): . }) | add // {}) as $byname + | ($req | split(",") | map(gsub("^\\s+|\\s+$"; "")) | map(select(length > 0)))[] | . as $name | ($byname[$name] // null) as $run | if $run == null then "\($name): missing" diff --git a/entrypoint.sh b/entrypoint.sh index 4acd4b6..d88ef8f 100755 --- a/entrypoint.sh +++ b/entrypoint.sh @@ -116,14 +116,25 @@ git push --force-with-lease HEAD_BRANCH_HEAD=$(git rev-parse HEAD) echo "(Potentially) Rebased commit hash of HEAD is: $HEAD_BRANCH_HEAD" -# Does the PR touch any of the given comma-separated path prefixes? Paginates -# the PR file list and stops at the first match. +# Does the PR touch any of the given comma-separated path prefixes? Paginates the +# PR file list. Returns 0 (in scope) on a match OR if the file list can't be +# fetched (fail-closed: an undeterminable scope must not silently weaken the +# gate); returns 1 only when the full file list is known and matches nothing. pr_touches_paths() { local prefixes_csv="$1" page=1 resp count files while : ; do - resp=$(curl -s -H "${AUTH_HEADER}" -H "${API_HEADER}" "${PR_URL}/files?per_page=100&page=$page") - if ! jq -e 'type == "array"' <<<"$resp" >/dev/null 2>&1; then - return 1 + resp="" + for _ in 1 2 3; do + resp=$(curl -s --max-time 30 -H "${AUTH_HEADER}" -H "${API_HEADER}" "${PR_URL}/files?per_page=100&page=$page") || resp="" + if [[ -n "$resp" ]] && jq -e 'type == "array"' <<<"$resp" >/dev/null 2>&1; then + break + fi + resp="" + sleep 3 + done + if [[ -z "$resp" ]]; then + echo "Could not fetch changed files after retries; enforcing required checks (fail-closed)." >&2 + return 0 fi count=$(jq 'length' <<<"$resp") if [[ "$count" -eq 0 ]]; then @@ -168,8 +179,13 @@ while true; do exit 1 fi - status_json=$(curl -s -H "${AUTH_HEADER}" -H "${API_HEADER}" "${URI}/repos/$GITHUB_REPOSITORY/commits/$HEAD_BRANCH_HEAD/status") - STATUS_STATE=$(echo "$status_json" | jq -r ".state") + status_json=$(curl -s --max-time 30 -H "${AUTH_HEADER}" -H "${API_HEADER}" "${URI}/repos/$GITHUB_REPOSITORY/commits/$HEAD_BRANCH_HEAD/status") || { + echo "Polling for CI: status fetch failed (transient), retrying..." + continue + } + # An error/rate-limit body, or a non-JSON edge response, must not abort the + # run: fall back to "null" so the case below simply keeps polling. + STATUS_STATE=$(jq -r '.state // "null"' <<<"$status_json" 2>/dev/null || echo "null") case "$STATUS_STATE" in success) ;; # legacy CI passed; check the check-runs next failure|error) @@ -180,7 +196,10 @@ while true; do continue ;; esac - check_runs_json=$(curl -s -H "${AUTH_HEADER}" -H "${API_HEADER}" "${URI}/repos/$GITHUB_REPOSITORY/commits/$HEAD_BRANCH_HEAD/check-runs") + check_runs_json=$(curl -s --max-time 30 -H "${AUTH_HEADER}" -H "${API_HEADER}" "${URI}/repos/$GITHUB_REPOSITORY/commits/$HEAD_BRANCH_HEAD/check-runs") || { + echo "Polling for CI: check-runs fetch failed (transient), retrying..." + continue + } if ! check_runs_payload_valid "$check_runs_json"; then echo "Polling for CI: check-runs endpoint returned no usable payload, retrying..." continue diff --git a/test/ci_checks_test.sh b/test/ci_checks_test.sh index 9062080..fc309bc 100644 --- a/test/ci_checks_test.sh +++ b/test/ci_checks_test.sh @@ -94,6 +94,34 @@ assert_eq "required cancelled superseded by rerun success: no failure" "" \ assert_eq "empty required list: no failures" "" \ "$(required_checks_failures "$ALL_GREEN" "")" +echo "# required names tolerate surrounding whitespace" +assert_eq "spaces after commas: none pending" "" \ + "$(required_checks_pending "$ALL_GREEN" "test, client_test, e2e")" +assert_eq "spaces around names: no failures" "" \ + "$(required_checks_failures "$ALL_GREEN" " test ,client_test, e2e ")" + +echo "# latest run per name is by id, so a newer run is never masked by an older one" +# Newer run (higher id) is still queued but has a null started_at. +RERUN_ID_QUEUED='{"total_count":2,"check_runs":[ + {"name":"e2e","status":"completed","conclusion":"success","started_at":"2026-07-02T10:00:00Z","id":100}, + {"name":"e2e","status":"queued","conclusion":null,"started_at":null,"id":200}]}' +# Newer run (higher id) completed as a failure, older as success. +RERUN_ID_FAILED='{"total_count":2,"check_runs":[ + {"name":"e2e","status":"completed","conclusion":"success","started_at":"2026-07-02T10:00:00Z","id":100}, + {"name":"e2e","status":"completed","conclusion":"failure","started_at":null,"id":200}]}' +assert_eq "newer queued run counts as incomplete" "1" "$(check_runs_incomplete_count "$RERUN_ID_QUEUED")" +assert_eq "newer queued run keeps required pending" "e2e" "$(required_checks_pending "$RERUN_ID_QUEUED" "e2e")" +assert_eq "newer failing run is not masked by older success" "e2e: failure" "$(check_runs_failures "$RERUN_ID_FAILED")" +assert_eq "newer failing required run reported" "e2e: failure" "$(required_checks_failures "$RERUN_ID_FAILED" "e2e")" + +echo "# a null check-run name must not crash the required-checks gate" +NULL_NAME='{"total_count":2,"check_runs":[ + {"name":null,"status":"completed","conclusion":"failure","started_at":"2026-07-02T10:00:00Z","id":1}, + {"name":"e2e","status":"completed","conclusion":"success","started_at":"2026-07-02T10:00:00Z","id":2}]}' +assert_eq "null name ignored: none pending" "" "$(required_checks_pending "$NULL_NAME" "e2e")" +assert_eq "null name ignored: no failures" "" "$(required_checks_failures "$NULL_NAME" "e2e")" +assert_eq "null name ignored: 0 incomplete" "0" "$(check_runs_incomplete_count "$NULL_NAME")" + echo "# any_path_has_prefix" FILES_MIXED=$'SensrTrxMES/app/x.js\nPackManager/db/schema.rb' FILES_PM_ONLY=$'PackManager/db/schema.rb\n.github/workflows/integrate.yml' From b0affcc0a71ae25b507fa728b902ee918d1a670e Mon Sep 17 00:00:00 2001 From: Sean Kirby Date: Thu, 2 Jul 2026 23:24:26 -0400 Subject: [PATCH 10/18] =?UTF-8?q?SPAR-348:=20Feat:=20anchor=20model=20?= =?UTF-8?q?=E2=80=94=20require=20named=20checks=20present=20AND=20gate=20o?= =?UTF-8?q?n=20all=20present?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per review: REQUIRED_CHECK_RUNS should not have to list every check. Combine both gates when in scope: - Always require every check-run present on the commit to pass (auto-covers newly-added checks with no config change). - Additionally require the NAMED checks to be present + completed. The named set anchors the wait: because sibling checks register together, a new check has registered by the time the anchor has, so the "all present" gate then catches it. You only touch REQUIRED_CHECK_RUNS to change the anchor, never to add a check. This replaces the reliance on Buildkite holding its status "pending" for minutes (which is going away for SensrTrxMES-only PRs) with an explicit anchor. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01CUQaePmRdFCZU5tNqBdTps --- README.md | 11 ++++++----- entrypoint.sh | 37 +++++++++++++++++++++++-------------- 2 files changed, 29 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index c6e6dd2..da49e92 100644 --- a/README.md +++ b/README.md @@ -6,10 +6,11 @@ Supports two commands: - `/integrate` -- Rebases, waits for CI, and merges the PR. "CI" means the legacy commit statuses (e.g. Buildkite) **and** GitHub Actions check-runs on the - rebased commit. By default the merge proceeds only when the legacy status is - `success` and every check-run present has completed with a `success`, - `neutral`, or `skipped` conclusion. See [Configuration](#configuration) to - require a specific named set of checks instead. + rebased commit. The merge proceeds only when the legacy status is `success` and + every check-run present on the commit has completed with a `success`, `neutral`, + or `skipped` conclusion. See [Configuration](#configuration) for + `REQUIRED_CHECK_RUNS`, which additionally waits for a named set of checks to + *appear* so a PR can't merge in the window before its checks have registered. - `/hotfix` -- Same as integrate, but appends `[skip tests]` to the merge commit message. # Example Usage @@ -59,7 +60,7 @@ All optional, passed via `env:` on the action step: | `GITHUB_TOKEN` | — | **Required.** Token allowed to merge into the PR's base branch. | | `ADD_CHANGE_LOGS` | `false` | Collect `Change log:` PR comments into the merge commit message. | | `CI_WAIT_TIMEOUT_SECONDS` | `14400` (4h) | Give up waiting for CI after this many seconds (fail, don't merge). Keep it above your slowest check and below the job's own timeout (GitHub's default is 6h). | -| `REQUIRED_CHECK_RUNS` | _(empty)_ | Comma-separated check-run names that must be **present and pass** before merging. When empty, the action gates on every check-run present on the commit. Set this to avoid trusting an empty/partial check-run set and to ignore unrelated/advisory checks. | +| `REQUIRED_CHECK_RUNS` | _(empty)_ | Comma-separated check-run names that must be **present** (and pass) before merging. The action *always* requires every check-run present on the commit to pass; this list additionally requires the named checks to have appeared, closing the window where a check hasn't registered yet and an empty/partial set looks "green". You do **not** add every new check here — a new check is caught by the always-on "all present must pass" rule — but keep at least one reliably-running check named as an anchor, so the wait can't finish before the suite registers. | | `REQUIRED_CHECK_RUNS_PATHS` | _(empty)_ | Comma-separated path **prefixes**. When set, `REQUIRED_CHECK_RUNS` is enforced only if the PR changes a file under one of them — so a PR that doesn't touch the relevant product isn't blocked waiting for checks that never run. When empty, `REQUIRED_CHECK_RUNS` always applies. | Example (a monorepo whose SFac product's tests run as GitHub Actions): diff --git a/entrypoint.sh b/entrypoint.sh index d88ef8f..0484ecb 100755 --- a/entrypoint.sh +++ b/entrypoint.sh @@ -205,24 +205,40 @@ while true; do continue fi + # When in scope, wait for the named required checks to APPEAR and finish. This + # anchors the wait: because sibling checks register together, a newly-added + # check will have registered by the time the anchor has, so the "all present" + # wait below then covers it without it being listed in REQUIRED_CHECK_RUNS. if [[ "$required_active" == "true" ]]; then pending=$(required_checks_pending "$check_runs_json" "$REQUIRED_CHECK_RUNS") if [[ -n "$pending" ]]; then echo "Polling for CI: waiting on required check-run(s): $(echo "$pending" | tr '\n' ' ')" continue fi - else - incomplete=$(check_runs_incomplete_count "$check_runs_json") - if [[ "$incomplete" -gt 0 ]]; then - echo "Polling for CI: $incomplete check-run(s) still running..." - continue - fi + fi + + # Wait for every check-run present on the commit to finish. + incomplete=$(check_runs_incomplete_count "$check_runs_json") + if [[ "$incomplete" -gt 0 ]]; then + echo "Polling for CI: $incomplete check-run(s) still running..." + continue fi break done -# Reaching here means the legacy status is "success"; fail on any check-run problem. +# Reaching here means the legacy status is "success". Every check-run present on +# the commit must have concluded acceptably... +failed_runs=$(check_runs_failures "$check_runs_json") +if [[ -n "$failed_runs" ]]; then + echo "CI did not pass. Failing check-runs for $HEAD_BRANCH @ $HEAD_BRANCH_HEAD:" + echo "$failed_runs" + exit 1 +fi + +# ...and, when in scope, the named required checks must additionally be PRESENT +# (not merely "not failing") -- this is what guarantees we didn't merge before +# they ran. if [[ "$required_active" == "true" ]]; then required_failures=$(required_checks_failures "$check_runs_json" "$REQUIRED_CHECK_RUNS") if [[ -n "$required_failures" ]]; then @@ -230,13 +246,6 @@ if [[ "$required_active" == "true" ]]; then echo "$required_failures" exit 1 fi -else - failed_runs=$(check_runs_failures "$check_runs_json") - if [[ -n "$failed_runs" ]]; then - echo "CI did not pass. Failing check-runs for $HEAD_BRANCH @ $HEAD_BRANCH_HEAD:" - echo "$failed_runs" - exit 1 - fi fi # Hit the merge button. Pass sha=$HEAD_BRANCH_HEAD so GitHub only merges if the From f5102e1d2d63b425f54a2f609757e412e13ad135 Mon Sep 17 00:00:00 2001 From: Sean Kirby Date: Fri, 3 Jul 2026 09:45:08 -0400 Subject: [PATCH 11/18] SPAR-348: Feat: normalize GraphQL statusCheckRollup into the check-run model Add rollup_payload_valid + normalize_rollup so a GitHub GraphQL statusCheckRollup response (which returns legacy StatusContexts AND Actions CheckRuns together) maps into the {check_runs:[...]} shape the existing eval helpers consume. StatusContext.state -> (status,conclusion); enums lowercased. Lets required checks name legacy statuses (e.g. buildkite/packmanager) and check-runs uniformly. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01CUQaePmRdFCZU5tNqBdTps --- ci_checks.sh | 34 ++++++++++++++++++++++++++++++++++ test/ci_checks_test.sh | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+) diff --git a/ci_checks.sh b/ci_checks.sh index ed09693..095667c 100644 --- a/ci_checks.sh +++ b/ci_checks.sh @@ -27,6 +27,40 @@ check_runs_payload_valid() { jq -e '(.check_runs | type) == "array"' <<<"$1" >/dev/null 2>&1 } +# True if $1 is a usable GraphQL statusCheckRollup response: no top-level errors +# and the commit object resolved. (A resolved commit with no checks yet has a +# null rollup, which is still usable -> normalizes to an empty set.) +rollup_payload_valid() { + jq -e '(.errors | not) and (.data.repository.object != null)' <<<"$1" >/dev/null 2>&1 +} + +# Normalize a GraphQL statusCheckRollup response into the {check_runs:[...]} shape +# the helpers below consume, so legacy StatusContexts (e.g. Buildkite) and Actions +# CheckRuns are evaluated uniformly. StatusContext.state maps onto (status, +# conclusion): SUCCESS -> completed/success; FAILURE|ERROR -> completed/failure; +# PENDING|EXPECTED -> in_progress/none (i.e. not yet completed). CheckRun enums +# are lowercased to match the REST vocabulary the helpers expect. +normalize_rollup() { + jq '{ + check_runs: [ + (.data.repository.object.statusCheckRollup.contexts.nodes // [])[] + | if .__typename == "CheckRun" then + { name: .name, + status: ((.status // "") | ascii_downcase), + conclusion: (if .conclusion == null then null else (.conclusion | ascii_downcase) end), + started_at: .startedAt, + id: (.databaseId // 0) } + else + { name: .context, + status: (if (.state == "SUCCESS" or .state == "FAILURE" or .state == "ERROR") then "completed" else "in_progress" end), + conclusion: (if .state == "SUCCESS" then "success" elif (.state == "FAILURE" or .state == "ERROR") then "failure" else null end), + started_at: .createdAt, + id: 0 } + end + ] + }' <<<"$1" +} + # Count latest-per-name check-runs that have not finished yet. check_runs_incomplete_count() { jq -r "$_ci_jq_latest"' diff --git a/test/ci_checks_test.sh b/test/ci_checks_test.sh index fc309bc..7cf9d99 100644 --- a/test/ci_checks_test.sh +++ b/test/ci_checks_test.sh @@ -122,6 +122,40 @@ assert_eq "null name ignored: none pending" "" "$(required_checks_pending "$NULL assert_eq "null name ignored: no failures" "" "$(required_checks_failures "$NULL_NAME" "e2e")" assert_eq "null name ignored: 0 incomplete" "0" "$(check_runs_incomplete_count "$NULL_NAME")" +echo "# GraphQL statusCheckRollup: validity + normalization into the check-run model" +ROLLUP_MIXED='{"data":{"repository":{"object":{"statusCheckRollup":{"state":"PENDING","contexts":{"nodes":[ + {"__typename":"CheckRun","name":"e2e","status":"COMPLETED","conclusion":"SUCCESS","startedAt":"2026-07-02T17:07:17Z","databaseId":3}, + {"__typename":"CheckRun","name":"test","status":"IN_PROGRESS","conclusion":null,"startedAt":"2026-07-02T17:07:15Z","databaseId":4}, + {"__typename":"StatusContext","context":"buildkite/packmanager","state":"SUCCESS","createdAt":"2026-07-02T17:07:07Z"}]}}}}}}' +ROLLUP_STATUS_FAIL='{"data":{"repository":{"object":{"statusCheckRollup":{"contexts":{"nodes":[ + {"__typename":"StatusContext","context":"buildkite/packmanager","state":"FAILURE","createdAt":"2026-07-02T17:07:07Z"}]}}}}}}' +ROLLUP_NULL='{"data":{"repository":{"object":{"statusCheckRollup":null}}}}' +ROLLUP_ERROR='{"errors":[{"message":"Something went wrong"}]}' +ROLLUP_NOOBJECT='{"data":{"repository":{"object":null}}}' + +assert_ok "mixed rollup is valid" rollup_payload_valid "$ROLLUP_MIXED" +assert_ok "null rollup (commit, no checks) is valid" rollup_payload_valid "$ROLLUP_NULL" +assert_notok "errors payload invalid" rollup_payload_valid "$ROLLUP_ERROR" +assert_notok "unresolved commit invalid" rollup_payload_valid "$ROLLUP_NOOBJECT" + +# Legacy status + check-runs evaluated uniformly after normalization. +assert_eq "normalized: 1 incomplete (test in_progress)" "1" \ + "$(check_runs_incomplete_count "$(normalize_rollup "$ROLLUP_MIXED")")" +assert_eq "normalized: no failures" "" \ + "$(check_runs_failures "$(normalize_rollup "$ROLLUP_MIXED")")" +assert_eq "normalized: buildkite (a StatusContext) satisfies a required name" "" \ + "$(required_checks_pending "$(normalize_rollup "$ROLLUP_MIXED")" "buildkite/packmanager,e2e")" +assert_eq "normalized: in-progress required still pending" "test" \ + "$(required_checks_pending "$(normalize_rollup "$ROLLUP_MIXED")" "test")" +assert_eq "normalized: absent required still pending" "client_test" \ + "$(required_checks_pending "$(normalize_rollup "$ROLLUP_MIXED")" "client_test")" +assert_eq "normalized: a FAILURE legacy status is a failure" "buildkite/packmanager: failure" \ + "$(check_runs_failures "$(normalize_rollup "$ROLLUP_STATUS_FAIL")")" +assert_eq "normalized: required legacy status failure reported" "buildkite/packmanager: failure" \ + "$(required_checks_failures "$(normalize_rollup "$ROLLUP_STATUS_FAIL")" "buildkite/packmanager")" +assert_eq "normalized null rollup: 0 incomplete" "0" \ + "$(check_runs_incomplete_count "$(normalize_rollup "$ROLLUP_NULL")")" + echo "# any_path_has_prefix" FILES_MIXED=$'SensrTrxMES/app/x.js\nPackManager/db/schema.rb' FILES_PM_ONLY=$'PackManager/db/schema.rb\n.github/workflows/integrate.yml' From 27f31cb7a137b780e60c780b892abc4b48aa7339 Mon Sep 17 00:00:00 2001 From: Sean Kirby Date: Fri, 3 Jul 2026 09:45:08 -0400 Subject: [PATCH 12/18] SPAR-348: Feat: fetch checks via GraphQL statusCheckRollup; REQUIRED_CHECKS JSON config Replace the two REST polls (combined status + check-runs) and the separate "legacy status must be success" gate with a single GraphQL statusCheckRollup query, so legacy status contexts and Actions check-runs are gated uniformly. Replace REQUIRED_CHECK_RUNS + REQUIRED_CHECK_RUNS_PATHS with one REQUIRED_CHECKS env: a JSON array of {paths?, checks} rules. The action carries no product knowledge; the consuming repo pairs its own paths with its own check/status names. A rule with no paths is always required; a path-scoped rule applies only when the PR touches a matching prefix. Rules are parsed by index (not read+IFS, which drops an empty paths field). This also removes the pure-Actions-repo hang: there is no unconditional legacy-status gate anymore. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01CUQaePmRdFCZU5tNqBdTps --- README.md | 24 ++++----- entrypoint.sh | 137 ++++++++++++++++++++++++++------------------------ 2 files changed, 83 insertions(+), 78 deletions(-) diff --git a/README.md b/README.md index da49e92..46cc2e7 100644 --- a/README.md +++ b/README.md @@ -4,13 +4,13 @@ Fork of the `cirrus-actions/rebase` repo for integrating a PR. Supports two commands: -- `/integrate` -- Rebases, waits for CI, and merges the PR. "CI" means the legacy - commit statuses (e.g. Buildkite) **and** GitHub Actions check-runs on the - rebased commit. The merge proceeds only when the legacy status is `success` and - every check-run present on the commit has completed with a `success`, `neutral`, - or `skipped` conclusion. See [Configuration](#configuration) for - `REQUIRED_CHECK_RUNS`, which additionally waits for a named set of checks to - *appear* so a PR can't merge in the window before its checks have registered. +- `/integrate` -- Rebases, waits for CI, and merges the PR. "CI" spans a commit's + GitHub Actions check-runs **and** legacy status contexts (e.g. Buildkite), + read together via GitHub's GraphQL `statusCheckRollup`. The merge proceeds only + when every check present on the commit has completed with a `success`, + `neutral`, or `skipped` conclusion. See [Configuration](#configuration) for + `REQUIRED_CHECKS`, which additionally waits for named checks to *appear* so a PR + can't merge in the window before its checks have registered. - `/hotfix` -- Same as integrate, but appends `[skip tests]` to the merge commit message. # Example Usage @@ -60,20 +60,18 @@ All optional, passed via `env:` on the action step: | `GITHUB_TOKEN` | — | **Required.** Token allowed to merge into the PR's base branch. | | `ADD_CHANGE_LOGS` | `false` | Collect `Change log:` PR comments into the merge commit message. | | `CI_WAIT_TIMEOUT_SECONDS` | `14400` (4h) | Give up waiting for CI after this many seconds (fail, don't merge). Keep it above your slowest check and below the job's own timeout (GitHub's default is 6h). | -| `REQUIRED_CHECK_RUNS` | _(empty)_ | Comma-separated check-run names that must be **present** (and pass) before merging. The action *always* requires every check-run present on the commit to pass; this list additionally requires the named checks to have appeared, closing the window where a check hasn't registered yet and an empty/partial set looks "green". You do **not** add every new check here — a new check is caught by the always-on "all present must pass" rule — but keep at least one reliably-running check named as an anchor, so the wait can't finish before the suite registers. | -| `REQUIRED_CHECK_RUNS_PATHS` | _(empty)_ | Comma-separated path **prefixes**. When set, `REQUIRED_CHECK_RUNS` is enforced only if the PR changes a file under one of them — so a PR that doesn't touch the relevant product isn't blocked waiting for checks that never run. When empty, `REQUIRED_CHECK_RUNS` always applies. | +| `REQUIRED_CHECKS` | _(empty)_ | JSON array of rules pairing path prefixes with check names that must be **present** (and pass) before merging — matching GitHub Actions check-runs *and* legacy status contexts (e.g. `buildkite/packmanager`). A rule with no `paths` always applies; with `paths` it applies only when the PR changes a file under one of those prefixes. The action *always* requires every check present on the commit to pass; these rules additionally require the named checks to have appeared, closing the window where a check hasn't registered yet and an empty/partial set looks "green". You do **not** list every check — a new check is caught by the always-on "all present must pass" rule — but name at least one reliably-running check per product as an anchor. Example: `[{"checks":["buildkite/packmanager"]},{"paths":["some/dir/"],"checks":["test","e2e"]}]` | -Example (a monorepo whose SFac product's tests run as GitHub Actions): +Example (a monorepo: Buildkite gates one product, GitHub Actions gates another): ```yml - uses: nulogy/integrate-action@v2.0.0 env: GITHUB_TOKEN: ${{ secrets.GITHUB_MERGING_TOKEN }} - REQUIRED_CHECK_RUNS: "test,client_test,e2e" - REQUIRED_CHECK_RUNS_PATHS: "SensrTrxMES/" + REQUIRED_CHECKS: '[{"checks":["buildkite/packmanager"]},{"paths":["SensrTrxMES/"],"checks":["test","client_test","e2e"]}]' ``` -**Requirement:** the action waits on the legacy combined-status API and merges only when it reports `success`, so a repo must have **at least one legacy commit status** (e.g. Buildkite) that stays `pending` through its build. A pure-GitHub-Actions repo with no legacy statuses is not yet supported — the combined status reads `pending` indefinitely and the action will wait until it times out. +The check state is read from GitHub's GraphQL `statusCheckRollup`, so Actions check-runs and legacy status contexts are gated the same way — the action works for Actions-only, status-only, or mixed repos, and needs no branch-protection required checks. (Only the first 100 contexts on a commit are considered.) # Versioning diff --git a/entrypoint.sh b/entrypoint.sh index 0484ecb..617d25e 100755 --- a/entrypoint.sh +++ b/entrypoint.sh @@ -48,18 +48,17 @@ AUTH_HEADER="Authorization: token $GITHUB_TOKEN" # CI-wait configuration (all optional; defaults preserve prior behavior). # Max seconds to wait for CI before giving up (fail-closed). Must exceed the -# slowest check (spark's SFac e2e has run ~2.6h) yet stay under this job's own -# timeout (GitHub's default is 6h). +# slowest check yet stay under this job's own timeout (GitHub's default is 6h). CI_WAIT_TIMEOUT_SECONDS="${CI_WAIT_TIMEOUT_SECONDS:-14400}" -# Comma-separated check-run names that MUST be present and pass before merging. -# When empty, the action instead gates on every check-run present on the commit -# (it cannot otherwise tell which checks are expected). -REQUIRED_CHECK_RUNS="${REQUIRED_CHECK_RUNS:-}" -# Comma-separated path prefixes. When set, REQUIRED_CHECK_RUNS is enforced only -# if the PR changes a file under one of these prefixes, so a PR that doesn't -# touch the relevant product isn't blocked waiting for checks that never run. -# When empty but REQUIRED_CHECK_RUNS is set, the required checks always apply. -REQUIRED_CHECK_RUNS_PATHS="${REQUIRED_CHECK_RUNS_PATHS:-}" +# JSON array of rules pairing path prefixes with required check names, e.g. +# [ {"checks":["buildkite/packmanager"]}, +# {"paths":["some/dir/"],"checks":["test","e2e"]} ] +# A rule with no "paths" is always required; with "paths" it applies only when the +# PR changes a file under one of those prefixes. A required check must be PRESENT +# (and pass) before merging. Names match both GitHub Actions check-runs and legacy +# commit-status contexts (e.g. "buildkite/packmanager"). When empty, the action +# gates only on whatever checks are present on the commit. +REQUIRED_CHECKS="${REQUIRED_CHECKS:-}" USER_URL=$(jq -r ".comment.user.url" "$GITHUB_EVENT_PATH") user_resp=$(curl -X GET -s -H "${API_HEADER}" -H "${AUTH_HEADER}" "${USER_URL}") @@ -151,25 +150,49 @@ pr_touches_paths() { done } -# Enforce REQUIRED_CHECK_RUNS on this PR only when in scope (see config above). -required_active="false" -if [[ -n "$REQUIRED_CHECK_RUNS" ]]; then - if [[ -z "$REQUIRED_CHECK_RUNS_PATHS" ]] || pr_touches_paths "$REQUIRED_CHECK_RUNS_PATHS"; then - required_active="true" +# Build the set of required check names for this PR from REQUIRED_CHECKS (see +# config above). A rule with no "paths" always applies; otherwise it applies when +# the PR changes a file under one of its prefixes. Names may refer to check-runs +# or legacy status contexts. +required_names="" +if [[ -n "$REQUIRED_CHECKS" ]]; then + if jq -e 'type == "array"' <<<"$REQUIRED_CHECKS" >/dev/null 2>&1; then + rule_count=$(jq 'length' <<<"$REQUIRED_CHECKS") + i=0 + # Extract each rule's fields by index (not via read+IFS: a tab delimiter is + # IFS-whitespace, which would drop an empty "paths" field and misread the row). + while [[ "$i" -lt "$rule_count" ]]; do + rule_paths=$(jq -r --argjson i "$i" '(.[$i].paths // []) | join(",")' <<<"$REQUIRED_CHECKS") + rule_checks=$(jq -r --argjson i "$i" '(.[$i].checks // []) | join(",")' <<<"$REQUIRED_CHECKS") + i=$((i + 1)) + if [[ -z "$rule_checks" ]]; then + continue + fi + if [[ -z "$rule_paths" ]] || pr_touches_paths "$rule_paths"; then + required_names="${required_names:+$required_names,}$rule_checks" + fi + done + else + echo "REQUIRED_CHECKS is not a JSON array; ignoring it." >&2 fi fi -if [[ "$required_active" == "true" ]]; then - echo "Required check-runs enforced for this PR: $REQUIRED_CHECK_RUNS" +if [[ -n "$required_names" ]]; then + echo "Required checks for this PR: $required_names" else - echo "No required check-runs in scope; gating on every check-run present on the commit." + echo "No required checks in scope; gating on every check present on the commit." fi -# Wait for BOTH CI systems to report on the rebased commit ($HEAD_BRANCH_HEAD): -# - PackManager CI (Buildkite) -> legacy commit status (/status) -# - SFac CI (GitHub Actions) -> check-runs (/check-runs), invisible to /status -# No pre-loop settle is needed: Buildkite posts a pending status within seconds -# of the force-push and holds it for minutes (until its build finishes), so this -# loop keeps waiting long after the Actions check-runs have registered. +# Poll the commit's checks via one GraphQL statusCheckRollup query, which returns +# legacy status contexts (e.g. Buildkite) AND GitHub Actions check-runs together, +# so both are gated uniformly. +OWNER="${GITHUB_REPOSITORY%%/*}" +REPO="${GITHUB_REPOSITORY##*/}" +# $owner/$name/$oid are GraphQL variables, not shell -- single quotes are correct. +# shellcheck disable=SC2016 +GQL_QUERY='query($owner:String!,$name:String!,$oid:GitObjectID!){repository(owner:$owner,name:$name){object(oid:$oid){... on Commit{statusCheckRollup{contexts(first:100){nodes{__typename ... on CheckRun{name status conclusion startedAt databaseId} ... on StatusContext{context state createdAt}}}}}}}}' + +# No pre-loop settle is needed: the required-check anchor below holds the loop +# until the expected checks have registered. deadline=$(( $(date +%s) + CI_WAIT_TIMEOUT_SECONDS )) while true; do sleep 10 @@ -179,70 +202,54 @@ while true; do exit 1 fi - status_json=$(curl -s --max-time 30 -H "${AUTH_HEADER}" -H "${API_HEADER}" "${URI}/repos/$GITHUB_REPOSITORY/commits/$HEAD_BRANCH_HEAD/status") || { - echo "Polling for CI: status fetch failed (transient), retrying..." + gql_payload=$(jq -n --arg q "$GQL_QUERY" --arg owner "$OWNER" --arg name "$REPO" --arg oid "$HEAD_BRANCH_HEAD" \ + '{query: $q, variables: {owner: $owner, name: $name, oid: $oid}}') + rollup_resp=$(curl -s --max-time 30 -H "${AUTH_HEADER}" -H "Content-Type: application/json" -d "$gql_payload" "${URI}/graphql") || { + echo "Polling for CI: rollup fetch failed (transient), retrying..." continue } - # An error/rate-limit body, or a non-JSON edge response, must not abort the - # run: fall back to "null" so the case below simply keeps polling. - STATUS_STATE=$(jq -r '.state // "null"' <<<"$status_json" 2>/dev/null || echo "null") - case "$STATUS_STATE" in - success) ;; # legacy CI passed; check the check-runs next - failure|error) - echo "CI did not pass (legacy status = $STATUS_STATE) for $HEAD_BRANCH @ $HEAD_BRANCH_HEAD. Cancelling integration." - exit 1 ;; - *) # "pending", or "null" from a transient error body - echo "Polling for CI: legacy statuses not final yet ($STATUS_STATE)..." - continue ;; - esac - - check_runs_json=$(curl -s --max-time 30 -H "${AUTH_HEADER}" -H "${API_HEADER}" "${URI}/repos/$GITHUB_REPOSITORY/commits/$HEAD_BRANCH_HEAD/check-runs") || { - echo "Polling for CI: check-runs fetch failed (transient), retrying..." - continue - } - if ! check_runs_payload_valid "$check_runs_json"; then - echo "Polling for CI: check-runs endpoint returned no usable payload, retrying..." + if ! rollup_payload_valid "$rollup_resp"; then + echo "Polling for CI: rollup response not usable yet, retrying..." continue fi - - # When in scope, wait for the named required checks to APPEAR and finish. This - # anchors the wait: because sibling checks register together, a newly-added - # check will have registered by the time the anchor has, so the "all present" - # wait below then covers it without it being listed in REQUIRED_CHECK_RUNS. - if [[ "$required_active" == "true" ]]; then - pending=$(required_checks_pending "$check_runs_json" "$REQUIRED_CHECK_RUNS") + check_runs_json=$(normalize_rollup "$rollup_resp") + + # Wait for the required checks to APPEAR and finish. This anchors the wait: + # because sibling checks register together, a newly-added check will have + # registered by the time an anchor has, so the "all present" wait below then + # covers it without it being listed in REQUIRED_CHECKS. + if [[ -n "$required_names" ]]; then + pending=$(required_checks_pending "$check_runs_json" "$required_names") if [[ -n "$pending" ]]; then - echo "Polling for CI: waiting on required check-run(s): $(echo "$pending" | tr '\n' ' ')" + echo "Polling for CI: waiting on required check(s): $(echo "$pending" | tr '\n' ' ')" continue fi fi - # Wait for every check-run present on the commit to finish. + # Wait for every check present on the commit to finish. incomplete=$(check_runs_incomplete_count "$check_runs_json") if [[ "$incomplete" -gt 0 ]]; then - echo "Polling for CI: $incomplete check-run(s) still running..." + echo "Polling for CI: $incomplete check(s) still running..." continue fi break done -# Reaching here means the legacy status is "success". Every check-run present on -# the commit must have concluded acceptably... +# Every check present on the commit must have concluded acceptably... failed_runs=$(check_runs_failures "$check_runs_json") if [[ -n "$failed_runs" ]]; then - echo "CI did not pass. Failing check-runs for $HEAD_BRANCH @ $HEAD_BRANCH_HEAD:" + echo "CI did not pass. Failing checks for $HEAD_BRANCH @ $HEAD_BRANCH_HEAD:" echo "$failed_runs" exit 1 fi -# ...and, when in scope, the named required checks must additionally be PRESENT -# (not merely "not failing") -- this is what guarantees we didn't merge before -# they ran. -if [[ "$required_active" == "true" ]]; then - required_failures=$(required_checks_failures "$check_runs_json" "$REQUIRED_CHECK_RUNS") +# ...and the required checks must additionally be PRESENT (not merely "not +# failing") -- this is what guarantees we didn't merge before they ran. +if [[ -n "$required_names" ]]; then + required_failures=$(required_checks_failures "$check_runs_json" "$required_names") if [[ -n "$required_failures" ]]; then - echo "CI did not pass. Required check-run problem(s) for $HEAD_BRANCH @ $HEAD_BRANCH_HEAD:" + echo "CI did not pass. Required check problem(s) for $HEAD_BRANCH @ $HEAD_BRANCH_HEAD:" echo "$required_failures" exit 1 fi From 2930474a2fb648a94db83b20706a87da495090c7 Mon Sep 17 00:00:00 2001 From: Sean Kirby Date: Fri, 3 Jul 2026 10:07:42 -0400 Subject: [PATCH 13/18] SPAR-348: Fix: floor empty-config mode; guard malformed REQUIRED_CHECKS and >100 checks From the GraphQL-rewrite verification pass: - HIGH: with an empty/default REQUIRED_CHECKS, the loop broke on the first poll of a freshly force-pushed commit that had no checks yet and merged with zero CI. Add a floor: in the no-required mode, keep waiting until at least one check has appeared. (With REQUIRED_CHECKS set, the anchor already did this.) Also add REQUIRED_CHECKS to the primary README example and correct the wording. - MED: a malformed-but-array REQUIRED_CHECKS (e.g. checks as a bare string, or an array of non-objects) errored in jq and aborted under set -e AFTER the force-push. Type-guard the per-rule extraction so a bad rule yields "" and is skipped instead. - LOW: statusCheckRollup returns at most 100 contexts; if a commit has more, refuse to merge (fail-closed) rather than merging on a partial view. - Docs: anchors must register no later than the checks they cover; check names and path prefixes must not contain commas. Verified via set -e harnesses (config parsing incl. malformed rules; the empty-mode floor + >100 guard decision table) and the 53 ci_checks unit tests. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01CUQaePmRdFCZU5tNqBdTps --- README.md | 12 +++++++++++- entrypoint.sh | 26 +++++++++++++++++++++++--- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 46cc2e7..73f72e4 100644 --- a/README.md +++ b/README.md @@ -36,6 +36,10 @@ Supports two commands: - uses: nulogy/integrate-action@master env: GITHUB_TOKEN: ${{ secrets.GITHUB_MERGING_TOKEN }} + # Strongly recommended (see Configuration): require your CI checks to be + # present and pass. Without it, the action gates only on whatever checks + # happen to exist when it polls. + REQUIRED_CHECKS: '[{"checks":["your-ci-check"]}]' always_job: name: Aways run job runs-on: ubuntu-latest @@ -71,7 +75,13 @@ Example (a monorepo: Buildkite gates one product, GitHub Actions gates another): REQUIRED_CHECKS: '[{"checks":["buildkite/packmanager"]},{"paths":["SensrTrxMES/"],"checks":["test","client_test","e2e"]}]' ``` -The check state is read from GitHub's GraphQL `statusCheckRollup`, so Actions check-runs and legacy status contexts are gated the same way — the action works for Actions-only, status-only, or mixed repos, and needs no branch-protection required checks. (Only the first 100 contexts on a commit are considered.) +Notes: + +- Check state is read from GitHub's GraphQL `statusCheckRollup`, so Actions check-runs and legacy status contexts are gated the same way — the action works for Actions-only, status-only, or mixed repos, and needs no branch-protection required checks. +- Without `REQUIRED_CHECKS` the action still won't merge on an *empty* check set (it waits for at least one check to appear), but it can only gate on whatever has appeared by then; set `REQUIRED_CHECKS` to guarantee specific checks ran. +- Each anchor should be a check that registers no later than the checks it stands in for (prefer a fast-registering check-run over a slow external status), so the "all present" rule can't finish before a sibling has appeared. +- Check names and path prefixes must not contain commas. +- If a commit has more than 100 checks, the action refuses to merge (it cannot see them all) rather than merging on a partial view. # Versioning diff --git a/entrypoint.sh b/entrypoint.sh index 617d25e..9a18042 100755 --- a/entrypoint.sh +++ b/entrypoint.sh @@ -162,8 +162,10 @@ if [[ -n "$REQUIRED_CHECKS" ]]; then # Extract each rule's fields by index (not via read+IFS: a tab delimiter is # IFS-whitespace, which would drop an empty "paths" field and misread the row). while [[ "$i" -lt "$rule_count" ]]; do - rule_paths=$(jq -r --argjson i "$i" '(.[$i].paths // []) | join(",")' <<<"$REQUIRED_CHECKS") - rule_checks=$(jq -r --argjson i "$i" '(.[$i].checks // []) | join(",")' <<<"$REQUIRED_CHECKS") + # Type-guard so a malformed rule (e.g. checks as a string, or a non-object) + # yields "" and is skipped, rather than erroring out and aborting under set -e. + rule_paths=$(jq -r --argjson i "$i" '(.[$i]) as $r | if ($r | type) == "object" and (($r.paths | type) == "array") then ($r.paths | map(select(type == "string")) | join(",")) else "" end' <<<"$REQUIRED_CHECKS") + rule_checks=$(jq -r --argjson i "$i" '(.[$i]) as $r | if ($r | type) == "object" and (($r.checks | type) == "array") then ($r.checks | map(select(type == "string")) | join(",")) else "" end' <<<"$REQUIRED_CHECKS") i=$((i + 1)) if [[ -z "$rule_checks" ]]; then continue @@ -189,7 +191,7 @@ OWNER="${GITHUB_REPOSITORY%%/*}" REPO="${GITHUB_REPOSITORY##*/}" # $owner/$name/$oid are GraphQL variables, not shell -- single quotes are correct. # shellcheck disable=SC2016 -GQL_QUERY='query($owner:String!,$name:String!,$oid:GitObjectID!){repository(owner:$owner,name:$name){object(oid:$oid){... on Commit{statusCheckRollup{contexts(first:100){nodes{__typename ... on CheckRun{name status conclusion startedAt databaseId} ... on StatusContext{context state createdAt}}}}}}}}' +GQL_QUERY='query($owner:String!,$name:String!,$oid:GitObjectID!){repository(owner:$owner,name:$name){object(oid:$oid){... on Commit{statusCheckRollup{contexts(first:100){totalCount nodes{__typename ... on CheckRun{name status conclusion startedAt databaseId} ... on StatusContext{context state createdAt}}}}}}}}' # No pre-loop settle is needed: the required-check anchor below holds the loop # until the expected checks have registered. @@ -212,6 +214,13 @@ while true; do echo "Polling for CI: rollup response not usable yet, retrying..." continue fi + # Refuse to merge on a partial view: the rollup returns at most 100 contexts, + # so if the commit has more, some checks are invisible to us. + context_total=$(jq '.data.repository.object.statusCheckRollup.contexts.totalCount // 0' <<<"$rollup_resp") + if [[ "$context_total" -gt 100 ]]; then + echo "Commit $HEAD_BRANCH_HEAD has $context_total checks; only 100 are fetched. Refusing to merge on a partial view. Cancelling integration." + exit 1 + fi check_runs_json=$(normalize_rollup "$rollup_resp") # Wait for the required checks to APPEAR and finish. This anchors the wait: @@ -226,6 +235,17 @@ while true; do fi fi + # Floor for the no-required-checks mode: never merge on an empty check set -- + # wait until at least one check has appeared so an unregistered CI run can't + # look "green". (With REQUIRED_CHECKS set, the anchor above already does this.) + if [[ -z "$required_names" ]]; then + present_count=$(jq '.check_runs | length' <<<"$check_runs_json") + if [[ "$present_count" -eq 0 ]]; then + echo "Polling for CI: no checks have appeared on the commit yet, retrying..." + continue + fi + fi + # Wait for every check present on the commit to finish. incomplete=$(check_runs_incomplete_count "$check_runs_json") if [[ "$incomplete" -gt 0 ]]; then From ba24dce818c9526cfcbe616d5bdd8a769006caef Mon Sep 17 00:00:00 2001 From: Sean Kirby Date: Fri, 3 Jul 2026 11:07:19 -0400 Subject: [PATCH 14/18] SPAR-348: Feat: rebase onto latest base + re-check before merge (narrow concurrent-merge race) Two concurrently-integrated PRs could each pass CI against a base that predated the other, then both merge-commit onto main -- so a semantic conflict (no textual conflict) could break main even though both PRs were green. Wrap rebase + CI-wait + verdict + merge in an outer loop: - each attempt rebases onto the LATEST base, force-pushes, and waits for CI on that commit; - right before merging, re-fetch the base; if it advanced during CI (another PR merged), re-rebase and re-verify instead of merging a stale-CI'd commit; - bounded by MAX_REBASE_ATTEMPTS (3) so a busy base can't loop forever; rebase conflicts and lost force-push leases fail closed. This narrows but does not fully close the race -- the gap between the pre-merge base check and the merge call itself needs a merge queue (or "require branches up to date") to close. Documented as such. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01CUQaePmRdFCZU5tNqBdTps --- entrypoint.sh | 61 ++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 48 insertions(+), 13 deletions(-) diff --git a/entrypoint.sh b/entrypoint.sh index 9a18042..33b7af8 100755 --- a/entrypoint.sh +++ b/entrypoint.sh @@ -104,16 +104,10 @@ git remote set-url origin https://x-access-token:$GITHUB_TOKEN@github.com/$GITHU git config --global user.email "action@github.com" git config --global user.name "GitHub Action" -# Make sure branches are up-to-date -git fetch origin $BASE_BRANCH +# Check out the PR branch. The base is (re)fetched and the branch (re)rebased +# per attempt inside the merge loop below. git fetch origin $HEAD_BRANCH - -# Rebase git checkout -b $HEAD_BRANCH origin/$HEAD_BRANCH -git rebase origin/$BASE_BRANCH -git push --force-with-lease -HEAD_BRANCH_HEAD=$(git rev-parse HEAD) -echo "(Potentially) Rebased commit hash of HEAD is: $HEAD_BRANCH_HEAD" # Does the PR touch any of the given comma-separated path prefixes? Paginates the # PR file list. Returns 0 (in scope) on a match OR if the file list can't be @@ -193,11 +187,39 @@ REPO="${GITHUB_REPOSITORY##*/}" # shellcheck disable=SC2016 GQL_QUERY='query($owner:String!,$name:String!,$oid:GitObjectID!){repository(owner:$owner,name:$name){object(oid:$oid){... on Commit{statusCheckRollup{contexts(first:100){totalCount nodes{__typename ... on CheckRun{name status conclusion startedAt databaseId} ... on StatusContext{context state createdAt}}}}}}}}' -# No pre-loop settle is needed: the required-check anchor below holds the loop -# until the expected checks have registered. -deadline=$(( $(date +%s) + CI_WAIT_TIMEOUT_SECONDS )) -while true; do - sleep 10 +# Outer loop: rebase onto the LATEST base, wait for CI, then re-check the base +# right before merging. If another PR merged during CI (base advanced), re-rebase +# and re-verify so we never merge a commit that was only CI'd against a stale +# base. The pre-merge re-check narrows -- but cannot fully close -- this window; +# the gap between that check and the merge call itself needs a merge queue. +MAX_REBASE_ATTEMPTS=3 +rebase_attempt=0 +while true; do # OUTER: rebase / CI / re-check base / merge + rebase_attempt=$((rebase_attempt + 1)) + if [[ "$rebase_attempt" -gt "$MAX_REBASE_ATTEMPTS" ]]; then + echo "Base advanced through $MAX_REBASE_ATTEMPTS CI cycles; giving up. Re-run /integrate." + exit 1 + fi + + git fetch origin $BASE_BRANCH + base_sha=$(git rev-parse "origin/$BASE_BRANCH") + if ! git rebase "origin/$BASE_BRANCH"; then + git rebase --abort || true + echo "Cannot rebase $HEAD_BRANCH onto updated $BASE_BRANCH (conflict). Cancelling integration." + exit 1 + fi + if ! git push --force-with-lease; then + echo "Could not force-push rebased $HEAD_BRANCH (was it pushed to during CI?). Cancelling integration." + exit 1 + fi + HEAD_BRANCH_HEAD=$(git rev-parse HEAD) + echo "Rebased HEAD is $HEAD_BRANCH_HEAD (attempt $rebase_attempt/$MAX_REBASE_ATTEMPTS onto base $base_sha)" + + # No pre-loop settle is needed: the required-check anchor below holds the loop + # until the expected checks have registered. + deadline=$(( $(date +%s) + CI_WAIT_TIMEOUT_SECONDS )) + while true; do + sleep 10 if (( $(date +%s) > deadline )); then echo "Timed out after ${CI_WAIT_TIMEOUT_SECONDS}s waiting for CI on $HEAD_BRANCH @ $HEAD_BRANCH_HEAD. Cancelling integration." @@ -275,6 +297,16 @@ if [[ -n "$required_names" ]]; then fi fi +# Re-check the base right before merging: if it advanced during CI, another PR +# merged, so re-rebase and re-verify rather than merge a stale-CI'd commit. This +# only narrows the race -- the gap between here and the merge call still needs a +# merge queue to fully close. +git fetch origin $BASE_BRANCH +if [[ "$(git rev-parse "origin/$BASE_BRANCH")" != "$base_sha" ]]; then + echo "Base advanced during CI; re-rebasing and re-verifying before merge." + continue +fi + # Hit the merge button. Pass sha=$HEAD_BRANCH_HEAD so GitHub only merges if the # branch head still matches the exact commit CI validated (a race push aborts). MERGE_COMMIT_TITLE="Merge branch '$HEAD_BRANCH' on behalf of $USER_FULL_NAME" @@ -319,3 +351,6 @@ if [[ $merge_resp != *"Pull Request successfully merged"* ]]; then echo "Could not merge PR. Error from GitHub: '$merge_resp'" exit 1 fi + +break # OUTER: merged successfully +done From 07887faaa8a583e331852f0c31d3c309e6309041 Mon Sep 17 00:00:00 2001 From: Sean Kirby Date: Fri, 3 Jul 2026 11:33:51 -0400 Subject: [PATCH 15/18] SPAR-348: Fix: cap total CI-wait across retries; shrink pre-merge race window Two low-severity items from the race-loop verification: - Anchor the CI-wait deadline ONCE before the outer loop so CI_WAIT_TIMEOUT_SECONDS is a total budget across rebase retries, instead of a fresh full timeout per attempt (which could stack past the enclosing job's own 6h timeout and lose the clean give-up path). - Assemble the full merge payload -- including the ADD_CHANGE_LOGS PR-comments fetch -- BEFORE the pre-merge base re-check, so the only work left between the re-check and the merge PUT is the PUT itself, keeping the residual TOCTOU window as small as possible. Verdict of the pass was ship-it; both fixes are fail-closed hardening. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01CUQaePmRdFCZU5tNqBdTps --- entrypoint.sh | 42 ++++++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/entrypoint.sh b/entrypoint.sh index 33b7af8..57a1282 100755 --- a/entrypoint.sh +++ b/entrypoint.sh @@ -47,8 +47,8 @@ API_HEADER="Accept: application/vnd.github.v3+json" AUTH_HEADER="Authorization: token $GITHUB_TOKEN" # CI-wait configuration (all optional; defaults preserve prior behavior). -# Max seconds to wait for CI before giving up (fail-closed). Must exceed the -# slowest check yet stay under this job's own timeout (GitHub's default is 6h). +# Total seconds to wait for CI, across all rebase retries, before giving up +# (fail-closed). Keep it under the enclosing job's own timeout (GitHub default 6h). CI_WAIT_TIMEOUT_SECONDS="${CI_WAIT_TIMEOUT_SECONDS:-14400}" # JSON array of rules pairing path prefixes with required check names, e.g. # [ {"checks":["buildkite/packmanager"]}, @@ -187,6 +187,10 @@ REPO="${GITHUB_REPOSITORY##*/}" # shellcheck disable=SC2016 GQL_QUERY='query($owner:String!,$name:String!,$oid:GitObjectID!){repository(owner:$owner,name:$name){object(oid:$oid){... on Commit{statusCheckRollup{contexts(first:100){totalCount nodes{__typename ... on CheckRun{name status conclusion startedAt databaseId} ... on StatusContext{context state createdAt}}}}}}}}' +# One overall CI-wait budget shared across all rebase attempts, so a base that +# keeps advancing can't run the action past the enclosing job's own timeout. +deadline=$(( $(date +%s) + CI_WAIT_TIMEOUT_SECONDS )) + # Outer loop: rebase onto the LATEST base, wait for CI, then re-check the base # right before merging. If another PR merged during CI (base advanced), re-rebase # and re-verify so we never merge a commit that was only CI'd against a stale @@ -215,9 +219,7 @@ while true; do # OUTER: rebase / CI / re-check base / merge HEAD_BRANCH_HEAD=$(git rev-parse HEAD) echo "Rebased HEAD is $HEAD_BRANCH_HEAD (attempt $rebase_attempt/$MAX_REBASE_ATTEMPTS onto base $base_sha)" - # No pre-loop settle is needed: the required-check anchor below holds the loop - # until the expected checks have registered. - deadline=$(( $(date +%s) + CI_WAIT_TIMEOUT_SECONDS )) + # Wait for CI on this rebased commit (against the shared deadline above). while true; do sleep 10 @@ -297,18 +299,10 @@ if [[ -n "$required_names" ]]; then fi fi -# Re-check the base right before merging: if it advanced during CI, another PR -# merged, so re-rebase and re-verify rather than merge a stale-CI'd commit. This -# only narrows the race -- the gap between here and the merge call still needs a -# merge queue to fully close. -git fetch origin $BASE_BRANCH -if [[ "$(git rev-parse "origin/$BASE_BRANCH")" != "$base_sha" ]]; then - echo "Base advanced during CI; re-rebasing and re-verifying before merge." - continue -fi - -# Hit the merge button. Pass sha=$HEAD_BRANCH_HEAD so GitHub only merges if the -# branch head still matches the exact commit CI validated (a race push aborts). +# Assemble the merge payload (including fetching change-log comments) BEFORE the +# base re-check, so the only work left between the re-check and the merge call is +# the merge itself -- keeping the residual race window as small as possible. +# sha=$HEAD_BRANCH_HEAD makes GitHub merge only the exact commit CI validated. MERGE_COMMIT_TITLE="Merge branch '$HEAD_BRANCH' on behalf of $USER_FULL_NAME" if [[ "$ACTION_MODE" == "hotfix" ]]; then MERGE_COMMIT_TITLE="$MERGE_COMMIT_TITLE [skip tests]" @@ -336,17 +330,25 @@ if [[ $ADD_CHANGE_LOGS = "true" ]]; then --arg message "$MERGE_COMMIT_MESSAGE" \ --arg sha "$HEAD_BRANCH_HEAD" \ '{commit_title: $title, commit_message: $message, sha: $sha}' ) - - merge_resp=$(curl -X PUT -s -H "${AUTH_HEADER}" -H "${API_HEADER}" -d "$JSON_STRING" "${PR_URL}/merge") else JSON_STRING=$( jq -n \ --arg title "$MERGE_COMMIT_TITLE" \ --arg sha "$HEAD_BRANCH_HEAD" \ '{commit_title: $title, sha: $sha}' ) +fi - merge_resp=$(curl -X PUT -s -H "${AUTH_HEADER}" -H "${API_HEADER}" -d "$JSON_STRING" "${PR_URL}/merge") +# Re-check the base as late as possible: if it advanced during CI, another PR +# merged, so re-rebase and re-verify rather than merge a stale-CI'd commit. The +# only work after this is the merge PUT itself; the residual gap between here and +# that call needs a merge queue to fully close. +git fetch origin $BASE_BRANCH +if [[ "$(git rev-parse "origin/$BASE_BRANCH")" != "$base_sha" ]]; then + echo "Base advanced during CI; re-rebasing and re-verifying before merge." + continue fi +merge_resp=$(curl -X PUT -s -H "${AUTH_HEADER}" -H "${API_HEADER}" -d "$JSON_STRING" "${PR_URL}/merge") + if [[ $merge_resp != *"Pull Request successfully merged"* ]]; then echo "Could not merge PR. Error from GitHub: '$merge_resp'" exit 1 From 5bc53bb5a2227f5a4de09894da66280632e1dc3f Mon Sep 17 00:00:00 2001 From: Sean Kirby Date: Fri, 3 Jul 2026 12:39:28 -0400 Subject: [PATCH 16/18] SPAR-348: Fix: evaluate checks per (name, source); drop dead payload validator Code-review findings 1 & 2 (silent false-green): latest_per_name grouped only by name, and every StatusContext normalized to id:0, so two distinct checks sharing a name (two apps' check-runs, or a legacy status and an Actions check-run) were collapsed to the highest-id run and the other's failure was dropped. - normalize_rollup now stamps a "source" on each node ("check:" or "status") and latest_per_name groups by (name, source), so reruns still dedup within a source but distinct same-named checks are kept and each is gated. - required_checks_pending/failures evaluate ALL runs for a required name (across sources) instead of collapsing to one, so a failing source can't be masked. - Remove check_runs_payload_valid (dead: the GraphQL path uses rollup_payload_valid) and its tests; fix the stale REST-payload header comment. - Add multi-source unit fixtures (two apps same name; status vs check same name). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01CUQaePmRdFCZU5tNqBdTps --- ci_checks.sh | 70 +++++++++++++++++++++--------------------- test/ci_checks_test.sh | 25 ++++++++++----- 2 files changed, 53 insertions(+), 42 deletions(-) diff --git a/ci_checks.sh b/ci_checks.sh index 095667c..981c3c7 100644 --- a/ci_checks.sh +++ b/ci_checks.sh @@ -2,31 +2,25 @@ # Pure helpers for evaluating CI state on a commit. No network and no globals: # every input is an argument, so these are unit-testable with fixtures. # -# The "$1" argument is a GET /commits/{sha}/check-runs REST payload: -# {"total_count":N,"check_runs":[{name,status,conclusion,started_at,id},...]} -# GitHub can list SEVERAL runs for the same name (reruns, concurrency-cancelled -# then re-created). Every helper evaluates only the LATEST run per name so a -# superseded run never masks or fails the current one. "Latest" = highest -# check-run id (monotonic at creation), then started_at as a tiebreak. +# "$1" is a check-runs-shaped payload as produced by normalize_rollup: +# {"check_runs":[{name,status,conclusion,started_at,id,source},...]} +# "source" distinguishes a check's origin (a check-run's app, or "status" for a +# legacy commit-status context). GitHub can list several runs for the same name +# (reruns), and two different sources can share a name; helpers evaluate the +# LATEST run per (name, source) -- newest id, then started_at -- so a rerun never +# masks the current run and two distinct same-named checks are never collapsed. # Check-run conclusions we treat as passing. Anything else on a completed run # (failure, timed_out, cancelled, action_required, stale, or null/unknown) -# blocks the merge. +# blocks the merge. (skipped passes: a skipped check must not block a merge.) CI_OK_CONCLUSIONS='["success","neutral","skipped"]' -# jq prelude defining `latest_per_name`: check_runs reduced to the newest run per -# name, with unnamed runs dropped (a null name cannot be an object key). +# jq prelude defining `latest_per_name`: reduce .check_runs to the newest run per +# (name, source). Unnamed runs are dropped (a null name cannot be an object key). _ci_jq_latest='def latest_per_name: - [ (.check_runs // []) | map(select(.name != null)) | group_by(.name)[] + [ (.check_runs // []) | map(select(.name != null)) | group_by([.name, .source])[] | max_by([(.id // 0), (.started_at // "")]) ];' -# True if $1 is a well-formed check-runs payload (has a .check_runs array), so -# callers can distinguish it from a transient API error body ({"message":...}) -# and keep polling instead of aborting. -check_runs_payload_valid() { - jq -e '(.check_runs | type) == "array"' <<<"$1" >/dev/null 2>&1 -} - # True if $1 is a usable GraphQL statusCheckRollup response: no top-level errors # and the commit object resolved. (A resolved commit with no checks yet has a # null rollup, which is still usable -> normalizes to an empty set.) @@ -39,7 +33,8 @@ rollup_payload_valid() { # CheckRuns are evaluated uniformly. StatusContext.state maps onto (status, # conclusion): SUCCESS -> completed/success; FAILURE|ERROR -> completed/failure; # PENDING|EXPECTED -> in_progress/none (i.e. not yet completed). CheckRun enums -# are lowercased to match the REST vocabulary the helpers expect. +# are lowercased. "source" keeps a status and a same-named check-run (or two +# same-named check-runs from different apps) distinct so neither is dropped. normalize_rollup() { jq '{ check_runs: [ @@ -49,26 +44,28 @@ normalize_rollup() { status: ((.status // "") | ascii_downcase), conclusion: (if .conclusion == null then null else (.conclusion | ascii_downcase) end), started_at: .startedAt, - id: (.databaseId // 0) } + id: (.databaseId // 0), + source: ("check:" + ((.checkSuite.app.databaseId // 0) | tostring)) } else { name: .context, status: (if (.state == "SUCCESS" or .state == "FAILURE" or .state == "ERROR") then "completed" else "in_progress" end), conclusion: (if .state == "SUCCESS" then "success" elif (.state == "FAILURE" or .state == "ERROR") then "failure" else null end), started_at: .createdAt, - id: 0 } + id: 0, + source: "status" } end ] }' <<<"$1" } -# Count latest-per-name check-runs that have not finished yet. +# Count latest-per-(name,source) check-runs that have not finished yet. check_runs_incomplete_count() { jq -r "$_ci_jq_latest"' latest_per_name | map(select(.status != "completed")) | length ' <<<"$1" } -# "name: conclusion" for each latest-per-name completed check-run whose +# "name: conclusion" for each latest-per-(name,source) completed check-run whose # conclusion is not acceptable. Empty output = all completed runs passed. check_runs_failures() { jq -r --argjson ok "$CI_OK_CONCLUSIONS" "$_ci_jq_latest"' @@ -80,30 +77,33 @@ check_runs_failures() { } # Of the comma-separated required names in $2 (surrounding whitespace trimmed), -# print those NOT yet present-and-completed (absent from the commit, or latest -# run still running). Empty output = every required check has a completed latest -# run. Used to keep waiting until the expected checks actually show up. +# print those NOT yet present-and-completed: absent from the commit, or ANY of +# their runs (across sources) not yet completed. Empty output = every required +# name has at least one run and all its runs have completed. required_checks_pending() { jq -r --arg req "$2" "$_ci_jq_latest"' - (latest_per_name | map({ (.name): . }) | add // {}) as $byname + (latest_per_name) as $runs | ($req | split(",") | map(gsub("^\\s+|\\s+$"; "")) | map(select(length > 0)))[] - | select( ($byname[.] // null) == null or $byname[.].status != "completed" ) + | . as $name + | ([ $runs[] | select(.name == $name) ]) as $entries + | select( ($entries | length) == 0 or ($entries | any(.status != "completed")) ) ' <<<"$1" } # Of the comma-separated required names in $2 (surrounding whitespace trimmed), -# print "name: reason" for each that is absent ("missing") or whose latest -# completed conclusion is not acceptable. Empty output = every required check is -# present and passed. +# print "name: reason" for each that is absent ("missing") or has ANY completed +# run (across sources) whose conclusion is not acceptable. Empty output = every +# required name is present and all its runs passed. required_checks_failures() { jq -r --arg req "$2" --argjson ok "$CI_OK_CONCLUSIONS" "$_ci_jq_latest"' - (latest_per_name | map({ (.name): . }) | add // {}) as $byname + (latest_per_name) as $runs | ($req | split(",") | map(gsub("^\\s+|\\s+$"; "")) | map(select(length > 0)))[] | . as $name - | ($byname[$name] // null) as $run - | if $run == null then "\($name): missing" - elif ($ok | index($run.conclusion)) then empty - else "\($name): \($run.conclusion // "incomplete")" + | ([ $runs[] | select(.name == $name) ]) as $entries + | if ($entries | length) == 0 then "\($name): missing" + else + ([ $entries[] | select(.status == "completed") | select(.conclusion as $c | ($ok | index($c)) | not) ]) as $bad + | if ($bad | length) > 0 then "\($name): \($bad[0].conclusion // "incomplete")" else empty end end ' <<<"$1" } diff --git a/test/ci_checks_test.sh b/test/ci_checks_test.sh index 7cf9d99..aad7397 100644 --- a/test/ci_checks_test.sh +++ b/test/ci_checks_test.sh @@ -44,7 +44,6 @@ RERUN='{"total_count":2,"check_runs":[ RERUN_RUNNING='{"total_count":2,"check_runs":[ {"name":"e2e","status":"completed","conclusion":"success","started_at":"2026-07-02T10:00:00Z"}, {"name":"e2e","status":"in_progress","conclusion":null,"started_at":"2026-07-02T10:30:00Z"}]}' -ERROR_PAYLOAD='{"message":"Not Found","documentation_url":"https://docs.github.com/rest"}' echo "# incomplete-count / failures (latest run per name)" assert_eq "all green: 0 incomplete" "0" "$(check_runs_incomplete_count "$ALL_GREEN")" @@ -62,12 +61,6 @@ assert_eq "rerun success supersedes cancelled: no failures" "" "$(check_runs_fai assert_eq "rerun success supersedes cancelled: 0 incomplete" "0" "$(check_runs_incomplete_count "$RERUN")" assert_eq "rerun still running: 1 incomplete" "1" "$(check_runs_incomplete_count "$RERUN_RUNNING")" -echo "# payload validity guard" -assert_ok "valid check-runs payload" check_runs_payload_valid "$ALL_GREEN" -assert_ok "empty check-runs payload valid" check_runs_payload_valid "$EMPTY" -assert_notok "error payload invalid" check_runs_payload_valid "$ERROR_PAYLOAD" -assert_notok "empty string invalid" check_runs_payload_valid "" - echo "# required_checks_pending (names absent or latest run not completed)" assert_eq "required all present+completed: none pending" "" \ "$(required_checks_pending "$ALL_GREEN" "test,client_test,e2e")" @@ -156,6 +149,24 @@ assert_eq "normalized: required legacy status failure reported" "buildkite/packm assert_eq "normalized null rollup: 0 incomplete" "0" \ "$(check_runs_incomplete_count "$(normalize_rollup "$ROLLUP_NULL")")" +echo "# same name from two sources is not collapsed (bugs 1 & 2)" +# Two check-runs named "build" from different apps: one fails, one passes. +ROLLUP_TWO_APPS='{"data":{"repository":{"object":{"statusCheckRollup":{"contexts":{"nodes":[ + {"__typename":"CheckRun","name":"build","status":"COMPLETED","conclusion":"FAILURE","startedAt":"2026-07-02T10:00:00Z","databaseId":50,"checkSuite":{"app":{"databaseId":111}}}, + {"__typename":"CheckRun","name":"build","status":"COMPLETED","conclusion":"SUCCESS","startedAt":"2026-07-02T10:01:00Z","databaseId":100,"checkSuite":{"app":{"databaseId":222}}}]}}}}}}' +# A legacy status "test" (FAILURE) alongside an Actions check-run "test" (SUCCESS). +ROLLUP_STATUS_VS_CHECK='{"data":{"repository":{"object":{"statusCheckRollup":{"contexts":{"nodes":[ + {"__typename":"CheckRun","name":"test","status":"COMPLETED","conclusion":"SUCCESS","startedAt":"2026-07-02T10:00:00Z","databaseId":5,"checkSuite":{"app":{"databaseId":15368}}}, + {"__typename":"StatusContext","context":"test","state":"FAILURE","createdAt":"2026-07-02T11:00:00Z"}]}}}}}}' +assert_eq "two apps same name: failing app not masked" "build: failure" \ + "$(check_runs_failures "$(normalize_rollup "$ROLLUP_TWO_APPS")")" +assert_eq "two apps same name: required build fails" "build: failure" \ + "$(required_checks_failures "$(normalize_rollup "$ROLLUP_TWO_APPS")" "build")" +assert_eq "status vs check same name: failing status not masked" "test: failure" \ + "$(check_runs_failures "$(normalize_rollup "$ROLLUP_STATUS_VS_CHECK")")" +assert_eq "status vs check same name: required test fails" "test: failure" \ + "$(required_checks_failures "$(normalize_rollup "$ROLLUP_STATUS_VS_CHECK")" "test")" + echo "# any_path_has_prefix" FILES_MIXED=$'SensrTrxMES/app/x.js\nPackManager/db/schema.rb' FILES_PM_ONLY=$'PackManager/db/schema.rb\n.github/workflows/integrate.yml' From a452a5f07fbd7a21953a700333a57b996b64157c Mon Sep 17 00:00:00 2001 From: Sean Kirby Date: Fri, 3 Jul 2026 12:39:29 -0400 Subject: [PATCH 17/18] SPAR-348: Fix: fail-closed config/scope errors; per-attempt CI budget; set-and-forget Code-review findings 3, 4, and the deadline tradeoff: - Malformed (non-array) REQUIRED_CHECKS now exits 1 instead of silently degrading to present-checks-only (was fail-open on a config typo). - Validate CI_WAIT_TIMEOUT_SECONDS and MAX_REBASE_ATTEMPTS are positive integers up front (rejects "0600" octal, "4h", "", negatives) so a bad value fails clearly instead of a misleading instant timeout. - pr_touches_paths now exits 1 (fast, fail-closed) when it can't fetch the PR file list after retries, instead of returning "in scope" and then blocking on a required check that never runs until the timeout. - CI-wait deadline is now PER rebase attempt (fresh budget each attempt) so a late base-advance doesn't starve the re-rebase's CI; CI_WAIT_TIMEOUT_SECONDS default lowered to 2h and MAX_REBASE_ATTEMPTS is configurable (default 100) for "set and forget" -- the job's own timeout-minutes is the real cap. Documented. - Query fetches checkSuite.app.databaseId to source-tag check-runs (see prior commit). Corrected the stale "defaults preserve prior behavior" comment. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01CUQaePmRdFCZU5tNqBdTps --- README.md | 5 ++++- entrypoint.sh | 50 +++++++++++++++++++++++++++++++++----------------- 2 files changed, 37 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 73f72e4..62c21dc 100644 --- a/README.md +++ b/README.md @@ -63,7 +63,8 @@ All optional, passed via `env:` on the action step: |---|---|---| | `GITHUB_TOKEN` | — | **Required.** Token allowed to merge into the PR's base branch. | | `ADD_CHANGE_LOGS` | `false` | Collect `Change log:` PR comments into the merge commit message. | -| `CI_WAIT_TIMEOUT_SECONDS` | `14400` (4h) | Give up waiting for CI after this many seconds (fail, don't merge). Keep it above your slowest check and below the job's own timeout (GitHub's default is 6h). | +| `CI_WAIT_TIMEOUT_SECONDS` | `7200` (2h) | Give up waiting for CI after this many seconds **per rebase attempt** (fail, don't merge). Must be a positive integer (no leading zeros / units) and above your slowest required check, or the action cancels a healthy PR. | +| `MAX_REBASE_ATTEMPTS` | `100` | How many times to rebase onto the latest base and re-run CI when the base advances during CI. High by default (set-and-forget); the enclosing job's own `timeout-minutes` is the real backstop for total runtime. Positive integer. | | `REQUIRED_CHECKS` | _(empty)_ | JSON array of rules pairing path prefixes with check names that must be **present** (and pass) before merging — matching GitHub Actions check-runs *and* legacy status contexts (e.g. `buildkite/packmanager`). A rule with no `paths` always applies; with `paths` it applies only when the PR changes a file under one of those prefixes. The action *always* requires every check present on the commit to pass; these rules additionally require the named checks to have appeared, closing the window where a check hasn't registered yet and an empty/partial set looks "green". You do **not** list every check — a new check is caught by the always-on "all present must pass" rule — but name at least one reliably-running check per product as an anchor. Example: `[{"checks":["buildkite/packmanager"]},{"paths":["some/dir/"],"checks":["test","e2e"]}]` | Example (a monorepo: Buildkite gates one product, GitHub Actions gates another): @@ -79,6 +80,8 @@ Notes: - Check state is read from GitHub's GraphQL `statusCheckRollup`, so Actions check-runs and legacy status contexts are gated the same way — the action works for Actions-only, status-only, or mixed repos, and needs no branch-protection required checks. - Without `REQUIRED_CHECKS` the action still won't merge on an *empty* check set (it waits for at least one check to appear), but it can only gate on whatever has appeared by then; set `REQUIRED_CHECKS` to guarantee specific checks ran. +- The action waits `CI_WAIT_TIMEOUT_SECONDS` **per rebase attempt** and retries up to `MAX_REBASE_ATTEMPTS` times, so worst-case runtime is roughly `CI_WAIT_TIMEOUT_SECONDS × MAX_REBASE_ATTEMPTS`. For long "set and forget" runs on a busy base, raise the workflow job's `timeout-minutes` (GitHub's default job timeout is 6h) — it, not this action, is the ultimate cap. +- A `skipped` (or `neutral`) check passes the gate — a skipped check never blocks a merge. - Each anchor should be a check that registers no later than the checks it stands in for (prefer a fast-registering check-run over a slow external status), so the "all present" rule can't finish before a sibling has appeared. - Check names and path prefixes must not contain commas. - If a commit has more than 100 checks, the action refuses to merge (it cannot see them all) rather than merging on a partial view. diff --git a/entrypoint.sh b/entrypoint.sh index 57a1282..f1f84db 100755 --- a/entrypoint.sh +++ b/entrypoint.sh @@ -46,10 +46,18 @@ URI=https://api.github.com API_HEADER="Accept: application/vnd.github.v3+json" AUTH_HEADER="Authorization: token $GITHUB_TOKEN" -# CI-wait configuration (all optional; defaults preserve prior behavior). -# Total seconds to wait for CI, across all rebase retries, before giving up -# (fail-closed). Keep it under the enclosing job's own timeout (GitHub default 6h). -CI_WAIT_TIMEOUT_SECONDS="${CI_WAIT_TIMEOUT_SECONDS:-14400}" +# CI-wait configuration (all optional). NOTE: v2 gates differently from v1 (see +# the README); leaving these unset does NOT reproduce v1's legacy-status-only gate. + +# Seconds to wait for CI PER rebase attempt before giving up (fail-closed). The +# action retries across rebases (see MAX_REBASE_ATTEMPTS), so total runtime is +# bounded by the enclosing job's own timeout -- raise the job's timeout-minutes +# for long "set and forget" runs. +CI_WAIT_TIMEOUT_SECONDS="${CI_WAIT_TIMEOUT_SECONDS:-7200}" +# How many times to rebase onto the latest base and re-run CI when the base keeps +# advancing during CI. High by default so /integrate is "set and forget"; the +# enclosing job's own timeout is the ultimate backstop. +MAX_REBASE_ATTEMPTS="${MAX_REBASE_ATTEMPTS:-100}" # JSON array of rules pairing path prefixes with required check names, e.g. # [ {"checks":["buildkite/packmanager"]}, # {"paths":["some/dir/"],"checks":["test","e2e"]} ] @@ -60,6 +68,15 @@ CI_WAIT_TIMEOUT_SECONDS="${CI_WAIT_TIMEOUT_SECONDS:-14400}" # gates only on whatever checks are present on the commit. REQUIRED_CHECKS="${REQUIRED_CHECKS:-}" +# Reject non-positive-integer numeric config up front (e.g. "4h", "0600", "") so +# it fails clearly instead of yielding a misleading timeout or a wrong bound. +for _var in CI_WAIT_TIMEOUT_SECONDS MAX_REBASE_ATTEMPTS; do + if ! [[ "${!_var}" =~ ^[1-9][0-9]*$ ]]; then + echo "$_var must be a positive integer, got '${!_var}'. Cancelling integration." + exit 1 + fi +done + USER_URL=$(jq -r ".comment.user.url" "$GITHUB_EVENT_PATH") user_resp=$(curl -X GET -s -H "${API_HEADER}" -H "${AUTH_HEADER}" "${USER_URL}") @@ -110,9 +127,10 @@ git fetch origin $HEAD_BRANCH git checkout -b $HEAD_BRANCH origin/$HEAD_BRANCH # Does the PR touch any of the given comma-separated path prefixes? Paginates the -# PR file list. Returns 0 (in scope) on a match OR if the file list can't be -# fetched (fail-closed: an undeterminable scope must not silently weaken the -# gate); returns 1 only when the full file list is known and matches nothing. +# PR file list. Returns 0 on a match, 1 when the full file list is known and +# matches nothing. If the file list can't be fetched after retries it exits the +# whole action (fail-closed: an undeterminable scope must not silently weaken the +# gate, and failing fast beats blocking on a check that will never run). pr_touches_paths() { local prefixes_csv="$1" page=1 resp count files while : ; do @@ -126,8 +144,8 @@ pr_touches_paths() { sleep 3 done if [[ -z "$resp" ]]; then - echo "Could not fetch changed files after retries; enforcing required checks (fail-closed)." >&2 - return 0 + echo "Could not fetch the PR's changed files after retries; cannot determine required-check scope. Cancelling integration (re-run /integrate)." + exit 1 fi count=$(jq 'length' <<<"$resp") if [[ "$count" -eq 0 ]]; then @@ -169,7 +187,8 @@ if [[ -n "$REQUIRED_CHECKS" ]]; then fi done else - echo "REQUIRED_CHECKS is not a JSON array; ignoring it." >&2 + echo "REQUIRED_CHECKS is set but is not a JSON array. Cancelling integration (fix the config)." + exit 1 fi fi if [[ -n "$required_names" ]]; then @@ -185,18 +204,13 @@ OWNER="${GITHUB_REPOSITORY%%/*}" REPO="${GITHUB_REPOSITORY##*/}" # $owner/$name/$oid are GraphQL variables, not shell -- single quotes are correct. # shellcheck disable=SC2016 -GQL_QUERY='query($owner:String!,$name:String!,$oid:GitObjectID!){repository(owner:$owner,name:$name){object(oid:$oid){... on Commit{statusCheckRollup{contexts(first:100){totalCount nodes{__typename ... on CheckRun{name status conclusion startedAt databaseId} ... on StatusContext{context state createdAt}}}}}}}}' - -# One overall CI-wait budget shared across all rebase attempts, so a base that -# keeps advancing can't run the action past the enclosing job's own timeout. -deadline=$(( $(date +%s) + CI_WAIT_TIMEOUT_SECONDS )) +GQL_QUERY='query($owner:String!,$name:String!,$oid:GitObjectID!){repository(owner:$owner,name:$name){object(oid:$oid){... on Commit{statusCheckRollup{contexts(first:100){totalCount nodes{__typename ... on CheckRun{name status conclusion startedAt databaseId checkSuite{app{databaseId}}} ... on StatusContext{context state createdAt}}}}}}}}' # Outer loop: rebase onto the LATEST base, wait for CI, then re-check the base # right before merging. If another PR merged during CI (base advanced), re-rebase # and re-verify so we never merge a commit that was only CI'd against a stale # base. The pre-merge re-check narrows -- but cannot fully close -- this window; # the gap between that check and the merge call itself needs a merge queue. -MAX_REBASE_ATTEMPTS=3 rebase_attempt=0 while true; do # OUTER: rebase / CI / re-check base / merge rebase_attempt=$((rebase_attempt + 1)) @@ -219,7 +233,9 @@ while true; do # OUTER: rebase / CI / re-check base / merge HEAD_BRANCH_HEAD=$(git rev-parse HEAD) echo "Rebased HEAD is $HEAD_BRANCH_HEAD (attempt $rebase_attempt/$MAX_REBASE_ATTEMPTS onto base $base_sha)" - # Wait for CI on this rebased commit (against the shared deadline above). + # Fresh CI-wait budget for THIS rebase attempt, so a base advance late in one + # attempt doesn't starve the next attempt's CI. + deadline=$(( $(date +%s) + CI_WAIT_TIMEOUT_SECONDS )) while true; do sleep 10 From 57c1e1ea75a4784c76013779444245917bd4b9c7 Mon Sep 17 00:00:00 2001 From: Sean Kirby Date: Fri, 3 Jul 2026 12:41:57 -0400 Subject: [PATCH 18/18] SPAR-348: Test: cover multi-source required-check edge cases One source of a required name still running keeps the name pending; both sources passing clears both pending and failures. Guards the (name, source) required-gate rewrite against regressions. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01CUQaePmRdFCZU5tNqBdTps --- test/ci_checks_test.sh | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/ci_checks_test.sh b/test/ci_checks_test.sh index aad7397..8732b7a 100644 --- a/test/ci_checks_test.sh +++ b/test/ci_checks_test.sh @@ -166,6 +166,20 @@ assert_eq "status vs check same name: failing status not masked" "test: failure" "$(check_runs_failures "$(normalize_rollup "$ROLLUP_STATUS_VS_CHECK")")" assert_eq "status vs check same name: required test fails" "test: failure" \ "$(required_checks_failures "$(normalize_rollup "$ROLLUP_STATUS_VS_CHECK")" "test")" +# One source of a required name still running -> the name stays pending. +ROLLUP_ONE_SOURCE_RUNNING='{"data":{"repository":{"object":{"statusCheckRollup":{"contexts":{"nodes":[ + {"__typename":"CheckRun","name":"build","status":"COMPLETED","conclusion":"SUCCESS","startedAt":"2026-07-02T10:00:00Z","databaseId":50,"checkSuite":{"app":{"databaseId":111}}}, + {"__typename":"CheckRun","name":"build","status":"IN_PROGRESS","conclusion":null,"startedAt":"2026-07-02T10:01:00Z","databaseId":100,"checkSuite":{"app":{"databaseId":222}}}]}}}}}}' +assert_eq "one source running: required build still pending" "build" \ + "$(required_checks_pending "$(normalize_rollup "$ROLLUP_ONE_SOURCE_RUNNING")" "build")" +# Both sources of a required name pass -> no failure, nothing pending. +ROLLUP_TWO_APPS_PASS='{"data":{"repository":{"object":{"statusCheckRollup":{"contexts":{"nodes":[ + {"__typename":"CheckRun","name":"build","status":"COMPLETED","conclusion":"SUCCESS","startedAt":"2026-07-02T10:00:00Z","databaseId":50,"checkSuite":{"app":{"databaseId":111}}}, + {"__typename":"CheckRun","name":"build","status":"COMPLETED","conclusion":"SUCCESS","startedAt":"2026-07-02T10:01:00Z","databaseId":100,"checkSuite":{"app":{"databaseId":222}}}]}}}}}}' +assert_eq "two apps both pass: required build not pending" "" \ + "$(required_checks_pending "$(normalize_rollup "$ROLLUP_TWO_APPS_PASS")" "build")" +assert_eq "two apps both pass: required build no failure" "" \ + "$(required_checks_failures "$(normalize_rollup "$ROLLUP_TWO_APPS_PASS")" "build")" echo "# any_path_has_prefix" FILES_MIXED=$'SensrTrxMES/app/x.js\nPackManager/db/schema.rb'