From 17851dc9817b7a4b324d91993012b279ac12c46a Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Sun, 12 Jul 2026 01:16:43 +0200 Subject: [PATCH 01/16] fix(auto-merge): gate arming on current-head review and pre-merge checks The privileged workflow approved and armed trusted-bot PRs immediately, so a bot PR could merge on green CI alone with no current-head review or pre-merge result (live case ksail#6058). A fail-closed gate script now proves both surfaces at the exact current head before the approve/arm steps run; anything missing, stale, mixed, superseded, or unparseable skips arming and leaves the PR to the maintenance agent's live check. Co-Authored-By: Claude Fable 5 --- .../comments.json | 9 ++ .../reviews.json | 10 ++ .../comments.json | 23 ++++ .../reviews.json | 1 + .../codex-findings-at-head/comments.json | 16 +++ .../codex-findings-at-head/reviews.json | 1 + .../comments.json | 16 +++ .../reviews.json | 1 + .../comments.json | 9 ++ .../reviews.json | 10 ++ .github/tests/merge-gate-fixtures/index.json | 54 ++++++++ .../newest-summary-wins/comments.json | 16 +++ .../newest-summary-wins/reviews.json | 10 ++ .../no-review-at-all/comments.json | 9 ++ .../no-review-at-all/reviews.json | 1 + .../premerge-failed-full/comments.json | 9 ++ .../premerge-failed-full/reviews.json | 10 ++ .../premerge-marker-missing/comments.json | 9 ++ .../premerge-marker-missing/reviews.json | 10 ++ .../premerge-mixed-compact/comments.json | 9 ++ .../premerge-mixed-compact/reviews.json | 10 ++ .../premerge-not-posted/comments.json | 1 + .../premerge-not-posted/reviews.json | 10 ++ .../stale-codex-clean/comments.json | 16 +++ .../stale-codex-clean/reviews.json | 1 + .../stale-cr-approval/comments.json | 9 ++ .../stale-cr-approval/reviews.json | 10 ++ .../test-enable-auto-merge-pentad-gate.sh | 53 ++++++++ .github/workflows/ci.yaml | 22 +++ .github/workflows/enable-auto-merge.yaml | 31 +++++ .scripts/check-merge-gates.sh | 126 ++++++++++++++++++ README.md | 2 +- 32 files changed, 523 insertions(+), 1 deletion(-) create mode 100644 .github/tests/merge-gate-fixtures/changes-requested-not-approval/comments.json create mode 100644 .github/tests/merge-gate-fixtures/changes-requested-not-approval/reviews.json create mode 100644 .github/tests/merge-gate-fixtures/codex-clean-superseded-by-findings/comments.json create mode 100644 .github/tests/merge-gate-fixtures/codex-clean-superseded-by-findings/reviews.json create mode 100644 .github/tests/merge-gate-fixtures/codex-findings-at-head/comments.json create mode 100644 .github/tests/merge-gate-fixtures/codex-findings-at-head/reviews.json create mode 100644 .github/tests/merge-gate-fixtures/green-codex-at-head-premerge-full/comments.json create mode 100644 .github/tests/merge-gate-fixtures/green-codex-at-head-premerge-full/reviews.json create mode 100644 .github/tests/merge-gate-fixtures/green-cr-at-head-premerge-compact/comments.json create mode 100644 .github/tests/merge-gate-fixtures/green-cr-at-head-premerge-compact/reviews.json create mode 100644 .github/tests/merge-gate-fixtures/index.json create mode 100644 .github/tests/merge-gate-fixtures/newest-summary-wins/comments.json create mode 100644 .github/tests/merge-gate-fixtures/newest-summary-wins/reviews.json create mode 100644 .github/tests/merge-gate-fixtures/no-review-at-all/comments.json create mode 100644 .github/tests/merge-gate-fixtures/no-review-at-all/reviews.json create mode 100644 .github/tests/merge-gate-fixtures/premerge-failed-full/comments.json create mode 100644 .github/tests/merge-gate-fixtures/premerge-failed-full/reviews.json create mode 100644 .github/tests/merge-gate-fixtures/premerge-marker-missing/comments.json create mode 100644 .github/tests/merge-gate-fixtures/premerge-marker-missing/reviews.json create mode 100644 .github/tests/merge-gate-fixtures/premerge-mixed-compact/comments.json create mode 100644 .github/tests/merge-gate-fixtures/premerge-mixed-compact/reviews.json create mode 100644 .github/tests/merge-gate-fixtures/premerge-not-posted/comments.json create mode 100644 .github/tests/merge-gate-fixtures/premerge-not-posted/reviews.json create mode 100644 .github/tests/merge-gate-fixtures/stale-codex-clean/comments.json create mode 100644 .github/tests/merge-gate-fixtures/stale-codex-clean/reviews.json create mode 100644 .github/tests/merge-gate-fixtures/stale-cr-approval/comments.json create mode 100644 .github/tests/merge-gate-fixtures/stale-cr-approval/reviews.json create mode 100755 .github/tests/test-enable-auto-merge-pentad-gate.sh create mode 100755 .scripts/check-merge-gates.sh diff --git a/.github/tests/merge-gate-fixtures/changes-requested-not-approval/comments.json b/.github/tests/merge-gate-fixtures/changes-requested-not-approval/comments.json new file mode 100644 index 00000000..740b8ad2 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/changes-requested-not-approval/comments.json @@ -0,0 +1,9 @@ +[ + { + "user": { + "login": "coderabbitai[bot]" + }, + "body": "\n\n\ud83d\udea5 Pre-merge checks | \u2705 5\n", + "created_at": "2026-07-11T10:00:00Z" + } +] \ No newline at end of file diff --git a/.github/tests/merge-gate-fixtures/changes-requested-not-approval/reviews.json b/.github/tests/merge-gate-fixtures/changes-requested-not-approval/reviews.json new file mode 100644 index 00000000..2f72cd61 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/changes-requested-not-approval/reviews.json @@ -0,0 +1,10 @@ +[ + { + "user": { + "login": "coderabbitai[bot]" + }, + "state": "CHANGES_REQUESTED", + "commit_id": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "submitted_at": "2026-07-11T10:00:00Z" + } +] \ No newline at end of file diff --git a/.github/tests/merge-gate-fixtures/codex-clean-superseded-by-findings/comments.json b/.github/tests/merge-gate-fixtures/codex-clean-superseded-by-findings/comments.json new file mode 100644 index 00000000..07d7ec8e --- /dev/null +++ b/.github/tests/merge-gate-fixtures/codex-clean-superseded-by-findings/comments.json @@ -0,0 +1,23 @@ +[ + { + "user": { + "login": "chatgpt-codex-connector[bot]" + }, + "body": "Codex Review: Didn't find any major issues.\n\n**Reviewed commit:** aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "created_at": "2026-07-11T09:00:00Z" + }, + { + "user": { + "login": "chatgpt-codex-connector[bot]" + }, + "body": "Codex Review: found 2 issues.\n\n**Reviewed commit:** aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "created_at": "2026-07-11T11:00:00Z" + }, + { + "user": { + "login": "coderabbitai[bot]" + }, + "body": "\n\n\ud83d\udea5 Pre-merge checks | \u2705 5\n", + "created_at": "2026-07-11T10:00:00Z" + } +] \ No newline at end of file diff --git a/.github/tests/merge-gate-fixtures/codex-clean-superseded-by-findings/reviews.json b/.github/tests/merge-gate-fixtures/codex-clean-superseded-by-findings/reviews.json new file mode 100644 index 00000000..0637a088 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/codex-clean-superseded-by-findings/reviews.json @@ -0,0 +1 @@ +[] \ No newline at end of file diff --git a/.github/tests/merge-gate-fixtures/codex-findings-at-head/comments.json b/.github/tests/merge-gate-fixtures/codex-findings-at-head/comments.json new file mode 100644 index 00000000..11129864 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/codex-findings-at-head/comments.json @@ -0,0 +1,16 @@ +[ + { + "user": { + "login": "chatgpt-codex-connector[bot]" + }, + "body": "Codex Review: found 2 issues.\n\n**Reviewed commit:** aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "created_at": "2026-07-11T10:00:00Z" + }, + { + "user": { + "login": "coderabbitai[bot]" + }, + "body": "\n\n\ud83d\udea5 Pre-merge checks | \u2705 5\n", + "created_at": "2026-07-11T10:00:00Z" + } +] \ No newline at end of file diff --git a/.github/tests/merge-gate-fixtures/codex-findings-at-head/reviews.json b/.github/tests/merge-gate-fixtures/codex-findings-at-head/reviews.json new file mode 100644 index 00000000..0637a088 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/codex-findings-at-head/reviews.json @@ -0,0 +1 @@ +[] \ No newline at end of file diff --git a/.github/tests/merge-gate-fixtures/green-codex-at-head-premerge-full/comments.json b/.github/tests/merge-gate-fixtures/green-codex-at-head-premerge-full/comments.json new file mode 100644 index 00000000..30676367 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/green-codex-at-head-premerge-full/comments.json @@ -0,0 +1,16 @@ +[ + { + "user": { + "login": "chatgpt-codex-connector[bot]" + }, + "body": "Codex Review: Didn't find any major issues.\n\n**Reviewed commit:** aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "created_at": "2026-07-11T10:00:00Z" + }, + { + "user": { + "login": "coderabbitai[bot]" + }, + "body": "\n\n## Pre-merge checks\n| Title check | \u2705 Passed | ok |\n| Linked Issues check | \u2705 Passed | ok |\n", + "created_at": "2026-07-11T10:00:00Z" + } +] \ No newline at end of file diff --git a/.github/tests/merge-gate-fixtures/green-codex-at-head-premerge-full/reviews.json b/.github/tests/merge-gate-fixtures/green-codex-at-head-premerge-full/reviews.json new file mode 100644 index 00000000..0637a088 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/green-codex-at-head-premerge-full/reviews.json @@ -0,0 +1 @@ +[] \ No newline at end of file diff --git a/.github/tests/merge-gate-fixtures/green-cr-at-head-premerge-compact/comments.json b/.github/tests/merge-gate-fixtures/green-cr-at-head-premerge-compact/comments.json new file mode 100644 index 00000000..740b8ad2 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/green-cr-at-head-premerge-compact/comments.json @@ -0,0 +1,9 @@ +[ + { + "user": { + "login": "coderabbitai[bot]" + }, + "body": "\n\n\ud83d\udea5 Pre-merge checks | \u2705 5\n", + "created_at": "2026-07-11T10:00:00Z" + } +] \ No newline at end of file diff --git a/.github/tests/merge-gate-fixtures/green-cr-at-head-premerge-compact/reviews.json b/.github/tests/merge-gate-fixtures/green-cr-at-head-premerge-compact/reviews.json new file mode 100644 index 00000000..f9c4581c --- /dev/null +++ b/.github/tests/merge-gate-fixtures/green-cr-at-head-premerge-compact/reviews.json @@ -0,0 +1,10 @@ +[ + { + "user": { + "login": "coderabbitai[bot]" + }, + "state": "APPROVED", + "commit_id": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "submitted_at": "2026-07-11T10:00:00Z" + } +] \ No newline at end of file diff --git a/.github/tests/merge-gate-fixtures/index.json b/.github/tests/merge-gate-fixtures/index.json new file mode 100644 index 00000000..4bf0cd64 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/index.json @@ -0,0 +1,54 @@ +[ + { + "name": "green-cr-at-head-premerge-compact", + "expect_green": true + }, + { + "name": "green-codex-at-head-premerge-full", + "expect_green": true + }, + { + "name": "stale-cr-approval", + "expect_green": false + }, + { + "name": "stale-codex-clean", + "expect_green": false + }, + { + "name": "codex-findings-at-head", + "expect_green": false + }, + { + "name": "codex-clean-superseded-by-findings", + "expect_green": false + }, + { + "name": "no-review-at-all", + "expect_green": false + }, + { + "name": "premerge-not-posted", + "expect_green": false + }, + { + "name": "premerge-mixed-compact", + "expect_green": false + }, + { + "name": "premerge-failed-full", + "expect_green": false + }, + { + "name": "premerge-marker-missing", + "expect_green": false + }, + { + "name": "newest-summary-wins", + "expect_green": false + }, + { + "name": "changes-requested-not-approval", + "expect_green": false + } +] \ No newline at end of file diff --git a/.github/tests/merge-gate-fixtures/newest-summary-wins/comments.json b/.github/tests/merge-gate-fixtures/newest-summary-wins/comments.json new file mode 100644 index 00000000..3430cd72 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/newest-summary-wins/comments.json @@ -0,0 +1,16 @@ +[ + { + "user": { + "login": "coderabbitai[bot]" + }, + "body": "\n\n\ud83d\udea5 Pre-merge checks | \u2705 5\n", + "created_at": "2026-07-11T09:00:00Z" + }, + { + "user": { + "login": "coderabbitai[bot]" + }, + "body": "\n\n\ud83d\udea5 Pre-merge checks | \u2705 4 | \u274c 1\n", + "created_at": "2026-07-11T11:00:00Z" + } +] \ No newline at end of file diff --git a/.github/tests/merge-gate-fixtures/newest-summary-wins/reviews.json b/.github/tests/merge-gate-fixtures/newest-summary-wins/reviews.json new file mode 100644 index 00000000..f9c4581c --- /dev/null +++ b/.github/tests/merge-gate-fixtures/newest-summary-wins/reviews.json @@ -0,0 +1,10 @@ +[ + { + "user": { + "login": "coderabbitai[bot]" + }, + "state": "APPROVED", + "commit_id": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "submitted_at": "2026-07-11T10:00:00Z" + } +] \ No newline at end of file diff --git a/.github/tests/merge-gate-fixtures/no-review-at-all/comments.json b/.github/tests/merge-gate-fixtures/no-review-at-all/comments.json new file mode 100644 index 00000000..740b8ad2 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/no-review-at-all/comments.json @@ -0,0 +1,9 @@ +[ + { + "user": { + "login": "coderabbitai[bot]" + }, + "body": "\n\n\ud83d\udea5 Pre-merge checks | \u2705 5\n", + "created_at": "2026-07-11T10:00:00Z" + } +] \ No newline at end of file diff --git a/.github/tests/merge-gate-fixtures/no-review-at-all/reviews.json b/.github/tests/merge-gate-fixtures/no-review-at-all/reviews.json new file mode 100644 index 00000000..0637a088 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/no-review-at-all/reviews.json @@ -0,0 +1 @@ +[] \ No newline at end of file diff --git a/.github/tests/merge-gate-fixtures/premerge-failed-full/comments.json b/.github/tests/merge-gate-fixtures/premerge-failed-full/comments.json new file mode 100644 index 00000000..4d9cbf8a --- /dev/null +++ b/.github/tests/merge-gate-fixtures/premerge-failed-full/comments.json @@ -0,0 +1,9 @@ +[ + { + "user": { + "login": "coderabbitai[bot]" + }, + "body": "\n\n## Pre-merge checks\n### \u274c Failed checks (1)\n| Docstring Coverage | \u274c Error | low |\n", + "created_at": "2026-07-11T10:00:00Z" + } +] \ No newline at end of file diff --git a/.github/tests/merge-gate-fixtures/premerge-failed-full/reviews.json b/.github/tests/merge-gate-fixtures/premerge-failed-full/reviews.json new file mode 100644 index 00000000..f9c4581c --- /dev/null +++ b/.github/tests/merge-gate-fixtures/premerge-failed-full/reviews.json @@ -0,0 +1,10 @@ +[ + { + "user": { + "login": "coderabbitai[bot]" + }, + "state": "APPROVED", + "commit_id": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "submitted_at": "2026-07-11T10:00:00Z" + } +] \ No newline at end of file diff --git a/.github/tests/merge-gate-fixtures/premerge-marker-missing/comments.json b/.github/tests/merge-gate-fixtures/premerge-marker-missing/comments.json new file mode 100644 index 00000000..f5c3d11d --- /dev/null +++ b/.github/tests/merge-gate-fixtures/premerge-marker-missing/comments.json @@ -0,0 +1,9 @@ +[ + { + "user": { + "login": "coderabbitai[bot]" + }, + "body": "\ud83d\udea5 Pre-merge checks | \u2705 5", + "created_at": "2026-07-11T10:00:00Z" + } +] \ No newline at end of file diff --git a/.github/tests/merge-gate-fixtures/premerge-marker-missing/reviews.json b/.github/tests/merge-gate-fixtures/premerge-marker-missing/reviews.json new file mode 100644 index 00000000..f9c4581c --- /dev/null +++ b/.github/tests/merge-gate-fixtures/premerge-marker-missing/reviews.json @@ -0,0 +1,10 @@ +[ + { + "user": { + "login": "coderabbitai[bot]" + }, + "state": "APPROVED", + "commit_id": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "submitted_at": "2026-07-11T10:00:00Z" + } +] \ No newline at end of file diff --git a/.github/tests/merge-gate-fixtures/premerge-mixed-compact/comments.json b/.github/tests/merge-gate-fixtures/premerge-mixed-compact/comments.json new file mode 100644 index 00000000..4ea4bc8e --- /dev/null +++ b/.github/tests/merge-gate-fixtures/premerge-mixed-compact/comments.json @@ -0,0 +1,9 @@ +[ + { + "user": { + "login": "coderabbitai[bot]" + }, + "body": "\n\n\ud83d\udea5 Pre-merge checks | \u2705 4 | \u274c 1\n", + "created_at": "2026-07-11T10:00:00Z" + } +] \ No newline at end of file diff --git a/.github/tests/merge-gate-fixtures/premerge-mixed-compact/reviews.json b/.github/tests/merge-gate-fixtures/premerge-mixed-compact/reviews.json new file mode 100644 index 00000000..f9c4581c --- /dev/null +++ b/.github/tests/merge-gate-fixtures/premerge-mixed-compact/reviews.json @@ -0,0 +1,10 @@ +[ + { + "user": { + "login": "coderabbitai[bot]" + }, + "state": "APPROVED", + "commit_id": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "submitted_at": "2026-07-11T10:00:00Z" + } +] \ No newline at end of file diff --git a/.github/tests/merge-gate-fixtures/premerge-not-posted/comments.json b/.github/tests/merge-gate-fixtures/premerge-not-posted/comments.json new file mode 100644 index 00000000..0637a088 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/premerge-not-posted/comments.json @@ -0,0 +1 @@ +[] \ No newline at end of file diff --git a/.github/tests/merge-gate-fixtures/premerge-not-posted/reviews.json b/.github/tests/merge-gate-fixtures/premerge-not-posted/reviews.json new file mode 100644 index 00000000..f9c4581c --- /dev/null +++ b/.github/tests/merge-gate-fixtures/premerge-not-posted/reviews.json @@ -0,0 +1,10 @@ +[ + { + "user": { + "login": "coderabbitai[bot]" + }, + "state": "APPROVED", + "commit_id": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "submitted_at": "2026-07-11T10:00:00Z" + } +] \ No newline at end of file diff --git a/.github/tests/merge-gate-fixtures/stale-codex-clean/comments.json b/.github/tests/merge-gate-fixtures/stale-codex-clean/comments.json new file mode 100644 index 00000000..381d8ca3 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/stale-codex-clean/comments.json @@ -0,0 +1,16 @@ +[ + { + "user": { + "login": "chatgpt-codex-connector[bot]" + }, + "body": "Codex Review: Didn't find any major issues.\n\n**Reviewed commit:** bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", + "created_at": "2026-07-11T10:00:00Z" + }, + { + "user": { + "login": "coderabbitai[bot]" + }, + "body": "\n\n\ud83d\udea5 Pre-merge checks | \u2705 5\n", + "created_at": "2026-07-11T10:00:00Z" + } +] \ No newline at end of file diff --git a/.github/tests/merge-gate-fixtures/stale-codex-clean/reviews.json b/.github/tests/merge-gate-fixtures/stale-codex-clean/reviews.json new file mode 100644 index 00000000..0637a088 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/stale-codex-clean/reviews.json @@ -0,0 +1 @@ +[] \ No newline at end of file diff --git a/.github/tests/merge-gate-fixtures/stale-cr-approval/comments.json b/.github/tests/merge-gate-fixtures/stale-cr-approval/comments.json new file mode 100644 index 00000000..740b8ad2 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/stale-cr-approval/comments.json @@ -0,0 +1,9 @@ +[ + { + "user": { + "login": "coderabbitai[bot]" + }, + "body": "\n\n\ud83d\udea5 Pre-merge checks | \u2705 5\n", + "created_at": "2026-07-11T10:00:00Z" + } +] \ No newline at end of file diff --git a/.github/tests/merge-gate-fixtures/stale-cr-approval/reviews.json b/.github/tests/merge-gate-fixtures/stale-cr-approval/reviews.json new file mode 100644 index 00000000..7df9c8ef --- /dev/null +++ b/.github/tests/merge-gate-fixtures/stale-cr-approval/reviews.json @@ -0,0 +1,10 @@ +[ + { + "user": { + "login": "coderabbitai[bot]" + }, + "state": "APPROVED", + "commit_id": "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", + "submitted_at": "2026-07-11T10:00:00Z" + } +] \ No newline at end of file diff --git a/.github/tests/test-enable-auto-merge-pentad-gate.sh b/.github/tests/test-enable-auto-merge-pentad-gate.sh new file mode 100755 index 00000000..70d6bca8 --- /dev/null +++ b/.github/tests/test-enable-auto-merge-pentad-gate.sh @@ -0,0 +1,53 @@ +#!/usr/bin/env bash +# Table-driven test for the fail-closed review/pre-merge gate the privileged +# auto-merge workflow runs before approving or arming a trusted-bot PR +# (actions#548). Each fixture is a (reviews.json, comments.json) pair plus the +# expected verdict; the gate must treat every missing, stale, mixed, failed, +# superseded, or unparseable state as NOT green. + +set -euo pipefail + +script="${1:-.scripts/check-merge-gates.sh}" +fixtures_dir="${2:-.github/tests/merge-gate-fixtures}" +workflow="${3:-.github/workflows/enable-auto-merge.yaml}" + +head_sha="$(printf 'a%.0s' {1..40})" +status=0 + +# The workflow must actually consume the gate: the gates step runs the script +# and both privileged steps are conditioned on its armable output. +if ! grep -q 'check-merge-gates.sh' "$workflow"; then + echo "::error file=$workflow::auto-merge workflow does not run check-merge-gates.sh" + status=1 +fi + +armable_conditions="$(yq -r ' + [.jobs."auto-merge".steps[] + | select(.name == "βœ… Approve PR" or .name == "πŸ”€ Enable Auto-Merge") + | .if // ""] + | join("\n")' "$workflow")" +if [[ "$(grep -c "steps.gates.outputs.armable == 'true'" <<<"$armable_conditions")" -ne 2 ]]; then + echo "::error file=$workflow::Approve and Enable Auto-Merge steps must both be gated on steps.gates.outputs.armable" + status=1 +fi + +while IFS= read -r fixture; do + name="$(jq -r '.name' <<<"$fixture")" + expect_green="$(jq -r '.expect_green' <<<"$fixture")" + + actual_green=false + if bash "$script" "$head_sha" \ + "$fixtures_dir/$name/reviews.json" \ + "$fixtures_dir/$name/comments.json" >/dev/null; then + actual_green=true + fi + + if [[ "$actual_green" != "$expect_green" ]]; then + echo "::error file=$fixtures_dir/index.json::fixture '$name' expected green=$expect_green, got $actual_green" + status=1 + else + echo "fixture '$name': green=$actual_green" + fi +done < <(jq -c '.[]' "$fixtures_dir/index.json") + +exit "$status" diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 63b363ed..927fa6a8 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -1797,6 +1797,26 @@ jobs: - name: πŸ§ͺ Verify privileged author gate run: bash .github/tests/test-enable-auto-merge-author-gate.sh + test-enable-auto-merge-pentad-gate: + if: "${{ !startsWith(github.head_ref, 'release-please--') && !startsWith(github.event.head_commit.message, 'chore(main): release ') }}" + name: "[Test] Enable Auto-Merge - Pentad Gate" + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - name: πŸ›‘οΈ Harden runner + uses: step-security/harden-runner@9af89fc71515a100421586dfdb3dc9c984fbf411 # v2.19.4 + with: + egress-policy: audit + + - name: πŸ“‘ Checkout + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: false + + - name: πŸ§ͺ Verify fail-closed review/pre-merge gate + run: bash .github/tests/test-enable-auto-merge-pentad-gate.sh + test-create-release: if: "${{ !startsWith(github.head_ref, 'release-please--') && !startsWith(github.event.head_commit.message, 'chore(main): release ') }}" name: "[Test] Create Release - Dry Run" @@ -2344,6 +2364,7 @@ jobs: - test-run-dotnet-tests-workflow - test-enable-auto-merge - test-enable-auto-merge-author-gate + - test-enable-auto-merge-pentad-gate - test-create-release - test-deploy-github-pages - test-publish-dotnet-library @@ -2424,6 +2445,7 @@ jobs: ${{ needs.test-run-dotnet-tests-workflow.result }} ${{ needs.test-enable-auto-merge.result }} ${{ needs.test-enable-auto-merge-author-gate.result }} + ${{ needs.test-enable-auto-merge-pentad-gate.result }} ${{ needs.test-create-release.result }} ${{ needs.test-deploy-github-pages.result }} ${{ needs.test-publish-dotnet-library.result }} diff --git a/.github/workflows/enable-auto-merge.yaml b/.github/workflows/enable-auto-merge.yaml index 016dea86..b59c1091 100644 --- a/.github/workflows/enable-auto-merge.yaml +++ b/.github/workflows/enable-auto-merge.yaml @@ -46,7 +46,37 @@ jobs: permission-contents: write permission-pull-requests: write + - name: πŸ“‘ Checkout + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: false + + # Fail-closed pentad gate (actions#548): approving and arming are only + # allowed once a CURRENT-HEAD green review (CodeRabbit APPROVED or Codex + # clean pass) AND a green CodeRabbit pre-merge result are both proven. + # Missing, stale, mixed, or unparseable state is NOT green β€” the step + # then skips arming and the maintenance agent arms after its own live + # pentad check. Never weaken this gate to warn-only. + - name: πŸ›‚ Verify review and pre-merge gates + id: gates + env: + GH_TOKEN: ${{ steps.app-token.outputs.token }} + PR_NUMBER: ${{ github.event.pull_request.number }} + REPOSITORY: ${{ github.repository }} + run: | + HEAD_SHA=$(gh pr view "$PR_NUMBER" --repo "$REPOSITORY" --json headRefOid --jq .headRefOid) + gh api "repos/$REPOSITORY/pulls/$PR_NUMBER/reviews" --paginate | jq -s 'add // []' > /tmp/reviews.json + gh api "repos/$REPOSITORY/issues/$PR_NUMBER/comments" --paginate | jq -s 'add // []' > /tmp/comments.json + + if bash .scripts/check-merge-gates.sh "$HEAD_SHA" /tmp/reviews.json /tmp/comments.json; then + echo "armable=true" >> "$GITHUB_OUTPUT" + else + echo "armable=false" >> "$GITHUB_OUTPUT" + echo "::notice::PR #${PR_NUMBER} not armed: current-head review/pre-merge gates are not green (fail-closed). The maintenance agent arms it after its live pentad check." + fi + - name: βœ… Approve PR + if: steps.gates.outputs.armable == 'true' env: GH_TOKEN: ${{ steps.app-token.outputs.token }} PR_NUMBER: ${{ github.event.pull_request.number }} @@ -68,6 +98,7 @@ jobs: fi - name: πŸ”€ Enable Auto-Merge + if: steps.gates.outputs.armable == 'true' env: GH_TOKEN: ${{ steps.app-token.outputs.token }} PR_NUMBER: ${{ github.event.pull_request.number }} diff --git a/.scripts/check-merge-gates.sh b/.scripts/check-merge-gates.sh new file mode 100755 index 00000000..51532173 --- /dev/null +++ b/.scripts/check-merge-gates.sh @@ -0,0 +1,126 @@ +#!/usr/bin/env bash +# Fail-closed review/pre-merge gate for the privileged auto-merge workflow. +# +# Usage: check-merge-gates.sh +# head-sha the pull request's current head commit SHA +# reviews-json file holding the FULL paginated `pulls//reviews` array +# comments-json file holding the FULL paginated `issues//comments` array +# +# Exits 0 only when BOTH gates are proven at the current head: +# 1. a green review β€” a CodeRabbit APPROVED review whose commit_id equals +# the head, or a Codex clean pass ("Didn't find any major issues") whose +# "Reviewed commit" equals the head and is Codex's LATEST result for it; +# 2. a green CodeRabbit pre-merge result β€” the newest auto-generated summary +# (stable marker required) whose pre-merge section is unambiguously +# green: a positive check-mark count and no error/inconclusive/warning +# marks. When walkthrough boundary markers exist, only the bounded +# region is parsed so echoed marker text elsewhere cannot spoof it. +# +# Everything else β€” missing, stale, mixed, unparseable, or absent state β€” is +# NOT green and exits 1 (the workflow then skips arming; the maintenance +# agent arms after its own live pentad check). Never weaken this to warn-only. + +set -euo pipefail + +if [[ $# -ne 3 ]]; then + echo "usage: $0 " >&2 + exit 2 +fi + +head_sha="$1" +reviews_json="$2" +comments_json="$3" + +review_state="missing" +premerge_state="not-posted" + +# --- Gate 1: green review at the current head ------------------------------- + +cr_approved_at_head="$(jq -r --arg sha "$head_sha" ' + [.[] | select(.user.login == "coderabbitai[bot]" and .state == "APPROVED")] + | map(select(.commit_id == $sha)) | length' "$reviews_json")" + +cr_approved_anywhere="$(jq -r ' + [.[] | select(.user.login == "coderabbitai[bot]" and .state == "APPROVED")] + | length' "$reviews_json")" + +if [[ "$cr_approved_at_head" -gt 0 ]]; then + review_state="green" +elif [[ "$cr_approved_anywhere" -gt 0 ]]; then + review_state="stale" +fi + +if [[ "$review_state" != "green" ]]; then + # Codex lane: its result is an ISSUE COMMENT carrying "**Reviewed commit:** + # ". Only the LATEST Codex result for the current head counts β€” an + # older clean pass superseded by a findings run must not read as green. + latest_codex_at_head="$(jq -r --arg sha "$head_sha" ' + [.[] + | select(.user.login == "chatgpt-codex-connector[bot]") + | select(.body | contains($sha))] + | sort_by(.created_at) | last | .body // empty' "$comments_json")" + + if [[ -n "$latest_codex_at_head" ]]; then + if [[ "$latest_codex_at_head" == *"Didn't find any major issues"* ]]; then + review_state="green" + else + review_state="needs-fix" + fi + fi +fi + +# --- Gate 2: green CodeRabbit pre-merge result ------------------------------ + +summary_marker='' + +premerge_body="$(jq -r --arg marker "$summary_marker" ' + [.[] + | select(.user.login == "coderabbitai[bot]") + | select(.body | contains($marker)) + | select((.body | contains("## Pre-merge checks")) + or (.body | contains("πŸš₯ Pre-merge checks |")))] + | sort_by(.created_at) | last | .body // empty' "$comments_json")" + +if [[ -n "$premerge_body" ]]; then + region="$premerge_body" + if [[ "$premerge_body" == *''* && + "$premerge_body" == *''* ]]; then + region="${premerge_body#*}" + region="${region%%*}" + fi + + compact_line="$(grep -oE 'πŸš₯ Pre-merge checks \|[^<]*' <<<"$region" | head -n 1 || true)" + if [[ -n "$compact_line" ]]; then + # Compact shape: green only with a positive βœ… count and no positive + # ❌ / ❓ / ⚠️ counter anywhere in the summary line. + if grep -qE 'βœ… [1-9][0-9]*' <<<"$compact_line" && + ! grep -qE '(❌|❓|⚠️) *[1-9][0-9]*' <<<"$compact_line"; then + premerge_state="green" + else + premerge_state="failed" + fi + elif [[ "$region" == *"## Pre-merge checks"* ]]; then + # Full shape: green only when at least one explicit pass exists and the + # bounded region carries no error/inconclusive mark at all. + if [[ "$region" == *"βœ…"* && "$region" != *"❌"* && "$region" != *"❓"* ]]; then + premerge_state="green" + else + premerge_state="failed" + fi + else + premerge_state="inconclusive" + fi +fi + +# --- Verdict ----------------------------------------------------------------- + +echo "review=$review_state" +echo "premerge=$premerge_state" + +if [[ "$review_state" == "green" && "$premerge_state" == "green" ]]; then + echo "gates=green" + exit 0 +fi + +echo "gates=not-green (fail-closed: arming skipped)" +exit 1 diff --git a/README.md b/README.md index 7a2f4d47..ebc22025 100644 --- a/README.md +++ b/README.md @@ -203,7 +203,7 @@ jobs:
Click to expand -[.github/workflows/enable-auto-merge.yaml](.github/workflows/enable-auto-merge.yaml) is a workflow that approves and enables auto-merge on pull requests from trusted bots and maintainers. +[.github/workflows/enable-auto-merge.yaml](.github/workflows/enable-auto-merge.yaml) is a workflow that approves and enables auto-merge on pull requests from an exact-match allowlist of trusted single-author bots β€” and only once a fail-closed gate proves, at the PR's **current head**, both a green review (a CodeRabbit `APPROVED` review or a Codex clean pass) and a green CodeRabbit pre-merge result (`.scripts/check-merge-gates.sh`). Missing, stale, mixed, or unparseable review/pre-merge state skips arming (the portfolio's maintenance agent arms such PRs after its own live checks). #### Usage From a0a635730642098599978c40f3bca7cda9fcc3d4 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Sun, 12 Jul 2026 03:08:01 +0200 Subject: [PATCH 02/16] fix(auto-merge): close the gate's async, staleness, and race holes Addresses the six review findings on the pentad gate: - checkout the gate script from the workflow-defining repository so required-workflow/workflow_call consumers do not run it from the caller's workspace (where it does not exist) - re-evaluate on pull_request_review and issue_comment events so review results that land after the pull_request events can still arm the PR, with live open/non-draft eligibility re-proven per event shape - take CodeRabbit's LATEST verdict at the head instead of any APPROVED, so a superseding CHANGES_REQUESTED or dismissal blocks arming and Codex cannot override an explicit head verdict - reject warning marks in the full pre-merge shape like the compact one - select the pre-merge summary by updated_at (CodeRabbit edits it in place) and require it not to predate the head commit - bind approval (commit_id) and arming (--match-head-commit) to the gate-proven head so a racing push invalidates both Five new fixtures pin the behaviors (18 total). --- .../comments.json | 3 + .../reviews.json | 3 + .../comments.json | 3 + .../reviews.json | 4 + .github/tests/merge-gate-fixtures/index.json | 126 ++++++++++------- .../comments.json | 4 + .../reviews.json | 3 + .../premerge-stale-before-head/comments.json | 3 + .../premerge-stale-before-head/reviews.json | 3 + .../premerge-warning-full/comments.json | 3 + .../premerge-warning-full/reviews.json | 3 + .../test-enable-auto-merge-pentad-gate.sh | 5 +- .github/workflows/enable-auto-merge.yaml | 108 +++++++++++++-- .scripts/check-merge-gates.sh | 129 +++++++++++------- README.md | 2 +- 15 files changed, 287 insertions(+), 115 deletions(-) create mode 100644 .github/tests/merge-gate-fixtures/cr-approval-dismissed-at-head/comments.json create mode 100644 .github/tests/merge-gate-fixtures/cr-approval-dismissed-at-head/reviews.json create mode 100644 .github/tests/merge-gate-fixtures/cr-approval-superseded-at-head/comments.json create mode 100644 .github/tests/merge-gate-fixtures/cr-approval-superseded-at-head/reviews.json create mode 100644 .github/tests/merge-gate-fixtures/premerge-edited-newest-updated-wins/comments.json create mode 100644 .github/tests/merge-gate-fixtures/premerge-edited-newest-updated-wins/reviews.json create mode 100644 .github/tests/merge-gate-fixtures/premerge-stale-before-head/comments.json create mode 100644 .github/tests/merge-gate-fixtures/premerge-stale-before-head/reviews.json create mode 100644 .github/tests/merge-gate-fixtures/premerge-warning-full/comments.json create mode 100644 .github/tests/merge-gate-fixtures/premerge-warning-full/reviews.json diff --git a/.github/tests/merge-gate-fixtures/cr-approval-dismissed-at-head/comments.json b/.github/tests/merge-gate-fixtures/cr-approval-dismissed-at-head/comments.json new file mode 100644 index 00000000..f330751d --- /dev/null +++ b/.github/tests/merge-gate-fixtures/cr-approval-dismissed-at-head/comments.json @@ -0,0 +1,3 @@ +[ + {"user":{"login":"coderabbitai[bot]"},"body":"\n\nπŸš₯ Pre-merge checks | βœ… 5\n","created_at":"2026-07-11T10:00:00Z"} +] diff --git a/.github/tests/merge-gate-fixtures/cr-approval-dismissed-at-head/reviews.json b/.github/tests/merge-gate-fixtures/cr-approval-dismissed-at-head/reviews.json new file mode 100644 index 00000000..ef88bee1 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/cr-approval-dismissed-at-head/reviews.json @@ -0,0 +1,3 @@ +[ + {"user":{"login":"coderabbitai[bot]"},"state":"DISMISSED","commit_id":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","submitted_at":"2026-07-11T10:00:00Z"} +] diff --git a/.github/tests/merge-gate-fixtures/cr-approval-superseded-at-head/comments.json b/.github/tests/merge-gate-fixtures/cr-approval-superseded-at-head/comments.json new file mode 100644 index 00000000..f330751d --- /dev/null +++ b/.github/tests/merge-gate-fixtures/cr-approval-superseded-at-head/comments.json @@ -0,0 +1,3 @@ +[ + {"user":{"login":"coderabbitai[bot]"},"body":"\n\nπŸš₯ Pre-merge checks | βœ… 5\n","created_at":"2026-07-11T10:00:00Z"} +] diff --git a/.github/tests/merge-gate-fixtures/cr-approval-superseded-at-head/reviews.json b/.github/tests/merge-gate-fixtures/cr-approval-superseded-at-head/reviews.json new file mode 100644 index 00000000..e4ddf943 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/cr-approval-superseded-at-head/reviews.json @@ -0,0 +1,4 @@ +[ + {"user":{"login":"coderabbitai[bot]"},"state":"APPROVED","commit_id":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","submitted_at":"2026-07-11T10:00:00Z"}, + {"user":{"login":"coderabbitai[bot]"},"state":"CHANGES_REQUESTED","commit_id":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","submitted_at":"2026-07-11T11:00:00Z"} +] diff --git a/.github/tests/merge-gate-fixtures/index.json b/.github/tests/merge-gate-fixtures/index.json index 4bf0cd64..4e66a19c 100644 --- a/.github/tests/merge-gate-fixtures/index.json +++ b/.github/tests/merge-gate-fixtures/index.json @@ -1,54 +1,74 @@ [ - { - "name": "green-cr-at-head-premerge-compact", - "expect_green": true - }, - { - "name": "green-codex-at-head-premerge-full", - "expect_green": true - }, - { - "name": "stale-cr-approval", - "expect_green": false - }, - { - "name": "stale-codex-clean", - "expect_green": false - }, - { - "name": "codex-findings-at-head", - "expect_green": false - }, - { - "name": "codex-clean-superseded-by-findings", - "expect_green": false - }, - { - "name": "no-review-at-all", - "expect_green": false - }, - { - "name": "premerge-not-posted", - "expect_green": false - }, - { - "name": "premerge-mixed-compact", - "expect_green": false - }, - { - "name": "premerge-failed-full", - "expect_green": false - }, - { - "name": "premerge-marker-missing", - "expect_green": false - }, - { - "name": "newest-summary-wins", - "expect_green": false - }, - { - "name": "changes-requested-not-approval", - "expect_green": false - } -] \ No newline at end of file + { + "name": "green-cr-at-head-premerge-compact", + "expect_green": true + }, + { + "name": "green-codex-at-head-premerge-full", + "expect_green": true + }, + { + "name": "stale-cr-approval", + "expect_green": false + }, + { + "name": "stale-codex-clean", + "expect_green": false + }, + { + "name": "codex-findings-at-head", + "expect_green": false + }, + { + "name": "codex-clean-superseded-by-findings", + "expect_green": false + }, + { + "name": "no-review-at-all", + "expect_green": false + }, + { + "name": "premerge-not-posted", + "expect_green": false + }, + { + "name": "premerge-mixed-compact", + "expect_green": false + }, + { + "name": "premerge-failed-full", + "expect_green": false + }, + { + "name": "premerge-marker-missing", + "expect_green": false + }, + { + "name": "newest-summary-wins", + "expect_green": false + }, + { + "name": "changes-requested-not-approval", + "expect_green": false + }, + { + "name": "cr-approval-superseded-at-head", + "expect_green": false + }, + { + "name": "cr-approval-dismissed-at-head", + "expect_green": false + }, + { + "name": "premerge-warning-full", + "expect_green": false + }, + { + "name": "premerge-stale-before-head", + "expect_green": false + }, + { + "name": "premerge-edited-newest-updated-wins", + "expect_green": true + } +] diff --git a/.github/tests/merge-gate-fixtures/premerge-edited-newest-updated-wins/comments.json b/.github/tests/merge-gate-fixtures/premerge-edited-newest-updated-wins/comments.json new file mode 100644 index 00000000..ce0341dd --- /dev/null +++ b/.github/tests/merge-gate-fixtures/premerge-edited-newest-updated-wins/comments.json @@ -0,0 +1,4 @@ +[ + {"user":{"login":"coderabbitai[bot]"},"body":"\n\nπŸš₯ Pre-merge checks | βœ… 5\n","created_at":"2026-07-11T10:00:00Z","updated_at":"2026-07-11T12:00:00Z"}, + {"user":{"login":"coderabbitai[bot]"},"body":"\n\nπŸš₯ Pre-merge checks | βœ… 4 | ❌ 1\n","created_at":"2026-07-11T11:00:00Z"} +] diff --git a/.github/tests/merge-gate-fixtures/premerge-edited-newest-updated-wins/reviews.json b/.github/tests/merge-gate-fixtures/premerge-edited-newest-updated-wins/reviews.json new file mode 100644 index 00000000..5b9eec05 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/premerge-edited-newest-updated-wins/reviews.json @@ -0,0 +1,3 @@ +[ + {"user":{"login":"coderabbitai[bot]"},"state":"APPROVED","commit_id":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","submitted_at":"2026-07-11T10:00:00Z"} +] diff --git a/.github/tests/merge-gate-fixtures/premerge-stale-before-head/comments.json b/.github/tests/merge-gate-fixtures/premerge-stale-before-head/comments.json new file mode 100644 index 00000000..15a85740 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/premerge-stale-before-head/comments.json @@ -0,0 +1,3 @@ +[ + {"user":{"login":"coderabbitai[bot]"},"body":"\n\nπŸš₯ Pre-merge checks | βœ… 5\n","created_at":"2026-07-10T08:00:00Z"} +] diff --git a/.github/tests/merge-gate-fixtures/premerge-stale-before-head/reviews.json b/.github/tests/merge-gate-fixtures/premerge-stale-before-head/reviews.json new file mode 100644 index 00000000..5b9eec05 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/premerge-stale-before-head/reviews.json @@ -0,0 +1,3 @@ +[ + {"user":{"login":"coderabbitai[bot]"},"state":"APPROVED","commit_id":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","submitted_at":"2026-07-11T10:00:00Z"} +] diff --git a/.github/tests/merge-gate-fixtures/premerge-warning-full/comments.json b/.github/tests/merge-gate-fixtures/premerge-warning-full/comments.json new file mode 100644 index 00000000..2e80da4c --- /dev/null +++ b/.github/tests/merge-gate-fixtures/premerge-warning-full/comments.json @@ -0,0 +1,3 @@ +[ + {"user":{"login":"coderabbitai[bot]"},"body":"\n\n## Pre-merge checks\n| Title check | βœ… Passed |\n| Docstring Coverage | ⚠️ Warning |\n","created_at":"2026-07-11T10:00:00Z"} +] diff --git a/.github/tests/merge-gate-fixtures/premerge-warning-full/reviews.json b/.github/tests/merge-gate-fixtures/premerge-warning-full/reviews.json new file mode 100644 index 00000000..5b9eec05 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/premerge-warning-full/reviews.json @@ -0,0 +1,3 @@ +[ + {"user":{"login":"coderabbitai[bot]"},"state":"APPROVED","commit_id":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","submitted_at":"2026-07-11T10:00:00Z"} +] diff --git a/.github/tests/test-enable-auto-merge-pentad-gate.sh b/.github/tests/test-enable-auto-merge-pentad-gate.sh index 70d6bca8..b536154d 100755 --- a/.github/tests/test-enable-auto-merge-pentad-gate.sh +++ b/.github/tests/test-enable-auto-merge-pentad-gate.sh @@ -12,6 +12,9 @@ fixtures_dir="${2:-.github/tests/merge-gate-fixtures}" workflow="${3:-.github/workflows/enable-auto-merge.yaml}" head_sha="$(printf 'a%.0s' {1..40})" +# The freshness floor the workflow derives from the head commit's committer +# date: fixture summaries dated on/after it are fresh, earlier ones stale. +head_committed_at="2026-07-11T09:00:00Z" status=0 # The workflow must actually consume the gate: the gates step runs the script @@ -36,7 +39,7 @@ while IFS= read -r fixture; do expect_green="$(jq -r '.expect_green' <<<"$fixture")" actual_green=false - if bash "$script" "$head_sha" \ + if bash "$script" "$head_sha" "$head_committed_at" \ "$fixtures_dir/$name/reviews.json" \ "$fixtures_dir/$name/comments.json" >/dev/null; then actual_green=true diff --git a/.github/workflows/enable-auto-merge.yaml b/.github/workflows/enable-auto-merge.yaml index b59c1091..c3e2de55 100644 --- a/.github/workflows/enable-auto-merge.yaml +++ b/.github/workflows/enable-auto-merge.yaml @@ -11,6 +11,14 @@ on: types: [opened, synchronize, reopened, ready_for_review] merge_group: ################################## + # Review results land AFTER the pull_request events above have run, so the + # gate re-evaluates when a reviewer bot posts or edits its result: a + # CodeRabbit approval arrives as a review, a Codex clean pass as an issue + # comment, and CodeRabbit edits its pre-merge summary comment in place. + pull_request_review: + types: [submitted] + issue_comment: + types: [created, edited] permissions: {} @@ -22,11 +30,36 @@ jobs: runs-on: ubuntu-latest if: >- ${{ - github.event_name == 'pull_request' && - !github.event.pull_request.draft && - contains( - fromJSON('["dependabot[bot]","renovate[bot]","github-actions[bot]","ksail-bot[bot]","coderabbitai[bot]"]'), - github.event.pull_request.user.login + ( + github.event_name == 'pull_request' && + !github.event.pull_request.draft && + contains( + fromJSON('["dependabot[bot]","renovate[bot]","github-actions[bot]","ksail-bot[bot]","coderabbitai[bot]"]'), + github.event.pull_request.user.login + ) + ) || + ( + github.event_name == 'pull_request_review' && + github.event.review.user.login == 'coderabbitai[bot]' && + github.event.review.state == 'approved' && + !github.event.pull_request.draft && + contains( + fromJSON('["dependabot[bot]","renovate[bot]","github-actions[bot]","ksail-bot[bot]","coderabbitai[bot]"]'), + github.event.pull_request.user.login + ) + ) || + ( + github.event_name == 'issue_comment' && + github.event.issue.pull_request && + github.event.issue.state == 'open' && + contains( + fromJSON('["coderabbitai[bot]","chatgpt-codex-connector[bot]"]'), + github.event.comment.user.login + ) && + contains( + fromJSON('["dependabot[bot]","renovate[bot]","github-actions[bot]","ksail-bot[bot]","coderabbitai[bot]"]'), + github.event.issue.user.login + ) ) }} @@ -46,11 +79,44 @@ jobs: permission-contents: write permission-pull-requests: write - - name: πŸ“‘ Checkout + # The gate script must come from the repository that DEFINES this + # workflow: in required-workflow / workflow_call consumers the default + # checkout resolves to the CALLER's workspace, which does not carry + # .scripts/check-merge-gates.sh. github.workflow_ref names the defining + # repository and ref, so the checkout follows it in every context. + - name: 🧭 Resolve workflow source + id: workflow-source + env: + WORKFLOW_REF: ${{ github.workflow_ref }} + run: | + echo "repository=${WORKFLOW_REF%%/.github/*}" >> "$GITHUB_OUTPUT" + echo "ref=${WORKFLOW_REF##*@}" >> "$GITHUB_OUTPUT" + + - name: πŸ“‘ Checkout gate script from the workflow repository uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 with: + repository: ${{ steps.workflow-source.outputs.repository }} + ref: ${{ steps.workflow-source.outputs.ref }} persist-credentials: false + # The issue_comment shape cannot prove the PR is open and non-draft from + # its payload alone, so eligibility is re-proven against live state for + # every event shape (fail-closed: an ineligible PR is never gated on). + - name: πŸ”Ž Resolve target pull request + id: pr + env: + GH_TOKEN: ${{ steps.app-token.outputs.token }} + PR_NUMBER: ${{ github.event.pull_request.number || github.event.issue.number }} + REPOSITORY: ${{ github.repository }} + run: | + echo "number=$PR_NUMBER" >> "$GITHUB_OUTPUT" + eligible=$(gh pr view "$PR_NUMBER" --repo "$REPOSITORY" --json isDraft,state \ + --jq 'if (.isDraft | not) and .state == "OPEN" then "true" else "false" end') + echo "eligible=$eligible" >> "$GITHUB_OUTPUT" + if [[ "$eligible" != "true" ]]; then + echo "::notice::PR #${PR_NUMBER} is draft or not open; skipping the auto-merge gate." + fi + # Fail-closed pentad gate (actions#548): approving and arming are only # allowed once a CURRENT-HEAD green review (CodeRabbit APPROVED or Codex # clean pass) AND a green CodeRabbit pre-merge result are both proven. @@ -59,16 +125,22 @@ jobs: # pentad check. Never weaken this gate to warn-only. - name: πŸ›‚ Verify review and pre-merge gates id: gates + if: steps.pr.outputs.eligible == 'true' env: GH_TOKEN: ${{ steps.app-token.outputs.token }} - PR_NUMBER: ${{ github.event.pull_request.number }} + PR_NUMBER: ${{ steps.pr.outputs.number }} REPOSITORY: ${{ github.repository }} run: | HEAD_SHA=$(gh pr view "$PR_NUMBER" --repo "$REPOSITORY" --json headRefOid --jq .headRefOid) + HEAD_COMMITTED_AT=$(gh api "repos/$REPOSITORY/commits/$HEAD_SHA" --jq .commit.committer.date) gh api "repos/$REPOSITORY/pulls/$PR_NUMBER/reviews" --paginate | jq -s 'add // []' > /tmp/reviews.json gh api "repos/$REPOSITORY/issues/$PR_NUMBER/comments" --paginate | jq -s 'add // []' > /tmp/comments.json - if bash .scripts/check-merge-gates.sh "$HEAD_SHA" /tmp/reviews.json /tmp/comments.json; then + # The proven head is exported so the privileged steps below bind to + # it: a push racing this gate must invalidate the arming, not ride it. + echo "head_sha=$HEAD_SHA" >> "$GITHUB_OUTPUT" + + if bash .scripts/check-merge-gates.sh "$HEAD_SHA" "$HEAD_COMMITTED_AT" /tmp/reviews.json /tmp/comments.json; then echo "armable=true" >> "$GITHUB_OUTPUT" else echo "armable=false" >> "$GITHUB_OUTPUT" @@ -79,16 +151,20 @@ jobs: if: steps.gates.outputs.armable == 'true' env: GH_TOKEN: ${{ steps.app-token.outputs.token }} - PR_NUMBER: ${{ github.event.pull_request.number }} + PR_NUMBER: ${{ steps.pr.outputs.number }} REPOSITORY: ${{ github.repository }} + HEAD_SHA: ${{ steps.gates.outputs.head_sha }} run: | set +e - REVIEW_OUTPUT=$(gh pr review "$PR_NUMBER" --approve --repo "$REPOSITORY" 2>&1) + # commit_id pins the approval to the gate-proven head: a commit + # pushed after the gate ran gets no approval from this run. + REVIEW_OUTPUT=$(gh api "repos/$REPOSITORY/pulls/$PR_NUMBER/reviews" \ + -f event=APPROVE -f commit_id="$HEAD_SHA" 2>&1) REVIEW_EXIT_CODE=$? set -e if [[ $REVIEW_EXIT_CODE -eq 0 ]]; then - echo "βœ… PR #${PR_NUMBER} approved" + echo "βœ… PR #${PR_NUMBER} approved at ${HEAD_SHA}" elif [[ "$REVIEW_OUTPUT" == *"Can not approve your own pull request"* ]]; then echo "::warning::Could not approve PR #${PR_NUMBER} because GitHub does not allow self-approval. Skipping approval." else @@ -101,13 +177,17 @@ jobs: if: steps.gates.outputs.armable == 'true' env: GH_TOKEN: ${{ steps.app-token.outputs.token }} - PR_NUMBER: ${{ github.event.pull_request.number }} + PR_NUMBER: ${{ steps.pr.outputs.number }} REPOSITORY: ${{ github.repository }} + HEAD_SHA: ${{ steps.gates.outputs.head_sha }} run: | REPO_INFO=$(gh api "repos/$REPOSITORY" --jq '.allow_auto_merge') if [[ "$REPO_INFO" != "true" ]]; then echo "::warning::Auto-merge is not enabled on this repository. Contact a repository admin to enable it in Settings > Pull Requests > Allow auto-merge." exit 0 fi - gh pr merge "$PR_NUMBER" --auto --squash --repo "$REPOSITORY" - echo "βœ… Auto-merge enabled for PR #${PR_NUMBER} using squash method" + # --match-head-commit binds the arming to the gate-proven head: a + # push racing this workflow makes the merge call fail instead of + # arming an unreviewed commit. + gh pr merge "$PR_NUMBER" --auto --squash --repo "$REPOSITORY" --match-head-commit "$HEAD_SHA" + echo "βœ… Auto-merge enabled for PR #${PR_NUMBER} at ${HEAD_SHA} using squash method" diff --git a/.scripts/check-merge-gates.sh b/.scripts/check-merge-gates.sh index 51532173..b3f0e569 100755 --- a/.scripts/check-merge-gates.sh +++ b/.scripts/check-merge-gates.sh @@ -1,20 +1,29 @@ #!/usr/bin/env bash # Fail-closed review/pre-merge gate for the privileged auto-merge workflow. # -# Usage: check-merge-gates.sh -# head-sha the pull request's current head commit SHA -# reviews-json file holding the FULL paginated `pulls//reviews` array -# comments-json file holding the FULL paginated `issues//comments` array +# Usage: check-merge-gates.sh +# head-sha the pull request's current head commit SHA +# head-committed-at the head commit's ISO8601 committer date (used as the +# freshness floor for the pre-merge summary, which CodeRabbit +# edits in place and which carries no commit SHA of its own) +# reviews-json file holding the FULL paginated `pulls//reviews` array +# comments-json file holding the FULL paginated `issues//comments` array # # Exits 0 only when BOTH gates are proven at the current head: -# 1. a green review β€” a CodeRabbit APPROVED review whose commit_id equals -# the head, or a Codex clean pass ("Didn't find any major issues") whose +# 1. a green review β€” CodeRabbit's LATEST review verdict at the head is +# APPROVED (an earlier approval superseded by CHANGES_REQUESTED or a +# dismissal is NOT green), or β€” when CodeRabbit has no verdict at the +# head β€” a Codex clean pass ("Didn't find any major issues") whose # "Reviewed commit" equals the head and is Codex's LATEST result for it; -# 2. a green CodeRabbit pre-merge result β€” the newest auto-generated summary -# (stable marker required) whose pre-merge section is unambiguously -# green: a positive check-mark count and no error/inconclusive/warning -# marks. When walkthrough boundary markers exist, only the bounded -# region is parsed so echoed marker text elsewhere cannot spoof it. +# 2. a green CodeRabbit pre-merge result β€” the most recently UPDATED +# auto-generated summary (stable marker required; CodeRabbit edits the +# summary in place, so created_at alone selects a stale revision) whose +# pre-merge section is unambiguously green: a positive check-mark count +# and no error/inconclusive/warning marks in either shape, and whose +# update time is not older than the head commit (a summary last touched +# before the head was committed can only describe an earlier state). +# When walkthrough boundary markers exist, only the bounded region is +# parsed so echoed marker text elsewhere cannot spoof it. # # Everything else β€” missing, stale, mixed, unparseable, or absent state β€” is # NOT green and exits 1 (the workflow then skips arming; the maintenance @@ -22,35 +31,46 @@ set -euo pipefail -if [[ $# -ne 3 ]]; then - echo "usage: $0 " >&2 +if [[ $# -ne 4 ]]; then + echo "usage: $0 " >&2 exit 2 fi head_sha="$1" -reviews_json="$2" -comments_json="$3" +head_committed_at="$2" +reviews_json="$3" +comments_json="$4" review_state="missing" premerge_state="not-posted" # --- Gate 1: green review at the current head ------------------------------- -cr_approved_at_head="$(jq -r --arg sha "$head_sha" ' - [.[] | select(.user.login == "coderabbitai[bot]" and .state == "APPROVED")] - | map(select(.commit_id == $sha)) | length' "$reviews_json")" +# CodeRabbit's verdict at the head is its LATEST verdict-bearing review there: +# an APPROVED superseded by CHANGES_REQUESTED (or dismissed) must not count. +cr_latest_verdict_at_head="$(jq -r --arg sha "$head_sha" ' + [.[] + | select(.user.login == "coderabbitai[bot]") + | select(.commit_id == $sha) + | select(.state == "APPROVED" or .state == "CHANGES_REQUESTED" + or .state == "DISMISSED")] + | sort_by(.submitted_at) | last | .state // empty' "$reviews_json")" cr_approved_anywhere="$(jq -r ' [.[] | select(.user.login == "coderabbitai[bot]" and .state == "APPROVED")] | length' "$reviews_json")" -if [[ "$cr_approved_at_head" -gt 0 ]]; then +if [[ "$cr_latest_verdict_at_head" == "APPROVED" ]]; then review_state="green" +elif [[ -n "$cr_latest_verdict_at_head" ]]; then + # An explicit non-approval verdict at the head blocks arming outright β€” a + # Codex clean pass must not override CodeRabbit's CHANGES_REQUESTED. + review_state="needs-fix" elif [[ "$cr_approved_anywhere" -gt 0 ]]; then review_state="stale" fi -if [[ "$review_state" != "green" ]]; then +if [[ "$review_state" == "missing" || "$review_state" == "stale" ]]; then # Codex lane: its result is an ISSUE COMMENT carrying "**Reviewed commit:** # ". Only the LATEST Codex result for the current head counts β€” an # older clean pass superseded by a findings run must not read as green. @@ -73,42 +93,59 @@ fi summary_marker='' -premerge_body="$(jq -r --arg marker "$summary_marker" ' +# CodeRabbit EDITS its auto-generated summary in place across review cycles, +# so the newest revision is the one with the greatest updated_at (falling back +# to created_at), never the newest created_at alone. One call returns the +# selected summary's touch time on the first line and its body after it. +premerge_selected="$(jq -r --arg marker "$summary_marker" ' [.[] | select(.user.login == "coderabbitai[bot]") | select(.body | contains($marker)) | select((.body | contains("## Pre-merge checks")) or (.body | contains("πŸš₯ Pre-merge checks |")))] - | sort_by(.created_at) | last | .body // empty' "$comments_json")" + | sort_by(.updated_at // .created_at) | last + | if . == null then empty + else ((.updated_at // .created_at) // "") + "\n" + .body end' "$comments_json")" -if [[ -n "$premerge_body" ]]; then - region="$premerge_body" - if [[ "$premerge_body" == *''* && - "$premerge_body" == *''* ]]; then - region="${premerge_body#*}" - region="${region%%*}" - fi +premerge_touched_at="${premerge_selected%%$'\n'*}" +premerge_body="${premerge_selected#*$'\n'}" - compact_line="$(grep -oE 'πŸš₯ Pre-merge checks \|[^<]*' <<<"$region" | head -n 1 || true)" - if [[ -n "$compact_line" ]]; then - # Compact shape: green only with a positive βœ… count and no positive - # ❌ / ❓ / ⚠️ counter anywhere in the summary line. - if grep -qE 'βœ… [1-9][0-9]*' <<<"$compact_line" && - ! grep -qE '(❌|❓|⚠️) *[1-9][0-9]*' <<<"$compact_line"; then - premerge_state="green" - else - premerge_state="failed" +if [[ -n "$premerge_body" ]]; then + # The summary carries no commit SHA, so freshness is the proxy tie to the + # head: a summary last updated before the head commit existed can only + # describe an earlier state. ISO8601 Zulu timestamps compare lexically. + if [[ -n "$head_committed_at" && "$premerge_touched_at" < "$head_committed_at" ]]; then + premerge_state="stale" + else + region="$premerge_body" + if [[ "$premerge_body" == *''* && + "$premerge_body" == *''* ]]; then + region="${premerge_body#*}" + region="${region%%*}" fi - elif [[ "$region" == *"## Pre-merge checks"* ]]; then - # Full shape: green only when at least one explicit pass exists and the - # bounded region carries no error/inconclusive mark at all. - if [[ "$region" == *"βœ…"* && "$region" != *"❌"* && "$region" != *"❓"* ]]; then - premerge_state="green" + + compact_line="$(grep -oE 'πŸš₯ Pre-merge checks \|[^<]*' <<<"$region" | head -n 1 || true)" + if [[ -n "$compact_line" ]]; then + # Compact shape: green only with a positive βœ… count and no positive + # ❌ / ❓ / ⚠️ counter anywhere in the summary line. + if grep -qE 'βœ… [1-9][0-9]*' <<<"$compact_line" && + ! grep -qE '(❌|❓|⚠️) *[1-9][0-9]*' <<<"$compact_line"; then + premerge_state="green" + else + premerge_state="failed" + fi + elif [[ "$region" == *"## Pre-merge checks"* ]]; then + # Full shape: green only when at least one explicit pass exists and the + # bounded region carries no error/inconclusive/warning mark at all. + if [[ "$region" == *"βœ…"* && "$region" != *"❌"* && + "$region" != *"❓"* && "$region" != *"⚠️"* ]]; then + premerge_state="green" + else + premerge_state="failed" + fi else - premerge_state="failed" + premerge_state="inconclusive" fi - else - premerge_state="inconclusive" fi fi diff --git a/README.md b/README.md index ebc22025..0807baf8 100644 --- a/README.md +++ b/README.md @@ -203,7 +203,7 @@ jobs:
Click to expand -[.github/workflows/enable-auto-merge.yaml](.github/workflows/enable-auto-merge.yaml) is a workflow that approves and enables auto-merge on pull requests from an exact-match allowlist of trusted single-author bots β€” and only once a fail-closed gate proves, at the PR's **current head**, both a green review (a CodeRabbit `APPROVED` review or a Codex clean pass) and a green CodeRabbit pre-merge result (`.scripts/check-merge-gates.sh`). Missing, stale, mixed, or unparseable review/pre-merge state skips arming (the portfolio's maintenance agent arms such PRs after its own live checks). +[.github/workflows/enable-auto-merge.yaml](.github/workflows/enable-auto-merge.yaml) is a workflow that approves and enables auto-merge on pull requests from an exact-match allowlist of trusted single-author bots β€” and only once a fail-closed gate proves, at the PR's **current head**, both a green review (CodeRabbit's **latest** head verdict `APPROVED`, or a Codex clean pass when CodeRabbit has no head verdict) and a green, **fresh** CodeRabbit pre-merge result (`.scripts/check-merge-gates.sh`; the summary's last update must not predate the head commit, and a `⚠️`/`❌`/`❓` mark in either summary shape fails it). Missing, stale, mixed, or unparseable review/pre-merge state skips arming (the portfolio's maintenance agent arms such PRs after its own live checks). Because review results land after the `pull_request` events, the gate also re-evaluates on `pull_request_review`/`issue_comment` activity from the reviewer bots, checks out the gate script from the **workflow-defining** repository (so required-workflow consumers work without vendoring it), and binds both the approval (`commit_id`) and the arming (`--match-head-commit`) to the gate-proven head so a racing push invalidates them. #### Usage From e469f87bb4adef6b2499353b26726bf14f990ecb Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Sun, 12 Jul 2026 03:54:25 +0200 Subject: [PATCH 03/16] feat(auto-merge): flag-gate enforcement, same-commit checkout, and disarm MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the second review round: - default-off enforcement (enforce-review-gates input / ENFORCE_MERGE_GATES variable) per the feature-flag-first rule, so consumers keep pre-gate bot auto-merge until the flag is flipped after validation - gate script checked out via job.workflow_repository/job.workflow_sha (the repo's same-commit self-checkout idiom) β€” correct in workflow_call, where github.workflow_ref names the CALLER, and never a PR-controlled ref - any CodeRabbit verdict re-evaluates (not only approvals) and a gate that turned red actively disarms a previously armed PR - caller-side trigger requirement documented (a reusable workflow cannot schedule its callers) --- .github/workflows/enable-auto-merge.yaml | 88 +++++++++++++++++------- README.md | 21 ++++-- 2 files changed, 81 insertions(+), 28 deletions(-) diff --git a/.github/workflows/enable-auto-merge.yaml b/.github/workflows/enable-auto-merge.yaml index c3e2de55..ca1d710c 100644 --- a/.github/workflows/enable-auto-merge.yaml +++ b/.github/workflows/enable-auto-merge.yaml @@ -6,15 +6,29 @@ on: APP_PRIVATE_KEY: required: true description: "The private key for the GitHub App" + inputs: + enforce-review-gates: + type: boolean + required: false + default: false + description: >- + Opt-in (default-off) enforcement of the fail-closed review/pre-merge + gate before approving and arming. Callers that enable it should also + trigger their caller workflow on pull_request_review and + issue_comment so review results that land after the pull_request + events still re-evaluate the gate (a reusable workflow cannot + schedule its callers). ### Required Workflow Triggers ### pull_request: types: [opened, synchronize, reopened, ready_for_review] merge_group: ################################## # Review results land AFTER the pull_request events above have run, so the - # gate re-evaluates when a reviewer bot posts or edits its result: a - # CodeRabbit approval arrives as a review, a Codex clean pass as an issue - # comment, and CodeRabbit edits its pre-merge summary comment in place. + # gate re-evaluates when a reviewer bot posts or edits its result: any + # CodeRabbit review verdict (approval OR changes-requested β€” the latter must + # be able to DISARM), a Codex result comment, and CodeRabbit's in-place + # pre-merge summary edits. These triggers only fire where this workflow file + # lives; workflow_call consumers add them to their caller (see the input). pull_request_review: types: [submitted] issue_comment: @@ -41,7 +55,6 @@ jobs: ( github.event_name == 'pull_request_review' && github.event.review.user.login == 'coderabbitai[bot]' && - github.event.review.state == 'approved' && !github.event.pull_request.draft && contains( fromJSON('["dependabot[bot]","renovate[bot]","github-actions[bot]","ksail-bot[bot]","coderabbitai[bot]"]'), @@ -79,24 +92,17 @@ jobs: permission-contents: write permission-pull-requests: write - # The gate script must come from the repository that DEFINES this - # workflow: in required-workflow / workflow_call consumers the default - # checkout resolves to the CALLER's workspace, which does not carry - # .scripts/check-merge-gates.sh. github.workflow_ref names the defining - # repository and ref, so the checkout follows it in every context. - - name: 🧭 Resolve workflow source - id: workflow-source - env: - WORKFLOW_REF: ${{ github.workflow_ref }} - run: | - echo "repository=${WORKFLOW_REF%%/.github/*}" >> "$GITHUB_OUTPUT" - echo "ref=${WORKFLOW_REF##*@}" >> "$GITHUB_OUTPUT" - - - name: πŸ“‘ Checkout gate script from the workflow repository + # Self-reference: check out THIS workflow's own repo at the exact commit + # it is running from (job.workflow_repository / job.workflow_sha) β€” never + # the caller's workspace (which does not carry the gate script) and never + # a PR-controlled ref. Same idiom as the repo's other same-commit + # self-checkouts (dependency-review.yaml). + - name: πŸ“₯ Checkout devantler-tech/actions (this workflow's commit) uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 with: - repository: ${{ steps.workflow-source.outputs.repository }} - ref: ${{ steps.workflow-source.outputs.ref }} + repository: ${{ job.workflow_repository }} + ref: ${{ job.workflow_sha }} + path: .devantler-tech-actions persist-credentials: false # The issue_comment shape cannot prove the PR is open and non-draft from @@ -123,6 +129,15 @@ jobs: # Missing, stale, mixed, or unparseable state is NOT green β€” the step # then skips arming and the maintenance agent arms after its own live # pentad check. Never weaken this gate to warn-only. + # + # Feature-flag-first rollout: enforcement is DEFAULT-OFF (the + # enforce-review-gates input for workflow_call callers, or the + # ENFORCE_MERGE_GATES repository/organization variable for the direct + # and required-workflow paths). With enforcement off, the pre-gate + # behavior is preserved (allowlisted trusted bots are armed) so + # consumers without CodeRabbit pre-merge summaries or agent-requested + # reviews do not silently lose bot auto-merge before the flag is + # flipped per repo/org after validation. - name: πŸ›‚ Verify review and pre-merge gates id: gates if: steps.pr.outputs.eligible == 'true' @@ -130,23 +145,48 @@ jobs: GH_TOKEN: ${{ steps.app-token.outputs.token }} PR_NUMBER: ${{ steps.pr.outputs.number }} REPOSITORY: ${{ github.repository }} + ENFORCE: ${{ (inputs.enforce-review-gates || vars.ENFORCE_MERGE_GATES == 'true') && 'true' || 'false' }} run: | HEAD_SHA=$(gh pr view "$PR_NUMBER" --repo "$REPOSITORY" --json headRefOid --jq .headRefOid) - HEAD_COMMITTED_AT=$(gh api "repos/$REPOSITORY/commits/$HEAD_SHA" --jq .commit.committer.date) - gh api "repos/$REPOSITORY/pulls/$PR_NUMBER/reviews" --paginate | jq -s 'add // []' > /tmp/reviews.json - gh api "repos/$REPOSITORY/issues/$PR_NUMBER/comments" --paginate | jq -s 'add // []' > /tmp/comments.json # The proven head is exported so the privileged steps below bind to # it: a push racing this gate must invalidate the arming, not ride it. echo "head_sha=$HEAD_SHA" >> "$GITHUB_OUTPUT" + echo "enforced=$ENFORCE" >> "$GITHUB_OUTPUT" - if bash .scripts/check-merge-gates.sh "$HEAD_SHA" "$HEAD_COMMITTED_AT" /tmp/reviews.json /tmp/comments.json; then + if [[ "$ENFORCE" != "true" ]]; then + echo "armable=true" >> "$GITHUB_OUTPUT" + echo "::notice::Review/pre-merge gate enforcement is off (default) β€” arming PR #${PR_NUMBER} on the trusted-author allowlist alone. Enable via the enforce-review-gates input or the ENFORCE_MERGE_GATES variable." + exit 0 + fi + + HEAD_COMMITTED_AT=$(gh api "repos/$REPOSITORY/commits/$HEAD_SHA" --jq .commit.committer.date) + gh api "repos/$REPOSITORY/pulls/$PR_NUMBER/reviews" --paginate | jq -s 'add // []' > /tmp/reviews.json + gh api "repos/$REPOSITORY/issues/$PR_NUMBER/comments" --paginate | jq -s 'add // []' > /tmp/comments.json + + if bash .devantler-tech-actions/.scripts/check-merge-gates.sh "$HEAD_SHA" "$HEAD_COMMITTED_AT" /tmp/reviews.json /tmp/comments.json; then echo "armable=true" >> "$GITHUB_OUTPUT" else echo "armable=false" >> "$GITHUB_OUTPUT" echo "::notice::PR #${PR_NUMBER} not armed: current-head review/pre-merge gates are not green (fail-closed). The maintenance agent arms it after its live pentad check." fi + # A gate that turned red AFTER an earlier run armed the PR (e.g. a + # CodeRabbit changes-requested landing behind a Codex clean pass) must + # actively revoke the arming, not just decline to re-arm. + - name: πŸ”’ Disarm auto-merge on failed gates + if: steps.gates.outputs.enforced == 'true' && steps.gates.outputs.armable == 'false' + env: + GH_TOKEN: ${{ steps.app-token.outputs.token }} + PR_NUMBER: ${{ steps.pr.outputs.number }} + REPOSITORY: ${{ github.repository }} + run: | + armed=$(gh pr view "$PR_NUMBER" --repo "$REPOSITORY" --json autoMergeRequest --jq '.autoMergeRequest != null') + if [[ "$armed" == "true" ]]; then + gh pr merge "$PR_NUMBER" --disable-auto --repo "$REPOSITORY" + echo "::warning::Auto-merge DISARMED for PR #${PR_NUMBER}: review/pre-merge gates are no longer green at the current head." + fi + - name: βœ… Approve PR if: steps.gates.outputs.armable == 'true' env: diff --git a/README.md b/README.md index 0807baf8..4dabbe06 100644 --- a/README.md +++ b/README.md @@ -203,23 +203,36 @@ jobs:
Click to expand -[.github/workflows/enable-auto-merge.yaml](.github/workflows/enable-auto-merge.yaml) is a workflow that approves and enables auto-merge on pull requests from an exact-match allowlist of trusted single-author bots β€” and only once a fail-closed gate proves, at the PR's **current head**, both a green review (CodeRabbit's **latest** head verdict `APPROVED`, or a Codex clean pass when CodeRabbit has no head verdict) and a green, **fresh** CodeRabbit pre-merge result (`.scripts/check-merge-gates.sh`; the summary's last update must not predate the head commit, and a `⚠️`/`❌`/`❓` mark in either summary shape fails it). Missing, stale, mixed, or unparseable review/pre-merge state skips arming (the portfolio's maintenance agent arms such PRs after its own live checks). Because review results land after the `pull_request` events, the gate also re-evaluates on `pull_request_review`/`issue_comment` activity from the reviewer bots, checks out the gate script from the **workflow-defining** repository (so required-workflow consumers work without vendoring it), and binds both the approval (`commit_id`) and the arming (`--match-head-commit`) to the gate-proven head so a racing push invalidates them. +[.github/workflows/enable-auto-merge.yaml](.github/workflows/enable-auto-merge.yaml) is a workflow that approves and enables auto-merge on pull requests from an exact-match allowlist of trusted single-author bots. It carries an **opt-in, default-off** fail-closed review/pre-merge gate (`.scripts/check-merge-gates.sh`, enabled via the `enforce-review-gates` input or the `ENFORCE_MERGE_GATES` repository/organization variable): when enforced, approval and arming require, at the PR's **current head**, a green review (CodeRabbit's **latest** head verdict `APPROVED`, or a Codex clean pass when CodeRabbit has no head verdict) and a green, **fresh** CodeRabbit pre-merge result (the summary's last update must not predate the head commit; a `⚠️`/`❌`/`❓` mark in either summary shape fails it) β€” and a gate that turns red later actively **disarms** a previously armed PR. Missing, stale, mixed, or unparseable state skips arming (the portfolio's maintenance agent arms such PRs after its own live checks). The gate script is checked out from the **workflow-defining** repository at the running commit (`job.workflow_repository`/`job.workflow_sha`), and both the approval (`commit_id`) and the arming (`--match-head-commit`) bind to the gate-proven head so a racing push invalidates them. Because review results land after the `pull_request` events, the workflow also triggers on `pull_request_review`/`issue_comment` where it lives; `workflow_call` consumers that enable enforcement must add those triggers to their **caller** workflow (a reusable workflow cannot schedule its callers). #### Usage ```yaml +on: + pull_request: + types: [opened, synchronize, reopened, ready_for_review] + # Required when enforce-review-gates is true: review results land after + # the pull_request events, so the caller must re-invoke the gate on them. + pull_request_review: + types: [submitted] + issue_comment: + types: [created, edited] + jobs: auto-merge: uses: devantler-tech/actions/.github/workflows/enable-auto-merge.yaml@{ref} # ref + with: + enforce-review-gates: false # default; flip after the repo's review lanes are validated secrets: APP_PRIVATE_KEY: ${{ secrets.APP_PRIVATE_KEY }} ``` #### Secrets and Inputs -| Key | Type | Default | Required | Description | -|-------------------|--------|---------|----------|------------------------| -| `APP_PRIVATE_KEY` | Secret | - | Yes | GitHub App private key | +| Key | Type | Default | Required | Description | +|------------------------|--------|---------|----------|--------------------------------------------------------------------| +| `APP_PRIVATE_KEY` | Secret | - | Yes | GitHub App private key | +| `enforce-review-gates` | Input | `false` | No | Opt-in fail-closed review/pre-merge gate before approving/arming |
From fabd7c9ae941c8f13717c5ca73a0494fa5a1efd7 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Sun, 12 Jul 2026 04:40:31 +0200 Subject: [PATCH 04/16] fix(auto-merge): disarm on dismissed reviews and push-time freshness floor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A dismissed CodeRabbit approval emits pull_request_review 'dismissed', which the workflow did not subscribe to, so a previously-armed PR kept auto-merge enabled until an unrelated event fired. And the pre-merge freshness floor came from the head commit's committer date, which a push of a previously-created commit object predates β€” the floor now uses the earliest check-suite created_at for the head SHA (when GitHub first saw it), falling back to committer date only when no suite exists. --- .github/workflows/enable-auto-merge.yaml | 24 ++++++++++++++++++------ .scripts/check-merge-gates.sh | 24 ++++++++++++++---------- 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/.github/workflows/enable-auto-merge.yaml b/.github/workflows/enable-auto-merge.yaml index ca1d710c..4b828f72 100644 --- a/.github/workflows/enable-auto-merge.yaml +++ b/.github/workflows/enable-auto-merge.yaml @@ -26,11 +26,12 @@ on: # Review results land AFTER the pull_request events above have run, so the # gate re-evaluates when a reviewer bot posts or edits its result: any # CodeRabbit review verdict (approval OR changes-requested β€” the latter must - # be able to DISARM), a Codex result comment, and CodeRabbit's in-place - # pre-merge summary edits. These triggers only fire where this workflow file - # lives; workflow_call consumers add them to their caller (see the input). + # be able to DISARM), a DISMISSED approval (dismissal is non-green and must + # also disarm), a Codex result comment, and CodeRabbit's in-place pre-merge + # summary edits. These triggers only fire where this workflow file lives; + # workflow_call consumers add them to their caller (see the input). pull_request_review: - types: [submitted] + types: [submitted, dismissed] issue_comment: types: [created, edited] @@ -160,11 +161,22 @@ jobs: exit 0 fi - HEAD_COMMITTED_AT=$(gh api "repos/$REPOSITORY/commits/$HEAD_SHA" --jq .commit.committer.date) + # Freshness floor for the pre-merge summary: when GitHub first SAW + # this head (earliest check-suite created_at for the SHA), not the + # commit's own committer date β€” pushing a previously-created commit + # object carries an old committer date that would let a stale + # summary pass as fresh. Committer date remains only as a fallback + # floor when no check suite exists yet (never an empty, fail-open + # floor). + HEAD_SEEN_AT=$(gh api "repos/$REPOSITORY/commits/$HEAD_SHA/check-suites" --paginate | + jq -rs '[.[].check_suites[]?.created_at | select(. != null)] | min // empty') + if [[ -z "$HEAD_SEEN_AT" ]]; then + HEAD_SEEN_AT=$(gh api "repos/$REPOSITORY/commits/$HEAD_SHA" --jq .commit.committer.date) + fi gh api "repos/$REPOSITORY/pulls/$PR_NUMBER/reviews" --paginate | jq -s 'add // []' > /tmp/reviews.json gh api "repos/$REPOSITORY/issues/$PR_NUMBER/comments" --paginate | jq -s 'add // []' > /tmp/comments.json - if bash .devantler-tech-actions/.scripts/check-merge-gates.sh "$HEAD_SHA" "$HEAD_COMMITTED_AT" /tmp/reviews.json /tmp/comments.json; then + if bash .devantler-tech-actions/.scripts/check-merge-gates.sh "$HEAD_SHA" "$HEAD_SEEN_AT" /tmp/reviews.json /tmp/comments.json; then echo "armable=true" >> "$GITHUB_OUTPUT" else echo "armable=false" >> "$GITHUB_OUTPUT" diff --git a/.scripts/check-merge-gates.sh b/.scripts/check-merge-gates.sh index b3f0e569..9eb082d3 100755 --- a/.scripts/check-merge-gates.sh +++ b/.scripts/check-merge-gates.sh @@ -1,11 +1,15 @@ #!/usr/bin/env bash # Fail-closed review/pre-merge gate for the privileged auto-merge workflow. # -# Usage: check-merge-gates.sh +# Usage: check-merge-gates.sh # head-sha the pull request's current head commit SHA -# head-committed-at the head commit's ISO8601 committer date (used as the -# freshness floor for the pre-merge summary, which CodeRabbit -# edits in place and which carries no commit SHA of its own) +# head-seen-at ISO8601 time GitHub first saw the head (the caller passes +# the earliest check-suite created_at for the SHA, falling +# back to the committer date) β€” the freshness floor for the +# pre-merge summary, which CodeRabbit edits in place and +# which carries no commit SHA of its own. Commit metadata +# alone is NOT a safe floor: pushing a previously-created +# commit object carries an old committer date. # reviews-json file holding the FULL paginated `pulls//reviews` array # comments-json file holding the FULL paginated `issues//comments` array # @@ -20,8 +24,8 @@ # summary in place, so created_at alone selects a stale revision) whose # pre-merge section is unambiguously green: a positive check-mark count # and no error/inconclusive/warning marks in either shape, and whose -# update time is not older than the head commit (a summary last touched -# before the head was committed can only describe an earlier state). +# update time is not older than the head-seen floor (a summary last +# touched before GitHub saw the head can only describe an earlier state). # When walkthrough boundary markers exist, only the bounded region is # parsed so echoed marker text elsewhere cannot spoof it. # @@ -32,12 +36,12 @@ set -euo pipefail if [[ $# -ne 4 ]]; then - echo "usage: $0 " >&2 + echo "usage: $0 " >&2 exit 2 fi head_sha="$1" -head_committed_at="$2" +head_seen_at="$2" reviews_json="$3" comments_json="$4" @@ -112,9 +116,9 @@ premerge_body="${premerge_selected#*$'\n'}" if [[ -n "$premerge_body" ]]; then # The summary carries no commit SHA, so freshness is the proxy tie to the - # head: a summary last updated before the head commit existed can only + # head: a summary last updated before GitHub first saw the head can only # describe an earlier state. ISO8601 Zulu timestamps compare lexically. - if [[ -n "$head_committed_at" && "$premerge_touched_at" < "$head_committed_at" ]]; then + if [[ -n "$head_seen_at" && "$premerge_touched_at" < "$head_seen_at" ]]; then premerge_state="stale" else region="$premerge_body" From c0fa04674b75fe796134dc1ef84ed09a8e8b908b Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Sun, 12 Jul 2026 08:28:47 +0200 Subject: [PATCH 05/16] fix: harden gate paths from codex review - mint checks:read so the freshness-floor check-suites read cannot 403 - exclude this run's own check suite from the freshness floor (only-suite wedge on retriggered heads) - enforcement-off runs arm only on pull_request events, preserving exact pre-gate behavior for the new review/comment retrigger paths - caller snippet subscribes review dismissals so disarm can fire Co-Authored-By: Claude Fable 5 --- .github/workflows/enable-auto-merge.yaml | 26 ++++++++++++++++++++---- README.md | 5 +++-- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/.github/workflows/enable-auto-merge.yaml b/.github/workflows/enable-auto-merge.yaml index 4b828f72..f3ca5f0b 100644 --- a/.github/workflows/enable-auto-merge.yaml +++ b/.github/workflows/enable-auto-merge.yaml @@ -89,9 +89,11 @@ jobs: with: client-id: ${{ vars.APP_CLIENT_ID }} private-key: ${{ secrets.APP_PRIVATE_KEY }} - # Least-privilege token scope: approving and enabling auto-merge on PRs. + # Least-privilege token scope: approving and enabling auto-merge on + # PRs, plus reading check suites for the gate's freshness floor. permission-contents: write permission-pull-requests: write + permission-checks: read # Self-reference: check out THIS workflow's own repo at the exact commit # it is running from (job.workflow_repository / job.workflow_sha) β€” never @@ -147,6 +149,7 @@ jobs: PR_NUMBER: ${{ steps.pr.outputs.number }} REPOSITORY: ${{ github.repository }} ENFORCE: ${{ (inputs.enforce-review-gates || vars.ENFORCE_MERGE_GATES == 'true') && 'true' || 'false' }} + EVENT_NAME: ${{ github.event_name }} run: | HEAD_SHA=$(gh pr view "$PR_NUMBER" --repo "$REPOSITORY" --json headRefOid --jq .headRefOid) @@ -156,8 +159,18 @@ jobs: echo "enforced=$ENFORCE" >> "$GITHUB_OUTPUT" if [[ "$ENFORCE" != "true" ]]; then - echo "armable=true" >> "$GITHUB_OUTPUT" - echo "::notice::Review/pre-merge gate enforcement is off (default) β€” arming PR #${PR_NUMBER} on the trusted-author allowlist alone. Enable via the enforce-review-gates input or the ENFORCE_MERGE_GATES variable." + # Default-off preserves the PRE-GATE behavior exactly: only the + # pull_request events armed before this gate existed, so the new + # review/comment retrigger paths must not arm while the flag is + # off (a changes-requested review or a findings comment must + # never be what re-arms a PR). + if [[ "$EVENT_NAME" == "pull_request" ]]; then + echo "armable=true" >> "$GITHUB_OUTPUT" + echo "::notice::Review/pre-merge gate enforcement is off (default) β€” arming PR #${PR_NUMBER} on the trusted-author allowlist alone. Enable via the enforce-review-gates input or the ENFORCE_MERGE_GATES variable." + else + echo "armable=false" >> "$GITHUB_OUTPUT" + echo "::notice::Review/pre-merge gate enforcement is off (default) β€” ${EVENT_NAME} runs do not arm PR #${PR_NUMBER} (pre-gate behavior preserved)." + fi exit 0 fi @@ -168,8 +181,13 @@ jobs: # summary pass as fresh. Committer date remains only as a fallback # floor when no check suite exists yet (never an empty, fail-open # floor). + # This run's own check suite is created by the triggering event + # itself β€” often AFTER the summary edit that triggered it β€” so as + # the only suite for a head it would poison the floor and wedge + # arming; exclude it before taking the minimum. + OWN_SUITE_ID=$(gh api "repos/$REPOSITORY/actions/runs/$GITHUB_RUN_ID" --jq '.check_suite_id // empty') HEAD_SEEN_AT=$(gh api "repos/$REPOSITORY/commits/$HEAD_SHA/check-suites" --paginate | - jq -rs '[.[].check_suites[]?.created_at | select(. != null)] | min // empty') + jq -rs --arg own "$OWN_SUITE_ID" '[.[].check_suites[]? | select((.id | tostring) != $own) | .created_at | select(. != null)] | min // empty') if [[ -z "$HEAD_SEEN_AT" ]]; then HEAD_SEEN_AT=$(gh api "repos/$REPOSITORY/commits/$HEAD_SHA" --jq .commit.committer.date) fi diff --git a/README.md b/README.md index 4dabbe06..522edb96 100644 --- a/README.md +++ b/README.md @@ -212,9 +212,10 @@ on: pull_request: types: [opened, synchronize, reopened, ready_for_review] # Required when enforce-review-gates is true: review results land after - # the pull_request events, so the caller must re-invoke the gate on them. + # the pull_request events, so the caller must re-invoke the gate on them + # β€” including dismissals, which are non-green and must be able to DISARM. pull_request_review: - types: [submitted] + types: [submitted, dismissed] issue_comment: types: [created, edited] From 1e90b4959a3cd579c5466eb135f4e5d03b31b772 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Sun, 12 Jul 2026 08:52:12 +0200 Subject: [PATCH 06/16] fix: run gate lookups on GITHUB_TOKEN and head-bind disarming The gate's read-only check-suite/run lookups move to GITHUB_TOKEN (checks:read + actions:read job permissions) so consumer App installations never need new permissions; the App token reverts to contents+pull-requests. Disarming re-checks the PR head so a stale gate run cannot revoke a newer head's arming. The default-off live self-test's enabled-state gap is recorded as a reasoned exception (a second live invocation would double-arm the same PR). Co-Authored-By: Claude Fable 5 --- .github/workflows/ci.yaml | 9 +++++++++ .github/workflows/enable-auto-merge.yaml | 23 ++++++++++++++++++++--- README.md | 6 ++++++ 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 927fa6a8..b512ba19 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -1771,9 +1771,18 @@ jobs: if: "${{ !startsWith(github.head_ref, 'release-please--') && !startsWith(github.event.head_commit.message, 'chore(main): release ') }}" name: "[Test] Enable Auto-Merge" uses: ./.github/workflows/enable-auto-merge.yaml + # Reasoned coverage gap (AGENTS.md both-states convention): this live + # invocation exercises the DEFAULT-OFF state only. A second invocation + # with enforce-review-gates: true would double-approve/arm the same PR + # alongside this one β€” conflicting side effects on one surface β€” so the + # enabled-state gate decisions are covered by the script-level fixture + # tests (test-enable-auto-merge-pentad-gate) and the wiring flips live + # per repo via ENFORCE_MERGE_GATES after validation. permissions: pull-requests: write contents: write + checks: read + actions: read secrets: APP_PRIVATE_KEY: ${{ secrets.APP_PRIVATE_KEY }} diff --git a/.github/workflows/enable-auto-merge.yaml b/.github/workflows/enable-auto-merge.yaml index f3ca5f0b..490a565a 100644 --- a/.github/workflows/enable-auto-merge.yaml +++ b/.github/workflows/enable-auto-merge.yaml @@ -42,6 +42,11 @@ jobs: permissions: pull-requests: write contents: write + # The gate's same-repo lookups (check suites for the freshness floor, + # this run's own suite id) run on GITHUB_TOKEN β€” the App token stays + # scoped to approving and arming only. + checks: read + actions: read runs-on: ubuntu-latest if: >- ${{ @@ -90,10 +95,11 @@ jobs: client-id: ${{ vars.APP_CLIENT_ID }} private-key: ${{ secrets.APP_PRIVATE_KEY }} # Least-privilege token scope: approving and enabling auto-merge on - # PRs, plus reading check suites for the gate's freshness floor. + # PRs. The gate's read-only lookups use GITHUB_TOKEN instead (see + # the job's permissions block), so no extra App permission is ever + # required of consumer installations. permission-contents: write permission-pull-requests: write - permission-checks: read # Self-reference: check out THIS workflow's own repo at the exact commit # it is running from (job.workflow_repository / job.workflow_sha) β€” never @@ -141,11 +147,14 @@ jobs: # consumers without CodeRabbit pre-merge summaries or agent-requested # reviews do not silently lose bot auto-merge before the flag is # flipped per repo/org after validation. + # Read-only: runs on GITHUB_TOKEN (checks: read + actions: read from + # the job's permissions), not the App token β€” consumer App + # installations never need Checks/Actions permissions for the gate. - name: πŸ›‚ Verify review and pre-merge gates id: gates if: steps.pr.outputs.eligible == 'true' env: - GH_TOKEN: ${{ steps.app-token.outputs.token }} + GH_TOKEN: ${{ github.token }} PR_NUMBER: ${{ steps.pr.outputs.number }} REPOSITORY: ${{ github.repository }} ENFORCE: ${{ (inputs.enforce-review-gates || vars.ENFORCE_MERGE_GATES == 'true') && 'true' || 'false' }} @@ -210,7 +219,15 @@ jobs: GH_TOKEN: ${{ steps.app-token.outputs.token }} PR_NUMBER: ${{ steps.pr.outputs.number }} REPOSITORY: ${{ github.repository }} + HEAD_SHA: ${{ steps.gates.outputs.head_sha }} run: | + # Head-bound like the approve/arm steps: a failed gate for an older + # head must not revoke arming that a newer, green head just earned. + CURRENT_HEAD=$(gh pr view "$PR_NUMBER" --repo "$REPOSITORY" --json headRefOid --jq .headRefOid) + if [[ "$CURRENT_HEAD" != "$HEAD_SHA" ]]; then + echo "::notice::PR #${PR_NUMBER} head moved (${HEAD_SHA} -> ${CURRENT_HEAD}); skipping disarm β€” the newer head's own gate run decides." + exit 0 + fi armed=$(gh pr view "$PR_NUMBER" --repo "$REPOSITORY" --json autoMergeRequest --jq '.autoMergeRequest != null') if [[ "$armed" == "true" ]]; then gh pr merge "$PR_NUMBER" --disable-auto --repo "$REPOSITORY" diff --git a/README.md b/README.md index 522edb96..472edc1a 100644 --- a/README.md +++ b/README.md @@ -222,6 +222,12 @@ on: jobs: auto-merge: uses: devantler-tech/actions/.github/workflows/enable-auto-merge.yaml@{ref} # ref + permissions: + pull-requests: write + contents: write + # The gate's read-only lookups run on GITHUB_TOKEN, not the App token: + checks: read + actions: read with: enforce-review-gates: false # default; flip after the repo's review lanes are validated secrets: From f5ef83fa6d2f0fe735ec5b2aa5f8b07e2b65bfa4 Mon Sep 17 00:00:00 2001 From: devantler Date: Sun, 12 Jul 2026 10:21:47 +0200 Subject: [PATCH 07/16] fix: isolate enforced-gate scopes, fail closed on lookups, re-check before arming - Job permissions revert to the legacy caller minimum; the enforced path's checks/actions read scopes move to a separate App token minted only on enforced runs, so existing workflow_call consumers stay valid. - Gate lookups run under set -euo pipefail: a failed API fetch aborts the gate (fail closed) instead of continuing on empty snapshots. - Per-PR concurrency serializes runs and the arm step re-runs the gate script immediately before gh pr merge --auto, so an older green run can no longer arm past a review that turned red mid-run. --- .../test-enable-auto-merge-pentad-gate.sh | 20 ++++-- .github/workflows/enable-auto-merge.yaml | 69 ++++++++++++++++--- README.md | 10 ++- 3 files changed, 83 insertions(+), 16 deletions(-) diff --git a/.github/tests/test-enable-auto-merge-pentad-gate.sh b/.github/tests/test-enable-auto-merge-pentad-gate.sh index b536154d..b1ec083e 100755 --- a/.github/tests/test-enable-auto-merge-pentad-gate.sh +++ b/.github/tests/test-enable-auto-merge-pentad-gate.sh @@ -17,10 +17,22 @@ head_sha="$(printf 'a%.0s' {1..40})" head_committed_at="2026-07-11T09:00:00Z" status=0 -# The workflow must actually consume the gate: the gates step runs the script -# and both privileged steps are conditioned on its armable output. -if ! grep -q 'check-merge-gates.sh' "$workflow"; then - echo "::error file=$workflow::auto-merge workflow does not run check-merge-gates.sh" +# The workflow must actually consume the gate: the gates step runs the script, +# both privileged steps are conditioned on its armable output, and the arming +# step re-runs the script immediately before `gh pr merge --auto` (an enforced +# green run must not arm past a review that turned red mid-run). +if [[ "$(grep -c 'check-merge-gates.sh' "$workflow")" -lt 2 ]]; then + echo "::error file=$workflow::auto-merge workflow must run check-merge-gates.sh in the gates step AND re-run it pre-arm in the Enable Auto-Merge step" + status=1 +fi + +# Backward compatibility for workflow_call consumers: a called workflow's +# GITHUB_TOKEN permissions can only be downgraded by callers' grants, so the +# job must never request more than the legacy documented minimum (the +# enforced path's extra read scopes come from the gate-lookup App token). +job_permissions="$(yq -r '.jobs."auto-merge".permissions | keys | sort | join(",")' "$workflow")" +if [[ "$job_permissions" != "contents,pull-requests" ]]; then + echo "::error file=$workflow::auto-merge job permissions must stay at the legacy caller minimum (contents, pull-requests); got: $job_permissions" status=1 fi diff --git a/.github/workflows/enable-auto-merge.yaml b/.github/workflows/enable-auto-merge.yaml index 490a565a..82fb09ac 100644 --- a/.github/workflows/enable-auto-merge.yaml +++ b/.github/workflows/enable-auto-merge.yaml @@ -39,14 +39,22 @@ permissions: {} jobs: auto-merge: + # Legacy documented minimum ONLY β€” a called workflow's GITHUB_TOKEN + # permissions must be a subset of what every caller grants (they can only + # be downgraded, never elevated), so adding scopes here would fail + # validation for existing workflow_call consumers even with enforcement + # off. The enforced path's extra read scopes come from a separate App + # token minted only on enforced runs (see πŸ”‘ gate-lookup token below). permissions: pull-requests: write contents: write - # The gate's same-repo lookups (check suites for the freshness floor, - # this run's own suite id) run on GITHUB_TOKEN β€” the App token stays - # scoped to approving and arming only. - checks: read - actions: read + # Serialize runs per PR so an older green gate run cannot interleave with + # a newer run that saw the gate turn red (the newer run's disarm always + # executes after the older run finishes); the pre-arm re-check in the + # πŸ”€ step closes the remaining in-run window. + concurrency: + group: enable-auto-merge-${{ github.event.pull_request.number || github.event.issue.number || github.run_id }} + cancel-in-progress: false runs-on: ubuntu-latest if: >- ${{ @@ -132,6 +140,25 @@ jobs: echo "::notice::PR #${PR_NUMBER} is draft or not open; skipping the auto-merge gate." fi + # Enforced-path-only read scopes, minted as a SEPARATE App token so the + # default-off path β€” and therefore every legacy workflow_call consumer's + # permissions block β€” never needs them. If a consumer's App installation + # lacks Checks/Actions read, the mint silently intersects them away and + # the gate's lookups fail, which fails the gate step CLOSED (no arming), + # never open. + - name: πŸ”‘ Generate gate-lookup token (enforced runs only) + uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 + id: gate-token + if: >- + steps.pr.outputs.eligible == 'true' && + (inputs.enforce-review-gates || vars.ENFORCE_MERGE_GATES == 'true') + with: + client-id: ${{ vars.APP_CLIENT_ID }} + private-key: ${{ secrets.APP_PRIVATE_KEY }} + permission-checks: read + permission-actions: read + permission-pull-requests: read + # Fail-closed pentad gate (actions#548): approving and arming are only # allowed once a CURRENT-HEAD green review (CodeRabbit APPROVED or Codex # clean pass) AND a green CodeRabbit pre-merge result are both proven. @@ -147,19 +174,23 @@ jobs: # consumers without CodeRabbit pre-merge summaries or agent-requested # reviews do not silently lose bot auto-merge before the flag is # flipped per repo/org after validation. - # Read-only: runs on GITHUB_TOKEN (checks: read + actions: read from - # the job's permissions), not the App token β€” consumer App - # installations never need Checks/Actions permissions for the gate. + # Read-only: enforced lookups run on the gate-lookup App token above, + # never the job's GITHUB_TOKEN β€” legacy callers' permissions blocks + # stay valid, and the privileged App token stays scoped to + # approving/arming. Fail-closed pipelines: any lookup error aborts the + # step (armable never set β†’ nothing arms) instead of continuing on + # empty snapshots that could erase a red verdict. - name: πŸ›‚ Verify review and pre-merge gates id: gates if: steps.pr.outputs.eligible == 'true' env: - GH_TOKEN: ${{ github.token }} + GH_TOKEN: ${{ steps.gate-token.outputs.token || steps.app-token.outputs.token }} PR_NUMBER: ${{ steps.pr.outputs.number }} REPOSITORY: ${{ github.repository }} ENFORCE: ${{ (inputs.enforce-review-gates || vars.ENFORCE_MERGE_GATES == 'true') && 'true' || 'false' }} EVENT_NAME: ${{ github.event_name }} run: | + set -euo pipefail HEAD_SHA=$(gh pr view "$PR_NUMBER" --repo "$REPOSITORY" --json headRefOid --jq .headRefOid) # The proven head is exported so the privileged steps below bind to @@ -200,6 +231,8 @@ jobs: if [[ -z "$HEAD_SEEN_AT" ]]; then HEAD_SEEN_AT=$(gh api "repos/$REPOSITORY/commits/$HEAD_SHA" --jq .commit.committer.date) fi + # Exported for the pre-arm re-check (head-bound, so stable per SHA). + echo "head_seen_at=$HEAD_SEEN_AT" >> "$GITHUB_OUTPUT" gh api "repos/$REPOSITORY/pulls/$PR_NUMBER/reviews" --paginate | jq -s 'add // []' > /tmp/reviews.json gh api "repos/$REPOSITORY/issues/$PR_NUMBER/comments" --paginate | jq -s 'add // []' > /tmp/comments.json @@ -264,15 +297,33 @@ jobs: if: steps.gates.outputs.armable == 'true' env: GH_TOKEN: ${{ steps.app-token.outputs.token }} + GATE_TOKEN: ${{ steps.gate-token.outputs.token }} PR_NUMBER: ${{ steps.pr.outputs.number }} REPOSITORY: ${{ github.repository }} HEAD_SHA: ${{ steps.gates.outputs.head_sha }} + HEAD_SEEN_AT: ${{ steps.gates.outputs.head_seen_at }} + ENFORCED: ${{ steps.gates.outputs.enforced }} run: | + set -euo pipefail REPO_INFO=$(gh api "repos/$REPOSITORY" --jq '.allow_auto_merge') if [[ "$REPO_INFO" != "true" ]]; then echo "::warning::Auto-merge is not enabled on this repository. Contact a repository admin to enable it in Settings > Pull Requests > Allow auto-merge." exit 0 fi + # Re-validate the gates IMMEDIATELY before arming: --match-head-commit + # only pins the SHA, so a review that turned red while this green run + # was still executing would otherwise be armed past. Per-PR + # concurrency serializes whole runs; this re-check closes the + # remaining in-run window. Fail-closed: a red or unreadable re-check + # skips arming (the later red event's own run also disarms). + if [[ "$ENFORCED" == "true" ]]; then + GH_TOKEN="$GATE_TOKEN" gh api "repos/$REPOSITORY/pulls/$PR_NUMBER/reviews" --paginate | jq -s 'add // []' > /tmp/reviews-arm.json + GH_TOKEN="$GATE_TOKEN" gh api "repos/$REPOSITORY/issues/$PR_NUMBER/comments" --paginate | jq -s 'add // []' > /tmp/comments-arm.json + if ! bash .devantler-tech-actions/.scripts/check-merge-gates.sh "$HEAD_SHA" "$HEAD_SEEN_AT" /tmp/reviews-arm.json /tmp/comments-arm.json; then + echo "::warning::PR #${PR_NUMBER} NOT armed: the review/pre-merge gates turned red between the gate check and arming (fail-closed pre-arm re-check)." + exit 0 + fi + fi # --match-head-commit binds the arming to the gate-proven head: a # push racing this workflow makes the merge call fail instead of # arming an unreviewed commit. diff --git a/README.md b/README.md index 472edc1a..98d08467 100644 --- a/README.md +++ b/README.md @@ -225,15 +225,19 @@ jobs: permissions: pull-requests: write contents: write - # The gate's read-only lookups run on GITHUB_TOKEN, not the App token: - checks: read - actions: read with: enforce-review-gates: false # default; flip after the repo's review lanes are validated secrets: APP_PRIVATE_KEY: ${{ secrets.APP_PRIVATE_KEY }} ``` +> **Note:** The caller grants only the legacy minimum above, with or without +> enforcement β€” the enforced gate's read-only lookups run on a separate App +> token minted only on enforced runs, so opting in requires the GitHub App +> installation (not the caller's `GITHUB_TOKEN`) to include **Checks: read** +> and **Actions: read**. If the installation lacks them, the gate fails +> closed (nothing is armed) rather than open. + #### Secrets and Inputs | Key | Type | Default | Required | Description | From c9a0defd0ae7741919f39d1f49fbd0b6be55ae8b Mon Sep 17 00:00:00 2001 From: devantler Date: Sun, 12 Jul 2026 10:42:48 +0200 Subject: [PATCH 08/16] fix: grant the gate-lookup token contents read for the committer-date fallback --- .github/workflows/enable-auto-merge.yaml | 4 ++++ README.md | 6 +++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/.github/workflows/enable-auto-merge.yaml b/.github/workflows/enable-auto-merge.yaml index 82fb09ac..8cfd4870 100644 --- a/.github/workflows/enable-auto-merge.yaml +++ b/.github/workflows/enable-auto-merge.yaml @@ -158,6 +158,10 @@ jobs: permission-checks: read permission-actions: read permission-pull-requests: read + # The committer-date freshness fallback reads + # GET /repos//commits/, which App tokens need Contents + # read for. + permission-contents: read # Fail-closed pentad gate (actions#548): approving and arming are only # allowed once a CURRENT-HEAD green review (CodeRabbit APPROVED or Codex diff --git a/README.md b/README.md index 98d08467..8a38f2a2 100644 --- a/README.md +++ b/README.md @@ -234,9 +234,9 @@ jobs: > **Note:** The caller grants only the legacy minimum above, with or without > enforcement β€” the enforced gate's read-only lookups run on a separate App > token minted only on enforced runs, so opting in requires the GitHub App -> installation (not the caller's `GITHUB_TOKEN`) to include **Checks: read** -> and **Actions: read**. If the installation lacks them, the gate fails -> closed (nothing is armed) rather than open. +> installation (not the caller's `GITHUB_TOKEN`) to include **Checks: read**, +> **Actions: read**, and **Contents: read**. If the installation lacks them, +> the gate fails closed (nothing is armed) rather than open. #### Secrets and Inputs From 2d110412286b655b431f64c4e3df90694830c92d Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Sun, 12 Jul 2026 12:25:47 +0200 Subject: [PATCH 09/16] fix: fail closed on gate lookup failure, COMMENTED findings, returned SHAs, and deleted evidence Round-6 review hardening of the auto-merge pentad gate: - disarm now also fires when the gate step itself fails (lookup errors previously skipped disarm via the implicit success() condition), with head binding skipped only when no head was proven - a CodeRabbit COMMENTED review at the head carrying actionable findings supersedes an earlier approval (clean 'Actionable comments posted: 0' incrementals keep it) - the pre-merge freshness floor is raised to the newest force-push time so a branch returning to an earlier SHA cannot pass an intervening head's summary as fresh - issue_comment deleted events re-evaluate the gate so removed evidence disarms Co-Authored-By: Claude Fable 5 --- .../comments.json | 9 +++ .../reviews.json | 19 ++++++ .../comments.json | 9 +++ .../reviews.json | 19 ++++++ .github/tests/merge-gate-fixtures/index.json | 8 +++ .../test-enable-auto-merge-pentad-gate.sh | 30 +++++++++ .github/workflows/enable-auto-merge.yaml | 62 +++++++++++++++---- .scripts/check-merge-gates.sh | 40 ++++++++++-- 8 files changed, 178 insertions(+), 18 deletions(-) create mode 100644 .github/tests/merge-gate-fixtures/cr-approval-then-commented-clean/comments.json create mode 100644 .github/tests/merge-gate-fixtures/cr-approval-then-commented-clean/reviews.json create mode 100644 .github/tests/merge-gate-fixtures/cr-approval-then-commented-findings/comments.json create mode 100644 .github/tests/merge-gate-fixtures/cr-approval-then-commented-findings/reviews.json diff --git a/.github/tests/merge-gate-fixtures/cr-approval-then-commented-clean/comments.json b/.github/tests/merge-gate-fixtures/cr-approval-then-commented-clean/comments.json new file mode 100644 index 00000000..740b8ad2 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/cr-approval-then-commented-clean/comments.json @@ -0,0 +1,9 @@ +[ + { + "user": { + "login": "coderabbitai[bot]" + }, + "body": "\n\n\ud83d\udea5 Pre-merge checks | \u2705 5\n", + "created_at": "2026-07-11T10:00:00Z" + } +] \ No newline at end of file diff --git a/.github/tests/merge-gate-fixtures/cr-approval-then-commented-clean/reviews.json b/.github/tests/merge-gate-fixtures/cr-approval-then-commented-clean/reviews.json new file mode 100644 index 00000000..91486fe0 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/cr-approval-then-commented-clean/reviews.json @@ -0,0 +1,19 @@ +[ + { + "user": { + "login": "coderabbitai[bot]" + }, + "state": "APPROVED", + "commit_id": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "submitted_at": "2026-07-11T10:00:00Z" + }, + { + "user": { + "login": "coderabbitai[bot]" + }, + "state": "COMMENTED", + "commit_id": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "submitted_at": "2026-07-11T11:00:00Z", + "body": "**Actionable comments posted: 0**\n\n
\nπŸ”‡ Additional comments (3)\ninformational only\n
" + } +] diff --git a/.github/tests/merge-gate-fixtures/cr-approval-then-commented-findings/comments.json b/.github/tests/merge-gate-fixtures/cr-approval-then-commented-findings/comments.json new file mode 100644 index 00000000..740b8ad2 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/cr-approval-then-commented-findings/comments.json @@ -0,0 +1,9 @@ +[ + { + "user": { + "login": "coderabbitai[bot]" + }, + "body": "\n\n\ud83d\udea5 Pre-merge checks | \u2705 5\n", + "created_at": "2026-07-11T10:00:00Z" + } +] \ No newline at end of file diff --git a/.github/tests/merge-gate-fixtures/cr-approval-then-commented-findings/reviews.json b/.github/tests/merge-gate-fixtures/cr-approval-then-commented-findings/reviews.json new file mode 100644 index 00000000..da6d4b22 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/cr-approval-then-commented-findings/reviews.json @@ -0,0 +1,19 @@ +[ + { + "user": { + "login": "coderabbitai[bot]" + }, + "state": "APPROVED", + "commit_id": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "submitted_at": "2026-07-11T10:00:00Z" + }, + { + "user": { + "login": "coderabbitai[bot]" + }, + "state": "COMMENTED", + "commit_id": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "submitted_at": "2026-07-11T11:00:00Z", + "body": "**Actionable comments posted: 2**\n\n
\n⚠️ Outside diff range comments (2)\nfindings here\n
" + } +] diff --git a/.github/tests/merge-gate-fixtures/index.json b/.github/tests/merge-gate-fixtures/index.json index 4e66a19c..2c461a5b 100644 --- a/.github/tests/merge-gate-fixtures/index.json +++ b/.github/tests/merge-gate-fixtures/index.json @@ -59,6 +59,14 @@ "name": "cr-approval-dismissed-at-head", "expect_green": false }, + { + "name": "cr-approval-then-commented-findings", + "expect_green": false + }, + { + "name": "cr-approval-then-commented-clean", + "expect_green": true + }, { "name": "premerge-warning-full", "expect_green": false diff --git a/.github/tests/test-enable-auto-merge-pentad-gate.sh b/.github/tests/test-enable-auto-merge-pentad-gate.sh index b1ec083e..7baa2bc2 100755 --- a/.github/tests/test-enable-auto-merge-pentad-gate.sh +++ b/.github/tests/test-enable-auto-merge-pentad-gate.sh @@ -46,6 +46,36 @@ if [[ "$(grep -c "steps.gates.outputs.armable == 'true'" <<<"$armable_conditions status=1 fi +# The disarm step must fail CLOSED when the gate step itself failed (a lookup +# error), not silently inherit success() and leave a stale arming in place: it +# needs a status-check override plus an explicit outcome-failure branch. +disarm_condition="$(yq -r ' + [.jobs."auto-merge".steps[] + | select(.name == "πŸ”’ Disarm auto-merge on failed gates") + | .if // ""] + | join("\n")' "$workflow")" +if [[ "$disarm_condition" != *"!cancelled()"* || + "$disarm_condition" != *"steps.gates.outcome == 'failure'"* ]]; then + echo "::error file=$workflow::the disarm step must run on gate-step failure (!cancelled() + steps.gates.outcome == 'failure'), not only on armable == 'false'" + status=1 +fi + +# Deleted reviewer evidence (a removed pre-merge summary or Codex clean pass) +# must re-trigger the gate so an enforced arming does not outlive its proof. +issue_comment_types="$(yq -r '.on.issue_comment.types | join(",")' "$workflow")" +if [[ "$issue_comment_types" != *"deleted"* ]]; then + echo "::error file=$workflow::issue_comment trigger must include the deleted type (evidence deletion is a disarm path); got: $issue_comment_types" + status=1 +fi + +# A branch returning to an earlier SHA re-uses that SHA's original check +# suites, so the freshness floor must also consider the newest force-push +# time β€” otherwise a summary written for an intervening head passes as fresh. +if ! grep -q 'head_ref_force_pushed' "$workflow"; then + echo "::error file=$workflow::the head-seen freshness floor must be raised by the newest head_ref_force_pushed timeline event" + status=1 +fi + while IFS= read -r fixture; do name="$(jq -r '.name' <<<"$fixture")" expect_green="$(jq -r '.expect_green' <<<"$fixture")" diff --git a/.github/workflows/enable-auto-merge.yaml b/.github/workflows/enable-auto-merge.yaml index 8cfd4870..aa90172f 100644 --- a/.github/workflows/enable-auto-merge.yaml +++ b/.github/workflows/enable-auto-merge.yaml @@ -24,16 +24,19 @@ on: merge_group: ################################## # Review results land AFTER the pull_request events above have run, so the - # gate re-evaluates when a reviewer bot posts or edits its result: any - # CodeRabbit review verdict (approval OR changes-requested β€” the latter must - # be able to DISARM), a DISMISSED approval (dismissal is non-green and must - # also disarm), a Codex result comment, and CodeRabbit's in-place pre-merge - # summary edits. These triggers only fire where this workflow file lives; - # workflow_call consumers add them to their caller (see the input). + # gate re-evaluates when a reviewer bot posts, edits, or deletes its result: + # any CodeRabbit review verdict (approval OR changes-requested β€” the latter + # must be able to DISARM), a DISMISSED approval (dismissal is non-green and + # must also disarm), a Codex result comment, and CodeRabbit's in-place + # pre-merge summary edits. `deleted` is a disarm path: evidence an enforced + # run armed on (a pre-merge summary or Codex clean pass) can be removed + # afterwards, and the gate must re-evaluate the now-missing state. These + # triggers only fire where this workflow file lives; workflow_call consumers + # add them to their caller (see the input). pull_request_review: types: [submitted, dismissed] issue_comment: - types: [created, edited] + types: [created, edited, deleted] permissions: {} @@ -235,6 +238,18 @@ jobs: if [[ -z "$HEAD_SEEN_AT" ]]; then HEAD_SEEN_AT=$(gh api "repos/$REPOSITORY/commits/$HEAD_SHA" --jq .commit.committer.date) fi + # A branch that moves away and later RETURNS to this same SHA + # re-uses the SHA's original check suites, so the earliest-suite + # floor would date from the FIRST time GitHub saw it β€” letting a + # summary written for an intervening head pass as fresh. Returning + # to an earlier SHA requires a force-push, so raise the floor to the + # newest force-push time when one exists (normal pushes create + # never-seen SHAs, where the earliest-suite floor already holds). + LAST_FORCE_PUSH_AT=$(gh api "repos/$REPOSITORY/issues/$PR_NUMBER/timeline" --paginate | + jq -rs '[.[][] | select(.event == "head_ref_force_pushed") | .created_at | select(. != null)] | max // empty') + if [[ -n "$LAST_FORCE_PUSH_AT" && "$LAST_FORCE_PUSH_AT" > "$HEAD_SEEN_AT" ]]; then + HEAD_SEEN_AT="$LAST_FORCE_PUSH_AT" + fi # Exported for the pre-arm re-check (head-bound, so stable per SHA). echo "head_seen_at=$HEAD_SEEN_AT" >> "$GITHUB_OUTPUT" gh api "repos/$REPOSITORY/pulls/$PR_NUMBER/reviews" --paginate | jq -s 'add // []' > /tmp/reviews.json @@ -249,9 +264,23 @@ jobs: # A gate that turned red AFTER an earlier run armed the PR (e.g. a # CodeRabbit changes-requested landing behind a Codex clean pass) must - # actively revoke the arming, not just decline to re-arm. + # actively revoke the arming, not just decline to re-arm. A gate step + # that FAILED outright (a lookup error under set -euo) is the same + # disarm condition, not a skip: unreadable gates on an enforced run must + # fail closed, so `!cancelled()` + outcome checks override the implicit + # success() the plain outputs-based condition would inherit. Enforcement + # is re-derived from the input/var because a failed gate step may have + # died before exporting its `enforced` output (with the flag off this + # whole condition stays false β€” pre-gate behavior never disarms). - name: πŸ”’ Disarm auto-merge on failed gates - if: steps.gates.outputs.enforced == 'true' && steps.gates.outputs.armable == 'false' + if: >- + ${{ + !cancelled() && + steps.pr.outputs.eligible == 'true' && + (inputs.enforce-review-gates || vars.ENFORCE_MERGE_GATES == 'true') && + (steps.gates.outcome == 'failure' || + (steps.gates.outcome == 'success' && steps.gates.outputs.armable == 'false')) + }} env: GH_TOKEN: ${{ steps.app-token.outputs.token }} PR_NUMBER: ${{ steps.pr.outputs.number }} @@ -260,10 +289,17 @@ jobs: run: | # Head-bound like the approve/arm steps: a failed gate for an older # head must not revoke arming that a newer, green head just earned. - CURRENT_HEAD=$(gh pr view "$PR_NUMBER" --repo "$REPOSITORY" --json headRefOid --jq .headRefOid) - if [[ "$CURRENT_HEAD" != "$HEAD_SHA" ]]; then - echo "::notice::PR #${PR_NUMBER} head moved (${HEAD_SHA} -> ${CURRENT_HEAD}); skipping disarm β€” the newer head's own gate run decides." - exit 0 + # A gate step that died before proving a head leaves HEAD_SHA empty + # β€” then disarm WITHOUT the head binding (fail-closed: unreadable + # gates must revoke arming; a later green run re-arms). + if [[ -n "$HEAD_SHA" ]]; then + CURRENT_HEAD=$(gh pr view "$PR_NUMBER" --repo "$REPOSITORY" --json headRefOid --jq .headRefOid) + if [[ "$CURRENT_HEAD" != "$HEAD_SHA" ]]; then + echo "::notice::PR #${PR_NUMBER} head moved (${HEAD_SHA} -> ${CURRENT_HEAD}); skipping disarm β€” the newer head's own gate run decides." + exit 0 + fi + else + echo "::warning::PR #${PR_NUMBER}: gate lookups failed before proving a head; disarming without head binding (fail-closed)." fi armed=$(gh pr view "$PR_NUMBER" --repo "$REPOSITORY" --json autoMergeRequest --jq '.autoMergeRequest != null') if [[ "$armed" == "true" ]]; then diff --git a/.scripts/check-merge-gates.sh b/.scripts/check-merge-gates.sh index 9eb082d3..501cbfaf 100755 --- a/.scripts/check-merge-gates.sh +++ b/.scripts/check-merge-gates.sh @@ -3,8 +3,10 @@ # # Usage: check-merge-gates.sh # head-sha the pull request's current head commit SHA -# head-seen-at ISO8601 time GitHub first saw the head (the caller passes -# the earliest check-suite created_at for the SHA, falling +# head-seen-at ISO8601 time GitHub last saw the head BECOME the head +# (the caller passes the earliest check-suite created_at +# for the SHA β€” raised to the newest force-push time when +# the branch later returned to an earlier SHA β€” falling # back to the committer date) β€” the freshness floor for the # pre-merge summary, which CodeRabbit edits in place and # which carries no commit SHA of its own. Commit metadata @@ -16,9 +18,11 @@ # Exits 0 only when BOTH gates are proven at the current head: # 1. a green review β€” CodeRabbit's LATEST review verdict at the head is # APPROVED (an earlier approval superseded by CHANGES_REQUESTED or a -# dismissal is NOT green), or β€” when CodeRabbit has no verdict at the -# head β€” a Codex clean pass ("Didn't find any major issues") whose -# "Reviewed commit" equals the head and is Codex's LATEST result for it; +# dismissal is NOT green, and a later COMMENTED review at the head +# carrying actionable findings supersedes the approval too), or β€” when +# CodeRabbit has no verdict at the head β€” a Codex clean pass ("Didn't +# find any major issues") whose "Reviewed commit" equals the head and is +# Codex's LATEST result for it; # 2. a green CodeRabbit pre-merge result β€” the most recently UPDATED # auto-generated summary (stable marker required; CodeRabbit edits the # summary in place, so created_at alone selects a stale revision) whose @@ -74,6 +78,32 @@ elif [[ "$cr_approved_anywhere" -gt 0 ]]; then review_state="stale" fi +# CodeRabbit posts incremental findings as a COMMENTED review WITHOUT revoking +# its earlier approval, so a COMMENTED review at the head that lands after the +# approval and carries actionable findings must supersede it. A clean +# incremental pass ("Actionable comments posted: 0") keeps the approval; a +# body that cannot be proven clean is treated as findings (fail-closed). +if [[ "$review_state" == "green" ]]; then + cr_commented_after_verdict="$(jq -r --arg sha "$head_sha" ' + ([.[] + | select(.user.login == "coderabbitai[bot]") + | select(.commit_id == $sha) + | select(.state == "APPROVED" or .state == "CHANGES_REQUESTED" + or .state == "DISMISSED")] + | sort_by(.submitted_at) | last | .submitted_at // "") as $verdict_at + | [.[] + | select(.user.login == "coderabbitai[bot]") + | select(.commit_id == $sha) + | select(.state == "COMMENTED") + | select(.submitted_at > $verdict_at)] + | sort_by(.submitted_at) | last | .body // empty' "$reviews_json")" + + if [[ -n "$cr_commented_after_verdict" && + "$cr_commented_after_verdict" != *"Actionable comments posted: 0"* ]]; then + review_state="needs-fix" + fi +fi + if [[ "$review_state" == "missing" || "$review_state" == "stale" ]]; then # Codex lane: its result is an ISSUE COMMENT carrying "**Reviewed commit:** # ". Only the LATEST Codex result for the current head counts β€” an From 54e71fa083b30869ee860c0d21ae67c06f769e52 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Sun, 12 Jul 2026 12:46:34 +0200 Subject: [PATCH 10/16] fix: disarm on token-mint failure, generalize COMMENTED gate, recompute pre-arm floor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-7 review hardening: - a failed gate-token mint (ungranted consumer App permission) now routes through the fail-closed disarm path, same as a failed gate step - any CodeRabbit COMMENTED review at the head that is not explicitly clean (Actionable comments posted: 0) blocks green β€” superseding approvals, covering blank bodies, and pre-empting the Codex fallback - the freshness floor moves to a shared compute-head-seen-floor.sh and the pre-arm re-check recomputes it, so a force-push away and back to the same SHA between gate and arm cannot ride the old floor - the caller example in README subscribes to issue_comment deleted Co-Authored-By: Claude Fable 5 --- .../comments.json | 9 +++ .../reviews.json | 18 ++++++ .../comments.json | 16 +++++ .../reviews.json | 11 ++++ .../comments.json | 16 +++++ .../reviews.json | 11 ++++ .github/tests/merge-gate-fixtures/index.json | 12 ++++ .../test-enable-auto-merge-pentad-gate.sh | 17 ++++-- .github/workflows/enable-auto-merge.yaml | 59 ++++++++----------- .scripts/check-merge-gates.sh | 56 ++++++++++-------- .scripts/compute-head-seen-floor.sh | 58 ++++++++++++++++++ README.md | 6 +- 12 files changed, 223 insertions(+), 66 deletions(-) create mode 100644 .github/tests/merge-gate-fixtures/cr-commented-blank-supersedes-approval/comments.json create mode 100644 .github/tests/merge-gate-fixtures/cr-commented-blank-supersedes-approval/reviews.json create mode 100644 .github/tests/merge-gate-fixtures/cr-commented-clean-allows-codex-fallback/comments.json create mode 100644 .github/tests/merge-gate-fixtures/cr-commented-clean-allows-codex-fallback/reviews.json create mode 100644 .github/tests/merge-gate-fixtures/cr-commented-findings-blocks-codex-fallback/comments.json create mode 100644 .github/tests/merge-gate-fixtures/cr-commented-findings-blocks-codex-fallback/reviews.json create mode 100644 .scripts/compute-head-seen-floor.sh diff --git a/.github/tests/merge-gate-fixtures/cr-commented-blank-supersedes-approval/comments.json b/.github/tests/merge-gate-fixtures/cr-commented-blank-supersedes-approval/comments.json new file mode 100644 index 00000000..740b8ad2 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/cr-commented-blank-supersedes-approval/comments.json @@ -0,0 +1,9 @@ +[ + { + "user": { + "login": "coderabbitai[bot]" + }, + "body": "\n\n\ud83d\udea5 Pre-merge checks | \u2705 5\n", + "created_at": "2026-07-11T10:00:00Z" + } +] \ No newline at end of file diff --git a/.github/tests/merge-gate-fixtures/cr-commented-blank-supersedes-approval/reviews.json b/.github/tests/merge-gate-fixtures/cr-commented-blank-supersedes-approval/reviews.json new file mode 100644 index 00000000..09f6f164 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/cr-commented-blank-supersedes-approval/reviews.json @@ -0,0 +1,18 @@ +[ + { + "user": { + "login": "coderabbitai[bot]" + }, + "state": "APPROVED", + "commit_id": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "submitted_at": "2026-07-11T10:00:00Z" + }, + { + "user": { + "login": "coderabbitai[bot]" + }, + "state": "COMMENTED", + "commit_id": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "submitted_at": "2026-07-11T11:00:00Z" + } +] diff --git a/.github/tests/merge-gate-fixtures/cr-commented-clean-allows-codex-fallback/comments.json b/.github/tests/merge-gate-fixtures/cr-commented-clean-allows-codex-fallback/comments.json new file mode 100644 index 00000000..30676367 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/cr-commented-clean-allows-codex-fallback/comments.json @@ -0,0 +1,16 @@ +[ + { + "user": { + "login": "chatgpt-codex-connector[bot]" + }, + "body": "Codex Review: Didn't find any major issues.\n\n**Reviewed commit:** aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "created_at": "2026-07-11T10:00:00Z" + }, + { + "user": { + "login": "coderabbitai[bot]" + }, + "body": "\n\n## Pre-merge checks\n| Title check | \u2705 Passed | ok |\n| Linked Issues check | \u2705 Passed | ok |\n", + "created_at": "2026-07-11T10:00:00Z" + } +] \ No newline at end of file diff --git a/.github/tests/merge-gate-fixtures/cr-commented-clean-allows-codex-fallback/reviews.json b/.github/tests/merge-gate-fixtures/cr-commented-clean-allows-codex-fallback/reviews.json new file mode 100644 index 00000000..369c5bca --- /dev/null +++ b/.github/tests/merge-gate-fixtures/cr-commented-clean-allows-codex-fallback/reviews.json @@ -0,0 +1,11 @@ +[ + { + "user": { + "login": "coderabbitai[bot]" + }, + "state": "COMMENTED", + "commit_id": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "submitted_at": "2026-07-11T11:00:00Z", + "body": "**Actionable comments posted: 0**\n\n
\nπŸ”‡ Additional comments (1)\ninformational only\n
" + } +] diff --git a/.github/tests/merge-gate-fixtures/cr-commented-findings-blocks-codex-fallback/comments.json b/.github/tests/merge-gate-fixtures/cr-commented-findings-blocks-codex-fallback/comments.json new file mode 100644 index 00000000..30676367 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/cr-commented-findings-blocks-codex-fallback/comments.json @@ -0,0 +1,16 @@ +[ + { + "user": { + "login": "chatgpt-codex-connector[bot]" + }, + "body": "Codex Review: Didn't find any major issues.\n\n**Reviewed commit:** aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "created_at": "2026-07-11T10:00:00Z" + }, + { + "user": { + "login": "coderabbitai[bot]" + }, + "body": "\n\n## Pre-merge checks\n| Title check | \u2705 Passed | ok |\n| Linked Issues check | \u2705 Passed | ok |\n", + "created_at": "2026-07-11T10:00:00Z" + } +] \ No newline at end of file diff --git a/.github/tests/merge-gate-fixtures/cr-commented-findings-blocks-codex-fallback/reviews.json b/.github/tests/merge-gate-fixtures/cr-commented-findings-blocks-codex-fallback/reviews.json new file mode 100644 index 00000000..cd7585df --- /dev/null +++ b/.github/tests/merge-gate-fixtures/cr-commented-findings-blocks-codex-fallback/reviews.json @@ -0,0 +1,11 @@ +[ + { + "user": { + "login": "coderabbitai[bot]" + }, + "state": "COMMENTED", + "commit_id": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "submitted_at": "2026-07-11T11:00:00Z", + "body": "**Actionable comments posted: 2**\n\n
\n⚠️ Outside diff range comments (2)\nfindings here\n
" + } +] diff --git a/.github/tests/merge-gate-fixtures/index.json b/.github/tests/merge-gate-fixtures/index.json index 2c461a5b..4413e971 100644 --- a/.github/tests/merge-gate-fixtures/index.json +++ b/.github/tests/merge-gate-fixtures/index.json @@ -67,6 +67,18 @@ "name": "cr-approval-then-commented-clean", "expect_green": true }, + { + "name": "cr-commented-blank-supersedes-approval", + "expect_green": false + }, + { + "name": "cr-commented-findings-blocks-codex-fallback", + "expect_green": false + }, + { + "name": "cr-commented-clean-allows-codex-fallback", + "expect_green": true + }, { "name": "premerge-warning-full", "expect_green": false diff --git a/.github/tests/test-enable-auto-merge-pentad-gate.sh b/.github/tests/test-enable-auto-merge-pentad-gate.sh index 7baa2bc2..229fcc81 100755 --- a/.github/tests/test-enable-auto-merge-pentad-gate.sh +++ b/.github/tests/test-enable-auto-merge-pentad-gate.sh @@ -55,8 +55,9 @@ disarm_condition="$(yq -r ' | .if // ""] | join("\n")' "$workflow")" if [[ "$disarm_condition" != *"!cancelled()"* || - "$disarm_condition" != *"steps.gates.outcome == 'failure'"* ]]; then - echo "::error file=$workflow::the disarm step must run on gate-step failure (!cancelled() + steps.gates.outcome == 'failure'), not only on armable == 'false'" + "$disarm_condition" != *"steps.gates.outcome == 'failure'"* || + "$disarm_condition" != *"steps.gate-token.outcome == 'failure'"* ]]; then + echo "::error file=$workflow::the disarm step must run on gate-step AND gate-token-mint failure (!cancelled() + both outcome == 'failure' branches), not only on armable == 'false'" status=1 fi @@ -71,8 +72,16 @@ fi # A branch returning to an earlier SHA re-uses that SHA's original check # suites, so the freshness floor must also consider the newest force-push # time β€” otherwise a summary written for an intervening head passes as fresh. -if ! grep -q 'head_ref_force_pushed' "$workflow"; then - echo "::error file=$workflow::the head-seen freshness floor must be raised by the newest head_ref_force_pushed timeline event" +# The floor lives in the shared compute-head-seen-floor.sh, and the workflow +# must consume it BOTH in the gate step and in the pre-arm re-check (reusing +# the gate step's floor would miss a force-push between gate and arm). +floor_script="${4:-.scripts/compute-head-seen-floor.sh}" +if ! grep -q 'head_ref_force_pushed' "$floor_script"; then + echo "::error file=$floor_script::the head-seen freshness floor must be raised by the newest head_ref_force_pushed timeline event" + status=1 +fi +if [[ "$(grep -c 'compute-head-seen-floor.sh' "$workflow")" -lt 2 ]]; then + echo "::error file=$workflow::the workflow must compute the freshness floor via compute-head-seen-floor.sh in the gate step AND recompute it in the pre-arm re-check" status=1 fi diff --git a/.github/workflows/enable-auto-merge.yaml b/.github/workflows/enable-auto-merge.yaml index aa90172f..fa54ddf8 100644 --- a/.github/workflows/enable-auto-merge.yaml +++ b/.github/workflows/enable-auto-merge.yaml @@ -221,37 +221,18 @@ jobs: exit 0 fi - # Freshness floor for the pre-merge summary: when GitHub first SAW - # this head (earliest check-suite created_at for the SHA), not the - # commit's own committer date β€” pushing a previously-created commit - # object carries an old committer date that would let a stale - # summary pass as fresh. Committer date remains only as a fallback - # floor when no check suite exists yet (never an empty, fail-open - # floor). - # This run's own check suite is created by the triggering event - # itself β€” often AFTER the summary edit that triggered it β€” so as - # the only suite for a head it would poison the floor and wedge - # arming; exclude it before taking the minimum. - OWN_SUITE_ID=$(gh api "repos/$REPOSITORY/actions/runs/$GITHUB_RUN_ID" --jq '.check_suite_id // empty') - HEAD_SEEN_AT=$(gh api "repos/$REPOSITORY/commits/$HEAD_SHA/check-suites" --paginate | - jq -rs --arg own "$OWN_SUITE_ID" '[.[].check_suites[]? | select((.id | tostring) != $own) | .created_at | select(. != null)] | min // empty') - if [[ -z "$HEAD_SEEN_AT" ]]; then - HEAD_SEEN_AT=$(gh api "repos/$REPOSITORY/commits/$HEAD_SHA" --jq .commit.committer.date) - fi - # A branch that moves away and later RETURNS to this same SHA - # re-uses the SHA's original check suites, so the earliest-suite - # floor would date from the FIRST time GitHub saw it β€” letting a - # summary written for an intervening head pass as fresh. Returning - # to an earlier SHA requires a force-push, so raise the floor to the - # newest force-push time when one exists (normal pushes create - # never-seen SHAs, where the earliest-suite floor already holds). - LAST_FORCE_PUSH_AT=$(gh api "repos/$REPOSITORY/issues/$PR_NUMBER/timeline" --paginate | - jq -rs '[.[][] | select(.event == "head_ref_force_pushed") | .created_at | select(. != null)] | max // empty') - if [[ -n "$LAST_FORCE_PUSH_AT" && "$LAST_FORCE_PUSH_AT" > "$HEAD_SEEN_AT" ]]; then - HEAD_SEEN_AT="$LAST_FORCE_PUSH_AT" - fi - # Exported for the pre-arm re-check (head-bound, so stable per SHA). - echo "head_seen_at=$HEAD_SEEN_AT" >> "$GITHUB_OUTPUT" + # Freshness floor for the pre-merge summary β€” the shared + # .scripts/compute-head-seen-floor.sh: earliest check-suite time for + # the SHA (excluding this run's own suite, which would poison the + # floor), committer-date fallback, raised to the newest force-push + # time (a branch returning to an earlier SHA re-uses its original + # suites). Rationale for each part lives in the script header. The + # pre-arm re-check RECOMPUTES it with the same script instead of + # reusing this value β€” a force-push away and back to the same SHA + # between gate and arm must raise the floor, not ride the old one. + HEAD_SEEN_AT=$(bash .devantler-tech-actions/.scripts/compute-head-seen-floor.sh \ + "$REPOSITORY" "$PR_NUMBER" "$HEAD_SHA" "$GITHUB_RUN_ID") + gh api "repos/$REPOSITORY/pulls/$PR_NUMBER/reviews" --paginate | jq -s 'add // []' > /tmp/reviews.json gh api "repos/$REPOSITORY/issues/$PR_NUMBER/comments" --paginate | jq -s 'add // []' > /tmp/comments.json @@ -271,14 +252,19 @@ jobs: # success() the plain outputs-based condition would inherit. Enforcement # is re-derived from the input/var because a failed gate step may have # died before exporting its `enforced` output (with the flag off this - # whole condition stays false β€” pre-gate behavior never disarms). + # whole condition stays false β€” pre-gate behavior never disarms). A + # FAILED gate-token mint is the same condition again: requesting a + # permission the consumer's App installation has not granted errors the + # mint step itself, so `steps.gates` never runs β€” unreadable gates, and + # an already-armed PR must still be revoked. - name: πŸ”’ Disarm auto-merge on failed gates if: >- ${{ !cancelled() && steps.pr.outputs.eligible == 'true' && (inputs.enforce-review-gates || vars.ENFORCE_MERGE_GATES == 'true') && - (steps.gates.outcome == 'failure' || + (steps.gate-token.outcome == 'failure' || + steps.gates.outcome == 'failure' || (steps.gates.outcome == 'success' && steps.gates.outputs.armable == 'false')) }} env: @@ -341,7 +327,6 @@ jobs: PR_NUMBER: ${{ steps.pr.outputs.number }} REPOSITORY: ${{ github.repository }} HEAD_SHA: ${{ steps.gates.outputs.head_sha }} - HEAD_SEEN_AT: ${{ steps.gates.outputs.head_seen_at }} ENFORCED: ${{ steps.gates.outputs.enforced }} run: | set -euo pipefail @@ -356,7 +341,13 @@ jobs: # concurrency serializes whole runs; this re-check closes the # remaining in-run window. Fail-closed: a red or unreadable re-check # skips arming (the later red event's own run also disarms). + # The freshness floor is RECOMPUTED here, not reused from the gate + # step: a branch force-pushed away and back to the same SHA between + # gate and arm keeps --match-head-commit satisfied while the old + # floor would let an intervening head's summary pass as fresh. if [[ "$ENFORCED" == "true" ]]; then + HEAD_SEEN_AT=$(GH_TOKEN="$GATE_TOKEN" bash .devantler-tech-actions/.scripts/compute-head-seen-floor.sh \ + "$REPOSITORY" "$PR_NUMBER" "$HEAD_SHA" "$GITHUB_RUN_ID") GH_TOKEN="$GATE_TOKEN" gh api "repos/$REPOSITORY/pulls/$PR_NUMBER/reviews" --paginate | jq -s 'add // []' > /tmp/reviews-arm.json GH_TOKEN="$GATE_TOKEN" gh api "repos/$REPOSITORY/issues/$PR_NUMBER/comments" --paginate | jq -s 'add // []' > /tmp/comments-arm.json if ! bash .devantler-tech-actions/.scripts/check-merge-gates.sh "$HEAD_SHA" "$HEAD_SEEN_AT" /tmp/reviews-arm.json /tmp/comments-arm.json; then diff --git a/.scripts/check-merge-gates.sh b/.scripts/check-merge-gates.sh index 501cbfaf..cf8ca95c 100755 --- a/.scripts/check-merge-gates.sh +++ b/.scripts/check-merge-gates.sh @@ -18,8 +18,9 @@ # Exits 0 only when BOTH gates are proven at the current head: # 1. a green review β€” CodeRabbit's LATEST review verdict at the head is # APPROVED (an earlier approval superseded by CHANGES_REQUESTED or a -# dismissal is NOT green, and a later COMMENTED review at the head -# carrying actionable findings supersedes the approval too), or β€” when +# dismissal is NOT green, and a COMMENTED review at the head that is not +# explicitly clean supersedes an approval and blocks the Codex +# fallback), or β€” when # CodeRabbit has no verdict at the head β€” a Codex clean pass ("Didn't # find any major issues") whose "Reviewed commit" equals the head and is # Codex's LATEST result for it; @@ -78,30 +79,33 @@ elif [[ "$cr_approved_anywhere" -gt 0 ]]; then review_state="stale" fi -# CodeRabbit posts incremental findings as a COMMENTED review WITHOUT revoking -# its earlier approval, so a COMMENTED review at the head that lands after the -# approval and carries actionable findings must supersede it. A clean -# incremental pass ("Actionable comments posted: 0") keeps the approval; a -# body that cannot be proven clean is treated as findings (fail-closed). -if [[ "$review_state" == "green" ]]; then - cr_commented_after_verdict="$(jq -r --arg sha "$head_sha" ' - ([.[] - | select(.user.login == "coderabbitai[bot]") - | select(.commit_id == $sha) - | select(.state == "APPROVED" or .state == "CHANGES_REQUESTED" - or .state == "DISMISSED")] - | sort_by(.submitted_at) | last | .submitted_at // "") as $verdict_at - | [.[] - | select(.user.login == "coderabbitai[bot]") - | select(.commit_id == $sha) - | select(.state == "COMMENTED") - | select(.submitted_at > $verdict_at)] - | sort_by(.submitted_at) | last | .body // empty' "$reviews_json")" - - if [[ -n "$cr_commented_after_verdict" && - "$cr_commented_after_verdict" != *"Actionable comments posted: 0"* ]]; then - review_state="needs-fix" - fi +# CodeRabbit posts incremental findings as a COMMENTED review WITHOUT issuing +# a verdict, so a COMMENTED review at the head that lands after the latest +# verdict (or with no verdict at all) must block a green outcome β€” it +# supersedes an earlier approval AND pre-empts the Codex fallback below. Only +# a body that explicitly proves clean ("Actionable comments posted: 0") +# preserves the state; a blank or unparseable body counts as findings +# (fail-closed β€” a bodyless COMMENTED review can still carry inline review +# comments). The EXISTENCE marker line distinguishes "no such review" from +# "review with an empty body". +cr_commented_probe="$(jq -r --arg sha "$head_sha" ' + ([.[] + | select(.user.login == "coderabbitai[bot]") + | select(.commit_id == $sha) + | select(.state == "APPROVED" or .state == "CHANGES_REQUESTED" + or .state == "DISMISSED")] + | sort_by(.submitted_at) | last | .submitted_at // "") as $verdict_at + | [.[] + | select(.user.login == "coderabbitai[bot]") + | select(.commit_id == $sha) + | select(.state == "COMMENTED") + | select(.submitted_at > $verdict_at)] + | sort_by(.submitted_at) | last + | if . == null then "absent" else "present\n" + (.body // "") end' "$reviews_json")" + +if [[ "$cr_commented_probe" == present* && + "$cr_commented_probe" != *"Actionable comments posted: 0"* ]]; then + review_state="needs-fix" fi if [[ "$review_state" == "missing" || "$review_state" == "stale" ]]; then diff --git a/.scripts/compute-head-seen-floor.sh b/.scripts/compute-head-seen-floor.sh new file mode 100644 index 00000000..3c916a83 --- /dev/null +++ b/.scripts/compute-head-seen-floor.sh @@ -0,0 +1,58 @@ +#!/usr/bin/env bash +# Freshness floor for CodeRabbit's pre-merge summary: the time GitHub last saw +# the given SHA BECOME the PR head. Shared by the gate step and the pre-arm +# re-check of the privileged auto-merge workflow (the re-check must recompute +# it β€” a branch force-pushed away and back to the same SHA between gate and +# arm would otherwise be compared against a stale floor). +# +# Usage: compute-head-seen-floor.sh [own-run-id] +# repository owner/name +# pr-number the pull request number (for the force-push timeline lookup) +# head-sha the head commit SHA +# own-run-id this workflow run's id β€” its own check suite is created by +# the triggering event itself (often AFTER the summary edit +# that triggered it), so as the only suite for a head it would +# poison the floor and wedge arming; it is excluded. +# +# Floor construction (each part raises, never lowers β€” fail-closed): +# 1. earliest check-suite created_at for the SHA (excluding this run's own +# suite) β€” when GitHub first saw the commit; commit metadata alone is NOT +# a safe floor (pushing a previously-created commit object carries an old +# committer date); +# 2. committer-date fallback when no other check suite exists yet (never an +# empty, fail-open floor); +# 3. raised to the newest head_ref_force_pushed time when one exists β€” a +# branch that RETURNS to an earlier SHA re-uses that SHA's original check +# suites, so without this a summary written for an intervening head would +# pass as fresh. + +set -euo pipefail + +if [[ $# -lt 3 || $# -gt 4 ]]; then + echo "usage: $0 [own-run-id]" >&2 + exit 2 +fi + +repository="$1" +pr_number="$2" +head_sha="$3" +own_run_id="${4:-}" + +own_suite_id="" +if [[ -n "$own_run_id" ]]; then + own_suite_id=$(gh api "repos/$repository/actions/runs/$own_run_id" --jq '.check_suite_id // empty') +fi + +floor=$(gh api "repos/$repository/commits/$head_sha/check-suites" --paginate | + jq -rs --arg own "$own_suite_id" '[.[].check_suites[]? | select((.id | tostring) != $own) | .created_at | select(. != null)] | min // empty') +if [[ -z "$floor" ]]; then + floor=$(gh api "repos/$repository/commits/$head_sha" --jq .commit.committer.date) +fi + +last_force_push_at=$(gh api "repos/$repository/issues/$pr_number/timeline" --paginate | + jq -rs '[.[][] | select(.event == "head_ref_force_pushed") | .created_at | select(. != null)] | max // empty') +if [[ -n "$last_force_push_at" && "$last_force_push_at" > "$floor" ]]; then + floor="$last_force_push_at" +fi + +printf '%s\n' "$floor" diff --git a/README.md b/README.md index 8a38f2a2..c4d15d4e 100644 --- a/README.md +++ b/README.md @@ -213,11 +213,13 @@ on: types: [opened, synchronize, reopened, ready_for_review] # Required when enforce-review-gates is true: review results land after # the pull_request events, so the caller must re-invoke the gate on them - # β€” including dismissals, which are non-green and must be able to DISARM. + # β€” including dismissals, which are non-green and must be able to DISARM, + # and deleted comments, since removing a pre-merge summary or Codex clean + # pass is evidence deletion the gate must re-evaluate (and disarm on). pull_request_review: types: [submitted, dismissed] issue_comment: - types: [created, edited] + types: [created, edited, deleted] jobs: auto-merge: From b2b534847f903d94f9c25c1250cd13d2800e3f08 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Sun, 12 Jul 2026 13:08:24 +0200 Subject: [PATCH 11/16] fix: revoke stale arming on red pre-arm, lookup failures, and merge-queue entries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-8 review hardening: - shared disarm-auto-merge.sh revokes BOTH arming shapes β€” the classic autoMergeRequest and a merge-queue entry (dequeuePullRequest), which --disable-auto alone cannot reach on merge-queue repos - a red pre-arm re-check now actively revokes any arming an earlier green run left behind instead of only declining to arm - a failed PR lookup routes through the disarm path (PR number re-derived from the event payload) - the freshness floor never falls back to the commit committer date: a pull_request-event run may use its own push-created suite; otherwise the script fails closed Co-Authored-By: Claude Fable 5 --- .../test-enable-auto-merge-pentad-gate.sh | 21 +++++++++ .github/workflows/enable-auto-merge.yaml | 36 +++++++------- .scripts/compute-head-seen-floor.sh | 31 ++++++++---- .scripts/disarm-auto-merge.sh | 47 +++++++++++++++++++ 4 files changed, 109 insertions(+), 26 deletions(-) create mode 100644 .scripts/disarm-auto-merge.sh diff --git a/.github/tests/test-enable-auto-merge-pentad-gate.sh b/.github/tests/test-enable-auto-merge-pentad-gate.sh index 229fcc81..d8f03ebd 100755 --- a/.github/tests/test-enable-auto-merge-pentad-gate.sh +++ b/.github/tests/test-enable-auto-merge-pentad-gate.sh @@ -85,6 +85,27 @@ if [[ "$(grep -c 'compute-head-seen-floor.sh' "$workflow")" -lt 2 ]]; then status=1 fi +# Commit metadata is never a safe floor: pushing a previously-created commit +# carries an old committer date, and a summary from the PREVIOUS head can +# postdate it. The floor script must fail closed instead of falling back. +if grep -q -- '--jq .commit.committer.date' "$floor_script"; then + echo "::error file=$floor_script::the freshness floor must never fall back to the commit committer date (fail closed instead)" + status=1 +fi + +# Revocation must cover BOTH arming shapes (autoMergeRequest + merge-queue +# entry) and fire from BOTH red paths: the disarm step and the pre-arm +# re-check (an earlier green run may already have armed/enqueued the PR). +disarm_script="${5:-.scripts/disarm-auto-merge.sh}" +if ! grep -q 'dequeuePullRequest' "$disarm_script"; then + echo "::error file=$disarm_script::disarm must dequeue merge-queue entries (dequeuePullRequest), not only --disable-auto" + status=1 +fi +if [[ "$(grep -c 'disarm-auto-merge.sh' "$workflow")" -lt 2 ]]; then + echo "::error file=$workflow::the workflow must revoke via disarm-auto-merge.sh in the disarm step AND in the red pre-arm re-check" + status=1 +fi + while IFS= read -r fixture; do name="$(jq -r '.name' <<<"$fixture")" expect_green="$(jq -r '.expect_green' <<<"$fixture")" diff --git a/.github/workflows/enable-auto-merge.yaml b/.github/workflows/enable-auto-merge.yaml index fa54ddf8..dcad88fa 100644 --- a/.github/workflows/enable-auto-merge.yaml +++ b/.github/workflows/enable-auto-merge.yaml @@ -161,10 +161,6 @@ jobs: permission-checks: read permission-actions: read permission-pull-requests: read - # The committer-date freshness fallback reads - # GET /repos//commits/, which App tokens need Contents - # read for. - permission-contents: read # Fail-closed pentad gate (actions#548): approving and arming are only # allowed once a CURRENT-HEAD green review (CodeRabbit APPROVED or Codex @@ -231,7 +227,7 @@ jobs: # reusing this value β€” a force-push away and back to the same SHA # between gate and arm must raise the floor, not ride the old one. HEAD_SEEN_AT=$(bash .devantler-tech-actions/.scripts/compute-head-seen-floor.sh \ - "$REPOSITORY" "$PR_NUMBER" "$HEAD_SHA" "$GITHUB_RUN_ID") + "$REPOSITORY" "$PR_NUMBER" "$HEAD_SHA" "$GITHUB_RUN_ID" "$EVENT_NAME") gh api "repos/$REPOSITORY/pulls/$PR_NUMBER/reviews" --paginate | jq -s 'add // []' > /tmp/reviews.json gh api "repos/$REPOSITORY/issues/$PR_NUMBER/comments" --paginate | jq -s 'add // []' > /tmp/comments.json @@ -256,20 +252,27 @@ jobs: # FAILED gate-token mint is the same condition again: requesting a # permission the consumer's App installation has not granted errors the # mint step itself, so `steps.gates` never runs β€” unreadable gates, and - # an already-armed PR must still be revoked. + # an already-armed PR must still be revoked. So is a FAILED PR lookup + # (steps.pr errors before exporting `eligible`): the PR number is then + # re-derived from the event payload. Revocation covers BOTH arming + # shapes via .scripts/disarm-auto-merge.sh β€” the classic + # autoMergeRequest AND a merge-queue entry (on merge-queue repos + # `--auto` ENQUEUES and autoMergeRequest reads null, so a red gate must + # dequeue or the queue merges it regardless). - name: πŸ”’ Disarm auto-merge on failed gates if: >- ${{ !cancelled() && - steps.pr.outputs.eligible == 'true' && + (steps.pr.outputs.eligible == 'true' || steps.pr.outcome == 'failure') && (inputs.enforce-review-gates || vars.ENFORCE_MERGE_GATES == 'true') && - (steps.gate-token.outcome == 'failure' || + (steps.pr.outcome == 'failure' || + steps.gate-token.outcome == 'failure' || steps.gates.outcome == 'failure' || (steps.gates.outcome == 'success' && steps.gates.outputs.armable == 'false')) }} env: GH_TOKEN: ${{ steps.app-token.outputs.token }} - PR_NUMBER: ${{ steps.pr.outputs.number }} + PR_NUMBER: ${{ steps.pr.outputs.number || github.event.pull_request.number || github.event.issue.number }} REPOSITORY: ${{ github.repository }} HEAD_SHA: ${{ steps.gates.outputs.head_sha }} run: | @@ -287,11 +290,7 @@ jobs: else echo "::warning::PR #${PR_NUMBER}: gate lookups failed before proving a head; disarming without head binding (fail-closed)." fi - armed=$(gh pr view "$PR_NUMBER" --repo "$REPOSITORY" --json autoMergeRequest --jq '.autoMergeRequest != null') - if [[ "$armed" == "true" ]]; then - gh pr merge "$PR_NUMBER" --disable-auto --repo "$REPOSITORY" - echo "::warning::Auto-merge DISARMED for PR #${PR_NUMBER}: review/pre-merge gates are no longer green at the current head." - fi + bash .devantler-tech-actions/.scripts/disarm-auto-merge.sh "$REPOSITORY" "$PR_NUMBER" - name: βœ… Approve PR if: steps.gates.outputs.armable == 'true' @@ -328,6 +327,7 @@ jobs: REPOSITORY: ${{ github.repository }} HEAD_SHA: ${{ steps.gates.outputs.head_sha }} ENFORCED: ${{ steps.gates.outputs.enforced }} + EVENT_NAME: ${{ github.event_name }} run: | set -euo pipefail REPO_INFO=$(gh api "repos/$REPOSITORY" --jq '.allow_auto_merge') @@ -347,11 +347,15 @@ jobs: # floor would let an intervening head's summary pass as fresh. if [[ "$ENFORCED" == "true" ]]; then HEAD_SEEN_AT=$(GH_TOKEN="$GATE_TOKEN" bash .devantler-tech-actions/.scripts/compute-head-seen-floor.sh \ - "$REPOSITORY" "$PR_NUMBER" "$HEAD_SHA" "$GITHUB_RUN_ID") + "$REPOSITORY" "$PR_NUMBER" "$HEAD_SHA" "$GITHUB_RUN_ID" "$EVENT_NAME") GH_TOKEN="$GATE_TOKEN" gh api "repos/$REPOSITORY/pulls/$PR_NUMBER/reviews" --paginate | jq -s 'add // []' > /tmp/reviews-arm.json GH_TOKEN="$GATE_TOKEN" gh api "repos/$REPOSITORY/issues/$PR_NUMBER/comments" --paginate | jq -s 'add // []' > /tmp/comments-arm.json if ! bash .devantler-tech-actions/.scripts/check-merge-gates.sh "$HEAD_SHA" "$HEAD_SEEN_AT" /tmp/reviews-arm.json /tmp/comments-arm.json; then - echo "::warning::PR #${PR_NUMBER} NOT armed: the review/pre-merge gates turned red between the gate check and arming (fail-closed pre-arm re-check)." + # NOT arming is not enough: an EARLIER green run may already + # have armed or enqueued this PR, and a red pre-arm re-check + # means the gates are red NOW β€” actively revoke, then bail. + bash .devantler-tech-actions/.scripts/disarm-auto-merge.sh "$REPOSITORY" "$PR_NUMBER" + echo "::warning::PR #${PR_NUMBER} NOT armed (and any pending auto-merge revoked): the review/pre-merge gates turned red between the gate check and arming (fail-closed pre-arm re-check)." exit 0 fi fi diff --git a/.scripts/compute-head-seen-floor.sh b/.scripts/compute-head-seen-floor.sh index 3c916a83..7319f908 100644 --- a/.scripts/compute-head-seen-floor.sh +++ b/.scripts/compute-head-seen-floor.sh @@ -5,22 +5,28 @@ # it β€” a branch force-pushed away and back to the same SHA between gate and # arm would otherwise be compared against a stale floor). # -# Usage: compute-head-seen-floor.sh [own-run-id] +# Usage: compute-head-seen-floor.sh [own-run-id] [event-name] # repository owner/name # pr-number the pull request number (for the force-push timeline lookup) # head-sha the head commit SHA # own-run-id this workflow run's id β€” its own check suite is created by # the triggering event itself (often AFTER the summary edit # that triggered it), so as the only suite for a head it would -# poison the floor and wedge arming; it is excluded. +# poison the floor and wedge arming; it is excluded from part 1. +# event-name the triggering event (github.event_name) β€” on a pull_request +# event the run's OWN suite was created by the push itself, so +# it is a safe floor when no other suite exists (part 2). # # Floor construction (each part raises, never lowers β€” fail-closed): # 1. earliest check-suite created_at for the SHA (excluding this run's own # suite) β€” when GitHub first saw the commit; commit metadata alone is NOT # a safe floor (pushing a previously-created commit object carries an old -# committer date); -# 2. committer-date fallback when no other check suite exists yet (never an -# empty, fail-open floor); +# committer date, and a pre-existing summary from the PREVIOUS head can +# postdate it β€” a committer-date floor would let that stale summary pass); +# 2. when no other suite exists yet: on a pull_request-event run the own +# suite's created_at IS push time and is used; on comment/review-driven +# runs there is NO provable head-seen time, so the script FAILS (exit 1) +# and the caller's gate fails closed β€” never a committer-date fallback; # 3. raised to the newest head_ref_force_pushed time when one exists β€” a # branch that RETURNS to an earlier SHA re-uses that SHA's original check # suites, so without this a summary written for an intervening head would @@ -28,8 +34,8 @@ set -euo pipefail -if [[ $# -lt 3 || $# -gt 4 ]]; then - echo "usage: $0 [own-run-id]" >&2 +if [[ $# -lt 3 || $# -gt 5 ]]; then + echo "usage: $0 [own-run-id] [event-name]" >&2 exit 2 fi @@ -37,16 +43,21 @@ repository="$1" pr_number="$2" head_sha="$3" own_run_id="${4:-}" +event_name="${5:-}" own_suite_id="" if [[ -n "$own_run_id" ]]; then own_suite_id=$(gh api "repos/$repository/actions/runs/$own_run_id" --jq '.check_suite_id // empty') fi -floor=$(gh api "repos/$repository/commits/$head_sha/check-suites" --paginate | - jq -rs --arg own "$own_suite_id" '[.[].check_suites[]? | select((.id | tostring) != $own) | .created_at | select(. != null)] | min // empty') +suites_json=$(gh api "repos/$repository/commits/$head_sha/check-suites" --paginate | jq -s '[.[].check_suites[]?]') +floor=$(jq -r --arg own "$own_suite_id" '[.[] | select((.id | tostring) != $own) | .created_at | select(. != null)] | min // empty' <<<"$suites_json") +if [[ -z "$floor" && "$event_name" == "pull_request" && -n "$own_suite_id" ]]; then + floor=$(jq -r --arg own "$own_suite_id" '[.[] | select((.id | tostring) == $own) | .created_at | select(. != null)] | min // empty' <<<"$suites_json") +fi if [[ -z "$floor" ]]; then - floor=$(gh api "repos/$repository/commits/$head_sha" --jq .commit.committer.date) + echo "::error::cannot prove when $head_sha became the PR head (no usable check suite on a ${event_name:-non-pull_request} run) β€” failing closed." >&2 + exit 1 fi last_force_push_at=$(gh api "repos/$repository/issues/$pr_number/timeline" --paginate | diff --git a/.scripts/disarm-auto-merge.sh b/.scripts/disarm-auto-merge.sh new file mode 100644 index 00000000..bbef9e79 --- /dev/null +++ b/.scripts/disarm-auto-merge.sh @@ -0,0 +1,47 @@ +#!/usr/bin/env bash +# Revoke any pending auto-merge for a pull request β€” BOTH arming shapes: +# - a classic autoMergeRequest (disable with `gh pr merge --disable-auto`); +# - a merge-queue entry (on merge-queue repos `--auto` ENQUEUES once +# requirements are met and autoMergeRequest reads null, so a red gate +# must DEQUEUE the entry or it merges from the queue regardless). +# Shared by the disarm step and the pre-arm re-check of the privileged +# auto-merge workflow. Prints what it revoked; exits 0 when nothing was +# pending. Fail-closed callers treat any error as a failed disarm. +# +# Usage: disarm-auto-merge.sh + +set -euo pipefail + +if [[ $# -ne 2 ]]; then + echo "usage: $0 " >&2 + exit 2 +fi + +repository="$1" +pr_number="$2" +owner="${repository%%/*}" +name="${repository#*/}" + +# shellcheck disable=SC2016 # GraphQL $variables, not shell expansion +state="$(gh api graphql \ + -f query='query($owner:String!,$name:String!,$number:Int!){repository(owner:$owner,name:$name){pullRequest(number:$number){id isInMergeQueue autoMergeRequest{enabledAt}}}}' \ + -f owner="$owner" -f name="$name" -F number="$pr_number" \ + --jq '.data.repository.pullRequest | "\(.id) \(.autoMergeRequest != null) \(.isInMergeQueue)"')" +read -r pr_id armed queued <<<"$state" + +if [[ "$armed" == "true" ]]; then + gh pr merge "$pr_number" --disable-auto --repo "$repository" + echo "::warning::Auto-merge DISARMED for PR #${pr_number}: review/pre-merge gates are not green at the current head." +fi + +if [[ "$queued" == "true" ]]; then + # shellcheck disable=SC2016 # GraphQL $variables, not shell expansion + gh api graphql \ + -f query='mutation($id:ID!){dequeuePullRequest(input:{id:$id}){clientMutationId}}' \ + -f id="$pr_id" >/dev/null + echo "::warning::PR #${pr_number} DEQUEUED from the merge queue: review/pre-merge gates are not green at the current head." +fi + +if [[ "$armed" != "true" && "$queued" != "true" ]]; then + echo "PR #${pr_number}: no pending auto-merge or merge-queue entry to revoke." +fi From 3e8d0682566b674b3c14eb3d299645cac92b5dac Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Sun, 12 Jul 2026 14:05:23 +0200 Subject: [PATCH 12/16] fix(auto-merge): fail closed on mutable review state --- .../comments.json | 16 ++ .../reviews.json | 11 ++ .../comments.json | 25 +++ .../reviews.json | 1 + .../comments.json | 16 ++ .../reviews.json | 11 ++ .../comments.json | 9 + .../reviews.json | 19 ++ .../comments.json | 16 ++ .../green-codex-abbreviated-head/reviews.json | 1 + .github/tests/merge-gate-fixtures/index.json | 28 +++ .../comments.json | 16 ++ .../reviews.json | 10 ++ .../premerge-full-unknown-row/comments.json | 16 ++ .../premerge-full-unknown-row/reviews.json | 1 + .../test-enable-auto-merge-pentad-gate.sh | 163 ++++++++++++++++-- .github/workflows/enable-auto-merge.yaml | 150 +++++++++------- .scripts/check-merge-gates.sh | 108 ++++++++---- .scripts/compute-head-seen-floor.sh | 34 ++-- .scripts/disarm-auto-merge.sh | 11 +- 20 files changed, 530 insertions(+), 132 deletions(-) create mode 100644 .github/tests/merge-gate-fixtures/codex-clean-superseded-by-review-findings/comments.json create mode 100644 .github/tests/merge-gate-fixtures/codex-clean-superseded-by-review-findings/reviews.json create mode 100644 .github/tests/merge-gate-fixtures/codex-comment-edited-after-clean/comments.json create mode 100644 .github/tests/merge-gate-fixtures/codex-comment-edited-after-clean/reviews.json create mode 100644 .github/tests/merge-gate-fixtures/codex-review-findings-superseded-by-clean/comments.json create mode 100644 .github/tests/merge-gate-fixtures/codex-review-findings-superseded-by-clean/reviews.json create mode 100644 .github/tests/merge-gate-fixtures/cr-approval-with-codex-review-findings/comments.json create mode 100644 .github/tests/merge-gate-fixtures/cr-approval-with-codex-review-findings/reviews.json create mode 100644 .github/tests/merge-gate-fixtures/green-codex-abbreviated-head/comments.json create mode 100644 .github/tests/merge-gate-fixtures/green-codex-abbreviated-head/reviews.json create mode 100644 .github/tests/merge-gate-fixtures/newest-summary-omits-premerge/comments.json create mode 100644 .github/tests/merge-gate-fixtures/newest-summary-omits-premerge/reviews.json create mode 100644 .github/tests/merge-gate-fixtures/premerge-full-unknown-row/comments.json create mode 100644 .github/tests/merge-gate-fixtures/premerge-full-unknown-row/reviews.json diff --git a/.github/tests/merge-gate-fixtures/codex-clean-superseded-by-review-findings/comments.json b/.github/tests/merge-gate-fixtures/codex-clean-superseded-by-review-findings/comments.json new file mode 100644 index 00000000..d2a96f4d --- /dev/null +++ b/.github/tests/merge-gate-fixtures/codex-clean-superseded-by-review-findings/comments.json @@ -0,0 +1,16 @@ +[ + { + "user": { + "login": "chatgpt-codex-connector[bot]" + }, + "body": "Codex Review: Didn't find any major issues.\n\n**Reviewed commit:** `aaaaaaaaaa`", + "created_at": "2026-07-11T10:00:00Z" + }, + { + "user": { + "login": "coderabbitai[bot]" + }, + "body": "\n\nπŸš₯ Pre-merge checks | βœ… 5\n", + "created_at": "2026-07-11T10:00:00Z" + } +] diff --git a/.github/tests/merge-gate-fixtures/codex-clean-superseded-by-review-findings/reviews.json b/.github/tests/merge-gate-fixtures/codex-clean-superseded-by-review-findings/reviews.json new file mode 100644 index 00000000..fb24abec --- /dev/null +++ b/.github/tests/merge-gate-fixtures/codex-clean-superseded-by-review-findings/reviews.json @@ -0,0 +1,11 @@ +[ + { + "user": { + "login": "chatgpt-codex-connector[bot]" + }, + "state": "COMMENTED", + "commit_id": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "submitted_at": "2026-07-11T11:00:00Z", + "body": "Codex Review: automated suggestions were posted." + } +] diff --git a/.github/tests/merge-gate-fixtures/codex-comment-edited-after-clean/comments.json b/.github/tests/merge-gate-fixtures/codex-comment-edited-after-clean/comments.json new file mode 100644 index 00000000..3e92fa1a --- /dev/null +++ b/.github/tests/merge-gate-fixtures/codex-comment-edited-after-clean/comments.json @@ -0,0 +1,25 @@ +[ + { + "user": { + "login": "chatgpt-codex-connector[bot]" + }, + "body": "Codex Review: Found a blocking issue.\n\n**Reviewed commit:** `aaaaaaaaaa`", + "created_at": "2026-07-11T09:30:00Z", + "updated_at": "2026-07-11T11:00:00Z" + }, + { + "user": { + "login": "chatgpt-codex-connector[bot]" + }, + "body": "Codex Review: Didn't find any major issues.\n\n**Reviewed commit:** `aaaaaaaaaa`", + "created_at": "2026-07-11T10:30:00Z", + "updated_at": "2026-07-11T10:30:00Z" + }, + { + "user": { + "login": "coderabbitai[bot]" + }, + "body": "\n\nπŸš₯ Pre-merge checks | βœ… 5\n", + "created_at": "2026-07-11T10:00:00Z" + } +] diff --git a/.github/tests/merge-gate-fixtures/codex-comment-edited-after-clean/reviews.json b/.github/tests/merge-gate-fixtures/codex-comment-edited-after-clean/reviews.json new file mode 100644 index 00000000..fe51488c --- /dev/null +++ b/.github/tests/merge-gate-fixtures/codex-comment-edited-after-clean/reviews.json @@ -0,0 +1 @@ +[] diff --git a/.github/tests/merge-gate-fixtures/codex-review-findings-superseded-by-clean/comments.json b/.github/tests/merge-gate-fixtures/codex-review-findings-superseded-by-clean/comments.json new file mode 100644 index 00000000..6f09fbe9 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/codex-review-findings-superseded-by-clean/comments.json @@ -0,0 +1,16 @@ +[ + { + "user": { + "login": "chatgpt-codex-connector[bot]" + }, + "body": "Codex Review: Didn't find any major issues.\n\n**Reviewed commit:** `aaaaaaaaaa`", + "created_at": "2026-07-11T11:00:00Z" + }, + { + "user": { + "login": "coderabbitai[bot]" + }, + "body": "\n\nπŸš₯ Pre-merge checks | βœ… 5\n", + "created_at": "2026-07-11T10:00:00Z" + } +] diff --git a/.github/tests/merge-gate-fixtures/codex-review-findings-superseded-by-clean/reviews.json b/.github/tests/merge-gate-fixtures/codex-review-findings-superseded-by-clean/reviews.json new file mode 100644 index 00000000..65510e02 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/codex-review-findings-superseded-by-clean/reviews.json @@ -0,0 +1,11 @@ +[ + { + "user": { + "login": "chatgpt-codex-connector[bot]" + }, + "state": "COMMENTED", + "commit_id": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "submitted_at": "2026-07-11T10:00:00Z", + "body": "Codex Review: automated suggestions were posted." + } +] diff --git a/.github/tests/merge-gate-fixtures/cr-approval-with-codex-review-findings/comments.json b/.github/tests/merge-gate-fixtures/cr-approval-with-codex-review-findings/comments.json new file mode 100644 index 00000000..8702c327 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/cr-approval-with-codex-review-findings/comments.json @@ -0,0 +1,9 @@ +[ + { + "user": { + "login": "coderabbitai[bot]" + }, + "body": "\n\nπŸš₯ Pre-merge checks | βœ… 5\n", + "created_at": "2026-07-11T10:00:00Z" + } +] diff --git a/.github/tests/merge-gate-fixtures/cr-approval-with-codex-review-findings/reviews.json b/.github/tests/merge-gate-fixtures/cr-approval-with-codex-review-findings/reviews.json new file mode 100644 index 00000000..89b13c05 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/cr-approval-with-codex-review-findings/reviews.json @@ -0,0 +1,19 @@ +[ + { + "user": { + "login": "coderabbitai[bot]" + }, + "state": "APPROVED", + "commit_id": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "submitted_at": "2026-07-11T10:00:00Z" + }, + { + "user": { + "login": "chatgpt-codex-connector[bot]" + }, + "state": "COMMENTED", + "commit_id": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "submitted_at": "2026-07-11T11:00:00Z", + "body": "Codex Review: automated suggestions were posted." + } +] diff --git a/.github/tests/merge-gate-fixtures/green-codex-abbreviated-head/comments.json b/.github/tests/merge-gate-fixtures/green-codex-abbreviated-head/comments.json new file mode 100644 index 00000000..d2a96f4d --- /dev/null +++ b/.github/tests/merge-gate-fixtures/green-codex-abbreviated-head/comments.json @@ -0,0 +1,16 @@ +[ + { + "user": { + "login": "chatgpt-codex-connector[bot]" + }, + "body": "Codex Review: Didn't find any major issues.\n\n**Reviewed commit:** `aaaaaaaaaa`", + "created_at": "2026-07-11T10:00:00Z" + }, + { + "user": { + "login": "coderabbitai[bot]" + }, + "body": "\n\nπŸš₯ Pre-merge checks | βœ… 5\n", + "created_at": "2026-07-11T10:00:00Z" + } +] diff --git a/.github/tests/merge-gate-fixtures/green-codex-abbreviated-head/reviews.json b/.github/tests/merge-gate-fixtures/green-codex-abbreviated-head/reviews.json new file mode 100644 index 00000000..fe51488c --- /dev/null +++ b/.github/tests/merge-gate-fixtures/green-codex-abbreviated-head/reviews.json @@ -0,0 +1 @@ +[] diff --git a/.github/tests/merge-gate-fixtures/index.json b/.github/tests/merge-gate-fixtures/index.json index 4413e971..42a1c01e 100644 --- a/.github/tests/merge-gate-fixtures/index.json +++ b/.github/tests/merge-gate-fixtures/index.json @@ -7,6 +7,10 @@ "name": "green-codex-at-head-premerge-full", "expect_green": true }, + { + "name": "green-codex-abbreviated-head", + "expect_green": true + }, { "name": "stale-cr-approval", "expect_green": false @@ -23,6 +27,22 @@ "name": "codex-clean-superseded-by-findings", "expect_green": false }, + { + "name": "codex-clean-superseded-by-review-findings", + "expect_green": false + }, + { + "name": "codex-review-findings-superseded-by-clean", + "expect_green": false + }, + { + "name": "codex-comment-edited-after-clean", + "expect_green": false + }, + { + "name": "cr-approval-with-codex-review-findings", + "expect_green": false + }, { "name": "no-review-at-all", "expect_green": false @@ -39,6 +59,10 @@ "name": "premerge-failed-full", "expect_green": false }, + { + "name": "premerge-full-unknown-row", + "expect_green": false + }, { "name": "premerge-marker-missing", "expect_green": false @@ -47,6 +71,10 @@ "name": "newest-summary-wins", "expect_green": false }, + { + "name": "newest-summary-omits-premerge", + "expect_green": false + }, { "name": "changes-requested-not-approval", "expect_green": false diff --git a/.github/tests/merge-gate-fixtures/newest-summary-omits-premerge/comments.json b/.github/tests/merge-gate-fixtures/newest-summary-omits-premerge/comments.json new file mode 100644 index 00000000..fb04d3ee --- /dev/null +++ b/.github/tests/merge-gate-fixtures/newest-summary-omits-premerge/comments.json @@ -0,0 +1,16 @@ +[ + { + "user": { + "login": "coderabbitai[bot]" + }, + "body": "\n\nπŸš₯ Pre-merge checks | βœ… 5\n", + "created_at": "2026-07-11T10:00:00Z" + }, + { + "user": { + "login": "coderabbitai[bot]" + }, + "body": "\nNo pre-merge evidence was generated for this revision.", + "created_at": "2026-07-11T11:00:00Z" + } +] diff --git a/.github/tests/merge-gate-fixtures/newest-summary-omits-premerge/reviews.json b/.github/tests/merge-gate-fixtures/newest-summary-omits-premerge/reviews.json new file mode 100644 index 00000000..f47d3a38 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/newest-summary-omits-premerge/reviews.json @@ -0,0 +1,10 @@ +[ + { + "user": { + "login": "coderabbitai[bot]" + }, + "state": "APPROVED", + "commit_id": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "submitted_at": "2026-07-11T10:00:00Z" + } +] diff --git a/.github/tests/merge-gate-fixtures/premerge-full-unknown-row/comments.json b/.github/tests/merge-gate-fixtures/premerge-full-unknown-row/comments.json new file mode 100644 index 00000000..3492d164 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/premerge-full-unknown-row/comments.json @@ -0,0 +1,16 @@ +[ + { + "user": { + "login": "chatgpt-codex-connector[bot]" + }, + "body": "Codex Review: Didn't find any major issues.\n\n**Reviewed commit:** `aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa`", + "created_at": "2026-07-11T10:00:00Z" + }, + { + "user": { + "login": "coderabbitai[bot]" + }, + "body": "\n\n## Pre-merge checks\n| Check | Status |\n| --- | --- |\n| Title check | βœ… Passed |\n| Linked Issues check | Pending |\n", + "created_at": "2026-07-11T10:00:00Z" + } +] diff --git a/.github/tests/merge-gate-fixtures/premerge-full-unknown-row/reviews.json b/.github/tests/merge-gate-fixtures/premerge-full-unknown-row/reviews.json new file mode 100644 index 00000000..fe51488c --- /dev/null +++ b/.github/tests/merge-gate-fixtures/premerge-full-unknown-row/reviews.json @@ -0,0 +1 @@ +[] diff --git a/.github/tests/test-enable-auto-merge-pentad-gate.sh b/.github/tests/test-enable-auto-merge-pentad-gate.sh index d8f03ebd..5fca13bd 100755 --- a/.github/tests/test-enable-auto-merge-pentad-gate.sh +++ b/.github/tests/test-enable-auto-merge-pentad-gate.sh @@ -1,6 +1,7 @@ #!/usr/bin/env bash # Table-driven test for the fail-closed review/pre-merge gate the privileged -# auto-merge workflow runs before approving or arming a trusted-bot PR +# auto-merge workflow runs before approving a trusted-bot PR. Enforced mode +# leaves arming to the maintenance agent after its live pentad check # (actions#548). Each fixture is a (reviews.json, comments.json) pair plus the # expected verdict; the gate must treat every missing, stale, mixed, failed, # superseded, or unparseable state as NOT green. @@ -12,17 +13,18 @@ fixtures_dir="${2:-.github/tests/merge-gate-fixtures}" workflow="${3:-.github/workflows/enable-auto-merge.yaml}" head_sha="$(printf 'a%.0s' {1..40})" -# The freshness floor the workflow derives from the head commit's committer -# date: fixture summaries dated on/after it are fresh, earlier ones stale. -head_committed_at="2026-07-11T09:00:00Z" +# The freshness floor supplied by the workflow: fixture summaries dated on or +# after it are fresh; earlier ones are stale. +head_seen_at="2026-07-11T09:00:00Z" status=0 # The workflow must actually consume the gate: the gates step runs the script, -# both privileged steps are conditioned on its armable output, and the arming -# step re-runs the script immediately before `gh pr merge --auto` (an enforced -# green run must not arm past a review that turned red mid-run). -if [[ "$(grep -c 'check-merge-gates.sh' "$workflow")" -lt 2 ]]; then - echo "::error file=$workflow::auto-merge workflow must run check-merge-gates.sh in the gates step AND re-run it pre-arm in the Enable Auto-Merge step" +# and both privileged steps are conditioned on its armable output. Enforced +# runs deliberately do not auto-arm because mutable review evidence cannot be +# bound atomically to `gh pr merge`; the maintenance agent performs that final +# live pentad check. +if [[ "$(grep -c 'check-merge-gates.sh' "$workflow")" -lt 1 ]]; then + echo "::error file=$workflow::auto-merge workflow must run check-merge-gates.sh in the gates step" status=1 fi @@ -62,26 +64,40 @@ if [[ "$disarm_condition" != *"!cancelled()"* || fi # Deleted reviewer evidence (a removed pre-merge summary or Codex clean pass) -# must re-trigger the gate so an enforced arming does not outlive its proof. +# must re-trigger the gate so stale legacy arming is actively revoked. issue_comment_types="$(yq -r '.on.issue_comment.types | join(",")' "$workflow")" if [[ "$issue_comment_types" != *"deleted"* ]]; then echo "::error file=$workflow::issue_comment trigger must include the deleted type (evidence deletion is a disarm path); got: $issue_comment_types" status=1 fi +pull_request_review_types="$(yq -r '.on.pull_request_review.types | join(",")' "$workflow")" +if [[ "$pull_request_review_types" != *"edited"* ]]; then + echo "::error file=$workflow::pull_request_review trigger must include the edited type so changed reviewer evidence re-evaluates the gate; got: $pull_request_review_types" + status=1 +fi + +# Codex findings are submitted as pull-request reviews, not issue comments. +# The job must therefore re-evaluate for both supported reviewer bots; parsing +# Codex review objects is ineffective if their submitted event never runs it. +job_condition="$(yq -r '.jobs."auto-merge".if // ""' "$workflow")" +if [[ "$(grep -oF 'chatgpt-codex-connector[bot]' <<<"$job_condition" | wc -l | tr -d ' ')" -lt 2 ]]; then + echo "::error file=$workflow::pull_request_review runs must include chatgpt-codex-connector[bot] so findings actively disarm" + status=1 +fi + # A branch returning to an earlier SHA re-uses that SHA's original check # suites, so the freshness floor must also consider the newest force-push # time β€” otherwise a summary written for an intervening head passes as fresh. # The floor lives in the shared compute-head-seen-floor.sh, and the workflow -# must consume it BOTH in the gate step and in the pre-arm re-check (reusing -# the gate step's floor would miss a force-push between gate and arm). +# must consume it in the gate step. floor_script="${4:-.scripts/compute-head-seen-floor.sh}" if ! grep -q 'head_ref_force_pushed' "$floor_script"; then echo "::error file=$floor_script::the head-seen freshness floor must be raised by the newest head_ref_force_pushed timeline event" status=1 fi -if [[ "$(grep -c 'compute-head-seen-floor.sh' "$workflow")" -lt 2 ]]; then - echo "::error file=$workflow::the workflow must compute the freshness floor via compute-head-seen-floor.sh in the gate step AND recompute it in the pre-arm re-check" +if [[ "$(grep -c 'compute-head-seen-floor.sh' "$workflow")" -lt 1 ]]; then + echo "::error file=$workflow::the workflow must compute the freshness floor via compute-head-seen-floor.sh in the gate step" status=1 fi @@ -94,15 +110,124 @@ if grep -q -- '--jq .commit.committer.date' "$floor_script"; then fi # Revocation must cover BOTH arming shapes (autoMergeRequest + merge-queue -# entry) and fire from BOTH red paths: the disarm step and the pre-arm -# re-check (an earlier green run may already have armed/enqueued the PR). +# entry) and fire from all three safety points: the red-gate disarm step, +# before an enforced approval (approval itself may satisfy a stale arming), +# and the enforced green handoff after approval. disarm_script="${5:-.scripts/disarm-auto-merge.sh}" if ! grep -q 'dequeuePullRequest' "$disarm_script"; then echo "::error file=$disarm_script::disarm must dequeue merge-queue entries (dequeuePullRequest), not only --disable-auto" status=1 fi -if [[ "$(grep -c 'disarm-auto-merge.sh' "$workflow")" -lt 2 ]]; then - echo "::error file=$workflow::the workflow must revoke via disarm-auto-merge.sh in the disarm step AND in the red pre-arm re-check" +if [[ "$(grep -c 'disarm-auto-merge.sh' "$workflow")" -lt 3 ]]; then + echo "::error file=$workflow::the workflow must revoke in the red-gate step, before enforced approval, and in the enforced green handoff" + status=1 +fi + +# Mutable review/pre-merge evidence can turn red after any final snapshot and +# before `gh pr merge`; GitHub has no atomic merge primitive that binds those +# surfaces. Enforced mode must therefore take the issue's conservative +# fallback: always revoke stale arming and exit before the merge call, leaving +# arming to the maintenance agent's live pentad check. The step must still run +# after an approval failure so it can revoke a previous arming first. +pre_arm_run="$(yq -r ' + [.jobs."auto-merge".steps[] + | select(.name == "πŸ”€ Enable Auto-Merge") + | .run // ""] + | join("\n")' "$workflow")" + +approve_id="$(yq -r ' + [.jobs."auto-merge".steps[] + | select(.name == "βœ… Approve PR") + | .id // ""] + | join("\n")' "$workflow")" +if [[ "$approve_id" != "approve" ]]; then + echo "::error file=$workflow::Approve PR step must expose id=approve so enforced cleanup can observe approval failure" + status=1 +fi + +approve_run="$(yq -r ' + [.jobs."auto-merge".steps[] + | select(.name == "βœ… Approve PR") + | .run // ""] + | join("\n")' "$workflow")" +approve_disarm_line="$(grep -nF 'disarm-auto-merge.sh' <<<"$approve_run" | head -1 | cut -d: -f1 || true)" +# shellcheck disable=SC2016 # Match the literal workflow shell, not this test's variables. +approve_api_line="$(grep -nF 'gh api "repos/$REPOSITORY/pulls/$PR_NUMBER/reviews"' <<<"$approve_run" | head -1 | cut -d: -f1 || true)" +if [[ -z "$approve_disarm_line" || -z "$approve_api_line" || "$approve_disarm_line" -ge "$approve_api_line" ]]; then + echo "::error file=$workflow::enforced approval must revoke stale auto-merge state BEFORE posting the approval" + status=1 +fi + +enable_condition="$(yq -r ' + [.jobs."auto-merge".steps[] + | select(.name == "πŸ”€ Enable Auto-Merge") + | .if // ""] + | join("\n")' "$workflow")" +if [[ "$enable_condition" != *"!cancelled()"* || + "$enable_condition" != *"steps.gates.outputs.armable == 'true'"* ]]; then + echo "::error file=$workflow::Enable Auto-Merge must run after approval failure (!cancelled + armable) so enforced cleanup cannot be skipped" + status=1 +fi + +if ! grep -Fq 'APPROVE_OUTCOME' <<<"$pre_arm_run"; then + echo "::error file=$workflow::Enable Auto-Merge must check the Approve PR outcome after enforced cleanup" + status=1 +fi +if ! grep -Fq 'enforced fallback leaves auto-arming to the maintenance agent' <<<"$pre_arm_run"; then + echo "::error file=$workflow::enforced mode must document the conservative no-auto-arm fallback" + status=1 +fi +# shellcheck disable=SC2016 # Match the literal workflow shell, not this test's variables. +if ! grep -Fq 'bash .devantler-tech-actions/.scripts/disarm-auto-merge.sh "$REPOSITORY" "$PR_NUMBER"' <<<"$pre_arm_run"; then + echo "::error file=$workflow::enforced green runs must revoke any stale auto-merge request before handing off" + status=1 +fi +if grep -Fq 'check-merge-gates.sh' <<<"$pre_arm_run" || grep -Fq 'GATE_TOKEN' <<<"$pre_arm_run"; then + echo "::error file=$workflow::Enable Auto-Merge must not pretend a second mutable evidence snapshot makes auto-arming atomic" + status=1 +fi + +handoff_line="$(grep -nF 'enforced fallback leaves auto-arming to the maintenance agent' <<<"$pre_arm_run" | head -1 | cut -d: -f1 || true)" +handoff_exit_line="$(awk -v start="$handoff_line" 'NR > start && /exit 0/ {print NR; exit}' <<<"$pre_arm_run")" +# shellcheck disable=SC2016 # Match the literal workflow shell, not this test's variables. +merge_line="$(grep -nF 'gh pr merge "$PR_NUMBER" --auto' <<<"$pre_arm_run" | head -1 | cut -d: -f1 || true)" +if [[ -z "$handoff_line" || -z "$handoff_exit_line" || -z "$merge_line" || "$handoff_exit_line" -ge "$merge_line" ]]; then + echo "::error file=$workflow::enforced fallback must exit before the legacy default-off gh pr merge call" + status=1 +fi + +# The head-seen floor must ignore check suites created for some other PR that +# happened to use the same commit SHA. Reusing the oldest cross-branch suite +# makes a stale summary look newer than the PR's adoption of the head. +floor_test_dir="$(mktemp -d)" +trap 'rm -rf "$floor_test_dir"' EXIT +mkdir -p "$floor_test_dir/bin" +cat >"$floor_test_dir/bin/gh" <<'EOF' +#!/usr/bin/env bash +set -euo pipefail +case "${2:-}" in + repos/test/repo/commits/*/check-suites) + cat <<'JSON' +{"check_suites":[ + {"id":1,"created_at":"2026-07-11T08:00:00Z","pull_requests":[{"number":999}]}, + {"id":2,"created_at":"2026-07-11T10:00:00Z","pull_requests":[{"number":42}]} +]} +JSON + ;; + repos/test/repo/issues/42/timeline) + printf '[]\n' + ;; + *) + echo "unexpected gh invocation: $*" >&2 + exit 1 + ;; +esac +EOF +chmod +x "$floor_test_dir/bin/gh" +floor_result="$(PATH="$floor_test_dir/bin:$PATH" bash "$floor_script" \ + test/repo 42 "$head_sha" '' issue_comment)" || status=1 +if [[ "$floor_result" != "2026-07-11T10:00:00Z" ]]; then + echo "::error file=$floor_script::head-seen floor must use this PR's earliest suite; got '$floor_result'" status=1 fi @@ -111,7 +236,7 @@ while IFS= read -r fixture; do expect_green="$(jq -r '.expect_green' <<<"$fixture")" actual_green=false - if bash "$script" "$head_sha" "$head_committed_at" \ + if bash "$script" "$head_sha" "$head_seen_at" \ "$fixtures_dir/$name/reviews.json" \ "$fixtures_dir/$name/comments.json" >/dev/null; then actual_green=true diff --git a/.github/workflows/enable-auto-merge.yaml b/.github/workflows/enable-auto-merge.yaml index dcad88fa..ff7fa243 100644 --- a/.github/workflows/enable-auto-merge.yaml +++ b/.github/workflows/enable-auto-merge.yaml @@ -13,7 +13,10 @@ on: default: false description: >- Opt-in (default-off) enforcement of the fail-closed review/pre-merge - gate before approving and arming. Callers that enable it should also + gate before approving. Enforced runs never auto-arm because mutable + reviewer evidence cannot be bound atomically to GitHub's merge call; + the maintenance agent performs the final live pentad check. Callers + that enable enforcement should also trigger their caller workflow on pull_request_review and issue_comment so review results that land after the pull_request events still re-evaluate the gate (a reusable workflow cannot @@ -25,16 +28,17 @@ on: ################################## # Review results land AFTER the pull_request events above have run, so the # gate re-evaluates when a reviewer bot posts, edits, or deletes its result: - # any CodeRabbit review verdict (approval OR changes-requested β€” the latter - # must be able to DISARM), a DISMISSED approval (dismissal is non-green and - # must also disarm), a Codex result comment, and CodeRabbit's in-place - # pre-merge summary edits. `deleted` is a disarm path: evidence an enforced - # run armed on (a pre-merge summary or Codex clean pass) can be removed - # afterwards, and the gate must re-evaluate the now-missing state. These - # triggers only fire where this workflow file lives; workflow_call consumers - # add them to their caller (see the input). + # any supported reviewer-bot review (CodeRabbit approval/changes-requested, + # or Codex findings β€” every red result must be able to DISARM), a DISMISSED + # approval, a Codex result comment, and CodeRabbit's in-place pre-merge + # summary edits. `deleted` is a disarm path: evidence an enforced approval + # relied on (a pre-merge summary or Codex clean pass) can be removed + # afterwards, and the gate must re-evaluate the now-missing state and revoke + # any stale legacy arming. These triggers only + # fire where this workflow file lives; workflow_call consumers add them to + # their caller (see the input). pull_request_review: - types: [submitted, dismissed] + types: [submitted, edited, dismissed] issue_comment: types: [created, edited, deleted] @@ -53,8 +57,8 @@ jobs: contents: write # Serialize runs per PR so an older green gate run cannot interleave with # a newer run that saw the gate turn red (the newer run's disarm always - # executes after the older run finishes); the pre-arm re-check in the - # πŸ”€ step closes the remaining in-run window. + # executes after the older run finishes). Enforced runs never auto-arm; + # their final step revokes any stale legacy arming before handing off. concurrency: group: enable-auto-merge-${{ github.event.pull_request.number || github.event.issue.number || github.run_id }} cancel-in-progress: false @@ -71,7 +75,10 @@ jobs: ) || ( github.event_name == 'pull_request_review' && - github.event.review.user.login == 'coderabbitai[bot]' && + contains( + fromJSON('["coderabbitai[bot]","chatgpt-codex-connector[bot]"]'), + github.event.review.user.login + ) && !github.event.pull_request.draft && contains( fromJSON('["dependabot[bot]","renovate[bot]","github-actions[bot]","ksail-bot[bot]","coderabbitai[bot]"]'), @@ -162,12 +169,15 @@ jobs: permission-actions: read permission-pull-requests: read - # Fail-closed pentad gate (actions#548): approving and arming are only - # allowed once a CURRENT-HEAD green review (CodeRabbit APPROVED or Codex - # clean pass) AND a green CodeRabbit pre-merge result are both proven. + # Fail-closed pentad gate (actions#548): approval is only allowed once a + # CURRENT-HEAD green review (CodeRabbit APPROVED or Codex clean pass) AND + # a green CodeRabbit pre-merge result are both proven. Enforced mode does + # not auto-arm: those reviewer surfaces can change after any snapshot, + # and GitHub's merge API cannot bind their state atomically to the + # request. The maintenance agent performs the final live pentad check. # Missing, stale, mixed, or unparseable state is NOT green β€” the step - # then skips arming and the maintenance agent arms after its own live - # pentad check. Never weaken this gate to warn-only. + # declines approval and revokes stale arming. The maintenance agent acts + # after its own live pentad check. Never weaken this gate to warn-only. # # Feature-flag-first rollout: enforcement is DEFAULT-OFF (the # enforce-review-gates input for workflow_call callers, or the @@ -196,8 +206,8 @@ jobs: set -euo pipefail HEAD_SHA=$(gh pr view "$PR_NUMBER" --repo "$REPOSITORY" --json headRefOid --jq .headRefOid) - # The proven head is exported so the privileged steps below bind to - # it: a push racing this gate must invalidate the arming, not ride it. + # The proven head is exported so approval is bound to the reviewed + # commit and the legacy default-off arming path cannot ride a push. echo "head_sha=$HEAD_SHA" >> "$GITHUB_OUTPUT" echo "enforced=$ENFORCE" >> "$GITHUB_OUTPUT" @@ -219,13 +229,11 @@ jobs: # Freshness floor for the pre-merge summary β€” the shared # .scripts/compute-head-seen-floor.sh: earliest check-suite time for - # the SHA (excluding this run's own suite, which would poison the - # floor), committer-date fallback, raised to the newest force-push - # time (a branch returning to an earlier SHA re-uses its original - # suites). Rationale for each part lives in the script header. The - # pre-arm re-check RECOMPUTES it with the same script instead of - # reusing this value β€” a force-push away and back to the same SHA - # between gate and arm must raise the floor, not ride the old one. + # the SHA associated with THIS PR (excluding this run's own suite, + # which would poison the floor), failing closed when no such time is + # provable, and raised to the newest force-push time (a branch + # returning to an earlier SHA re-uses its original suites). Rationale + # for each part lives in the script header. HEAD_SEEN_AT=$(bash .devantler-tech-actions/.scripts/compute-head-seen-floor.sh \ "$REPOSITORY" "$PR_NUMBER" "$HEAD_SHA" "$GITHUB_RUN_ID" "$EVENT_NAME") @@ -276,11 +284,13 @@ jobs: REPOSITORY: ${{ github.repository }} HEAD_SHA: ${{ steps.gates.outputs.head_sha }} run: | - # Head-bound like the approve/arm steps: a failed gate for an older - # head must not revoke arming that a newer, green head just earned. + # Head-bound like the approve/default-off arm steps: a failed gate + # for an older head must not revoke state that a newer head's own run + # already evaluated. # A gate step that died before proving a head leaves HEAD_SHA empty # β€” then disarm WITHOUT the head binding (fail-closed: unreadable - # gates must revoke arming; a later green run re-arms). + # gates must revoke arming; a later live maintenance check decides + # whether to arm again). if [[ -n "$HEAD_SHA" ]]; then CURRENT_HEAD=$(gh pr view "$PR_NUMBER" --repo "$REPOSITORY" --json headRefOid --jq .headRefOid) if [[ "$CURRENT_HEAD" != "$HEAD_SHA" ]]; then @@ -293,13 +303,28 @@ jobs: bash .devantler-tech-actions/.scripts/disarm-auto-merge.sh "$REPOSITORY" "$PR_NUMBER" - name: βœ… Approve PR + id: approve if: steps.gates.outputs.armable == 'true' env: GH_TOKEN: ${{ steps.app-token.outputs.token }} PR_NUMBER: ${{ steps.pr.outputs.number }} REPOSITORY: ${{ github.repository }} HEAD_SHA: ${{ steps.gates.outputs.head_sha }} + ENFORCED: ${{ steps.gates.outputs.enforced }} run: | + set -euo pipefail + + # Approval can itself satisfy the last protection on an auto-merge + # request left by an earlier default-off run. Revoke that stale state + # BEFORE approving so enforced mode cannot merge in the gap between + # approval and the final handoff cleanup. + if [[ "$ENFORCED" == "true" ]]; then + if ! bash .devantler-tech-actions/.scripts/disarm-auto-merge.sh "$REPOSITORY" "$PR_NUMBER"; then + echo "::error::PR #${PR_NUMBER}: stale auto-merge state could not be revoked before approval." + exit 1 + fi + fi + set +e # commit_id pins the approval to the gate-proven head: a commit # pushed after the gate ran gets no approval from this run. @@ -319,48 +344,53 @@ jobs: fi - name: πŸ”€ Enable Auto-Merge - if: steps.gates.outputs.armable == 'true' + if: >- + ${{ + !cancelled() && + steps.gates.outputs.armable == 'true' + }} env: GH_TOKEN: ${{ steps.app-token.outputs.token }} - GATE_TOKEN: ${{ steps.gate-token.outputs.token }} PR_NUMBER: ${{ steps.pr.outputs.number }} REPOSITORY: ${{ github.repository }} HEAD_SHA: ${{ steps.gates.outputs.head_sha }} ENFORCED: ${{ steps.gates.outputs.enforced }} - EVENT_NAME: ${{ github.event_name }} + APPROVE_OUTCOME: ${{ steps.approve.outcome }} run: | set -euo pipefail + + # Enforced fallback: review and pre-merge evidence is mutable after + # every snapshot, while GitHub's merge API can bind only the head + # SHA. There is therefore no atomic auto-arm operation that proves + # the full pentad remained green. Always revoke any stale legacy + # autoMergeRequest/queue entry and leave auto-arming to the + # maintenance agent after its live pentad check (actions#548). + if [[ "$ENFORCED" == "true" ]]; then + if ! bash .devantler-tech-actions/.scripts/disarm-auto-merge.sh "$REPOSITORY" "$PR_NUMBER"; then + echo "::error::PR #${PR_NUMBER}: enforced gates were green, but fail-closed revocation of stale auto-merge state failed." + exit 1 + fi + if [[ "$APPROVE_OUTCOME" != "success" ]]; then + echo "::error::PR #${PR_NUMBER}: approval failed; stale auto-merge state was revoked." + exit 1 + fi + echo "::notice::PR #${PR_NUMBER}: enforced fallback leaves auto-arming to the maintenance agent after its live pentad check; stale auto-merge state was revoked." + exit 0 + fi + + # Preserve default-off behavior: an unexpected approval failure is + # a hard stop and must never fall through to the legacy arming path. + if [[ "$APPROVE_OUTCOME" != "success" ]]; then + echo "::error::PR #${PR_NUMBER}: approval failed; auto-merge was not enabled." + exit 1 + fi + REPO_INFO=$(gh api "repos/$REPOSITORY" --jq '.allow_auto_merge') if [[ "$REPO_INFO" != "true" ]]; then echo "::warning::Auto-merge is not enabled on this repository. Contact a repository admin to enable it in Settings > Pull Requests > Allow auto-merge." exit 0 fi - # Re-validate the gates IMMEDIATELY before arming: --match-head-commit - # only pins the SHA, so a review that turned red while this green run - # was still executing would otherwise be armed past. Per-PR - # concurrency serializes whole runs; this re-check closes the - # remaining in-run window. Fail-closed: a red or unreadable re-check - # skips arming (the later red event's own run also disarms). - # The freshness floor is RECOMPUTED here, not reused from the gate - # step: a branch force-pushed away and back to the same SHA between - # gate and arm keeps --match-head-commit satisfied while the old - # floor would let an intervening head's summary pass as fresh. - if [[ "$ENFORCED" == "true" ]]; then - HEAD_SEEN_AT=$(GH_TOKEN="$GATE_TOKEN" bash .devantler-tech-actions/.scripts/compute-head-seen-floor.sh \ - "$REPOSITORY" "$PR_NUMBER" "$HEAD_SHA" "$GITHUB_RUN_ID" "$EVENT_NAME") - GH_TOKEN="$GATE_TOKEN" gh api "repos/$REPOSITORY/pulls/$PR_NUMBER/reviews" --paginate | jq -s 'add // []' > /tmp/reviews-arm.json - GH_TOKEN="$GATE_TOKEN" gh api "repos/$REPOSITORY/issues/$PR_NUMBER/comments" --paginate | jq -s 'add // []' > /tmp/comments-arm.json - if ! bash .devantler-tech-actions/.scripts/check-merge-gates.sh "$HEAD_SHA" "$HEAD_SEEN_AT" /tmp/reviews-arm.json /tmp/comments-arm.json; then - # NOT arming is not enough: an EARLIER green run may already - # have armed or enqueued this PR, and a red pre-arm re-check - # means the gates are red NOW β€” actively revoke, then bail. - bash .devantler-tech-actions/.scripts/disarm-auto-merge.sh "$REPOSITORY" "$PR_NUMBER" - echo "::warning::PR #${PR_NUMBER} NOT armed (and any pending auto-merge revoked): the review/pre-merge gates turned red between the gate check and arming (fail-closed pre-arm re-check)." - exit 0 - fi - fi - # --match-head-commit binds the arming to the gate-proven head: a - # push racing this workflow makes the merge call fail instead of - # arming an unreviewed commit. + # --match-head-commit preserves the legacy default-off behavior + # while binding its arming request to the allowlist-proven head. gh pr merge "$PR_NUMBER" --auto --squash --repo "$REPOSITORY" --match-head-commit "$HEAD_SHA" echo "βœ… Auto-merge enabled for PR #${PR_NUMBER} at ${HEAD_SHA} using squash method" diff --git a/.scripts/check-merge-gates.sh b/.scripts/check-merge-gates.sh index cf8ca95c..539475b1 100755 --- a/.scripts/check-merge-gates.sh +++ b/.scripts/check-merge-gates.sh @@ -6,12 +6,12 @@ # head-seen-at ISO8601 time GitHub last saw the head BECOME the head # (the caller passes the earliest check-suite created_at # for the SHA β€” raised to the newest force-push time when -# the branch later returned to an earlier SHA β€” falling -# back to the committer date) β€” the freshness floor for the -# pre-merge summary, which CodeRabbit edits in place and -# which carries no commit SHA of its own. Commit metadata -# alone is NOT a safe floor: pushing a previously-created -# commit object carries an old committer date. +# the branch later returned to an earlier SHA). This is +# the freshness floor for the pre-merge summary, which +# CodeRabbit edits in place and which carries no commit +# SHA of its own. Commit metadata alone is NOT a safe +# floor: pushing a previously-created commit object +# carries an old committer date. # reviews-json file holding the FULL paginated `pulls//reviews` array # comments-json file holding the FULL paginated `issues//comments` array # @@ -20,23 +20,26 @@ # APPROVED (an earlier approval superseded by CHANGES_REQUESTED or a # dismissal is NOT green, and a COMMENTED review at the head that is not # explicitly clean supersedes an approval and blocks the Codex -# fallback), or β€” when -# CodeRabbit has no verdict at the head β€” a Codex clean pass ("Didn't -# find any major issues") whose "Reviewed commit" equals the head and is -# Codex's LATEST result for it; +# fallback), or β€” when CodeRabbit has no blocking verdict at the head and +# no current-head Codex findings review exists β€” the latest Codex result +# comment is a clean pass ("Didn't find any major issues") whose +# "Reviewed commit" equals the head; # 2. a green CodeRabbit pre-merge result β€” the most recently UPDATED # auto-generated summary (stable marker required; CodeRabbit edits the -# summary in place, so created_at alone selects a stale revision) whose -# pre-merge section is unambiguously green: a positive check-mark count -# and no error/inconclusive/warning marks in either shape, and whose -# update time is not older than the head-seen floor (a summary last -# touched before GitHub saw the head can only describe an earlier state). +# summary in place, so created_at alone selects a stale revision). That +# newest summary itself must carry an unambiguously green pre-merge +# section: a positive check-mark count and no error/inconclusive/warning +# marks in either shape, and its update time must not be older than the +# head-seen floor (a summary last touched before GitHub saw the head can +# only describe an earlier state). A newer summary with no pre-merge +# section supersedes an older green one and fails closed. # When walkthrough boundary markers exist, only the bounded region is # parsed so echoed marker text elsewhere cannot spoof it. # # Everything else β€” missing, stale, mixed, unparseable, or absent state β€” is -# NOT green and exits 1 (the workflow then skips arming; the maintenance -# agent arms after its own live pentad check). Never weaken this to warn-only. +# NOT green and exits 1 (the workflow declines approval and revokes stale +# arming; the maintenance agent acts after its own live pentad check). Never +# weaken this to warn-only. set -euo pipefail @@ -108,22 +111,43 @@ if [[ "$cr_commented_probe" == present* && review_state="needs-fix" fi -if [[ "$review_state" == "missing" || "$review_state" == "stale" ]]; then - # Codex lane: its result is an ISSUE COMMENT carrying "**Reviewed commit:** - # ". Only the LATEST Codex result for the current head counts β€” an - # older clean pass superseded by a findings run must not read as green. - latest_codex_at_head="$(jq -r --arg sha "$head_sha" ' - [.[] - | select(.user.login == "chatgpt-codex-connector[bot]") - | select(.body | contains($sha))] - | sort_by(.created_at) | last | .body // empty' "$comments_json")" +# Codex lane: clean results are ISSUE COMMENTS carrying "**Reviewed commit:** +# " (often abbreviated), while findings arrive as review objects at the +# exact head. Any current-head findings review is red evidence even when a +# later clean comment exists: the REST review snapshot exposes submitted_at but +# not an edit timestamp, so allowing a comment to supersede it could miss an +# older review edited red later. This conservative result can only withhold the +# workflow's approval; the maintenance agent still evaluates the live pentad. +codex_findings_at_head="$(jq -r --arg sha "$head_sha" ' + [.[] + | select(.user.login == "chatgpt-codex-connector[bot]") + | select((.commit_id // "" | ascii_downcase) == ($sha | ascii_downcase)) + | select(.state == "COMMENTED" or .state == "CHANGES_REQUESTED")] + | length' "$reviews_json")" - if [[ -n "$latest_codex_at_head" ]]; then - if [[ "$latest_codex_at_head" == *"Didn't find any major issues"* ]]; then +latest_codex_comment_probe="$(jq -r --arg sha "$head_sha" ' + ($sha | ascii_downcase) as $head + | [.[] + | select(.user.login == "chatgpt-codex-connector[bot]") + | (.body // "") as $body + | (try ($body + | capture("\\*\\*Reviewed commit:\\*\\*[[:space:]]*`?(?[0-9a-fA-F]{7,40})") + | .sha | ascii_downcase) catch "") as $reviewed + | select($reviewed != "" and ($head | startswith($reviewed))) + | {at: (.updated_at // .created_at), body: $body}] + | sort_by(.at) | last + | if . == null then "absent" else "present\n" + .body end' "$comments_json")" + +if [[ "$codex_findings_at_head" -gt 0 ]]; then + review_state="needs-fix" +elif [[ "$latest_codex_comment_probe" == present* ]]; then + latest_codex_body="${latest_codex_comment_probe#*$'\n'}" + if [[ "$latest_codex_body" == *"Didn't find any major issues"* ]]; then + if [[ "$review_state" == "missing" || "$review_state" == "stale" ]]; then review_state="green" - else - review_state="needs-fix" fi + else + review_state="needs-fix" fi fi @@ -138,9 +162,7 @@ summary_marker='\n\nπŸš₯ Pre-merge checks | βœ… 5\n", + "created_at": "2026-07-11T10:00:00Z" + } +] diff --git a/.github/tests/merge-gate-fixtures/codex-abbreviated-head-unresolvable/reviews.json b/.github/tests/merge-gate-fixtures/codex-abbreviated-head-unresolvable/reviews.json new file mode 100644 index 00000000..fe51488c --- /dev/null +++ b/.github/tests/merge-gate-fixtures/codex-abbreviated-head-unresolvable/reviews.json @@ -0,0 +1 @@ +[] diff --git a/.github/tests/merge-gate-fixtures/index.json b/.github/tests/merge-gate-fixtures/index.json index 42a1c01e..15d74d13 100644 --- a/.github/tests/merge-gate-fixtures/index.json +++ b/.github/tests/merge-gate-fixtures/index.json @@ -11,6 +11,10 @@ "name": "green-codex-abbreviated-head", "expect_green": true }, + { + "name": "codex-abbreviated-head-unresolvable", + "expect_green": false + }, { "name": "stale-cr-approval", "expect_green": false diff --git a/.github/tests/test-enable-auto-merge-pentad-gate.sh b/.github/tests/test-enable-auto-merge-pentad-gate.sh index 542127d1..bb10b03a 100755 --- a/.github/tests/test-enable-auto-merge-pentad-gate.sh +++ b/.github/tests/test-enable-auto-merge-pentad-gate.sh @@ -39,6 +39,16 @@ if [[ "$job_permissions" != "contents,pull-requests" ]]; then status=1 fi +gate_contents_permission="$(yq -r ' + [.jobs."auto-merge".steps[] + | select(.id == "gate-token") + | .with."permission-contents" // ""] + | join("\n")' "$workflow")" +if [[ "$gate_contents_permission" != "read" ]]; then + echo "::error file=$workflow::the enforced gate token needs Contents: read to resolve abbreviated Codex commit IDs uniquely" + status=1 +fi + armable_conditions="$(yq -r ' [.jobs."auto-merge".steps[] | select(.name == "βœ… Approve PR" or .name == "πŸ”€ Enable Auto-Merge") @@ -232,6 +242,13 @@ JSON repos/test/repo/issues/42/timeline) printf '[]\n' ;; + repos/test/repo/commits/aaaaaaaaaa) + if [[ "${MOCK_UNRESOLVABLE_PREFIX:-false}" == "true" ]]; then + echo "ambiguous commit prefix" >&2 + exit 1 + fi + printf '%s\n' "$(printf 'a%.0s' {1..40})" + ;; *) echo "unexpected gh invocation: $*" >&2 exit 1 @@ -251,7 +268,12 @@ while IFS= read -r fixture; do expect_green="$(jq -r '.expect_green' <<<"$fixture")" actual_green=false - if bash "$script" "$head_sha" "$head_seen_at" \ + mock_unresolvable_prefix=false + if [[ "$name" == "codex-abbreviated-head-unresolvable" ]]; then + mock_unresolvable_prefix=true + fi + if REPOSITORY=test/repo MOCK_UNRESOLVABLE_PREFIX="$mock_unresolvable_prefix" \ + PATH="$floor_test_dir/bin:$PATH" bash "$script" "$head_sha" "$head_seen_at" \ "$fixtures_dir/$name/reviews.json" \ "$fixtures_dir/$name/comments.json" >/dev/null; then actual_green=true diff --git a/.github/workflows/enable-auto-merge.yaml b/.github/workflows/enable-auto-merge.yaml index ff7fa243..5bf9af9f 100644 --- a/.github/workflows/enable-auto-merge.yaml +++ b/.github/workflows/enable-auto-merge.yaml @@ -167,6 +167,7 @@ jobs: private-key: ${{ secrets.APP_PRIVATE_KEY }} permission-checks: read permission-actions: read + permission-contents: read permission-pull-requests: read # Fail-closed pentad gate (actions#548): approval is only allowed once a diff --git a/.scripts/check-merge-gates.sh b/.scripts/check-merge-gates.sh index 539475b1..4d1e5522 100755 --- a/.scripts/check-merge-gates.sh +++ b/.scripts/check-merge-gates.sh @@ -118,6 +118,8 @@ fi # not an edit timestamp, so allowing a comment to supersede it could miss an # older review edited red later. This conservative result can only withhold the # workflow's approval; the maintenance agent still evaluates the live pentad. +# An abbreviated clean SHA is accepted only when GitHub's commit endpoint +# resolves it uniquely to the exact head; raw prefix matching is not proof. codex_findings_at_head="$(jq -r --arg sha "$head_sha" ' [.[] | select(.user.login == "chatgpt-codex-connector[bot]") @@ -134,19 +136,37 @@ latest_codex_comment_probe="$(jq -r --arg sha "$head_sha" ' | capture("\\*\\*Reviewed commit:\\*\\*[[:space:]]*`?(?[0-9a-fA-F]{7,40})") | .sha | ascii_downcase) catch "") as $reviewed | select($reviewed != "" and ($head | startswith($reviewed))) - | {at: (.updated_at // .created_at), body: $body}] + | {at: (.updated_at // .created_at), reviewed: $reviewed, body: $body}] | sort_by(.at) | last - | if . == null then "absent" else "present\n" + .body end' "$comments_json")" + | if . == null then "absent" + else "present\n" + .reviewed + "\n" + .body + end' "$comments_json")" if [[ "$codex_findings_at_head" -gt 0 ]]; then review_state="needs-fix" elif [[ "$latest_codex_comment_probe" == present* ]]; then - latest_codex_body="${latest_codex_comment_probe#*$'\n'}" - if [[ "$latest_codex_body" == *"Didn't find any major issues"* ]]; then + latest_codex_payload="${latest_codex_comment_probe#*$'\n'}" + latest_codex_reviewed="${latest_codex_payload%%$'\n'*}" + latest_codex_body="${latest_codex_payload#*$'\n'}" + codex_comment_matches_head=false + + if [[ ${#latest_codex_reviewed} -eq 40 && "$latest_codex_reviewed" == "$head_sha" ]]; then + codex_comment_matches_head=true + elif [[ ${#latest_codex_reviewed} -lt 40 && -n "${REPOSITORY:-}" ]]; then + resolved_codex_sha="" + if resolved_codex_sha=$(gh api \ + "repos/$REPOSITORY/commits/$latest_codex_reviewed" --jq .sha 2>/dev/null) && + [[ "$resolved_codex_sha" == "$head_sha" ]]; then + codex_comment_matches_head=true + fi + fi + + if [[ "$codex_comment_matches_head" == "true" && + "$latest_codex_body" == *"Didn't find any major issues"* ]]; then if [[ "$review_state" == "missing" || "$review_state" == "stale" ]]; then review_state="green" fi - else + elif [[ "$codex_comment_matches_head" == "true" ]]; then review_state="needs-fix" fi fi From 2a4c8c3dcd13b103ee0f7e357f3b618ff1a5a915 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Sun, 12 Jul 2026 15:12:13 +0200 Subject: [PATCH 15/16] fix(auto-merge): reject rate-limited summaries --- .github/tests/merge-gate-fixtures/index.json | 4 ++++ .../comments.json | 17 +++++++++++++++++ .../reviews.json | 1 + .scripts/check-merge-gates.sh | 9 ++++++++- 4 files changed, 30 insertions(+), 1 deletion(-) create mode 100644 .github/tests/merge-gate-fixtures/premerge-rate-limited-retains-green/comments.json create mode 100644 .github/tests/merge-gate-fixtures/premerge-rate-limited-retains-green/reviews.json diff --git a/.github/tests/merge-gate-fixtures/index.json b/.github/tests/merge-gate-fixtures/index.json index 15d74d13..169f1f7e 100644 --- a/.github/tests/merge-gate-fixtures/index.json +++ b/.github/tests/merge-gate-fixtures/index.json @@ -59,6 +59,10 @@ "name": "premerge-mixed-compact", "expect_green": false }, + { + "name": "premerge-rate-limited-retains-green", + "expect_green": false + }, { "name": "premerge-failed-full", "expect_green": false diff --git a/.github/tests/merge-gate-fixtures/premerge-rate-limited-retains-green/comments.json b/.github/tests/merge-gate-fixtures/premerge-rate-limited-retains-green/comments.json new file mode 100644 index 00000000..26ead379 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/premerge-rate-limited-retains-green/comments.json @@ -0,0 +1,17 @@ +[ + { + "user": { + "login": "chatgpt-codex-connector[bot]" + }, + "body": "Codex Review: Didn't find any major issues.\n\n**Reviewed commit:** `aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa`", + "created_at": "2026-07-11T10:00:00Z" + }, + { + "user": { + "login": "coderabbitai[bot]" + }, + "body": "\n\n## Review limit reached\n\nπŸš₯ Pre-merge checks | βœ… 5\n\n", + "created_at": "2026-07-11T09:00:00Z", + "updated_at": "2026-07-11T11:00:00Z" + } +] diff --git a/.github/tests/merge-gate-fixtures/premerge-rate-limited-retains-green/reviews.json b/.github/tests/merge-gate-fixtures/premerge-rate-limited-retains-green/reviews.json new file mode 100644 index 00000000..fe51488c --- /dev/null +++ b/.github/tests/merge-gate-fixtures/premerge-rate-limited-retains-green/reviews.json @@ -0,0 +1 @@ +[] diff --git a/.scripts/check-merge-gates.sh b/.scripts/check-merge-gates.sh index 4d1e5522..cb87c6e8 100755 --- a/.scripts/check-merge-gates.sh +++ b/.scripts/check-merge-gates.sh @@ -32,7 +32,8 @@ # marks in either shape, and its update time must not be older than the # head-seen floor (a summary last touched before GitHub saw the head can # only describe an earlier state). A newer summary with no pre-merge -# section supersedes an older green one and fails closed. +# section β€” or a rate-limit overlay that retained an older green section β€” +# supersedes the older result and fails closed. # When walkthrough boundary markers exist, only the bounded region is # parsed so echoed marker text elsewhere cannot spoof it. # @@ -196,6 +197,12 @@ if [[ -n "$premerge_body" ]]; then # describe an earlier state. ISO8601 Zulu timestamps compare lexically. if [[ -n "$head_seen_at" && "$premerge_touched_at" < "$head_seen_at" ]]; then premerge_state="stale" + elif [[ "$premerge_body" == *''* || + "$premerge_body" == *"## Review limit reached"* ]]; then + # CodeRabbit edits the existing summary when rate-limited and can retain + # the previous run's compact `βœ… 5` section underneath the warning. That + # stale display is not a current evaluation and must never become green. + premerge_state="inconclusive" else region="$premerge_body" if [[ "$premerge_body" == *''* && From 9584038d8c135076db7020cd68e890c59d35a894 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Sun, 12 Jul 2026 15:24:08 +0200 Subject: [PATCH 16/16] fix(auto-merge): order edited review evidence --- .../comments.json | 23 +++++++++++ .../reviews.json | 1 + .../comments.json | 9 +++++ .../reviews.json | 22 +++++++++++ .../comments.json | 9 +++++ .../reviews.json | 22 +++++++++++ .../comments.json | 9 +++++ .../reviews.json | 20 ++++++++++ .github/tests/merge-gate-fixtures/index.json | 16 ++++++++ .../test-enable-auto-merge-pentad-gate.sh | 10 +++++ .github/workflows/enable-auto-merge.yaml | 31 ++++++++++++++- .scripts/check-merge-gates.sh | 38 ++++++++++--------- 12 files changed, 192 insertions(+), 18 deletions(-) create mode 100644 .github/tests/merge-gate-fixtures/codex-newest-malformed-supersedes-clean/comments.json create mode 100644 .github/tests/merge-gate-fixtures/codex-newest-malformed-supersedes-clean/reviews.json create mode 100644 .github/tests/merge-gate-fixtures/cr-commented-edited-after-approval/comments.json create mode 100644 .github/tests/merge-gate-fixtures/cr-commented-edited-after-approval/reviews.json create mode 100644 .github/tests/merge-gate-fixtures/cr-commented-edited-before-submit/comments.json create mode 100644 .github/tests/merge-gate-fixtures/cr-commented-edited-before-submit/reviews.json create mode 100644 .github/tests/merge-gate-fixtures/cr-commented-findings-then-approval/comments.json create mode 100644 .github/tests/merge-gate-fixtures/cr-commented-findings-then-approval/reviews.json diff --git a/.github/tests/merge-gate-fixtures/codex-newest-malformed-supersedes-clean/comments.json b/.github/tests/merge-gate-fixtures/codex-newest-malformed-supersedes-clean/comments.json new file mode 100644 index 00000000..bc3572e2 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/codex-newest-malformed-supersedes-clean/comments.json @@ -0,0 +1,23 @@ +[ + { + "user": { + "login": "chatgpt-codex-connector[bot]" + }, + "body": "Codex Review: Didn't find any major issues.\n\n**Reviewed commit:** `aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa`", + "created_at": "2026-07-11T10:00:00Z" + }, + { + "user": { + "login": "chatgpt-codex-connector[bot]" + }, + "body": "Codex Review: result could not be parsed", + "created_at": "2026-07-11T11:00:00Z" + }, + { + "user": { + "login": "coderabbitai[bot]" + }, + "body": "\n\nπŸš₯ Pre-merge checks | βœ… 5\n", + "created_at": "2026-07-11T10:00:00Z" + } +] diff --git a/.github/tests/merge-gate-fixtures/codex-newest-malformed-supersedes-clean/reviews.json b/.github/tests/merge-gate-fixtures/codex-newest-malformed-supersedes-clean/reviews.json new file mode 100644 index 00000000..fe51488c --- /dev/null +++ b/.github/tests/merge-gate-fixtures/codex-newest-malformed-supersedes-clean/reviews.json @@ -0,0 +1 @@ +[] diff --git a/.github/tests/merge-gate-fixtures/cr-commented-edited-after-approval/comments.json b/.github/tests/merge-gate-fixtures/cr-commented-edited-after-approval/comments.json new file mode 100644 index 00000000..72d1bdf8 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/cr-commented-edited-after-approval/comments.json @@ -0,0 +1,9 @@ +[ + { + "user": { + "login": "coderabbitai[bot]" + }, + "body": "\n\nπŸš₯ Pre-merge checks | βœ… 5\n", + "created_at": "2026-07-11T11:00:00Z" + } +] diff --git a/.github/tests/merge-gate-fixtures/cr-commented-edited-after-approval/reviews.json b/.github/tests/merge-gate-fixtures/cr-commented-edited-after-approval/reviews.json new file mode 100644 index 00000000..a3211b02 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/cr-commented-edited-after-approval/reviews.json @@ -0,0 +1,22 @@ +[ + { + "user": { + "login": "coderabbitai[bot]" + }, + "state": "COMMENTED", + "commit_id": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "submitted_at": "2026-07-11T10:00:00Z", + "last_edited_at": "2026-07-11T12:00:00Z", + "body": "Actionable comments posted: 1" + }, + { + "user": { + "login": "coderabbitai[bot]" + }, + "state": "APPROVED", + "commit_id": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "submitted_at": "2026-07-11T11:00:00Z", + "last_edited_at": null, + "body": "" + } +] diff --git a/.github/tests/merge-gate-fixtures/cr-commented-edited-before-submit/comments.json b/.github/tests/merge-gate-fixtures/cr-commented-edited-before-submit/comments.json new file mode 100644 index 00000000..72d1bdf8 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/cr-commented-edited-before-submit/comments.json @@ -0,0 +1,9 @@ +[ + { + "user": { + "login": "coderabbitai[bot]" + }, + "body": "\n\nπŸš₯ Pre-merge checks | βœ… 5\n", + "created_at": "2026-07-11T11:00:00Z" + } +] diff --git a/.github/tests/merge-gate-fixtures/cr-commented-edited-before-submit/reviews.json b/.github/tests/merge-gate-fixtures/cr-commented-edited-before-submit/reviews.json new file mode 100644 index 00000000..5fc9e6b2 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/cr-commented-edited-before-submit/reviews.json @@ -0,0 +1,22 @@ +[ + { + "user": { + "login": "coderabbitai[bot]" + }, + "state": "APPROVED", + "commit_id": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "submitted_at": "2026-07-11T10:30:00Z", + "last_edited_at": null, + "body": "" + }, + { + "user": { + "login": "coderabbitai[bot]" + }, + "state": "COMMENTED", + "commit_id": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "submitted_at": "2026-07-11T11:00:00Z", + "last_edited_at": "2026-07-11T10:00:00Z", + "body": "Actionable comments posted: 1" + } +] diff --git a/.github/tests/merge-gate-fixtures/cr-commented-findings-then-approval/comments.json b/.github/tests/merge-gate-fixtures/cr-commented-findings-then-approval/comments.json new file mode 100644 index 00000000..72d1bdf8 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/cr-commented-findings-then-approval/comments.json @@ -0,0 +1,9 @@ +[ + { + "user": { + "login": "coderabbitai[bot]" + }, + "body": "\n\nπŸš₯ Pre-merge checks | βœ… 5\n", + "created_at": "2026-07-11T11:00:00Z" + } +] diff --git a/.github/tests/merge-gate-fixtures/cr-commented-findings-then-approval/reviews.json b/.github/tests/merge-gate-fixtures/cr-commented-findings-then-approval/reviews.json new file mode 100644 index 00000000..952f69c3 --- /dev/null +++ b/.github/tests/merge-gate-fixtures/cr-commented-findings-then-approval/reviews.json @@ -0,0 +1,20 @@ +[ + { + "user": { + "login": "coderabbitai[bot]" + }, + "state": "COMMENTED", + "commit_id": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "submitted_at": "2026-07-11T10:00:00Z", + "body": "Actionable comments posted: 1" + }, + { + "user": { + "login": "coderabbitai[bot]" + }, + "state": "APPROVED", + "commit_id": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "submitted_at": "2026-07-11T11:00:00Z", + "body": "" + } +] diff --git a/.github/tests/merge-gate-fixtures/index.json b/.github/tests/merge-gate-fixtures/index.json index 169f1f7e..71d5254c 100644 --- a/.github/tests/merge-gate-fixtures/index.json +++ b/.github/tests/merge-gate-fixtures/index.json @@ -43,6 +43,10 @@ "name": "codex-comment-edited-after-clean", "expect_green": false }, + { + "name": "codex-newest-malformed-supersedes-clean", + "expect_green": false + }, { "name": "cr-approval-with-codex-review-findings", "expect_green": false @@ -103,6 +107,18 @@ "name": "cr-approval-then-commented-clean", "expect_green": true }, + { + "name": "cr-commented-findings-then-approval", + "expect_green": true + }, + { + "name": "cr-commented-edited-after-approval", + "expect_green": false + }, + { + "name": "cr-commented-edited-before-submit", + "expect_green": false + }, { "name": "cr-commented-blank-supersedes-approval", "expect_green": false diff --git a/.github/tests/test-enable-auto-merge-pentad-gate.sh b/.github/tests/test-enable-auto-merge-pentad-gate.sh index bb10b03a..cff1c163 100755 --- a/.github/tests/test-enable-auto-merge-pentad-gate.sh +++ b/.github/tests/test-enable-auto-merge-pentad-gate.sh @@ -88,6 +88,16 @@ if [[ "$pull_request_review_types" != *"edited"* ]]; then status=1 fi +if ! grep -q 'lastEditedAt' "$workflow" || ! grep -q 'last_edited_at' "$script"; then + echo "::error file=$workflow::review snapshots must preserve GraphQL lastEditedAt so edited CodeRabbit bodies are ordered by their durable edit time" + status=1 +fi +if ! grep -q 'author{login __typename}' "$workflow" || + ! grep -q 'author.__typename == "Bot"' "$workflow"; then + echo "::error file=$workflow::GraphQL Bot logins must be normalized to the REST-style [bot] names consumed by the gate parser" + status=1 +fi + # Reusable workflows cannot schedule their callers. The caller-facing Usage # block must therefore carry the same edited-review disarm trigger as the # workflow itself; consumers copy this example when opting into enforcement. diff --git a/.github/workflows/enable-auto-merge.yaml b/.github/workflows/enable-auto-merge.yaml index 5bf9af9f..f43de0f2 100644 --- a/.github/workflows/enable-auto-merge.yaml +++ b/.github/workflows/enable-auto-merge.yaml @@ -238,7 +238,36 @@ jobs: HEAD_SEEN_AT=$(bash .devantler-tech-actions/.scripts/compute-head-seen-floor.sh \ "$REPOSITORY" "$PR_NUMBER" "$HEAD_SHA" "$GITHUB_RUN_ID" "$EVENT_NAME") - gh api "repos/$REPOSITORY/pulls/$PR_NUMBER/reviews" --paginate | jq -s 'add // []' > /tmp/reviews.json + OWNER="${REPOSITORY%%/*}" + NAME="${REPOSITORY#*/}" + # REST exposes only submitted_at, so an edited review can look older + # than an approval it actually superseded. GraphQL's lastEditedAt is + # durable across later event shapes and keeps fail-closed ordering. + # shellcheck disable=SC2016 # GraphQL $variables, not shell expansion. + gh api graphql --paginate \ + -f query='query($owner:String!,$name:String!,$number:Int!,$endCursor:String){ + repository(owner:$owner,name:$name){pullRequest(number:$number){ + reviews(first:100,after:$endCursor){ + nodes{author{login __typename} body state commit{oid} submittedAt lastEditedAt} + pageInfo{hasNextPage endCursor} + } + }} + }' \ + -f owner="$OWNER" -f name="$NAME" -F number="$PR_NUMBER" \ + | jq -s '[.[].data.repository.pullRequest.reviews.nodes[]? | { + user: {login: ( + if .author.__typename == "Bot" and + ((.author.login // "") | endswith("[bot]") | not) + then ((.author.login // "") + "[bot]") + else (.author.login // "") + end + )}, + body: (.body // ""), + state: .state, + commit_id: (.commit.oid // ""), + submitted_at: .submittedAt, + last_edited_at: .lastEditedAt + }]' > /tmp/reviews.json gh api "repos/$REPOSITORY/issues/$PR_NUMBER/comments" --paginate | jq -s 'add // []' > /tmp/comments.json if bash .devantler-tech-actions/.scripts/check-merge-gates.sh "$HEAD_SHA" "$HEAD_SEEN_AT" /tmp/reviews.json /tmp/comments.json; then diff --git a/.scripts/check-merge-gates.sh b/.scripts/check-merge-gates.sh index cb87c6e8..57ca8c3e 100755 --- a/.scripts/check-merge-gates.sh +++ b/.scripts/check-merge-gates.sh @@ -12,7 +12,8 @@ # SHA of its own. Commit metadata alone is NOT a safe # floor: pushing a previously-created commit object # carries an old committer date. -# reviews-json file holding the FULL paginated `pulls//reviews` array +# reviews-json file holding the FULL paginated GraphQL review array, +# normalized to REST-style keys plus last_edited_at # comments-json file holding the FULL paginated `issues//comments` array # # Exits 0 only when BOTH gates are proven at the current head: @@ -62,12 +63,14 @@ premerge_state="not-posted" # CodeRabbit's verdict at the head is its LATEST verdict-bearing review there: # an APPROVED superseded by CHANGES_REQUESTED (or dismissed) must not count. cr_latest_verdict_at_head="$(jq -r --arg sha "$head_sha" ' + def effective_at: + [.submitted_at, .last_edited_at] | map(select(. != null)) | max // ""; [.[] | select(.user.login == "coderabbitai[bot]") | select(.commit_id == $sha) | select(.state == "APPROVED" or .state == "CHANGES_REQUESTED" or .state == "DISMISSED")] - | sort_by(.submitted_at) | last | .state // empty' "$reviews_json")" + | sort_by(effective_at) | last | .state // empty' "$reviews_json")" cr_approved_anywhere="$(jq -r ' [.[] | select(.user.login == "coderabbitai[bot]" and .state == "APPROVED")] @@ -90,21 +93,24 @@ fi # a body that explicitly proves clean ("Actionable comments posted: 0") # preserves the state; a blank or unparseable body counts as findings # (fail-closed β€” a bodyless COMMENTED review can still carry inline review -# comments). The EXISTENCE marker line distinguishes "no such review" from -# "review with an empty body". +# comments). GraphQL lastEditedAt makes an edited older review supersede a +# later-submitted approval durably. The EXISTENCE marker line distinguishes +# "no such review" from "review with an empty body". cr_commented_probe="$(jq -r --arg sha "$head_sha" ' + def effective_at: + [.submitted_at, .last_edited_at] | map(select(. != null)) | max // ""; ([.[] | select(.user.login == "coderabbitai[bot]") | select(.commit_id == $sha) | select(.state == "APPROVED" or .state == "CHANGES_REQUESTED" or .state == "DISMISSED")] - | sort_by(.submitted_at) | last | .submitted_at // "") as $verdict_at + | sort_by(effective_at) | last | effective_at) as $verdict_at | [.[] | select(.user.login == "coderabbitai[bot]") | select(.commit_id == $sha) | select(.state == "COMMENTED") - | select(.submitted_at > $verdict_at)] - | sort_by(.submitted_at) | last + | select(effective_at >= $verdict_at)] + | sort_by(effective_at) | last | if . == null then "absent" else "present\n" + (.body // "") end' "$reviews_json")" if [[ "$cr_commented_probe" == present* && @@ -115,9 +121,7 @@ fi # Codex lane: clean results are ISSUE COMMENTS carrying "**Reviewed commit:** # " (often abbreviated), while findings arrive as review objects at the # exact head. Any current-head findings review is red evidence even when a -# later clean comment exists: the REST review snapshot exposes submitted_at but -# not an edit timestamp, so allowing a comment to supersede it could miss an -# older review edited red later. This conservative result can only withhold the +# later clean comment exists. This conservative result can only withhold the # workflow's approval; the maintenance agent still evaluates the live pentad. # An abbreviated clean SHA is accepted only when GitHub's commit endpoint # resolves it uniquely to the exact head; raw prefix matching is not proof. @@ -128,15 +132,15 @@ codex_findings_at_head="$(jq -r --arg sha "$head_sha" ' | select(.state == "COMMENTED" or .state == "CHANGES_REQUESTED")] | length' "$reviews_json")" -latest_codex_comment_probe="$(jq -r --arg sha "$head_sha" ' - ($sha | ascii_downcase) as $head - | [.[] +latest_codex_comment_probe="$(jq -r ' + [.[] | select(.user.login == "chatgpt-codex-connector[bot]") | (.body // "") as $body - | (try ($body - | capture("\\*\\*Reviewed commit:\\*\\*[[:space:]]*`?(?[0-9a-fA-F]{7,40})") - | .sha | ascii_downcase) catch "") as $reviewed - | select($reviewed != "" and ($head | startswith($reviewed))) + | select($body | contains("Codex Review:")) + | ([try ($body + | capture("\\*\\*Reviewed commit:\\*\\*[[:space:]]*`?(?[0-9a-fA-F]{7,40})") + | .sha) catch ""] + | first // "" | ascii_downcase) as $reviewed | {at: (.updated_at // .created_at), reviewed: $reviewed, body: $body}] | sort_by(.at) | last | if . == null then "absent"