diff --git a/slash-commands/README.md b/slash-commands/README.md index 11e981f..7cda899 100644 --- a/slash-commands/README.md +++ b/slash-commands/README.md @@ -208,11 +208,17 @@ enforces. Make the `status-context` value a required status check. **States:** -| Labels | Commit status | Description | +| Condition | Commit status | Description | |---|---|---| +| `synchronize` event + `invalidate-on-push: true` | `pending` (🟡) | New commits — re-approve | | `do-not-merge/hold` present | `failure` (❌) | Blocked by hold | | `approved` present, no hold | `success` (✅) | Approved via /lgtm | -| neither | `pending` (🟡) | Awaiting /lgtm | +| otherwise | `pending` (🟡) | Awaiting /lgtm | + +The `synchronize` special-case is a race guard: the event payload's labels +reflect the *pre-push* state, so without it this action would read a stale +`approved` and set `success` on the new SHA while the `handle` job is still +removing the label in parallel — letting auto-merge slip through. **Inputs:** @@ -222,6 +228,7 @@ enforces. Make the `status-context` value a required status check. | `approved-label` | | `approved` | Must match the main action's `approved-label` | | `hold-label` | | `do-not-merge/hold` | Must match the main action's `hold-label` | | `status-context` | | `slash-commands/approval` | Name shown in the merge box. Mark this as a required status check. | +| `invalidate-on-push` | | `'true'` | On `synchronize`, force pending regardless of labels. **Must match** the main action's `invalidate-on-push`. Disabling this opens an auto-merge race. | **Outputs:** diff --git a/slash-commands/status/action.yml b/slash-commands/status/action.yml index ae63fd2..8c460cb 100644 --- a/slash-commands/status/action.yml +++ b/slash-commands/status/action.yml @@ -27,6 +27,16 @@ inputs: and what you mark as a required status check in branch protection. required: false default: slash-commands/approval + invalidate-on-push: + description: >- + When 'true', a push (synchronize event) forces the status to pending + regardless of labels. This must match the main slash-commands action's + invalidate-on-push setting. Without this, on a push the event payload + still contains the pre-push label state, so this action would set + success on the new SHA (racing with the handle job removing the label) + and auto-merge can slip through. + required: false + default: 'true' outputs: state: @@ -43,6 +53,7 @@ runs: APPROVED_LABEL: ${{ inputs.approved-label }} HOLD_LABEL: ${{ inputs.hold-label }} STATUS_CONTEXT: ${{ inputs.status-context }} + INVALIDATE_ON_PUSH: ${{ inputs.invalidate-on-push }} with: github-token: ${{ inputs.github-token }} script: | @@ -53,21 +64,32 @@ runs: return; } const sha = pr.head.sha; - const labels = pr.labels.map(l => l.name); const approvedLabel = process.env.APPROVED_LABEL; const holdLabel = process.env.HOLD_LABEL; const statusContext = process.env.STATUS_CONTEXT; + const invalidateOnPush = process.env.INVALIDATE_ON_PUSH === 'true'; + // On push, the event payload's labels reflect the PRE-push state. + // If invalidation is on, the handle job is about to (or already did) + // remove the approved label in parallel — but we can't see that here. + // Force pending so auto-merge can't slip through on a stale success. + // The subsequent unlabeled event will re-evaluate (and stay pending). let state, description; - if (labels.includes(holdLabel)) { - state = 'failure'; - description = `Blocked by ${holdLabel} — comment /unhold to clear`; - } else if (labels.includes(approvedLabel)) { - state = 'success'; - description = `Approved via /lgtm`; - } else { + if (invalidateOnPush && context.payload.action === 'synchronize') { state = 'pending'; - description = `Awaiting /lgtm from a maintainer or CODEOWNER`; + description = `New commits pushed — awaiting re-approval via /lgtm`; + } else { + const labels = pr.labels.map(l => l.name); + if (labels.includes(holdLabel)) { + state = 'failure'; + description = `Blocked by ${holdLabel} — comment /unhold to clear`; + } else if (labels.includes(approvedLabel)) { + state = 'success'; + description = `Approved via /lgtm`; + } else { + state = 'pending'; + description = `Awaiting /lgtm from a maintainer or CODEOWNER`; + } } await github.rest.repos.createCommitStatus({