From a73ce075063e155d25980f279bf4b55e4446c463 Mon Sep 17 00:00:00 2001 From: Timo Derstappen Date: Thu, 23 Apr 2026 09:01:07 +0200 Subject: [PATCH] fix: pr approve-align-files evaluates GitHub Actions check_runs Previously the loop over check_runs was skipped when combinedStatus state was 'success' or 'failure'. combinedStatus only reflects legacy commit statuses (e.g. CircleCI), so a green CircleCI made the tool ignore failing GitHub Actions checks like 'pre-commit', causing PRs to be approved (and auto-merged on repos without required status checks) despite real failures. Made-with: Cursor --- CHANGELOG.md | 4 ++++ cmd/pr/approvealign/runner.go | 36 +++++++++++++++++++---------------- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a8b5f8a5e..541673080 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ## [Unreleased] +### Fixed + +- `pr approve-align-files`: also evaluate GitHub Actions check_runs when the legacy commit status (e.g. CircleCI) is `success`, so a failing Actions check such as `pre-commit` no longer slips through and gets approved. + ## [7.40.0] - 2026-04-23 ### Added diff --git a/cmd/pr/approvealign/runner.go b/cmd/pr/approvealign/runner.go index e8fadc081..49c4502b5 100644 --- a/cmd/pr/approvealign/runner.go +++ b/cmd/pr/approvealign/runner.go @@ -203,24 +203,28 @@ func (r *runner) processPR(ctx context.Context, githubClient *github.Client, ps } } - // Check individual check runs (GitHub Actions checks) + // Check individual check runs (GitHub Actions checks). + // + // combinedStatus only reflects legacy commit statuses (e.g. CircleCI's + // "ci/circleci: ..."). GitHub Actions report via the check_runs API + // instead, so a "success" combinedStatus does NOT imply that all + // GitHub Actions checks have passed. Always evaluate check_runs so a + // failing Actions check (e.g. "pre-commit") is not silently ignored + // when CircleCI is green. if checkRuns != nil && len(checkRuns.CheckRuns) > 0 && !hasFailedChecks { - // Only check runs if combinedStatus didn't already give us a definitive answer - if combinedStatus == nil || (combinedStatus.GetState() != "success" && combinedStatus.GetState() != "failure") { - for _, run := range checkRuns.CheckRuns { - conclusion := run.GetConclusion() - status := run.GetStatus() - - if status == "completed" { - if conclusion == "failure" || conclusion == "cancelled" || conclusion == "timed_out" { - hasFailedChecks = true - break - } - // success, neutral, skipped are OK - } else { - // Check is not completed (queued, in_progress) - checksPending = true + for _, run := range checkRuns.CheckRuns { + conclusion := run.GetConclusion() + status := run.GetStatus() + + if status == "completed" { + if conclusion == "failure" || conclusion == "cancelled" || conclusion == "timed_out" { + hasFailedChecks = true + break } + // success, neutral, skipped are OK + } else { + // Check is not completed (queued, in_progress) + checksPending = true } } }