Skip to content

Commit 087a9ae

Browse files
committed
ci: harden release gate required check matching
1 parent 7c60dab commit 087a9ae

3 files changed

Lines changed: 393 additions & 67 deletions

File tree

.claude/hooks/main-ci-status.sh

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -158,32 +158,49 @@ checks_jsonl=$(gh api \
158158
# that each define a `detect-changes` job). Mirrors the Ruby dedup in
159159
# `validate_main_ci_status!` (rakelib/release.rake). Keep the two in sync.
160160
checks_json=$(echo "${checks_jsonl}" | jq -s '
161-
[.[] | {id, name, status, conclusion, html_url, suite_id: (.check_suite.id // .id)}]
162-
| group_by([.suite_id, .name])
161+
[.[] | {id, name, status, conclusion, html_url, suite_id: (.check_suite.id // .id), app_id: (.app.id // null)}]
162+
| group_by([.suite_id, .name, .app_id])
163163
| map(max_by(.id))
164164
' 2>/dev/null) || fail_open "jq slurp failed"
165165

166166
required_json=$(gh api \
167167
"repos/${repo_slug}/branches/main/protection/required_status_checks" \
168-
--jq '(.contexts // []) + (.checks // [] | map(.context)) | unique' \
168+
--jq '{contexts: (.contexts // []), checks: (.checks // [] | map({context, app_id}))}' \
169169
2>/dev/null || echo "null")
170170
case "${required_json}" in
171-
\[*\]) ;;
171+
\{*\}) ;;
172172
*) required_json="null" ;;
173173
esac
174174

175175
# Aggregate counts with jq. `success`, `skipped`, `neutral` are all "passing".
176176
# Anything completed with another conclusion is a failure. Anything not yet
177177
# completed is in_progress.
178178
summary=$(echo "${checks_json}" | jq -r --argjson required_names "${required_json}" '
179+
def app_wildcard($app_id): $app_id == null or $app_id == -1;
180+
def check_matches($required):
181+
.name == $required.context and (app_wildcard($required.app_id) or .app_id == $required.app_id);
182+
def check_label:
183+
if app_wildcard(.app_id) then .context else "\(.context) (app_id: \(.app_id))" end;
184+
179185
. as $all
180-
| ($all | map(.name)) as $observed_names
181186
| {
182187
total: length,
183188
passed: [.[] | select(.status == "completed" and (.conclusion | IN("success", "skipped", "neutral")))] | length,
184189
failed: [.[] | select(.status == "completed" and (.conclusion | IN("success", "skipped", "neutral") | not))],
185190
in_progress: [.[] | select(.status != "completed")],
186-
missing_required: (if $required_names == null then [] else ($required_names - $observed_names) end)
191+
missing_required: (
192+
if $required_names == null then []
193+
else
194+
# NOTE: commit statuses are not fetched here; this hook is a fail-open
195+
# display tool. The Ruby release gate owns legacy status enforcement.
196+
(($required_names.contexts // [])
197+
| map(select(. as $context | ($all | any(.name == $context) | not))))
198+
+
199+
(($required_names.checks // [])
200+
| map(select(. as $required | ($all | any(. as $run | ($run | check_matches($required))) | not)))
201+
| map(check_label))
202+
end
203+
)
187204
}
188205
| "TOTAL=\(.total)",
189206
"PASSED=\(.passed)",

rakelib/release.rake

Lines changed: 185 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -608,9 +608,7 @@ def fetch_main_ci_checks(monorepo_root:, allow_override: false, dry_run: false)
608608
end
609609

610610
begin
611-
check_runs = output.lines.reject { |line| line.strip.empty? }.map do |line|
612-
JSON.parse(line)
613-
end
611+
check_runs = parse_gh_jsonl(output)
614612
rescue JSON::ParserError => e
615613
handle_main_ci_status_violation!(
616614
message: "❌ Failed to parse check_runs response from gh: #{e.message}\n\nOutput:\n#{output}",
@@ -624,14 +622,93 @@ def fetch_main_ci_checks(monorepo_root:, allow_override: false, dry_run: false)
624622
end
625623
# rubocop:enable Metrics/MethodLength
626624

625+
def parse_gh_jsonl(output)
626+
output.lines.reject { |line| line.strip.empty? }.map do |line|
627+
JSON.parse(line)
628+
end
629+
end
630+
631+
def fetch_main_commit_statuses(repo_slug:, sha:, allow_override:, dry_run:)
632+
api_path = "repos/#{repo_slug}/commits/#{sha}/statuses"
633+
634+
begin
635+
output, status = Open3.capture2e(
636+
"gh", "api", "--paginate", "--jq", ".[]", api_path
637+
)
638+
rescue Errno::ENOENT
639+
handle_main_ci_status_violation!(
640+
message: "❌ GitHub CLI (`gh`) is not installed. Install it from https://cli.github.com/ and retry.",
641+
allow_override: allow_override,
642+
dry_run: dry_run
643+
)
644+
return nil
645+
end
646+
647+
unless status.success?
648+
handle_main_ci_status_violation!(
649+
message: "❌ Unable to query GitHub Statuses API for #{sha}.\n\n#{output}",
650+
allow_override: allow_override,
651+
dry_run: dry_run
652+
)
653+
return nil
654+
end
655+
656+
begin
657+
parse_gh_jsonl(output)
658+
rescue JSON::ParserError => e
659+
handle_main_ci_status_violation!(
660+
message: "❌ Failed to parse statuses response from gh: #{e.message}\n\nOutput:\n#{output}",
661+
allow_override: allow_override,
662+
dry_run: dry_run
663+
)
664+
nil
665+
end
666+
end
667+
668+
def normalize_status_as_check_run(status)
669+
state = status["state"]
670+
{
671+
"id" => status["id"],
672+
"name" => status["context"],
673+
"status" => state == "pending" ? "pending" : "completed",
674+
"conclusion" => state == "success" ? "success" : (state || "error"),
675+
"html_url" => status["target_url"] || status["url"]
676+
}
677+
end
678+
679+
def latest_commit_statuses(statuses)
680+
statuses
681+
.group_by { |status| status["context"] }
682+
.map { |_context, context_statuses| context_statuses.max_by { |status| status["id"].to_i } }
683+
end
684+
685+
def normalize_required_check_entries(checks)
686+
Array(checks).filter_map do |check|
687+
context = check["context"].to_s
688+
next if context.empty?
689+
690+
{ context: context, app_id: check["app_id"]&.to_i }
691+
end.uniq
692+
end
693+
694+
def normalize_required_checks_payload(parsed)
695+
return nil unless parsed.is_a?(Hash)
696+
697+
contexts = Array(parsed["contexts"]).map(&:to_s).reject(&:empty?).uniq
698+
checks = normalize_required_check_entries(parsed["checks"])
699+
700+
# No required names parseable is treated the same as "no branch protection
701+
# visible" — fail-safe to evaluating every check run.
702+
contexts.empty? && checks.empty? ? nil : { contexts: contexts, checks: checks }
703+
end
704+
627705
def required_check_names_for_main(monorepo_root:, repo_slug: nil)
628706
repo_slug ||= github_repo_slug(monorepo_root)
629707
api_path = "repos/#{repo_slug}/branches/main/protection/required_status_checks"
630-
# Combine the legacy `contexts` list (older protection rules) with the newer
631-
# `checks[].context` list. Branch protection set up via the `checks` API
632-
# leaves `contexts` as `[]`, so reading only `contexts` would yield an empty
633-
# array and trip the `:no_required_checks` abort path even when CI is green.
634-
jq_query = "(.contexts // []) + (.checks // [] | map(.context)) | unique"
708+
# Keep legacy `contexts` separate from modern `checks` entries. Modern
709+
# required checks can be pinned to a GitHub App via `app_id`; legacy contexts
710+
# may be satisfied by either a Checks API run or a commit-status context.
711+
jq_query = "{contexts: (.contexts // []), checks: (.checks // [] | map({context, app_id}))}"
635712
# Precondition: `fetch_main_ci_checks` already verified `gh` is installed
636713
# before `validate_main_ci_status!` calls this helper. The remaining failure
637714
# mode here is "branch protection unknown", which returns nil so the caller
@@ -644,16 +721,54 @@ def required_check_names_for_main(monorepo_root:, repo_slug: nil)
644721

645722
begin
646723
parsed = JSON.parse(output)
647-
return nil unless parsed.is_a?(Array)
648-
649-
# Empty array (no required names parseable) is treated the same as "no
650-
# branch protection visible" — fail-safe to evaluating every check run.
651-
parsed.empty? ? nil : parsed
724+
normalize_required_checks_payload(parsed)
652725
rescue JSON::ParserError
653726
nil
654727
end
655728
end
656729

730+
def check_run_app_id(run)
731+
app_id = run.dig("app", "id")
732+
app_id&.to_i
733+
end
734+
735+
def required_check_app_wildcard?(app_id)
736+
app_id.nil? || app_id.to_i == -1
737+
end
738+
739+
def required_check_matches_run?(required_check, run)
740+
required_check[:context] == run["name"] &&
741+
(required_check_app_wildcard?(required_check[:app_id]) || required_check[:app_id].to_i == check_run_app_id(run))
742+
end
743+
744+
def required_check_label(required_check)
745+
return required_check[:context] if required_check_app_wildcard?(required_check[:app_id])
746+
747+
"#{required_check[:context]} (app_id: #{required_check[:app_id]})"
748+
end
749+
750+
def required_check_labels(required_checks)
751+
(required_checks[:contexts] + required_checks[:checks].map { |check| required_check_label(check) }).uniq
752+
end
753+
754+
def missing_required_check_labels(required_checks:, check_runs:, legacy_status_runs:)
755+
missing_modern = required_checks[:checks].reject do |required_check|
756+
check_runs.any? { |run| required_check_matches_run?(required_check, run) }
757+
end
758+
missing_legacy = required_checks[:contexts].reject do |context|
759+
check_runs.any? { |run| run["name"] == context } || legacy_status_runs.any? { |run| run["name"] == context }
760+
end
761+
762+
(missing_legacy + missing_modern.map { |check| required_check_label(check) }).uniq
763+
end
764+
765+
def legacy_status_runs_for_required_contexts(required_checks:, check_runs:, statuses:)
766+
latest_commit_statuses(statuses)
767+
.select { |status| required_checks[:contexts].include?(status["context"]) }
768+
.reject { |status| check_runs.any? { |run| run["name"] == status["context"] } }
769+
.map { |status| normalize_status_as_check_run(status) }
770+
end
771+
657772
def format_main_ci_status_violation(kind:, short_sha:, runs:) # rubocop:disable Metrics/CyclomaticComplexity
658773
header = case kind
659774
when :in_progress
@@ -721,15 +836,6 @@ def validate_main_ci_status!(monorepo_root:, is_prerelease:, allow_override:, dr
721836
repo_slug = data[:repo_slug]
722837
check_runs = data[:check_runs]
723838

724-
if check_runs.empty?
725-
handle_main_ci_status_violation!(
726-
message: format_main_ci_status_violation(kind: :no_checks, short_sha: short_sha, runs: nil),
727-
allow_override: allow_override,
728-
dry_run: dry_run
729-
)
730-
return
731-
end
732-
733839
# Collapse multiple runs per (check_suite_id, name) to the most recent
734840
# attempt (highest check_run id). The key intentionally includes
735841
# check_suite_id so we only collapse *true* reruns (same workflow run,
@@ -745,7 +851,7 @@ def validate_main_ci_status!(monorepo_root:, is_prerelease:, allow_override:, dr
745851
# another. The GitHub Actions Checks API always populates `check_suite`,
746852
# so this only matters for external check integrations.
747853
check_runs = check_runs
748-
.group_by { |run| [run.dig("check_suite", "id") || run["id"], run["name"]] }
854+
.group_by { |run| [run.dig("check_suite", "id") || run["id"], run["name"], check_run_app_id(run)] }
749855
.map { |_key, runs| runs.max_by { |run| run["id"].to_i } }
750856

751857
# Always query branch-protection required checks (when configured) so the
@@ -756,12 +862,57 @@ def validate_main_ci_status!(monorepo_root:, is_prerelease:, allow_override:, dr
756862
required_args = { monorepo_root: monorepo_root }
757863
required_args[:repo_slug] = repo_slug if repo_slug
758864
required_names = required_check_names_for_main(**required_args)
865+
legacy_status_runs = []
866+
if required_names && required_names[:contexts].any?
867+
statuses = fetch_main_commit_statuses(
868+
repo_slug: repo_slug,
869+
sha: sha,
870+
allow_override: allow_override,
871+
dry_run: dry_run
872+
)
873+
return if statuses.nil?
874+
875+
legacy_status_runs = legacy_status_runs_for_required_contexts(
876+
required_checks: required_names,
877+
check_runs: check_runs,
878+
statuses: statuses
879+
)
880+
end
881+
882+
if check_runs.empty? && legacy_status_runs.empty?
883+
handle_main_ci_status_violation!(
884+
message: format_main_ci_status_violation(kind: :no_checks, short_sha: short_sha, runs: nil),
885+
allow_override: allow_override,
886+
dry_run: dry_run
887+
)
888+
return
889+
end
890+
759891
evaluated = if is_prerelease && required_names
760-
check_runs.select { |run| required_names.include?(run["name"]) }
892+
check_runs.select do |run|
893+
required_names[:contexts].include?(run["name"]) ||
894+
required_names[:checks].any? { |required_check| required_check_matches_run?(required_check, run) }
895+
end + legacy_status_runs
761896
else
762-
check_runs
897+
check_runs + legacy_status_runs
763898
end
764899

900+
# Report visible failures before missing/in-progress runs. If both are
901+
# present, the operator needs to know about the failure right away; this also
902+
# prevents same-label legacy/modern required checks from hiding a failed
903+
# legacy status behind a "missing required" message.
904+
failed = evaluated.select do |run|
905+
run["status"] == "completed" && !CI_PASSING_CONCLUSIONS.include?(run["conclusion"])
906+
end
907+
if failed.any?
908+
handle_main_ci_status_violation!(
909+
message: format_main_ci_status_violation(kind: :failed, short_sha: short_sha, runs: failed),
910+
allow_override: allow_override,
911+
dry_run: dry_run
912+
)
913+
return
914+
end
915+
765916
# When branch protection lists required checks, treat any missing required
766917
# check as blocking — for stable AND prerelease. Branch protection would
767918
# refuse the merge in this state, so a release that ignored the gap would
@@ -771,43 +922,31 @@ def validate_main_ci_status!(monorepo_root:, is_prerelease:, allow_override:, dr
771922
# required workflows ran, others never registered — usually a renamed or
772923
# deleted workflow that branch protection still requires).
773924
unless required_names.nil?
774-
observed_names = check_runs.map { |run| run["name"] }
775-
missing_names = required_names - observed_names
776-
if missing_names.length == required_names.length
925+
required_labels = required_check_labels(required_names)
926+
missing_names = missing_required_check_labels(
927+
required_checks: required_names,
928+
check_runs: check_runs,
929+
legacy_status_runs: legacy_status_runs
930+
)
931+
if missing_names.length == required_labels.length
777932
handle_main_ci_status_violation!(
778933
message: format_main_ci_status_violation(kind: :no_required_checks, short_sha: short_sha, runs: nil) +
779-
"\nRequired: #{required_names.join(', ')}",
934+
"\nRequired: #{required_labels.join(', ')}",
780935
allow_override: allow_override,
781936
dry_run: dry_run
782937
)
783938
return
784939
elsif missing_names.any?
785940
handle_main_ci_status_violation!(
786941
message: format_main_ci_status_violation(kind: :missing_required_checks, short_sha: short_sha, runs: nil) +
787-
"\nRequired: #{required_names.join(', ')}\nMissing: #{missing_names.join(', ')}",
942+
"\nRequired: #{required_labels.join(', ')}\nMissing: #{missing_names.join(', ')}",
788943
allow_override: allow_override,
789944
dry_run: dry_run
790945
)
791946
return
792947
end
793948
end
794949

795-
# Report failures before in-progress runs. If both are present, the operator
796-
# needs to know about the failure right away — telling them to "wait or
797-
# override" would just make them wait and re-run before seeing the real
798-
# blocker.
799-
failed = evaluated.select do |run|
800-
run["status"] == "completed" && !CI_PASSING_CONCLUSIONS.include?(run["conclusion"])
801-
end
802-
if failed.any?
803-
handle_main_ci_status_violation!(
804-
message: format_main_ci_status_violation(kind: :failed, short_sha: short_sha, runs: failed),
805-
allow_override: allow_override,
806-
dry_run: dry_run
807-
)
808-
return
809-
end
810-
811950
in_progress = evaluated.select { |run| CI_INCOMPLETE_STATUSES.include?(run["status"]) }
812951
if in_progress.any?
813952
handle_main_ci_status_violation!(

0 commit comments

Comments
 (0)