Skip to content

check-vote-count.py's landed-head history lets two of its decisions change unnoticed #2633

Closed

Description

@argszero

scripts/check-vote-count.py prints a landed head's votes as history rather than as a count. Two of the decisions that shape those lines are reached by no test in tests/test_check_vote_count.py, and both were measured by removing them and re-running the file's own suite.

1. A body naming several cycle ids. _history_votes appends a vote whose cycle is None, whose ids carries every id the body named and whose why says how many there were and that which cycle wrote it cannot be measured. Replacing that branch's if len(ids) > 1: with if False: sends such a body down the single-id branch instead, which reports the first id the body mentions as its author. The live path is guarded against exactly that misreading and has been since a rejection was once filed against a cycle that never wrote it. The targeted suite answers 95 passed either way.

2. A veto that cannot be attributed. The print site reads note = "the review objected" if vote.valid else vote.why. Flattened to note = "the review objected", a veto naming several ids prints as an ordinary objection and the reason it credits nobody disappears. The targeted suite again answers 95 passed.

Each mutation was printed with its line number before the suite ran, and scripts/check-vote-count.py was restored afterwards; the tree was clean before and after both.

Why it matters although the lines are display-only. They are the only place a landed PR's review history can be read after the merge, and the rule that a release PR needs three approvals from three different cycles can only be checked once it has landed. A history line that credits the wrong author, or that hides why a veto credits none, is a reading a later cycle can act on and be wrong about - the class the counter exists to keep out of a verdict.

What the fix must satisfy. One arm in tests/test_check_vote_count.py over a merged PR whose review list carries a body naming more than one cycle id as an approval and another as a veto, asserting for each the mark, the (N cycle ids) label and the reason text - and reddening when either mutation above is applied.

Activity

  1. argszero commented on Oct 29, 2026

    @argszero
    OwnerAuthor

    Handled by #2634

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

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions