Merge pull request #13 from modelcontextprotocol/den/lgtm-status-direct-write

slash-commands: direct commit-status writes + /lgtm force
This commit is contained in:
Den Delimarsky 2026-03-23 20:10:29 -07:00 • committed by GitHub
commit 9105fff368
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 52 additions and 9 deletions

View file

@ -13,6 +13,11 @@ Listens for PR comments and push events, then:
review with body "Approved on behalf of @user via `/lgtm`."
- **`/lgtm cancel`** — removes the `approved` label and dismisses the bot
review.
- **`/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 cancel`** or **`/unhold`** — removes the hold label.
- **`/stageblog`** — if the PR touches any blog paths (default `blog/**`),
@ -189,6 +194,7 @@ jobs:
| `approved-label` | | `approved` | Label added by `/lgtm` |
| `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 |
| `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. |
| `submit-review` | | `'true'` | Submit a bot APPROVE review alongside the label on `/lgtm`. Set `'false'` to use label only. |
@ -200,7 +206,7 @@ jobs:
| Output | Description |
|---|---|
| `result` | One of: `lgtm-added`, `lgtm-removed`, `hold-added`, `hold-removed`, `invalidated`, `unauthorized`, `self-lgtm-blocked`, `stageblog-dispatched`, `stageblog-not-blog`, `stageblog-disabled`, `noop` |
| `result` | One of: `lgtm-added`, `lgtm-forced`, `lgtm-removed`, `hold-added`, `hold-removed`, `invalidated`, `unauthorized`, `force-unauthorized`, `self-lgtm-blocked`, `stageblog-dispatched`, `stageblog-not-blog`, `stageblog-disabled`, `noop` |
| `actor` | Login of the commenter (empty for non-comment triggers) |
## `slash-commands/status` sub-action
@ -265,7 +271,10 @@ Last-match-wins per file, consistent with GitHub's native CODEOWNERS semantics.
- **Workflow modification** — `issue_comment` and `pull_request_target` run
from the default-branch workflow definition. The action does not check out
PR code.
- **Self-approval** — explicitly blocked before the auth check.
- **Self-approval** — explicitly blocked before the auth check. The
`/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

View file

@ -38,6 +38,13 @@ inputs:
/lgtm or /hold any PR regardless of CODEOWNERS
required: false
default: core-maintainers
force-allow-teams:
description: >-
Comma-separated team slugs (within the repo's org) whose members may
use /lgtm force. This bypasses the self-approval guard and CODEOWNERS —
only membership in one of these teams grants it.
required: false
default: core-maintainers,lead-maintainers
codeowners-path:
description: Path to the CODEOWNERS file (fetched from the PR base branch)
required: false
@ -79,9 +86,9 @@ inputs:
outputs:
result:
description: >-
One of: lgtm-added, lgtm-removed, hold-added, hold-removed, invalidated,
unauthorized, self-lgtm-blocked, stageblog-dispatched, stageblog-not-blog,
stageblog-disabled, noop
One of: lgtm-added, lgtm-forced, lgtm-removed, hold-added, hold-removed,
invalidated, unauthorized, force-unauthorized, self-lgtm-blocked,
stageblog-dispatched, stageblog-not-blog, stageblog-disabled, noop
value: ${{ steps.handle.outputs.result }}
actor:
description: Login of the user who triggered the action (commenter)
@ -97,6 +104,7 @@ runs:
APPROVED_LABEL: ${{ inputs.approved-label }}
HOLD_LABEL: ${{ inputs.hold-label }}
ALWAYS_ALLOW_TEAMS: ${{ inputs.always-allow-teams }}
FORCE_ALLOW_TEAMS: ${{ inputs.force-allow-teams }}
CODEOWNERS_PATH: ${{ inputs.codeowners-path }}
INVALIDATE_ON_PUSH: ${{ inputs.invalidate-on-push }}
SUBMIT_REVIEW: ${{ inputs.submit-review }}

View file

@ -8,6 +8,8 @@ module.exports = async ({ github, context, core }) => {
const holdLabel = process.env.HOLD_LABEL;
const alwaysAllowTeams = process.env.ALWAYS_ALLOW_TEAMS
.split(',').map(s => s.trim()).filter(Boolean);
const forceAllowTeams = process.env.FORCE_ALLOW_TEAMS
.split(',').map(s => s.trim()).filter(Boolean);
const codeownersPath = process.env.CODEOWNERS_PATH;
const invalidateOnPush = process.env.INVALIDATE_ON_PUSH === 'true';
const submitReview = process.env.SUBMIT_REVIEW === 'true';
@ -185,10 +187,11 @@ module.exports = async ({ github, context, core }) => {
owner, repo, comment_id: commentId, content,
});
}
async function submitApproval(prNumber, onBehalfOf) {
async function submitApproval(prNumber, onBehalfOf, forced) {
const via = forced ? '/lgtm force' : '/lgtm';
await github.rest.pulls.createReview({
owner, repo, pull_number: prNumber, event: 'APPROVE',
body: `${reviewMarker}\nApproved on behalf of @${onBehalfOf} via \`/lgtm\`.`,
body: `${reviewMarker}\nApproved on behalf of @${onBehalfOf} via \`${via}\`.`,
});
}
async function dismissBotApprovals(prNumber, reason) {
@ -257,11 +260,16 @@ module.exports = async ({ github, context, core }) => {
const commentId = context.payload.comment.id;
const actor = context.payload.comment.user.login;
const firstLine = commentBody.split('\n')[0].trim();
const m = firstLine.match(/^\/(lgtm|hold|unhold|stageblog)\b\s*(cancel)?\s*$/i);
const m = firstLine.match(/^\/(lgtm|hold|unhold|stageblog)\b\s*(cancel|force)?\s*$/i);
if (!m) return setResult('noop', actor);
const cmd = m[1].toLowerCase();
const cancel = !!m[2] || cmd === 'unhold';
const arg = (m[2] || '').toLowerCase();
const cancel = arg === 'cancel' || cmd === 'unhold';
// `force` is only meaningful for /lgtm — reject `/hold force` etc. as noop
// rather than silently treating it as the base command.
if (arg === 'force' && cmd !== 'lgtm') return setResult('noop', actor);
const force = arg === 'force';
const prNumber = context.payload.issue.number;
const { data: pr } = await github.rest.pulls.get({ owner, repo, pull_number: prNumber });
@ -271,6 +279,24 @@ module.exports = async ({ github, context, core }) => {
const hadApproved = preLabels.has(approvedLabel);
const hadHold = preLabels.has(holdLabel);
// /lgtm force — restricted to force-allow-teams. Bypasses the self-approval
// guard and the CODEOWNERS path entirely; only team membership grants this.
if (cmd === 'lgtm' && force) {
let allowed = false;
for (const team of forceAllowTeams) {
if (await isTeamMember(actor, team)) { allowed = true; break; }
}
if (!allowed) {
await react(commentId, '-1');
return setResult('force-unauthorized', actor);
}
await addLabel(prNumber, approvedLabel);
if (submitReview) await submitApproval(prNumber, actor, true);
await react(commentId, '+1');
await setCommitStatus(pr.head.sha, true, hadHold);
return setResult('lgtm-forced', actor);
}
// Self-approval guard (only for /lgtm, not /hold)
if (cmd === 'lgtm' && !cancel && actor.toLowerCase() === pr.user.login.toLowerCase()) {
await react(commentId, '-1');