From 858c311fa60bef4c473daabb0c4741972e29938c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 19:06:39 +0000 Subject: [PATCH 1/3] pr-automerge.yml: remove the direct-merge fallback, never bypass CI The workflow's own fallback logic called github.rest.pulls.merge() directly whenever GraphQL's viewerCanEnableAutoMerge read false, gated only on pr.mergeable === 'MERGEABLE' -- which means "no git conflicts with the base branch," not "required checks passed." An admin-capable AUTOMERGE_TOKEN makes viewerCanEnableAutoMerge read false immediately on a fresh PR (nothing to queue -- the actor can already bypass required checks), so this fallback merged PR #447 within ~20 seconds of it opening, before any CI check had even started. Confirmed directly from the actual workflow run's job log. Removed both direct-merge fallback branches (in the "Enable Auto-merge via PAT" and "Enable Auto-merge via GITHUB_TOKEN" steps). The workflow now only ever queues a merge via enablePullRequestAutoMerge, which GitHub itself will not complete until required checks and reviews genuinely pass -- it never calls pulls.merge() directly under any code path. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_017SQ1rvJxDbE71pTXJ4QvLV --- .github/workflows/pr-automerge.yml | 46 ++++++++++-------------------- 1 file changed, 15 insertions(+), 31 deletions(-) diff --git a/.github/workflows/pr-automerge.yml b/.github/workflows/pr-automerge.yml index 26f3f6b7..a9211264 100644 --- a/.github/workflows/pr-automerge.yml +++ b/.github/workflows/pr-automerge.yml @@ -102,16 +102,18 @@ jobs: const pre = await github.graphql(ql, { owner, repo, num: number }); const pr = pre.repository.pullRequest; + // derived requirement: NEVER merge directly from this workflow -- only ever queue + // via enablePullRequestAutoMerge, which GitHub itself will not complete until + // required checks/reviews genuinely pass. A prior version of this script fell back + // to github.rest.pulls.merge() here (and in the catch block below) whenever + // viewerCanEnableAutoMerge read false, gated only on pr.mergeable === 'MERGEABLE' -- + // which means "no git conflicts with base," NOT "required checks passed." That + // fallback merged a real PR (#447) within ~20 seconds of it opening, before any CI + // check had even started, because an admin-capable AUTOMERGE_TOKEN made + // viewerCanEnableAutoMerge read false immediately (nothing to queue -- the actor can + // already bypass). Do not reintroduce a direct-merge fallback of any kind here. if (!pr.viewerCanEnableAutoMerge) { - core.info(`#${number}: viewerCanEnable=false (PR likely immediately mergeable); trying direct merge`); - if (pr.mergeable === 'MERGEABLE') { - try { - await github.rest.pulls.merge({ owner, repo, pull_number: number, merge_method: 'squash' }); - core.info(`#${number}: Merged directly (viewerCanEnable=false path) [OK]`); - } catch (mergeErr) { - core.info(`#${number}: Direct merge failed: ${mergeErr.message || String(mergeErr)}`); - } - } + core.info(`#${number}: viewerCanEnable=false (permissions, already armed, or repo auto-merge disabled); skipping -- this workflow never merges directly, only queues.`); continue; } @@ -125,17 +127,7 @@ jobs: } } catch (e) { const msg = e?.errors ? JSON.stringify(e.errors[0]) : String(e.message||e); - core.info(`#${number}: enable failed via PAT (GraphQL): ${msg}; checking for immediate merge`); - // Direct merge fallback: enablePullRequestAutoMerge fails when all required checks - // have already passed (PR immediately mergeable). Fall back to REST merge in that case. - if (pr.mergeable === 'MERGEABLE') { - try { - await github.rest.pulls.merge({ owner, repo, pull_number: number, merge_method: 'squash' }); - core.info(`#${number}: Merged directly (PR was immediately mergeable) [OK]`); - } catch (mergeErr) { - core.info(`#${number}: Direct merge failed: ${mergeErr.message || String(mergeErr)}`); - } - } + core.info(`#${number}: enable failed via PAT (GraphQL): ${msg}`); } } env: @@ -177,18 +169,10 @@ jobs: core.info(`#${number}: enable via GITHUB_TOKEN reported success, but REST auto_merge is null (permissions likely).`); } } catch (e) { + // derived requirement: no direct-merge fallback here either -- see the matching + // comment in the "Enable Auto-merge via PAT" step above for why (PR #447). const msg = e?.errors ? JSON.stringify(e.errors[0]) : String(e.message||e); - core.info(`#${number}: enable failed via GITHUB_TOKEN (GraphQL): ${msg}; checking for immediate merge`); - // Direct merge fallback: enablePullRequestAutoMerge fails when all required checks - // have already passed (PR immediately mergeable). Fall back to REST merge in that case. - if (prRest.mergeable === true) { - try { - await github.rest.pulls.merge({ owner, repo, pull_number: number, merge_method: 'squash' }); - core.info(`#${number}: Merged directly (PR was immediately mergeable) [OK]`); - } catch (mergeErr) { - core.info(`#${number}: Direct merge failed: ${mergeErr.message || String(mergeErr)}`); - } - } + core.info(`#${number}: enable failed via GITHUB_TOKEN (GraphQL): ${msg}`); } } env: From de7566bb915e4bc466227ceb11a00cf65812f173 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 19:11:31 +0000 Subject: [PATCH 2/3] pr-automerge.yml: attempt enable regardless of viewerCanEnableAutoMerge CodeRabbit review finding on PR #448: skipping outright when viewerCanEnableAutoMerge reads false meant a genuinely still-pending PR could fail to ever get queued if that client-side hint happened to read false for an unrelated reason (e.g. before AUTOMERGE_TOKEN is rescoped off admin/bypass). The GraphQL mutation itself is the real source of truth -- always attempt it now; a failure is just logged, never treated as license to merge directly (no pulls.merge() call was reintroduced). Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_017SQ1rvJxDbE71pTXJ4QvLV --- .github/workflows/pr-automerge.yml | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/.github/workflows/pr-automerge.yml b/.github/workflows/pr-automerge.yml index a9211264..28a72196 100644 --- a/.github/workflows/pr-automerge.yml +++ b/.github/workflows/pr-automerge.yml @@ -112,9 +112,16 @@ jobs: // check had even started, because an admin-capable AUTOMERGE_TOKEN made // viewerCanEnableAutoMerge read false immediately (nothing to queue -- the actor can // already bypass). Do not reintroduce a direct-merge fallback of any kind here. + // + // derived requirement (CodeRabbit review on PR #448): attempt the enable call + // regardless of viewerCanEnableAutoMerge rather than skipping outright -- that + // field is a client-side hint, not authoritative, and can read false for a + // genuinely still-pending PR too (e.g. before an admin-capable token is rescoped). + // The GraphQL mutation itself is the real source of truth: if it succeeds, the PR + // is genuinely queued; if it fails, the catch block below just logs it -- never a + // license to merge directly. if (!pr.viewerCanEnableAutoMerge) { - core.info(`#${number}: viewerCanEnable=false (permissions, already armed, or repo auto-merge disabled); skipping -- this workflow never merges directly, only queues.`); - continue; + core.info(`#${number}: viewerCanEnable=false; attempting enable anyway (queue-only, never merges directly)`); } try { From a76597de7af7b73509096918ece596e2b1d45e82 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 19:28:37 +0000 Subject: [PATCH 3/3] Try AUTOMERGE_TOKEN_NONADMIN_NO_BYPASS before the admin-capable legacy token Adds a new, preferred first step in the auto-merge job that authenticates with AUTOMERGE_TOKEN_NONADMIN_NO_BYPASS (a fine-grained PAT scoped to Pull requests: Read and write only). The existing admin-capable AUTOMERGE_TOKEN step stays wired as a fallback -- its own per-PR "already armed (REST)" check makes it a safe no-op for anything the new token already enabled -- until the new token is confirmed working across several real PRs, at which point AUTOMERGE_TOKEN's own step should be removed. The new step never blocks the fallback: every per-PR REST/GraphQL call is wrapped in try/catch and any failure -- including an outright auth failure on the very first call -- is logged via core.warning (not core.info) so it stands out in the Actions UI, and continue-on-error guards against the action itself failing to initialize (e.g. a malformed token) before the script body even runs. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_017SQ1rvJxDbE71pTXJ4QvLV --- .github/workflows/pr-automerge.yml | 135 +++++++++++++++++++++++------ 1 file changed, 110 insertions(+), 25 deletions(-) diff --git a/.github/workflows/pr-automerge.yml b/.github/workflows/pr-automerge.yml index 28a72196..828e5f05 100644 --- a/.github/workflows/pr-automerge.yml +++ b/.github/workflows/pr-automerge.yml @@ -23,6 +23,14 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 20 env: + # derived requirement (user rollout, 2026-08-21): AUTOMERGE_TOKEN_NONADMIN_NO_BYPASS is a + # fine-grained PAT scoped to "Pull requests: Read and write" only -- unlike AUTOMERGE_TOKEN + # (an older, admin-capable token; see the PR #447 incident documented on the step below), + # it should NOT be able to bypass required-status-check branch protection even if this + # workflow's own logic ever regressed back toward a direct-merge fallback. Tried first; + # AUTOMERGE_TOKEN stays wired as a fallback until the new token is confirmed working across + # several real PRs, then AUTOMERGE_TOKEN's own step should be removed. + AUTOMERGE_TOKEN_NONADMIN_NO_BYPASS: ${{ secrets.AUTOMERGE_TOKEN_NONADMIN_NO_BYPASS }} AUTOMERGE_TOKEN: ${{ secrets.AUTOMERGE_TOKEN }} steps: - name: Wait for automated reviews (PR opened only) @@ -62,7 +70,96 @@ jobs: const nums = await numbersFromEvent(); core.setOutput('numbers', JSON.stringify(nums)); - - name: Enable Auto-merge via PAT + - name: Enable Auto-merge via AUTOMERGE_TOKEN_NONADMIN_NO_BYPASS (preferred) + if: ${{ steps.prs.outputs.numbers != '[]' && env.AUTOMERGE_TOKEN_NONADMIN_NO_BYPASS }} + # derived requirement: this step must never block the AUTOMERGE_TOKEN legacy-fallback + # step below, even if the new token turns out to be malformed enough that the + # actions/github-script action itself fails before the script body ever runs (e.g. at + # Octokit client construction). continue-on-error keeps the job's overall status healthy + # for the fallback step's own (unconditioned-on-this-step) `if:` to still evaluate and + # run, while this step's own outcome/log still faithfully shows failure for troubleshooting. + continue-on-error: true + uses: actions/github-script@v8 + with: + github-token: ${{ secrets.AUTOMERGE_TOKEN_NONADMIN_NO_BYPASS }} + script: | + const owner = context.repo.owner; + const repo = context.repo.repo; + const nums = JSON.parse(process.env.numbers || '[]'); + core.info('Auth path: PAT (AUTOMERGE_TOKEN_NONADMIN_NO_BYPASS, preferred non-admin token)'); + + const ql = ` + query($owner:String!,$repo:String!,$num:Int!){ + repository(owner:$owner,name:$repo){ + pullRequest(number:$num){ + id number isDraft mergeable reviewDecision viewerCanEnableAutoMerge + autoMergeRequest { enabledAt } + headRepository { nameWithOwner } baseRepository { nameWithOwner } + } + } + } + `; + const enable = ` + mutation($id:ID!){ + enablePullRequestAutoMerge(input:{pullRequestId:$id,mergeMethod:SQUASH}){ clientMutationId } + } + `; + + // derived requirement: NEVER merge directly from this workflow -- only ever queue + // via enablePullRequestAutoMerge, which GitHub itself will not complete until + // required checks/reviews genuinely pass. A prior version of this script fell back + // to github.rest.pulls.merge() here whenever viewerCanEnableAutoMerge read false, + // gated only on pr.mergeable === 'MERGEABLE' -- which means "no git conflicts with + // base," NOT "required checks passed." That fallback merged a real PR (#447) within + // ~20 seconds of it opening, before any CI check had even started, because an + // admin-capable AUTOMERGE_TOKEN made viewerCanEnableAutoMerge read false immediately + // (nothing to queue -- the actor can already bypass). Do not reintroduce a + // direct-merge fallback of any kind here, in this step or any of its siblings. + // + // derived requirement (CodeRabbit review on PR #448): attempt the enable call + // regardless of viewerCanEnableAutoMerge rather than skipping outright -- that + // field is a client-side hint, not authoritative, and can read false for a + // genuinely still-pending PR too. The GraphQL mutation itself is the real source of + // truth: if it succeeds, the PR is genuinely queued; if it fails, it's just logged -- + // never a license to merge directly. + for (const number of nums) { + try { + const { data: prRest } = await github.rest.pulls.get({ owner, repo, pull_number: number }); + const labels = (prRest.labels || []).map(l => (l.name||'').toLowerCase()); + const isSameRepo = prRest.head?.repo?.full_name === prRest.base?.repo?.full_name; + + if (prRest.state !== 'open' || prRest.draft) { core.info(`#${number}: skip (closed/draft)`); continue; } + if (!isSameRepo) { core.info(`#${number}: skip (fork PR)`); continue; } + if (labels.includes('no-automerge')) { core.info(`#${number}: skip (has 'no-automerge')`); continue; } + if (prRest.auto_merge) { core.info(`#${number}: already armed (REST)`); continue; } + + const pre = await github.graphql(ql, { owner, repo, num: number }); + const pr = pre.repository.pullRequest; + if (!pr.viewerCanEnableAutoMerge) { + core.info(`#${number}: viewerCanEnable=false; attempting enable anyway (queue-only, never merges directly)`); + } + + await github.graphql(enable, { id: pr.id }); + const { data: after } = await github.rest.pulls.get({ owner, repo, pull_number: number }); + if (after.auto_merge) { + core.info(`#${number}: Auto-merge enabled (Squash) via AUTOMERGE_TOKEN_NONADMIN_NO_BYPASS [OK]`); + } else { + core.info(`#${number}: enable via AUTOMERGE_TOKEN_NONADMIN_NO_BYPASS reported success, but REST auto_merge is null (will rely on diagnostics/legacy fallback).`); + } + } catch (e) { + // core.warning (not core.info) so an outright failure of the new token -- + // including an auth/permission failure on the very first REST call, not just the + // enable mutation -- is visually distinct in the Actions log and shows up as its + // own check-run annotation, making it easy to confirm from the logs whether + // AUTOMERGE_TOKEN_NONADMIN_NO_BYPASS's scopes are sufficient. + const msg = e?.errors ? JSON.stringify(e.errors[0]) : String(e.message||e); + core.warning(`#${number}: AUTOMERGE_TOKEN_NONADMIN_NO_BYPASS FAILED (${msg}) -- falling back to AUTOMERGE_TOKEN (legacy) below.`); + } + } + env: + numbers: ${{ steps.prs.outputs.numbers }} + + - name: Enable Auto-merge via AUTOMERGE_TOKEN (legacy, admin-capable -- remove once AUTOMERGE_TOKEN_NONADMIN_NO_BYPASS is confirmed working) if: ${{ steps.prs.outputs.numbers != '[]' && env.AUTOMERGE_TOKEN }} uses: actions/github-script@v8 with: @@ -71,7 +168,7 @@ jobs: const owner = context.repo.owner; const repo = context.repo.repo; const nums = JSON.parse(process.env.numbers || '[]'); - core.info('Auth path: PAT (AUTOMERGE_TOKEN)'); + core.info('Auth path: PAT (AUTOMERGE_TOKEN, legacy admin-capable fallback)'); const ql = ` query($owner:String!,$repo:String!,$num:Int!){ @@ -90,6 +187,12 @@ jobs: } `; + // derived requirement: no direct-merge fallback here either -- see the matching + // comment in the AUTOMERGE_TOKEN_NONADMIN_NO_BYPASS step above for the PR #447 + // incident this protects against, and for why the enable call is always attempted + // regardless of viewerCanEnableAutoMerge. This step is a fallback for any PR the + // preferred token's step above could not arm (its own `already armed (REST)` check + // just above makes it a safe no-op for anything already armed by that step). for (const number of nums) { const { data: prRest } = await github.rest.pulls.get({ owner, repo, pull_number: number }); const labels = (prRest.labels || []).map(l => (l.name||'').toLowerCase()); @@ -102,24 +205,6 @@ jobs: const pre = await github.graphql(ql, { owner, repo, num: number }); const pr = pre.repository.pullRequest; - // derived requirement: NEVER merge directly from this workflow -- only ever queue - // via enablePullRequestAutoMerge, which GitHub itself will not complete until - // required checks/reviews genuinely pass. A prior version of this script fell back - // to github.rest.pulls.merge() here (and in the catch block below) whenever - // viewerCanEnableAutoMerge read false, gated only on pr.mergeable === 'MERGEABLE' -- - // which means "no git conflicts with base," NOT "required checks passed." That - // fallback merged a real PR (#447) within ~20 seconds of it opening, before any CI - // check had even started, because an admin-capable AUTOMERGE_TOKEN made - // viewerCanEnableAutoMerge read false immediately (nothing to queue -- the actor can - // already bypass). Do not reintroduce a direct-merge fallback of any kind here. - // - // derived requirement (CodeRabbit review on PR #448): attempt the enable call - // regardless of viewerCanEnableAutoMerge rather than skipping outright -- that - // field is a client-side hint, not authoritative, and can read false for a - // genuinely still-pending PR too (e.g. before an admin-capable token is rescoped). - // The GraphQL mutation itself is the real source of truth: if it succeeds, the PR - // is genuinely queued; if it fails, the catch block below just logs it -- never a - // license to merge directly. if (!pr.viewerCanEnableAutoMerge) { core.info(`#${number}: viewerCanEnable=false; attempting enable anyway (queue-only, never merges directly)`); } @@ -128,20 +213,20 @@ jobs: await github.graphql(enable, { id: pr.id }); const { data: after } = await github.rest.pulls.get({ owner, repo, pull_number: number }); if (after.auto_merge) { - core.info(`#${number}: Auto-merge enabled (Squash) via PAT [OK]`); + core.info(`#${number}: Auto-merge enabled (Squash) via AUTOMERGE_TOKEN (legacy) [OK]`); } else { - core.info(`#${number}: enable via PAT reported success, but REST auto_merge is null (will rely on diagnostics).`); + core.info(`#${number}: enable via AUTOMERGE_TOKEN (legacy) reported success, but REST auto_merge is null (will rely on diagnostics).`); } } catch (e) { const msg = e?.errors ? JSON.stringify(e.errors[0]) : String(e.message||e); - core.info(`#${number}: enable failed via PAT (GraphQL): ${msg}`); + core.info(`#${number}: enable failed via AUTOMERGE_TOKEN (legacy) (GraphQL): ${msg}`); } } env: numbers: ${{ steps.prs.outputs.numbers }} - name: Enable Auto-merge via GITHUB_TOKEN - if: ${{ steps.prs.outputs.numbers != '[]' && !env.AUTOMERGE_TOKEN }} + if: ${{ steps.prs.outputs.numbers != '[]' && !env.AUTOMERGE_TOKEN_NONADMIN_NO_BYPASS && !env.AUTOMERGE_TOKEN }} uses: actions/github-script@v8 with: github-token: ${{ github.token }} @@ -177,7 +262,7 @@ jobs: } } catch (e) { // derived requirement: no direct-merge fallback here either -- see the matching - // comment in the "Enable Auto-merge via PAT" step above for why (PR #447). + // comment in the AUTOMERGE_TOKEN_NONADMIN_NO_BYPASS step above for why (PR #447). const msg = e?.errors ? JSON.stringify(e.errors[0]) : String(e.message||e); core.info(`#${number}: enable failed via GITHUB_TOKEN (GraphQL): ${msg}`); }