Skip to content

ci: count format-spec votes cast on any commit - #9356

Open
wjones127 wants to merge 2 commits into
lance-format:mainfrom
wjones127:ci/relax-format-vote-latest-commit
Open

wjones127 wants to merge 2 commits into
lance-format:mainfrom
wjones127:ci/relax-format-vote-latest-commit

Conversation

@wjones127

Copy link
Copy Markdown
Contributor

The format-spec vote gate only counted a PMC approval if it was cast on the PR's head commit, so any push — a rebase, a typo fix, a reworded sentence — invalidated all three approvals and sent every voter back to re-approve. For a proposal the PMC has already read, that is more ceremony than it buys.

Approvals now count whatever commit they were cast on, with one added condition: at least one non-author PMC approval must be on the latest commit. That member is confirming that nothing substantive has changed since the earlier approvals, and should ask the other voters for fresh votes rather than approving if it has. The confirming approval counts toward the three; it is not a fourth vote. The gate's PR comment says as much, and now lists who approved the latest commit alongside the overall tally.

Vetoes, the 72-hour voting period, and the format-waived label are unchanged.

Not included

GitHub's own "Dismiss stale pull request approvals when new commits are pushed" branch-protection setting would defeat this, since a dismissed approval stops counting as one. It is currently off for main, so no admin change is needed — worth knowing before anyone turns it on.

🤖 Generated with Claude Code

@github-actions github-actions Bot added A-docs Documentation A-ci CI / build workflows ci Github Action or Test issues labels Sep 17, 2026
@wjones127

Copy link
Copy Markdown
Contributor Author

Decision

Format-spec approvals count whatever commit they were cast on. Additionally, at least one non-author PMC approval must be on the head commit; that member is responsible for confirming nothing substantive changed since the earlier approvals. The head-commit approval counts toward the required three rather than being a fourth vote.

Status

Implemented here: tally_reviews returns (approvals, head_approvals, vetoes), and decide_verdict gained an unconfirmed blocking state between insufficient and waiting_period. Verdict priority is veto > insufficient > unconfirmed > waiting_period.

Options and criteria

Criteria: a rebase or typo fix must not force a re-vote; someone must still be accountable for the delta; the rule has to be expressible from the reviews API alone, under a fork-safe pull_request_target token.

  • Chosen — any-commit approvals + one head-commit approval. Keeps a named person on the hook for the delta without asking three people to re-read it.
  • Verified prerequisite: dismiss_stale_reviews and require_last_push_approval are both false in main's branch protection today, so carried-over approvals keep state: APPROVED in the reviews API and are countable. Turning on either setting would silently defeat this change.

Rejected, with the reason and when it was tried

  • Approvals on any commit, with no head-commit requirement (2026-09-17, considered while drafting) — nothing then vouches for the newest commit, so a substantive change pushed after the third +1 merges unread.
  • Require the head-commit approval to be a fourth, additional vote (same) — makes the common case (rebase, typo) stricter than the status quo for a three-member PMC quorum, which is the opposite of the ask.
  • Rely on GitHub's native require_last_push_approval (same) — it is not PMC-aware and counts any approval from anyone with write access, so it cannot replace the gate's roster logic. It is also an admin-only setting, invisible to the repo.
  • Diff the head commit against the last-approved commit and auto-waive whitespace/typo-only deltas (same) — an automated "substantive" judgment is exactly what the PMC should be making, and the failure mode is a silent false pass.

Open questions

  • format-spec-vote is not currently in main's required status checks (required_status_checks.contexts is empty), so the gate is advisory today regardless of this change. Separate admin action, filed here only so it is not lost.

The format-spec vote gate only counted PMC approvals whose commit id was
the PR head, so every rebase or typo fix sent all three voters back to
re-approve. That is too onerous for a change the PMC has already read.

Approvals now count whatever commit they were cast on, with one added
condition: at least one non-author PMC approval must be on the latest
commit. That member is vouching that nothing substantive has changed
since the earlier approvals — and should ask for fresh votes instead of
approving if it has. The confirming approval counts toward the three; it
is not a fourth vote.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@wjones127
wjones127 force-pushed the ci/relax-format-vote-latest-commit branch from d20e33c to c071b9b Compare September 17, 2026 20:33
@wjones127

Copy link
Copy Markdown
Contributor Author

Status

Ran a falsification pass over the assumptions this change rests on, against the GitHub docs plus ~300 recent PRs in this repo (76 of them force-pushed) and two repos that do have stale dismissal enabled. All four load-bearing claims held:

  • Approvals survive pushes and force-pushes. Zero DISMISSED reviews across the sampled PRs; e.g. ci: require crates.io dependencies to be at least 48h old #9312 head 0ce97488 still carries APPROVED 48d519bd. The only mechanism that removes an approval is the branch-protection setting, which is off here.
  • commit_id is stable and still resolves after the commit is orphaned. The REST schema does declare it nullable with no documented trigger; none were observed, and commit_id == head_sha is null-safe either way.
  • Repeat reviews come back as separate chronological entries, so per-member latest-stance dedupe is correct. The head-commit check reads from that deduped map, not the raw list — a member who approved the head commit and then requested changes does not satisfy the head requirement.
  • CHANGES_REQUESTED is never auto-dismissed, with or without the setting, so the veto path needs no change.

One correction to the "Not included" note in the description: the gate cannot detect the stale-dismissal setting itself. branches/{b}/protection and /rulesets both need Administration: read, which is not a scope GITHUB_TOKEN can request, and the metadata-only rules/branches/{b} endpoint is ruleset-only (it returns [] here despite classic protection being active). Documented in the workflow header instead: if the setting is ever turned on, the gate reverts to requiring all three approvals on the head commit. It fails closed, but silently.

Also worth noting for the record: with dismissal off, a base-branch advance changes the effective diff without changing the head SHA, so a head-commit approval does not by itself prove anyone read the merged result. main has strict: true (branches must be up to date before merging), which forces a new head commit in that case — and therefore a fresh head-commit approval — so the gap closes in practice.

@wjones127
wjones127 marked this pull request as ready for review September 17, 2026 21:13

@lance-gatekeeper lance-gatekeeper Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gate recommendation: approve.

Carrying earlier PMC approvals while requiring one non-author PMC member to approve the current head is a coherent reduction in re-vote ceremony: the latest stance still wins, vetoes remain absolute, and the confirmer is accountable for the delta. Compared with requiring all three voters to reapprove every push, this matches the existing one-member discretion for trivial-change waivers without adding a fourth vote. The implementation preserves the trusted-base and fork authority boundary, fails closed if stale approvals are dismissed, and covers the new tally and verdict transitions.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Sep 17, 2026
Comment thread docs/src/community/voting.md Outdated
Co-authored-by: Weston Pace <weston.pace@gmail.com>
@lance-gatekeeper lance-gatekeeper Bot removed the K-approved Latest Gatekeeper recommendation permits acceptance. label Sep 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ci CI / build workflows A-docs Documentation ci Github Action or Test issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants