Skip to content

merge-gate does not check for a maintainer approve, so T2/T3 can merge with none #691

Closed as not planned

Description

@Birfy

Summary

The merge gate never looks at approvals. Six pull requests merged on 2026-10-22 with zero reviews of any kind — #675 (5aeec754), #676 (ef340354), #682 (a3cbbcdc), #684 (75ebb1a1), #685 (a8a8f21a), #686 (8580b011) — each with a merge_group run that passed and the queue then merged it. main is b5de55d0. So the rule in AGENTS.md, "T2/T3 need a maintainer approve at the tip plus a PASS", is currently enforced by nobody: only the PASS half is in the gate.

Where it is missing

judge() in .github/scripts/merge-gate.mjs:95 takes { labels, files, head, author, reports, policy, expectedHead, mode } and its reasons can only come from hold, a head mismatch, and decide()'s verdict (PASS / INC). There is no approval input and no branch that would use one. .github/workflows/merge-gate.yml already resolves the pull request from the queue branch and hands the script its labels, files, head, author and comment bodies, so the data source exists; only the field and the rule are missing.

Design (decided)

  1. judge() gains an approvals input, injected by the workflow exactly like labels and reports — judge() stays pure and testable, with no HTTP inside it. Shape: [{ login, state, commit_id, type, permission }] (type is the API's user.type; permission is the account's repository permission).
  2. The rule. When tier is T2 or T3 and the entry is not exempt, at least one approval must satisfy all of: state === 'APPROVED'; commit_id starts with the judged head (an approval at an older tip does not count — the same rule as PASS at the tip); type !== 'Bot'; permission in admin / write / maintain; login different from the pull request author (Birfy cannot approve a PR it authored). Otherwise push `${tier} entry has no approved review` onto reasons — for example T3 entry has no approved review.
  3. Unchanged: T0/T1 and the mechanical-release exemption skip both the PASS and the approval (the existing exempt path — a refusal here would stop every routine fix and every release, Read the verdict from its first line, and test the merge gate through its own entry point #640); mode: 'report' still never fails a check, so only the queue entry refuses.
  4. .github/workflows/merge-gate.yml: fetch GET /pulls/{n}/reviews?per_page=100, keep each review's user.login, state, commit_id and user.type, and resolve permission from GET /repos/{o}/{r}/collaborators/{login}/permission for the candidates only. Dismissed approvals come back as DISMISSED and must not count.

Acceptance

  • T3 file set, no approval → ok: false and reasons contains T3 entry has no approved review.
  • T3 + one APPROVED review at the judged head from a write/admin non-author account → ok: true.
  • T0 file set, no approval → ok: true (exempt path unchanged).
  • Guard cases: APPROVED at an older commit → refused; APPROVED from the pull request author, or from an account whose type is Bot → refused.
  • Red form: remove the approval branch from judge() → the first case goes green and its test is red; nothing else changes colour.
  • node --test .github/scripts/merge-gate.test.mjs green at the tip (fixtures, no network).

State the consequence in the PR body

Today there are zero APPROVED reviews in this repository, and not by accident: every PR is authored by the shared Birfy account and GitHub refuses a self-approval, so the only possible approver is the maintainer's own account. The moment this gate lands, every T2/T3 stops until the maintainer clicks Approve at the tip — which is the intent, but the first red merge-gate run it produces must read as "waiting for the human", not as a regression.

Tier: .github/** → T3.

Activity

  1. added
    bugSomething isn't working
    agent-readyMaintainer-approved spec: an agent may take it. Only maintainers set this; never in a template
    area:automationScheduled tasks and automation
    on Oct 22, 2026
  2. Birfy commented on Oct 22, 2026

    @Birfy
    CollaboratorAuthor

    Written-by: Dev3

    Claimed for #691. Branch: fix/691-approve-gate.

    Building it to the design in this issue: judge() gains an approvals input ({ login, state, commit_id, type, permission }) injected by the workflow like labels and reports; a T2/T3 entry that is not exempt is refused with <tier> entry has no approved review unless one approval is APPROVED, names the judged head, comes from a non-Bot account with admin/write/maintain permission, and is not the pull request's author. .github/workflows/merge-gate.yml fills the input from GET /pulls/{n}/reviews and resolves permission only for the candidates. T0/T1 and the mechanical release PR keep the existing exempt path, and mode: 'report' still never fails a check.

    Acceptance cases from this issue (T3 without an approval refuses, T3 with one passes, T0 without one passes, plus the stale-tip / author / Bot guard cases) go into merge-gate.test.mjs as fixture cases; the red form is removing the approval branch and watching the T3 case go green.

  3. added
    agent-workingAn agent has claimed this issue (the claim comment names the branch)
    and removed
    agent-readyMaintainer-approved spec: an agent may take it. Only maintainers set this; never in a template
    on Oct 22, 2026
  4. Birfy commented on Oct 22, 2026

    @Birfy
    CollaboratorAuthor

    Triage label record (2026-10-22): agent-pr added — PR #693 (fix(merge-gate): refuse a T2/T3 entry that no maintainer approved, tip 6a3ec9a0, 9/9 green, clean) implements this issue.

    Tier is T3 (.github/**), so the queue still needs a maintainer approve at the tip plus a PASS.

  5. Birfy commented on Oct 22, 2026

    @Birfy
    CollaboratorAuthor

    Closing as not planned: the requirement this issue adds is withdrawn by the user's decision at 07:15Z on 2026-10-22.

    The user asked for T2/T3 to merge without a human approve. That makes "T2/T3 needs an APPROVED review at the head" the wrong gate to add, and it would also have hard-stopped the pipeline: every pull request here is authored by the shared Birfy account and GitHub refuses a self-approval, so no approval could ever exist without the maintainer clicking it personally.

    What stands in its place (written down in the follow-up documentation issue): the mechanical gates are a PASS at the tip, the three checks, and merge-gate re-judging the head at enqueue time. hold remains human-only, and an implementer still cannot review its own pull request.

    PR #693 implemented this issue and is closed unmerged for the same reason (the user's decision, recorded there by its author).

  6. removed
    agent-workingAn agent has claimed this issue (the claim comment names the branch)
    agent-prPull request opened by an agent
    on Oct 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:automationScheduled tasks and automationbugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions