diff --git a/slash-commands/README.md b/slash-commands/README.md index 3cc179a..be8a157 100644 --- a/slash-commands/README.md +++ b/slash-commands/README.md @@ -45,6 +45,16 @@ review; `/lgtm` satisfies that via the App's approval and turns on auto-merge. The PR merges as soon as CI passes. Manually adding the `accepted` label in the GitHub UI has no effect — the gate is the review, not the label. +> [!IMPORTANT] +> If your branch protection enables **Require review from Code Owners**, the +> App's approval will _not_ satisfy it — GitHub Apps cannot be members of org +> teams, so they cannot be code owners via a team entry. Auto-merge also does +> not inherit the bypass privileges of the App that enabled it. To make +> `/lgtm` work under this rule, **add the GitHub App to the ruleset's bypass +> list** (Settings → Rules → Rulesets → _your ruleset_ → Bypass list → add +> the App with mode _Always_). The action detects this case and falls back to +> a direct merge, which _does_ use the App's bypass. + ## Prerequisites - **A GitHub App** installed on the caller repo with the permissions listed diff --git a/slash-commands/main.js b/slash-commands/main.js index 8015aad..218d4b6 100644 --- a/slash-commands/main.js +++ b/slash-commands/main.js @@ -201,29 +201,47 @@ module.exports = async ({ github, context, core }) => { } async function enableAutoMerge(prNodeId, prNumber) { if (!autoMerge) return; + async function mergeDirect(reason) { + core.info(`${reason} — merging directly`); + try { + await github.rest.pulls.merge({ + owner, repo, pull_number: prNumber, + merge_method: autoMergeMethod.toLowerCase(), + }); + core.info('Merged'); + } catch (e) { + core.warning(`Direct merge failed: ${e.message}. If branch protection requires code-owner review, add this GitHub App to the ruleset bypass list.`); + } + } try { - await github.graphql( + const result = await github.graphql( `mutation($id: ID!, $m: PullRequestMergeMethod!) { enablePullRequestAutoMerge(input: {pullRequestId: $id, mergeMethod: $m}) { - pullRequest { autoMergeRequest { enabledAt } } + pullRequest { autoMergeRequest { enabledAt } reviewDecision } } }`, { id: prNodeId, m: autoMergeMethod }, ); core.info(`Auto-merge enabled (${autoMergeMethod})`); + // Auto-merge does not inherit the ruleset-bypass privileges of the + // actor who enabled it — it only fires once the PR is clean for + // everyone. If we submitted an approval and reviewDecision is still + // REVIEW_REQUIRED, the bot's approval did not satisfy + // require_code_owner_review (GitHub Apps cannot be team members), so + // auto-merge will never fire. Merge directly instead — a direct merge + // DOES use the bot's bypass. + const decision = result.enablePullRequestAutoMerge.pullRequest.reviewDecision; + if (submitReview && decision === 'REVIEW_REQUIRED') { + return mergeDirect('Bot approval did not satisfy review requirement'); + } } catch (e) { const msg = e.message || String(e); // enablePullRequestAutoMerge rejects PRs that are already mergeable - // ("clean status") because there is nothing to wait for. Merge directly. + // ("clean status") because there is nothing to wait for. if (/clean status/i.test(msg)) { - core.info('PR already mergeable — merging directly'); - await github.rest.pulls.merge({ - owner, repo, pull_number: prNumber, - merge_method: autoMergeMethod.toLowerCase(), - }); - } else { - core.warning(`Could not enable auto-merge: ${msg}`); + return mergeDirect('PR already mergeable'); } + core.warning(`Could not enable auto-merge: ${msg}`); } } async function disableAutoMerge(prNodeId) {