diff --git a/README.md b/README.md index 86a84b6..ae0d7de 100644 --- a/README.md +++ b/README.md @@ -9,5 +9,5 @@ GitHub Actions relevant to the management of MCP repositories. | [`cloudflare-pages-preview/deploy`](cloudflare-pages-preview/README.md) | Deploy a static HTML directory to Cloudflare Pages under a per-PR branch alias, inject noindex headers, and post a sticky PR comment with preview URLs | | [`cloudflare-pages-preview/cleanup`](cloudflare-pages-preview/README.md) | Delete Cloudflare Pages deployments for a closed PR's branch alias and update the sticky comment | | [`hugo-build`](hugo-build/README.md) | Install Go + Hugo extended and build a Hugo site (preview or production mode) | -| [`slash-commands`](slash-commands/README.md) | Handle `/lgtm`, `/hold`, and `/stageblog` PR slash commands gated by team membership + CODEOWNERS; invalidate approval on push | -| [`slash-commands/status`](slash-commands/README.md#slash-commandsstatus-sub-action) | Set a pending/success/failure commit status based on approval/hold labels — yellow pending check until `/lgtm`, not red | +| [`slash-commands`](slash-commands/README.md) | Handle `/lgtm` (core-maintainers only: label + approve + auto-merge), `/hold`, and `/stageblog` PR slash commands; invalidate approval on push | +| [`slash-commands/status`](slash-commands/README.md#slash-commandsstatus-sub-action) | _Optional:_ set a pending/failure commit status for callers using status-check-based gating instead of required-reviews + auto-merge | diff --git a/slash-commands/README.md b/slash-commands/README.md index b2ea502..5ca111e 100644 --- a/slash-commands/README.md +++ b/slash-commands/README.md @@ -8,17 +8,19 @@ CODEOWNERS. Listens for PR comments and push events, then: -- **`/lgtm`** — if the commenter is a core maintainer _or_ a CODEOWNER of the - PR's changed files, adds an `approved` label and submits a bot **APPROVE** - review with body "Approved on behalf of @user via `/lgtm`." -- **`/lgtm cancel`** — removes the `approved` label and dismisses the bot - review. +- **`/lgtm`** — **core maintainers only** (`always-allow-teams`). Adds an + `accepted` label, submits a bot **APPROVE** review on behalf of the team, + and **enables auto-merge**. The PR merges automatically once all required + checks pass. CODEOWNERS does _not_ grant `/lgtm` authorization. +- **`/lgtm cancel`** — removes the `accepted` label, dismisses the bot review, + and disables auto-merge. - **`/lgtm force`** — same as `/lgtm`, but **only** members of `force-allow-teams` (default: core + lead maintainers) may use it, and it **bypasses the self-approval guard**. Intended as an escape hatch for maintainers who need to approve their own PRs. The bot review body notes the approval came via `/lgtm force`. -- **`/hold`** — adds a `do-not-merge/hold` label (same authorization rule). +- **`/hold`** — adds a `do-not-merge/hold` label. Core maintainers _or_ + CODEOWNERS of the PR's changed files may use this. - **`/hold cancel`** or **`/unhold`** — removes the hold label. - **`/stageblog`** — if the PR touches any blog paths (default `blog/**`), dispatches a caller-defined `workflow_dispatch` workflow passing the PR @@ -26,27 +28,36 @@ Listens for PR comments and push events, then: letting maintainers manually stage blog previews for fork PRs (where the normal `pull_request`-triggered preview workflow can't access secrets). Disabled unless `stageblog-workflow` is set. -- **New commits pushed** — removes the `approved` label, dismisses the bot - review, and posts a brief comment asking for re-approval. +- **New commits pushed** — removes the `accepted` label, dismisses the bot + review, disables auto-merge, and posts a brief comment asking for + re-approval. Commands are parsed from the **first line** of the comment (case-insensitive, must match `^/cmd\b`). Unauthorized attempts, or a PR author trying to `/lgtm` their own PR, receive a 👎 reaction with no further noise. Successful commands receive a 👍 reaction. -The action writes a commit status (`slash-commands/approval` by default) after -each label mutation — this is the merge gate. Mark it as a required status -check in branch protection. The commit status can **only** be set to `success` -by an authorized `/lgtm`; manually adding the `approved` label through the -GitHub UI has no effect on the gate. The caller workflow pairs this with the -`slash-commands/status` sub-action, which sets the status to `pending` on open -and on every push (fresh SHA = fresh approval needed). +### Merge gate + +The primary merge gate is **GitHub's native required-reviews rule** plus +**auto-merge**: configure branch protection to require at least one approving +review, and enable "Allow auto-merge" in repo Settings. `/lgtm` then satisfies +the review requirement 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. + +An optional commit status (`slash-commands/approval`) is also written for +callers who prefer status-check-based gating — set `status-context: ''` to +disable it. See the [`slash-commands/status` sub-action](#slash-commandsstatus-sub-action) +if you want to use the status-check flow instead of auto-merge. ## Prerequisites -- The `approved` and `do-not-merge/hold` labels **must already exist** in the +- **Allow auto-merge** must be enabled in the caller repo + (Settings → General → Pull Requests → Allow auto-merge). +- The `accepted` and `do-not-merge/hold` labels **must already exist** in the caller repo (the action does not auto-create them). Suggested colors: - `approved` → `#0e8a16`, `do-not-merge/hold` → `#d93f0b`. + `accepted` → `#0e8a16`, `do-not-merge/hold` → `#d93f0b`. - A CODEOWNERS file at `.github/CODEOWNERS` (or pass a different path via `codeowners-path`). Without one, only `always-allow-teams` members can act. @@ -127,19 +138,15 @@ jobs: - uses: modelcontextprotocol/actions/slash-commands@main with: github-token: ${{ secrets.SLASH_COMMANDS_TOKEN }} - always-allow-teams: core-maintainers,lead-maintainers + # auto-merge-method: squash # default; also: merge, rebase # stageblog-workflow: stage-blog.yml # uncomment to enable /stageblog - - # Merge-gate commit status. Mark `slash-commands/approval` as a required - # status check in branch protection. This stays pending (yellow) until - # /lgtm adds the approved label — no red ❌ on fresh PRs. - status: - if: github.event_name == 'pull_request_target' - runs-on: ubuntu-latest - steps: - - uses: modelcontextprotocol/actions/slash-commands/status@main ``` +**Branch protection settings** for this flow: enable _Require a pull request +before merging_ → _Require approvals_ (1), and _Allow auto-merge_ in repo +Settings. The bot's APPROVE review satisfies the requirement and auto-merge +handles the rest. + ### Enabling `/stageblog` — companion workflow When `stageblog-workflow` is set, you also need a separate workflow in the @@ -191,14 +198,16 @@ jobs: |---|---|---|---| | `github-token` | ✅ | — | PAT or App token with `read:org` + repo `contents: read` (+ `actions: write` for `/stageblog`). Used for team membership checks, CODEOWNERS fetch, and workflow dispatch. **Default `GITHUB_TOKEN` will not work** for team checks. | | `bot-token` | | `${{ github.token }}` | Token for user-visible side effects (comments, reactions, labels, reviews) — defaults to `GITHUB_TOKEN` so these show as `github-actions[bot]`. Rarely needs overriding. | -| `approved-label` | | `approved` | Label added by `/lgtm` | +| `approved-label` | | `accepted` | Label added by `/lgtm` to mark the PR as accepted | | `hold-label` | | `do-not-merge/hold` | Label added by `/hold` | -| `always-allow-teams` | | `core-maintainers` | Comma-separated team slugs (in the repo's org) whose members can `/lgtm` or `/hold` any PR regardless of CODEOWNERS | +| `always-allow-teams` | | `core-maintainers` | Comma-separated team slugs (in the repo's org) whose members can `/lgtm` any PR. **Only these teams can `/lgtm`** — CODEOWNERS does not grant it. Members may also `/hold`/`/stageblog` any PR. | | `force-allow-teams` | | `core-maintainers,lead-maintainers` | Comma-separated team slugs whose members may use `/lgtm force` (bypasses self-approval guard). CODEOWNERS does **not** grant this — only team membership. | -| `codeowners-path` | | `.github/CODEOWNERS` | Path to CODEOWNERS, fetched from the PR's **base** ref (never head — tamper-proof) | -| `invalidate-on-push` | | `'true'` | Remove `approved` label + dismiss bot review + comment when new commits are pushed. Set `'false'` to keep approval across pushes. | +| `codeowners-path` | | `.github/CODEOWNERS` | Path to CODEOWNERS, fetched from the PR's **base** ref (never head — tamper-proof). Grants `/hold` and `/stageblog` only, not `/lgtm`. | +| `invalidate-on-push` | | `'true'` | Remove `accepted` label + dismiss bot review + disable auto-merge + comment when new commits are pushed. Set `'false'` to keep approval across pushes. | | `submit-review` | | `'true'` | Submit a bot APPROVE review alongside the label on `/lgtm`. Set `'false'` to use label only. | -| `status-context` | | `slash-commands/approval` | Commit-status context written after each label mutation. Must match the `status` sub-action's `status-context`. Mark this as a required status check in branch protection. | +| `enable-auto-merge` | | `'true'` | Enable auto-merge after `/lgtm` (disabled on `/lgtm cancel` and push invalidation). Requires repo setting "Allow auto-merge". | +| `auto-merge-method` | | `squash` | Merge method for auto-merge: `squash`, `merge`, or `rebase` | +| `status-context` | | `slash-commands/approval` | Optional commit-status context. Set to `''` to disable status writes (recommended when using auto-merge + required reviews as the gate). | | `stageblog-workflow` | | _(empty)_ | Workflow file name (e.g. `stage-blog.yml`) to dispatch when `/stageblog` is invoked. Empty = command disabled. The workflow must accept `pr_number` and `head_sha` string inputs. | | `stageblog-paths` | | `blog/**` | Comma-separated CODEOWNERS-style glob patterns. `/stageblog` is refused if no changed file matches. | @@ -211,6 +220,11 @@ jobs: ## `slash-commands/status` sub-action +> **Optional** — only needed if you're using the commit-status merge gate +> instead of (or alongside) the required-reviews + auto-merge flow. If your +> branch protection uses required reviews, you can skip this sub-action and +> set `status-context: ''` on the main action. + Sets the **initial** `pending` commit status when a PR opens, and forces it back to `pending` on every push (new SHA = new approval required). Run in a `status` job on `pull_request_target` events. Mark the `status-context` value @@ -275,10 +289,9 @@ Last-match-wins per file, consistent with GitHub's native CODEOWNERS semantics. `/lgtm force` escape hatch bypasses this, but is gated strictly to `force-allow-teams` membership (CODEOWNERS does not grant it) and the review body records that `force` was used. -- **Manual label bypass** — the commit status (the actual merge gate) is only - ever set to `success` by the `/lgtm` handler after the auth check passes. - The `status` sub-action sets `pending`/`failure` but never `success`, and the - caller workflow does not trigger on `labeled` events. Adding the `approved` +- **Manual label bypass** — the merge gate is the bot's APPROVE review (or, + in the status-check flow, the commit status), both of which are only written + by the `/lgtm` handler after the auth check passes. Adding the `accepted` label via the GitHub UI is cosmetic. - **`/stageblog` SHA pinning** — the dispatch passes `pr.head.sha` captured at the moment the maintainer comments. The companion workflow **must** check diff --git a/slash-commands/action.yml b/slash-commands/action.yml index 9ca92a5..2be0347 100644 --- a/slash-commands/action.yml +++ b/slash-commands/action.yml @@ -1,10 +1,10 @@ name: PR Slash Commands description: >- - Handles /lgtm, /hold, and /stageblog slash commands on PRs. Authorized - users (core maintainers or CODEOWNERS of the PR's changed files) can approve - via /lgtm (adds a label + bot APPROVE review), block via /hold, or trigger - a blog preview staging workflow via /stageblog. Approval is invalidated when - new commits are pushed. + Handles /lgtm, /hold, and /stageblog slash commands on PRs. Core maintainers + can approve via /lgtm (adds an accepted label, submits a bot APPROVE review, + and enables auto-merge). Core maintainers or CODEOWNERS can block via /hold + or trigger a blog preview staging workflow via /stageblog. Approval is + invalidated when new commits are pushed. inputs: github-token: @@ -25,9 +25,9 @@ inputs: required: false default: ${{ github.token }} approved-label: - description: Label added by /lgtm and used by the status job as the merge gate + description: Label added by /lgtm to mark the PR as accepted required: false - default: approved + default: accepted hold-label: description: Label added by /hold to block merging required: false @@ -35,7 +35,8 @@ inputs: always-allow-teams: description: >- Comma-separated team slugs (within the repo's org) whose members may - /lgtm or /hold any PR regardless of CODEOWNERS + /lgtm any PR. Only these teams can /lgtm — CODEOWNERS does not grant + /lgtm authorization. Members may also /hold and /stageblog any PR. required: false default: core-maintainers force-allow-teams: @@ -61,11 +62,24 @@ inputs: Set to 'false' to rely on the label alone. required: false default: 'true' + enable-auto-merge: + description: >- + Enable auto-merge on the PR after a successful /lgtm. The PR will merge + automatically once all required checks pass. Requires the repo to have + "Allow auto-merge" enabled in Settings. Disabled on /lgtm cancel and on + push invalidation. + required: false + default: 'true' + auto-merge-method: + description: Merge method for auto-merge — one of squash, merge, rebase + required: false + default: squash status-context: description: >- - Commit-status context name written after each label mutation. Must match - the status sub-action's status-context so both jobs write to the same - check. Mark this context as a required status check in branch protection. + Commit-status context name written after each label mutation. Set to + empty string to disable status writes (recommended when using + enable-auto-merge + required reviews as the merge gate instead). If set, + must match the status sub-action's status-context. required: false default: slash-commands/approval stageblog-workflow: @@ -108,6 +122,8 @@ runs: CODEOWNERS_PATH: ${{ inputs.codeowners-path }} INVALIDATE_ON_PUSH: ${{ inputs.invalidate-on-push }} SUBMIT_REVIEW: ${{ inputs.submit-review }} + ENABLE_AUTO_MERGE: ${{ inputs.enable-auto-merge }} + AUTO_MERGE_METHOD: ${{ inputs.auto-merge-method }} STATUS_CONTEXT: ${{ inputs.status-context }} STAGEBLOG_WORKFLOW: ${{ inputs.stageblog-workflow }} STAGEBLOG_PATHS: ${{ inputs.stageblog-paths }} diff --git a/slash-commands/main.js b/slash-commands/main.js index a6b3530..8af1c5d 100644 --- a/slash-commands/main.js +++ b/slash-commands/main.js @@ -14,6 +14,8 @@ module.exports = async ({ github, context, core }) => { const invalidateOnPush = process.env.INVALIDATE_ON_PUSH === 'true'; const submitReview = process.env.SUBMIT_REVIEW === 'true'; const statusContext = process.env.STATUS_CONTEXT; + const autoMerge = process.env.ENABLE_AUTO_MERGE === 'true'; + const autoMergeMethod = process.env.AUTO_MERGE_METHOD.toUpperCase(); const reviewMarker = ''; // `github` is authed with bot-token (defaults to GITHUB_TOKEN) so comments, // reactions, labels, and reviews show as github-actions[bot]. The PAT is @@ -116,14 +118,19 @@ module.exports = async ({ github, context, core }) => { return ok; } - async function isAuthorized(user, pr) { - // 1. always-allow teams + async function isInAlwaysAllowTeam(user) { for (const team of alwaysAllowTeams) { if (await isTeamMember(user, team)) { core.info(`@${user} authorized via always-allow team @${owner}/${team}`); return true; } } + return false; + } + + async function isAuthorized(user, pr) { + // 1. always-allow teams + if (await isInAlwaysAllowTeam(user)) return true; // 2. CODEOWNERS of the PR's changed files (from BASE ref) let content; try { @@ -217,6 +224,7 @@ module.exports = async ({ github, context, core }) => { // state=success — the status sub-action deliberately never does, so that a // manually-added approved label (triage+ perms suffice) cannot bypass auth. async function setCommitStatus(sha, approved, held) { + if (!statusContext) return; let state, description; if (held) { state = 'failure'; @@ -233,6 +241,49 @@ module.exports = async ({ github, context, core }) => { }); core.info(`Set commit status ${state} on ${sha.slice(0,7)}: ${description}`); } + async function enableAutoMerge(prNodeId, prNumber) { + if (!autoMerge) return; + try { + await github.graphql( + `mutation($id: ID!, $m: PullRequestMergeMethod!) { + enablePullRequestAutoMerge(input: {pullRequestId: $id, mergeMethod: $m}) { + pullRequest { autoMergeRequest { enabledAt } } + } + }`, + { id: prNodeId, m: autoMergeMethod }, + ); + core.info(`Auto-merge enabled (${autoMergeMethod})`); + } 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. + 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}`); + } + } + } + async function disableAutoMerge(prNodeId) { + if (!autoMerge) return; + try { + await github.graphql( + `mutation($id: ID!) { + disablePullRequestAutoMerge(input: {pullRequestId: $id}) { + pullRequest { id } + } + }`, + { id: prNodeId }, + ); + core.info('Auto-merge disabled'); + } catch (e) { + core.info(`Auto-merge disable skipped: ${e.message}`); + } + } // --- Dispatch --------------------------------------------------- const ev = context.eventName; @@ -245,6 +296,7 @@ module.exports = async ({ github, context, core }) => { if (!hasApproved) return setResult('noop'); await removeLabel(pr.number, approvedLabel); await dismissBotApprovals(pr.number, 'New commits pushed — approval invalidated.'); + await disableAutoMerge(pr.node_id); await github.rest.issues.createComment({ owner, repo, issue_number: pr.number, body: `New commits were pushed — removed the \`${approvedLabel}\` label. Re-approve with \`/lgtm\`.`, @@ -294,6 +346,7 @@ module.exports = async ({ github, context, core }) => { if (submitReview) await submitApproval(prNumber, actor, true); await react(commentId, '+1'); await setCommitStatus(pr.head.sha, true, hadHold); + await enableAutoMerge(pr.node_id, prNumber); return setResult('lgtm-forced', actor); } @@ -303,8 +356,13 @@ module.exports = async ({ github, context, core }) => { return setResult('self-lgtm-blocked', actor); } - // Auth check (same rule for all commands) - if (!(await isAuthorized(actor, pr))) { + // Auth check. /lgtm is restricted to always-allow-teams (core maintainers) + // only — CODEOWNERS does not grant it. /hold and /stageblog still accept + // CODEOWNERS of the PR's changed files. + const authorized = cmd === 'lgtm' + ? await isInAlwaysAllowTeam(actor) + : await isAuthorized(actor, pr); + if (!authorized) { await react(commentId, '-1'); return setResult('unauthorized', actor); } @@ -359,11 +417,13 @@ module.exports = async ({ github, context, core }) => { if (submitReview) await submitApproval(prNumber, actor); await react(commentId, '+1'); await setCommitStatus(pr.head.sha, true, hadHold); + await enableAutoMerge(pr.node_id, prNumber); return setResult('lgtm-added', actor); } if (cmd === 'lgtm' && cancel) { await removeLabel(prNumber, approvedLabel); await dismissBotApprovals(prNumber, `@${actor} cancelled approval via /lgtm cancel.`); + await disableAutoMerge(pr.node_id); await react(commentId, '+1'); await setCommitStatus(pr.head.sha, false, hadHold); return setResult('lgtm-removed', actor);