From 6101cfdfa022f3ecb78cc924e014667169f51d08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Falc=C3=B3n?= Date: Mon, 2 Feb 2026 08:58:47 +0100 Subject: [PATCH] fix: Prevent automerge when CI checks have failed (#95) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - Fixes a race condition where the automerge workflow merged PR #94 despite `browser-tests` failing - Adds SHA verification to ensure the CI run matches the PR's latest commit (prevents stale merges from earlier pushes) - Adds a failed-checks guard that inspects all PR check statuses before merging ## What happened PR #94 had two CI runs: an earlier one that passed and a later one where `browser-tests` failed. The automerge workflow triggered on the first successful run and merged the PR before the second run completed, because it only checked `workflow_run.conclusion == 'success'` without verifying it was for the latest commit. ## Test plan - [x] Push a PR with the Automerge label, verify it merges when all checks pass - [x] Push a follow-up commit that breaks browser-tests — verify the earlier success doesn't trigger a merge - [x] Verify PRs without the Automerge label are unaffected --- .github/workflows/automerge.yml | 19 ++++++++++++++++++- tests/Browser/BankAccountsTest.php | 8 ++++---- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/.github/workflows/automerge.yml b/.github/workflows/automerge.yml index 2ad9beb9..9974202c 100644 --- a/.github/workflows/automerge.yml +++ b/.github/workflows/automerge.yml @@ -36,5 +36,22 @@ jobs: exit 0 fi - echo "PR #$PR_NUMBER has Automerge label and CI passed. Merging..." + # Ensure the CI run matches the PR's latest commit to avoid merging stale results + CI_SHA="${{ github.event.workflow_run.head_sha }}" + PR_SHA=$(gh pr view "$PR_NUMBER" --repo "$REPO" --json headRefOid --jq '.headRefOid') + + if [ "$CI_SHA" != "$PR_SHA" ]; then + echo "CI ran on $CI_SHA but PR head is $PR_SHA, skipping" + exit 0 + fi + + # Ensure no checks have failed on the PR + FAILED=$(gh pr checks "$PR_NUMBER" --repo "$REPO" --json state --jq '[.[] | select(.state == "FAILURE")] | length') + + if [ "$FAILED" -gt 0 ]; then + echo "PR #$PR_NUMBER has $FAILED failed check(s), skipping merge" + exit 0 + fi + + echo "PR #$PR_NUMBER has Automerge label and all checks passed. Merging..." gh pr merge "$PR_NUMBER" --squash --repo "$REPO" --delete-branch diff --git a/tests/Browser/BankAccountsTest.php b/tests/Browser/BankAccountsTest.php index c4699c0d..fa22507a 100644 --- a/tests/Browser/BankAccountsTest.php +++ b/tests/Browser/BankAccountsTest.php @@ -148,7 +148,7 @@ it('can edit an existing account via dropdown menu', function () { // Create account via UI to ensure it syncs to IndexedDB createAccountViaUI($page, 'Old Account Name', 'Edit Bank', 'Checking', 'USD'); - $page->navigate('/settings/accounts')->wait(3); + $page->navigate('/settings/accounts')->wait(5); $page->assertSee('Bank accounts') ->click('button[aria-label="Open menu"]') @@ -160,7 +160,7 @@ it('can edit an existing account via dropdown menu', function () { ->click('button[type="submit"]:has-text("Update")') ->wait(2); - $page->navigate('/settings/accounts')->wait(3); + $page->navigate('/settings/accounts')->wait(5); $page->assertSee('Updated Account Name') ->assertNoJavascriptErrors(); @@ -179,7 +179,7 @@ it('can delete an account via dropdown menu', function () { // Create account via UI to ensure it syncs to IndexedDB createAccountViaUI($page, 'Account To Delete', 'Delete Bank', 'Checking', 'USD'); - $page->navigate('/settings/accounts')->wait(3); + $page->navigate('/settings/accounts')->wait(5); $page->assertSee('Bank accounts') ->assertSee('Account To Delete') @@ -193,7 +193,7 @@ it('can delete an account via dropdown menu', function () { ->click('button[type="submit"]:has-text("Delete")') ->wait(2); - $page->navigate('/settings/accounts')->wait(3); + $page->navigate('/settings/accounts')->wait(5); $page->assertDontSee('Account To Delete') ->assertNoJavascriptErrors();