slash-commands: fall back to direct merge when bot approval fails code-owner check
When branch protection enables require_code_owner_review, the GitHub
App's APPROVE review does not satisfy it (Apps cannot be team members).
Auto-merge also does not inherit the App's ruleset-bypass privileges,
so the PR sits in BLOCKED/REVIEW_REQUIRED forever with auto-merge
enabled but never firing.
Detect this by reading reviewDecision back from the
enablePullRequestAutoMerge mutation. If it is still REVIEW_REQUIRED
after we submitted an approval, attempt a direct merge instead — a
direct merge does use the App's bypass.
Both direct-merge paths (this new one and the existing clean-status
fallback) now go through a shared mergeDirect helper that catches
failures and emits an actionable warning pointing at the ruleset
bypass-list setup.
🏠 Remote-Dev: homespace
This commit is contained in:
parent
8cde341728
commit
cf4e6c7382
2 changed files with 38 additions and 10 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
Loading…
Reference in a new issue